From b8eb38a23dbc0f383ffb5e6a8febb5e33c0d3710 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Sat, 27 Jul 2019 16:00:12 +0200 Subject: [PATCH 1/7] Add transaction pool to babe import queue --- core/consensus/babe/src/lib.rs | 4 +++- core/service/src/chain_ops.rs | 3 ++- core/service/src/components.rs | 6 +++++- core/service/src/lib.rs | 26 +++++++++++++++----------- node/cli/src/service.rs | 19 ++++++++++++------- 5 files changed, 37 insertions(+), 21 deletions(-) diff --git a/core/consensus/babe/src/lib.rs b/core/consensus/babe/src/lib.rs index ae484de5b6d04..e8b718ae42688 100644 --- a/core/consensus/babe/src/lib.rs +++ b/core/consensus/babe/src/lib.rs @@ -1129,7 +1129,7 @@ impl BlockImport for BabeBlockImport, I, RA, PRA>( +pub fn import_queue, I, RA, PRA, T>( config: Config, block_import: I, justification_import: Option>, @@ -1137,6 +1137,7 @@ pub fn import_queue, I, RA, PRA>( client: Arc>, api: Arc, inherent_data_providers: InherentDataProviders, + _transaction_pool: Option>, ) -> ClientResult<( BabeImportQueue, BabeLink, @@ -1150,6 +1151,7 @@ pub fn import_queue, I, RA, PRA>( RA: Send + Sync + 'static, PRA: ProvideRuntimeApi + ProvideCache + Send + Sync + AuxStore + 'static, PRA::Api: BlockBuilderApi + BabeApi, + T: Send + Sync + 'static, { register_babe_inherent_data_provider(&inherent_data_providers, config.get())?; initialize_authorities_cache(&*api)?; diff --git a/core/service/src/chain_ops.rs b/core/service/src/chain_ops.rs index c977b265bb998..c801b81186f18 100644 --- a/core/service/src/chain_ops.rs +++ b/core/service/src/chain_ops.rs @@ -146,7 +146,8 @@ pub fn import_blocks( let (mut queue, _) = components::FullComponents::::build_import_queue( &mut config, client.clone(), - select_chain + select_chain, + None, )?; let (exit_send, exit_recv) = std::sync::mpsc::channel(); diff --git a/core/service/src/components.rs b/core/service/src/components.rs index 76387e6c8886f..6119ff5689c1a 100644 --- a/core/service/src/components.rs +++ b/core/service/src/components.rs @@ -380,6 +380,7 @@ pub trait ServiceFactory: 'static + Sized { config: &mut FactoryFullConfiguration, _client: Arc>, _select_chain: Self::SelectChain, + _transaction_pool: Option>>, ) -> Result { if let Some(name) = config.chain_spec.consensus_engine() { match name { @@ -453,6 +454,7 @@ pub trait Components: Sized + 'static { config: &mut FactoryFullConfiguration, client: Arc>, select_chain: Option, + _transaction_pool: Option>>, ) -> Result<(Self::ImportQueue, Option>>), error::Error>; /// Finality proof provider for serving network requests. @@ -571,10 +573,11 @@ impl Components for FullComponents { config: &mut FactoryFullConfiguration, client: Arc>, select_chain: Option, + transaction_pool: Option>>, ) -> Result<(Self::ImportQueue, Option>>), error::Error> { let select_chain = select_chain .ok_or(error::Error::SelectChainRequired)?; - Factory::build_full_import_queue(config, client, select_chain) + Factory::build_full_import_queue(config, client, select_chain, transaction_pool) .map(|queue| (queue, None)) } @@ -694,6 +697,7 @@ impl Components for LightComponents { config: &mut FactoryFullConfiguration, client: Arc>, _select_chain: Option, + _transaction_pool: Option>>, ) -> Result<(Self::ImportQueue, Option>>), error::Error> { Factory::build_light_import_queue(config, client) .map(|(queue, builder)| (queue, Some(builder))) diff --git a/core/service/src/lib.rs b/core/service/src/lib.rs index 3b2d73c124ccc..c4289a44fef5a 100644 --- a/core/service/src/lib.rs +++ b/core/service/src/lib.rs @@ -166,10 +166,21 @@ impl Service { let (client, on_demand) = Components::build_client(&config, executor, Some(keystore.clone()))?; let select_chain = Components::build_select_chain(&mut config, client.clone())?; + + let transaction_pool = Arc::new( + Components::build_transaction_pool(config.transaction_pool.clone(), client.clone())? + ); + let transaction_pool_adapter = Arc::new(TransactionPoolAdapter { + imports_external_transactions: !config.roles.is_light(), + pool: transaction_pool.clone(), + client: client.clone(), + }); + let (import_queue, finality_proof_request_builder) = Components::build_import_queue( &mut config, client.clone(), select_chain.clone(), + Some(transaction_pool.clone()), )?; let import_queue = Box::new(import_queue); let finality_proof_provider = Components::build_finality_proof_provider(client.clone())?; @@ -190,14 +201,6 @@ impl Service { ); let network_protocol = ::build_network_protocol(&config)?; - let transaction_pool = Arc::new( - Components::build_transaction_pool(config.transaction_pool.clone(), client.clone())? - ); - let transaction_pool_adapter = Arc::new(TransactionPoolAdapter { - imports_external_transactions: !config.roles.is_light(), - pool: transaction_pool.clone(), - client: client.clone(), - }); let protocol_id = { let protocol_id_full = match config.chain_spec.protocol_id() { @@ -946,7 +949,7 @@ where /// LightService = LightComponents /// { |config| >::new(config) }, /// FullImportQueue = BasicQueue -/// { |_, client, _| Ok(BasicQueue::new(MyVerifier, Box::new(client), None, None)) }, +/// { |_, client, _, _| Ok(BasicQueue::new(MyVerifier, Box::new(client), None, None)) }, /// LightImportQueue = BasicQueue /// { |_, client| { /// let fprb = Box::new(DummyFinalityProofRequestBuilder::default()) as Box<_>; @@ -1039,9 +1042,10 @@ macro_rules! construct_service_factory { fn build_full_import_queue( config: &mut $crate::FactoryFullConfiguration, client: $crate::Arc<$crate::FullClient>, - select_chain: Self::SelectChain + select_chain: Self::SelectChain, + transaction_pool: Option>>, ) -> $crate::Result { - ( $( $full_import_queue_init )* ) (config, client, select_chain) + ( $( $full_import_queue_init )* ) (config, client, select_chain, transaction_pool) } fn build_light_import_queue( diff --git a/node/cli/src/service.rs b/node/cli/src/service.rs index 35f813efb3301..13b3a9d934547 100644 --- a/node/cli/src/service.rs +++ b/node/cli/src/service.rs @@ -203,12 +203,15 @@ construct_service_factory! { }, LightService = LightComponents { |config| >::new(config) }, - FullImportQueue = BabeImportQueue { - | - config: &mut FactoryFullConfiguration, - client: Arc>, - select_chain: Self::SelectChain - | { + FullImportQueue = BabeImportQueue + { + | + config: &mut FactoryFullConfiguration, + client: Arc>, + select_chain: Self::SelectChain, + transaction_pool: Option>>, + | + { let (block_import, link_half) = grandpa::block_import::<_, _, _, RuntimeApi, FullClient, _>( client.clone(), client.clone(), select_chain @@ -223,6 +226,7 @@ construct_service_factory! { client.clone(), client, config.custom.inherent_data_providers.clone(), + transaction_pool, )?; config.custom.import_setup = Some((babe_block_import.clone(), link_half, babe_link)); @@ -246,7 +250,7 @@ construct_service_factory! { finality_proof_import.create_finality_proof_request_builder(); // FIXME: pruning task isn't started since light client doesn't do `AuthoritySetup`. - let (import_queue, ..) = import_queue( + let (import_queue, ..) = import_queue::<_, _, _, _, _, _, TransactionPool>( Config::get_or_compute(&*client)?, block_import, None, @@ -254,6 +258,7 @@ construct_service_factory! { client.clone(), client, config.custom.inherent_data_providers.clone(), + None, )?; Ok((import_queue, finality_proof_request_builder)) From bbc9d196b729db49fab1c967a368331704763536 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Sat, 27 Jul 2019 16:32:18 +0200 Subject: [PATCH 2/7] Add transaction pool to Babe check header --- core/consensus/babe/src/lib.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/core/consensus/babe/src/lib.rs b/core/consensus/babe/src/lib.rs index e8b718ae42688..9c6da4d55a536 100644 --- a/core/consensus/babe/src/lib.rs +++ b/core/consensus/babe/src/lib.rs @@ -464,7 +464,7 @@ fn find_next_epoch_digest(header: &B::Header) -> Result /// /// This digest item will always return `Some` when used with `as_babe_pre_digest`. // FIXME #1018 needs misbehavior types -fn check_header( +fn check_header( client: &C, slot_now: u64, mut header: B::Header, @@ -473,8 +473,10 @@ fn check_header( randomness: [u8; 32], epoch_index: u64, c: (u64, u64), -) -> Result, DigestItemFor)>, String> - where DigestItemFor: CompatibleDigestItem, + _transaction_pool: &T, +) -> Result, DigestItemFor)>, String> where + DigestItemFor: CompatibleDigestItem, + T: Send + Sync + 'static, { trace!(target: "babe", "Checking header"); let seal = match header.digest_mut().pop() { @@ -548,14 +550,15 @@ fn check_header( pub struct BabeLink(Arc, Vec<(Instant, u64)>)>>); /// A verifier for Babe blocks. -pub struct BabeVerifier { +pub struct BabeVerifier { api: Arc, inherent_data_providers: inherents::InherentDataProviders, config: Config, time_source: BabeLink, + transaction_pool: Option>, } -impl BabeVerifier { +impl BabeVerifier { fn check_inherents( &self, block: B, @@ -625,9 +628,10 @@ fn median_algorithm( } } -impl Verifier for BabeVerifier where +impl Verifier for BabeVerifier where C: ProvideRuntimeApi + Send + Sync + AuxStore + ProvideCache, C::Api: BlockBuilderApi + BabeApi, + T: Send + Sync + 'static, { fn verify( &mut self, @@ -662,7 +666,7 @@ impl Verifier for BabeVerifier where // We add one to allow for some small drift. // FIXME #1019 in the future, alter this queue to allow deferring of headers - let checked_header = check_header::( + let checked_header = check_header::>>( &self.api, slot_now + 1, header, @@ -671,6 +675,7 @@ impl Verifier for BabeVerifier where randomness, epoch_index, self.config.c(), + &self.transaction_pool, )?; match checked_header { @@ -1137,7 +1142,7 @@ pub fn import_queue, I, RA, PRA, T>( client: Arc>, api: Arc, inherent_data_providers: InherentDataProviders, - _transaction_pool: Option>, + transaction_pool: Option>, ) -> ClientResult<( BabeImportQueue, BabeLink, @@ -1161,6 +1166,7 @@ pub fn import_queue, I, RA, PRA, T>( inherent_data_providers, time_source: Default::default(), config, + transaction_pool, }; #[allow(deprecated)] From 8be77efb6de310e533317885cf77847df66e9a95 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Sat, 27 Jul 2019 22:23:33 +0200 Subject: [PATCH 3/7] Fix tests --- core/consensus/babe/src/tests.rs | 3 ++- node-template/src/service.rs | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/core/consensus/babe/src/tests.rs b/core/consensus/babe/src/tests.rs index eaa4fbe099914..01e0acb96402a 100644 --- a/core/consensus/babe/src/tests.rs +++ b/core/consensus/babe/src/tests.rs @@ -88,7 +88,7 @@ type TestHeader = ::Header; type TestExtrinsic = ::Extrinsic; pub struct TestVerifier { - inner: BabeVerifier, + inner: BabeVerifier, mutator: Mutator, } @@ -143,6 +143,7 @@ impl TestNetFactory for BabeTestNet { inherent_data_providers, config, time_source: Default::default(), + transaction_pool : Default::default(), }, mutator: MUTATOR.with(|s| s.borrow().clone()), } diff --git a/node-template/src/service.rs b/node-template/src/service.rs index e0dba17bbf388..2ef22defaeb92 100644 --- a/node-template/src/service.rs +++ b/node-template/src/service.rs @@ -95,7 +95,12 @@ construct_service_factory! { FullImportQueue = AuraImportQueue< Self::Block, > - { |config: &mut FactoryFullConfiguration , client: Arc>, _select_chain: Self::SelectChain| { + { | + config: &mut FactoryFullConfiguration, + client: Arc>, + _select_chain: Self::SelectChain, + _transaction_pool: Option>>, + | { import_queue::<_, _, aura_primitives::sr25519::AuthorityPair>( SlotDuration::get_or_compute(&*client)?, Box::new(client.clone()), From cf77c262d5c36150033336005da3b585d8443054 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Tue, 30 Jul 2019 12:15:30 +0200 Subject: [PATCH 4/7] Add tx pool to Aura import_queue --- core/consensus/aura/src/lib.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/core/consensus/aura/src/lib.rs b/core/consensus/aura/src/lib.rs index fa5b0533b61e2..a6cde5bdd3c75 100644 --- a/core/consensus/aura/src/lib.rs +++ b/core/consensus/aura/src/lib.rs @@ -391,17 +391,19 @@ fn find_pre_digest(header: &B::Header) -> Result( +fn check_header( client: &C, slot_now: u64, mut header: B::Header, hash: B::Hash, authorities: &[AuthorityId

], + _transaction_pool: &T, ) -> Result)>, String> where DigestItemFor: CompatibleDigestItem

, P::Signature: Decode, C: client::backend::AuxStore, P::Public: Encode + Decode + PartialEq + Clone, + T: Send + Sync + 'static, { let seal = match header.digest_mut().pop() { Some(x) => x, @@ -451,13 +453,14 @@ fn check_header( } /// A verifier for Aura blocks. -pub struct AuraVerifier { +pub struct AuraVerifier { client: Arc, phantom: PhantomData

, inherent_data_providers: inherents::InherentDataProviders, + transaction_pool: Option>, } -impl AuraVerifier +impl AuraVerifier where P: Send + Sync + 'static { fn check_inherents( @@ -510,13 +513,14 @@ impl AuraVerifier } #[forbid(deprecated)] -impl Verifier for AuraVerifier where +impl Verifier for AuraVerifier where C: ProvideRuntimeApi + Send + Sync + client::backend::AuxStore + ProvideCache + BlockOf, C::Api: BlockBuilderApi + AuraApi>, DigestItemFor: CompatibleDigestItem

, P: Pair + Send + Sync + 'static, P::Public: Send + Sync + Hash + Eq + Clone + Decode + Encode + Debug + 'static, P::Signature: Encode + Decode, + T: Send + Sync + 'static, { fn verify( &mut self, @@ -536,12 +540,13 @@ impl Verifier for AuraVerifier where // we add one to allow for some small drift. // FIXME #1019 in the future, alter this queue to allow deferring of // headers - let checked_header = check_header::( + let checked_header = check_header::>>( &self.client, slot_now + 1, header, hash, &authorities[..], + &self.transaction_pool, )?; match checked_header { CheckedHeader::Checked(pre_header, (slot_num, seal)) => { @@ -680,13 +685,14 @@ fn register_aura_inherent_data_provider( } /// Start an import queue for the Aura consensus algorithm. -pub fn import_queue( +pub fn import_queue( slot_duration: SlotDuration, block_import: BoxBlockImport, justification_import: Option>, finality_proof_import: Option>, client: Arc, inherent_data_providers: InherentDataProviders, + transaction_pool: Option>, ) -> Result, consensus_common::Error> where B: BlockT, C: 'static + ProvideRuntimeApi + BlockOf + ProvideCache + Send + Sync + AuxStore, @@ -695,6 +701,7 @@ pub fn import_queue( P: Pair + Send + Sync + 'static, P::Public: Clone + Eq + Send + Sync + Hash + Debug + Encode + Decode, P::Signature: Encode + Decode, + T: Send + Sync + 'static, { register_aura_inherent_data_provider(&inherent_data_providers, slot_duration.get())?; initialize_authorities_cache(&*client)?; @@ -703,6 +710,7 @@ pub fn import_queue( client: client.clone(), inherent_data_providers, phantom: PhantomData, + transaction_pool, }; Ok(BasicQueue::new( verifier, From 5e29bdf2704fd62ecdc30e8b74401dca88ba9b42 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Tue, 30 Jul 2019 12:46:41 +0200 Subject: [PATCH 5/7] Fix tests, node-template --- core/consensus/aura/src/lib.rs | 3 ++- node-template/src/service.rs | 8 +++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/core/consensus/aura/src/lib.rs b/core/consensus/aura/src/lib.rs index a6cde5bdd3c75..d7b7664dec0a1 100644 --- a/core/consensus/aura/src/lib.rs +++ b/core/consensus/aura/src/lib.rs @@ -781,7 +781,7 @@ mod tests { impl TestNetFactory for AuraTestNet { type Specialization = DummySpecialization; - type Verifier = AuraVerifier; + type Verifier = AuraVerifier; type PeerData = (); /// Create new test network with peers and given config. @@ -808,6 +808,7 @@ mod tests { AuraVerifier { client, inherent_data_providers, + transaction_pool: Default::default(), phantom: Default::default(), } }, diff --git a/node-template/src/service.rs b/node-template/src/service.rs index 2ef22defaeb92..7f2c80c48b2b8 100644 --- a/node-template/src/service.rs +++ b/node-template/src/service.rs @@ -99,15 +99,16 @@ construct_service_factory! { config: &mut FactoryFullConfiguration, client: Arc>, _select_chain: Self::SelectChain, - _transaction_pool: Option>>, + transaction_pool: Option>>, | { - import_queue::<_, _, aura_primitives::sr25519::AuthorityPair>( + import_queue::<_, _, aura_primitives::sr25519::AuthorityPair, _>( SlotDuration::get_or_compute(&*client)?, Box::new(client.clone()), None, None, client, config.custom.inherent_data_providers.clone(), + transaction_pool, ).map_err(Into::into) } }, @@ -116,13 +117,14 @@ construct_service_factory! { > { |config: &mut FactoryFullConfiguration, client: Arc>| { let fprb = Box::new(DummyFinalityProofRequestBuilder::default()) as Box<_>; - import_queue::<_, _, AuraAuthorityPair>( + import_queue::<_, _, AuraAuthorityPair, TransactionPool>( SlotDuration::get_or_compute(&*client)?, Box::new(client.clone()), None, None, client, config.custom.inherent_data_providers.clone(), + None, ).map(|q| (q, fprb)).map_err(Into::into) } }, From 3fbdd5501129d551bc519d03c068693954dec73d Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Tue, 30 Jul 2019 13:45:22 +0200 Subject: [PATCH 6/7] Add comments regarding unused _transaction_pool --- core/consensus/aura/src/lib.rs | 3 ++- core/consensus/babe/src/lib.rs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/core/consensus/aura/src/lib.rs b/core/consensus/aura/src/lib.rs index d7b7664dec0a1..1565de815e698 100644 --- a/core/consensus/aura/src/lib.rs +++ b/core/consensus/aura/src/lib.rs @@ -390,7 +390,8 @@ fn find_pre_digest(header: &B::Header) -> Result( client: &C, slot_now: u64, diff --git a/core/consensus/babe/src/lib.rs b/core/consensus/babe/src/lib.rs index 9c6da4d55a536..ebba95619324f 100644 --- a/core/consensus/babe/src/lib.rs +++ b/core/consensus/babe/src/lib.rs @@ -463,7 +463,8 @@ fn find_next_epoch_digest(header: &B::Header) -> Result /// unsigned. This is required for security and must not be changed. /// /// This digest item will always return `Some` when used with `as_babe_pre_digest`. -// FIXME #1018 needs misbehavior types +// FIXME #1018 needs misbehavior types. The `transaction_pool` parameter will be +// used to submit such misbehavior reports. fn check_header( client: &C, slot_now: u64, From c872edebc9f052a44abf11b63ff646b7484739e5 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Wed, 31 Jul 2019 13:53:30 +0200 Subject: [PATCH 7/7] Make tx pool optional in check_header --- core/consensus/aura/src/lib.rs | 6 +++--- core/consensus/babe/src/lib.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/core/consensus/aura/src/lib.rs b/core/consensus/aura/src/lib.rs index 1565de815e698..74799837c4047 100644 --- a/core/consensus/aura/src/lib.rs +++ b/core/consensus/aura/src/lib.rs @@ -398,7 +398,7 @@ fn check_header( mut header: B::Header, hash: B::Hash, authorities: &[AuthorityId

], - _transaction_pool: &T, + _transaction_pool: Option<&T>, ) -> Result)>, String> where DigestItemFor: CompatibleDigestItem

, P::Signature: Decode, @@ -541,13 +541,13 @@ impl Verifier for AuraVerifier where // we add one to allow for some small drift. // FIXME #1019 in the future, alter this queue to allow deferring of // headers - let checked_header = check_header::>>( + let checked_header = check_header::( &self.client, slot_now + 1, header, hash, &authorities[..], - &self.transaction_pool, + self.transaction_pool.as_ref().map(|x| &**x), )?; match checked_header { CheckedHeader::Checked(pre_header, (slot_num, seal)) => { diff --git a/core/consensus/babe/src/lib.rs b/core/consensus/babe/src/lib.rs index ebba95619324f..e46594dd1e876 100644 --- a/core/consensus/babe/src/lib.rs +++ b/core/consensus/babe/src/lib.rs @@ -474,7 +474,7 @@ fn check_header( randomness: [u8; 32], epoch_index: u64, c: (u64, u64), - _transaction_pool: &T, + _transaction_pool: Option<&T>, ) -> Result, DigestItemFor)>, String> where DigestItemFor: CompatibleDigestItem, T: Send + Sync + 'static, @@ -667,7 +667,7 @@ impl Verifier for BabeVerifier where // We add one to allow for some small drift. // FIXME #1019 in the future, alter this queue to allow deferring of headers - let checked_header = check_header::>>( + let checked_header = check_header::( &self.api, slot_now + 1, header, @@ -676,7 +676,7 @@ impl Verifier for BabeVerifier where randomness, epoch_index, self.config.c(), - &self.transaction_pool, + self.transaction_pool.as_ref().map(|x| &**x), )?; match checked_header {