diff --git a/crates/transaction-pool/src/pool/parked.rs b/crates/transaction-pool/src/pool/parked.rs index da656381346c..d985ea4cf8ee 100644 --- a/crates/transaction-pool/src/pool/parked.rs +++ b/crates/transaction-pool/src/pool/parked.rs @@ -152,7 +152,7 @@ impl ParkedPool { ) -> Vec>> { if self.len() <= limit.max_txs { // if we are below the limits, we don't need to drop anything - return Vec::new() + return Vec::new(); } let mut removed = Vec::new(); @@ -173,7 +173,7 @@ impl ParkedPool { } } drop -= list.len(); - continue + continue; } // Otherwise drop only last few transactions @@ -252,7 +252,7 @@ impl ParkedPool> { // still parked -> skip descendant transactions 'this: while let Some((peek, _)) = iter.peek() { if peek.sender != id.sender { - break 'this + break 'this; } iter.next(); } @@ -465,8 +465,8 @@ impl Ord for QueuedOrd { #[cfg(test)] mod tests { use super::*; - use crate::test_utils::{MockTransaction, MockTransactionFactory}; - use reth_primitives::address; + use crate::test_utils::{MockTransaction, MockTransactionFactory, MockTransactionSet}; + use reth_primitives::{address, TxType}; use std::collections::HashSet; #[test] @@ -530,31 +530,29 @@ mod tests { let mut f = MockTransactionFactory::default(); let mut pool = ParkedPool::>::default(); - let a = address!("000000000000000000000000000000000000000a"); - let b = address!("000000000000000000000000000000000000000b"); - let c = address!("000000000000000000000000000000000000000c"); - let d = address!("000000000000000000000000000000000000000d"); + let a_sender = address!("000000000000000000000000000000000000000a"); + let b_sender = address!("000000000000000000000000000000000000000b"); + let c_sender = address!("000000000000000000000000000000000000000c"); + let d_sender = address!("000000000000000000000000000000000000000d"); - // TODO: make creating these mock tx chains easier // create a chain of transactions by sender A, B, C - let a1 = MockTransaction::eip1559().with_sender(a); - let a2 = a1.next(); - let a3 = a2.next(); - let a4 = a3.next(); + let mut tx_set = MockTransactionSet::dependent(a_sender, 0, 4, TxType::EIP1559); + let a = tx_set.clone().into_vec(); - let b1 = MockTransaction::eip1559().with_sender(b); - let b2 = b1.next(); - let b3 = b2.next(); + let b = MockTransactionSet::dependent(b_sender, 0, 3, TxType::EIP1559).into_vec(); + tx_set.extend(b.clone()); // C has the same number of txs as B - let c1 = MockTransaction::eip1559().with_sender(c); - let c2 = c1.next(); - let c3 = c2.next(); + let c = MockTransactionSet::dependent(c_sender, 0, 3, TxType::EIP1559).into_vec(); + tx_set.extend(c.clone()); - let d1 = MockTransaction::eip1559().with_sender(d); + let d = MockTransactionSet::dependent(d_sender, 0, 1, TxType::EIP1559).into_vec(); + tx_set.extend(d.clone()); + + let all_txs = tx_set.into_vec(); // just construct a list of all txs to add - let expected_parked = vec![c1.clone(), c2.clone(), c3.clone(), d1.clone()] + let expected_parked = vec![c[0].clone(), c[1].clone(), c[2].clone(), d[0].clone()] .into_iter() .map(|tx| (tx.sender(), tx.nonce())) .collect::>(); @@ -563,18 +561,17 @@ mod tests { // txs based on when they were submitted, removing the oldest txs first, until the pool is // not over the limit let expected_removed = vec![ - a1.clone(), - a2.clone(), - a3.clone(), - a4.clone(), - b1.clone(), - b2.clone(), - b3.clone(), + a[0].clone(), + a[1].clone(), + a[2].clone(), + a[3].clone(), + b[0].clone(), + b[1].clone(), + b[2].clone(), ] .into_iter() .map(|tx| (tx.sender(), tx.nonce())) .collect::>(); - let all_txs = vec![a1, a2, a3, a4, b1, b2, b3, c1, c2, c3, d1]; // add all the transactions to the pool for tx in all_txs { @@ -608,41 +605,30 @@ mod tests { let mut f = MockTransactionFactory::default(); let mut pool = ParkedPool::>::default(); - let a = address!("000000000000000000000000000000000000000a"); - let b = address!("000000000000000000000000000000000000000b"); - let c = address!("000000000000000000000000000000000000000c"); - let d = address!("000000000000000000000000000000000000000d"); + let a_sender = address!("000000000000000000000000000000000000000a"); + let b_sender = address!("000000000000000000000000000000000000000b"); + let c_sender = address!("000000000000000000000000000000000000000c"); + let d_sender = address!("000000000000000000000000000000000000000d"); // create a chain of transactions by sender A, B, C - let a1 = MockTransaction::eip1559().with_sender(a); - let a2 = a1.next(); - let a3 = a2.next(); - let a4 = a3.next(); + let mut tx_set = + MockTransactionSet::dependent(a_sender, 0, 4, reth_primitives::TxType::EIP1559); + let a = tx_set.clone().into_vec(); - let b1 = MockTransaction::eip1559().with_sender(b); - let b2 = b1.next(); - let b3 = b2.next(); + let b = MockTransactionSet::dependent(b_sender, 0, 3, reth_primitives::TxType::EIP1559) + .into_vec(); + tx_set.extend(b.clone()); // C has the same number of txs as B - let c1 = MockTransaction::eip1559().with_sender(c); - let c2 = c1.next(); - let c3 = c2.next(); - - let d1 = MockTransaction::eip1559().with_sender(d); - - let all_txs = vec![ - a1.clone(), - a2.clone(), - a3.clone(), - a4.clone(), - b1.clone(), - b2.clone(), - b3.clone(), - c1.clone(), - c2.clone(), - c3.clone(), - d1.clone(), - ]; + let c = MockTransactionSet::dependent(c_sender, 0, 3, reth_primitives::TxType::EIP1559) + .into_vec(); + tx_set.extend(c.clone()); + + let d = MockTransactionSet::dependent(d_sender, 0, 1, reth_primitives::TxType::EIP1559) + .into_vec(); + tx_set.extend(d.clone()); + + let all_txs = tx_set.into_vec(); // add all the transactions to the pool for tx in all_txs { @@ -656,12 +642,15 @@ mod tests { .map(|s| s.sender_id) .collect::>(); assert_eq!(senders.len(), 4); - let expected_senders = - vec![d, c, b, a].into_iter().map(|s| f.ids.sender_id(&s).unwrap()).collect::>(); + let expected_senders = vec![d_sender, c_sender, b_sender, a_sender] + .into_iter() + .map(|s| f.ids.sender_id(&s).unwrap()) + .collect::>(); assert_eq!(senders, expected_senders); + // manually order the txs let mut pool = ParkedPool::>::default(); - let all_txs = vec![a1, b1, c1, d1, a2, b2, c2, a3, b3, c3, a4]; + let all_txs = vec![d[0].clone(), b[0].clone(), c[0].clone(), a[0].clone()]; // add all the transactions to the pool for tx in all_txs { @@ -674,8 +663,10 @@ mod tests { .map(|s| s.sender_id) .collect::>(); assert_eq!(senders.len(), 4); - let expected_senders = - vec![a, c, b, d].into_iter().map(|s| f.ids.sender_id(&s).unwrap()).collect::>(); + let expected_senders = vec![a_sender, c_sender, b_sender, d_sender] + .into_iter() + .map(|s| f.ids.sender_id(&s).unwrap()) + .collect::>(); assert_eq!(senders, expected_senders); } } diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index 34c2d0f711db..44409c68543f 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -185,7 +185,7 @@ impl PendingPool { // Remove all dependent transactions. 'this: while let Some((next_id, next_tx)) = transactions_iter.peek() { if next_id.sender != id.sender { - break 'this + break 'this; } removed.push(Arc::clone(&next_tx.transaction)); transactions_iter.next(); @@ -228,7 +228,7 @@ impl PendingPool { // Remove all dependent transactions. 'this: while let Some((next_id, next_tx)) = transactions_iter.peek() { if next_id.sender != id.sender { - break 'this + break 'this; } removed.push(Arc::clone(&next_tx.transaction)); transactions_iter.next(); @@ -420,12 +420,12 @@ impl PendingPool { } } - return + return; } if !remove_locals && tx.transaction.is_local() { non_local_senders -= 1; - continue + continue; } total_size += tx.transaction.size(); @@ -460,7 +460,7 @@ impl PendingPool { self.remove_to_limit(&limit, false, &mut removed); if self.size() <= limit.max_size && self.len() <= limit.max_txs { - return removed + return removed; } // now repeat for local transactions @@ -570,7 +570,7 @@ mod tests { use super::*; use crate::{ - test_utils::{MockOrdering, MockTransaction, MockTransactionFactory}, + test_utils::{MockOrdering, MockTransaction, MockTransactionFactory, MockTransactionSet}, PoolTransaction, }; @@ -661,31 +661,31 @@ mod tests { let mut f = MockTransactionFactory::default(); let mut pool = PendingPool::new(MockOrdering::default()); - let a = address!("000000000000000000000000000000000000000a"); - let b = address!("000000000000000000000000000000000000000b"); - let c = address!("000000000000000000000000000000000000000c"); - let d = address!("000000000000000000000000000000000000000d"); + let a_sender = address!("000000000000000000000000000000000000000a"); + let b_sender = address!("000000000000000000000000000000000000000b"); + let c_sender = address!("000000000000000000000000000000000000000c"); + let d_sender = address!("000000000000000000000000000000000000000d"); // create a chain of transactions by sender A, B, C - let a1 = MockTransaction::eip1559().with_sender(a); - let a2 = a1.next(); - let a3 = a2.next(); - let a4 = a3.next(); + let mut tx_set = + MockTransactionSet::dependent(a_sender, 0, 4, reth_primitives::TxType::EIP1559); + let a = tx_set.clone().into_vec(); - let b1 = MockTransaction::eip1559().with_sender(b); - let b2 = b1.next(); - let b3 = b2.next(); + let b = MockTransactionSet::dependent(b_sender, 0, 3, reth_primitives::TxType::EIP1559) + .into_vec(); + tx_set.extend(b.clone()); // C has the same number of txs as B - let c1 = MockTransaction::eip1559().with_sender(c); - let c2 = c1.next(); - let c3 = c2.next(); + let c = MockTransactionSet::dependent(c_sender, 0, 3, reth_primitives::TxType::EIP1559) + .into_vec(); + tx_set.extend(c.clone()); - let d1 = MockTransaction::eip1559().with_sender(d); + let d = MockTransactionSet::dependent(d_sender, 0, 1, reth_primitives::TxType::EIP1559) + .into_vec(); + tx_set.extend(d.clone()); // add all the transactions to the pool - let all_txs = - vec![a1, a2, a3, a4.clone(), b1, b2, b3.clone(), c1, c2, c3.clone(), d1.clone()]; + let all_txs = tx_set.into_vec(); for tx in all_txs { pool.add_transaction(f.validated_arc(tx), 0); } @@ -694,8 +694,10 @@ mod tests { // the independent set is the roots of each of these tx chains, these are the highest // nonces for each sender - let expected_highest_nonces = - vec![d1, c3, b3, a4].iter().map(|tx| (tx.sender(), tx.nonce())).collect::>(); + let expected_highest_nonces = vec![d[0].clone(), c[2].clone(), b[2].clone(), a[3].clone()] + .iter() + .map(|tx| (tx.sender(), tx.nonce())) + .collect::>(); let actual_highest_nonces = pool .highest_nonces .iter() diff --git a/crates/transaction-pool/src/test_utils/mock.rs b/crates/transaction-pool/src/test_utils/mock.rs index ea1a0509d1d1..a1c778af8857 100644 --- a/crates/transaction-pool/src/test_utils/mock.rs +++ b/crates/transaction-pool/src/test_utils/mock.rs @@ -246,6 +246,55 @@ impl MockTransaction { } } + /// Returns a new EIP2930 transaction with random address and hash and empty values + pub fn eip2930() -> Self { + MockTransaction::Eip2930 { + hash: B256::random(), + sender: Address::random(), + nonce: 0, + to: TransactionKind::Call(Address::random()), + gas_limit: 0, + input: Bytes::new(), + value: Default::default(), + gas_price: 0, + accesslist: Default::default(), + } + } + + /// Returns a new deposit transaction with random address and hash and empty values + #[cfg(feature = "optimism")] + pub fn deposit() -> Self { + MockTransaction::Deposit(TxDeposit { + source_hash: B256::random(), + from: Address::random(), + to: TransactionKind::Call(Address::random()), + mint: Some(0), + value: Default::default(), + gas_limit: 0, + is_system_transaction: false, + input: Bytes::new(), + }) + } + + /// Creates a new transaction with the given [TxType]. + /// + /// See the default constructors for each of the transaction types: + /// + /// * [MockTransaction::legacy] + /// * [MockTransaction::eip2930] + /// * [MockTransaction::eip1559] + /// * [MockTransaction::eip4844] + pub fn new_from_type(tx_type: TxType) -> Self { + match tx_type { + TxType::Legacy => Self::legacy(), + TxType::EIP2930 => Self::eip2930(), + TxType::EIP1559 => Self::eip1559(), + TxType::EIP4844 => Self::eip4844(), + #[cfg(feature = "optimism")] + TxType::DEPOSIT => Self::deposit(), + } + } + /// Sets the max fee per blob gas for EIP-4844 transactions, pub fn with_blob_fee(mut self, val: u128) -> Self { self.set_blob_fee(val); @@ -588,12 +637,12 @@ impl PoolTransaction for MockTransaction { 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 + 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)) + return Some(fee.min(priority_fee)); } Some(fee) @@ -1000,6 +1049,7 @@ impl MockTransactionFactory { pub fn validated(&mut self, transaction: MockTransaction) -> MockValidTx { self.validated_with_origin(TransactionOrigin::External, transaction) } + pub fn validated_arc(&mut self, transaction: MockTransaction) -> Arc { Arc::new(self.validated(transaction)) } @@ -1083,6 +1133,43 @@ impl MockTransactionDistribution { } } +/// A set of [MockTransaction]s that can be modified at once +#[derive(Debug, Clone)] +pub struct MockTransactionSet { + pub(crate) transactions: Vec, +} + +impl MockTransactionSet { + /// Create a new [MockTransactionSet] from a list of transactions + fn new(transactions: Vec) -> Self { + Self { transactions } + } + + /// Create a list of dependent transactions with a common sender. The transactions start at the + /// given nonce, and the sender is incremented by the given tx_count. + pub fn dependent(sender: Address, from_nonce: u64, tx_count: usize, tx_type: TxType) -> Self { + let mut txs = Vec::with_capacity(tx_count); + let mut curr_tx = MockTransaction::new_from_type(tx_type).with_nonce(from_nonce); + for i in 0..tx_count { + let nonce = from_nonce + i as u64; + curr_tx = curr_tx.next().with_sender(sender); + txs.push(curr_tx.clone()); + } + + MockTransactionSet::new(txs) + } + + /// Add transactions to the [MockTransactionSet] + pub fn extend>(&mut self, txs: T) { + self.transactions.extend(txs); + } + + /// Extract the inner [Vec] of [MockTransaction]s + pub fn into_vec(self) -> Vec { + self.transactions + } +} + #[test] fn test_mock_priority() { let o = MockOrdering;