diff --git a/crates/engine/primitives/src/forkchoice.rs b/crates/engine/primitives/src/forkchoice.rs index 69cb5990711..533bbeb41bb 100644 --- a/crates/engine/primitives/src/forkchoice.rs +++ b/crates/engine/primitives/src/forkchoice.rs @@ -21,7 +21,8 @@ impl ForkchoiceStateTracker { /// `sync_target` to `None`, since we're now fully synced. pub const fn set_latest(&mut self, state: ForkchoiceState, status: ForkchoiceStatus) { if status.is_valid() { - self.set_valid(state); + self.last_syncing = None; + self.last_valid = Some(state); } else if status.is_syncing() { self.last_syncing = Some(state); } @@ -30,11 +31,24 @@ impl ForkchoiceStateTracker { self.latest = Some(received); } - const fn set_valid(&mut self, state: ForkchoiceState) { - // we no longer need to sync to this state. + /// Promotes a previously tracked syncing forkchoice state to valid, without overwriting a + /// newer `latest` state. + /// + /// This is used when a `Syncing` FCU's head finally becomes canonical via the downloaded-block + /// flow, so the safe/finalized anchors of that FCU can be applied. Unlike + /// [`Self::set_latest`], this preserves a newer `latest` (e.g. an `Invalid` FCU received + /// after the syncing one) and only flips `latest` to `Valid` when it still refers to the same + /// syncing FCU being promoted. + pub fn promote_sync_target_to_valid(&mut self, state: ForkchoiceState) { self.last_syncing = None; - self.last_valid = Some(state); + + if let Some(received) = self.latest.as_mut() && + received.state == state && + received.status.is_syncing() + { + received.status = ForkchoiceStatus::Valid; + } } /// Returns the [`ForkchoiceStatus`] of the latest received FCU. diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index 7837063decc..1edf1496e23 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -1917,9 +1917,37 @@ where self.on_canonical_chain_update(chain_update); } + self.on_canonicalized_sync_target(target); + Ok(()) } + /// Applies the tracked forkchoice state once its sync target head becomes canonical. + fn on_canonicalized_sync_target(&mut self, target: B256) { + let Some(sync_target_state) = self + .state + .forkchoice_state_tracker + .sync_target_state() + .filter(|state| state.head_block_hash == target) + else { + return; + }; + + if let Err(outcome) = self.ensure_consistent_forkchoice_state(sync_target_state) { + debug!( + target: "engine::tree", + head = %sync_target_state.head_block_hash, + safe = %sync_target_state.safe_block_hash, + finalized = %sync_target_state.finalized_block_hash, + ?outcome, + "Canonicalized sync target head before safe/finalized could be applied" + ); + return; + } + + self.state.forkchoice_state_tracker.promote_sync_target_to_valid(sync_target_state); + } + /// Convenience function to handle an optional tree event. fn on_maybe_tree_event(&mut self, event: Option) -> ProviderResult<()> { if let Some(event) = event { diff --git a/crates/engine/tree/src/tree/tests.rs b/crates/engine/tree/src/tree/tests.rs index 552a21d11a5..193200f2a32 100644 --- a/crates/engine/tree/src/tree/tests.rs +++ b/crates/engine/tree/src/tree/tests.rs @@ -2254,3 +2254,65 @@ fn test_on_valid_downloaded_head_sync_target_returns_make_canonical() { other => panic!("Expected MakeCanonical for head block, got: {other:?}"), } } + +/// Tests that canonicalizing a downloaded sync target head also applies the tracked finalized +/// block from the original `SYNCING` forkchoice state. +#[test] +fn test_canonicalizing_downloaded_sync_target_head_updates_finalized() { + reth_tracing::init_test_tracing(); + + let chain_spec = MAINNET.clone(); + let mut test_harness = TestHarness::new(chain_spec); + + let blocks: Vec<_> = test_harness.block_builder.get_executed_blocks(0..3).collect(); + let genesis = &blocks[0]; + let finalized_block = &blocks[1]; + let head_block = &blocks[2]; + + test_harness = test_harness.with_blocks(vec![ + genesis.clone(), + finalized_block.clone(), + head_block.clone(), + ]); + + let finalized_num_hash = finalized_block.recovered_block().num_hash(); + let head_num_hash = head_block.recovered_block().num_hash(); + + test_harness.tree.state.tree_state.set_canonical_head(genesis.recovered_block().num_hash()); + + let fcu_state = ForkchoiceState { + head_block_hash: head_num_hash.hash, + safe_block_hash: head_num_hash.hash, + finalized_block_hash: finalized_num_hash.hash, + }; + test_harness + .tree + .state + .forkchoice_state_tracker + .set_latest(fcu_state, ForkchoiceStatus::Syncing); + + let event = test_harness + .tree + .on_valid_downloaded_block(head_num_hash) + .unwrap() + .expect("expected canonicalization event for sync target head"); + + test_harness.tree.on_tree_event(event).unwrap(); + + assert_eq!(test_harness.tree.state.tree_state.canonical_block_hash(), head_num_hash.hash); + assert_eq!( + test_harness.tree.canonical_in_memory_state.get_finalized_num_hash(), + Some(finalized_num_hash), + "Finalized block from the syncing FCU should be applied once the head becomes canonical" + ); + assert_eq!( + test_harness.tree.canonical_in_memory_state.get_safe_num_hash(), + Some(head_num_hash), + "Safe block from the syncing FCU should be applied once the head becomes canonical" + ); + assert_eq!( + test_harness.tree.state.forkchoice_state_tracker.last_valid_state(), + Some(fcu_state) + ); + assert!(test_harness.tree.state.forkchoice_state_tracker.sync_target_state().is_none()); +}