From 5f869fa5791ad1886e349a6f87ede63009252eff Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 29 Sep 2022 19:56:11 +0200 Subject: [PATCH 01/35] feat(txpool): initial sketch --- Cargo.lock | 279 ++++++++++++++++++++++- crates/transaction-pool/Cargo.toml | 25 ++ crates/transaction-pool/src/chain.rs | 10 + crates/transaction-pool/src/config.rs | 12 + crates/transaction-pool/src/error.rs | 8 + crates/transaction-pool/src/lib.rs | 49 ++++ crates/transaction-pool/src/traits.rs | 8 + crates/transaction-pool/src/validate.rs | 33 +++ crates/transaction-pool/tests/it/main.rs | 3 + 9 files changed, 422 insertions(+), 5 deletions(-) create mode 100644 crates/transaction-pool/Cargo.toml create mode 100644 crates/transaction-pool/src/chain.rs create mode 100644 crates/transaction-pool/src/config.rs create mode 100644 crates/transaction-pool/src/error.rs create mode 100644 crates/transaction-pool/src/lib.rs create mode 100644 crates/transaction-pool/src/traits.rs create mode 100644 crates/transaction-pool/src/validate.rs create mode 100644 crates/transaction-pool/tests/it/main.rs diff --git a/Cargo.lock b/Cargo.lock index b4582d4a108..8175a113b91 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -17,6 +17,17 @@ version = "0.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8da52d66c7071e2e3fa2a1e5c6d088fec47b593032b254f5e980de8ea54454d6" +[[package]] +name = "async-trait" +version = "0.1.57" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "76464446b8bc32758d7e88ee1a804d9914cd9b1cb264c029899680b0be29826f" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "auto_impl" version = "1.0.1" @@ -47,6 +58,12 @@ version = "1.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ea2b2456fd614d856680dcd9fcc660a51a820fa09daef2e49772b56a193c8474" +[[package]] +name = "bitflags" +version = "1.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" + [[package]] name = "bitvec" version = "1.0.1" @@ -323,6 +340,95 @@ version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6d5a32815ae3f33302d95fdcb2ce17862f8c65363dcfd29360480ba1001fc9c" +[[package]] +name = "futures" +version = "0.3.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f21eda599937fba36daeb58a22e8f5cee2d14c4a17b5b7739c7c8e5e3b8230c" +dependencies = [ + "futures-channel", + "futures-core", + "futures-executor", + "futures-io", + "futures-sink", + "futures-task", + "futures-util", +] + +[[package]] +name = "futures-channel" +version = "0.3.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "30bdd20c28fadd505d0fd6712cdfcb0d4b5648baf45faef7f852afb2399bb050" +dependencies = [ + "futures-core", + "futures-sink", +] + +[[package]] +name = "futures-core" +version = "0.3.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4e5aa3de05362c3fb88de6531e6296e85cde7739cccad4b9dfeeb7f6ebce56bf" + +[[package]] +name = "futures-executor" +version = "0.3.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9ff63c23854bee61b6e9cd331d523909f238fc7636290b96826e9cfa5faa00ab" +dependencies = [ + "futures-core", + "futures-task", + "futures-util", +] + +[[package]] +name = "futures-io" +version = "0.3.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbf4d2a7a308fd4578637c0b17c7e1c7ba127b8f6ba00b29f717e9655d85eb68" + +[[package]] +name = "futures-macro" +version = "0.3.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "42cd15d1c7456c04dbdf7e88bcd69760d74f3a798d6444e16974b505b0e62f17" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "futures-sink" +version = "0.3.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "21b20ba5a92e727ba30e72834706623d94ac93a725410b6a6b6fbc1b07f7ba56" + +[[package]] +name = "futures-task" +version = "0.3.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a6508c467c73851293f390476d4491cf4d227dbabcd4170f3bb6044959b294f1" + +[[package]] +name = "futures-util" +version = "0.3.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "44fb6cb1be61cc1d2e43b262516aafcf63b241cffdb1d3fa115f91d9c7b09c90" +dependencies = [ + "futures-channel", + "futures-core", + "futures-io", + "futures-macro", + "futures-sink", + "futures-task", + "memchr", + "pin-project-lite", + "pin-utils", + "slab", +] + [[package]] name = "generic-array" version = "0.14.6" @@ -445,6 +551,16 @@ version = "0.2.133" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c0f80d65747a3e43d1596c7c5492d95d5edddaabd45a7fcdb02b95f644164966" +[[package]] +name = "lock_api" +version = "0.4.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "435011366fe56583b16cf956f9df0095b405b82d76425bc8981c0e22e60ec4df" +dependencies = [ + "autocfg", + "scopeguard", +] + [[package]] name = "memchr" version = "2.5.0" @@ -502,6 +618,41 @@ dependencies = [ "syn", ] +[[package]] +name = "parking_lot" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3742b2c103b9f06bc9fff0a37ff4912935851bee6d36f3c02bcc755bcfec228f" +dependencies = [ + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09a279cbf25cb0757810394fbc1e359949b59e348145c643a939a525692e6929" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall", + "smallvec", + "windows-sys", +] + +[[package]] +name = "pin-project-lite" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e0a7ae3ac2f1173085d398531c705756c94a4c56843785df85a60c1a0afac116" + +[[package]] +name = "pin-utils" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" + [[package]] name = "pkcs8" version = "0.9.0" @@ -620,6 +771,15 @@ dependencies = [ "getrandom", ] +[[package]] +name = "redox_syscall" +version = "0.2.16" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fb5a58c1855b4b6819d59012155603f0b22ad30cad752600aadfcb695265519a" +dependencies = [ + "bitflags", +] + [[package]] name = "regex" version = "1.6.0" @@ -646,12 +806,25 @@ name = "reth-crate-template" version = "0.1.0" [[package]] -name = "reth-primtiives" +name = "reth-primitives" version = "0.1.0" dependencies = [ "ethers-core", ] +[[package]] +name = "reth-transaction-pool" +version = "0.1.0" +dependencies = [ + "async-trait", + "futures", + "parking_lot", + "reth-primitives", + "serde", + "thiserror", + "tracing", +] + [[package]] name = "rfc6979" version = "0.3.0" @@ -713,6 +886,12 @@ version = "1.0.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4501abdff3ae82a1c1b477a17252eb69cee9e66eb915c1abaa4f44d873df9f09" +[[package]] +name = "scopeguard" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" + [[package]] name = "sec1" version = "0.3.0" @@ -789,6 +968,21 @@ dependencies = [ "rand_core", ] +[[package]] +name = "slab" +version = "0.4.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4614a76b2a8be0058caa9dbbaf66d988527d86d003c11a94fbd335d7661edcef" +dependencies = [ + "autocfg", +] + +[[package]] +name = "smallvec" +version = "1.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2fd0db749597d91ff862fd1d55ea87f7855a744a8425a64695b6fca237d1dad1" + [[package]] name = "spki" version = "0.6.0" @@ -852,18 +1046,18 @@ checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" [[package]] name = "thiserror" -version = "1.0.36" +version = "1.0.37" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0a99cb8c4b9a8ef0e7907cd3b617cc8dc04d571c4e73c8ae403d80ac160bb122" +checksum = "10deb33631e3c9018b9baf9dcbbc4f737320d2b576bac10f6aefa048fa407e3e" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.36" +version = "1.0.37" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3a891860d3c8d66fec8e73ddb3765f90082374dbaaa833407b904a94f1a7eb43" +checksum = "982d17546b47146b28f7c22e3d08465f6b8903d0ea13c1660d9d84a6e7adcdbb" dependencies = [ "proc-macro2", "quote", @@ -888,6 +1082,38 @@ dependencies = [ "serde", ] +[[package]] +name = "tracing" +version = "0.1.36" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2fce9567bd60a67d08a16488756721ba392f24f29006402881e43b19aac64307" +dependencies = [ + "cfg-if", + "pin-project-lite", + "tracing-attributes", + "tracing-core", +] + +[[package]] +name = "tracing-attributes" +version = "0.1.22" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "11c75893af559bc8e10716548bdef5cb2b983f8e637db9d0e15126b61b484ee2" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "tracing-core" +version = "0.1.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5aeea4303076558a00714b823f9ad67d58a3bbda1df83d8827d21193156e22f7" +dependencies = [ + "once_cell", +] + [[package]] name = "typenum" version = "1.15.0" @@ -930,6 +1156,49 @@ version = "0.11.0+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" +[[package]] +name = "windows-sys" +version = "0.36.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ea04155a16a59f9eab786fe12a4a450e75cdb175f9e0d80da1e17db09f55b8d2" +dependencies = [ + "windows_aarch64_msvc", + "windows_i686_gnu", + "windows_i686_msvc", + "windows_x86_64_gnu", + "windows_x86_64_msvc", +] + +[[package]] +name = "windows_aarch64_msvc" +version = "0.36.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9bb8c3fd39ade2d67e9874ac4f3db21f0d710bee00fe7cab16949ec184eeaa47" + +[[package]] +name = "windows_i686_gnu" +version = "0.36.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "180e6ccf01daf4c426b846dfc66db1fc518f074baa793aa7d9b9aaeffad6a3b6" + +[[package]] +name = "windows_i686_msvc" +version = "0.36.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e2e7917148b2812d1eeafaeb22a97e4813dfa60a3f8f78ebe204bcc88f12f024" + +[[package]] +name = "windows_x86_64_gnu" +version = "0.36.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4dcd171b8776c41b97521e5da127a2d86ad280114807d0b2ab1e462bc764d9e1" + +[[package]] +name = "windows_x86_64_msvc" +version = "0.36.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c811ca4a8c853ef420abd8592ba53ddbbac90410fab6903b3e79972a631f7680" + [[package]] name = "wyz" version = "0.5.0" diff --git a/crates/transaction-pool/Cargo.toml b/crates/transaction-pool/Cargo.toml new file mode 100644 index 00000000000..4369c542f3d --- /dev/null +++ b/crates/transaction-pool/Cargo.toml @@ -0,0 +1,25 @@ +[package] +name = "reth-transaction-pool" +version = "0.1.0" +edition = "2021" +license = "MIT OR Apache-2.0" +repository = "https://github.com/foundry-rs/reth" +readme = "README.md" +description = """ +Transaction pool implementation +""" + +[dependencies] + +# eth +reth-primitives = { path = "../primitives" } + +# async/futures +async-trait = "0.1" +futures = "0.3" +parking_lot = "0.12" + +# misc +thiserror = "1.0" +tracing = "0.1" +serde = { version = "1.0", features = ["derive"] } diff --git a/crates/transaction-pool/src/chain.rs b/crates/transaction-pool/src/chain.rs new file mode 100644 index 00000000000..339222d529a --- /dev/null +++ b/crates/transaction-pool/src/chain.rs @@ -0,0 +1,10 @@ +//! Provides access to the chain's storage + +// TODO probably merge with `Validator` trait? since the validator also needs chain access +#[async_trait::async_trait] +pub trait ChainInfo: Send + Sync { + /// The error type that can be converted to the crate's internal Error + type Error: Into; + + // TODO add functions to fetch Block/Hashes etc... +} diff --git a/crates/transaction-pool/src/config.rs b/crates/transaction-pool/src/config.rs new file mode 100644 index 00000000000..99c69136271 --- /dev/null +++ b/crates/transaction-pool/src/config.rs @@ -0,0 +1,12 @@ +///! Configuration options for the Transaction pool. +#[derive(Debug, Clone)] +pub struct PoolConfig { + // TODO add limits for subpools + // TODO config whether to check if transactions are banned +} + +impl Default for PoolConfig { + fn default() -> Self { + todo!() + } +} diff --git a/crates/transaction-pool/src/error.rs b/crates/transaction-pool/src/error.rs new file mode 100644 index 00000000000..23f9cf72f11 --- /dev/null +++ b/crates/transaction-pool/src/error.rs @@ -0,0 +1,8 @@ +//! Transaction pool errors + +/// Transaction pool result type. +pub type Result = std::result::Result; + +/// All errors the Transaction pool can throw. +#[derive(Debug, thiserror::Error)] +pub enum Error {} diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs new file mode 100644 index 00000000000..c464f565ecd --- /dev/null +++ b/crates/transaction-pool/src/lib.rs @@ -0,0 +1,49 @@ +#![warn(missing_debug_implementations, missing_docs, unreachable_pub)] +#![deny(unused_must_use, rust_2018_idioms)] +#![doc(test( + no_crate_inject, + attr(deny(warnings, rust_2018_idioms), allow(dead_code, unused_variables)) +))] + +//! reth's transaction pool implementation + +pub mod error; + +mod chain; +mod validate; + +pub use chain::ChainInfo; +use std::sync::Arc; + +mod config; +use crate::traits::TransactionPool; +pub use config::PoolConfig; + +mod traits; + +/// A generic `TransactionPool` implementation. +pub struct Pool { + // TODO rm later + _marker: std::marker::PhantomData<(ChainApi, Transaction)>, +} + +// === impl Pool === + +impl Pool +where + ChainApi: ChainInfo, +{ + /// Creates a new `Pool` with the given config and chain api + pub fn new(config: PoolConfig, api: Arc) -> Self { + unimplemented!() + } +} + +/// implements the `TransactionPool` interface for the `Poll`. +impl TransactionPool for Pool +where + ChainApi: ChainInfo, + // TODO this could be unified by moving it ChaiApi trait + Transaction: Send + Sync, +{ +} diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs new file mode 100644 index 00000000000..a628dac847d --- /dev/null +++ b/crates/transaction-pool/src/traits.rs @@ -0,0 +1,8 @@ +/// General purpose abstraction fo a transaction-pool +#[async_trait::async_trait] +pub trait TransactionPool: Send + Sync { + // TODO probably need associated `Transaction` type here + // TODO needs transaction type + + // TODO add interfaces for adding new transactions +} diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs new file mode 100644 index 00000000000..d1484c68528 --- /dev/null +++ b/crates/transaction-pool/src/validate.rs @@ -0,0 +1,33 @@ +//! Transaction validation abstractions. + +/// Result returned after checking a transaction's validity +type TransactionValidationResult = Result<(), TransactionValidationError>; + +/// Provides support for validating transaction at any given state of the chain +#[async_trait::async_trait] +pub trait TransactionValidator { + /// The transaction type to validate + type Transaction: Send + Sync; + + /// Validates the transaction and returns a validated outcome + /// + /// This is used by the transaction-pool check the transaction's validity against the state of + /// the given block hash. + async fn validate_transaction( + &self, + transaction: Self::Transaction, + block_hash: (), + ) -> TransactionValidationResult { + unimplemented!() + } +} + +/// Errors thrown during validity checks of a transaction +#[derive(Clone, PartialEq, Eq)] +pub enum TransactionValidationError { + /// The transaction is considered invalid. + /// + /// Note: This does not indicate whether the transaction will not be valid in the future + Invalid(Transaction), + // TODO need variants for `Never`, or `At`? +} diff --git a/crates/transaction-pool/tests/it/main.rs b/crates/transaction-pool/tests/it/main.rs new file mode 100644 index 00000000000..1a707e61550 --- /dev/null +++ b/crates/transaction-pool/tests/it/main.rs @@ -0,0 +1,3 @@ +//! transaction-pool integration tests + +fn main() {} From 91efdc2f119bfd5a95cdf6f6feba8b65abe8db4b Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Fri, 30 Sep 2022 15:43:31 +0200 Subject: [PATCH 02/35] more abstractions --- Cargo.lock | 7 +++ crates/transaction-pool/Cargo.toml | 1 + crates/transaction-pool/src/chain.rs | 10 ---- crates/transaction-pool/src/client.rs | 22 ++++++++ crates/transaction-pool/src/lib.rs | 40 +++++++++------ crates/transaction-pool/src/ordering.rs | 4 ++ crates/transaction-pool/src/pool/events.rs | 21 ++++++++ crates/transaction-pool/src/pool/listener.rs | 18 +++++++ crates/transaction-pool/src/pool/mod.rs | 53 ++++++++++++++++++++ crates/transaction-pool/src/pool/pending.rs | 1 + crates/transaction-pool/src/pool/queued.rs | 1 + crates/transaction-pool/src/validate.rs | 25 ++++++++- 12 files changed, 177 insertions(+), 26 deletions(-) delete mode 100644 crates/transaction-pool/src/chain.rs create mode 100644 crates/transaction-pool/src/client.rs create mode 100644 crates/transaction-pool/src/ordering.rs create mode 100644 crates/transaction-pool/src/pool/events.rs create mode 100644 crates/transaction-pool/src/pool/listener.rs create mode 100644 crates/transaction-pool/src/pool/mod.rs create mode 100644 crates/transaction-pool/src/pool/pending.rs create mode 100644 crates/transaction-pool/src/pool/queued.rs diff --git a/Cargo.lock b/Cargo.lock index 8175a113b91..15280653fa0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -551,6 +551,12 @@ version = "0.2.133" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c0f80d65747a3e43d1596c7c5492d95d5edddaabd45a7fcdb02b95f644164966" +[[package]] +name = "linked-hash-map" +version = "0.5.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0717cef1bc8b636c6e1c1bbdefc09e6322da8a9321966e8928ef80d20f7f770f" + [[package]] name = "lock_api" version = "0.4.9" @@ -818,6 +824,7 @@ version = "0.1.0" dependencies = [ "async-trait", "futures", + "linked-hash-map", "parking_lot", "reth-primitives", "serde", diff --git a/crates/transaction-pool/Cargo.toml b/crates/transaction-pool/Cargo.toml index 4369c542f3d..0073e4d8e81 100644 --- a/crates/transaction-pool/Cargo.toml +++ b/crates/transaction-pool/Cargo.toml @@ -23,3 +23,4 @@ parking_lot = "0.12" thiserror = "1.0" tracing = "0.1" serde = { version = "1.0", features = ["derive"] } +linked-hash-map = "0.5" diff --git a/crates/transaction-pool/src/chain.rs b/crates/transaction-pool/src/chain.rs deleted file mode 100644 index 339222d529a..00000000000 --- a/crates/transaction-pool/src/chain.rs +++ /dev/null @@ -1,10 +0,0 @@ -//! Provides access to the chain's storage - -// TODO probably merge with `Validator` trait? since the validator also needs chain access -#[async_trait::async_trait] -pub trait ChainInfo: Send + Sync { - /// The error type that can be converted to the crate's internal Error - type Error: Into; - - // TODO add functions to fetch Block/Hashes etc... -} diff --git a/crates/transaction-pool/src/client.rs b/crates/transaction-pool/src/client.rs new file mode 100644 index 00000000000..b8df3e27cd9 --- /dev/null +++ b/crates/transaction-pool/src/client.rs @@ -0,0 +1,22 @@ +//! Provides access to the chain's storage + +use crate::{traits, validate::TransactionValidator}; +use std::hash; + +// TODO could just merge with `TransactionValidator` into a single trait +#[async_trait::async_trait] +pub trait PoolClient: Send + Sync + TransactionValidator { + /// Error type that can be converted to the crate's internal Error. + type Error: Into; + + /// Transaction type for this client. + type Transaction; + + /// Transaction hash type. + type Hash: hash::Hash + Eq; + + /// Block hash type + type BlockHash: hash::Hash + Eq; + + // TODO add functions to fetch Block/Hashes etc... +} diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index c464f565ecd..8753b9c6a0b 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -9,41 +9,51 @@ pub mod error; -mod chain; +mod client; mod validate; -pub use chain::ChainInfo; +pub use client::PoolClient; +use parking_lot::RwLock; use std::sync::Arc; mod config; -use crate::traits::TransactionPool; pub use config::PoolConfig; -mod traits; +mod ordering; + +mod pool; -/// A generic `TransactionPool` implementation. -pub struct Pool { - // TODO rm later - _marker: std::marker::PhantomData<(ChainApi, Transaction)>, +mod traits; +pub use crate::{traits::TransactionPool, validate::TransactionValidator}; + +/// A generic, customizable `TransactionPool` implementation. +// TODO: This is a more feature rich pool, any additional features should go here, like metrics, +// etc... +pub struct Pool { + /// The actual transaction pool where transactions are handled. + inner: Arc>, + /// Chain/Storage access + client: Arc, + // TODO how to revalidate + // TODO provide a way to add listeners for ready transactions } // === impl Pool === -impl Pool +impl Pool where - ChainApi: ChainInfo, + PoolApi: PoolClient, { /// Creates a new `Pool` with the given config and chain api - pub fn new(config: PoolConfig, api: Arc) -> Self { + pub fn new(config: PoolConfig, api: Arc) -> Self { unimplemented!() } } /// implements the `TransactionPool` interface for the `Poll`. -impl TransactionPool for Pool +impl TransactionPool for Pool where - ChainApi: ChainInfo, - // TODO this could be unified by moving it ChaiApi trait - Transaction: Send + Sync, + PoolApi: PoolClient, + PoolApi: TransactionValidator::Transaction>, { } diff --git a/crates/transaction-pool/src/ordering.rs b/crates/transaction-pool/src/ordering.rs new file mode 100644 index 00000000000..f01326809cf --- /dev/null +++ b/crates/transaction-pool/src/ordering.rs @@ -0,0 +1,4 @@ +/// Transaction ordering. +/// +/// Decides how transactions should be ordered within the pool. +pub trait TransactionOrdering {} diff --git a/crates/transaction-pool/src/pool/events.rs b/crates/transaction-pool/src/pool/events.rs new file mode 100644 index 00000000000..981efdf116a --- /dev/null +++ b/crates/transaction-pool/src/pool/events.rs @@ -0,0 +1,21 @@ +use serde::{Deserialize, Serialize}; + +/// Various events that describe status changes of a transaction. +#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)] +pub enum TransactionEvent { + /// Transaction has been added to the pending pool. + Pending, + /// Transaction has been added to the queued pool. + Queued, + /// Transaction has been included in the block belonging to this hash. + Included(BlockHash), + /// Transaction has been replaced by the transaction belonging to the hash. + /// + /// E.g. same (sender + nonce) pair + Replaced(Hash), + /// Transaction was dropped due to configured limits. + Dropped, + /// Transaction became invalid indefinitely. + Invalid, + // TODO Timedout?, broadcasted(peers) +} diff --git a/crates/transaction-pool/src/pool/listener.rs b/crates/transaction-pool/src/pool/listener.rs new file mode 100644 index 00000000000..4139387b4ae --- /dev/null +++ b/crates/transaction-pool/src/pool/listener.rs @@ -0,0 +1,18 @@ +//! Listeners for the transaction-pool + +use crate::pool::events::TransactionEvent; +use futures::channel::mpsc::UnboundedSender; +use std::{collections::HashMap, hash}; + +/// Transaction pool event listeners. +pub struct PoolEventListener { + /// All listeners for certain transactions. + listeners: HashMap>>, +} + +/// Sender half(s) of the event channels for a specific transaction +#[derive(Debug)] +pub struct PoolEventListenerSender { + /// Corresponding receiver half(s) for the transaction + receivers: Vec>>, +} diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs new file mode 100644 index 00000000000..fc744fae50e --- /dev/null +++ b/crates/transaction-pool/src/pool/mod.rs @@ -0,0 +1,53 @@ +//! Transaction Pool internals. +//! +//! Incoming transactions are validated first. The validation outcome can have 3 states: +//! 1. Transaction can _never_ be valid +//! 2. Transaction is _currently_ valid +//! 3. Transaction is _currently_ invalid, but could potentially become valid in the future +//! +//! However, (2.) and (3.) of a transaction can only be determined on the basis of the current +//! state, whereas (1.) holds indefinitely. This means once the state changes (2.) and (3.) need to +//! be reevaluated again. +//! +//! The transaction pool is responsible for storing new, valid transactions and providing the next +//! best transactions sorted by their priority. Where priority is determined by the transaction's +//! score. +//! +//! However, the score is also only valid for the current state. +//! +//! In essence the transaction pool is made of two separate sub-pools for currently valid (2.) and +//! currently invalid (3.). +//! +//! Depending on the use case, consumers of the [`TransactionPool`](crate::traits::TransactionPool) +//! are interested in (2.) and/or (3.). + +//! A generic [`TransactionPool`](crate::traits::TransactionPool) that only handles transactions. +//! +//! This Pool maintains two separate sub-pools for (2.) and (3.) +//! +//! ## Terminology +//! +//! - _Pending_: pending transactions are transactions that fall under (2.). Those transactions +//! are _currently_ ready to be executed and are stored in the `pending` sub-pool +//! - _Queued_: queued transactions are transactions that fall under category (3.). Those +//! transactions are _currently_ waiting for state changes that eventually move them into +//! category (2.) and become pending. +use crate::{pool::listener::PoolEventListener, PoolClient, PoolConfig}; +use parking_lot::RwLock; +use std::sync::Arc; + +mod events; +mod listener; +mod pending; +mod queued; + +// TODO find better name +pub struct Pool { + /// Chain/Storage access + client: Arc, + /// Pool settings + config: PoolConfig, + /// Listeners for transaction state change events + listeners: RwLock>, /* TODO needs the actual sub-pools + * TODO needs listeners for incoming transactions */ +} diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/crates/transaction-pool/src/pool/pending.rs @@ -0,0 +1 @@ + diff --git a/crates/transaction-pool/src/pool/queued.rs b/crates/transaction-pool/src/pool/queued.rs new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/crates/transaction-pool/src/pool/queued.rs @@ -0,0 +1 @@ + diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index d1484c68528..8ba788ddd0f 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -1,11 +1,13 @@ //! Transaction validation abstractions. +use std::{fmt, hash::Hash}; + /// Result returned after checking a transaction's validity type TransactionValidationResult = Result<(), TransactionValidationError>; /// Provides support for validating transaction at any given state of the chain #[async_trait::async_trait] -pub trait TransactionValidator { +pub trait TransactionValidator: Send + Sync { /// The transaction type to validate type Transaction: Send + Sync; @@ -31,3 +33,24 @@ pub enum TransactionValidationError { Invalid(Transaction), // TODO need variants for `Never`, or `At`? } + +/// Already validated transaction that can be queued. +pub trait ValidatedTransaction: fmt::Debug { + /// Transaction hash type. + type Hash: fmt::Debug + fmt::LowerHex + Eq + Clone + Hash; + + /// Transaction sender type. + type Sender: fmt::Debug + Eq + Clone + Hash + Send; + + /// Transaction hash + fn hash(&self) -> &Self::Hash; + + /// Memory usage of this transaction + fn size(&self) -> usize; + + /// Transaction sender + fn sender(&self) -> &Self::Sender; + + /// Does it have zero gas price? + fn has_zero_gas_price(&self) -> bool; +} From 4f8b9d97f6e66879ee6442695f5fd678ea0a0cb0 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Fri, 30 Sep 2022 19:56:35 +0200 Subject: [PATCH 03/35] work on listeners --- crates/transaction-pool/src/pool/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index fc744fae50e..22b45ca0c33 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -43,11 +43,11 @@ mod queued; // TODO find better name pub struct Pool { - /// Chain/Storage access + /// Chain/Storage access. client: Arc, - /// Pool settings + /// Pool settings. config: PoolConfig, - /// Listeners for transaction state change events + /// Listeners for transaction state change events. listeners: RwLock>, /* TODO needs the actual sub-pools * TODO needs listeners for incoming transactions */ } From f67914083d102bb0be944abbbd834fc64022ce28 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Mon, 3 Oct 2022 14:55:14 +0200 Subject: [PATCH 04/35] work on tx pool --- Cargo.lock | 7 +++++ crates/transaction-pool/Cargo.toml | 1 + crates/transaction-pool/src/client.rs | 8 +++--- crates/transaction-pool/src/lib.rs | 4 +-- crates/transaction-pool/src/pool/mod.rs | 26 ++++++++++++++++--- crates/transaction-pool/src/pool/pending.rs | 10 +++++++ crates/transaction-pool/src/pool/queued.rs | 10 +++++++ .../transaction-pool/src/pool/transactions.rs | 15 +++++++++++ crates/transaction-pool/src/traits.rs | 17 ++++++++++++ crates/transaction-pool/src/validate.rs | 3 ++- 10 files changed, 91 insertions(+), 10 deletions(-) create mode 100644 crates/transaction-pool/src/pool/transactions.rs diff --git a/Cargo.lock b/Cargo.lock index 7c9f9bde268..0ae8d06ab11 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -334,6 +334,12 @@ dependencies = [ "static_assertions", ] +[[package]] +name = "fnv" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" + [[package]] name = "funty" version = "2.0.0" @@ -827,6 +833,7 @@ name = "reth-transaction-pool" version = "0.1.0" dependencies = [ "async-trait", + "fnv", "futures", "linked-hash-map", "parking_lot", diff --git a/crates/transaction-pool/Cargo.toml b/crates/transaction-pool/Cargo.toml index 0073e4d8e81..157c6ef32d1 100644 --- a/crates/transaction-pool/Cargo.toml +++ b/crates/transaction-pool/Cargo.toml @@ -24,3 +24,4 @@ thiserror = "1.0" tracing = "0.1" serde = { version = "1.0", features = ["derive"] } linked-hash-map = "0.5" +fnv = "1.0.7" diff --git a/crates/transaction-pool/src/client.rs b/crates/transaction-pool/src/client.rs index b8df3e27cd9..fcfc9515af6 100644 --- a/crates/transaction-pool/src/client.rs +++ b/crates/transaction-pool/src/client.rs @@ -1,6 +1,6 @@ //! Provides access to the chain's storage -use crate::{traits, validate::TransactionValidator}; +use crate::{traits, traits::PoolTransaction, validate::TransactionValidator}; use std::hash; // TODO could just merge with `TransactionValidator` into a single trait @@ -10,13 +10,13 @@ pub trait PoolClient: Send + Sync + TransactionValidator { type Error: Into; /// Transaction type for this client. - type Transaction; + type Transaction: PoolTransaction + Send + Sync; /// Transaction hash type. - type Hash: hash::Hash + Eq; + type Hash: hash::Hash + Eq + Send + Sync; /// Block hash type - type BlockHash: hash::Hash + Eq; + type BlockHash: hash::Hash + Eq + Send + Sync; // TODO add functions to fetch Block/Hashes etc... } diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index 8753b9c6a0b..5bb9fb751fa 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -29,9 +29,9 @@ pub use crate::{traits::TransactionPool, validate::TransactionValidator}; /// A generic, customizable `TransactionPool` implementation. // TODO: This is a more feature rich pool, any additional features should go here, like metrics, // etc... -pub struct Pool { +pub struct Pool { /// The actual transaction pool where transactions are handled. - inner: Arc>, + inner: Arc>, /// Chain/Storage access client: Arc, // TODO how to revalidate diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 22b45ca0c33..4b57cf0d877 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -32,7 +32,14 @@ //! - _Queued_: queued transactions are transactions that fall under category (3.). Those //! transactions are _currently_ waiting for state changes that eventually move them into //! category (2.) and become pending. -use crate::{pool::listener::PoolEventListener, PoolClient, PoolConfig}; +use crate::{ + pool::{ + listener::PoolEventListener, pending::PendingTransactions, queued::QueuedTransactions, + transactions::TransactionsPerSender, + }, + traits::PoolTransaction, + PoolClient, PoolConfig, +}; use parking_lot::RwLock; use std::sync::Arc; @@ -40,14 +47,27 @@ mod events; mod listener; mod pending; mod queued; +mod transactions; // TODO find better name pub struct Pool { + pool: Arc>, +} + +// A pool that manages transactions +pub struct PoolInner { /// Chain/Storage access. client: Arc, /// Pool settings. config: PoolConfig, + /// Stats about senders + // TODO this should perhaps be moved to `QueuedTransactions` + transactions_per_sender: TransactionsPerSender<::Transaction>, /// Listeners for transaction state change events. - listeners: RwLock>, /* TODO needs the actual sub-pools - * TODO needs listeners for incoming transactions */ + listeners: RwLock>, + /// Sub-Pool of transactions that are ready and waiting to be executed + pending: PendingTransactions<::Transaction>, + /// Sub-Pool of transactions that are waiting for state changes that eventually turn them + /// valid, so they can be moved in the `pending` pool. + queued: QueuedTransactions<::Transaction>, } diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index 8b137891791..bbefe75bf28 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -1 +1,11 @@ +use crate::traits::PoolTransaction; +/// A pool of validated transactions that are ready on the current state and are waiting to be +/// included in a block. +/// +/// Each transaction in this pool is valid on its own, i.e. they are not dependent on transaction +/// that must be executed first. Each of these transaction can be executed independently on the +/// current state +pub struct PendingTransactions { + i: T, +} diff --git a/crates/transaction-pool/src/pool/queued.rs b/crates/transaction-pool/src/pool/queued.rs index 8b137891791..a70a6ef91e6 100644 --- a/crates/transaction-pool/src/pool/queued.rs +++ b/crates/transaction-pool/src/pool/queued.rs @@ -1 +1,11 @@ +use crate::traits::PoolTransaction; +/// A pool of transactions that are not ready on the current state and are waiting for state changes +/// that turn them valid. +/// +/// This could include: +/// - transactions that are waiting until a pending or queued transactions are mined +/// - state changes that turns them valid (e.g. basefee) +pub struct QueuedTransactions { + i: T, +} diff --git a/crates/transaction-pool/src/pool/transactions.rs b/crates/transaction-pool/src/pool/transactions.rs new file mode 100644 index 00000000000..06815ace276 --- /dev/null +++ b/crates/transaction-pool/src/pool/transactions.rs @@ -0,0 +1,15 @@ +//! pool-internal transaction related types + +use crate::traits::PoolTransaction; +use std::collections::HashMap; + +/// Current stats about all senders and their transactions +pub struct TransactionsPerSender { + transactions: HashMap, + + /// How many transactions should be stored at most per sender + max_per_peer: usize, +} + +/// Current stats about one specific sender +pub struct TransactionSender {} diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index a628dac847d..5c5f84e7278 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -1,3 +1,5 @@ +use std::{fmt, hash::Hash}; + /// General purpose abstraction fo a transaction-pool #[async_trait::async_trait] pub trait TransactionPool: Send + Sync { @@ -6,3 +8,18 @@ pub trait TransactionPool: Send + Sync { // TODO add interfaces for adding new transactions } + +/// Trait for transaction types used inside the pool +pub trait PoolTransaction: Send + Send { + /// Transaction hash type. + type Hash: fmt::Debug + fmt::LowerHex + Eq + Clone + Hash; + + /// Transaction sender type. + type Sender: fmt::Debug + Eq + Clone + Hash + Send; + + /// Hash of the transaction + fn hash(&self) -> &Self::Hash; + + /// The Sender of the transaction + fn sender(&self) -> &Self::Sender; +} diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index 8ba788ddd0f..14f53a1a426 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -1,5 +1,6 @@ //! Transaction validation abstractions. +use crate::traits::PoolTransaction; use std::{fmt, hash::Hash}; /// Result returned after checking a transaction's validity @@ -9,7 +10,7 @@ type TransactionValidationResult = Result<(), TransactionValidation #[async_trait::async_trait] pub trait TransactionValidator: Send + Sync { /// The transaction type to validate - type Transaction: Send + Sync; + type Transaction: PoolTransaction + Send + Sync; /// Validates the transaction and returns a validated outcome /// From 623861965dd244bb0e6570ecb0070ef7d63070b4 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Mon, 3 Oct 2022 16:04:08 +0200 Subject: [PATCH 05/35] work on tx pool --- Cargo.lock | 49 ++++++++++++++++++--------- crates/transaction-pool/src/traits.rs | 2 +- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 83d9519fd3b..318ce030c09 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -402,6 +402,7 @@ checksum = "7f21eda599937fba36daeb58a22e8f5cee2d14c4a17b5b7739c7c8e5e3b8230c" dependencies = [ "futures-channel", "futures-core", + "futures-executor", "futures-io", "futures-sink", "futures-task", @@ -424,6 +425,17 @@ version = "0.3.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4e5aa3de05362c3fb88de6531e6296e85cde7739cccad4b9dfeeb7f6ebce56bf" +[[package]] +name = "futures-executor" +version = "0.3.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9ff63c23854bee61b6e9cd331d523909f238fc7636290b96826e9cfa5faa00ab" +dependencies = [ + "futures-core", + "futures-task", + "futures-util", +] + [[package]] name = "futures-io" version = "0.3.24" @@ -821,6 +833,12 @@ version = "0.2.134" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "329c933548736bc49fd575ee68c89e8be4d260064184389a5b77517cddd99ffb" +[[package]] +name = "linked-hash-map" +version = "0.5.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0717cef1bc8b636c6e1c1bbdefc09e6322da8a9321966e8928ef80d20f7f770f" + [[package]] name = "lock_api" version = "0.4.9" @@ -840,22 +858,6 @@ dependencies = [ "cfg-if", ] -[[package]] -name = "linked-hash-map" -version = "0.5.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0717cef1bc8b636c6e1c1bbdefc09e6322da8a9321966e8928ef80d20f7f770f" - -[[package]] -name = "lock_api" -version = "0.4.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "435011366fe56583b16cf956f9df0095b405b82d76425bc8981c0e22e60ec4df" -dependencies = [ - "autocfg", - "scopeguard", -] - [[package]] name = "memchr" version = "2.5.0" @@ -1199,6 +1201,21 @@ dependencies = [ "thiserror", ] +[[package]] +name = "reth-transaction-pool" +version = "0.1.0" +dependencies = [ + "async-trait", + "fnv", + "futures", + "linked-hash-map", + "parking_lot", + "reth-primitives", + "serde", + "thiserror", + "tracing", +] + [[package]] name = "rfc6979" version = "0.3.0" diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 5c5f84e7278..f0dc101ff80 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -15,7 +15,7 @@ pub trait PoolTransaction: Send + Send { type Hash: fmt::Debug + fmt::LowerHex + Eq + Clone + Hash; /// Transaction sender type. - type Sender: fmt::Debug + Eq + Clone + Hash + Send; + type Sender: fmt::Debug + Eq + Clone + Hash + Send + Sync; /// Hash of the transaction fn hash(&self) -> &Self::Hash; From 22f1d8885abe4a3d3b3c8fe8b3bc6b29315948dd Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Mon, 3 Oct 2022 18:22:56 +0200 Subject: [PATCH 06/35] feat: more transaction types --- crates/transaction-pool/src/client.rs | 2 +- crates/transaction-pool/src/pool/pending.rs | 65 ++++++++++++++++++++- crates/transaction-pool/src/pool/queued.rs | 28 ++++++++- crates/transaction-pool/src/traits.rs | 10 +++- crates/transaction-pool/src/validate.rs | 41 +++++++------ 5 files changed, 118 insertions(+), 28 deletions(-) diff --git a/crates/transaction-pool/src/client.rs b/crates/transaction-pool/src/client.rs index fcfc9515af6..35153afca4c 100644 --- a/crates/transaction-pool/src/client.rs +++ b/crates/transaction-pool/src/client.rs @@ -13,7 +13,7 @@ pub trait PoolClient: Send + Sync + TransactionValidator { type Transaction: PoolTransaction + Send + Sync; /// Transaction hash type. - type Hash: hash::Hash + Eq + Send + Sync; + type Hash: hash::Hash + Eq + Send + Sync + Sync; /// Block hash type type BlockHash: hash::Hash + Eq + Send + Sync; diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index bbefe75bf28..d78ef747814 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -1,4 +1,11 @@ -use crate::traits::PoolTransaction; +use crate::{traits::PoolTransaction, validate::ValidPoolTransaction}; +use parking_lot::RwLock; +use reth_primitives::{TxHash, H256}; +use std::{ + cmp::Ordering, + collections::{BTreeSet, HashMap}, + sync::Arc, +}; /// A pool of validated transactions that are ready on the current state and are waiting to be /// included in a block. @@ -7,5 +14,59 @@ use crate::traits::PoolTransaction; /// that must be executed first. Each of these transaction can be executed independently on the /// current state pub struct PendingTransactions { - i: T, + /// Keeps track of transactions inserted in the pool. + /// + /// This way we can determine when transactions where submitted to the pool. + id: u64, + /// Markers that are provided by `PendingTransaction`s + provided_ids: HashMap, + /// All Transactions that are currently ready. + ready_transactions: Arc>>>, + /// Independent transactions that can be included directly and don't require other transactions + /// + /// Sorted by their scoring value. + independent_transactions: BTreeSet>, +} + +/// A transaction that is ready to be included in a block. +#[derive(Debug, Clone)] +pub struct PendingTransaction { + /// Reference to the actual transaction. + pub transaction: PoolTransactionRef, + /// Tracks the transactions that get unlocked by this transaction. + pub unlocks: Vec, + /// Amount of required markers that are inherently provided + pub requires_offset: usize, +} + +/// A reference to a transaction in the pool +#[derive(Debug, Clone)] +pub struct PoolTransactionRef { + /// Actual transaction. + pub transaction: Arc>, + /// Identifier that tags when transaction was submitted in the pool. + pub submit_id: u64, + /// The priority value assigned by the used `Scoring` function. + pub priority: (), + // TODO add timestamp +} + +impl Eq for PoolTransactionRef {} + +impl PartialEq for PoolTransactionRef { + fn eq(&self, other: &Self) -> bool { + self.cmp(other) == Ordering::Equal + } +} + +impl PartialOrd for PoolTransactionRef { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for PoolTransactionRef { + fn cmp(&self, other: &Self) -> Ordering { + todo!() + } } diff --git a/crates/transaction-pool/src/pool/queued.rs b/crates/transaction-pool/src/pool/queued.rs index a70a6ef91e6..96b24f55750 100644 --- a/crates/transaction-pool/src/pool/queued.rs +++ b/crates/transaction-pool/src/pool/queued.rs @@ -1,4 +1,10 @@ -use crate::traits::PoolTransaction; +use crate::{traits::PoolTransaction, validate::ValidPoolTransaction}; +use reth_primitives::TxHash; +use std::{ + collections::{HashMap, HashSet}, + sync::Arc, + time::Instant, +}; /// A pool of transactions that are not ready on the current state and are waiting for state changes /// that turn them valid. @@ -6,6 +12,24 @@ use crate::traits::PoolTransaction; /// This could include: /// - transactions that are waiting until a pending or queued transactions are mined /// - state changes that turns them valid (e.g. basefee) +/// +/// Keeps a set of transactions that are waiting until their dependencies are unlocked. pub struct QueuedTransactions { - i: T, + /// Dependencies that aren't yet provided by any transaction. + dependencies: HashMap>, + /// Mapping of the dependencies of a transaction to the hash of the transaction + waiting: HashMap, T::Hash>, + /// the transactions that are not ready yet are waiting for another tx to finish + waiting_queue: HashMap>, +} + +/// A transaction in the pool +#[derive(Clone)] +pub struct QueuedPoolTransaction { + /// Actual transaction. + pub transaction: Arc>, + /// All Ids required and have not been satisfied yet by other transactions in the pool. + pub missing_dependencies: HashSet, + /// Timestamp when the tx was added + pub added_at: Instant, } diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index f0dc101ff80..11612d3a55f 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -10,9 +10,12 @@ pub trait TransactionPool: Send + Sync { } /// Trait for transaction types used inside the pool -pub trait PoolTransaction: Send + Send { +pub trait PoolTransaction: fmt::Debug + Send + Send { /// Transaction hash type. - type Hash: fmt::Debug + fmt::LowerHex + Eq + Clone + Hash; + type Hash: fmt::Debug + fmt::LowerHex + Eq + Clone + Hash + Send + Sync; + + /// Unique identifier for this transaction. + type Id: fmt::Debug + fmt::LowerHex + Eq + Clone + Hash + Send + Sync; /// Transaction sender type. type Sender: fmt::Debug + Eq + Clone + Hash + Send + Sync; @@ -22,4 +25,7 @@ pub trait PoolTransaction: Send + Send { /// The Sender of the transaction fn sender(&self) -> &Self::Sender; + + /// Creates the unique identifier for this transaction. + fn id(&self) -> Self::Id; } diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index 14f53a1a426..2ad4e32c70c 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -16,16 +16,19 @@ pub trait TransactionValidator: Send + Sync { /// /// This is used by the transaction-pool check the transaction's validity against the state of /// the given block hash. + /// + /// This is supposed to extend the `transaction` with its identifying markers in the graph of + /// transactions for the sender. async fn validate_transaction( &self, transaction: Self::Transaction, block_hash: (), - ) -> TransactionValidationResult { + ) -> TransactionValidationResult> { unimplemented!() } } -/// Errors thrown during validity checks of a transaction +/// Errors thrown during validity checks of a transaction. #[derive(Clone, PartialEq, Eq)] pub enum TransactionValidationError { /// The transaction is considered invalid. @@ -35,23 +38,19 @@ pub enum TransactionValidationError { // TODO need variants for `Never`, or `At`? } -/// Already validated transaction that can be queued. -pub trait ValidatedTransaction: fmt::Debug { - /// Transaction hash type. - type Hash: fmt::Debug + fmt::LowerHex + Eq + Clone + Hash; - - /// Transaction sender type. - type Sender: fmt::Debug + Eq + Clone + Hash + Send; - - /// Transaction hash - fn hash(&self) -> &Self::Hash; - - /// Memory usage of this transaction - fn size(&self) -> usize; - - /// Transaction sender - fn sender(&self) -> &Self::Sender; - - /// Does it have zero gas price? - fn has_zero_gas_price(&self) -> bool; +/// A valida transaction in the pool. +#[derive(Debug)] +pub struct ValidPoolTransaction { + /// The transaction + pub transaction: T, + /// Ids required by the transaction. + /// + /// This lists all unique transactions that need to be mined before this transaction can be + /// considered `pending` and itself be included. + pub depends_on: Vec, + /// Ids that this transaction provides + /// + /// This contains the inverse of `depends_on` which provides the dependencies this transaction + /// unlocks once it's mined. + pub provides: Vec, } From 6028f47401d63c5dffec76a934a8246045545990 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Mon, 3 Oct 2022 18:48:26 +0200 Subject: [PATCH 07/35] chore: cleanup --- crates/transaction-pool/src/config.rs | 1 + crates/transaction-pool/src/lib.rs | 16 ++++++++++------ crates/transaction-pool/src/ordering.rs | 9 ++++++++- crates/transaction-pool/src/pool/mod.rs | 19 +++++++------------ crates/transaction-pool/src/pool/pending.rs | 2 +- crates/transaction-pool/src/pool/queued.rs | 2 +- .../transaction-pool/src/pool/transactions.rs | 15 --------------- crates/transaction-pool/src/validate.rs | 13 ++++++++++++- 8 files changed, 40 insertions(+), 37 deletions(-) delete mode 100644 crates/transaction-pool/src/pool/transactions.rs diff --git a/crates/transaction-pool/src/config.rs b/crates/transaction-pool/src/config.rs index 99c69136271..9671dcd41ce 100644 --- a/crates/transaction-pool/src/config.rs +++ b/crates/transaction-pool/src/config.rs @@ -2,6 +2,7 @@ #[derive(Debug, Clone)] pub struct PoolConfig { // TODO add limits for subpools + // TODO limits for per peer // TODO config whether to check if transactions are banned } diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index 5bb9fb751fa..b3ea52a9cc4 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -21,17 +21,19 @@ pub use config::PoolConfig; mod ordering; -mod pool; +pub mod pool; mod traits; -pub use crate::{traits::TransactionPool, validate::TransactionValidator}; +pub use crate::{ + ordering::TransactionOrdering, traits::TransactionPool, validate::TransactionValidator, +}; /// A generic, customizable `TransactionPool` implementation. // TODO: This is a more feature rich pool, any additional features should go here, like metrics, // etc... -pub struct Pool { +pub struct Pool { /// The actual transaction pool where transactions are handled. - inner: Arc>, + inner: Arc>, /// Chain/Storage access client: Arc, // TODO how to revalidate @@ -40,9 +42,10 @@ pub struct Pool { // === impl Pool === -impl Pool +impl Pool where PoolApi: PoolClient, + Ordering: TransactionOrdering, { /// Creates a new `Pool` with the given config and chain api pub fn new(config: PoolConfig, api: Arc) -> Self { @@ -51,9 +54,10 @@ where } /// implements the `TransactionPool` interface for the `Poll`. -impl TransactionPool for Pool +impl TransactionPool for Pool where PoolApi: PoolClient, PoolApi: TransactionValidator::Transaction>, + Ordering: TransactionOrdering, { } diff --git a/crates/transaction-pool/src/ordering.rs b/crates/transaction-pool/src/ordering.rs index f01326809cf..bb44497759a 100644 --- a/crates/transaction-pool/src/ordering.rs +++ b/crates/transaction-pool/src/ordering.rs @@ -1,4 +1,11 @@ +use std::fmt; + /// Transaction ordering. /// /// Decides how transactions should be ordered within the pool. -pub trait TransactionOrdering {} +/// +/// The returned priority must reflect natural `Ordering`. +pub trait TransactionOrdering: Send + Sync { + /// Priority of a transaction. + type Priority: Ord + Clone + Default + fmt::Debug + fmt::LowerHex + Send + Sync; +} diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 4b57cf0d877..b43420a96f3 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -33,12 +33,9 @@ //! transactions are _currently_ waiting for state changes that eventually move them into //! category (2.) and become pending. use crate::{ - pool::{ - listener::PoolEventListener, pending::PendingTransactions, queued::QueuedTransactions, - transactions::TransactionsPerSender, - }, + pool::{listener::PoolEventListener, pending::PendingTransactions, queued::QueuedTransactions}, traits::PoolTransaction, - PoolClient, PoolConfig, + PoolClient, PoolConfig, TransactionOrdering, }; use parking_lot::RwLock; use std::sync::Arc; @@ -47,22 +44,20 @@ mod events; mod listener; mod pending; mod queued; -mod transactions; // TODO find better name -pub struct Pool { - pool: Arc>, +pub struct Pool { + pool: Arc>, } // A pool that manages transactions -pub struct PoolInner { +pub struct PoolInner { /// Chain/Storage access. client: Arc, + /// How to order transactions. + ordering: Ordering, /// Pool settings. config: PoolConfig, - /// Stats about senders - // TODO this should perhaps be moved to `QueuedTransactions` - transactions_per_sender: TransactionsPerSender<::Transaction>, /// Listeners for transaction state change events. listeners: RwLock>, /// Sub-Pool of transactions that are ready and waiting to be executed diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index d78ef747814..4ec85b9caa0 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -13,7 +13,7 @@ use std::{ /// Each transaction in this pool is valid on its own, i.e. they are not dependent on transaction /// that must be executed first. Each of these transaction can be executed independently on the /// current state -pub struct PendingTransactions { +pub(crate) struct PendingTransactions { /// Keeps track of transactions inserted in the pool. /// /// This way we can determine when transactions where submitted to the pool. diff --git a/crates/transaction-pool/src/pool/queued.rs b/crates/transaction-pool/src/pool/queued.rs index 96b24f55750..48377235727 100644 --- a/crates/transaction-pool/src/pool/queued.rs +++ b/crates/transaction-pool/src/pool/queued.rs @@ -14,7 +14,7 @@ use std::{ /// - state changes that turns them valid (e.g. basefee) /// /// Keeps a set of transactions that are waiting until their dependencies are unlocked. -pub struct QueuedTransactions { +pub(crate) struct QueuedTransactions { /// Dependencies that aren't yet provided by any transaction. dependencies: HashMap>, /// Mapping of the dependencies of a transaction to the hash of the transaction diff --git a/crates/transaction-pool/src/pool/transactions.rs b/crates/transaction-pool/src/pool/transactions.rs deleted file mode 100644 index 06815ace276..00000000000 --- a/crates/transaction-pool/src/pool/transactions.rs +++ /dev/null @@ -1,15 +0,0 @@ -//! pool-internal transaction related types - -use crate::traits::PoolTransaction; -use std::collections::HashMap; - -/// Current stats about all senders and their transactions -pub struct TransactionsPerSender { - transactions: HashMap, - - /// How many transactions should be stored at most per sender - max_per_peer: usize, -} - -/// Current stats about one specific sender -pub struct TransactionSender {} diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index 2ad4e32c70c..235ec3ea206 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -39,7 +39,6 @@ pub enum TransactionValidationError { } /// A valida transaction in the pool. -#[derive(Debug)] pub struct ValidPoolTransaction { /// The transaction pub transaction: T, @@ -54,3 +53,15 @@ pub struct ValidPoolTransaction { /// unlocks once it's mined. pub provides: Vec, } + +impl fmt::Debug for ValidPoolTransaction { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(fmt, "Transaction {{ ")?; + write!(fmt, "hash: {:?}, ", &self.transaction.hash())?; + write!(fmt, "provides: {:?}, ", &self.provides)?; + write!(fmt, "depends_on: {:?}, ", &self.depends_on)?; + write!(fmt, "raw tx: {:?}", &self.transaction)?; + write!(fmt, "}}")?; + Ok(()) + } +} From b2ff8b7b1ecbd7f4b62d84c07cf4c1c7fae47f6f Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Tue, 4 Oct 2022 13:59:49 +0200 Subject: [PATCH 08/35] work on queued pool --- crates/transaction-pool/src/error.rs | 9 +- crates/transaction-pool/src/pool/mod.rs | 3 +- crates/transaction-pool/src/pool/pending.rs | 111 ++++++++-- crates/transaction-pool/src/pool/queued.rs | 192 +++++++++++++++++- .../transaction-pool/src/pool/transaction.rs | 1 + crates/transaction-pool/src/traits.rs | 13 +- crates/transaction-pool/src/validate.rs | 9 + 7 files changed, 304 insertions(+), 34 deletions(-) create mode 100644 crates/transaction-pool/src/pool/transaction.rs diff --git a/crates/transaction-pool/src/error.rs b/crates/transaction-pool/src/error.rs index 23f9cf72f11..347b9a731b4 100644 --- a/crates/transaction-pool/src/error.rs +++ b/crates/transaction-pool/src/error.rs @@ -5,4 +5,11 @@ pub type Result = std::result::Result; /// All errors the Transaction pool can throw. #[derive(Debug, thiserror::Error)] -pub enum Error {} +pub enum Error { + /// Thrown if a replacement transaction's gas price is below the already imported transaction + #[error("Tx: insufficient gas price to replace existing transaction")] + // #[error("Tx: [{0:?}] insufficient gas price to replace existing transaction")] + // ReplacementUnderpriced(Box), + ReplacementUnderpriced, + // TODO make error generic over `Transaction` +} diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index b43420a96f3..1f7b898eea7 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -44,6 +44,7 @@ mod events; mod listener; mod pending; mod queued; +mod transaction; // TODO find better name pub struct Pool { @@ -61,7 +62,7 @@ pub struct PoolInner { /// Listeners for transaction state change events. listeners: RwLock>, /// Sub-Pool of transactions that are ready and waiting to be executed - pending: PendingTransactions<::Transaction>, + pending: PendingTransactions<::Transaction, Ordering>, /// Sub-Pool of transactions that are waiting for state changes that eventually turn them /// valid, so they can be moved in the `pending` pool. queued: QueuedTransactions<::Transaction>, diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index 4ec85b9caa0..806fe38a620 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -1,6 +1,6 @@ -use crate::{traits::PoolTransaction, validate::ValidPoolTransaction}; +use crate::{traits::PoolTransaction, validate::ValidPoolTransaction, TransactionOrdering}; use parking_lot::RwLock; -use reth_primitives::{TxHash, H256}; +use reth_primitives::{TxHash, H256, U256}; use std::{ cmp::Ordering, collections::{BTreeSet, HashMap}, @@ -13,60 +13,131 @@ use std::{ /// Each transaction in this pool is valid on its own, i.e. they are not dependent on transaction /// that must be executed first. Each of these transaction can be executed independently on the /// current state -pub(crate) struct PendingTransactions { +pub(crate) struct PendingTransactions { /// Keeps track of transactions inserted in the pool. /// /// This way we can determine when transactions where submitted to the pool. id: u64, - /// Markers that are provided by `PendingTransaction`s + /// Dependencies that are provided by `PendingTransaction`s provided_ids: HashMap, + /// Pending transactions that are currently on hold until the `baseFee` of the pending block + /// changes in favor of the parked transactions: the `pendingBlock.baseFee` must decrease + /// before they can be moved to the ready pool and are ready to be executed. + parked: ParkedTransactions, /// All Transactions that are currently ready. - ready_transactions: Arc>>>, - /// Independent transactions that can be included directly and don't require other transactions + /// + /// Meaning, there are no nonce gaps in these transactions and all of them satisfy the + /// `baseFee` condition: transaction `maxFeePerGas >= pendingBlock.baseFee` + ready_transactions: Arc>>>, + /// Independent transactions that can be included directly and don't require other + /// transactions. /// /// Sorted by their scoring value. - independent_transactions: BTreeSet>, + independent_transactions: BTreeSet>, } /// A transaction that is ready to be included in a block. #[derive(Debug, Clone)] -pub struct PendingTransaction { +pub struct PendingTransaction { /// Reference to the actual transaction. - pub transaction: PoolTransactionRef, + pub transaction: PoolTransactionRef, /// Tracks the transactions that get unlocked by this transaction. pub unlocks: Vec, /// Amount of required markers that are inherently provided pub requires_offset: usize, } -/// A reference to a transaction in the pool +/// A reference to a transaction in the _pending_ pool #[derive(Debug, Clone)] -pub struct PoolTransactionRef { +pub struct PoolTransactionRef { /// Actual transaction. pub transaction: Arc>, /// Identifier that tags when transaction was submitted in the pool. - pub submit_id: u64, - /// The priority value assigned by the used `Scoring` function. - pub priority: (), - // TODO add timestamp + pub submission_id: u64, + /// The priority value assigned by the used `Ordering` function. + pub priority: O::Priority, +} + +impl Eq for PoolTransactionRef {} + +impl PartialEq for PoolTransactionRef { + fn eq(&self, other: &Self) -> bool { + self.cmp(other) == Ordering::Equal + } +} + +impl PartialOrd for PoolTransactionRef { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for PoolTransactionRef { + fn cmp(&self, other: &Self) -> Ordering { + // This compares by `priority` and only if two tx have the exact same priority this compares + // the unique `submission_id`. This ensures that transactions with same priority are not + // equal, so they're not replaced in the set + self.priority + .cmp(&other.priority) + .then_with(|| other.submission_id.cmp(&self.submission_id)) + } +} + +/// Pending Transactions that are currently parked until their set baseFee becomes valid +struct ParkedTransactions { + /// Keeps track of transactions inserted in the pool. + /// + /// This way we can determine when transactions where submitted to the pool. + id: u64, + /// All transactions that are currently parked due to their fee. + parked_transactions: HashMap>, + /// Same transactions but sorted by their fee and priority + sorted_transactions: BTreeSet>, +} + +/// A transaction that is ready to be included in a block. +#[derive(Debug, Clone)] +pub struct ParkedTransaction { + /// Reference to the actual transaction. + transaction: PoolTransactionRef, + /// Tracks the transactions that get unlocked by this transaction. + unlocks: Vec, + /// Amount of required markers that are inherently provided + requires_offset: usize, +} + +/// A reference to a currently _parked_ transaction. +struct ParkedTransactionRef { + /// Actual transaction. + transaction: Arc>, + /// Identifier that tags when transaction was submitted in the pool. + submission_id: u64, + /// The priority value assigned by the used `Ordering` function. + priority: O::Priority, + /// EIP-1559 Max base fee the caller is willing to pay. + max_fee_per_gas: U256, } -impl Eq for PoolTransactionRef {} +impl Eq for ParkedTransactionRef {} -impl PartialEq for PoolTransactionRef { +impl PartialEq for ParkedTransactionRef { fn eq(&self, other: &Self) -> bool { self.cmp(other) == Ordering::Equal } } -impl PartialOrd for PoolTransactionRef { +impl PartialOrd for ParkedTransactionRef { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) } } -impl Ord for PoolTransactionRef { +impl Ord for ParkedTransactionRef { fn cmp(&self, other: &Self) -> Ordering { - todo!() + // This compares the `max_fee_per_gas` value of the transaction + self.max_fee_per_gas + .cmp(&other.max_fee_per_gas) + .then_with(|| self.priority.cmp(&other.priority)) + .then_with(|| other.submission_id.cmp(&self.submission_id)) } } diff --git a/crates/transaction-pool/src/pool/queued.rs b/crates/transaction-pool/src/pool/queued.rs index 48377235727..d3cbdcf7711 100644 --- a/crates/transaction-pool/src/pool/queued.rs +++ b/crates/transaction-pool/src/pool/queued.rs @@ -1,35 +1,205 @@ -use crate::{traits::PoolTransaction, validate::ValidPoolTransaction}; -use reth_primitives::TxHash; +use crate::{error, error::Error, traits::PoolTransaction, validate::ValidPoolTransaction}; use std::{ collections::{HashMap, HashSet}, + fmt, sync::Arc, time::Instant, }; +use tracing::warn; /// A pool of transactions that are not ready on the current state and are waiting for state changes /// that turn them valid. /// -/// This could include: -/// - transactions that are waiting until a pending or queued transactions are mined -/// - state changes that turns them valid (e.g. basefee) +/// This could include transactions with nonce gaps: Transactions that are waiting until for a +/// transaction to arrive that closes the nonce gap /// /// Keeps a set of transactions that are waiting until their dependencies are unlocked. pub(crate) struct QueuedTransactions { /// Dependencies that aren't yet provided by any transaction. - dependencies: HashMap>, + required_dependencies: HashMap>, /// Mapping of the dependencies of a transaction to the hash of the transaction - waiting: HashMap, T::Hash>, + waiting_dependencies: HashMap, T::Hash>, /// the transactions that are not ready yet are waiting for another tx to finish waiting_queue: HashMap>, } -/// A transaction in the pool +// == impl QueuedTransactions == + +impl QueuedTransactions { + /// Returns the number of transactions that are currently waiting in this pool for new + /// transactions to satisfy their dependencies + pub fn len(&self) -> usize { + self.waiting_queue.len() + } + + /// Whether this pool is empty + pub fn is_empty(&self) -> bool { + self.waiting_queue.is_empty() + } + + /// Returns an iterator over all transactions waiting in this pool + pub fn transactions(&self) -> impl Iterator>> + '_ { + self.waiting_queue.values().map(|tx| Arc::clone(&tx.transaction)) + } + + /// Adds a transaction to the queue of transactions + pub fn add_transaction(&mut self, tx: QueuedPoolTransaction) -> error::Result<()> { + assert!(!tx.is_ready(), "transaction must not be ready"); + assert!( + !self.waiting_queue.contains_key(tx.transaction.hash()), + "transaction is already added" + ); + + if let Some(replace) = self + .waiting_dependencies + .get(&tx.transaction.provides) + .and_then(|hash| self.waiting_queue.get(hash)) + { + // TODO handle transaction underpriced + // // check if underpriced + // if tx.transaction.gas_price() < replace.transaction.gas_price() { + // warn!(target: "txpool", "pending replacement transaction underpriced [{:?}]", + // tx.transaction.hash()); return Err(Error::ReplacementUnderpriced) + // } + } + + // add all missing dependencies + for dependency in &tx.missing_dependencies { + self.required_dependencies + .entry(dependency.clone()) + .or_default() + .insert(*tx.transaction.hash()); + } + + // also track identifying dependencies + self.waiting_dependencies.insert(tx.transaction.provides.clone(), *tx.transaction.hash()); + + // add tx to the queue + self.waiting_queue.insert(*tx.transaction.hash(), tx); + + Ok(()) + } + + /// Returns true if given transaction is part of the queue + pub fn contains(&self, hash: &T::Hash) -> bool { + self.waiting_queue.contains_key(hash) + } + + /// Returns the transaction for the hash if it's waiting + pub fn get(&self, hash: &T::Hash) -> Option<&QueuedPoolTransaction> { + self.waiting_queue.get(hash) + } + + /// This will check off the dependencies of queued transactions. + /// + /// Returns the those transactions that become unlocked (all dependencies checked) and can be + /// moved to the ready queue. + pub fn satisfy_and_unlock( + &mut self, + dependencies: impl IntoIterator>, + ) -> Vec> { + let mut unlocked_ready = Vec::new(); + for mark in dependencies { + let mark = mark.as_ref(); + if let Some(tx_hashes) = self.required_dependencies.remove(mark) { + for hash in tx_hashes { + let tx = self.waiting_queue.get_mut(&hash).expect("tx is included;"); + tx.satisfy(mark); + + if tx.is_ready() { + let tx = self.waiting_queue.remove(&hash).expect("tx is included;"); + self.waiting_dependencies.remove(&tx.transaction.provides); + + unlocked_ready.push(tx); + } + } + } + } + + unlocked_ready + } + + /// Removes the transactions associated with the given hashes + /// + /// Returns all removed transactions. + pub fn remove(&mut self, hashes: Vec) -> Vec>> { + let mut removed = vec![]; + for hash in hashes { + if let Some(waiting_tx) = self.waiting_queue.remove(&hash) { + self.waiting_dependencies.remove(&waiting_tx.transaction.provides); + for dependency in waiting_tx.missing_dependencies { + let remove = + if let Some(required) = self.required_dependencies.get_mut(&dependency) { + required.remove(&hash); + required.is_empty() + } else { + false + }; + if remove { + self.required_dependencies.remove(&dependency); + } + } + removed.push(waiting_tx.transaction) + } + } + removed + } +} + +/// A transaction submitted to the pool. #[derive(Clone)] pub struct QueuedPoolTransaction { - /// Actual transaction. + /// The actual validated transaction pub transaction: Arc>, - /// All Ids required and have not been satisfied yet by other transactions in the pool. + /// Transactions required for and have not been satisfied yet by other transactions in the + /// pool. + /// + /// This will be an empty list if there are no nonce gaps across multiple transactions of the + /// same sender in the pool. If there are gaps, this will include the missing transactions. pub missing_dependencies: HashSet, - /// Timestamp when the tx was added + /// Timestamp when the tx was added. pub added_at: Instant, } + +// === impl QuQueuedPoolTransaction === + +impl QueuedPoolTransaction { + /// Creates a new `QueuedPoolTransaction`. + /// + /// Determines the dependent transaction that are still missing before this transaction can be + /// moved to the queue. + pub fn new(transaction: ValidPoolTransaction, provided: &HashMap) -> Self { + let missing_dependencies = transaction + .depends_on + .iter() + .filter(|id| { + // is true if the dependency id is already satisfied either via transaction in the + // pool + !provided.contains_key(&**id) + }) + .cloned() + .collect(); + + Self { transaction: Arc::new(transaction), missing_dependencies, added_at: Instant::now() } + } + + /// Removes the required dependency + pub fn satisfy(&mut self, id: &T::Id) { + self.missing_dependencies.remove(id); + } + + /// Returns true if transaction has all dependencies are satisfied. + pub fn is_ready(&self) -> bool { + self.missing_dependencies.is_empty() + } +} + +impl fmt::Debug for QueuedPoolTransaction { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(fmt, "QueuedPoolTransaction {{ ")?; + write!(fmt, "added_at: {:?}, ", self.added_at)?; + write!(fmt, "tx: {:?}, ", self.transaction)?; + write!(fmt, "missing_dependencies: [{:?}]", &self.missing_dependencies)?; + write!(fmt, "}}") + } +} diff --git a/crates/transaction-pool/src/pool/transaction.rs b/crates/transaction-pool/src/pool/transaction.rs new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/crates/transaction-pool/src/pool/transaction.rs @@ -0,0 +1 @@ + diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 11612d3a55f..258cbe608f4 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -1,3 +1,4 @@ +use reth_primitives::U256; use std::{fmt, hash::Hash}; /// General purpose abstraction fo a transaction-pool @@ -12,7 +13,7 @@ pub trait TransactionPool: Send + Sync { /// Trait for transaction types used inside the pool pub trait PoolTransaction: fmt::Debug + Send + Send { /// Transaction hash type. - type Hash: fmt::Debug + fmt::LowerHex + Eq + Clone + Hash + Send + Sync; + type Hash: fmt::Debug + fmt::LowerHex + Eq + Clone + Copy + Hash + Send + Sync; /// Unique identifier for this transaction. type Id: fmt::Debug + fmt::LowerHex + Eq + Clone + Hash + Send + Sync; @@ -28,4 +29,14 @@ pub trait PoolTransaction: fmt::Debug + Send + Send { /// Creates the unique identifier for this transaction. fn id(&self) -> Self::Id; + + /// Returns the EIP-1559 Max base fee the caller is willing to pay. + /// + /// This will return `None` for non-EIP1559 transactions + fn max_fee_per_gas(&self) -> Option<&U256>; + + /// Returns the EIP-1559 Priority fee the caller is paying to the block author. + /// + /// This will return `None` for non-EIP1559 transactions + fn max_priority_fee_per_gas(&self) -> Option<&U256>; } diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index 235ec3ea206..a67551592c4 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -54,6 +54,15 @@ pub struct ValidPoolTransaction { pub provides: Vec, } +// === impl ValidPoolTransaction === + +impl ValidPoolTransaction { + /// Returns the hash of the transaction + pub fn hash(&self) -> &T::Hash { + self.transaction.hash() + } +} + impl fmt::Debug for ValidPoolTransaction { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { write!(fmt, "Transaction {{ ")?; From 9e1e377989e3267c1f1e455ef0ad1ad9290e572a Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Tue, 4 Oct 2022 14:30:37 +0200 Subject: [PATCH 09/35] work on pending pool --- crates/transaction-pool/src/ordering.rs | 3 + crates/transaction-pool/src/pool/mod.rs | 2 +- crates/transaction-pool/src/pool/pending.rs | 325 +++++++++++++++++++- crates/transaction-pool/src/pool/queued.rs | 16 +- 4 files changed, 329 insertions(+), 17 deletions(-) diff --git a/crates/transaction-pool/src/ordering.rs b/crates/transaction-pool/src/ordering.rs index bb44497759a..86bcd4711ba 100644 --- a/crates/transaction-pool/src/ordering.rs +++ b/crates/transaction-pool/src/ordering.rs @@ -8,4 +8,7 @@ use std::fmt; pub trait TransactionOrdering: Send + Sync { /// Priority of a transaction. type Priority: Ord + Clone + Default + fmt::Debug + fmt::LowerHex + Send + Sync; + + /// Returns the priority score for the given transaction. + fn priority(&self, transaction: ()) -> Self::Priority; } diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 1f7b898eea7..d44a64e67ba 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -56,7 +56,7 @@ pub struct PoolInner { /// Chain/Storage access. client: Arc, /// How to order transactions. - ordering: Ordering, + ordering: Arc, /// Pool settings. config: PoolConfig, /// Listeners for transaction state change events. diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index 806fe38a620..7818c9394f9 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -1,11 +1,14 @@ -use crate::{traits::PoolTransaction, validate::ValidPoolTransaction, TransactionOrdering}; +use crate::{error, traits::PoolTransaction, validate::ValidPoolTransaction, TransactionOrdering}; use parking_lot::RwLock; -use reth_primitives::{TxHash, H256, U256}; +use reth_primitives::{H256, U256}; use std::{ cmp::Ordering, - collections::{BTreeSet, HashMap}, + collections::{BTreeSet, HashMap, HashSet}, + fmt, sync::Arc, }; +use tracing::{trace, warn}; +use crate::pool::queued::QueuedPoolTransaction; /// A pool of validated transactions that are ready on the current state and are waiting to be /// included in a block. @@ -18,8 +21,10 @@ pub(crate) struct PendingTransactions, /// Dependencies that are provided by `PendingTransaction`s - provided_ids: HashMap, + provided_dependencies: HashMap, /// Pending transactions that are currently on hold until the `baseFee` of the pending block /// changes in favor of the parked transactions: the `pendingBlock.baseFee` must decrease /// before they can be moved to the ready pool and are ready to be executed. @@ -36,19 +41,313 @@ pub(crate) struct PendingTransactions>, } +// === impl PendingTransactions === + +impl PendingTransactions { + // /// Returns an iterator over all transactions + // pub fn get_transactions(&self) -> TransactionsIterator { + // TransactionsIterator { + // all: self.ready_tx.read().clone(), + // independent: self.independent_transactions.clone(), + // awaiting: Default::default(), + // _invalid: Default::default(), + // } + // } + + /// Returns true if the transaction is part of the queue. + pub fn contains(&self, hash: &T::Hash) -> bool { + self.ready_transactions.read().contains_key(hash) + } + + /// Returns the transaction for the hash if it's in the ready pool but not yet mined + pub fn get(&self, hash: &T::Hash) -> Option> { + self.ready_transactions.read().get(hash).cloned() + } + + pub fn provided_dependencies(&self) -> &HashMap { + &self.provided_dependencies + } + + fn next_id(&mut self) -> u64 { + let id = self.id; + self.id = self.id.wrapping_add(1); + id + } + + /// Adds a new transactions to the pending queue + /// + /// # Panics + /// + /// if the pending transaction is not ready + /// or the transaction is already included + pub fn add_transaction( + &mut self, + tx: QueuedPoolTransaction, + ) -> error::Result>>> { + assert!(tx.is_ready(), "transaction must be ready",); + assert!( + !self.ready_transactions.read().contains_key(tx.transaction.hash()), + "transaction already included" + ); + + let (replaced_tx, unlocks) = self.replaced_transactions(&tx.transaction)?; + + let submission_id = self.next_id(); + let hash = *tx.transaction.hash(); + + let mut independent = true; + let mut requires_offset = 0; + let mut ready = self.ready_transactions.write(); + + // Add links to transactions that unlock the current one + for dependency in &tx.transaction.depends_on { + // Check if the transaction that satisfies the mark is still in the queue. + if let Some(other) = self.provided_dependencies.get(dependency) { + let tx = ready.get_mut(other).expect("hash included;"); + tx.unlocks.push(hash); + // tx still depends on other tx + independent = false; + } else { + requires_offset += 1; + } + } + + // update dependencies + for mark in tx.transaction.provides.iter().cloned() { + self.provided_dependencies.insert(mark, hash); + } + + let priority = self.ordering.priority(()); + let transaction = + PoolTransactionRef { submission_id, transaction: tx.transaction, priority }; + + // TODO check basefee requirement + + // add to the independent set + if independent { + self.independent_transactions.insert(transaction.clone()); + } + + // insert to ready queue + ready.insert(hash, PendingTransaction { transaction, unlocks, requires_offset }); + + Ok(replaced_tx) + } + + /// Removes and returns those transactions that got replaced by the `tx` + fn replaced_transactions( + &mut self, + tx: &ValidPoolTransaction, + ) -> error::Result<(Vec>>, Vec)> { + // check if we are replacing transactions + let remove_hashes: HashSet<_> = + tx.provides.iter().filter_map(|mark| self.provided_dependencies.get(mark)).collect(); + + // early exit if we are not replacing anything. + if remove_hashes.is_empty() { + return Ok((Vec::new(), Vec::new())) + } + + // check if we're replacing the same transaction and if it can be replaced + + let mut unlocked_tx = Vec::new(); + { + // construct a list of unlocked transactions + // also check for transactions that shouldn't be replaced because underpriced + let ready = self.ready_transactions.read(); + for to_remove in remove_hashes.iter().filter_map(|hash| ready.get(hash)) { + // if we're attempting to replace a transaction that provides the exact same dependencies + // (addr + nonce) then we check for gas price + if to_remove.provides() == tx.provides { + // check if underpriced + // TODO check if underpriced + // if tx.pending_transaction.transaction.gas_price() <= to_remove.gas_price() { + // warn!(target: "txpool", "ready replacement transaction underpriced [{:?}]", tx.hash()); + // return Err(PoolError::ReplacementUnderpriced(Box::new(tx.clone()))) + // } else { + // trace!(target: "txpool", "replacing ready transaction [{:?}] with higher gas price [{:?}]", to_remove.transaction.transaction.hash(), tx.hash()); + // } + } + + unlocked_tx.extend(to_remove.unlocks.iter().cloned()) + } + } + + let remove_hashes = remove_hashes.into_iter().copied().collect::>(); + + let new_provides = tx.provides.iter().cloned().collect::>(); + let removed_tx = self.remove_with_dependencies(remove_hashes, Some(new_provides)); + + Ok((removed_tx, unlocked_tx)) + } + + /// Removes the transactions from the ready queue and returns the removed transactions. + /// This will also remove all transactions that depend on those. + pub fn clear_transactions( + &mut self, + tx_hashes: &[T::Hash], + ) -> Vec>> { + self.remove_with_dependencies(tx_hashes.to_vec(), None) + } + + /// Removes the transactions that provide the dependency id. + /// + /// This will also remove all transactions that lead to the transaction that provides the + /// id. + pub fn prune_tags(&mut self, id: T::Id) -> Vec>> { + let mut removed_tx = vec![]; + + // the dependencies to remove + let mut remove = vec![id]; + + while let Some(dependency) = remove.pop() { + let res = self + .provided_dependencies + .remove(&dependency) + .and_then(|hash| self.ready_transactions.write().remove(&hash)); + + if let Some(tx) = res { + let unlocks = tx.unlocks; + self.independent_transactions.remove(&tx.transaction); + let tx = tx.transaction.transaction; + + // also remove previous transactions + { + let hash = tx.hash(); + let mut ready = self.ready_transactions.write(); + + let mut previous_dependency = |dependency| -> Option> { + let prev_hash = self.provided_dependencies.get(dependency)?; + let tx2 = ready.get_mut(prev_hash)?; + // remove hash + if let Some(idx) = tx2.unlocks.iter().position(|i| i == hash) { + tx2.unlocks.swap_remove(idx); + } + if tx2.unlocks.is_empty() { + Some(tx2.transaction.transaction.provides.clone()) + } else { + None + } + }; + + // find previous transactions + for dep in &tx.depends_on { + if let Some(mut dependency_to_remove) = previous_dependency(dep) { + remove.append(&mut dependency_to_remove); + } + } + } + + // add the transactions that just got unlocked to independent set + for hash in unlocks { + if let Some(tx) = self.ready_transactions.write().get_mut(&hash) { + tx.requires_offset += 1; + if tx.requires_offset == tx.transaction.transaction.depends_on.len() { + self.independent_transactions.insert(tx.transaction.clone()); + } + } + } + // finally, remove the dependencies that this transaction provides + let current_dependency = &dependency; + for dependency in &tx.provides { + let removed = self.provided_dependencies.remove(dependency); + assert_eq!( + removed.as_ref(), + if current_dependency == dependency { None } else { Some(tx.hash()) }, + "The pool contains exactly one transaction providing given tag; the removed transaction + claims to provide that tag, so it has to be mapped to it's hash; qed" + ); + } + removed_tx.push(tx); + } + } + + removed_tx + } + + /// Removes transactions and those that depend on them and satisfy at least one dependency in the + /// given filter set. + pub fn remove_with_dependencies( + &mut self, + mut tx_hashes: Vec, + dependency_filter: Option>, + ) -> Vec>> { + let mut removed = Vec::new(); + let mut ready = self.ready_transactions.write(); + + while let Some(hash) = tx_hashes.pop() { + if let Some(mut tx) = ready.remove(&hash) { + let invalidated = tx.transaction.transaction.provides.iter().filter(|mark| { + dependency_filter.as_ref().map(|filter| !filter.contains(&**mark)).unwrap_or(true) + }); + + let mut removed_some_marks = false; + // remove entries from provided_dependencies + for mark in invalidated { + removed_some_marks = true; + self.provided_dependencies.remove(mark); + } + + // remove from unlocks + for mark in &tx.transaction.transaction.depends_on { + if let Some(hash) = self.provided_dependencies.get(mark) { + if let Some(tx) = ready.get_mut(hash) { + if let Some(idx) = tx.unlocks.iter().position(|i| i == hash) { + tx.unlocks.swap_remove(idx); + } + } + } + } + + // remove from the independent set + self.independent_transactions.remove(&tx.transaction); + + if removed_some_marks { + // remove all transactions that the current one unlocks + tx_hashes.append(&mut tx.unlocks); + } + + // remove transaction + removed.push(tx.transaction.transaction); + } + } + + removed + } +} + /// A transaction that is ready to be included in a block. -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct PendingTransaction { /// Reference to the actual transaction. pub transaction: PoolTransactionRef, /// Tracks the transactions that get unlocked by this transaction. pub unlocks: Vec, - /// Amount of required markers that are inherently provided + /// Amount of required dependencies that are inherently provided. pub requires_offset: usize, } +// == impl PendingTransaction === + +impl PendingTransaction { + /// Returns all ids this transaction satisfies. + pub fn provides(&self) -> &[T::Id] { + &self.transaction.transaction.provides + } +} + +impl Clone for PendingTransaction { + fn clone(&self) -> Self { + Self { + transaction: self.transaction.clone(), + unlocks: self.unlocks.clone(), + requires_offset: self.requires_offset + } + } +} + /// A reference to a transaction in the _pending_ pool -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct PoolTransactionRef { /// Actual transaction. pub transaction: Arc>, @@ -58,6 +357,16 @@ pub struct PoolTransactionRef { pub priority: O::Priority, } +impl Clone for PoolTransactionRef { + fn clone(&self) -> Self { + Self { + transaction: Arc::clone(&self.transaction), + submission_id: self.submission_id, + priority: self.priority.clone() + } + } +} + impl Eq for PoolTransactionRef {} impl PartialEq for PoolTransactionRef { @@ -102,7 +411,7 @@ pub struct ParkedTransaction { transaction: PoolTransactionRef, /// Tracks the transactions that get unlocked by this transaction. unlocks: Vec, - /// Amount of required markers that are inherently provided + /// Amount of required dependencies that are inherently provided requires_offset: usize, } diff --git a/crates/transaction-pool/src/pool/queued.rs b/crates/transaction-pool/src/pool/queued.rs index d3cbdcf7711..5928c0f99f6 100644 --- a/crates/transaction-pool/src/pool/queued.rs +++ b/crates/transaction-pool/src/pool/queued.rs @@ -11,15 +11,15 @@ use tracing::warn; /// that turn them valid. /// /// This could include transactions with nonce gaps: Transactions that are waiting until for a -/// transaction to arrive that closes the nonce gap +/// transaction to arrive that closes the nonce gap. /// /// Keeps a set of transactions that are waiting until their dependencies are unlocked. pub(crate) struct QueuedTransactions { /// Dependencies that aren't yet provided by any transaction. required_dependencies: HashMap>, - /// Mapping of the dependencies of a transaction to the hash of the transaction + /// Mapping of the dependencies of a transaction to the hash of the transaction, waiting_dependencies: HashMap, T::Hash>, - /// the transactions that are not ready yet are waiting for another tx to finish + /// Transactions that are not ready yet are waiting for another tx to finish, waiting_queue: HashMap>, } @@ -27,17 +27,17 @@ pub(crate) struct QueuedTransactions { impl QueuedTransactions { /// Returns the number of transactions that are currently waiting in this pool for new - /// transactions to satisfy their dependencies + /// transactions to satisfy their dependencies. pub fn len(&self) -> usize { self.waiting_queue.len() } - /// Whether this pool is empty + /// Whether this pool is empty. pub fn is_empty(&self) -> bool { self.waiting_queue.is_empty() } - /// Returns an iterator over all transactions waiting in this pool + /// Returns an iterator over all transactions waiting in this pool. pub fn transactions(&self) -> impl Iterator>> + '_ { self.waiting_queue.values().map(|tx| Arc::clone(&tx.transaction)) } @@ -149,7 +149,7 @@ impl QueuedTransactions { /// A transaction submitted to the pool. #[derive(Clone)] pub struct QueuedPoolTransaction { - /// The actual validated transaction + /// The actual validated transaction. pub transaction: Arc>, /// Transactions required for and have not been satisfied yet by other transactions in the /// pool. @@ -183,7 +183,7 @@ impl QueuedPoolTransaction { Self { transaction: Arc::new(transaction), missing_dependencies, added_at: Instant::now() } } - /// Removes the required dependency + /// Removes the required dependency. pub fn satisfy(&mut self, id: &T::Id) { self.missing_dependencies.remove(id); } From 3e208689fe94801a27c49f029de1a6c9547d24f5 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Tue, 4 Oct 2022 16:25:39 +0200 Subject: [PATCH 10/35] docs: more docs --- crates/transaction-pool/src/client.rs | 5 +- crates/transaction-pool/src/error.rs | 8 +- crates/transaction-pool/src/lib.rs | 1 - crates/transaction-pool/src/pool/mod.rs | 195 +++++++++++++++++++- crates/transaction-pool/src/pool/pending.rs | 51 ++--- crates/transaction-pool/src/pool/queued.rs | 19 +- crates/transaction-pool/src/traits.rs | 4 +- 7 files changed, 242 insertions(+), 41 deletions(-) diff --git a/crates/transaction-pool/src/client.rs b/crates/transaction-pool/src/client.rs index 35153afca4c..fed75073bcb 100644 --- a/crates/transaction-pool/src/client.rs +++ b/crates/transaction-pool/src/client.rs @@ -7,10 +7,7 @@ use std::hash; #[async_trait::async_trait] pub trait PoolClient: Send + Sync + TransactionValidator { /// Error type that can be converted to the crate's internal Error. - type Error: Into; - - /// Transaction type for this client. - type Transaction: PoolTransaction + Send + Sync; + type Error: Into; /// Transaction hash type. type Hash: hash::Hash + Eq + Send + Sync + Sync; diff --git a/crates/transaction-pool/src/error.rs b/crates/transaction-pool/src/error.rs index 347b9a731b4..d7b7be781d8 100644 --- a/crates/transaction-pool/src/error.rs +++ b/crates/transaction-pool/src/error.rs @@ -1,15 +1,19 @@ //! Transaction pool errors /// Transaction pool result type. -pub type Result = std::result::Result; +pub type PoolResult = Result; /// All errors the Transaction pool can throw. #[derive(Debug, thiserror::Error)] -pub enum Error { +pub enum PoolError { /// Thrown if a replacement transaction's gas price is below the already imported transaction #[error("Tx: insufficient gas price to replace existing transaction")] // #[error("Tx: [{0:?}] insufficient gas price to replace existing transaction")] // ReplacementUnderpriced(Box), ReplacementUnderpriced, // TODO make error generic over `Transaction` + #[error("[{0:?}] Already added")] + AlreadyAdded(Box), + #[error("Transaction with cyclic dependent transactions")] + CyclicTransaction, } diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index b3ea52a9cc4..de220751c5c 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -57,7 +57,6 @@ where impl TransactionPool for Pool where PoolApi: PoolClient, - PoolApi: TransactionValidator::Transaction>, Ordering: TransactionOrdering, { } diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index d44a64e67ba..dcb39144516 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -33,12 +33,20 @@ //! transactions are _currently_ waiting for state changes that eventually move them into //! category (2.) and become pending. use crate::{ - pool::{listener::PoolEventListener, pending::PendingTransactions, queued::QueuedTransactions}, + error::{PoolError, PoolResult}, + pool::{ + listener::PoolEventListener, + pending::PendingTransactions, + queued::{QueuedPoolTransaction, QueuedTransactions}, + }, traits::PoolTransaction, + validate::ValidPoolTransaction, PoolClient, PoolConfig, TransactionOrdering, }; use parking_lot::RwLock; -use std::sync::Arc; +use reth_primitives::TxHash; +use std::{collections::VecDeque, fmt, sync::Arc}; +use tracing::{debug, trace, warn}; mod events; mod listener; @@ -62,8 +70,187 @@ pub struct PoolInner { /// Listeners for transaction state change events. listeners: RwLock>, /// Sub-Pool of transactions that are ready and waiting to be executed - pending: PendingTransactions<::Transaction, Ordering>, + pending: PendingTransactions, /// Sub-Pool of transactions that are waiting for state changes that eventually turn them /// valid, so they can be moved in the `pending` pool. - queued: QueuedTransactions<::Transaction>, + queued: QueuedTransactions, +} + +type TransactionHashFor = ::Hash; + +// === impl PoolInner === + +impl PoolInner { + /// Returns if the transaction for the given hash is already included in this pool + pub fn contains(&self, tx_hash: &TransactionHashFor) -> bool { + self.queued.contains(tx_hash) || self.pending.contains(tx_hash) + } + + /// Adds the transaction into the pool + /// + /// This pool consists of two sub-pools: `Queued` and `Pending`. + /// + /// The `Queued` pool contains transaction with gaps in its dependency tree: It requires + /// additional transaction that are note yet present in the pool. + /// + /// The `Pending` pool contains all transactions that have all their dependencies satisfied (no + /// nonce gaps). It consists of two parts: `Parked` and `Ready`. + /// + /// The `Ready` queue contains transactions that are ready to be included in the pending block. + /// With the EIP-1559, transactions can become executable or not without any changes to the + /// sender's balance or nonce and instead their feeCap determines whether the transaction is + /// _currently_ (on the current state) ready or needs to be parked until the feeCap satisfies + /// the block's basFee. + fn add_transaction( + &mut self, + tx: ValidPoolTransaction, + ) -> PoolResult> { + if self.contains(tx.hash()) { + warn!(target: "txpool", "[{:?}] Already added", tx.hash()); + return Err(PoolError::AlreadyAdded(Box::new(*tx.hash()))) + } + + let tx = QueuedPoolTransaction::new(tx, self.pending.provided_dependencies()); + trace!(target: "txpool", "[{:?}] {:?}", tx.transaction.hash(), tx); + + // If all markers are not satisfied import to future + if !tx.is_satisfied() { + let hash = *tx.transaction.hash(); + self.queued.add_transaction(tx)?; + return Ok(AddedTransaction::Queued { hash }) + } + self.add_pending_transaction(tx) + } + + /// Adds the transaction to the pending pool. + /// + /// This will also move all transaction that get unlocked by the dependency id this transaction provides from the queued pool into the pending pool. + /// + /// CAUTION: this expects that transaction's dependencies are fully satisfied + fn add_pending_transaction( + &mut self, + tx: QueuedPoolTransaction, + ) -> PoolResult> { + let hash = *tx.transaction.hash(); + trace!(target: "txpool", "adding pending transaction [{:?}]", hash); + let mut pending = AddedPendingTransaction::new(hash); + + // tracks all transaction that can be moved to the pending pool, starting the given transaction + let mut pending_transactions = VecDeque::from([tx]); + // tracks whether we're processing the given `tx` + let mut is_new_tx = true; + + // take first transaction from the list + while let Some(current_tx) = pending_transactions.pop_front() { + // also add the transaction that the current transaction unlocks + pending_transactions.extend(self.queued.satisfy_and_unlock(¤t_tx.transaction.provides)); + + let current_hash = *current_tx.transaction.hash(); + + // try to add the transaction to the ready pool + match self.pending.add_transaction(current_tx) { + Ok(replaced_transactions) => { + if !is_new_tx { + pending.promoted.push(current_hash); + } + // tx removed from ready pool + pending.removed.extend(replaced_transactions); + } + Err(err) => { + // failed to add transaction + if is_new_tx { + debug!(target: "txpool", "[{:?}] Failed to add tx: {:?}", current_hash, + err); + return Err(err) + } else { + pending.discarded.push(current_hash); + } + } + } + is_new_tx = false; + } + + // check for a cycle where importing a transaction resulted in pending transactions to be + // added while removing current transaction. in which case we move this transaction back to + // the pending queue + if pending.removed.iter().any(|tx| *tx.hash() == hash) { + self.pending.clear_transactions(&pending.promoted); + return Err(PoolError::CyclicTransaction) + } + + Ok(AddedTransaction::Pending(pending)) + } +} + +// /// Represents the outcome of a prune +// pub struct PruneResult { +// /// a list of added transactions that a pruned marker satisfied +// pub promoted: Vec, +// /// all transactions that failed to be promoted and now are discarded +// pub failed: Vec, +// /// all transactions that were pruned from the ready pool +// pub pruned: Vec>, +// } +// +// impl fmt::Debug for PruneResult { +// fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { +// write!(fmt, "PruneResult {{ ")?; +// write!( +// fmt, +// "promoted: {:?}, ", +// self.promoted.iter().map(|tx| *tx.hash()).collect::>() +// )?; +// write!(fmt, "failed: {:?}, ", self.failed)?; +// write!( +// fmt, +// "pruned: {:?}, ", +// self.pruned.iter().map(|tx| *tx.pending_transaction.hash()).collect::>() +// )?; +// write!(fmt, "}}")?; +// Ok(()) +// } +// } + +#[derive(Debug, Clone)] +pub struct AddedPendingTransaction { + /// the hash of the submitted transaction + hash: T::Hash, + /// transactions promoted to the ready queue + promoted: Vec, + /// transaction that failed and became discarded + discarded: Vec, + /// Transactions removed from the Ready pool + removed: Vec>>, +} + +impl AddedPendingTransaction { + pub fn new(hash: T::Hash) -> Self { + Self { + hash, + promoted: Default::default(), + discarded: Default::default(), + removed: Default::default(), + } + } +} + +#[derive(Debug, Clone)] +pub enum AddedTransaction { + /// Transaction was successfully added and moved to the pending pool. + Pending(AddedPendingTransaction), + /// Transaction was successfully added but not yet queued for processing and moved to the + /// queued pool instead. + Queued { + /// the hash of the submitted transaction + hash: T::Hash, + }, +} + +impl AddedTransaction { + pub fn hash(&self) -> &T::Hash { + match self { + AddedTransaction::Pending(tx) => &tx.hash, + AddedTransaction::Queued { hash } => hash, + } + } } diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index 7818c9394f9..bd1f764d580 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -1,4 +1,7 @@ -use crate::{error, traits::PoolTransaction, validate::ValidPoolTransaction, TransactionOrdering}; +use crate::{ + error, error::PoolResult, pool::queued::QueuedPoolTransaction, traits::PoolTransaction, + validate::ValidPoolTransaction, TransactionOrdering, +}; use parking_lot::RwLock; use reth_primitives::{H256, U256}; use std::{ @@ -8,7 +11,6 @@ use std::{ sync::Arc, }; use tracing::{trace, warn}; -use crate::pool::queued::QueuedPoolTransaction; /// A pool of validated transactions that are ready on the current state and are waiting to be /// included in a block. @@ -60,7 +62,7 @@ impl PendingTransactions { } /// Returns the transaction for the hash if it's in the ready pool but not yet mined - pub fn get(&self, hash: &T::Hash) -> Option> { + pub(crate) fn get(&self, hash: &T::Hash) -> Option> { self.ready_transactions.read().get(hash).cloned() } @@ -74,7 +76,9 @@ impl PendingTransactions { id } - /// Adds a new transactions to the pending queue + /// Adds a new transactions to the pending queue. + /// + /// Depending on the transaction's feeCap, this will either move it into the ready queue or park it until a future baseFee unlocks it. /// /// # Panics /// @@ -83,8 +87,8 @@ impl PendingTransactions { pub fn add_transaction( &mut self, tx: QueuedPoolTransaction, - ) -> error::Result>>> { - assert!(tx.is_ready(), "transaction must be ready",); + ) -> PoolResult>>> { + assert!(tx.is_satisfied(), "transaction must be ready",); assert!( !self.ready_transactions.read().contains_key(tx.transaction.hash()), "transaction already included" @@ -138,7 +142,7 @@ impl PendingTransactions { fn replaced_transactions( &mut self, tx: &ValidPoolTransaction, - ) -> error::Result<(Vec>>, Vec)> { + ) -> PoolResult<(Vec>>, Vec)> { // check if we are replacing transactions let remove_hashes: HashSet<_> = tx.provides.iter().filter_map(|mark| self.provided_dependencies.get(mark)).collect(); @@ -156,16 +160,18 @@ impl PendingTransactions { // also check for transactions that shouldn't be replaced because underpriced let ready = self.ready_transactions.read(); for to_remove in remove_hashes.iter().filter_map(|hash| ready.get(hash)) { - // if we're attempting to replace a transaction that provides the exact same dependencies - // (addr + nonce) then we check for gas price + // if we're attempting to replace a transaction that provides the exact same + // dependencies (addr + nonce) then we check for gas price if to_remove.provides() == tx.provides { // check if underpriced // TODO check if underpriced // if tx.pending_transaction.transaction.gas_price() <= to_remove.gas_price() { - // warn!(target: "txpool", "ready replacement transaction underpriced [{:?}]", tx.hash()); - // return Err(PoolError::ReplacementUnderpriced(Box::new(tx.clone()))) + // warn!(target: "txpool", "ready replacement transaction underpriced + // [{:?}]", tx.hash()); return + // Err(PoolError::ReplacementUnderpriced(Box::new(tx.clone()))) // } else { - // trace!(target: "txpool", "replacing ready transaction [{:?}] with higher gas price [{:?}]", to_remove.transaction.transaction.hash(), tx.hash()); + // trace!(target: "txpool", "replacing ready transaction [{:?}] with higher + // gas price [{:?}]", to_remove.transaction.transaction.hash(), tx.hash()); // } } @@ -265,8 +271,8 @@ impl PendingTransactions { removed_tx } - /// Removes transactions and those that depend on them and satisfy at least one dependency in the - /// given filter set. + /// Removes transactions and those that depend on them and satisfy at least one dependency in + /// the given filter set. pub fn remove_with_dependencies( &mut self, mut tx_hashes: Vec, @@ -278,7 +284,10 @@ impl PendingTransactions { while let Some(hash) = tx_hashes.pop() { if let Some(mut tx) = ready.remove(&hash) { let invalidated = tx.transaction.transaction.provides.iter().filter(|mark| { - dependency_filter.as_ref().map(|filter| !filter.contains(&**mark)).unwrap_or(true) + dependency_filter + .as_ref() + .map(|filter| !filter.contains(&**mark)) + .unwrap_or(true) }); let mut removed_some_marks = false; @@ -318,13 +327,13 @@ impl PendingTransactions { /// A transaction that is ready to be included in a block. #[derive(Debug)] -pub struct PendingTransaction { +pub(crate) struct PendingTransaction { /// Reference to the actual transaction. - pub transaction: PoolTransactionRef, + transaction: PoolTransactionRef, /// Tracks the transactions that get unlocked by this transaction. - pub unlocks: Vec, + unlocks: Vec, /// Amount of required dependencies that are inherently provided. - pub requires_offset: usize, + requires_offset: usize, } // == impl PendingTransaction === @@ -341,7 +350,7 @@ impl Clone for PendingTransaction Clone for PoolTransactionRef QueuedTransactions { } /// Adds a transaction to the queue of transactions - pub fn add_transaction(&mut self, tx: QueuedPoolTransaction) -> error::Result<()> { - assert!(!tx.is_ready(), "transaction must not be ready"); + pub fn add_transaction(&mut self, tx: QueuedPoolTransaction) -> PoolResult<()> { + assert!(!tx.is_satisfied(), "transaction must not be ready"); assert!( !self.waiting_queue.contains_key(tx.transaction.hash()), "transaction is already added" @@ -99,14 +104,14 @@ impl QueuedTransactions { dependencies: impl IntoIterator>, ) -> Vec> { let mut unlocked_ready = Vec::new(); - for mark in dependencies { - let mark = mark.as_ref(); + for dependency in dependencies { + let mark = dependency.as_ref(); if let Some(tx_hashes) = self.required_dependencies.remove(mark) { for hash in tx_hashes { let tx = self.waiting_queue.get_mut(&hash).expect("tx is included;"); tx.satisfy(mark); - if tx.is_ready() { + if tx.is_satisfied() { let tx = self.waiting_queue.remove(&hash).expect("tx is included;"); self.waiting_dependencies.remove(&tx.transaction.provides); @@ -189,7 +194,7 @@ impl QueuedPoolTransaction { } /// Returns true if transaction has all dependencies are satisfied. - pub fn is_ready(&self) -> bool { + pub fn is_satisfied(&self) -> bool { self.missing_dependencies.is_empty() } } diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 258cbe608f4..f259b0177d1 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -13,10 +13,10 @@ pub trait TransactionPool: Send + Sync { /// Trait for transaction types used inside the pool pub trait PoolTransaction: fmt::Debug + Send + Send { /// Transaction hash type. - type Hash: fmt::Debug + fmt::LowerHex + Eq + Clone + Copy + Hash + Send + Sync; + type Hash: fmt::Debug + fmt::LowerHex + Eq + Clone + Copy + Hash + Send + Sync + 'static; /// Unique identifier for this transaction. - type Id: fmt::Debug + fmt::LowerHex + Eq + Clone + Hash + Send + Sync; + type Id: fmt::Debug + fmt::LowerHex + Eq + Clone + Hash + AsRef + Send + Sync; /// Transaction sender type. type Sender: fmt::Debug + Eq + Clone + Hash + Send + Sync; From 6b285608a43e0cd0fbf967882bbaf3ae3601aa27 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Tue, 4 Oct 2022 16:57:41 +0200 Subject: [PATCH 11/35] feat: add transactions iterator --- crates/transaction-pool/src/client.rs | 5 +- crates/transaction-pool/src/pool/mod.rs | 27 +++-- crates/transaction-pool/src/pool/pending.rs | 119 +++++++++++++++++++- 3 files changed, 135 insertions(+), 16 deletions(-) diff --git a/crates/transaction-pool/src/client.rs b/crates/transaction-pool/src/client.rs index fed75073bcb..22f57e1ff00 100644 --- a/crates/transaction-pool/src/client.rs +++ b/crates/transaction-pool/src/client.rs @@ -3,15 +3,12 @@ use crate::{traits, traits::PoolTransaction, validate::TransactionValidator}; use std::hash; -// TODO could just merge with `TransactionValidator` into a single trait +/// The interface used to interact with the blockchain and access storage. #[async_trait::async_trait] pub trait PoolClient: Send + Sync + TransactionValidator { /// Error type that can be converted to the crate's internal Error. type Error: Into; - /// Transaction hash type. - type Hash: hash::Hash + Eq + Send + Sync + Sync; - /// Block hash type type BlockHash: hash::Hash + Eq + Send + Sync; diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index dcb39144516..ae9853b2b55 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -36,12 +36,12 @@ use crate::{ error::{PoolError, PoolResult}, pool::{ listener::PoolEventListener, - pending::PendingTransactions, + pending::{PendingTransactions, TransactionsIterator}, queued::{QueuedPoolTransaction, QueuedTransactions}, }, traits::PoolTransaction, validate::ValidPoolTransaction, - PoolClient, PoolConfig, TransactionOrdering, + PoolClient, PoolConfig, TransactionOrdering, TransactionValidator, }; use parking_lot::RwLock; use reth_primitives::TxHash; @@ -59,6 +59,9 @@ pub struct Pool { pool: Arc>, } +type TransactionHashFor = + <::Transaction as PoolTransaction>::Hash; + // A pool that manages transactions pub struct PoolInner { /// Chain/Storage access. @@ -68,7 +71,7 @@ pub struct PoolInner { /// Pool settings. config: PoolConfig, /// Listeners for transaction state change events. - listeners: RwLock>, + listeners: RwLock, PoolApi::BlockHash>>, /// Sub-Pool of transactions that are ready and waiting to be executed pending: PendingTransactions, /// Sub-Pool of transactions that are waiting for state changes that eventually turn them @@ -76,16 +79,19 @@ pub struct PoolInner { queued: QueuedTransactions, } -type TransactionHashFor = ::Hash; - // === impl PoolInner === -impl PoolInner { +impl PoolInner { /// Returns if the transaction for the given hash is already included in this pool pub fn contains(&self, tx_hash: &TransactionHashFor) -> bool { self.queued.contains(tx_hash) || self.pending.contains(tx_hash) } + /// Returns an iterator that yields transactions that are ready to be included in the block. + pub fn ready(&self) -> TransactionsIterator { + self.pending.get_transactions() + } + /// Adds the transaction into the pool /// /// This pool consists of two sub-pools: `Queued` and `Pending`. @@ -124,7 +130,8 @@ impl PoolInner PoolInner PoolInner PendingTransactions { + /// Returns an iterator over all transactions that are _currently_ ready. + /// + /// 1. The iterator _always_ returns transaction in order: It never returns a transaction with + /// an unsatisfied dependency and only returns them if dependency transaction were yielded + /// previously. In other words: The nonces of transactions with the same sender will _always_ + /// increase by exactly 1. + /// + /// The order of transactions which satisfy (1.) is determent by their computed priority: A + /// transaction with a higher priority is returned before a transaction with a lower priority. + /// + /// If two transactions have the same priority score, then the transactions which spent more + /// time in pool (were added earlier) are returned first. + /// + /// NOTE: while this iterator returns transaction that pool considers valid at this point, they + /// could potentially be become invalid at point of execution. Therefore this iterator + /// provides a way to mark transactions that the consumer of this iterator considers invalid. In + /// which case the transaction's subgraph is also automatically marked invalid, See (1.). + /// Invalid transactions are skipped. + pub fn get_transactions(&self) -> TransactionsIterator { + TransactionsIterator { + all: self.ready_transactions.read().clone(), + independent: self.independent_transactions.clone(), + awaiting: Default::default(), + invalid: Default::default(), + } + } + // /// Returns an iterator over all transactions // pub fn get_transactions(&self) -> TransactionsIterator { // TransactionsIterator { @@ -78,7 +105,8 @@ impl PendingTransactions { /// Adds a new transactions to the pending queue. /// - /// Depending on the transaction's feeCap, this will either move it into the ready queue or park it until a future baseFee unlocks it. + /// Depending on the transaction's feeCap, this will either move it into the ready queue or park + /// it until a future baseFee unlocks it. /// /// # Panics /// @@ -459,3 +487,88 @@ impl Ord for ParkedTransactionRef { + all: HashMap>, + awaiting: HashMap)>, + independent: BTreeSet>, + invalid: HashSet, +} + +// == impl TransactionsIterator == + +impl TransactionsIterator { + /// Mark the transaction as invalid. + /// + /// As a consequence, all values that depend on the invalid one will be skipped. + /// When given transaction is not in the pool it has no effect. + /// When invoked on a fully drained iterator it has no effect either. + pub fn mark_invalid(&mut self, tx: &Arc>) { + if let Some(invalid_transaction) = self.all.get(tx.hash()) { + debug!( + target: "txpool", + "[{:?}] Marked as invalid", + invalid_transaction.transaction.transaction.hash() + ); + for hash in &invalid_transaction.unlocks { + self.invalid.insert(hash.clone()); + } + } + } + + /// Depending on number of satisfied requirements insert given ref + /// either to awaiting set or to best set. + fn independent_or_awaiting(&mut self, satisfied: usize, tx_ref: PoolTransactionRef) { + if satisfied >= tx_ref.transaction.depends_on.len() { + // If we have satisfied all deps insert to best + self.independent.insert(tx_ref); + } else { + // otherwise we're still waiting for some deps + self.awaiting.insert(*tx_ref.transaction.hash(), (satisfied, tx_ref)); + } + } +} + +impl Iterator for TransactionsIterator { + type Item = Arc>; + + fn next(&mut self) -> Option { + loop { + let best = self.independent.iter().next_back()?.clone(); + let best = self.independent.take(&best)?; + let hash = best.transaction.hash(); + + // skip transactions that were marked as invalid + if self.invalid.contains(hash) { + debug!( + target: "txpool", + "[{:?}] skipping invalid transaction", + hash + ); + continue + } + + let ready = + if let Some(ready) = self.all.get(hash).cloned() { ready } else { continue }; + + // Insert transactions that just got unlocked. + for hash in &ready.unlocks { + // first check local awaiting transactions + let res = if let Some((mut satisfied, tx_ref)) = self.awaiting.remove(hash) { + satisfied += 1; + Some((satisfied, tx_ref)) + // then get from the pool + } else { + self.all + .get(hash) + .map(|next| (next.requires_offset + 1, next.transaction.clone())) + }; + if let Some((satisfied, tx_ref)) = res { + self.independent_or_awaiting(satisfied, tx_ref) + } + } + + return Some(best.transaction) + } + } +} From 07aab5cc37399dfa61e8ebda9e054ac64ed8f798 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Tue, 4 Oct 2022 17:24:58 +0200 Subject: [PATCH 12/35] impl transaction pruning --- crates/transaction-pool/src/pool/mod.rs | 112 +++++++++++++++----- crates/transaction-pool/src/pool/pending.rs | 2 +- crates/transaction-pool/src/validate.rs | 2 +- 3 files changed, 86 insertions(+), 30 deletions(-) diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index ae9853b2b55..50d05b73216 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -62,6 +62,9 @@ pub struct Pool { type TransactionHashFor = <::Transaction as PoolTransaction>::Hash; +type TransactionIdFor = + <::Transaction as PoolTransaction>::Id; + // A pool that manages transactions pub struct PoolInner { /// Chain/Storage access. @@ -189,36 +192,89 @@ impl PoolInner { Ok(AddedTransaction::Pending(pending)) } + + /// Prunes the transactions that provide the given dependencies. + /// + /// This will effectively remove those transactions that satisfy the dependencies. + /// And queued transactions might get promoted if the pruned dependencies unlock them. + pub fn prune_dependencies( + &mut self, + dependencies: impl IntoIterator>, + ) -> PruneResult { + let mut imports = vec![]; + let mut pruned = vec![]; + + for dependency in dependencies { + // mark as satisfied and store the transactions that got unlocked + imports.extend(self.queued.satisfy_and_unlock(Some(&dependency))); + // prune transactions + pruned.extend(self.pending.prune_dependencies(dependency.clone())); + } + + let mut promoted = vec![]; + let mut failed = vec![]; + for tx in imports { + let hash = *tx.transaction.hash(); + match self.add_pending_transaction(tx) { + Ok(res) => promoted.push(res), + Err(e) => { + warn!(target: "txpool", "Failed to promote tx [{:?}] : {:?}", hash, e); + failed.push(hash) + } + } + } + + PruneResult { pruned, failed, promoted } + } + + /// Remove the given transactions from the pool + pub fn remove_invalid( + &mut self, + tx_hashes: Vec>, + ) -> Vec>> { + // early exit in case there is no invalid transactions. + if tx_hashes.is_empty() { + return vec![] + } + trace!(target: "txpool", "Removing invalid transactions: {:?}", tx_hashes); + + let mut removed = self.pending.remove_with_dependencies(tx_hashes.clone(), None); + removed.extend(self.queued.remove(tx_hashes)); + + trace!(target: "txpool", "Removed invalid transactions: {:?}", removed); + + removed + } +} + +/// Represents the outcome of a prune +pub struct PruneResult { + /// a list of added transactions that a pruned marker satisfied + pub promoted: Vec>, + /// all transactions that failed to be promoted and now are discarded + pub failed: Vec, + /// all transactions that were pruned from the ready pool + pub pruned: Vec>>, } -// /// Represents the outcome of a prune -// pub struct PruneResult { -// /// a list of added transactions that a pruned marker satisfied -// pub promoted: Vec, -// /// all transactions that failed to be promoted and now are discarded -// pub failed: Vec, -// /// all transactions that were pruned from the ready pool -// pub pruned: Vec>, -// } -// -// impl fmt::Debug for PruneResult { -// fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { -// write!(fmt, "PruneResult {{ ")?; -// write!( -// fmt, -// "promoted: {:?}, ", -// self.promoted.iter().map(|tx| *tx.hash()).collect::>() -// )?; -// write!(fmt, "failed: {:?}, ", self.failed)?; -// write!( -// fmt, -// "pruned: {:?}, ", -// self.pruned.iter().map(|tx| *tx.pending_transaction.hash()).collect::>() -// )?; -// write!(fmt, "}}")?; -// Ok(()) -// } -// } +impl fmt::Debug for PruneResult { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(fmt, "PruneResult {{ ")?; + write!( + fmt, + "promoted: {:?}, ", + self.promoted.iter().map(|tx| *tx.hash()).collect::>() + )?; + write!(fmt, "failed: {:?}, ", self.failed)?; + write!( + fmt, + "pruned: {:?}, ", + self.pruned.iter().map(|tx| *tx.transaction.hash()).collect::>() + )?; + write!(fmt, "}}")?; + Ok(()) + } +} #[derive(Debug, Clone)] pub struct AddedPendingTransaction { diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index ef672303f5a..ee0654d8a1d 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -228,7 +228,7 @@ impl PendingTransactions { /// /// This will also remove all transactions that lead to the transaction that provides the /// id. - pub fn prune_tags(&mut self, id: T::Id) -> Vec>> { + pub fn prune_dependencies(&mut self, id: T::Id) -> Vec>> { let mut removed_tx = vec![]; // the dependencies to remove diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index a67551592c4..66dd8de2baf 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -1,7 +1,7 @@ //! Transaction validation abstractions. use crate::traits::PoolTransaction; -use std::{fmt, hash::Hash}; +use std::fmt; /// Result returned after checking a transaction's validity type TransactionValidationResult = Result<(), TransactionValidationError>; From 56dfb76f4522300138eb0f36571c94a9ba95acaf Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Tue, 4 Oct 2022 17:42:55 +0200 Subject: [PATCH 13/35] refactor: separate pool functions --- crates/transaction-pool/src/lib.rs | 4 +- crates/transaction-pool/src/pool/mod.rs | 78 +++++++++++++++---------- 2 files changed, 49 insertions(+), 33 deletions(-) diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index de220751c5c..2a4940ba758 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -5,7 +5,7 @@ attr(deny(warnings, rust_2018_idioms), allow(dead_code, unused_variables)) ))] -//! reth's transaction pool implementation +//! Reth's transaction pool implementation pub mod error; @@ -33,7 +33,7 @@ pub use crate::{ // etc... pub struct Pool { /// The actual transaction pool where transactions are handled. - inner: Arc>, + inner: Arc>, /// Chain/Storage access client: Arc, // TODO how to revalidate diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 50d05b73216..d01bd7a0dfa 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -43,7 +43,8 @@ use crate::{ validate::ValidPoolTransaction, PoolClient, PoolConfig, TransactionOrdering, TransactionValidator, }; -use parking_lot::RwLock; +use futures::channel::mpsc::Sender; +use parking_lot::{Mutex, RwLock}; use reth_primitives::TxHash; use std::{collections::VecDeque, fmt, sync::Arc}; use tracing::{debug, trace, warn}; @@ -54,44 +55,52 @@ mod pending; mod queued; mod transaction; -// TODO find better name -pub struct Pool { - pool: Arc>, -} - +// Helper type aliases for associated types type TransactionHashFor = <::Transaction as PoolTransaction>::Hash; - type TransactionIdFor = <::Transaction as PoolTransaction>::Id; -// A pool that manages transactions -pub struct PoolInner { +// TODO find better name +pub struct Pool { /// Chain/Storage access. client: Arc, - /// How to order transactions. - ordering: Arc, + /// The internal pool + pool: Arc>, /// Pool settings. config: PoolConfig, /// Listeners for transaction state change events. - listeners: RwLock, PoolApi::BlockHash>>, + event_listeners: RwLock, PoolApi::BlockHash>>, + /// Listeners for new ready transactions. + added_transaction_listener: Mutex>>>, +} + +/// A pool that only manages transactions. +/// +/// This pool maintains a dependency graph of transactions and provides the currently ready +/// transactions. + +// TODO could unify over `TransactionOrdering::Transaction` +pub struct PoolInner { + /// How to order transactions. + ordering: Arc, /// Sub-Pool of transactions that are ready and waiting to be executed - pending: PendingTransactions, + pending: PendingTransactions, /// Sub-Pool of transactions that are waiting for state changes that eventually turn them /// valid, so they can be moved in the `pending` pool. - queued: QueuedTransactions, + queued: QueuedTransactions, } // === impl PoolInner === -impl PoolInner { +impl PoolInner { /// Returns if the transaction for the given hash is already included in this pool - pub fn contains(&self, tx_hash: &TransactionHashFor) -> bool { + pub fn contains(&self, tx_hash: &T::Hash) -> bool { self.queued.contains(tx_hash) || self.pending.contains(tx_hash) } /// Returns an iterator that yields transactions that are ready to be included in the block. - pub fn ready(&self) -> TransactionsIterator { + pub fn ready(&self) -> TransactionsIterator { self.pending.get_transactions() } @@ -109,11 +118,8 @@ impl PoolInner { /// With the EIP-1559, transactions can become executable or not without any changes to the /// sender's balance or nonce and instead their feeCap determines whether the transaction is /// _currently_ (on the current state) ready or needs to be parked until the feeCap satisfies - /// the block's basFee. - fn add_transaction( - &mut self, - tx: ValidPoolTransaction, - ) -> PoolResult> { + /// the block's baseFee. + fn add_transaction(&mut self, tx: ValidPoolTransaction) -> PoolResult> { if self.contains(tx.hash()) { warn!(target: "txpool", "[{:?}] Already added", tx.hash()); return Err(PoolError::AlreadyAdded(Box::new(*tx.hash()))) @@ -139,8 +145,8 @@ impl PoolInner { /// CAUTION: this expects that transaction's dependencies are fully satisfied fn add_pending_transaction( &mut self, - tx: QueuedPoolTransaction, - ) -> PoolResult> { + tx: QueuedPoolTransaction, + ) -> PoolResult> { let hash = *tx.transaction.hash(); trace!(target: "txpool", "adding pending transaction [{:?}]", hash); let mut pending = AddedPendingTransaction::new(hash); @@ -199,8 +205,8 @@ impl PoolInner { /// And queued transactions might get promoted if the pruned dependencies unlock them. pub fn prune_dependencies( &mut self, - dependencies: impl IntoIterator>, - ) -> PruneResult { + dependencies: impl IntoIterator, + ) -> PruneResult { let mut imports = vec![]; let mut pruned = vec![]; @@ -227,11 +233,8 @@ impl PoolInner { PruneResult { pruned, failed, promoted } } - /// Remove the given transactions from the pool - pub fn remove_invalid( - &mut self, - tx_hashes: Vec>, - ) -> Vec>> { + /// Remove the given transactions from the pool. + pub fn remove_invalid(&mut self, tx_hashes: Vec) -> Vec>> { // early exit in case there is no invalid transactions. if tx_hashes.is_empty() { return vec![] @@ -245,6 +248,19 @@ impl PoolInner { removed } + + /// Returns the current size of the entire pool + pub fn size_of(&self) -> usize { + unimplemented!() + } + + /// Ensures that the transactions in the sub-pools are within the given bounds. + /// + /// If the current size exceeds the given bounds, the worst transactions are evicted from the + /// pool and returned. + pub fn enforce_size_limits(&mut self) { + unimplemented!() + } } /// Represents the outcome of a prune From df66604cda87063b0e1e2a7d0ecaddc98051dda2 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Tue, 4 Oct 2022 17:47:50 +0200 Subject: [PATCH 14/35] refactor: different pool types --- crates/transaction-pool/src/lib.rs | 2 +- crates/transaction-pool/src/pool/mod.rs | 40 ++++++++++++++++++++----- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index 2a4940ba758..9ee94f2c43b 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -33,7 +33,7 @@ pub use crate::{ // etc... pub struct Pool { /// The actual transaction pool where transactions are handled. - inner: Arc>, + inner: Arc>, /// Chain/Storage access client: Arc, // TODO how to revalidate diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index d01bd7a0dfa..c4acb0ed7c0 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -61,18 +61,42 @@ type TransactionHashFor = type TransactionIdFor = <::Transaction as PoolTransaction>::Id; -// TODO find better name +/// Shareable Transaction pool. pub struct Pool { + /// Arc'ed instance of the pool internals + pool: Arc>, +} + +// === impl Pool === + +impl Pool {} + +impl Clone for Pool { + fn clone(&self) -> Self { + Self { + pool: Arc::clone(&self.pool) + } + } +} + +/// Transaction pool internals. +pub struct PoolInner { /// Chain/Storage access. - client: Arc, - /// The internal pool - pool: Arc>, + client: Arc

, + /// The internal pool that manages + pool: RwLock>, /// Pool settings. config: PoolConfig, /// Listeners for transaction state change events. - event_listeners: RwLock, PoolApi::BlockHash>>, + event_listeners: RwLock, P::BlockHash>>, /// Listeners for new ready transactions. - added_transaction_listener: Mutex>>>, + added_transaction_listener: Mutex>>>, +} + +// === impl PoolInner === + +impl PoolInner { + } /// A pool that only manages transactions. @@ -81,7 +105,7 @@ pub struct Pool { /// transactions. // TODO could unify over `TransactionOrdering::Transaction` -pub struct PoolInner { +pub struct GraphPool { /// How to order transactions. ordering: Arc, /// Sub-Pool of transactions that are ready and waiting to be executed @@ -93,7 +117,7 @@ pub struct PoolInner { // === impl PoolInner === -impl PoolInner { +impl GraphPool { /// Returns if the transaction for the given hash is already included in this pool pub fn contains(&self, tx_hash: &T::Hash) -> bool { self.queued.contains(tx_hash) || self.pending.contains(tx_hash) From 8ba150146cef6736ab2e2c395200f8f734de7161 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Tue, 4 Oct 2022 20:07:48 +0200 Subject: [PATCH 15/35] feat: work on PoolApi --- crates/transaction-pool/src/client.rs | 23 ++++++-- crates/transaction-pool/src/error.rs | 5 ++ crates/transaction-pool/src/lib.rs | 3 +- crates/transaction-pool/src/pool/mod.rs | 72 +++++++++++++++++++++---- crates/transaction-pool/src/validate.rs | 8 ++- 5 files changed, 92 insertions(+), 19 deletions(-) diff --git a/crates/transaction-pool/src/client.rs b/crates/transaction-pool/src/client.rs index 22f57e1ff00..ee87fbb8cdf 100644 --- a/crates/transaction-pool/src/client.rs +++ b/crates/transaction-pool/src/client.rs @@ -1,16 +1,29 @@ //! Provides access to the chain's storage -use crate::{traits, traits::PoolTransaction, validate::TransactionValidator}; +use crate::{ + error, + error::{PoolError, PoolResult}, + traits, + traits::PoolTransaction, + validate::TransactionValidator, +}; +use reth_primitives::{BlockId, U64}; use std::hash; /// The interface used to interact with the blockchain and access storage. #[async_trait::async_trait] pub trait PoolClient: Send + Sync + TransactionValidator { /// Error type that can be converted to the crate's internal Error. - type Error: Into; + type Error: Into; - /// Block hash type - type BlockHash: hash::Hash + Eq + Send + Sync; + /// Returns the block number for the given block identifier. + fn convert_block_id(&self, block_id: &BlockId) -> PoolResult>; - // TODO add functions to fetch Block/Hashes etc... + /// Same as [`PoolClient::convert_block_id()`] but returns an error if no matching block number + /// was found + fn ensure_block_number(&self, block_id: &BlockId) -> PoolResult { + self.convert_block_id(block_id).and_then(|number| { + number.ok_or_else(|| PoolError::BlockNumberNotFound(block_id.clone())) + }) + } } diff --git a/crates/transaction-pool/src/error.rs b/crates/transaction-pool/src/error.rs index d7b7be781d8..1ae937649bf 100644 --- a/crates/transaction-pool/src/error.rs +++ b/crates/transaction-pool/src/error.rs @@ -1,5 +1,7 @@ //! Transaction pool errors +use reth_primitives::BlockId; + /// Transaction pool result type. pub type PoolResult = Result; @@ -16,4 +18,7 @@ pub enum PoolError { AlreadyAdded(Box), #[error("Transaction with cyclic dependent transactions")] CyclicTransaction, + /// Thrown if no number was found for the given block id + #[error("Invalid block id: {0:?}")] + BlockNumberNotFound(BlockId), } diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index 9ee94f2c43b..1cbcc2acdd0 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -18,6 +18,7 @@ use std::sync::Arc; mod config; pub use config::PoolConfig; +use reth_primitives::BlockId; mod ordering; @@ -33,7 +34,7 @@ pub use crate::{ // etc... pub struct Pool { /// The actual transaction pool where transactions are handled. - inner: Arc>, + inner: pool::PoolInner, /// Chain/Storage access client: Arc, // TODO how to revalidate diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index c4acb0ed7c0..a022f0f40d7 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -33,6 +33,7 @@ //! transactions are _currently_ waiting for state changes that eventually move them into //! category (2.) and become pending. use crate::{ + error, error::{PoolError, PoolResult}, pool::{ listener::PoolEventListener, @@ -40,13 +41,17 @@ use crate::{ queued::{QueuedPoolTransaction, QueuedTransactions}, }, traits::PoolTransaction, - validate::ValidPoolTransaction, - PoolClient, PoolConfig, TransactionOrdering, TransactionValidator, + validate::{TransactionValidationResult, ValidPoolTransaction}, + BlockId, PoolClient, PoolConfig, TransactionOrdering, TransactionValidator, }; use futures::channel::mpsc::Sender; use parking_lot::{Mutex, RwLock}; -use reth_primitives::TxHash; -use std::{collections::VecDeque, fmt, sync::Arc}; +use reth_primitives::{TxHash, H256, U64}; +use std::{ + collections::{HashMap, VecDeque}, + fmt, + sync::Arc, +}; use tracing::{debug, trace, warn}; mod events; @@ -69,13 +74,55 @@ pub struct Pool { // === impl Pool === -impl Pool {} +impl Pool { + /// Returns the actual block number for the block id + fn resolve_block_number(&self, block_id: &BlockId) -> PoolResult { + self.pool.client().ensure_block_number(block_id) + } + + /// Returns future that validates all transaction in the given iterator at the block the + /// `block_id` points to. + async fn validate_all( + &self, + block_id: &BlockId, + transactions: impl IntoIterator, + ) -> PoolResult, TransactionValidationResult>> + { + // get the actual block number which is required to validate to validate the transactions + let block_number = self.resolve_block_number(block_id)?; + + let outcome = futures::future::join_all( + transactions.into_iter().map(|tx| self.validate(block_id, block_number, tx)), + ) + .await + .into_iter() + .collect::>(); + + Ok(outcome) + } + + /// Validates the given transaction at the given block + /// + /// Returns future that validates single transaction at given block. + async fn validate( + &self, + block_id: &BlockId, + block_number: U64, + transaction: P::Transaction, + ) -> (TransactionHashFor

, TransactionValidationResult) { + let hash = *transaction.hash(); + // TODO this is where additional validate checks would go, like banned senders etc... + let res = self.pool.client().validate_transaction(block_id, transaction).await; + + // TODO blockstamp the transaction + + todo!() + } +} -impl Clone for Pool { +impl Clone for Pool { fn clone(&self) -> Self { - Self { - pool: Arc::clone(&self.pool) - } + Self { pool: Arc::clone(&self.pool) } } } @@ -88,7 +135,7 @@ pub struct PoolInner { /// Pool settings. config: PoolConfig, /// Listeners for transaction state change events. - event_listeners: RwLock, P::BlockHash>>, + event_listeners: RwLock, H256>>, /// Listeners for new ready transactions. added_transaction_listener: Mutex>>>, } @@ -96,7 +143,10 @@ pub struct PoolInner { // === impl PoolInner === impl PoolInner { - + /// Get client reference. + pub fn client(&self) -> &P { + &self.client + } } /// A pool that only manages transactions. diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index 66dd8de2baf..1e1a049771d 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -1,10 +1,12 @@ //! Transaction validation abstractions. use crate::traits::PoolTransaction; +use reth_primitives::BlockId; use std::fmt; /// Result returned after checking a transaction's validity -type TransactionValidationResult = Result<(), TransactionValidationError>; +pub type TransactionValidationResult = + Result<(), TransactionValidationError>; /// Provides support for validating transaction at any given state of the chain #[async_trait::async_trait] @@ -21,8 +23,9 @@ pub trait TransactionValidator: Send + Sync { /// transactions for the sender. async fn validate_transaction( &self, + block_id: &BlockId, transaction: Self::Transaction, - block_hash: (), + // TODO this should return a subset of `ValidPoolTransaction`, maybe an enum ) -> TransactionValidationResult> { unimplemented!() } @@ -52,6 +55,7 @@ pub struct ValidPoolTransaction { /// This contains the inverse of `depends_on` which provides the dependencies this transaction /// unlocks once it's mined. pub provides: Vec, + // TODO add a block timestamp that marks validity } // === impl ValidPoolTransaction === From 153e5bfb384a44576babb6f5e273f4472a413672 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 5 Oct 2022 12:13:39 +0200 Subject: [PATCH 16/35] docs: more pool docs --- crates/transaction-pool/src/client.rs | 5 ++- crates/transaction-pool/src/pool/mod.rs | 48 +++++++++++++++++++++---- 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/crates/transaction-pool/src/client.rs b/crates/transaction-pool/src/client.rs index ee87fbb8cdf..f9cdc5b1cf0 100644 --- a/crates/transaction-pool/src/client.rs +++ b/crates/transaction-pool/src/client.rs @@ -22,8 +22,7 @@ pub trait PoolClient: Send + Sync + TransactionValidator { /// Same as [`PoolClient::convert_block_id()`] but returns an error if no matching block number /// was found fn ensure_block_number(&self, block_id: &BlockId) -> PoolResult { - self.convert_block_id(block_id).and_then(|number| { - number.ok_or_else(|| PoolError::BlockNumberNotFound(block_id.clone())) - }) + self.convert_block_id(block_id) + .and_then(|number| number.ok_or_else(|| PoolError::BlockNumberNotFound(*block_id))) } } diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index a022f0f40d7..33a0dbbe557 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -1,9 +1,10 @@ //! Transaction Pool internals. //! //! Incoming transactions are validated first. The validation outcome can have 3 states: -//! 1. Transaction can _never_ be valid -//! 2. Transaction is _currently_ valid -//! 3. Transaction is _currently_ invalid, but could potentially become valid in the future +//! +//! 1. Transaction can _never_ be valid +//! 2. Transaction is _currently_ valid +//! 3. Transaction is _currently_ invalid, but could potentially become valid in the future //! //! However, (2.) and (3.) of a transaction can only be determined on the basis of the current //! state, whereas (1.) holds indefinitely. This means once the state changes (2.) and (3.) need to @@ -15,8 +16,41 @@ //! //! However, the score is also only valid for the current state. //! -//! In essence the transaction pool is made of two separate sub-pools for currently valid (2.) and -//! currently invalid (3.). +//! Furthermore, the following characteristics fall under (3.): +//! +//! a) Nonce of a transaction is higher than the expected nonce for the next transaction of its +//! sender. A distinction is made here whether multiple transactions from the same sender have +//! gapless nonce increments. a)(1) If _no_ transaction is missing in a chain of multiple +//! transactions from the same sender (all nonce in row), all of them can in principle be executed +//! on the current state one after the other. a)(2) If there's a nonce gap, then all +//! transactions after the missing transaction are blocked until the missing transaction arrives. +//! b) Transaction does not meet the dynamic fee cap requirement introduced by EIP-1559: The fee +//! cap of the transaction needs to be no less than the base fee of block. +//! +//! +//! In essence the transaction pool is made of two separate sub-pools: +//! +//! _Pending Pool_: Contains all transactions that are valid on the current state and satisfy +//! (3. a)(1): _No_ nonce gaps _Queued Pool_: Contains all transactions that are currently +//! blocked by missing transactions: (3. a)(2): _With_ nonce gaps +//! +//! To account for the dynamic base fee requirement (3. b) which could render an EIP-1559 and all +//! subsequent transactions of the sender currently invalid, the pending pool itself consists of two +//! queues: +//! +//! _Ready Queue_: Contains all transactions that can be executed on the current state +//! _Parked Queue_: Contains all transactions that either do not currently meet the dynamic +//! base fee requirement or are blocked by a previous transaction that violates it. +//! +//! The classification of transaction in which queue it belongs depends on the current base fee and +//! must be updated after changes: +//! +//! - Base Fee increases: recheck the _Ready Queue_ and evict transactions that don't satisfy +//! the new base fee, or depend on a transaction that no longer satisfies it, and move them +//! to the _Parked Queue_. +//! - Base Fee decreases: recheck the _Parked Queue_ and move all transactions that now satisfy +//! the new base fee to the _Ready Queue_. +//! //! //! Depending on the use case, consumers of the [`TransactionPool`](crate::traits::TransactionPool) //! are interested in (2.) and/or (3.). @@ -88,7 +122,7 @@ impl Pool { transactions: impl IntoIterator, ) -> PoolResult, TransactionValidationResult>> { - // get the actual block number which is required to validate to validate the transactions + // get the actual block number which is required to validate the transactions let block_number = self.resolve_block_number(block_id)?; let outcome = futures::future::join_all( @@ -189,7 +223,7 @@ impl GraphPool { /// nonce gaps). It consists of two parts: `Parked` and `Ready`. /// /// The `Ready` queue contains transactions that are ready to be included in the pending block. - /// With the EIP-1559, transactions can become executable or not without any changes to the + /// With EIP-1559, transactions can become executable or not without any changes to the /// sender's balance or nonce and instead their feeCap determines whether the transaction is /// _currently_ (on the current state) ready or needs to be parked until the feeCap satisfies /// the block's baseFee. From a31929039eb234e6338219b51656876d5e612179 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 5 Oct 2022 12:59:03 +0200 Subject: [PATCH 17/35] refactor: unify transaction associated types --- crates/transaction-pool/src/lib.rs | 2 +- crates/transaction-pool/src/ordering.rs | 6 +- crates/transaction-pool/src/pool/mod.rs | 67 ++++++---- crates/transaction-pool/src/pool/pending.rs | 131 +++++++++++--------- crates/transaction-pool/src/pool/queued.rs | 42 +++---- crates/transaction-pool/src/validate.rs | 2 +- 6 files changed, 136 insertions(+), 114 deletions(-) diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index 1cbcc2acdd0..84e3a1d3d1b 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -58,6 +58,6 @@ where impl TransactionPool for Pool where PoolApi: PoolClient, - Ordering: TransactionOrdering, + Ordering: TransactionOrdering::Transaction>, { } diff --git a/crates/transaction-pool/src/ordering.rs b/crates/transaction-pool/src/ordering.rs index 86bcd4711ba..6c192bfc68f 100644 --- a/crates/transaction-pool/src/ordering.rs +++ b/crates/transaction-pool/src/ordering.rs @@ -1,3 +1,4 @@ +use crate::traits::PoolTransaction; use std::fmt; /// Transaction ordering. @@ -9,6 +10,9 @@ pub trait TransactionOrdering: Send + Sync { /// Priority of a transaction. type Priority: Ord + Clone + Default + fmt::Debug + fmt::LowerHex + Send + Sync; + /// The transaction type to score. + type Transaction: PoolTransaction + Send + Sync; + /// Returns the priority score for the given transaction. - fn priority(&self, transaction: ()) -> Self::Priority; + fn priority(&self, transaction: &Self::Transaction) -> Self::Priority; } diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 33a0dbbe557..a616634bc91 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -95,20 +95,25 @@ mod queued; mod transaction; // Helper type aliases for associated types -type TransactionHashFor = - <::Transaction as PoolTransaction>::Hash; -type TransactionIdFor = - <::Transaction as PoolTransaction>::Id; +pub(crate) type TransactionHashFor = + <::Transaction as PoolTransaction>::Hash; + +pub(crate) type TransactionIdFor = + <::Transaction as PoolTransaction>::Id; /// Shareable Transaction pool. -pub struct Pool { +pub struct Pool { /// Arc'ed instance of the pool internals - pool: Arc>, + pool: Arc>, } // === impl Pool === -impl Pool { +impl Pool +where + P: PoolClient, + T: TransactionOrdering::Transaction>, +{ /// Returns the actual block number for the block id fn resolve_block_number(&self, block_id: &BlockId) -> PoolResult { self.pool.client().ensure_block_number(block_id) @@ -120,7 +125,7 @@ impl Pool { &self, block_id: &BlockId, transactions: impl IntoIterator, - ) -> PoolResult, TransactionValidationResult>> + ) -> PoolResult, TransactionValidationResult>> { // get the actual block number which is required to validate the transactions let block_number = self.resolve_block_number(block_id)?; @@ -143,7 +148,7 @@ impl Pool { block_id: &BlockId, block_number: U64, transaction: P::Transaction, - ) -> (TransactionHashFor

, TransactionValidationResult) { + ) -> (TransactionHashFor, TransactionValidationResult) { let hash = *transaction.hash(); // TODO this is where additional validate checks would go, like banned senders etc... let res = self.pool.client().validate_transaction(block_id, transaction).await; @@ -161,22 +166,26 @@ impl Clone for Pool { } /// Transaction pool internals. -pub struct PoolInner { +pub struct PoolInner { /// Chain/Storage access. client: Arc

, /// The internal pool that manages - pool: RwLock>, + pool: RwLock>, /// Pool settings. config: PoolConfig, /// Listeners for transaction state change events. - event_listeners: RwLock, H256>>, + event_listeners: RwLock, H256>>, /// Listeners for new ready transactions. - added_transaction_listener: Mutex>>>, + added_transaction_listener: Mutex>>>, } // === impl PoolInner === -impl PoolInner { +impl PoolInner +where + P: PoolClient, + T: TransactionOrdering::Transaction>, +{ /// Get client reference. pub fn client(&self) -> &P { &self.client @@ -187,13 +196,11 @@ impl PoolInner { /// /// This pool maintains a dependency graph of transactions and provides the currently ready /// transactions. - -// TODO could unify over `TransactionOrdering::Transaction` -pub struct GraphPool { +pub struct GraphPool { /// How to order transactions. - ordering: Arc, + ordering: Arc, /// Sub-Pool of transactions that are ready and waiting to be executed - pending: PendingTransactions, + pending: PendingTransactions, /// Sub-Pool of transactions that are waiting for state changes that eventually turn them /// valid, so they can be moved in the `pending` pool. queued: QueuedTransactions, @@ -201,14 +208,14 @@ pub struct GraphPool { // === impl PoolInner === -impl GraphPool { +impl GraphPool { /// Returns if the transaction for the given hash is already included in this pool - pub fn contains(&self, tx_hash: &T::Hash) -> bool { + pub fn contains(&self, tx_hash: &TransactionHashFor) -> bool { self.queued.contains(tx_hash) || self.pending.contains(tx_hash) } /// Returns an iterator that yields transactions that are ready to be included in the block. - pub fn ready(&self) -> TransactionsIterator { + pub fn ready(&self) -> TransactionsIterator { self.pending.get_transactions() } @@ -227,7 +234,10 @@ impl GraphPool { /// sender's balance or nonce and instead their feeCap determines whether the transaction is /// _currently_ (on the current state) ready or needs to be parked until the feeCap satisfies /// the block's baseFee. - fn add_transaction(&mut self, tx: ValidPoolTransaction) -> PoolResult> { + fn add_transaction( + &mut self, + tx: ValidPoolTransaction, + ) -> PoolResult> { if self.contains(tx.hash()) { warn!(target: "txpool", "[{:?}] Already added", tx.hash()); return Err(PoolError::AlreadyAdded(Box::new(*tx.hash()))) @@ -254,7 +264,7 @@ impl GraphPool { fn add_pending_transaction( &mut self, tx: QueuedPoolTransaction, - ) -> PoolResult> { + ) -> PoolResult> { let hash = *tx.transaction.hash(); trace!(target: "txpool", "adding pending transaction [{:?}]", hash); let mut pending = AddedPendingTransaction::new(hash); @@ -313,8 +323,8 @@ impl GraphPool { /// And queued transactions might get promoted if the pruned dependencies unlock them. pub fn prune_dependencies( &mut self, - dependencies: impl IntoIterator, - ) -> PruneResult { + dependencies: impl IntoIterator>, + ) -> PruneResult { let mut imports = vec![]; let mut pruned = vec![]; @@ -342,7 +352,10 @@ impl GraphPool { } /// Remove the given transactions from the pool. - pub fn remove_invalid(&mut self, tx_hashes: Vec) -> Vec>> { + pub fn remove_invalid( + &mut self, + tx_hashes: Vec>, + ) -> Vec>> { // early exit in case there is no invalid transactions. if tx_hashes.is_empty() { return vec![] diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index ee0654d8a1d..ee57450ff09 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -1,6 +1,10 @@ use crate::{ - error, error::PoolResult, pool::queued::QueuedPoolTransaction, traits::PoolTransaction, - validate::ValidPoolTransaction, TransactionOrdering, + error, + error::PoolResult, + pool::{queued::QueuedPoolTransaction, TransactionHashFor, TransactionIdFor}, + traits::PoolTransaction, + validate::ValidPoolTransaction, + TransactionOrdering, }; use parking_lot::RwLock; use reth_primitives::{TxHash, H256, U256}; @@ -18,34 +22,34 @@ use tracing::{debug, trace, warn}; /// Each transaction in this pool is valid on its own, i.e. they are not dependent on transaction /// that must be executed first. Each of these transaction can be executed independently on the /// current state -pub(crate) struct PendingTransactions { +pub(crate) struct PendingTransactions { /// Keeps track of transactions inserted in the pool. /// /// This way we can determine when transactions where submitted to the pool. id: u64, /// How to order transactions. - ordering: Arc, + ordering: Arc, /// Dependencies that are provided by `PendingTransaction`s - provided_dependencies: HashMap, + provided_dependencies: HashMap, TransactionHashFor>, /// Pending transactions that are currently on hold until the `baseFee` of the pending block /// changes in favor of the parked transactions: the `pendingBlock.baseFee` must decrease /// before they can be moved to the ready pool and are ready to be executed. - parked: ParkedTransactions, + parked: ParkedTransactions, /// All Transactions that are currently ready. /// /// Meaning, there are no nonce gaps in these transactions and all of them satisfy the /// `baseFee` condition: transaction `maxFeePerGas >= pendingBlock.baseFee` - ready_transactions: Arc>>>, + ready_transactions: Arc, PendingTransaction>>>, /// Independent transactions that can be included directly and don't require other /// transactions. /// /// Sorted by their scoring value. - independent_transactions: BTreeSet>, + independent_transactions: BTreeSet>, } // === impl PendingTransactions === -impl PendingTransactions { +impl PendingTransactions { /// Returns an iterator over all transactions that are _currently_ ready. /// /// 1. The iterator _always_ returns transaction in order: It never returns a transaction with @@ -64,7 +68,7 @@ impl PendingTransactions { /// provides a way to mark transactions that the consumer of this iterator considers invalid. In /// which case the transaction's subgraph is also automatically marked invalid, See (1.). /// Invalid transactions are skipped. - pub fn get_transactions(&self) -> TransactionsIterator { + pub fn get_transactions(&self) -> TransactionsIterator { TransactionsIterator { all: self.ready_transactions.read().clone(), independent: self.independent_transactions.clone(), @@ -84,16 +88,16 @@ impl PendingTransactions { // } /// Returns true if the transaction is part of the queue. - pub fn contains(&self, hash: &T::Hash) -> bool { + pub fn contains(&self, hash: &TransactionHashFor) -> bool { self.ready_transactions.read().contains_key(hash) } /// Returns the transaction for the hash if it's in the ready pool but not yet mined - pub(crate) fn get(&self, hash: &T::Hash) -> Option> { + pub(crate) fn get(&self, hash: &TransactionHashFor) -> Option> { self.ready_transactions.read().get(hash).cloned() } - pub fn provided_dependencies(&self) -> &HashMap { + pub fn provided_dependencies(&self) -> &HashMap, TransactionHashFor> { &self.provided_dependencies } @@ -115,7 +119,7 @@ impl PendingTransactions { pub fn add_transaction( &mut self, tx: QueuedPoolTransaction, - ) -> PoolResult>>> { + ) -> PoolResult>>> { assert!(tx.is_satisfied(), "transaction must be ready",); assert!( !self.ready_transactions.read().contains_key(tx.transaction.hash()), @@ -149,7 +153,8 @@ impl PendingTransactions { self.provided_dependencies.insert(mark, hash); } - let priority = self.ordering.priority(()); + let priority = self.ordering.priority(&tx.transaction.transaction); + let transaction = PoolTransactionRef { submission_id, transaction: tx.transaction, priority }; @@ -169,8 +174,9 @@ impl PendingTransactions { /// Removes and returns those transactions that got replaced by the `tx` fn replaced_transactions( &mut self, - tx: &ValidPoolTransaction, - ) -> PoolResult<(Vec>>, Vec)> { + tx: &ValidPoolTransaction, + ) -> PoolResult<(Vec>>, Vec>)> + { // check if we are replacing transactions let remove_hashes: HashSet<_> = tx.provides.iter().filter_map(|mark| self.provided_dependencies.get(mark)).collect(); @@ -219,8 +225,8 @@ impl PendingTransactions { /// This will also remove all transactions that depend on those. pub fn clear_transactions( &mut self, - tx_hashes: &[T::Hash], - ) -> Vec>> { + tx_hashes: &[TransactionHashFor], + ) -> Vec>> { self.remove_with_dependencies(tx_hashes.to_vec(), None) } @@ -228,7 +234,10 @@ impl PendingTransactions { /// /// This will also remove all transactions that lead to the transaction that provides the /// id. - pub fn prune_dependencies(&mut self, id: T::Id) -> Vec>> { + pub fn prune_dependencies( + &mut self, + id: TransactionIdFor, + ) -> Vec>> { let mut removed_tx = vec![]; // the dependencies to remove @@ -250,7 +259,7 @@ impl PendingTransactions { let hash = tx.hash(); let mut ready = self.ready_transactions.write(); - let mut previous_dependency = |dependency| -> Option> { + let mut previous_dependency = |dependency| -> Option>> { let prev_hash = self.provided_dependencies.get(dependency)?; let tx2 = ready.get_mut(prev_hash)?; // remove hash @@ -303,9 +312,9 @@ impl PendingTransactions { /// the given filter set. pub fn remove_with_dependencies( &mut self, - mut tx_hashes: Vec, - dependency_filter: Option>, - ) -> Vec>> { + mut tx_hashes: Vec>, + dependency_filter: Option>>, + ) -> Vec>> { let mut removed = Vec::new(); let mut ready = self.ready_transactions.write(); @@ -355,25 +364,25 @@ impl PendingTransactions { /// A transaction that is ready to be included in a block. #[derive(Debug)] -pub(crate) struct PendingTransaction { +pub(crate) struct PendingTransaction { /// Reference to the actual transaction. - transaction: PoolTransactionRef, + transaction: PoolTransactionRef, /// Tracks the transactions that get unlocked by this transaction. - unlocks: Vec, + unlocks: Vec>, /// Amount of required dependencies that are inherently provided. requires_offset: usize, } // == impl PendingTransaction === -impl PendingTransaction { +impl PendingTransaction { /// Returns all ids this transaction satisfies. - pub fn provides(&self) -> &[T::Id] { + pub fn provides(&self) -> &[TransactionIdFor] { &self.transaction.transaction.provides } } -impl Clone for PendingTransaction { +impl Clone for PendingTransaction { fn clone(&self) -> Self { Self { transaction: self.transaction.clone(), @@ -385,16 +394,16 @@ impl Clone for PendingTransaction { +pub struct PoolTransactionRef { /// Actual transaction. - pub transaction: Arc>, + pub transaction: Arc>, /// Identifier that tags when transaction was submitted in the pool. pub submission_id: u64, /// The priority value assigned by the used `Ordering` function. - pub priority: O::Priority, + pub priority: T::Priority, } -impl Clone for PoolTransactionRef { +impl Clone for PoolTransactionRef { fn clone(&self) -> Self { Self { transaction: Arc::clone(&self.transaction), @@ -404,21 +413,21 @@ impl Clone for PoolTransactionRef Eq for PoolTransactionRef {} +impl Eq for PoolTransactionRef {} -impl PartialEq for PoolTransactionRef { +impl PartialEq for PoolTransactionRef { fn eq(&self, other: &Self) -> bool { self.cmp(other) == Ordering::Equal } } -impl PartialOrd for PoolTransactionRef { +impl PartialOrd for PoolTransactionRef { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) } } -impl Ord for PoolTransactionRef { +impl Ord for PoolTransactionRef { fn cmp(&self, other: &Self) -> Ordering { // This compares by `priority` and only if two tx have the exact same priority this compares // the unique `submission_id`. This ensures that transactions with same priority are not @@ -430,55 +439,55 @@ impl Ord for PoolTransactionRef { +struct ParkedTransactions { /// Keeps track of transactions inserted in the pool. /// /// This way we can determine when transactions where submitted to the pool. id: u64, /// All transactions that are currently parked due to their fee. - parked_transactions: HashMap>, + parked_transactions: HashMap, ParkedTransaction>, /// Same transactions but sorted by their fee and priority - sorted_transactions: BTreeSet>, + sorted_transactions: BTreeSet>, } /// A transaction that is ready to be included in a block. #[derive(Debug, Clone)] -pub struct ParkedTransaction { +pub struct ParkedTransaction { /// Reference to the actual transaction. - transaction: PoolTransactionRef, + transaction: PoolTransactionRef, /// Tracks the transactions that get unlocked by this transaction. - unlocks: Vec, + unlocks: Vec>, /// Amount of required dependencies that are inherently provided requires_offset: usize, } /// A reference to a currently _parked_ transaction. -struct ParkedTransactionRef { +struct ParkedTransactionRef { /// Actual transaction. - transaction: Arc>, + transaction: Arc>, /// Identifier that tags when transaction was submitted in the pool. submission_id: u64, /// The priority value assigned by the used `Ordering` function. - priority: O::Priority, + priority: T::Priority, /// EIP-1559 Max base fee the caller is willing to pay. max_fee_per_gas: U256, } -impl Eq for ParkedTransactionRef {} +impl Eq for ParkedTransactionRef {} -impl PartialEq for ParkedTransactionRef { +impl PartialEq for ParkedTransactionRef { fn eq(&self, other: &Self) -> bool { self.cmp(other) == Ordering::Equal } } -impl PartialOrd for ParkedTransactionRef { +impl PartialOrd for ParkedTransactionRef { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) } } -impl Ord for ParkedTransactionRef { +impl Ord for ParkedTransactionRef { fn cmp(&self, other: &Self) -> Ordering { // This compares the `max_fee_per_gas` value of the transaction self.max_fee_per_gas @@ -488,22 +497,22 @@ impl Ord for ParkedTransactionRef { - all: HashMap>, - awaiting: HashMap)>, - independent: BTreeSet>, - invalid: HashSet, +pub struct TransactionsIterator { + all: HashMap, PendingTransaction>, + awaiting: HashMap, (usize, PoolTransactionRef)>, + independent: BTreeSet>, + invalid: HashSet>, } // == impl TransactionsIterator == -impl TransactionsIterator { +impl TransactionsIterator { /// Mark the transaction as invalid. /// /// As a consequence, all values that depend on the invalid one will be skipped. /// When given transaction is not in the pool it has no effect. /// When invoked on a fully drained iterator it has no effect either. - pub fn mark_invalid(&mut self, tx: &Arc>) { + pub fn mark_invalid(&mut self, tx: &Arc>) { if let Some(invalid_transaction) = self.all.get(tx.hash()) { debug!( target: "txpool", @@ -518,7 +527,7 @@ impl TransactionsIterator { /// Depending on number of satisfied requirements insert given ref /// either to awaiting set or to best set. - fn independent_or_awaiting(&mut self, satisfied: usize, tx_ref: PoolTransactionRef) { + fn independent_or_awaiting(&mut self, satisfied: usize, tx_ref: PoolTransactionRef) { if satisfied >= tx_ref.transaction.depends_on.len() { // If we have satisfied all deps insert to best self.independent.insert(tx_ref); @@ -529,8 +538,8 @@ impl TransactionsIterator { } } -impl Iterator for TransactionsIterator { - type Item = Arc>; +impl Iterator for TransactionsIterator { + type Item = Arc>; fn next(&mut self) -> Option { loop { diff --git a/crates/transaction-pool/src/pool/queued.rs b/crates/transaction-pool/src/pool/queued.rs index d734d6598df..23701f8b5f3 100644 --- a/crates/transaction-pool/src/pool/queued.rs +++ b/crates/transaction-pool/src/pool/queued.rs @@ -1,9 +1,4 @@ -use crate::{ - error, - error::{PoolError, PoolResult}, - traits::PoolTransaction, - validate::ValidPoolTransaction, -}; +use crate::{error, error::{PoolError, PoolResult}, traits::PoolTransaction, TransactionOrdering, validate::ValidPoolTransaction}; use std::{ collections::{HashMap, HashSet}, fmt, @@ -11,6 +6,7 @@ use std::{ time::Instant, }; use tracing::warn; +use crate::pool::{TransactionHashFor, TransactionIdFor}; /// A pool of transactions that are not ready on the current state and are waiting for state changes /// that turn them valid. @@ -19,18 +15,18 @@ use tracing::warn; /// transaction to arrive that closes the nonce gap. /// /// Keeps a set of transactions that are waiting until their dependencies are unlocked. -pub(crate) struct QueuedTransactions { +pub(crate) struct QueuedTransactions { /// Dependencies that aren't yet provided by any transaction. - required_dependencies: HashMap>, + required_dependencies: HashMap, HashSet>>, /// Mapping of the dependencies of a transaction to the hash of the transaction, - waiting_dependencies: HashMap, T::Hash>, + waiting_dependencies: HashMap>, TransactionHashFor>, /// Transactions that are not ready yet are waiting for another tx to finish, - waiting_queue: HashMap>, + waiting_queue: HashMap, QueuedPoolTransaction>, } // == impl QueuedTransactions == -impl QueuedTransactions { +impl QueuedTransactions { /// Returns the number of transactions that are currently waiting in this pool for new /// transactions to satisfy their dependencies. pub fn len(&self) -> usize { @@ -43,7 +39,7 @@ impl QueuedTransactions { } /// Returns an iterator over all transactions waiting in this pool. - pub fn transactions(&self) -> impl Iterator>> + '_ { + pub fn transactions(&self) -> impl Iterator>> + '_ { self.waiting_queue.values().map(|tx| Arc::clone(&tx.transaction)) } @@ -86,12 +82,12 @@ impl QueuedTransactions { } /// Returns true if given transaction is part of the queue - pub fn contains(&self, hash: &T::Hash) -> bool { + pub fn contains(&self, hash: &TransactionHashFor) -> bool { self.waiting_queue.contains_key(hash) } /// Returns the transaction for the hash if it's waiting - pub fn get(&self, hash: &T::Hash) -> Option<&QueuedPoolTransaction> { + pub fn get(&self, hash: &TransactionHashFor) -> Option<&QueuedPoolTransaction> { self.waiting_queue.get(hash) } @@ -101,7 +97,7 @@ impl QueuedTransactions { /// moved to the ready queue. pub fn satisfy_and_unlock( &mut self, - dependencies: impl IntoIterator>, + dependencies: impl IntoIterator>>, ) -> Vec> { let mut unlocked_ready = Vec::new(); for dependency in dependencies { @@ -127,7 +123,7 @@ impl QueuedTransactions { /// Removes the transactions associated with the given hashes /// /// Returns all removed transactions. - pub fn remove(&mut self, hashes: Vec) -> Vec>> { + pub fn remove(&mut self, hashes: Vec>) -> Vec>> { let mut removed = vec![]; for hash in hashes { if let Some(waiting_tx) = self.waiting_queue.remove(&hash) { @@ -153,27 +149,27 @@ impl QueuedTransactions { /// A transaction submitted to the pool. #[derive(Clone)] -pub struct QueuedPoolTransaction { +pub struct QueuedPoolTransaction { /// The actual validated transaction. - pub transaction: Arc>, + pub transaction: Arc>, /// Transactions required for and have not been satisfied yet by other transactions in the /// pool. /// /// This will be an empty list if there are no nonce gaps across multiple transactions of the /// same sender in the pool. If there are gaps, this will include the missing transactions. - pub missing_dependencies: HashSet, + pub missing_dependencies: HashSet>, /// Timestamp when the tx was added. pub added_at: Instant, } // === impl QuQueuedPoolTransaction === -impl QueuedPoolTransaction { +impl QueuedPoolTransaction { /// Creates a new `QueuedPoolTransaction`. /// /// Determines the dependent transaction that are still missing before this transaction can be /// moved to the queue. - pub fn new(transaction: ValidPoolTransaction, provided: &HashMap) -> Self { + pub fn new(transaction: ValidPoolTransaction, provided: &HashMap, TransactionHashFor>) -> Self { let missing_dependencies = transaction .depends_on .iter() @@ -189,7 +185,7 @@ impl QueuedPoolTransaction { } /// Removes the required dependency. - pub fn satisfy(&mut self, id: &T::Id) { + pub fn satisfy(&mut self, id: &TransactionIdFor) { self.missing_dependencies.remove(id); } @@ -199,7 +195,7 @@ impl QueuedPoolTransaction { } } -impl fmt::Debug for QueuedPoolTransaction { +impl fmt::Debug for QueuedPoolTransaction { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { write!(fmt, "QueuedPoolTransaction {{ ")?; write!(fmt, "added_at: {:?}, ", self.added_at)?; diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index 1e1a049771d..7e511a9fca7 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -11,7 +11,7 @@ pub type TransactionValidationResult = /// Provides support for validating transaction at any given state of the chain #[async_trait::async_trait] pub trait TransactionValidator: Send + Sync { - /// The transaction type to validate + /// The transaction type to validate. type Transaction: PoolTransaction + Send + Sync; /// Validates the transaction and returns a validated outcome From fc0e773d56135670143e5f71c70dff0567dfee89 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 5 Oct 2022 14:16:24 +0200 Subject: [PATCH 18/35] feat: add basic event handling --- crates/transaction-pool/src/pool/events.rs | 7 +- crates/transaction-pool/src/pool/listener.rs | 78 ++++++++++++++++-- crates/transaction-pool/src/pool/mod.rs | 86 +++++++++++++++++++- crates/transaction-pool/src/pool/queued.rs | 39 +++++++-- crates/transaction-pool/src/validate.rs | 2 + 5 files changed, 193 insertions(+), 19 deletions(-) diff --git a/crates/transaction-pool/src/pool/events.rs b/crates/transaction-pool/src/pool/events.rs index 981efdf116a..c7e860c01de 100644 --- a/crates/transaction-pool/src/pool/events.rs +++ b/crates/transaction-pool/src/pool/events.rs @@ -1,14 +1,17 @@ +use reth_primitives::H256; use serde::{Deserialize, Serialize}; /// Various events that describe status changes of a transaction. #[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)] -pub enum TransactionEvent { +pub enum TransactionEvent { + /// Transaction has been added to the ready queue. + Ready, /// Transaction has been added to the pending pool. Pending, /// Transaction has been added to the queued pool. Queued, /// Transaction has been included in the block belonging to this hash. - Included(BlockHash), + Included(H256), /// Transaction has been replaced by the transaction belonging to the hash. /// /// E.g. same (sender + nonce) pair diff --git a/crates/transaction-pool/src/pool/listener.rs b/crates/transaction-pool/src/pool/listener.rs index 4139387b4ae..93d44ed8480 100644 --- a/crates/transaction-pool/src/pool/listener.rs +++ b/crates/transaction-pool/src/pool/listener.rs @@ -3,16 +3,82 @@ use crate::pool::events::TransactionEvent; use futures::channel::mpsc::UnboundedSender; use std::{collections::HashMap, hash}; +use tracing::trace; + +type EventSink = UnboundedSender>; /// Transaction pool event listeners. -pub struct PoolEventListener { - /// All listeners for certain transactions. - listeners: HashMap>>, +pub struct PoolEventListener { + /// All listeners for certain transaction events. + listeners: HashMap>, +} + +impl PoolEventListener { + /// Calls the notification callback with the `PoolEventListenerSender` that belongs to the hash. + fn notify_with(&mut self, hash: &Hash, callback: F) + where + F: FnOnce(&mut PoolEventNotifier), + { + let is_done = if let Some(sink) = self.listeners.get_mut(hash) { + callback(sink); + sink.is_done() + } else { + false + }; + + if is_done { + self.listeners.remove(hash); + } + } + + /// Notify listeners about a transaction that was added to the ready queue. + pub fn ready(&mut self, tx: &Hash, replaced: Option<&Hash>) { + self.notify_with(tx, |notifier| notifier.ready()); + + if let Some(replaced) = replaced { + // notify listeners that this transaction was replaced + self.notify_with(replaced, |notifier| notifier.replaced(tx.clone())); + } + } + + /// Notify listeners about a transaction that was added to the queued pool. + pub fn queued(&mut self, tx: &Hash) { + self.notify_with(tx, |notifier| notifier.queued()); + } } /// Sender half(s) of the event channels for a specific transaction #[derive(Debug)] -pub struct PoolEventListenerSender { - /// Corresponding receiver half(s) for the transaction - receivers: Vec>>, +pub struct PoolEventNotifier { + /// Tracks whether the transaction this notifier can stop because the transaction was + /// completed, or removed. + is_done: bool, + /// Corresponding sender half(s) for event listener channel + senders: Vec>, +} + +impl PoolEventNotifier { + fn notify(&mut self, event: TransactionEvent) { + self.senders.retain(|sender| sender.unbounded_send(event.clone()).is_ok()) + } + + fn is_done(&self) -> bool { + self.senders.is_empty() || self.is_done + } + + /// Transaction became ready. + pub fn ready(&mut self) { + self.notify(TransactionEvent::Pending) + } + + /// Transaction was moved to the queued pool + pub fn queued(&mut self) { + self.notify(TransactionEvent::Queued) + } + + /// Transaction was replaced with the given transaction + pub fn replaced(&mut self, hash: Hash) { + self.notify(TransactionEvent::Replaced(hash)); + self.is_done = true; + } } diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index a616634bc91..568bb090247 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -78,7 +78,7 @@ use crate::{ validate::{TransactionValidationResult, ValidPoolTransaction}, BlockId, PoolClient, PoolConfig, TransactionOrdering, TransactionValidator, }; -use futures::channel::mpsc::Sender; +use futures::channel::mpsc::{channel, Receiver, Sender}; use parking_lot::{Mutex, RwLock}; use reth_primitives::{TxHash, H256, U64}; use std::{ @@ -173,10 +173,10 @@ pub struct PoolInner { pool: RwLock>, /// Pool settings. config: PoolConfig, - /// Listeners for transaction state change events. - event_listeners: RwLock, H256>>, + /// Manages listeners for transaction state change events. + event_listener: RwLock>>, /// Listeners for new ready transactions. - added_transaction_listener: Mutex>>>, + ready_transaction_listener: Mutex>>>, } // === impl PoolInner === @@ -190,6 +190,74 @@ where pub fn client(&self) -> &P { &self.client } + + /// Adds a new transaction listener to the pool that gets notified about every new ready + /// transaction + pub fn add_ready_listener(&self) -> Receiver> { + const TX_LISTENER_BUFFER_SIZE: usize = 2048; + let (tx, rx) = channel(TX_LISTENER_BUFFER_SIZE); + self.ready_transaction_listener.lock().push(tx); + rx + } + + /// Resubmits transactions back into the pool. + pub fn resubmit( + &self, + transactions: HashMap, ValidPoolTransaction>, + ) { + unimplemented!() + } + + /// Add a single validated transaction into the pool. + fn add_transaction( + &self, + tx: ValidPoolTransaction, + ) -> PoolResult> { + let added = self.pool.write().add_transaction(tx)?; + + if let Some(ready) = added.as_ready() { + self.on_new_ready_transaction(ready); + } + + self.notify_event_listeners(&added); + + Ok(*added.hash()) + } + + /// Notify all listeners about the new transaction. + fn on_new_ready_transaction(&self, ready: &TransactionHashFor) { + let mut transaction_listeners = self.ready_transaction_listener.lock(); + transaction_listeners.retain_mut(|listener| match listener.try_send(*ready) { + Ok(()) => true, + Err(e) => { + if e.is_full() { + warn!( + target: "txpool", + "[{:?}] dropping full ready transaction listener", + ready, + ); + true + } else { + false + } + } + }); + } + + /// Fire events for the newly added transaction. + fn notify_event_listeners(&self, tx: &AddedTransaction) { + let mut listener = self.event_listener.write(); + + match tx { + AddedTransaction::Pending(tx) => { + listener.ready(&tx.hash, None); + // TODO more listeners for discarded, removed etc... + } + AddedTransaction::Queued { hash } => { + listener.queued(hash); + } + } + } } /// A pool that only manages transactions. @@ -449,6 +517,16 @@ pub enum AddedTransaction { } impl AddedTransaction { + /// Returns the hash of the transaction if it's ready + pub fn as_ready(&self) -> Option<&T::Hash> { + if let AddedTransaction::Pending(tx) = self { + Some(&tx.hash) + } else { + None + } + } + + /// Returns the hash of the transaction pub fn hash(&self) -> &T::Hash { match self { AddedTransaction::Pending(tx) => &tx.hash, diff --git a/crates/transaction-pool/src/pool/queued.rs b/crates/transaction-pool/src/pool/queued.rs index 23701f8b5f3..446e7701c12 100644 --- a/crates/transaction-pool/src/pool/queued.rs +++ b/crates/transaction-pool/src/pool/queued.rs @@ -1,4 +1,11 @@ -use crate::{error, error::{PoolError, PoolResult}, traits::PoolTransaction, TransactionOrdering, validate::ValidPoolTransaction}; +use crate::{ + error, + error::{PoolError, PoolResult}, + pool::{TransactionHashFor, TransactionIdFor}, + traits::PoolTransaction, + validate::ValidPoolTransaction, + TransactionOrdering, +}; use std::{ collections::{HashMap, HashSet}, fmt, @@ -6,7 +13,6 @@ use std::{ time::Instant, }; use tracing::warn; -use crate::pool::{TransactionHashFor, TransactionIdFor}; /// A pool of transactions that are not ready on the current state and are waiting for state changes /// that turn them valid. @@ -39,7 +45,9 @@ impl QueuedTransactions { } /// Returns an iterator over all transactions waiting in this pool. - pub fn transactions(&self) -> impl Iterator>> + '_ { + pub fn transactions( + &self, + ) -> impl Iterator>> + '_ { self.waiting_queue.values().map(|tx| Arc::clone(&tx.transaction)) } @@ -87,8 +95,19 @@ impl QueuedTransactions { } /// Returns the transaction for the hash if it's waiting - pub fn get(&self, hash: &TransactionHashFor) -> Option<&QueuedPoolTransaction> { - self.waiting_queue.get(hash) + pub fn get(&self, tx_hash: &TransactionHashFor) -> Option<&QueuedPoolTransaction> { + self.waiting_queue.get(tx_hash) + } + + /// Returns the transactions for the given hashes, `None` if no transaction exists + pub fn get_all( + &self, + tx_hashes: &[TransactionHashFor], + ) -> Vec>>> { + tx_hashes + .iter() + .map(|hash| self.waiting_queue.get(hash).map(|tx| Arc::clone(&tx.transaction))) + .collect() } /// This will check off the dependencies of queued transactions. @@ -123,7 +142,10 @@ impl QueuedTransactions { /// Removes the transactions associated with the given hashes /// /// Returns all removed transactions. - pub fn remove(&mut self, hashes: Vec>) -> Vec>> { + pub fn remove( + &mut self, + hashes: Vec>, + ) -> Vec>> { let mut removed = vec![]; for hash in hashes { if let Some(waiting_tx) = self.waiting_queue.remove(&hash) { @@ -169,7 +191,10 @@ impl QueuedPoolTransaction { /// /// Determines the dependent transaction that are still missing before this transaction can be /// moved to the queue. - pub fn new(transaction: ValidPoolTransaction, provided: &HashMap, TransactionHashFor>) -> Self { + pub fn new( + transaction: ValidPoolTransaction, + provided: &HashMap, TransactionHashFor>, + ) -> Self { let missing_dependencies = transaction .depends_on .iter() diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index 7e511a9fca7..9b45e81284d 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -55,6 +55,8 @@ pub struct ValidPoolTransaction { /// This contains the inverse of `depends_on` which provides the dependencies this transaction /// unlocks once it's mined. pub provides: Vec, + /// Whether to propagate the transaction. + pub propagate: bool, // TODO add a block timestamp that marks validity } From 957e3302e5786370a9c1bd09b539e39e67e77532 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 5 Oct 2022 14:20:10 +0200 Subject: [PATCH 19/35] feat: more helper functions --- crates/transaction-pool/src/pool/mod.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 568bb090247..19e12a38429 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -224,6 +224,18 @@ where Ok(*added.hash()) } + /// Adds all transactions in the iterator to the pool, returning a list of results. + pub fn add_transactions( + &self, + transactions: impl IntoIterator>, + ) -> Vec>> { + let added = transactions.into_iter().map(|tx| self.add_transaction(tx)).collect::>(); + + // TODO check pool limits + + added + } + /// Notify all listeners about the new transaction. fn on_new_ready_transaction(&self, ready: &TransactionHashFor) { let mut transaction_listeners = self.ready_transaction_listener.lock(); From 9fdea85de9c0af0282757169083a7246f322894b Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 5 Oct 2022 14:38:05 +0200 Subject: [PATCH 20/35] style: cleanup, clippy fixes --- crates/transaction-pool/src/client.rs | 6 +-- crates/transaction-pool/src/error.rs | 5 +- crates/transaction-pool/src/lib.rs | 27 +++++------ crates/transaction-pool/src/pool/listener.rs | 15 +++--- crates/transaction-pool/src/pool/mod.rs | 25 +++++----- crates/transaction-pool/src/pool/pending.rs | 51 +++++++++++--------- crates/transaction-pool/src/pool/queued.rs | 38 +++++++-------- crates/transaction-pool/src/validate.rs | 6 +-- 8 files changed, 85 insertions(+), 88 deletions(-) diff --git a/crates/transaction-pool/src/client.rs b/crates/transaction-pool/src/client.rs index f9cdc5b1cf0..ef63f5238a4 100644 --- a/crates/transaction-pool/src/client.rs +++ b/crates/transaction-pool/src/client.rs @@ -1,14 +1,10 @@ //! Provides access to the chain's storage use crate::{ - error, error::{PoolError, PoolResult}, - traits, - traits::PoolTransaction, validate::TransactionValidator, }; use reth_primitives::{BlockId, U64}; -use std::hash; /// The interface used to interact with the blockchain and access storage. #[async_trait::async_trait] @@ -23,6 +19,6 @@ pub trait PoolClient: Send + Sync + TransactionValidator { /// was found fn ensure_block_number(&self, block_id: &BlockId) -> PoolResult { self.convert_block_id(block_id) - .and_then(|number| number.ok_or_else(|| PoolError::BlockNumberNotFound(*block_id))) + .and_then(|number| number.ok_or(PoolError::BlockNumberNotFound(*block_id))) } } diff --git a/crates/transaction-pool/src/error.rs b/crates/transaction-pool/src/error.rs index 1ae937649bf..dee11ec5a7d 100644 --- a/crates/transaction-pool/src/error.rs +++ b/crates/transaction-pool/src/error.rs @@ -10,12 +10,11 @@ pub type PoolResult = Result; pub enum PoolError { /// Thrown if a replacement transaction's gas price is below the already imported transaction #[error("Tx: insufficient gas price to replace existing transaction")] - // #[error("Tx: [{0:?}] insufficient gas price to replace existing transaction")] - // ReplacementUnderpriced(Box), ReplacementUnderpriced, - // TODO make error generic over `Transaction` + /// Encountered a transaction that was already added into the poll #[error("[{0:?}] Already added")] AlreadyAdded(Box), + /// Encountered a cycle in the graph pool #[error("Transaction with cyclic dependent transactions")] CyclicTransaction, /// Thrown if no number was found for the given block id diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index 84e3a1d3d1b..9d267854488 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -1,4 +1,4 @@ -#![warn(missing_debug_implementations, missing_docs, unreachable_pub)] +#![warn(missing_docs)] // unreachable_pub, missing_debug_implementations #![deny(unused_must_use, rust_2018_idioms)] #![doc(test( no_crate_inject, @@ -7,26 +7,23 @@ //! Reth's transaction pool implementation -pub mod error; - -mod client; -mod validate; - -pub use client::PoolClient; -use parking_lot::RwLock; +use reth_primitives::BlockId; use std::sync::Arc; +mod client; mod config; -pub use config::PoolConfig; -use reth_primitives::BlockId; - +pub mod error; mod ordering; - pub mod pool; - mod traits; +mod validate; + pub use crate::{ - ordering::TransactionOrdering, traits::TransactionPool, validate::TransactionValidator, + client::PoolClient, + config::PoolConfig, + ordering::TransactionOrdering, + traits::{PoolTransaction, TransactionPool}, + validate::TransactionValidator, }; /// A generic, customizable `TransactionPool` implementation. @@ -49,7 +46,7 @@ where Ordering: TransactionOrdering, { /// Creates a new `Pool` with the given config and chain api - pub fn new(config: PoolConfig, api: Arc) -> Self { + pub fn new(_config: PoolConfig, _api: Arc) -> Self { unimplemented!() } } diff --git a/crates/transaction-pool/src/pool/listener.rs b/crates/transaction-pool/src/pool/listener.rs index 93d44ed8480..b3dde002603 100644 --- a/crates/transaction-pool/src/pool/listener.rs +++ b/crates/transaction-pool/src/pool/listener.rs @@ -3,12 +3,11 @@ use crate::pool::events::TransactionEvent; use futures::channel::mpsc::UnboundedSender; use std::{collections::HashMap, hash}; -use tracing::trace; type EventSink = UnboundedSender>; /// Transaction pool event listeners. -pub struct PoolEventListener { +pub(crate) struct PoolEventListener { /// All listeners for certain transaction events. listeners: HashMap>, } @@ -32,7 +31,7 @@ impl PoolEventListener { } /// Notify listeners about a transaction that was added to the ready queue. - pub fn ready(&mut self, tx: &Hash, replaced: Option<&Hash>) { + pub(crate) fn ready(&mut self, tx: &Hash, replaced: Option<&Hash>) { self.notify_with(tx, |notifier| notifier.ready()); if let Some(replaced) = replaced { @@ -42,14 +41,14 @@ impl PoolEventListener { } /// Notify listeners about a transaction that was added to the queued pool. - pub fn queued(&mut self, tx: &Hash) { + pub(crate) fn queued(&mut self, tx: &Hash) { self.notify_with(tx, |notifier| notifier.queued()); } } /// Sender half(s) of the event channels for a specific transaction #[derive(Debug)] -pub struct PoolEventNotifier { +struct PoolEventNotifier { /// Tracks whether the transaction this notifier can stop because the transaction was /// completed, or removed. is_done: bool, @@ -67,17 +66,17 @@ impl PoolEventNotifier { } /// Transaction became ready. - pub fn ready(&mut self) { + fn ready(&mut self) { self.notify(TransactionEvent::Pending) } /// Transaction was moved to the queued pool - pub fn queued(&mut self) { + fn queued(&mut self) { self.notify(TransactionEvent::Queued) } /// Transaction was replaced with the given transaction - pub fn replaced(&mut self, hash: Hash) { + fn replaced(&mut self, hash: Hash) { self.notify(TransactionEvent::Replaced(hash)); self.is_done = true; } diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 19e12a38429..2dbaf518c84 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -67,11 +67,10 @@ //! transactions are _currently_ waiting for state changes that eventually move them into //! category (2.) and become pending. use crate::{ - error, error::{PoolError, PoolResult}, pool::{ listener::PoolEventListener, - pending::{PendingTransactions, TransactionsIterator}, + pending::PendingTransactions, queued::{QueuedPoolTransaction, QueuedTransactions}, }, traits::PoolTransaction, @@ -80,7 +79,7 @@ use crate::{ }; use futures::channel::mpsc::{channel, Receiver, Sender}; use parking_lot::{Mutex, RwLock}; -use reth_primitives::{TxHash, H256, U64}; +use reth_primitives::U64; use std::{ collections::{HashMap, VecDeque}, fmt, @@ -94,6 +93,9 @@ mod pending; mod queued; mod transaction; +pub use events::TransactionEvent; +pub use pending::TransactionsIterator; + // Helper type aliases for associated types pub(crate) type TransactionHashFor = <::Transaction as PoolTransaction>::Hash; @@ -146,12 +148,12 @@ where async fn validate( &self, block_id: &BlockId, - block_number: U64, + _block_number: U64, transaction: P::Transaction, ) -> (TransactionHashFor, TransactionValidationResult) { - let hash = *transaction.hash(); + let _hash = *transaction.hash(); // TODO this is where additional validate checks would go, like banned senders etc... - let res = self.pool.client().validate_transaction(block_id, transaction).await; + let _res = self.pool.client().validate_transaction(block_id, transaction).await; // TODO blockstamp the transaction @@ -203,7 +205,7 @@ where /// Resubmits transactions back into the pool. pub fn resubmit( &self, - transactions: HashMap, ValidPoolTransaction>, + _transactions: HashMap, ValidPoolTransaction>, ) { unimplemented!() } @@ -229,11 +231,9 @@ where &self, transactions: impl IntoIterator>, ) -> Vec>> { - let added = transactions.into_iter().map(|tx| self.add_transaction(tx)).collect::>(); - // TODO check pool limits - added + transactions.into_iter().map(|tx| self.add_transaction(tx)).collect::>() } /// Notify all listeners about the new transaction. @@ -493,6 +493,7 @@ impl fmt::Debug for PruneResult { } } +/// Tracks an added transaction and all graph changes caused by adding it. #[derive(Debug, Clone)] pub struct AddedPendingTransaction { /// the hash of the submitted transaction @@ -506,7 +507,8 @@ pub struct AddedPendingTransaction { } impl AddedPendingTransaction { - pub fn new(hash: T::Hash) -> Self { + /// Create a new, empty transaction. + fn new(hash: T::Hash) -> Self { Self { hash, promoted: Default::default(), @@ -516,6 +518,7 @@ impl AddedPendingTransaction { } } +/// Represents a transaction that was added into the pool and its state #[derive(Debug, Clone)] pub enum AddedTransaction { /// Transaction was successfully added and moved to the pending pool. diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index ee57450ff09..218a6528d66 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -1,20 +1,23 @@ use crate::{ - error, error::PoolResult, pool::{queued::QueuedPoolTransaction, TransactionHashFor, TransactionIdFor}, - traits::PoolTransaction, validate::ValidPoolTransaction, TransactionOrdering, }; use parking_lot::RwLock; -use reth_primitives::{TxHash, H256, U256}; +use reth_primitives::U256; use std::{ cmp::Ordering, collections::{BTreeSet, HashMap, HashSet}, - fmt, sync::Arc, }; -use tracing::{debug, trace, warn}; +use tracing::debug; + +/// Type alias for replaced transactions +pub(crate) type ReplacedTransactions = ( + Vec::Transaction>>>, + Vec>, +); /// A pool of validated transactions that are ready on the current state and are waiting to be /// included in a block. @@ -68,7 +71,7 @@ impl PendingTransactions { /// provides a way to mark transactions that the consumer of this iterator considers invalid. In /// which case the transaction's subgraph is also automatically marked invalid, See (1.). /// Invalid transactions are skipped. - pub fn get_transactions(&self) -> TransactionsIterator { + pub(crate) fn get_transactions(&self) -> TransactionsIterator { TransactionsIterator { all: self.ready_transactions.read().clone(), independent: self.independent_transactions.clone(), @@ -78,7 +81,7 @@ impl PendingTransactions { } // /// Returns an iterator over all transactions - // pub fn get_transactions(&self) -> TransactionsIterator { + // pub(crate) fn get_transactions(&self) -> TransactionsIterator { // TransactionsIterator { // all: self.ready_tx.read().clone(), // independent: self.independent_transactions.clone(), @@ -88,7 +91,7 @@ impl PendingTransactions { // } /// Returns true if the transaction is part of the queue. - pub fn contains(&self, hash: &TransactionHashFor) -> bool { + pub(crate) fn contains(&self, hash: &TransactionHashFor) -> bool { self.ready_transactions.read().contains_key(hash) } @@ -97,7 +100,9 @@ impl PendingTransactions { self.ready_transactions.read().get(hash).cloned() } - pub fn provided_dependencies(&self) -> &HashMap, TransactionHashFor> { + pub(crate) fn provided_dependencies( + &self, + ) -> &HashMap, TransactionHashFor> { &self.provided_dependencies } @@ -116,7 +121,7 @@ impl PendingTransactions { /// /// if the pending transaction is not ready /// or the transaction is already included - pub fn add_transaction( + pub(crate) fn add_transaction( &mut self, tx: QueuedPoolTransaction, ) -> PoolResult>>> { @@ -175,8 +180,7 @@ impl PendingTransactions { fn replaced_transactions( &mut self, tx: &ValidPoolTransaction, - ) -> PoolResult<(Vec>>, Vec>)> - { + ) -> PoolResult> { // check if we are replacing transactions let remove_hashes: HashSet<_> = tx.provides.iter().filter_map(|mark| self.provided_dependencies.get(mark)).collect(); @@ -223,7 +227,7 @@ impl PendingTransactions { /// Removes the transactions from the ready queue and returns the removed transactions. /// This will also remove all transactions that depend on those. - pub fn clear_transactions( + pub(crate) fn clear_transactions( &mut self, tx_hashes: &[TransactionHashFor], ) -> Vec>> { @@ -234,7 +238,7 @@ impl PendingTransactions { /// /// This will also remove all transactions that lead to the transaction that provides the /// id. - pub fn prune_dependencies( + pub(crate) fn prune_dependencies( &mut self, id: TransactionIdFor, ) -> Vec>> { @@ -310,7 +314,7 @@ impl PendingTransactions { /// Removes transactions and those that depend on them and satisfy at least one dependency in /// the given filter set. - pub fn remove_with_dependencies( + pub(crate) fn remove_with_dependencies( &mut self, mut tx_hashes: Vec>, dependency_filter: Option>>, @@ -377,7 +381,7 @@ pub(crate) struct PendingTransaction { impl PendingTransaction { /// Returns all ids this transaction satisfies. - pub fn provides(&self) -> &[TransactionIdFor] { + pub(crate) fn provides(&self) -> &[TransactionIdFor] { &self.transaction.transaction.provides } } @@ -394,13 +398,13 @@ impl Clone for PendingTransaction { /// A reference to a transaction in the _pending_ pool #[derive(Debug)] -pub struct PoolTransactionRef { +pub(crate) struct PoolTransactionRef { /// Actual transaction. - pub transaction: Arc>, + pub(crate) transaction: Arc>, /// Identifier that tags when transaction was submitted in the pool. - pub submission_id: u64, + pub(crate) submission_id: u64, /// The priority value assigned by the used `Ordering` function. - pub priority: T::Priority, + pub(crate) priority: T::Priority, } impl Clone for PoolTransactionRef { @@ -452,7 +456,7 @@ struct ParkedTransactions { /// A transaction that is ready to be included in a block. #[derive(Debug, Clone)] -pub struct ParkedTransaction { +pub(crate) struct ParkedTransaction { /// Reference to the actual transaction. transaction: PoolTransactionRef, /// Tracks the transactions that get unlocked by this transaction. @@ -497,6 +501,7 @@ impl Ord for ParkedTransactionRef { } } +/// An iterator that returns transactions that can be executed on the current state. pub struct TransactionsIterator { all: HashMap, PendingTransaction>, awaiting: HashMap, (usize, PoolTransactionRef)>, @@ -512,7 +517,7 @@ impl TransactionsIterator { /// As a consequence, all values that depend on the invalid one will be skipped. /// When given transaction is not in the pool it has no effect. /// When invoked on a fully drained iterator it has no effect either. - pub fn mark_invalid(&mut self, tx: &Arc>) { + pub(crate) fn mark_invalid(&mut self, tx: &Arc>) { if let Some(invalid_transaction) = self.all.get(tx.hash()) { debug!( target: "txpool", @@ -520,7 +525,7 @@ impl TransactionsIterator { invalid_transaction.transaction.transaction.hash() ); for hash in &invalid_transaction.unlocks { - self.invalid.insert(hash.clone()); + self.invalid.insert(*hash); } } } diff --git a/crates/transaction-pool/src/pool/queued.rs b/crates/transaction-pool/src/pool/queued.rs index 446e7701c12..cfb79c47437 100644 --- a/crates/transaction-pool/src/pool/queued.rs +++ b/crates/transaction-pool/src/pool/queued.rs @@ -1,6 +1,5 @@ use crate::{ - error, - error::{PoolError, PoolResult}, + error::PoolResult, pool::{TransactionHashFor, TransactionIdFor}, traits::PoolTransaction, validate::ValidPoolTransaction, @@ -12,7 +11,6 @@ use std::{ sync::Arc, time::Instant, }; -use tracing::warn; /// A pool of transactions that are not ready on the current state and are waiting for state changes /// that turn them valid. @@ -35,31 +33,31 @@ pub(crate) struct QueuedTransactions { impl QueuedTransactions { /// Returns the number of transactions that are currently waiting in this pool for new /// transactions to satisfy their dependencies. - pub fn len(&self) -> usize { + pub(crate) fn len(&self) -> usize { self.waiting_queue.len() } /// Whether this pool is empty. - pub fn is_empty(&self) -> bool { + pub(crate) fn is_empty(&self) -> bool { self.waiting_queue.is_empty() } /// Returns an iterator over all transactions waiting in this pool. - pub fn transactions( + pub(crate) fn transactions( &self, ) -> impl Iterator>> + '_ { self.waiting_queue.values().map(|tx| Arc::clone(&tx.transaction)) } /// Adds a transaction to the queue of transactions - pub fn add_transaction(&mut self, tx: QueuedPoolTransaction) -> PoolResult<()> { + pub(crate) fn add_transaction(&mut self, tx: QueuedPoolTransaction) -> PoolResult<()> { assert!(!tx.is_satisfied(), "transaction must not be ready"); assert!( !self.waiting_queue.contains_key(tx.transaction.hash()), "transaction is already added" ); - if let Some(replace) = self + if let Some(_replace) = self .waiting_dependencies .get(&tx.transaction.provides) .and_then(|hash| self.waiting_queue.get(hash)) @@ -90,17 +88,17 @@ impl QueuedTransactions { } /// Returns true if given transaction is part of the queue - pub fn contains(&self, hash: &TransactionHashFor) -> bool { + pub(crate) fn contains(&self, hash: &TransactionHashFor) -> bool { self.waiting_queue.contains_key(hash) } /// Returns the transaction for the hash if it's waiting - pub fn get(&self, tx_hash: &TransactionHashFor) -> Option<&QueuedPoolTransaction> { + pub(crate) fn get(&self, tx_hash: &TransactionHashFor) -> Option<&QueuedPoolTransaction> { self.waiting_queue.get(tx_hash) } /// Returns the transactions for the given hashes, `None` if no transaction exists - pub fn get_all( + pub(crate) fn get_all( &self, tx_hashes: &[TransactionHashFor], ) -> Vec>>> { @@ -114,7 +112,7 @@ impl QueuedTransactions { /// /// Returns the those transactions that become unlocked (all dependencies checked) and can be /// moved to the ready queue. - pub fn satisfy_and_unlock( + pub(crate) fn satisfy_and_unlock( &mut self, dependencies: impl IntoIterator>>, ) -> Vec> { @@ -142,7 +140,7 @@ impl QueuedTransactions { /// Removes the transactions associated with the given hashes /// /// Returns all removed transactions. - pub fn remove( + pub(crate) fn remove( &mut self, hashes: Vec>, ) -> Vec>> { @@ -171,17 +169,17 @@ impl QueuedTransactions { /// A transaction submitted to the pool. #[derive(Clone)] -pub struct QueuedPoolTransaction { +pub(crate) struct QueuedPoolTransaction { /// The actual validated transaction. - pub transaction: Arc>, + pub(crate) transaction: Arc>, /// Transactions required for and have not been satisfied yet by other transactions in the /// pool. /// /// This will be an empty list if there are no nonce gaps across multiple transactions of the /// same sender in the pool. If there are gaps, this will include the missing transactions. - pub missing_dependencies: HashSet>, + pub(crate) missing_dependencies: HashSet>, /// Timestamp when the tx was added. - pub added_at: Instant, + pub(crate) added_at: Instant, } // === impl QuQueuedPoolTransaction === @@ -191,7 +189,7 @@ impl QueuedPoolTransaction { /// /// Determines the dependent transaction that are still missing before this transaction can be /// moved to the queue. - pub fn new( + pub(crate) fn new( transaction: ValidPoolTransaction, provided: &HashMap, TransactionHashFor>, ) -> Self { @@ -210,12 +208,12 @@ impl QueuedPoolTransaction { } /// Removes the required dependency. - pub fn satisfy(&mut self, id: &TransactionIdFor) { + pub(crate) fn satisfy(&mut self, id: &TransactionIdFor) { self.missing_dependencies.remove(id); } /// Returns true if transaction has all dependencies are satisfied. - pub fn is_satisfied(&self) -> bool { + pub(crate) fn is_satisfied(&self) -> bool { self.missing_dependencies.is_empty() } } diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index 9b45e81284d..7644e33dc0e 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -5,7 +5,7 @@ use reth_primitives::BlockId; use std::fmt; /// Result returned after checking a transaction's validity -pub type TransactionValidationResult = +pub(crate) type TransactionValidationResult = Result<(), TransactionValidationError>; /// Provides support for validating transaction at any given state of the chain @@ -23,8 +23,8 @@ pub trait TransactionValidator: Send + Sync { /// transactions for the sender. async fn validate_transaction( &self, - block_id: &BlockId, - transaction: Self::Transaction, + _block_id: &BlockId, + _transaction: Self::Transaction, // TODO this should return a subset of `ValidPoolTransaction`, maybe an enum ) -> TransactionValidationResult> { unimplemented!() From 1d710a3de920f8bcc55f1c7b0d0ba6470ce1806a Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 5 Oct 2022 15:02:24 +0200 Subject: [PATCH 21/35] rename pool --- crates/transaction-pool/src/lib.rs | 5 ++--- crates/transaction-pool/src/pool/mod.rs | 6 +++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index 9d267854488..169eabc9460 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -22,16 +22,15 @@ pub use crate::{ client::PoolClient, config::PoolConfig, ordering::TransactionOrdering, + pool::BasicPool, traits::{PoolTransaction, TransactionPool}, validate::TransactionValidator, }; /// A generic, customizable `TransactionPool` implementation. -// TODO: This is a more feature rich pool, any additional features should go here, like metrics, -// etc... pub struct Pool { /// The actual transaction pool where transactions are handled. - inner: pool::PoolInner, + pool: BasicPool, /// Chain/Storage access client: Arc, // TODO how to revalidate diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 2dbaf518c84..59826e75140 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -104,14 +104,14 @@ pub(crate) type TransactionIdFor = <::Transaction as PoolTransaction>::Id; /// Shareable Transaction pool. -pub struct Pool { +pub struct BasicPool { /// Arc'ed instance of the pool internals pool: Arc>, } // === impl Pool === -impl Pool +impl BasicPool where P: PoolClient, T: TransactionOrdering::Transaction>, @@ -161,7 +161,7 @@ where } } -impl Clone for Pool { +impl Clone for BasicPool { fn clone(&self) -> Self { Self { pool: Arc::clone(&self.pool) } } From 5152cd1fe5710c5b1c7a442e088660586c2fe845 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 5 Oct 2022 15:16:24 +0200 Subject: [PATCH 22/35] feat: implement constructors --- crates/transaction-pool/src/lib.rs | 29 ++++++++++---------- crates/transaction-pool/src/pool/listener.rs | 9 ++++++ crates/transaction-pool/src/pool/mod.rs | 22 +++++++++++++++ crates/transaction-pool/src/pool/pending.rs | 24 ++++++++++++++-- crates/transaction-pool/src/pool/queued.rs | 10 +++++++ 5 files changed, 77 insertions(+), 17 deletions(-) diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index 169eabc9460..da01116172f 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -28,32 +28,31 @@ pub use crate::{ }; /// A generic, customizable `TransactionPool` implementation. -pub struct Pool { +pub struct Pool { /// The actual transaction pool where transactions are handled. - pool: BasicPool, + pool: BasicPool, /// Chain/Storage access - client: Arc, - // TODO how to revalidate - // TODO provide a way to add listeners for ready transactions + client: Arc

, } // === impl Pool === -impl Pool -where - PoolApi: PoolClient, - Ordering: TransactionOrdering, +impl Pool + where + P: PoolClient, + T: TransactionOrdering::Transaction>, { - /// Creates a new `Pool` with the given config and chain api - pub fn new(_config: PoolConfig, _api: Arc) -> Self { - unimplemented!() + /// Creates a new `Pool` with the given config and client and ordering. + pub fn new(client: Arc

, ordering: Arc, config: PoolConfig) -> Self { + let pool = BasicPool::new(Arc::clone(&client), ordering, config); + Self { pool, client } } } /// implements the `TransactionPool` interface for the `Poll`. -impl TransactionPool for Pool +impl TransactionPool for Pool where - PoolApi: PoolClient, - Ordering: TransactionOrdering::Transaction>, + P: PoolClient, + T: TransactionOrdering::Transaction>, { } diff --git a/crates/transaction-pool/src/pool/listener.rs b/crates/transaction-pool/src/pool/listener.rs index b3dde002603..a2641eece4f 100644 --- a/crates/transaction-pool/src/pool/listener.rs +++ b/crates/transaction-pool/src/pool/listener.rs @@ -46,6 +46,15 @@ impl PoolEventListener { } } + +impl Default for PoolEventListener { + fn default() -> Self { + Self { + listeners: Default::default() + } + } +} + /// Sender half(s) of the event channels for a specific transaction #[derive(Debug)] struct PoolEventNotifier { diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 59826e75140..7e582e7294a 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -116,6 +116,11 @@ where P: PoolClient, T: TransactionOrdering::Transaction>, { + /// Create a new transaction pool instance. + pub fn new(client: Arc

, ordering: Arc, config: PoolConfig) -> Self { + Self { pool: Arc::new(PoolInner::new(client, ordering, config)) } + } + /// Returns the actual block number for the block id fn resolve_block_number(&self, block_id: &BlockId) -> PoolResult { self.pool.client().ensure_block_number(block_id) @@ -188,6 +193,17 @@ where P: PoolClient, T: TransactionOrdering::Transaction>, { + /// Create a new transaction pool instance. + pub fn new(client: Arc

, ordering: Arc, config: PoolConfig) -> Self { + Self { + client, + config, + event_listener: Default::default(), + pool: RwLock::new(GraphPool::new(ordering)), + ready_transaction_listener: Default::default(), + } + } + /// Get client reference. pub fn client(&self) -> &P { &self.client @@ -289,6 +305,12 @@ pub struct GraphPool { // === impl PoolInner === impl GraphPool { + /// Create a new graph pool instance. + pub fn new(ordering: Arc) -> Self { + let pending = PendingTransactions::new(Arc::clone(&ordering)); + Self { ordering, pending, queued: Default::default() } + } + /// Returns if the transaction for the given hash is already included in this pool pub fn contains(&self, tx_hash: &TransactionHashFor) -> bool { self.queued.contains(tx_hash) || self.pending.contains(tx_hash) diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index 218a6528d66..170551415c7 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -1,8 +1,7 @@ use crate::{ error::PoolResult, pool::{queued::QueuedPoolTransaction, TransactionHashFor, TransactionIdFor}, - validate::ValidPoolTransaction, - TransactionOrdering, + validate::ValidPoolTransaction, TransactionOrdering, }; use parking_lot::RwLock; use reth_primitives::U256; @@ -53,6 +52,17 @@ pub(crate) struct PendingTransactions { // === impl PendingTransactions === impl PendingTransactions { + /// Create a new pool instance + pub(crate) fn new(ordering: Arc) -> Self { + Self { + id: 0, + provided_dependencies: Default::default(), + parked: Default::default(), + ready_transactions: Arc::new(Default::default()), + ordering, + independent_transactions: Default::default(), + } + } /// Returns an iterator over all transactions that are _currently_ ready. /// /// 1. The iterator _always_ returns transaction in order: It never returns a transaction with @@ -454,6 +464,16 @@ struct ParkedTransactions { sorted_transactions: BTreeSet>, } +impl Default for ParkedTransactions { + fn default() -> Self { + Self { + id: 0, + parked_transactions: Default::default(), + sorted_transactions: Default::default() + } + } +} + /// A transaction that is ready to be included in a block. #[derive(Debug, Clone)] pub(crate) struct ParkedTransaction { diff --git a/crates/transaction-pool/src/pool/queued.rs b/crates/transaction-pool/src/pool/queued.rs index cfb79c47437..facbbdb8633 100644 --- a/crates/transaction-pool/src/pool/queued.rs +++ b/crates/transaction-pool/src/pool/queued.rs @@ -182,6 +182,16 @@ pub(crate) struct QueuedPoolTransaction { pub(crate) added_at: Instant, } +impl Default for QueuedTransactions { + fn default() -> Self { + Self { + required_dependencies: Default::default(), + waiting_dependencies: Default::default(), + waiting_queue: Default::default() + } + } +} + // === impl QuQueuedPoolTransaction === impl QueuedPoolTransaction { From acea23a695ebb1277c15788e9e99eb385a2df372 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 5 Oct 2022 15:49:16 +0200 Subject: [PATCH 23/35] feat: extend transaction pool api --- crates/transaction-pool/src/lib.rs | 49 +++++++++-- crates/transaction-pool/src/pool/listener.rs | 5 +- crates/transaction-pool/src/pool/pending.rs | 15 +++- crates/transaction-pool/src/pool/queued.rs | 4 +- crates/transaction-pool/src/traits.rs | 87 ++++++++++++++++++-- 5 files changed, 138 insertions(+), 22 deletions(-) diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index da01116172f..ba2aa87965a 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -7,8 +7,8 @@ //! Reth's transaction pool implementation -use reth_primitives::BlockId; -use std::sync::Arc; +use reth_primitives::{BlockId, U64}; +use std::sync::{mpsc::Receiver, Arc}; mod client; mod config; @@ -23,9 +23,10 @@ pub use crate::{ config::PoolConfig, ordering::TransactionOrdering, pool::BasicPool, - traits::{PoolTransaction, TransactionPool}, + traits::{PoolTransaction, ReadyTransactions, TransactionPool}, validate::TransactionValidator, }; +use crate::{error::PoolResult, traits::HashFor, validate::ValidPoolTransaction}; /// A generic, customizable `TransactionPool` implementation. pub struct Pool { @@ -38,9 +39,9 @@ pub struct Pool { // === impl Pool === impl Pool - where - P: PoolClient, - T: TransactionOrdering::Transaction>, +where + P: PoolClient, + T: TransactionOrdering::Transaction>, { /// Creates a new `Pool` with the given config and client and ordering. pub fn new(client: Arc

, ordering: Arc, config: PoolConfig) -> Self { @@ -50,9 +51,45 @@ impl Pool } /// implements the `TransactionPool` interface for the `Poll`. +#[async_trait::async_trait] impl TransactionPool for Pool where P: PoolClient, T: TransactionOrdering::Transaction>, { + type Transaction = T::Transaction; + + async fn add_transaction( + &self, + _block_id: &BlockId, + _transaction: Self::Transaction, + ) -> PoolResult> { + todo!() + } + + async fn add_transactions( + &self, + _block_id: &BlockId, + _transaction: Self::Transaction, + ) -> PoolResult>>> { + todo!() + } + + fn ready_transactions(&self) -> Receiver> { + todo!() + } + + async fn ready_transactions_at( + &self, + _block: U64, + ) -> Box>> { + todo!() + } + + fn remove_invalid( + &self, + _tx_hashes: &[HashFor], + ) -> Vec>> { + todo!() + } } diff --git a/crates/transaction-pool/src/pool/listener.rs b/crates/transaction-pool/src/pool/listener.rs index a2641eece4f..070d6e41215 100644 --- a/crates/transaction-pool/src/pool/listener.rs +++ b/crates/transaction-pool/src/pool/listener.rs @@ -46,12 +46,9 @@ impl PoolEventListener { } } - impl Default for PoolEventListener { fn default() -> Self { - Self { - listeners: Default::default() - } + Self { listeners: Default::default() } } } diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index 170551415c7..2bee04319e0 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -1,7 +1,9 @@ use crate::{ error::PoolResult, pool::{queued::QueuedPoolTransaction, TransactionHashFor, TransactionIdFor}, - validate::ValidPoolTransaction, TransactionOrdering, + traits::ReadyTransactions, + validate::ValidPoolTransaction, + TransactionOrdering, }; use parking_lot::RwLock; use reth_primitives::U256; @@ -464,12 +466,12 @@ struct ParkedTransactions { sorted_transactions: BTreeSet>, } -impl Default for ParkedTransactions { +impl Default for ParkedTransactions { fn default() -> Self { Self { id: 0, parked_transactions: Default::default(), - sorted_transactions: Default::default() + sorted_transactions: Default::default(), } } } @@ -536,7 +538,6 @@ impl TransactionsIterator { /// /// As a consequence, all values that depend on the invalid one will be skipped. /// When given transaction is not in the pool it has no effect. - /// When invoked on a fully drained iterator it has no effect either. pub(crate) fn mark_invalid(&mut self, tx: &Arc>) { if let Some(invalid_transaction) = self.all.get(tx.hash()) { debug!( @@ -563,6 +564,12 @@ impl TransactionsIterator { } } +impl ReadyTransactions for TransactionsIterator { + fn mark_invalid(&mut self, tx: &Self::Item) { + TransactionsIterator::mark_invalid(self, tx) + } +} + impl Iterator for TransactionsIterator { type Item = Arc>; diff --git a/crates/transaction-pool/src/pool/queued.rs b/crates/transaction-pool/src/pool/queued.rs index facbbdb8633..87ec98ddd90 100644 --- a/crates/transaction-pool/src/pool/queued.rs +++ b/crates/transaction-pool/src/pool/queued.rs @@ -182,12 +182,12 @@ pub(crate) struct QueuedPoolTransaction { pub(crate) added_at: Instant, } -impl Default for QueuedTransactions { +impl Default for QueuedTransactions { fn default() -> Self { Self { required_dependencies: Default::default(), waiting_dependencies: Default::default(), - waiting_queue: Default::default() + waiting_queue: Default::default(), } } } diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index f259b0177d1..043e5c80216 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -1,13 +1,88 @@ -use reth_primitives::U256; -use std::{fmt, hash::Hash}; +use crate::{error::PoolResult, validate::ValidPoolTransaction, BlockId}; +use reth_primitives::{U256, U64}; +use std::{ + fmt, + hash::Hash, + sync::{mpsc::Receiver, Arc}, +}; -/// General purpose abstraction fo a transaction-pool +pub type HashFor = <::Transaction as PoolTransaction>::Hash; + +/// General purpose abstraction fo a transaction-pool. +/// +/// This is intended to be used by API-consumers such as RPC that need inject new incoming, +/// unverified transactions. And by block production that needs to get transactions to execute in a +/// new block. #[async_trait::async_trait] pub trait TransactionPool: Send + Sync { - // TODO probably need associated `Transaction` type here - // TODO needs transaction type + /// The transaction type of the pool + type Transaction: PoolTransaction + Send + Sync; + + /// Adds an unvalidated transaction into the pool. + /// + /// Consumer: RPC + async fn add_transaction( + &self, + block_id: &BlockId, + transaction: Self::Transaction, + ) -> PoolResult>; + + /// Adds all unvalidated transaction into the pool. + /// + /// Returns a list of results. + /// + /// Consumer: RPC + async fn add_transactions( + &self, + block_id: &BlockId, + transaction: Self::Transaction, + ) -> PoolResult>>>; + + /// Returns a new Stream that yields transactions hashes for new ready transactions. + /// + /// Consumer: RPC + fn ready_transactions(&self) -> Receiver>; + + /// Returns an iterator that yields transactions that are ready for block production. + /// + /// This provides the block at which the pool should be updated at. + /// + /// Implementers must ensure that the iterator yields only transaction that are valid for the + /// given `block` and return `None` otherwise. + /// + /// Consumer: Block production + async fn ready_transactions_at( + &self, + block: U64, + ) -> Box>>; + + /// Removes all transactions corresponding to the given hashes. + /// + /// Also removes all dependent transactions. + /// + /// Consumer: Block production + fn remove_invalid( + &self, + tx_hashes: &[HashFor], + ) -> Vec>>; +} + +/// An `Iterator` that only returns transactions that are ready to be executed. +/// +/// This makes no assumptions about the order of the transactions, but expects that _all_ +/// transactions are valid (no nonce gaps.). +pub trait ReadyTransactions: Iterator + Send { + /// Mark the transaction as invalid. + /// + /// Implementers must ensure all subsequent transaction _don't_ depend on this transaction. + /// In other words, this must remove the given transaction _and_ drain all transaction that + /// depend on it. + fn mark_invalid(&mut self, transaction: &Self::Item); +} - // TODO add interfaces for adding new transactions +/// A no-op implementation that yields no transactions. +impl ReadyTransactions for std::iter::Empty { + fn mark_invalid(&mut self, _tx: &T) {} } /// Trait for transaction types used inside the pool From 9ea4337083321f2580a6d654d5c096231d66e329 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 5 Oct 2022 17:19:06 +0200 Subject: [PATCH 24/35] feat: more pool api impls --- crates/transaction-pool/src/lib.rs | 21 ++++----- crates/transaction-pool/src/pool/mod.rs | 59 ++++++++++++++++++++----- crates/transaction-pool/src/traits.rs | 17 +++---- crates/transaction-pool/src/validate.rs | 27 +++++------ 4 files changed, 77 insertions(+), 47 deletions(-) diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index ba2aa87965a..54e3e0274c3 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -7,8 +7,9 @@ //! Reth's transaction pool implementation +use futures::channel::mpsc::Receiver; use reth_primitives::{BlockId, U64}; -use std::sync::{mpsc::Receiver, Arc}; +use std::sync::Arc; mod client; mod config; @@ -24,7 +25,7 @@ pub use crate::{ ordering::TransactionOrdering, pool::BasicPool, traits::{PoolTransaction, ReadyTransactions, TransactionPool}, - validate::TransactionValidator, + validate::{TransactionValidationOutcome, TransactionValidator}, }; use crate::{error::PoolResult, traits::HashFor, validate::ValidPoolTransaction}; @@ -50,7 +51,7 @@ where } } -/// implements the `TransactionPool` interface for the `Poll`. +/// implements the `TransactionPool` interface for various transaction pool API consumers. #[async_trait::async_trait] impl TransactionPool for Pool where @@ -61,22 +62,22 @@ where async fn add_transaction( &self, - _block_id: &BlockId, - _transaction: Self::Transaction, + block_id: BlockId, + transaction: Self::Transaction, ) -> PoolResult> { - todo!() + self.pool.clone().add_transaction(&block_id, transaction).await } async fn add_transactions( &self, - _block_id: &BlockId, - _transaction: Self::Transaction, + block_id: BlockId, + transactions: Vec, ) -> PoolResult>>> { - todo!() + self.pool.clone().add_transactions(&block_id, transactions).await } fn ready_transactions(&self) -> Receiver> { - todo!() + self.pool.ready_transactions() } async fn ready_transactions_at( diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 7e582e7294a..5822d3e9f75 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -74,7 +74,7 @@ use crate::{ queued::{QueuedPoolTransaction, QueuedTransactions}, }, traits::PoolTransaction, - validate::{TransactionValidationResult, ValidPoolTransaction}, + validate::ValidPoolTransaction, BlockId, PoolClient, PoolConfig, TransactionOrdering, TransactionValidator, }; use futures::channel::mpsc::{channel, Receiver, Sender}; @@ -93,6 +93,7 @@ mod pending; mod queued; mod transaction; +use crate::validate::TransactionValidationOutcome; pub use events::TransactionEvent; pub use pending::TransactionsIterator; @@ -126,13 +127,36 @@ where self.pool.client().ensure_block_number(block_id) } + /// Add a single _unverified_ transaction into the pool. + pub async fn add_transaction( + &self, + block_id: &BlockId, + transaction: P::Transaction, + ) -> PoolResult> { + self.add_transactions(block_id, Some(transaction)) + .await? + .pop() + .expect("transaction exists; qed") + } + + /// Adds all given transactions into the pool + pub async fn add_transactions( + &self, + block_id: &BlockId, + transactions: impl IntoIterator, + ) -> PoolResult>>> { + let validated = self.validate_all(block_id, transactions).await?; + let transactions = self.pool.add_transactions(validated.into_values()); + Ok(transactions) + } + /// Returns future that validates all transaction in the given iterator at the block the /// `block_id` points to. async fn validate_all( &self, block_id: &BlockId, transactions: impl IntoIterator, - ) -> PoolResult, TransactionValidationResult>> + ) -> PoolResult, TransactionValidationOutcome>> { // get the actual block number which is required to validate the transactions let block_number = self.resolve_block_number(block_id)?; @@ -155,7 +179,7 @@ where block_id: &BlockId, _block_number: U64, transaction: P::Transaction, - ) -> (TransactionHashFor, TransactionValidationResult) { + ) -> (TransactionHashFor, TransactionValidationOutcome) { let _hash = *transaction.hash(); // TODO this is where additional validate checks would go, like banned senders etc... let _res = self.pool.client().validate_transaction(block_id, transaction).await; @@ -164,6 +188,11 @@ where todo!() } + + /// Registers a new transaction listener and returns the receiver stream. + pub fn ready_transactions(&self) -> Receiver> { + self.pool.add_ready_listener() + } } impl Clone for BasicPool { @@ -229,23 +258,31 @@ where /// Add a single validated transaction into the pool. fn add_transaction( &self, - tx: ValidPoolTransaction, + tx: TransactionValidationOutcome, ) -> PoolResult> { - let added = self.pool.write().add_transaction(tx)?; + match tx { + TransactionValidationOutcome::Valid(tx) => { + let added = self.pool.write().add_transaction(tx)?; - if let Some(ready) = added.as_ready() { - self.on_new_ready_transaction(ready); - } + if let Some(ready) = added.as_ready() { + self.on_new_ready_transaction(ready); + } - self.notify_event_listeners(&added); + self.notify_event_listeners(&added); - Ok(*added.hash()) + Ok(*added.hash()) + } + TransactionValidationOutcome::Invalid(_tx, err) => { + // TODO notify listeners about invalid + Err(err) + } + } } /// Adds all transactions in the iterator to the pool, returning a list of results. pub fn add_transactions( &self, - transactions: impl IntoIterator>, + transactions: impl IntoIterator>, ) -> Vec>> { // TODO check pool limits diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 043e5c80216..8bd8cb72ae1 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -1,10 +1,7 @@ use crate::{error::PoolResult, validate::ValidPoolTransaction, BlockId}; +use futures::channel::mpsc::Receiver; use reth_primitives::{U256, U64}; -use std::{ - fmt, - hash::Hash, - sync::{mpsc::Receiver, Arc}, -}; +use std::{fmt, hash::Hash, sync::Arc}; pub type HashFor = <::Transaction as PoolTransaction>::Hash; @@ -18,24 +15,24 @@ pub trait TransactionPool: Send + Sync { /// The transaction type of the pool type Transaction: PoolTransaction + Send + Sync; - /// Adds an unvalidated transaction into the pool. + /// Adds an _unvalidated_ transaction into the pool. /// /// Consumer: RPC async fn add_transaction( &self, - block_id: &BlockId, + block_id: BlockId, transaction: Self::Transaction, ) -> PoolResult>; - /// Adds all unvalidated transaction into the pool. + /// Adds the given _unvalidated_ transaction into the pool. /// /// Returns a list of results. /// /// Consumer: RPC async fn add_transactions( &self, - block_id: &BlockId, - transaction: Self::Transaction, + block_id: BlockId, + transactions: Vec, ) -> PoolResult>>>; /// Returns a new Stream that yields transactions hashes for new ready transactions. diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index 7644e33dc0e..36184d2e91b 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -1,12 +1,18 @@ //! Transaction validation abstractions. -use crate::traits::PoolTransaction; +use crate::{error::PoolError, traits::PoolTransaction}; use reth_primitives::BlockId; use std::fmt; -/// Result returned after checking a transaction's validity -pub(crate) type TransactionValidationResult = - Result<(), TransactionValidationError>; +/// A Result type returned after checking a transaction's validity. +pub enum TransactionValidationOutcome { + /// Transaction successfully validated + Valid(ValidPoolTransaction), + /// The transaction is considered invalid. + /// + /// Note: This does not indicate whether the transaction will not be valid in the future + Invalid(T, PoolError), +} /// Provides support for validating transaction at any given state of the chain #[async_trait::async_trait] @@ -25,22 +31,11 @@ pub trait TransactionValidator: Send + Sync { &self, _block_id: &BlockId, _transaction: Self::Transaction, - // TODO this should return a subset of `ValidPoolTransaction`, maybe an enum - ) -> TransactionValidationResult> { + ) -> TransactionValidationOutcome { unimplemented!() } } -/// Errors thrown during validity checks of a transaction. -#[derive(Clone, PartialEq, Eq)] -pub enum TransactionValidationError { - /// The transaction is considered invalid. - /// - /// Note: This does not indicate whether the transaction will not be valid in the future - Invalid(Transaction), - // TODO need variants for `Never`, or `At`? -} - /// A valida transaction in the pool. pub struct ValidPoolTransaction { /// The transaction From 53ec97443f89631af2cd7b63bf5c9e062c79f118 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 5 Oct 2022 17:33:14 +0200 Subject: [PATCH 25/35] feat: introduce chainevents --- crates/transaction-pool/src/lib.rs | 7 ++++++- crates/transaction-pool/src/traits.rs | 28 ++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index 54e3e0274c3..56796974120 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -24,7 +24,7 @@ pub use crate::{ config::PoolConfig, ordering::TransactionOrdering, pool::BasicPool, - traits::{PoolTransaction, ReadyTransactions, TransactionPool}, + traits::{ChainEvent, PoolTransaction, ReadyTransactions, TransactionPool}, validate::{TransactionValidationOutcome, TransactionValidator}, }; use crate::{error::PoolResult, traits::HashFor, validate::ValidPoolTransaction}; @@ -60,6 +60,11 @@ where { type Transaction = T::Transaction; + async fn on_chain_event(&self, event: ChainEvent) { + // TODO perform maintenance: update pool accordingly + todo!() + } + async fn add_transaction( &self, block_id: BlockId, diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 8bd8cb72ae1..01ac198ee1e 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -1,6 +1,6 @@ use crate::{error::PoolResult, validate::ValidPoolTransaction, BlockId}; use futures::channel::mpsc::Receiver; -use reth_primitives::{U256, U64}; +use reth_primitives::{H256, U256, U64}; use std::{fmt, hash::Hash, sync::Arc}; pub type HashFor = <::Transaction as PoolTransaction>::Hash; @@ -15,6 +15,13 @@ pub trait TransactionPool: Send + Sync { /// The transaction type of the pool type Transaction: PoolTransaction + Send + Sync; + /// Event listener for chain events that affect the pool. + /// + /// Implementers need to update the pool accordingly. + /// For example the base fee of the pending block is determined after a block is mined which + /// affects the dynamic fee requirement of pending transactions in the pool. + async fn on_chain_event(&self, event: ChainEvent); + /// Adds an _unvalidated_ transaction into the pool. /// /// Consumer: RPC @@ -64,6 +71,25 @@ pub trait TransactionPool: Send + Sync { ) -> Vec>>; } +/// Various Events the pool listens for and needs to apply changes. +#[derive(Debug, Clone)] +pub enum ChainEvent { + /// New best block have been added to the chain. + NewBestBlock { + /// Hash of the added block. + hash: H256, + /// EIP-1559 Base fee of the _next_ (pending) block + /// + /// The base fee of a block depends on the utilization of the last block and its base fee. + next_base_fee: U256, + }, + /// An existing block has been finalized. + Finalized { + /// Hash of just finalized block. + hash: H256, + }, +} + /// An `Iterator` that only returns transactions that are ready to be executed. /// /// This makes no assumptions about the order of the transactions, but expects that _all_ From b6936fa5b3a385bf229cd2d98caea61ce5f31f27 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 5 Oct 2022 18:08:39 +0200 Subject: [PATCH 26/35] feat: more pool api --- crates/transaction-pool/src/lib.rs | 13 ++++++------- crates/transaction-pool/src/ordering.rs | 6 +++--- crates/transaction-pool/src/pool/mod.rs | 14 ++++++++++++-- crates/transaction-pool/src/traits.rs | 16 +++++----------- crates/transaction-pool/src/validate.rs | 2 +- 5 files changed, 27 insertions(+), 24 deletions(-) diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index 56796974120..a19a24f7bab 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -60,7 +60,7 @@ where { type Transaction = T::Transaction; - async fn on_chain_event(&self, event: ChainEvent) { + async fn on_chain_event(&self, _event: ChainEvent) { // TODO perform maintenance: update pool accordingly todo!() } @@ -81,15 +81,14 @@ where self.pool.clone().add_transactions(&block_id, transactions).await } - fn ready_transactions(&self) -> Receiver> { - self.pool.ready_transactions() + fn ready_transactions_listener(&self) -> Receiver> { + self.pool.ready_transactions_listener() } - async fn ready_transactions_at( + fn ready_transactions( &self, - _block: U64, - ) -> Box>> { - todo!() + ) -> Box>>> { + Box::new(self.pool.inner().ready_transactions()) } fn remove_invalid( diff --git a/crates/transaction-pool/src/ordering.rs b/crates/transaction-pool/src/ordering.rs index 6c192bfc68f..bdf66cd88d1 100644 --- a/crates/transaction-pool/src/ordering.rs +++ b/crates/transaction-pool/src/ordering.rs @@ -6,12 +6,12 @@ use std::fmt; /// Decides how transactions should be ordered within the pool. /// /// The returned priority must reflect natural `Ordering`. -pub trait TransactionOrdering: Send + Sync { +pub trait TransactionOrdering: Send + Sync + 'static { /// Priority of a transaction. - type Priority: Ord + Clone + Default + fmt::Debug + fmt::LowerHex + Send + Sync; + type Priority: Ord + Clone + Default + fmt::Debug + Send + Sync; /// The transaction type to score. - type Transaction: PoolTransaction + Send + Sync; + type Transaction: PoolTransaction + Send + Sync + 'static; /// Returns the priority score for the given transaction. fn priority(&self, transaction: &Self::Transaction) -> Self::Priority; diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 5822d3e9f75..324cc3407de 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -122,6 +122,11 @@ where Self { pool: Arc::new(PoolInner::new(client, ordering, config)) } } + /// Returns the wrapped pool + pub(crate) fn inner(&self) -> &PoolInner { + &self.pool + } + /// Returns the actual block number for the block id fn resolve_block_number(&self, block_id: &BlockId) -> PoolResult { self.pool.client().ensure_block_number(block_id) @@ -190,7 +195,7 @@ where } /// Registers a new transaction listener and returns the receiver stream. - pub fn ready_transactions(&self) -> Receiver> { + pub fn ready_transactions_listener(&self) -> Receiver> { self.pool.add_ready_listener() } } @@ -323,6 +328,11 @@ where } } } + + /// Returns an iterator that yields transactions that are ready to be included in the block. + pub(crate) fn ready_transactions(&self) -> TransactionsIterator { + self.pool.read().ready_transactions() + } } /// A pool that only manages transactions. @@ -354,7 +364,7 @@ impl GraphPool { } /// Returns an iterator that yields transactions that are ready to be included in the block. - pub fn ready(&self) -> TransactionsIterator { + pub fn ready_transactions(&self) -> TransactionsIterator { self.pending.get_transactions() } diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 01ac198ee1e..82e1d23e13f 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -13,7 +13,7 @@ pub type HashFor = <::Transaction as PoolTransaction>:: #[async_trait::async_trait] pub trait TransactionPool: Send + Sync { /// The transaction type of the pool - type Transaction: PoolTransaction + Send + Sync; + type Transaction: PoolTransaction + Send + Sync + 'static; /// Event listener for chain events that affect the pool. /// @@ -45,20 +45,14 @@ pub trait TransactionPool: Send + Sync { /// Returns a new Stream that yields transactions hashes for new ready transactions. /// /// Consumer: RPC - fn ready_transactions(&self) -> Receiver>; + fn ready_transactions_listener(&self) -> Receiver>; /// Returns an iterator that yields transactions that are ready for block production. /// - /// This provides the block at which the pool should be updated at. - /// - /// Implementers must ensure that the iterator yields only transaction that are valid for the - /// given `block` and return `None` otherwise. - /// /// Consumer: Block production - async fn ready_transactions_at( + fn ready_transactions( &self, - block: U64, - ) -> Box>>; + ) -> Box>>>; /// Removes all transactions corresponding to the given hashes. /// @@ -109,7 +103,7 @@ impl ReadyTransactions for std::iter::Empty { } /// Trait for transaction types used inside the pool -pub trait PoolTransaction: fmt::Debug + Send + Send { +pub trait PoolTransaction: fmt::Debug + Send + Send + 'static { /// Transaction hash type. type Hash: fmt::Debug + fmt::LowerHex + Eq + Clone + Copy + Hash + Send + Sync + 'static; diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index 36184d2e91b..792b9ec8e81 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -18,7 +18,7 @@ pub enum TransactionValidationOutcome { #[async_trait::async_trait] pub trait TransactionValidator: Send + Sync { /// The transaction type to validate. - type Transaction: PoolTransaction + Send + Sync; + type Transaction: PoolTransaction + Send + Sync + 'static; /// Validates the transaction and returns a validated outcome /// From eb0fa842c4e1cbfbe51d73751dcc520739600aec Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 5 Oct 2022 18:26:34 +0200 Subject: [PATCH 27/35] feat: start integrating dynamic fee --- crates/transaction-pool/src/lib.rs | 20 +++++++++++++---- crates/transaction-pool/src/pool/mod.rs | 24 ++++++++++++++++++--- crates/transaction-pool/src/pool/pending.rs | 16 ++++++-------- crates/transaction-pool/src/traits.rs | 2 +- 4 files changed, 45 insertions(+), 17 deletions(-) diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index a19a24f7bab..23bc7b7713a 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -8,7 +8,8 @@ //! Reth's transaction pool implementation use futures::channel::mpsc::Receiver; -use reth_primitives::{BlockId, U64}; +use parking_lot::Mutex; +use reth_primitives::{BlockId, U256, U64}; use std::sync::Arc; mod client; @@ -31,9 +32,11 @@ use crate::{error::PoolResult, traits::HashFor, validate::ValidPoolTransaction}; /// A generic, customizable `TransactionPool` implementation. pub struct Pool { - /// The actual transaction pool where transactions are handled. + /// The actual transaction pool where transactions and subscriptions are handled. pool: BasicPool, - /// Chain/Storage access + /// Tracks status updates linked to chain events. + update_status: Arc>, + /// Chain/Storage access. client: Arc

, } @@ -47,7 +50,7 @@ where /// Creates a new `Pool` with the given config and client and ordering. pub fn new(client: Arc

, ordering: Arc, config: PoolConfig) -> Self { let pool = BasicPool::new(Arc::clone(&client), ordering, config); - Self { pool, client } + Self { pool, update_status: Arc::new(Default::default()), client } } } @@ -98,3 +101,12 @@ where todo!() } } + +/// Tracks the current update status of the pool. +#[derive(Debug, Clone, Default)] +struct UpdateStatus { + /// Block number when the pool was last updated. + updated_at: U64, + /// Current base fee that needs to be enforced + base_fee: U256, +} diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 324cc3407de..e18c8342566 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -75,7 +75,7 @@ use crate::{ }, traits::PoolTransaction, validate::ValidPoolTransaction, - BlockId, PoolClient, PoolConfig, TransactionOrdering, TransactionValidator, + BlockId, PoolClient, PoolConfig, TransactionOrdering, TransactionValidator, U256, }; use futures::channel::mpsc::{channel, Receiver, Sender}; use parking_lot::{Mutex, RwLock}; @@ -238,6 +238,11 @@ where } } + /// Updates the pool + pub(crate) fn update_base_fee(&self, base_fee: U256) { + self.pool.write().update_base_fee(base_fee); + } + /// Get client reference. pub fn client(&self) -> &P { &self.client @@ -358,13 +363,26 @@ impl GraphPool { Self { ordering, pending, queued: Default::default() } } + /// Updates the pool based on the changed base fee. + /// + /// This enforces the dynamic fee requirement. + /// If the `new_base_fee` is _higher_ than previous base fee, all EIP-1559 transactions in the + /// ready queue that now violate the dynamic fee requirement need to parked. + /// If the `new_base_fee` is _lower_ than the previous base fee, all parked transactions that + /// now satisfy the dynamic fee requirement need to moved to the ready queue. + pub(crate) fn update_base_fee(&mut self, new_base_fee: U256) { + let _old_base_fee = self.pending.set_next_base_fee(new_base_fee); + // TODO update according to the changed base_fee + todo!() + } + /// Returns if the transaction for the given hash is already included in this pool - pub fn contains(&self, tx_hash: &TransactionHashFor) -> bool { + pub(crate) fn contains(&self, tx_hash: &TransactionHashFor) -> bool { self.queued.contains(tx_hash) || self.pending.contains(tx_hash) } /// Returns an iterator that yields transactions that are ready to be included in the block. - pub fn ready_transactions(&self) -> TransactionsIterator { + pub(crate) fn ready_transactions(&self) -> TransactionsIterator { self.pending.get_transactions() } diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index 2bee04319e0..6b1f763b71f 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -33,6 +33,8 @@ pub(crate) struct PendingTransactions { id: u64, /// How to order transactions. ordering: Arc, + /// Base fee of the next block. + next_base_fee: U256, /// Dependencies that are provided by `PendingTransaction`s provided_dependencies: HashMap, TransactionHashFor>, /// Pending transactions that are currently on hold until the `baseFee` of the pending block @@ -63,6 +65,7 @@ impl PendingTransactions { ready_transactions: Arc::new(Default::default()), ordering, independent_transactions: Default::default(), + next_base_fee: Default::default(), } } /// Returns an iterator over all transactions that are _currently_ ready. @@ -92,15 +95,10 @@ impl PendingTransactions { } } - // /// Returns an iterator over all transactions - // pub(crate) fn get_transactions(&self) -> TransactionsIterator { - // TransactionsIterator { - // all: self.ready_tx.read().clone(), - // independent: self.independent_transactions.clone(), - // awaiting: Default::default(), - // _invalid: Default::default(), - // } - // } + /// Sets the given base fee and returns the old one. + pub(crate) fn set_next_base_fee(&mut self, base_fee: U256) -> U256 { + std::mem::replace(&mut self.next_base_fee, base_fee) + } /// Returns true if the transaction is part of the queue. pub(crate) fn contains(&self, hash: &TransactionHashFor) -> bool { diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 82e1d23e13f..5ea47c37381 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -1,6 +1,6 @@ use crate::{error::PoolResult, validate::ValidPoolTransaction, BlockId}; use futures::channel::mpsc::Receiver; -use reth_primitives::{H256, U256, U64}; +use reth_primitives::{H256, U256}; use std::{fmt, hash::Hash, sync::Arc}; pub type HashFor = <::Transaction as PoolTransaction>::Hash; From 62a99533f57e51a1eff521ef3b601962b7177ddb Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 6 Oct 2022 12:31:17 +0200 Subject: [PATCH 28/35] simplify ids --- crates/transaction-pool/src/identifier.rs | 53 +++++++++++++++ crates/transaction-pool/src/lib.rs | 1 + crates/transaction-pool/src/ordering.rs | 2 + crates/transaction-pool/src/pool/mod.rs | 6 +- crates/transaction-pool/src/pool/pending.rs | 72 ++++++++++----------- crates/transaction-pool/src/pool/queued.rs | 33 +++++----- crates/transaction-pool/src/traits.rs | 3 +- crates/transaction-pool/src/validate.rs | 10 ++- 8 files changed, 116 insertions(+), 64 deletions(-) create mode 100644 crates/transaction-pool/src/identifier.rs diff --git a/crates/transaction-pool/src/identifier.rs b/crates/transaction-pool/src/identifier.rs new file mode 100644 index 00000000000..db3aaa27011 --- /dev/null +++ b/crates/transaction-pool/src/identifier.rs @@ -0,0 +1,53 @@ +use fnv::FnvHashMap; +use reth_primitives::Address; +use std::collections::HashMap; + +/// An internal mapping of addresses. +/// +/// This assigns a _unique_ `SenderId` for a new `Address`. +#[derive(Debug)] +pub struct SenderIdentifiers { + /// The identifier to use next. + id: u64, + /// Assigned `SenderId` for an `Address`. + address_to_id: HashMap, + /// Reverse mapping of `SenderId` to `Address`. + sender_to_address: FnvHashMap, +} + +impl SenderIdentifiers { + /// Returns the address for the given identifier. + pub fn address(&self, id: &SenderId) -> Option<&Address> { + self.sender_to_address.get(id) + } + + /// Returns the `SenderId` that belongs to the given address, if it exists + pub fn sender_id(&self, addr: &Address) -> Option { + self.address_to_id.get(addr).copied() + } + + /// Returns the existing `SendId` or assigns a new one if it's missing + pub fn sender_id_or_create(&mut self, addr: Address) -> SenderId { + if let Some(id) = self.sender_id(&addr) { + return id + } + let id = self.next_id(); + self.address_to_id.insert(addr, id); + self.sender_to_address.insert(id, addr); + id + } + + /// Returns a new address + fn next_id(&mut self) -> SenderId { + let id = self.id; + self.id = self.id.wrapping_add(1); + SenderId(id) + } +} + +/// A _unique_ identifier for a sender of an address. +/// +/// This is the identifier of an internal `address` mapping that is valid in the context of this +/// program. +#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] +pub struct SenderId(u64); diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index 23bc7b7713a..9ec8ae8501d 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -15,6 +15,7 @@ use std::sync::Arc; mod client; mod config; pub mod error; +mod identifier; mod ordering; pub mod pool; mod traits; diff --git a/crates/transaction-pool/src/ordering.rs b/crates/transaction-pool/src/ordering.rs index bdf66cd88d1..456bd5b5f76 100644 --- a/crates/transaction-pool/src/ordering.rs +++ b/crates/transaction-pool/src/ordering.rs @@ -6,6 +6,8 @@ use std::fmt; /// Decides how transactions should be ordered within the pool. /// /// The returned priority must reflect natural `Ordering`. +// TODO: for custom, more advanced scoring it would be ideal to determine the priority in the +// context of the entire pool instead of standalone by alone looking at a single transaction pub trait TransactionOrdering: Send + Sync + 'static { /// Priority of a transaction. type Priority: Ord + Clone + Default + fmt::Debug + Send + Sync; diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index e18c8342566..edc35bd1ea3 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -446,7 +446,7 @@ impl GraphPool { while let Some(current_tx) = pending_transactions.pop_front() { // also add the transaction that the current transaction unlocks pending_transactions - .extend(self.queued.satisfy_and_unlock(¤t_tx.transaction.provides)); + .extend(self.queued.satisfy_and_unlock(¤t_tx.transaction.transaction_id)); let current_hash = *current_tx.transaction.hash(); @@ -497,9 +497,9 @@ impl GraphPool { for dependency in dependencies { // mark as satisfied and store the transactions that got unlocked - imports.extend(self.queued.satisfy_and_unlock(Some(&dependency))); + imports.extend(self.queued.satisfy_and_unlock(&dependency)); // prune transactions - pruned.extend(self.pending.prune_dependencies(dependency.clone())); + pruned.extend(self.pending.remove_mined(dependency.clone())); } let mut promoted = vec![]; diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index 6b1f763b71f..153186aad02 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -34,7 +34,7 @@ pub(crate) struct PendingTransactions { /// How to order transactions. ordering: Arc, /// Base fee of the next block. - next_base_fee: U256, + pending_base_fee: U256, /// Dependencies that are provided by `PendingTransaction`s provided_dependencies: HashMap, TransactionHashFor>, /// Pending transactions that are currently on hold until the `baseFee` of the pending block @@ -65,7 +65,7 @@ impl PendingTransactions { ready_transactions: Arc::new(Default::default()), ordering, independent_transactions: Default::default(), - next_base_fee: Default::default(), + pending_base_fee: Default::default(), } } /// Returns an iterator over all transactions that are _currently_ ready. @@ -82,7 +82,7 @@ impl PendingTransactions { /// time in pool (were added earlier) are returned first. /// /// NOTE: while this iterator returns transaction that pool considers valid at this point, they - /// could potentially be become invalid at point of execution. Therefore this iterator + /// could potentially be become invalid at point of execution. Therefore, this iterator /// provides a way to mark transactions that the consumer of this iterator considers invalid. In /// which case the transaction's subgraph is also automatically marked invalid, See (1.). /// Invalid transactions are skipped. @@ -97,7 +97,7 @@ impl PendingTransactions { /// Sets the given base fee and returns the old one. pub(crate) fn set_next_base_fee(&mut self, base_fee: U256) -> U256 { - std::mem::replace(&mut self.next_base_fee, base_fee) + std::mem::replace(&mut self.pending_base_fee, base_fee) } /// Returns true if the transaction is part of the queue. @@ -164,9 +164,7 @@ impl PendingTransactions { } // update dependencies - for mark in tx.transaction.provides.iter().cloned() { - self.provided_dependencies.insert(mark, hash); - } + self.provided_dependencies.insert(tx.transaction.transaction_id.clone(), hash); let priority = self.ordering.priority(&tx.transaction.transaction); @@ -193,7 +191,11 @@ impl PendingTransactions { ) -> PoolResult> { // check if we are replacing transactions let remove_hashes: HashSet<_> = - tx.provides.iter().filter_map(|mark| self.provided_dependencies.get(mark)).collect(); + if let Some(hash) = self.provided_dependencies.get(&tx.transaction_id) { + HashSet::from([hash]) + } else { + return Ok((Vec::new(), Vec::new())) + }; // early exit if we are not replacing anything. if remove_hashes.is_empty() { @@ -210,7 +212,7 @@ impl PendingTransactions { for to_remove in remove_hashes.iter().filter_map(|hash| ready.get(hash)) { // if we're attempting to replace a transaction that provides the exact same // dependencies (addr + nonce) then we check for gas price - if to_remove.provides() == tx.provides { + if to_remove.id().eq(&tx.transaction_id) { // check if underpriced // TODO check if underpriced // if tx.pending_transaction.transaction.gas_price() <= to_remove.gas_price() { @@ -229,7 +231,7 @@ impl PendingTransactions { let remove_hashes = remove_hashes.into_iter().copied().collect::>(); - let new_provides = tx.provides.iter().cloned().collect::>(); + let new_provides = HashSet::from([tx.transaction_id.clone()]); let removed_tx = self.remove_with_dependencies(remove_hashes, Some(new_provides)); Ok((removed_tx, unlocked_tx)) @@ -244,11 +246,11 @@ impl PendingTransactions { self.remove_with_dependencies(tx_hashes.to_vec(), None) } - /// Removes the transactions that provide the dependency id. + /// Removes the transactions that was mined. /// /// This will also remove all transactions that lead to the transaction that provides the /// id. - pub(crate) fn prune_dependencies( + pub(crate) fn remove_mined( &mut self, id: TransactionIdFor, ) -> Vec>> { @@ -281,7 +283,7 @@ impl PendingTransactions { tx2.unlocks.swap_remove(idx); } if tx2.unlocks.is_empty() { - Some(tx2.transaction.transaction.provides.clone()) + Some(vec![tx2.transaction.transaction.transaction_id.clone()]) } else { None } @@ -306,15 +308,13 @@ impl PendingTransactions { } // finally, remove the dependencies that this transaction provides let current_dependency = &dependency; - for dependency in &tx.provides { - let removed = self.provided_dependencies.remove(dependency); - assert_eq!( + let removed = self.provided_dependencies.remove(&tx.transaction_id); + assert_eq!( removed.as_ref(), - if current_dependency == dependency { None } else { Some(tx.hash()) }, + if current_dependency.eq(&tx.transaction_id) { None } else { Some(tx.hash()) }, "The pool contains exactly one transaction providing given tag; the removed transaction claims to provide that tag, so it has to be mapped to it's hash; qed" ); - } removed_tx.push(tx); } } @@ -334,23 +334,23 @@ impl PendingTransactions { while let Some(hash) = tx_hashes.pop() { if let Some(mut tx) = ready.remove(&hash) { - let invalidated = tx.transaction.transaction.provides.iter().filter(|mark| { - dependency_filter - .as_ref() - .map(|filter| !filter.contains(&**mark)) - .unwrap_or(true) - }); - - let mut removed_some_marks = false; - // remove entries from provided_dependencies - for mark in invalidated { - removed_some_marks = true; - self.provided_dependencies.remove(mark); - } + let id = &tx.transaction.transaction.transaction_id; + + // remove the transactions + let removed_transaction = if dependency_filter + .as_ref() + .map(|filter| !filter.contains(id)) + .unwrap_or(true) + { + self.provided_dependencies.remove(id); + true + } else { + false + }; // remove from unlocks - for mark in &tx.transaction.transaction.depends_on { - if let Some(hash) = self.provided_dependencies.get(mark) { + for dependency in &tx.transaction.transaction.depends_on { + if let Some(hash) = self.provided_dependencies.get(dependency) { if let Some(tx) = ready.get_mut(hash) { if let Some(idx) = tx.unlocks.iter().position(|i| i == hash) { tx.unlocks.swap_remove(idx); @@ -362,7 +362,7 @@ impl PendingTransactions { // remove from the independent set self.independent_transactions.remove(&tx.transaction); - if removed_some_marks { + if removed_transaction { // remove all transactions that the current one unlocks tx_hashes.append(&mut tx.unlocks); } @@ -391,8 +391,8 @@ pub(crate) struct PendingTransaction { impl PendingTransaction { /// Returns all ids this transaction satisfies. - pub(crate) fn provides(&self) -> &[TransactionIdFor] { - &self.transaction.transaction.provides + pub(crate) fn id(&self) -> &TransactionIdFor { + &self.transaction.transaction.transaction_id } } diff --git a/crates/transaction-pool/src/pool/queued.rs b/crates/transaction-pool/src/pool/queued.rs index 87ec98ddd90..f528627192a 100644 --- a/crates/transaction-pool/src/pool/queued.rs +++ b/crates/transaction-pool/src/pool/queued.rs @@ -23,7 +23,7 @@ pub(crate) struct QueuedTransactions { /// Dependencies that aren't yet provided by any transaction. required_dependencies: HashMap, HashSet>>, /// Mapping of the dependencies of a transaction to the hash of the transaction, - waiting_dependencies: HashMap>, TransactionHashFor>, + waiting_dependencies: HashMap, TransactionHashFor>, /// Transactions that are not ready yet are waiting for another tx to finish, waiting_queue: HashMap, QueuedPoolTransaction>, } @@ -59,7 +59,7 @@ impl QueuedTransactions { if let Some(_replace) = self .waiting_dependencies - .get(&tx.transaction.provides) + .get(&tx.transaction.transaction_id) .and_then(|hash| self.waiting_queue.get(hash)) { // TODO handle transaction underpriced @@ -79,7 +79,8 @@ impl QueuedTransactions { } // also track identifying dependencies - self.waiting_dependencies.insert(tx.transaction.provides.clone(), *tx.transaction.hash()); + self.waiting_dependencies + .insert(tx.transaction.transaction_id.clone(), *tx.transaction.hash()); // add tx to the queue self.waiting_queue.insert(*tx.transaction.hash(), tx); @@ -114,22 +115,17 @@ impl QueuedTransactions { /// moved to the ready queue. pub(crate) fn satisfy_and_unlock( &mut self, - dependencies: impl IntoIterator>>, + id: &TransactionIdFor, ) -> Vec> { let mut unlocked_ready = Vec::new(); - for dependency in dependencies { - let mark = dependency.as_ref(); - if let Some(tx_hashes) = self.required_dependencies.remove(mark) { - for hash in tx_hashes { - let tx = self.waiting_queue.get_mut(&hash).expect("tx is included;"); - tx.satisfy(mark); - - if tx.is_satisfied() { - let tx = self.waiting_queue.remove(&hash).expect("tx is included;"); - self.waiting_dependencies.remove(&tx.transaction.provides); - - unlocked_ready.push(tx); - } + if let Some(tx_hashes) = self.required_dependencies.remove(id) { + for hash in tx_hashes { + let tx = self.waiting_queue.get_mut(&hash).expect("tx is included;"); + tx.satisfy(id); + if tx.is_satisfied() { + let tx = self.waiting_queue.remove(&hash).expect("tx is included;"); + self.waiting_dependencies.remove(&tx.transaction.transaction_id); + unlocked_ready.push(tx); } } } @@ -147,7 +143,7 @@ impl QueuedTransactions { let mut removed = vec![]; for hash in hashes { if let Some(waiting_tx) = self.waiting_queue.remove(&hash) { - self.waiting_dependencies.remove(&waiting_tx.transaction.provides); + self.waiting_dependencies.remove(&waiting_tx.transaction.transaction_id); for dependency in waiting_tx.missing_dependencies { let remove = if let Some(required) = self.required_dependencies.get_mut(&dependency) { @@ -177,6 +173,7 @@ pub(crate) struct QueuedPoolTransaction { /// /// This will be an empty list if there are no nonce gaps across multiple transactions of the /// same sender in the pool. If there are gaps, this will include the missing transactions. + // TODO rename to nonce pub(crate) missing_dependencies: HashSet>, /// Timestamp when the tx was added. pub(crate) added_at: Instant, diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 5ea47c37381..01e5d4d5e7a 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -75,7 +75,7 @@ pub enum ChainEvent { /// EIP-1559 Base fee of the _next_ (pending) block /// /// The base fee of a block depends on the utilization of the last block and its base fee. - next_base_fee: U256, + pending_block_base_fee: U256, }, /// An existing block has been finalized. Finalized { @@ -120,6 +120,7 @@ pub trait PoolTransaction: fmt::Debug + Send + Send + 'static { fn sender(&self) -> &Self::Sender; /// Creates the unique identifier for this transaction. + // TODO change this to nonce fn id(&self) -> Self::Id; /// Returns the EIP-1559 Max base fee the caller is willing to pay. diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index 792b9ec8e81..4306218d14a 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -44,12 +44,10 @@ pub struct ValidPoolTransaction { /// /// This lists all unique transactions that need to be mined before this transaction can be /// considered `pending` and itself be included. + // TODO change this to previous nonce pub depends_on: Vec, - /// Ids that this transaction provides - /// - /// This contains the inverse of `depends_on` which provides the dependencies this transaction - /// unlocks once it's mined. - pub provides: Vec, + /// The identifier for this transaction. + pub transaction_id: T::Id, /// Whether to propagate the transaction. pub propagate: bool, // TODO add a block timestamp that marks validity @@ -68,7 +66,7 @@ impl fmt::Debug for ValidPoolTransaction { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { write!(fmt, "Transaction {{ ")?; write!(fmt, "hash: {:?}, ", &self.transaction.hash())?; - write!(fmt, "provides: {:?}, ", &self.provides)?; + write!(fmt, "provides: {:?}, ", &self.transaction_id)?; write!(fmt, "depends_on: {:?}, ", &self.depends_on)?; write!(fmt, "raw tx: {:?}", &self.transaction)?; write!(fmt, "}}")?; From 9368ec098a4c1d6f5390ba260781dcb1132535bb Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 6 Oct 2022 12:47:07 +0200 Subject: [PATCH 29/35] use address type --- crates/transaction-pool/src/traits.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 01e5d4d5e7a..3c9305d8878 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -1,6 +1,6 @@ use crate::{error::PoolResult, validate::ValidPoolTransaction, BlockId}; use futures::channel::mpsc::Receiver; -use reth_primitives::{H256, U256}; +use reth_primitives::{Address, H256, U256}; use std::{fmt, hash::Hash, sync::Arc}; pub type HashFor = <::Transaction as PoolTransaction>::Hash; @@ -110,14 +110,11 @@ pub trait PoolTransaction: fmt::Debug + Send + Send + 'static { /// Unique identifier for this transaction. type Id: fmt::Debug + fmt::LowerHex + Eq + Clone + Hash + AsRef + Send + Sync; - /// Transaction sender type. - type Sender: fmt::Debug + Eq + Clone + Hash + Send + Sync; - /// Hash of the transaction fn hash(&self) -> &Self::Hash; - /// The Sender of the transaction - fn sender(&self) -> &Self::Sender; + /// The Sender of the transaction. + fn sender(&self) -> &Address; /// Creates the unique identifier for this transaction. // TODO change this to nonce From 24c10c35a5d1b540ba303e9fb555a434014f88c0 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 6 Oct 2022 13:03:00 +0200 Subject: [PATCH 30/35] add sender info --- crates/transaction-pool/src/pool/mod.rs | 51 +++++++++++++++++++------ crates/transaction-pool/src/traits.rs | 3 ++ crates/transaction-pool/src/validate.rs | 15 ++++++-- 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index edc35bd1ea3..9ea656e9cb6 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -77,6 +77,7 @@ use crate::{ validate::ValidPoolTransaction, BlockId, PoolClient, PoolConfig, TransactionOrdering, TransactionValidator, U256, }; +use fnv::FnvHashMap; use futures::channel::mpsc::{channel, Receiver, Sender}; use parking_lot::{Mutex, RwLock}; use reth_primitives::U64; @@ -93,7 +94,7 @@ mod pending; mod queued; mod transaction; -use crate::validate::TransactionValidationOutcome; +use crate::{identifier::SenderId, validate::TransactionValidationOutcome}; pub use events::TransactionEvent; pub use pending::TransactionsIterator; @@ -271,16 +272,20 @@ where tx: TransactionValidationOutcome, ) -> PoolResult> { match tx { - TransactionValidationOutcome::Valid(tx) => { - let added = self.pool.write().add_transaction(tx)?; - - if let Some(ready) = added.as_ready() { - self.on_new_ready_transaction(ready); - } - - self.notify_event_listeners(&added); - - Ok(*added.hash()) + TransactionValidationOutcome::Valid { balance, state_nonce, transaction } => { + // TODO create `ValidPoolTransaction` + + // let added = self.pool.write().add_transaction(tx)?; + // + // if let Some(ready) = added.as_ready() { + // self.on_new_ready_transaction(ready); + // } + // + // self.notify_event_listeners(&added); + // + // Ok(*added.hash()) + + todo!() } TransactionValidationOutcome::Invalid(_tx, err) => { // TODO notify listeners about invalid @@ -347,6 +352,8 @@ where pub struct GraphPool { /// How to order transactions. ordering: Arc, + /// Contains the currently known info + sender_info: FnvHashMap, /// Sub-Pool of transactions that are ready and waiting to be executed pending: PendingTransactions, /// Sub-Pool of transactions that are waiting for state changes that eventually turn them @@ -360,7 +367,7 @@ impl GraphPool { /// Create a new graph pool instance. pub fn new(ordering: Arc) -> Self { let pending = PendingTransactions::new(Arc::clone(&ordering)); - Self { ordering, pending, queued: Default::default() } + Self { ordering, sender_info: Default::default(), pending, queued: Default::default() } } /// Updates the pool based on the changed base fee. @@ -605,6 +612,26 @@ impl AddedPendingTransaction { } } +/// Stores relevant context about a sender. +#[derive(Debug, Clone)] +struct SenderInfo { + /// current nonce of the sender + state_nonce: u64, + /// Balance of the sender at the current point. + balance: U256, + /// How many transactions of this sender are currently in the pool. + num_transactions: u64, +} + +// === impl SenderInfo === + +impl SenderInfo { + /// Creates a new entry for an incoming, not yet tracked sender. + fn new_incoming(state_nonce: u64, balance: U256) -> Self { + Self { state_nonce, balance, num_transactions: 1 } + } +} + /// Represents a transaction that was added into the pool and its state #[derive(Debug, Clone)] pub enum AddedTransaction { diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 3c9305d8878..520f49bbd29 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -120,6 +120,9 @@ pub trait PoolTransaction: fmt::Debug + Send + Send + 'static { // TODO change this to nonce fn id(&self) -> Self::Id; + /// Returns the nonce for this transaction. + fn nonce(&self) -> u64; + /// Returns the EIP-1559 Max base fee the caller is willing to pay. /// /// This will return `None` for non-EIP1559 transactions diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index 4306218d14a..8222384e995 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -1,13 +1,20 @@ //! Transaction validation abstractions. -use crate::{error::PoolError, traits::PoolTransaction}; -use reth_primitives::BlockId; +use crate::{error::PoolError, identifier::SenderId, traits::PoolTransaction}; +use reth_primitives::{BlockId, U256}; use std::fmt; /// A Result type returned after checking a transaction's validity. pub enum TransactionValidationOutcome { /// Transaction successfully validated - Valid(ValidPoolTransaction), + Valid { + /// Balance of the sender at the current point. + balance: U256, + /// current nonce of the sender + state_nonce: u64, + /// Validated transaction. + transaction: T, + }, /// The transaction is considered invalid. /// /// Note: This does not indicate whether the transaction will not be valid in the future @@ -50,6 +57,8 @@ pub struct ValidPoolTransaction { pub transaction_id: T::Id, /// Whether to propagate the transaction. pub propagate: bool, + /// Internal `Sender` identifier + pub sender_id: SenderId, // TODO add a block timestamp that marks validity } From 3002938b52753560ea4f5f5afb02deec3f3a0161 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 6 Oct 2022 13:18:54 +0200 Subject: [PATCH 31/35] more refactoring --- crates/transaction-pool/src/identifier.rs | 9 ++++ crates/transaction-pool/src/lib.rs | 8 ++-- crates/transaction-pool/src/pool/mod.rs | 6 +-- crates/transaction-pool/src/pool/pending.rs | 6 +-- crates/transaction-pool/src/pool/queued.rs | 1 - crates/transaction-pool/src/traits.rs | 50 ++++++++++----------- crates/transaction-pool/src/validate.rs | 4 +- 7 files changed, 44 insertions(+), 40 deletions(-) diff --git a/crates/transaction-pool/src/identifier.rs b/crates/transaction-pool/src/identifier.rs index db3aaa27011..d55e06f22ea 100644 --- a/crates/transaction-pool/src/identifier.rs +++ b/crates/transaction-pool/src/identifier.rs @@ -51,3 +51,12 @@ impl SenderIdentifiers { /// program. #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] pub struct SenderId(u64); + +/// A unique identifier of a transaction of a Sender. +#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] +pub struct TransactionId { + /// Sender of this transaction + pub sender: SenderId, + /// Nonce of this transaction + pub nonce: u64, +} diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index 9ec8ae8501d..f7df5dac466 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -26,7 +26,7 @@ pub use crate::{ config::PoolConfig, ordering::TransactionOrdering, pool::BasicPool, - traits::{ChainEvent, PoolTransaction, ReadyTransactions, TransactionPool}, + traits::{BestTransactions, NewBlockEvent, PoolTransaction, TransactionPool}, validate::{TransactionValidationOutcome, TransactionValidator}, }; use crate::{error::PoolResult, traits::HashFor, validate::ValidPoolTransaction}; @@ -64,7 +64,7 @@ where { type Transaction = T::Transaction; - async fn on_chain_event(&self, _event: ChainEvent) { + async fn on_new_block(&self, _event: NewBlockEvent) { // TODO perform maintenance: update pool accordingly todo!() } @@ -89,9 +89,9 @@ where self.pool.ready_transactions_listener() } - fn ready_transactions( + fn best_transactions( &self, - ) -> Box>>> { + ) -> Box>>> { Box::new(self.pool.inner().ready_transactions()) } diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 9ea656e9cb6..6e2d234f9cb 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -178,8 +178,6 @@ where } /// Validates the given transaction at the given block - /// - /// Returns future that validates single transaction at given block. async fn validate( &self, block_id: &BlockId, @@ -420,7 +418,7 @@ impl GraphPool { let tx = QueuedPoolTransaction::new(tx, self.pending.provided_dependencies()); trace!(target: "txpool", "[{:?}] {:?}", tx.transaction.hash(), tx); - // If all markers are not satisfied import to future + // If all ids are not satisfied import to queued if !tx.is_satisfied() { let hash = *tx.transaction.hash(); self.queued.add_transaction(tx)?; @@ -495,7 +493,7 @@ impl GraphPool { /// /// This will effectively remove those transactions that satisfy the dependencies. /// And queued transactions might get promoted if the pruned dependencies unlock them. - pub fn prune_dependencies( + pub fn prune_transactions( &mut self, dependencies: impl IntoIterator>, ) -> PruneResult { diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index 153186aad02..961e3b8bebf 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -1,7 +1,7 @@ use crate::{ error::PoolResult, pool::{queued::QueuedPoolTransaction, TransactionHashFor, TransactionIdFor}, - traits::ReadyTransactions, + traits::BestTransactions, validate::ValidPoolTransaction, TransactionOrdering, }; @@ -553,7 +553,7 @@ impl TransactionsIterator { /// either to awaiting set or to best set. fn independent_or_awaiting(&mut self, satisfied: usize, tx_ref: PoolTransactionRef) { if satisfied >= tx_ref.transaction.depends_on.len() { - // If we have satisfied all deps insert to best + // If we have satisfied all deps insert to the best set self.independent.insert(tx_ref); } else { // otherwise we're still waiting for some deps @@ -562,7 +562,7 @@ impl TransactionsIterator { } } -impl ReadyTransactions for TransactionsIterator { +impl BestTransactions for TransactionsIterator { fn mark_invalid(&mut self, tx: &Self::Item) { TransactionsIterator::mark_invalid(self, tx) } diff --git a/crates/transaction-pool/src/pool/queued.rs b/crates/transaction-pool/src/pool/queued.rs index f528627192a..2cfa5294c25 100644 --- a/crates/transaction-pool/src/pool/queued.rs +++ b/crates/transaction-pool/src/pool/queued.rs @@ -173,7 +173,6 @@ pub(crate) struct QueuedPoolTransaction { /// /// This will be an empty list if there are no nonce gaps across multiple transactions of the /// same sender in the pool. If there are gaps, this will include the missing transactions. - // TODO rename to nonce pub(crate) missing_dependencies: HashSet>, /// Timestamp when the tx was added. pub(crate) added_at: Instant, diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 520f49bbd29..3fda10109ad 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -15,12 +15,12 @@ pub trait TransactionPool: Send + Sync { /// The transaction type of the pool type Transaction: PoolTransaction + Send + Sync + 'static; - /// Event listener for chain events that affect the pool. + /// Event listener for when a new block was mined. /// /// Implementers need to update the pool accordingly. /// For example the base fee of the pending block is determined after a block is mined which /// affects the dynamic fee requirement of pending transactions in the pool. - async fn on_chain_event(&self, event: ChainEvent); + async fn on_new_block(&self, event: NewBlockEvent); /// Adds an _unvalidated_ transaction into the pool. /// @@ -50,9 +50,9 @@ pub trait TransactionPool: Send + Sync { /// Returns an iterator that yields transactions that are ready for block production. /// /// Consumer: Block production - fn ready_transactions( + fn best_transactions( &self, - ) -> Box>>>; + ) -> Box>>>; /// Removes all transactions corresponding to the given hashes. /// @@ -65,30 +65,25 @@ pub trait TransactionPool: Send + Sync { ) -> Vec>>; } -/// Various Events the pool listens for and needs to apply changes. +/// Event fired when a new block was mined #[derive(Debug, Clone)] -pub enum ChainEvent { - /// New best block have been added to the chain. - NewBestBlock { - /// Hash of the added block. - hash: H256, - /// EIP-1559 Base fee of the _next_ (pending) block - /// - /// The base fee of a block depends on the utilization of the last block and its base fee. - pending_block_base_fee: U256, - }, - /// An existing block has been finalized. - Finalized { - /// Hash of just finalized block. - hash: H256, - }, +pub struct NewBlockEvent { + /// Hash of the added block. + pub hash: H256, + /// EIP-1559 Base fee of the _next_ (pending) block + /// + /// The base fee of a block depends on the utilization of the last block and its base fee. + pub pending_block_base_fee: U256, + /// Provides a set of state changes that affected the accounts. + // TODO based on the account changes, we can recheck balance + pub state_changes: (), } /// An `Iterator` that only returns transactions that are ready to be executed. /// /// This makes no assumptions about the order of the transactions, but expects that _all_ /// transactions are valid (no nonce gaps.). -pub trait ReadyTransactions: Iterator + Send { +pub trait BestTransactions: Iterator + Send { /// Mark the transaction as invalid. /// /// Implementers must ensure all subsequent transaction _don't_ depend on this transaction. @@ -98,14 +93,14 @@ pub trait ReadyTransactions: Iterator + Send { } /// A no-op implementation that yields no transactions. -impl ReadyTransactions for std::iter::Empty { +impl BestTransactions for std::iter::Empty { fn mark_invalid(&mut self, _tx: &T) {} } /// Trait for transaction types used inside the pool pub trait PoolTransaction: fmt::Debug + Send + Send + 'static { /// Transaction hash type. - type Hash: fmt::Debug + fmt::LowerHex + Eq + Clone + Copy + Hash + Send + Sync + 'static; + type Hash: fmt::Debug + Eq + Clone + Copy + Hash + Send + Sync + 'static; /// Unique identifier for this transaction. type Id: fmt::Debug + fmt::LowerHex + Eq + Clone + Hash + AsRef + Send + Sync; @@ -116,13 +111,14 @@ pub trait PoolTransaction: fmt::Debug + Send + Send + 'static { /// The Sender of the transaction. fn sender(&self) -> &Address; - /// Creates the unique identifier for this transaction. - // TODO change this to nonce - fn id(&self) -> Self::Id; - /// Returns the nonce for this transaction. fn nonce(&self) -> u64; + /// Calculates the cost that this transaction is allowed to consume: + /// + /// For EIP-1559 transactions that is `feeCap x gasLimit + transferred_value` + fn cost(&self) -> U256; + /// Returns the EIP-1559 Max base fee the caller is willing to pay. /// /// This will return `None` for non-EIP1559 transactions diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index 8222384e995..960bc249810 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -32,7 +32,7 @@ pub trait TransactionValidator: Send + Sync { /// This is used by the transaction-pool check the transaction's validity against the state of /// the given block hash. /// - /// This is supposed to extend the `transaction` with its identifying markers in the graph of + /// This is supposed to extend the `transaction` with its id in the graph of /// transactions for the sender. async fn validate_transaction( &self, @@ -59,6 +59,8 @@ pub struct ValidPoolTransaction { pub propagate: bool, /// Internal `Sender` identifier pub sender_id: SenderId, + /// Total cost of the transaction: `feeCap x gasLimit + transferred_value` + pub cost: U256, // TODO add a block timestamp that marks validity } From 0eb1539b3288b67241309a2747ca8117c6fd87e2 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 6 Oct 2022 13:39:00 +0200 Subject: [PATCH 32/35] refactor: remove associated Id type --- crates/transaction-pool/src/identifier.rs | 3 +++ crates/transaction-pool/src/pool/mod.rs | 10 +++++----- crates/transaction-pool/src/pool/pending.rs | 17 ++++++++--------- crates/transaction-pool/src/pool/queued.rs | 19 ++++++++----------- crates/transaction-pool/src/traits.rs | 3 --- crates/transaction-pool/src/validate.rs | 11 +++++++---- 6 files changed, 31 insertions(+), 32 deletions(-) diff --git a/crates/transaction-pool/src/identifier.rs b/crates/transaction-pool/src/identifier.rs index d55e06f22ea..0b78ce23604 100644 --- a/crates/transaction-pool/src/identifier.rs +++ b/crates/transaction-pool/src/identifier.rs @@ -53,6 +53,9 @@ impl SenderIdentifiers { pub struct SenderId(u64); /// A unique identifier of a transaction of a Sender. +/// +/// This serves as an identifier for dependencies of a transaction: +/// A transaction with a nonce higher than the current state nonce depends on `tx.nonce - 1`. #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] pub struct TransactionId { /// Sender of this transaction diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 6e2d234f9cb..0f172273f9a 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -94,7 +94,10 @@ mod pending; mod queued; mod transaction; -use crate::{identifier::SenderId, validate::TransactionValidationOutcome}; +use crate::{ + identifier::{SenderId, TransactionId}, + validate::TransactionValidationOutcome, +}; pub use events::TransactionEvent; pub use pending::TransactionsIterator; @@ -102,9 +105,6 @@ pub use pending::TransactionsIterator; pub(crate) type TransactionHashFor = <::Transaction as PoolTransaction>::Hash; -pub(crate) type TransactionIdFor = - <::Transaction as PoolTransaction>::Id; - /// Shareable Transaction pool. pub struct BasicPool { /// Arc'ed instance of the pool internals @@ -495,7 +495,7 @@ impl GraphPool { /// And queued transactions might get promoted if the pruned dependencies unlock them. pub fn prune_transactions( &mut self, - dependencies: impl IntoIterator>, + dependencies: impl IntoIterator, ) -> PruneResult { let mut imports = vec![]; let mut pruned = vec![]; diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index 961e3b8bebf..51923396836 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -1,6 +1,7 @@ use crate::{ error::PoolResult, - pool::{queued::QueuedPoolTransaction, TransactionHashFor, TransactionIdFor}, + identifier::TransactionId, + pool::{queued::QueuedPoolTransaction, TransactionHashFor}, traits::BestTransactions, validate::ValidPoolTransaction, TransactionOrdering, @@ -36,7 +37,7 @@ pub(crate) struct PendingTransactions { /// Base fee of the next block. pending_base_fee: U256, /// Dependencies that are provided by `PendingTransaction`s - provided_dependencies: HashMap, TransactionHashFor>, + provided_dependencies: HashMap>, /// Pending transactions that are currently on hold until the `baseFee` of the pending block /// changes in favor of the parked transactions: the `pendingBlock.baseFee` must decrease /// before they can be moved to the ready pool and are ready to be executed. @@ -110,9 +111,7 @@ impl PendingTransactions { self.ready_transactions.read().get(hash).cloned() } - pub(crate) fn provided_dependencies( - &self, - ) -> &HashMap, TransactionHashFor> { + pub(crate) fn provided_dependencies(&self) -> &HashMap> { &self.provided_dependencies } @@ -252,7 +251,7 @@ impl PendingTransactions { /// id. pub(crate) fn remove_mined( &mut self, - id: TransactionIdFor, + id: TransactionId, ) -> Vec>> { let mut removed_tx = vec![]; @@ -275,7 +274,7 @@ impl PendingTransactions { let hash = tx.hash(); let mut ready = self.ready_transactions.write(); - let mut previous_dependency = |dependency| -> Option>> { + let mut previous_dependency = |dependency| -> Option> { let prev_hash = self.provided_dependencies.get(dependency)?; let tx2 = ready.get_mut(prev_hash)?; // remove hash @@ -327,7 +326,7 @@ impl PendingTransactions { pub(crate) fn remove_with_dependencies( &mut self, mut tx_hashes: Vec>, - dependency_filter: Option>>, + dependency_filter: Option>, ) -> Vec>> { let mut removed = Vec::new(); let mut ready = self.ready_transactions.write(); @@ -391,7 +390,7 @@ pub(crate) struct PendingTransaction { impl PendingTransaction { /// Returns all ids this transaction satisfies. - pub(crate) fn id(&self) -> &TransactionIdFor { + pub(crate) fn id(&self) -> &TransactionId { &self.transaction.transaction.transaction_id } } diff --git a/crates/transaction-pool/src/pool/queued.rs b/crates/transaction-pool/src/pool/queued.rs index 2cfa5294c25..457fb165cf7 100644 --- a/crates/transaction-pool/src/pool/queued.rs +++ b/crates/transaction-pool/src/pool/queued.rs @@ -1,9 +1,6 @@ use crate::{ - error::PoolResult, - pool::{TransactionHashFor, TransactionIdFor}, - traits::PoolTransaction, - validate::ValidPoolTransaction, - TransactionOrdering, + error::PoolResult, identifier::TransactionId, pool::TransactionHashFor, + traits::PoolTransaction, validate::ValidPoolTransaction, TransactionOrdering, }; use std::{ collections::{HashMap, HashSet}, @@ -21,9 +18,9 @@ use std::{ /// Keeps a set of transactions that are waiting until their dependencies are unlocked. pub(crate) struct QueuedTransactions { /// Dependencies that aren't yet provided by any transaction. - required_dependencies: HashMap, HashSet>>, + required_dependencies: HashMap>>, /// Mapping of the dependencies of a transaction to the hash of the transaction, - waiting_dependencies: HashMap, TransactionHashFor>, + waiting_dependencies: HashMap>, /// Transactions that are not ready yet are waiting for another tx to finish, waiting_queue: HashMap, QueuedPoolTransaction>, } @@ -115,7 +112,7 @@ impl QueuedTransactions { /// moved to the ready queue. pub(crate) fn satisfy_and_unlock( &mut self, - id: &TransactionIdFor, + id: &TransactionId, ) -> Vec> { let mut unlocked_ready = Vec::new(); if let Some(tx_hashes) = self.required_dependencies.remove(id) { @@ -173,7 +170,7 @@ pub(crate) struct QueuedPoolTransaction { /// /// This will be an empty list if there are no nonce gaps across multiple transactions of the /// same sender in the pool. If there are gaps, this will include the missing transactions. - pub(crate) missing_dependencies: HashSet>, + pub(crate) missing_dependencies: HashSet, /// Timestamp when the tx was added. pub(crate) added_at: Instant, } @@ -197,7 +194,7 @@ impl QueuedPoolTransaction { /// moved to the queue. pub(crate) fn new( transaction: ValidPoolTransaction, - provided: &HashMap, TransactionHashFor>, + provided: &HashMap>, ) -> Self { let missing_dependencies = transaction .depends_on @@ -214,7 +211,7 @@ impl QueuedPoolTransaction { } /// Removes the required dependency. - pub(crate) fn satisfy(&mut self, id: &TransactionIdFor) { + pub(crate) fn satisfy(&mut self, id: &TransactionId) { self.missing_dependencies.remove(id); } diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 3fda10109ad..43674033627 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -102,9 +102,6 @@ pub trait PoolTransaction: fmt::Debug + Send + Send + 'static { /// Transaction hash type. type Hash: fmt::Debug + Eq + Clone + Copy + Hash + Send + Sync + 'static; - /// Unique identifier for this transaction. - type Id: fmt::Debug + fmt::LowerHex + Eq + Clone + Hash + AsRef + Send + Sync; - /// Hash of the transaction fn hash(&self) -> &Self::Hash; diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index 960bc249810..a9c5f4431a6 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -1,6 +1,10 @@ //! Transaction validation abstractions. -use crate::{error::PoolError, identifier::SenderId, traits::PoolTransaction}; +use crate::{ + error::PoolError, + identifier::{SenderId, TransactionId}, + traits::PoolTransaction, +}; use reth_primitives::{BlockId, U256}; use std::fmt; @@ -51,10 +55,9 @@ pub struct ValidPoolTransaction { /// /// This lists all unique transactions that need to be mined before this transaction can be /// considered `pending` and itself be included. - // TODO change this to previous nonce - pub depends_on: Vec, + pub depends_on: Vec, /// The identifier for this transaction. - pub transaction_id: T::Id, + pub transaction_id: TransactionId, /// Whether to propagate the transaction. pub propagate: bool, /// Internal `Sender` identifier From 9f23ade1da21ca68cff1eee12a66c0a47367d11a Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 6 Oct 2022 13:53:07 +0200 Subject: [PATCH 33/35] ease warnings --- crates/transaction-pool/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index f7df5dac466..2c6eef4d0f0 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -1,4 +1,5 @@ #![warn(missing_docs)] // unreachable_pub, missing_debug_implementations +#![allow(unused)] // TODO(mattsse) remove after progress was made #![deny(unused_must_use, rust_2018_idioms)] #![doc(test( no_crate_inject, From 82c5c4b82470b30051e9adf965426483d11ddcad Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 6 Oct 2022 13:58:33 +0200 Subject: [PATCH 34/35] add dependency function --- crates/transaction-pool/src/identifier.rs | 30 +++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/crates/transaction-pool/src/identifier.rs b/crates/transaction-pool/src/identifier.rs index 0b78ce23604..cbebe4cbc14 100644 --- a/crates/transaction-pool/src/identifier.rs +++ b/crates/transaction-pool/src/identifier.rs @@ -1,3 +1,4 @@ +use crate::U256; use fnv::FnvHashMap; use reth_primitives::Address; use std::collections::HashMap; @@ -63,3 +64,32 @@ pub struct TransactionId { /// Nonce of this transaction pub nonce: u64, } + +// === impl TransactionId === + +impl TransactionId { + /// Create a new identifier pair + pub fn new(sender: SenderId, nonce: u64) -> Self { + Self { sender, nonce } + } + + /// Returns the id a transactions depends on + /// + /// This returns `transaction_nonce - 1` if `transaction_nonce` is higher than the + /// `on_chain_none` + pub fn dependency( + transaction_nonce: u64, + on_chain_nonce: u64, + sender: SenderId, + ) -> Option { + if transaction_nonce == on_chain_nonce { + return None + } + let prev_nonce = transaction_nonce.saturating_sub(1); + if on_chain_nonce <= prev_nonce { + Some(Self::new(sender, prev_nonce)) + } else { + None + } + } +} From af1aaf397d0b02da7407df3a62ddd43e956f37cf Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 6 Oct 2022 14:02:44 +0200 Subject: [PATCH 35/35] remove Hash associated type --- crates/transaction-pool/src/lib.rs | 12 ++--- crates/transaction-pool/src/pool/mod.rs | 56 +++++++++------------ crates/transaction-pool/src/pool/pending.rs | 48 ++++++++---------- crates/transaction-pool/src/pool/queued.rs | 25 ++++----- crates/transaction-pool/src/traits.rs | 17 +++---- crates/transaction-pool/src/validate.rs | 4 +- 6 files changed, 72 insertions(+), 90 deletions(-) diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index 2c6eef4d0f0..ca8ff4f3715 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -10,7 +10,7 @@ use futures::channel::mpsc::Receiver; use parking_lot::Mutex; -use reth_primitives::{BlockId, U256, U64}; +use reth_primitives::{BlockId, TxHash, U256, U64}; use std::sync::Arc; mod client; @@ -30,7 +30,7 @@ pub use crate::{ traits::{BestTransactions, NewBlockEvent, PoolTransaction, TransactionPool}, validate::{TransactionValidationOutcome, TransactionValidator}, }; -use crate::{error::PoolResult, traits::HashFor, validate::ValidPoolTransaction}; +use crate::{error::PoolResult, validate::ValidPoolTransaction}; /// A generic, customizable `TransactionPool` implementation. pub struct Pool { @@ -74,7 +74,7 @@ where &self, block_id: BlockId, transaction: Self::Transaction, - ) -> PoolResult> { + ) -> PoolResult { self.pool.clone().add_transaction(&block_id, transaction).await } @@ -82,11 +82,11 @@ where &self, block_id: BlockId, transactions: Vec, - ) -> PoolResult>>> { + ) -> PoolResult>> { self.pool.clone().add_transactions(&block_id, transactions).await } - fn ready_transactions_listener(&self) -> Receiver> { + fn ready_transactions_listener(&self) -> Receiver { self.pool.ready_transactions_listener() } @@ -98,7 +98,7 @@ where fn remove_invalid( &self, - _tx_hashes: &[HashFor], + _tx_hashes: &[TxHash], ) -> Vec>> { todo!() } diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 0f172273f9a..f95096fef65 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -80,7 +80,7 @@ use crate::{ use fnv::FnvHashMap; use futures::channel::mpsc::{channel, Receiver, Sender}; use parking_lot::{Mutex, RwLock}; -use reth_primitives::U64; +use reth_primitives::{TxHash, U64}; use std::{ collections::{HashMap, VecDeque}, fmt, @@ -101,10 +101,6 @@ use crate::{ pub use events::TransactionEvent; pub use pending::TransactionsIterator; -// Helper type aliases for associated types -pub(crate) type TransactionHashFor = - <::Transaction as PoolTransaction>::Hash; - /// Shareable Transaction pool. pub struct BasicPool { /// Arc'ed instance of the pool internals @@ -138,7 +134,7 @@ where &self, block_id: &BlockId, transaction: P::Transaction, - ) -> PoolResult> { + ) -> PoolResult { self.add_transactions(block_id, Some(transaction)) .await? .pop() @@ -150,7 +146,7 @@ where &self, block_id: &BlockId, transactions: impl IntoIterator, - ) -> PoolResult>>> { + ) -> PoolResult>> { let validated = self.validate_all(block_id, transactions).await?; let transactions = self.pool.add_transactions(validated.into_values()); Ok(transactions) @@ -162,8 +158,7 @@ where &self, block_id: &BlockId, transactions: impl IntoIterator, - ) -> PoolResult, TransactionValidationOutcome>> - { + ) -> PoolResult>> { // get the actual block number which is required to validate the transactions let block_number = self.resolve_block_number(block_id)?; @@ -183,7 +178,7 @@ where block_id: &BlockId, _block_number: U64, transaction: P::Transaction, - ) -> (TransactionHashFor, TransactionValidationOutcome) { + ) -> (TxHash, TransactionValidationOutcome) { let _hash = *transaction.hash(); // TODO this is where additional validate checks would go, like banned senders etc... let _res = self.pool.client().validate_transaction(block_id, transaction).await; @@ -194,7 +189,7 @@ where } /// Registers a new transaction listener and returns the receiver stream. - pub fn ready_transactions_listener(&self) -> Receiver> { + pub fn ready_transactions_listener(&self) -> Receiver { self.pool.add_ready_listener() } } @@ -214,9 +209,9 @@ pub struct PoolInner { /// Pool settings. config: PoolConfig, /// Manages listeners for transaction state change events. - event_listener: RwLock>>, + event_listener: RwLock>, /// Listeners for new ready transactions. - ready_transaction_listener: Mutex>>>, + ready_transaction_listener: Mutex>>, } // === impl PoolInner === @@ -249,7 +244,7 @@ where /// Adds a new transaction listener to the pool that gets notified about every new ready /// transaction - pub fn add_ready_listener(&self) -> Receiver> { + pub fn add_ready_listener(&self) -> Receiver { const TX_LISTENER_BUFFER_SIZE: usize = 2048; let (tx, rx) = channel(TX_LISTENER_BUFFER_SIZE); self.ready_transaction_listener.lock().push(tx); @@ -257,10 +252,7 @@ where } /// Resubmits transactions back into the pool. - pub fn resubmit( - &self, - _transactions: HashMap, ValidPoolTransaction>, - ) { + pub fn resubmit(&self, _transactions: HashMap>) { unimplemented!() } @@ -268,7 +260,7 @@ where fn add_transaction( &self, tx: TransactionValidationOutcome, - ) -> PoolResult> { + ) -> PoolResult { match tx { TransactionValidationOutcome::Valid { balance, state_nonce, transaction } => { // TODO create `ValidPoolTransaction` @@ -296,14 +288,14 @@ where pub fn add_transactions( &self, transactions: impl IntoIterator>, - ) -> Vec>> { + ) -> Vec> { // TODO check pool limits transactions.into_iter().map(|tx| self.add_transaction(tx)).collect::>() } /// Notify all listeners about the new transaction. - fn on_new_ready_transaction(&self, ready: &TransactionHashFor) { + fn on_new_ready_transaction(&self, ready: &TxHash) { let mut transaction_listeners = self.ready_transaction_listener.lock(); transaction_listeners.retain_mut(|listener| match listener.try_send(*ready) { Ok(()) => true, @@ -382,7 +374,7 @@ impl GraphPool { } /// Returns if the transaction for the given hash is already included in this pool - pub(crate) fn contains(&self, tx_hash: &TransactionHashFor) -> bool { + pub(crate) fn contains(&self, tx_hash: &TxHash) -> bool { self.queued.contains(tx_hash) || self.pending.contains(tx_hash) } @@ -504,7 +496,7 @@ impl GraphPool { // mark as satisfied and store the transactions that got unlocked imports.extend(self.queued.satisfy_and_unlock(&dependency)); // prune transactions - pruned.extend(self.pending.remove_mined(dependency.clone())); + pruned.extend(self.pending.remove_mined(dependency)); } let mut promoted = vec![]; @@ -526,7 +518,7 @@ impl GraphPool { /// Remove the given transactions from the pool. pub fn remove_invalid( &mut self, - tx_hashes: Vec>, + tx_hashes: Vec, ) -> Vec>> { // early exit in case there is no invalid transactions. if tx_hashes.is_empty() { @@ -561,7 +553,7 @@ pub struct PruneResult { /// a list of added transactions that a pruned marker satisfied pub promoted: Vec>, /// all transactions that failed to be promoted and now are discarded - pub failed: Vec, + pub failed: Vec, /// all transactions that were pruned from the ready pool pub pruned: Vec>>, } @@ -589,18 +581,18 @@ impl fmt::Debug for PruneResult { #[derive(Debug, Clone)] pub struct AddedPendingTransaction { /// the hash of the submitted transaction - hash: T::Hash, + hash: TxHash, /// transactions promoted to the ready queue - promoted: Vec, + promoted: Vec, /// transaction that failed and became discarded - discarded: Vec, + discarded: Vec, /// Transactions removed from the Ready pool removed: Vec>>, } impl AddedPendingTransaction { /// Create a new, empty transaction. - fn new(hash: T::Hash) -> Self { + fn new(hash: TxHash) -> Self { Self { hash, promoted: Default::default(), @@ -639,13 +631,13 @@ pub enum AddedTransaction { /// queued pool instead. Queued { /// the hash of the submitted transaction - hash: T::Hash, + hash: TxHash, }, } impl AddedTransaction { /// Returns the hash of the transaction if it's ready - pub fn as_ready(&self) -> Option<&T::Hash> { + pub fn as_ready(&self) -> Option<&TxHash> { if let AddedTransaction::Pending(tx) = self { Some(&tx.hash) } else { @@ -654,7 +646,7 @@ impl AddedTransaction { } /// Returns the hash of the transaction - pub fn hash(&self) -> &T::Hash { + pub fn hash(&self) -> &TxHash { match self { AddedTransaction::Pending(tx) => &tx.hash, AddedTransaction::Queued { hash } => hash, diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index 51923396836..11eb6f3f710 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -1,13 +1,9 @@ use crate::{ - error::PoolResult, - identifier::TransactionId, - pool::{queued::QueuedPoolTransaction, TransactionHashFor}, - traits::BestTransactions, - validate::ValidPoolTransaction, - TransactionOrdering, + error::PoolResult, identifier::TransactionId, pool::queued::QueuedPoolTransaction, + traits::BestTransactions, validate::ValidPoolTransaction, TransactionOrdering, }; use parking_lot::RwLock; -use reth_primitives::U256; +use reth_primitives::{TxHash, U256}; use std::{ cmp::Ordering, collections::{BTreeSet, HashMap, HashSet}, @@ -16,10 +12,8 @@ use std::{ use tracing::debug; /// Type alias for replaced transactions -pub(crate) type ReplacedTransactions = ( - Vec::Transaction>>>, - Vec>, -); +pub(crate) type ReplacedTransactions = + (Vec::Transaction>>>, Vec); /// A pool of validated transactions that are ready on the current state and are waiting to be /// included in a block. @@ -37,7 +31,7 @@ pub(crate) struct PendingTransactions { /// Base fee of the next block. pending_base_fee: U256, /// Dependencies that are provided by `PendingTransaction`s - provided_dependencies: HashMap>, + provided_dependencies: HashMap, /// Pending transactions that are currently on hold until the `baseFee` of the pending block /// changes in favor of the parked transactions: the `pendingBlock.baseFee` must decrease /// before they can be moved to the ready pool and are ready to be executed. @@ -46,7 +40,7 @@ pub(crate) struct PendingTransactions { /// /// Meaning, there are no nonce gaps in these transactions and all of them satisfy the /// `baseFee` condition: transaction `maxFeePerGas >= pendingBlock.baseFee` - ready_transactions: Arc, PendingTransaction>>>, + ready_transactions: Arc>>>, /// Independent transactions that can be included directly and don't require other /// transactions. /// @@ -102,16 +96,16 @@ impl PendingTransactions { } /// Returns true if the transaction is part of the queue. - pub(crate) fn contains(&self, hash: &TransactionHashFor) -> bool { + pub(crate) fn contains(&self, hash: &TxHash) -> bool { self.ready_transactions.read().contains_key(hash) } /// Returns the transaction for the hash if it's in the ready pool but not yet mined - pub(crate) fn get(&self, hash: &TransactionHashFor) -> Option> { + pub(crate) fn get(&self, hash: &TxHash) -> Option> { self.ready_transactions.read().get(hash).cloned() } - pub(crate) fn provided_dependencies(&self) -> &HashMap> { + pub(crate) fn provided_dependencies(&self) -> &HashMap { &self.provided_dependencies } @@ -163,7 +157,7 @@ impl PendingTransactions { } // update dependencies - self.provided_dependencies.insert(tx.transaction.transaction_id.clone(), hash); + self.provided_dependencies.insert(tx.transaction.transaction_id, hash); let priority = self.ordering.priority(&tx.transaction.transaction); @@ -230,7 +224,7 @@ impl PendingTransactions { let remove_hashes = remove_hashes.into_iter().copied().collect::>(); - let new_provides = HashSet::from([tx.transaction_id.clone()]); + let new_provides = HashSet::from([tx.transaction_id]); let removed_tx = self.remove_with_dependencies(remove_hashes, Some(new_provides)); Ok((removed_tx, unlocked_tx)) @@ -240,7 +234,7 @@ impl PendingTransactions { /// This will also remove all transactions that depend on those. pub(crate) fn clear_transactions( &mut self, - tx_hashes: &[TransactionHashFor], + tx_hashes: &[TxHash], ) -> Vec>> { self.remove_with_dependencies(tx_hashes.to_vec(), None) } @@ -282,7 +276,7 @@ impl PendingTransactions { tx2.unlocks.swap_remove(idx); } if tx2.unlocks.is_empty() { - Some(vec![tx2.transaction.transaction.transaction_id.clone()]) + Some(vec![tx2.transaction.transaction.transaction_id]) } else { None } @@ -325,7 +319,7 @@ impl PendingTransactions { /// the given filter set. pub(crate) fn remove_with_dependencies( &mut self, - mut tx_hashes: Vec>, + mut tx_hashes: Vec, dependency_filter: Option>, ) -> Vec>> { let mut removed = Vec::new(); @@ -381,7 +375,7 @@ pub(crate) struct PendingTransaction { /// Reference to the actual transaction. transaction: PoolTransactionRef, /// Tracks the transactions that get unlocked by this transaction. - unlocks: Vec>, + unlocks: Vec, /// Amount of required dependencies that are inherently provided. requires_offset: usize, } @@ -458,7 +452,7 @@ struct ParkedTransactions { /// This way we can determine when transactions where submitted to the pool. id: u64, /// All transactions that are currently parked due to their fee. - parked_transactions: HashMap, ParkedTransaction>, + parked_transactions: HashMap>, /// Same transactions but sorted by their fee and priority sorted_transactions: BTreeSet>, } @@ -479,7 +473,7 @@ pub(crate) struct ParkedTransaction { /// Reference to the actual transaction. transaction: PoolTransactionRef, /// Tracks the transactions that get unlocked by this transaction. - unlocks: Vec>, + unlocks: Vec, /// Amount of required dependencies that are inherently provided requires_offset: usize, } @@ -522,10 +516,10 @@ impl Ord for ParkedTransactionRef { /// An iterator that returns transactions that can be executed on the current state. pub struct TransactionsIterator { - all: HashMap, PendingTransaction>, - awaiting: HashMap, (usize, PoolTransactionRef)>, + all: HashMap>, + awaiting: HashMap)>, independent: BTreeSet>, - invalid: HashSet>, + invalid: HashSet, } // == impl TransactionsIterator == diff --git a/crates/transaction-pool/src/pool/queued.rs b/crates/transaction-pool/src/pool/queued.rs index 457fb165cf7..3333781f4ef 100644 --- a/crates/transaction-pool/src/pool/queued.rs +++ b/crates/transaction-pool/src/pool/queued.rs @@ -1,7 +1,8 @@ use crate::{ - error::PoolResult, identifier::TransactionId, pool::TransactionHashFor, - traits::PoolTransaction, validate::ValidPoolTransaction, TransactionOrdering, + error::PoolResult, identifier::TransactionId, traits::PoolTransaction, + validate::ValidPoolTransaction, TransactionOrdering, }; +use reth_primitives::TxHash; use std::{ collections::{HashMap, HashSet}, fmt, @@ -18,11 +19,11 @@ use std::{ /// Keeps a set of transactions that are waiting until their dependencies are unlocked. pub(crate) struct QueuedTransactions { /// Dependencies that aren't yet provided by any transaction. - required_dependencies: HashMap>>, + required_dependencies: HashMap>, /// Mapping of the dependencies of a transaction to the hash of the transaction, - waiting_dependencies: HashMap>, + waiting_dependencies: HashMap, /// Transactions that are not ready yet are waiting for another tx to finish, - waiting_queue: HashMap, QueuedPoolTransaction>, + waiting_queue: HashMap>, } // == impl QueuedTransactions == @@ -70,14 +71,14 @@ impl QueuedTransactions { // add all missing dependencies for dependency in &tx.missing_dependencies { self.required_dependencies - .entry(dependency.clone()) + .entry(*dependency) .or_default() .insert(*tx.transaction.hash()); } // also track identifying dependencies self.waiting_dependencies - .insert(tx.transaction.transaction_id.clone(), *tx.transaction.hash()); + .insert(tx.transaction.transaction_id, *tx.transaction.hash()); // add tx to the queue self.waiting_queue.insert(*tx.transaction.hash(), tx); @@ -86,19 +87,19 @@ impl QueuedTransactions { } /// Returns true if given transaction is part of the queue - pub(crate) fn contains(&self, hash: &TransactionHashFor) -> bool { + pub(crate) fn contains(&self, hash: &TxHash) -> bool { self.waiting_queue.contains_key(hash) } /// Returns the transaction for the hash if it's waiting - pub(crate) fn get(&self, tx_hash: &TransactionHashFor) -> Option<&QueuedPoolTransaction> { + pub(crate) fn get(&self, tx_hash: &TxHash) -> Option<&QueuedPoolTransaction> { self.waiting_queue.get(tx_hash) } /// Returns the transactions for the given hashes, `None` if no transaction exists pub(crate) fn get_all( &self, - tx_hashes: &[TransactionHashFor], + tx_hashes: &[TxHash], ) -> Vec>>> { tx_hashes .iter() @@ -135,7 +136,7 @@ impl QueuedTransactions { /// Returns all removed transactions. pub(crate) fn remove( &mut self, - hashes: Vec>, + hashes: Vec, ) -> Vec>> { let mut removed = vec![]; for hash in hashes { @@ -194,7 +195,7 @@ impl QueuedPoolTransaction { /// moved to the queue. pub(crate) fn new( transaction: ValidPoolTransaction, - provided: &HashMap>, + provided: &HashMap, ) -> Self { let missing_dependencies = transaction .depends_on diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 43674033627..b750b84f526 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -1,10 +1,8 @@ use crate::{error::PoolResult, validate::ValidPoolTransaction, BlockId}; use futures::channel::mpsc::Receiver; -use reth_primitives::{Address, H256, U256}; +use reth_primitives::{Address, TxHash, H256, U256}; use std::{fmt, hash::Hash, sync::Arc}; -pub type HashFor = <::Transaction as PoolTransaction>::Hash; - /// General purpose abstraction fo a transaction-pool. /// /// This is intended to be used by API-consumers such as RPC that need inject new incoming, @@ -29,7 +27,7 @@ pub trait TransactionPool: Send + Sync { &self, block_id: BlockId, transaction: Self::Transaction, - ) -> PoolResult>; + ) -> PoolResult; /// Adds the given _unvalidated_ transaction into the pool. /// @@ -40,12 +38,12 @@ pub trait TransactionPool: Send + Sync { &self, block_id: BlockId, transactions: Vec, - ) -> PoolResult>>>; + ) -> PoolResult>>; /// Returns a new Stream that yields transactions hashes for new ready transactions. /// /// Consumer: RPC - fn ready_transactions_listener(&self) -> Receiver>; + fn ready_transactions_listener(&self) -> Receiver; /// Returns an iterator that yields transactions that are ready for block production. /// @@ -61,7 +59,7 @@ pub trait TransactionPool: Send + Sync { /// Consumer: Block production fn remove_invalid( &self, - tx_hashes: &[HashFor], + tx_hashes: &[TxHash], ) -> Vec>>; } @@ -99,11 +97,8 @@ impl BestTransactions for std::iter::Empty { /// Trait for transaction types used inside the pool pub trait PoolTransaction: fmt::Debug + Send + Send + 'static { - /// Transaction hash type. - type Hash: fmt::Debug + Eq + Clone + Copy + Hash + Send + Sync + 'static; - /// Hash of the transaction - fn hash(&self) -> &Self::Hash; + fn hash(&self) -> &TxHash; /// The Sender of the transaction. fn sender(&self) -> &Address; diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index a9c5f4431a6..81ddf8ced34 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -5,7 +5,7 @@ use crate::{ identifier::{SenderId, TransactionId}, traits::PoolTransaction, }; -use reth_primitives::{BlockId, U256}; +use reth_primitives::{BlockId, TxHash, U256}; use std::fmt; /// A Result type returned after checking a transaction's validity. @@ -71,7 +71,7 @@ pub struct ValidPoolTransaction { impl ValidPoolTransaction { /// Returns the hash of the transaction - pub fn hash(&self) -> &T::Hash { + pub fn hash(&self) -> &TxHash { self.transaction.hash() } }