perf(txpool): eliminate allocations in basefee enforcement (#18218)

Co-authored-by: Matthias Seitz <[email protected]>

Author: yongkangc

crates/transaction-pool/src/pool/parked.rs

@@ -295,18 +295,43 @@ impl<T: PoolTransaction> ParkedPool<BasefeeOrd<T>> {
         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<Arc<ValidPoolTransaction<T>>> {
+    pub(crate) fn enforce_basefee_with<F>(&mut self, basefee: u64, mut tx_handler: F)
+    where
+        F: FnMut(Arc<ValidPoolTransaction<T>>),
+    {
         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<Arc<ValidPoolTransaction<T>>> {
+        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::<BasefeeOrd<_>>::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
+    }
 }

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<T: TransactionOrdering> TxPool<T> {
                 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.