From 6d7ccff53e28bd6d4d1e2c2a364c79f2a988ff9a Mon Sep 17 00:00:00 2001
From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
Date: Tue, 11 Feb 2025 16:09:20 +0100
Subject: [PATCH] review comments
---
.../src/fork_aware_txpool/dropped_watcher.rs | 7 ++-
.../src/fork_aware_txpool/metrics.rs | 7 ++-
.../fork_aware_txpool/multi_view_listener.rs | 46 +++++++++----------
3 files changed, 34 insertions(+), 26 deletions(-)
diff --git a/substrate/client/transaction-pool/src/fork_aware_txpool/dropped_watcher.rs b/substrate/client/transaction-pool/src/fork_aware_txpool/dropped_watcher.rs
index 9e078617635..e04c826a1d5 100644
--- a/substrate/client/transaction-pool/src/fork_aware_txpool/dropped_watcher.rs
+++ b/substrate/client/transaction-pool/src/fork_aware_txpool/dropped_watcher.rs
@@ -262,7 +262,12 @@ where
let (tx_hash, status) = event;
match status {
TransactionStatus::Future => {
- self.future_transaction_views.entry(tx_hash).or_default().insert(block_hash);
+ // see note below:
+ if let Some(mut views_keeping_tx_valid) = self.transaction_views(tx_hash) {
+ views_keeping_tx_valid.get_mut().insert(block_hash);
+ } else {
+ self.future_transaction_views.entry(tx_hash).or_default().insert(block_hash);
+ }
},
TransactionStatus::Ready | TransactionStatus::InBlock(..) => {
// note: if future transaction was once seen as the ready we may want to treat it
diff --git a/substrate/client/transaction-pool/src/fork_aware_txpool/metrics.rs b/substrate/client/transaction-pool/src/fork_aware_txpool/metrics.rs
index 6b8d579c693..c99fb4d67c5 100644
--- a/substrate/client/transaction-pool/src/fork_aware_txpool/metrics.rs
+++ b/substrate/client/transaction-pool/src/fork_aware_txpool/metrics.rs
@@ -23,6 +23,8 @@ use prometheus_endpoint::{
histogram_opts, linear_buckets, register, Counter, Gauge, Histogram, PrometheusError, Registry,
U64,
};
+#[cfg(doc)]
+use sc_transaction_pool_api::TransactionPool;
/// A helper alias for the Prometheus's metrics endpoint.
pub type MetricsLink = GenericMetricsLink<Metrics>;
@@ -40,6 +42,9 @@ pub struct Metrics {
/// Total number of unwatched transactions in txpool.
pub unwatched_txs: Gauge<U64>,
/// Total number of transactions reported as invalid.
+ ///
+ /// This only includes transaction reported as invalid by the
+ /// [`TransactionPool::report_invalid`] method.
pub reported_invalid_txs: Counter<U64>,
/// Total number of transactions removed as invalid.
pub removed_invalid_txs: Counter<U64>,
@@ -104,7 +109,7 @@ impl MetricsRegistrant for Metrics {
reported_invalid_txs: register(
Counter::new(
"substrate_sub_txpool_reported_invalid_txs_total",
- "Total number of transactions reported as invalid.",
+ "Total number of transactions reported as invalid by external entities using TxPool API.",
)?,
registry,
)?,
diff --git a/substrate/client/transaction-pool/src/fork_aware_txpool/multi_view_listener.rs b/substrate/client/transaction-pool/src/fork_aware_txpool/multi_view_listener.rs
index bf91a109bfd..a22f3fff3a6 100644
--- a/substrate/client/transaction-pool/src/fork_aware_txpool/multi_view_listener.rs
+++ b/substrate/client/transaction-pool/src/fork_aware_txpool/multi_view_listener.rs
@@ -112,6 +112,25 @@ where
}
}
}
+impl<ChainApi> Into<TransactionStatus<ExtrinsicHash<ChainApi>, BlockHash<ChainApi>>>
+ for TransactionStatusUpdate<ChainApi>
+where
+ ChainApi: graph::ChainApi,
+{
+ fn into(self) -> TransactionStatus<ExtrinsicHash<ChainApi>, BlockHash<ChainApi>> {
+ match self {
+ Self::TransactionInvalidated(..) => TransactionStatus::Invalid,
+ Self::TransactionFinalized(_, block, index) =>
+ TransactionStatus::Finalized((block, index)),
+ Self::TransactionBroadcasted(_, peers) => TransactionStatus::Broadcast(peers),
+ Self::TransactionDropped(_, DroppedReason::LimitsEnforced) =>
+ TransactionStatus::Dropped,
+ Self::TransactionDropped(_, DroppedReason::Usurped(by)) =>
+ TransactionStatus::Usurped(by),
+ Self::TransactionDropped(_, DroppedReason::Invalid) => TransactionStatus::Invalid,
+ }
+ }
+}
impl<ChainApi> std::fmt::Debug for TransactionStatusUpdate<ChainApi>
where
@@ -288,30 +307,9 @@ where
&mut self,
request: TransactionStatusUpdate<ChainApi>,
) -> Option<TransactionStatus<ExtrinsicHash<ChainApi>, BlockHash<ChainApi>>> {
- match request {
- TransactionStatusUpdate::TransactionInvalidated(..) => {
- self.terminate = true;
- return Some(TransactionStatus::Invalid)
- },
- TransactionStatusUpdate::TransactionFinalized(_, block, index) => {
- self.terminate = true;
- return Some(TransactionStatus::Finalized((block, index)))
- },
- TransactionStatusUpdate::TransactionBroadcasted(_, peers) =>
- return Some(TransactionStatus::Broadcast(peers)),
- TransactionStatusUpdate::TransactionDropped(_, DroppedReason::LimitsEnforced) => {
- self.terminate = true;
- return Some(TransactionStatus::Dropped)
- },
- TransactionStatusUpdate::TransactionDropped(_, DroppedReason::Usurped(by)) => {
- self.terminate = true;
- return Some(TransactionStatus::Usurped(by))
- },
- TransactionStatusUpdate::TransactionDropped(_, DroppedReason::Invalid) => {
- self.terminate = true;
- return Some(TransactionStatus::Invalid)
- },
- };
+ let status = Into::<TransactionStatus<_, _>>::into(request);
+ status.is_final().then(|| self.terminate = true);
+ return Some(status);
}
/// Handles various transaction status updates from individual views and manages internal states
--
GitLab