diff --git a/prdoc/pr_7777.prdoc b/prdoc/pr_7777.prdoc
new file mode 100644
index 0000000000000000000000000000000000000000..e62abf1bf6fdfb7ed9e7d9c457db6eda74ada4bd
--- /dev/null
+++ b/prdoc/pr_7777.prdoc
@@ -0,0 +1,10 @@
+title: '`fatxpool`: report_invalid: do not ban Future/Stale txs from re-entering the
+ view'
+doc:
+- audience: Node Dev
+ description: |-
+ Avoid banning future/stale transactions reported as invalid by the authorship module.
+
+crates:
+- name: sc-transaction-pool
+ bump: minor
diff --git a/substrate/client/transaction-pool/src/common/tracing_log_xt.rs b/substrate/client/transaction-pool/src/common/tracing_log_xt.rs
index 9be9cddd978fd1ab4721a4de48cad3452f670421..ef13642bbb9038381214ac8650b6a0eabe13772f 100644
--- a/substrate/client/transaction-pool/src/common/tracing_log_xt.rs
+++ b/substrate/client/transaction-pool/src/common/tracing_log_xt.rs
@@ -53,6 +53,18 @@ macro_rules! log_xt {
}
};
}
+macro_rules! log_xt_debug {
+ (data: $datatype:ident, target: $target:expr, $($arg:tt)+) => {
+ $crate::common::tracing_log_xt::log_xt!(data: $datatype, target: $target, tracing::Level::DEBUG, $($arg)+);
+ };
+ (target: $target:expr, $tx_collection:expr, $text_with_format:expr) => {
+ $crate::common::tracing_log_xt::log_xt!(data: hash, target: $target, tracing::Level::DEBUG, $tx_collection, $text_with_format);
+ };
+ (target: $target:expr, $tx_collection:expr, $text_with_format:expr, $($arg:expr)*) => {
+ $crate::common::tracing_log_xt::log_xt!(data: hash, target: $target, tracing::Level::DEBUG, $tx_collection, $text_with_format, $($arg)*);
+ };
+}
+
macro_rules! log_xt_trace {
(data: $datatype:ident, target: $target:expr, $($arg:tt)+) => {
$crate::common::tracing_log_xt::log_xt!(data: $datatype, target: $target, tracing::Level::TRACE, $($arg)+);
@@ -66,4 +78,5 @@ macro_rules! log_xt_trace {
}
pub(crate) use log_xt;
+pub(crate) use log_xt_debug;
pub(crate) use log_xt_trace;
diff --git a/substrate/client/transaction-pool/src/fork_aware_txpool/fork_aware_txpool.rs b/substrate/client/transaction-pool/src/fork_aware_txpool/fork_aware_txpool.rs
index 337da1dd61d4f9b3440d4f45d4672c6aea7846db..855e9b55e8564f644f2e9a6404a87553050fd336 100644
--- a/substrate/client/transaction-pool/src/fork_aware_txpool/fork_aware_txpool.rs
+++ b/substrate/client/transaction-pool/src/fork_aware_txpool/fork_aware_txpool.rs
@@ -29,7 +29,7 @@ use super::{
};
use crate::{
api::FullChainApi,
- common::tracing_log_xt::log_xt_trace,
+ common::tracing_log_xt::{log_xt_debug, log_xt_trace},
enactment_state::{EnactmentAction, EnactmentState},
fork_aware_txpool::{
dropped_watcher::{DroppedReason, DroppedTransaction},
@@ -891,7 +891,7 @@ where
invalid_tx_errors: TxInvalidityReportMap<TxHash<Self>>,
) -> Vec<Arc<Self::InPoolTransaction>> {
debug!(target: LOG_TARGET, len = ?invalid_tx_errors.len(), "fatp::report_invalid");
- log_xt_trace!(data: tuple, target:LOG_TARGET, invalid_tx_errors.iter(), "fatp::report_invalid {:?}");
+ log_xt_debug!(data: tuple, target:LOG_TARGET, invalid_tx_errors.iter(), "fatp::report_invalid {:?}");
self.metrics
.report(|metrics| metrics.reported_invalid_txs.inc_by(invalid_tx_errors.len() as _));
diff --git a/substrate/client/transaction-pool/src/fork_aware_txpool/view.rs b/substrate/client/transaction-pool/src/fork_aware_txpool/view.rs
index d496b24fb4e7385132336cb3d01deb128502bf59..5c025b22573a9721acf9eb45436f14ae3af0e4c0 100644
--- a/substrate/client/transaction-pool/src/fork_aware_txpool/view.rs
+++ b/substrate/client/transaction-pool/src/fork_aware_txpool/view.rs
@@ -656,6 +656,7 @@ where
pub fn remove_subtree<F>(
&self,
hashes: &[ExtrinsicHash<ChainApi>],
+ ban_transactions: bool,
listener_action: F,
) -> Vec<TransactionFor<ChainApi>>
where
@@ -664,6 +665,8 @@ where
ExtrinsicHash<ChainApi>,
),
{
- self.pool.validated_pool().remove_subtree(hashes, listener_action)
+ self.pool
+ .validated_pool()
+ .remove_subtree(hashes, ban_transactions, listener_action)
}
}
diff --git a/substrate/client/transaction-pool/src/fork_aware_txpool/view_store.rs b/substrate/client/transaction-pool/src/fork_aware_txpool/view_store.rs
index f61e300f36462dc87f1155944acaf6dd890633e6..3ee81d88b9c3815729bdf5666fd8dc5b08011aac 100644
--- a/substrate/client/transaction-pool/src/fork_aware_txpool/view_store.rs
+++ b/substrate/client/transaction-pool/src/fork_aware_txpool/view_store.rs
@@ -647,8 +647,7 @@ where
///
/// Invalid future and stale transaction will be removed only from given `at` view, and will be
/// kept in the view_store. Such transaction will not be reported in returned vector. They
- /// also will not be banned from re-entering the pool (however can be rejected from re-entring
- /// the view). No event will be triggered.
+ /// also will not be banned from re-entering the pool. No event will be triggered.
///
/// For other errors, the transaction will be removed from the view_store, and it will be
/// included in the returned vector. Additionally, transactions provided as input will be banned
@@ -685,7 +684,7 @@ where
// be in the pool.
at.map(|at| {
self.get_view_at(at, true)
- .map(|(view, _)| view.remove_subtree(&remove_from_view, |_, _| {}))
+ .map(|(view, _)| view.remove_subtree(&remove_from_view, false, |_, _| {}))
});
let mut removed = vec![];
@@ -757,7 +756,7 @@ where
));
},
PreInsertAction::RemoveSubtree(ref removal) => {
- view.remove_subtree(&[removal.xt_hash], &*removal.listener_action);
+ view.remove_subtree(&[removal.xt_hash], true, &*removal.listener_action);
},
}
}
@@ -862,7 +861,7 @@ where
.iter()
.chain(self.inactive_views.read().iter())
.filter(|(_, view)| view.is_imported(&xt_hash))
- .flat_map(|(_, view)| view.remove_subtree(&[xt_hash], &listener_action))
+ .flat_map(|(_, view)| view.remove_subtree(&[xt_hash], true, &listener_action))
.filter_map(|xt| seen.insert(xt.hash).then(|| xt.clone()))
.collect();
diff --git a/substrate/client/transaction-pool/src/graph/validated_pool.rs b/substrate/client/transaction-pool/src/graph/validated_pool.rs
index 8eb967421bd70e00529758a6de936e360fb7bafc..281dc32e57c4cb9f1fbb84fb772c99c8d9bc1ec2 100644
--- a/substrate/client/transaction-pool/src/graph/validated_pool.rs
+++ b/substrate/client/transaction-pool/src/graph/validated_pool.rs
@@ -734,7 +734,7 @@ impl<B: ChainApi, L: EventHandler<B>> ValidatedPool<B, L> {
return vec![]
}
- let invalid = self.remove_subtree(hashes, |listener, removed_tx_hash| {
+ let invalid = self.remove_subtree(hashes, true, |listener, removed_tx_hash| {
listener.invalid(&removed_tx_hash);
});
@@ -795,8 +795,8 @@ impl<B: ChainApi, L: EventHandler<B>> ValidatedPool<B, L> {
/// This function traverses the dependency graph of transactions and removes the specified
/// transaction along with all its descendant transactions from the pool.
///
- /// The root transaction will be banned from re-entrering the pool. Descendant transactions may
- /// be re-submitted to the pool if required.
+ /// The root transactions will be banned from re-entrering the pool if `ban_transactions` is
+ /// true. Descendant transactions may be re-submitted to the pool if required.
///
/// A `event_disaptcher_action` callback function is invoked for every transaction that is
/// removed, providing a reference to the pool's event dispatcher and the hash of the removed
@@ -807,13 +807,16 @@ impl<B: ChainApi, L: EventHandler<B>> ValidatedPool<B, L> {
pub fn remove_subtree<F>(
&self,
hashes: &[ExtrinsicHash<B>],
+ ban_transactions: bool,
event_dispatcher_action: F,
) -> Vec<TransactionFor<B>>
where
F: Fn(&mut EventDispatcher<B, L>, ExtrinsicHash<B>),
{
- // temporarily ban removed transactions
- self.rotator.ban(&Instant::now(), hashes.iter().cloned());
+ // temporarily ban removed transactions if requested
+ if ban_transactions {
+ self.rotator.ban(&Instant::now(), hashes.iter().cloned());
+ };
let removed = self.pool.write().remove_subtree(hashes);
removed