diff --git a/substrate/bin/node-template/node/src/service.rs b/substrate/bin/node-template/node/src/service.rs index a3aca89ef7..4061dce438 100644 --- a/substrate/bin/node-template/node/src/service.rs +++ b/substrate/bin/node-template/node/src/service.rs @@ -52,6 +52,7 @@ pub fn new_partial(config: &Configuration) -> Result Result for rpc::Error { fn from(e: Error) -> Self { @@ -158,6 +161,14 @@ impl From for rpc::Error { message: "Immediately Dropped".into(), data: Some("The transaction couldn't enter the pool because of the limit".into()), }, + Error::Pool(PoolError::Unactionable) => rpc::Error { + code: rpc::ErrorCode::ServerError(POOL_UNACTIONABLE), + message: "Unactionable".into(), + data: Some( + "The transaction is unactionable since it is not propagable and \ + the local node does not author blocks".into(), + ), + }, Error::UnsupportedKeyType => rpc::Error { code: rpc::ErrorCode::ServerError(UNSUPPORTED_KEY_TYPE), message: "Unknown key type crypto" .into(), diff --git a/substrate/client/rpc/src/author/tests.rs b/substrate/client/rpc/src/author/tests.rs index 9dd4f1b143..0e7cb55395 100644 --- a/substrate/client/rpc/src/author/tests.rs +++ b/substrate/client/rpc/src/author/tests.rs @@ -65,6 +65,7 @@ impl Default for TestSetup { let spawner = sp_core::testing::TaskExecutor::new(); let pool = BasicPool::new_full( Default::default(), + true.into(), None, spawner, client.clone(), diff --git a/substrate/client/service/src/lib.rs b/substrate/client/service/src/lib.rs index 170b7f79d1..4880b8cffd 100644 --- a/substrate/client/service/src/lib.rs +++ b/substrate/client/service/src/lib.rs @@ -599,6 +599,7 @@ mod tests { let spawner = sp_core::testing::TaskExecutor::new(); let pool = BasicPool::new_full( Default::default(), + true.into(), None, spawner, client.clone(), diff --git a/substrate/client/transaction-pool/graph/benches/basics.rs b/substrate/client/transaction-pool/graph/benches/basics.rs index f7096b0214..21e3d1006d 100644 --- a/substrate/client/transaction-pool/graph/benches/basics.rs +++ b/substrate/client/transaction-pool/graph/benches/basics.rs @@ -164,13 +164,19 @@ fn benchmark_main(c: &mut Criterion) { c.bench_function("sequential 50 tx", |b| { b.iter(|| { - bench_configured(Pool::new(Default::default(), TestApi::new_dependant().into()), 50); + bench_configured( + Pool::new(Default::default(), true.into(), TestApi::new_dependant().into()), + 50, + ); }); }); c.bench_function("random 100 tx", |b| { b.iter(|| { - bench_configured(Pool::new(Default::default(), TestApi::default().into()), 100); + bench_configured( + Pool::new(Default::default(), true.into(), TestApi::default().into()), + 100, + ); }); }); } diff --git a/substrate/client/transaction-pool/graph/src/lib.rs b/substrate/client/transaction-pool/graph/src/lib.rs index b8d36d0399..c61b05befa 100644 --- a/substrate/client/transaction-pool/graph/src/lib.rs +++ b/substrate/client/transaction-pool/graph/src/lib.rs @@ -39,6 +39,6 @@ pub mod watcher; pub use self::base_pool::Transaction; pub use self::pool::{ - Pool, Options, ChainApi, EventStream, ExtrinsicFor, ExtrinsicHash, - BlockHash, NumberFor, TransactionFor, ValidatedTransaction, + BlockHash, ChainApi, EventStream, ExtrinsicFor, ExtrinsicHash, IsValidator, NumberFor, Options, + Pool, TransactionFor, ValidatedTransaction, }; diff --git a/substrate/client/transaction-pool/graph/src/pool.rs b/substrate/client/transaction-pool/graph/src/pool.rs index 8255370df5..eee14049d4 100644 --- a/substrate/client/transaction-pool/graph/src/pool.rs +++ b/substrate/client/transaction-pool/graph/src/pool.rs @@ -36,7 +36,7 @@ use wasm_timer::Instant; use futures::channel::mpsc::Receiver; use crate::validated_pool::ValidatedPool; -pub use crate::validated_pool::ValidatedTransaction; +pub use crate::validated_pool::{IsValidator, ValidatedTransaction}; /// Modification notification event stream type; pub type EventStream = Receiver; @@ -150,9 +150,9 @@ where impl Pool { /// Create a new transaction pool. - pub fn new(options: Options, api: Arc) -> Self { + pub fn new(options: Options, is_validator: IsValidator, api: Arc) -> Self { Pool { - validated_pool: Arc::new(ValidatedPool::new(options, api)), + validated_pool: Arc::new(ValidatedPool::new(options, is_validator, api)), } } @@ -497,43 +497,58 @@ mod tests { ) -> Self::ValidationFuture { let hash = self.hash_and_length(&uxt).0; let block_number = self.block_id_to_number(at).unwrap().unwrap(); - let nonce = uxt.transfer().nonce; - // This is used to control the test flow. - if nonce > 0 { - let opt = self.delay.lock().take(); - if let Some(delay) = opt { - if delay.recv().is_err() { - println!("Error waiting for delay!"); + let res = match uxt { + Extrinsic::Transfer { transfer, .. } => { + let nonce = transfer.nonce; + + // This is used to control the test flow. + if nonce > 0 { + let opt = self.delay.lock().take(); + if let Some(delay) = opt { + if delay.recv().is_err() { + println!("Error waiting for delay!"); + } + } } - } - } - if self.invalidate.lock().contains(&hash) { - return futures::future::ready(Ok(InvalidTransaction::Custom(0).into())); - } + if self.invalidate.lock().contains(&hash) { + InvalidTransaction::Custom(0).into() + } else if nonce < block_number { + InvalidTransaction::Stale.into() + } else { + let mut transaction = ValidTransaction { + priority: 4, + requires: if nonce > block_number { vec![vec![nonce as u8 - 1]] } else { vec![] }, + provides: if nonce == INVALID_NONCE { vec![] } else { vec![vec![nonce as u8]] }, + longevity: 3, + propagate: true, + }; - futures::future::ready(if nonce < block_number { - Ok(InvalidTransaction::Stale.into()) - } else { - let mut transaction = ValidTransaction { - priority: 4, - requires: if nonce > block_number { vec![vec![nonce as u8 - 1]] } else { vec![] }, - provides: if nonce == INVALID_NONCE { vec![] } else { vec![vec![nonce as u8]] }, - longevity: 3, - propagate: true, - }; + if self.clear_requirements.lock().contains(&hash) { + transaction.requires.clear(); + } - if self.clear_requirements.lock().contains(&hash) { - transaction.requires.clear(); - } + if self.add_requirements.lock().contains(&hash) { + transaction.requires.push(vec![128]); + } - if self.add_requirements.lock().contains(&hash) { - transaction.requires.push(vec![128]); - } + Ok(transaction) + } + }, + Extrinsic::IncludeData(_) => { + Ok(ValidTransaction { + priority: 9001, + requires: vec![], + provides: vec![vec![42]], + longevity: 9001, + propagate: false, + }) + }, + _ => unimplemented!(), + }; - Ok(Ok(transaction)) - }) + futures::future::ready(Ok(res)) } /// Returns a block number given the block id. @@ -579,7 +594,7 @@ mod tests { } fn pool() -> Pool { - Pool::new(Default::default(), TestApi::default().into()) + Pool::new(Default::default(), true.into(), TestApi::default().into()) } #[test] @@ -620,6 +635,26 @@ mod tests { assert_matches!(res.unwrap_err(), error::Error::TemporarilyBanned); } + #[test] + fn should_reject_unactionable_transactions() { + // given + let pool = Pool::new( + Default::default(), + // the node does not author blocks + false.into(), + TestApi::default().into(), + ); + + // after validation `IncludeData` will be set to non-propagable + let uxt = Extrinsic::IncludeData(vec![42]); + + // when + let res = block_on(pool.submit_one(&BlockId::Number(0), SOURCE, uxt)); + + // then + assert_matches!(res.unwrap_err(), error::Error::Unactionable); + } + #[test] fn should_notify_about_pool_events() { let (stream, hash0, hash1) = { @@ -722,11 +757,14 @@ mod tests { count: 100, total_bytes: 200, }; - let pool = Pool::new(Options { + + let options = Options { ready: limit.clone(), future: limit.clone(), ..Default::default() - }, TestApi::default().into()); + }; + + let pool = Pool::new(options, true.into(), TestApi::default().into()); let hash1 = block_on(pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(1)), @@ -757,11 +795,14 @@ mod tests { count: 100, total_bytes: 10, }; - let pool = Pool::new(Options { + + let options = Options { ready: limit.clone(), future: limit.clone(), ..Default::default() - }, TestApi::default().into()); + }; + + let pool = Pool::new(options, true.into(), TestApi::default().into()); // when block_on(pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Transfer { @@ -939,11 +980,13 @@ mod tests { count: 1, total_bytes: 1000, }; - let pool = Pool::new(Options { + let options = Options { ready: limit.clone(), future: limit.clone(), ..Default::default() - }, TestApi::default().into()); + }; + + let pool = Pool::new(options, true.into(), TestApi::default().into()); let xt = uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(1)), @@ -977,7 +1020,7 @@ mod tests { let (tx, rx) = std::sync::mpsc::sync_channel(1); let mut api = TestApi::default(); api.delay = Arc::new(Mutex::new(rx.into())); - let pool = Arc::new(Pool::new(Default::default(), api.into())); + let pool = Arc::new(Pool::new(Default::default(), true.into(), api.into())); // when let xt = uxt(Transfer { diff --git a/substrate/client/transaction-pool/graph/src/validated_pool.rs b/substrate/client/transaction-pool/graph/src/validated_pool.rs index ef68943627..c02aab47d8 100644 --- a/substrate/client/transaction-pool/graph/src/validated_pool.rs +++ b/substrate/client/transaction-pool/graph/src/validated_pool.rs @@ -90,9 +90,25 @@ pub type ValidatedTransactionFor = ValidatedTransaction< ::Error, >; +/// A closure that returns true if the local node is a validator that can author blocks. +pub struct IsValidator(Box bool + Send + Sync>); + +impl From for IsValidator { + fn from(is_validator: bool) -> Self { + IsValidator(Box::new(move || is_validator)) + } +} + +impl From bool + Send + Sync>> for IsValidator { + fn from(is_validator: Box bool + Send + Sync>) -> Self { + IsValidator(is_validator) + } +} + /// Pool that deals with validated transactions. pub struct ValidatedPool { api: Arc, + is_validator: IsValidator, options: Options, listener: RwLock, B>>, pool: RwLock ValidatedPool { /// Create a new transaction pool. - pub fn new(options: Options, api: Arc) -> Self { + pub fn new(options: Options, is_validator: IsValidator, api: Arc) -> Self { let base_pool = base::BasePool::new(options.reject_future_transactions); ValidatedPool { + is_validator, options, listener: Default::default(), api, @@ -183,6 +200,10 @@ impl ValidatedPool { fn submit_one(&self, tx: ValidatedTransactionFor) -> Result, B::Error> { match tx { ValidatedTransaction::Valid(tx) => { + if !tx.propagate && !(self.is_validator.0)() { + return Err(error::Error::Unactionable.into()); + } + let imported = self.pool.write().import(tx)?; if let base::Imported::Ready { ref hash, .. } = imported { diff --git a/substrate/client/transaction-pool/src/lib.rs b/substrate/client/transaction-pool/src/lib.rs index e9a1c3906f..32525065b9 100644 --- a/substrate/client/transaction-pool/src/lib.rs +++ b/substrate/client/transaction-pool/src/lib.rs @@ -163,7 +163,7 @@ impl BasicPool pub fn new_test( pool_api: Arc, ) -> (Self, Pin + Send>>, intervalier::BackSignalControl) { - let pool = Arc::new(sc_transaction_graph::Pool::new(Default::default(), pool_api.clone())); + let pool = Arc::new(sc_transaction_graph::Pool::new(Default::default(), true.into(), pool_api.clone())); let (revalidation_queue, background_task, notifier) = revalidation::RevalidationQueue::new_test(pool_api.clone(), pool.clone()); ( @@ -184,12 +184,13 @@ impl BasicPool /// revalidation type. pub fn with_revalidation_type( options: sc_transaction_graph::Options, + is_validator: txpool::IsValidator, pool_api: Arc, prometheus: Option<&PrometheusRegistry>, revalidation_type: RevalidationType, spawner: impl SpawnNamed, ) -> Self { - let pool = Arc::new(sc_transaction_graph::Pool::new(options, pool_api.clone())); + let pool = Arc::new(sc_transaction_graph::Pool::new(options, is_validator, pool_api.clone())); let (revalidation_queue, background_task) = match revalidation_type { RevalidationType::Light => (revalidation::RevalidationQueue::new(pool_api.clone(), pool.clone()), None), RevalidationType::Full => { @@ -346,7 +347,7 @@ where ) -> Self { let pool_api = Arc::new(LightChainApi::new(client, fetcher)); Self::with_revalidation_type( - options, pool_api, prometheus, RevalidationType::Light, spawner, + options, false.into(), pool_api, prometheus, RevalidationType::Light, spawner, ) } } @@ -364,13 +365,14 @@ where /// Create new basic transaction pool for a full node with the provided api. pub fn new_full( options: sc_transaction_graph::Options, + is_validator: txpool::IsValidator, prometheus: Option<&PrometheusRegistry>, spawner: impl SpawnNamed, client: Arc, ) -> Arc { let pool_api = Arc::new(FullChainApi::new(client.clone(), prometheus)); let pool = Arc::new(Self::with_revalidation_type( - options, pool_api, prometheus, RevalidationType::Full, spawner + options, is_validator, pool_api, prometheus, RevalidationType::Full, spawner )); // make transaction pool available for off-chain runtime calls. diff --git a/substrate/client/transaction-pool/src/revalidation.rs b/substrate/client/transaction-pool/src/revalidation.rs index 69b601484c..fc18b0694d 100644 --- a/substrate/client/transaction-pool/src/revalidation.rs +++ b/substrate/client/transaction-pool/src/revalidation.rs @@ -370,7 +370,7 @@ mod tests { fn setup() -> (Arc, Pool) { let test_api = Arc::new(TestApi::empty()); - let pool = Pool::new(Default::default(), test_api.clone()); + let pool = Pool::new(Default::default(), true.into(), test_api.clone()); (test_api, pool) } diff --git a/substrate/client/transaction-pool/src/testing/pool.rs b/substrate/client/transaction-pool/src/testing/pool.rs index 6e00af4760..a41632ed8d 100644 --- a/substrate/client/transaction-pool/src/testing/pool.rs +++ b/substrate/client/transaction-pool/src/testing/pool.rs @@ -37,7 +37,7 @@ use sc_block_builder::BlockBuilderProvider; use sp_consensus::BlockOrigin; fn pool() -> Pool { - Pool::new(Default::default(), TestApi::with_alice_nonce(209).into()) + Pool::new(Default::default(), true.into(), TestApi::with_alice_nonce(209).into()) } fn maintained_pool() -> ( @@ -161,7 +161,7 @@ fn should_correctly_prune_transactions_providing_more_than_one_tag() { api.set_valid_modifier(Box::new(|v: &mut ValidTransaction| { v.provides.push(vec![155]); })); - let pool = Pool::new(Default::default(), api.clone()); + let pool = Pool::new(Default::default(), true.into(), api.clone()); let xt = uxt(Alice, 209); block_on(pool.submit_one(&BlockId::number(0), SOURCE, xt.clone())).expect("1. Imported"); assert_eq!(pool.validated_pool().status().ready, 1); diff --git a/substrate/primitives/transaction-pool/src/error.rs b/substrate/primitives/transaction-pool/src/error.rs index 62d4a5281c..dd2d6401c1 100644 --- a/substrate/primitives/transaction-pool/src/error.rs +++ b/substrate/primitives/transaction-pool/src/error.rs @@ -60,6 +60,9 @@ pub enum Error { #[error("Transaction couldn't enter the pool because of the limit")] ImmediatelyDropped, + #[error("Transaction cannot be propagated and the local node does not author blocks")] + Unactionable, + #[from(ignore)] #[error("{0}")] InvalidBlockId(String), diff --git a/substrate/utils/frame/rpc/system/src/lib.rs b/substrate/utils/frame/rpc/system/src/lib.rs index db19652507..57c0cda9cc 100644 --- a/substrate/utils/frame/rpc/system/src/lib.rs +++ b/substrate/utils/frame/rpc/system/src/lib.rs @@ -301,6 +301,7 @@ mod tests { let spawner = sp_core::testing::TaskExecutor::new(); let pool = BasicPool::new_full( Default::default(), + true.into(), None, spawner, client.clone(), @@ -340,6 +341,7 @@ mod tests { let spawner = sp_core::testing::TaskExecutor::new(); let pool = BasicPool::new_full( Default::default(), + true.into(), None, spawner, client.clone(), @@ -363,6 +365,7 @@ mod tests { let spawner = sp_core::testing::TaskExecutor::new(); let pool = BasicPool::new_full( Default::default(), + true.into(), None, spawner, client.clone(), @@ -395,6 +398,7 @@ mod tests { let spawner = sp_core::testing::TaskExecutor::new(); let pool = BasicPool::new_full( Default::default(), + true.into(), None, spawner, client.clone(),