diff --git a/crates/execution/src/engine.rs b/crates/execution/src/engine.rs index 6be748a9..972ce5cc 100644 --- a/crates/execution/src/engine.rs +++ b/crates/execution/src/engine.rs @@ -5,7 +5,7 @@ use crate::{ database::{CipherBftDatabase, Provider}, - error::{ExecutionError, Result, TxErrorCategory}, + error::{ExecutionError, Result, SkipReason, TxErrorCategory}, evm::CipherBftEvmConfig, precompiles::{GenesisValidatorData, StakingPrecompile}, receipts::{ @@ -45,12 +45,14 @@ const BLOCK_HASH_CACHE_SIZE: NonZeroUsize = match NonZeroUsize::new(256) { /// - Logs emitted by each transaction /// - Total transaction fees collected (gas_used × effective_gas_price) /// - Executed transaction bytes (only transactions with receipts, not skipped ones) +/// - Hashes of permanently-skipped transactions (NonceTooLow) type ProcessTransactionsResult = ( Vec, u64, Vec>, U256, Vec, + Vec, ); /// ExecutionLayer trait defines the interface for block execution. @@ -350,6 +352,7 @@ impl ExecutionEngine

{ let mut all_logs = Vec::new(); let mut total_fees = U256::ZERO; let mut executed_tx_bytes = Vec::new(); + let mut skipped_tx_hashes: Vec = Vec::new(); // Parallel signature recovery - recovers sender addresses from all transactions // This is the CPU-intensive part that benefits from parallelization @@ -388,6 +391,9 @@ impl ExecutionEngine

{ Ok(result) => result, Err(e) => match e.category() { TxErrorCategory::Skip { reason } => { + if reason == SkipReason::NonceTooLow { + skipped_tx_hashes.push(recovered_tx.tx_hash); + } tracing::warn!( tx_index, ?reason, @@ -508,6 +514,7 @@ impl ExecutionEngine

{ all_logs, total_fees, executed_tx_bytes, + skipped_tx_hashes, )) } @@ -544,7 +551,7 @@ impl ExecutionLayer for ExecutionEngine

{ // Process all transactions and collect fees // executed_tx_bytes contains only transactions that actually executed (have receipts) - let (receipts, gas_used, all_logs, total_fees, executed_tx_bytes) = self + let (receipts, gas_used, all_logs, total_fees, executed_tx_bytes, skipped_tx_hashes) = self .process_transactions( &input.transactions, input.block_number, @@ -645,6 +652,7 @@ impl ExecutionLayer for ExecutionEngine

{ receipts, logs_bloom, executed_transactions: executed_tx_bytes, + skipped_tx_hashes, }) } @@ -939,6 +947,7 @@ mod tests { receipts: vec![], logs_bloom: Bloom::ZERO, executed_transactions: vec![], + skipped_tx_hashes: vec![], }; let sealed = engine @@ -1000,6 +1009,7 @@ mod tests { receipts: vec![], logs_bloom: Bloom::ZERO, executed_transactions: vec![], + skipped_tx_hashes: vec![], }; let sealed = engine diff --git a/crates/execution/src/lib.rs b/crates/execution/src/lib.rs index 28508624..0bb43767 100644 --- a/crates/execution/src/lib.rs +++ b/crates/execution/src/lib.rs @@ -594,6 +594,7 @@ mod tests { receipts: vec![], logs_bloom: Bloom::ZERO, executed_transactions: vec![], + skipped_tx_hashes: vec![], }; let sealed = execution_layer diff --git a/crates/execution/src/types.rs b/crates/execution/src/types.rs index b856b108..564e8399 100644 --- a/crates/execution/src/types.rs +++ b/crates/execution/src/types.rs @@ -359,6 +359,15 @@ pub struct ExecutionResult { /// the mempool pending map - only executed transactions should be removed. #[serde(default)] pub executed_transactions: Vec, + + /// Hashes of transactions that were permanently skipped (NonceTooLow). + /// + /// These transactions can never succeed because their nonce has already + /// been used. The node removes them from the RPC pending map to prevent + /// infinite retry loops where stale transactions cycle through + /// consensus and execution repeatedly. + #[serde(default)] + pub skipped_tx_hashes: Vec, } /// Transaction receipt. diff --git a/crates/node/src/execution_bridge.rs b/crates/node/src/execution_bridge.rs index f6d8cca3..cfea2618 100644 --- a/crates/node/src/execution_bridge.rs +++ b/crates/node/src/execution_bridge.rs @@ -37,6 +37,11 @@ pub struct BlockExecutionResult { /// in execution order. Used by the node to store transactions /// for `eth_getTransactionByHash` RPC queries. pub executed_transactions: Vec, + /// Hashes of permanently-skipped transactions (NonceTooLow). + /// + /// These should be removed from the RPC pending map to prevent + /// infinite retry loops. + pub skipped_tx_hashes: Vec, } /// Bridge between consensus and execution layers @@ -435,6 +440,7 @@ impl ExecutionBridge { // Use executed_transactions from the execution result - this only contains // transactions that actually executed (have receipts), not skipped ones let executed_transactions = result.executed_transactions.clone(); + let skipped_tx_hashes = result.skipped_tx_hashes.clone(); Ok(BlockExecutionResult { execution_result: result, @@ -442,6 +448,7 @@ impl ExecutionBridge { parent_hash, timestamp, executed_transactions, + skipped_tx_hashes, }) } diff --git a/crates/node/src/node.rs b/crates/node/src/node.rs index f3d07700..6cf8836f 100644 --- a/crates/node/src/node.rs +++ b/crates/node/src/node.rs @@ -1678,9 +1678,8 @@ impl Node { debug!("Broadcast block {} to WebSocket subscribers", height.0); } - // Clean up executed transactions and retry pending ones - // This prevents stale transactions from accumulating and - // ensures skipped transactions (e.g., NonceTooLow) are retried + // Clean up executed and permanently-invalid transactions, + // then retry remaining pending ones if let Some(ref mempool) = rpc_mempool { use alloy_rlp::Decodable; use reth_primitives::TransactionSigned; @@ -1704,11 +1703,27 @@ impl Node { ); } + // Remove permanently-skipped transactions (NonceTooLow) + // from the pending map. These can never succeed because + // their nonce has already been used. Without this, they + // would be retried every block in an infinite loop. + if !block_result.skipped_tx_hashes.is_empty() { + mempool.remove_included(&block_result.skipped_tx_hashes); + debug!( + "Removed {} permanently-skipped (NonceTooLow) transactions from mempool pending map", + block_result.skipped_tx_hashes.len() + ); + } + + // Collect all hashes to exclude from retry + let mut exclude_from_retry = tx_hashes; + exclude_from_retry.extend_from_slice(&block_result.skipped_tx_hashes); + // ALWAYS retry pending transactions after every block // Previously this was inside the if block, causing pending // transactions to only retry when a block had executed txs. // This led to multi-minute delays when the chain had empty blocks. - let retried = mempool.retry_pending(&tx_hashes).await; + let retried = mempool.retry_pending(&exclude_from_retry).await; if retried > 0 { debug!( "Re-queued {} pending transactions for retry",