From c7d90b31a746d0a737a9265b5fcf627f2de35ec8 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 3 Jul 2026 11:38:18 +0200 Subject: [PATCH] Improve FCR lifecycle robustness --- .../beacon_chain/src/canonical_head.rs | 17 ++++- .../beacon_chain/src/state_advance_timer.rs | 4 +- consensus/fast_confirmation/src/lib.rs | 67 +++++++++---------- 3 files changed, 51 insertions(+), 37 deletions(-) diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index 9b8fd575224..81963622de1 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -609,13 +609,24 @@ impl BeaconChain { /// can't abort block import because an error is returned here. #[instrument(name = "lh_recompute_head_at_slot", skip(self), level = "info", fields(slot = %current_slot))] pub async fn recompute_head_at_slot(self: &Arc, current_slot: Slot) { + self.recompute_head_at_slot_with_fcr(current_slot, true) + .await; + } + + #[instrument(name = "lh_recompute_head_at_slot_without_fcr", skip(self), level = "info", fields(slot = %current_slot))] + pub async fn recompute_head_at_slot_without_fcr(self: &Arc, current_slot: Slot) { + self.recompute_head_at_slot_with_fcr(current_slot, false) + .await; + } + + async fn recompute_head_at_slot_with_fcr(self: &Arc, current_slot: Slot, run_fcr: bool) { metrics::inc_counter(&metrics::FORK_CHOICE_REQUESTS); let _timer = metrics::start_timer(&metrics::FORK_CHOICE_TIMES); let chain = self.clone(); match self .spawn_blocking_handle( - move || chain.recompute_head_at_slot_internal(current_slot), + move || chain.recompute_head_at_slot_internal(current_slot, run_fcr), "recompute_head_internal", ) .await @@ -664,6 +675,7 @@ impl BeaconChain { fn recompute_head_at_slot_internal( self: &Arc, current_slot: Slot, + run_fcr: bool, ) -> Result>>, Error> { let recompute_head_lock = self.canonical_head.recompute_head_lock.lock(); @@ -746,7 +758,8 @@ impl BeaconChain { // Skip FCR while the head is more than `MAX_ADVANCE_DISTANCE` behind wall-clock (deep // sync): the state-advance timer won't have cached the head state FCR needs, so running // it would force an expensive load+advance under the fork-choice lock. - if let Some(ref fcr_mutex) = self.canonical_head.fast_confirmation + if run_fcr + && let Some(ref fcr_mutex) = self.canonical_head.fast_confirmation && new_head_proto_block.slot.as_u64() + MAX_ADVANCE_DISTANCE >= current_slot.as_u64() { let mut fcr = fcr_mutex.lock(); diff --git a/beacon_node/beacon_chain/src/state_advance_timer.rs b/beacon_node/beacon_chain/src/state_advance_timer.rs index 1b3790eb221..89040738f5b 100644 --- a/beacon_node/beacon_chain/src/state_advance_timer.rs +++ b/beacon_node/beacon_chain/src/state_advance_timer.rs @@ -209,7 +209,9 @@ async fn state_advance_timer( } // Re-compute the head, dequeuing attestations for the current slot early. - beacon_chain.recompute_head_at_slot(next_slot).await; + beacon_chain + .recompute_head_at_slot_without_fcr(next_slot) + .await; // Prepare proposers so that the node can send payload attributes in the case where // it decides to abandon a proposer boost re-org. diff --git a/consensus/fast_confirmation/src/lib.rs b/consensus/fast_confirmation/src/lib.rs index b4fed7304cd..2ac2adb46b5 100644 --- a/consensus/fast_confirmation/src/lib.rs +++ b/consensus/fast_confirmation/src/lib.rs @@ -245,22 +245,17 @@ impl FastConfirmationRule { let _span = debug_span!("fcr_update_variables", slot = %current_slot).entered(); let current_epoch = current_slot.epoch(E::slots_per_epoch()); - // Rebuild the head-derived caches when the head changes (including within a slot, e.g. a - // late block or reorg). Each cache is keyed on the head's dependent root; rebuild it from - // scratch, independently, when its own dependent root is stale (a reorg past the - // previous-epoch boundary, or a new epoch). - if self.current_slot_head != head_root { - let head_dependent_root = dependent_root::(state, current_epoch)?; - - if self.head_assignments.dependent_root() != head_dependent_root { - let _span = debug_span!("fcr_rebuild_assignments").entered(); - self.head_assignments = SlotAssignments::new::(state)?; - } + // Rebuild the head-derived caches when their own dependent root is stale. + let head_dependent_root = dependent_root::(state, current_epoch)?; - if self.head_balance_source.dependent_root != head_dependent_root { - let _span = debug_span!("fcr_rebuild_head_balance").entered(); - self.head_balance_source = BalanceSourceData::for_epoch(state, current_epoch)?; - } + if self.head_assignments.dependent_root() != head_dependent_root { + let _span = debug_span!("fcr_rebuild_assignments").entered(); + self.head_assignments = SlotAssignments::new::(state)?; + } + + if self.head_balance_source.dependent_root != head_dependent_root { + let _span = debug_span!("fcr_rebuild_head_balance").entered(); + self.head_balance_source = BalanceSourceData::for_epoch(state, current_epoch)?; } // Spec: update_fast_confirmation_variables must be called at most once per slot. @@ -313,26 +308,30 @@ impl FastConfirmationRule { // Revert to finalized block if either of the following is true: let should_revert_to_finalized_reason = - if get_block_epoch::(confirmed_root, proto_array)?.safe_add(1)? < current_epoch { - // 1) the latest confirmed block's epoch is older than the previous epoch, - Some("epoch_too_old") - } else if !is_ancestor(head_root, confirmed_root, proto_array)? { - // 2) the latest confirmed block does not belong to the canonical chain, - Some("not_ancestor") - } else if is_epoch_start - && let Some(chain_unsafe_reason) = self.is_confirmed_chain_safe::( - // 3) the confirmed chain starting from the current epoch observed justified - // checkpoint cannot be re-confirmed at the start of the current epoch. - confirmed_root, - current_slot, - proto_array, - votes, - equivocating_indices, - )? - { - Some(chain_unsafe_reason) + if let Ok(confirmed_epoch) = get_block_epoch::(confirmed_root, proto_array) { + if confirmed_epoch.safe_add(1)? < current_epoch { + // 1) the latest confirmed block's epoch is older than the previous epoch, + Some("epoch_too_old") + } else if !is_ancestor(head_root, confirmed_root, proto_array)? { + // 2) the latest confirmed block does not belong to the canonical chain, + Some("not_ancestor") + } else if is_epoch_start + && let Some(chain_unsafe_reason) = self.is_confirmed_chain_safe::( + // 3) the confirmed chain starting from the current epoch observed justified + // checkpoint cannot be re-confirmed at the start of the current epoch. + confirmed_root, + current_slot, + proto_array, + votes, + equivocating_indices, + )? + { + Some(chain_unsafe_reason) + } else { + None + } } else { - None + Some("unknown") }; if let Some(reason) = should_revert_to_finalized_reason { debug!(