From 53e73fa37673c70b5beb572281fdd5d21d4e40cc Mon Sep 17 00:00:00 2001 From: Lion - dapplion <35266934+dapplion@users.noreply.github.com> Date: Wed, 12 Nov 2025 00:42:17 -0300 Subject: [PATCH] Remove duplicate state in ProtoArray (#8324) Part of a fork-choice tech debt clean-up https://github.com/sigp/lighthouse/issues/8325 https://github.com/sigp/lighthouse/issues/7089 (non-finalized checkpoint sync) changes the meaning of the checkpoints inside fork-choice. It turns out that we persist the justified and finalized checkpoints **twice** in fork-choice 1. Inside the fork-choice store 2. Inside the proto-array There's no reason for 2. except for making the function signature of some methods smallers. It's not consistent with the rest of the crate, because in some functions we pass the external variable of time (current_slot) via args, but then read the finalized checkpoint from the internal state. Passing both variables as args makes fork-choice easier to reason about at the cost of a few extra lines. Remove the unnecessary state (`justified_checkpoint`, `finalized_checkpoint`) inside `ProtoArray`, to make it easier to reason about. Co-Authored-By: dapplion <35266934+dapplion@users.noreply.github.com> Co-Authored-By: Michael Sproul --- beacon_node/beacon_chain/src/beacon_chain.rs | 6 +- .../src/schema_change/migration_schema_v23.rs | 2 +- beacon_node/http_api/src/lib.rs | 4 +- beacon_node/http_api/tests/tests.rs | 4 +- consensus/fork_choice/src/fork_choice.rs | 10 +- .../src/fork_choice_test_definition.rs | 15 ++- consensus/proto_array/src/proto_array.rs | 120 +++++++++++++----- .../src/proto_array_fork_choice.rs | 105 +++++++++++---- consensus/proto_array/src/ssz_container.rs | 20 +-- 9 files changed, 204 insertions(+), 82 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 5ffdf951ac..494346e7ff 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1412,10 +1412,10 @@ impl BeaconChain { /// /// 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::() + .heads_descended_from_finalization::(fork_choice.finalized_checkpoint()) .iter() .map(|node| (node.root, node.slot)) .collect() diff --git a/beacon_node/beacon_chain/src/schema_change/migration_schema_v23.rs b/beacon_node/beacon_chain/src/schema_change/migration_schema_v23.rs index e8bd526e19..e238e1efb6 100644 --- a/beacon_node/beacon_chain/src/schema_change/migration_schema_v23.rs +++ b/beacon_node/beacon_chain/src/schema_change/migration_schema_v23.rs @@ -122,7 +122,7 @@ pub fn downgrade_from_v23( let heads = fork_choice .proto_array() - .heads_descended_from_finalization::(); + .heads_descended_from_finalization::(fork_choice.finalized_checkpoint()); let head_roots = heads.iter().map(|node| node.root).collect(); let head_slots = heads.iter().map(|node| node.slot).collect(); diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 9026792b91..e0fb39c42c 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -3037,8 +3037,8 @@ pub fn serve( }) .collect::>(); 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, }) }) diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 6fb5a8ed8a..b3486da5ad 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -3057,11 +3057,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 = expected_proto_array diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index fe1f5fba9e..6565e7cdaf 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -627,7 +627,7 @@ where op: &InvalidationOperation, ) -> Result<(), Error> { self.proto_array - .process_execution_payload_invalidation::(op) + .process_execution_payload_invalidation::(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::(block_root) + .is_finalized_checkpoint_or_descendant::(block_root, self.finalized_checkpoint()) } pub fn is_descendant(&self, ancestor_root: Hash256, descendant_root: Hash256) -> bool { @@ -1508,7 +1510,9 @@ where /// be instantiated again later. pub fn to_persisted(&self) -> PersistedForkChoice { PersistedForkChoice { - proto_array: self.proto_array().as_ssz_container(), + proto_array: self + .proto_array() + .as_ssz_container(self.justified_checkpoint(), self.finalized_checkpoint()), queued_attestations: self.queued_attestations().to_vec(), } } diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index 20987dff26..43a7e3b77f 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -212,7 +212,12 @@ impl ForkChoiceTestDefinition { unrealized_finalized_checkpoint: None, }; fork_choice - .process_block::(block, slot) + .process_block::( + 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::(&op) + .process_execution_payload_invalidation::( + &op, + self.finalized_checkpoint, + ) .unwrap() } Operation::AssertWeight { block_root, weight } => assert_eq!( @@ -305,7 +313,8 @@ fn get_checkpoint(i: u64) -> Checkpoint { } fn check_bytes_round_trip(original: &ProtoArrayForkChoice) { - let bytes = original.as_bytes(); + // The checkpoint are ignored `ProtoArrayForkChoice::from_bytes` so any value is ok + let bytes = original.as_bytes(Checkpoint::default(), Checkpoint::default()); let decoded = ProtoArrayForkChoice::from_bytes(&bytes, original.balances.clone()) .expect("fork choice should decode from bytes"); assert!( diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 18af2dfc24..1d78ce9f44 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/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, pub indices: HashMap, pub previous_proposer_boost: ProposerBoost, @@ -155,8 +153,8 @@ impl ProtoArray { pub fn apply_score_changes( &mut self, mut deltas: Vec, - 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(&mut self, block: Block, current_slot: Slot) -> Result<(), Error> { + pub fn on_block( + &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( &mut self, op: &InvalidationOperation, + best_finalized_checkpoint: Checkpoint, ) -> Result<(), Error> { let mut invalidated_indices: HashSet = <_>::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::(ancestor_root) + && self.is_finalized_checkpoint_or_descendant::( + 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 { 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::(best_node, current_slot) { + if !self.node_is_viable_for_head::( + 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::(child, current_slot)?; + let child_leads_to_viable_head = self.node_leads_to_viable_head::( + 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::(best_child, current_slot)?; + let best_child_leads_to_viable_head = self.node_leads_to_viable_head::( + 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 { 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::(best_descendant, current_slot) + self.node_is_viable_for_head::( + 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::(node, current_slot)) + || self.node_is_viable_for_head::( + 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(&self, node: &ProtoNode, current_slot: Slot) -> bool { + fn node_is_viable_for_head( + &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::(node.root); + let correct_finalized = best_finalized_checkpoint.epoch == genesis_epoch + || self + .is_finalized_checkpoint_or_descendant::(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(&self, root: Hash256) -> bool { - let finalized_root = self.finalized_checkpoint.root; - let finalized_slot = self - .finalized_checkpoint + pub fn is_finalized_checkpoint_or_descendant( + &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(&self) -> Vec<&ProtoNode> { + pub fn heads_descended_from_finalization( + &self, + best_finalized_checkpoint: Checkpoint, + ) -> Vec<&ProtoNode> { self.nodes .iter() .filter(|node| { node.best_child.is_none() - && self.is_finalized_checkpoint_or_descendant::(node.root) + && self.is_finalized_checkpoint_or_descendant::( + node.root, + best_finalized_checkpoint, + ) }) .collect() } diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index dea853d245..137471ce36 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -424,8 +424,6 @@ impl ProtoArrayForkChoice { ) -> Result { let mut proto_array = ProtoArray { prune_threshold: DEFAULT_PRUNE_THRESHOLD, - justified_checkpoint, - finalized_checkpoint, nodes: Vec::with_capacity(1), indices: HashMap::with_capacity(1), previous_proposer_boost: ProposerBoost::default(), @@ -449,7 +447,12 @@ impl ProtoArrayForkChoice { }; proto_array - .on_block::(block, current_slot) + .on_block::( + block, + current_slot, + justified_checkpoint, + finalized_checkpoint, + ) .map_err(|e| format!("Failed to add finalized block to proto_array: {:?}", e))?; Ok(Self { @@ -473,9 +476,10 @@ impl ProtoArrayForkChoice { pub fn process_execution_payload_invalidation( &mut self, op: &InvalidationOperation, + finalized_checkpoint: Checkpoint, ) -> Result<(), String> { self.proto_array - .propagate_execution_payload_invalidation::(op) + .propagate_execution_payload_invalidation::(op, finalized_checkpoint) .map_err(|e| format!("Failed to process invalid payload: {:?}", e)) } @@ -499,13 +503,20 @@ impl ProtoArrayForkChoice { &mut self, block: Block, current_slot: Slot, + justified_checkpoint: Checkpoint, + finalized_checkpoint: Checkpoint, ) -> Result<(), String> { if block.parent_root.is_none() { return Err("Missing parent root".to_string()); } self.proto_array - .on_block::(block, current_slot) + .on_block::( + block, + current_slot, + justified_checkpoint, + finalized_checkpoint, + ) .map_err(|e| format!("process_block_error: {:?}", e)) } @@ -547,7 +558,12 @@ impl ProtoArrayForkChoice { *old_balances = new_balances.clone(); self.proto_array - .find_head::(&justified_checkpoint.root, current_slot) + .find_head::( + &justified_checkpoint.root, + current_slot, + justified_checkpoint, + finalized_checkpoint, + ) .map_err(|e| format!("find_head failed: {:?}", e)) } @@ -884,9 +900,10 @@ impl ProtoArrayForkChoice { pub fn is_finalized_checkpoint_or_descendant( &self, descendant_root: Hash256, + best_finalized_checkpoint: Checkpoint, ) -> bool { self.proto_array - .is_finalized_checkpoint_or_descendant::(descendant_root) + .is_finalized_checkpoint_or_descendant::(descendant_root, best_finalized_checkpoint) } pub fn latest_message(&self, validator_index: usize) -> Option<(Hash256, Epoch)> { @@ -916,12 +933,21 @@ impl ProtoArrayForkChoice { self.proto_array.iter_block_roots(block_root) } - pub fn as_ssz_container(&self) -> SszContainer { - SszContainer::from(self) + pub fn as_ssz_container( + &self, + justified_checkpoint: Checkpoint, + finalized_checkpoint: Checkpoint, + ) -> SszContainer { + SszContainer::from_proto_array(self, justified_checkpoint, finalized_checkpoint) } - pub fn as_bytes(&self) -> Vec { - SszContainer::from(self).as_ssz_bytes() + pub fn as_bytes( + &self, + justified_checkpoint: Checkpoint, + finalized_checkpoint: Checkpoint, + ) -> Vec { + self.as_ssz_container(justified_checkpoint, finalized_checkpoint) + .as_ssz_bytes() } pub fn from_bytes(bytes: &[u8], balances: JustifiedBalances) -> Result { @@ -954,8 +980,12 @@ impl ProtoArrayForkChoice { } /// Returns all nodes that have zero children and are descended from the finalized checkpoint. - pub fn heads_descended_from_finalization(&self) -> Vec<&ProtoNode> { - self.proto_array.heads_descended_from_finalization::() + pub fn heads_descended_from_finalization( + &self, + best_finalized_checkpoint: Checkpoint, + ) -> Vec<&ProtoNode> { + self.proto_array + .heads_descended_from_finalization::(best_finalized_checkpoint) } } @@ -1125,6 +1155,8 @@ mod test_compute_deltas { unrealized_finalized_checkpoint: Some(genesis_checkpoint), }, genesis_slot + 1, + genesis_checkpoint, + genesis_checkpoint, ) .unwrap(); @@ -1148,6 +1180,8 @@ mod test_compute_deltas { unrealized_finalized_checkpoint: None, }, genesis_slot + 1, + genesis_checkpoint, + genesis_checkpoint, ) .unwrap(); @@ -1161,10 +1195,24 @@ mod test_compute_deltas { assert!(!fc.is_descendant(finalized_root, not_finalized_desc)); assert!(!fc.is_descendant(finalized_root, unknown)); - assert!(fc.is_finalized_checkpoint_or_descendant::(finalized_root)); - assert!(fc.is_finalized_checkpoint_or_descendant::(finalized_desc)); - assert!(!fc.is_finalized_checkpoint_or_descendant::(not_finalized_desc)); - assert!(!fc.is_finalized_checkpoint_or_descendant::(unknown)); + assert!(fc.is_finalized_checkpoint_or_descendant::( + finalized_root, + genesis_checkpoint + )); + assert!(fc.is_finalized_checkpoint_or_descendant::( + finalized_desc, + genesis_checkpoint + )); + assert!(!fc.is_finalized_checkpoint_or_descendant::( + not_finalized_desc, + genesis_checkpoint + )); + assert!( + !fc.is_finalized_checkpoint_or_descendant::( + unknown, + genesis_checkpoint + ) + ); assert!(!fc.is_descendant(finalized_desc, not_finalized_desc)); assert!(fc.is_descendant(finalized_desc, finalized_desc)); @@ -1260,6 +1308,8 @@ mod test_compute_deltas { unrealized_finalized_checkpoint: Some(genesis_checkpoint), }, Slot::from(block.slot), + genesis_checkpoint, + genesis_checkpoint, ) .unwrap(); }; @@ -1314,29 +1364,34 @@ mod test_compute_deltas { // Set the finalized checkpoint to finalize the first slot of epoch 1 on // the canonical chain. - fc.proto_array.finalized_checkpoint = Checkpoint { + let finalized_checkpoint = Checkpoint { root: finalized_root, epoch: Epoch::new(1), }; assert!( fc.proto_array - .is_finalized_checkpoint_or_descendant::(finalized_root), + .is_finalized_checkpoint_or_descendant::( + finalized_root, + finalized_checkpoint + ), "the finalized checkpoint is the finalized checkpoint" ); assert!( fc.proto_array - .is_finalized_checkpoint_or_descendant::(get_block_root( - canonical_slot - )), + .is_finalized_checkpoint_or_descendant::( + get_block_root(canonical_slot), + finalized_checkpoint + ), "the canonical block is a descendant of the finalized checkpoint" ); assert!( !fc.proto_array - .is_finalized_checkpoint_or_descendant::(get_block_root( - non_canonical_slot - )), + .is_finalized_checkpoint_or_descendant::( + get_block_root(non_canonical_slot), + finalized_checkpoint + ), "although the non-canonical block is a descendant of the finalized block, \ it's not a descendant of the finalized checkpoint" ); diff --git a/consensus/proto_array/src/ssz_container.rs b/consensus/proto_array/src/ssz_container.rs index 0bb3f2b35d..1e01b74c8c 100644 --- a/consensus/proto_array/src/ssz_container.rs +++ b/consensus/proto_array/src/ssz_container.rs @@ -26,22 +26,28 @@ pub struct SszContainer { #[superstruct(only(V17))] pub balances: Vec, pub prune_threshold: usize, - pub justified_checkpoint: Checkpoint, - pub finalized_checkpoint: Checkpoint, + // Deprecated, remove in a future schema migration + justified_checkpoint: Checkpoint, + // Deprecated, remove in a future schema migration + finalized_checkpoint: Checkpoint, pub nodes: Vec, pub indices: Vec<(Hash256, usize)>, pub previous_proposer_boost: ProposerBoost, } -impl From<&ProtoArrayForkChoice> for SszContainer { - fn from(from: &ProtoArrayForkChoice) -> Self { +impl SszContainer { + pub fn from_proto_array( + from: &ProtoArrayForkChoice, + justified_checkpoint: Checkpoint, + finalized_checkpoint: Checkpoint, + ) -> Self { let proto_array = &from.proto_array; Self { votes: from.votes.0.clone(), prune_threshold: proto_array.prune_threshold, - justified_checkpoint: proto_array.justified_checkpoint, - finalized_checkpoint: proto_array.finalized_checkpoint, + justified_checkpoint, + finalized_checkpoint, nodes: proto_array.nodes.clone(), indices: proto_array.indices.iter().map(|(k, v)| (*k, *v)).collect(), previous_proposer_boost: proto_array.previous_proposer_boost, @@ -55,8 +61,6 @@ impl TryFrom<(SszContainer, JustifiedBalances)> for ProtoArrayForkChoice { fn try_from((from, balances): (SszContainer, JustifiedBalances)) -> Result { let proto_array = ProtoArray { prune_threshold: from.prune_threshold, - justified_checkpoint: from.justified_checkpoint, - finalized_checkpoint: from.finalized_checkpoint, nodes: from.nodes, indices: from.indices.into_iter().collect::>(), previous_proposer_boost: from.previous_proposer_boost,