diff --git a/Cargo.lock b/Cargo.lock index 69204ccaecd..a79d2e206cd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1296,6 +1296,7 @@ dependencies = [ "directory", "dirs", "environment", + "eth2", "eth2_config", "execution_layer", "genesis", diff --git a/beacon_node/Cargo.toml b/beacon_node/Cargo.toml index 5352814dd5d..19fad953b8d 100644 --- a/beacon_node/Cargo.toml +++ b/beacon_node/Cargo.toml @@ -27,6 +27,7 @@ client = { path = "client" } directory = { workspace = true } dirs = { workspace = true } environment = { workspace = true } +eth2 = { workspace = true } eth2_config = { workspace = true } execution_layer = { workspace = true } genesis = { workspace = true } diff --git a/beacon_node/beacon_chain/src/attestation_verification.rs b/beacon_node/beacon_chain/src/attestation_verification.rs index faa396966ff..bee4e68d9fc 100644 --- a/beacon_node/beacon_chain/src/attestation_verification.rs +++ b/beacon_node/beacon_chain/src/attestation_verification.rs @@ -1436,6 +1436,7 @@ fn verify_attestation_is_finalized_checkpoint_or_descendant let attestation_block_root = attestation_data.beacon_block_root; let finalized_slot = fork_choice .finalized_checkpoint() + .local() .epoch .start_slot(T::EthSpec::slots_per_epoch()); let split = chain.store.get_split_info(); diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index ec791537854..8b075360d59 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -41,7 +41,7 @@ use crate::light_client_optimistic_update_verification::{ Error as LightClientOptimisticUpdateError, VerifiedLightClientOptimisticUpdate, }; use crate::light_client_server_cache::LightClientServerCache; -use crate::migrate::{BackgroundMigrator, ManualFinalizationNotification}; +use crate::migrate::BackgroundMigrator; use crate::naive_aggregation_pool::{ AggregatedAttestationMap, Error as NaiveAggregationError, NaiveAggregationPool, SyncContributionAggregateMap, @@ -125,8 +125,8 @@ use std::sync::Arc; use std::time::Duration; use store::iter::{BlockRootsIterator, ParentRootBlockIterator, StateRootsIterator}; use store::{ - BlobSidecarListFromRoot, DBColumn, DatabaseBlock, Error as DBError, HotColdDB, HotStateSummary, - KeyValueStore, KeyValueStoreOp, StoreItem, StoreOp, + BlobSidecarListFromRoot, DBColumn, DatabaseBlock, Error as DBError, HotColdDB, KeyValueStore, + KeyValueStoreOp, StoreItem, StoreOp, }; use task_executor::{RayonPoolType, ShutdownReason, TaskExecutor}; use tokio_stream::Stream; @@ -550,10 +550,12 @@ impl BeaconChain { block_root: &Hash256, block_slot: Slot, ) -> Result { + // Used by the API: finalized if prior to the network finality not the local view let finalized_slot = self .canonical_head .cached_head() .finalized_checkpoint() + .on_chain() .epoch .start_slot(T::EthSpec::slots_per_epoch()); let is_canonical = self @@ -580,10 +582,12 @@ impl BeaconChain { state_root: &Hash256, state_slot: Slot, ) -> Result { + // Used by the API: finalized if prior to the network finality not the local view let finalized_slot = self .canonical_head .cached_head() .finalized_checkpoint() + .on_chain() .epoch .start_slot(T::EthSpec::slots_per_epoch()); let slot_is_finalized = state_slot <= finalized_slot; @@ -1210,6 +1214,26 @@ impl BeaconChain { .map(Some) } + pub fn get_execution_hash( + &self, + block_root: &Hash256, + ) -> Result, Error> { + if let Some(execution_hash) = self + .canonical_head + .fork_choice_read_lock() + .execution_hash(*block_root) + { + return Ok(Some(execution_hash)); + } + Ok(self.store.get_blinded_block(block_root)?.and_then(|block| { + if let Ok(payload) = block.message().execution_payload() { + Some(payload.block_hash()) + } else { + None + } + })) + } + /// Returns the blobs at the given root, if any. /// /// ## Errors @@ -1435,7 +1459,9 @@ impl BeaconChain { let fork_choice = self.canonical_head.fork_choice_read_lock(); fork_choice .proto_array() - .heads_descended_from_finalization::(fork_choice.finalized_checkpoint()) + .heads_descended_from_finalization::( + fork_choice.finalized_checkpoint().as_local(), + ) .iter() .map(|node| (node.root, node.slot)) .collect() @@ -1692,39 +1718,6 @@ impl BeaconChain { self.store_migrator.process_manual_compaction(); } - pub fn manually_finalize_state( - &self, - state_root: Hash256, - checkpoint: Checkpoint, - ) -> Result<(), Error> { - let HotStateSummary { - slot, - latest_block_root, - .. - } = self - .store - .load_hot_state_summary(&state_root) - .map_err(BeaconChainError::DBError)? - .ok_or(BeaconChainError::MissingHotStateSummary(state_root))?; - - if slot != checkpoint.epoch.start_slot(T::EthSpec::slots_per_epoch()) - || latest_block_root != *checkpoint.root - { - return Err(BeaconChainError::InvalidCheckpoint { - state_root, - checkpoint, - }); - } - - let notif = ManualFinalizationNotification { - state_root: state_root.into(), - checkpoint, - }; - - self.store_migrator.process_manual_finalization(notif); - Ok(()) - } - /// Returns an aggregated `Attestation`, if any, that has a matching `attestation.data`. /// /// The attestation will be obtained from `self.naive_aggregation_pool`. @@ -4173,8 +4166,10 @@ impl BeaconChain { // We are doing this to ensure that we detect changes in finalization. It's possible // that fork choice has already been updated to the finalized checkpoint in the block // we're importing. - let current_head_finalized_checkpoint = - self.canonical_head.cached_head().finalized_checkpoint(); + let current_head_finalized_checkpoint = self + .canonical_head + .cached_head() + .finalized_checkpoint_from_head_state(); // Compare the existing finalized checkpoint with the incoming block's finalized checkpoint. let new_finalized_checkpoint = state.finalized_checkpoint(); @@ -6003,16 +5998,16 @@ impl BeaconChain { let justified_block = self .spawn_blocking_handle( move || { - chain - .canonical_head - .fork_choice_read_lock() - .get_justified_block() + let fork_choice = chain.canonical_head.fork_choice_read_lock(); + fork_choice.get_block(&fork_choice.justified_checkpoint().on_chain().root) }, "invalid_payload_fork_choice_get_justified", ) - .await??; + .await?; - if justified_block.execution_status.is_invalid() { + if let Some(justified_block) = justified_block + && justified_block.execution_status.is_invalid() + { crit!( msg = "ensure you are not connected to a malicious network. This error is not \ recoverable, please reach out to the lighthouse developers for assistance.", @@ -7230,7 +7225,7 @@ impl BeaconChain { // Check if finalization is advancing. let current_epoch = current_slot.epoch(T::EthSpec::slots_per_epoch()); let epochs_since_finalization = - current_epoch.saturating_sub(cached_head.finalized_checkpoint().epoch); + current_epoch.saturating_sub(cached_head.finalized_checkpoint().on_chain().epoch); let finalization_check = epochs_since_finalization.as_usize() <= self.config.builder_fallback_epochs_since_finalization; 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 60487f9c469..d49c2b09f3d 100644 --- a/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs +++ b/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs @@ -6,8 +6,7 @@ use crate::{BeaconSnapshot, metrics}; use educe::Educe; -use fixed_bytes::FixedBytesExtended; -use fork_choice::ForkChoiceStore; +use fork_choice::{ForkChoiceCheckpoint, ForkChoiceStore}; use proto_array::JustifiedBalances; use safe_arith::ArithError; use ssz_derive::{Decode, Encode}; @@ -18,7 +17,7 @@ use store::{Error as StoreError, HotColdDB, ItemStore}; use superstruct::superstruct; use types::{ AbstractExecPayload, BeaconBlockRef, BeaconState, BeaconStateError, Checkpoint, Epoch, EthSpec, - Hash256, Slot, + Hash256, SignedBeaconBlock, Slot, }; #[derive(Debug)] @@ -32,6 +31,16 @@ pub enum Error { Arith(ArithError), } +pub enum ForkChoiceStoreInit { + FinalizedState(Box>), + NonFinalizedState { + anchor_state: Box>, + justified_state: Box>, + justified_block: Box>, + finalized_block: Box>, + }, +} + impl From for Error { fn from(e: BeaconStateError) -> Self { Error::BeaconStateError(e) @@ -137,6 +146,7 @@ pub struct BeaconForkChoiceStore, Cold: ItemStore< time: Slot, finalized_checkpoint: Checkpoint, justified_checkpoint: Checkpoint, + local_irreversible_checkpoint: Checkpoint, justified_balances: JustifiedBalances, justified_state_root: Hash256, unrealized_justified_checkpoint: Checkpoint, @@ -165,6 +175,29 @@ where /// /// It is assumed that `anchor` is already persisted in `store`. pub fn get_forkchoice_store( + store: Arc>, + init: ForkChoiceStoreInit, + ) -> Result { + match init { + ForkChoiceStoreInit::FinalizedState(anchor) => { + Self::from_finalized_anchor(store, *anchor) + } + ForkChoiceStoreInit::NonFinalizedState { + anchor_state, + justified_state, + justified_block, + finalized_block, + } => Self::from_unfinalized_anchor( + store, + *anchor_state, + *justified_state, + *justified_block, + *finalized_block, + ), + } + } + + pub fn from_finalized_anchor( store: Arc>, anchor: BeaconSnapshot, ) -> Result { @@ -206,15 +239,77 @@ where justified_balances, justified_state_root, finalized_checkpoint, + local_irreversible_checkpoint: justified_checkpoint, unrealized_justified_checkpoint: justified_checkpoint, unrealized_justified_state_root: justified_state_root, unrealized_finalized_checkpoint: finalized_checkpoint, - proposer_boost_root: Hash256::zero(), + proposer_boost_root: Hash256::ZERO, + equivocating_indices: BTreeSet::new(), + _phantom: PhantomData, + }) + } + + pub fn from_unfinalized_anchor( + store: Arc>, + anchor_state: BeaconState, + mut justified_state: BeaconState, + justified_block: SignedBeaconBlock, + finalized_block: SignedBeaconBlock, + ) -> Result { + let anchor_checkpoint = Self::anchor_checkpoint(&anchor_state)?; + let mut justified_checkpoint_on_chain = anchor_state.current_justified_checkpoint(); + let mut finalized_checkpoint_on_chain = anchor_state.finalized_checkpoint(); + + if justified_checkpoint_on_chain.root == Hash256::ZERO { + justified_checkpoint_on_chain = anchor_checkpoint; + } else { + justified_checkpoint_on_chain.root = justified_block.canonical_root(); + } + if finalized_checkpoint_on_chain.root == Hash256::ZERO { + finalized_checkpoint_on_chain = anchor_checkpoint; + } else { + finalized_checkpoint_on_chain.root = finalized_block.canonical_root(); + } + + let justified_state_root = justified_state.canonical_root()?; + let justified_balances = JustifiedBalances::from_justified_state(&justified_state)?; + + let anchor_block_checkpoint = Self::anchor_checkpoint(&anchor_state)?; + + Ok(Self { + store, + balances_cache: <_>::default(), + time: anchor_state.slot(), + justified_checkpoint: justified_checkpoint_on_chain, + justified_balances, + justified_state_root, + finalized_checkpoint: finalized_checkpoint_on_chain, + local_irreversible_checkpoint: anchor_block_checkpoint, + unrealized_justified_checkpoint: justified_checkpoint_on_chain, + unrealized_justified_state_root: justified_state_root, + unrealized_finalized_checkpoint: finalized_checkpoint_on_chain, + proposer_boost_root: Hash256::ZERO, equivocating_indices: BTreeSet::new(), _phantom: PhantomData, }) } + fn anchor_checkpoint(state: &BeaconState) -> Result { + if !state.slot().as_u64().is_multiple_of(E::slots_per_epoch()) { + return Err(Error::UnalignedCheckpoint { + block_slot: state.latest_block_header().slot, + state_slot: state.slot(), + }); + } + + let mut state_for_root = state.clone(); + let state_root = state_for_root.canonical_root()?; + Ok(Checkpoint { + epoch: state.current_epoch(), + root: state.get_latest_block_root(state_root), + }) + } + /// Save the current state of `Self` to a `PersistedForkChoiceStore` which can be stored to the /// on-disk database. pub fn to_persisted(&self) -> PersistedForkChoiceStore { @@ -223,6 +318,7 @@ where finalized_checkpoint: self.finalized_checkpoint, justified_checkpoint: self.justified_checkpoint, justified_state_root: self.justified_state_root, + local_irreversible_checkpoint: self.local_irreversible_checkpoint, unrealized_justified_checkpoint: self.unrealized_justified_checkpoint, unrealized_justified_state_root: self.unrealized_justified_state_root, unrealized_finalized_checkpoint: self.unrealized_finalized_checkpoint, @@ -251,6 +347,7 @@ where justified_checkpoint: persisted.justified_checkpoint, justified_balances, justified_state_root, + local_irreversible_checkpoint: persisted.finalized_checkpoint, unrealized_justified_checkpoint: persisted.unrealized_justified_checkpoint, unrealized_justified_state_root, unrealized_finalized_checkpoint: persisted.unrealized_finalized_checkpoint, @@ -283,6 +380,7 @@ where justified_checkpoint, justified_balances, justified_state_root, + local_irreversible_checkpoint: persisted.local_irreversible_checkpoint, unrealized_justified_checkpoint: persisted.unrealized_justified_checkpoint, unrealized_justified_state_root: persisted.unrealized_justified_state_root, unrealized_finalized_checkpoint: persisted.unrealized_finalized_checkpoint, @@ -318,8 +416,11 @@ where self.balances_cache.process_state(block_root, state) } - fn justified_checkpoint(&self) -> &Checkpoint { - &self.justified_checkpoint + fn justified_checkpoint(&self) -> ForkChoiceCheckpoint { + ForkChoiceCheckpoint::new( + self.local_irreversible_checkpoint, + self.justified_checkpoint, + ) } fn justified_state_root(&self) -> Hash256 { @@ -330,8 +431,11 @@ where &self.justified_balances } - fn finalized_checkpoint(&self) -> &Checkpoint { - &self.finalized_checkpoint + fn finalized_checkpoint(&self) -> ForkChoiceCheckpoint { + ForkChoiceCheckpoint::new( + self.local_irreversible_checkpoint, + self.finalized_checkpoint, + ) } fn unrealized_justified_checkpoint(&self) -> &Checkpoint { @@ -395,6 +499,14 @@ where self.unrealized_finalized_checkpoint = checkpoint; } + fn set_local_irreversible_checkpoint(&mut self, checkpoint: Checkpoint) { + self.local_irreversible_checkpoint = checkpoint; + // Do not update the justified balances. They should match the network justified balances, + // such that all nodes have a consistent fork-choice view. The current balances are cached + // and stored in the fork-choice store. Even the node can't access the justified state, the + // justified balances will remain available. + } + fn set_proposer_boost_root(&mut self, proposer_boost_root: Hash256) { self.proposer_boost_root = proposer_boost_root; } @@ -408,11 +520,11 @@ where } } -pub type PersistedForkChoiceStore = PersistedForkChoiceStoreV28; +pub type PersistedForkChoiceStore = PersistedForkChoiceStoreV29; /// A container which allows persisting the `BeaconForkChoiceStore` to the on-disk database. #[superstruct( - variants(V17, V28), + variants(V17, V28, V29), variant_attributes(derive(Encode, Decode)), no_enum )] @@ -423,14 +535,16 @@ pub struct PersistedForkChoiceStore { pub time: Slot, pub finalized_checkpoint: Checkpoint, pub justified_checkpoint: Checkpoint, + #[superstruct(only(V29))] + pub local_irreversible_checkpoint: Checkpoint, /// The justified balances were removed from disk storage in schema V28. #[superstruct(only(V17))] pub justified_balances: Vec, /// The justified state root is stored so that it can be used to load the justified balances. - #[superstruct(only(V28))] + #[superstruct(only(V28, V29))] pub justified_state_root: Hash256, pub unrealized_justified_checkpoint: Checkpoint, - #[superstruct(only(V28))] + #[superstruct(only(V28, V29))] pub unrealized_justified_state_root: Hash256, pub unrealized_finalized_checkpoint: Checkpoint, pub proposer_boost_root: Hash256, @@ -453,3 +567,36 @@ impl From<(PersistedForkChoiceStoreV28, JustifiedBalances)> for PersistedForkCho } } } + +impl From for PersistedForkChoiceStoreV29 { + fn from(fcs: PersistedForkChoiceStoreV28) -> Self { + Self { + time: fcs.time, + finalized_checkpoint: fcs.finalized_checkpoint, + justified_checkpoint: fcs.justified_checkpoint, + local_irreversible_checkpoint: fcs.finalized_checkpoint, + justified_state_root: fcs.justified_state_root, + unrealized_justified_checkpoint: fcs.unrealized_justified_checkpoint, + unrealized_finalized_checkpoint: fcs.unrealized_finalized_checkpoint, + unrealized_justified_state_root: fcs.unrealized_justified_state_root, + proposer_boost_root: fcs.proposer_boost_root, + equivocating_indices: fcs.equivocating_indices, + } + } +} + +impl From for PersistedForkChoiceStoreV28 { + fn from(fcs: PersistedForkChoiceStoreV29) -> Self { + Self { + time: fcs.time, + finalized_checkpoint: fcs.finalized_checkpoint, + justified_checkpoint: fcs.justified_checkpoint, + justified_state_root: fcs.justified_state_root, + unrealized_justified_checkpoint: fcs.unrealized_justified_checkpoint, + unrealized_finalized_checkpoint: fcs.unrealized_finalized_checkpoint, + unrealized_justified_state_root: fcs.unrealized_justified_state_root, + proposer_boost_root: fcs.proposer_boost_root, + equivocating_indices: fcs.equivocating_indices, + } + } +} diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index fe111628dbf..2d4dde5d225 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -439,6 +439,7 @@ pub fn validate_blob_sidecar_for_gossip( block_root: Hash256, chain: &BeaconChain, ) -> Result<(), BlockError> { - // The finalized checkpoint is being read from fork choice, rather than the cached head. - // - // Fork choice has the most up-to-date view of finalization and there's no point importing a - // block which conflicts with the fork-choice view of finalization. + // Reject blocks that conflict with the local node's irreversible slot. Could be the finalized + // slot, or a more recent slot that the user marked as irreversible. let finalized_slot = chain - .canonical_head - .cached_head() + .head() .finalized_checkpoint() + .local() .epoch .start_slot(T::EthSpec::slots_per_epoch()); @@ -1746,8 +1744,10 @@ pub fn check_block_is_finalized_checkpoint_or_descendant< // descended from that split block. It's important not to try checking `is_descendant` if // finality is ahead of the split and the split block has been pruned, as `is_descendant` will // return `false` in this case. - let finalized_slot = fork_choice + let finalized_slot = chain + .head() .finalized_checkpoint() + .local() .epoch .start_slot(T::EthSpec::slots_per_epoch()); let split = chain.store.get_split_info(); diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index e5b656adf8d..c97b4bede22 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -19,7 +19,8 @@ use crate::shuffling_cache::{BlockShufflingIds, ShufflingCache}; use crate::validator_monitor::{ValidatorMonitor, ValidatorMonitorConfig}; use crate::validator_pubkey_cache::ValidatorPubkeyCache; use crate::{ - BeaconChain, BeaconChainTypes, BeaconForkChoiceStore, BeaconSnapshot, ServerSentEventHandler, + BeaconChain, BeaconChainTypes, BeaconForkChoiceStore, BeaconSnapshot, ForkChoiceStoreInit, + ServerSentEventHandler, }; use bls::Signature; use execution_layer::ExecutionLayer; @@ -401,13 +402,15 @@ where .map_err(|e| format!("Failed to initialize genesis data column info: {:?}", e))?, ); - let fc_store = BeaconForkChoiceStore::get_forkchoice_store(store, genesis.clone()) - .map_err(|e| format!("Unable to initialize fork choice store: {e:?}"))?; + let fc_store = BeaconForkChoiceStore::get_forkchoice_store( + store, + ForkChoiceStoreInit::FinalizedState(Box::new(genesis.clone())), + ) + .map_err(|e| format!("Unable to initialize fork choice store: {e:?}"))?; let current_slot = None; let fork_choice = ForkChoice::from_anchor( fc_store, - genesis.beacon_block_root, &genesis.beacon_block, &genesis.beacon_state, current_slot, @@ -427,6 +430,7 @@ where weak_subj_block: SignedBeaconBlock, weak_subj_blobs: Option>, genesis_state: BeaconState, + fork_choice_store_init: ForkChoiceStoreInit, ) -> Result { let store = self .store @@ -623,12 +627,28 @@ where beacon_state: weak_subj_state, }; - let fc_store = BeaconForkChoiceStore::get_forkchoice_store(store, snapshot.clone()) + let fork_choice_store_init = match fork_choice_store_init { + ForkChoiceStoreInit::FinalizedState(_) => { + ForkChoiceStoreInit::FinalizedState(Box::new(snapshot.clone())) + } + ForkChoiceStoreInit::NonFinalizedState { + justified_state, + justified_block, + finalized_block, + .. + } => ForkChoiceStoreInit::NonFinalizedState { + anchor_state: Box::new(snapshot.beacon_state.clone()), + justified_state, + justified_block, + finalized_block, + }, + }; + + let fc_store = BeaconForkChoiceStore::get_forkchoice_store(store, fork_choice_store_init) .map_err(|e| format!("Unable to initialize fork choice store: {e:?}"))?; let fork_choice = ForkChoice::from_anchor( fc_store, - snapshot.beacon_block_root, &snapshot.beacon_block, &snapshot.beacon_state, Some(weak_subj_slot), @@ -807,7 +827,14 @@ where let (_head_state_root, head_state) = store .get_advanced_hot_state(head_block_root, current_slot, head_block.state_root()) .map_err(|e| descriptive_db_error("head state", &e))? - .ok_or("Head state not found in store")?; + .ok_or_else(|| { + format!( + "Head state not found in store block_root {:?} slot {} state_root {:?}", + head_block_root, + current_slot, + head_block.state_root() + ) + })?; // If the head reverted then we need to reset fork choice using the new head's finalized // checkpoint. @@ -838,7 +865,7 @@ where // consistent. // // This is a sanity check to detect database corruption. - let fc_finalized = fork_choice.finalized_checkpoint(); + let fc_finalized = fork_choice.finalized_checkpoint().on_chain(); let head_finalized = head_snapshot.beacon_state.finalized_checkpoint(); if fc_finalized.epoch < head_finalized.epoch { return Err(format!( @@ -917,7 +944,8 @@ where let genesis_validators_root = head_snapshot.beacon_state.genesis_validators_root(); let genesis_time = head_snapshot.beacon_state.genesis_time(); - let canonical_head = CanonicalHead::new(fork_choice, Arc::new(head_snapshot)); + let canonical_head = CanonicalHead::new(&store, fork_choice, Arc::new(head_snapshot)) + .map_err(|e| format!("Error creating canonical head: {:?}", e))?; let shuffling_cache_size = self.chain_config.shuffling_cache_size; let complete_blob_backfill = self.chain_config.complete_blob_backfill; diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index 1a08ac3f88d..e00197b4086 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -43,8 +43,8 @@ use crate::{ }; use eth2::types::{EventKind, SseChainReorg, SseFinalizedCheckpoint, SseLateHead}; use fork_choice::{ - ExecutionStatus, ForkChoiceStore, ForkChoiceView, ForkchoiceUpdateParameters, ProtoBlock, - ResetPayloadStatuses, + ExecutionStatus, ForkChoiceCheckpoint, ForkChoiceStore, ForkChoiceView, + ForkchoiceUpdateParameters, ProtoBlock, ResetPayloadStatuses, }; use itertools::process_results; @@ -102,19 +102,16 @@ pub struct CachedHead { /// /// This value may be distinct to the `self.snapshot.beacon_state.justified_checkpoint`. /// This value should be used over the beacon state value in practically all circumstances. - justified_checkpoint: Checkpoint, + justified_checkpoint: ForkChoiceCheckpoint, /// The finalized checkpoint as per `self.fork_choice`. /// /// This value may be distinct to the `self.snapshot.beacon_state.finalized_checkpoint`. /// This value should be used over the beacon state value in practically all circumstances. - finalized_checkpoint: Checkpoint, - /// The `execution_payload.block_hash` of the block at the head of the chain. Set to `None` - /// before Bellatrix. - head_hash: Option, - /// The `execution_payload.block_hash` of the justified block. Set to `None` before Bellatrix. - justified_hash: Option, - /// The `execution_payload.block_hash` of the finalized block. Set to `None` before Bellatrix. - finalized_hash: Option, + finalized_checkpoint: ForkChoiceCheckpoint, + /// The `execution_payload.block_hash` of the block at the head of the chain, the justified root + /// and the finalized root. Maybe None before Bellatrix, and if the blocks they reference are + /// unknown. + forkchoice_update_parameters: ForkchoiceUpdateParameters, } impl CachedHead { @@ -205,7 +202,7 @@ impl CachedHead { /// This is *not* the finalized checkpoint of the `head_snapshot.beacon_state`, rather it is the /// best finalized checkpoint that has been observed by `self.fork_choice`. It is possible that /// the `head_snapshot.beacon_state` finalized value is earlier than the one returned here. - pub fn finalized_checkpoint(&self) -> Checkpoint { + pub fn finalized_checkpoint(&self) -> ForkChoiceCheckpoint { self.finalized_checkpoint } @@ -217,20 +214,19 @@ impl CachedHead { /// it is the justified checkpoint in the view of `self.fork_choice`. It is possible that the /// `head_snapshot.beacon_state` justified value is different to, but not conflicting with, the /// one returned here. - pub fn justified_checkpoint(&self) -> Checkpoint { + pub fn justified_checkpoint(&self) -> ForkChoiceCheckpoint { self.justified_checkpoint } + pub fn finalized_checkpoint_from_head_state(&self) -> Checkpoint { + self.snapshot.beacon_state.finalized_checkpoint() + } + /// Returns the cached values of `ForkChoice::forkchoice_update_parameters`. /// /// Useful for supplying to the execution layer. pub fn forkchoice_update_parameters(&self) -> ForkchoiceUpdateParameters { - ForkchoiceUpdateParameters { - head_root: self.snapshot.beacon_block_root, - head_hash: self.head_hash, - justified_hash: self.justified_hash, - finalized_hash: self.finalized_hash, - } + self.forkchoice_update_parameters } } @@ -260,25 +256,43 @@ pub struct CanonicalHead { impl CanonicalHead { /// Instantiate `Self`. pub fn new( + store: &BeaconStore, fork_choice: BeaconForkChoice, snapshot: Arc>, - ) -> Self { - let fork_choice_view = fork_choice.cached_fork_choice_view(); - let forkchoice_update_params = fork_choice.get_forkchoice_update_parameters(); - let cached_head = CachedHead { - snapshot, - justified_checkpoint: fork_choice_view.justified_checkpoint, - finalized_checkpoint: fork_choice_view.finalized_checkpoint, - head_hash: forkchoice_update_params.head_hash, - justified_hash: forkchoice_update_params.justified_hash, - finalized_hash: forkchoice_update_params.finalized_hash, - }; - - Self { + ) -> Result { + let cached_head = Self::new_cached_head(store, &fork_choice, snapshot)?; + Ok(Self { fork_choice: CanonicalHeadRwLock::new(fork_choice), cached_head: CanonicalHeadRwLock::new(cached_head), recompute_head_lock: Mutex::new(()), - } + }) + } + + fn new_cached_head( + store: &BeaconStore, + fork_choice: &BeaconForkChoice, + snapshot: Arc>, + ) -> Result, Error> { + let head_block_root = snapshot.beacon_block_root; + let justified_checkpoint = fork_choice.justified_checkpoint(); + let finalized_checkpoint = fork_choice.finalized_checkpoint(); + let forkchoice_update_parameters = compute_fork_choice_update_parameters::( + fork_choice, + store, + ForkChoiceView { + head_block_root, + justified_checkpoint, + finalized_checkpoint, + }, + None, + )?; + + Ok(CachedHead { + snapshot, + justified_checkpoint, + finalized_checkpoint, + forkchoice_update_parameters, + }) } /// Load a persisted version of `BeaconForkChoice` from the `store` and restore `self` to that @@ -296,15 +310,14 @@ impl CanonicalHead { store: &BeaconStore, spec: &ChainSpec, ) -> Result<(), Error> { - let fork_choice = + let mut fork_choice = >::load_fork_choice(store.clone(), reset_payload_statuses, spec)? .ok_or(Error::MissingPersistedForkChoice)?; - let fork_choice_view = fork_choice.cached_fork_choice_view(); - let beacon_block_root = fork_choice_view.head_block_root; + let current_slot = fork_choice.fc_store().get_current_slot(); + let beacon_block_root = fork_choice.get_head(current_slot, spec)?; let beacon_block = store .get_full_block(&beacon_block_root)? .ok_or(Error::MissingBeaconBlock(beacon_block_root))?; - let current_slot = fork_choice.fc_store().get_current_slot(); let (_, beacon_state) = store .get_advanced_hot_state(beacon_block_root, current_slot, beacon_block.state_root())? .ok_or(Error::MissingBeaconState(beacon_block.state_root()))?; @@ -315,15 +328,7 @@ impl CanonicalHead { beacon_state, }; - let forkchoice_update_params = fork_choice.get_forkchoice_update_parameters(); - let cached_head = CachedHead { - snapshot: Arc::new(snapshot), - justified_checkpoint: fork_choice_view.justified_checkpoint, - finalized_checkpoint: fork_choice_view.finalized_checkpoint, - head_hash: forkchoice_update_params.head_hash, - justified_hash: forkchoice_update_params.justified_hash, - finalized_hash: forkchoice_update_params.finalized_hash, - }; + let cached_head = Self::new_cached_head(store, &fork_choice, snapshot.into())?; *fork_choice_write_lock = fork_choice; // Avoid interleaving the fork choice and cached head locks. @@ -526,7 +531,7 @@ impl BeaconChain { .spawn_blocking_handle( move || { let _guard = span.enter(); - chain.recompute_head_at_slot_internal(current_slot) + chain.recompute_head_at_slot_internal(current_slot, false) }, "recompute_head_internal", ) @@ -569,6 +574,44 @@ impl BeaconChain { } } + /// Execute fork choice and update the canonical head, returning any error. + /// + /// Unlike `Self::recompute_head_at_slot`, this method propagates post-update errors (e.g. + /// finalization side-effects) to the caller. + pub async fn recompute_head_at_slot_strict( + self: &Arc, + current_slot: Slot, + ) -> Result<(), Error> { + let span = info_span!( + "lh_recompute_head_at_slot", + slot = %current_slot + ); + + metrics::inc_counter(&metrics::FORK_CHOICE_REQUESTS); + let _timer = metrics::start_timer(&metrics::FORK_CHOICE_TIMES); + + let chain = self.clone(); + let opt_el_update = self + .spawn_blocking_handle( + move || { + let _guard = span.enter(); + chain.recompute_head_at_slot_internal(current_slot, true) + }, + "recompute_head_internal", + ) + .await??; + + if let Some(join_handle) = opt_el_update { + match join_handle.await { + Ok(Some(())) => Ok(()), + Ok(None) => Err(Error::RuntimeShutdown), + Err(e) => Err(Error::TokioJoin(e)), + } + } else { + Ok(()) + } + } + /// A non-async (blocking) function which recomputes the canonical head and spawns async tasks. /// /// This function performs long-running, heavy-lifting tasks which should not be performed on @@ -576,6 +619,7 @@ impl BeaconChain { fn recompute_head_at_slot_internal( self: &Arc, current_slot: Slot, + strict_finalization_errors: bool, ) -> Result>>, Error> { let recompute_head_lock = self.canonical_head.recompute_head_lock.lock(); @@ -597,19 +641,22 @@ impl BeaconChain { let mut fork_choice_write_lock = self.canonical_head.fork_choice_write_lock(); // Recompute the current head via the fork choice algorithm. - fork_choice_write_lock.get_head(current_slot, &self.spec)?; + let new_head_block_root = fork_choice_write_lock.get_head(current_slot, &self.spec)?; // Downgrade the fork choice write-lock to a read lock, without allowing access to any // other writers. let fork_choice_read_lock = RwLockWriteGuard::downgrade(fork_choice_write_lock); // Read the current head value from the fork choice algorithm. - let new_view = fork_choice_read_lock.cached_fork_choice_view(); + let new_view = ForkChoiceView { + head_block_root: new_head_block_root, + justified_checkpoint: fork_choice_read_lock.justified_checkpoint(), + finalized_checkpoint: fork_choice_read_lock.finalized_checkpoint(), + }; // Check to ensure that the finalized block hasn't been marked as invalid. If it has, // shut down Lighthouse. - let finalized_proto_block = fork_choice_read_lock.get_finalized_block()?; - check_finalized_payload_validity(self, &finalized_proto_block)?; + check_finalized_payload_validity(self, &fork_choice_read_lock.local_irreversible_block()?)?; // Sanity check the finalized checkpoint. // @@ -654,8 +701,13 @@ impl BeaconChain { // Get the parameters to update the execution layer since either the head or some finality // parameters have changed. - let new_forkchoice_update_parameters = - fork_choice_read_lock.get_forkchoice_update_parameters(); + let new_forkchoice_update_parameters = compute_fork_choice_update_parameters::( + &fork_choice_read_lock, + &self.store, + new_view, + // Pass the old cached params to prevent DB reads + Some((old_view, old_cached_head.forkchoice_update_parameters())), + )?; perform_debug_logging::(&old_view, &new_view, &fork_choice_read_lock); @@ -697,9 +749,7 @@ impl BeaconChain { snapshot: Arc::new(new_snapshot), justified_checkpoint: new_view.justified_checkpoint, finalized_checkpoint: new_view.finalized_checkpoint, - head_hash: new_forkchoice_update_parameters.head_hash, - justified_hash: new_forkchoice_update_parameters.justified_hash, - finalized_hash: new_forkchoice_update_parameters.finalized_hash, + forkchoice_update_parameters: new_forkchoice_update_parameters, }; let new_head = { @@ -724,9 +774,7 @@ impl BeaconChain { snapshot: old_cached_head.snapshot.clone(), justified_checkpoint: new_view.justified_checkpoint, finalized_checkpoint: new_view.finalized_checkpoint, - head_hash: new_forkchoice_update_parameters.head_hash, - justified_hash: new_forkchoice_update_parameters.justified_hash, - finalized_hash: new_forkchoice_update_parameters.finalized_hash, + forkchoice_update_parameters: new_forkchoice_update_parameters, }; let mut cached_head_write_lock = self.canonical_head.cached_head_write_lock(); @@ -764,8 +812,11 @@ impl BeaconChain { // is a dead-lock risk to hold any other lock on fork choice at this point. if new_view.finalized_checkpoint != old_view.finalized_checkpoint && let Err(e) = - self.after_finalization(&new_cached_head, new_view, finalized_proto_block) + self.after_finalization(&new_cached_head, new_view, strict_finalization_errors) { + if strict_finalization_errors { + return Err(e); + } crit!( error = ?e, "Error updating finalization" @@ -896,31 +947,33 @@ impl BeaconChain { self: &Arc, new_cached_head: &CachedHead, new_view: ForkChoiceView, - finalized_proto_block: ProtoBlock, + force_foreground_migration: bool, ) -> Result<(), Error> { let _timer = metrics::start_timer(&metrics::FORK_CHOICE_AFTER_FINALIZATION_TIMES); let new_snapshot = &new_cached_head.snapshot; - let finalized_block_is_optimistic = finalized_proto_block - .execution_status - .is_optimistic_or_invalid(); - self.observed_block_producers.write().prune( - new_view - .finalized_checkpoint - .epoch - .start_slot(T::EthSpec::slots_per_epoch()), - ); + let on_chain_finalized_checkpoint = new_view.finalized_checkpoint.on_chain(); + let new_local_finalized_checkpoint = new_view.finalized_checkpoint.local(); + let new_local_finalized_slot = new_local_finalized_checkpoint + .epoch + .start_slot(T::EthSpec::slots_per_epoch()); - self.observed_blob_sidecars.write().prune( - new_view - .finalized_checkpoint - .epoch - .start_slot(T::EthSpec::slots_per_epoch()), - ); + // observed_block_producers uses the finalized slot to reject blocks prior to that slot. It + // acts as a local irreversible slot, so use the fork-choice's local to match gossip + // verification. + self.observed_block_producers + .write() + .prune(new_local_finalized_slot); + + // Same as observed_block_producers + self.observed_blob_sidecars + .write() + .prune(new_local_finalized_slot); self.observed_column_sidecars.write().prune( new_view .finalized_checkpoint + .local() .epoch .start_slot(T::EthSpec::slots_per_epoch()), ); @@ -928,21 +981,28 @@ impl BeaconChain { self.observed_slashable.write().prune( new_view .finalized_checkpoint + .local() .epoch .start_slot(T::EthSpec::slots_per_epoch()), ); if let Some(event_handler) = self.event_handler.as_ref() && event_handler.has_finalized_subscribers() + && let Some(on_chain_finalized_block) = self + .canonical_head + .fork_choice_read_lock() + .get_block(&on_chain_finalized_checkpoint.root) { event_handler.register(EventKind::FinalizedCheckpoint(SseFinalizedCheckpoint { - epoch: new_view.finalized_checkpoint.epoch, - block: new_view.finalized_checkpoint.root, + epoch: on_chain_finalized_checkpoint.epoch, + block: on_chain_finalized_checkpoint.root, // Provide the state root of the latest finalized block, rather than the // specific state root at the first slot of the finalized epoch (which // might be a skip slot). - state: finalized_proto_block.state_root, - execution_optimistic: finalized_block_is_optimistic, + state: on_chain_finalized_block.state_root, + execution_optimistic: on_chain_finalized_block + .execution_status + .is_optimistic_or_invalid(), })); } @@ -952,29 +1012,36 @@ impl BeaconChain { // // Use the `StateRootsIterator` directly rather than `BeaconChain::state_root_at_slot` // to ensure we use the same state that we just set as the head. - let new_finalized_slot = new_view - .finalized_checkpoint + let new_local_finalized_slot = new_local_finalized_checkpoint .epoch .start_slot(T::EthSpec::slots_per_epoch()); - let new_finalized_state_root = process_results( - StateRootsIterator::new(&self.store, &new_snapshot.beacon_state), - |mut iter| { - iter.find_map(|(state_root, slot)| { - if slot == new_finalized_slot { - Some(state_root) - } else { - None - } - }) - }, - )? - .ok_or(Error::MissingFinalizedStateRoot(new_finalized_slot))?; + let new_local_finalized_state_root = + if new_local_finalized_slot == new_snapshot.beacon_state.slot() { + new_snapshot.beacon_block.state_root() + } else { + process_results( + StateRootsIterator::new(&self.store, &new_snapshot.beacon_state), + |mut iter| { + iter.find_map(|(state_root, slot)| { + if slot == new_local_finalized_slot { + Some(state_root) + } else { + None + } + }) + }, + )? + .ok_or(Error::MissingFinalizedStateRoot { + state_slot: new_snapshot.beacon_state.slot(), + target_slot: new_local_finalized_slot, + })? + }; let update_cache = true; let new_finalized_state = self .store - .get_hot_state(&new_finalized_state_root, update_cache)? - .ok_or(Error::MissingBeaconState(new_finalized_state_root))?; + .get_hot_state(&new_local_finalized_state_root, update_cache)? + .ok_or(Error::MissingBeaconState(new_local_finalized_state_root))?; self.op_pool.prune_all( &new_snapshot.beacon_block, @@ -987,9 +1054,13 @@ impl BeaconChain { // We just pass the state root to the finalization thread. It should be able to reload the // state from the state_cache near instantly anyway. We could experiment with sending the // state over a channel in future, but it's probably no quicker. + // + // For manual finalization we force foreground migration so API callers get an accurate + // success/failure response. self.store_migrator.process_finalization( - new_finalized_state_root.into(), - new_view.finalized_checkpoint, + new_local_finalized_state_root.into(), + new_local_finalized_checkpoint, + force_foreground_migration, )?; // Prune blobs in the background. @@ -1004,6 +1075,31 @@ impl BeaconChain { Ok(()) } + pub async fn manual_finalization( + self: &Arc, + checkpoint: Checkpoint, + ) -> Result<(), Error> { + // Set the irreversible checkpoint first. This will cause finality to advance if the + // checkpoint is greater than current finality. + let chain = self.clone(); + self.spawn_blocking_handle( + move || { + chain + .canonical_head + .fork_choice_write_lock() + .set_local_irreversible_checkpoint(checkpoint) + }, + "set_local_irreversible_checkpoint", + ) + .await??; + + // Allow to set an irreversible checkpoint that is not an ancestor of the current head. If + // that happens the current head will become non-viable. Re-compute head in case the + // current head is not a descendant of the irreversible checkpoint. + self.recompute_head_at_slot_strict(self.slot()?).await?; + Ok(()) + } + /// Persist fork choice to disk, writing immediately. pub fn persist_fork_choice(&self) -> Result<(), Error> { let _fork_choice_timer = metrics::start_timer(&metrics::PERSIST_FORK_CHOICE); @@ -1034,6 +1130,58 @@ impl BeaconChain { } } +fn compute_fork_choice_update_parameters( + fork_choice: &BeaconForkChoice, + store: &BeaconStore, + new_view: ForkChoiceView, + old_view_and_params: Option<(ForkChoiceView, ForkchoiceUpdateParameters)>, +) -> Result { + // TODO(non-fin-cp-sync): This values maybe be None. It's unclear if ELs can handle + // zero hashes. We decide to only send to the EL the "real" network on chain finalized + // and justified hashes such that the "safe" tag retains the same economic security no + // matter in what mode this beacon node is running. + let get_execution_hash = |block_root: &Hash256| -> Result, Error> { + if let Some(execution_hash) = fork_choice.execution_hash(*block_root) { + return Ok(Some(execution_hash)); + } + Ok(store.get_blinded_block(block_root)?.and_then(|block| { + if let Ok(payload) = block.message().execution_payload() { + Some(payload.block_hash()) + } else { + None + } + })) + }; + + Ok(ForkchoiceUpdateParameters { + head_root: new_view.head_block_root, + // Only fetch the execution hashes of head and finality checkpoints if necessary. The + // fork-choice may not include nodes for the finalized / justified blocks, so a DB read + // may be necessary. + head_hash: if let Some((old_view, old_params)) = old_view_and_params + && new_view.head_block_root == old_view.head_block_root + { + old_params.head_hash + } else { + get_execution_hash(&new_view.head_block_root)? + }, + justified_hash: if let Some((old_view, old_params)) = old_view_and_params + && new_view.justified_checkpoint.on_chain() == old_view.justified_checkpoint.on_chain() + { + old_params.justified_hash + } else { + get_execution_hash(&new_view.justified_checkpoint.on_chain().root)? + }, + finalized_hash: if let Some((old_view, old_params)) = old_view_and_params + && new_view.finalized_checkpoint.on_chain() == old_view.finalized_checkpoint.on_chain() + { + old_params.finalized_hash + } else { + get_execution_hash(&new_view.finalized_checkpoint.on_chain().root)? + }, + }) +} + /// Check to see if the `finalized_proto_block` has an invalid execution payload. If so, shut down /// Lighthouse. /// @@ -1075,16 +1223,17 @@ fn check_against_finality_reversion( old_view: &ForkChoiceView, new_view: &ForkChoiceView, ) -> Result<(), Error> { - let finalization_equal = new_view.finalized_checkpoint == old_view.finalized_checkpoint; - let finalization_advanced = - new_view.finalized_checkpoint.epoch > old_view.finalized_checkpoint.epoch; + let new_checkpoint = new_view.finalized_checkpoint.on_chain(); + let old_checkpoint = old_view.finalized_checkpoint.on_chain(); + let finalization_equal = new_checkpoint == old_checkpoint; + let finalization_advanced = new_checkpoint.epoch > old_checkpoint.epoch; if finalization_equal || finalization_advanced { Ok(()) } else { Err(Error::RevertedFinalizedEpoch { - old: old_view.finalized_checkpoint, - new: new_view.finalized_checkpoint, + old: old_checkpoint, + new: new_checkpoint, }) } } @@ -1107,19 +1256,15 @@ fn perform_debug_logging( } if new_view.justified_checkpoint != old_view.justified_checkpoint { debug!( - new_root = ?new_view.justified_checkpoint.root, - new_epoch = %new_view.justified_checkpoint.epoch, - old_root = ?old_view.justified_checkpoint.root, - old_epoch = %old_view.justified_checkpoint.epoch, + new = %new_view.justified_checkpoint, + old = %old_view.justified_checkpoint, "Fork choice justified" ) } if new_view.finalized_checkpoint != old_view.finalized_checkpoint { debug!( - new_root = ?new_view.finalized_checkpoint.root, - new_epoch = %new_view.finalized_checkpoint.epoch, - old_root = ?old_view.finalized_checkpoint.root, - old_epoch = %old_view.finalized_checkpoint.epoch, + new = %new_view.finalized_checkpoint, + old = %old_view.finalized_checkpoint, "Fork choice finalized" ) } diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index e266e02f7f2..01fd1718b68 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -611,11 +611,7 @@ async fn availability_cache_maintenance_service( continue; } - let finalized_epoch = chain - .canonical_head - .fork_choice_read_lock() - .finalized_checkpoint() - .epoch; + let finalized_epoch = chain.head().finalized_checkpoint().on_chain().epoch; let Some(min_epochs_for_blobs) = chain .spec diff --git a/beacon_node/beacon_chain/src/data_column_verification.rs b/beacon_node/beacon_chain/src/data_column_verification.rs index cf3385ec5b0..906580efce9 100644 --- a/beacon_node/beacon_chain/src/data_column_verification.rs +++ b/beacon_node/beacon_chain/src/data_column_verification.rs @@ -777,6 +777,7 @@ fn verify_slot_greater_than_latest_finalized_slot( let latest_finalized_slot = chain .head() .finalized_checkpoint() + .local() .epoch .start_slot(T::EthSpec::slots_per_epoch()); if column_slot <= latest_finalized_slot { diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index 816e75fd242..a07bd44b527 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -75,7 +75,10 @@ pub enum BeaconChainError { ProposerSlashingValidationError(ProposerSlashingValidationError), AttesterSlashingValidationError(AttesterSlashingValidationError), BlsExecutionChangeValidationError(BlsExecutionChangeValidationError), - MissingFinalizedStateRoot(Slot), + MissingFinalizedStateRoot { + state_slot: Slot, + target_slot: Slot, + }, SszTypesError(SszTypesError), NoProposerForSlot(Slot), CanonicalHeadLockTimeout, @@ -181,10 +184,7 @@ pub enum BeaconChainError { execution_block_hash: Option, }, ForkchoiceUpdate(execution_layer::Error), - InvalidCheckpoint { - state_root: Hash256, - checkpoint: Checkpoint, - }, + InvalidCheckpoint(String), InvalidSlot(Slot), HeadBlockNotFullyVerified { beacon_block_root: Hash256, diff --git a/beacon_node/beacon_chain/src/fork_revert.rs b/beacon_node/beacon_chain/src/fork_revert.rs index 4db79790d38..076bc652987 100644 --- a/beacon_node/beacon_chain/src/fork_revert.rs +++ b/beacon_node/beacon_chain/src/fork_revert.rs @@ -1,4 +1,4 @@ -use crate::{BeaconForkChoiceStore, BeaconSnapshot}; +use crate::{BeaconForkChoiceStore, BeaconSnapshot, ForkChoiceStoreInit}; use fork_choice::{ForkChoice, PayloadVerificationStatus}; use itertools::process_results; use state_processing::state_advance::complete_state_advance; @@ -137,18 +137,19 @@ pub fn reset_fork_choice_to_finalization, Cold: It ) })?; let finalized_snapshot = BeaconSnapshot { - beacon_block_root: finalized_block_root, + beacon_block_root: finalized_block.canonical_root(), beacon_block: Arc::new(finalized_block), beacon_state: finalized_state, }; - let fc_store = - BeaconForkChoiceStore::get_forkchoice_store(store.clone(), finalized_snapshot.clone()) - .map_err(|e| format!("Unable to reset fork choice store for revert: {e:?}"))?; + let fc_store = BeaconForkChoiceStore::get_forkchoice_store( + store.clone(), + ForkChoiceStoreInit::FinalizedState(Box::new(finalized_snapshot.clone())), + ) + .map_err(|e| format!("Unable to reset fork choice store for revert: {e:?}"))?; let mut fork_choice = ForkChoice::from_anchor( fc_store, - finalized_block_root, &finalized_snapshot.beacon_block, &finalized_snapshot.beacon_state, current_slot, diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index e77739e2d53..5ee0896715f 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -71,8 +71,8 @@ pub use self::errors::{BeaconChainError, BlockProductionError}; pub use self::historical_blocks::HistoricalBlockError; pub use attestation_verification::Error as AttestationError; pub use beacon_fork_choice_store::{ - BeaconForkChoiceStore, Error as ForkChoiceStoreError, PersistedForkChoiceStoreV17, - PersistedForkChoiceStoreV28, + BeaconForkChoiceStore, Error as ForkChoiceStoreError, ForkChoiceStoreInit, + PersistedForkChoiceStoreV17, PersistedForkChoiceStoreV28, }; pub use block_verification::{ BlockError, ExecutionPayloadError, ExecutionPendingBlock, GossipVerifiedBlock, diff --git a/beacon_node/beacon_chain/src/migrate.rs b/beacon_node/beacon_chain/src/migrate.rs index 24258d2d31b..4b6692c8714 100644 --- a/beacon_node/beacon_chain/src/migrate.rs +++ b/beacon_node/beacon_chain/src/migrate.rs @@ -120,15 +120,9 @@ pub enum Notification { Finalization(FinalizationNotification), Reconstruction, PruneBlobs(Epoch), - ManualFinalization(ManualFinalizationNotification), ManualCompaction, } -pub struct ManualFinalizationNotification { - pub state_root: BeaconStateHash, - pub checkpoint: Checkpoint, -} - pub struct FinalizationNotification { pub finalized_state_root: BeaconStateHash, pub finalized_checkpoint: Checkpoint, @@ -164,6 +158,7 @@ impl, Cold: ItemStore> BackgroundMigrator Result<(), BeaconChainError> { let notif = FinalizationNotification { finalized_state_root, @@ -171,11 +166,15 @@ impl, Cold: ItemStore> BackgroundMigrator, Cold: ItemStore> BackgroundMigrator, Cold: ItemStore> BackgroundMigrator>, - notif: ManualFinalizationNotification, - ) { - // We create a "dummy" prev migration - let prev_migration = PrevMigration { - epoch: Epoch::new(1), - epochs_per_migration: 2, - }; - let notif = FinalizationNotification { - finalized_state_root: notif.state_root, - finalized_checkpoint: notif.checkpoint, - prev_migration: Arc::new(prev_migration.into()), - }; - Self::run_migration(db, notif); - } - /// Perform the actual work of `process_finalization`. - fn run_migration(db: Arc>, notif: FinalizationNotification) { + fn run_migration( + db: Arc>, + notif: FinalizationNotification, + force_run: bool, + ) -> Result<(), BeaconChainError> { // Do not run too frequently. let epoch = notif.finalized_checkpoint.epoch; let mut prev_migration = notif.prev_migration.lock(); - if epoch < prev_migration.epoch + prev_migration.epochs_per_migration { + if !force_run && epoch < prev_migration.epoch + prev_migration.epochs_per_migration { debug!( last_finalized_epoch = %prev_migration.epoch, new_finalized_epoch = %epoch, epochs_per_migration = prev_migration.epochs_per_migration, "Database consolidation deferred" ); - return; + return Ok(()); } // Update the previous migration epoch immediately to avoid holding the lock. If the @@ -332,14 +310,12 @@ impl, Cold: ItemStore> BackgroundMigrator state, - other => { - error!( - state_root = ?finalized_state_root, - error = ?other, - "Migrator failed to load state" - ); - return; + Ok(None) => { + return Err(BeaconChainError::MissingBeaconState( + finalized_state_root.into(), + )); } + Err(e) => return Err(BeaconChainError::DBError(e)), }; let split_prior_to_migration = match migrate_database( @@ -361,8 +337,7 @@ impl, Cold: ItemStore> BackgroundMigrator { - warn!(error = ?e, "Database migration failed"); - return; + return Err(BeaconChainError::DBError(e)); } }; @@ -377,31 +352,22 @@ impl, Cold: ItemStore> BackgroundMigrator old_finalized_checkpoint_epoch, Ok(PruningOutcome::DeferredConcurrentHeadTrackerMutation) => { - warn!( - message = "this is expected only very rarely!", - "Pruning deferred because of a concurrent mutation" - ); - return; + return Err(BeaconChainError::DBInconsistent( + "Pruning deferred due to concurrent head tracker mutation".to_string(), + )); } Ok(PruningOutcome::OutOfOrderFinalization { old_finalized_checkpoint, new_finalized_checkpoint, }) => { - warn!( - old_finalized_epoch = %old_finalized_checkpoint.epoch, - new_finalized_epoch = %new_finalized_checkpoint.epoch, - message = "this is expected occasionally due to a (harmless) race condition", - "Ignoring out of order finalization request" - ); - return; - } - Err(e) => { - warn!( - error = ?e, - "Hot DB pruning failed" - ); - return; + return Err(BeaconChainError::PruningError( + PruningError::FinalizedStateOutOfOrder { + old_finalized_checkpoint, + new_finalized_checkpoint, + }, + )); } + Err(e) => return Err(e), }; // Finally, compact the database so that new free space is properly reclaimed. @@ -410,10 +376,11 @@ impl, Cold: ItemStore> BackgroundMigrator>) { @@ -437,13 +404,11 @@ impl, Cold: ItemStore> BackgroundMigrator reconstruction_notif = Some(notif), Notification::Finalization(fin) => finalization_notif = Some(fin), - Notification::ManualFinalization(fin) => manual_finalization_notif = Some(fin), Notification::PruneBlobs(dab) => prune_blobs_notif = Some(dab), Notification::ManualCompaction => manual_compaction_notif = Some(notif), } @@ -452,15 +417,6 @@ impl, Cold: ItemStore> BackgroundMigrator reconstruction_notif = Some(notif), Notification::ManualCompaction => manual_compaction_notif = Some(notif), - Notification::ManualFinalization(fin) => { - if let Some(current) = manual_finalization_notif.as_mut() { - if fin.checkpoint.epoch > current.checkpoint.epoch { - *current = fin; - } - } else { - manual_finalization_notif = Some(fin); - } - } Notification::Finalization(fin) => { if let Some(current) = finalization_notif.as_mut() { if fin.finalized_checkpoint.epoch @@ -480,11 +436,10 @@ impl, Cold: ItemStore> BackgroundMigrator Result { + let decompressed_bytes = store_config + .decompress_bytes(bytes) + .map_err(Error::Compression)?; + Self::from_ssz_bytes(&decompressed_bytes).map_err(Into::into) + } + + // For the v29 to v28 schema migration + pub fn as_bytes(&self, store_config: &StoreConfig) -> Result, Error> { + store_config + .compress_bytes(&self.as_ssz_bytes()) + .map_err(Error::Compression) + } +} + +// For the v28 to v29 schema migration +impl From for PersistedForkChoiceV29 { + fn from(persisted_fork_choice_v28: PersistedForkChoiceV28) -> Self { + Self { + fork_choice: persisted_fork_choice_v28.fork_choice, + fork_choice_store: persisted_fork_choice_v28.fork_choice_store_v28.into(), + } + } +} + +// For the v29 to v28 schema migration +impl From for PersistedForkChoiceV28 { + fn from(persisted_fork_choice_v29: PersistedForkChoiceV29) -> Self { + Self { + fork_choice: persisted_fork_choice_v29.fork_choice, + fork_choice_store_v28: persisted_fork_choice_v29.fork_choice_store.into(), + } + } +} + +impl PersistedForkChoiceV29 { pub fn from_bytes(bytes: &[u8], store_config: &StoreConfig) -> Result { let decompressed_bytes = store_config .decompress_bytes(bytes) diff --git a/beacon_node/beacon_chain/src/schema_change.rs b/beacon_node/beacon_chain/src/schema_change.rs index ddc59783394..aa4925deb30 100644 --- a/beacon_node/beacon_chain/src/schema_change.rs +++ b/beacon_node/beacon_chain/src/schema_change.rs @@ -5,6 +5,7 @@ mod migration_schema_v25; mod migration_schema_v26; mod migration_schema_v27; mod migration_schema_v28; +mod migration_schema_v29; use crate::beacon_chain::BeaconChainTypes; use std::sync::Arc; @@ -88,6 +89,14 @@ pub fn migrate_schema( let ops = migration_schema_v28::downgrade_from_v28::(db.clone())?; db.store_schema_version_atomically(to, ops) } + (SchemaVersion(28), SchemaVersion(29)) => { + let ops = migration_schema_v29::upgrade_to_v29::(db.clone())?; + db.store_schema_version_atomically(to, ops) + } + (SchemaVersion(29), SchemaVersion(28)) => { + let ops = migration_schema_v29::downgrade_from_v29::(db.clone())?; + db.store_schema_version_atomically(to, ops) + } // Anything else is an error. (_, _) => Err(HotColdDBError::UnsupportedSchemaVersion { target_version: to, diff --git a/beacon_node/beacon_chain/src/schema_change/migration_schema_v23.rs b/beacon_node/beacon_chain/src/schema_change/migration_schema_v23.rs index e238e1efb6c..cbbd5598df9 100644 --- a/beacon_node/beacon_chain/src/schema_change/migration_schema_v23.rs +++ b/beacon_node/beacon_chain/src/schema_change/migration_schema_v23.rs @@ -122,7 +122,9 @@ pub fn downgrade_from_v23( let heads = fork_choice .proto_array() - .heads_descended_from_finalization::(fork_choice.finalized_checkpoint()); + .heads_descended_from_finalization::( + fork_choice.finalized_checkpoint().as_local(), + ); let head_roots = heads.iter().map(|node| node.root).collect(); let head_slots = heads.iter().map(|node| node.slot).collect(); diff --git a/beacon_node/beacon_chain/src/schema_change/migration_schema_v28.rs b/beacon_node/beacon_chain/src/schema_change/migration_schema_v28.rs index 5885eaabc00..f37df5e0f71 100644 --- a/beacon_node/beacon_chain/src/schema_change/migration_schema_v28.rs +++ b/beacon_node/beacon_chain/src/schema_change/migration_schema_v28.rs @@ -121,7 +121,7 @@ pub fn downgrade_from_v28( // Recreate V28 persisted fork choice, then convert each field back to its V17 version. let persisted_fork_choice = PersistedForkChoiceV28 { fork_choice: fork_choice.to_persisted(), - fork_choice_store: fork_choice.fc_store().to_persisted(), + fork_choice_store_v28: fork_choice.fc_store().to_persisted().into(), }; let justified_balances = fork_choice.fc_store().justified_balances(); @@ -134,7 +134,7 @@ pub fn downgrade_from_v28( .into(); let fork_choice_store_v17: PersistedForkChoiceStoreV17 = ( - persisted_fork_choice.fork_choice_store, + persisted_fork_choice.fork_choice_store_v28, justified_balances.clone(), ) .into(); diff --git a/beacon_node/beacon_chain/src/schema_change/migration_schema_v29.rs b/beacon_node/beacon_chain/src/schema_change/migration_schema_v29.rs new file mode 100644 index 00000000000..7ad432c5b7d --- /dev/null +++ b/beacon_node/beacon_chain/src/schema_change/migration_schema_v29.rs @@ -0,0 +1,52 @@ +use crate::BeaconChainTypes; +use crate::beacon_chain::FORK_CHOICE_DB_KEY; +use crate::persisted_fork_choice::{PersistedForkChoiceV28, PersistedForkChoiceV29}; +use std::sync::Arc; +use store::{DBColumn, Error, HotColdDB, KeyValueStore, KeyValueStoreOp}; + +/// Upgrade fork-choice store to v29. +pub fn upgrade_to_v29( + db: Arc>, +) -> Result, Error> { + let persisted_fork_choice_v28 = PersistedForkChoiceV28::from_bytes( + &db.hot_db + .get_bytes(DBColumn::ForkChoice, FORK_CHOICE_DB_KEY.as_slice())? + .ok_or( + // Fork choice should exist if the database exists. + Error::MigrationError("No fork choice found in DB".to_string()), + )?, + db.get_config(), + )?; + + // Set local_irreversible_checkpoint = finalized_checkpoint + let persisted_fork_choice_v29: PersistedForkChoiceV29 = persisted_fork_choice_v28.into(); + + Ok(vec![KeyValueStoreOp::PutKeyValue( + DBColumn::ForkChoice, + FORK_CHOICE_DB_KEY.to_vec(), + persisted_fork_choice_v29.as_bytes(db.get_config())?, + )]) +} + +pub fn downgrade_from_v29( + db: Arc>, +) -> Result, Error> { + let persisted_fork_choice_v29 = PersistedForkChoiceV29::from_bytes( + &db.hot_db + .get_bytes(DBColumn::ForkChoice, FORK_CHOICE_DB_KEY.as_slice())? + .ok_or( + // Fork choice should exist if the database exists. + Error::MigrationError("No fork choice found in DB".to_string()), + )?, + db.get_config(), + )?; + + // Drop local_irreversible_checkpoint property + let persisted_fork_choice_v28: PersistedForkChoiceV28 = persisted_fork_choice_v29.into(); + + Ok(vec![KeyValueStoreOp::PutKeyValue( + DBColumn::ForkChoice, + FORK_CHOICE_DB_KEY.to_vec(), + persisted_fork_choice_v28.as_bytes(db.get_config())?, + )]) +} diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index f816dbac53d..d0838bec194 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -65,6 +65,7 @@ use store::database::interface::BeaconNodeBackend; use store::{HotColdDB, ItemStore, MemoryStore, config::StoreConfig}; use task_executor::TaskExecutor; use task_executor::{ShutdownReason, test_utils::TestRuntime}; +use tracing::info; use tree_hash::TreeHash; use typenum::U4294967296; use types::attestation::IndexedAttestationBase; @@ -875,6 +876,7 @@ where .canonical_head .cached_head() .finalized_checkpoint() + .local() } pub fn justified_checkpoint(&self) -> Checkpoint { @@ -882,6 +884,7 @@ where .canonical_head .cached_head() .justified_checkpoint() + .local() } pub fn get_current_slot(&self) -> Slot { @@ -3180,6 +3183,15 @@ where sync_committee_strategy: SyncCommitteeStrategy, light_client_strategy: LightClientStrategy, ) -> Hash256 { + info!( + num_blocks, + ?block_strategy, + ?attestation_strategy, + ?sync_committee_strategy, + ?light_client_strategy, + "Harness extend_chain_with_sync" + ); + let (mut state, slots) = match block_strategy { BlockStrategy::OnCanonicalHead => { let current_slot: u64 = self.get_current_slot().into(); diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 417d2811dda..018a6b4960b 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -1879,7 +1879,7 @@ async fn import_duplicate_block_unrealized_justification() { // must be at epoch 1. { let fc = chain.canonical_head.fork_choice_read_lock(); - assert_eq!(fc.justified_checkpoint().epoch, 0); + assert_eq!(fc.justified_checkpoint().on_chain().epoch, 0); assert_eq!(fc.unrealized_justified_checkpoint().epoch, 1); drop(fc); } @@ -1917,7 +1917,7 @@ async fn import_duplicate_block_unrealized_justification() { // The store's global unrealized justification should update immediately and match the block. let unrealized_justification = { let fc = chain.canonical_head.fork_choice_read_lock(); - assert_eq!(fc.justified_checkpoint().epoch, 0); + assert_eq!(fc.justified_checkpoint().on_chain().epoch, 0); let unrealized_justification = fc.unrealized_justified_checkpoint(); assert_eq!(unrealized_justification.epoch, 2); // The fork choice node for the block should have unrealized justification. @@ -1940,7 +1940,7 @@ async fn import_duplicate_block_unrealized_justification() { // Unrealized justification should still be updated. let fc3 = chain.canonical_head.fork_choice_read_lock(); - assert_eq!(fc3.justified_checkpoint().epoch, 0); + assert_eq!(fc3.justified_checkpoint().on_chain().epoch, 0); assert_eq!( fc3.unrealized_justified_checkpoint(), unrealized_justification diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index 1204412d651..0cc0667dc17 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -3,8 +3,8 @@ use beacon_chain::block_verification_types::RpcBlock; use beacon_chain::{ BeaconChainError, BlockError, ChainConfig, ExecutionPayloadError, - INVALID_JUSTIFIED_PAYLOAD_SHUTDOWN_REASON, NotifyExecutionLayer, OverrideForkchoiceUpdate, - StateSkipConfig, WhenSlotSkipped, + INVALID_JUSTIFIED_PAYLOAD_SHUTDOWN_REASON, NotifyExecutionLayer, StateSkipConfig, + WhenSlotSkipped, canonical_head::{CachedHead, CanonicalHead}, test_utils::{BeaconChainHarness, EphemeralHarnessType}, }; @@ -150,16 +150,6 @@ impl InvalidPayloadRig { .unwrap(); } - fn latest_execution_block_hash(&self) -> ExecutionBlockHash { - let mock_execution_layer = self.harness.mock_execution_layer.as_ref().unwrap(); - mock_execution_layer - .server - .execution_block_generator() - .latest_execution_block() - .unwrap() - .block_hash - } - async fn build_blocks(&mut self, num_blocks: u64, is_valid: Payload) -> Vec { let mut roots = Vec::with_capacity(num_blocks as usize); for _ in 0..num_blocks { @@ -1108,79 +1098,6 @@ async fn invalid_parent() { )); } -/// Tests to ensure that we will still send a proposer preparation -#[tokio::test] -async fn payload_preparation_before_transition_block() { - let rig = InvalidPayloadRig::new(); - let el = rig.execution_layer(); - - // Run the watchdog routine so that the status of the execution engine is set. This ensures - // that we don't end up with `eth_syncing` requests later in this function that will impede - // testing. - el.watchdog_task().await; - - let head = rig.harness.chain.head_snapshot(); - assert_eq!( - head.beacon_block - .message() - .body() - .execution_payload() - .unwrap() - .block_hash(), - ExecutionBlockHash::zero(), - "the head block is post-bellatrix but pre-transition" - ); - - let current_slot = rig.harness.chain.slot().unwrap(); - let next_slot = current_slot + 1; - let proposer = head - .beacon_state - .get_beacon_proposer_index(next_slot, &rig.harness.chain.spec) - .unwrap(); - let fee_recipient = Address::repeat_byte(99); - - // Provide preparation data to the EL for `proposer`. - el.update_proposer_preparation( - Epoch::new(0), - [( - &ProposerPreparationData { - validator_index: proposer as u64, - fee_recipient, - }, - &None, - )], - ) - .await; - - rig.move_to_terminal_block(); - - rig.harness - .chain - .prepare_beacon_proposer(current_slot) - .await - .unwrap(); - let forkchoice_update_params = rig - .harness - .chain - .canonical_head - .fork_choice_read_lock() - .get_forkchoice_update_parameters(); - rig.harness - .chain - .update_execution_engine_forkchoice( - current_slot, - forkchoice_update_params, - OverrideForkchoiceUpdate::Yes, - ) - .await - .unwrap(); - - let (fork_choice_state, payload_attributes) = rig.previous_forkchoice_update_params(); - let latest_block_hash = rig.latest_execution_block_hash(); - assert_eq!(payload_attributes.suggested_fee_recipient(), fee_recipient); - assert_eq!(fork_choice_state.head_block_hash, latest_block_hash); -} - #[tokio::test] async fn attesting_to_optimistic_head() { let mut rig = InvalidPayloadRig::new(); @@ -1312,7 +1229,7 @@ impl InvalidHeadSetup { // Import blocks until the first time the chain finalizes. This avoids // some edge-cases around genesis. - while rig.cached_head().finalized_checkpoint().epoch == 0 { + while rig.cached_head().finalized_checkpoint().on_chain().epoch == 0 { rig.import_block(Payload::Syncing).await; } diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index ea5f735bded..05d76df3670 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -16,7 +16,7 @@ use beacon_chain::test_utils::{ }; use beacon_chain::{ BeaconChain, BeaconChainError, BeaconChainTypes, BeaconSnapshot, BlockError, ChainConfig, - NotifyExecutionLayer, ServerSentEventHandler, WhenSlotSkipped, + ForkChoiceStoreInit, NotifyExecutionLayer, ServerSentEventHandler, WhenSlotSkipped, beacon_proposer_cache::{ compute_proposer_duties_from_head, ensure_state_can_determine_proposers_for_epoch, }, @@ -36,6 +36,7 @@ use state_processing::{BlockReplayer, state_advance::complete_state_advance}; use std::collections::HashMap; use std::collections::HashSet; use std::convert::TryInto; +use std::pin::Pin; use std::str::FromStr; use std::sync::{Arc, LazyLock}; use std::time::Duration; @@ -190,7 +191,8 @@ async fn light_client_bootstrap_test() { .chain .canonical_head .cached_head() - .finalized_checkpoint(); + .finalized_checkpoint() + .on_chain(); let block_root = finalized_checkpoint.root; @@ -2709,23 +2711,33 @@ async fn garbage_collect_temp_states_from_failed_block_on_finalization() { #[tokio::test] async fn weak_subjectivity_sync_easy() { - let num_initial_slots = E::slots_per_epoch() * 11; + let num_initial_slots = E::slots_per_epoch() * 13; let checkpoint_slot = Slot::new(E::slots_per_epoch() * 9); let slots = (1..num_initial_slots).map(Slot::new).collect(); - weak_subjectivity_sync_test(slots, checkpoint_slot, None, true).await + weak_subjectivity_sync_test(WeakSubjectivitySyncTestConfig::finality( + slots, + checkpoint_slot, + )) + .await; } #[tokio::test] async fn weak_subjectivity_sync_single_block_batches() { - let num_initial_slots = E::slots_per_epoch() * 11; + let num_initial_slots = E::slots_per_epoch() * 13; let checkpoint_slot = Slot::new(E::slots_per_epoch() * 9); let slots = (1..num_initial_slots).map(Slot::new).collect(); - weak_subjectivity_sync_test(slots, checkpoint_slot, Some(1), true).await + weak_subjectivity_sync_test(WeakSubjectivitySyncTestConfig { + slots_with_blocks: slots, + checkpoint_slot, + backfill_batch_size: Some(1), + ..Default::default() + }) + .await; } #[tokio::test] async fn weak_subjectivity_sync_unaligned_advanced_checkpoint() { - let num_initial_slots = E::slots_per_epoch() * 11; + let num_initial_slots = E::slots_per_epoch() * 13; let checkpoint_slot = Slot::new(E::slots_per_epoch() * 9); let slots = (1..num_initial_slots) .map(Slot::new) @@ -2734,12 +2746,16 @@ async fn weak_subjectivity_sync_unaligned_advanced_checkpoint() { slot <= checkpoint_slot - 3 || slot > checkpoint_slot }) .collect(); - weak_subjectivity_sync_test(slots, checkpoint_slot, None, true).await + weak_subjectivity_sync_test(WeakSubjectivitySyncTestConfig::finality( + slots, + checkpoint_slot, + )) + .await; } #[tokio::test] async fn weak_subjectivity_sync_unaligned_unadvanced_checkpoint() { - let num_initial_slots = E::slots_per_epoch() * 11; + let num_initial_slots = E::slots_per_epoch() * 13; let checkpoint_slot = Slot::new(E::slots_per_epoch() * 9 - 3); let slots = (1..num_initial_slots) .map(Slot::new) @@ -2748,7 +2764,11 @@ async fn weak_subjectivity_sync_unaligned_unadvanced_checkpoint() { slot <= checkpoint_slot || slot > checkpoint_slot + 3 }) .collect(); - weak_subjectivity_sync_test(slots, checkpoint_slot, None, true).await + weak_subjectivity_sync_test(WeakSubjectivitySyncTestConfig::finality( + slots, + checkpoint_slot, + )) + .await; } // Regression test for https://github.com/sigp/lighthouse/issues/4817 @@ -2757,10 +2777,14 @@ async fn weak_subjectivity_sync_unaligned_unadvanced_checkpoint() { #[tokio::test] async fn weak_subjectivity_sync_skips_at_genesis() { let start_slot = 4; - let end_slot = E::slots_per_epoch() * 4; + let end_slot = E::slots_per_epoch() * 6; let slots = (start_slot..end_slot).map(Slot::new).collect(); let checkpoint_slot = Slot::new(E::slots_per_epoch() * 2); - weak_subjectivity_sync_test(slots, checkpoint_slot, None, true).await + weak_subjectivity_sync_test(WeakSubjectivitySyncTestConfig::finality( + slots, + checkpoint_slot, + )) + .await; } // Checkpoint sync from the genesis state. @@ -2770,10 +2794,14 @@ async fn weak_subjectivity_sync_skips_at_genesis() { #[tokio::test] async fn weak_subjectivity_sync_from_genesis() { let start_slot = 1; - let end_slot = E::slots_per_epoch() * 2; + let end_slot = E::slots_per_epoch() * 4; let slots = (start_slot..end_slot).map(Slot::new).collect(); let checkpoint_slot = Slot::new(0); - weak_subjectivity_sync_test(slots, checkpoint_slot, None, true).await + weak_subjectivity_sync_test(WeakSubjectivitySyncTestConfig::finality( + slots, + checkpoint_slot, + )) + .await; } // Test checkpoint sync without providing blobs - backfill should fetch them. @@ -2783,7 +2811,67 @@ async fn weak_subjectivity_sync_without_blobs() { let end_slot = E::slots_per_epoch() * 4; let slots = (start_slot..end_slot).map(Slot::new).collect(); let checkpoint_slot = Slot::new(E::slots_per_epoch() * 2); - weak_subjectivity_sync_test(slots, checkpoint_slot, None, false).await + weak_subjectivity_sync_test(WeakSubjectivitySyncTestConfig { + slots_with_blocks: slots, + checkpoint_slot, + without_blobs: true, + ..Default::default() + }) + .await; +} + +// Checkpoint sync from the genesis state. +// +// This is a regression test for a bug we had involving the storage of the genesis state in the hot +// DB. +#[tokio::test] +async fn weak_subjectivity_sync_non_finality() { + let end_slot = E::slots_per_epoch() * 13; + let checkpoint_slot = E::slots_per_epoch() * 9; + let slots_with_blocks = (1..=checkpoint_slot).map(Slot::new).collect(); + + weak_subjectivity_sync_test(WeakSubjectivitySyncTestConfig { + slots_with_blocks, + slots_with_not_attested_blocks: end_slot - checkpoint_slot, + checkpoint_slot: checkpoint_slot.into(), + extra_steps: Some(Arc::new(|harness, beacon_chain| { + let end_slot = E::slots_per_epoch() * 13; + let checkpoint_slot = E::slots_per_epoch() * 9; + Box::pin(async move { + // Check that the checkpoint slot is not finalized + let finalized_slot = get_finalized_slot(&beacon_chain).as_u64(); + assert!( + checkpoint_slot > finalized_slot, + "checkpoint_slot {checkpoint_slot} finalized_slot {finalized_slot}" + ); + + let slots_to_finalize = E::slots_per_epoch() * 3; + info!( + count = slots_to_finalize, + "Producing blocks with attestations to finalize again" + ); + harness.advance_slot(); + harness + .extend_chain( + slots_to_finalize as usize, + BlockStrategy::OnCanonicalHead, + AttestationStrategy::AllValidators, + ) + .await; + sync_blocks_from_harness_to_chain(&harness, &beacon_chain, Slot::new(end_slot)) + .await; + + // Check that the checkpoint slot is finalized + let finalized_slot = get_finalized_slot(&beacon_chain).as_u64(); + assert!( + checkpoint_slot < finalized_slot, + "checkpoint_slot {checkpoint_slot} finalized_slot {finalized_slot}" + ); + }) + })), + ..Default::default() + }) + .await; } // Ensures that an unaligned checkpoint sync (the block is older than the state) @@ -2895,10 +2983,15 @@ async fn reproduction_unaligned_checkpoint_sync_pruned_payload() { .custom_spec(spec.clone().into()) .task_executor(harness.chain.task_executor.clone()) .weak_subjectivity_state( - wss_state, + wss_state.clone(), wss_block.clone(), wss_blobs_opt.clone(), genesis_state, + ForkChoiceStoreInit::FinalizedState(Box::new(BeaconSnapshot { + beacon_block_root: wss_block.canonical_root(), + beacon_block: Arc::new(wss_block.clone()), + beacon_state: wss_state, + })), ) .unwrap() .store_migrator_config(MigratorConfig::default().blocking()) @@ -2938,15 +3031,56 @@ async fn reproduction_unaligned_checkpoint_sync_pruned_payload() { ); } -async fn weak_subjectivity_sync_test( - slots: Vec, +type WeakSubjectivitySyncTestExtraSteps = Arc< + dyn Fn( + TestHarness, + Arc>>, + ) -> Pin + Send + 'static>> + + Send + + Sync, +>; + +#[derive(Default)] +struct WeakSubjectivitySyncTestConfig { + slots_with_blocks: Vec, + slots_with_not_attested_blocks: u64, checkpoint_slot: Slot, backfill_batch_size: Option, - provide_blobs: bool, -) { - // Build an initial chain on one harness, representing a synced node with full history. - let num_final_blocks = E::slots_per_epoch() * 2; + extra_steps: Option>, + without_blobs: bool, +} +impl WeakSubjectivitySyncTestConfig { + fn finality(slots: Vec, checkpoint_slot: Slot) -> Self { + Self { + slots_with_blocks: slots, + slots_with_not_attested_blocks: 0, + checkpoint_slot, + backfill_batch_size: None, + extra_steps: None, + without_blobs: false, + } + } +} + +fn get_finalized_slot(chain: &BeaconChain) -> Slot { + chain + .head() + .finalized_checkpoint() + .on_chain() + .epoch + .start_slot(T::EthSpec::slots_per_epoch()) +} + +async fn weak_subjectivity_sync_test(config: WeakSubjectivitySyncTestConfig) { + let WeakSubjectivitySyncTestConfig { + slots_with_blocks, + slots_with_not_attested_blocks, + checkpoint_slot, + backfill_batch_size, + extra_steps, + without_blobs, + } = config; let temp1 = tempdir().unwrap(); let full_store = get_store(&temp1); @@ -2958,12 +3092,16 @@ async fn weak_subjectivity_sync_test( let all_validators = (0..LOW_VALIDATOR_COUNT).collect::>(); + info!( + max_slot = ?slots_with_blocks.iter().max(), + "Producing blocks with attestations" + ); let (genesis_state, genesis_state_root) = harness.get_current_state_and_root(); harness .add_attested_blocks_at_slots( genesis_state.clone(), genesis_state_root, - &slots, + &slots_with_blocks, &all_validators, ) .await; @@ -2972,7 +3110,7 @@ async fn weak_subjectivity_sync_test( .chain .block_root_at_slot(checkpoint_slot, WhenSlotSkipped::Prev) .unwrap() - .unwrap(); + .unwrap_or_else(|| panic!("Block not found {checkpoint_slot}")); let wss_state_root = harness .chain .state_root_at_slot(checkpoint_slot) @@ -2996,15 +3134,21 @@ async fn weak_subjectivity_sync_test( let wss_state_slot = wss_state.slot(); let wss_block_slot = wss_block.slot(); - // Add more blocks that advance finalization further. - harness.advance_slot(); - harness - .extend_chain( - num_final_blocks as usize, - BlockStrategy::OnCanonicalHead, - AttestationStrategy::AllValidators, - ) - .await; + if slots_with_not_attested_blocks > 0 { + info!( + count = slots_with_not_attested_blocks, + "Producing blocks without attestations" + ); + harness.advance_slot(); + harness + .extend_chain( + slots_with_not_attested_blocks as usize, + BlockStrategy::OnCanonicalHead, + // No validators attest to this blocks + AttestationStrategy::SomeValidators(vec![]), + ) + .await; + } let (shutdown_tx, _shutdown_rx) = futures::channel::mpsc::channel(1); @@ -3041,14 +3185,19 @@ async fn weak_subjectivity_sync_test( .custom_spec(test_spec::().into()) .task_executor(harness.chain.task_executor.clone()) .weak_subjectivity_state( - wss_state, + wss_state.clone(), wss_block.clone(), - if provide_blobs { - wss_blobs_opt.clone() - } else { + if without_blobs { None + } else { + wss_blobs_opt.clone() }, genesis_state, + ForkChoiceStoreInit::FinalizedState(Box::new(BeaconSnapshot { + beacon_block_root: wss_block.canonical_root(), + beacon_block: Arc::new(wss_block.clone()), + beacon_state: wss_state, + })), ) .unwrap() .store_migrator_config(MigratorConfig::default().blocking()) @@ -3080,47 +3229,7 @@ async fn weak_subjectivity_sync_test( assert_eq!(store_wss_blobs_opt, wss_blobs_opt); } - // Apply blocks forward to reach head. - let chain_dump = harness.chain.chain_dump().unwrap(); - let new_blocks = chain_dump - .iter() - .filter(|snapshot| snapshot.beacon_block.slot() > checkpoint_slot); - - for snapshot in new_blocks { - let block_root = snapshot.beacon_block_root; - let full_block = harness - .chain - .get_block(&snapshot.beacon_block_root) - .await - .unwrap() - .unwrap(); - - let slot = full_block.slot(); - let full_block_root = full_block.canonical_root(); - let state_root = full_block.state_root(); - - info!(block_root = ?full_block_root, ?state_root, %slot, "Importing block from chain dump"); - beacon_chain.slot_clock.set_slot(slot.as_u64()); - beacon_chain - .process_block( - full_block_root, - harness.build_rpc_block_from_store_blobs(Some(block_root), Arc::new(full_block)), - NotifyExecutionLayer::Yes, - BlockImportSource::Lookup, - || Ok(()), - ) - .await - .unwrap(); - beacon_chain.recompute_head_at_current_slot().await; - - // Check that the new block's state can be loaded correctly. - let mut state = beacon_chain - .store - .get_state(&state_root, Some(slot), CACHE_STATE_IN_TESTS) - .unwrap() - .unwrap(); - assert_eq!(state.update_tree_hash_cache().unwrap(), state_root); - } + sync_blocks_from_harness_to_chain(&harness, &beacon_chain, checkpoint_slot).await; if checkpoint_slot != 0 { // Forwards iterator from 0 should fail as we lack blocks (unless checkpoint slot is 0). @@ -3158,6 +3267,7 @@ async fn weak_subjectivity_sync_test( assert_eq!(beacon_chain.state_root_at_slot(Slot::new(1)).unwrap(), None); // Supply blocks backwards to reach genesis. Omit the genesis block to check genesis handling. + let chain_dump = harness.chain.chain_dump().unwrap(); let historical_blocks = chain_dump[..wss_block.slot().as_usize()] .iter() .filter(|s| s.beacon_block.slot() != 0) @@ -3359,6 +3469,63 @@ async fn weak_subjectivity_sync_test( store.clone().reconstruct_historic_states(None).unwrap(); assert_eq!(store.get_anchor_info().anchor_slot, wss_aligned_slot); assert_eq!(store.get_anchor_info().state_upper_limit, Slot::new(0)); + + if let Some(f) = extra_steps { + f(harness, beacon_chain).await; + } +} + +async fn sync_blocks_from_harness_to_chain( + harness: &TestHarness, + beacon_chain: &Arc>>, + from_slot: Slot, +) { + // Apply blocks forward to reach head. + let chain_dump = harness.chain.chain_dump().unwrap(); + let new_blocks = chain_dump + .iter() + .filter(|snapshot| snapshot.beacon_block.slot() > from_slot) + .collect::>(); + info!( + block_count = new_blocks.len(), + "Importing blocks to new node" + ); + + for snapshot in new_blocks { + let block_root = snapshot.beacon_block_root; + let full_block = harness + .chain + .get_block(&snapshot.beacon_block_root) + .await + .unwrap() + .unwrap(); + + let slot = full_block.slot(); + let full_block_root = full_block.canonical_root(); + let state_root = full_block.state_root(); + + info!(block_root = ?full_block_root, ?state_root, %slot, "Importing block from chain dump"); + beacon_chain.slot_clock.set_slot(slot.as_u64()); + beacon_chain + .process_block( + full_block_root, + harness.build_rpc_block_from_store_blobs(Some(block_root), Arc::new(full_block)), + NotifyExecutionLayer::Yes, + BlockImportSource::Lookup, + || Ok(()), + ) + .await + .unwrap(); + beacon_chain.recompute_head_at_current_slot().await; + + // Check that the new block's state can be loaded correctly. + let mut state = beacon_chain + .store + .get_state(&state_root, Some(slot), CACHE_STATE_IN_TESTS) + .unwrap() + .unwrap(); + assert_eq!(state.update_tree_hash_cache().unwrap(), state_root); + } } // This test prunes data columns from epoch 0 and then tries to re-import them via diff --git a/beacon_node/beacon_chain/tests/tests.rs b/beacon_node/beacon_chain/tests/tests.rs index 1884429a6a8..d629d8866b4 100644 --- a/beacon_node/beacon_chain/tests/tests.rs +++ b/beacon_node/beacon_chain/tests/tests.rs @@ -980,10 +980,7 @@ async fn pseudo_finalize_test_generic( }; // pseudo finalize - harness - .chain - .manually_finalize_state(head.beacon_state_root(), checkpoint) - .unwrap(); + harness.chain.manual_finalization(checkpoint).await.unwrap(); let split = harness.chain.store.get_split_info(); let pseudo_finalized_slot = split.slot; @@ -998,11 +995,13 @@ async fn pseudo_finalize_test_generic( 0, "We pseudo finalized, but our finalized checkpoint should still be unset" ); - assert_eq!( - split.slot, - head.beacon_state.slot(), - "We pseudo finalized, our split point should be at the current head slot" - ); + if expect_true_finalization_migration { + assert_eq!( + split.slot, + head.beacon_state.slot(), + "We pseudo finalized, our split point should be at the current head slot" + ); + } // finalize the chain harness diff --git a/beacon_node/client/Cargo.toml b/beacon_node/client/Cargo.toml index 3c4b2572c9a..a0652ac2600 100644 --- a/beacon_node/client/Cargo.toml +++ b/beacon_node/client/Cargo.toml @@ -31,6 +31,7 @@ serde_json = { workspace = true } slasher = { workspace = true } slasher_service = { path = "../../slasher/service" } slot_clock = { workspace = true } +state_processing = { workspace = true } store = { workspace = true } task_executor = { workspace = true } time = "0.3.5" @@ -43,5 +44,4 @@ types = { workspace = true } [dev-dependencies] operation_pool = { workspace = true } serde_yaml = { workspace = true } -state_processing = { workspace = true } tokio = { workspace = true } diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index 1b395ac8da5..7e8553b56e8 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -16,7 +16,7 @@ use beacon_chain::{ state_advance_timer::spawn_state_advance_timer, store::{HotColdDB, ItemStore, StoreConfig}, }; -use beacon_chain::{Kzg, LightClientProducerEvent}; +use beacon_chain::{ForkChoiceStoreInit, Kzg, LightClientProducerEvent}; use beacon_processor::{BeaconProcessor, BeaconProcessorChannels}; use beacon_processor::{BeaconProcessorConfig, BeaconProcessorQueueLengths}; use environment::RuntimeContext; @@ -36,6 +36,7 @@ use rand::SeedableRng; use rand::rngs::{OsRng, StdRng}; use slasher::Slasher; use slasher_service::SlasherService; +use state_processing::per_slot_processing; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Duration; @@ -369,13 +370,18 @@ where let genesis_state = genesis_state(&runtime_context, &config).await?; builder.weak_subjectivity_state( - anchor_state, - anchor_block, + anchor_state.clone(), + anchor_block.clone(), anchor_blobs, genesis_state, + ForkChoiceStoreInit::FinalizedState(Box::new(beacon_chain::BeaconSnapshot { + beacon_block_root: anchor_block.canonical_root(), + beacon_block: Arc::new(anchor_block), + beacon_state: anchor_state, + })), )? } - ClientGenesis::CheckpointSyncUrl { url } => { + ClientGenesis::CheckpointSyncUrl { url, state_id } => { info!( remote_url = %url, "Starting checkpoint sync" @@ -391,20 +397,20 @@ where )), ); - debug!("Downloading finalized state"); + debug!("Downloading checkpoint state {state_id}"); let state = remote - .get_debug_beacon_states_ssz::(StateId::Finalized, &spec) + .get_debug_beacon_states_ssz::(state_id, &spec) .await .map_err(|e| format!("Error loading checkpoint state from remote: {:?}", e))? .ok_or_else(|| "Checkpoint state missing from remote".to_string())?; - debug!(slot = ?state.slot(), "Downloaded finalized state"); + debug!(slot = ?state.slot(), "Downloaded checkpoint state {state_id}"); - let finalized_block_slot = state.latest_block_header().slot; + let block_slot = state.latest_block_header().slot; - debug!(block_slot = ?finalized_block_slot,"Downloading finalized block"); + debug!(?block_slot, "Downloading checkpoint block {state_id}"); let block = remote - .get_beacon_blocks_ssz::(BlockId::Slot(finalized_block_slot), &spec) + .get_beacon_blocks_ssz::(BlockId::Slot(block_slot), &spec) .await .map_err(|e| match e { ApiError::InvalidSsz(e) => format!( @@ -412,25 +418,41 @@ where node for the correct network", e ), - e => format!("Error fetching finalized block from remote: {:?}", e), + e => format!("Error fetching checkpoint block remote: {:?}", e), })? - .ok_or("Finalized block missing from remote, it returned 404")?; + .ok_or("Checkpoint block from remote, it returned 404")?; let block_root = block.canonical_root(); - debug!("Downloaded finalized block"); + debug!("Downloaded checkpoint block {block_slot}"); + + // The checkpointz server is trusted. If the user requests `Finalized` or + // `Genesis`, the returned checkpoint state is finalized and we can safely use + // finalized-anchor fork-choice initialization. For any other tag, treat the + // checkpoint as potentially non-finalized and load explicit justified/finalized + // fork-choice inputs. + let fork_choice_store_init = match state_id { + StateId::Finalized | StateId::Genesis => ForkChoiceStoreInit::FinalizedState( + Box::new(beacon_chain::BeaconSnapshot { + beacon_block_root: block_root, + beacon_block: Arc::new(block.clone()), + beacon_state: state.clone(), + }), + ), + _ => load_non_finalized_fork_choice_init(&remote, &state, &spec).await?, + }; // `get_blob_sidecars` API is deprecated from Fulu and may not be supported by all servers - let is_before_fulu = !spec - .fork_name_at_slot::(finalized_block_slot) - .fulu_enabled(); + let is_before_fulu = !spec.fork_name_at_slot::(block_slot).fulu_enabled(); let blobs = if is_before_fulu && block.message().body().has_blobs() { - debug!("Downloading finalized blobs"); + debug!("Downloading checkpoint blobs {block_slot} {block_root}"); if let Some(response) = remote .get_blob_sidecars::(BlockId::Root(block_root), None, &spec) .await - .map_err(|e| format!("Error fetching finalized blobs from remote: {e:?}"))? + .map_err(|e| { + format!("Error fetching checkpoint blobs from remote: {e:?}") + })? { - debug!("Downloaded finalized blobs"); + debug!("Downloaded checkpoint blobs"); Some(response.into_data()) } else { warn!( @@ -454,7 +476,13 @@ where "Loaded checkpoint block and state" ); - builder.weak_subjectivity_state(state, block, blobs, genesis_state)? + builder.weak_subjectivity_state( + state, + block, + blobs, + genesis_state, + fork_choice_store_init, + )? } ClientGenesis::DepositContract => { return Err("Loading genesis from deposit contract no longer supported".to_string()); @@ -931,3 +959,78 @@ async fn genesis_state( .await? .ok_or_else(|| "Genesis state is unknown".to_string()) } + +async fn load_non_finalized_fork_choice_init( + remote: &BeaconNodeHttpClient, + checkpoint_state: &BeaconState, + spec: &ChainSpec, +) -> Result, String> { + let justified_checkpoint = checkpoint_state.current_justified_checkpoint(); + let finalized_checkpoint = checkpoint_state.finalized_checkpoint(); + + if justified_checkpoint.root == Hash256::ZERO { + return Err( + "Non-finalized checkpoint init requested, but checkpoint state has no justified checkpoint" + .to_string(), + ); + } + + let justified_epoch_start_slot = justified_checkpoint.epoch.start_slot(E::slots_per_epoch()); + debug!( + root = ?justified_checkpoint.root, + target_slot = ?justified_epoch_start_slot, + "Downloading justified checkpoint block" + ); + let justified_block = remote + .get_beacon_blocks_ssz::(BlockId::Root(justified_checkpoint.root), spec) + .await + .map_err(|e| format!("Error loading justified checkpoint block from remote: {e:?}"))? + .ok_or_else(|| "Justified checkpoint block missing from remote".to_string())?; + let justified_state_root = justified_block.message().state_root(); + + let finalized_block = if finalized_checkpoint.root == Hash256::ZERO { + justified_block.clone() + } else { + debug!(root = ?finalized_checkpoint.root, "Downloading finalized checkpoint block"); + remote + .get_beacon_blocks_ssz::(BlockId::Root(finalized_checkpoint.root), spec) + .await + .map_err(|e| format!("Error loading finalized checkpoint block from remote: {e:?}"))? + .ok_or_else(|| "Finalized checkpoint block missing from remote".to_string())? + }; + + debug!( + root = ?justified_state_root, + "Downloading justified checkpoint state" + ); + let mut justified_state = remote + .get_debug_beacon_states_ssz::(StateId::Root(justified_state_root), spec) + .await + .map_err(|e| format!("Error loading justified checkpoint state from remote: {e:?}"))? + .ok_or_else(|| "Justified checkpoint state missing from remote".to_string())?; + + if justified_state.slot() > justified_epoch_start_slot { + return Err(format!( + "Justified checkpoint state slot {} is after expected epoch boundary slot {}", + justified_state.slot(), + justified_epoch_start_slot + )); + } + + while justified_state.slot() < justified_epoch_start_slot { + per_slot_processing(&mut justified_state, None, spec) + .map_err(|e| format!("Error advancing justified checkpoint state: {e:?}"))?; + } + + debug!( + slot = ?justified_state.slot(), + epoch = ?justified_checkpoint.epoch, + "Downloaded and aligned justified checkpoint state" + ); + Ok(ForkChoiceStoreInit::NonFinalizedState { + anchor_state: Box::new(checkpoint_state.clone()), + justified_state: Box::new(justified_state), + justified_block: Box::new(justified_block), + finalized_block: Box::new(finalized_block), + }) +} diff --git a/beacon_node/client/src/config.rs b/beacon_node/client/src/config.rs index aeaa196df86..2edcd0e1238 100644 --- a/beacon_node/client/src/config.rs +++ b/beacon_node/client/src/config.rs @@ -3,6 +3,7 @@ use beacon_chain::validator_monitor::ValidatorMonitorConfig; use beacon_processor::BeaconProcessorConfig; use directory::DEFAULT_ROOT_DIR; use environment::LoggerConfig; +use eth2::types::StateId; use kzg::trusted_setup::get_trusted_setup; use network::NetworkConfig; use sensitive_url::SensitiveUrl; @@ -17,7 +18,7 @@ const DEFAULT_FREEZER_DB_DIR: &str = "freezer_db"; const DEFAULT_BLOBS_DB_DIR: &str = "blobs_db"; /// Defines how the client should initialize the `BeaconChain` and other components. -#[derive(Debug, Clone, Serialize, Deserialize, Default)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Default)] pub enum ClientGenesis { /// Creates a genesis state as per the 2019 Canada interop specifications. Interop { @@ -44,6 +45,7 @@ pub enum ClientGenesis { }, CheckpointSyncUrl { url: SensitiveUrl, + state_id: StateId, }, } @@ -61,9 +63,6 @@ pub struct Config { /// Graffiti to be inserted everytime we create a block if the validator doesn't specify. pub beacon_graffiti: GraffitiOrigin, pub validator_monitor: ValidatorMonitorConfig, - #[serde(skip)] - /// The `genesis` field is not serialized or deserialized by `serde` to ensure it is defined - /// via the CLI at runtime, instead of from a configuration file saved to disk. pub genesis: ClientGenesis, pub store: store::StoreConfig, pub network: network::NetworkConfig, diff --git a/beacon_node/client/src/notifier.rs b/beacon_node/client/src/notifier.rs index 3f01622c352..92bbbc05ed3 100644 --- a/beacon_node/client/src/notifier.rs +++ b/beacon_node/client/src/notifier.rs @@ -225,8 +225,7 @@ pub fn spawn_notifier( debug!( peers = peer_count_pretty(connected_peer_count), - finalized_root = %finalized_checkpoint.root, - finalized_epoch = %finalized_checkpoint.epoch, + %finalized_checkpoint, head_block = %head_root, %head_slot, %current_slot, @@ -395,8 +394,7 @@ pub fn spawn_notifier( info!( peers = peer_count_pretty(connected_peer_count), exec_hash = block_hash, - finalized_root = %finalized_checkpoint.root, - finalized_epoch = %finalized_checkpoint.epoch, + %finalized_checkpoint, epoch = %current_epoch, block = block_info, slot = %current_slot, @@ -406,8 +404,7 @@ pub fn spawn_notifier( metrics::set_gauge(&metrics::IS_SYNCED, 0); info!( peers = peer_count_pretty(connected_peer_count), - finalized_root = %finalized_checkpoint.root, - finalized_epoch = %finalized_checkpoint.epoch, + %finalized_checkpoint, %head_slot, %current_slot, "Searching for peers" diff --git a/beacon_node/http_api/src/block_id.rs b/beacon_node/http_api/src/block_id.rs index e6b1ed08794..97295d934ee 100644 --- a/beacon_node/http_api/src/block_id.rs +++ b/beacon_node/http_api/src/block_id.rs @@ -58,15 +58,23 @@ impl BlockId { } CoreBlockId::Genesis => Ok((chain.genesis_block_root, false, true)), CoreBlockId::Finalized => { - let finalized_checkpoint = - chain.canonical_head.cached_head().finalized_checkpoint(); + // Return the network's finalized checkpoint not the node's local irreversible view. + let finalized_checkpoint = chain + .canonical_head + .cached_head() + .finalized_checkpoint() + .on_chain(); let (_slot, execution_optimistic) = checkpoint_slot_and_execution_optimistic(chain, finalized_checkpoint)?; Ok((finalized_checkpoint.root, execution_optimistic, true)) } CoreBlockId::Justified => { - let justified_checkpoint = - chain.canonical_head.cached_head().justified_checkpoint(); + // Return the network's justified checkpoint not the node's local irreversible view. + let justified_checkpoint = chain + .canonical_head + .cached_head() + .justified_checkpoint() + .on_chain(); let (_slot, execution_optimistic) = checkpoint_slot_and_execution_optimistic(chain, justified_checkpoint)?; Ok((justified_checkpoint.root, execution_optimistic, false)) @@ -91,6 +99,7 @@ impl BlockId { .canonical_head .cached_head() .finalized_checkpoint() + .on_chain() .epoch .start_slot(T::EthSpec::slots_per_epoch()); Ok((root, execution_optimistic, finalized)) diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 095c52fb292..50ab90a2fab 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -43,7 +43,9 @@ use crate::validator::post_validator_liveness_epoch; use crate::validator::*; use crate::version::beacon_response; use beacon::states; -use beacon_chain::{BeaconChain, BeaconChainError, BeaconChainTypes, WhenSlotSkipped}; +use beacon_chain::{ + BeaconChain, BeaconChainError, BeaconChainTypes, ForkChoiceError, WhenSlotSkipped, +}; use beacon_processor::BeaconProcessorSend; pub use block_id::BlockId; use builder_states::get_next_withdrawals; @@ -2109,8 +2111,8 @@ pub fn serve( }) .collect::>(); Ok(ForkChoice { - justified_checkpoint: beacon_fork_choice.justified_checkpoint(), - finalized_checkpoint: beacon_fork_choice.finalized_checkpoint(), + justified_checkpoint: beacon_fork_choice.justified_checkpoint().on_chain(), + finalized_checkpoint: beacon_fork_choice.finalized_checkpoint().on_chain(), fork_choice_nodes, }) }) @@ -2576,20 +2578,27 @@ pub fn serve( |request_data: api_types::ManualFinalizationRequestData, task_spawner: TaskSpawner, chain: Arc>| { - task_spawner.blocking_json_task(Priority::P0, move || { + task_spawner.spawn_async_with_rejection(Priority::P0, async move { let checkpoint = Checkpoint { epoch: request_data.epoch, root: request_data.block_root, }; chain - .manually_finalize_state(request_data.state_root, checkpoint) - .map(|_| api_types::GenericResponse::from(request_data)) - .map_err(|e| { - warp_utils::reject::custom_bad_request(format!( + .manual_finalization(checkpoint) + .await + .map_err(|e| match e { + BeaconChainError::ForkChoiceError( + ForkChoiceError::BadIrreversibleCheckpoint(msg), + ) => warp_utils::reject::custom_bad_request(msg), + e => warp_utils::reject::custom_server_error(format!( "Failed to finalize state due to error: {e:?}" - )) - }) + )), + })?; + Ok::<_, warp::reject::Rejection>( + warp::reply::json(&api_types::GenericResponse::from(request_data)) + .into_response(), + ) }) }, ); diff --git a/beacon_node/http_api/src/state_id.rs b/beacon_node/http_api/src/state_id.rs index 13fb9b2c585..1579b6d2cb6 100644 --- a/beacon_node/http_api/src/state_id.rs +++ b/beacon_node/http_api/src/state_id.rs @@ -39,15 +39,15 @@ impl StateId { } CoreStateId::Genesis => return Ok((chain.genesis_state_root, false, true)), CoreStateId::Finalized => { - let finalized_checkpoint = - chain.canonical_head.cached_head().finalized_checkpoint(); + // Return the network's finalized checkpoint not the node's local irreversible view. + let finalized_checkpoint = chain.head().finalized_checkpoint().on_chain(); let (slot, execution_optimistic) = checkpoint_slot_and_execution_optimistic(chain, finalized_checkpoint)?; (slot, execution_optimistic, true) } CoreStateId::Justified => { - let justified_checkpoint = - chain.canonical_head.cached_head().justified_checkpoint(); + // Return the network's justified checkpoint not the node's local irreversible view. + let justified_checkpoint = chain.head().justified_checkpoint().on_chain(); let (slot, execution_optimistic) = checkpoint_slot_and_execution_optimistic(chain, justified_checkpoint)?; (slot, execution_optimistic, false) @@ -59,9 +59,9 @@ impl StateId { .map_err(warp_utils::reject::unhandled_error)?, *slot <= chain - .canonical_head - .cached_head() + .head() .finalized_checkpoint() + .on_chain() .epoch .start_slot(T::EthSpec::slots_per_epoch()), ), @@ -104,10 +104,9 @@ impl StateId { .map_err(warp_utils::reject::unhandled_error)? { let fork_choice = chain.canonical_head.fork_choice_read_lock(); - let finalized_root = fork_choice - .cached_fork_choice_view() - .finalized_checkpoint - .root; + // Retrieve the status of the oldest irreversible block in fork-choice. The + // network finalized checkpoint may not be available. + let finalized_root = fork_choice.finalized_checkpoint().local().root; let execution_optimistic = fork_choice .is_optimistic_or_invalid_block_no_fallback(&finalized_root) .map_err(BeaconChainError::ForkChoiceError) @@ -262,12 +261,15 @@ pub fn checkpoint_slot_and_execution_optimistic( ) -> Result<(Slot, ExecutionOptimistic), warp::reject::Rejection> { let slot = checkpoint.epoch.start_slot(T::EthSpec::slots_per_epoch()); let fork_choice = chain.canonical_head.fork_choice_read_lock(); - let finalized_checkpoint = fork_choice.cached_fork_choice_view().finalized_checkpoint; + // Use the local irreversible checkpoint of fork-choice. We need to default to a known block in + // fork-choice and oldest block in fork-choice might be more recent that the network's finalized + // checkpoint. + let local_finalized_checkpoint = fork_choice.finalized_checkpoint().local(); // If the checkpoint is pre-finalization, just use the optimistic status of the finalized // block. - let root = if checkpoint.epoch < finalized_checkpoint.epoch { - &finalized_checkpoint.root + let root = if checkpoint.epoch < local_finalized_checkpoint.epoch { + &local_finalized_checkpoint.root } else { &checkpoint.root }; diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index bef9fe6acda..73e294759d6 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -269,6 +269,7 @@ impl ApiTester { .canonical_head .cached_head() .finalized_checkpoint() + .on_chain() .epoch, 2, "precondition: finality" @@ -278,6 +279,7 @@ impl ApiTester { .canonical_head .cached_head() .justified_checkpoint() + .on_chain() .epoch, 3, "precondition: justification" @@ -3069,11 +3071,11 @@ impl ApiTester { assert_eq!( result.justified_checkpoint, - beacon_fork_choice.justified_checkpoint() + beacon_fork_choice.justified_checkpoint().on_chain() ); assert_eq!( result.finalized_checkpoint, - beacon_fork_choice.finalized_checkpoint() + beacon_fork_choice.finalized_checkpoint().on_chain() ); let expected_fork_choice_nodes: Vec = expected_proto_array diff --git a/beacon_node/network/src/network_beacon_processor/rpc_methods.rs b/beacon_node/network/src/network_beacon_processor/rpc_methods.rs index 5edd661bb6a..e71ade89329 100644 --- a/beacon_node/network/src/network_beacon_processor/rpc_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/rpc_methods.rs @@ -101,11 +101,7 @@ impl NetworkBeaconProcessor { } else { // Remote finalized epoch is less than ours. let remote_finalized_slot = start_slot(*remote.finalized_epoch()); - if remote_finalized_slot < self.chain.store.get_oldest_block_slot() { - // Peer's finalized checkpoint is older than anything in our DB. We are unlikely - // to be able to help them sync. - Some("Old finality out of range".to_string()) - } else if remote_finalized_slot < self.chain.store.get_split_slot() { + if remote_finalized_slot < self.chain.store.get_split_slot() { // Peer's finalized slot is in range for a quick block root check in our freezer DB. // If that block root check fails, reject them as they're on a different finalized // chain. @@ -864,11 +860,13 @@ impl NetworkBeaconProcessor { req_type: &str, ) -> Result, (RpcErrorResponse, &'static str)> { let start_time = std::time::Instant::now(); + // We care about blocks being in fork-choice, use its view of finality let finalized_slot = self .chain .canonical_head .cached_head() .finalized_checkpoint() + .local() .epoch .start_slot(T::EthSpec::slots_per_epoch()); diff --git a/beacon_node/network/src/status.rs b/beacon_node/network/src/status.rs index c571a40485c..05ad71ac8ce 100644 --- a/beacon_node/network/src/status.rs +++ b/beacon_node/network/src/status.rs @@ -21,7 +21,7 @@ impl ToStatusMessage for BeaconChain { pub(crate) fn status_message(beacon_chain: &BeaconChain) -> StatusMessage { let fork_digest = beacon_chain.enr_fork_id().fork_digest; let cached_head = beacon_chain.canonical_head.cached_head(); - let mut finalized_checkpoint = cached_head.finalized_checkpoint(); + let mut finalized_checkpoint = cached_head.finalized_checkpoint().on_chain(); // Alias the genesis checkpoint root to `0x00`. let spec = &beacon_chain.spec; diff --git a/beacon_node/network/src/sync/custody_backfill_sync/mod.rs b/beacon_node/network/src/sync/custody_backfill_sync/mod.rs index fa8b70c8b46..62361e96e65 100644 --- a/beacon_node/network/src/sync/custody_backfill_sync/mod.rs +++ b/beacon_node/network/src/sync/custody_backfill_sync/mod.rs @@ -182,6 +182,7 @@ impl CustodyBackFillSync { .canonical_head .cached_head() .finalized_checkpoint() + .local() .epoch; // Check that the earliest data column epoch is a finalized epoch. diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index 096ed9c3282..f52c2266c6b 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -42,12 +42,12 @@ use super::peer_sync_info::{PeerSyncType, remote_sync_type}; use super::range_sync::{EPOCHS_PER_BATCH, RangeSync, RangeSyncType}; use crate::network_beacon_processor::{ChainSegmentProcessId, NetworkBeaconProcessor}; use crate::service::NetworkMessage; -use crate::status::ToStatusMessage; use crate::sync::block_lookups::{ BlobRequestState, BlockComponent, BlockRequestState, CustodyRequestState, DownloadResult, }; use crate::sync::custody_backfill_sync::CustodyBackFillSync; use crate::sync::network_context::{PeerGroup, RpcResponseResult}; +use crate::sync::peer_sync_info::{LocalSyncInfo, PeerSyncTypeAdvanced}; use beacon_chain::block_verification_types::AsBlock; use beacon_chain::validator_monitor::timestamp_now; use beacon_chain::{ @@ -384,16 +384,7 @@ impl SyncManager { /// If the peer is within the `SLOT_IMPORT_TOLERANCE`, then it's head is sufficiently close to /// ours that we consider it fully sync'd with respect to our current chain. fn add_peer(&mut self, peer_id: PeerId, remote: SyncInfo) { - // ensure the beacon chain still exists - let status = self.chain.status_message(); - let local = SyncInfo { - head_slot: *status.head_slot(), - head_root: *status.head_root(), - finalized_epoch: *status.finalized_epoch(), - finalized_root: *status.finalized_root(), - earliest_available_slot: status.earliest_available_slot().ok().cloned(), - }; - + let local = LocalSyncInfo::new(&self.chain); let sync_type = remote_sync_type(&local, &remote, &self.chain); // update the state of the peer. @@ -401,9 +392,9 @@ impl SyncManager { if is_still_connected { match sync_type { PeerSyncType::Behind => {} // Do nothing - PeerSyncType::Advanced => { + PeerSyncType::Advanced(advanced_type) => { self.range_sync - .add_peer(&mut self.network, local, peer_id, remote); + .add_peer(&mut self.network, local, peer_id, advanced_type); } PeerSyncType::FullySynced => { // Sync considers this peer close enough to the head to not trigger range sync. @@ -438,15 +429,7 @@ impl SyncManager { head_root: Hash256, head_slot: Option, ) { - let status = self.chain.status_message(); - let local = SyncInfo { - head_slot: *status.head_slot(), - head_root: *status.head_root(), - finalized_epoch: *status.finalized_epoch(), - finalized_root: *status.finalized_root(), - earliest_available_slot: status.earliest_available_slot().ok().cloned(), - }; - + let local = LocalSyncInfo::new(&self.chain); let head_slot = head_slot.unwrap_or_else(|| { debug!( local_head_slot = %local.head_slot, @@ -456,18 +439,17 @@ impl SyncManager { local.head_slot }); - let remote = SyncInfo { - head_slot, - head_root, - // Set finalized to same as local to trigger Head sync - finalized_epoch: local.finalized_epoch, - finalized_root: local.finalized_root, - earliest_available_slot: local.earliest_available_slot, - }; - for peer_id in peers { - self.range_sync - .add_peer(&mut self.network, local.clone(), *peer_id, remote.clone()); + self.range_sync.add_peer( + &mut self.network, + local.clone(), + *peer_id, + PeerSyncTypeAdvanced::Head { + target_root: head_root, + target_slot: head_slot, + start_epoch: local.local_irreversible_epoch, + }, + ); } } @@ -542,7 +524,7 @@ impl SyncManager { fn update_peer_sync_state( &mut self, peer_id: &PeerId, - local_sync_info: &SyncInfo, + local_sync_info: &LocalSyncInfo, remote_sync_info: &SyncInfo, sync_type: &PeerSyncType, ) -> bool { diff --git a/beacon_node/network/src/sync/peer_sync_info.rs b/beacon_node/network/src/sync/peer_sync_info.rs index 5ea1533d350..0c764708bc5 100644 --- a/beacon_node/network/src/sync/peer_sync_info.rs +++ b/beacon_node/network/src/sync/peer_sync_info.rs @@ -1,14 +1,16 @@ use super::manager::SLOT_IMPORT_TOLERANCE; +use crate::status::ToStatusMessage; use beacon_chain::{BeaconChain, BeaconChainTypes}; use lighthouse_network::{SyncInfo, SyncStatus as PeerSyncStatus}; use std::cmp::Ordering; +use types::{Epoch, EthSpec, Hash256, Slot}; /// The type of peer relative to our current state. pub enum PeerSyncType { /// The peer is on our chain and is fully synced with respect to our chain. FullySynced, /// The peer has a greater knowledge of the chain than us that warrants a full sync. - Advanced, + Advanced(PeerSyncTypeAdvanced), /// A peer is behind in the sync and not useful to us for downloading blocks. Behind, } @@ -18,13 +20,45 @@ impl PeerSyncType { match self { PeerSyncType::FullySynced => PeerSyncStatus::Synced { info: info.clone() }, PeerSyncType::Behind => PeerSyncStatus::Behind { info: info.clone() }, - PeerSyncType::Advanced => PeerSyncStatus::Advanced { info: info.clone() }, + PeerSyncType::Advanced(_) => PeerSyncStatus::Advanced { info: info.clone() }, + } + } +} + +pub enum PeerSyncTypeAdvanced { + Finalized { + target_slot: Slot, + target_root: Hash256, + start_epoch: Epoch, + }, + Head { + target_slot: Slot, + target_root: Hash256, + start_epoch: Epoch, + }, +} + +#[derive(Clone)] +pub(crate) struct LocalSyncInfo { + pub head_slot: Slot, + pub finalized_epoch: Epoch, + pub local_irreversible_epoch: Epoch, +} + +impl LocalSyncInfo { + pub fn new(chain: &BeaconChain) -> Self { + let status = chain.status_message(); + let local_finalized_checkpoint = chain.head().finalized_checkpoint().local(); + Self { + head_slot: *status.head_slot(), + finalized_epoch: *status.finalized_epoch(), + local_irreversible_epoch: local_finalized_checkpoint.epoch, } } } pub fn remote_sync_type( - local: &SyncInfo, + local: &LocalSyncInfo, remote: &SyncInfo, chain: &BeaconChain, ) -> PeerSyncType { @@ -33,6 +67,10 @@ pub fn remote_sync_type( let near_range_start = local.head_slot.saturating_sub(SLOT_IMPORT_TOLERANCE); let near_range_end = local.head_slot.saturating_add(SLOT_IMPORT_TOLERANCE); + // With the remote peer's status message let's figure out if there are enough blocks to discover + // that we trigger sync from them. We don't want to sync any blocks from epochs prior to the + // local irreversible epoch. Our finalized epoch may be less than the local irreversible epoch. + match remote.finalized_epoch.cmp(&local.finalized_epoch) { Ordering::Less => { // The node has a lower finalized epoch, their chain is not useful to us. There are two @@ -63,7 +101,11 @@ pub fn remote_sync_type( { // This peer has a head ahead enough of ours and we have no knowledge of their best // block. - PeerSyncType::Advanced + PeerSyncType::Advanced(PeerSyncTypeAdvanced::Head { + target_root: remote.head_root, + target_slot: remote.head_slot, + start_epoch: local.local_irreversible_epoch, + }) } else { // This peer is either in the tolerance range, or ahead us with an already rejected // block. @@ -71,16 +113,43 @@ pub fn remote_sync_type( } } Ordering::Greater => { - if (local.finalized_epoch + 1 == remote.finalized_epoch - && near_range_start <= remote.head_slot - && remote.head_slot <= near_range_end) - || chain.block_is_known_to_fork_choice(&remote.head_root) - { - // This peer is near enough to us to be considered synced, or - // we have already synced up to this peer's head + if chain.block_is_known_to_fork_choice(&remote.head_root) { + // We have already synced up to this peer's head PeerSyncType::FullySynced } else { - PeerSyncType::Advanced + let finality_advanced = remote.finalized_epoch > local.finalized_epoch + 1; + let head_advanced = remote.head_slot > near_range_end; + let finality_ahead_local_irreversible = + remote.finalized_epoch > local.local_irreversible_epoch; + + if finality_advanced { + if finality_ahead_local_irreversible { + PeerSyncType::Advanced(PeerSyncTypeAdvanced::Finalized { + target_root: remote.finalized_root, + target_slot: remote + .finalized_epoch + .start_slot(T::EthSpec::slots_per_epoch()), + start_epoch: local.local_irreversible_epoch, + }) + } else if head_advanced { + PeerSyncType::Advanced(PeerSyncTypeAdvanced::Head { + target_root: remote.head_root, + target_slot: remote.head_slot, + start_epoch: local.local_irreversible_epoch, + }) + } else { + PeerSyncType::FullySynced + } + } else if head_advanced { + PeerSyncType::Advanced(PeerSyncTypeAdvanced::Head { + target_root: remote.head_root, + target_slot: remote.head_slot, + start_epoch: local.local_irreversible_epoch, + }) + } else { + // This peer is near enough to us to be considered synced + PeerSyncType::FullySynced + } } } } diff --git a/beacon_node/network/src/sync/range_sync/chain_collection.rs b/beacon_node/network/src/sync/range_sync/chain_collection.rs index 1d57ee6c3dc..0aebc30dca4 100644 --- a/beacon_node/network/src/sync/range_sync/chain_collection.rs +++ b/beacon_node/network/src/sync/range_sync/chain_collection.rs @@ -7,14 +7,14 @@ use super::chain::{ChainId, ProcessingResult, RemoveChain, SyncingChain}; use super::sync_type::RangeSyncType; use crate::metrics; use crate::sync::network_context::SyncNetworkContext; +use crate::sync::peer_sync_info::LocalSyncInfo; +use crate::sync::range_sync::range::AwaitingHeadPeers; use beacon_chain::{BeaconChain, BeaconChainTypes}; use fnv::FnvHashMap; use lighthouse_network::PeerId; -use lighthouse_network::SyncInfo; use lighthouse_network::service::api_types::Id; use logging::crit; use smallvec::SmallVec; -use std::collections::HashMap; use std::collections::hash_map::Entry; use std::sync::Arc; use tracing::{debug, error}; @@ -193,24 +193,18 @@ impl ChainCollection { pub fn update( &mut self, network: &mut SyncNetworkContext, - local: &SyncInfo, - awaiting_head_peers: &mut HashMap, + local: &LocalSyncInfo, + awaiting_head_peers: &mut AwaitingHeadPeers, ) { // Remove any outdated finalized/head chains self.purge_outdated_chains(local, awaiting_head_peers); - let local_head_epoch = local.head_slot.epoch(T::EthSpec::slots_per_epoch()); // Choose the best finalized chain if one needs to be selected. - self.update_finalized_chains(network, local.finalized_epoch, local_head_epoch); + self.update_finalized_chains(network, local); if !matches!(self.state, RangeSyncState::Finalized(_)) { // Handle head syncing chains if there are no finalized chains left. - self.update_head_chains( - network, - local.finalized_epoch, - local_head_epoch, - awaiting_head_peers, - ); + self.update_head_chains(network, local, awaiting_head_peers); } } @@ -253,8 +247,7 @@ impl ChainCollection { fn update_finalized_chains( &mut self, network: &mut SyncNetworkContext, - local_epoch: Epoch, - local_head_epoch: Epoch, + local: &LocalSyncInfo, ) { // Find the chain with most peers and check if it is already syncing if let Some((mut new_id, max_peers)) = self @@ -303,8 +296,11 @@ impl ChainCollection { // update the state to a new finalized state self.state = RangeSyncState::Finalized(new_id); - if let Err(remove_reason) = chain.start_syncing(network, local_epoch, local_head_epoch) - { + if let Err(remove_reason) = chain.start_syncing( + network, + local.local_irreversible_epoch, + local.head_slot.epoch(T::EthSpec::slots_per_epoch()), + ) { if remove_reason.is_critical() { crit!(chain = new_id, reason = ?remove_reason, "Chain removed while switching chains"); } else { @@ -321,17 +317,16 @@ impl ChainCollection { fn update_head_chains( &mut self, network: &mut SyncNetworkContext, - local_epoch: Epoch, - local_head_epoch: Epoch, - awaiting_head_peers: &mut HashMap, + local: &LocalSyncInfo, + awaiting_head_peers: &mut AwaitingHeadPeers, ) { // Include the awaiting head peers - for (peer_id, peer_sync_info) in awaiting_head_peers.drain() { + for (peer_id, (target_root, target_slot)) in awaiting_head_peers.drain() { debug!("including head peer"); self.add_peer_or_create_chain( - local_epoch, - peer_sync_info.head_root, - peer_sync_info.head_slot, + local.local_irreversible_epoch, + target_root, + target_slot, peer_id, RangeSyncType::Head, network, @@ -361,9 +356,11 @@ impl ChainCollection { if !chain.is_syncing() { debug!(id = chain.id(), "New head chain started syncing"); } - if let Err(remove_reason) = - chain.start_syncing(network, local_epoch, local_head_epoch) - { + if let Err(remove_reason) = chain.start_syncing( + network, + local.local_irreversible_epoch, + local.head_slot.epoch(T::EthSpec::slots_per_epoch()), + ) { self.head_chains.remove(&id); if remove_reason.is_critical() { crit!(chain = id, reason = ?remove_reason, "Chain removed while switching head chains"); @@ -396,11 +393,11 @@ impl ChainCollection { /// finalized block slot. Peers that would create outdated chains are removed too. pub fn purge_outdated_chains( &mut self, - local_info: &SyncInfo, - awaiting_head_peers: &mut HashMap, + local_info: &LocalSyncInfo, + awaiting_head_peers: &mut AwaitingHeadPeers, ) { let local_finalized_slot = local_info - .finalized_epoch + .local_irreversible_epoch .start_slot(T::EthSpec::slots_per_epoch()); let beacon_chain = &self.beacon_chain; @@ -411,9 +408,8 @@ impl ChainCollection { }; // Retain only head peers that remain relevant - awaiting_head_peers.retain(|_peer_id, peer_sync_info| { - !is_outdated(&peer_sync_info.head_slot, &peer_sync_info.head_root) - }); + awaiting_head_peers + .retain(|_peer_id, (target_root, target_slot)| !is_outdated(target_slot, target_root)); // Remove chains that are out-dated let mut removed_chains = Vec::new(); diff --git a/beacon_node/network/src/sync/range_sync/range.rs b/beacon_node/network/src/sync/range_sync/range.rs index c9656ad1d0d..734d1107d25 100644 --- a/beacon_node/network/src/sync/range_sync/range.rs +++ b/beacon_node/network/src/sync/range_sync/range.rs @@ -43,25 +43,27 @@ use super::chain::{ChainId, RemoveChain, SyncingChain}; use super::chain_collection::{ChainCollection, SyncChainStatus}; use super::sync_type::RangeSyncType; use crate::metrics; -use crate::status::ToStatusMessage; use crate::sync::BatchProcessResult; use crate::sync::batch::BatchId; use crate::sync::network_context::{RpcResponseError, SyncNetworkContext}; +use crate::sync::peer_sync_info::{LocalSyncInfo, PeerSyncTypeAdvanced}; use beacon_chain::block_verification_types::RpcBlock; use beacon_chain::{BeaconChain, BeaconChainTypes}; +use lighthouse_network::PeerId; use lighthouse_network::rpc::GoodbyeReason; use lighthouse_network::service::api_types::Id; -use lighthouse_network::{PeerId, SyncInfo}; use logging::crit; use lru_cache::LRUTimeCache; use std::collections::HashMap; use std::sync::Arc; use tracing::{debug, trace, warn}; -use types::{Epoch, EthSpec, Hash256}; +use types::{Epoch, EthSpec, Hash256, Slot}; /// For how long we store failed finalized chains to prevent retries. const FAILED_CHAINS_EXPIRY_SECONDS: u64 = 30; +pub(crate) type AwaitingHeadPeers = HashMap; + /// The primary object dealing with long range/batch syncing. This contains all the active and /// non-active chains that need to be processed before the syncing is considered complete. This /// holds the current state of the long range sync. @@ -70,7 +72,7 @@ pub struct RangeSync { beacon_chain: Arc>, /// Last known sync info of our useful connected peers. We use this information to create Head /// chains after all finalized chains have ended. - awaiting_head_peers: HashMap, + awaiting_head_peers: AwaitingHeadPeers, /// A collection of chains that need to be downloaded. This stores any head or finalized chains /// that need to be downloaded. chains: ChainCollection, @@ -110,29 +112,28 @@ where pub fn add_peer( &mut self, network: &mut SyncNetworkContext, - local_info: SyncInfo, + local_info: LocalSyncInfo, peer_id: PeerId, - remote_info: SyncInfo, + advanced_type: PeerSyncTypeAdvanced, ) { // evaluate which chain to sync from // determine if we need to run a sync to the nearest finalized state or simply sync to // its current head - // convenience variable - let remote_finalized_slot = remote_info - .finalized_epoch - .start_slot(T::EthSpec::slots_per_epoch()); - // NOTE: A peer that has been re-status'd may now exist in multiple finalized chains. This // is OK since we since only one finalized chain at a time. // determine which kind of sync to perform and set up the chains - match RangeSyncType::new(self.beacon_chain.as_ref(), &local_info, &remote_info) { - RangeSyncType::Finalized => { + match advanced_type { + PeerSyncTypeAdvanced::Finalized { + target_root, + target_slot, + start_epoch, + } => { // Make sure we have not recently tried this chain - if self.failed_chains.contains(&remote_info.finalized_root) { - debug!(failed_root = ?remote_info.finalized_root, %peer_id,"Disconnecting peer that belongs to previously failed chain"); + if self.failed_chains.contains(&target_root) { + debug!(failed_root = ?target_root, %peer_id,"Disconnecting peer that belongs to previously failed chain"); network.goodbye_peer(peer_id, GoodbyeReason::IrrelevantNetwork); return; } @@ -145,15 +146,14 @@ where // to using exact epoch boundaries for batches (rather than one slot past the epoch // boundary), we need to sync finalized sync to 2 epochs + 1 slot past our peer's // finalized slot in order to finalize the chain locally. - let target_head_slot = - remote_finalized_slot + (2 * T::EthSpec::slots_per_epoch()) + 1; + let target_head_slot = target_slot + (2 * T::EthSpec::slots_per_epoch()) + 1; // Note: We keep current head chains. These can continue syncing whilst we complete // this new finalized chain. self.chains.add_peer_or_create_chain( - local_info.finalized_epoch, - remote_info.finalized_root, + start_epoch, + target_root, target_head_slot, peer_id, RangeSyncType::Finalized, @@ -163,14 +163,19 @@ where self.chains .update(network, &local_info, &mut self.awaiting_head_peers); } - RangeSyncType::Head => { + PeerSyncTypeAdvanced::Head { + target_root, + target_slot, + start_epoch, + } => { // This peer requires a head chain sync if self.chains.is_finalizing_sync() { // If there are finalized chains to sync, finish these first, before syncing head // chains. trace!(%peer_id, awaiting_head_peers = &self.awaiting_head_peers.len(),"Waiting for finalized sync to complete"); - self.awaiting_head_peers.insert(peer_id, remote_info); + self.awaiting_head_peers + .insert(peer_id, (target_root, target_slot)); return; } @@ -181,12 +186,10 @@ where // The new peer has the same finalized (earlier filters should prevent a peer with an // earlier finalized chain from reaching here). - let start_epoch = std::cmp::min(local_info.head_slot, remote_finalized_slot) - .epoch(T::EthSpec::slots_per_epoch()); self.chains.add_peer_or_create_chain( start_epoch, - remote_info.head_root, - remote_info.head_slot, + target_root, + target_slot, peer_id, RangeSyncType::Head, network, @@ -357,16 +360,8 @@ where network.status_peers(self.beacon_chain.as_ref(), chain.peers()); - let status = self.beacon_chain.status_message(); - let local = SyncInfo { - head_slot: *status.head_slot(), - head_root: *status.head_root(), - finalized_epoch: *status.finalized_epoch(), - finalized_root: *status.finalized_root(), - earliest_available_slot: status.earliest_available_slot().ok().cloned(), - }; - // update the state of the collection + let local = LocalSyncInfo::new(&self.beacon_chain); self.chains .update(network, &local, &mut self.awaiting_head_peers); } diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 9553fe60ba2..bbcee916457 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -1224,6 +1224,18 @@ pub fn cli_app() -> Command { .conflicts_with("checkpoint-state") .display_order(0) ) + .arg( + Arg::new("checkpoint-sync-state-id") + .long("checkpoint-sync-state-id") + .help("Set the state ID to checkpoint sync to using a beacon node HTTP endpoint. \ + Possible values: 'finalized', 'justified', , ") + .value_name("CHECKPOINT_SYNC_STATE_ID") + .action(ArgAction::Set) + .conflicts_with("checkpoint-state") + .requires("checkpoint-sync-url") + .default_value("finalized") + .display_order(0) + ) .arg( Arg::new("checkpoint-sync-url-timeout") .long("checkpoint-sync-url-timeout") diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 752cf105505..3d5123a4f7c 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -13,6 +13,7 @@ use clap_utils::{parse_flag, parse_required}; use client::{ClientConfig, ClientGenesis}; use directory::{DEFAULT_BEACON_NODE_DIR, DEFAULT_NETWORK_DIR, DEFAULT_ROOT_DIR}; use environment::RuntimeContext; +use eth2::types::StateId; use execution_layer::DEFAULT_JWT_FILE; use http_api::TlsConfig; use lighthouse_network::{Enr, Multiaddr, NetworkConfig, PeerIdSerialized}; @@ -539,8 +540,15 @@ pub fn get_config( } else if let Some(remote_bn_url) = cli_args.get_one::("checkpoint-sync-url") { let url = SensitiveUrl::parse(remote_bn_url) .map_err(|e| format!("Invalid checkpoint sync URL: {:?}", e))?; - - ClientGenesis::CheckpointSyncUrl { url } + let state_id = cli_args + .get_one::("checkpoint-sync-state-id") + .ok_or("Missing --checkpoint-sync-state-id flag")?; + + ClientGenesis::CheckpointSyncUrl { + url, + state_id: StateId::from_str(state_id) + .map_err(|e| format!("Invalid state-id {e:?}"))?, + } } else { ClientGenesis::GenesisState } diff --git a/beacon_node/store/src/metadata.rs b/beacon_node/store/src/metadata.rs index cf494684515..215cdb2b64d 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::{Hash256, Slot}; -pub const CURRENT_SCHEMA_VERSION: SchemaVersion = SchemaVersion(28); +pub const CURRENT_SCHEMA_VERSION: SchemaVersion = SchemaVersion(29); // All the keys that get stored under the `BeaconMeta` column. // diff --git a/book/src/help_bn.md b/book/src/help_bn.md index d3aa27c8a77..05e1cce703c 100644 --- a/book/src/help_bn.md +++ b/book/src/help_bn.md @@ -66,6 +66,10 @@ Options: Set a checkpoint state to start syncing from. Must be aligned and match --checkpoint-block. Using --checkpoint-sync-url instead is recommended. + --checkpoint-sync-state-id + Set the state ID to checkpoint sync to using a beacon node HTTP + endpoint. Possible values: 'finalized', 'justified', , [default: finalized] --checkpoint-sync-url Set the remote beacon node HTTP endpoint to use for checkpoint sync. --checkpoint-sync-url-timeout diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index 4acfe3a640e..43d63ca706a 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -120,7 +120,7 @@ impl fmt::Display for BlockId { } } -#[derive(Debug, Copy, Clone, PartialEq)] +#[derive(Debug, Copy, Clone, Serialize, Deserialize, PartialEq)] pub enum StateId { Head, Genesis, @@ -1499,8 +1499,9 @@ pub struct StandardLivenessResponseData { #[derive(Debug, Serialize, Deserialize)] pub struct ManualFinalizationRequestData { - pub state_root: Hash256, + /// Irreversible checkpoint epoch pub epoch: Epoch, + /// Irreversible checkpoint root pub block_root: Hash256, } diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 9744b9fa084..f3fc842e854 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -4,7 +4,7 @@ use fixed_bytes::FixedBytesExtended; use logging::crit; use proto_array::{ Block as ProtoBlock, DisallowedReOrgOffsets, ExecutionStatus, JustifiedBalances, - ProposerHeadError, ProposerHeadInfo, ProtoArrayForkChoice, ReOrgThreshold, + LocalCheckpoint, ProposerHeadError, ProposerHeadInfo, ProtoArrayForkChoice, ReOrgThreshold, }; use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; @@ -61,12 +61,8 @@ pub enum Error { block_root: Hash256, payload_verification_status: PayloadVerificationStatus, }, - MissingJustifiedBlock { - justified_checkpoint: Checkpoint, - }, - MissingFinalizedBlock { - finalized_checkpoint: Checkpoint, - }, + MissingJustifiedBlock(Hash256), + MissingFinalizedBlock(Hash256), WrongSlotForGetProposerHead { current_slot: Slot, fc_store_slot: Slot, @@ -76,6 +72,7 @@ pub enum Error { }, UnrealizedVoteProcessing(state_processing::EpochProcessingError), ValidatorStatuses(BeaconStateError), + BadIrreversibleCheckpoint(String), ChainSpecError(String), } @@ -295,11 +292,50 @@ pub struct ForkchoiceUpdateParameters { pub finalized_hash: Option, } +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct ForkChoiceCheckpoint { + local: Checkpoint, + on_chain: Checkpoint, +} + +impl ForkChoiceCheckpoint { + pub fn new(local: Checkpoint, on_chain: Checkpoint) -> Self { + Self { local, on_chain } + } + + pub fn on_chain(&self) -> Checkpoint { + self.on_chain + } + + pub fn local(&self) -> Checkpoint { + self.on_chain.clamp_min(self.local) + } + + pub fn as_local(&self) -> LocalCheckpoint { + LocalCheckpoint::new(self.on_chain, self.local) + } +} + +impl std::fmt::Display for ForkChoiceCheckpoint { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if self.on_chain.epoch >= self.local.epoch { + write!(f, "{}/{}", self.on_chain.root, self.on_chain.epoch) + } else { + // Only log local if local is ahead: only happens if explicitly set + write!( + f, + "{}/{}/local/{}/{}", + self.on_chain.root, self.on_chain.epoch, self.local.root, self.local.epoch, + ) + } + } +} + #[derive(Clone, Copy, Debug, PartialEq)] pub struct ForkChoiceView { pub head_block_root: Hash256, - pub justified_checkpoint: Checkpoint, - pub finalized_checkpoint: Checkpoint, + pub justified_checkpoint: ForkChoiceCheckpoint, + pub finalized_checkpoint: ForkChoiceCheckpoint, } /// Provides an implementation of "Ethereum 2.0 Phase 0 -- Beacon Chain Fork Choice": @@ -319,8 +355,6 @@ pub struct ForkChoice { proto_array: ProtoArrayForkChoice, /// Attestations that arrived at the current slot and must be queued for later processing. queued_attestations: Vec, - /// Stores a cache of the values required to be sent to the execution layer. - forkchoice_update_parameters: ForkchoiceUpdateParameters, _phantom: PhantomData, } @@ -344,7 +378,6 @@ where /// Instantiates `Self` from an anchor (genesis or another finalized checkpoint). pub fn from_anchor( fc_store: T, - anchor_block_root: Hash256, anchor_block: &SignedBeaconBlock, anchor_state: &BeaconState, current_slot: Option, @@ -358,15 +391,18 @@ where }); } - let finalized_block_slot = anchor_block.slot(); - let finalized_block_state_root = anchor_block.state_root(); + // If the current slot is not provided, use the value that was last provided to the store. + let current_slot = current_slot.unwrap_or_else(|| fc_store.get_current_slot()); + + let anchor_block_root = anchor_block.canonical_root(); + let anchor_block_slot = anchor_block.slot(); + let anchor_block_state_root = anchor_block.state_root(); let current_epoch_shuffling_id = AttestationShufflingId::new(anchor_block_root, anchor_state, RelativeEpoch::Current) .map_err(Error::BeaconStateError)?; let next_epoch_shuffling_id = AttestationShufflingId::new(anchor_block_root, anchor_state, RelativeEpoch::Next) .map_err(Error::BeaconStateError)?; - let execution_status = anchor_block.message().execution_payload().map_or_else( // If the block doesn't have an execution payload then it can't have // execution enabled. @@ -376,22 +412,21 @@ where // A default payload does not have execution enabled. ExecutionStatus::irrelevant() } else { - // Assume that this payload is valid, since the anchor should be a trusted block and - // state. + // Assume that this payload is valid, since the anchor should be a trusted block + // and state. ExecutionStatus::Valid(execution_payload.block_hash()) } }, ); - // If the current slot is not provided, use the value that was last provided to the store. - let current_slot = current_slot.unwrap_or_else(|| fc_store.get_current_slot()); - let proto_array = ProtoArrayForkChoice::new::( current_slot, - finalized_block_slot, - finalized_block_state_root, - *fc_store.justified_checkpoint(), - *fc_store.finalized_checkpoint(), + anchor_block_slot, + anchor_block_root, + anchor_block_state_root, + fc_store.justified_checkpoint().on_chain(), + fc_store.finalized_checkpoint().on_chain(), + fc_store.finalized_checkpoint().local(), current_epoch_shuffling_id, next_epoch_shuffling_id, execution_status, @@ -401,14 +436,6 @@ where fc_store, proto_array, queued_attestations: vec![], - // This will be updated during the next call to `Self::get_head`. - forkchoice_update_parameters: ForkchoiceUpdateParameters { - head_hash: None, - justified_hash: None, - finalized_hash: None, - // This will be updated during the next call to `Self::get_head`. - head_root: Hash256::zero(), - }, _phantom: PhantomData, }; @@ -418,14 +445,6 @@ where Ok(fork_choice) } - /// Returns cached information that can be used to issue a `forkchoiceUpdated` message to an - /// execution engine. - /// - /// These values are updated each time `Self::get_head` is called. - pub fn get_forkchoice_update_parameters(&self) -> ForkchoiceUpdateParameters { - self.forkchoice_update_parameters - } - /// Returns the block root of an ancestor of `block_root` at the given `slot`. (Note: `slot` refers /// to the block that is *returned*, not the one that is supplied.) /// @@ -489,8 +508,9 @@ where let store = &mut self.fc_store; let head_root = self.proto_array.find_head::( - *store.justified_checkpoint(), - *store.finalized_checkpoint(), + store.justified_checkpoint().on_chain(), + store.finalized_checkpoint().on_chain(), + store.finalized_checkpoint().local(), store.justified_balances(), store.proposer_boost_root(), store.equivocating_indices(), @@ -498,25 +518,6 @@ where spec, )?; - // Cache some values for the next forkchoiceUpdate call to the execution layer. - let head_hash = self - .get_block(&head_root) - .and_then(|b| b.execution_status.block_hash()); - let justified_root = self.justified_checkpoint().root; - let finalized_root = self.finalized_checkpoint().root; - let justified_hash = self - .get_block(&justified_root) - .and_then(|b| b.execution_status.block_hash()); - let finalized_hash = self - .get_block(&finalized_root) - .and_then(|b| b.execution_status.block_hash()); - self.forkchoice_update_parameters = ForkchoiceUpdateParameters { - head_root, - head_hash, - justified_hash, - finalized_hash, - }; - Ok(head_root) } @@ -592,24 +593,12 @@ where .map_err(ProposerHeadError::convert_inner_error) } - /// Return information about: - /// - /// - The LMD head of the chain. - /// - The FFG checkpoints. - /// - /// The information is "cached" since the last call to `Self::get_head`. - /// - /// ## Notes - /// - /// The finalized/justified checkpoints are determined from the fork choice store. Therefore, - /// it's possible that the state corresponding to `get_state(get_block(head_block_root))` will - /// have *differing* finalized and justified information. - pub fn cached_fork_choice_view(&self) -> ForkChoiceView { - ForkChoiceView { - head_block_root: self.forkchoice_update_parameters.head_root, - justified_checkpoint: self.justified_checkpoint(), - finalized_checkpoint: self.finalized_checkpoint(), - } + /// Returns the ExecutionBlockHash of a block. Return None if `block_root` is not known or if + /// the exeucution status of the block is `Irrelevant` + pub fn execution_hash(&self, block_root: Hash256) -> Option { + self.proto_array + .get_block(&block_root) + .and_then(|block| block.execution_status.block_hash()) } /// See `ProtoArrayForkChoice::process_execution_payload_validation` for documentation. @@ -628,7 +617,7 @@ where op: &InvalidationOperation, ) -> Result<(), Error> { self.proto_array - .process_execution_payload_invalidation::(op, self.finalized_checkpoint()) + .process_execution_payload_invalidation::(op, self.finalized_checkpoint().as_local()) .map_err(Error::FailedToProcessInvalidExecutionPayload) } @@ -701,7 +690,7 @@ where // Check that block is later than the finalized epoch slot (optimization to reduce calls to // get_ancestor). let finalized_slot = - compute_start_slot_at_epoch::(self.fc_store.finalized_checkpoint().epoch); + compute_start_slot_at_epoch::(self.fc_store.finalized_checkpoint().local().epoch); if block.slot() <= finalized_slot { return Err(Error::InvalidBlock(InvalidBlock::FinalizedSlot { finalized_slot, @@ -719,7 +708,7 @@ where // // https://github.com/ethereum/eth2.0-specs/pull/1884 let block_ancestor = self.get_ancestor(block.parent_root(), finalized_slot)?; - let finalized_root = self.fc_store.finalized_checkpoint().root; + let finalized_root = self.fc_store.finalized_checkpoint().local().root; if block_ancestor != Some(finalized_root) { return Err(Error::InvalidBlock(InvalidBlock::NotFinalizedDescendant { finalized_root, @@ -910,14 +899,23 @@ where unrealized_finalized_checkpoint: Some(unrealized_finalized_checkpoint), }, current_slot, - self.justified_checkpoint(), - self.finalized_checkpoint(), + self.justified_checkpoint().on_chain(), + self.finalized_checkpoint().as_local(), )?; Ok(()) } - /// Update checkpoints in store if necessary + /// Update checkpoints in store if necessary. + /// + /// Compares against `.local()` (= max of on_chain and local_irreversible) rather than + /// `.on_chain()`. During non-finalized checkpoint sync the local_irreversible epoch may be + /// ahead of the network's justified/finalized epochs. We must not advance the store's + /// on-chain checkpoints past the local irreversible because `set_justified_checkpoint` loads + /// the justified state from the store to recompute balances, and states prior to the anchor + /// are unavailable. The on-chain values are allowed to remain stale because `find_head` + /// uses `local` checkpoints for tree traversal, and `node_is_viable_for_head` has a leniency + /// rule for justified matching (`voting_source.epoch + 2 >= current_epoch`). fn update_checkpoints( &mut self, justified_checkpoint: Checkpoint, @@ -925,7 +923,7 @@ where justified_state_root_producer: impl FnOnce() -> Result>, ) -> Result<(), Error> { // Update justified checkpoint. - if justified_checkpoint.epoch > self.fc_store.justified_checkpoint().epoch { + if justified_checkpoint.epoch > self.fc_store.justified_checkpoint().local().epoch { let justified_state_root = justified_state_root_producer()?; self.fc_store .set_justified_checkpoint(justified_checkpoint, justified_state_root) @@ -933,7 +931,7 @@ where } // Update finalized checkpoint. - if finalized_checkpoint.epoch > self.fc_store.finalized_checkpoint().epoch { + if finalized_checkpoint.epoch > self.fc_store.finalized_checkpoint().local().epoch { self.fc_store.set_finalized_checkpoint(finalized_checkpoint); } @@ -1266,33 +1264,12 @@ where self.proto_array.get_weight(block_root) } - /// Returns the `ProtoBlock` for the justified checkpoint. - /// - /// ## Notes - /// - /// This does *not* return the "best justified checkpoint". It returns the justified checkpoint - /// that is used for computing balances. - pub fn get_justified_block(&self) -> Result> { - let justified_checkpoint = self.justified_checkpoint(); - self.get_block(&justified_checkpoint.root) - .ok_or(Error::MissingJustifiedBlock { - justified_checkpoint, - }) - } - - /// Returns the `ProtoBlock` for the finalized checkpoint. - pub fn get_finalized_block(&self) -> Result> { - let finalized_checkpoint = self.finalized_checkpoint(); - self.get_block(&finalized_checkpoint.root) - .ok_or(Error::MissingFinalizedBlock { - finalized_checkpoint, - }) - } - /// Return `true` if `block_root` is equal to the finalized checkpoint, or a known descendant of it. pub fn is_finalized_checkpoint_or_descendant(&self, block_root: Hash256) -> bool { - self.proto_array - .is_finalized_checkpoint_or_descendant::(block_root, self.finalized_checkpoint()) + self.proto_array.is_finalized_checkpoint_or_descendant::( + block_root, + self.finalized_checkpoint().as_local(), + ) } pub fn is_descendant(&self, ancestor_root: Hash256, descendant_root: Hash256) -> bool { @@ -1317,7 +1294,7 @@ where Ok(status.is_optimistic_or_invalid()) } else { Ok(self - .get_finalized_block()? + .local_irreversible_block()? .execution_status .is_optimistic_or_invalid()) } @@ -1339,14 +1316,21 @@ where } } + /// Returns the local irreversible block, which may be ahead of the network's finalized block + pub fn local_irreversible_block(&self) -> Result> { + let local_irreversible_root = self.finalized_checkpoint().local().root; + self.get_block(&local_irreversible_root) + .ok_or(Error::MissingFinalizedBlock(local_irreversible_root)) + } + /// Return the current finalized checkpoint. - pub fn finalized_checkpoint(&self) -> Checkpoint { - *self.fc_store.finalized_checkpoint() + pub fn finalized_checkpoint(&self) -> ForkChoiceCheckpoint { + self.fc_store.finalized_checkpoint() } /// Return the justified checkpoint. - pub fn justified_checkpoint(&self) -> Checkpoint { - *self.fc_store.justified_checkpoint() + pub fn justified_checkpoint(&self) -> ForkChoiceCheckpoint { + self.fc_store.justified_checkpoint() } pub fn unrealized_justified_checkpoint(&self) -> Checkpoint { @@ -1357,6 +1341,53 @@ where *self.fc_store.unrealized_finalized_checkpoint() } + /// Set the local irreversible checkpoint, which may be ahead of the network's finalized + /// checkpoint. Only blocks that descend from this checkpoint will be considered viable heads. + /// + /// This is used during non-finalized checkpoint sync, where the anchor state may be ahead of + /// the network's finalized checkpoint, and for manual finalization via the HTTP API. + /// + /// ## Validation + /// + /// The checkpoint root must be known in fork choice and must be a descendant of the current + /// finalized checkpoint. The checkpoint epoch must not be less than the block's slot epoch. + /// It must also not move the local irreversible checkpoint backwards. + pub fn set_local_irreversible_checkpoint( + &mut self, + checkpoint: Checkpoint, + ) -> Result<(), Error> { + let current_local_irreversible = self.finalized_checkpoint().local(); + + // Irreversible checkpoint is potentially user input, sanity check it + if let Some(block) = self.proto_array.get_block(&checkpoint.root) { + if checkpoint.epoch < block.slot.epoch(E::slots_per_epoch()) { + return Err(Error::BadIrreversibleCheckpoint(format!( + "Epoch {} less than block slot {}", + checkpoint.epoch, block.slot + ))); + } + if !self.is_finalized_checkpoint_or_descendant(checkpoint.root) { + return Err(Error::BadIrreversibleCheckpoint(format!( + "Block {:?} is not descendant of finalized checkpoint", + checkpoint.root + ))); + } + if !self.is_descendant(current_local_irreversible.root, checkpoint.root) { + return Err(Error::BadIrreversibleCheckpoint(format!( + "Block {:?} is not descendant of current local irreversible checkpoint {:?}", + checkpoint.root, current_local_irreversible.root, + ))); + } + } else { + return Err(Error::BadIrreversibleCheckpoint(format!( + "Unknown root {:?}", + checkpoint.root + ))); + } + self.fc_store.set_local_irreversible_checkpoint(checkpoint); + Ok(()) + } + /// Returns the latest message for a given validator, if any. /// /// Returns `(block_root, block_slot)`. @@ -1397,7 +1428,7 @@ where /// Prunes the underlying fork choice DAG. pub fn prune(&mut self) -> Result<(), Error> { - let finalized_root = self.fc_store.finalized_checkpoint().root; + let finalized_root = self.fc_store.finalized_checkpoint().local().root; self.proto_array .maybe_prune(finalized_root) @@ -1475,13 +1506,6 @@ where proto_array, queued_attestations: persisted.queued_attestations, // Will be updated in the following call to `Self::get_head`. - forkchoice_update_parameters: ForkchoiceUpdateParameters { - head_hash: None, - justified_hash: None, - finalized_hash: None, - // Will be updated in the following call to `Self::get_head`. - head_root: Hash256::zero(), - }, _phantom: PhantomData, }; @@ -1512,9 +1536,10 @@ where /// be instantiated again later. pub fn to_persisted(&self) -> PersistedForkChoice { PersistedForkChoice { - proto_array: self - .proto_array() - .as_ssz_container(self.justified_checkpoint(), self.finalized_checkpoint()), + proto_array: self.proto_array().as_ssz_container( + self.justified_checkpoint().local(), + self.finalized_checkpoint().local(), + ), queued_attestations: self.queued_attestations().to_vec(), } } diff --git a/consensus/fork_choice/src/fork_choice_store.rs b/consensus/fork_choice/src/fork_choice_store.rs index caa0ae9be24..51cc222869e 100644 --- a/consensus/fork_choice/src/fork_choice_store.rs +++ b/consensus/fork_choice/src/fork_choice_store.rs @@ -1,3 +1,4 @@ +use crate::ForkChoiceCheckpoint; use proto_array::JustifiedBalances; use std::collections::BTreeSet; use std::fmt::Debug; @@ -42,7 +43,7 @@ pub trait ForkChoiceStore: Sized { ) -> Result<(), Self::Error>; /// Returns the `justified_checkpoint`. - fn justified_checkpoint(&self) -> &Checkpoint; + fn justified_checkpoint(&self) -> ForkChoiceCheckpoint; /// Returns the state root of the justified checkpoint. fn justified_state_root(&self) -> Hash256; @@ -51,7 +52,7 @@ pub trait ForkChoiceStore: Sized { fn justified_balances(&self) -> &JustifiedBalances; /// Returns the `finalized_checkpoint`. - fn finalized_checkpoint(&self) -> &Checkpoint; + fn finalized_checkpoint(&self) -> ForkChoiceCheckpoint; /// Returns the `unrealized_justified_checkpoint`. fn unrealized_justified_checkpoint(&self) -> &Checkpoint; @@ -75,6 +76,10 @@ pub trait ForkChoiceStore: Sized { state_root: Hash256, ) -> Result<(), Self::Error>; + /// Sets the local irreversible checkpoint, which may be ahead of the network's justified + /// checkpoint. Only blocks descendant of this checkpoint are viable heads. + fn set_local_irreversible_checkpoint(&mut self, checkpoint: Checkpoint); + /// Sets the `unrealized_justified_checkpoint`. fn set_unrealized_justified_checkpoint(&mut self, checkpoint: Checkpoint, state_root: Hash256); diff --git a/consensus/fork_choice/src/lib.rs b/consensus/fork_choice/src/lib.rs index afe06dee1bc..4593050069e 100644 --- a/consensus/fork_choice/src/lib.rs +++ b/consensus/fork_choice/src/lib.rs @@ -3,9 +3,10 @@ mod fork_choice_store; mod metrics; pub use crate::fork_choice::{ - AttestationFromBlock, Error, ForkChoice, ForkChoiceView, ForkchoiceUpdateParameters, - InvalidAttestation, InvalidBlock, PayloadVerificationStatus, PersistedForkChoice, - PersistedForkChoiceV17, PersistedForkChoiceV28, QueuedAttestation, ResetPayloadStatuses, + AttestationFromBlock, Error, ForkChoice, ForkChoiceCheckpoint, ForkChoiceView, + ForkchoiceUpdateParameters, InvalidAttestation, InvalidBlock, PayloadVerificationStatus, + PersistedForkChoice, PersistedForkChoiceV17, PersistedForkChoiceV28, QueuedAttestation, + ResetPayloadStatuses, }; pub use fork_choice_store::ForkChoiceStore; pub use proto_array::{ diff --git a/consensus/fork_choice/tests/tests.rs b/consensus/fork_choice/tests/tests.rs index d3a84ee85be..8d89fc606f7 100644 --- a/consensus/fork_choice/tests/tests.rs +++ b/consensus/fork_choice/tests/tests.rs @@ -88,7 +88,7 @@ impl ForkChoiceTest { /// Assert the epochs match. pub fn assert_finalized_epoch(self, epoch: u64) -> Self { assert_eq!( - self.get(|fc_store| fc_store.finalized_checkpoint().epoch), + self.get(|fc_store| fc_store.finalized_checkpoint().on_chain().epoch), Epoch::new(epoch), "finalized_epoch" ); @@ -98,7 +98,7 @@ impl ForkChoiceTest { /// Assert the epochs match. pub fn assert_justified_epoch(self, epoch: u64) -> Self { assert_eq!( - self.get(|fc_store| fc_store.justified_checkpoint().epoch), + self.get(|fc_store| fc_store.justified_checkpoint().on_chain().epoch), Epoch::new(epoch), "justified_epoch" ); @@ -370,7 +370,7 @@ impl ForkChoiceTest { let state_root = harness .chain .store - .get_blinded_block(&fc.fc_store().justified_checkpoint().root) + .get_blinded_block(&fc.fc_store().justified_checkpoint().local().root) .unwrap() .unwrap() .message() @@ -517,22 +517,6 @@ impl ForkChoiceTest { } } -#[test] -fn justified_and_finalized_blocks() { - let tester = ForkChoiceTest::new(); - let fork_choice = tester.harness.chain.canonical_head.fork_choice_read_lock(); - - let justified_checkpoint = fork_choice.justified_checkpoint(); - assert_eq!(justified_checkpoint.epoch, 0); - assert!(justified_checkpoint.root != Hash256::zero()); - assert!(fork_choice.get_justified_block().is_ok()); - - let finalized_checkpoint = fork_choice.finalized_checkpoint(); - assert_eq!(finalized_checkpoint.epoch, 0); - assert!(finalized_checkpoint.root != Hash256::zero()); - assert!(fork_choice.get_finalized_block().is_ok()); -} - /// - The new justified checkpoint descends from the current. Near genesis. #[tokio::test] async fn justified_checkpoint_updates_with_descendent_first_justification() { @@ -1293,3 +1277,35 @@ async fn progressive_balances_cache_proposer_slashing() { .apply_blocks(E::slots_per_epoch() as usize) .await; } + +#[tokio::test] +async fn local_irreversible_checkpoint_cannot_move_backwards() { + let test = ForkChoiceTest::new() + .apply_blocks_while(|_, state| state.finalized_checkpoint().epoch == 0) + .await + .unwrap() + .apply_blocks(2) + .await; + + let chain = &test.harness.chain; + let mut fork_choice = chain.canonical_head.fork_choice_write_lock(); + let previous_local_irreversible = fork_choice.finalized_checkpoint().local(); + let head_root = chain.head_beacon_block_root(); + let head_slot = fork_choice + .get_block(&head_root) + .expect("head block exists in fork choice") + .slot; + let advanced_checkpoint = Checkpoint { + epoch: head_slot.epoch(E::slots_per_epoch()), + root: head_root, + }; + + fork_choice + .set_local_irreversible_checkpoint(advanced_checkpoint) + .expect("advancing local irreversible checkpoint should succeed"); + + let err = fork_choice + .set_local_irreversible_checkpoint(previous_local_irreversible) + .expect_err("moving local irreversible checkpoint backwards should fail"); + assert!(matches!(err, ForkChoiceError::BadIrreversibleCheckpoint(_))); +} diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index e9deb6759fc..bae8cc57060 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -3,6 +3,7 @@ mod ffg_updates; mod no_votes; mod votes; +use crate::proto_array::LocalCheckpoint; use crate::proto_array_fork_choice::{Block, ExecutionStatus, ProtoArrayForkChoice}; use crate::{InvalidationOperation, JustifiedBalances}; use fixed_bytes::FixedBytesExtended; @@ -80,12 +81,16 @@ impl ForkChoiceTestDefinition { let junk_shuffling_id = AttestationShufflingId::from_components(Epoch::new(0), Hash256::zero()); + let anchor_block_root = self.justified_checkpoint.root; + let anchor_block_state_root = Hash256::zero(); let mut fork_choice = ProtoArrayForkChoice::new::( self.finalized_block_slot, self.finalized_block_slot, - Hash256::zero(), + anchor_block_root, + anchor_block_state_root, self.justified_checkpoint, self.finalized_checkpoint, + self.finalized_checkpoint, junk_shuffling_id.clone(), junk_shuffling_id, ExecutionStatus::Optimistic(ExecutionBlockHash::zero()), @@ -108,6 +113,7 @@ impl ForkChoiceTestDefinition { .find_head::( justified_checkpoint, finalized_checkpoint, + finalized_checkpoint, &justified_balances, Hash256::zero(), &equivocating_indices, @@ -139,6 +145,7 @@ impl ForkChoiceTestDefinition { .find_head::( justified_checkpoint, finalized_checkpoint, + finalized_checkpoint, &justified_balances, proposer_boost_root, &equivocating_indices, @@ -167,6 +174,7 @@ impl ForkChoiceTestDefinition { let result = fork_choice.find_head::( justified_checkpoint, finalized_checkpoint, + finalized_checkpoint, &justified_balances, Hash256::zero(), &equivocating_indices, @@ -217,7 +225,10 @@ impl ForkChoiceTestDefinition { block, slot, self.justified_checkpoint, - self.finalized_checkpoint, + LocalCheckpoint::new( + self.finalized_checkpoint, + self.finalized_checkpoint, + ), ) .unwrap_or_else(|e| { panic!( @@ -280,7 +291,10 @@ impl ForkChoiceTestDefinition { fork_choice .process_execution_payload_invalidation::( &op, - self.finalized_checkpoint, + LocalCheckpoint::new( + self.finalized_checkpoint, + self.finalized_checkpoint, + ), ) .unwrap() } diff --git a/consensus/proto_array/src/lib.rs b/consensus/proto_array/src/lib.rs index 964e836d91d..5e7d12a55c6 100644 --- a/consensus/proto_array/src/lib.rs +++ b/consensus/proto_array/src/lib.rs @@ -6,7 +6,9 @@ mod proto_array_fork_choice; mod ssz_container; pub use crate::justified_balances::JustifiedBalances; -pub use crate::proto_array::{InvalidationOperation, calculate_committee_fraction}; +pub use crate::proto_array::{ + InvalidationOperation, LocalCheckpoint, calculate_committee_fraction, +}; pub use crate::proto_array_fork_choice::{ Block, DisallowedReOrgOffsets, DoNotReOrg, ExecutionStatus, ProposerHeadError, ProposerHeadInfo, ProtoArrayForkChoice, ReOrgThreshold, diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 5bfcdae463d..15f51defadd 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -126,6 +126,27 @@ impl Default for ProposerBoost { } } +/// Represents a checkpoint with a root that **MUST** have a node in ProtoArray. To enable +/// non-finalized checkpoint sync, the fork-choice deals with checkpoints with a block root that +/// is prior to the oldest node in the ProtoArray. However, some functions will error if you pass a +/// checkpoint for an unknown root. This type makes this distinction explicit. +/// +/// Thanks to the invariant: +/// +/// > Either the on chain finalized checkpoint or the local irreversible checkpoint root have a node +/// > in the ProtoArray. +/// +/// We can assure that a LocalCheckpoint created with `new` is a checkpoint whose root is in the +/// ProtoArray. +#[derive(Clone, Copy)] +pub struct LocalCheckpoint(Checkpoint); + +impl LocalCheckpoint { + pub fn new(checkpoint: Checkpoint, local_irreversible_checkpoint: Checkpoint) -> Self { + Self(checkpoint.clamp_min(local_irreversible_checkpoint)) + } +} + #[derive(PartialEq, Debug, Serialize, Deserialize, Clone)] pub struct ProtoArray { /// Do not attempt to prune the tree unless it has at least this many nodes. Small prunes @@ -155,7 +176,7 @@ impl ProtoArray { &mut self, mut deltas: Vec, best_justified_checkpoint: Checkpoint, - best_finalized_checkpoint: Checkpoint, + local_finalized_checkpoint: LocalCheckpoint, new_justified_balances: &JustifiedBalances, proposer_boost_root: Hash256, current_slot: Slot, @@ -289,7 +310,7 @@ impl ProtoArray { node_index, current_slot, best_justified_checkpoint, - best_finalized_checkpoint, + local_finalized_checkpoint, )?; } } @@ -305,7 +326,7 @@ impl ProtoArray { block: Block, current_slot: Slot, best_justified_checkpoint: Checkpoint, - best_finalized_checkpoint: Checkpoint, + local_finalized_checkpoint: LocalCheckpoint, ) -> Result<(), Error> { // If the block is already known, simply ignore it. if self.indices.contains_key(&block.root) { @@ -358,7 +379,7 @@ impl ProtoArray { node_index, current_slot, best_justified_checkpoint, - best_finalized_checkpoint, + local_finalized_checkpoint, )?; if matches!(block.execution_status, ExecutionStatus::Valid(_)) { @@ -441,7 +462,7 @@ impl ProtoArray { pub fn propagate_execution_payload_invalidation( &mut self, op: &InvalidationOperation, - best_finalized_checkpoint: Checkpoint, + local_finalized_checkpoint: LocalCheckpoint, ) -> Result<(), Error> { let mut invalidated_indices: HashSet = <_>::default(); let head_block_root = op.block_root(); @@ -472,7 +493,7 @@ impl ProtoArray { self.is_descendant(ancestor_root, head_block_root) && self.is_finalized_checkpoint_or_descendant::( ancestor_root, - best_finalized_checkpoint, + local_finalized_checkpoint, ) }); @@ -634,16 +655,17 @@ impl ProtoArray { /// best-child/best-descendant links. pub fn find_head( &self, - justified_root: &Hash256, current_slot: Slot, best_justified_checkpoint: Checkpoint, - best_finalized_checkpoint: Checkpoint, + local_justified_checkpoint: LocalCheckpoint, + local_finalized_checkpoint: LocalCheckpoint, ) -> Result { + let justified_root = local_justified_checkpoint.0.root; let justified_index = self .indices - .get(justified_root) + .get(&justified_root) .copied() - .ok_or(Error::JustifiedNodeUnknown(*justified_root))?; + .ok_or(Error::JustifiedNodeUnknown(justified_root))?; let justified_node = self .nodes @@ -658,9 +680,7 @@ impl ProtoArray { // // This scenario is *unsupported*. It represents a serious consensus failure. if justified_node.execution_status.is_invalid() { - return Err(Error::InvalidJustifiedCheckpointExecutionStatus { - justified_root: *justified_root, - }); + return Err(Error::InvalidJustifiedCheckpointExecutionStatus { justified_root }); } let best_descendant_index = justified_node.best_descendant.unwrap_or(justified_index); @@ -675,13 +695,13 @@ impl ProtoArray { best_node, current_slot, best_justified_checkpoint, - best_finalized_checkpoint, + local_finalized_checkpoint, ) { return Err(Error::InvalidBestNode(Box::new(InvalidBestNodeInfo { current_slot, - start_root: *justified_root, + start_root: justified_root, justified_checkpoint: best_justified_checkpoint, - finalized_checkpoint: best_finalized_checkpoint, + finalized_checkpoint: local_finalized_checkpoint.0, head_root: best_node.root, head_justified_checkpoint: best_node.justified_checkpoint, head_finalized_checkpoint: best_node.finalized_checkpoint, @@ -779,7 +799,7 @@ impl ProtoArray { child_index: usize, current_slot: Slot, best_justified_checkpoint: Checkpoint, - best_finalized_checkpoint: Checkpoint, + local_finalized_checkpoint: LocalCheckpoint, ) -> Result<(), Error> { let child = self .nodes @@ -795,7 +815,7 @@ impl ProtoArray { child, current_slot, best_justified_checkpoint, - best_finalized_checkpoint, + local_finalized_checkpoint, )?; // These three variables are aliases to the three options that we may set the @@ -829,7 +849,7 @@ impl ProtoArray { best_child, current_slot, best_justified_checkpoint, - best_finalized_checkpoint, + local_finalized_checkpoint, )?; if child_leads_to_viable_head && !best_child_leads_to_viable_head { @@ -880,7 +900,7 @@ impl ProtoArray { node: &ProtoNode, current_slot: Slot, best_justified_checkpoint: Checkpoint, - best_finalized_checkpoint: Checkpoint, + local_finalized_checkpoint: LocalCheckpoint, ) -> Result { let best_descendant_is_viable_for_head = if let Some(best_descendant_index) = node.best_descendant { @@ -893,7 +913,7 @@ impl ProtoArray { best_descendant, current_slot, best_justified_checkpoint, - best_finalized_checkpoint, + local_finalized_checkpoint, ) } else { false @@ -904,7 +924,7 @@ impl ProtoArray { node, current_slot, best_justified_checkpoint, - best_finalized_checkpoint, + local_finalized_checkpoint, )) } @@ -919,7 +939,7 @@ impl ProtoArray { node: &ProtoNode, current_slot: Slot, best_justified_checkpoint: Checkpoint, - best_finalized_checkpoint: Checkpoint, + local_finalized_checkpoint: LocalCheckpoint, ) -> bool { if node.execution_status.is_invalid() { return false; @@ -946,9 +966,9 @@ impl ProtoArray { || voting_source.epoch == best_justified_checkpoint.epoch || voting_source.epoch + 2 >= current_epoch; - let correct_finalized = best_finalized_checkpoint.epoch == genesis_epoch + let correct_finalized = local_finalized_checkpoint.0.epoch == genesis_epoch || self - .is_finalized_checkpoint_or_descendant::(node.root, best_finalized_checkpoint); + .is_finalized_checkpoint_or_descendant::(node.root, local_finalized_checkpoint); correct_justified && correct_finalized } @@ -1006,10 +1026,11 @@ impl ProtoArray { pub fn is_finalized_checkpoint_or_descendant( &self, root: Hash256, - best_finalized_checkpoint: Checkpoint, + local_finalized_checkpoint: LocalCheckpoint, ) -> bool { - let finalized_root = best_finalized_checkpoint.root; - let finalized_slot = best_finalized_checkpoint + let finalized_root = local_finalized_checkpoint.0.root; + let finalized_slot = local_finalized_checkpoint + .0 .epoch .start_slot(E::slots_per_epoch()); @@ -1032,7 +1053,7 @@ impl ProtoArray { // If the conditions don't match for this node then they're unlikely to // start matching for its ancestors. for checkpoint in &[node.finalized_checkpoint, node.justified_checkpoint] { - if checkpoint == &best_finalized_checkpoint { + if checkpoint == &local_finalized_checkpoint.0 { return true; } } @@ -1041,7 +1062,7 @@ impl ProtoArray { node.unrealized_finalized_checkpoint, node.unrealized_justified_checkpoint, ] { - if checkpoint.is_some_and(|cp| cp == best_finalized_checkpoint) { + if checkpoint.is_some_and(|cp| cp == local_finalized_checkpoint.0) { return true; } } @@ -1091,7 +1112,7 @@ impl ProtoArray { /// definition of "head" used by pruning (which does not consider viability) and fork choice. pub fn heads_descended_from_finalization( &self, - best_finalized_checkpoint: Checkpoint, + local_finalized_checkpoint: LocalCheckpoint, ) -> Vec<&ProtoNode> { self.nodes .iter() @@ -1099,7 +1120,7 @@ impl ProtoArray { node.best_child.is_none() && self.is_finalized_checkpoint_or_descendant::( node.root, - best_finalized_checkpoint, + local_finalized_checkpoint, ) }) .collect() diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 3edf1e0644d..bc7d9683015 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -2,7 +2,7 @@ use crate::{ JustifiedBalances, error::Error, proto_array::{ - InvalidationOperation, Iter, ProposerBoost, ProtoArray, ProtoNode, + InvalidationOperation, Iter, LocalCheckpoint, ProposerBoost, ProtoArray, ProtoNode, calculate_committee_fraction, }, ssz_container::SszContainer, @@ -415,10 +415,12 @@ impl ProtoArrayForkChoice { #[allow(clippy::too_many_arguments)] pub fn new( current_slot: Slot, - finalized_block_slot: Slot, - finalized_block_state_root: Hash256, + anchor_block_slot: Slot, + anchor_block_root: Hash256, + anchor_block_state_root: Hash256, justified_checkpoint: Checkpoint, finalized_checkpoint: Checkpoint, + local_irreversible_checkpoint: Checkpoint, current_epoch_shuffling_id: AttestationShufflingId, next_epoch_shuffling_id: AttestationShufflingId, execution_status: ExecutionStatus, @@ -431,13 +433,12 @@ impl ProtoArrayForkChoice { }; let block = Block { - slot: finalized_block_slot, - root: finalized_checkpoint.root, + slot: anchor_block_slot, + root: anchor_block_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_checkpoint.root, + state_root: anchor_block_state_root, + // TODO: What root to use here? + target_root: anchor_block_root, current_epoch_shuffling_id, next_epoch_shuffling_id, justified_checkpoint, @@ -447,12 +448,15 @@ impl ProtoArrayForkChoice { unrealized_finalized_checkpoint: Some(finalized_checkpoint), }; + let local_finalized_checkpoint = + LocalCheckpoint::new(finalized_checkpoint, local_irreversible_checkpoint); + proto_array .on_block::( block, current_slot, justified_checkpoint, - finalized_checkpoint, + local_finalized_checkpoint, ) .map_err(|e| format!("Failed to add finalized block to proto_array: {:?}", e))?; @@ -477,10 +481,10 @@ impl ProtoArrayForkChoice { pub fn process_execution_payload_invalidation( &mut self, op: &InvalidationOperation, - finalized_checkpoint: Checkpoint, + local_finalized_checkpoint: LocalCheckpoint, ) -> Result<(), String> { self.proto_array - .propagate_execution_payload_invalidation::(op, finalized_checkpoint) + .propagate_execution_payload_invalidation::(op, local_finalized_checkpoint) .map_err(|e| format!("Failed to process invalid payload: {:?}", e)) } @@ -504,8 +508,8 @@ impl ProtoArrayForkChoice { &mut self, block: Block, current_slot: Slot, - justified_checkpoint: Checkpoint, - finalized_checkpoint: Checkpoint, + best_justified_checkpoint: Checkpoint, + local_finalized_checkpoint: LocalCheckpoint, ) -> Result<(), String> { if block.parent_root.is_none() { return Err("Missing parent root".to_string()); @@ -515,8 +519,8 @@ impl ProtoArrayForkChoice { .on_block::( block, current_slot, - justified_checkpoint, - finalized_checkpoint, + best_justified_checkpoint, + local_finalized_checkpoint, ) .map_err(|e| format!("process_block_error: {:?}", e)) } @@ -526,6 +530,7 @@ impl ProtoArrayForkChoice { &mut self, justified_checkpoint: Checkpoint, finalized_checkpoint: Checkpoint, + local_irreversible_checkpoint: Checkpoint, justified_state_balances: &JustifiedBalances, proposer_boost_root: Hash256, equivocating_indices: &BTreeSet, @@ -544,11 +549,16 @@ impl ProtoArrayForkChoice { ) .map_err(|e| format!("find_head compute_deltas failed: {:?}", e))?; + let local_justified_checkpoint = + LocalCheckpoint::new(justified_checkpoint, local_irreversible_checkpoint); + let local_finalized_checkpoint = + LocalCheckpoint::new(finalized_checkpoint, local_irreversible_checkpoint); + self.proto_array .apply_score_changes::( deltas, justified_checkpoint, - finalized_checkpoint, + local_finalized_checkpoint, new_balances, proposer_boost_root, current_slot, @@ -560,10 +570,10 @@ impl ProtoArrayForkChoice { self.proto_array .find_head::( - &justified_checkpoint.root, current_slot, justified_checkpoint, - finalized_checkpoint, + local_justified_checkpoint, + local_finalized_checkpoint, ) .map_err(|e| format!("find_head failed: {:?}", e)) } @@ -901,10 +911,10 @@ impl ProtoArrayForkChoice { pub fn is_finalized_checkpoint_or_descendant( &self, descendant_root: Hash256, - best_finalized_checkpoint: Checkpoint, + local_finalized_checkpoint: LocalCheckpoint, ) -> bool { self.proto_array - .is_finalized_checkpoint_or_descendant::(descendant_root, best_finalized_checkpoint) + .is_finalized_checkpoint_or_descendant::(descendant_root, local_finalized_checkpoint) } pub fn latest_message(&self, validator_index: usize) -> Option<(Hash256, Epoch)> { @@ -983,10 +993,10 @@ impl ProtoArrayForkChoice { /// Returns all nodes that have zero children and are descended from the finalized checkpoint. pub fn heads_descended_from_finalization( &self, - best_finalized_checkpoint: Checkpoint, + local_finalized_checkpoint: LocalCheckpoint, ) -> Vec<&ProtoNode> { self.proto_array - .heads_descended_from_finalization::(best_finalized_checkpoint) + .heads_descended_from_finalization::(local_finalized_checkpoint) } } @@ -1108,9 +1118,10 @@ mod test_compute_deltas { fn finalized_descendant() { let genesis_slot = Slot::new(0); let genesis_epoch = Epoch::new(0); + let genesis_block_root = Hash256::from_low_u64_be(1); let state_root = Hash256::from_low_u64_be(0); - let finalized_root = Hash256::from_low_u64_be(1); + let finalized_root = genesis_block_root; let finalized_desc = Hash256::from_low_u64_be(2); let not_finalized_desc = Hash256::from_low_u64_be(3); let unknown = Hash256::from_low_u64_be(4); @@ -1122,6 +1133,7 @@ mod test_compute_deltas { epoch: genesis_epoch, root: finalized_root, }; + let local_genesis_checkpoint = LocalCheckpoint::new(genesis_checkpoint, genesis_checkpoint); let junk_checkpoint = Checkpoint { epoch: Epoch::new(42), root: Hash256::repeat_byte(42), @@ -1130,9 +1142,11 @@ mod test_compute_deltas { let mut fc = ProtoArrayForkChoice::new::( genesis_slot, genesis_slot, + genesis_block_root, state_root, genesis_checkpoint, genesis_checkpoint, + genesis_checkpoint, junk_shuffling_id.clone(), junk_shuffling_id.clone(), execution_status, @@ -1158,7 +1172,7 @@ mod test_compute_deltas { }, genesis_slot + 1, genesis_checkpoint, - genesis_checkpoint, + local_genesis_checkpoint, ) .unwrap(); @@ -1183,7 +1197,7 @@ mod test_compute_deltas { }, genesis_slot + 1, genesis_checkpoint, - genesis_checkpoint, + local_genesis_checkpoint, ) .unwrap(); @@ -1199,22 +1213,20 @@ mod test_compute_deltas { assert!(fc.is_finalized_checkpoint_or_descendant::( finalized_root, - genesis_checkpoint + local_genesis_checkpoint )); assert!(fc.is_finalized_checkpoint_or_descendant::( finalized_desc, - genesis_checkpoint + local_genesis_checkpoint )); assert!(!fc.is_finalized_checkpoint_or_descendant::( not_finalized_desc, - genesis_checkpoint + local_genesis_checkpoint + )); + assert!(!fc.is_finalized_checkpoint_or_descendant::( + unknown, + local_genesis_checkpoint )); - assert!( - !fc.is_finalized_checkpoint_or_descendant::( - unknown, - genesis_checkpoint - ) - ); assert!(!fc.is_descendant(finalized_desc, not_finalized_desc)); assert!(fc.is_descendant(finalized_desc, finalized_desc)); @@ -1265,6 +1277,7 @@ mod test_compute_deltas { let junk_shuffling_id = AttestationShufflingId::from_components(Epoch::new(0), Hash256::zero()); let execution_status = ExecutionStatus::irrelevant(); + let genesis_block_root = get_block_root(0); let genesis_checkpoint = Checkpoint { epoch: Epoch::new(0), @@ -1274,9 +1287,11 @@ mod test_compute_deltas { let mut fc = ProtoArrayForkChoice::new::( genesis_slot, genesis_slot, + genesis_block_root, junk_state_root, genesis_checkpoint, genesis_checkpoint, + genesis_checkpoint, junk_shuffling_id.clone(), junk_shuffling_id.clone(), execution_status, @@ -1311,7 +1326,7 @@ mod test_compute_deltas { }, Slot::from(block.slot), genesis_checkpoint, - genesis_checkpoint, + LocalCheckpoint::new(genesis_checkpoint, genesis_checkpoint), ) .unwrap(); }; @@ -1370,12 +1385,14 @@ mod test_compute_deltas { root: finalized_root, epoch: Epoch::new(1), }; + let local_finalized_checkpoint = + LocalCheckpoint::new(finalized_checkpoint, finalized_checkpoint); assert!( fc.proto_array .is_finalized_checkpoint_or_descendant::( finalized_root, - finalized_checkpoint + local_finalized_checkpoint, ), "the finalized checkpoint is the finalized checkpoint" ); @@ -1384,7 +1401,7 @@ mod test_compute_deltas { fc.proto_array .is_finalized_checkpoint_or_descendant::( get_block_root(canonical_slot), - finalized_checkpoint + local_finalized_checkpoint, ), "the canonical block is a descendant of the finalized checkpoint" ); @@ -1392,7 +1409,7 @@ mod test_compute_deltas { !fc.proto_array .is_finalized_checkpoint_or_descendant::( get_block_root(non_canonical_slot), - finalized_checkpoint + local_finalized_checkpoint, ), "although the non-canonical block is a descendant of the finalized block, \ it's not a descendant of the finalized checkpoint" diff --git a/consensus/types/src/attestation/checkpoint.rs b/consensus/types/src/attestation/checkpoint.rs index f5a95f0ad94..f6c5f27b437 100644 --- a/consensus/types/src/attestation/checkpoint.rs +++ b/consensus/types/src/attestation/checkpoint.rs @@ -35,6 +35,17 @@ pub struct Checkpoint { pub root: Hash256, } +impl Checkpoint { + /// Returns `self` if its epoch is >= than the `other` checkpoint epoch + pub fn clamp_min(&self, other: Checkpoint) -> Checkpoint { + if self.epoch >= other.epoch { + *self + } else { + other + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index a2fad31f657..11872fe3e95 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -6,14 +6,16 @@ use beacon_node::beacon_chain::chain_config::{ }; use beacon_node::beacon_chain::custody_context::NodeCustodyType; use beacon_node::{ - ClientConfig as Config, beacon_chain::graffiti_calculator::GraffitiOrigin, + ClientConfig as Config, ClientGenesis, beacon_chain::graffiti_calculator::GraffitiOrigin, beacon_chain::store::config::DatabaseBackend as BeaconNodeBackend, }; use beacon_processor::BeaconProcessorConfig; +use eth2::types::StateId; use lighthouse_network::PeerId; use network_utils::unused_port::{ unused_tcp4_port, unused_tcp6_port, unused_udp4_port, unused_udp6_port, }; +use sensitive_url::SensitiveUrl; use std::fs::File; use std::io::{Read, Write}; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; @@ -211,6 +213,42 @@ fn fork_choice_before_proposal_timeout_zero() { .with_config(|config| assert_eq!(config.chain.fork_choice_before_proposal_timeout_ms, 0)); } +#[test] +fn checkpoint_sync_state_id_default() { + CommandLineTest::new() + .flag("checkpoint-sync-url", Some("http://beacon.node")) + .run_with_zero_port_and_no_genesis_sync() + .with_config(|config| { + assert_eq!( + config.genesis, + ClientGenesis::CheckpointSyncUrl { + url: SensitiveUrl::parse("http://beacon.node").unwrap(), + state_id: StateId::Finalized + } + ); + }); +} + +#[test] +fn checkpoint_sync_state_id_root() { + CommandLineTest::new() + .flag("checkpoint-sync-url", Some("http://beacon.node")) + .flag( + "checkpoint-sync-state-id", + Some("0x0000000000000000000000000000000000000000000000000000000000000000"), + ) + .run_with_zero_port_and_no_genesis_sync() + .with_config(|config| { + assert_eq!( + config.genesis, + ClientGenesis::CheckpointSyncUrl { + url: SensitiveUrl::parse("http://beacon.node").unwrap(), + state_id: StateId::Root(Hash256::ZERO) + } + ); + }); +} + #[test] fn checkpoint_sync_url_timeout_flag() { CommandLineTest::new() diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 45bed7c6cd4..885defc63a4 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -824,13 +824,14 @@ impl Tester { } pub fn check_justified_checkpoint(&self, expected_checkpoint: Checkpoint) -> Result<(), Error> { - let head_checkpoint = self.find_head()?.justified_checkpoint(); + let head_checkpoint = self.find_head()?.justified_checkpoint().local(); let fc_checkpoint = self .harness .chain .canonical_head .fork_choice_read_lock() - .justified_checkpoint(); + .justified_checkpoint() + .local(); assert_checkpoints_eq("justified_checkpoint", head_checkpoint, fc_checkpoint); @@ -841,13 +842,14 @@ impl Tester { &self, expected_checkpoint_root: Hash256, ) -> Result<(), Error> { - let head_checkpoint = self.find_head()?.justified_checkpoint(); + let head_checkpoint = self.find_head()?.justified_checkpoint().local(); let fc_checkpoint = self .harness .chain .canonical_head .fork_choice_read_lock() - .justified_checkpoint(); + .justified_checkpoint() + .local(); assert_checkpoints_eq("justified_checkpoint_root", head_checkpoint, fc_checkpoint); @@ -859,13 +861,14 @@ impl Tester { } pub fn check_finalized_checkpoint(&self, expected_checkpoint: Checkpoint) -> Result<(), Error> { - let head_checkpoint = self.find_head()?.finalized_checkpoint(); + let head_checkpoint = self.find_head()?.finalized_checkpoint().local(); let fc_checkpoint = self .harness .chain .canonical_head .fork_choice_read_lock() - .finalized_checkpoint(); + .finalized_checkpoint() + .local(); assert_checkpoints_eq("finalized_checkpoint", head_checkpoint, fc_checkpoint);