Skip to content

revert: "perf(txpool): eliminate allocations in basefee enforcement" #18215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattsse merged 1 commit into from
Sep 2, 2025
Merged

revert: "perf(txpool): eliminate allocations in basefee enforcement" #18215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 5 additions & 94 deletions crates/transaction-pool/src/pool/parked.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,43 +295,18 @@ impl<T: PoolTransaction> ParkedPool<BasefeeOrd<T>> {
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<F>(&mut self, basefee: u64, mut tx_handler: F)
where
F: FnMut(Arc<ValidPoolTransaction<T>>),
{
pub(crate) fn enforce_basefee(&mut self, basefee: u64) -> Vec<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 {
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<Arc<ValidPoolTransaction<T>>> {
let mut removed = Vec::new();
self.enforce_basefee_with(basefee, |tx| {
removed.push(tx);
});
removed
}
}
Expand Down Expand Up @@ -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::<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
}
}
136 changes: 12 additions & 124 deletions crates/transaction-pool/src/pool/txpool.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,40 +293,19 @@ impl<T: TransactionOrdering> TxPool<T> {
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
}
Expand Down Expand Up @@ -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.
Expand Down
Loading