diff --git a/Cargo.lock b/Cargo.lock index f2ff533ab9..9919f959ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5870,10 +5870,12 @@ dependencies = [ "async-trait", "auto_impl", "bitflags 1.3.2", + "criterion", "fnv", "futures-util", "parking_lot 0.12.1", "paste", + "proptest", "rand 0.8.5", "reth-interfaces", "reth-metrics", diff --git a/crates/transaction-pool/Cargo.toml b/crates/transaction-pool/Cargo.toml index 6866cf1ea2..0275c0313d 100644 --- a/crates/transaction-pool/Cargo.toml +++ b/crates/transaction-pool/Cargo.toml @@ -44,12 +44,20 @@ auto_impl = "1.0" # testing rand = { workspace = true, optional = true } paste = { version = "1.0", optional = true } +proptest = { version = "1.0", optional = true } [dev-dependencies] paste = "1.0" rand = "0.8" +proptest = "1.0" +criterion = "0.5" [features] default = ["serde"] serde = ["dep:serde"] test-utils = ["rand", "paste", "serde"] +arbitrary = ["proptest", "reth-primitives/arbitrary"] + +[[bench]] +name = "reorder" +harness = false diff --git a/crates/transaction-pool/benches/reorder.rs b/crates/transaction-pool/benches/reorder.rs new file mode 100644 index 0000000000..129f1c17ff --- /dev/null +++ b/crates/transaction-pool/benches/reorder.rs @@ -0,0 +1,220 @@ +use criterion::{ + black_box, criterion_group, criterion_main, measurement::WallTime, BenchmarkGroup, Criterion, +}; +use proptest::{ + prelude::*, + strategy::{Strategy, ValueTree}, + test_runner::TestRunner, +}; +use reth_transaction_pool::test_utils::MockTransaction; + +/// Transaction Pool trait for benching. +pub trait BenchTxPool: Default { + fn add_transaction(&mut self, tx: MockTransaction); + fn reorder(&mut self, base_fee: u64); +} + +pub fn txpool_reordering(c: &mut Criterion) { + let mut group = c.benchmark_group("Transaction Pool Reordering"); + + for seed_size in [1_000, 10_000, 50_000, 100_000] { + for input_size in [10, 100, 1_000] { + let (txs, new_txs, base_fee) = generate_test_data(seed_size, input_size); + + use implementations::*; + + // Vanilla sorting of unsorted collection + txpool_reordering_bench::( + &mut group, + "VecTxPoolSortStable", + txs.clone(), + new_txs.clone(), + base_fee, + ); + + // Unstable sorting of unsorted collection + txpool_reordering_bench::( + &mut group, + "VecTxPoolSortUnstable", + txs.clone(), + new_txs.clone(), + base_fee, + ); + + // BinaryHeap that is resorted on each update + txpool_reordering_bench::( + &mut group, + "BinaryHeapTxPool", + txs, + new_txs, + base_fee, + ); + } + } +} + +fn txpool_reordering_bench( + group: &mut BenchmarkGroup, + description: &str, + seed: Vec, + new_txs: Vec, + base_fee: u64, +) { + let setup = || { + let mut txpool = T::default(); + txpool.reorder(base_fee); + + for tx in seed.iter() { + txpool.add_transaction(tx.clone()); + } + (txpool, new_txs.clone()) + }; + + let group_id = format!( + "txpool | seed size: {} | input size: {} | {}", + seed.len(), + new_txs.len(), + description + ); + group.bench_function(group_id, |b| { + b.iter_with_setup(setup, |(mut txpool, new_txs)| { + black_box({ + // Reorder with new base fee + let bigger_base_fee = base_fee.saturating_add(10); + txpool.reorder(bigger_base_fee); + + // Reorder with new base fee after adding transactions. + for new_tx in new_txs { + txpool.add_transaction(new_tx); + } + let smaller_base_fee = base_fee.saturating_sub(10); + txpool.reorder(smaller_base_fee); + }) + }); + }); +} + +fn generate_test_data( + seed_size: usize, + input_size: usize, +) -> (Vec, Vec, u64) { + let config = ProptestConfig::default(); + let mut runner = TestRunner::new(config); + + let txs = prop::collection::vec(any::(), seed_size) + .new_tree(&mut runner) + .unwrap() + .current(); + + let new_txs = prop::collection::vec(any::(), input_size) + .new_tree(&mut runner) + .unwrap() + .current(); + + let base_fee = any::().new_tree(&mut runner).unwrap().current(); + + (txs, new_txs, base_fee) +} + +mod implementations { + use super::*; + use reth_transaction_pool::PoolTransaction; + use std::collections::BinaryHeap; + + /// This implementation appends the transactions and uses [Vec::sort_by] function for sorting. + #[derive(Default)] + pub struct VecTxPoolSortStable { + inner: Vec, + } + + impl BenchTxPool for VecTxPoolSortStable { + fn add_transaction(&mut self, tx: MockTransaction) { + self.inner.push(tx); + } + + fn reorder(&mut self, base_fee: u64) { + self.inner.sort_by(|a, b| { + a.effective_tip_per_gas(base_fee) + .expect("exists") + .cmp(&b.effective_tip_per_gas(base_fee).expect("exists")) + }) + } + } + + /// This implementation appends the transactions and uses [Vec::sort_unstable_by] function for + /// sorting. + #[derive(Default)] + pub struct VecTxPoolSortUnstable { + inner: Vec, + } + + impl BenchTxPool for VecTxPoolSortUnstable { + fn add_transaction(&mut self, tx: MockTransaction) { + self.inner.push(tx); + } + + fn reorder(&mut self, base_fee: u64) { + self.inner.sort_unstable_by(|a, b| { + a.effective_tip_per_gas(base_fee) + .expect("exists") + .cmp(&b.effective_tip_per_gas(base_fee).expect("exists")) + }) + } + } + + struct MockTransactionWithPriority { + tx: MockTransaction, + priority: u128, + } + + impl PartialEq for MockTransactionWithPriority { + fn eq(&self, other: &Self) -> bool { + self.priority.eq(&other.priority) + } + } + + impl Eq for MockTransactionWithPriority {} + + impl PartialOrd for MockTransactionWithPriority { + fn partial_cmp(&self, other: &Self) -> Option { + self.priority.partial_cmp(&other.priority) + } + } + + impl Ord for MockTransactionWithPriority { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.priority.cmp(&other.priority) + } + } + + /// This implementation uses BinaryHeap which is drained and reconstructed on each reordering. + #[derive(Default)] + pub struct BinaryHeapTxPool { + inner: BinaryHeap, + base_fee: Option, + } + + impl BenchTxPool for BinaryHeapTxPool { + fn add_transaction(&mut self, tx: MockTransaction) { + let priority = self + .base_fee + .as_ref() + .map(|bf| tx.effective_tip_per_gas(*bf).expect("set")) + .unwrap_or_default(); + self.inner.push(MockTransactionWithPriority { tx, priority }); + } + + fn reorder(&mut self, base_fee: u64) { + self.base_fee = Some(base_fee); + + let drained = self.inner.drain(); + self.inner = BinaryHeap::from_iter(drained.map(|mock| { + let priority = mock.tx.effective_tip_per_gas(base_fee).expect("set"); + MockTransactionWithPriority { tx: mock.tx, priority } + })); + } + } +} + +criterion_group!(reorder, txpool_reordering); +criterion_main!(reorder); diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index d4c25f047e..81b5e224f7 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -10,8 +10,7 @@ rust_2018_idioms, unreachable_pub, missing_debug_implementations, - rustdoc::broken_intra_doc_links, - unused_crate_dependencies + rustdoc::broken_intra_doc_links )] #![doc(test( no_crate_inject, diff --git a/crates/transaction-pool/src/test_utils/mock.rs b/crates/transaction-pool/src/test_utils/mock.rs index 62be5c66ef..94d3050323 100644 --- a/crates/transaction-pool/src/test_utils/mock.rs +++ b/crates/transaction-pool/src/test_utils/mock.rs @@ -14,7 +14,7 @@ use rand::{ use reth_primitives::{ constants::MIN_PROTOCOL_BASE_FEE, hex, Address, FromRecoveredTransaction, IntoRecoveredTransaction, Signature, Transaction, TransactionKind, TransactionSigned, - TransactionSignedEcRecovered, TxEip1559, TxHash, TxLegacy, TxType, H256, U128, U256, + TransactionSignedEcRecovered, TxEip1559, TxEip2930, TxHash, TxLegacy, TxType, H256, U128, U256, }; use std::{ops::Range, sync::Arc, time::Instant}; @@ -360,6 +360,21 @@ impl PoolTransaction for MockTransaction { } } + fn effective_tip_per_gas(&self, base_fee: u64) -> Option { + let base_fee = base_fee as u128; + let max_fee_per_gas = self.max_fee_per_gas(); + if max_fee_per_gas < base_fee { + return None + } + + let fee = max_fee_per_gas - base_fee; + if let Some(priority_fee) = self.max_priority_fee_per_gas() { + return Some(fee.min(priority_fee)) + } + + Some(fee) + } + fn kind(&self) -> &TransactionKind { match self { MockTransaction::Legacy { to, .. } => to, @@ -461,6 +476,66 @@ impl IntoRecoveredTransaction for MockTransaction { } } +#[cfg(any(test, feature = "arbitrary"))] +impl proptest::arbitrary::Arbitrary for MockTransaction { + type Parameters = (); + type Strategy = proptest::strategy::BoxedStrategy; + + fn arbitrary_with(_: Self::Parameters) -> Self::Strategy { + use proptest::prelude::{any, Strategy}; + + any::<(Transaction, Address, H256)>() + .prop_map(|(tx, sender, tx_hash)| match &tx { + Transaction::Legacy(TxLegacy { + nonce, + gas_price, + gas_limit, + to, + value, + input, + .. + }) | + Transaction::Eip2930(TxEip2930 { + nonce, + gas_price, + gas_limit, + to, + value, + input, + .. + }) => MockTransaction::Legacy { + sender, + hash: tx_hash, + nonce: *nonce, + gas_price: *gas_price, + gas_limit: *gas_limit, + to: *to, + value: U256::from(*value), + }, + Transaction::Eip1559(TxEip1559 { + nonce, + gas_limit, + max_fee_per_gas, + max_priority_fee_per_gas, + to, + value, + input, + .. + }) => MockTransaction::Eip1559 { + sender, + hash: tx_hash, + nonce: *nonce, + max_fee_per_gas: *max_fee_per_gas, + max_priority_fee_per_gas: *max_priority_fee_per_gas, + gas_limit: *gas_limit, + to: *to, + value: U256::from(*value), + }, + }) + .boxed() + } +} + #[derive(Default)] pub struct MockTransactionFactory { pub(crate) ids: SenderIdentifiers, diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 9d3319832a..c4c1b513a4 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -491,6 +491,12 @@ pub trait PoolTransaction: /// This will return `None` for non-EIP1559 transactions fn max_priority_fee_per_gas(&self) -> Option; + /// Returns the effective tip for this transaction. + /// + /// For EIP-1559 transactions: `min(max_fee_per_gas - base_fee, max_priority_fee_per_gas)`. + /// For legacy transactions: `gas_price - base_fee`. + fn effective_tip_per_gas(&self, base_fee: u64) -> Option; + /// Returns the transaction's [`TransactionKind`], which is the address of the recipient or /// [`TransactionKind::Create`] if the transaction is a contract creation. fn kind(&self) -> &TransactionKind; @@ -599,6 +605,14 @@ impl PoolTransaction for PooledTransaction { } } + /// Returns the effective tip for this transaction. + /// + /// For EIP-1559 transactions: `min(max_fee_per_gas - base_fee, max_priority_fee_per_gas)`. + /// For legacy transactions: `gas_price - base_fee`. + fn effective_tip_per_gas(&self, base_fee: u64) -> Option { + self.transaction.effective_tip_per_gas(base_fee) + } + /// Returns the transaction's [`TransactionKind`], which is the address of the recipient or /// [`TransactionKind::Create`] if the transaction is a contract creation. fn kind(&self) -> &TransactionKind { diff --git a/crates/transaction-pool/src/validate/mod.rs b/crates/transaction-pool/src/validate/mod.rs index 2e96099f93..3b374e5ab8 100644 --- a/crates/transaction-pool/src/validate/mod.rs +++ b/crates/transaction-pool/src/validate/mod.rs @@ -179,6 +179,14 @@ impl ValidPoolTransaction { self.transaction.max_fee_per_gas() } + /// Returns the effective tip for this transaction. + /// + /// For EIP-1559 transactions: `min(max_fee_per_gas - base_fee, max_priority_fee_per_gas)`. + /// For legacy transactions: `gas_price - base_fee`. + pub fn effective_tip_per_gas(&self, base_fee: u64) -> Option { + self.transaction.effective_tip_per_gas(base_fee) + } + /// Maximum amount of gas that the transaction is allowed to consume. pub fn gas_limit(&self) -> u64 { self.transaction.gas_limit()