From 08f45cd1562c1a142771726042cfbf75a452f5a7 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Mon, 10 Aug 2020 03:11:56 +0200 Subject: [PATCH 1/9] pow: check can_author_with before calling check_inherents --- client/consensus/pow/src/lib.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/client/consensus/pow/src/lib.rs b/client/consensus/pow/src/lib.rs index 42d1bc050192c..9b7806f09970d 100644 --- a/client/consensus/pow/src/lib.rs +++ b/client/consensus/pow/src/lib.rs @@ -186,16 +186,17 @@ pub trait PowAlgorithm { } /// A block importer for PoW. -pub struct PowBlockImport { +pub struct PowBlockImport { algorithm: Algorithm, inner: I, select_chain: Option, client: Arc, inherent_data_providers: sp_inherents::InherentDataProviders, check_inherents_after: <::Header as HeaderT>::Number, + can_author_with: CAW, } -impl Clone for PowBlockImport { +impl Clone for PowBlockImport { fn clone(&self) -> Self { Self { algorithm: self.algorithm.clone(), @@ -204,17 +205,19 @@ impl Clone for PowBlockImpor client: self.client.clone(), inherent_data_providers: self.inherent_data_providers.clone(), check_inherents_after: self.check_inherents_after.clone(), + can_author_with: self.can_author_with.clone(), } } } -impl PowBlockImport where +impl PowBlockImport where B: BlockT, I: BlockImport> + Send + Sync, I::Error: Into, C: ProvideRuntimeApi + Send + Sync + HeaderBackend + AuxStore + ProvideCache + BlockOf, C::Api: BlockBuilderApi, Algorithm: PowAlgorithm, + CAW: CanAuthorWith, { /// Create a new block import suitable to be used in PoW pub fn new( @@ -224,9 +227,10 @@ impl PowBlockImport where check_inherents_after: <::Header as HeaderT>::Number, select_chain: Option, inherent_data_providers: sp_inherents::InherentDataProviders, + can_author_with: CAW, ) -> Self { Self { inner, client, algorithm, check_inherents_after, - select_chain, inherent_data_providers } + select_chain, inherent_data_providers, can_author_with, } } fn check_inherents( @@ -242,6 +246,10 @@ impl PowBlockImport where return Ok(()) } + if let Err(_) = self.can_author_with.can_author_with(&block_id) { + return Ok(()) + } + let inherent_res = self.client.runtime_api().check_inherents( &block_id, block, @@ -270,7 +278,7 @@ impl PowBlockImport where } } -impl BlockImport for PowBlockImport where +impl BlockImport for PowBlockImport where B: BlockT, I: BlockImport> + Send + Sync, I::Error: Into, @@ -279,6 +287,7 @@ impl BlockImport for PowBlockImport, Algorithm: PowAlgorithm, Algorithm::Difficulty: 'static, + CAW: CanAuthorWith, { type Error = ConsensusError; type Transaction = sp_api::TransactionFor; @@ -649,7 +658,7 @@ fn mine_loop( }; log::info!("✅ Successfully mined block: {}", best_hash); - + let (hash, seal) = { let seal = DigestItem::Seal(POW_ENGINE_ID, seal); let mut header = header.clone(); From 7408c03a0a0bd4281b07b8417ee060a5263599b1 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Mon, 10 Aug 2020 03:22:15 +0200 Subject: [PATCH 2/9] babe: check can_author_with before calling check_inherents --- client/consensus/babe/src/lib.rs | 18 ++++++++++++++---- client/consensus/babe/src/tests.rs | 5 +++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/client/consensus/babe/src/lib.rs b/client/consensus/babe/src/lib.rs index 951d1467b4983..9ae7250174468 100644 --- a/client/consensus/babe/src/lib.rs +++ b/client/consensus/babe/src/lib.rs @@ -787,22 +787,24 @@ impl BabeLink { } /// A verifier for Babe blocks. -pub struct BabeVerifier { +pub struct BabeVerifier { client: Arc, select_chain: SelectChain, inherent_data_providers: sp_inherents::InherentDataProviders, config: Config, epoch_changes: SharedEpochChanges, time_source: TimeSource, + can_author_with: CAW } -impl BabeVerifier +impl BabeVerifier where Block: BlockT, Client: AuxStore + HeaderBackend + HeaderMetadata + ProvideRuntimeApi, Client::Api: BlockBuilderApi + BabeApi, SelectChain: sp_consensus::SelectChain, + CAW: CanAuthorWith, { fn check_inherents( &self, @@ -810,6 +812,10 @@ where block_id: BlockId, inherent_data: InherentData, ) -> Result<(), Error> { + if let Err(_) = self.can_author_with.can_author_with(&block_id) { + return Ok(()) + } + let inherent_res = self.client.runtime_api().check_inherents( &block_id, block, @@ -908,13 +914,14 @@ where } } -impl Verifier for BabeVerifier +impl Verifier for BabeVerifier where Block: BlockT, Client: HeaderMetadata + HeaderBackend + ProvideRuntimeApi + Send + Sync + AuxStore + ProvideCache, Client::Api: BlockBuilderApi + BabeApi, SelectChain: sp_consensus::SelectChain, + CAW: CanAuthorWith + Send + Sync, { fn verify( &mut self, @@ -1422,7 +1429,7 @@ pub fn block_import( /// /// The block import object provided must be the `BabeBlockImport` or a wrapper /// of it, otherwise crucial import logic will be omitted. -pub fn import_queue( +pub fn import_queue( babe_link: BabeLink, block_import: Inner, justification_import: Option>, @@ -1432,6 +1439,7 @@ pub fn import_queue( inherent_data_providers: InherentDataProviders, spawner: &impl sp_core::traits::SpawnNamed, registry: Option<&Registry>, + can_author_with: CAW ) -> ClientResult> where Inner: BlockImport> + Send + Sync + 'static, @@ -1439,6 +1447,7 @@ pub fn import_queue( Client: HeaderBackend + HeaderMetadata, Client::Api: BlockBuilderApi + BabeApi + ApiExt, SelectChain: sp_consensus::SelectChain + 'static, + CAW: CanAuthorWith + Send + Sync + 'static, { register_babe_inherent_data_provider(&inherent_data_providers, babe_link.config.slot_duration)?; @@ -1449,6 +1458,7 @@ pub fn import_queue( config: babe_link.config, epoch_changes: babe_link.epoch_changes, time_source: babe_link.time_source, + can_author_with, }; Ok(BasicQueue::new( diff --git a/client/consensus/babe/src/tests.rs b/client/consensus/babe/src/tests.rs index 958d7845edbc6..e302a3b3d0a61 100644 --- a/client/consensus/babe/src/tests.rs +++ b/client/consensus/babe/src/tests.rs @@ -31,7 +31,7 @@ use sp_consensus_babe::{ }; use sc_block_builder::{BlockBuilder, BlockBuilderProvider}; use sp_consensus::{ - NoNetwork as DummyOracle, Proposal, RecordProof, + NoNetwork as DummyOracle, Proposal, RecordProof, AlwaysCanAuthor, import_queue::{BoxBlockImport, BoxJustificationImport, BoxFinalityProofImport}, }; use sc_network_test::*; @@ -220,7 +220,7 @@ type TestSelectChain = substrate_test_runtime_client::LongestChain< >; pub struct TestVerifier { - inner: BabeVerifier, + inner: BabeVerifier, mutator: Mutator, } @@ -320,6 +320,7 @@ impl TestNetFactory for BabeTestNet { config: data.link.config.clone(), epoch_changes: data.link.epoch_changes.clone(), time_source: data.link.time_source.clone(), + can_author_with: AlwaysCanAuthor, }, mutator: MUTATOR.with(|m| m.borrow().clone()), } From af2edbb12cb9d9b013b9b65fe050acdbf4abdee1 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Mon, 10 Aug 2020 03:22:33 +0200 Subject: [PATCH 3/9] aura: check can_author_with before calling check_inherents --- client/consensus/aura/src/lib.rs | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs index 4e6cb49f1129d..c518b3b6c3587 100644 --- a/client/consensus/aura/src/lib.rs +++ b/client/consensus/aura/src/lib.rs @@ -492,14 +492,16 @@ fn check_header( } /// A verifier for Aura blocks. -pub struct AuraVerifier { +pub struct AuraVerifier { client: Arc, phantom: PhantomData

, inherent_data_providers: sp_inherents::InherentDataProviders, + can_author_with: CAW, } -impl AuraVerifier - where P: Send + Sync + 'static +impl AuraVerifier where + P: Send + Sync + 'static, + CAW: Send + Sync + 'static, { fn check_inherents( &self, @@ -507,11 +509,16 @@ impl AuraVerifier block_id: BlockId, inherent_data: InherentData, timestamp_now: u64, - ) -> Result<(), Error> - where C: ProvideRuntimeApi, C::Api: BlockBuilderApi + ) -> Result<(), Error> where + C: ProvideRuntimeApi, C::Api: BlockBuilderApi, + CAW: CanAuthorWith, { const MAX_TIMESTAMP_DRIFT_SECS: u64 = 60; + if let Err(_) = self.can_author_with.can_author_with(&block_id) { + return Ok(()) + } + let inherent_res = self.client.runtime_api().check_inherents( &block_id, block, @@ -553,7 +560,7 @@ impl AuraVerifier } #[forbid(deprecated)] -impl Verifier for AuraVerifier where +impl Verifier for AuraVerifier where C: ProvideRuntimeApi + Send + Sync + @@ -565,6 +572,7 @@ impl Verifier for AuraVerifier where P: Pair + Send + Sync + 'static, P::Public: Send + Sync + Hash + Eq + Clone + Decode + Encode + Debug + 'static, P::Signature: Encode + Decode, + CAW: CanAuthorWith + Send + Sync + 'static, { fn verify( &mut self, @@ -812,7 +820,7 @@ impl BlockImport for AuraBlockImport( +pub fn import_queue( slot_duration: SlotDuration, block_import: I, justification_import: Option>, @@ -821,6 +829,7 @@ pub fn import_queue( inherent_data_providers: InherentDataProviders, spawner: &S, registry: Option<&Registry>, + can_author_with: CAW, ) -> Result, sp_consensus::Error> where B: BlockT, C::Api: BlockBuilderApi + AuraApi> + ApiExt, @@ -831,6 +840,7 @@ pub fn import_queue( P::Public: Clone + Eq + Send + Sync + Hash + Debug + Encode + Decode, P::Signature: Encode + Decode, S: sp_core::traits::SpawnNamed, + CAW: CanAuthorWith + Send + Sync + 'static, { register_aura_inherent_data_provider(&inherent_data_providers, slot_duration.get())?; initialize_authorities_cache(&*client)?; @@ -839,6 +849,7 @@ pub fn import_queue( client, inherent_data_providers, phantom: PhantomData, + can_author_with, }; Ok(BasicQueue::new( @@ -854,7 +865,7 @@ pub fn import_queue( #[cfg(test)] mod tests { use super::*; - use sp_consensus::{NoNetwork as DummyOracle, Proposal, RecordProof}; + use sp_consensus::{NoNetwork as DummyOracle, Proposal, RecordProof, AlwaysCanAuthor}; use sc_network_test::{Block as TestBlock, *}; use sp_runtime::traits::{Block as BlockT, DigestFor}; use sc_network::config::ProtocolConfig; @@ -924,7 +935,7 @@ mod tests { } impl TestNetFactory for AuraTestNet { - type Verifier = AuraVerifier; + type Verifier = AuraVerifier; type PeerData = (); /// Create new test network with peers and given config. @@ -951,6 +962,7 @@ mod tests { client, inherent_data_providers, phantom: Default::default(), + can_author_with: AlwaysCanAuthor, } }, PeersClient::Light(_, _) => unreachable!("No (yet) tests for light client + Aura"), From 2a2d938c2d27c899f602e6683ceff863c8ae09c2 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Mon, 10 Aug 2020 03:56:56 +0200 Subject: [PATCH 4/9] Fix node and node template compile --- bin/node-template/node/src/service.rs | 6 ++++-- bin/node/cli/src/service.rs | 2 ++ primitives/consensus/common/src/lib.rs | 9 +++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/bin/node-template/node/src/service.rs b/bin/node-template/node/src/service.rs index 021d0ac8f7d1e..81d62d6424cda 100644 --- a/bin/node-template/node/src/service.rs +++ b/bin/node-template/node/src/service.rs @@ -54,7 +54,7 @@ pub fn new_partial(config: &Configuration) -> Result( + let import_queue = sc_consensus_aura::import_queue::<_, _, _, AuraPair, _, _>( sc_consensus_aura::slot_duration(&*client)?, aura_block_import, Some(Box::new(grandpa_block_import.clone())), @@ -63,6 +63,7 @@ pub fn new_partial(config: &Configuration) -> Result Result { let finality_proof_request_builder = finality_proof_import.create_finality_proof_request_builder(); - let import_queue = sc_consensus_aura::import_queue::<_, _, _, AuraPair, _>( + let import_queue = sc_consensus_aura::import_queue::<_, _, _, AuraPair, _, _>( sc_consensus_aura::slot_duration(&*client)?, grandpa_block_import, None, @@ -249,6 +250,7 @@ pub fn new_light(config: Configuration) -> Result { InherentDataProviders::new(), &task_manager.spawn_handle(), config.prometheus_registry(), + sp_consensus::NeverCanAuthor, )?; let finality_proof_provider = diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index fd2c240a44e3b..e5bcb6443fe11 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -94,6 +94,7 @@ pub fn new_partial(config: &Configuration) -> Result Result<( inherent_data_providers.clone(), &task_manager.spawn_handle(), config.prometheus_registry(), + sp_consensus::NeverCanAuthor, )?; let finality_proof_provider = diff --git a/primitives/consensus/common/src/lib.rs b/primitives/consensus/common/src/lib.rs index 04b65a723e4a8..0e4dd91dd497e 100644 --- a/primitives/consensus/common/src/lib.rs +++ b/primitives/consensus/common/src/lib.rs @@ -247,6 +247,15 @@ impl CanAuthorWith for AlwaysCanAuthor { } } +/// Never can author. +pub struct NeverCanAuthor; + +impl CanAuthorWith for NeverCanAuthor { + fn can_author_with(&self, _: &BlockId) -> Result<(), String> { + Err("Authoring is always disabled.".to_string()) + } +} + /// A type from which a slot duration can be obtained. pub trait SlotData { /// Gets the slot duration. From ae14175db3a559bb04e7f41a5b1d04b62b84e09c Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Tue, 11 Aug 2020 02:11:13 +0200 Subject: [PATCH 5/9] Add missing comma --- client/consensus/babe/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/consensus/babe/src/lib.rs b/client/consensus/babe/src/lib.rs index 9ae7250174468..0a9a58f8c68b8 100644 --- a/client/consensus/babe/src/lib.rs +++ b/client/consensus/babe/src/lib.rs @@ -794,7 +794,7 @@ pub struct BabeVerifier { config: Config, epoch_changes: SharedEpochChanges, time_source: TimeSource, - can_author_with: CAW + can_author_with: CAW, } impl BabeVerifier @@ -1439,7 +1439,7 @@ pub fn import_queue( inherent_data_providers: InherentDataProviders, spawner: &impl sp_core::traits::SpawnNamed, registry: Option<&Registry>, - can_author_with: CAW + can_author_with: CAW, ) -> ClientResult> where Inner: BlockImport> + Send + Sync + 'static, From bb6e3a0d239e8f042adc1da6518424920d020ef9 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Tue, 11 Aug 2020 02:11:50 +0200 Subject: [PATCH 6/9] Put each parameter on its own line --- client/consensus/pow/src/lib.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/client/consensus/pow/src/lib.rs b/client/consensus/pow/src/lib.rs index 9b7806f09970d..eef6d704051f2 100644 --- a/client/consensus/pow/src/lib.rs +++ b/client/consensus/pow/src/lib.rs @@ -229,8 +229,15 @@ impl PowBlockImport wher inherent_data_providers: sp_inherents::InherentDataProviders, can_author_with: CAW, ) -> Self { - Self { inner, client, algorithm, check_inherents_after, - select_chain, inherent_data_providers, can_author_with, } + Self { + inner, + client, + algorithm, + check_inherents_after, + select_chain, + inherent_data_providers, + can_author_with, + } } fn check_inherents( From fc1841bd0e962904d4a17098d499364e186b1f73 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Tue, 11 Aug 2020 02:14:13 +0200 Subject: [PATCH 7/9] Add debug print --- client/consensus/aura/src/lib.rs | 8 +++++++- client/consensus/babe/src/lib.rs | 8 +++++++- client/consensus/pow/src/lib.rs | 8 +++++++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs index c518b3b6c3587..4204028711328 100644 --- a/client/consensus/aura/src/lib.rs +++ b/client/consensus/aura/src/lib.rs @@ -515,7 +515,13 @@ impl AuraVerifier where { const MAX_TIMESTAMP_DRIFT_SECS: u64 = 60; - if let Err(_) = self.can_author_with.can_author_with(&block_id) { + if let Err(e) = self.can_author_with.can_author_with(&block_id) { + debug!( + target: "aura", + "Skipping `check_inherents` as authoring version is not compatible: {}", + e, + ); + return Ok(()) } diff --git a/client/consensus/babe/src/lib.rs b/client/consensus/babe/src/lib.rs index 0a9a58f8c68b8..8f88a54eba319 100644 --- a/client/consensus/babe/src/lib.rs +++ b/client/consensus/babe/src/lib.rs @@ -812,7 +812,13 @@ where block_id: BlockId, inherent_data: InherentData, ) -> Result<(), Error> { - if let Err(_) = self.can_author_with.can_author_with(&block_id) { + if let Err(e) = self.can_author_with.can_author_with(&block_id) { + debug!( + target: "babe", + "Skipping `check_inherents` as authoring version is not compatible: {}", + e, + ); + return Ok(()) } diff --git a/client/consensus/pow/src/lib.rs b/client/consensus/pow/src/lib.rs index eef6d704051f2..753ea6531c0dd 100644 --- a/client/consensus/pow/src/lib.rs +++ b/client/consensus/pow/src/lib.rs @@ -253,7 +253,13 @@ impl PowBlockImport wher return Ok(()) } - if let Err(_) = self.can_author_with.can_author_with(&block_id) { + if let Err(e) = self.can_author_with.can_author_with(&block_id) { + debug!( + target: "pow", + "Skipping `check_inherents` as authoring version is not compatible: {}", + e, + ); + return Ok(()) } From 04bbfed7cf5bf7499b731d0ee6c2fb8eb2339686 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Tue, 11 Aug 2020 03:03:20 +0200 Subject: [PATCH 8/9] Fix line width too long --- client/consensus/babe/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/consensus/babe/src/lib.rs b/client/consensus/babe/src/lib.rs index 8f88a54eba319..9e7c3c9081b54 100644 --- a/client/consensus/babe/src/lib.rs +++ b/client/consensus/babe/src/lib.rs @@ -920,7 +920,8 @@ where } } -impl Verifier for BabeVerifier +impl Verifier + for BabeVerifier where Block: BlockT, Client: HeaderMetadata + HeaderBackend + ProvideRuntimeApi From 983d87c594f04051d8b135ccd0331392bd892032 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Tue, 11 Aug 2020 03:23:55 +0200 Subject: [PATCH 9/9] Fix pow line width issue --- client/consensus/pow/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/client/consensus/pow/src/lib.rs b/client/consensus/pow/src/lib.rs index 753ea6531c0dd..f8da1877665fc 100644 --- a/client/consensus/pow/src/lib.rs +++ b/client/consensus/pow/src/lib.rs @@ -196,7 +196,9 @@ pub struct PowBlockImport { can_author_with: CAW, } -impl Clone for PowBlockImport { +impl Clone + for PowBlockImport +{ fn clone(&self) -> Self { Self { algorithm: self.algorithm.clone(),