Use descent from finality instead of viability

Author: michaelsproul

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -1410,13 +1410,10 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     ///
     /// Returns `(block_root, block_slot)`.
     pub fn heads(&self) -> Vec<(Hash256, Slot)> {
-        // Fetching the current slot is only likely to fail pre-genesis. Default to 0 in this case
-        // rather than erroring, so that the heads HTTP API endpoint can work pre-genesis.
-        let current_slot = self.slot().unwrap_or_default();
         self.canonical_head
             .fork_choice_read_lock()
             .proto_array()
-            .viable_heads::<T::EthSpec>(current_slot)
+            .heads_descended_from_finalization::<T::EthSpec>()
             .iter()
             .map(|node| (node.root, node.slot))
             .collect()

beacon_node/beacon_chain/src/schema_change/migration_schema_v23.rs

@@ -64,7 +64,7 @@ pub fn downgrade_from_v23<T: BeaconChainTypes>(
 
     // TODO: what value to choose here?
     let reset_payload_statuses = ResetPayloadStatuses::OnlyWithInvalidPayload;
-    let mut fork_choice = ForkChoice::from_persisted(
+    let fork_choice = ForkChoice::from_persisted(
         persisted_fork_choice.fork_choice,
         reset_payload_statuses,
         fc_store,
@@ -75,21 +75,9 @@ pub fn downgrade_from_v23<T: BeaconChainTypes>(
         Error::MigrationError(format!("Error loading fork choice from persisted: {e:?}"))
     })?;
 
-    // TODO: initialize clock
-    let current_slot = Slot::new(0);
-    let head_block_root = fork_choice
-        .get_head(current_slot, &db.spec)
-        .map_err(|e| Error::MigrationError(format!("Error computing get_head: {e:?}")))?;
-
-    let head_proto_block = fork_choice
-        .get_block(&head_block_root)
-        .ok_or(Error::MigrationError(format!(
-            "HeadBlockMissingFromForkChoice({head_block_root:?})"
-        )))?;
-
     let heads = fork_choice
         .proto_array()
-        .viable_heads::<T::EthSpec>(head_proto_block.slot);
+        .heads_descended_from_finalization::<T::EthSpec>();
 
     let head_roots = heads.iter().map(|node| node.root).collect();
     let head_slots = heads.iter().map(|node| node.slot).collect();

consensus/proto_array/src/proto_array.rs

@@ -1042,28 +1042,18 @@ impl ProtoArray {
             .map(|node| node.root)
     }
 
-    /// Returns all nodes that have zero children and are viable heads
-    pub fn viable_heads<E: EthSpec>(&self, current_slot: Slot) -> Vec<&ProtoNode> {
-        self.nodes_without_children()
-            .into_iter()
-            .filter_map(|index| {
-                // An unknown index is not a viable head
-                if let Some(node) = self.nodes.get(index) {
-                    if self.node_is_viable_for_head::<E>(node, current_slot) {
-                        return Some(node);
-                    }
-                }
-                None
-            })
-            .collect()
-    }
-
-    /// Returns all node indices that have zero children. May include unviable nodes.
-    fn nodes_without_children(&self) -> Vec<usize> {
+    /// Returns all nodes that have zero children and are descended from the finalized checkpoint.
+    ///
+    /// For informational purposes like the beacon HTTP API, we use this as the list of known heads,
+    /// even though some of them might not be viable. We do this to maintain consistency between the
+    /// definition of "head" used by pruning (which does not consider viability) and fork choice.
+    pub fn heads_descended_from_finalization<E: EthSpec>(&self) -> Vec<&ProtoNode> {
         self.nodes
             .iter()
-            .enumerate()
-            .filter_map(|(i, node)| node.best_child.is_none().then_some(i))
+            .filter(|node| {
+                node.best_child.is_none()
+                    && self.is_finalized_checkpoint_or_descendant::<E>(node.root)
+            })
             .collect()
     }
 }

consensus/proto_array/src/proto_array_fork_choice.rs

@@ -886,9 +886,9 @@ impl ProtoArrayForkChoice {
         &mut self.proto_array
     }
 
-    /// Returns all nodes that have zero children and are viable heads
-    pub fn viable_heads<E: EthSpec>(&self, current_slot: Slot) -> Vec<&ProtoNode> {
-        self.proto_array.viable_heads::<E>(current_slot)
+    /// Returns all nodes that have zero children and are descended from the finalized checkpoint.
+    pub fn heads_descended_from_finalization<E: EthSpec>(&self) -> Vec<&ProtoNode> {
+        self.proto_array.heads_descended_from_finalization::<E>()
     }
 }