Remove proposer boost weight during upgrade

beacon_node/beacon_chain/src/schema_change/migration_schema_v29.rs

@@ -1,7 +1,9 @@
 use crate::beacon_chain::{BeaconChainTypes, FORK_CHOICE_DB_KEY};
 use crate::persisted_fork_choice::{PersistedForkChoiceV28, PersistedForkChoiceV29};
+use std::collections::HashMap;
 use store::hot_cold_store::HotColdDB;
 use store::{DBColumn, Error as StoreError, KeyValueStore, KeyValueStoreOp};
+use tracing::warn;
 use types::EthSpec;
 
 /// Upgrade from schema v28 to v29.
@@ -49,8 +51,49 @@ pub fn upgrade_to_v29<T: BeaconChainTypes>(
         }
     }
 
-    // Convert to v29 and encode.
+    // Read the previous proposer boost before converting to V29 (V29 no longer stores it).
-    let persisted_v29 = PersistedForkChoiceV29::from(persisted_v28);
+    let previous_proposer_boost = persisted_v28
+        .fork_choice_v28
+        .proto_array_v28
+        .previous_proposer_boost;
+
+    // Convert to v29.
+    let mut persisted_v29 = PersistedForkChoiceV29::from(persisted_v28);
+
+    // Subtract the proposer boost from the boosted node and all its ancestors.
+    //
+    // In the V28 schema, `apply_score_changes` baked the proposer boost directly into node
+    // weights and back-propagated it up the parent chain. In V29, the boost is computed
+    // on-the-fly during the virtual tree walk. If we don't subtract the baked-in boost here,
+    // it will be double-counted after the upgrade.
+    if !previous_proposer_boost.root.is_zero() && previous_proposer_boost.score > 0 {
+        let score = previous_proposer_boost.score;
+        let indices: HashMap<_, _> = persisted_v29
+            .fork_choice
+            .proto_array
+            .indices
+            .iter()
+            .cloned()
+            .collect();
+
+        if let Some(node_index) = indices.get(&previous_proposer_boost.root).copied() {
+            let nodes = &mut persisted_v29.fork_choice.proto_array.nodes;
+            let mut current = Some(node_index);
+            while let Some(idx) = current {
+                if let Some(node) = nodes.get_mut(idx) {
+                    *node.weight_mut() = node.weight().saturating_sub(score);
+                    current = node.parent();
+                } else {
+                    break;
+                }
+            }
+        } else {
+            warn!(
+                root = ?previous_proposer_boost.root,
+                "Proposer boost node missing from fork choice"
+            );
+        }
+    }
 
     Ok(vec![
         persisted_v29.as_kv_store_op(FORK_CHOICE_DB_KEY, db.get_config())?,

beacon_node/beacon_chain/tests/payload_invalidation.rs

@@ -1438,7 +1438,7 @@ async fn weights_after_resetting_optimistic_status() {
         .canonical_head
         .fork_choice_write_lock()
         .proto_array_mut()
-        .set_all_blocks_to_optimistic::<E>(&rig.harness.chain.spec)
+        .set_all_blocks_to_optimistic::<E>()
         .unwrap();
 
     let new_weights = rig

consensus/fork_choice/src/fork_choice.rs

@@ -1614,7 +1614,6 @@ where
         persisted_proto_array: proto_array::core::SszContainer,
         justified_balances: JustifiedBalances,
         reset_payload_statuses: ResetPayloadStatuses,
-        spec: &ChainSpec,
     ) -> Result<ProtoArrayForkChoice, Error<T::Error>> {
         let mut proto_array = ProtoArrayForkChoice::from_container(
             persisted_proto_array.clone(),
@@ -1639,7 +1638,7 @@ where
 
         // Reset all blocks back to being "optimistic". This helps recover from an EL consensus
         // fault where an invalid payload becomes valid.
-        if let Err(e) = proto_array.set_all_blocks_to_optimistic::<E>(spec) {
+        if let Err(e) = proto_array.set_all_blocks_to_optimistic::<E>() {
             // If there is an error resetting the optimistic status then log loudly and revert
             // back to a proto-array which does not have the reset applied. This indicates a
             // significant error in Lighthouse and warrants detailed investigation.
@@ -1669,7 +1668,6 @@ where
             persisted.proto_array,
             justified_balances,
             reset_payload_statuses,
-            spec,
         )?;
 
         let current_slot = fc_store.get_current_slot();
@@ -1703,7 +1701,7 @@ where
             // get a different result.
             fork_choice
                 .proto_array
-                .set_all_blocks_to_optimistic::<E>(spec)?;
+                .set_all_blocks_to_optimistic::<E>()?;
             // If the second attempt at finding a head fails, return an error since we do not
             // expect this scenario.
             let _ = fork_choice.get_head(current_slot, spec)?;

consensus/proto_array/src/proto_array.rs

@@ -369,7 +369,6 @@ pub struct ProtoArray {
     pub prune_threshold: usize,
     pub nodes: Vec<ProtoNode>,
     pub indices: HashMap<Hash256, usize>,
-    pub previous_proposer_boost: ProposerBoost,
 }
 
 impl ProtoArray {
@@ -502,10 +501,6 @@ impl ProtoArray {
             }
         }
 
-        // Proposer boost is now applied on-the-fly in `get_weight` during the
-        // walk, so clear any stale boost from a prior call.
-        self.previous_proposer_boost = ProposerBoost::default();
-
         Ok(())
     }
 

consensus/proto_array/src/proto_array_fork_choice.rs

@@ -2,8 +2,7 @@ use crate::{
     JustifiedBalances,
     error::Error,
     proto_array::{
-        InvalidationOperation, Iter, NodeDelta, ProposerBoost, ProtoArray, ProtoNode,
+        InvalidationOperation, Iter, NodeDelta, ProtoArray, ProtoNode, calculate_committee_fraction,
-        calculate_committee_fraction,
     },
     ssz_container::SszContainer,
 };
@@ -527,7 +526,6 @@ impl ProtoArrayForkChoice {
             prune_threshold: DEFAULT_PRUNE_THRESHOLD,
             nodes: Vec::with_capacity(1),
             indices: HashMap::with_capacity(1),
-            previous_proposer_boost: ProposerBoost::default(),
         };
 
         let block = Block {
@@ -880,10 +878,7 @@ impl ProtoArrayForkChoice {
     /// status to be optimistic.
     ///
     /// In practice this means forgetting any `VALID` or `INVALID` statuses.
-    pub fn set_all_blocks_to_optimistic<E: EthSpec>(
+    pub fn set_all_blocks_to_optimistic<E: EthSpec>(&mut self) -> Result<(), String> {
-        &mut self,
-        spec: &ChainSpec,
-    ) -> Result<(), String> {
         // Iterate backwards through all nodes in the `proto_array`. Whilst it's not strictly
         // required to do this process in reverse, it seems natural when we consider how LMD votes
         // are counted.
@@ -906,7 +901,7 @@ impl ProtoArrayForkChoice {
 
                     // Restore the weight of the node, it would have been set to `0` in
                     // `apply_score_changes` when it was invalidated.
-                    let mut restored_weight: u64 = self
+                    let restored_weight: u64 = self
                         .votes
                         .0
                         .iter()
@@ -922,26 +917,6 @@ impl ProtoArrayForkChoice {
                         })
                         .sum();
 
-                    // If the invalid root was boosted, apply the weight to it and
-                    // ancestors.
-                    if let Some(proposer_score_boost) = spec.proposer_score_boost
-                        && self.proto_array.previous_proposer_boost.root == node.root()
-                    {
-                        // Compute the score based upon the current balances. We can't rely on
-                        // the `previous_proposr_boost.score` since it is set to zero with an
-                        // invalid node.
-                        let proposer_score =
-                            calculate_committee_fraction::<E>(&self.balances, proposer_score_boost)
-                                .ok_or("Failed to compute proposer boost")?;
-                        // Store the score we've applied here so it can be removed in
-                        // a later call to `apply_score_changes`.
-                        self.proto_array.previous_proposer_boost.score = proposer_score;
-                        // Apply this boost to this node.
-                        restored_weight = restored_weight
-                            .checked_add(proposer_score)
-                            .ok_or("Overflow when adding boost to weight")?;
-                    }
-
                     // Add the restored weight to the node and all ancestors.
                     if restored_weight > 0 {
                         let mut node_or_ancestor = node;

consensus/proto_array/src/ssz_container.rs

@@ -38,6 +38,7 @@ pub struct SszContainer {
     #[superstruct(only(V29))]
     pub nodes: Vec<ProtoNode>,
     pub indices: Vec<(Hash256, usize)>,
+    #[superstruct(only(V28))]
     pub previous_proposer_boost: ProposerBoost,
 }
 
@@ -50,7 +51,6 @@ impl SszContainerV29 {
             prune_threshold: proto_array.prune_threshold,
             nodes: proto_array.nodes.clone(),
             indices: proto_array.indices.iter().map(|(k, v)| (*k, *v)).collect(),
-            previous_proposer_boost: proto_array.previous_proposer_boost,
         }
     }
 }
@@ -63,7 +63,6 @@ impl TryFrom<(SszContainerV29, JustifiedBalances)> for ProtoArrayForkChoice {
             prune_threshold: from.prune_threshold,
             nodes: from.nodes,
             indices: from.indices.into_iter().collect::<HashMap<_, _>>(),
-            previous_proposer_boost: from.previous_proposer_boost,
         };
 
         Ok(Self {
@@ -92,7 +91,6 @@ impl From<SszContainerV28> for SszContainerV29 {
                 })
                 .collect(),
             indices: v28.indices,
-            previous_proposer_boost: v28.previous_proposer_boost,
         }
     }
 }
@@ -116,7 +114,8 @@ impl From<SszContainerV29> for SszContainerV28 {
                 })
                 .collect(),
             indices: v29.indices,
-            previous_proposer_boost: v29.previous_proposer_boost,
+            // Proposer boost is not tracked in V29 (computed on-the-fly), so reset it.
+            previous_proposer_boost: ProposerBoost::default(),
         }
     }
 }