Remove duplicate state in ProtoArray

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -1406,10 +1406,10 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     ///
     /// Returns `(block_root, block_slot)`.
     pub fn heads(&self) -> Vec<(Hash256, Slot)> {
-        self.canonical_head
-            .fork_choice_read_lock()
+        let fork_choice = self.canonical_head.fork_choice_read_lock();
+        fork_choice
             .proto_array()
-            .heads_descended_from_finalization::<T::EthSpec>()
+            .heads_descended_from_finalization::<T::EthSpec>(fork_choice.finalized_checkpoint())
             .iter()
             .map(|node| (node.root, node.slot))
             .collect()

beacon_node/beacon_chain/src/schema_change/migration_schema_v23.rs

@@ -122,7 +122,7 @@ pub fn downgrade_from_v23<T: BeaconChainTypes>(
 
     let heads = fork_choice
         .proto_array()
-        .heads_descended_from_finalization::<T::EthSpec>();
+        .heads_descended_from_finalization::<T::EthSpec>(fork_choice.finalized_checkpoint());
 
     let head_roots = heads.iter().map(|node| node.root).collect();
     let head_slots = heads.iter().map(|node| node.slot).collect();

beacon_node/http_api/src/lib.rs

@@ -3037,8 +3037,8 @@ pub fn serve<T: BeaconChainTypes>(
                         })
                         .collect::<Vec<_>>();
                     Ok(ForkChoice {
-                        justified_checkpoint: proto_array.justified_checkpoint,
-                        finalized_checkpoint: proto_array.finalized_checkpoint,
+                        justified_checkpoint: beacon_fork_choice.justified_checkpoint(),
+                        finalized_checkpoint: beacon_fork_choice.finalized_checkpoint(),
                         fork_choice_nodes,
                     })
                 })

beacon_node/http_api/tests/tests.rs

@@ -3040,11 +3040,11 @@ impl ApiTester {
 
         assert_eq!(
             result.justified_checkpoint,
-            expected_proto_array.justified_checkpoint
+            beacon_fork_choice.justified_checkpoint()
         );
         assert_eq!(
             result.finalized_checkpoint,
-            expected_proto_array.finalized_checkpoint
+            beacon_fork_choice.finalized_checkpoint()
         );
 
         let expected_fork_choice_nodes: Vec<ForkChoiceNode> = expected_proto_array

consensus/fork_choice/src/fork_choice.rs

@@ -627,7 +627,7 @@ where
         op: &InvalidationOperation,
     ) -> Result<(), Error<T::Error>> {
         self.proto_array
-            .process_execution_payload_invalidation::<E>(op)
+            .process_execution_payload_invalidation::<E>(op, self.finalized_checkpoint())
             .map_err(Error::FailedToProcessInvalidExecutionPayload)
     }
 
@@ -908,6 +908,8 @@ where
                 unrealized_finalized_checkpoint: Some(unrealized_finalized_checkpoint),
             },
             current_slot,
+            self.justified_checkpoint(),
+            self.finalized_checkpoint(),
         )?;
 
         Ok(())
@@ -1288,7 +1290,7 @@ where
     /// Return `true` if `block_root` is equal to the finalized checkpoint, or a known descendant of it.
     pub fn is_finalized_checkpoint_or_descendant(&self, block_root: Hash256) -> bool {
         self.proto_array
-            .is_finalized_checkpoint_or_descendant::<E>(block_root)
+            .is_finalized_checkpoint_or_descendant::<E>(block_root, self.finalized_checkpoint())
     }
 
     pub fn is_descendant(&self, ancestor_root: Hash256, descendant_root: Hash256) -> bool {

consensus/proto_array/src/fork_choice_test_definition.rs

@@ -212,7 +212,12 @@ impl ForkChoiceTestDefinition {
                         unrealized_finalized_checkpoint: None,
                     };
                     fork_choice
-                        .process_block::<MainnetEthSpec>(block, slot)
+                        .process_block::<MainnetEthSpec>(
+                            block,
+                            slot,
+                            self.justified_checkpoint,
+                            self.finalized_checkpoint,
+                        )
                         .unwrap_or_else(|e| {
                             panic!(
                                 "process_block op at index {} returned error: {:?}",
@@ -272,7 +277,10 @@ impl ForkChoiceTestDefinition {
                         }
                     };
                     fork_choice
-                        .process_execution_payload_invalidation::<MainnetEthSpec>(&op)
+                        .process_execution_payload_invalidation::<MainnetEthSpec>(
+                            &op,
+                            self.finalized_checkpoint,
+                        )
                         .unwrap()
                 }
                 Operation::AssertWeight { block_root, weight } => assert_eq!(

consensus/proto_array/src/proto_array.rs

@@ -130,8 +130,6 @@ pub struct ProtoArray {
     /// Do not attempt to prune the tree unless it has at least this many nodes. Small prunes
     /// simply waste time.
     pub prune_threshold: usize,
-    pub justified_checkpoint: Checkpoint,
-    pub finalized_checkpoint: Checkpoint,
     pub nodes: Vec<ProtoNode>,
     pub indices: HashMap<Hash256, usize>,
     pub previous_proposer_boost: ProposerBoost,
@@ -155,8 +153,8 @@ impl ProtoArray {
     pub fn apply_score_changes<E: EthSpec>(
         &mut self,
         mut deltas: Vec<i64>,
-        justified_checkpoint: Checkpoint,
-        finalized_checkpoint: Checkpoint,
+        best_justified_checkpoint: Checkpoint,
+        best_finalized_checkpoint: Checkpoint,
         new_justified_balances: &JustifiedBalances,
         proposer_boost_root: Hash256,
         current_slot: Slot,
@@ -169,13 +167,6 @@ impl ProtoArray {
             });
         }
 
-        if justified_checkpoint != self.justified_checkpoint
-            || finalized_checkpoint != self.finalized_checkpoint
-        {
-            self.justified_checkpoint = justified_checkpoint;
-            self.finalized_checkpoint = finalized_checkpoint;
-        }
-
         // Default the proposer boost score to zero.
         let mut proposer_score = 0;
 
@@ -296,6 +287,8 @@ impl ProtoArray {
                     parent_index,
                     node_index,
                     current_slot,
+                    best_justified_checkpoint,
+                    best_finalized_checkpoint,
                 )?;
             }
         }
@@ -306,7 +299,13 @@ impl ProtoArray {
     /// Register a block with the fork choice.
     ///
     /// It is only sane to supply a `None` parent for the genesis block.
-    pub fn on_block<E: EthSpec>(&mut self, block: Block, current_slot: Slot) -> Result<(), Error> {
+    pub fn on_block<E: EthSpec>(
+        &mut self,
+        block: Block,
+        current_slot: Slot,
+        best_justified_checkpoint: Checkpoint,
+        best_finalized_checkpoint: Checkpoint,
+    ) -> Result<(), Error> {
         // If the block is already known, simply ignore it.
         if self.indices.contains_key(&block.root) {
             return Ok(());
@@ -357,6 +356,8 @@ impl ProtoArray {
                 parent_index,
                 node_index,
                 current_slot,
+                best_justified_checkpoint,
+                best_finalized_checkpoint,
             )?;
 
             if matches!(block.execution_status, ExecutionStatus::Valid(_)) {
@@ -439,6 +440,7 @@ impl ProtoArray {
     pub fn propagate_execution_payload_invalidation<E: EthSpec>(
         &mut self,
         op: &InvalidationOperation,
+        best_finalized_checkpoint: Checkpoint,
     ) -> Result<(), Error> {
         let mut invalidated_indices: HashSet<usize> = <_>::default();
         let head_block_root = op.block_root();
@@ -467,7 +469,10 @@ impl ProtoArray {
         let latest_valid_ancestor_is_descendant =
             latest_valid_ancestor_root.is_some_and(|ancestor_root| {
                 self.is_descendant(ancestor_root, head_block_root)
-                    && self.is_finalized_checkpoint_or_descendant::<E>(ancestor_root)
+                    && self.is_finalized_checkpoint_or_descendant::<E>(
+                        ancestor_root,
+                        best_finalized_checkpoint,
+                    )
             });
 
         // Collect all *ancestors* which were declared invalid since they reside between the
@@ -630,6 +635,8 @@ impl ProtoArray {
         &self,
         justified_root: &Hash256,
         current_slot: Slot,
+        best_justified_checkpoint: Checkpoint,
+        best_finalized_checkpoint: Checkpoint,
     ) -> Result<Hash256, Error> {
         let justified_index = self
             .indices
@@ -663,12 +670,17 @@ impl ProtoArray {
             .ok_or(Error::InvalidBestDescendant(best_descendant_index))?;
 
         // Perform a sanity check that the node is indeed valid to be the head.
-        if !self.node_is_viable_for_head::<E>(best_node, current_slot) {
+        if !self.node_is_viable_for_head::<E>(
+            best_node,
+            current_slot,
+            best_justified_checkpoint,
+            best_finalized_checkpoint,
+        ) {
             return Err(Error::InvalidBestNode(Box::new(InvalidBestNodeInfo {
                 current_slot,
                 start_root: *justified_root,
-                justified_checkpoint: self.justified_checkpoint,
-                finalized_checkpoint: self.finalized_checkpoint,
+                justified_checkpoint: best_justified_checkpoint,
+                finalized_checkpoint: best_finalized_checkpoint,
                 head_root: best_node.root,
                 head_justified_checkpoint: best_node.justified_checkpoint,
                 head_finalized_checkpoint: best_node.finalized_checkpoint,
@@ -765,6 +777,8 @@ impl ProtoArray {
         parent_index: usize,
         child_index: usize,
         current_slot: Slot,
+        best_justified_checkpoint: Checkpoint,
+        best_finalized_checkpoint: Checkpoint,
     ) -> Result<(), Error> {
         let child = self
             .nodes
@@ -776,8 +790,12 @@ impl ProtoArray {
             .get(parent_index)
             .ok_or(Error::InvalidNodeIndex(parent_index))?;
 
-        let child_leads_to_viable_head =
-            self.node_leads_to_viable_head::<E>(child, current_slot)?;
+        let child_leads_to_viable_head = self.node_leads_to_viable_head::<E>(
+            child,
+            current_slot,
+            best_justified_checkpoint,
+            best_finalized_checkpoint,
+        )?;
 
         // These three variables are aliases to the three options that we may set the
         // `parent.best_child` and `parent.best_descendant` to.
@@ -806,8 +824,12 @@ impl ProtoArray {
                         .get(best_child_index)
                         .ok_or(Error::InvalidBestDescendant(best_child_index))?;
 
-                    let best_child_leads_to_viable_head =
-                        self.node_leads_to_viable_head::<E>(best_child, current_slot)?;
+                    let best_child_leads_to_viable_head = self.node_leads_to_viable_head::<E>(
+                        best_child,
+                        current_slot,
+                        best_justified_checkpoint,
+                        best_finalized_checkpoint,
+                    )?;
 
                     if child_leads_to_viable_head && !best_child_leads_to_viable_head {
                         // The child leads to a viable head, but the current best-child doesn't.
@@ -856,6 +878,8 @@ impl ProtoArray {
         &self,
         node: &ProtoNode,
         current_slot: Slot,
+        best_justified_checkpoint: Checkpoint,
+        best_finalized_checkpoint: Checkpoint,
     ) -> Result<bool, Error> {
         let best_descendant_is_viable_for_head =
             if let Some(best_descendant_index) = node.best_descendant {
@@ -864,13 +888,23 @@ impl ProtoArray {
                     .get(best_descendant_index)
                     .ok_or(Error::InvalidBestDescendant(best_descendant_index))?;
 
-                self.node_is_viable_for_head::<E>(best_descendant, current_slot)
+                self.node_is_viable_for_head::<E>(
+                    best_descendant,
+                    current_slot,
+                    best_justified_checkpoint,
+                    best_finalized_checkpoint,
+                )
             } else {
                 false
             };
 
         Ok(best_descendant_is_viable_for_head
-            || self.node_is_viable_for_head::<E>(node, current_slot))
+            || self.node_is_viable_for_head::<E>(
+                node,
+                current_slot,
+                best_justified_checkpoint,
+                best_finalized_checkpoint,
+            ))
     }
 
     /// This is the equivalent to the `filter_block_tree` function in the eth2 spec:
@@ -879,7 +913,13 @@ impl ProtoArray {
     ///
     /// Any node that has a different finalized or justified epoch should not be viable for the
     /// head.
-    fn node_is_viable_for_head<E: EthSpec>(&self, node: &ProtoNode, current_slot: Slot) -> bool {
+    fn node_is_viable_for_head<E: EthSpec>(
+        &self,
+        node: &ProtoNode,
+        current_slot: Slot,
+        best_justified_checkpoint: Checkpoint,
+        best_finalized_checkpoint: Checkpoint,
+    ) -> bool {
         if node.execution_status.is_invalid() {
             return false;
         }
@@ -901,12 +941,13 @@ impl ProtoArray {
             node_justified_checkpoint
         };
 
-        let correct_justified = self.justified_checkpoint.epoch == genesis_epoch
-            || voting_source.epoch == self.justified_checkpoint.epoch
+        let correct_justified = best_justified_checkpoint.epoch == genesis_epoch
+            || voting_source.epoch == best_justified_checkpoint.epoch
             || voting_source.epoch + 2 >= current_epoch;
 
-        let correct_finalized = self.finalized_checkpoint.epoch == genesis_epoch
-            || self.is_finalized_checkpoint_or_descendant::<E>(node.root);
+        let correct_finalized = best_finalized_checkpoint.epoch == genesis_epoch
+            || self
+                .is_finalized_checkpoint_or_descendant::<E>(node.root, best_finalized_checkpoint);
 
         correct_justified && correct_finalized
     }
@@ -961,10 +1002,13 @@ impl ProtoArray {
     ///
     /// Notably, this function is checking ancestory of the finalized
     /// *checkpoint* not the finalized *block*.
-    pub fn is_finalized_checkpoint_or_descendant<E: EthSpec>(&self, root: Hash256) -> bool {
-        let finalized_root = self.finalized_checkpoint.root;
-        let finalized_slot = self
-            .finalized_checkpoint
+    pub fn is_finalized_checkpoint_or_descendant<E: EthSpec>(
+        &self,
+        root: Hash256,
+        best_finalized_checkpoint: Checkpoint,
+    ) -> bool {
+        let finalized_root = best_finalized_checkpoint.root;
+        let finalized_slot = best_finalized_checkpoint
             .epoch
             .start_slot(E::slots_per_epoch());
 
@@ -987,7 +1031,7 @@ impl ProtoArray {
         // If the conditions don't match for this node then they're unlikely to
         // start matching for its ancestors.
         for checkpoint in &[node.finalized_checkpoint, node.justified_checkpoint] {
-            if checkpoint == &self.finalized_checkpoint {
+            if checkpoint == &best_finalized_checkpoint {
                 return true;
             }
         }
@@ -996,7 +1040,7 @@ impl ProtoArray {
             node.unrealized_finalized_checkpoint,
             node.unrealized_justified_checkpoint,
         ] {
-            if checkpoint.is_some_and(|cp| cp == self.finalized_checkpoint) {
+            if checkpoint.is_some_and(|cp| cp == best_finalized_checkpoint) {
                 return true;
             }
         }
@@ -1044,12 +1088,18 @@ impl ProtoArray {
     /// 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> {
+    pub fn heads_descended_from_finalization<E: EthSpec>(
+        &self,
+        best_finalized_checkpoint: Checkpoint,
+    ) -> Vec<&ProtoNode> {
         self.nodes
             .iter()
             .filter(|node| {
                 node.best_child.is_none()
-                    && self.is_finalized_checkpoint_or_descendant::<E>(node.root)
+                    && self.is_finalized_checkpoint_or_descendant::<E>(
+                        node.root,
+                        best_finalized_checkpoint,
+                    )
             })
             .collect()
     }