Store execution block hash in fork choice (#2643)

* - Update the fork choice `ProtoNode` to include `is_merge_complete`
- Add database migration for the persisted fork choice

* update tests

* Small cleanup

* lints

* store execution block hash in fork choice rather than bool

Author: realbigsean

beacon_node/beacon_chain/src/schema_change.rs

@@ -1,7 +1,9 @@
 //! Utilities for managing database schema changes.
-use crate::beacon_chain::{BeaconChainTypes, OP_POOL_DB_KEY};
+use crate::beacon_chain::{BeaconChainTypes, FORK_CHOICE_DB_KEY, OP_POOL_DB_KEY};
+use crate::persisted_fork_choice::PersistedForkChoice;
 use crate::validator_pubkey_cache::ValidatorPubkeyCache;
 use operation_pool::{PersistedOperationPool, PersistedOperationPoolBase};
+use proto_array::ProtoArrayForkChoice;
 use ssz::{Decode, Encode};
 use ssz_derive::{Decode, Encode};
 use std::fs;
@@ -93,6 +95,28 @@ pub fn migrate_schema<T: BeaconChainTypes>(
 
             Ok(())
         }
+        // Migration for adding `is_merge_complete` field to the fork choice store.
+        (SchemaVersion(5), SchemaVersion(6)) => {
+            let fork_choice_opt = db
+                .get_item::<PersistedForkChoice>(&FORK_CHOICE_DB_KEY)?
+                .map(|mut persisted_fork_choice| {
+                    let fork_choice = ProtoArrayForkChoice::from_bytes_legacy(
+                        &persisted_fork_choice.fork_choice.proto_array_bytes,
+                    )?;
+                    persisted_fork_choice.fork_choice.proto_array_bytes = fork_choice.as_bytes();
+                    Ok::<_, String>(persisted_fork_choice)
+                })
+                .transpose()
+                .map_err(StoreError::SchemaMigrationError)?;
+            if let Some(fork_choice) = fork_choice_opt {
+                // Store the converted fork choice store under the same key.
+                db.put_item::<PersistedForkChoice>(&FORK_CHOICE_DB_KEY, &fork_choice)?;
+            }
+
+            db.store_schema_version(to)?;
+
+            Ok(())
+        }
         // Anything else is an error.
         (_, _) => Err(HotColdDBError::UnsupportedSchemaVersion {
             target_version: to,

beacon_node/store/src/metadata.rs

@@ -4,7 +4,7 @@ use ssz::{Decode, Encode};
 use ssz_derive::{Decode, Encode};
 use types::{Checkpoint, Hash256, Slot};
 
-pub const CURRENT_SCHEMA_VERSION: SchemaVersion = SchemaVersion(5);
+pub const CURRENT_SCHEMA_VERSION: SchemaVersion = SchemaVersion(6);
 
 // All the keys that get stored under the `BeaconMeta` column.
 //

beacon_node/websocket_server/Cargo.toml

@@ -23,6 +23,7 @@ pub enum Error<T> {
     InvalidBlock(InvalidBlock),
     ProtoArrayError(String),
     InvalidProtoArrayBytes(String),
+    InvalidLegacyProtoArrayBytes(String),
     MissingProtoArrayBlock(Hash256),
     UnknownAncestor {
         ancestor_slot: Slot,
@@ -279,6 +280,12 @@ where
             AttestationShufflingId::new(anchor_block_root, anchor_state, RelativeEpoch::Next)
                 .map_err(Error::BeaconStateError)?;
 
+        // Default any non-merge execution block hashes to 0x000..000.
+        let execution_block_hash = anchor_block.message_merge().map_or_else(
+            |()| Hash256::zero(),
+            |message| message.body.execution_payload.block_hash,
+        );
+
         let proto_array = ProtoArrayForkChoice::new(
             finalized_block_slot,
             finalized_block_state_root,
@@ -287,6 +294,7 @@ where
             fc_store.finalized_checkpoint().root,
             current_epoch_shuffling_id,
             next_epoch_shuffling_id,
+            execution_block_hash,
         )?;
 
         Ok(Self {
@@ -576,6 +584,12 @@ where
             .on_verified_block(block, block_root, state)
             .map_err(Error::AfterBlockFailed)?;
 
+        // Default any non-merge execution block hashes to 0x000..000.
+        let execution_block_hash = block.body_merge().map_or_else(
+            |()| Hash256::zero(),
+            |body| body.execution_payload.block_hash,
+        );
+
         // This does not apply a vote to the block, it just makes fork choice aware of the block so
         // it can still be identified as the head even if it doesn't have any votes.
         self.proto_array.process_block(ProtoBlock {
@@ -598,6 +612,7 @@ where
             state_root: block.state_root(),
             justified_epoch: state.current_justified_checkpoint().epoch,
             finalized_epoch: state.finalized_checkpoint().epoch,
+            execution_block_hash,
         })?;
 
         Ok(())
@@ -893,7 +908,7 @@ where
 /// This is used when persisting the state of the fork choice to disk.
 #[derive(Encode, Decode, Clone)]
 pub struct PersistedForkChoice {
-    proto_array_bytes: Vec<u8>,
+    pub proto_array_bytes: Vec<u8>,
     queued_attestations: Vec<QueuedAttestation>,
 }
 

beacon_node/websocket_server/src/lib.rs

@@ -57,6 +57,7 @@ impl ForkChoiceTestDefinition {
     pub fn run(self) {
         let junk_shuffling_id =
             AttestationShufflingId::from_components(Epoch::new(0), Hash256::zero());
+        let execution_block_hash = Hash256::zero();
         let mut fork_choice = ProtoArrayForkChoice::new(
             self.finalized_block_slot,
             Hash256::zero(),
@@ -65,6 +66,7 @@ impl ForkChoiceTestDefinition {
             self.finalized_root,
             junk_shuffling_id.clone(),
             junk_shuffling_id,
+            execution_block_hash,
         )
         .expect("should create fork choice struct");
 
@@ -139,6 +141,7 @@ impl ForkChoiceTestDefinition {
                         ),
                         justified_epoch,
                         finalized_epoch,
+                        execution_block_hash,
                     };
                     fork_choice.process_block(block).unwrap_or_else(|e| {
                         panic!(

consensus/fork_choice/src/fork_choice.rs

@@ -35,6 +35,52 @@ pub struct ProtoNode {
     best_child: Option<usize>,
     #[ssz(with = "four_byte_option_usize")]
     best_descendant: Option<usize>,
+    /// It's necessary to track this so that we can refuse to propagate post-merge blocks without
+    /// execution payloads, without confusing these with pre-merge blocks.
+    ///
+    /// Relevant spec issue: https://github.com/ethereum/consensus-specs/issues/2618
+    pub execution_block_hash: Hash256,
+}
+
+/// Only used for SSZ deserialization of the persisted fork choice during the database migration
+/// from schema 4 to schema 5.
+#[derive(Encode, Decode)]
+pub struct LegacyProtoNode {
+    pub slot: Slot,
+    pub state_root: Hash256,
+    pub target_root: Hash256,
+    pub current_epoch_shuffling_id: AttestationShufflingId,
+    pub next_epoch_shuffling_id: AttestationShufflingId,
+    pub root: Hash256,
+    #[ssz(with = "four_byte_option_usize")]
+    pub parent: Option<usize>,
+    pub justified_epoch: Epoch,
+    pub finalized_epoch: Epoch,
+    weight: u64,
+    #[ssz(with = "four_byte_option_usize")]
+    best_child: Option<usize>,
+    #[ssz(with = "four_byte_option_usize")]
+    best_descendant: Option<usize>,
+}
+
+impl Into<ProtoNode> for LegacyProtoNode {
+    fn into(self) -> ProtoNode {
+        ProtoNode {
+            slot: self.slot,
+            state_root: self.state_root,
+            target_root: self.target_root,
+            current_epoch_shuffling_id: self.current_epoch_shuffling_id,
+            next_epoch_shuffling_id: self.next_epoch_shuffling_id,
+            root: self.root,
+            parent: self.parent,
+            justified_epoch: self.justified_epoch,
+            finalized_epoch: self.finalized_epoch,
+            weight: self.weight,
+            best_child: self.best_child,
+            best_descendant: self.best_descendant,
+            execution_block_hash: Hash256::zero(),
+        }
+    }
 }
 
 #[derive(PartialEq, Debug, Serialize, Deserialize, Clone)]
@@ -178,6 +224,7 @@ impl ProtoArray {
             weight: 0,
             best_child: None,
             best_descendant: None,
+            execution_block_hash: block.execution_block_hash,
         };
 
         self.indices.insert(node.root, node_index);

consensus/proto_array/src/fork_choice_test_definition.rs

@@ -1,6 +1,6 @@
 use crate::error::Error;
 use crate::proto_array::ProtoArray;
-use crate::ssz_container::SszContainer;
+use crate::ssz_container::{LegacySszContainer, SszContainer};
 use ssz::{Decode, Encode};
 use ssz_derive::{Decode, Encode};
 use std::collections::HashMap;
@@ -29,6 +29,7 @@ pub struct Block {
     pub next_epoch_shuffling_id: AttestationShufflingId,
     pub justified_epoch: Epoch,
     pub finalized_epoch: Epoch,
+    pub execution_block_hash: Hash256,
 }
 
 /// A Vec-wrapper which will grow to match any request.
@@ -66,6 +67,7 @@ pub struct ProtoArrayForkChoice {
 }
 
 impl ProtoArrayForkChoice {
+    #[allow(clippy::too_many_arguments)]
     pub fn new(
         finalized_block_slot: Slot,
         finalized_block_state_root: Hash256,
@@ -74,6 +76,7 @@ impl ProtoArrayForkChoice {
         finalized_root: Hash256,
         current_epoch_shuffling_id: AttestationShufflingId,
         next_epoch_shuffling_id: AttestationShufflingId,
+        execution_block_hash: Hash256,
     ) -> Result<Self, String> {
         let mut proto_array = ProtoArray {
             prune_threshold: DEFAULT_PRUNE_THRESHOLD,
@@ -95,6 +98,7 @@ impl ProtoArrayForkChoice {
             next_epoch_shuffling_id,
             justified_epoch,
             finalized_epoch,
+            execution_block_hash,
         };
 
         proto_array
@@ -204,6 +208,7 @@ impl ProtoArrayForkChoice {
             next_epoch_shuffling_id: block.next_epoch_shuffling_id.clone(),
             justified_epoch: block.justified_epoch,
             finalized_epoch: block.finalized_epoch,
+            execution_block_hash: block.execution_block_hash,
         })
     }
 
@@ -252,6 +257,22 @@ impl ProtoArrayForkChoice {
             .map_err(|e| format!("Failed to decode ProtoArrayForkChoice: {:?}", e))
     }
 
+    /// Only used for SSZ deserialization of the persisted fork choice during the database migration
+    /// from schema 4 to schema 5.
+    pub fn from_bytes_legacy(bytes: &[u8]) -> Result<Self, String> {
+        LegacySszContainer::from_ssz_bytes(bytes)
+            .map(|legacy_container| {
+                let container: SszContainer = legacy_container.into();
+                container.into()
+            })
+            .map_err(|e| {
+                format!(
+                    "Failed to decode ProtoArrayForkChoice during schema migration: {:?}",
+                    e
+                )
+            })
+    }
+
     /// Returns a read-lock to core `ProtoArray` struct.
     ///
     /// Should only be used when encoding/decoding during troubleshooting.
@@ -351,6 +372,7 @@ mod test_compute_deltas {
         let unknown = Hash256::from_low_u64_be(4);
         let junk_shuffling_id =
             AttestationShufflingId::from_components(Epoch::new(0), Hash256::zero());
+        let execution_block_hash = Hash256::zero();
 
         let mut fc = ProtoArrayForkChoice::new(
             genesis_slot,
@@ -360,6 +382,7 @@ mod test_compute_deltas {
             finalized_root,
             junk_shuffling_id.clone(),
             junk_shuffling_id.clone(),
+            execution_block_hash,
         )
         .unwrap();
 
@@ -375,6 +398,7 @@ mod test_compute_deltas {
                 next_epoch_shuffling_id: junk_shuffling_id.clone(),
                 justified_epoch: genesis_epoch,
                 finalized_epoch: genesis_epoch,
+                execution_block_hash,
             })
             .unwrap();
 
@@ -390,6 +414,7 @@ mod test_compute_deltas {
                 next_epoch_shuffling_id: junk_shuffling_id,
                 justified_epoch: genesis_epoch,
                 finalized_epoch: genesis_epoch,
+                execution_block_hash,
             })
             .unwrap();
 

consensus/proto_array/src/proto_array.rs

@@ -1,3 +1,4 @@
+use crate::proto_array::LegacyProtoNode;
 use crate::{
     proto_array::{ProtoArray, ProtoNode},
     proto_array_fork_choice::{ElasticList, ProtoArrayForkChoice, VoteTracker},
@@ -17,6 +18,35 @@ pub struct SszContainer {
     indices: Vec<(Hash256, usize)>,
 }
 
+/// Only used for SSZ deserialization of the persisted fork choice during the database migration
+/// from schema 4 to schema 5.
+#[derive(Encode, Decode)]
+pub struct LegacySszContainer {
+    votes: Vec<VoteTracker>,
+    balances: Vec<u64>,
+    prune_threshold: usize,
+    justified_epoch: Epoch,
+    finalized_epoch: Epoch,
+    nodes: Vec<LegacyProtoNode>,
+    indices: Vec<(Hash256, usize)>,
+}
+
+impl Into<SszContainer> for LegacySszContainer {
+    fn into(self) -> SszContainer {
+        let nodes = self.nodes.into_iter().map(Into::into).collect();
+
+        SszContainer {
+            votes: self.votes,
+            balances: self.balances,
+            prune_threshold: self.prune_threshold,
+            justified_epoch: self.justified_epoch,
+            finalized_epoch: self.finalized_epoch,
+            nodes,
+            indices: self.indices,
+        }
+    }
+}
+
 impl From<&ProtoArrayForkChoice> for SszContainer {
     fn from(from: &ProtoArrayForkChoice) -> Self {
         let proto_array = &from.proto_array;