diff --git a/substrate/client/rpc-api/src/author/mod.rs b/substrate/client/rpc-api/src/author/mod.rs index c2fbe229c1..8d2b51faf8 100644 --- a/substrate/client/rpc-api/src/author/mod.rs +++ b/substrate/client/rpc-api/src/author/mod.rs @@ -60,6 +60,9 @@ pub trait AuthorApi { ) -> Result>; /// Submit an extrinsic to watch. + /// + /// See [`TransactionStatus`](sp_transaction_pool::TransactionStatus) for details on transaction + /// lifecycle. #[pubsub( subscription = "author_extrinsicUpdate", subscribe, diff --git a/substrate/client/rpc-api/src/system/mod.rs b/substrate/client/rpc-api/src/system/mod.rs index 29a92e16b6..22c1b3bb2a 100644 --- a/substrate/client/rpc-api/src/system/mod.rs +++ b/substrate/client/rpc-api/src/system/mod.rs @@ -22,9 +22,8 @@ pub mod helpers; use crate::helpers::Receiver; use jsonrpc_derive::rpc; use futures::{future::BoxFuture, compat::Compat}; -use std::pin::Pin; -use self::error::{Error, Result}; +use self::error::Result as SystemResult; pub use self::helpers::{Properties, SystemInfo, Health, PeerInfo, NodeRole}; pub use self::gen_client::Client as SystemClient; @@ -34,19 +33,19 @@ pub use self::gen_client::Client as SystemClient; pub trait SystemApi { /// Get the node's implementation name. Plain old string. #[rpc(name = "system_name")] - fn system_name(&self) -> Result; + fn system_name(&self) -> SystemResult; /// Get the node implementation's version. Should be a semver string. #[rpc(name = "system_version")] - fn system_version(&self) -> Result; + fn system_version(&self) -> SystemResult; /// Get the chain's type. Given as a string identifier. #[rpc(name = "system_chain")] - fn system_chain(&self) -> Result; + fn system_chain(&self) -> SystemResult; /// Get a custom set of properties as a JSON object, defined in the chain spec. #[rpc(name = "system_properties")] - fn system_properties(&self) -> Result; + fn system_properties(&self) -> SystemResult; /// Return health status of the node. /// @@ -74,13 +73,13 @@ pub trait SystemApi { /// is an example of a valid, passing multiaddr with PeerId attached. #[rpc(name = "system_addReservedPeer", returns = "()")] fn system_add_reserved_peer(&self, peer: String) - -> Compat>>; + -> Compat>>; /// Remove a reserved peer. Returns the empty string or an error. The string /// should encode only the PeerId e.g. `QmSk5HQbn6LhUwDiNMseVUjuRYhEtYj4aUZ6WfWoGURpdV`. #[rpc(name = "system_removeReservedPeer", returns = "()")] fn system_remove_reserved_peer(&self, peer_id: String) - -> Compat>>; + -> Compat>>; /// Returns the roles the node is running as. #[rpc(name = "system_nodeRoles", returns = "Vec")] diff --git a/substrate/client/transaction-pool/graph/src/listener.rs b/substrate/client/transaction-pool/graph/src/listener.rs index 865255c9c7..a6d65a93ce 100644 --- a/substrate/client/transaction-pool/graph/src/listener.rs +++ b/substrate/client/transaction-pool/graph/src/listener.rs @@ -103,6 +103,6 @@ impl Listene /// Transaction was pruned from the pool. pub fn pruned(&mut self, header_hash: H2, tx: &H) { debug!(target: "txpool", "[{:?}] Pruned at {:?}", tx, header_hash); - self.fire(tx, |watcher| watcher.finalized(header_hash)) + self.fire(tx, |watcher| watcher.in_block(header_hash)) } } diff --git a/substrate/client/transaction-pool/graph/src/pool.rs b/substrate/client/transaction-pool/graph/src/pool.rs index bb5f59ef87..4e4224695e 100644 --- a/substrate/client/transaction-pool/graph/src/pool.rs +++ b/substrate/client/transaction-pool/graph/src/pool.rs @@ -806,7 +806,7 @@ mod tests { // then let mut stream = futures::executor::block_on_stream(watcher.into_stream()); assert_eq!(stream.next(), Some(TransactionStatus::Ready)); - assert_eq!(stream.next(), Some(TransactionStatus::Finalized(H256::from_low_u64_be(2).into()))); + assert_eq!(stream.next(), Some(TransactionStatus::InBlock(H256::from_low_u64_be(2).into()))); assert_eq!(stream.next(), None); } @@ -831,7 +831,7 @@ mod tests { // then let mut stream = futures::executor::block_on_stream(watcher.into_stream()); assert_eq!(stream.next(), Some(TransactionStatus::Ready)); - assert_eq!(stream.next(), Some(TransactionStatus::Finalized(H256::from_low_u64_be(2).into()))); + assert_eq!(stream.next(), Some(TransactionStatus::InBlock(H256::from_low_u64_be(2).into()))); assert_eq!(stream.next(), None); } diff --git a/substrate/client/transaction-pool/graph/src/watcher.rs b/substrate/client/transaction-pool/graph/src/watcher.rs index f222c8b621..ded849e380 100644 --- a/substrate/client/transaction-pool/graph/src/watcher.rs +++ b/substrate/client/transaction-pool/graph/src/watcher.rs @@ -84,12 +84,13 @@ impl Sender { /// Some state change (perhaps another extrinsic was included) rendered this extrinsic invalid. pub fn usurped(&mut self, hash: H) { - self.send(TransactionStatus::Usurped(hash)) + self.send(TransactionStatus::Usurped(hash)); + self.finalized = true; } - /// Extrinsic has been finalized in block with given hash. - pub fn finalized(&mut self, hash: H2) { - self.send(TransactionStatus::Finalized(hash)); + /// Extrinsic has been included in block with given hash. + pub fn in_block(&mut self, hash: H2) { + self.send(TransactionStatus::InBlock(hash)); self.finalized = true; } @@ -103,6 +104,7 @@ impl Sender { /// Transaction has been dropped from the pool because of the limit. pub fn dropped(&mut self) { self.send(TransactionStatus::Dropped); + self.finalized = true; } /// The extrinsic has been broadcast to the given peers. diff --git a/substrate/primitives/transaction-pool/src/pool.rs b/substrate/primitives/transaction-pool/src/pool.rs index 009b9c7863..52a3282f87 100644 --- a/substrate/primitives/transaction-pool/src/pool.rs +++ b/substrate/primitives/transaction-pool/src/pool.rs @@ -55,6 +55,38 @@ impl PoolStatus { } /// Possible transaction status events. +/// +/// This events are being emitted by `TransactionPool` watchers, +/// which are also exposed over RPC. +/// +/// The status events can be grouped based on their kinds as: +/// 1. Entering/Moving within the pool: +/// - `Future` +/// - `Ready` +/// 2. Inside `Ready` queue: +/// - `Broadcast` +/// 3. Leaving the pool: +/// - `InBlock` +/// - `Invalid` +/// - `Usurped` +/// - `Dropped` +/// +/// The events will always be received in the order described above, however +/// there might be cases where transactions alternate between `Future` and `Ready` +/// pool, and are `Broadcast` in the meantime. +/// +/// There is also only single event causing the transaction to leave the pool. +/// +/// Note that there are conditions that may cause transactions to reappear in the pool. +/// 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. +/// 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). +/// +/// However the user needs to re-subscribe to receive such notifications. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub enum TransactionStatus { @@ -62,15 +94,17 @@ pub enum TransactionStatus { Future, /// Transaction is part of the ready queue. Ready, - /// Transaction has been finalized in block with given hash. - Finalized(BlockHash), - /// Some state change (perhaps another transaction was included) rendered this transaction invalid. - Usurped(Hash), /// The transaction has been broadcast to the given peers. Broadcast(Vec), + /// Transaction has been included in block with given hash. + #[serde(rename = "finalized")] // See #4438 + InBlock(BlockHash), + /// Transaction has been replaced in the pool, by another transaction + /// that provides the same tags. (e.g. same (sender, nonce)). + Usurped(Hash), /// Transaction has been dropped from the pool because of the limit. Dropped, - /// Transaction was detected as invalid. + /// Transaction is no longer valid in the current state. Invalid, }