diff --git a/crates/transaction-pool/src/pool/parked.rs b/crates/transaction-pool/src/pool/parked.rs index 528fbd2aa31..86f02ae0b8e 100644 --- a/crates/transaction-pool/src/pool/parked.rs +++ b/crates/transaction-pool/src/pool/parked.rs @@ -295,43 +295,18 @@ impl ParkedPool> { transactions } - /// 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. + /// Removes all transactions and their dependent transaction from the subpool that no longer + /// satisfy the given basefee. /// /// Note: the transactions are not returned in a particular order. - pub(crate) fn enforce_basefee_with(&mut self, basefee: u64, mut tx_handler: F) - where - F: FnMut(Arc>), - { + pub(crate) fn enforce_basefee(&mut self, basefee: u64) -> Vec>> { 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 { - if let Some(tx) = self.remove_transaction(&id) { - tx_handler(tx); - } + removed.push(self.remove_transaction(&id).expect("transaction exists")); } - } - /// 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 } } @@ -1064,68 +1039,4 @@ 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 dc2a5e20e76..6b69336e00f 100644 --- a/crates/transaction-pool/src/pool/txpool.rs +++ b/crates/transaction-pool/src/pool/txpool.rs @@ -293,40 +293,19 @@ impl TxPool { Ordering::Greater } Ordering::Less => { - // 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 = + // 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 = self.all_transactions.txs.get_mut(tx.id()).expect("tx exists in set"); - 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 - unreachable!("BaseFee transactions should become Pending after basefee decrease"); - } - } - }, - ); + tx.state.insert(TxState::ENOUGH_FEE_CAP_BLOCK); + tx.subpool = tx.state.into(); + tx.subpool + }; + self.add_transaction_to_subpool(to, tx); + } Ordering::Less } @@ -2975,97 +2954,6 @@ 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.