diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index 06731e26324..a49c4ec04f8 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -725,6 +725,196 @@ 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 the latest block tracks the given canonical header by resetting + /// + /// # Arguments + /// * `canonical_header` - The canonical header to set as the new head + /// + /// # Returns + /// * `ProviderResult<()>` - Ok(()) on success, error if state update fails + /// + /// Caution: This unwinds the canonical chain + fn update_latest_block_to_canonical_ancestor( + &mut self, + canonical_header: &SealedHeader, + ) -> ProviderResult<()> { + debug!(target: "engine::tree", head = ?canonical_header.num_hash(), "Update latest block to canonical ancestor"); + 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(); + + // 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 { + 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" + ); + + self.handle_canonical_chain_unwind(current_head_number, canonical_header) + } else { + 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" + ); + self.handle_chain_advance_or_same_height(canonical_header) + } + } + + /// Handles chain unwind scenarios by collecting blocks to remove and performing an unwind back + /// to the canonical header + fn handle_canonical_chain_unwind( + &self, + current_head_number: u64, + canonical_header: &SealedHeader, + ) -> ProviderResult<()> { + let new_head_number = canonical_header.number(); + 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_canonical_unwind(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 to a canonical block. + fn collect_blocks_for_canonical_unwind( + &self, + new_head_number: u64, + current_head_number: u64, + ) -> Vec> { + let mut old_blocks = Vec::new(); + + 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); + debug!( + target: "engine::tree", + block_number = block_num, + "Collected block for removal from in-memory state" + ); + } + } + + if old_blocks.is_empty() { + 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( + &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( + &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(&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 @@ -868,9 +1058,8 @@ where if let Ok(Some(canonical_header)) = self.find_canonical_header(state.head_block_hash) { debug!(target: "engine::tree", head = canonical_header.number(), "fcu head block is already canonical"); - // For OpStack the proposers are allowed to reorg their own chain at will, so we need to - // always trigger a new payload job if requested. - // Also allow forcing this behavior via a config flag. + // For OpStack, or if explicitly configured, the proposers are allowed to reorg their + // own chain at will, so we need to always trigger a new payload job if requested. if self.engine_kind.is_opstack() || self.config.always_process_payload_attributes_on_canonical_head() { @@ -880,6 +1069,14 @@ where self.process_payload_attributes(attr, &canonical_header, state, version); return Ok(TreeOutcome::new(updated)) } + + // At this point, no alternative block has been triggered, so we need effectively + // unwind the _canonical_ chain to the FCU's head, which is part of the canonical + // chain. We need to 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 677861119b4..d650ed6488a 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,84 @@ 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" + ); +}