CLI flag to configure tx ban duration (#11786)

* 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>

substrate/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 <https://www.gnu.org/licenses/>.
 
+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,

substrate/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<TransactionPoolOptions> {
-		Ok(self.pool_config.transaction_pool())
+	fn transaction_pool(&self, is_dev: bool) -> Result<TransactionPoolOptions> {
+		Ok(self.pool_config.transaction_pool(is_dev))
 	}
 
 	fn max_runtime_instances(&self) -> Result<Option<usize>> {

substrate/client/cli/src/config.rs

@@ -145,7 +145,7 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 	/// Get the transaction pool options
 	///
 	/// By default this is `TransactionPoolOptions::default()`.
-	fn transaction_pool(&self) -> Result<TransactionPoolOptions> {
+	fn transaction_pool(&self, _is_dev: bool) -> Result<TransactionPoolOptions> {
 		Ok(Default::default())
 	}
 
@@ -523,7 +523,7 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: 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,

substrate/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<u64>,
 }
 
 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
 	}
 }

substrate/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 <https://www.gnu.org/licenses/>.
 
-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),
 		}
 	}
 }

substrate/client/transaction-pool/src/graph/rotator.rs

@@ -51,6 +51,11 @@ impl<Hash: hash::Hash + Eq> Default for PoolRotator<Hash> {
 }
 
 impl<Hash: hash::Hash + Eq + Clone> PoolRotator<Hash> {
+	/// 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)

substrate/client/transaction-pool/src/graph/validated_pool.rs

@@ -125,6 +125,7 @@ impl<B: ChainApi> ValidatedPool<B> {
 	/// Create a new transaction pool.
 	pub fn new(options: Options, is_validator: IsValidator, api: Arc<B>) -> 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<B: ChainApi> ValidatedPool<B> {
 			api,
 			pool: RwLock::new(base_pool),
 			import_notification_sinks: Default::default(),
-			rotator: Default::default(),
+			rotator: PoolRotator::new(ban_time),
 		}
 	}