Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

client/beefy: drop justification on block import if pallet-beefy not enabled #13422

Merged
merged 1 commit into from
Feb 20, 2023
Merged

client/beefy: drop justification on block import if pallet-beefy not enabled #13422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 34 additions & 22 deletions client/beefy/src/import.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,23 @@ where
number: NumberFor<Block>,
hash: <Block as BlockT>::Hash,
) -> Result<BeefyVersionedFinalityProof<Block>, ConsensusError> {
use ConsensusError::ClientImport as ImportError;
let block_id = BlockId::hash(hash);
let beefy_genesis = self
.runtime
.runtime_api()
.beefy_genesis(&block_id)
.map_err(|e| ImportError(e.to_string()))?
.ok_or_else(|| ImportError("Unknown BEEFY genesis".to_string()))?;
if number < beefy_genesis {
return Err(ImportError("BEEFY genesis is set for future block".to_string()))
}
let validator_set = self
.runtime
.runtime_api()
.validator_set(&block_id)
.map_err(|e| ConsensusError::ClientImport(e.to_string()))?
.ok_or_else(|| ConsensusError::ClientImport("Unknown validator set".to_string()))?;
.map_err(|e| ImportError(e.to_string()))?
.ok_or_else(|| ImportError("Unknown validator set".to_string()))?;

decode_and_verify_finality_proof::<Block>(&encoded[..], number, &validator_set)
}
Expand Down Expand Up @@ -142,26 +152,28 @@ where

match (beefy_encoded, &inner_import_result) {
(Some(encoded), ImportResult::Imported(_)) => {
if let Ok(proof) = self.decode_and_verify(&encoded, number, hash) {
// The proof is valid and the block is imported and final, we can import.
debug!(
target: LOG_TARGET,
"🥩 import justif {:?} for block number {:?}.", proof, number
);
// Send the justification to the BEEFY voter for processing.
self.justification_sender
.notify(|| Ok::<_, ()>(proof))
.expect("forwards closure result; the closure always returns Ok; qed.");

metric_inc!(self, beefy_good_justification_imports);
} else {
debug!(
target: LOG_TARGET,
"🥩 error decoding justification: {:?} for imported block {:?}",
encoded,
number,
);
metric_inc!(self, beefy_bad_justification_imports);
match self.decode_and_verify(&encoded, number, hash) {
Ok(proof) => {
// The proof is valid and the block is imported and final, we can import.
debug!(
target: LOG_TARGET,
"🥩 import justif {:?} for block number {:?}.", proof, number
);
// Send the justification to the BEEFY voter for processing.
self.justification_sender
.notify(|| Ok::<_, ()>(proof))
.expect("the closure always returns Ok; qed.");
metric_inc!(self, beefy_good_justification_imports);
},
Err(err) => {
debug!(
target: LOG_TARGET,
"🥩 error importing BEEFY justification for block {:?}: {:?}",
number,
err,
);
metric_inc!(self, beefy_bad_justification_imports);
},
}
},
_ => (),
Expand Down
98 changes: 55 additions & 43 deletions client/beefy/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ use sp_runtime::{
codec::Encode,
generic::BlockId,
traits::{Header as HeaderT, NumberFor},
BuildStorage, DigestItem, Justifications, Storage,
BuildStorage, DigestItem, EncodedJustification, Justifications, Storage,
};
use std::{collections::HashMap, marker::PhantomData, sync::Arc, task::Poll};
use substrate_test_runtime_client::{runtime::Header, ClientExt};
Expand Down Expand Up @@ -107,11 +107,13 @@ pub(crate) struct PeerData {
#[derive(Default)]
pub(crate) struct BeefyTestNet {
peers: Vec<BeefyPeer>,
pub beefy_genesis: NumberFor<Block>,
}

impl BeefyTestNet {
pub(crate) fn new(n_authority: usize) -> Self {
let mut net = BeefyTestNet { peers: Vec::with_capacity(n_authority) };
let beefy_genesis = 1;
let mut net = BeefyTestNet { peers: Vec::with_capacity(n_authority), beefy_genesis };

for i in 0..n_authority {
let (rx, cfg) = on_demand_justifications_protocol_config(GENESIS_HASH, None);
Expand Down Expand Up @@ -202,7 +204,7 @@ impl TestNetFactory for BeefyTestNet {
) {
let keys = &[BeefyKeyring::Alice, BeefyKeyring::Bob];
let validator_set = ValidatorSet::new(make_beefy_ids(keys), 0).unwrap();
let api = Arc::new(TestApi::with_validator_set(&validator_set));
let api = Arc::new(TestApi::new(self.beefy_genesis, &validator_set, GOOD_MMR_ROOT));
let inner = BlockImportAdapter::new(client.clone());
let (block_import, voter_links, rpc_links) =
beefy_block_import_and_links(inner, client.as_backend(), api, None);
Expand Down Expand Up @@ -716,7 +718,7 @@ async fn correct_beefy_payload() {
}

#[tokio::test]
async fn beefy_importing_blocks() {
async fn beefy_importing_justifications() {
use futures::{future::poll_fn, task::Poll};
use sc_block_builder::BlockBuilderProvider;
use sc_client_api::BlockBackend;
Expand All @@ -726,11 +728,15 @@ async fn beefy_importing_blocks() {
let mut net = BeefyTestNet::new(2);
let keys = &[BeefyKeyring::Alice, BeefyKeyring::Bob];
let good_set = ValidatorSet::new(make_beefy_ids(keys), 0).unwrap();
// Set BEEFY genesis to block 3.
net.beefy_genesis = 3;

let client = net.peer(0).client().clone();
let full_client = client.as_client();
let (mut block_import, _, peer_data) = net.make_block_import(client.clone());
let PeerData { beefy_voter_links, .. } = peer_data;
let justif_stream = beefy_voter_links.lock().take().unwrap().from_block_import_justif_stream;
let mut justif_recv = justif_stream.subscribe(100_000);

let params = |block: Block, justifications: Option<Justifications>| {
let mut import = BlockImportParams::new(BlockOrigin::File, block.header);
Expand All @@ -740,15 +746,19 @@ async fn beefy_importing_blocks() {
import.fork_choice = Some(ForkChoiceStrategy::LongestChain);
import
};
let backend_justif_for = |block_hash: H256| -> Option<EncodedJustification> {
full_client
.justifications(block_hash)
.unwrap()
.and_then(|j| j.get(BEEFY_ENGINE_ID).cloned())
};

let full_client = client.as_client();
let parent_id = BlockId::Number(0);
let builder = full_client.new_block_at(&parent_id, Default::default(), false).unwrap();
let block = builder.build().unwrap().block;
let hashof1 = block.header.hash();

// Import without justifications.
let mut justif_recv = justif_stream.subscribe(100_000);
// Import block 1 without justifications.
assert_eq!(
block_import
.import_block(params(block.clone(), None), HashMap::new())
Expand All @@ -760,16 +770,32 @@ async fn beefy_importing_blocks() {
block_import.import_block(params(block, None), HashMap::new()).await.unwrap(),
ImportResult::AlreadyInChain,
);
// Verify no BEEFY justifications present:

// Import block 2 with "valid" justification (beefy pallet genesis block not yet reached).
let parent_id = BlockId::Number(1);
let block_num = 2;
let builder = full_client.new_block_at(&parent_id, Default::default(), false).unwrap();
let block = builder.build().unwrap().block;
let hashof2 = block.header.hash();

let proof = crate::justification::tests::new_finality_proof(block_num, &good_set, keys);
let versioned_proof: VersionedFinalityProof<NumberFor<Block>, Signature> = proof.into();
let encoded = versioned_proof.encode();
let justif = Some(Justifications::from((BEEFY_ENGINE_ID, encoded)));
assert_eq!(
block_import.import_block(params(block, justif), HashMap::new()).await.unwrap(),
ImportResult::Imported(ImportedAux {
bad_justification: false,
is_new_best: true,
..Default::default()
}),
);

// Verify no BEEFY justifications present (for either block 1 or 2):
{
// none in backend,
assert_eq!(
full_client
.justifications(hashof1)
.unwrap()
.and_then(|j| j.get(BEEFY_ENGINE_ID).cloned()),
None
);
assert_eq!(backend_justif_for(hashof1), None);
assert_eq!(backend_justif_for(hashof2), None);
// and none sent to BEEFY worker.
poll_fn(move |cx| {
assert_eq!(justif_recv.poll_next_unpin(cx), Poll::Pending);
Expand All @@ -778,17 +804,16 @@ async fn beefy_importing_blocks() {
.await;
}

// Import with valid justification.
let parent_id = BlockId::Number(1);
let block_num = 2;
// Import block 3 with valid justification.
let parent_id = BlockId::Number(2);
let block_num = 3;
let builder = full_client.new_block_at(&parent_id, Default::default(), false).unwrap();
let block = builder.build().unwrap().block;
let hashof3 = block.header.hash();
let proof = crate::justification::tests::new_finality_proof(block_num, &good_set, keys);
let versioned_proof: VersionedFinalityProof<NumberFor<Block>, Signature> = proof.into();
let encoded = versioned_proof.encode();
let justif = Some(Justifications::from((BEEFY_ENGINE_ID, encoded)));

let builder = full_client.new_block_at(&parent_id, Default::default(), false).unwrap();
let block = builder.build().unwrap().block;
let hashof2 = block.header.hash();
let mut justif_recv = justif_stream.subscribe(100_000);
assert_eq!(
block_import.import_block(params(block, justif), HashMap::new()).await.unwrap(),
Expand All @@ -801,13 +826,7 @@ async fn beefy_importing_blocks() {
// Verify BEEFY justification successfully imported:
{
// still not in backend (worker is responsible for appending to backend),
assert_eq!(
full_client
.justifications(hashof2)
.unwrap()
.and_then(|j| j.get(BEEFY_ENGINE_ID).cloned()),
None
);
assert_eq!(backend_justif_for(hashof3), None);
// but sent to BEEFY worker
// (worker will append it to backend when all previous mandatory justifs are there as well).
poll_fn(move |cx| {
Expand All @@ -820,19 +839,18 @@ async fn beefy_importing_blocks() {
.await;
}

// Import with invalid justification (incorrect validator set).
let parent_id = BlockId::Number(2);
let block_num = 3;
// Import block 4 with invalid justification (incorrect validator set).
let parent_id = BlockId::Number(3);
let block_num = 4;
let builder = full_client.new_block_at(&parent_id, Default::default(), false).unwrap();
let block = builder.build().unwrap().block;
let hashof4 = block.header.hash();
let keys = &[BeefyKeyring::Alice];
let bad_set = ValidatorSet::new(make_beefy_ids(keys), 1).unwrap();
let proof = crate::justification::tests::new_finality_proof(block_num, &bad_set, keys);
let versioned_proof: VersionedFinalityProof<NumberFor<Block>, Signature> = proof.into();
let encoded = versioned_proof.encode();
let justif = Some(Justifications::from((BEEFY_ENGINE_ID, encoded)));

let builder = full_client.new_block_at(&parent_id, Default::default(), false).unwrap();
let block = builder.build().unwrap().block;
let hashof3 = block.header.hash();
let mut justif_recv = justif_stream.subscribe(100_000);
assert_eq!(
block_import.import_block(params(block, justif), HashMap::new()).await.unwrap(),
Expand All @@ -846,13 +864,7 @@ async fn beefy_importing_blocks() {
// Verify bad BEEFY justifications was not imported:
{
// none in backend,
assert_eq!(
full_client
.justifications(hashof3)
.unwrap()
.and_then(|j| j.get(BEEFY_ENGINE_ID).cloned()),
None
);
assert_eq!(backend_justif_for(hashof4), None);
// and none sent to BEEFY worker.
poll_fn(move |cx| {
assert_eq!(justif_recv.poll_next_unpin(cx), Poll::Pending);
Expand Down