diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index 47e68a25c4f..1f44a036024 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -701,6 +701,227 @@ where Ok(Some(NewCanonicalChain::Reorg { new: new_chain, old: old_chain })) } + /// Updates the latest block state to the specified canonical ancestor. + /// + /// This method ensures that state providers and the transaction pool operate with + /// the correct chain state after forkchoice update processing. It detects unwind + /// scenarios and provides appropriate logging for debugging purposes. + /// + /// # Arguments + /// * `canonical_header` - The canonical header to set as the new head + /// + /// # Returns + /// * `ProviderResult<()>` - Ok(()) on success, error if state update fails + fn update_latest_block_to_canonical_ancestor( + &mut self, + canonical_header: &SealedHeader, + ) -> ProviderResult<()> { + let current_head_number = self.state.tree_state.canonical_block_number(); + let new_head_number = canonical_header.number(); + let new_head_hash = canonical_header.hash(); + + // Log the type of chain state update being performed + self.log_chain_update_type(current_head_number, new_head_number, new_head_hash); + + // Update tree state with the new canonical head + self.state.tree_state.set_canonical_head(canonical_header.num_hash()); + + // Handle the state update based on whether this is an unwind scenario + if new_head_number < current_head_number { + self.handle_chain_unwind(current_head_number, canonical_header) + } else { + self.handle_chain_advance_or_same_height(canonical_header) + } + } + + /// Logs the type of chain state update being performed. + fn log_chain_update_type( + &self, + current_head_number: u64, + new_head_number: u64, + new_head_hash: B256, + ) { + match new_head_number.cmp(¤t_head_number) { + std::cmp::Ordering::Less => { + debug!( + target: "engine::tree", + current_head = current_head_number, + new_head = new_head_number, + new_head_hash = ?new_head_hash, + "FCU unwind detected: reverting to canonical ancestor" + ); + } + std::cmp::Ordering::Greater => { + debug!( + target: "engine::tree", + previous_head = current_head_number, + new_head = new_head_number, + new_head_hash = ?new_head_hash, + "Advancing latest block to canonical ancestor" + ); + } + std::cmp::Ordering::Equal => { + debug!( + target: "engine::tree", + head_number = new_head_number, + new_head_hash = ?new_head_hash, + "Updating latest block to canonical ancestor at same height" + ); + } + } + } + + /// Handles chain unwind scenarios by collecting blocks to remove and performing a reorg. + fn handle_chain_unwind( + &mut self, + current_head_number: u64, + canonical_header: &SealedHeader, + ) -> ProviderResult<()> { + let new_head_number = canonical_header.number(); + let new_head_hash = canonical_header.hash(); + + debug!( + target: "engine::tree", + from = current_head_number, + to = new_head_number, + "Handling unwind: collecting blocks to remove from in-memory state" + ); + + // Collect blocks that need to be removed from memory + let old_blocks = self.collect_blocks_for_removal(new_head_number, current_head_number); + + // Load and apply the canonical ancestor block + self.apply_canonical_ancestor_via_reorg(canonical_header, old_blocks) + } + + /// Collects blocks from memory that need to be removed during an unwind. + fn collect_blocks_for_removal( + &self, + new_head_number: u64, + current_head_number: u64, + ) -> Vec> { + let mut old_blocks = Vec::new(); + let mut found_blocks_to_remove = false; + + for block_num in (new_head_number + 1)..=current_head_number { + if let Some(block_state) = self.canonical_in_memory_state.state_by_number(block_num) { + let executed_block = block_state.block_ref().block.clone(); + old_blocks.push(executed_block); + found_blocks_to_remove = true; + debug!( + target: "engine::tree", + block_number = block_num, + "Collected block for removal from in-memory state" + ); + } + } + + if !found_blocks_to_remove { + debug!( + target: "engine::tree", + "No blocks found in memory to remove, will clear and reset state" + ); + } + + old_blocks + } + + /// Applies the canonical ancestor block via a reorg operation. + fn apply_canonical_ancestor_via_reorg( + &mut self, + canonical_header: &SealedHeader, + old_blocks: Vec>, + ) -> ProviderResult<()> { + let new_head_hash = canonical_header.hash(); + let new_head_number = canonical_header.number(); + + // Try to load the canonical ancestor's block + match self.canonical_block_by_hash(new_head_hash)? { + Some(executed_block) => { + let block_with_trie = ExecutedBlockWithTrieUpdates { + block: executed_block, + trie: ExecutedTrieUpdates::Missing, + }; + + // Perform the reorg to properly handle the unwind + self.canonical_in_memory_state.update_chain(NewCanonicalChain::Reorg { + new: vec![block_with_trie], + old: old_blocks, + }); + + // CRITICAL: Update the canonical head after the reorg + // This ensures get_canonical_head() returns the correct block + self.canonical_in_memory_state.set_canonical_head(canonical_header.clone()); + + debug!( + target: "engine::tree", + block_number = new_head_number, + block_hash = ?new_head_hash, + "Successfully loaded canonical ancestor into memory via reorg" + ); + } + None => { + // Fallback: update header only if block cannot be found + warn!( + target: "engine::tree", + block_hash = ?new_head_hash, + "Could not find canonical ancestor block, updating header only" + ); + self.canonical_in_memory_state.set_canonical_head(canonical_header.clone()); + } + } + + Ok(()) + } + + /// Handles chain advance or same height scenarios. + fn handle_chain_advance_or_same_height( + &mut self, + canonical_header: &SealedHeader, + ) -> ProviderResult<()> { + let new_head_number = canonical_header.number(); + let new_head_hash = canonical_header.hash(); + + // Update the canonical head header + self.canonical_in_memory_state.set_canonical_head(canonical_header.clone()); + + // Load the block into memory if it's not already present + self.ensure_block_in_memory(new_head_number, new_head_hash) + } + + /// Ensures a block is loaded into memory if not already present. + fn ensure_block_in_memory( + &mut self, + block_number: u64, + block_hash: B256, + ) -> ProviderResult<()> { + // Check if block is already in memory + if self.canonical_in_memory_state.state_by_number(block_number).is_some() { + return Ok(()); + } + + // Try to load the block from storage + if let Some(executed_block) = self.canonical_block_by_hash(block_hash)? { + let block_with_trie = ExecutedBlockWithTrieUpdates { + block: executed_block, + trie: ExecutedTrieUpdates::Missing, + }; + + self.canonical_in_memory_state.update_chain(NewCanonicalChain::Commit { + new: vec![block_with_trie], + }); + + debug!( + target: "engine::tree", + block_number, + block_hash = ?block_hash, + "Added canonical block to in-memory state" + ); + } + + Ok(()) + } + /// Determines if the given block is part of a fork by checking that these /// conditions are true: /// * walking back from the target hash to verify that the target hash is not part of an @@ -856,6 +1077,12 @@ where self.process_payload_attributes(attr, &canonical_header, state, version); return Ok(TreeOutcome::new(updated)) } + + // Update the latest block state to reflect the canonical ancestor. + // This ensures that state providers and the transaction pool operate with + // the correct chain state after forkchoice update processing. + self.update_latest_block_to_canonical_ancestor(&canonical_header)?; + } // 2. Client software MAY skip an update of the forkchoice state and MUST NOT begin a diff --git a/crates/engine/tree/src/tree/tests.rs b/crates/engine/tree/src/tree/tests.rs index ffb327e63d5..4ae8f347016 100644 --- a/crates/engine/tree/src/tree/tests.rs +++ b/crates/engine/tree/src/tree/tests.rs @@ -23,6 +23,7 @@ use std::{ str::FromStr, sync::mpsc::{channel, Sender}, }; +use tokio::sync::oneshot; /// Mock engine validator for tests #[derive(Debug, Clone)] @@ -867,3 +868,90 @@ async fn test_engine_tree_live_sync_transition_required_blocks_requested() { _ => panic!("Unexpected event: {event:#?}"), } } + +#[tokio::test] +async fn test_fcu_with_canonical_ancestor_updates_latest_block() { + // Test for issue where FCU with canonical ancestor doesn't update Latest block state + // This was causing "nonce too low" errors when discard_reorged_transactions is enabled + + reth_tracing::init_test_tracing(); + let chain_spec = MAINNET.clone(); + + // Create test harness + let mut test_harness = TestHarness::new(chain_spec.clone()); + + // Set engine kind to OpStack to ensure the fix is triggered + test_harness.tree.engine_kind = EngineApiKind::OpStack; + let mut test_block_builder = TestBlockBuilder::eth().with_chain_spec((*chain_spec).clone()); + + // Create a chain of blocks + let blocks: Vec<_> = test_block_builder.get_executed_blocks(1..5).collect(); + test_harness = test_harness.with_blocks(blocks.clone()); + + // Set block 4 as the current canonical head + let current_head = blocks[3].recovered_block().clone(); // Block 4 (0-indexed as blocks[3]) + let current_head_sealed = current_head.clone_sealed_header(); + test_harness.tree.state.tree_state.set_canonical_head(current_head.num_hash()); + test_harness.tree.canonical_in_memory_state.set_canonical_head(current_head_sealed); + + // Verify the current head is set correctly + assert_eq!( + test_harness.tree.state.tree_state.canonical_block_number(), + current_head.number() + ); + assert_eq!( + test_harness.tree.state.tree_state.canonical_block_hash(), + current_head.hash() + ); + + // Now perform FCU to a canonical ancestor (block 2) + let ancestor_block = blocks[1].recovered_block().clone(); // Block 2 (0-indexed as blocks[1]) + + // Send FCU to the canonical ancestor + let (tx, rx) = oneshot::channel(); + test_harness + .tree + .on_engine_message(FromEngine::Request( + BeaconEngineMessage::ForkchoiceUpdated { + state: ForkchoiceState { + head_block_hash: ancestor_block.hash(), + safe_block_hash: B256::ZERO, + finalized_block_hash: B256::ZERO, + }, + payload_attrs: None, + tx, + version: EngineApiMessageVersion::default(), + } + .into(), + )) + .unwrap(); + + // Verify FCU succeeds + let response = rx.await.unwrap().unwrap().await.unwrap(); + assert!(response.payload_status.is_valid()); + + // The critical test: verify that Latest block has been updated to the canonical ancestor + // Check tree state + assert_eq!( + test_harness.tree.state.tree_state.canonical_block_number(), + ancestor_block.number(), + "Tree state: Latest block number should be updated to canonical ancestor" + ); + assert_eq!( + test_harness.tree.state.tree_state.canonical_block_hash(), + ancestor_block.hash(), + "Tree state: Latest block hash should be updated to canonical ancestor" + ); + + // Also verify canonical in-memory state is synchronized + assert_eq!( + test_harness.tree.canonical_in_memory_state.get_canonical_head().number, + ancestor_block.number(), + "In-memory state: Latest block number should be updated to canonical ancestor" + ); + assert_eq!( + test_harness.tree.canonical_in_memory_state.get_canonical_head().hash(), + ancestor_block.hash(), + "In-memory state: Latest block hash should be updated to canonical ancestor" + ); +} diff --git a/crates/ethereum/primitives/src/receipt.rs b/crates/ethereum/primitives/src/receipt.rs index 6c81f8bd69d..4d2d231ac45 100644 --- a/crates/ethereum/primitives/src/receipt.rs +++ b/crates/ethereum/primitives/src/receipt.rs @@ -319,8 +319,7 @@ where tx_type: value.tx_type(), success: value.is_success(), cumulative_gas_used: value.cumulative_gas_used(), - // TODO: remove after - logs: value.logs().to_vec(), + logs: value.into_logs(), } } } diff --git a/crates/evm/evm/src/execute.rs b/crates/evm/evm/src/execute.rs index 93ced85fabe..49e442a2b7c 100644 --- a/crates/evm/evm/src/execute.rs +++ b/crates/evm/evm/src/execute.rs @@ -10,7 +10,6 @@ use alloy_evm::{ Evm, EvmEnv, EvmFactory, RecoveredTx, ToTxEnv, }; use alloy_primitives::{Address, B256}; -use core::fmt::Debug; pub use reth_execution_errors::{ BlockExecutionError, BlockValidationError, InternalBlockExecutionError, }; diff --git a/crates/evm/execution-types/src/execution_outcome.rs b/crates/evm/execution-types/src/execution_outcome.rs index 098b3c8aeed..f3edacf2edf 100644 --- a/crates/evm/execution-types/src/execution_outcome.rs +++ b/crates/evm/execution-types/src/execution_outcome.rs @@ -558,7 +558,7 @@ mod tests { use alloy_primitives::{bytes, Address, LogData, B256}; #[test] - fn test_initialisation() { + fn test_initialization() { // Create a new BundleState object with initial data let bundle = BundleState::new( vec![(Address::new([2; 20]), None, Some(AccountInfo::default()), HashMap::default())], diff --git a/crates/optimism/cli/src/receipt_file_codec.rs b/crates/optimism/cli/src/receipt_file_codec.rs index f5b6b48b268..8cd50037c57 100644 --- a/crates/optimism/cli/src/receipt_file_codec.rs +++ b/crates/optimism/cli/src/receipt_file_codec.rs @@ -148,7 +148,7 @@ pub(crate) mod test { bloom: Bloom::from(hex!( "00000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000400000000000100000000000000200000000002000000000000001000000000000000000004000000000000000000000000000040000400000100400000000000000100000000000000000000000000000020000000000000000000000000000000000000000000000001000000000000000000000100000000000000000000000000000000000000000000000000000000000000088000000080000000000010000000000000000000000000000800008000120000000000000000000000000000000002000" )), - logs: receipt.receipt.logs().to_vec(), + logs: receipt.receipt.into_logs(), tx_hash: b256!("0x5e77a04531c7c107af1882d76cbff9486d0a9aa53701c30888509d4f5f2b003a"), contract_address: address!("0x0000000000000000000000000000000000000000"), gas_used: 202813, block_hash: b256!("0xbee7192e575af30420cae0c7776304ac196077ee72b048970549e4f08e875453"), block_number: receipt.number, diff --git a/crates/optimism/evm/src/lib.rs b/crates/optimism/evm/src/lib.rs index 4973600d3d3..7e5ae9e5b4c 100644 --- a/crates/optimism/evm/src/lib.rs +++ b/crates/optimism/evm/src/lib.rs @@ -550,7 +550,7 @@ mod tests { } #[test] - fn test_initialisation() { + fn test_initialization() { // Create a new BundleState object with initial data let bundle = BundleState::new( vec![(Address::new([2; 20]), None, Some(AccountInfo::default()), HashMap::default())], diff --git a/crates/rpc/rpc-convert/src/transaction.rs b/crates/rpc/rpc-convert/src/transaction.rs index cf845067410..e33d254e272 100644 --- a/crates/rpc/rpc-convert/src/transaction.rs +++ b/crates/rpc/rpc-convert/src/transaction.rs @@ -223,6 +223,55 @@ where } } +/// Converts `TxReq` into `SimTx`. +/// +/// Where: +/// * `TxReq` is a transaction request received from an RPC API +/// * `SimTx` is the corresponding consensus layer transaction for execution simulation +/// +/// The `SimTxConverter` has two blanket implementations: +/// * `()` assuming `TxReq` implements [`TryIntoSimTx`] and is used as default for [`RpcConverter`]. +/// * `Fn(TxReq) -> Result>` and can be applied using +/// [`RpcConverter::with_sim_tx_converter`]. +/// +/// One should prefer to implement [`TryIntoSimTx`] for `TxReq` to get the `SimTxConverter` +/// implementation for free, thanks to the blanket implementation, unless the conversion requires +/// more context. For example, some configuration parameters or access handles to database, network, +/// etc. +pub trait SimTxConverter: Clone + Debug + Unpin + Send + Sync + 'static { + /// An associated error that can occur during the conversion. + type Err: Error; + + /// Performs the conversion from `tx_req` into `SimTx`. + /// + /// See [`SimTxConverter`] for more information. + fn convert_sim_tx(&self, tx_req: TxReq) -> Result; +} + +impl SimTxConverter for () +where + TxReq: TryIntoSimTx + Debug, +{ + type Err = ValueError; + + fn convert_sim_tx(&self, tx_req: TxReq) -> Result { + tx_req.try_into_sim_tx() + } +} + +impl SimTxConverter for F +where + TxReq: Debug, + E: Error, + F: Fn(TxReq) -> Result + Clone + Debug + Unpin + Send + Sync + 'static, +{ + type Err = E; + + fn convert_sim_tx(&self, tx_req: TxReq) -> Result { + self(tx_req) + } +} + /// Converts `self` into `T`. /// /// Should create a fake transaction for simulation using [`TransactionRequest`]. @@ -402,12 +451,13 @@ pub struct TransactionConversionError(String); /// is [`TransactionInfo`] then `()` can be used as `Map` which trivially passes over the input /// object. #[derive(Debug)] -pub struct RpcConverter { +pub struct RpcConverter { network: PhantomData, evm: PhantomData, receipt_converter: Receipt, header_converter: Header, mapper: Map, + sim_tx_converter: SimTx, } impl RpcConverter { @@ -419,20 +469,24 @@ impl RpcConverter { receipt_converter, header_converter: (), mapper: (), + sim_tx_converter: (), } } } -impl RpcConverter { +impl + RpcConverter +{ /// Converts the network type - pub fn with_network(self) -> RpcConverter { - let Self { receipt_converter, header_converter, mapper, evm, .. } = self; + pub fn with_network(self) -> RpcConverter { + let Self { receipt_converter, header_converter, mapper, evm, sim_tx_converter, .. } = self; RpcConverter { receipt_converter, header_converter, mapper, network: Default::default(), evm, + sim_tx_converter, } } @@ -440,27 +494,39 @@ impl RpcConverter( self, header_converter: HeaderNew, - ) -> RpcConverter { - let Self { receipt_converter, header_converter: _, mapper, network, evm } = self; - RpcConverter { receipt_converter, header_converter, mapper, network, evm } + ) -> RpcConverter { + let Self { receipt_converter, header_converter: _, mapper, network, evm, sim_tx_converter } = + self; + RpcConverter { receipt_converter, header_converter, mapper, network, evm, sim_tx_converter } } /// Configures the mapper. pub fn with_mapper( self, mapper: MapNew, - ) -> RpcConverter { - let Self { receipt_converter, header_converter, mapper: _, network, evm } = self; - RpcConverter { receipt_converter, header_converter, mapper, network, evm } + ) -> RpcConverter { + let Self { receipt_converter, header_converter, mapper: _, network, evm, sim_tx_converter } = + self; + RpcConverter { receipt_converter, header_converter, mapper, network, evm, sim_tx_converter } + } + + /// Swaps the simulate transaction converter with `sim_tx_converter`. + pub fn with_sim_tx_converter( + self, + sim_tx_converter: SimTxNew, + ) -> RpcConverter { + let Self { receipt_converter, header_converter, mapper, network, evm, .. } = self; + RpcConverter { receipt_converter, header_converter, mapper, network, evm, sim_tx_converter } } } -impl Default - for RpcConverter +impl Default + for RpcConverter where Receipt: Default, Header: Default, Map: Default, + SimTx: Default, { fn default() -> Self { Self { @@ -469,12 +535,13 @@ where receipt_converter: Default::default(), header_converter: Default::default(), mapper: Default::default(), + sim_tx_converter: Default::default(), } } } -impl Clone - for RpcConverter +impl Clone + for RpcConverter { fn clone(&self) -> Self { Self { @@ -483,18 +550,19 @@ impl Clone receipt_converter: self.receipt_converter.clone(), header_converter: self.header_converter.clone(), mapper: self.mapper.clone(), + sim_tx_converter: self.sim_tx_converter.clone(), } } } -impl RpcConvert - for RpcConverter +impl RpcConvert + for RpcConverter where N: NodePrimitives, Network: RpcTypes + Send + Sync + Unpin + Clone + Debug, Evm: ConfigureEvm + 'static, TxTy: IntoRpcTx + Clone + Debug, - RpcTxReq: TryIntoSimTx> + TryIntoTxEnv>, + RpcTxReq: TryIntoTxEnv>, Receipt: ReceiptConverter< N, RpcReceipt = RpcReceipt, @@ -521,6 +589,7 @@ where + Send + Sync + 'static, + SimTx: SimTxConverter, TxTy>, { type Primitives = N; type Network = Network; @@ -542,7 +611,10 @@ where &self, request: RpcTxReq, ) -> Result, Self::Error> { - Ok(request.try_into_sim_tx().map_err(|e| TransactionConversionError(e.to_string()))?) + Ok(self + .sim_tx_converter + .convert_sim_tx(request) + .map_err(|e| TransactionConversionError(e.to_string()))?) } fn tx_env( diff --git a/crates/storage/codecs/src/alloy/transaction/legacy.rs b/crates/storage/codecs/src/alloy/transaction/legacy.rs index 60250ba64af..1667893dc33 100644 --- a/crates/storage/codecs/src/alloy/transaction/legacy.rs +++ b/crates/storage/codecs/src/alloy/transaction/legacy.rs @@ -42,7 +42,7 @@ pub(crate) struct TxLegacy { value: U256, /// Input has two uses depending if transaction is Create or Call (if `to` field is None or /// Some). pub init: An unlimited size byte array specifying the - /// EVM-code for the account initialisation procedure CREATE, + /// EVM-code for the account initialization procedure CREATE, /// data: An unlimited size byte array specifying the /// input data of the message call, formally Td. input: Bytes, diff --git a/crates/storage/provider/src/providers/blockchain_provider.rs b/crates/storage/provider/src/providers/blockchain_provider.rs index af1b4fe91d5..f0bea8a2894 100644 --- a/crates/storage/provider/src/providers/blockchain_provider.rs +++ b/crates/storage/provider/src/providers/blockchain_provider.rs @@ -1341,7 +1341,7 @@ mod tests { async fn test_canon_state_subscriptions() -> eyre::Result<()> { let factory = create_test_provider_factory(); - // Generate a random block to initialise the blockchain provider. + // Generate a random block to initialize the blockchain provider. let mut test_block_builder = TestBlockBuilder::eth(); let block_1 = test_block_builder.generate_random_block(0, B256::ZERO); let block_hash_1 = block_1.hash();