From aa534f89899f1e9e04e6f161aab9f08502b6fd2b Mon Sep 17 00:00:00 2001 From: realbigsean Date: Tue, 28 Sep 2021 05:56:49 -0400 Subject: [PATCH] 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 --- beacon_node/beacon_chain/src/schema_change.rs | 26 +++++++++- beacon_node/store/src/metadata.rs | 2 +- beacon_node/websocket_server/Cargo.toml | 0 beacon_node/websocket_server/src/lib.rs | 0 consensus/fork_choice/src/fork_choice.rs | 17 ++++++- .../src/fork_choice_test_definition.rs | 3 ++ consensus/proto_array/src/proto_array.rs | 47 +++++++++++++++++++ .../src/proto_array_fork_choice.rs | 27 ++++++++++- consensus/proto_array/src/ssz_container.rs | 30 ++++++++++++ 9 files changed, 148 insertions(+), 4 deletions(-) delete mode 100644 beacon_node/websocket_server/Cargo.toml delete mode 100644 beacon_node/websocket_server/src/lib.rs diff --git a/beacon_node/beacon_chain/src/schema_change.rs b/beacon_node/beacon_chain/src/schema_change.rs index ec92b7c8a..45f973147 100644 --- a/beacon_node/beacon_chain/src/schema_change.rs +++ b/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( Ok(()) } + // Migration for adding `is_merge_complete` field to the fork choice store. + (SchemaVersion(5), SchemaVersion(6)) => { + let fork_choice_opt = db + .get_item::(&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::(&FORK_CHOICE_DB_KEY, &fork_choice)?; + } + + db.store_schema_version(to)?; + + Ok(()) + } // Anything else is an error. (_, _) => Err(HotColdDBError::UnsupportedSchemaVersion { target_version: to, diff --git a/beacon_node/store/src/metadata.rs b/beacon_node/store/src/metadata.rs index fd20a5880..cc0535ef5 100644 --- a/beacon_node/store/src/metadata.rs +++ b/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. // diff --git a/beacon_node/websocket_server/Cargo.toml b/beacon_node/websocket_server/Cargo.toml deleted file mode 100644 index e69de29bb..000000000 diff --git a/beacon_node/websocket_server/src/lib.rs b/beacon_node/websocket_server/src/lib.rs deleted file mode 100644 index e69de29bb..000000000 diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index f1b9a6999..6b09cdc9c 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -18,6 +18,7 @@ pub enum Error { InvalidBlock(InvalidBlock), ProtoArrayError(String), InvalidProtoArrayBytes(String), + InvalidLegacyProtoArrayBytes(String), MissingProtoArrayBlock(Hash256), UnknownAncestor { ancestor_slot: Slot, @@ -274,6 +275,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, @@ -282,6 +289,7 @@ where fc_store.finalized_checkpoint().root, current_epoch_shuffling_id, next_epoch_shuffling_id, + execution_block_hash, )?; Ok(Self { @@ -572,6 +580,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 { @@ -594,6 +608,7 @@ where state_root: block.state_root(), justified_epoch: state.current_justified_checkpoint().epoch, finalized_epoch: state.finalized_checkpoint().epoch, + execution_block_hash, })?; Ok(()) @@ -904,7 +919,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, + pub proto_array_bytes: Vec, queued_attestations: Vec, } diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index 688878e1a..c713ad3b1 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.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!( diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index b4d6dd9e0..a4b811c5d 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -35,6 +35,52 @@ pub struct ProtoNode { best_child: Option, #[ssz(with = "four_byte_option_usize")] best_descendant: Option, + /// 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, + pub justified_epoch: Epoch, + pub finalized_epoch: Epoch, + weight: u64, + #[ssz(with = "four_byte_option_usize")] + best_child: Option, + #[ssz(with = "four_byte_option_usize")] + best_descendant: Option, +} + +impl Into 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); diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 36bdab2db..18417151b 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.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 { 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 { + 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(); diff --git a/consensus/proto_array/src/ssz_container.rs b/consensus/proto_array/src/ssz_container.rs index c79c433e3..cf1da1233 100644 --- a/consensus/proto_array/src/ssz_container.rs +++ b/consensus/proto_array/src/ssz_container.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, + balances: Vec, + prune_threshold: usize, + justified_epoch: Epoch, + finalized_epoch: Epoch, + nodes: Vec, + indices: Vec<(Hash256, usize)>, +} + +impl Into 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;