Fix compilation, clear best_child/best_descendant in migration

- Fix leaf detection in heads_descended_from_finalization (parent()
  method call, map away enumerate index)
- Clear best_child and best_descendant in v28->v29 migration (no
  longer used, replaced by virtual tree walk)
- Migration now rewrites fork choice data instead of being a no-op

beacon_node/beacon_chain/src/schema_change/migration_schema_v29.rs

@@ -2,17 +2,18 @@ use crate::beacon_chain::BeaconChainTypes;
 use crate::persisted_fork_choice::{PersistedForkChoiceV28, PersistedForkChoiceV29};
 use ssz::Decode;
 use store::hot_cold_store::HotColdDB;
-use store::{DBColumn, Error as StoreError, KeyValueStore};
+use store::{DBColumn, Error as StoreError, KeyValueStore, KeyValueStoreOp};
 use types::{EthSpec, Hash256};
 
 /// The key used to store the fork choice in the database.
 const FORK_CHOICE_DB_KEY: Hash256 = Hash256::ZERO;
 
-/// Upgrade from schema v28 to v29 (no-op).
+/// Upgrade from schema v28 to v29.
 ///
-/// Fails if the persisted fork choice contains any V17 (pre-Gloas) proto nodes at or after the
-/// Gloas fork slot. Such nodes indicate the node synced a broken sidechain with Gloas disabled
-/// and would not be able to sync the v29 chain.
+/// - Clears `best_child` and `best_descendant` on all nodes (replaced by
+///   virtual tree walk).
+/// - Fails if the persisted fork choice contains any V17 (pre-Gloas) proto
+///   nodes at or after the Gloas fork slot.
 pub fn upgrade_to_v29<T: BeaconChainTypes>(
     db: &HotColdDB<T::EthSpec, T::HotStore, T::ColdStore>,
 ) -> Result<(), StoreError> {
@@ -21,11 +22,6 @@ pub fn upgrade_to_v29<T: BeaconChainTypes>(
         .gloas_fork_epoch
         .map(|epoch| epoch.start_slot(T::EthSpec::slots_per_epoch()));
 
-    // If Gloas is not configured, the upgrade is a safe no-op.
-    let Some(gloas_fork_slot) = gloas_fork_slot else {
-        return Ok(());
-    };
-
     // Load the persisted fork choice (v28 format, uncompressed SSZ).
     let Some(fc_bytes) = db
         .hot_db
@@ -34,26 +30,45 @@ pub fn upgrade_to_v29<T: BeaconChainTypes>(
         return Ok(());
     };
 
-    let persisted_v28 =
+    let mut persisted_v28 =
         PersistedForkChoiceV28::from_ssz_bytes(&fc_bytes).map_err(StoreError::SszDecodeError)?;
 
-    // In v28 format, all nodes are ProtoNodeV17. Check if any are at/after the Gloas fork slot.
-    let bad_node = persisted_v28
-        .fork_choice_v28
-        .proto_array_v28
-        .nodes
-        .iter()
-        .find(|node| node.slot >= gloas_fork_slot);
+    // Check for V17 nodes at/after the Gloas fork slot.
+    if let Some(gloas_fork_slot) = gloas_fork_slot {
+        let bad_node = persisted_v28
+            .fork_choice_v28
+            .proto_array_v28
+            .nodes
+            .iter()
+            .find(|node| node.slot >= gloas_fork_slot);
 
-    if let Some(node) = bad_node {
-        return Err(StoreError::MigrationError(format!(
-            "cannot upgrade from v28 to v29: found V17 proto node at slot {} (root: {:?}) \
-             which is at or after the Gloas fork slot {}. This node has synced a chain with \
-             Gloas disabled and cannot be upgraded. Please resync from scratch.",
-            node.slot, node.root, gloas_fork_slot,
-        )));
+        if let Some(node) = bad_node {
+            return Err(StoreError::MigrationError(format!(
+                "cannot upgrade from v28 to v29: found V17 proto node at slot {} (root: {:?}) \
+                 which is at or after the Gloas fork slot {}. This node has synced a chain with \
+                 Gloas disabled and cannot be upgraded. Please resync from scratch.",
+                node.slot, node.root, gloas_fork_slot,
+            )));
+        }
     }
 
+    // Clear best_child/best_descendant — replaced by the virtual tree walk.
+    for node in &mut persisted_v28.fork_choice_v28.proto_array_v28.nodes {
+        node.best_child = None;
+        node.best_descendant = None;
+    }
+
+    // Convert to v29 and write back.
+    let persisted_v29 = PersistedForkChoiceV29::from(persisted_v28);
+    let fc_bytes = persisted_v29
+        .as_bytes(db.get_config())
+        .map_err(|e| StoreError::MigrationError(format!("failed to encode v29: {:?}", e)))?;
+    db.hot_db.do_atomically(vec![KeyValueStoreOp::PutKeyValue(
+        DBColumn::ForkChoice,
+        FORK_CHOICE_DB_KEY.as_slice().to_vec(),
+        fc_bytes,
+    )])?;
+
     Ok(())
 }
 

consensus/fork_choice/src/fork_choice.rs

@@ -1008,8 +1008,6 @@ where
                 proposer_index: Some(block.proposer_index()),
             },
             current_slot,
-            self.justified_checkpoint(),
-            self.finalized_checkpoint(),
             spec,
             block_delay,
         )?;

consensus/proto_array/src/fork_choice_test_definition.rs

@@ -283,14 +283,7 @@ impl ForkChoiceTestDefinition {
                         proposer_index: Some(0),
                     };
                     fork_choice
-                        .process_block::<MainnetEthSpec>(
-                            block,
-                            slot,
-                            self.justified_checkpoint,
-                            self.finalized_checkpoint,
-                            &spec,
-                            Duration::ZERO,
-                        )
+                        .process_block::<MainnetEthSpec>(block, slot, &spec, Duration::ZERO)
                         .unwrap_or_else(|e| {
                             panic!(
                                 "process_block op at index {} returned error: {:?}",

consensus/proto_array/src/proto_array.rs

@@ -389,9 +389,6 @@ impl ProtoArray {
     pub fn apply_score_changes<E: EthSpec>(
         &mut self,
         mut deltas: Vec<NodeDelta>,
-        best_justified_checkpoint: Checkpoint,
-        best_finalized_checkpoint: Checkpoint,
-        current_slot: Slot,
     ) -> Result<(), Error> {
         if deltas.len() != self.indices.len() {
             return Err(Error::InvalidDeltaLen {
@@ -518,8 +515,6 @@ impl ProtoArray {
         &mut self,
         block: Block,
         current_slot: Slot,
-        best_justified_checkpoint: Checkpoint,
-        best_finalized_checkpoint: Checkpoint,
         spec: &ChainSpec,
         time_into_slot: Duration,
     ) -> Result<(), Error> {
@@ -677,10 +672,10 @@ impl ProtoArray {
         self.indices.insert(node.root(), node_index);
         self.nodes.push(node.clone());
 
-        if let Some(parent_index) = node.parent() {
-            if matches!(block.execution_status, ExecutionStatus::Valid(_)) {
-                self.propagate_execution_payload_validation_by_index(parent_index)?;
-            }
+        if let Some(parent_index) = node.parent()
+            && matches!(block.execution_status, ExecutionStatus::Valid(_))
+        {
+            self.propagate_execution_payload_validation_by_index(parent_index)?;
         }
 
         Ok(())
@@ -1773,17 +1768,14 @@ impl ProtoArray {
             .enumerate()
             .filter(|(i, node)| {
                 // TODO(gloas): we unoptimized this for Gloas fork choice, could re-optimize.
-                let num_children = self
-                    .nodes
-                    .iter()
-                    .filter(|node| node.parent == Some(i))
-                    .count();
+                let num_children = self.nodes.iter().filter(|n| n.parent() == Some(*i)).count();
                 num_children == 0
                     && self.is_finalized_checkpoint_or_descendant::<E>(
                         node.root(),
                         best_finalized_checkpoint,
                     )
             })
+            .map(|(_, node)| node)
             .collect()
     }
 }

consensus/proto_array/src/proto_array_fork_choice.rs

@@ -512,8 +512,6 @@ impl ProtoArrayForkChoice {
             .on_block::<E>(
                 block,
                 current_slot,
-                justified_checkpoint,
-                finalized_checkpoint,
                 spec,
                 // Anchor block is always timely (delay=0 ensures both timeliness
                 // checks pass). Combined with `is_genesis` override in on_block,
@@ -615,8 +613,6 @@ impl ProtoArrayForkChoice {
         &mut self,
         block: Block,
         current_slot: Slot,
-        justified_checkpoint: Checkpoint,
-        finalized_checkpoint: Checkpoint,
         spec: &ChainSpec,
         time_into_slot: Duration,
     ) -> Result<(), String> {
@@ -625,14 +621,7 @@ impl ProtoArrayForkChoice {
         }
 
         self.proto_array
-            .on_block::<E>(
-                block,
-                current_slot,
-                justified_checkpoint,
-                finalized_checkpoint,
-                spec,
-                time_into_slot,
-            )
+            .on_block::<E>(block, current_slot, spec, time_into_slot)
             .map_err(|e| format!("process_block_error: {:?}", e))
     }
 
@@ -667,12 +656,7 @@ impl ProtoArrayForkChoice {
         .map_err(|e| format!("find_head compute_deltas failed: {:?}", e))?;
 
         self.proto_array
-            .apply_score_changes::<E>(
-                deltas,
-                justified_checkpoint,
-                finalized_checkpoint,
-                current_slot,
-            )
+            .apply_score_changes::<E>(deltas)
             .map_err(|e| format!("find_head apply_score_changes failed: {:?}", e))?;
 
         *old_balances = new_balances.clone();
@@ -1354,8 +1338,6 @@ mod test_compute_deltas {
                     proposer_index: Some(0),
                 },
                 genesis_slot + 1,
-                genesis_checkpoint,
-                genesis_checkpoint,
                 &spec,
                 Duration::ZERO,
             )
@@ -1384,8 +1366,6 @@ mod test_compute_deltas {
                     proposer_index: Some(0),
                 },
                 genesis_slot + 1,
-                genesis_checkpoint,
-                genesis_checkpoint,
                 &spec,
                 Duration::ZERO,
             )
@@ -1521,8 +1501,6 @@ mod test_compute_deltas {
                         proposer_index: Some(0),
                     },
                     Slot::from(block.slot),
-                    genesis_checkpoint,
-                    genesis_checkpoint,
                     &spec,
                     Duration::ZERO,
                 )