🤖 Improve FCR lifecycle robustness

beacon_node/beacon_chain/src/canonical_head.rs

@@ -609,13 +609,24 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     /// 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<Self>, 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<Self>, current_slot: Slot) {
+        self.recompute_head_at_slot_with_fcr(current_slot, false)
+            .await;
+    }
+
+    async fn recompute_head_at_slot_with_fcr(self: &Arc<Self>, 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<T: BeaconChainTypes> BeaconChain<T> {
     fn recompute_head_at_slot_internal(
         self: &Arc<Self>,
         current_slot: Slot,
+        run_fcr: bool,
     ) -> Result<Option<JoinHandle<Option<()>>>, Error> {
         let recompute_head_lock = self.canonical_head.recompute_head_lock.lock();
 
@@ -746,7 +758,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         // 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();

beacon_node/beacon_chain/src/state_advance_timer.rs

@@ -209,7 +209,9 @@ async fn state_advance_timer<T: BeaconChainTypes>(
                 }
 
                 // 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.

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::<E>(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::<E>(state)?;
-            }
+        // Rebuild the head-derived caches when their own dependent root is stale.
+        let head_dependent_root = dependent_root::<E>(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::<E>(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::<E>(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::<E>(
-                    // 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::<E>(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::<E>(
+                        // 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!(