diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 98ebe601fd..78f6b4c080 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -2484,29 +2484,56 @@ impl Blockchain { } }; + // Compute the new tx's cost once and reuse for both the single-tx + // balance check and the cumulative-balance check below. + let tx_cost = tx + .cost_without_base_fee() + .ok_or(MempoolError::InvalidTxGasvalues)?; + let maybe_sender_acc_info = self.storage.get_account_info(header_no, sender).await?; - if let Some(sender_acc_info) = maybe_sender_acc_info { + let sender_balance = if let Some(sender_acc_info) = maybe_sender_acc_info { if nonce < sender_acc_info.nonce || nonce == u64::MAX { return Err(MempoolError::NonceTooLow); } - let tx_cost = tx - .cost_without_base_fee() - .ok_or(MempoolError::InvalidTxGasvalues)?; - if tx_cost > sender_acc_info.balance { return Err(MempoolError::NotEnoughBalance); } + + sender_acc_info.balance } else { // An account that is not in the database cannot possibly have enough balance to cover the transaction cost return Err(MempoolError::NotEnoughBalance); - } + }; // Check the nonce of pendings TXs in the mempool from the same sender // If it exists check if the new tx has higher fees let tx_to_replace_hash = self.mempool.find_tx_to_replace(sender, nonce, tx)?; + // Cumulative balance check across this sender's pending transactions. + // Without this, a sender at the per-sender slot cap can have only one + // of their N pending txs be fundable, with the other N-1 being + // guaranteed-fail spam wasting pool space. + // + // `sum_cost_for_sender` recomputes the sender total excluding the + // tx being replaced (instead of subtracting after the fact) so a + // `None`-cost or missing tx can't silently zero the total via + // `MAX - MAX = 0`. It also fails closed on any inconsistency so the + // gate can't be bypassed by an invariant violation. + let existing_cost = self + .mempool + .sum_cost_for_sender(sender, tx_to_replace_hash)?; + let total = existing_cost + .checked_add(tx_cost) + .ok_or(MempoolError::InvalidTxGasvalues)?; + if total > sender_balance { + return Err(MempoolError::InsufficientCumulativeBalance { + required: total, + available: sender_balance, + }); + } + if tx .chain_id() .is_some_and(|chain_id| chain_id != config.chain_id) diff --git a/crates/blockchain/error.rs b/crates/blockchain/error.rs index 39436472dd..00c9aef8fd 100644 --- a/crates/blockchain/error.rs +++ b/crates/blockchain/error.rs @@ -1,5 +1,5 @@ use ethrex_common::{ - H256, + H256, U256, types::{BlobsBundleError, BlockHash}, }; use ethrex_rlp::error::RLPDecodeError; @@ -107,6 +107,8 @@ pub enum MempoolError { InvalidChainId(u64), #[error("Account does not have enough balance to cover the tx cost")] NotEnoughBalance, + #[error("Sender's cumulative pending-tx cost ({required}) exceeds balance ({available})")] + InsufficientCumulativeBalance { required: U256, available: U256 }, #[error("Transaction gas fields are invalid")] InvalidTxGasvalues, #[error("Invalid pooled TxType, expected: {0}")] diff --git a/crates/blockchain/mempool.rs b/crates/blockchain/mempool.rs index bdc31f8aaa..7847a7fb49 100644 --- a/crates/blockchain/mempool.rs +++ b/crates/blockchain/mempool.rs @@ -364,6 +364,50 @@ impl Mempool { .map(|((_address, nonce), _hash)| nonce + 1)) } + /// Returns the sum of `cost_without_base_fee()` for every pending + /// transaction from `sender` currently in the pool, optionally excluding + /// `exclude` (used by the cumulative-balance admission gate to drop the + /// cost of a tx that's about to be replaced at the same nonce). + /// + /// Used at mempool admission to gate a sender's cumulative pending cost + /// against their on-chain balance: without this check, a sender at the + /// per-sender slot cap can have most of their pending txs be + /// guaranteed-fail at execution time and waste pool space. + /// + /// Fails closed on any inconsistency: if `txs_by_sender_nonce` references + /// a hash missing from `transaction_pool`, or if any included tx's cost + /// can't be computed, the function returns an error rather than silently + /// undercounting (which would let a malformed or invariant-violating tx + /// bypass the cumulative check). + pub fn sum_cost_for_sender( + &self, + sender: Address, + exclude: Option, + ) -> Result { + let inner = self.read()?; + let mut total = U256::zero(); + for (_key, hash) in inner + .txs_by_sender_nonce + .range((sender, 0)..=(sender, u64::MAX)) + { + if Some(*hash) == exclude { + continue; + } + let tx = inner.transaction_pool.get(hash).ok_or_else(|| { + MempoolError::StoreError(StoreError::Custom(format!( + "mempool index/pool inconsistency: hash {hash:?} in sender-nonce index but missing from transaction_pool", + ))) + })?; + let cost = tx + .cost_without_base_fee() + .ok_or(MempoolError::InvalidTxGasvalues)?; + total = total + .checked_add(cost) + .ok_or(MempoolError::InvalidTxGasvalues)?; + } + Ok(total) + } + pub fn get_mempool_size(&self) -> Result<(u64, u64), MempoolError> { let txs_size = { let pool_lock = &self.read()?.transaction_pool; @@ -574,3 +618,97 @@ pub fn transaction_intrinsic_gas( Ok(gas) } + +#[cfg(test)] +mod tests { + use super::*; + use ethrex_common::types::{EIP1559Transaction, TxKind}; + + const MEMPOOL_MAX_SIZE_TEST: usize = 10_000; + + fn build_tx(nonce: u64, max_fee_per_gas: u64, gas_limit: u64, value: U256) -> Transaction { + Transaction::EIP1559Transaction(EIP1559Transaction { + nonce, + max_priority_fee_per_gas: 0, + max_fee_per_gas, + gas_limit, + to: TxKind::Call(Address::from_low_u64_be(1)), + value, + ..Default::default() + }) + } + + fn insert_tx(mempool: &Mempool, sender: Address, tx: Transaction) -> H256 { + let hash = H256::random(); + mempool + .add_transaction(hash, sender, MempoolTransaction::new(tx, sender)) + .expect("add_transaction"); + hash + } + + #[test] + fn sum_cost_for_sender_empty_pool_is_zero() { + let mempool = Mempool::new(MEMPOOL_MAX_SIZE_TEST); + let sender = Address::random(); + + let total = mempool.sum_cost_for_sender(sender, None).expect("sum"); + assert_eq!(total, U256::zero()); + } + + #[test] + fn sum_cost_for_sender_sums_multiple_nonces() { + let mempool = Mempool::new(MEMPOOL_MAX_SIZE_TEST); + let sender = Address::random(); + + let tx0 = build_tx(0, 10, 21_000, U256::from(100u64)); + let tx1 = build_tx(1, 20, 21_000, U256::from(200u64)); + let tx2 = build_tx(2, 30, 21_000, U256::from(300u64)); + + let expected = tx0.cost_without_base_fee().unwrap() + + tx1.cost_without_base_fee().unwrap() + + tx2.cost_without_base_fee().unwrap(); + + insert_tx(&mempool, sender, tx0); + insert_tx(&mempool, sender, tx1); + insert_tx(&mempool, sender, tx2); + + let total = mempool.sum_cost_for_sender(sender, None).expect("sum"); + assert_eq!(total, expected); + } + + #[test] + fn sum_cost_for_sender_ignores_other_senders() { + let mempool = Mempool::new(MEMPOOL_MAX_SIZE_TEST); + let sender_a = Address::random(); + let sender_b = Address::random(); + + let tx_a = build_tx(0, 10, 21_000, U256::from(100u64)); + let tx_b = build_tx(0, 50, 21_000, U256::from(999u64)); + + let expected_a = tx_a.cost_without_base_fee().unwrap(); + + insert_tx(&mempool, sender_a, tx_a); + insert_tx(&mempool, sender_b, tx_b); + + let total_a = mempool.sum_cost_for_sender(sender_a, None).expect("sum a"); + assert_eq!(total_a, expected_a); + } + + #[test] + fn sum_cost_for_sender_after_remove_drops_that_tx() { + let mempool = Mempool::new(MEMPOOL_MAX_SIZE_TEST); + let sender = Address::random(); + + let tx0 = build_tx(0, 10, 21_000, U256::from(100u64)); + let tx1 = build_tx(1, 10, 21_000, U256::from(200u64)); + let expected_after = tx1.cost_without_base_fee().unwrap(); + + let hash0 = insert_tx(&mempool, sender, tx0); + insert_tx(&mempool, sender, tx1); + + mempool.remove_transaction(&hash0).expect("remove"); + + let total = mempool.sum_cost_for_sender(sender, None).expect("sum"); + assert_eq!(total, expected_after); + } +}