From d0ce630c6a6c609aef25803015f6d0506db15739 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Date: Mon, 12 Feb 2024 11:53:00 +0200 Subject: [PATCH] transaction-pool: Improve transaction status documentation and add helpers (#3215) This PR improves the transaction status documentation. - Added doc references for describing the main states - Extra comment wrt pool ready / future queues - `FinalityTimeout` no longer describes a lagging finality gadget, it signals that the maximum number of finality gadgets has been reached A few helper methods are added to indicate when: - a final event is generated by the transaction pool for a given event - a final event is provided, although the transaction might become valid at a later time and could be re-submitted The helper methods are used and taken from https://github.com/paritytech/polkadot-sdk/pull/3079 to help us better keep it in sync. cc @paritytech/subxt-team --------- Signed-off-by: Alexandru Vasile --- .../client/transaction-pool/api/src/error.rs | 24 +++++ .../client/transaction-pool/api/src/lib.rs | 89 +++++++++++++++---- 2 files changed, 97 insertions(+), 16 deletions(-) diff --git a/substrate/client/transaction-pool/api/src/error.rs b/substrate/client/transaction-pool/api/src/error.rs index e521502f66fb1..d0744bfa3e192 100644 --- a/substrate/client/transaction-pool/api/src/error.rs +++ b/substrate/client/transaction-pool/api/src/error.rs @@ -71,6 +71,30 @@ pub enum Error { RejectedFutureTransaction, } +impl Error { + /// Returns true if the transaction could be re-submitted to the pool in the future. + /// + /// For example, `Error::ImmediatelyDropped` is retriable, because the transaction + /// may enter the pool if there is space for it in the future. + pub fn is_retriable(&self) -> bool { + match self { + // An invalid transaction is temporarily banned, however it can + // become valid at a later time. + Error::TemporarilyBanned | + // The pool is full at the moment. + Error::ImmediatelyDropped | + // The block id is not known to the pool. + // The node might be lagging behind, or during a warp sync. + Error::InvalidBlockId(_) | + // The pool is configured to not accept future transactions. + Error::RejectedFutureTransaction => { + true + } + _ => false + } + } +} + /// Transaction pool error conversion. pub trait IntoPoolError: std::error::Error + Send + Sized + Sync { /// Try to extract original `Error` diff --git a/substrate/client/transaction-pool/api/src/lib.rs b/substrate/client/transaction-pool/api/src/lib.rs index a795917528f9c..0a313c5b782d9 100644 --- a/substrate/client/transaction-pool/api/src/lib.rs +++ b/substrate/client/transaction-pool/api/src/lib.rs @@ -62,20 +62,27 @@ impl PoolStatus { /// /// The status events can be grouped based on their kinds as: /// 1. Entering/Moving within the pool: -/// - `Future` -/// - `Ready` +/// - [Future](TransactionStatus::Future) +/// - [Ready](TransactionStatus::Ready) /// 2. Inside `Ready` queue: -/// - `Broadcast` +/// - [Broadcast](TransactionStatus::Broadcast) /// 3. Leaving the pool: -/// - `InBlock` -/// - `Invalid` -/// - `Usurped` -/// - `Dropped` +/// - [InBlock](TransactionStatus::InBlock) +/// - [Invalid](TransactionStatus::Invalid) +/// - [Usurped](TransactionStatus::Usurped) +/// - [Dropped](TransactionStatus::Dropped) /// 4. Re-entering the pool: -/// - `Retracted` +/// - [Retracted](TransactionStatus::Retracted) /// 5. Block finalized: -/// - `Finalized` -/// - `FinalityTimeout` +/// - [Finalized](TransactionStatus::Finalized) +/// - [FinalityTimeout](TransactionStatus::FinalityTimeout) +/// +/// Transactions are first placed in either the `Ready` or `Future` queues of the transaction pool. +/// Substrate validates the transaction before it enters the pool. +/// +/// A transaction is placed in the `Future` queue if it will become valid at a future time. +/// For example, submitting a transaction with a higher account nonce than the current +/// expected nonce will place the transaction in the `Future` queue. /// /// The events will always be received in the order described above, however /// there might be cases where transactions alternate between `Future` and `Ready` @@ -88,19 +95,37 @@ impl PoolStatus { /// 1. Due to possible forks, the transaction that ends up being in included /// in one block, may later re-enter the pool or be marked as invalid. /// 2. Transaction `Dropped` at one point, may later re-enter the pool if some other -/// transactions are removed. +/// transactions are removed. A `Dropped` transaction may re-enter the pool only if it is +/// resubmitted. /// 3. `Invalid` transaction may become valid at some point in the future. /// (Note that runtimes are encouraged to use `UnknownValidity` to inform the pool about -/// such case). +/// such case). An `Invalid` transaction may re-enter the pool only if it is resubmitted. /// 4. `Retracted` transactions might be included in some next block. /// -/// The stream is considered finished only when either `Finalized` or `FinalityTimeout` -/// event is triggered. You are however free to unsubscribe from notifications at any point. -/// The first one will be emitted when the block, in which transaction was included gets -/// finalized. The `FinalityTimeout` event will be emitted when the block did not reach finality +/// The `FinalityTimeout` event will be emitted when the block did not reach finality /// within 512 blocks. This either indicates that finality is not available for your chain, /// or that finality gadget is lagging behind. If you choose to wait for finality longer, you can /// re-subscribe for a particular transaction hash manually again. +/// +/// ### Last Event +/// +/// The stream is considered finished when one of the following events happen: +/// - [Finalized](TransactionStatus::Finalized) +/// - [FinalityTimeout](TransactionStatus::FinalityTimeout) +/// - [Usurped](TransactionStatus::Usurped) +/// - [Invalid](TransactionStatus::Invalid) +/// - [Dropped](TransactionStatus::Dropped) +/// +/// See [`TransactionStatus::is_final`] for more details. +/// +/// ### Resubmit Transactions +/// +/// Users might resubmit the transaction at a later time for the following events: +/// - [FinalityTimeout](TransactionStatus::FinalityTimeout) +/// - [Invalid](TransactionStatus::Invalid) +/// - [Dropped](TransactionStatus::Dropped) +/// +/// See [`TransactionStatus::is_retriable`] for more details. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub enum TransactionStatus { @@ -131,6 +156,38 @@ pub enum TransactionStatus { Invalid, } +impl TransactionStatus { + /// Returns true if this is the last event emitted by [`TransactionStatusStream`]. + pub fn is_final(&self) -> bool { + // The state must be kept in sync with `crate::graph::Sender`. + match self { + Self::Usurped(_) | + Self::Finalized(_) | + Self::FinalityTimeout(_) | + Self::Invalid | + Self::Dropped => true, + _ => false, + } + } + + /// Returns true if the transaction could be re-submitted to the pool in the future. + /// + /// For example, `TransactionStatus::Dropped` is retriable, because the transaction + /// may enter the pool if there is space for it in the future. + pub fn is_retriable(&self) -> bool { + match self { + // The number of finality watchers has been reached. + Self::FinalityTimeout(_) | + // An invalid transaction might be valid at a later time. + Self::Invalid | + // The transaction was dropped because of the limits of the pool. + // It can reenter the pool when other transactions are removed / finalized. + Self::Dropped => true, + _ => false, + } + } +} + /// The stream of transaction events. pub type TransactionStatusStream = dyn Stream> + Send;