From b22ac95d7fe1734c5c204c1ed8adb3814e4a5deb Mon Sep 17 00:00:00 2001 From: realbigsean Date: Mon, 13 Dec 2021 20:43:22 +0000 Subject: [PATCH] v1.1.6 Fork Choice changes (#2822) ## Issue Addressed Resolves: https://github.com/sigp/lighthouse/issues/2741 Includes: https://github.com/sigp/lighthouse/pull/2853 so that we can get ssz static tests passing here on v1.1.6. If we want to merge that first, we can make this diff slightly smaller ## Proposed Changes - Changes the `justified_epoch` and `finalized_epoch` in the `ProtoArrayNode` each to an `Option`. The `Option` is necessary only for the migration, so not ideal. But does allow us to add a default logic to `None` on these fields during the database migration. - Adds a database migration from a legacy fork choice struct to the new one, search for all necessary block roots in fork choice by iterating through blocks in the db. - updates related to https://github.com/ethereum/consensus-specs/pull/2727 - We will have to update the persisted forkchoice to make sure the justified checkpoint stored is correct according to the updated fork choice logic. This boils down to setting the forkchoice store's justified checkpoint to the justified checkpoint of the block that advanced the finalized checkpoint to the current one. - AFAICT there's no migration steps necessary for the update to allow applying attestations from prior blocks, but would appreciate confirmation on that - I updated the consensus spec tests to v1.1.6 here, but they will fail until we also implement the proposer score boost updates. I confirmed that the previously failing scenario `new_finalized_slot_is_justified_checkpoint_ancestor` will now pass after the boost updates, but haven't confirmed _all_ tests will pass because I just quickly stubbed out the proposer boost test scenario formatting. - This PR now also includes proposer boosting https://github.com/ethereum/consensus-specs/pull/2730 ## Additional Info I realized checking justified and finalized roots in fork choice makes it more likely that we trigger this bug: https://github.com/ethereum/consensus-specs/pull/2727 It's possible the combination of justified checkpoint and finalized checkpoint in the forkchoice store is different from in any block in fork choice. So when trying to startup our store's justified checkpoint seems invalid to the rest of fork choice (but it should be valid). When this happens we get an `InvalidBestNode` error and fail to start up. So I'm including that bugfix in this branch. Todo: - [x] Fix fork choice tests - [x] Self review - [x] Add fix for https://github.com/ethereum/consensus-specs/pull/2727 - [x] Rebase onto Kintusgi - [x] Fix `num_active_validators` calculation as @michaelsproul pointed out - [x] Clean up db migrations Co-authored-by: realbigsean --- Cargo.lock | 66 +--- beacon_node/beacon_chain/Cargo.toml | 1 + beacon_node/beacon_chain/src/beacon_chain.rs | 25 +- .../src/beacon_fork_choice_store.rs | 57 +-- beacon_node/beacon_chain/src/builder.rs | 2 +- beacon_node/beacon_chain/src/fork_revert.rs | 3 + .../beacon_chain/src/persisted_fork_choice.rs | 43 ++- beacon_node/beacon_chain/src/schema_change.rs | 98 ++++- .../beacon_chain/src/schema_change/README.md | 74 ++++ .../src/schema_change/migration_schema_v6.rs | 28 ++ .../src/schema_change/migration_schema_v7.rs | 327 ++++++++++++++++ .../beacon_chain/src/schema_change/types.rs | 192 ++++++++++ beacon_node/beacon_chain/tests/store_tests.rs | 5 +- beacon_node/client/src/builder.rs | 5 +- beacon_node/lighthouse_network/Cargo.toml | 2 +- beacon_node/src/lib.rs | 8 +- beacon_node/store/src/hot_cold_store.rs | 15 + beacon_node/store/src/iter.rs | 9 + beacon_node/store/src/metadata.rs | 2 +- .../mainnet/config.yaml | 6 + .../prater/config.yaml | 5 + .../pyrmont/config.yaml | 5 + common/slot_clock/src/lib.rs | 18 +- consensus/fork_choice/src/fork_choice.rs | 140 ++++--- .../fork_choice/src/fork_choice_store.rs | 8 +- consensus/fork_choice/src/lib.rs | 4 +- consensus/fork_choice/tests/tests.rs | 3 + consensus/proto_array/src/error.rs | 22 +- .../src/fork_choice_test_definition.rs | 78 ++-- .../ffg_updates.rs | 217 ++++++----- .../fork_choice_test_definition/no_votes.rs | 172 ++++++--- .../src/fork_choice_test_definition/votes.rs | 353 +++++++++++++----- consensus/proto_array/src/lib.rs | 4 +- consensus/proto_array/src/proto_array.rs | 181 +++++---- .../src/proto_array_fork_choice.rs | 122 +++--- consensus/proto_array/src/ssz_container.rs | 63 ++-- consensus/types/Cargo.toml | 2 +- consensus/types/src/chain_spec.rs | 8 + consensus/types/src/consts.rs | 3 + .../environment/tests/testnet_dir/config.yaml | 4 + testing/ef_tests/check_all_files_accessed.py | 16 +- testing/ef_tests/src/cases/fork_choice.rs | 55 ++- testing/ef_tests/src/cases/transition.rs | 2 +- testing/ef_tests/tests/tests.rs | 7 - 44 files changed, 1802 insertions(+), 658 deletions(-) create mode 100644 beacon_node/beacon_chain/src/schema_change/README.md create mode 100644 beacon_node/beacon_chain/src/schema_change/migration_schema_v6.rs create mode 100644 beacon_node/beacon_chain/src/schema_change/migration_schema_v7.rs create mode 100644 beacon_node/beacon_chain/src/schema_change/types.rs diff --git a/Cargo.lock b/Cargo.lock index 3e426228f..c08719091 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -330,6 +330,7 @@ dependencies = [ "state_processing", "store", "strum", + "superstruct", "task_executor", "tempfile", "tokio", @@ -976,38 +977,14 @@ dependencies = [ "zeroize", ] -[[package]] -name = "darling" -version = "0.12.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5f2c43f534ea4b0b049015d00269734195e6d3f0f6635cb692251aca6f9f8b3c" -dependencies = [ - "darling_core 0.12.4", - "darling_macro 0.12.4", -] - [[package]] name = "darling" version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "757c0ded2af11d8e739c4daea1ac623dd1624b06c844cf3f5a39f1bdbd99bb12" dependencies = [ - "darling_core 0.13.0", - "darling_macro 0.13.0", -] - -[[package]] -name = "darling_core" -version = "0.12.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8e91455b86830a1c21799d94524df0845183fa55bafd9aa137b01c7d1065fa36" -dependencies = [ - "fnv", - "ident_case", - "proc-macro2", - "quote", - "strsim 0.10.0", - "syn", + "darling_core", + "darling_macro", ] [[package]] @@ -1024,24 +1001,13 @@ dependencies = [ "syn", ] -[[package]] -name = "darling_macro" -version = "0.12.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "29b5acf0dea37a7f66f7b25d2c5e93fd46f8f6968b1a5d7a3e02e97768afc95a" -dependencies = [ - "darling_core 0.12.4", - "quote", - "syn", -] - [[package]] name = "darling_macro" version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ade7bff147130fe5e6d39f089c6bd49ec0250f35d70b2eebf72afdfc919f15cc" dependencies = [ - "darling_core 0.13.0", + "darling_core", "quote", "syn", ] @@ -1121,14 +1087,14 @@ dependencies = [ [[package]] name = "derive_more" -version = "0.99.16" +version = "0.99.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "40eebddd2156ce1bb37b20bbe5151340a31828b1f2d22ba4141f3531710e38df" +checksum = "4fb810d30a7c1953f91334de7244731fc3f3c10d7fe163338a35b9f640960321" dependencies = [ "convert_case", "proc-macro2", "quote", - "rustc_version 0.3.3", + "rustc_version 0.4.0", "syn", ] @@ -1597,7 +1563,7 @@ dependencies = [ name = "eth2_ssz_derive" version = "0.3.0" dependencies = [ - "darling 0.13.0", + "darling", "proc-macro2", "quote", "syn", @@ -1609,7 +1575,7 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "635b86d2c941bb71e7419a571e1763d65c93e51a1bafc400352e3bef6ff59fc9" dependencies = [ - "darling 0.13.0", + "darling", "proc-macro2", "quote", "syn", @@ -5671,11 +5637,11 @@ checksum = "6bdef32e8150c2a081110b42772ffe7d7c9032b606bc226c8260fd97e0976601" [[package]] name = "superstruct" -version = "0.2.0" +version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8bf7f6700d7c135cf4e4900c2cfba9a12ecad1fdc45594aad48f6b344b2589a0" +checksum = "ecffe12af481bd0b8950f90676d61fb1e5fc33f1f1c41ce5df11e83fb509aaab" dependencies = [ - "darling 0.12.4", + "darling", "itertools", "proc-macro2", "quote", @@ -5693,9 +5659,9 @@ dependencies = [ [[package]] name = "syn" -version = "1.0.81" +version = "1.0.82" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f2afee18b8beb5a596ecb4a2dce128c719b4ba399d34126b9e4396e3f9860966" +checksum = "8daf5dd0bb60cbd4137b1b587d2fc0ae729bc07cf01cd70b36a1ed5ade3b9d59" dependencies = [ "proc-macro2", "quote", @@ -6195,7 +6161,7 @@ dependencies = [ name = "tree_hash_derive" version = "0.4.0" dependencies = [ - "darling 0.13.0", + "darling", "quote", "syn", ] @@ -6206,7 +6172,7 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3cd22d128157837a4434bb51119aef11103f17bfe8c402ce688cf25aa1e608ad" dependencies = [ - "darling 0.13.0", + "darling", "quote", "syn", ] diff --git a/beacon_node/beacon_chain/Cargo.toml b/beacon_node/beacon_chain/Cargo.toml index d503a01b8..c4bd3bf7b 100644 --- a/beacon_node/beacon_chain/Cargo.toml +++ b/beacon_node/beacon_chain/Cargo.toml @@ -58,6 +58,7 @@ strum = { version = "0.21.0", features = ["derive"] } logging = { path = "../../common/logging" } execution_layer = { path = "../execution_layer" } sensitive_url = { path = "../../common/sensitive_url" } +superstruct = "0.3.0" [[test]] name = "beacon_chain_tests" diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index ca11c8a7b..1f36e0e65 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -51,7 +51,7 @@ use eth2::types::{ EventKind, SseBlock, SseChainReorg, SseFinalizedCheckpoint, SseHead, SseLateHead, SyncDuty, }; use execution_layer::ExecutionLayer; -use fork_choice::ForkChoice; +use fork_choice::{AttestationFromBlock, ForkChoice}; use futures::channel::mpsc::Sender; use itertools::process_results; use itertools::Itertools; @@ -1700,7 +1700,11 @@ impl BeaconChain { self.fork_choice .write() - .on_attestation(self.slot()?, verified.indexed_attestation()) + .on_attestation( + self.slot()?, + verified.indexed_attestation(), + AttestationFromBlock::False, + ) .map_err(Into::into) } @@ -2443,11 +2447,17 @@ impl BeaconChain { { let _fork_choice_block_timer = metrics::start_timer(&metrics::FORK_CHOICE_PROCESS_BLOCK_TIMES); + let block_delay = self + .slot_clock + .seconds_from_current_slot_start(self.spec.seconds_per_slot) + .ok_or(Error::UnableToComputeTimeAtSlot)?; + fork_choice .on_block( current_slot, &block, block_root, + block_delay, &state, payload_verification_status, &self.spec, @@ -2472,7 +2482,11 @@ impl BeaconChain { let indexed_attestation = get_indexed_attestation(committee.committee, attestation) .map_err(|e| BlockError::BeaconChainError(e.into()))?; - match fork_choice.on_attestation(current_slot, &indexed_attestation) { + match fork_choice.on_attestation( + current_slot, + &indexed_attestation, + AttestationFromBlock::True, + ) { Ok(()) => Ok(()), // Ignore invalid attestations whilst importing attestations from a block. The // block might be very old and therefore the attestations useless to fork choice. @@ -3009,7 +3023,10 @@ impl BeaconChain { fn fork_choice_internal(&self) -> Result<(), Error> { // Determine the root of the block that is the head of the chain. - let beacon_block_root = self.fork_choice.write().get_head(self.slot()?)?; + let beacon_block_root = self + .fork_choice + .write() + .get_head(self.slot()?, &self.spec)?; let current_head = self.head_info()?; let old_finalized_checkpoint = current_head.finalized_checkpoint; diff --git a/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs b/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs index 7d9e42fb8..956c50e03 100644 --- a/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs +++ b/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs @@ -1,15 +1,17 @@ //! Defines the `BeaconForkChoiceStore` which provides the persistent storage for the `ForkChoice` //! struct. //! -//! Additionally, the private `BalancesCache` struct is defined; a cache designed to avoid database +//! Additionally, the `BalancesCache` struct is defined; a cache designed to avoid database //! reads when fork choice requires the validator balances of the justified state. use crate::{metrics, BeaconSnapshot}; +use derivative::Derivative; use fork_choice::ForkChoiceStore; use ssz_derive::{Decode, Encode}; use std::marker::PhantomData; use std::sync::Arc; use store::{Error as StoreError, HotColdDB, ItemStore}; +use superstruct::superstruct; use types::{BeaconBlock, BeaconState, BeaconStateError, Checkpoint, EthSpec, Hash256, Slot}; #[derive(Debug)] @@ -68,7 +70,7 @@ struct CacheItem { /// /// It is effectively a mapping of `epoch_boundary_block_root -> state.balances`. #[derive(PartialEq, Clone, Default, Debug, Encode, Decode)] -struct BalancesCache { +pub struct BalancesCache { items: Vec, } @@ -154,8 +156,10 @@ impl BalancesCache { /// Implements `fork_choice::ForkChoiceStore` in order to provide a persistent backing to the /// `fork_choice::ForkChoice` struct. -#[derive(Debug)] +#[derive(Debug, Derivative)] +#[derivative(PartialEq(bound = "E: EthSpec, Hot: ItemStore, Cold: ItemStore"))] pub struct BeaconForkChoiceStore, Cold: ItemStore> { + #[derivative(PartialEq = "ignore")] store: Arc>, balances_cache: BalancesCache, time: Slot, @@ -163,26 +167,10 @@ pub struct BeaconForkChoiceStore, Cold: ItemStore< justified_checkpoint: Checkpoint, justified_balances: Vec, best_justified_checkpoint: Checkpoint, + proposer_boost_root: Hash256, _phantom: PhantomData, } -impl PartialEq for BeaconForkChoiceStore -where - E: EthSpec, - Hot: ItemStore, - Cold: ItemStore, -{ - /// This implementation ignores the `store` and `slot_clock`. - fn eq(&self, other: &Self) -> bool { - self.balances_cache == other.balances_cache - && self.time == other.time - && self.finalized_checkpoint == other.finalized_checkpoint - && self.justified_checkpoint == other.justified_checkpoint - && self.justified_balances == other.justified_balances - && self.best_justified_checkpoint == other.best_justified_checkpoint - } -} - impl BeaconForkChoiceStore where E: EthSpec, @@ -225,6 +213,7 @@ where justified_balances: anchor_state.balances().clone().into(), finalized_checkpoint, best_justified_checkpoint: justified_checkpoint, + proposer_boost_root: Hash256::zero(), _phantom: PhantomData, } } @@ -239,6 +228,7 @@ where justified_checkpoint: self.justified_checkpoint, justified_balances: self.justified_balances.clone(), best_justified_checkpoint: self.best_justified_checkpoint, + proposer_boost_root: self.proposer_boost_root, } } @@ -255,6 +245,7 @@ where justified_checkpoint: persisted.justified_checkpoint, justified_balances: persisted.justified_balances, best_justified_checkpoint: persisted.best_justified_checkpoint, + proposer_boost_root: persisted.proposer_boost_root, _phantom: PhantomData, }) } @@ -301,6 +292,10 @@ where &self.finalized_checkpoint } + fn proposer_boost_root(&self) -> Hash256 { + self.proposer_boost_root + } + fn set_finalized_checkpoint(&mut self, checkpoint: Checkpoint) { self.finalized_checkpoint = checkpoint } @@ -336,15 +331,23 @@ where fn set_best_justified_checkpoint(&mut self, checkpoint: Checkpoint) { self.best_justified_checkpoint = checkpoint } + + fn set_proposer_boost_root(&mut self, proposer_boost_root: Hash256) { + self.proposer_boost_root = proposer_boost_root; + } } /// A container which allows persisting the `BeaconForkChoiceStore` to the on-disk database. -#[derive(Encode, Decode)] +#[superstruct(variants(V1, V7), variant_attributes(derive(Encode, Decode)), no_enum)] pub struct PersistedForkChoiceStore { - balances_cache: BalancesCache, - time: Slot, - finalized_checkpoint: Checkpoint, - justified_checkpoint: Checkpoint, - justified_balances: Vec, - best_justified_checkpoint: Checkpoint, + pub balances_cache: BalancesCache, + pub time: Slot, + pub finalized_checkpoint: Checkpoint, + pub justified_checkpoint: Checkpoint, + pub justified_balances: Vec, + pub best_justified_checkpoint: Checkpoint, + #[superstruct(only(V7))] + pub proposer_boost_root: Hash256, } + +pub type PersistedForkChoiceStore = PersistedForkChoiceStoreV7; diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 48e3ff6a4..54397a7d5 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -591,7 +591,7 @@ where }; let initial_head_block_root = fork_choice - .get_head(current_slot) + .get_head(current_slot, &self.spec) .map_err(|e| format!("Unable to get fork choice head: {:?}", e))?; // Try to decode the head block according to the current fork, if that fails, try diff --git a/beacon_node/beacon_chain/src/fork_revert.rs b/beacon_node/beacon_chain/src/fork_revert.rs index 610e27eb9..880eb8e67 100644 --- a/beacon_node/beacon_chain/src/fork_revert.rs +++ b/beacon_node/beacon_chain/src/fork_revert.rs @@ -5,6 +5,7 @@ use slog::{info, warn, Logger}; use state_processing::state_advance::complete_state_advance; use state_processing::{per_block_processing, per_block_processing::BlockSignatureStrategy}; use std::sync::Arc; +use std::time::Duration; use store::{iter::ParentRootBlockIterator, HotColdDB, ItemStore}; use types::{BeaconState, ChainSpec, EthSpec, ForkName, Hash256, SignedBeaconBlock, Slot}; @@ -176,6 +177,8 @@ pub fn reset_fork_choice_to_finalization, Cold: It block.slot(), &block, block.canonical_root(), + // Reward proposer boost. We are reinforcing the canonical chain. + Duration::from_secs(0), &state, payload_verification_status, spec, diff --git a/beacon_node/beacon_chain/src/persisted_fork_choice.rs b/beacon_node/beacon_chain/src/persisted_fork_choice.rs index ed84b7fc2..666ae6e85 100644 --- a/beacon_node/beacon_chain/src/persisted_fork_choice.rs +++ b/beacon_node/beacon_chain/src/persisted_fork_choice.rs @@ -1,25 +1,38 @@ -use crate::beacon_fork_choice_store::PersistedForkChoiceStore as ForkChoiceStore; -use fork_choice::PersistedForkChoice as ForkChoice; +use crate::beacon_fork_choice_store::{PersistedForkChoiceStoreV1, PersistedForkChoiceStoreV7}; use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; use store::{DBColumn, Error, StoreItem}; +use superstruct::superstruct; -#[derive(Encode, Decode)] +// If adding a new version you should update this type alias and fix the breakages. +pub type PersistedForkChoice = PersistedForkChoiceV7; + +#[superstruct(variants(V1, V7), variant_attributes(derive(Encode, Decode)), no_enum)] pub struct PersistedForkChoice { - pub fork_choice: ForkChoice, - pub fork_choice_store: ForkChoiceStore, + pub fork_choice: fork_choice::PersistedForkChoice, + #[superstruct(only(V1))] + pub fork_choice_store: PersistedForkChoiceStoreV1, + #[superstruct(only(V7))] + pub fork_choice_store: PersistedForkChoiceStoreV7, } -impl StoreItem for PersistedForkChoice { - fn db_column() -> DBColumn { - DBColumn::ForkChoice - } +macro_rules! impl_store_item { + ($type:ty) => { + impl StoreItem for $type { + fn db_column() -> DBColumn { + DBColumn::ForkChoice + } - fn as_store_bytes(&self) -> Vec { - self.as_ssz_bytes() - } + fn as_store_bytes(&self) -> Vec { + self.as_ssz_bytes() + } - fn from_store_bytes(bytes: &[u8]) -> std::result::Result { - Self::from_ssz_bytes(bytes).map_err(Into::into) - } + fn from_store_bytes(bytes: &[u8]) -> std::result::Result { + Self::from_ssz_bytes(bytes).map_err(Into::into) + } + } + }; } + +impl_store_item!(PersistedForkChoiceV1); +impl_store_item!(PersistedForkChoiceV7); diff --git a/beacon_node/beacon_chain/src/schema_change.rs b/beacon_node/beacon_chain/src/schema_change.rs index 45f973147..c0ab245df 100644 --- a/beacon_node/beacon_chain/src/schema_change.rs +++ b/beacon_node/beacon_chain/src/schema_change.rs @@ -1,9 +1,14 @@ //! Utilities for managing database schema changes. +mod migration_schema_v6; +mod migration_schema_v7; +mod types; + use crate::beacon_chain::{BeaconChainTypes, FORK_CHOICE_DB_KEY, OP_POOL_DB_KEY}; -use crate::persisted_fork_choice::PersistedForkChoice; +use crate::persisted_fork_choice::{PersistedForkChoiceV1, PersistedForkChoiceV7}; +use crate::store::{get_key_for_col, KeyValueStoreOp}; use crate::validator_pubkey_cache::ValidatorPubkeyCache; use operation_pool::{PersistedOperationPool, PersistedOperationPoolBase}; -use proto_array::ProtoArrayForkChoice; +use slog::{warn, Logger}; use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; use std::fs; @@ -22,6 +27,7 @@ pub fn migrate_schema( datadir: &Path, from: SchemaVersion, to: SchemaVersion, + log: Logger, ) -> Result<(), StoreError> { match (from, to) { // Migrating from the current schema version to iself is always OK, a no-op. @@ -29,8 +35,8 @@ pub fn migrate_schema( // Migrate across multiple versions by recursively migrating one step at a time. (_, _) if from.as_u64() + 1 < to.as_u64() => { let next = SchemaVersion(from.as_u64() + 1); - migrate_schema::(db.clone(), datadir, from, next)?; - migrate_schema::(db, datadir, next, to) + migrate_schema::(db.clone(), datadir, from, next, log.clone())?; + migrate_schema::(db, datadir, next, to, log) } // Migration from v0.3.0 to v0.3.x, adding the temporary states column. // Nothing actually needs to be done, but once a DB uses v2 it shouldn't go back. @@ -95,25 +101,77 @@ pub fn migrate_schema( Ok(()) } - // Migration for adding `is_merge_complete` field to the fork choice store. + // Migration for adding `execution_status` 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)?; + // Database operations to be done atomically + let mut ops = vec![]; + + // The top-level `PersistedForkChoice` struct is still V1 but will have its internal + // bytes for the fork choice updated to V6. + let fork_choice_opt = db.get_item::(&FORK_CHOICE_DB_KEY)?; + if let Some(mut persisted_fork_choice) = fork_choice_opt { + migration_schema_v6::update_execution_statuses::(&mut persisted_fork_choice) + .map_err(StoreError::SchemaMigrationError)?; + + let column = PersistedForkChoiceV1::db_column().into(); + let key = FORK_CHOICE_DB_KEY.as_bytes(); + let db_key = get_key_for_col(column, key); + let op = + KeyValueStoreOp::PutKeyValue(db_key, persisted_fork_choice.as_store_bytes()); + ops.push(op); } - db.store_schema_version(to)?; + db.store_schema_version_atomically(to, ops)?; + + Ok(()) + } + // 1. Add `proposer_boost_root`. + // 2. Update `justified_epoch` to `justified_checkpoint` and `finalized_epoch` to + // `finalized_checkpoint`. + // 3. This migration also includes a potential update to the justified + // checkpoint in case the fork choice store's justified checkpoint and finalized checkpoint + // combination does not actually exist for any blocks in fork choice. This was possible in + // the consensus spec prior to v1.1.6. + // + // Relevant issues: + // + // https://github.com/sigp/lighthouse/issues/2741 + // https://github.com/ethereum/consensus-specs/pull/2727 + // https://github.com/ethereum/consensus-specs/pull/2730 + (SchemaVersion(6), SchemaVersion(7)) => { + // Database operations to be done atomically + let mut ops = vec![]; + + let fork_choice_opt = db.get_item::(&FORK_CHOICE_DB_KEY)?; + if let Some(persisted_fork_choice_v1) = fork_choice_opt { + // This migrates the `PersistedForkChoiceStore`, adding the `proposer_boost_root` field. + let mut persisted_fork_choice_v7 = persisted_fork_choice_v1.into(); + + let result = migration_schema_v7::update_fork_choice::( + &mut persisted_fork_choice_v7, + db.clone(), + ); + + // Fall back to re-initializing fork choice from an anchor state if necessary. + if let Err(e) = result { + warn!(log, "Unable to migrate to database schema 7, re-initializing fork choice"; "error" => ?e); + migration_schema_v7::update_with_reinitialized_fork_choice::( + &mut persisted_fork_choice_v7, + db.clone(), + ) + .map_err(StoreError::SchemaMigrationError)?; + } + + // Store the converted fork choice store under the same key. + let column = PersistedForkChoiceV7::db_column().into(); + let key = FORK_CHOICE_DB_KEY.as_bytes(); + let db_key = get_key_for_col(column, key); + let op = + KeyValueStoreOp::PutKeyValue(db_key, persisted_fork_choice_v7.as_store_bytes()); + ops.push(op); + } + + db.store_schema_version_atomically(to, ops)?; Ok(()) } diff --git a/beacon_node/beacon_chain/src/schema_change/README.md b/beacon_node/beacon_chain/src/schema_change/README.md new file mode 100644 index 000000000..1a33b3c12 --- /dev/null +++ b/beacon_node/beacon_chain/src/schema_change/README.md @@ -0,0 +1,74 @@ +Database Schema Migrations +==== + +This document is an attempt to record some best practices and design conventions for applying +database schema migrations within Lighthouse. + +## General Structure + +If you make a breaking change to an on-disk data structure you need to increment the +`SCHEMA_VERSION` in `beacon_node/store/src/metadata.rs` and add a migration from the previous +version to the new version. + +The entry-point for database migrations is in `schema_change.rs`, _not_ `migrate.rs` (which deals +with finalization). Supporting code for a specific migration may be added in +`schema_change/migration_schema_vX.rs`, where `X` is the version being migrated _to_. + +## Combining Schema Changes + +Schema changes may be combined if they are part of the same pull request to +`unstable`. Once a schema version is defined in `unstable` we should not apply changes to it +without incrementing the version. This prevents conflicts between versions that appear to be the +same. This allows us to deploy `unstable` to nodes without having to worry about needing to resync +because of a sneaky schema change. + +Changing the on-disk structure for a version _before_ it is merged to `unstable` is OK. You will +just have to handle manually resyncing any test nodes (use checkpoint sync). + +## Naming Conventions + +Prefer to name versions of structs by _the version at which the change was introduced_. For example +if you add a field to `Foo` in v9, call the previous version `FooV1` (assuming this is `Foo`'s first +migration) and write a schema change that migrates from `FooV1` to `FooV9`. + +Prefer to use explicit version names in `schema_change.rs` and the `schema_change` module. To +interface with the outside either: + +1. Define a type alias to the latest version, e.g. `pub type Foo = FooV9`, or +2. Define a mapping from the latest version to the version used elsewhere, e.g. + ```rust + impl From for Foo {} + ``` + +Avoid names like: + +* `LegacyFoo` +* `OldFoo` +* `FooWithoutX` + +## First-version vs Last-version + +Previously the schema migration code would name types by the _last_ version at which they were +valid. For example if `Foo` changed in `V9` then we would name the two variants `FooV8` and `FooV9`. +The problem with this scheme is that if `Foo` changes again in the future at say v12 then `FooV9` would +need to be renamed to `FooV11`, which is annoying. Using the _first_ valid version as described +above does not have this issue. + +## Using SuperStruct + +If possible, consider using [`superstruct`](https://crates.io/crates/superstruct) to handle data +structure changes between versions. + +* Use `superstruct(no_enum)` to avoid generating an unnecessary top-level enum. + +## Example + +A field is added to `Foo` in v9, and there are two variants: `FooV1` and `FooV9`. There is a +migration from `FooV1` to `FooV9`. `Foo` is aliased to `FooV9`. + +Some time later another field is added to `Foo` in v12. A new `FooV12` is created, along with a +migration from `FooV9` to `FooV12`. The primary `Foo` type gets re-aliased to `FooV12`. The previous +migration from V1 to V9 shouldn't break because the schema migration refers to `FooV9` explicitly +rather than `Foo`. Due to the re-aliasing (or re-mapping) the compiler will check every usage +of `Foo` to make sure that it still makes sense with `FooV12`. + diff --git a/beacon_node/beacon_chain/src/schema_change/migration_schema_v6.rs b/beacon_node/beacon_chain/src/schema_change/migration_schema_v6.rs new file mode 100644 index 000000000..231da838c --- /dev/null +++ b/beacon_node/beacon_chain/src/schema_change/migration_schema_v6.rs @@ -0,0 +1,28 @@ +///! These functions and structs are only relevant to the database migration from schema 5 to 6. +use crate::persisted_fork_choice::PersistedForkChoiceV1; +use crate::schema_change::types::{SszContainerV1, SszContainerV6}; +use crate::BeaconChainTypes; +use ssz::four_byte_option_impl; +use ssz::{Decode, Encode}; + +// Define a "legacy" implementation of `Option` which uses four bytes for encoding the union +// selector. +four_byte_option_impl!(four_byte_option_usize, usize); + +pub(crate) fn update_execution_statuses( + persisted_fork_choice: &mut PersistedForkChoiceV1, +) -> Result<(), String> { + let ssz_container_v1 = + SszContainerV1::from_ssz_bytes(&persisted_fork_choice.fork_choice.proto_array_bytes) + .map_err(|e| { + format!( + "Failed to decode ProtoArrayForkChoice during schema migration: {:?}", + e + ) + })?; + + let ssz_container_v6: SszContainerV6 = ssz_container_v1.into(); + + persisted_fork_choice.fork_choice.proto_array_bytes = ssz_container_v6.as_ssz_bytes(); + Ok(()) +} diff --git a/beacon_node/beacon_chain/src/schema_change/migration_schema_v7.rs b/beacon_node/beacon_chain/src/schema_change/migration_schema_v7.rs new file mode 100644 index 000000000..a40b33412 --- /dev/null +++ b/beacon_node/beacon_chain/src/schema_change/migration_schema_v7.rs @@ -0,0 +1,327 @@ +///! These functions and structs are only relevant to the database migration from schema 6 to 7. +use crate::beacon_chain::BeaconChainTypes; +use crate::beacon_fork_choice_store::{PersistedForkChoiceStoreV1, PersistedForkChoiceStoreV7}; +use crate::persisted_fork_choice::{PersistedForkChoiceV1, PersistedForkChoiceV7}; +use crate::schema_change::types::{ProtoNodeV6, SszContainerV6, SszContainerV7}; +use crate::types::{Checkpoint, Epoch, Hash256}; +use crate::types::{EthSpec, Slot}; +use crate::{BeaconForkChoiceStore, BeaconSnapshot}; +use fork_choice::ForkChoice; +use proto_array::{core::ProtoNode, core::SszContainer, ProtoArrayForkChoice}; +use ssz::four_byte_option_impl; +use ssz::{Decode, Encode}; +use std::collections::{HashMap, HashSet}; +use std::sync::Arc; +use store::hot_cold_store::HotColdDB; +use store::iter::BlockRootsIterator; +use store::Error as StoreError; + +// Define a "legacy" implementation of `Option` which uses four bytes for encoding the union +// selector. +four_byte_option_impl!(four_byte_option_usize, usize); + +/// This method is used to re-initialize fork choice from the finalized state in case we hit an +/// error during this migration. +pub(crate) fn update_with_reinitialized_fork_choice( + persisted_fork_choice: &mut PersistedForkChoiceV7, + db: Arc>, +) -> Result<(), String> { + let anchor_block_root = persisted_fork_choice + .fork_choice_store + .finalized_checkpoint + .root; + let anchor_block = db + .get_block(&anchor_block_root) + .map_err(|e| format!("{:?}", e))? + .ok_or_else(|| "Missing anchor beacon block".to_string())?; + let anchor_state = db + .get_state(&anchor_block.state_root(), Some(anchor_block.slot())) + .map_err(|e| format!("{:?}", e))? + .ok_or_else(|| "Missing anchor beacon state".to_string())?; + let snapshot = BeaconSnapshot { + beacon_block: anchor_block, + beacon_block_root: anchor_block_root, + beacon_state: anchor_state, + }; + let store = BeaconForkChoiceStore::get_forkchoice_store(db, &snapshot); + let fork_choice = ForkChoice::from_anchor( + store, + anchor_block_root, + &snapshot.beacon_block, + &snapshot.beacon_state, + ) + .map_err(|e| format!("{:?}", e))?; + persisted_fork_choice.fork_choice = fork_choice.to_persisted(); + Ok(()) +} + +pub(crate) fn update_fork_choice( + persisted_fork_choice: &mut PersistedForkChoiceV7, + db: Arc>, +) -> Result<(), StoreError> { + // `PersistedForkChoice` stores the `ProtoArray` as a `Vec`. Deserialize these + // bytes assuming the legacy struct, and transform them to the new struct before + // re-serializing. + let ssz_container_v6 = + SszContainerV6::from_ssz_bytes(&persisted_fork_choice.fork_choice.proto_array_bytes) + .map_err(|e| { + StoreError::SchemaMigrationError(format!( + "Failed to decode ProtoArrayForkChoice during schema migration: {:?}", + e + )) + })?; + + // Clone the V6 proto nodes in order to maintain information about `node.justified_epoch` + // and `node.finalized_epoch`. + let nodes_v6 = ssz_container_v6.nodes.clone(); + + let justified_checkpoint = persisted_fork_choice.fork_choice_store.justified_checkpoint; + let finalized_checkpoint = persisted_fork_choice.fork_choice_store.finalized_checkpoint; + + // These transformations instantiate `node.justified_checkpoint` and `node.finalized_checkpoint` + // to `None`. + let ssz_container_v7: SszContainerV7 = + ssz_container_v6.into_ssz_container_v7(justified_checkpoint, finalized_checkpoint); + let ssz_container: SszContainer = ssz_container_v7.into(); + let mut fork_choice: ProtoArrayForkChoice = ssz_container.into(); + + update_checkpoints::(finalized_checkpoint.root, &nodes_v6, &mut fork_choice, db) + .map_err(StoreError::SchemaMigrationError)?; + + // Update the justified checkpoint in the store in case we have a discrepancy + // between the store and the proto array nodes. + update_store_justified_checkpoint(persisted_fork_choice, &mut fork_choice) + .map_err(StoreError::SchemaMigrationError)?; + + Ok(()) +} + +struct HeadInfo { + index: usize, + root: Hash256, + slot: Slot, +} + +fn update_checkpoints( + finalized_root: Hash256, + nodes_v6: &[ProtoNodeV6], + fork_choice: &mut ProtoArrayForkChoice, + db: Arc>, +) -> Result<(), String> { + let heads = find_finalized_descendant_heads(finalized_root, fork_choice); + + // For each head, first gather all epochs we will need to find justified or finalized roots for. + for head in heads { + // `relevant_epochs` are epochs for which we will need to find the root at the start slot. + // We don't need to worry about whether the are finalized or justified epochs. + let mut relevant_epochs = HashSet::new(); + let relevant_epoch_finder = |index, _: &mut ProtoNode| { + let (justified_epoch, finalized_epoch) = nodes_v6 + .get(index) + .map(|node: &ProtoNodeV6| (node.justified_epoch, node.finalized_epoch)) + .ok_or_else(|| "Index not found in legacy proto nodes".to_string())?; + relevant_epochs.insert(justified_epoch); + relevant_epochs.insert(finalized_epoch); + Ok(()) + }; + + apply_to_chain_of_ancestors( + finalized_root, + head.index, + fork_choice, + relevant_epoch_finder, + )?; + + // find the block roots associated with each relevant epoch. + let roots_by_epoch = + map_relevant_epochs_to_roots::(head.root, head.slot, relevant_epochs, db.clone())?; + + // Apply this mutator to the chain of descendants from this head, adding justified + // and finalized checkpoints for each. + let node_mutator = |index, node: &mut ProtoNode| { + let (justified_epoch, finalized_epoch) = nodes_v6 + .get(index) + .map(|node: &ProtoNodeV6| (node.justified_epoch, node.finalized_epoch)) + .ok_or_else(|| "Index not found in legacy proto nodes".to_string())?; + + // Update the checkpoints only if they haven't already been populated. + if node.justified_checkpoint.is_none() { + let justified_checkpoint = + roots_by_epoch + .get(&justified_epoch) + .map(|&root| Checkpoint { + epoch: justified_epoch, + root, + }); + node.justified_checkpoint = justified_checkpoint; + } + if node.finalized_checkpoint.is_none() { + let finalized_checkpoint = + roots_by_epoch + .get(&finalized_epoch) + .map(|&root| Checkpoint { + epoch: finalized_epoch, + root, + }); + node.finalized_checkpoint = finalized_checkpoint; + } + + Ok(()) + }; + + apply_to_chain_of_ancestors(finalized_root, head.index, fork_choice, node_mutator)?; + } + Ok(()) +} + +/// Coverts the given `HashSet` to a `Vec` then reverse sorts by `Epoch`. Next, a +/// single `BlockRootsIterator` is created which is used to iterate backwards from the given +/// `head_root` and `head_slot`, finding the block root at the start slot of each epoch. +fn map_relevant_epochs_to_roots( + head_root: Hash256, + head_slot: Slot, + epochs: HashSet, + db: Arc>, +) -> Result, String> { + // Convert the `HashSet` to a `Vec` and reverse sort the epochs. + let mut relevant_epochs = epochs.into_iter().collect::>(); + relevant_epochs.sort_unstable_by(|a, b| b.cmp(a)); + + // Iterate backwards from the given `head_root` and `head_slot` and find the block root at each epoch. + let mut iter = std::iter::once(Ok((head_root, head_slot))) + .chain(BlockRootsIterator::from_block(db, head_root).map_err(|e| format!("{:?}", e))?); + let mut roots_by_epoch = HashMap::new(); + for epoch in relevant_epochs { + let start_slot = epoch.start_slot(T::EthSpec::slots_per_epoch()); + + let root = iter + .find_map(|next| match next { + Ok((root, slot)) => (slot == start_slot).then(|| Ok(root)), + Err(e) => Some(Err(format!("{:?}", e))), + }) + .transpose()? + .ok_or_else(|| "Justified root not found".to_string())?; + roots_by_epoch.insert(epoch, root); + } + Ok(roots_by_epoch) +} + +/// Applies a mutator to every node in a chain, starting from the node at the given +/// `head_index` and iterating through ancestors until the `finalized_root` is reached. +fn apply_to_chain_of_ancestors( + finalized_root: Hash256, + head_index: usize, + fork_choice: &mut ProtoArrayForkChoice, + mut node_mutator: F, +) -> Result<(), String> +where + F: FnMut(usize, &mut ProtoNode) -> Result<(), String>, +{ + let head = fork_choice + .core_proto_array_mut() + .nodes + .get_mut(head_index) + .ok_or_else(|| "Head index not found in proto nodes".to_string())?; + + node_mutator(head_index, head)?; + + let mut parent_index_opt = head.parent; + let mut parent_opt = + parent_index_opt.and_then(|index| fork_choice.core_proto_array_mut().nodes.get_mut(index)); + + // Iterate backwards through all parents until there is no reference to a parent or we reach + // the `finalized_root` node. + while let (Some(parent), Some(parent_index)) = (parent_opt, parent_index_opt) { + node_mutator(parent_index, parent)?; + + // Break out of this while loop *after* the `node_mutator` has been applied to the finalized + // node. + if parent.root == finalized_root { + break; + } + + // Update parent values + parent_index_opt = parent.parent; + parent_opt = parent_index_opt + .and_then(|index| fork_choice.core_proto_array_mut().nodes.get_mut(index)); + } + Ok(()) +} + +/// Finds all heads by finding all nodes in the proto array that are not referenced as parents. Then +/// checks that these nodes are descendants of the finalized root in order to determine if they are +/// relevant. +fn find_finalized_descendant_heads( + finalized_root: Hash256, + fork_choice: &ProtoArrayForkChoice, +) -> Vec { + let nodes_referenced_as_parents: HashSet = fork_choice + .core_proto_array() + .nodes + .iter() + .filter_map(|node| node.parent) + .collect::>(); + + fork_choice + .core_proto_array() + .nodes + .iter() + .enumerate() + .filter_map(|(index, node)| { + (!nodes_referenced_as_parents.contains(&index) + && fork_choice.is_descendant(finalized_root, node.root)) + .then(|| HeadInfo { + index, + root: node.root, + slot: node.slot, + }) + }) + .collect::>() +} + +fn update_store_justified_checkpoint( + persisted_fork_choice: &mut PersistedForkChoiceV7, + fork_choice: &mut ProtoArrayForkChoice, +) -> Result<(), String> { + let justified_checkpoint = fork_choice + .core_proto_array() + .nodes + .iter() + .filter_map(|node| { + (node.finalized_checkpoint + == Some(persisted_fork_choice.fork_choice_store.finalized_checkpoint)) + .then(|| node.justified_checkpoint) + .flatten() + }) + .max_by_key(|justified_checkpoint| justified_checkpoint.epoch) + .ok_or("Proto node with current finalized checkpoint not found")?; + + fork_choice.core_proto_array_mut().justified_checkpoint = justified_checkpoint; + persisted_fork_choice.fork_choice.proto_array_bytes = fork_choice.as_bytes(); + persisted_fork_choice.fork_choice_store.justified_checkpoint = justified_checkpoint; + Ok(()) +} + +// Add a zero `proposer_boost_root` when migrating from V1-6 to V7. +impl From for PersistedForkChoiceStoreV7 { + fn from(other: PersistedForkChoiceStoreV1) -> Self { + Self { + balances_cache: other.balances_cache, + time: other.time, + finalized_checkpoint: other.finalized_checkpoint, + justified_checkpoint: other.justified_checkpoint, + justified_balances: other.justified_balances, + best_justified_checkpoint: other.best_justified_checkpoint, + proposer_boost_root: Hash256::zero(), + } + } +} + +impl From for PersistedForkChoiceV7 { + fn from(other: PersistedForkChoiceV1) -> Self { + Self { + fork_choice: other.fork_choice, + fork_choice_store: other.fork_choice_store.into(), + } + } +} diff --git a/beacon_node/beacon_chain/src/schema_change/types.rs b/beacon_node/beacon_chain/src/schema_change/types.rs new file mode 100644 index 000000000..8d41a384f --- /dev/null +++ b/beacon_node/beacon_chain/src/schema_change/types.rs @@ -0,0 +1,192 @@ +use crate::types::{AttestationShufflingId, Checkpoint, Epoch, Hash256, Slot}; +use proto_array::core::{ProposerBoost, ProtoNode, SszContainer, VoteTracker}; +use proto_array::ExecutionStatus; +use ssz::four_byte_option_impl; +use ssz::Encode; +use ssz_derive::{Decode, Encode}; +use superstruct::superstruct; + +// Define a "legacy" implementation of `Option` which uses four bytes for encoding the union +// selector. +four_byte_option_impl!(four_byte_option_usize, usize); +four_byte_option_impl!(four_byte_option_checkpoint, Checkpoint); + +#[superstruct( + variants(V1, V6, V7), + variant_attributes(derive(Clone, PartialEq, Debug, Encode, Decode)), + no_enum +)] +pub struct ProtoNode { + 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, + #[superstruct(only(V1, V6))] + pub justified_epoch: Epoch, + #[superstruct(only(V1, V6))] + pub finalized_epoch: Epoch, + #[ssz(with = "four_byte_option_checkpoint")] + #[superstruct(only(V7))] + pub justified_checkpoint: Option, + #[ssz(with = "four_byte_option_checkpoint")] + #[superstruct(only(V7))] + pub finalized_checkpoint: Option, + pub weight: u64, + #[ssz(with = "four_byte_option_usize")] + pub best_child: Option, + #[ssz(with = "four_byte_option_usize")] + pub best_descendant: Option, + #[superstruct(only(V6, V7))] + pub execution_status: ExecutionStatus, +} + +impl Into for ProtoNodeV1 { + fn into(self) -> ProtoNodeV6 { + ProtoNodeV6 { + 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, + // We set the following execution value as if the block is a pre-merge-fork block. This + // is safe as long as we never import a merge block with the old version of proto-array. + // This will be safe since we can't actually process merge blocks until we've made this + // change to fork choice. + execution_status: ExecutionStatus::irrelevant(), + } + } +} + +impl Into for ProtoNodeV6 { + fn into(self) -> ProtoNodeV7 { + ProtoNodeV7 { + 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_checkpoint: None, + finalized_checkpoint: None, + weight: self.weight, + best_child: self.best_child, + best_descendant: self.best_descendant, + execution_status: self.execution_status, + } + } +} + +impl Into for ProtoNodeV7 { + 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_checkpoint: self.justified_checkpoint, + finalized_checkpoint: self.finalized_checkpoint, + weight: self.weight, + best_child: self.best_child, + best_descendant: self.best_descendant, + execution_status: self.execution_status, + } + } +} + +#[superstruct( + variants(V1, V6, V7), + variant_attributes(derive(Encode, Decode)), + no_enum +)] +#[derive(Encode, Decode)] +pub struct SszContainer { + pub votes: Vec, + pub balances: Vec, + pub prune_threshold: usize, + #[superstruct(only(V1, V6))] + pub justified_epoch: Epoch, + #[superstruct(only(V1, V6))] + pub finalized_epoch: Epoch, + #[superstruct(only(V7))] + pub justified_checkpoint: Checkpoint, + #[superstruct(only(V7))] + pub finalized_checkpoint: Checkpoint, + #[superstruct(only(V1))] + pub nodes: Vec, + #[superstruct(only(V6))] + pub nodes: Vec, + #[superstruct(only(V7))] + pub nodes: Vec, + pub indices: Vec<(Hash256, usize)>, + #[superstruct(only(V7))] + pub previous_proposer_boost: ProposerBoost, +} + +impl Into for SszContainerV1 { + fn into(self) -> SszContainerV6 { + let nodes = self.nodes.into_iter().map(Into::into).collect(); + + SszContainerV6 { + 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 SszContainerV6 { + pub(crate) fn into_ssz_container_v7( + self, + justified_checkpoint: Checkpoint, + finalized_checkpoint: Checkpoint, + ) -> SszContainerV7 { + let nodes = self.nodes.into_iter().map(Into::into).collect(); + + SszContainerV7 { + votes: self.votes, + balances: self.balances, + prune_threshold: self.prune_threshold, + justified_checkpoint, + finalized_checkpoint, + nodes, + indices: self.indices, + previous_proposer_boost: ProposerBoost::default(), + } + } +} + +impl Into for SszContainerV7 { + 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_checkpoint: self.justified_checkpoint, + finalized_checkpoint: self.finalized_checkpoint, + nodes, + indices: self.indices, + previous_proposer_boost: self.previous_proposer_boost, + } + } +} diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index f9af16bbe..24aba9e20 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -2223,9 +2223,10 @@ fn assert_chains_pretty_much_the_same(a: &BeaconChain, b ); let slot = a.slot().unwrap(); + let spec = T::EthSpec::default_spec(); assert!( - a.fork_choice.write().get_head(slot).unwrap() - == b.fork_choice.write().get_head(slot).unwrap(), + a.fork_choice.write().get_head(slot, &spec).unwrap() + == b.fork_choice.write().get_head(slot, &spec).unwrap(), "fork_choice heads should be equal" ); } diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index bcf0ee198..30bc34dda 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -23,7 +23,7 @@ use monitoring_api::{MonitoringHttpClient, ProcessType}; use network::{NetworkConfig, NetworkMessage, NetworkService}; use slasher::Slasher; use slasher_service::SlasherService; -use slog::{debug, info, warn}; +use slog::{debug, info, warn, Logger}; use std::net::TcpListener; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -748,6 +748,7 @@ where hot_path: &Path, cold_path: &Path, config: StoreConfig, + log: Logger, ) -> Result { let context = self .runtime_context @@ -763,7 +764,7 @@ where self.freezer_db_path = Some(cold_path.into()); let schema_upgrade = |db, from, to| { - migrate_schema::>(db, datadir, from, to) + migrate_schema::>(db, datadir, from, to, log) }; let store = HotColdDB::open( diff --git a/beacon_node/lighthouse_network/Cargo.toml b/beacon_node/lighthouse_network/Cargo.toml index 710a705f0..4945dbfdf 100644 --- a/beacon_node/lighthouse_network/Cargo.toml +++ b/beacon_node/lighthouse_network/Cargo.toml @@ -37,7 +37,7 @@ rand = "0.7.3" directory = { path = "../../common/directory" } regex = "1.3.9" strum = { version = "0.21.0", features = ["derive"] } -superstruct = "0.2.0" +superstruct = "0.3.0" [dependencies.libp2p] version = "0.41.0" diff --git a/beacon_node/src/lib.rs b/beacon_node/src/lib.rs index 4ff474571..773a0d2eb 100644 --- a/beacon_node/src/lib.rs +++ b/beacon_node/src/lib.rs @@ -84,7 +84,13 @@ impl ProductionBeaconNode { .runtime_context(context) .chain_spec(spec) .http_api_config(client_config.http_api.clone()) - .disk_store(&datadir, &db_path, &freezer_db_path, store_config)?; + .disk_store( + &datadir, + &db_path, + &freezer_db_path, + store_config, + log.clone(), + )?; let builder = if let Some(slasher_config) = client_config.slasher.clone() { let slasher = Arc::new( diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index cfa49847d..05a0eb3dd 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -975,6 +975,21 @@ impl, Cold: ItemStore> HotColdDB self.hot_db.put(&SCHEMA_VERSION_KEY, &schema_version) } + /// Store the database schema version atomically with additional operations. + pub fn store_schema_version_atomically( + &self, + schema_version: SchemaVersion, + mut ops: Vec, + ) -> Result<(), Error> { + let column = SchemaVersion::db_column().into(); + let key = SCHEMA_VERSION_KEY.as_bytes(); + let db_key = get_key_for_col(column, key); + let op = KeyValueStoreOp::PutKeyValue(db_key, schema_version.as_store_bytes()); + ops.push(op); + + self.hot_db.do_atomically(ops) + } + /// Initialise the anchor info for checkpoint sync starting from `block`. pub fn init_anchor_info(&self, block: BeaconBlockRef<'_, E>) -> Result { let anchor_slot = block.slot(); diff --git a/beacon_node/store/src/iter.rs b/beacon_node/store/src/iter.rs index 8b11a6cc9..a4d34cd3c 100644 --- a/beacon_node/store/src/iter.rs +++ b/beacon_node/store/src/iter.rs @@ -125,6 +125,15 @@ impl<'a, T: EthSpec, Hot: ItemStore, Cold: ItemStore> BlockRootsIterator<' inner: RootsIterator::owned(store, beacon_state), } } + + pub fn from_block( + store: Arc>, + block_hash: Hash256, + ) -> Result { + Ok(Self { + inner: RootsIterator::from_block(store, block_hash)?, + }) + } } impl<'a, T: EthSpec, Hot: ItemStore, Cold: ItemStore> Iterator diff --git a/beacon_node/store/src/metadata.rs b/beacon_node/store/src/metadata.rs index cc0535ef5..17800bb6c 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(6); +pub const CURRENT_SCHEMA_VERSION: SchemaVersion = SchemaVersion(7); // All the keys that get stored under the `BeaconMeta` column. // diff --git a/common/eth2_network_config/built_in_network_configs/mainnet/config.yaml b/common/eth2_network_config/built_in_network_configs/mainnet/config.yaml index a1d305bac..4d17356ce 100644 --- a/common/eth2_network_config/built_in_network_configs/mainnet/config.yaml +++ b/common/eth2_network_config/built_in_network_configs/mainnet/config.yaml @@ -71,6 +71,12 @@ MIN_PER_EPOCH_CHURN_LIMIT: 4 CHURN_LIMIT_QUOTIENT: 65536 +# Fork choice +# --------------------------------------------------------------- +# TODO: enable once proposer boosting is desired on mainnet +# 70% +# PROPOSER_SCORE_BOOST: 70 + # Deposit contract # --------------------------------------------------------------- # Ethereum PoW Mainnet diff --git a/common/eth2_network_config/built_in_network_configs/prater/config.yaml b/common/eth2_network_config/built_in_network_configs/prater/config.yaml index 5fc23d6af..aa375ab2e 100644 --- a/common/eth2_network_config/built_in_network_configs/prater/config.yaml +++ b/common/eth2_network_config/built_in_network_configs/prater/config.yaml @@ -71,6 +71,11 @@ MIN_PER_EPOCH_CHURN_LIMIT: 4 CHURN_LIMIT_QUOTIENT: 65536 +# Fork choice +# --------------------------------------------------------------- +# 70% +PROPOSER_SCORE_BOOST: 70 + # Deposit contract # --------------------------------------------------------------- # Ethereum Goerli testnet diff --git a/common/eth2_network_config/built_in_network_configs/pyrmont/config.yaml b/common/eth2_network_config/built_in_network_configs/pyrmont/config.yaml index 352a4e918..b5f841580 100644 --- a/common/eth2_network_config/built_in_network_configs/pyrmont/config.yaml +++ b/common/eth2_network_config/built_in_network_configs/pyrmont/config.yaml @@ -71,6 +71,11 @@ MIN_PER_EPOCH_CHURN_LIMIT: 4 CHURN_LIMIT_QUOTIENT: 65536 +# Fork choice +# --------------------------------------------------------------- +# 70% +PROPOSER_SCORE_BOOST: 70 + # Deposit contract # --------------------------------------------------------------- # Ethereum Goerli testnet diff --git a/common/slot_clock/src/lib.rs b/common/slot_clock/src/lib.rs index 9fa24a022..f50931c6f 100644 --- a/common/slot_clock/src/lib.rs +++ b/common/slot_clock/src/lib.rs @@ -11,6 +11,7 @@ pub use crate::manual_slot_clock::ManualSlotClock; pub use crate::manual_slot_clock::ManualSlotClock as TestingSlotClock; pub use crate::system_time_slot_clock::SystemTimeSlotClock; pub use metrics::scrape_for_metrics; +use types::consts::merge::INTERVALS_PER_SLOT; pub use types::Slot; /// A clock that reports the current slot. @@ -82,24 +83,33 @@ pub trait SlotClock: Send + Sync + Sized + Clone { /// Returns the delay between the start of the slot and when unaggregated attestations should be /// produced. fn unagg_attestation_production_delay(&self) -> Duration { - self.slot_duration() / 3 + self.slot_duration() / INTERVALS_PER_SLOT as u32 } /// Returns the delay between the start of the slot and when sync committee messages should be /// produced. fn sync_committee_message_production_delay(&self) -> Duration { - self.slot_duration() / 3 + self.slot_duration() / INTERVALS_PER_SLOT as u32 } /// Returns the delay between the start of the slot and when aggregated attestations should be /// produced. fn agg_attestation_production_delay(&self) -> Duration { - self.slot_duration() * 2 / 3 + self.slot_duration() * 2 / INTERVALS_PER_SLOT as u32 } /// Returns the delay between the start of the slot and when partially aggregated `SyncCommitteeContribution` should be /// produced. fn sync_committee_contribution_production_delay(&self) -> Duration { - self.slot_duration() * 2 / 3 + self.slot_duration() * 2 / INTERVALS_PER_SLOT as u32 + } + + /// Returns the `Duration` since the start of the current `Slot`. Useful in determining whether to apply proposer boosts. + fn seconds_from_current_slot_start(&self, seconds_per_slot: u64) -> Option { + self.now_duration() + .and_then(|now| now.checked_sub(self.genesis_duration())) + .map(|duration_into_slot| { + Duration::from_secs(duration_into_slot.as_secs() % seconds_per_slot) + }) } } diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 93ed1c3ba..86b32aab1 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1,14 +1,14 @@ -use std::marker::PhantomData; - +use crate::ForkChoiceStore; use proto_array::{Block as ProtoBlock, ExecutionStatus, ProtoArrayForkChoice}; use ssz_derive::{Decode, Encode}; -use types::{ - AttestationShufflingId, BeaconBlock, BeaconState, BeaconStateError, ChainSpec, Checkpoint, - Epoch, EthSpec, Hash256, IndexedAttestation, RelativeEpoch, SignedBeaconBlock, Slot, -}; - -use crate::ForkChoiceStore; use std::cmp::Ordering; +use std::marker::PhantomData; +use std::time::Duration; +use types::{ + consts::merge::INTERVALS_PER_SLOT, AttestationShufflingId, BeaconBlock, BeaconState, + BeaconStateError, ChainSpec, Checkpoint, Epoch, EthSpec, Hash256, IndexedAttestation, + RelativeEpoch, SignedBeaconBlock, Slot, +}; #[derive(Debug)] pub enum Error { @@ -168,6 +168,13 @@ where store.set_current_slot(time); let current_slot = store.get_current_slot(); + + // Reset proposer boost if this is a new slot. + if current_slot > previous_slot { + store.set_proposer_boost_root(Hash256::zero()); + } + + // Not a new epoch, return. if !(current_slot > previous_slot && compute_slots_since_epoch_start::(current_slot) == 0) { return Ok(()); } @@ -218,6 +225,15 @@ fn dequeue_attestations( std::mem::replace(queued_attestations, remaining) } +/// Denotes whether an attestation we are processing was received from a block or from gossip. +/// Equivalent to the `is_from_block` `bool` in: +/// +/// https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/fork-choice.md#validate_on_attestation +pub enum AttestationFromBlock { + True, + False, +} + /// Provides an implementation of "Ethereum 2.0 Phase 0 -- Beacon Chain Fork Choice": /// /// https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/fork-choice.md#ethereum-20-phase-0----beacon-chain-fork-choice @@ -292,9 +308,8 @@ where let proto_array = ProtoArrayForkChoice::new( finalized_block_slot, finalized_block_state_root, - fc_store.justified_checkpoint().epoch, - fc_store.finalized_checkpoint().epoch, - fc_store.finalized_checkpoint().root, + *fc_store.justified_checkpoint(), + *fc_store.finalized_checkpoint(), current_epoch_shuffling_id, next_epoch_shuffling_id, execution_status, @@ -377,17 +392,22 @@ where /// Is equivalent to: /// /// https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/fork-choice.md#get_head - pub fn get_head(&mut self, current_slot: Slot) -> Result> { + pub fn get_head( + &mut self, + current_slot: Slot, + spec: &ChainSpec, + ) -> Result> { self.update_time(current_slot)?; let store = &mut self.fc_store; self.proto_array - .find_head( - store.justified_checkpoint().epoch, - store.justified_checkpoint().root, - store.finalized_checkpoint().epoch, + .find_head::( + *store.justified_checkpoint(), + *store.finalized_checkpoint(), store.justified_balances(), + store.proposer_boost_root(), + spec, ) .map_err(Into::into) } @@ -462,11 +482,13 @@ where /// /// The supplied block **must** pass the `state_transition` function as it will not be run /// here. + #[allow(clippy::too_many_arguments)] pub fn on_block( &mut self, current_slot: Slot, block: &BeaconBlock, block_root: Hash256, + block_delay: Duration, state: &BeaconState, payload_verification_status: PayloadVerificationStatus, spec: &ChainSpec, @@ -520,6 +542,13 @@ where })); } + // Add proposer score boost if the block is timely. + let is_before_attesting_interval = + block_delay < Duration::from_secs(spec.seconds_per_slot / INTERVALS_PER_SLOT); + if current_slot == block.slot() && is_before_attesting_interval { + self.fc_store.set_proposer_boost_root(block_root); + } + // Update justified checkpoint. if state.current_justified_checkpoint().epoch > self.fc_store.justified_checkpoint().epoch { if state.current_justified_checkpoint().epoch @@ -539,25 +568,9 @@ where if state.finalized_checkpoint().epoch > self.fc_store.finalized_checkpoint().epoch { self.fc_store .set_finalized_checkpoint(state.finalized_checkpoint()); - let finalized_slot = - compute_start_slot_at_epoch::(self.fc_store.finalized_checkpoint().epoch); - - // Note: the `if` statement here is not part of the specification, but I claim that it - // is an optimization and equivalent to the specification. See this PR for more - // information: - // - // https://github.com/ethereum/eth2.0-specs/pull/1880 - if *self.fc_store.justified_checkpoint() != state.current_justified_checkpoint() - && (state.current_justified_checkpoint().epoch - > self.fc_store.justified_checkpoint().epoch - || self - .get_ancestor(self.fc_store.justified_checkpoint().root, finalized_slot)? - != Some(self.fc_store.finalized_checkpoint().root)) - { - self.fc_store - .set_justified_checkpoint(state.current_justified_checkpoint()) - .map_err(Error::UnableToSetJustifiedCheckpoint)?; - } + self.fc_store + .set_justified_checkpoint(state.current_justified_checkpoint()) + .map_err(Error::UnableToSetJustifiedCheckpoint)?; } let target_slot = block @@ -623,14 +636,43 @@ where ) .map_err(Error::BeaconStateError)?, state_root: block.state_root(), - justified_epoch: state.current_justified_checkpoint().epoch, - finalized_epoch: state.finalized_checkpoint().epoch, + justified_checkpoint: state.current_justified_checkpoint(), + finalized_checkpoint: state.finalized_checkpoint(), execution_status, })?; Ok(()) } + /// Validates the `epoch` against the current time according to the fork choice store. + /// + /// ## Specification + /// + /// Equivalent to: + /// + /// https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/fork-choice.md#validate_target_epoch_against_current_time + fn validate_target_epoch_against_current_time( + &self, + target_epoch: Epoch, + ) -> Result<(), InvalidAttestation> { + let slot_now = self.fc_store.get_current_slot(); + let epoch_now = slot_now.epoch(E::slots_per_epoch()); + + // Attestation must be from the current or previous epoch. + if target_epoch > epoch_now { + return Err(InvalidAttestation::FutureEpoch { + attestation_epoch: target_epoch, + current_epoch: epoch_now, + }); + } else if target_epoch + 1 < epoch_now { + return Err(InvalidAttestation::PastEpoch { + attestation_epoch: target_epoch, + current_epoch: epoch_now, + }); + } + Ok(()) + } + /// Validates the `indexed_attestation` for application to fork choice. /// /// ## Specification @@ -641,6 +683,7 @@ where fn validate_on_attestation( &self, indexed_attestation: &IndexedAttestation, + is_from_block: AttestationFromBlock, ) -> Result<(), InvalidAttestation> { // There is no point in processing an attestation with an empty bitfield. Reject // it immediately. @@ -651,21 +694,10 @@ where return Err(InvalidAttestation::EmptyAggregationBitfield); } - let slot_now = self.fc_store.get_current_slot(); - let epoch_now = slot_now.epoch(E::slots_per_epoch()); let target = indexed_attestation.data.target; - // Attestation must be from the current or previous epoch. - if target.epoch > epoch_now { - return Err(InvalidAttestation::FutureEpoch { - attestation_epoch: target.epoch, - current_epoch: epoch_now, - }); - } else if target.epoch + 1 < epoch_now { - return Err(InvalidAttestation::PastEpoch { - attestation_epoch: target.epoch, - current_epoch: epoch_now, - }); + if matches!(is_from_block, AttestationFromBlock::False) { + self.validate_target_epoch_against_current_time(target.epoch)?; } if target.epoch != indexed_attestation.data.slot.epoch(E::slots_per_epoch()) { @@ -748,6 +780,7 @@ where &mut self, current_slot: Slot, attestation: &IndexedAttestation, + is_from_block: AttestationFromBlock, ) -> Result<(), Error> { // Ensure the store is up-to-date. self.update_time(current_slot)?; @@ -769,7 +802,7 @@ where return Ok(()); } - self.validate_on_attestation(attestation)?; + self.validate_on_attestation(attestation, is_from_block)?; if attestation.data.slot < self.fc_store.get_current_slot() { for validator_index in attestation.attesting_indices.iter() { @@ -895,6 +928,11 @@ where &self.queued_attestations } + /// Returns the store's `proposer_boost_root`. + pub fn proposer_boost_root(&self) -> Hash256 { + self.fc_store.proposer_boost_root() + } + /// Prunes the underlying fork choice DAG. pub fn prune(&mut self) -> Result<(), Error> { let finalized_root = self.fc_store.finalized_checkpoint().root; diff --git a/consensus/fork_choice/src/fork_choice_store.rs b/consensus/fork_choice/src/fork_choice_store.rs index c74610cc0..9b85708f3 100644 --- a/consensus/fork_choice/src/fork_choice_store.rs +++ b/consensus/fork_choice/src/fork_choice_store.rs @@ -19,7 +19,7 @@ use types::{BeaconBlock, BeaconState, Checkpoint, EthSpec, Hash256, Slot}; pub trait ForkChoiceStore: Sized { type Error; - /// Returns the last value passed to `Self::update_time`. + /// Returns the last value passed to `Self::set_current_slot`. fn get_current_slot(&self) -> Slot; /// Set the value to be returned by `Self::get_current_slot`. @@ -50,6 +50,9 @@ pub trait ForkChoiceStore: Sized { /// Returns the `finalized_checkpoint`. fn finalized_checkpoint(&self) -> &Checkpoint; + /// Returns the `proposer_boost_root`. + fn proposer_boost_root(&self) -> Hash256; + /// Sets `finalized_checkpoint`. fn set_finalized_checkpoint(&mut self, checkpoint: Checkpoint); @@ -58,4 +61,7 @@ pub trait ForkChoiceStore: Sized { /// Sets the `best_justified_checkpoint`. fn set_best_justified_checkpoint(&mut self, checkpoint: Checkpoint); + + /// Sets the proposer boost root. + fn set_proposer_boost_root(&mut self, proposer_boost_root: Hash256); } diff --git a/consensus/fork_choice/src/lib.rs b/consensus/fork_choice/src/lib.rs index 7dd80b798..ba031cdf7 100644 --- a/consensus/fork_choice/src/lib.rs +++ b/consensus/fork_choice/src/lib.rs @@ -2,8 +2,8 @@ mod fork_choice; mod fork_choice_store; pub use crate::fork_choice::{ - Error, ForkChoice, InvalidAttestation, InvalidBlock, PayloadVerificationStatus, - PersistedForkChoice, QueuedAttestation, + AttestationFromBlock, Error, ForkChoice, InvalidAttestation, InvalidBlock, + PayloadVerificationStatus, PersistedForkChoice, QueuedAttestation, }; pub use fork_choice_store::ForkChoiceStore; pub use proto_array::Block as ProtoBlock; diff --git a/consensus/fork_choice/tests/tests.rs b/consensus/fork_choice/tests/tests.rs index 129b79c39..42b56f6ab 100644 --- a/consensus/fork_choice/tests/tests.rs +++ b/consensus/fork_choice/tests/tests.rs @@ -2,6 +2,7 @@ use std::fmt; use std::sync::Mutex; +use std::time::Duration; use beacon_chain::test_utils::{ AttestationStrategy, BeaconChainHarness, BlockStrategy, EphemeralHarnessType, @@ -274,6 +275,7 @@ impl ForkChoiceTest { current_slot, &block, block.canonical_root(), + Duration::from_secs(0), &state, PayloadVerificationStatus::Verified, &self.harness.chain.spec, @@ -316,6 +318,7 @@ impl ForkChoiceTest { current_slot, &block, block.canonical_root(), + Duration::from_secs(0), &state, PayloadVerificationStatus::Verified, &self.harness.chain.spec, diff --git a/consensus/proto_array/src/error.rs b/consensus/proto_array/src/error.rs index c3892bde5..adb10c035 100644 --- a/consensus/proto_array/src/error.rs +++ b/consensus/proto_array/src/error.rs @@ -1,4 +1,4 @@ -use types::{Epoch, Hash256}; +use types::{Checkpoint, Epoch, Hash256}; #[derive(Clone, PartialEq, Debug)] pub enum Error { @@ -13,6 +13,7 @@ pub enum Error { InvalidParentDelta(usize), InvalidNodeDelta(usize), DeltaOverflow(usize), + ProposerBoostOverflow(usize), IndexOverflow(&'static str), InvalidDeltaLen { deltas: usize, @@ -22,16 +23,19 @@ pub enum Error { current_finalized_epoch: Epoch, new_finalized_epoch: Epoch, }, - InvalidBestNode { - start_root: Hash256, - justified_epoch: Epoch, - finalized_epoch: Epoch, - head_root: Hash256, - head_justified_epoch: Epoch, - head_finalized_epoch: Epoch, - }, + InvalidBestNode(Box), InvalidAncestorOfValidPayload { ancestor_block_root: Hash256, ancestor_payload_block_hash: Hash256, }, } + +#[derive(Clone, PartialEq, Debug)] +pub struct InvalidBestNodeInfo { + pub start_root: Hash256, + pub justified_checkpoint: Checkpoint, + pub finalized_checkpoint: Checkpoint, + pub head_root: Hash256, + pub head_justified_checkpoint: Option, + pub head_finalized_checkpoint: Option, +} diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index 44036911c..e28fc6771 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -4,7 +4,7 @@ mod votes; use crate::proto_array_fork_choice::{Block, ExecutionStatus, ProtoArrayForkChoice}; use serde_derive::{Deserialize, Serialize}; -use types::{AttestationShufflingId, Epoch, Hash256, Slot}; +use types::{AttestationShufflingId, Checkpoint, Epoch, EthSpec, Hash256, MainnetEthSpec, Slot}; pub use ffg_updates::*; pub use no_votes::*; @@ -13,24 +13,22 @@ pub use votes::*; #[derive(Debug, Clone, Serialize, Deserialize)] pub enum Operation { FindHead { - justified_epoch: Epoch, - justified_root: Hash256, - finalized_epoch: Epoch, + justified_checkpoint: Checkpoint, + finalized_checkpoint: Checkpoint, justified_state_balances: Vec, expected_head: Hash256, }, InvalidFindHead { - justified_epoch: Epoch, - justified_root: Hash256, - finalized_epoch: Epoch, + justified_checkpoint: Checkpoint, + finalized_checkpoint: Checkpoint, justified_state_balances: Vec, }, ProcessBlock { slot: Slot, root: Hash256, parent_root: Hash256, - justified_epoch: Epoch, - finalized_epoch: Epoch, + justified_checkpoint: Checkpoint, + finalized_checkpoint: Checkpoint, }, ProcessAttestation { validator_index: usize, @@ -47,9 +45,8 @@ pub enum Operation { #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ForkChoiceTestDefinition { pub finalized_block_slot: Slot, - pub justified_epoch: Epoch, - pub finalized_epoch: Epoch, - pub finalized_root: Hash256, + pub justified_checkpoint: Checkpoint, + pub finalized_checkpoint: Checkpoint, pub operations: Vec, } @@ -61,9 +58,8 @@ impl ForkChoiceTestDefinition { let mut fork_choice = ProtoArrayForkChoice::new( self.finalized_block_slot, Hash256::zero(), - self.justified_epoch, - self.finalized_epoch, - self.finalized_root, + self.justified_checkpoint, + self.finalized_checkpoint, junk_shuffling_id.clone(), junk_shuffling_id, execution_status, @@ -73,21 +69,22 @@ impl ForkChoiceTestDefinition { for (op_index, op) in self.operations.into_iter().enumerate() { match op.clone() { Operation::FindHead { - justified_epoch, - justified_root, - finalized_epoch, + justified_checkpoint, + finalized_checkpoint, justified_state_balances, expected_head, } => { let head = fork_choice - .find_head( - justified_epoch, - justified_root, - finalized_epoch, + .find_head::( + justified_checkpoint, + finalized_checkpoint, &justified_state_balances, + Hash256::zero(), + &MainnetEthSpec::default_spec(), ) - .unwrap_or_else(|_| { - panic!("find_head op at index {} returned error", op_index) + .map_err(|e| e) + .unwrap_or_else(|e| { + panic!("find_head op at index {} returned error {}", op_index, e) }); assert_eq!( @@ -98,16 +95,16 @@ impl ForkChoiceTestDefinition { check_bytes_round_trip(&fork_choice); } Operation::InvalidFindHead { - justified_epoch, - justified_root, - finalized_epoch, + justified_checkpoint, + finalized_checkpoint, justified_state_balances, } => { - let result = fork_choice.find_head( - justified_epoch, - justified_root, - finalized_epoch, + let result = fork_choice.find_head::( + justified_checkpoint, + finalized_checkpoint, &justified_state_balances, + Hash256::zero(), + &MainnetEthSpec::default_spec(), ); assert!( @@ -122,8 +119,8 @@ impl ForkChoiceTestDefinition { slot, root, parent_root, - justified_epoch, - finalized_epoch, + justified_checkpoint, + finalized_checkpoint, } => { let block = Block { slot, @@ -139,8 +136,8 @@ impl ForkChoiceTestDefinition { Epoch::new(0), Hash256::zero(), ), - justified_epoch, - finalized_epoch, + justified_checkpoint, + finalized_checkpoint, execution_status, }; fork_choice.process_block(block).unwrap_or_else(|e| { @@ -193,7 +190,16 @@ impl ForkChoiceTestDefinition { /// Gives a hash that is not the zero hash (unless i is `usize::max_value)`. fn get_hash(i: u64) -> Hash256 { - Hash256::from_low_u64_be(i) + Hash256::from_low_u64_be(i + 1) +} + +/// Gives a checkpoint with a root that is not the zero hash (unless i is `usize::max_value)`. +/// `Epoch` will always equal `i`. +fn get_checkpoint(i: u64) -> Checkpoint { + Checkpoint { + epoch: Epoch::new(i), + root: get_hash(i), + } } fn check_bytes_round_trip(original: &ProtoArrayForkChoice) { diff --git a/consensus/proto_array/src/fork_choice_test_definition/ffg_updates.rs b/consensus/proto_array/src/fork_choice_test_definition/ffg_updates.rs index 4b7eb25d7..a12906450 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/ffg_updates.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/ffg_updates.rs @@ -6,9 +6,8 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { // Ensure that the head starts at the finalized block. ops.push(Operation::FindHead { - justified_epoch: Epoch::new(0), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_hash(0), }); @@ -26,22 +25,22 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { slot: Slot::new(1), root: get_hash(1), parent_root: get_hash(0), - justified_epoch: Epoch::new(0), - finalized_epoch: Epoch::new(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), }); ops.push(Operation::ProcessBlock { slot: Slot::new(2), root: get_hash(2), parent_root: get_hash(1), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(0), + justified_checkpoint: get_checkpoint(1), + finalized_checkpoint: get_checkpoint(0), }); ops.push(Operation::ProcessBlock { slot: Slot::new(3), root: get_hash(3), parent_root: get_hash(2), - justified_epoch: Epoch::new(2), - finalized_epoch: Epoch::new(1), + justified_checkpoint: get_checkpoint(2), + finalized_checkpoint: get_checkpoint(1), }); // Ensure that with justified epoch 0 we find 3 @@ -54,9 +53,8 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { // | // 3 <- head ops.push(Operation::FindHead { - justified_epoch: Epoch::new(0), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_hash(3), }); @@ -71,9 +69,8 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { // | // 3 <- head ops.push(Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: get_hash(2), - finalized_epoch: Epoch::new(0), + justified_checkpoint: get_checkpoint(1), + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_hash(2), }); @@ -88,9 +85,8 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { // | // 3 <- start + head ops.push(Operation::FindHead { - justified_epoch: Epoch::new(2), - justified_root: get_hash(3), - finalized_epoch: Epoch::new(1), + justified_checkpoint: get_checkpoint(2), + finalized_checkpoint: get_checkpoint(1), justified_state_balances: balances, expected_head: get_hash(3), }); @@ -98,9 +94,8 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { // END OF TESTS ForkChoiceTestDefinition { finalized_block_slot: Slot::new(0), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(1), - finalized_root: get_hash(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), operations: ops, } } @@ -111,9 +106,8 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { // Ensure that the head starts at the finalized block. ops.push(Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(1), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_hash(0), }); @@ -137,36 +131,48 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { slot: Slot::new(1), root: get_hash(1), parent_root: get_hash(0), - justified_epoch: Epoch::new(0), - finalized_epoch: Epoch::new(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), }); ops.push(Operation::ProcessBlock { slot: Slot::new(2), root: get_hash(3), parent_root: get_hash(1), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(1), + }, + finalized_checkpoint: get_checkpoint(0), }); ops.push(Operation::ProcessBlock { slot: Slot::new(3), root: get_hash(5), parent_root: get_hash(3), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(1), + }, + finalized_checkpoint: get_checkpoint(0), }); ops.push(Operation::ProcessBlock { slot: Slot::new(4), root: get_hash(7), parent_root: get_hash(5), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(1), + }, + finalized_checkpoint: get_checkpoint(0), }); ops.push(Operation::ProcessBlock { - slot: Slot::new(4), + slot: Slot::new(5), root: get_hash(9), parent_root: get_hash(7), - justified_epoch: Epoch::new(2), - finalized_epoch: Epoch::new(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(3), + }, + finalized_checkpoint: get_checkpoint(0), }); // Right branch @@ -174,36 +180,42 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { slot: Slot::new(1), root: get_hash(2), parent_root: get_hash(0), - justified_epoch: Epoch::new(0), - finalized_epoch: Epoch::new(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), }); ops.push(Operation::ProcessBlock { slot: Slot::new(2), root: get_hash(4), parent_root: get_hash(2), - justified_epoch: Epoch::new(0), - finalized_epoch: Epoch::new(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), }); ops.push(Operation::ProcessBlock { slot: Slot::new(3), root: get_hash(6), parent_root: get_hash(4), - justified_epoch: Epoch::new(0), - finalized_epoch: Epoch::new(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), }); ops.push(Operation::ProcessBlock { slot: Slot::new(4), root: get_hash(8), parent_root: get_hash(6), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(2), + }, + finalized_checkpoint: get_checkpoint(0), }); ops.push(Operation::ProcessBlock { - slot: Slot::new(4), + slot: Slot::new(5), root: get_hash(10), parent_root: get_hash(8), - justified_epoch: Epoch::new(2), - finalized_epoch: Epoch::new(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(4), + }, + finalized_checkpoint: get_checkpoint(0), }); // Ensure that if we start at 0 we find 10 (just: 0, fin: 0). @@ -220,25 +232,28 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { // | | // 9 10 <-- head ops.push(Operation::FindHead { - justified_epoch: Epoch::new(0), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_hash(10), }); // Same as above, but with justified epoch 2. ops.push(Operation::FindHead { - justified_epoch: Epoch::new(2), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(4), + }, + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_hash(10), }); // Same as above, but with justified epoch 3 (should be invalid). ops.push(Operation::InvalidFindHead { - justified_epoch: Epoch::new(3), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(3), + root: get_hash(6), + }, + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), }); @@ -275,25 +290,28 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { // | | // head -> 9 10 ops.push(Operation::FindHead { - justified_epoch: Epoch::new(0), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_hash(9), }); // Save as above but justified epoch 2. ops.push(Operation::FindHead { - justified_epoch: Epoch::new(2), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(3), + }, + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_hash(9), }); // Save as above but justified epoch 3 (should fail). ops.push(Operation::InvalidFindHead { - justified_epoch: Epoch::new(3), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(3), + root: get_hash(5), + }, + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), }); @@ -330,25 +348,28 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { // | | // 9 10 <-- head ops.push(Operation::FindHead { - justified_epoch: Epoch::new(0), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_hash(10), }); // Same as above but justified epoch 2. ops.push(Operation::FindHead { - justified_epoch: Epoch::new(2), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(4), + }, + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_hash(10), }); // Same as above but justified epoch 3 (should fail). ops.push(Operation::InvalidFindHead { - justified_epoch: Epoch::new(3), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(3), + root: get_hash(6), + }, + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), }); @@ -366,25 +387,31 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { // | | // head -> 9 10 ops.push(Operation::FindHead { - justified_epoch: Epoch::new(0), - justified_root: get_hash(1), - finalized_epoch: Epoch::new(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(0), + root: get_hash(1), + }, + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_hash(9), }); // Same as above but justified epoch 2. ops.push(Operation::FindHead { - justified_epoch: Epoch::new(2), - justified_root: get_hash(1), - finalized_epoch: Epoch::new(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(3), + }, + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_hash(9), }); // Same as above but justified epoch 3 (should fail). ops.push(Operation::InvalidFindHead { - justified_epoch: Epoch::new(3), - justified_root: get_hash(1), - finalized_epoch: Epoch::new(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(3), + root: get_hash(5), + }, + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), }); @@ -402,34 +429,36 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { // | | // 9 10 <- head ops.push(Operation::FindHead { - justified_epoch: Epoch::new(0), - justified_root: get_hash(2), - finalized_epoch: Epoch::new(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_hash(10), }); // Same as above but justified epoch 2. ops.push(Operation::FindHead { - justified_epoch: Epoch::new(2), - justified_root: get_hash(2), - finalized_epoch: Epoch::new(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(4), + }, + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_hash(10), }); // Same as above but justified epoch 3 (should fail). ops.push(Operation::InvalidFindHead { - justified_epoch: Epoch::new(3), - justified_root: get_hash(2), - finalized_epoch: Epoch::new(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(3), + root: get_hash(6), + }, + finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances, }); // END OF TESTS ForkChoiceTestDefinition { finalized_block_slot: Slot::new(0), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(1), - finalized_root: get_hash(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), operations: ops, } } diff --git a/consensus/proto_array/src/fork_choice_test_definition/no_votes.rs b/consensus/proto_array/src/fork_choice_test_definition/no_votes.rs index e42abe288..0fbcafc5d 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/no_votes.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/no_votes.rs @@ -6,9 +6,14 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { let operations = vec![ // Check that the head is the finalized block. Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: Hash256::zero(), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, justified_state_balances: balances.clone(), expected_head: Hash256::zero(), }, @@ -18,11 +23,17 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { // / // 2 Operation::ProcessBlock { - slot: Slot::new(0), + slot: Slot::new(1), root: get_hash(2), - parent_root: get_hash(0), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(1), + parent_root: Hash256::zero(), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, }, // Ensure the head is 2 // @@ -30,9 +41,14 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { // / // 2 <- head Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: Hash256::zero(), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, justified_state_balances: balances.clone(), expected_head: get_hash(2), }, @@ -42,11 +58,17 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { // / \ // 2 1 Operation::ProcessBlock { - slot: Slot::new(0), + slot: Slot::new(1), root: get_hash(1), parent_root: get_hash(0), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, }, // Ensure the head is still 2 // @@ -54,9 +76,14 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { // / \ // head-> 2 1 Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: Hash256::zero(), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, justified_state_balances: balances.clone(), expected_head: get_hash(2), }, @@ -68,11 +95,17 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { // | // 3 Operation::ProcessBlock { - slot: Slot::new(0), + slot: Slot::new(2), root: get_hash(3), parent_root: get_hash(1), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, }, // Ensure 2 is still the head // @@ -82,9 +115,14 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { // | // 3 Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: Hash256::zero(), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, justified_state_balances: balances.clone(), expected_head: get_hash(2), }, @@ -96,11 +134,17 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { // | | // 4 3 Operation::ProcessBlock { - slot: Slot::new(0), + slot: Slot::new(2), root: get_hash(4), parent_root: get_hash(2), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, }, // Ensure the head is 4. // @@ -110,9 +154,14 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { // | | // head-> 4 3 Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: Hash256::zero(), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, justified_state_balances: balances.clone(), expected_head: get_hash(4), }, @@ -126,11 +175,14 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { // | // 5 <- justified epoch = 2 Operation::ProcessBlock { - slot: Slot::new(0), + slot: Slot::new(3), root: get_hash(5), parent_root: get_hash(4), - justified_epoch: Epoch::new(2), - finalized_epoch: Epoch::new(1), + justified_checkpoint: get_checkpoint(2), + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, }, // Ensure the head is still 4 whilst the justified epoch is 0. // @@ -142,9 +194,14 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { // | // 5 Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: Hash256::zero(), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, justified_state_balances: balances.clone(), expected_head: get_hash(4), }, @@ -158,9 +215,14 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { // | // 5 <- starting from 5 with justified epoch 0 should error. Operation::InvalidFindHead { - justified_epoch: Epoch::new(1), - justified_root: get_hash(5), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(5), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, justified_state_balances: balances.clone(), }, // Set the justified epoch to 2 and the start block to 5 and ensure 5 is the head. @@ -173,9 +235,11 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { // | // 5 <- head Operation::FindHead { - justified_epoch: Epoch::new(2), - justified_root: get_hash(5), - finalized_epoch: Epoch::new(1), + justified_checkpoint: get_checkpoint(2), + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, justified_state_balances: balances.clone(), expected_head: get_hash(5), }, @@ -191,11 +255,14 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { // | // 6 Operation::ProcessBlock { - slot: Slot::new(0), + slot: Slot::new(4), root: get_hash(6), parent_root: get_hash(5), - justified_epoch: Epoch::new(2), - finalized_epoch: Epoch::new(1), + justified_checkpoint: get_checkpoint(2), + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, }, // Ensure 6 is the head // @@ -209,9 +276,11 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { // | // 6 <- head Operation::FindHead { - justified_epoch: Epoch::new(2), - justified_root: get_hash(5), - finalized_epoch: Epoch::new(1), + justified_checkpoint: get_checkpoint(2), + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, justified_state_balances: balances, expected_head: get_hash(6), }, @@ -219,9 +288,14 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { ForkChoiceTestDefinition { finalized_block_slot: Slot::new(0), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(1), - finalized_root: get_hash(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: Hash256::zero(), + }, operations, } } diff --git a/consensus/proto_array/src/fork_choice_test_definition/votes.rs b/consensus/proto_array/src/fork_choice_test_definition/votes.rs index ac9513c5f..f65177a84 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/votes.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/votes.rs @@ -6,9 +6,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // Ensure that the head starts at the finalized block. ops.push(Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, justified_state_balances: balances.clone(), expected_head: get_hash(0), }); @@ -19,11 +24,17 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // / // 2 ops.push(Operation::ProcessBlock { - slot: Slot::new(0), + slot: Slot::new(1), root: get_hash(2), parent_root: get_hash(0), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, }); // Ensure that the head is 2 @@ -32,9 +43,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // / // head-> 2 ops.push(Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, justified_state_balances: balances.clone(), expected_head: get_hash(2), }); @@ -46,11 +62,17 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // / \ // 2 1 ops.push(Operation::ProcessBlock { - slot: Slot::new(0), + slot: Slot::new(1), root: get_hash(1), parent_root: get_hash(0), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, }); // Ensure that the head is still 2 @@ -59,9 +81,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // / \ // head-> 2 1 ops.push(Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, justified_state_balances: balances.clone(), expected_head: get_hash(2), }); @@ -77,15 +104,20 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { target_epoch: Epoch::new(2), }); - // Ensure that the head is now 1, beacuse 1 has a vote. + // Ensure that the head is now 1, because 1 has a vote. // // 0 // / \ // 2 1 <- head ops.push(Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, justified_state_balances: balances.clone(), expected_head: get_hash(1), }); @@ -107,9 +139,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // / \ // head-> 2 1 ops.push(Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, justified_state_balances: balances.clone(), expected_head: get_hash(2), }); @@ -122,11 +159,17 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // | // 3 ops.push(Operation::ProcessBlock { - slot: Slot::new(0), + slot: Slot::new(2), root: get_hash(3), parent_root: get_hash(1), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, }); // Ensure that the head is still 2 @@ -137,9 +180,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // | // 3 ops.push(Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, justified_state_balances: balances.clone(), expected_head: get_hash(2), }); @@ -165,9 +213,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // | // 3 ops.push(Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, justified_state_balances: balances.clone(), expected_head: get_hash(2), }); @@ -194,9 +247,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // | // 3 <- head ops.push(Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, justified_state_balances: balances.clone(), expected_head: get_hash(3), }); @@ -211,11 +269,17 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // | // 4 ops.push(Operation::ProcessBlock { - slot: Slot::new(0), + slot: Slot::new(3), root: get_hash(4), parent_root: get_hash(3), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, }); // Ensure that the head is now 4 @@ -228,9 +292,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // | // 4 <- head ops.push(Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, justified_state_balances: balances.clone(), expected_head: get_hash(4), }); @@ -247,11 +316,17 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // / // 5 <- justified epoch = 2 ops.push(Operation::ProcessBlock { - slot: Slot::new(0), + slot: Slot::new(4), root: get_hash(5), parent_root: get_hash(4), - justified_epoch: Epoch::new(2), - finalized_epoch: Epoch::new(2), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(1), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(1), + }, }); // Ensure that 5 is filtered out and the head stays at 4. @@ -266,9 +341,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // / // 5 ops.push(Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, justified_state_balances: balances.clone(), expected_head: get_hash(4), }); @@ -288,8 +368,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { slot: Slot::new(0), root: get_hash(6), parent_root: get_hash(4), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, }); // Move both votes to 5. @@ -336,22 +422,40 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { slot: Slot::new(0), root: get_hash(7), parent_root: get_hash(5), - justified_epoch: Epoch::new(2), - finalized_epoch: Epoch::new(2), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, }); ops.push(Operation::ProcessBlock { slot: Slot::new(0), root: get_hash(8), parent_root: get_hash(7), - justified_epoch: Epoch::new(2), - finalized_epoch: Epoch::new(2), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, }); ops.push(Operation::ProcessBlock { slot: Slot::new(0), root: get_hash(9), parent_root: get_hash(8), - justified_epoch: Epoch::new(2), - finalized_epoch: Epoch::new(2), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, }); // Ensure that 6 is the head, even though 5 has all the votes. This is testing to ensure @@ -373,9 +477,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // / // 9 ops.push(Operation::FindHead { - justified_epoch: Epoch::new(1), - justified_root: get_hash(0), - finalized_epoch: Epoch::new(1), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, justified_state_balances: balances.clone(), expected_head: get_hash(6), }); @@ -401,9 +510,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // / // head-> 9 ops.push(Operation::FindHead { - justified_epoch: Epoch::new(2), - justified_root: get_hash(5), - finalized_epoch: Epoch::new(2), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, justified_state_balances: balances.clone(), expected_head: get_hash(9), }); @@ -460,15 +574,26 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { slot: Slot::new(0), root: get_hash(10), parent_root: get_hash(8), - justified_epoch: Epoch::new(2), - finalized_epoch: Epoch::new(2), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, }); // Double-check the head is still 9 (no diagram this time) ops.push(Operation::FindHead { - justified_epoch: Epoch::new(2), - justified_root: get_hash(5), - finalized_epoch: Epoch::new(2), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, justified_state_balances: balances.clone(), expected_head: get_hash(9), }); @@ -522,9 +647,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // / \ // 9 10 <- head ops.push(Operation::FindHead { - justified_epoch: Epoch::new(2), - justified_root: get_hash(5), - finalized_epoch: Epoch::new(2), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, justified_state_balances: balances.clone(), expected_head: get_hash(10), }); @@ -542,9 +672,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // / \ // head-> 9 10 ops.push(Operation::FindHead { - justified_epoch: Epoch::new(2), - justified_root: get_hash(5), - finalized_epoch: Epoch::new(2), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, justified_state_balances: balances.clone(), expected_head: get_hash(9), }); @@ -562,9 +697,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // / \ // 9 10 <- head ops.push(Operation::FindHead { - justified_epoch: Epoch::new(2), - justified_root: get_hash(5), - finalized_epoch: Epoch::new(2), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, justified_state_balances: balances.clone(), expected_head: get_hash(10), }); @@ -583,9 +723,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // / \ // head-> 9 10 ops.push(Operation::FindHead { - justified_epoch: Epoch::new(2), - justified_root: get_hash(5), - finalized_epoch: Epoch::new(2), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, justified_state_balances: balances.clone(), expected_head: get_hash(9), }); @@ -599,9 +744,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // Run find-head, ensure the no-op prune didn't change the head. ops.push(Operation::FindHead { - justified_epoch: Epoch::new(2), - justified_root: get_hash(5), - finalized_epoch: Epoch::new(2), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, justified_state_balances: balances.clone(), expected_head: get_hash(9), }); @@ -632,9 +782,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // Run find-head, ensure the prune didn't change the head. ops.push(Operation::FindHead { - justified_epoch: Epoch::new(2), - justified_root: get_hash(5), - finalized_epoch: Epoch::new(2), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, justified_state_balances: balances.clone(), expected_head: get_hash(9), }); @@ -654,8 +809,14 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { slot: Slot::new(0), root: get_hash(11), parent_root: get_hash(9), - justified_epoch: Epoch::new(2), - finalized_epoch: Epoch::new(2), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, }); // Ensure the head is now 11 @@ -670,18 +831,28 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // | // head-> 11 ops.push(Operation::FindHead { - justified_epoch: Epoch::new(2), - justified_root: get_hash(5), - finalized_epoch: Epoch::new(2), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(2), + root: get_hash(5), + }, justified_state_balances: balances, expected_head: get_hash(11), }); ForkChoiceTestDefinition { finalized_block_slot: Slot::new(0), - justified_epoch: Epoch::new(1), - finalized_epoch: Epoch::new(1), - finalized_root: get_hash(0), + justified_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, + finalized_checkpoint: Checkpoint { + epoch: Epoch::new(1), + root: get_hash(0), + }, operations: ops, } } diff --git a/consensus/proto_array/src/lib.rs b/consensus/proto_array/src/lib.rs index 7594f5b12..216d189fb 100644 --- a/consensus/proto_array/src/lib.rs +++ b/consensus/proto_array/src/lib.rs @@ -8,5 +8,7 @@ pub use crate::proto_array_fork_choice::{Block, ExecutionStatus, ProtoArrayForkC pub use error::Error; pub mod core { - pub use super::proto_array::ProtoArray; + pub use super::proto_array::{ProposerBoost, ProtoArray, ProtoNode}; + pub use super::proto_array_fork_choice::VoteTracker; + pub use super::ssz_container::SszContainer; } diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 6732e0fba..465ef9d4f 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -1,13 +1,16 @@ +use crate::error::InvalidBestNodeInfo; use crate::{error::Error, Block, ExecutionStatus}; use serde_derive::{Deserialize, Serialize}; use ssz::four_byte_option_impl; +use ssz::Encode; use ssz_derive::{Decode, Encode}; use std::collections::HashMap; -use types::{AttestationShufflingId, Epoch, Hash256, Slot}; +use types::{AttestationShufflingId, ChainSpec, Checkpoint, Epoch, EthSpec, Hash256, Slot}; // Define a "legacy" implementation of `Option` which uses four bytes for encoding the union // selector. four_byte_option_impl!(four_byte_option_usize, usize); +four_byte_option_impl!(four_byte_option_checkpoint, Checkpoint); #[derive(Clone, PartialEq, Debug, Encode, Decode, Serialize, Deserialize)] pub struct ProtoNode { @@ -28,59 +31,31 @@ pub struct ProtoNode { 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_checkpoint")] + pub justified_checkpoint: Option, + #[ssz(with = "four_byte_option_checkpoint")] + pub finalized_checkpoint: Option, + pub weight: u64, #[ssz(with = "four_byte_option_usize")] - best_child: Option, + pub best_child: Option, #[ssz(with = "four_byte_option_usize")] - best_descendant: Option, + pub best_descendant: Option, /// Indicates if an execution node has marked this block as valid. Also contains the execution /// block hash. pub execution_status: ExecutionStatus, } -/// 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, +#[derive(PartialEq, Debug, Encode, Decode, Serialize, Deserialize, Copy, Clone)] +pub struct ProposerBoost { 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, + pub score: u64, } -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, - // We set the following execution value as if the block is a pre-merge-fork block. This - // is safe as long as we never import a merge block with the old version of proto-array. - // This will be safe since we can't actually process merge blocks until we've made this - // change to fork choice. - execution_status: ExecutionStatus::irrelevant(), +impl Default for ProposerBoost { + fn default() -> Self { + Self { + root: Hash256::zero(), + score: 0, } } } @@ -90,10 +65,11 @@ 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_epoch: Epoch, - pub finalized_epoch: Epoch, + pub justified_checkpoint: Checkpoint, + pub finalized_checkpoint: Checkpoint, pub nodes: Vec, pub indices: HashMap, + pub previous_proposer_boost: ProposerBoost, } impl ProtoArray { @@ -110,11 +86,14 @@ impl ProtoArray { /// - Compare the current node with the parents best-child, updating it if the current node /// should become the best child. /// - If required, update the parents best-descendant with the current node or its best-descendant. - pub fn apply_score_changes( + pub fn apply_score_changes( &mut self, mut deltas: Vec, - justified_epoch: Epoch, - finalized_epoch: Epoch, + justified_checkpoint: Checkpoint, + finalized_checkpoint: Checkpoint, + new_balances: &[u64], + proposer_boost_root: Hash256, + spec: &ChainSpec, ) -> Result<(), Error> { if deltas.len() != self.indices.len() { return Err(Error::InvalidDeltaLen { @@ -123,11 +102,16 @@ impl ProtoArray { }); } - if justified_epoch != self.justified_epoch || finalized_epoch != self.finalized_epoch { - self.justified_epoch = justified_epoch; - self.finalized_epoch = finalized_epoch; + 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; + // Iterate backwards through all indices in `self.nodes`. for node_index in (0..self.nodes.len()).rev() { let node = self @@ -142,11 +126,35 @@ impl ProtoArray { continue; } - let node_delta = deltas + let mut node_delta = deltas .get(node_index) .copied() .ok_or(Error::InvalidNodeDelta(node_index))?; + // If we find the node for which the proposer boost was previously applied, decrease + // the delta by the previous score amount. + if self.previous_proposer_boost.root != Hash256::zero() + && self.previous_proposer_boost.root == node.root + { + node_delta = node_delta + .checked_sub(self.previous_proposer_boost.score as i64) + .ok_or(Error::DeltaOverflow(node_index))?; + } + // If we find the node matching the current proposer boost root, increase + // the delta by the new score amount. + // + // https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/fork-choice.md#get_latest_attesting_balance + if let Some(proposer_score_boost) = spec.proposer_score_boost { + if proposer_boost_root != Hash256::zero() && proposer_boost_root == node.root { + proposer_score = + calculate_proposer_boost::(new_balances, proposer_score_boost) + .ok_or(Error::ProposerBoostOverflow(node_index))?; + node_delta = node_delta + .checked_add(proposer_score as i64) + .ok_or(Error::DeltaOverflow(node_index))?; + } + } + // Apply the delta to the node. if node_delta < 0 { // Note: I am conflicted about whether to use `saturating_sub` or `checked_sub` @@ -180,6 +188,12 @@ impl ProtoArray { } } + // After applying all deltas, update the `previous_proposer_boost`. + self.previous_proposer_boost = ProposerBoost { + root: proposer_boost_root, + score: proposer_score, + }; + // A second time, iterate backwards through all indices in `self.nodes`. // // We _must_ perform these functions separate from the weight-updating loop above to ensure @@ -221,8 +235,8 @@ impl ProtoArray { parent: block .parent_root .and_then(|parent| self.indices.get(&parent).copied()), - justified_epoch: block.justified_epoch, - finalized_epoch: block.finalized_epoch, + justified_checkpoint: Some(block.justified_checkpoint), + finalized_checkpoint: Some(block.finalized_checkpoint), weight: 0, best_child: None, best_descendant: None, @@ -315,14 +329,14 @@ impl ProtoArray { // Perform a sanity check that the node is indeed valid to be the head. if !self.node_is_viable_for_head(best_node) { - return Err(Error::InvalidBestNode { + return Err(Error::InvalidBestNode(Box::new(InvalidBestNodeInfo { start_root: *justified_root, - justified_epoch: self.justified_epoch, - finalized_epoch: self.finalized_epoch, + justified_checkpoint: self.justified_checkpoint, + finalized_checkpoint: self.finalized_checkpoint, head_root: justified_node.root, - head_justified_epoch: justified_node.justified_epoch, - head_finalized_epoch: justified_node.finalized_epoch, - }); + head_justified_checkpoint: justified_node.justified_checkpoint, + head_finalized_checkpoint: justified_node.finalized_checkpoint, + }))); } Ok(best_node.root) @@ -523,9 +537,16 @@ 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) -> bool { - (node.justified_epoch == self.justified_epoch || self.justified_epoch == Epoch::new(0)) - && (node.finalized_epoch == self.finalized_epoch - || self.finalized_epoch == Epoch::new(0)) + if let (Some(node_justified_checkpoint), Some(node_finalized_checkpoint)) = + (node.justified_checkpoint, node.finalized_checkpoint) + { + (node_justified_checkpoint == self.justified_checkpoint + || self.justified_checkpoint.epoch == Epoch::new(0)) + && (node_finalized_checkpoint == self.finalized_checkpoint + || self.finalized_checkpoint.epoch == Epoch::new(0)) + } else { + false + } } /// Return a reverse iterator over the nodes which comprise the chain ending at `block_root`. @@ -549,6 +570,38 @@ impl ProtoArray { } } +/// A helper method to calculate the proposer boost based on the given `validator_balances`. +/// This does *not* do any verification about whether a boost should or should not be applied. +/// The `validator_balances` array used here is assumed to be structured like the one stored in +/// the `BalancesCache`, where *effective* balances are stored and inactive balances are defaulted +/// to zero. +/// +/// Returns `None` if there is an overflow or underflow when calculating the score. +/// +/// https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/fork-choice.md#get_latest_attesting_balance +fn calculate_proposer_boost( + validator_balances: &[u64], + proposer_score_boost: u64, +) -> Option { + let mut total_balance: u64 = 0; + let mut num_validators: u64 = 0; + for &balance in validator_balances { + // We need to filter zero balances here to get an accurate active validator count. + // This is because we default inactive validator balances to zero when creating + // this balances array. + if balance != 0 { + total_balance = total_balance.checked_add(balance)?; + num_validators = num_validators.checked_add(1)?; + } + } + let average_balance = total_balance.checked_div(num_validators)?; + let committee_size = num_validators.checked_div(E::slots_per_epoch())?; + let committee_weight = committee_size.checked_mul(average_balance)?; + committee_weight + .checked_mul(proposer_score_boost)? + .checked_div(100) +} + /// Reverse iterator over one path through a `ProtoArray`. pub struct Iter<'a> { next_node_index: Option, diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index d0abea4f1..891eafabe 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -1,11 +1,11 @@ use crate::error::Error; -use crate::proto_array::ProtoArray; -use crate::ssz_container::{LegacySszContainer, SszContainer}; +use crate::proto_array::{ProposerBoost, ProtoArray}; +use crate::ssz_container::SszContainer; use serde_derive::{Deserialize, Serialize}; use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; use std::collections::HashMap; -use types::{AttestationShufflingId, Epoch, Hash256, Slot}; +use types::{AttestationShufflingId, ChainSpec, Checkpoint, Epoch, EthSpec, Hash256, Slot}; pub const DEFAULT_PRUNE_THRESHOLD: usize = 256; @@ -63,8 +63,8 @@ pub struct Block { pub target_root: Hash256, pub current_epoch_shuffling_id: AttestationShufflingId, pub next_epoch_shuffling_id: AttestationShufflingId, - pub justified_epoch: Epoch, - pub finalized_epoch: Epoch, + pub justified_checkpoint: Checkpoint, + pub finalized_checkpoint: Checkpoint, /// Indicates if an execution node has marked this block as valid. Also contains the execution /// block hash. pub execution_status: ExecutionStatus, @@ -109,33 +109,33 @@ impl ProtoArrayForkChoice { pub fn new( finalized_block_slot: Slot, finalized_block_state_root: Hash256, - justified_epoch: Epoch, - finalized_epoch: Epoch, - finalized_root: Hash256, + justified_checkpoint: Checkpoint, + finalized_checkpoint: Checkpoint, current_epoch_shuffling_id: AttestationShufflingId, next_epoch_shuffling_id: AttestationShufflingId, execution_status: ExecutionStatus, ) -> Result { let mut proto_array = ProtoArray { prune_threshold: DEFAULT_PRUNE_THRESHOLD, - justified_epoch, - finalized_epoch, + justified_checkpoint, + finalized_checkpoint, nodes: Vec::with_capacity(1), indices: HashMap::with_capacity(1), + previous_proposer_boost: ProposerBoost::default(), }; let block = Block { slot: finalized_block_slot, - root: finalized_root, + root: finalized_checkpoint.root, parent_root: None, state_root: finalized_block_state_root, // We are using the finalized_root as the target_root, since it always lies on an // epoch boundary. - target_root: finalized_root, + target_root: finalized_checkpoint.root, current_epoch_shuffling_id, next_epoch_shuffling_id, - justified_epoch, - finalized_epoch, + justified_checkpoint, + finalized_checkpoint, execution_status, }; @@ -176,12 +176,13 @@ impl ProtoArrayForkChoice { .map_err(|e| format!("process_block_error: {:?}", e)) } - pub fn find_head( + pub fn find_head( &mut self, - justified_epoch: Epoch, - justified_root: Hash256, - finalized_epoch: Epoch, + justified_checkpoint: Checkpoint, + finalized_checkpoint: Checkpoint, justified_state_balances: &[u64], + proposer_boost_root: Hash256, + spec: &ChainSpec, ) -> Result { let old_balances = &mut self.balances; @@ -196,13 +197,20 @@ impl ProtoArrayForkChoice { .map_err(|e| format!("find_head compute_deltas failed: {:?}", e))?; self.proto_array - .apply_score_changes(deltas, justified_epoch, finalized_epoch) + .apply_score_changes::( + deltas, + justified_checkpoint, + finalized_checkpoint, + new_balances, + proposer_boost_root, + spec, + ) .map_err(|e| format!("find_head apply_score_changes failed: {:?}", e))?; *old_balances = new_balances.to_vec(); self.proto_array - .find_head(&justified_root) + .find_head(&justified_checkpoint.root) .map_err(|e| format!("find_head failed: {:?}", e)) } @@ -236,18 +244,27 @@ impl ProtoArrayForkChoice { .and_then(|i| self.proto_array.nodes.get(i)) .map(|parent| parent.root); - Some(Block { - slot: block.slot, - root: block.root, - parent_root, - state_root: block.state_root, - target_root: block.target_root, - current_epoch_shuffling_id: block.current_epoch_shuffling_id.clone(), - next_epoch_shuffling_id: block.next_epoch_shuffling_id.clone(), - justified_epoch: block.justified_epoch, - finalized_epoch: block.finalized_epoch, - execution_status: block.execution_status, - }) + // If a node does not have a `finalized_checkpoint` or `justified_checkpoint` populated, + // it means it is not a descendant of the finalized checkpoint, so it is valid to return + // `None` here. + if let (Some(justified_checkpoint), Some(finalized_checkpoint)) = + (block.justified_checkpoint, block.finalized_checkpoint) + { + Some(Block { + slot: block.slot, + root: block.root, + parent_root, + state_root: block.state_root, + target_root: block.target_root, + current_epoch_shuffling_id: block.current_epoch_shuffling_id.clone(), + next_epoch_shuffling_id: block.next_epoch_shuffling_id.clone(), + justified_checkpoint, + finalized_checkpoint, + execution_status: block.execution_status, + }) + } else { + None + } } /// Returns `true` if the `descendant_root` has an ancestor with `ancestor_root`. Always @@ -295,28 +312,19 @@ 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 5 to schema 6. - 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. pub fn core_proto_array(&self) -> &ProtoArray { &self.proto_array } + + /// Returns a mutable reference to the core `ProtoArray` struct. + /// + /// Should only be used during database schema migrations. + pub fn core_proto_array_mut(&mut self) -> &mut ProtoArray { + &mut self.proto_array + } } /// Returns a list of `deltas`, where there is one delta for each of the indices in @@ -412,12 +420,16 @@ mod test_compute_deltas { AttestationShufflingId::from_components(Epoch::new(0), Hash256::zero()); let execution_status = ExecutionStatus::irrelevant(); + let genesis_checkpoint = Checkpoint { + epoch: genesis_epoch, + root: finalized_root, + }; + let mut fc = ProtoArrayForkChoice::new( genesis_slot, state_root, - genesis_epoch, - genesis_epoch, - finalized_root, + genesis_checkpoint, + genesis_checkpoint, junk_shuffling_id.clone(), junk_shuffling_id.clone(), execution_status, @@ -434,8 +446,8 @@ mod test_compute_deltas { target_root: finalized_root, current_epoch_shuffling_id: junk_shuffling_id.clone(), next_epoch_shuffling_id: junk_shuffling_id.clone(), - justified_epoch: genesis_epoch, - finalized_epoch: genesis_epoch, + justified_checkpoint: genesis_checkpoint, + finalized_checkpoint: genesis_checkpoint, execution_status, }) .unwrap(); @@ -450,8 +462,8 @@ mod test_compute_deltas { target_root: finalized_root, current_epoch_shuffling_id: junk_shuffling_id.clone(), next_epoch_shuffling_id: junk_shuffling_id, - justified_epoch: genesis_epoch, - finalized_epoch: genesis_epoch, + justified_checkpoint: genesis_checkpoint, + finalized_checkpoint: genesis_checkpoint, execution_status, }) .unwrap(); diff --git a/consensus/proto_array/src/ssz_container.rs b/consensus/proto_array/src/ssz_container.rs index a0aaf1941..7f7ef79fe 100644 --- a/consensus/proto_array/src/ssz_container.rs +++ b/consensus/proto_array/src/ssz_container.rs @@ -1,50 +1,27 @@ -use crate::proto_array::LegacyProtoNode; +use crate::proto_array::ProposerBoost; use crate::{ proto_array::{ProtoArray, ProtoNode}, proto_array_fork_choice::{ElasticList, ProtoArrayForkChoice, VoteTracker}, }; +use ssz::{four_byte_option_impl, Encode}; use ssz_derive::{Decode, Encode}; use std::collections::HashMap; -use types::{Epoch, Hash256}; +use types::{Checkpoint, Hash256}; + +// Define a "legacy" implementation of `Option` which uses four bytes for encoding the union +// selector. +four_byte_option_impl!(four_byte_option_checkpoint, Checkpoint); #[derive(Encode, Decode)] pub struct SszContainer { - votes: Vec, - balances: Vec, - prune_threshold: usize, - justified_epoch: Epoch, - finalized_epoch: Epoch, - nodes: Vec, - indices: Vec<(Hash256, usize)>, -} - -/// Only used for SSZ deserialization of the persisted fork choice during the database migration -/// from schema 5 to schema 6. -#[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, - } - } + pub votes: Vec, + pub balances: Vec, + pub prune_threshold: usize, + pub justified_checkpoint: Checkpoint, + pub finalized_checkpoint: Checkpoint, + pub nodes: Vec, + pub indices: Vec<(Hash256, usize)>, + pub previous_proposer_boost: ProposerBoost, } impl From<&ProtoArrayForkChoice> for SszContainer { @@ -55,10 +32,11 @@ impl From<&ProtoArrayForkChoice> for SszContainer { votes: from.votes.0.clone(), balances: from.balances.clone(), prune_threshold: proto_array.prune_threshold, - justified_epoch: proto_array.justified_epoch, - finalized_epoch: proto_array.finalized_epoch, + justified_checkpoint: proto_array.justified_checkpoint, + finalized_checkpoint: proto_array.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, } } } @@ -67,10 +45,11 @@ impl From for ProtoArrayForkChoice { fn from(from: SszContainer) -> Self { let proto_array = ProtoArray { prune_threshold: from.prune_threshold, - justified_epoch: from.justified_epoch, - finalized_epoch: from.finalized_epoch, + 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, }; Self { diff --git a/consensus/types/Cargo.toml b/consensus/types/Cargo.toml index aa3c6c32c..f62fcf599 100644 --- a/consensus/types/Cargo.toml +++ b/consensus/types/Cargo.toml @@ -43,7 +43,7 @@ regex = "1.3.9" lazy_static = "1.4.0" parking_lot = "0.11.1" itertools = "0.10.0" -superstruct = "0.2.0" +superstruct = "0.3.0" [dev-dependencies] criterion = "0.3.3" diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index e5eabec20..68a5175a9 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -101,6 +101,7 @@ pub struct ChainSpec { * Fork choice */ pub safe_slots_to_update_justified: u64, + pub proposer_score_boost: Option, /* * Eth1 @@ -489,6 +490,7 @@ impl ChainSpec { * Fork choice */ safe_slots_to_update_justified: 8, + proposer_score_boost: None, /* * Eth1 @@ -657,6 +659,8 @@ pub struct Config { #[serde(with = "eth2_serde_utils::quoted_u64")] churn_limit_quotient: u64, + proposer_score_boost: Option>, + #[serde(with = "eth2_serde_utils::quoted_u64")] deposit_chain_id: u64, #[serde(with = "eth2_serde_utils::quoted_u64")] @@ -746,6 +750,8 @@ impl Config { churn_limit_quotient: spec.churn_limit_quotient, min_per_epoch_churn_limit: spec.min_per_epoch_churn_limit, + proposer_score_boost: spec.proposer_score_boost.map(|value| MaybeQuoted { value }), + deposit_chain_id: spec.deposit_chain_id, deposit_network_id: spec.deposit_network_id, deposit_contract_address: spec.deposit_contract_address, @@ -784,6 +790,7 @@ impl Config { ejection_balance, min_per_epoch_churn_limit, churn_limit_quotient, + proposer_score_boost, deposit_chain_id, deposit_network_id, deposit_contract_address, @@ -812,6 +819,7 @@ impl Config { ejection_balance, min_per_epoch_churn_limit, churn_limit_quotient, + proposer_score_boost: proposer_score_boost.map(|q| q.value), deposit_chain_id, deposit_network_id, deposit_contract_address, diff --git a/consensus/types/src/consts.rs b/consensus/types/src/consts.rs index 04e8e60ee..a9377bc3e 100644 --- a/consensus/types/src/consts.rs +++ b/consensus/types/src/consts.rs @@ -19,3 +19,6 @@ pub mod altair { pub const NUM_FLAG_INDICES: usize = 3; } +pub mod merge { + pub const INTERVALS_PER_SLOT: u64 = 3; +} diff --git a/lighthouse/environment/tests/testnet_dir/config.yaml b/lighthouse/environment/tests/testnet_dir/config.yaml index c06e89653..ac5403efd 100644 --- a/lighthouse/environment/tests/testnet_dir/config.yaml +++ b/lighthouse/environment/tests/testnet_dir/config.yaml @@ -70,6 +70,10 @@ MIN_PER_EPOCH_CHURN_LIMIT: 4 # 2**16 (= 65,536) CHURN_LIMIT_QUOTIENT: 65536 +# Fork choice +# --------------------------------------------------------------- +# 70% +PROPOSER_SCORE_BOOST: 70 # Deposit contract # --------------------------------------------------------------- diff --git a/testing/ef_tests/check_all_files_accessed.py b/testing/ef_tests/check_all_files_accessed.py index b1dfbdb4f..ce9e1d6b4 100755 --- a/testing/ef_tests/check_all_files_accessed.py +++ b/testing/ef_tests/check_all_files_accessed.py @@ -39,19 +39,9 @@ excluded_paths = [ "tests/minimal/altair/merkle/single_proof", "tests/mainnet/merge/merkle/single_proof", "tests/minimal/merge/merkle/single_proof", - # Temporarily disabled due to addition of proposer boosting. - # - # These tests will be reintroduced in: - # https://github.com/sigp/lighthouse/pull/2822 - "tests/minimal/phase0/fork_choice", - "tests/minimal/altair/fork_choice", - "tests/minimal/merge/fork_choice", - "tests/mainnet/phase0/fork_choice", - "tests/mainnet/altair/fork_choice", - "tests/mainnet/merge/fork_choice", - # Tests yet to be implemented. - "tests/mainnet/merge/transition", - "tests/minimal/merge/transition", + # FIXME(merge): Merge transition tests are now available but not yet passing + "tests/mainnet/merge/transition/", + "tests/minimal/merge/transition/", ] def normalize_path(path): diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 682fa8146..ecdfebc28 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -1,6 +1,7 @@ use super::*; use crate::decode::{ssz_decode_file, ssz_decode_file_with, ssz_decode_state, yaml_decode_file}; use ::fork_choice::PayloadVerificationStatus; +use beacon_chain::slot_clock::SlotClock; use beacon_chain::{ attestation_verification::{ obtain_indexed_attestation_and_committees_per_slot, VerifiedAttestation, @@ -23,10 +24,6 @@ pub struct PowBlock { pub block_hash: Hash256, pub parent_hash: Hash256, pub total_difficulty: Uint256, - // This field is not used and I expect it to be removed. See: - // - // https://github.com/ethereum/consensus-specs/pull/2720 - pub difficulty: Uint256, } #[derive(Debug, Clone, Copy, PartialEq, Deserialize)] @@ -46,6 +43,7 @@ pub struct Checks { justified_checkpoint_root: Option, finalized_checkpoint: Option, best_justified_checkpoint: Option, + proposer_boost_root: Option, } #[derive(Debug, Clone, Deserialize)] @@ -74,6 +72,15 @@ pub struct ForkChoiceTest { pub steps: Vec, Attestation, PowBlock>>, } +/// Spec for fork choice tests, with proposer boosting enabled. +/// +/// This function can be deleted once `ChainSpec::mainnet` enables proposer boosting by default. +pub fn fork_choice_spec(fork_name: ForkName) -> ChainSpec { + let mut spec = testing_spec::(fork_name); + spec.proposer_score_boost = Some(70); + spec +} + impl LoadCase for ForkChoiceTest { fn load_from_dir(path: &Path, fork_name: ForkName) -> Result { let description = path @@ -83,7 +90,7 @@ impl LoadCase for ForkChoiceTest { .to_str() .expect("path must be valid OsStr") .to_string(); - let spec = &testing_spec::(fork_name); + let spec = &fork_choice_spec::(fork_name); let steps: Vec> = yaml_decode_file(&path.join("steps.yaml"))?; // Resolve the object names in `steps.yaml` into actual decoded block/attestation objects. let steps = steps @@ -145,18 +152,15 @@ impl Case for ForkChoiceTest { } fn result(&self, _case_index: usize, fork_name: ForkName) -> Result<(), Error> { - let tester = Tester::new(self, testing_spec::(fork_name))?; + let tester = Tester::new(self, fork_choice_spec::(fork_name))?; - // The reason for this failure is documented here: + // TODO(merge): enable these tests before production. + // This test will fail until this PR is merged and released: // - // https://github.com/sigp/lighthouse/issues/2741 - // - // We should eventually solve the above issue and remove this `SkippedKnownFailure`. - if self.description == "new_finalized_slot_is_justified_checkpoint_ancestor" + // https://github.com/ethereum/consensus-specs/pull/2760 + if self.description == "shorter_chain_but_heavier_weight" // This test is skipped until we can do retrospective confirmations of the terminal // block after an optimistic sync. - // - // TODO(merge): enable this test before production. || self.description == "block_lookup_failed" { return Err(Error::SkippedKnownFailure); @@ -180,6 +184,7 @@ impl Case for ForkChoiceTest { justified_checkpoint_root, finalized_checkpoint, best_justified_checkpoint, + proposer_boost_root, } = checks.as_ref(); if let Some(expected_head) = head { @@ -211,6 +216,10 @@ impl Case for ForkChoiceTest { tester .check_best_justified_checkpoint(*expected_best_justified_checkpoint)?; } + + if let Some(expected_proposer_boost_root) = proposer_boost_root { + tester.check_expected_proposer_boost_root(*expected_proposer_boost_root)?; + } } } } @@ -352,11 +361,19 @@ impl Tester { ) .unwrap(); + let block_delay = self + .harness + .chain + .slot_clock + .seconds_from_current_slot_start(self.spec.seconds_per_slot) + .unwrap(); + let (block, _) = block.deconstruct(); let result = self.harness.chain.fork_choice.write().on_block( self.harness.chain.slot().unwrap(), &block, block_root, + block_delay, &state, PayloadVerificationStatus::Irrelevant, &self.harness.chain.spec, @@ -494,6 +511,18 @@ impl Tester { expected_checkpoint, ) } + + pub fn check_expected_proposer_boost_root( + &self, + expected_proposer_boost_root: Hash256, + ) -> Result<(), Error> { + let proposer_boost_root = self.harness.chain.fork_choice.read().proposer_boost_root(); + check_equal( + "proposer_boost_root", + proposer_boost_root, + expected_proposer_boost_root, + ) + } } /// Checks that the `head` checkpoint from the beacon chain head matches the `fc` checkpoint gleaned diff --git a/testing/ef_tests/src/cases/transition.rs b/testing/ef_tests/src/cases/transition.rs index 5d8fa1434..6ac56858a 100644 --- a/testing/ef_tests/src/cases/transition.rs +++ b/testing/ef_tests/src/cases/transition.rs @@ -72,7 +72,7 @@ impl Case for TransitionTest { fn is_enabled_for_fork(fork_name: ForkName) -> bool { // Upgrades exist targeting all forks except phase0/base. // Transition tests also need BLS. - // FIXME(merge): enable merge tests once available + // FIXME(merge): Merge transition tests are now available but not yet passing cfg!(not(feature = "fake_crypto")) && fork_name != ForkName::Base && fork_name != ForkName::Merge diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index a74f0a0ba..2201bc5ee 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -411,12 +411,6 @@ fn finality() { FinalityHandler::::default().run(); } -/* - * Temporarily disabled due to addition of proposer boosting. - * - * These tests will be reintroduced in: - * https://github.com/sigp/lighthouse/pull/2822 - * #[test] fn fork_choice_get_head() { ForkChoiceGetHeadHandler::::default().run(); @@ -434,7 +428,6 @@ fn fork_choice_on_merge_block() { ForkChoiceOnMergeBlockHandler::::default().run(); ForkChoiceOnMergeBlockHandler::::default().run(); } -*/ #[test] fn genesis_initialization() {