From 43fd9a9dc2ba2644f88531b52a196a4f0800c2bf Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Tue, 12 Jul 2022 23:19:13 +1200 Subject: [PATCH] CLI flag to configure tx ban duration (#11786) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * add tx-ban-seconds * fix * trigger CI * trigger CI * remove test print * Update client/cli/src/params/transaction_pool_params.rs Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> --- bin/node/cli/benches/transaction_pool.rs | 3 +++ client/cli/src/commands/run_cmd.rs | 4 ++-- client/cli/src/config.rs | 4 ++-- client/cli/src/params/transaction_pool_params.rs | 14 +++++++++++++- client/transaction-pool/src/graph/pool.rs | 5 ++++- client/transaction-pool/src/graph/rotator.rs | 5 +++++ .../transaction-pool/src/graph/validated_pool.rs | 3 ++- 7 files changed, 31 insertions(+), 7 deletions(-) diff --git a/bin/node/cli/benches/transaction_pool.rs b/bin/node/cli/benches/transaction_pool.rs index f1fce16d8c1b3..5b96355b9ca70 100644 --- a/bin/node/cli/benches/transaction_pool.rs +++ b/bin/node/cli/benches/transaction_pool.rs @@ -16,6 +16,8 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . +use std::time::Duration; + use criterion::{criterion_group, criterion_main, BatchSize, Criterion, Throughput}; use futures::{future, StreamExt}; use node_cli::service::{create_extrinsic, fetch_nonce, FullClient, TransactionPool}; @@ -58,6 +60,7 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase { ready: PoolLimit { count: 100_000, total_bytes: 100 * 1024 * 1024 }, future: PoolLimit { count: 100_000, total_bytes: 100 * 1024 * 1024 }, reject_future_transactions: false, + ban_time: Duration::from_secs(30 * 60), }, network: network_config, keystore: KeystoreConfig::InMemory, diff --git a/client/cli/src/commands/run_cmd.rs b/client/cli/src/commands/run_cmd.rs index b4a3885e39906..12774eecc4174 100644 --- a/client/cli/src/commands/run_cmd.rs +++ b/client/cli/src/commands/run_cmd.rs @@ -480,8 +480,8 @@ impl CliConfiguration for RunCmd { Ok(self.ws_max_out_buffer_capacity) } - fn transaction_pool(&self) -> Result { - Ok(self.pool_config.transaction_pool()) + fn transaction_pool(&self, is_dev: bool) -> Result { + Ok(self.pool_config.transaction_pool(is_dev)) } fn max_runtime_instances(&self) -> Result> { diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index 6e1317c11fbc4..01c541bf75255 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -145,7 +145,7 @@ pub trait CliConfiguration: Sized { /// Get the transaction pool options /// /// By default this is `TransactionPoolOptions::default()`. - fn transaction_pool(&self) -> Result { + fn transaction_pool(&self, _is_dev: bool) -> Result { Ok(Default::default()) } @@ -523,7 +523,7 @@ pub trait CliConfiguration: Sized { impl_name: C::impl_name(), impl_version: C::impl_version(), tokio_handle, - transaction_pool: self.transaction_pool()?, + transaction_pool: self.transaction_pool(is_dev)?, network: self.network_config( &chain_spec, is_dev, diff --git a/client/cli/src/params/transaction_pool_params.rs b/client/cli/src/params/transaction_pool_params.rs index efb78430ced55..6429dfec3f908 100644 --- a/client/cli/src/params/transaction_pool_params.rs +++ b/client/cli/src/params/transaction_pool_params.rs @@ -29,11 +29,15 @@ pub struct TransactionPoolParams { /// Maximum number of kilobytes of all transactions stored in the pool. #[clap(long, value_name = "COUNT", default_value = "20480")] pub pool_kbytes: usize, + + /// How long a transaction is banned for, if it is considered invalid. Defaults to 1800s. + #[clap(long, value_name = "SECONDS")] + pub tx_ban_seconds: Option, } impl TransactionPoolParams { /// Fill the given `PoolConfiguration` by looking at the cli parameters. - pub fn transaction_pool(&self) -> TransactionPoolOptions { + pub fn transaction_pool(&self, is_dev: bool) -> TransactionPoolOptions { let mut opts = TransactionPoolOptions::default(); // ready queue @@ -45,6 +49,14 @@ impl TransactionPoolParams { opts.future.count = self.pool_limit / factor; opts.future.total_bytes = self.pool_kbytes * 1024 / factor; + opts.ban_time = if let Some(ban_seconds) = self.tx_ban_seconds { + std::time::Duration::from_secs(ban_seconds) + } else if is_dev { + std::time::Duration::from_secs(0) + } else { + std::time::Duration::from_secs(30 * 60) + }; + opts } } diff --git a/client/transaction-pool/src/graph/pool.rs b/client/transaction-pool/src/graph/pool.rs index 618ba8ccf24d5..4ce7954f8d479 100644 --- a/client/transaction-pool/src/graph/pool.rs +++ b/client/transaction-pool/src/graph/pool.rs @@ -16,7 +16,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use std::{collections::HashMap, sync::Arc}; +use std::{collections::HashMap, sync::Arc, time::Duration}; use futures::{channel::mpsc::Receiver, Future}; use sc_transaction_pool_api::error; @@ -108,6 +108,8 @@ pub struct Options { pub future: base::Limit, /// Reject future transactions. pub reject_future_transactions: bool, + /// How long the extrinsic is banned for. + pub ban_time: Duration, } impl Default for Options { @@ -116,6 +118,7 @@ impl Default for Options { ready: base::Limit { count: 8192, total_bytes: 20 * 1024 * 1024 }, future: base::Limit { count: 512, total_bytes: 1 * 1024 * 1024 }, reject_future_transactions: false, + ban_time: Duration::from_secs(60 * 30), } } } diff --git a/client/transaction-pool/src/graph/rotator.rs b/client/transaction-pool/src/graph/rotator.rs index b897fe7885033..c91c8e407bc0f 100644 --- a/client/transaction-pool/src/graph/rotator.rs +++ b/client/transaction-pool/src/graph/rotator.rs @@ -51,6 +51,11 @@ impl Default for PoolRotator { } impl PoolRotator { + /// New rotator instance with specified ban time. + pub fn new(ban_time: Duration) -> Self { + Self { ban_time, banned_until: Default::default() } + } + /// Returns `true` if extrinsic hash is currently banned. pub fn is_banned(&self, hash: &Hash) -> bool { self.banned_until.read().contains_key(hash) diff --git a/client/transaction-pool/src/graph/validated_pool.rs b/client/transaction-pool/src/graph/validated_pool.rs index 084b04842ee90..e59c5afbdc512 100644 --- a/client/transaction-pool/src/graph/validated_pool.rs +++ b/client/transaction-pool/src/graph/validated_pool.rs @@ -125,6 +125,7 @@ impl ValidatedPool { /// Create a new transaction pool. pub fn new(options: Options, is_validator: IsValidator, api: Arc) -> Self { let base_pool = base::BasePool::new(options.reject_future_transactions); + let ban_time = options.ban_time; Self { is_validator, options, @@ -132,7 +133,7 @@ impl ValidatedPool { api, pool: RwLock::new(base_pool), import_notification_sinks: Default::default(), - rotator: Default::default(), + rotator: PoolRotator::new(ban_time), } }