diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs index b12dfcdc515f5e188093d3843305cb28c46c8a52..603df5b7d9693468bbb3fcd7af23939f59b96694 100644 --- a/substrate/bin/node/cli/src/service.rs +++ b/substrate/bin/node/cli/src/service.rs @@ -565,7 +565,8 @@ mod tests { storage_changes: None, finalized: false, auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::LongestChain, + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::LongestChain), allow_missing_state: false, import_existing: false, }; diff --git a/substrate/bin/node/transaction-factory/src/lib.rs b/substrate/bin/node/transaction-factory/src/lib.rs index f3dcdb087500274289469d77dbd7af5817bfbdf3..a0c6a4f663c2adede2a4e036c71d11dd1a6c0399 100644 --- a/substrate/bin/node/transaction-factory/src/lib.rs +++ b/substrate/bin/node/transaction-factory/src/lib.rs @@ -202,7 +202,8 @@ fn import_block<Backend, Exec, Block, RtApi>( finalized: false, justification: None, auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::LongestChain, + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::LongestChain), allow_missing_state: false, import_existing: false, }; diff --git a/substrate/client/consensus/aura/src/lib.rs b/substrate/client/consensus/aura/src/lib.rs index 95dc08afaf5ef3d4e2b9947bb839e7c02bdda489..270bd1942220103b1b16ab3106270683a43b489e 100644 --- a/substrate/client/consensus/aura/src/lib.rs +++ b/substrate/client/consensus/aura/src/lib.rs @@ -290,7 +290,8 @@ impl<B, C, E, I, P, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for AuraW storage_changes: Some(storage_changes), finalized: false, auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::LongestChain, + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::LongestChain), allow_missing_state: false, import_existing: false, } @@ -644,7 +645,8 @@ impl<B: BlockT, C, P, T> Verifier<B> for AuraVerifier<C, P, T> where finalized: false, justification, auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::LongestChain, + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::LongestChain), allow_missing_state: false, import_existing: false, }; diff --git a/substrate/client/consensus/babe/src/lib.rs b/substrate/client/consensus/babe/src/lib.rs index 770f8a4eec6e2dd0d39681632fdad33400eb3437..848097231109f32338f99d036e2abc0cc6445675 100644 --- a/substrate/client/consensus/babe/src/lib.rs +++ b/substrate/client/consensus/babe/src/lib.rs @@ -451,10 +451,8 @@ impl<B, C, E, I, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for BabeWork storage_changes: Some(storage_changes), finalized: false, auxiliary: Vec::new(), // block-weight is written in block import. - // TODO: block-import handles fork choice and this shouldn't even have the - // option to specify one. - // https://github.com/paritytech/substrate/issues/3623 - fork_choice: ForkChoiceStrategy::LongestChain, + intermediates: Default::default(), + fork_choice: None, allow_missing_state: false, import_existing: false, } @@ -807,10 +805,8 @@ impl<B, E, Block, RA, PRA> Verifier<Block> for BabeVerifier<B, E, Block, RA, PRA finalized: false, justification, auxiliary: Vec::new(), - // TODO: block-import handles fork choice and this shouldn't even have the - // option to specify one. - // https://github.com/paritytech/substrate/issues/3623 - fork_choice: ForkChoiceStrategy::LongestChain, + intermediates: Default::default(), + fork_choice: None, allow_missing_state: false, import_existing: false, }; @@ -1085,13 +1081,13 @@ impl<B, E, Block, I, RA, PRA> BlockImport<Block> for BabeBlockImport<B, E, Block )? }; - ForkChoiceStrategy::Custom(if total_weight > last_best_weight { + Some(ForkChoiceStrategy::Custom(if total_weight > last_best_weight { true } else if total_weight == last_best_weight { number > last_best_number } else { false - }) + })) }; let import_result = self.inner.import_block(block, new_cache); diff --git a/substrate/client/consensus/babe/src/tests.rs b/substrate/client/consensus/babe/src/tests.rs index 880e028000b42bc75e5aaf45868340d3735bd0be..3339c06d650dcdd33bc302b286174c8ae0568b8b 100644 --- a/substrate/client/consensus/babe/src/tests.rs +++ b/substrate/client/consensus/babe/src/tests.rs @@ -594,7 +594,8 @@ fn propose_and_import_block<Transaction>( storage_changes: None, finalized: false, auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::LongestChain, + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::LongestChain), allow_missing_state: false, import_existing: false, }, diff --git a/substrate/client/consensus/pow/src/lib.rs b/substrate/client/consensus/pow/src/lib.rs index 2ea0cbdf0f7ae17062c1040b2bf74ab2159801d1..ca7285b7fe55963fdf8602994bf1d6721344449a 100644 --- a/substrate/client/consensus/pow/src/lib.rs +++ b/substrate/client/consensus/pow/src/lib.rs @@ -32,6 +32,7 @@ use std::sync::Arc; use std::thread; use std::collections::HashMap; +use std::marker::PhantomData; use sc_client_api::{BlockOf, backend::AuxStore}; use sp_blockchain::{HeaderBackend, ProvideCache, well_known_cache_keys::Id as CacheKeyId}; use sp_block_builder::BlockBuilder as BlockBuilderApi; @@ -43,7 +44,8 @@ use sp_consensus_pow::{Seal, TotalDifficulty, POW_ENGINE_ID}; use sp_inherents::{InherentDataProviders, InherentData}; use sp_consensus::{ BlockImportParams, BlockOrigin, ForkChoiceStrategy, SyncOracle, Environment, Proposer, - SelectChain, Error as ConsensusError, CanAuthorWith, RecordProof, + SelectChain, Error as ConsensusError, CanAuthorWith, RecordProof, BlockImport, + BlockCheckParams, ImportResult, }; use sp_consensus::import_queue::{BoxBlockImport, BasicQueue, Verifier}; use codec::{Encode, Decode}; @@ -59,6 +61,10 @@ pub enum Error<B: BlockT> { HeaderUnsealed(B::Hash), #[display(fmt = "PoW validation error: invalid seal")] InvalidSeal, + #[display(fmt = "PoW validation error: invalid difficulty")] + InvalidDifficulty, + #[display(fmt = "PoW block import expects an intermediate, but not found one")] + NoIntermediate, #[display(fmt = "Rejecting block too far in future")] TooFarInFuture, #[display(fmt = "Fetching best header failed using select chain: {:?}", _0)] @@ -89,6 +95,12 @@ impl<B: BlockT> std::convert::From<Error<B>> for String { } } +impl<B: BlockT> std::convert::From<Error<B>> for ConsensusError { + fn from(error: Error<B>) -> ConsensusError { + ConsensusError::ClientImport(error.to_string()) + } +} + /// Auxiliary storage prefix for PoW engine. pub const POW_AUX_PREFIX: [u8; 4] = *b"PoW:"; @@ -97,6 +109,18 @@ fn aux_key<T: AsRef<[u8]>>(hash: &T) -> Vec<u8> { POW_AUX_PREFIX.iter().chain(hash.as_ref()).copied().collect() } +/// 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, + /// Difficulty of the block, if known. + pub difficulty: Option<Difficulty>, +} + +/// Intermediate key for PoW engine. +pub static INTERMEDIATE_KEY: &[u8] = b"pow1"; + /// Auxiliary storage data for PoW. #[derive(Encode, Decode, Clone, Debug, Default)] pub struct PowAux<Difficulty> { @@ -126,14 +150,22 @@ pub trait PowAlgorithm<B: BlockT> { type Difficulty: TotalDifficulty + Default + Encode + Decode + Ord + Clone + Copy; /// Get the next block's difficulty. + /// + /// This function will be called twice during the import process, so the implementation + /// should be properly cached. fn difficulty(&self, parent: &BlockId<B>) -> Result<Self::Difficulty, Error<B>>; - /// Verify proof of work against the given difficulty. - fn verify( + /// Verify that the seal is valid against given pre hash. + fn verify_seal( &self, - parent: &BlockId<B>, pre_hash: &B::Hash, seal: &Seal, + ) -> Result<bool, Error<B>>; + /// Verify that the difficulty is valid against given seal. + fn verify_difficulty( + &self, difficulty: Self::Difficulty, + parent: &BlockId<B>, + seal: &Seal, ) -> Result<bool, Error<B>>; /// Mine a seal that satisfies the given difficulty. fn mine( @@ -145,59 +177,35 @@ pub trait PowAlgorithm<B: BlockT> { ) -> Result<Option<Seal>, Error<B>>; } -/// A verifier for PoW blocks. -pub struct PowVerifier<B: BlockT, C, S, Algorithm> { - client: Arc<C>, +/// A block importer for PoW. +pub struct PowBlockImport<B: BlockT, I, C, S, Algorithm> { algorithm: Algorithm, - inherent_data_providers: sp_inherents::InherentDataProviders, + inner: I, select_chain: Option<S>, + client: Arc<C>, + inherent_data_providers: sp_inherents::InherentDataProviders, check_inherents_after: <<B as BlockT>::Header as HeaderT>::Number, } -impl<B: BlockT, C, S, Algorithm> PowVerifier<B, C, S, Algorithm> { +impl<B, I, C, S, Algorithm> PowBlockImport<B, I, C, S, Algorithm> where + B: BlockT, + I: BlockImport<B, Transaction = sp_api::TransactionFor<C, B>> + Send + Sync, + I::Error: Into<ConsensusError>, + C: ProvideRuntimeApi<B> + Send + Sync + HeaderBackend<B> + AuxStore + ProvideCache<B> + BlockOf, + C::Api: BlockBuilderApi<B, Error = sp_blockchain::Error>, + Algorithm: PowAlgorithm<B>, +{ + /// Create a new block import suitable to be used in PoW pub fn new( + inner: I, client: Arc<C>, algorithm: Algorithm, check_inherents_after: <<B as BlockT>::Header as HeaderT>::Number, select_chain: Option<S>, inherent_data_providers: sp_inherents::InherentDataProviders, ) -> Self { - Self { client, algorithm, inherent_data_providers, select_chain, check_inherents_after } - } - - fn check_header( - &self, - mut header: B::Header, - parent_block_id: BlockId<B>, - ) -> Result<(B::Header, Algorithm::Difficulty, DigestItem<B::Hash>), Error<B>> where - Algorithm: PowAlgorithm<B>, - { - let hash = header.hash(); - - let (seal, inner_seal) = match header.digest_mut().pop() { - Some(DigestItem::Seal(id, seal)) => { - if id == POW_ENGINE_ID { - (DigestItem::Seal(id, seal.clone()), seal) - } else { - return Err(Error::WrongEngine(id)) - } - }, - _ => return Err(Error::HeaderUnsealed(hash)), - }; - - let pre_hash = header.hash(); - let difficulty = self.algorithm.difficulty(&parent_block_id)?; - - if !self.algorithm.verify( - &parent_block_id, - &pre_hash, - &inner_seal, - difficulty, - )? { - return Err(Error::InvalidSeal); - } - - Ok((header, difficulty, seal)) + Self { inner, client, algorithm, check_inherents_after, + select_chain, inherent_data_providers } } fn check_inherents( @@ -206,9 +214,7 @@ impl<B: BlockT, C, S, Algorithm> PowVerifier<B, C, S, Algorithm> { block_id: BlockId<B>, inherent_data: InherentData, timestamp_now: u64, - ) -> Result<(), Error<B>> where - C: ProvideRuntimeApi<B>, C::Api: BlockBuilderApi<B, Error = sp_blockchain::Error> - { + ) -> Result<(), Error<B>> { const MAX_TIMESTAMP_DRIFT_SECS: u64 = 60; if *block.header().number() < self.check_inherents_after { @@ -243,55 +249,165 @@ impl<B: BlockT, C, S, Algorithm> PowVerifier<B, C, S, Algorithm> { } } -impl<B: BlockT, C, S, Algorithm> Verifier<B> for PowVerifier<B, C, S, Algorithm> where +impl<B, I, C, S, Algorithm> BlockImport<B> for PowBlockImport<B, I, C, S, Algorithm> where + B: BlockT, + I: BlockImport<B, Transaction = sp_api::TransactionFor<C, B>> + Send + Sync, + I::Error: Into<ConsensusError>, + S: SelectChain<B>, C: ProvideRuntimeApi<B> + Send + Sync + HeaderBackend<B> + AuxStore + ProvideCache<B> + BlockOf, C::Api: BlockBuilderApi<B, Error = sp_blockchain::Error>, - S: SelectChain<B>, - Algorithm: PowAlgorithm<B> + Send + Sync, + Algorithm: PowAlgorithm<B>, { - fn verify( + type Error = ConsensusError; + type Transaction = sp_api::TransactionFor<C, B>; + + fn check_block( &mut self, - origin: BlockOrigin, - header: B::Header, - justification: Option<Justification>, - mut body: Option<Vec<B::Extrinsic>>, - ) -> Result<(BlockImportParams<B, ()>, Option<Vec<(CacheKeyId, Vec<u8>)>>), String> { - let inherent_data = self.inherent_data_providers - .create_inherent_data().map_err(|e| e.into_string())?; - let timestamp_now = inherent_data.timestamp_inherent_data().map_err(|e| e.into_string())?; + block: BlockCheckParams<B>, + ) -> Result<ImportResult, Self::Error> { + self.inner.check_block(block).map_err(Into::into) + } + fn import_block( + &mut self, + mut block: BlockImportParams<B, Self::Transaction>, + new_cache: HashMap<CacheKeyId, Vec<u8>>, + ) -> Result<ImportResult, Self::Error> { let best_hash = match self.select_chain.as_ref() { Some(select_chain) => select_chain.best_chain() .map_err(|e| format!("Fetch best chain failed via select chain: {:?}", e))? .hash(), None => self.client.info().best_hash, }; - let hash = header.hash(); - let parent_hash = *header.parent_hash(); + + let parent_hash = *block.header.parent_hash(); let best_aux = PowAux::read::<_, B>(self.client.as_ref(), &best_hash)?; let mut aux = PowAux::read::<_, B>(self.client.as_ref(), &parent_hash)?; - let (checked_header, difficulty, seal) = self.check_header( - header, - BlockId::Hash(parent_hash), - )?; - aux.difficulty = difficulty; - aux.total_difficulty.increment(difficulty); + if let Some(inner_body) = block.body.take() { + let inherent_data = self.inherent_data_providers + .create_inherent_data().map_err(|e| e.into_string())?; + let timestamp_now = inherent_data.timestamp_inherent_data().map_err(|e| e.into_string())?; - if let Some(inner_body) = body.take() { - let block = B::new(checked_header.clone(), inner_body); + let check_block = B::new(block.header.clone(), inner_body); self.check_inherents( - block.clone(), + check_block.clone(), BlockId::Hash(parent_hash), inherent_data, timestamp_now )?; - body = Some(block.deconstruct().1); + block.body = Some(check_block.deconstruct().1); } - let key = aux_key(&hash); + let inner_seal = match block.post_digests.last() { + Some(DigestItem::Seal(id, seal)) => { + if id == &POW_ENGINE_ID { + seal.clone() + } else { + return Err(Error::<B>::WrongEngine(*id).into()) + } + }, + _ => return Err(Error::<B>::HeaderUnsealed(block.header.hash()).into()), + }; + + let intermediate = PowIntermediate::<B, Algorithm::Difficulty>::decode( + &mut &block.intermediates.remove(INTERMEDIATE_KEY) + .ok_or(Error::<B>::NoIntermediate)?[..] + ).map_err(|_| Error::<B>::NoIntermediate)?; + + let difficulty = match intermediate.difficulty { + Some(difficulty) => difficulty, + None => self.algorithm.difficulty(&BlockId::hash(parent_hash))?, + }; + + if !self.algorithm.verify_difficulty( + difficulty, + &BlockId::hash(parent_hash), + &inner_seal, + )? { + return Err(Error::<B>::InvalidDifficulty.into()) + } + + aux.difficulty = difficulty; + aux.total_difficulty.increment(difficulty); + + let key = aux_key(&intermediate.hash); + block.auxiliary.push((key, Some(aux.encode()))); + if block.fork_choice.is_none() { + block.fork_choice = Some(ForkChoiceStrategy::Custom( + aux.total_difficulty > best_aux.total_difficulty + )); + } + + self.inner.import_block(block, new_cache).map_err(Into::into) + } +} + +/// A verifier for PoW blocks. +pub struct PowVerifier<B: BlockT, Algorithm> { + algorithm: Algorithm, + _marker: PhantomData<B>, +} + +impl<B: BlockT, Algorithm> PowVerifier<B, Algorithm> { + pub fn new( + algorithm: Algorithm, + ) -> Self { + Self { algorithm, _marker: PhantomData } + } + + fn check_header( + &self, + mut header: B::Header, + ) -> Result<(B::Header, DigestItem<B::Hash>), Error<B>> where + Algorithm: PowAlgorithm<B>, + { + let hash = header.hash(); + + let (seal, inner_seal) = match header.digest_mut().pop() { + Some(DigestItem::Seal(id, seal)) => { + if id == POW_ENGINE_ID { + (DigestItem::Seal(id, seal.clone()), seal) + } else { + return Err(Error::WrongEngine(id)) + } + }, + _ => return Err(Error::HeaderUnsealed(hash)), + }; + + let pre_hash = header.hash(); + + if !self.algorithm.verify_seal( + &pre_hash, + &inner_seal, + )? { + return Err(Error::InvalidSeal); + } + + Ok((header, seal)) + } +} + +impl<B: BlockT, Algorithm> Verifier<B> for PowVerifier<B, Algorithm> where + Algorithm: PowAlgorithm<B> + Send + Sync, +{ + fn verify( + &mut self, + origin: BlockOrigin, + header: B::Header, + justification: Option<Justification>, + body: Option<Vec<B::Extrinsic>>, + ) -> Result<(BlockImportParams<B, ()>, Option<Vec<(CacheKeyId, Vec<u8>)>>), String> { + let hash = header.hash(); + let (checked_header, seal) = self.check_header(header)?; + + let intermediate = PowIntermediate::<B, Algorithm::Difficulty> { + hash: hash, + difficulty: None, + }; + let import_block = BlockImportParams { origin, header: checked_header, @@ -300,10 +416,13 @@ impl<B: BlockT, C, S, Algorithm> Verifier<B> for PowVerifier<B, C, S, Algorithm> storage_changes: None, finalized: false, justification, - auxiliary: vec![(key, Some(aux.encode()))], - fork_choice: ForkChoiceStrategy::Custom( - aux.total_difficulty > best_aux.total_difficulty - ), + intermediates: { + let mut ret = HashMap::new(); + ret.insert(INTERMEDIATE_KEY.to_vec(), intermediate.encode()); + ret + }, + auxiliary: vec![], + fork_choice: None, allow_missing_state: false, import_existing: false, }; @@ -332,10 +451,7 @@ pub type PowImportQueue<B, Transaction> = BasicQueue<B, Transaction>; /// Import queue for PoW engine. pub fn import_queue<B, C, S, Algorithm>( block_import: BoxBlockImport<B, sp_api::TransactionFor<C, B>>, - client: Arc<C>, algorithm: Algorithm, - check_inherents_after: <<B as BlockT>::Header as HeaderT>::Number, - select_chain: Option<S>, inherent_data_providers: InherentDataProviders, ) -> Result< PowImportQueue<B, sp_api::TransactionFor<C, B>>, @@ -345,18 +461,12 @@ pub fn import_queue<B, C, S, Algorithm>( C: ProvideRuntimeApi<B> + HeaderBackend<B> + BlockOf + ProvideCache<B> + AuxStore, C: Send + Sync + AuxStore + 'static, C::Api: BlockBuilderApi<B, Error = sp_blockchain::Error>, - Algorithm: PowAlgorithm<B> + Send + Sync + 'static, + Algorithm: PowAlgorithm<B> + Clone + Send + Sync + 'static, S: SelectChain<B> + 'static, { register_pow_inherent_data_provider(&inherent_data_providers)?; - let verifier = PowVerifier::new( - client.clone(), - algorithm, - check_inherents_after, - select_chain, - inherent_data_providers, - ); + let verifier = PowVerifier::new(algorithm); Ok(BasicQueue::new( verifier, @@ -485,7 +595,6 @@ fn mine_loop<B: BlockT, C, Algorithm, E, SO, S, CAW>( continue 'outer } - let mut aux = PowAux::read(client, &best_hash)?; let mut proposer = futures::executor::block_on(env.init(&best_header)) .map_err(|e| Error::Environment(format!("{:?}", e)))?; @@ -526,38 +635,36 @@ fn mine_loop<B: BlockT, C, Algorithm, E, SO, S, CAW>( } }; - aux.difficulty = difficulty; - aux.total_difficulty.increment(difficulty); - let hash = { + let (hash, seal) = { + let seal = DigestItem::Seal(POW_ENGINE_ID, seal); let mut header = header.clone(); - header.digest_mut().push(DigestItem::Seal(POW_ENGINE_ID, seal.clone())); - header.hash() + header.digest_mut().push(seal); + let hash = header.hash(); + let seal = header.digest_mut().pop() + .expect("Pushed one seal above; length greater than zero; qed"); + (hash, seal) }; - let key = aux_key(&hash); - let best_hash = match select_chain { - Some(select_chain) => select_chain.best_chain() - .map_err(Error::BestHashSelectChain)? - .hash(), - None => client.info().best_hash, + let intermediate = PowIntermediate::<B, Algorithm::Difficulty> { + hash, + difficulty: Some(difficulty), }; - let best_aux = PowAux::<Algorithm::Difficulty>::read(client, &best_hash)?; - - // if the best block has changed in the meantime drop our proposal - if best_aux.total_difficulty > aux.total_difficulty { - continue 'outer - } let import_block = BlockImportParams { origin: BlockOrigin::Own, header, justification: None, - post_digests: vec![DigestItem::Seal(POW_ENGINE_ID, seal)], + post_digests: vec![seal], body: Some(body), storage_changes: Some(proposal.storage_changes), + intermediates: { + let mut ret = HashMap::new(); + ret.insert(INTERMEDIATE_KEY.to_vec(), intermediate.encode()); + ret + }, finalized: false, - auxiliary: vec![(key, Some(aux.encode()))], - fork_choice: ForkChoiceStrategy::Custom(true), + auxiliary: vec![], + fork_choice: None, allow_missing_state: false, import_existing: false, }; diff --git a/substrate/client/finality-grandpa/src/light_import.rs b/substrate/client/finality-grandpa/src/light_import.rs index 0da22487789220c03ef201dfcf0c05be35d75d52..4ae35167be8ba00c3f3e4dd92ecd84eb947de422 100644 --- a/substrate/client/finality-grandpa/src/light_import.rs +++ b/substrate/client/finality-grandpa/src/light_import.rs @@ -697,7 +697,8 @@ pub mod tests { storage_changes: None, finalized: false, auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::LongestChain, + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::LongestChain), allow_missing_state: true, import_existing: false, }; diff --git a/substrate/client/finality-grandpa/src/tests.rs b/substrate/client/finality-grandpa/src/tests.rs index 9ad12c6c317adcbb87bda1229f74af950a13fbbc..889250c54d7b769c382a5d3188f6c5fd76f276f8 100644 --- a/substrate/client/finality-grandpa/src/tests.rs +++ b/substrate/client/finality-grandpa/src/tests.rs @@ -1032,7 +1032,8 @@ fn allows_reimporting_change_blocks() { storage_changes: None, finalized: false, auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::LongestChain, + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::LongestChain), allow_missing_state: false, import_existing: false, } @@ -1091,7 +1092,8 @@ fn test_bad_justification() { storage_changes: None, finalized: false, auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::LongestChain, + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::LongestChain), allow_missing_state: false, import_existing: false, } @@ -1829,7 +1831,8 @@ fn imports_justification_for_regular_blocks_on_import() { storage_changes: None, finalized: false, auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::LongestChain, + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::LongestChain), allow_missing_state: false, import_existing: false, }; diff --git a/substrate/client/network/test/src/lib.rs b/substrate/client/network/test/src/lib.rs index 1b13e83343e5801cc33cb57db9a290b0b1182f1c..2369ba4f22b81a1c94ea1b353209a0290fcc71c3 100644 --- a/substrate/client/network/test/src/lib.rs +++ b/substrate/client/network/test/src/lib.rs @@ -95,7 +95,8 @@ impl<B: BlockT> Verifier<B> for PassThroughVerifier { justification, post_digests: vec![], auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::LongestChain, + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::LongestChain), allow_missing_state: false, import_existing: false, }, maybe_keys)) diff --git a/substrate/client/src/client.rs b/substrate/client/src/client.rs index c74a005c6fea617e00370f1eb0b8ac7020ee6edb..26b077277f8acb193bec202f108162b561117aef 100644 --- a/substrate/client/src/client.rs +++ b/substrate/client/src/client.rs @@ -818,12 +818,19 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where finalized, auxiliary, fork_choice, + intermediates, import_existing, .. } = import_block; assert!(justification.is_some() && finalized || justification.is_none()); + if !intermediates.is_empty() { + return Err(Error::IncompletePipeline) + } + + let fork_choice = fork_choice.ok_or(Error::IncompletePipeline)?; + let import_headers = if post_digests.is_empty() { PrePostHeader::Same(header) } else { diff --git a/substrate/primitives/blockchain/src/error.rs b/substrate/primitives/blockchain/src/error.rs index d51ab787c72fb93ee80bc5a490eb58918ed869dd..24872448ab6f2781a78cc0230e665d9631d24838 100644 --- a/substrate/primitives/blockchain/src/error.rs +++ b/substrate/primitives/blockchain/src/error.rs @@ -126,6 +126,9 @@ pub enum Error { /// Invalid calculated state root on block import. #[display(fmt = "Calculated state root does not match.")] InvalidStateRoot, + /// Incomplete block import pipeline. + #[display(fmt = "Incomplete block import pipeline.")] + IncompletePipeline, /// A convenience variant for String #[display(fmt = "{}", _0)] Msg(String), diff --git a/substrate/primitives/consensus/common/src/block_import.rs b/substrate/primitives/consensus/common/src/block_import.rs index f8d05e10548d2877fb726f016b634ed04b5757aa..71315b63ef3389d0f0d8cfbce2468aed2d490ad2 100644 --- a/substrate/primitives/consensus/common/src/block_import.rs +++ b/substrate/primitives/consensus/common/src/block_import.rs @@ -142,13 +142,21 @@ pub struct BlockImportParams<Block: BlockT, Transaction> { /// Is this block finalized already? /// `true` implies instant finality. pub finalized: bool, + /// Intermediate values that are interpreted by block importers. Each block importer, + /// upon handling a value, removes it from the intermediate list. The final block importer + /// rejects block import if there are still intermediate values that remain unhandled. + pub intermediates: HashMap<Vec<u8>, Vec<u8>>, /// Auxiliary consensus data produced by the block. /// Contains a list of key-value pairs. If values are `None`, the keys /// will be deleted. pub auxiliary: Vec<(Vec<u8>, Option<Vec<u8>>)>, /// Fork choice strategy of this import. This should only be set by a /// synchronous import, otherwise it may race against other imports. - pub fork_choice: ForkChoiceStrategy, + /// `None` indicates that the current verifier or importer cannot yet + /// determine the fork choice value, and it expects subsequent importer + /// to modify it. If `None` is passed all the way down to bottom block + /// importer, the import fails with an `IncompletePipeline` error. + pub fork_choice: Option<ForkChoiceStrategy>, /// Allow importing the block skipping state verification if parent state is missing. pub allow_missing_state: bool, /// Re-validate existing block. @@ -210,6 +218,7 @@ impl<Block: BlockT, Transaction> BlockImportParams<Block, Transaction> { storage_changes: None, finalized: self.finalized, auxiliary: self.auxiliary, + intermediates: self.intermediates, allow_missing_state: self.allow_missing_state, fork_choice: self.fork_choice, import_existing: self.import_existing, diff --git a/substrate/test-utils/client/src/client_ext.rs b/substrate/test-utils/client/src/client_ext.rs index aa7383e3ab367fc3560b464236ed8be2857ddc9a..e29ee787d030c6ac2e7afe0700378083e10b1a47 100644 --- a/substrate/test-utils/client/src/client_ext.rs +++ b/substrate/test-utils/client/src/client_ext.rs @@ -96,7 +96,8 @@ impl<Block: BlockT, T, Transaction> ClientBlockImportExt<Block> for std::sync::A storage_changes: None, finalized: false, auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::LongestChain, + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::LongestChain), allow_missing_state: false, import_existing: false, }; @@ -115,7 +116,8 @@ impl<Block: BlockT, T, Transaction> ClientBlockImportExt<Block> for std::sync::A storage_changes: None, finalized: false, auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::Custom(true), + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::Custom(true)), allow_missing_state: false, import_existing: false, }; @@ -134,7 +136,8 @@ impl<Block: BlockT, T, Transaction> ClientBlockImportExt<Block> for std::sync::A storage_changes: None, finalized: true, auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::Custom(true), + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::Custom(true)), allow_missing_state: false, import_existing: false, }; @@ -158,7 +161,8 @@ impl<Block: BlockT, T, Transaction> ClientBlockImportExt<Block> for std::sync::A storage_changes: None, finalized: true, auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::LongestChain, + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::LongestChain), allow_missing_state: false, import_existing: false, }; @@ -182,7 +186,8 @@ impl<B, E, RA, Block: BlockT> ClientBlockImportExt<Block> for Client<B, E, Block storage_changes: None, finalized: false, auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::LongestChain, + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::LongestChain), allow_missing_state: false, import_existing: false, }; @@ -201,7 +206,8 @@ impl<B, E, RA, Block: BlockT> ClientBlockImportExt<Block> for Client<B, E, Block storage_changes: None, finalized: false, auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::Custom(true), + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::Custom(true)), allow_missing_state: false, import_existing: false, }; @@ -220,7 +226,8 @@ impl<B, E, RA, Block: BlockT> ClientBlockImportExt<Block> for Client<B, E, Block storage_changes: None, finalized: true, auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::Custom(true), + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::Custom(true)), allow_missing_state: false, import_existing: false, }; @@ -244,7 +251,8 @@ impl<B, E, RA, Block: BlockT> ClientBlockImportExt<Block> for Client<B, E, Block storage_changes: None, finalized: true, auxiliary: Vec::new(), - fork_choice: ForkChoiceStrategy::LongestChain, + intermediates: Default::default(), + fork_choice: Some(ForkChoiceStrategy::LongestChain), allow_missing_state: false, import_existing: false, };