diff --git a/cmd/ethrex/cli.rs b/cmd/ethrex/cli.rs index 74a66aab1e..cfa3bd87b9 100644 --- a/cmd/ethrex/cli.rs +++ b/cmd/ethrex/cli.rs @@ -10,7 +10,8 @@ use std::{ use clap::{ArgAction, Parser as ClapParser, Subcommand as ClapSubcommand}; use ethrex_blockchain::{ - BlockchainOptions, BlockchainType, L2Config, + BlockchainOptions, BlockchainType, DEFAULT_BLOB_PRICE_BUMP_PERCENT, DEFAULT_PRICE_BUMP_PERCENT, + L2Config, error::{ChainError, InvalidBlockError}, }; use ethrex_common::types::{Block, DEFAULT_BUILDER_GAS_CEIL, Genesis, validate_block_body}; @@ -183,6 +184,24 @@ pub struct Options { env = "ETHREX_MEMPOOL_MAX_SIZE" )] pub mempool_max_size: usize, + #[arg( + help = "Minimum fee bump (in percent) required to replace a non-blob pooled transaction at the same (sender, nonce).", + long = "mempool.price-bump", + default_value_t = DEFAULT_PRICE_BUMP_PERCENT, + value_name = "PERCENT", + help_heading = "Node options", + env = "ETHREX_MEMPOOL_PRICE_BUMP" + )] + pub mempool_price_bump: u64, + #[arg( + help = "Minimum fee bump (in percent) required to replace an EIP-4844 blob pooled transaction.", + long = "mempool.blob-price-bump", + default_value_t = DEFAULT_BLOB_PRICE_BUMP_PERCENT, + value_name = "PERCENT", + help_heading = "Node options", + env = "ETHREX_MEMPOOL_BLOB_PRICE_BUMP" + )] + pub mempool_blob_price_bump: u64, #[arg( long = "http.addr", default_value = "127.0.0.1", @@ -464,6 +483,8 @@ impl Default for Options { dev: Default::default(), force: false, mempool_max_size: Default::default(), + mempool_price_bump: DEFAULT_PRICE_BUMP_PERCENT, + mempool_blob_price_bump: DEFAULT_BLOB_PRICE_BUMP_PERCENT, tx_broadcasting_time_interval: Default::default(), target_peers: Default::default(), lookup_interval: Default::default(), diff --git a/cmd/ethrex/initializers.rs b/cmd/ethrex/initializers.rs index 4f76b6eb5c..1ce2d84041 100644 --- a/cmd/ethrex/initializers.rs +++ b/cmd/ethrex/initializers.rs @@ -531,6 +531,8 @@ pub async fn init_l1( max_blobs_per_block: opts.max_blobs_per_block, precompute_witnesses: opts.precompute_witnesses, precompile_cache_enabled: !opts.no_precompile_cache, + price_bump_percent: opts.mempool_price_bump, + blob_price_bump_percent: opts.mempool_blob_price_bump, }, ); diff --git a/cmd/ethrex/l2/initializers.rs b/cmd/ethrex/l2/initializers.rs index 668c1d9d55..4c917ceb02 100644 --- a/cmd/ethrex/l2/initializers.rs +++ b/cmd/ethrex/l2/initializers.rs @@ -228,6 +228,8 @@ pub async fn init_l2( max_blobs_per_block: None, // L2 doesn't support blob transactions precompute_witnesses: opts.node_opts.precompute_witnesses, precompile_cache_enabled: true, + price_bump_percent: opts.node_opts.mempool_price_bump, + blob_price_bump_percent: opts.node_opts.mempool_blob_price_bump, }; let blockchain = init_blockchain(store.clone(), blockchain_opts.clone()); diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 8f3f301d8f..e2bb892ab5 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -229,6 +229,15 @@ pub struct BlockchainOptions { /// warmer thread and the executor. Set to false (via `--no-precompile-cache`) to /// disable the cache for benchmarking purposes. pub precompile_cache_enabled: bool, + /// Minimum fee-field bump (in percent) required to replace a non-blob + /// transaction at the same `(sender, nonce)`. Matches the 10% + /// default of every peer EL client. + pub price_bump_percent: u64, + /// Minimum fee-field bump (in percent) required to replace an EIP-4844 + /// blob transaction at the same `(sender, nonce)`. Matches the 100% + /// default of every peer EL client. Blob replacements are deliberately + /// expensive because blob sidecars are large to re-propagate. + pub blob_price_bump_percent: u64, } impl Default for BlockchainOptions { @@ -240,10 +249,21 @@ impl Default for BlockchainOptions { max_blobs_per_block: None, precompute_witnesses: false, precompile_cache_enabled: true, + price_bump_percent: DEFAULT_PRICE_BUMP_PERCENT, + blob_price_bump_percent: DEFAULT_BLOB_PRICE_BUMP_PERCENT, } } } +/// Default 10% bump required for non-blob RBF replacements (matches geth +/// `PriceBump`, reth `default_price_bump`, nethermind `PriceBump`, +/// erigon `PriceBump`, besu `DEFAULT_PRICE_BUMP`). +pub const DEFAULT_PRICE_BUMP_PERCENT: u64 = 10; +/// Default 100% bump required for blob RBF replacements (matches geth +/// `blobpool.PriceBump`, reth `replace_blob_tx_price_bump`, nethermind +/// blob comparison, erigon `BlobPriceBump`, besu `DEFAULT_BLOB_PRICE_BUMP`). +pub const DEFAULT_BLOB_PRICE_BUMP_PERCENT: u64 = 100; + #[derive(Debug, Clone)] pub struct BatchBlockProcessingFailure { pub last_valid_hash: H256, @@ -2539,7 +2559,13 @@ impl Blockchain { // 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)?; + let tx_to_replace_hash = self.mempool.find_tx_to_replace( + sender, + nonce, + tx, + self.options.price_bump_percent, + self.options.blob_price_bump_percent, + )?; if tx .chain_id() diff --git a/crates/blockchain/error.rs b/crates/blockchain/error.rs index 3bb0ea553f..73d7d7f438 100644 --- a/crates/blockchain/error.rs +++ b/crates/blockchain/error.rs @@ -119,6 +119,10 @@ pub enum MempoolError { InvalidTxSender(#[from] ethrex_crypto::CryptoError), #[error("Attempted to replace a pooled transaction with an underpriced transaction")] UnderpricedReplacement, + #[error( + "Attempted to replace a pooled transaction with one of a different type (e.g. blob vs. non-blob)" + )] + ReplacementTypeMismatch, } #[derive(Debug)] diff --git a/crates/blockchain/mempool.rs b/crates/blockchain/mempool.rs index 3db0f80a6f..e724276240 100644 --- a/crates/blockchain/mempool.rs +++ b/crates/blockchain/mempool.rs @@ -457,38 +457,75 @@ impl Mempool { sender: Address, nonce: u64, tx: &Transaction, + price_bump_percent: u64, + blob_price_bump_percent: u64, ) -> Result, MempoolError> { let Some(tx_in_pool) = self.contains_sender_nonce(sender, nonce, tx.hash())? else { return Ok(None); }; - let is_a_replacement_tx = { - // EIP-1559 values - let old_tx_max_fee_per_gas = tx_in_pool.max_fee_per_gas().unwrap_or_default(); - let old_tx_max_priority_fee_per_gas = tx_in_pool.max_priority_fee().unwrap_or_default(); - let new_tx_max_fee_per_gas = tx.max_fee_per_gas().unwrap_or_default(); - let new_tx_max_priority_fee_per_gas = tx.max_priority_fee().unwrap_or_default(); - - // Legacy tx values - let old_tx_gas_price = tx_in_pool.gas_price(); - let new_tx_gas_price = tx.gas_price(); - - // EIP-4844 values - let old_tx_max_fee_per_blob = tx_in_pool.max_fee_per_blob_gas(); - let new_tx_max_fee_per_blob = tx.max_fee_per_blob_gas(); - - let eip4844_higher_fees = if let (Some(old_blob_fee), Some(new_blob_fee)) = - (old_tx_max_fee_per_blob, new_tx_max_fee_per_blob) - { - new_blob_fee > old_blob_fee - } else { - true // We are marking it as always true if the tx is not eip-4844 - }; - let eip1559_higher_fees = new_tx_max_fee_per_gas > old_tx_max_fee_per_gas - && new_tx_max_priority_fee_per_gas > old_tx_max_priority_fee_per_gas; - let legacy_higher_fees = new_tx_gas_price > old_tx_gas_price; + // Reject type-change replacements. Peer clients keep blob and + // non-blob transactions in separate sub-pools precisely so a + // cheap-to-replicate non-blob tx can't displace a blob tx (with + // its expensive sidecar) or vice versa. ethrex has a single pool, + // so the same guarantee has to be enforced here. + if std::mem::discriminant(tx) != std::mem::discriminant(tx_in_pool.transaction()) { + return Err(MempoolError::ReplacementTypeMismatch); + } - eip4844_higher_fees && (eip1559_higher_fees || legacy_higher_fees) + // Blob replacements use a stricter bump (default 100%) because blob + // sidecars are expensive to re-propagate; all other tx types use the + // base bump (default 10%). + let bump = if matches!(tx, Transaction::EIP4844Transaction(_)) { + blob_price_bump_percent + } else { + price_bump_percent + }; + + // The new tx must bump every applicable fee field on its own tx type + // by at least `bump` percent compared to the in-pool tx at the same + // (sender, nonce). Each peer EL client enforces this independently per + // field. + let is_a_replacement_tx = match tx { + Transaction::LegacyTransaction(_) => { + is_bumped_u256(tx_in_pool.gas_price(), tx.gas_price(), bump) + } + Transaction::EIP4844Transaction(_) => { + let bumped_fee = is_bumped_u64( + tx_in_pool.max_fee_per_gas().unwrap_or_default(), + tx.max_fee_per_gas().unwrap_or_default(), + bump, + ); + let bumped_tip = is_bumped_u64( + tx_in_pool.max_priority_fee().unwrap_or_default(), + tx.max_priority_fee().unwrap_or_default(), + bump, + ); + let bumped_blob = is_bumped_u256( + tx_in_pool.max_fee_per_blob_gas().unwrap_or_default(), + tx.max_fee_per_blob_gas().unwrap_or_default(), + bump, + ); + bumped_fee && bumped_tip && bumped_blob + } + // EIP-2930 / EIP-1559 / EIP-7702 / FeeToken / Privileged: 1559-style + // pair of fee fields. (PrivilegedL2 transactions short-circuit + // before `validate_transaction` ever reaches `find_tx_to_replace`, + // so the Privileged variant of this arm is unreachable in practice; + // the other variants do hit it.) + _ => { + let bumped_fee = is_bumped_u64( + tx_in_pool.max_fee_per_gas().unwrap_or_default(), + tx.max_fee_per_gas().unwrap_or_default(), + bump, + ); + let bumped_tip = is_bumped_u64( + tx_in_pool.max_priority_fee().unwrap_or_default(), + tx.max_priority_fee().unwrap_or_default(), + bump, + ); + bumped_fee && bumped_tip + } }; if !is_a_replacement_tx { @@ -499,6 +536,33 @@ impl Mempool { } } +/// Returns true iff `new >= floor(existing * (100 + bump_percent) / 100)`. +/// Uses `u128` intermediates with checked arithmetic so an overflow on the +/// threshold computation is treated as "reject" rather than silently +/// admitting an under-priced replacement. A `bump_percent` of 0 collapses to +/// `new >= existing`. +fn is_bumped_u64(existing: u64, new: u64, bump_percent: u64) -> bool { + let multiplier = 100u128 + bump_percent as u128; + let Some(threshold) = (existing as u128).checked_mul(multiplier).map(|v| v / 100) else { + return false; + }; + (new as u128) >= threshold +} + +/// U256 variant of [`is_bumped_u64`]. Used for `gas_price` (legacy) and +/// `max_fee_per_blob_gas` (EIP-4844). Same overflow → reject semantic via +/// `checked_mul`. +fn is_bumped_u256(existing: U256, new: U256, bump_percent: u64) -> bool { + let multiplier = U256::from(100u64 + bump_percent); + let Some(threshold) = existing + .checked_mul(multiplier) + .map(|v| v / U256::from(100u64)) + else { + return false; + }; + new >= threshold +} + /// Filter applied by the payload builder when querying pending transactions /// from the pool. NOT a mempool admission gate — all fields here are /// query-time filters used to pick block-includable transactions. Admission @@ -599,3 +663,248 @@ pub fn transaction_intrinsic_gas( Ok(gas) } + +#[cfg(test)] +mod tests { + use super::*; + use ethrex_common::types::{EIP1559Transaction, EIP4844Transaction}; + + // --- helpers -------------------------------------------------------- + + fn add_to_pool(pool: &Mempool, sender: Address, tx: Transaction) -> H256 { + let mtx = MempoolTransaction::new(tx, sender); + let hash = mtx.hash(); + pool.add_transaction(hash, sender, mtx).unwrap(); + hash + } + + fn eip1559(nonce: u64, max_fee: u64, max_priority: u64) -> Transaction { + Transaction::EIP1559Transaction(EIP1559Transaction { + nonce, + max_fee_per_gas: max_fee, + max_priority_fee_per_gas: max_priority, + ..Default::default() + }) + } + + fn eip4844(nonce: u64, max_fee: u64, max_priority: u64, blob_fee: u64) -> Transaction { + Transaction::EIP4844Transaction(EIP4844Transaction { + nonce, + max_fee_per_gas: max_fee, + max_priority_fee_per_gas: max_priority, + max_fee_per_blob_gas: U256::from(blob_fee), + ..Default::default() + }) + } + + // --- is_bumped_u64 ------------------------------------------------- + + #[test] + fn is_bumped_u64_exact_10_percent_accepted() { + assert!(is_bumped_u64(100, 110, 10)); + } + + #[test] + fn is_bumped_u64_just_below_10_percent_rejected() { + assert!(!is_bumped_u64(100, 109, 10)); + } + + #[test] + fn is_bumped_u64_zero_bump_allows_equal() { + assert!(is_bumped_u64(100, 100, 0)); + } + + #[test] + fn is_bumped_u64_zero_existing_always_accepted() { + assert!(is_bumped_u64(0, 0, 100)); + assert!(is_bumped_u64(0, 1, 100)); + } + + #[test] + fn is_bumped_u64_huge_existing_rejects_under_floor() { + // At `existing = u64::MAX` the 100%-bumped threshold is ~3.69e19, + // which doesn't fit in u64. Any new value (capped at u64::MAX, + // ~1.84e19) is strictly below threshold and must be rejected. + // The previous saturating-mul implementation silently *admitted* + // this case because it saturated the threshold to u64::MAX/100; + // the new checked-arithmetic implementation rejects, which is + // the correct semantic. + assert!(!is_bumped_u64(u64::MAX, u64::MAX, 100)); + // And the helper does not panic on extreme inputs. + let _ = is_bumped_u64(u64::MAX, 0, u64::MAX); + } + + // --- is_bumped_u256 ------------------------------------------------ + + #[test] + fn is_bumped_u256_blob_100_percent_accepted() { + assert!(is_bumped_u256(U256::from(50u64), U256::from(100u64), 100)); + } + + #[test] + fn is_bumped_u256_blob_99_percent_rejected() { + assert!(!is_bumped_u256(U256::from(100u64), U256::from(199u64), 100)); + } + + // --- find_tx_to_replace --------------------------------------------- + + #[test] + fn replacement_1_wei_bump_rejected_at_10_percent() { + // Spec scenario: "Strict-greater-than but below 10% bump rejected" + let pool = Mempool::new(64); + let sender = Address::from_low_u64_be(1); + let old = eip1559(0, 1_000, 100); + add_to_pool(&pool, sender, old); + + let new = eip1559(0, 1_001, 100); // +1 wei on max_fee, no bump on tip + let err = pool + .find_tx_to_replace(sender, 0, &new, 10, 100) + .unwrap_err(); + assert!(matches!(err, MempoolError::UnderpricedReplacement)); + } + + #[test] + fn replacement_full_10_percent_bump_on_both_axes_accepted() { + // Spec scenario: "10% bump on both axes accepted" + let pool = Mempool::new(64); + let sender = Address::from_low_u64_be(1); + let old = eip1559(0, 1_000, 100); + let old_hash = add_to_pool(&pool, sender, old); + + let new = eip1559(0, 1_100, 110); + let found = pool + .find_tx_to_replace(sender, 0, &new, 10, 100) + .unwrap() + .expect("replacement should be admitted"); + assert_eq!(found, old_hash); + } + + #[test] + fn replacement_asymmetric_bump_rejected() { + // Spec scenario: "10% bump on only one axis rejected" + let pool = Mempool::new(64); + let sender = Address::from_low_u64_be(1); + let old = eip1559(0, 1_000, 100); + add_to_pool(&pool, sender, old); + + let new = eip1559(0, 1_100, 105); // 10% on fee cap, 5% on tip + let err = pool + .find_tx_to_replace(sender, 0, &new, 10, 100) + .unwrap_err(); + assert!(matches!(err, MempoolError::UnderpricedReplacement)); + } + + #[test] + fn blob_replacement_50_percent_bump_rejected() { + // Spec scenario: "Blob 50% bump rejected" + let pool = Mempool::new(64); + let sender = Address::from_low_u64_be(2); + let old = eip4844(0, 1_000, 100, 50); + add_to_pool(&pool, sender, old); + + // 50% bump on all three fields, but blob threshold is 100%. + let new = eip4844(0, 1_500, 150, 75); + let err = pool + .find_tx_to_replace(sender, 0, &new, 10, 100) + .unwrap_err(); + assert!(matches!(err, MempoolError::UnderpricedReplacement)); + } + + #[test] + fn blob_replacement_100_percent_bump_on_three_axes_accepted() { + // Spec scenario: "Blob 100% bump on three axes accepted" + let pool = Mempool::new(64); + let sender = Address::from_low_u64_be(2); + let old = eip4844(0, 1_000, 100, 50); + let old_hash = add_to_pool(&pool, sender, old); + + let new = eip4844(0, 2_000, 200, 100); + let found = pool + .find_tx_to_replace(sender, 0, &new, 10, 100) + .unwrap() + .expect("blob replacement should be admitted"); + assert_eq!(found, old_hash); + } + + #[test] + fn no_existing_tx_returns_none() { + let pool = Mempool::new(64); + let sender = Address::from_low_u64_be(1); + let new = eip1559(0, 1_000, 100); + let res = pool.find_tx_to_replace(sender, 0, &new, 10, 100).unwrap(); + assert!(res.is_none()); + } + + // --- type-change rejection ----------------------------------------- + + #[test] + fn blob_cannot_be_replaced_by_non_blob() { + // An EIP-4844 tx with a blob sidecar must not be displaced by a + // cheaper non-blob tx at the same (sender, nonce), regardless of + // the fee bump. + let pool = Mempool::new(64); + let sender = Address::from_low_u64_be(3); + add_to_pool(&pool, sender, eip4844(0, 1_000, 100, 50)); + + let new = eip1559(0, u64::MAX, u64::MAX); // very high non-blob fees + let err = pool + .find_tx_to_replace(sender, 0, &new, 10, 100) + .unwrap_err(); + assert!(matches!(err, MempoolError::ReplacementTypeMismatch)); + } + + #[test] + fn non_blob_cannot_be_replaced_by_blob() { + // The inverse: a 1559 tx in the pool can't be replaced by a 4844 + // tx that suddenly demands sidecar handling. + let pool = Mempool::new(64); + let sender = Address::from_low_u64_be(3); + add_to_pool(&pool, sender, eip1559(0, 1_000, 100)); + + let new = eip4844(0, 2_000, 200, 100); + let err = pool + .find_tx_to_replace(sender, 0, &new, 10, 100) + .unwrap_err(); + assert!(matches!(err, MempoolError::ReplacementTypeMismatch)); + } + + // --- legacy path --------------------------------------------------- + + #[test] + fn legacy_replacement_requires_10_percent_bump() { + // The legacy branch was missing test coverage. Pin it. + let pool = Mempool::new(64); + let sender = Address::from_low_u64_be(4); + + let old = Transaction::LegacyTransaction(ethrex_common::types::LegacyTransaction { + nonce: 0, + gas_price: U256::from(1_000u64), + ..Default::default() + }); + add_to_pool(&pool, sender, old); + + // 1-wei bump rejected. + let too_small = Transaction::LegacyTransaction(ethrex_common::types::LegacyTransaction { + nonce: 0, + gas_price: U256::from(1_001u64), + ..Default::default() + }); + assert!(matches!( + pool.find_tx_to_replace(sender, 0, &too_small, 10, 100) + .unwrap_err(), + MempoolError::UnderpricedReplacement + )); + + // 10% bump accepted. + let ok = Transaction::LegacyTransaction(ethrex_common::types::LegacyTransaction { + nonce: 0, + gas_price: U256::from(1_100u64), + ..Default::default() + }); + let found = pool + .find_tx_to_replace(sender, 0, &ok, 10, 100) + .unwrap() + .expect("legacy replacement at 10% bump should be admitted"); + assert!(!found.is_zero()); + } +} diff --git a/docs/CLI.md b/docs/CLI.md index 3456578a0f..bf9341ec2b 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -91,6 +91,18 @@ Node options: [env: ETHREX_MEMPOOL_MAX_SIZE=] [default: 10000] + --mempool.price-bump + Minimum fee bump (in percent) required to replace a non-blob pooled transaction at the same (sender, nonce). + + [env: ETHREX_MEMPOOL_PRICE_BUMP=] + [default: 10] + + --mempool.blob-price-bump + Minimum fee bump (in percent) required to replace an EIP-4844 blob pooled transaction. + + [env: ETHREX_MEMPOOL_BLOB_PRICE_BUMP=] + [default: 100] + --precompute-witnesses Once synced, computes execution witnesses upon receiving newPayload messages and stores them in local storage