From 83a9b31911756279d11aca6aec8df6d0115f65e6 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Mon, 17 Oct 2022 16:13:40 +0200 Subject: [PATCH] feat(txpool): add no parked ancestors condition (#84) * feat(txpool): add no parked ancestors condition * chore: rustfmt --- crates/transaction-pool/src/pool/state.rs | 53 +++++++++++++++-- crates/transaction-pool/src/pool/txpool.rs | 69 +++++++++++++++++++++- 2 files changed, 113 insertions(+), 9 deletions(-) diff --git a/crates/transaction-pool/src/pool/state.rs b/crates/transaction-pool/src/pool/state.rs index 5b965d72e..2d7d74241 100644 --- a/crates/transaction-pool/src/pool/state.rs +++ b/crates/transaction-pool/src/pool/state.rs @@ -4,6 +4,8 @@ bitflags::bitflags! { /// This mirrors [erigon's ephemeral state field](https://github.com/ledgerwatch/erigon/wiki/Transaction-Pool-Design#ordering-function). #[derive(Default)] pub(crate) struct TxState: u8 { + /// Set to `1` if all ancestor transactions are pending. + const NO_PARKED_ANCESTORS = 0b100000; /// Set to `1` of the transaction is either the next transaction of the sender (on chain nonce == tx.nonce) or all prior transactions are also present in the pool. const NO_NONCE_GAPS = 0b010000; /// Bit derived from the sender's balance. @@ -19,9 +21,25 @@ bitflags::bitflags! { const ENOUGH_FEE_CAP_BLOCK = 0b000010; const IS_LOCAL = 0b000001; - const BASE_FEE_POOL_BITS = Self::NO_NONCE_GAPS.bits | Self::ENOUGH_BALANCE.bits | Self::NOT_TOO_MUCH_GAS.bits; + const PENDING_POOL_BITS = Self::NO_PARKED_ANCESTORS.bits | Self::NO_NONCE_GAPS.bits | Self::ENOUGH_BALANCE.bits | Self::NOT_TOO_MUCH_GAS.bits | Self::ENOUGH_FEE_CAP_BLOCK.bits; - const QUEUED_POOL_BITS = 0b100000; + const BASE_FEE_POOL_BITS = Self::NO_PARKED_ANCESTORS.bits | Self::NO_NONCE_GAPS.bits | Self::ENOUGH_BALANCE.bits | Self::NOT_TOO_MUCH_GAS.bits; + + const QUEUED_POOL_BITS = Self::NO_PARKED_ANCESTORS.bits; + + } +} + +// === impl TxState === + +impl TxState { + /// The state of a transaction is considered `pending`, if the transaction has: + /// - _No_ parked ancestors + /// - enough balance + /// - enough fee cap + #[inline] + pub(crate) fn is_pending(&self) -> bool { + *self >= TxState::PENDING_POOL_BITS } } @@ -50,10 +68,10 @@ impl SubPool { impl From for SubPool { fn from(value: TxState) -> Self { - if value > TxState::BASE_FEE_POOL_BITS { + if value.is_pending() { return SubPool::Pending } - if value < TxState::QUEUED_POOL_BITS { + if value < TxState::BASE_FEE_POOL_BITS { return SubPool::Queued } SubPool::BaseFee @@ -72,8 +90,31 @@ mod tests { } #[test] - fn test_tx_queud() { - let mut state = TxState::default(); + fn test_tx_queued() { + let state = TxState::default(); + assert_eq!(SubPool::Queued, state.into()); + + let state = TxState::NO_PARKED_ANCESTORS | + TxState::NO_NONCE_GAPS | + TxState::NOT_TOO_MUCH_GAS | + TxState::ENOUGH_FEE_CAP_BLOCK; assert_eq!(SubPool::Queued, state.into()); } + + #[test] + fn test_tx_pending() { + let state = TxState::PENDING_POOL_BITS; + assert_eq!(SubPool::Pending, state.into()); + assert!(state.is_pending()); + + let bits = 0b111110; + let state = TxState::from_bits(bits).unwrap(); + assert_eq!(SubPool::Pending, state.into()); + assert!(state.is_pending()); + + let bits = 0b111111; + let state = TxState::from_bits(bits).unwrap(); + assert_eq!(SubPool::Pending, state.into()); + assert!(state.is_pending()); + } } diff --git a/crates/transaction-pool/src/pool/txpool.rs b/crates/transaction-pool/src/pool/txpool.rs index d091acfd4..a03994430 100644 --- a/crates/transaction-pool/src/pool/txpool.rs +++ b/crates/transaction-pool/src/pool/txpool.rs @@ -474,9 +474,10 @@ impl AllTransactions { let predecessor = TransactionId::ancestor(transaction.transaction.nonce(), on_chain_nonce, tx_id.sender); - // If there's no predecessor then this is the next transaction + // If there's no predecessor then this is the next transaction. if predecessor.is_none() { state.insert(TxState::NO_NONCE_GAPS); + state.insert(TxState::NO_PARKED_ANCESTORS); } // Check dynamic fee @@ -535,14 +536,33 @@ impl AllTransactions { // The next transaction of this sender let on_chain_id = TransactionId::new(transaction.sender_id(), on_chain_nonce); { + let mut descendants = self.descendant_txs_mut(&on_chain_id).peekable(); + // Tracks the next nonce we expect if the transactions are gapless let mut next_nonce = on_chain_id.nonce; + // We need to find out if the next transaction of the sender is considered pending + // + let mut has_parked_ancestor = if predecessor.is_none() { + // the new transaction is the next one + false + } else { + // SAFETY: the transaction was added above so the _inclusive_ descendants iterator + // returns at least 1 tx. + let (id, tx) = descendants.peek().expect("Includes >= 1; qed."); + if id.nonce < tx_id.nonce { + !tx.state.is_pending() + } else { + true + } + }; + // Traverse all transactions of the sender and update existing transactions - for (id, tx) in self.descendant_txs_mut(&on_chain_id) { + for (id, tx) in descendants { let current_pool = tx.subpool; + + // If there's a nonce gap, we can shortcircuit if next_nonce != id.nonce { - // nothing to update break } @@ -562,6 +582,14 @@ impl AllTransactions { tx.state.insert(TxState::ENOUGH_BALANCE); } + // Update ancestor condition. + if has_parked_ancestor { + tx.state.remove(TxState::NO_PARKED_ANCESTORS); + } else { + tx.state.insert(TxState::NO_PARKED_ANCESTORS); + } + has_parked_ancestor = !tx.state.is_pending(); + if tx_id.eq(id) { // if it is the new transaction, track the state state = tx.state; @@ -846,6 +874,7 @@ mod tests { let _res = pool.insert_tx(first.clone(), on_chain_balance, on_chain_nonce); let first_in_pool = pool.get(first.id()).unwrap(); + // has nonce gap assert!(!first_in_pool.state.contains(TxState::NO_NONCE_GAPS)); @@ -895,4 +924,38 @@ mod tests { assert!(first_in_pool.state.contains(TxState::NO_NONCE_GAPS)); assert_eq!(SubPool::Pending, first_in_pool.subpool); } + + #[test] + fn insert_previous_blocking() { + let on_chain_balance = U256::from(1_000); + let on_chain_nonce = 0; + let mut f = MockTransactionFactory::default(); + let mut pool = AllTransactions::default(); + pool.pending_basefee = pool.minimal_protocol_basefee + 1; + let tx = MockTransaction::eip1559().inc_nonce().inc_limit(); + let first = f.validated(tx.clone()); + + let _res = pool.insert_tx(first.clone(), on_chain_balance, on_chain_nonce); + + let first_in_pool = pool.get(first.id()).unwrap(); + + assert!(tx.get_gas_price() < pool.pending_basefee); + // has nonce gap + assert!(!first_in_pool.state.contains(TxState::NO_NONCE_GAPS)); + + let prev = f.validated(tx.prev()); + let InsertOk { updates, replaced_tx, state, move_to, .. } = + pool.insert_tx(prev, on_chain_balance, on_chain_nonce).unwrap(); + + assert!(!state.contains(TxState::ENOUGH_FEE_CAP_BLOCK)); + // no updates since still in queued pool + assert!(updates.is_empty()); + assert!(replaced_tx.is_none()); + assert!(state.contains(TxState::NO_NONCE_GAPS)); + assert_eq!(move_to, SubPool::BaseFee); + + let first_in_pool = pool.get(first.id()).unwrap(); + // has non nonce gap + assert!(first_in_pool.state.contains(TxState::NO_NONCE_GAPS)); + } }