diff --git a/crates/transaction-pool/src/pool/parked.rs b/crates/transaction-pool/src/pool/parked.rs index 86f02ae0b8e..528fbd2aa31 100644 --- a/crates/transaction-pool/src/pool/parked.rs +++ b/crates/transaction-pool/src/pool/parked.rs @@ -295,18 +295,43 @@ impl ParkedPool> { transactions } - /// Removes all transactions and their dependent transaction from the subpool that no longer - /// satisfy the given basefee. + /// Removes all transactions from this subpool that can afford the given basefee, + /// invoking the provided handler for each transaction as it is removed. + /// + /// This method enforces the basefee constraint by identifying transactions that now + /// satisfy the basefee requirement (typically after a basefee decrease) and processing + /// them via the provided transaction handler closure. + /// + /// Respects per-sender nonce ordering: if the lowest-nonce transaction for a sender + /// still cannot afford the basefee, higher-nonce transactions from that sender are skipped. /// /// Note: the transactions are not returned in a particular order. - pub(crate) fn enforce_basefee(&mut self, basefee: u64) -> Vec>> { + pub(crate) fn enforce_basefee_with(&mut self, basefee: u64, mut tx_handler: F) + where + F: FnMut(Arc>), + { let to_remove = self.satisfy_base_fee_ids(basefee as u128); - let mut removed = Vec::with_capacity(to_remove.len()); for id in to_remove { - removed.push(self.remove_transaction(&id).expect("transaction exists")); + if let Some(tx) = self.remove_transaction(&id) { + tx_handler(tx); + } } + } + /// Removes all transactions and their dependent transaction from the subpool that no longer + /// satisfy the given basefee. + /// + /// Legacy method maintained for compatibility with read-only queries. + /// For basefee enforcement, prefer `enforce_basefee_with` for better performance. + /// + /// Note: the transactions are not returned in a particular order. + #[cfg(test)] + pub(crate) fn enforce_basefee(&mut self, basefee: u64) -> Vec>> { + let mut removed = Vec::new(); + self.enforce_basefee_with(basefee, |tx| { + removed.push(tx); + }); removed } } @@ -1039,4 +1064,68 @@ mod tests { assert!(removed.is_some()); assert!(!pool.contains(&tx_id)); } + + #[test] + fn test_enforce_basefee_with_handler_zero_allocation() { + let mut f = MockTransactionFactory::default(); + let mut pool = ParkedPool::>::default(); + + // Add multiple transactions across different fee ranges + let sender_a = address!("0x000000000000000000000000000000000000000a"); + let sender_b = address!("0x000000000000000000000000000000000000000b"); + + // Add transactions where nonce ordering allows proper processing: + // Sender A: both transactions can afford basefee (500 >= 400, 600 >= 400) + // Sender B: transaction cannot afford basefee (300 < 400) + let txs = vec![ + f.validated_arc( + MockTransaction::eip1559() + .set_sender(sender_a) + .set_nonce(0) + .set_max_fee(500) + .clone(), + ), + f.validated_arc( + MockTransaction::eip1559() + .set_sender(sender_a) + .set_nonce(1) + .set_max_fee(600) + .clone(), + ), + f.validated_arc( + MockTransaction::eip1559() + .set_sender(sender_b) + .set_nonce(0) + .set_max_fee(300) + .clone(), + ), + ]; + + let expected_affordable = vec![txs[0].clone(), txs[1].clone()]; // Both sender A txs + for tx in txs { + pool.add_transaction(tx); + } + + // Test the handler approach with zero allocations + let mut processed_txs = Vec::new(); + let mut handler_call_count = 0; + + pool.enforce_basefee_with(400, |tx| { + processed_txs.push(tx); + handler_call_count += 1; + }); + + // Verify correct number of transactions processed + assert_eq!(handler_call_count, 2); + assert_eq!(processed_txs.len(), 2); + + // Verify the correct transactions were processed (those with fee >= 400) + let processed_ids: Vec<_> = processed_txs.iter().map(|tx| *tx.id()).collect(); + for expected_tx in expected_affordable { + assert!(processed_ids.contains(expected_tx.id())); + } + + // Verify transactions were removed from pool + assert_eq!(pool.len(), 1); // Only the 300 fee tx should remain + } } diff --git a/crates/transaction-pool/src/pool/txpool.rs b/crates/transaction-pool/src/pool/txpool.rs index 6b69336e00f..4a0672e42a9 100644 --- a/crates/transaction-pool/src/pool/txpool.rs +++ b/crates/transaction-pool/src/pool/txpool.rs @@ -40,7 +40,7 @@ use std::{ ops::Bound::{Excluded, Unbounded}, sync::Arc, }; -use tracing::trace; +use tracing::{trace, warn}; #[cfg_attr(doc, aquamarine::aquamarine)] // TODO: Inlined diagram due to a bug in aquamarine library, should become an include when it's @@ -293,19 +293,40 @@ impl TxPool { Ordering::Greater } Ordering::Less => { - // decreased base fee: recheck basefee pool and promote all that are now valid - let removed = - self.basefee_pool.enforce_basefee(self.all_transactions.pending_fees.base_fee); - for tx in removed { - let to = { - let tx = + // Base fee decreased: recheck BaseFee and promote. + // Invariants: + // - BaseFee contains only non-blob txs (blob txs live in Blob) and they already + // have ENOUGH_BLOB_FEE_CAP_BLOCK. + // - PENDING_POOL_BITS = BASE_FEE_POOL_BITS | ENOUGH_FEE_CAP_BLOCK | + // ENOUGH_BLOB_FEE_CAP_BLOCK. + // With the lower base fee they gain ENOUGH_FEE_CAP_BLOCK, so we can set the bit and + // insert directly into Pending (skip generic routing). + self.basefee_pool.enforce_basefee_with( + self.all_transactions.pending_fees.base_fee, + |tx| { + // Update transaction state — guaranteed Pending by the invariants above + let meta = self.all_transactions.txs.get_mut(tx.id()).expect("tx exists in set"); - tx.state.insert(TxState::ENOUGH_FEE_CAP_BLOCK); - tx.subpool = tx.state.into(); - tx.subpool - }; - self.add_transaction_to_subpool(to, tx); - } + meta.state.insert(TxState::ENOUGH_FEE_CAP_BLOCK); + meta.subpool = meta.state.into(); + + trace!(target: "txpool", hash=%tx.transaction.hash(), pool=?meta.subpool, "Adding transaction to a subpool"); + match meta.subpool { + SubPool::Queued => self.queued_pool.add_transaction(tx), + SubPool::Pending => { + self.pending_pool.add_transaction(tx, self.all_transactions.pending_fees.base_fee); + } + SubPool::Blob => { + self.blob_pool.add_transaction(tx); + } + SubPool::BaseFee => { + // This should be unreachable as transactions from BaseFee pool with + // decreased basefee are guaranteed to become Pending + warn!( target: "txpool", "BaseFee transactions should become Pending after basefee decrease"); + } + } + }, + ); Ordering::Less } @@ -2954,6 +2975,97 @@ mod tests { assert_eq!(pool.all_transactions.txs.get(&id).unwrap().subpool, SubPool::BaseFee) } + #[test] + fn basefee_decrease_promotes_affordable_and_keeps_unaffordable() { + use alloy_primitives::address; + let mut f = MockTransactionFactory::default(); + let mut pool = TxPool::new(MockOrdering::default(), Default::default()); + + // Create transactions that will be in basefee pool (can't afford initial high fee) + // Use different senders to avoid nonce gap issues + let sender_a = address!("0x000000000000000000000000000000000000000a"); + let sender_b = address!("0x000000000000000000000000000000000000000b"); + let sender_c = address!("0x000000000000000000000000000000000000000c"); + + let tx1 = MockTransaction::eip1559() + .set_sender(sender_a) + .set_nonce(0) + .set_max_fee(500) + .inc_limit(); + let tx2 = MockTransaction::eip1559() + .set_sender(sender_b) + .set_nonce(0) + .set_max_fee(600) + .inc_limit(); + let tx3 = MockTransaction::eip1559() + .set_sender(sender_c) + .set_nonce(0) + .set_max_fee(400) + .inc_limit(); + + // Set high initial basefee so transactions go to basefee pool + let mut block_info = pool.block_info(); + block_info.pending_basefee = 700; + pool.set_block_info(block_info); + + let validated1 = f.validated(tx1); + let validated2 = f.validated(tx2); + let validated3 = f.validated(tx3); + let id1 = *validated1.id(); + let id2 = *validated2.id(); + let id3 = *validated3.id(); + + // Add transactions - they should go to basefee pool due to high basefee + // All transactions have nonce 0 from different senders, so on_chain_nonce should be 0 for + // all + pool.add_transaction(validated1, U256::from(10_000), 0, None).unwrap(); + pool.add_transaction(validated2, U256::from(10_000), 0, None).unwrap(); + pool.add_transaction(validated3, U256::from(10_000), 0, None).unwrap(); + + // Debug: Check where transactions ended up + println!("Basefee pool len: {}", pool.basefee_pool.len()); + println!("Pending pool len: {}", pool.pending_pool.len()); + println!("tx1 subpool: {:?}", pool.all_transactions.txs.get(&id1).unwrap().subpool); + println!("tx2 subpool: {:?}", pool.all_transactions.txs.get(&id2).unwrap().subpool); + println!("tx3 subpool: {:?}", pool.all_transactions.txs.get(&id3).unwrap().subpool); + + // Verify they're in basefee pool + assert_eq!(pool.basefee_pool.len(), 3); + assert_eq!(pool.pending_pool.len(), 0); + assert_eq!(pool.all_transactions.txs.get(&id1).unwrap().subpool, SubPool::BaseFee); + assert_eq!(pool.all_transactions.txs.get(&id2).unwrap().subpool, SubPool::BaseFee); + assert_eq!(pool.all_transactions.txs.get(&id3).unwrap().subpool, SubPool::BaseFee); + + // Now decrease basefee to trigger the zero-allocation optimization + let mut block_info = pool.block_info(); + block_info.pending_basefee = 450; // tx1 (500) and tx2 (600) can now afford it, tx3 (400) cannot + pool.set_block_info(block_info); + + // Verify the optimization worked correctly: + // - tx1 and tx2 should be promoted to pending (mathematical certainty) + // - tx3 should remain in basefee pool + // - All state transitions should be correct + assert_eq!(pool.basefee_pool.len(), 1); + assert_eq!(pool.pending_pool.len(), 2); + + // tx3 should still be in basefee pool (fee 400 < basefee 450) + assert_eq!(pool.all_transactions.txs.get(&id3).unwrap().subpool, SubPool::BaseFee); + + // tx1 and tx2 should be in pending pool with correct state bits + let tx1_meta = pool.all_transactions.txs.get(&id1).unwrap(); + let tx2_meta = pool.all_transactions.txs.get(&id2).unwrap(); + assert_eq!(tx1_meta.subpool, SubPool::Pending); + assert_eq!(tx2_meta.subpool, SubPool::Pending); + assert!(tx1_meta.state.contains(TxState::ENOUGH_FEE_CAP_BLOCK)); + assert!(tx2_meta.state.contains(TxState::ENOUGH_FEE_CAP_BLOCK)); + + // Verify that best_transactions returns the promoted transactions + let best: Vec<_> = pool.best_transactions().take(3).collect(); + assert_eq!(best.len(), 2); // Only tx1 and tx2 should be returned + assert!(best.iter().any(|tx| tx.id() == &id1)); + assert!(best.iter().any(|tx| tx.id() == &id2)); + } + #[test] fn get_highest_transaction_by_sender_and_nonce() { // Set up a mock transaction factory and a new transaction pool.