From c7b09b642ac7068f181474bec99c4a7d61179f64 Mon Sep 17 00:00:00 2001 From: Wei Tang <hi@that.world> Date: Mon, 17 Feb 2020 10:18:53 +0100 Subject: [PATCH] Refactor BlockImportParams to be non_exhaustive (#4936) * Refactor BlockImportParams to be non_exhaustive * Fix cargo check compile --- substrate/bin/node/cli/src/service.rs | 31 ++-- substrate/bin/node/testing/benches/import.rs | 19 +-- .../bin/node/transaction-factory/src/lib.rs | 17 +-- substrate/client/consensus/aura/src/lib.rs | 47 ++---- substrate/client/consensus/babe/src/lib.rs | 62 +++----- substrate/client/consensus/babe/src/tests.rs | 33 ++-- .../client/consensus/manual-seal/src/lib.rs | 19 +-- .../manual-seal/src/seal_new_block.rs | 18 +-- substrate/client/consensus/pow/src/lib.rs | 68 +++------ substrate/client/consensus/slots/src/lib.rs | 4 +- .../client/finality-grandpa/src/import.rs | 2 +- .../finality-grandpa/src/light_import.rs | 23 +-- .../client/finality-grandpa/src/tests.rs | 59 ++------ substrate/client/network/test/src/lib.rs | 20 +-- .../consensus/common/src/block_import.rs | 61 ++++---- substrate/test-utils/client/src/client_ext.rs | 142 ++++-------------- 16 files changed, 187 insertions(+), 438 deletions(-) diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs index a88f2d1add5..dd2684d50e7 100644 --- a/substrate/bin/node/cli/src/service.rs +++ b/substrate/bin/node/cli/src/service.rs @@ -369,7 +369,7 @@ pub fn new_light(config: NodeConfiguration) #[cfg(test)] mod tests { - use std::{sync::Arc, collections::HashMap, borrow::Cow, any::Any}; + use std::{sync::Arc, borrow::Cow, any::Any}; use sc_consensus_babe::{ CompatibleDigestItem, BabeIntermediate, INTERMEDIATE_KEY }; @@ -557,27 +557,14 @@ mod tests { ); slot_num += 1; - let params = BlockImportParams { - origin: BlockOrigin::File, - header: new_header, - justification: None, - post_digests: vec![item], - body: Some(new_body), - storage_changes: None, - finalized: false, - auxiliary: Vec::new(), - intermediates: { - let mut intermediates = HashMap::new(); - intermediates.insert( - Cow::from(INTERMEDIATE_KEY), - Box::new(BabeIntermediate { epoch }) as Box<dyn Any>, - ); - intermediates - }, - fork_choice: Some(ForkChoiceStrategy::LongestChain), - allow_missing_state: false, - import_existing: false, - }; + let mut params = BlockImportParams::new(BlockOrigin::File, new_header); + params.post_digests.push(item); + params.body = Some(new_body); + params.intermediates.insert( + Cow::from(INTERMEDIATE_KEY), + Box::new(BabeIntermediate { epoch }) as Box<dyn Any>, + ); + params.fork_choice = Some(ForkChoiceStrategy::LongestChain); block_import.import_block(params, Default::default()) .expect("error importing test block"); diff --git a/substrate/bin/node/testing/benches/import.rs b/substrate/bin/node/testing/benches/import.rs index 86092a78360..8c8439fc92e 100644 --- a/substrate/bin/node/testing/benches/import.rs +++ b/substrate/bin/node/testing/benches/import.rs @@ -358,20 +358,9 @@ fn generate_block_import(client: &Client, keyring: &BenchKeyring) -> Block { // Import generated block. fn import_block(client: &mut Client, block: Block) { - let import_params = BlockImportParams { - origin: BlockOrigin::NetworkBroadcast, - header: block.header().clone(), - post_digests: Default::default(), - body: Some(block.extrinsics().to_vec()), - storage_changes: Default::default(), - finalized: false, - justification: Default::default(), - auxiliary: Default::default(), - intermediates: Default::default(), - fork_choice: Some(ForkChoiceStrategy::LongestChain), - allow_missing_state: false, - import_existing: false, - }; + let mut import_params = BlockImportParams::new(BlockOrigin::NetworkBroadcast, block.header.clone()); + import_params.body = Some(block.extrinsics().to_vec()); + import_params.fork_choice = Some(ForkChoiceStrategy::LongestChain); assert_eq!(client.chain_info().best_number, 0); @@ -500,4 +489,4 @@ fn profile_block_import(c: &mut Criterion) { ); }, ); -} \ No newline at end of file +} diff --git a/substrate/bin/node/transaction-factory/src/lib.rs b/substrate/bin/node/transaction-factory/src/lib.rs index acee44625f2..7dafeed4061 100644 --- a/substrate/bin/node/transaction-factory/src/lib.rs +++ b/substrate/bin/node/transaction-factory/src/lib.rs @@ -156,20 +156,9 @@ where best_hash = block.header().hash(); best_block_id = BlockId::<Block>::hash(best_hash); - let import = BlockImportParams { - origin: BlockOrigin::File, - header: block.header().clone(), - post_digests: Vec::new(), - body: Some(block.extrinsics().to_vec()), - storage_changes: None, - finalized: false, - justification: None, - auxiliary: Vec::new(), - intermediates: Default::default(), - fork_choice: Some(ForkChoiceStrategy::LongestChain), - allow_missing_state: false, - import_existing: false, - }; + let mut import = BlockImportParams::new(BlockOrigin::File, block.header().clone()); + import.body = Some(block.extrinsics().to_vec()); + import.fork_choice = Some(ForkChoiceStrategy::LongestChain); client.clone().import_block(import, HashMap::new()).expect("Failed to import block"); info!("Imported block at {}", factory_state.block_number()); diff --git a/substrate/client/consensus/aura/src/lib.rs b/substrate/client/consensus/aura/src/lib.rs index 434314a8535..db872f28c1b 100644 --- a/substrate/client/consensus/aura/src/lib.rs +++ b/substrate/client/consensus/aura/src/lib.rs @@ -282,20 +282,13 @@ impl<B, C, E, I, P, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for AuraW let signature = pair.sign(header_hash.as_ref()); let signature_digest_item = <DigestItemFor<B> as CompatibleDigestItem<P>>::aura_seal(signature); - BlockImportParams { - origin: BlockOrigin::Own, - header, - justification: None, - post_digests: vec![signature_digest_item], - body: Some(body), - storage_changes: Some(storage_changes), - finalized: false, - auxiliary: Vec::new(), - intermediates: Default::default(), - fork_choice: Some(ForkChoiceStrategy::LongestChain), - allow_missing_state: false, - import_existing: false, - } + let mut import_block = BlockImportParams::new(BlockOrigin::Own, header); + import_block.post_digests.push(signature_digest_item); + import_block.body = Some(body); + import_block.storage_changes = Some(storage_changes); + import_block.fork_choice = Some(ForkChoiceStrategy::LongestChain); + + import_block }) } @@ -637,22 +630,14 @@ impl<B: BlockT, C, P, T> Verifier<B> for AuraVerifier<C, P, T> where _ => None, }); - let block_import_params = BlockImportParams { - origin, - header: pre_header, - post_digests: vec![seal], - body, - storage_changes: None, - finalized: false, - justification, - auxiliary: Vec::new(), - intermediates: Default::default(), - fork_choice: Some(ForkChoiceStrategy::LongestChain), - allow_missing_state: false, - import_existing: false, - }; - - Ok((block_import_params, maybe_keys)) + let mut import_block = BlockImportParams::new(origin, pre_header); + import_block.post_digests.push(seal); + import_block.body = body; + import_block.justification = justification; + import_block.fork_choice = Some(ForkChoiceStrategy::LongestChain); + import_block.post_hash = Some(hash); + + Ok((import_block, maybe_keys)) } CheckedHeader::Deferred(a, b) => { debug!(target: "aura", "Checking {:?} failed; {:?}, {:?}.", hash, a, b); @@ -790,7 +775,7 @@ impl<Block: BlockT, C, I, P> BlockImport<Block> for AuraBlockImport<Block, C, I, block: BlockImportParams<Block, Self::Transaction>, new_cache: HashMap<CacheKeyId, Vec<u8>>, ) -> Result<ImportResult, Self::Error> { - let hash = block.post_header().hash(); + let hash = block.post_hash(); let slot_number = find_pre_digest::<Block, P>(&block.header) .expect("valid Aura headers must contain a predigest; \ header has been already verified; qed"); diff --git a/substrate/client/consensus/babe/src/lib.rs b/substrate/client/consensus/babe/src/lib.rs index f9e3ef98d67..51ea37f6d50 100644 --- a/substrate/client/consensus/babe/src/lib.rs +++ b/substrate/client/consensus/babe/src/lib.rs @@ -487,27 +487,16 @@ impl<B, C, E, I, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for BabeWork let signature = pair.sign(header_hash.as_ref()); let digest_item = <DigestItemFor<B> as CompatibleDigestItem>::babe_seal(signature); - BlockImportParams { - origin: BlockOrigin::Own, - header, - justification: None, - post_digests: vec![digest_item], - body: Some(body), - storage_changes: Some(storage_changes), - finalized: false, - auxiliary: Vec::new(), // block-weight is written in block import. - intermediates: { - let mut intermediates = HashMap::new(); - intermediates.insert( - Cow::from(INTERMEDIATE_KEY), - Box::new(BabeIntermediate { epoch }) as Box<dyn Any>, - ); - intermediates - }, - fork_choice: None, - allow_missing_state: false, - import_existing: false, - } + let mut import_block = BlockImportParams::new(BlockOrigin::Own, header); + import_block.post_digests.push(digest_item); + import_block.body = Some(body); + import_block.storage_changes = Some(storage_changes); + import_block.intermediates.insert( + Cow::from(INTERMEDIATE_KEY), + Box::new(BabeIntermediate { epoch }) as Box<dyn Any>, + ); + + import_block }) } @@ -861,30 +850,17 @@ impl<B, E, Block, RA, PRA> Verifier<Block> for BabeVerifier<B, E, Block, RA, PRA "babe.checked_and_importing"; "pre_header" => ?pre_header); - let mut intermediates = HashMap::new(); - intermediates.insert( + let mut import_block = BlockImportParams::new(origin, pre_header); + import_block.post_digests.push(verified_info.seal); + import_block.body = body; + import_block.justification = justification; + import_block.intermediates.insert( Cow::from(INTERMEDIATE_KEY), - Box::new(BabeIntermediate { - epoch, - }) as Box<dyn Any>, + Box::new(BabeIntermediate { epoch }) as Box<dyn Any>, ); + import_block.post_hash = Some(hash); - let block_import_params = BlockImportParams { - origin, - header: pre_header, - post_digests: vec![verified_info.seal], - body, - storage_changes: None, - finalized: false, - justification, - auxiliary: Vec::new(), - intermediates, - fork_choice: None, - allow_missing_state: false, - import_existing: false, - }; - - Ok((block_import_params, Default::default())) + Ok((import_block, Default::default())) } CheckedHeader::Deferred(a, b) => { debug!(target: "babe", "Checking {:?} failed; {:?}, {:?}.", hash, a, b); @@ -981,7 +957,7 @@ impl<B, E, Block, I, RA, PRA> BlockImport<Block> for BabeBlockImport<B, E, Block mut block: BlockImportParams<Block, Self::Transaction>, new_cache: HashMap<CacheKeyId, Vec<u8>>, ) -> Result<ImportResult, Self::Error> { - let hash = block.post_header().hash(); + let hash = block.post_hash(); let number = block.header.number().clone(); // early exit if block already in chain, otherwise the check for diff --git a/substrate/client/consensus/babe/src/tests.rs b/substrate/client/consensus/babe/src/tests.rs index 687f23e646f..1151b82d2e4 100644 --- a/substrate/client/consensus/babe/src/tests.rs +++ b/substrate/client/consensus/babe/src/tests.rs @@ -593,30 +593,15 @@ fn propose_and_import_block<Transaction>( h }; - let import_result = block_import.import_block( - BlockImportParams { - origin: BlockOrigin::Own, - header: block.header, - justification: None, - post_digests: vec![seal], - body: Some(block.extrinsics), - storage_changes: None, - finalized: false, - auxiliary: Vec::new(), - intermediates: { - let mut intermediates = HashMap::new(); - intermediates.insert( - Cow::from(INTERMEDIATE_KEY), - Box::new(BabeIntermediate { epoch }) as Box<dyn Any>, - ); - intermediates - }, - fork_choice: Some(ForkChoiceStrategy::LongestChain), - allow_missing_state: false, - import_existing: false, - }, - Default::default(), - ).unwrap(); + let mut import = BlockImportParams::new(BlockOrigin::Own, block.header); + import.post_digests.push(seal); + import.body = Some(block.extrinsics); + import.intermediates.insert( + Cow::from(INTERMEDIATE_KEY), + Box::new(BabeIntermediate { epoch }) as Box<dyn Any>, + ); + import.fork_choice = Some(ForkChoiceStrategy::LongestChain); + let import_result = block_import.import_block(import, Default::default()).unwrap(); match import_result { ImportResult::Imported(_) => {}, diff --git a/substrate/client/consensus/manual-seal/src/lib.rs b/substrate/client/consensus/manual-seal/src/lib.rs index c820407e748..a334e7c72f0 100644 --- a/substrate/client/consensus/manual-seal/src/lib.rs +++ b/substrate/client/consensus/manual-seal/src/lib.rs @@ -89,20 +89,11 @@ impl<B: BlockT> Verifier<B> for ManualSealVerifier { justification: Option<Justification>, body: Option<Vec<B::Extrinsic>>, ) -> Result<(BlockImportParams<B, ()>, Option<Vec<(CacheKeyId, Vec<u8>)>>), String> { - let import_params = BlockImportParams { - origin, - header, - justification, - post_digests: Vec::new(), - body, - storage_changes: None, - finalized: true, - auxiliary: Vec::new(), - intermediates: HashMap::new(), - fork_choice: Some(ForkChoiceStrategy::LongestChain), - allow_missing_state: false, - import_existing: false, - }; + let mut import_params = BlockImportParams::new(origin, header); + import_params.justification = justification; + import_params.body = body; + import_params.finalized = true; + import_params.fork_choice = Some(ForkChoiceStrategy::LongestChain); Ok((import_params, None)) } diff --git a/substrate/client/consensus/manual-seal/src/seal_new_block.rs b/substrate/client/consensus/manual-seal/src/seal_new_block.rs index 2b8d867ce3c..39d73e16ab7 100644 --- a/substrate/client/consensus/manual-seal/src/seal_new_block.rs +++ b/substrate/client/consensus/manual-seal/src/seal_new_block.rs @@ -121,20 +121,10 @@ pub async fn seal_new_block<B, SC, CB, E, T, P>( } let (header, body) = proposal.block.deconstruct(); - let params = BlockImportParams { - origin: BlockOrigin::Own, - header: header.clone(), - justification: None, - post_digests: Vec::new(), - body: Some(body), - finalized: finalize, - storage_changes: None, - auxiliary: Vec::new(), - intermediates: HashMap::new(), - fork_choice: Some(ForkChoiceStrategy::LongestChain), - allow_missing_state: false, - import_existing: false, - }; + let mut params = BlockImportParams::new(BlockOrigin::Own, header.clone()); + params.body = Some(body); + params.finalized = finalize; + params.fork_choice = Some(ForkChoiceStrategy::LongestChain); match block_import.import_block(params, HashMap::new())? { ImportResult::Imported(aux) => { diff --git a/substrate/client/consensus/pow/src/lib.rs b/substrate/client/consensus/pow/src/lib.rs index d656f71a15b..49d2e64f605 100644 --- a/substrate/client/consensus/pow/src/lib.rs +++ b/substrate/client/consensus/pow/src/lib.rs @@ -111,9 +111,7 @@ fn aux_key<T: AsRef<[u8]>>(hash: &T) -> Vec<u8> { /// Intermediate value passed to block importer. #[derive(Encode, Decode, Clone, Debug, Default)] -pub struct PowIntermediate<B: BlockT, Difficulty> { - /// The header hash with seal, used for auxiliary key. - pub hash: B::Hash, +pub struct PowIntermediate<Difficulty> { /// Difficulty of the block, if known. pub difficulty: Option<Difficulty>, } @@ -331,7 +329,7 @@ impl<B, I, C, S, Algorithm> BlockImport<B> for PowBlockImport<B, I, C, S, Algori _ => return Err(Error::<B>::HeaderUnsealed(block.header.hash()).into()), }; - let intermediate = block.take_intermediate::<PowIntermediate::<B, Algorithm::Difficulty>>( + let intermediate = block.take_intermediate::<PowIntermediate::<Algorithm::Difficulty>>( INTERMEDIATE_KEY )?; @@ -353,7 +351,7 @@ impl<B, I, C, S, Algorithm> BlockImport<B> for PowBlockImport<B, I, C, S, Algori aux.difficulty = difficulty; aux.total_difficulty.increment(difficulty); - let key = aux_key(&intermediate.hash); + let key = aux_key(&block.post_hash()); block.auxiliary.push((key, Some(aux.encode()))); if block.fork_choice.is_none() { block.fork_choice = Some(ForkChoiceStrategy::Custom( @@ -421,29 +419,19 @@ impl<B: BlockT, Algorithm> Verifier<B> for PowVerifier<B, Algorithm> where let hash = header.hash(); let (checked_header, seal) = self.check_header(header)?; - let intermediate = PowIntermediate::<B, Algorithm::Difficulty> { - hash: hash, + let intermediate = PowIntermediate::<Algorithm::Difficulty> { difficulty: None, }; - let import_block = BlockImportParams { - origin, - header: checked_header, - post_digests: vec![seal], - body, - storage_changes: None, - finalized: false, - justification, - intermediates: { - let mut ret = HashMap::new(); - ret.insert(Cow::from(INTERMEDIATE_KEY), Box::new(intermediate) as Box<dyn Any>); - ret - }, - auxiliary: vec![], - fork_choice: None, - allow_missing_state: false, - import_existing: false, - }; + let mut import_block = BlockImportParams::new(origin, checked_header); + import_block.post_digests.push(seal); + import_block.body = body; + import_block.justification = justification; + import_block.intermediates.insert( + Cow::from(INTERMEDIATE_KEY), + Box::new(intermediate) as Box<dyn Any> + ); + import_block.post_hash = Some(hash); Ok((import_block, None)) } @@ -661,29 +649,19 @@ fn mine_loop<B: BlockT, C, Algorithm, E, SO, S, CAW>( (hash, seal) }; - let intermediate = PowIntermediate::<B, Algorithm::Difficulty> { - hash, + let intermediate = PowIntermediate::<Algorithm::Difficulty> { difficulty: Some(difficulty), }; - let import_block = BlockImportParams { - origin: BlockOrigin::Own, - header, - justification: None, - post_digests: vec![seal], - body: Some(body), - storage_changes: Some(proposal.storage_changes), - intermediates: { - let mut ret = HashMap::new(); - ret.insert(Cow::from(INTERMEDIATE_KEY), Box::new(intermediate) as Box<dyn Any>); - ret - }, - finalized: false, - auxiliary: vec![], - fork_choice: None, - allow_missing_state: false, - import_existing: false, - }; + let mut import_block = BlockImportParams::new(BlockOrigin::Own, header); + import_block.post_digests.push(seal); + import_block.body = Some(body); + import_block.storage_changes = Some(proposal.storage_changes); + import_block.intermediates.insert( + Cow::from(INTERMEDIATE_KEY), + Box::new(intermediate) as Box<dyn Any> + ); + import_block.post_hash = Some(hash); block_import.import_block(import_block, HashMap::default()) .map_err(|e| Error::BlockBuiltError(best_hash, e))?; diff --git a/substrate/client/consensus/slots/src/lib.rs b/substrate/client/consensus/slots/src/lib.rs index 8bc2547a49e..892c8b13643 100644 --- a/substrate/client/consensus/slots/src/lib.rs +++ b/substrate/client/consensus/slots/src/lib.rs @@ -287,13 +287,13 @@ pub trait SimpleSlotWorker<B: BlockT> { info!( "Pre-sealed block for proposal at {}. Hash now {:?}, previously {:?}.", header_num, - block_import_params.post_header().hash(), + block_import_params.post_hash(), header_hash, ); telemetry!(CONSENSUS_INFO; "slots.pre_sealed_block"; "header_num" => ?header_num, - "hash_now" => ?block_import_params.post_header().hash(), + "hash_now" => ?block_import_params.post_hash(), "hash_previously" => ?header_hash, ); diff --git a/substrate/client/finality-grandpa/src/import.rs b/substrate/client/finality-grandpa/src/import.rs index 64fc62bfe7a..2eb1b4a7c88 100644 --- a/substrate/client/finality-grandpa/src/import.rs +++ b/substrate/client/finality-grandpa/src/import.rs @@ -398,7 +398,7 @@ impl<B, E, Block: BlockT, RA, SC> BlockImport<Block> mut block: BlockImportParams<Block, Self::Transaction>, new_cache: HashMap<well_known_cache_keys::Id, Vec<u8>>, ) -> Result<ImportResult, Self::Error> { - let hash = block.post_header().hash(); + let hash = block.post_hash(); let number = block.header.number().clone(); // early exit if block already in chain, otherwise the check for diff --git a/substrate/client/finality-grandpa/src/light_import.rs b/substrate/client/finality-grandpa/src/light_import.rs index 4ae35167be8..0181a93f108 100644 --- a/substrate/client/finality-grandpa/src/light_import.rs +++ b/substrate/client/finality-grandpa/src/light_import.rs @@ -257,7 +257,7 @@ fn do_import_block<B, C, Block: BlockT, J>( DigestFor<Block>: Encode, J: ProvableJustification<Block::Header>, { - let hash = block.post_header().hash(); + let hash = block.post_hash(); let number = block.header.number().clone(); // we don't want to finalize on `inner.import_block` @@ -682,26 +682,19 @@ pub mod tests { authority_set: LightAuthoritySet::genesis(vec![(AuthorityId::from_slice(&[1; 32]), 1)]), consensus_changes: ConsensusChanges::empty(), }; - let block = BlockImportParams { - origin: BlockOrigin::Own, - header: Header { + let mut block = BlockImportParams::new( + BlockOrigin::Own, + Header { number: 1, parent_hash: client.chain_info().best_hash, state_root: Default::default(), digest: Default::default(), extrinsics_root: Default::default(), }, - justification, - post_digests: Vec::new(), - body: None, - storage_changes: None, - finalized: false, - auxiliary: Vec::new(), - intermediates: Default::default(), - fork_choice: Some(ForkChoiceStrategy::LongestChain), - allow_missing_state: true, - import_existing: false, - }; + ); + block.justification = justification; + block.fork_choice = Some(ForkChoiceStrategy::LongestChain); + do_import_block::<_, _, _, TestJustification>( &client, &mut import_data, diff --git a/substrate/client/finality-grandpa/src/tests.rs b/substrate/client/finality-grandpa/src/tests.rs index cf340c69545..71cc72b059f 100644 --- a/substrate/client/finality-grandpa/src/tests.rs +++ b/substrate/client/finality-grandpa/src/tests.rs @@ -1011,20 +1011,11 @@ fn allows_reimporting_change_blocks() { let block = || { let block = block.clone(); - BlockImportParams { - origin: BlockOrigin::File, - header: block.header, - justification: None, - post_digests: Vec::new(), - body: Some(block.extrinsics), - storage_changes: None, - finalized: false, - auxiliary: Vec::new(), - intermediates: Default::default(), - fork_choice: Some(ForkChoiceStrategy::LongestChain), - allow_missing_state: false, - import_existing: false, - } + let mut import = BlockImportParams::new(BlockOrigin::File, block.header); + import.body = Some(block.extrinsics); + import.fork_choice = Some(ForkChoiceStrategy::LongestChain); + + import }; assert_eq!( @@ -1071,20 +1062,12 @@ fn test_bad_justification() { let block = || { let block = block.clone(); - BlockImportParams { - origin: BlockOrigin::File, - header: block.header, - justification: Some(Vec::new()), - post_digests: Vec::new(), - body: Some(block.extrinsics), - storage_changes: None, - finalized: false, - auxiliary: Vec::new(), - intermediates: Default::default(), - fork_choice: Some(ForkChoiceStrategy::LongestChain), - allow_missing_state: false, - import_existing: false, - } + let mut import = BlockImportParams::new(BlockOrigin::File, block.header); + import.justification = Some(Vec::new()); + import.body = Some(block.extrinsics); + import.fork_choice = Some(ForkChoiceStrategy::LongestChain); + + import }; assert_eq!( @@ -1805,23 +1788,13 @@ fn imports_justification_for_regular_blocks_on_import() { }; // we import the block with justification attached - let block = BlockImportParams { - origin: BlockOrigin::File, - header: block.header, - justification: Some(justification.encode()), - post_digests: Vec::new(), - body: Some(block.extrinsics), - storage_changes: None, - finalized: false, - auxiliary: Vec::new(), - intermediates: Default::default(), - fork_choice: Some(ForkChoiceStrategy::LongestChain), - allow_missing_state: false, - import_existing: false, - }; + let mut import = BlockImportParams::new(BlockOrigin::File, block.header); + import.justification = Some(justification.encode()); + import.body = Some(block.extrinsics); + import.fork_choice = Some(ForkChoiceStrategy::LongestChain); assert_eq!( - block_import.import_block(block, HashMap::new()).unwrap(), + block_import.import_block(import, HashMap::new()).unwrap(), ImportResult::Imported(ImportedAux { needs_justification: false, clear_justification_requests: false, diff --git a/substrate/client/network/test/src/lib.rs b/substrate/client/network/test/src/lib.rs index 22e340a2a81..94a64732e9c 100644 --- a/substrate/client/network/test/src/lib.rs +++ b/substrate/client/network/test/src/lib.rs @@ -85,21 +85,13 @@ impl<B: BlockT> Verifier<B> for PassThroughVerifier { .or_else(|| l.try_as_raw(OpaqueDigestItemId::Consensus(b"babe"))) ) .map(|blob| vec![(well_known_cache_keys::AUTHORITIES, blob.to_vec())]); + let mut import = BlockImportParams::new(origin, header); + import.body = body; + import.finalized = self.0; + import.justification = justification; + import.fork_choice = Some(ForkChoiceStrategy::LongestChain); - Ok((BlockImportParams { - origin, - header, - body, - storage_changes: None, - finalized: self.0, - justification, - post_digests: vec![], - auxiliary: Vec::new(), - intermediates: Default::default(), - fork_choice: Some(ForkChoiceStrategy::LongestChain), - allow_missing_state: false, - import_existing: false, - }, maybe_keys)) + Ok((import, maybe_keys)) } } diff --git a/substrate/primitives/consensus/common/src/block_import.rs b/substrate/primitives/consensus/common/src/block_import.rs index dabe6331e81..76655acbd67 100644 --- a/substrate/primitives/consensus/common/src/block_import.rs +++ b/substrate/primitives/consensus/common/src/block_import.rs @@ -113,6 +113,7 @@ pub struct BlockCheckParams<Block: BlockT> { } /// Data required to import a Block. +#[non_exhaustive] pub struct BlockImportParams<Block: BlockT, Transaction> { /// Origin of the Block pub origin: BlockOrigin, @@ -162,46 +163,47 @@ pub struct BlockImportParams<Block: BlockT, Transaction> { pub allow_missing_state: bool, /// Re-validate existing block. pub import_existing: bool, + /// Cached full header hash (with post-digests applied). + pub post_hash: Option<Block::Hash>, } impl<Block: BlockT, Transaction> BlockImportParams<Block, Transaction> { - /// Deconstruct the justified header into parts. - pub fn into_inner(self) - -> ( - BlockOrigin, - <Block as BlockT>::Header, - Option<Justification>, - Vec<DigestItemFor<Block>>, - Option<Vec<Block::Extrinsic>>, - Option<sp_state_machine::StorageChanges<Transaction, HasherFor<Block>, NumberFor<Block>>>, - bool, - Vec<(Vec<u8>, Option<Vec<u8>>)>, - ) { - ( - self.origin, - self.header, - self.justification, - self.post_digests, - self.body, - self.storage_changes, - self.finalized, - self.auxiliary, - ) + /// Create a new block import params. + pub fn new( + origin: BlockOrigin, + header: Block::Header, + ) -> Self { + Self { + origin, header, + justification: None, + post_digests: Vec::new(), + body: None, + storage_changes: None, + finalized: false, + intermediates: HashMap::new(), + auxiliary: Vec::new(), + fork_choice: None, + allow_missing_state: false, + import_existing: false, + post_hash: None, + } } - /// Get a handle to full header (with post-digests applied). - pub fn post_header(&self) -> Cow<Block::Header> { - if self.post_digests.is_empty() { - Cow::Borrowed(&self.header) + /// Get the full header hash (with post-digests applied). + pub fn post_hash(&self) -> Block::Hash { + if let Some(hash) = self.post_hash { + hash } else { - Cow::Owned({ + if self.post_digests.is_empty() { + self.header.hash() + } else { let mut hdr = self.header.clone(); for digest_item in &self.post_digests { hdr.digest_mut().push(digest_item.clone()); } - hdr - }) + hdr.hash() + } } } @@ -223,6 +225,7 @@ impl<Block: BlockT, Transaction> BlockImportParams<Block, Transaction> { allow_missing_state: self.allow_missing_state, fork_choice: self.fork_choice, import_existing: self.import_existing, + post_hash: self.post_hash, } } diff --git a/substrate/test-utils/client/src/client_ext.rs b/substrate/test-utils/client/src/client_ext.rs index e29ee787d03..6d6b539483e 100644 --- a/substrate/test-utils/client/src/client_ext.rs +++ b/substrate/test-utils/client/src/client_ext.rs @@ -87,60 +87,28 @@ impl<Block: BlockT, T, Transaction> ClientBlockImportExt<Block> for std::sync::A { fn import(&mut self, origin: BlockOrigin, block: Block) -> Result<(), ConsensusError> { let (header, extrinsics) = block.deconstruct(); - let import = BlockImportParams { - origin, - header, - justification: None, - post_digests: vec![], - body: Some(extrinsics), - storage_changes: None, - finalized: false, - auxiliary: Vec::new(), - intermediates: Default::default(), - fork_choice: Some(ForkChoiceStrategy::LongestChain), - allow_missing_state: false, - import_existing: false, - }; + let mut import = BlockImportParams::new(origin, header); + import.body = Some(extrinsics); + import.fork_choice = Some(ForkChoiceStrategy::LongestChain); BlockImport::import_block(self, import, HashMap::new()).map(|_| ()) } fn import_as_best(&mut self, origin: BlockOrigin, block: Block) -> Result<(), ConsensusError> { let (header, extrinsics) = block.deconstruct(); - let import = BlockImportParams { - origin, - header, - justification: None, - post_digests: vec![], - body: Some(extrinsics), - storage_changes: None, - finalized: false, - auxiliary: Vec::new(), - intermediates: Default::default(), - fork_choice: Some(ForkChoiceStrategy::Custom(true)), - allow_missing_state: false, - import_existing: false, - }; + let mut import = BlockImportParams::new(origin, header); + import.body = Some(extrinsics); + import.fork_choice = Some(ForkChoiceStrategy::Custom(true)); BlockImport::import_block(self, import, HashMap::new()).map(|_| ()) } fn import_as_final(&mut self, origin: BlockOrigin, block: Block) -> Result<(), ConsensusError> { let (header, extrinsics) = block.deconstruct(); - let import = BlockImportParams { - origin, - header, - justification: None, - post_digests: vec![], - body: Some(extrinsics), - storage_changes: None, - finalized: true, - auxiliary: Vec::new(), - intermediates: Default::default(), - fork_choice: Some(ForkChoiceStrategy::Custom(true)), - allow_missing_state: false, - import_existing: false, - }; + let mut import = BlockImportParams::new(origin, header); + import.body = Some(extrinsics); + import.finalized = true; + import.fork_choice = Some(ForkChoiceStrategy::Custom(true)); BlockImport::import_block(self, import, HashMap::new()).map(|_| ()) } @@ -152,20 +120,11 @@ impl<Block: BlockT, T, Transaction> ClientBlockImportExt<Block> for std::sync::A justification: Justification, ) -> Result<(), ConsensusError> { let (header, extrinsics) = block.deconstruct(); - let import = BlockImportParams { - origin, - header, - justification: Some(justification), - post_digests: vec![], - body: Some(extrinsics), - storage_changes: None, - finalized: true, - auxiliary: Vec::new(), - intermediates: Default::default(), - fork_choice: Some(ForkChoiceStrategy::LongestChain), - allow_missing_state: false, - import_existing: false, - }; + let mut import = BlockImportParams::new(origin, header); + import.justification = Some(justification); + import.body = Some(extrinsics); + import.finalized = true; + import.fork_choice = Some(ForkChoiceStrategy::LongestChain); BlockImport::import_block(self, import, HashMap::new()).map(|_| ()) } @@ -177,60 +136,28 @@ impl<B, E, RA, Block: BlockT> ClientBlockImportExt<Block> for Client<B, E, Block { fn import(&mut self, origin: BlockOrigin, block: Block) -> Result<(), ConsensusError> { let (header, extrinsics) = block.deconstruct(); - let import = BlockImportParams { - origin, - header, - justification: None, - post_digests: vec![], - body: Some(extrinsics), - storage_changes: None, - finalized: false, - auxiliary: Vec::new(), - intermediates: Default::default(), - fork_choice: Some(ForkChoiceStrategy::LongestChain), - allow_missing_state: false, - import_existing: false, - }; + let mut import = BlockImportParams::new(origin, header); + import.body = Some(extrinsics); + import.fork_choice = Some(ForkChoiceStrategy::LongestChain); BlockImport::import_block(self, import, HashMap::new()).map(|_| ()) } fn import_as_best(&mut self, origin: BlockOrigin, block: Block) -> Result<(), ConsensusError> { let (header, extrinsics) = block.deconstruct(); - let import = BlockImportParams { - origin, - header, - justification: None, - post_digests: vec![], - body: Some(extrinsics), - storage_changes: None, - finalized: false, - auxiliary: Vec::new(), - intermediates: Default::default(), - fork_choice: Some(ForkChoiceStrategy::Custom(true)), - allow_missing_state: false, - import_existing: false, - }; + let mut import = BlockImportParams::new(origin, header); + import.body = Some(extrinsics); + import.fork_choice = Some(ForkChoiceStrategy::Custom(true)); BlockImport::import_block(self, import, HashMap::new()).map(|_| ()) } fn import_as_final(&mut self, origin: BlockOrigin, block: Block) -> Result<(), ConsensusError> { let (header, extrinsics) = block.deconstruct(); - let import = BlockImportParams { - origin, - header, - justification: None, - post_digests: vec![], - body: Some(extrinsics), - storage_changes: None, - finalized: true, - auxiliary: Vec::new(), - intermediates: Default::default(), - fork_choice: Some(ForkChoiceStrategy::Custom(true)), - allow_missing_state: false, - import_existing: false, - }; + let mut import = BlockImportParams::new(origin, header); + import.body = Some(extrinsics); + import.finalized = true; + import.fork_choice = Some(ForkChoiceStrategy::Custom(true)); BlockImport::import_block(self, import, HashMap::new()).map(|_| ()) } @@ -242,20 +169,11 @@ impl<B, E, RA, Block: BlockT> ClientBlockImportExt<Block> for Client<B, E, Block justification: Justification, ) -> Result<(), ConsensusError> { let (header, extrinsics) = block.deconstruct(); - let import = BlockImportParams { - origin, - header, - justification: Some(justification), - post_digests: vec![], - body: Some(extrinsics), - storage_changes: None, - finalized: true, - auxiliary: Vec::new(), - intermediates: Default::default(), - fork_choice: Some(ForkChoiceStrategy::LongestChain), - allow_missing_state: false, - import_existing: false, - }; + let mut import = BlockImportParams::new(origin, header); + import.justification = Some(justification); + import.body = Some(extrinsics); + import.finalized = true; + import.fork_choice = Some(ForkChoiceStrategy::LongestChain); BlockImport::import_block(self, import, HashMap::new()).map(|_| ()) } -- GitLab