From 5353710e0ae2fea78559daac76903b1e54166658 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Tue, 31 Mar 2026 00:18:24 -0500 Subject: [PATCH] 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 --- .../src/schema_change/migration_schema_v29.rs | 65 ++++++++++++------- consensus/fork_choice/src/fork_choice.rs | 2 - .../src/fork_choice_test_definition.rs | 9 +-- consensus/proto_array/src/proto_array.rs | 20 ++---- .../src/proto_array_fork_choice.rs | 26 +------- 5 files changed, 49 insertions(+), 73 deletions(-) diff --git a/beacon_node/beacon_chain/src/schema_change/migration_schema_v29.rs b/beacon_node/beacon_chain/src/schema_change/migration_schema_v29.rs index c88a1cada1..6c82e8a737 100644 --- a/beacon_node/beacon_chain/src/schema_change/migration_schema_v29.rs +++ b/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( db: &HotColdDB, ) -> Result<(), StoreError> { @@ -21,11 +22,6 @@ pub fn upgrade_to_v29( .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( 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(()) } diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 1f25afee8e..3b13cd4429 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/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, )?; diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index b6ccc4d435..34d7f2e48e 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -283,14 +283,7 @@ impl ForkChoiceTestDefinition { proposer_index: Some(0), }; fork_choice - .process_block::( - block, - slot, - self.justified_checkpoint, - self.finalized_checkpoint, - &spec, - Duration::ZERO, - ) + .process_block::(block, slot, &spec, Duration::ZERO) .unwrap_or_else(|e| { panic!( "process_block op at index {} returned error: {:?}", diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 456bbc5fda..f68d3eb71b 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -389,9 +389,6 @@ impl ProtoArray { pub fn apply_score_changes( &mut self, mut deltas: Vec, - 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::( node.root(), best_finalized_checkpoint, ) }) + .map(|(_, node)| node) .collect() } } diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 18d593f0e6..6c90af1302 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -512,8 +512,6 @@ impl ProtoArrayForkChoice { .on_block::( 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::( - block, - current_slot, - justified_checkpoint, - finalized_checkpoint, - spec, - time_into_slot, - ) + .on_block::(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::( - deltas, - justified_checkpoint, - finalized_checkpoint, - current_slot, - ) + .apply_score_changes::(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, )