diff --git a/Cargo.lock b/Cargo.lock index 6ed7bfd0b60..1445846322d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1300,6 +1300,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 46ba14f596b..33c5ceff086 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -42,7 +42,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; @@ -552,10 +552,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 @@ -582,10 +584,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; @@ -1207,6 +1211,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 @@ -1418,7 +1442,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() @@ -1675,39 +1701,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`. @@ -4109,8 +4102,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() + .finalied_checkpoint_from_head_state(); // Compare the existing finalized checkpoint with the incoming block's finalized checkpoint. let new_finalized_checkpoint = state.finalized_checkpoint(); @@ -5938,16 +5933,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.", @@ -7165,7 +7160,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..388de6c28bd 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}; @@ -137,6 +136,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, @@ -168,9 +168,7 @@ where store: Arc>, anchor: BeaconSnapshot, ) -> Result { - let unadvanced_state_root = anchor.beacon_state_root(); let mut anchor_state = anchor.beacon_state; - let mut anchor_block_header = anchor_state.latest_block_header().clone(); // The anchor state MUST be on an epoch boundary (it should be advanced by the caller). if !anchor_state @@ -179,37 +177,47 @@ where .is_multiple_of(E::slots_per_epoch()) { return Err(Error::UnalignedCheckpoint { - block_slot: anchor_block_header.slot, + block_slot: anchor_state.latest_block_header().slot, state_slot: anchor_state.slot(), }); } - // Compute the accurate block root for the checkpoint block. - if anchor_block_header.state_root.is_zero() { - anchor_block_header.state_root = unadvanced_state_root; - } - let anchor_block_root = anchor_block_header.canonical_root(); let anchor_epoch = anchor_state.current_epoch(); - let justified_checkpoint = Checkpoint { + let anchor_block_checkpoint = Checkpoint { epoch: anchor_epoch, - root: anchor_block_root, + root: anchor.beacon_block_root, }; - let finalized_checkpoint = justified_checkpoint; + + // For post-genesis states this justified balances will not match the justified checkpoint. + // TODO(non-fin-cp-sync): Fetch the justified state from checkpointz server and get the + // justified balances from it. let justified_balances = JustifiedBalances::from_justified_state(&anchor_state)?; - let justified_state_root = anchor_state.canonical_root()?; + let anchor_state_root = anchor_state.canonical_root()?; + + let mut justified_checkpoint_on_chain = anchor_state.current_justified_checkpoint(); + let mut finalized_checkpoint_on_chain = anchor_state.finalized_checkpoint(); + + // If the network has not justified or finalized yet, use anchor checkpoint + if justified_checkpoint_on_chain.root == Hash256::ZERO { + justified_checkpoint_on_chain = anchor_block_checkpoint; + } + if finalized_checkpoint_on_chain.root == Hash256::ZERO { + finalized_checkpoint_on_chain = anchor_block_checkpoint; + } Ok(Self { store, balances_cache: <_>::default(), time: anchor_state.slot(), - justified_checkpoint, + justified_checkpoint: justified_checkpoint_on_chain, justified_balances, - justified_state_root, - finalized_checkpoint, - unrealized_justified_checkpoint: justified_checkpoint, - unrealized_justified_state_root: justified_state_root, - unrealized_finalized_checkpoint: finalized_checkpoint, - proposer_boost_root: Hash256::zero(), + justified_state_root: anchor_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: anchor_state_root, + unrealized_finalized_checkpoint: finalized_checkpoint_on_chain, + proposer_boost_root: Hash256::ZERO, equivocating_indices: BTreeSet::new(), _phantom: PhantomData, }) @@ -223,6 +231,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 +260,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 +293,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 +329,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 +344,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 +412,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 +433,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 +448,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 +480,19 @@ impl From<(PersistedForkChoiceStoreV28, JustifiedBalances)> for PersistedForkCho } } } + +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 874673b52e8..0a9c208ce7b 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -437,6 +437,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()); @@ -1725,8 +1723,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 58dbf1c35e8..114f129fa63 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -406,7 +406,6 @@ where let fork_choice = ForkChoice::from_anchor( fc_store, - genesis.beacon_block_root, &genesis.beacon_block, &genesis.beacon_state, current_slot, @@ -627,7 +626,6 @@ where let fork_choice = ForkChoice::from_anchor( fc_store, - snapshot.beacon_block_root, &snapshot.beacon_block, &snapshot.beacon_state, Some(weak_subj_slot), @@ -806,7 +804,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. @@ -837,7 +842,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!( @@ -916,7 +921,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 7dd4c88c513..937b11c56ac 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -1,4 +1,4 @@ -//! This module provides all functionality for finding the canonical head, updating all necessary +//! This module prvides all functionality for finding the canonical head, updating all necessary //! components (e.g. caches) and maintaining a cached head block and state. //! //! For practically all applications, the "canonical head" can be read using @@ -43,8 +43,8 @@ use crate::{ }; use eth2::types::{EventKind, SseChainReorg, SseFinalizedCheckpoint, SseHead, SseLateHead}; use fork_choice::{ - ExecutionStatus, ForkChoiceStore, ForkChoiceView, ForkchoiceUpdateParameters, ProtoBlock, - ResetPayloadStatuses, + ExecutionStatus, ForkChoiceCheckpoint, ForkChoiceStore, ForkChoiceView, + ForkchoiceUpdateParameters, ProtoBlock, ResetPayloadStatuses, }; use itertools::process_results; use lighthouse_tracing::SPAN_RECOMPUTE_HEAD; @@ -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 finalied_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. @@ -597,19 +602,23 @@ 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)?; + // TODO: Just with get_head, is it possible for finality to advance? + 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 +663,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 +711,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 +736,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(); @@ -763,8 +773,7 @@ impl BeaconChain { // The `after_finalization` function will take a write-lock on `fork_choice`, therefore it // 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) + && let Err(e) = self.after_finalization(&new_cached_head, new_view) { crit!( error = ?e, @@ -929,49 +938,53 @@ impl BeaconChain { self: &Arc, new_cached_head: &CachedHead, new_view: ForkChoiceView, - finalized_proto_block: ProtoBlock, ) -> 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); - self.observed_slashable.write().prune( - new_view - .finalized_checkpoint - .epoch - .start_slot(T::EthSpec::slots_per_epoch()), - ); + // Same as observed_block_producers + self.observed_blob_sidecars + .write() + .prune(new_local_finalized_slot); + + self.observed_slashable + .write() + .prune(new_local_finalized_slot); + // TODO: Should this use the local finality of the network? self.attester_cache - .prune_below(new_view.finalized_checkpoint.epoch); + .prune_below(new_local_finalized_checkpoint.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(), })); } @@ -981,29 +994,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, @@ -1017,8 +1037,8 @@ impl BeaconChain { // 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. self.store_migrator.process_finalization( - new_finalized_state_root.into(), - new_view.finalized_checkpoint, + new_local_finalized_state_root.into(), + new_local_finalized_checkpoint, )?; // Prune blobs in the background. @@ -1033,6 +1053,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(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); @@ -1063,6 +1108,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. /// @@ -1104,16 +1201,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, }) } } @@ -1136,19 +1234,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 3e859456b18..ef5a2c15a8f 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -719,11 +719,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 b9986025667..41c0752e7bd 100644 --- a/beacon_node/beacon_chain/src/data_column_verification.rs +++ b/beacon_node/beacon_chain/src/data_column_verification.rs @@ -740,6 +740,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 b021df2c33b..94b5a5cd4e8 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -76,7 +76,10 @@ pub enum BeaconChainError { ProposerSlashingValidationError(ProposerSlashingValidationError), AttesterSlashingValidationError(AttesterSlashingValidationError), BlsExecutionChangeValidationError(BlsExecutionChangeValidationError), - MissingFinalizedStateRoot(Slot), + MissingFinalizedStateRoot { + state_slot: Slot, + target_slot: Slot, + }, SszTypesError(SszTypesError), NoProposerForSlot(Slot), CanonicalHeadLockTimeout, @@ -182,10 +185,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..ab11de2fb42 100644 --- a/beacon_node/beacon_chain/src/fork_revert.rs +++ b/beacon_node/beacon_chain/src/fork_revert.rs @@ -137,7 +137,7 @@ 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, }; @@ -148,7 +148,6 @@ pub fn reset_fork_choice_to_finalization, Cold: It 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/persisted_fork_choice.rs b/beacon_node/beacon_chain/src/persisted_fork_choice.rs index d8fcc0901bf..cace30dd5fd 100644 --- a/beacon_node/beacon_chain/src/persisted_fork_choice.rs +++ b/beacon_node/beacon_chain/src/persisted_fork_choice.rs @@ -1,5 +1,7 @@ use crate::{ - beacon_fork_choice_store::{PersistedForkChoiceStoreV17, PersistedForkChoiceStoreV28}, + beacon_fork_choice_store::{ + PersistedForkChoiceStoreV17, PersistedForkChoiceStoreV28, PersistedForkChoiceStoreV29, + }, metrics, }; use ssz::{Decode, Encode}; @@ -9,22 +11,24 @@ use superstruct::superstruct; use types::Hash256; // If adding a new version you should update this type alias and fix the breakages. -pub type PersistedForkChoice = PersistedForkChoiceV28; +pub type PersistedForkChoice = PersistedForkChoiceV29; #[superstruct( - variants(V17, V28), + variants(V17, V28, V29), variant_attributes(derive(Encode, Decode)), no_enum )] pub struct PersistedForkChoice { #[superstruct(only(V17))] pub fork_choice_v17: fork_choice::PersistedForkChoiceV17, - #[superstruct(only(V28))] + #[superstruct(only(V28, V29))] pub fork_choice: fork_choice::PersistedForkChoiceV28, #[superstruct(only(V17))] pub fork_choice_store_v17: PersistedForkChoiceStoreV17, #[superstruct(only(V28))] - pub fork_choice_store: PersistedForkChoiceStoreV28, + pub fork_choice_store_v28: PersistedForkChoiceStoreV28, + #[superstruct(only(V29))] + pub fork_choice_store: PersistedForkChoiceStoreV29, } macro_rules! impl_store_item { @@ -47,7 +51,7 @@ macro_rules! impl_store_item { impl_store_item!(PersistedForkChoiceV17); -impl PersistedForkChoiceV28 { +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/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/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 6d17d6d85c5..f5f090aef13 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::data_column_custody_group::CustodyIndex; @@ -840,6 +841,7 @@ where .canonical_head .cached_head() .finalized_checkpoint() + .local() } pub fn justified_checkpoint(&self) -> Checkpoint { @@ -847,6 +849,7 @@ where .canonical_head .cached_head() .justified_checkpoint() + .local() } pub fn get_current_slot(&self) -> Slot { @@ -3056,6 +3059,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 2644b74b28e..085a2e5b803 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -1779,7 +1779,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); } @@ -1811,7 +1811,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. @@ -1834,7 +1834,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 5bd43835e33..a55bb7cd5f9 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -1141,12 +1141,7 @@ async fn payload_preparation_before_transition_block() { .prepare_beacon_proposer(current_slot) .await .unwrap(); - let forkchoice_update_params = rig - .harness - .chain - .canonical_head - .fork_choice_read_lock() - .get_forkchoice_update_parameters(); + let forkchoice_update_params = rig.harness.chain.head().forkchoice_update_parameters(); rig.harness .chain .update_execution_engine_forkchoice( @@ -1294,7 +1289,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 ba0621ae720..b81febcba3d 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -37,6 +37,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; @@ -191,7 +192,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,170 +2811,119 @@ 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; } -// Ensures that an unaligned checkpoint sync (the block is older than the state) -// works correctly even when `prune_payloads` is enabled. +// Checkpoint sync from the genesis state. // -// Previously, the `HotColdDB` would refuse to load the execution payload for the -// anchor block because it was considered "pruned", causing the node to fail startup. +// 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 reproduction_unaligned_checkpoint_sync_pruned_payload() { - let spec = test_spec::(); - - // Requires Execution Payloads. - let Some(_) = spec.deneb_fork_epoch else { - return; - }; - - // Create an unaligned checkpoint with a gap of 3 slots. - let num_initial_slots = E::slots_per_epoch() * 11; - let checkpoint_slot = Slot::new(E::slots_per_epoch() * 9 - 3); - - let slots = (1..num_initial_slots) - .map(Slot::new) - .filter(|&slot| slot <= checkpoint_slot || slot > checkpoint_slot + 3) - .collect::>(); - - let temp1 = tempdir().unwrap(); - let full_store = get_store_generic(&temp1, StoreConfig::default(), spec.clone()); - - let harness = get_harness_import_all_data_columns(full_store.clone(), LOW_VALIDATOR_COUNT); - let all_validators = (0..LOW_VALIDATOR_COUNT).collect::>(); - - 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, - &all_validators, - ) - .await; - - // Extract snapshot data from the harness. - let wss_block_root = harness - .chain - .block_root_at_slot(checkpoint_slot, WhenSlotSkipped::Prev) - .unwrap() - .unwrap(); - let wss_state_root = harness - .chain - .state_root_at_slot(checkpoint_slot) - .unwrap() - .unwrap(); - - let wss_block = harness - .chain - .store - .get_full_block(&wss_block_root) - .unwrap() - .unwrap(); - - // The test premise requires the anchor block to have a payload. - assert!(wss_block.message().execution_payload().is_ok()); - - let wss_blobs_opt = harness - .chain - .get_or_reconstruct_blobs(&wss_block_root) - .unwrap(); - - let wss_state = full_store - .get_state(&wss_state_root, Some(checkpoint_slot), CACHE_STATE_IN_TESTS) - .unwrap() - .unwrap(); - - // Configure the client with `prune_payloads = true`. - // This triggers the path where `try_get_full_block` must explicitly handle the anchor block. - let temp2 = tempdir().unwrap(); - let store_config = StoreConfig { - prune_payloads: true, - ..StoreConfig::default() - }; - - let store = get_store_generic(&temp2, store_config, spec.clone()); - - let slot_clock = TestingSlotClock::new( - Slot::new(0), - Duration::from_secs(harness.chain.genesis_time), - Duration::from_secs(spec.seconds_per_slot), - ); - slot_clock.set_slot(harness.get_current_slot().as_u64()); - - let chain_config = ChainConfig { - reconstruct_historic_states: true, - ..ChainConfig::default() - }; - - let trusted_setup = get_kzg(&spec); - let (shutdown_tx, _shutdown_rx) = futures::channel::mpsc::channel(1); - let mock = mock_execution_layer_from_parts( - harness.spec.clone(), - harness.runtime.task_executor.clone(), - ); - let all_custody_columns = (0..spec.number_of_custody_groups).collect::>(); - - // Attempt to build the BeaconChain. - // If the bug is present, this will panic with `MissingFullBlockExecutionPayloadPruned`. - let beacon_chain = BeaconChainBuilder::>::new(MinimalEthSpec, trusted_setup) - .chain_config(chain_config) - .store(store.clone()) - .custom_spec(spec.clone().into()) - .task_executor(harness.chain.task_executor.clone()) - .weak_subjectivity_state( - wss_state, - wss_block.clone(), - wss_blobs_opt.clone(), - genesis_state, - ) - .unwrap() - .store_migrator_config(MigratorConfig::default().blocking()) - .slot_clock(slot_clock) - .shutdown_sender(shutdown_tx) - .event_handler(Some(ServerSentEventHandler::new_with_capacity(1))) - .execution_layer(Some(mock.el)) - .ordered_custody_column_indices(all_custody_columns) - .rng(Box::new(StdRng::seed_from_u64(42))) - .build(); - - assert!( - beacon_chain.is_ok(), - "Beacon Chain failed to build. The anchor payload may have been incorrectly pruned. Error: {:?}", - beacon_chain.err() - ); - - let chain = beacon_chain.as_ref().unwrap(); - let wss_block_slot = wss_block.slot(); - - assert_ne!( - wss_block_slot, - chain.head_snapshot().beacon_state.slot(), - "Test invalid: Checkpoint was aligned (Slot {} == Slot {}). The test did not trigger the unaligned edge case.", - wss_block_slot, - chain.head_snapshot().beacon_state.slot() - ); +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 payload_exists = chain - .store - .execution_payload_exists(&wss_block_root) - .unwrap_or(false); + 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; - assert!( - payload_exists, - "Split block payload must exist in the new node's store after checkpoint sync" - ); + // 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; } -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 +2935,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 +2953,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 +2977,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); @@ -3044,10 +3031,10 @@ async fn weak_subjectivity_sync_test( .weak_subjectivity_state( wss_state, wss_block.clone(), - if provide_blobs { - wss_blobs_opt.clone() - } else { + if without_blobs { None + } else { + wss_blobs_opt.clone() }, genesis_state, ) @@ -3081,47 +3068,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). @@ -3159,6 +3106,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) @@ -3356,6 +3304,215 @@ 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); + } +} + +// Ensures that an unaligned checkpoint sync (the block is older than the state) +// works correctly even when `prune_payloads` is enabled. +// +// Previously, the `HotColdDB` would refuse to load the execution payload for the +// anchor block because it was considered "pruned", causing the node to fail startup. +#[tokio::test] +async fn reproduction_unaligned_checkpoint_sync_pruned_payload() { + let spec = test_spec::(); + + // Requires Execution Payloads. + let Some(_) = spec.deneb_fork_epoch else { + return; + }; + + // Create an unaligned checkpoint with a gap of 3 slots. + let num_initial_slots = E::slots_per_epoch() * 11; + let checkpoint_slot = Slot::new(E::slots_per_epoch() * 9 - 3); + + let slots = (1..num_initial_slots) + .map(Slot::new) + .filter(|&slot| slot <= checkpoint_slot || slot > checkpoint_slot + 3) + .collect::>(); + + let temp1 = tempdir().unwrap(); + let full_store = get_store_generic(&temp1, StoreConfig::default(), spec.clone()); + + let harness = get_harness_import_all_data_columns(full_store.clone(), LOW_VALIDATOR_COUNT); + let all_validators = (0..LOW_VALIDATOR_COUNT).collect::>(); + + 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, + &all_validators, + ) + .await; + + // Extract snapshot data from the harness. + let wss_block_root = harness + .chain + .block_root_at_slot(checkpoint_slot, WhenSlotSkipped::Prev) + .unwrap() + .unwrap(); + let wss_state_root = harness + .chain + .state_root_at_slot(checkpoint_slot) + .unwrap() + .unwrap(); + + let wss_block = harness + .chain + .store + .get_full_block(&wss_block_root) + .unwrap() + .unwrap(); + + // The test premise requires the anchor block to have a payload. + assert!(wss_block.message().execution_payload().is_ok()); + + let wss_blobs_opt = harness + .chain + .get_or_reconstruct_blobs(&wss_block_root) + .unwrap(); + + let wss_state = full_store + .get_state(&wss_state_root, Some(checkpoint_slot), CACHE_STATE_IN_TESTS) + .unwrap() + .unwrap(); + + // Configure the client with `prune_payloads = true`. + // This triggers the path where `try_get_full_block` must explicitly handle the anchor block. + let temp2 = tempdir().unwrap(); + let store_config = StoreConfig { + prune_payloads: true, + ..StoreConfig::default() + }; + + let store = get_store_generic(&temp2, store_config, spec.clone()); + + let slot_clock = TestingSlotClock::new( + Slot::new(0), + Duration::from_secs(harness.chain.genesis_time), + Duration::from_secs(spec.seconds_per_slot), + ); + slot_clock.set_slot(harness.get_current_slot().as_u64()); + + let chain_config = ChainConfig { + reconstruct_historic_states: true, + ..ChainConfig::default() + }; + + let trusted_setup = get_kzg(&spec); + let (shutdown_tx, _shutdown_rx) = futures::channel::mpsc::channel(1); + let mock = mock_execution_layer_from_parts( + harness.spec.clone(), + harness.runtime.task_executor.clone(), + ); + let all_custody_columns = (0..spec.number_of_custody_groups).collect::>(); + + // Attempt to build the BeaconChain. + // If the bug is present, this will panic with `MissingFullBlockExecutionPayloadPruned`. + let beacon_chain = BeaconChainBuilder::>::new(MinimalEthSpec, trusted_setup) + .chain_config(chain_config) + .store(store.clone()) + .custom_spec(spec.clone().into()) + .task_executor(harness.chain.task_executor.clone()) + .weak_subjectivity_state( + wss_state, + wss_block.clone(), + wss_blobs_opt.clone(), + genesis_state, + ) + .unwrap() + .store_migrator_config(MigratorConfig::default().blocking()) + .slot_clock(slot_clock) + .shutdown_sender(shutdown_tx) + .event_handler(Some(ServerSentEventHandler::new_with_capacity(1))) + .execution_layer(Some(mock.el)) + .ordered_custody_column_indices(all_custody_columns) + .rng(Box::new(StdRng::seed_from_u64(42))) + .build(); + + assert!( + beacon_chain.is_ok(), + "Beacon Chain failed to build. The anchor payload may have been incorrectly pruned. Error: {:?}", + beacon_chain.err() + ); + + let chain = beacon_chain.as_ref().unwrap(); + let wss_block_slot = wss_block.slot(); + + assert_ne!( + wss_block_slot, + chain.head_snapshot().beacon_state.slot(), + "Test invalid: Checkpoint was aligned (Slot {} == Slot {}). The test did not trigger the unaligned edge case.", + wss_block_slot, + chain.head_snapshot().beacon_state.slot() + ); + + let payload_exists = chain + .store + .execution_payload_exists(&wss_block_root) + .unwrap_or(false); + + assert!( + payload_exists, + "Split block payload must exist in the new node's store after checkpoint sync" + ); } // 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 17d9c5f697f..00ec553a05e 100644 --- a/beacon_node/beacon_chain/tests/tests.rs +++ b/beacon_node/beacon_chain/tests/tests.rs @@ -956,10 +956,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; @@ -974,11 +971,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/src/builder.rs b/beacon_node/client/src/builder.rs index c48021e45d4..3fa0af27145 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -20,10 +20,7 @@ use beacon_chain::{Kzg, LightClientProducerEvent}; use beacon_processor::{BeaconProcessor, BeaconProcessorChannels}; use beacon_processor::{BeaconProcessorConfig, BeaconProcessorQueueLengths}; use environment::RuntimeContext; -use eth2::{ - BeaconNodeHttpClient, Error as ApiError, Timeouts, - types::{BlockId, StateId}, -}; +use eth2::{BeaconNodeHttpClient, Error as ApiError, Timeouts, types::BlockId}; use execution_layer::ExecutionLayer; use execution_layer::test_utils::generate_genesis_header; use futures::channel::mpsc::Receiver; @@ -375,7 +372,7 @@ where genesis_state, )? } - ClientGenesis::CheckpointSyncUrl { url } => { + ClientGenesis::CheckpointSyncUrl { url, state_id } => { info!( remote_url = %url, "Starting checkpoint sync" @@ -391,20 +388,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 +409,25 @@ 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}"); // `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!( 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 52a3b92cb60..db5576c4bf8 100644 --- a/beacon_node/client/src/notifier.rs +++ b/beacon_node/client/src/notifier.rs @@ -227,8 +227,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, @@ -397,8 +396,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, @@ -408,8 +406,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 ea8b47f91ef..c622cead005 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 58cd2a3bdbc..1edd21635b1 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -2088,8 +2088,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, }) }) @@ -2555,20 +2555,21 @@ 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!( - "Failed to finalize state due to error: {e:?}" - )) - }) + chain.manual_finalization(checkpoint).await.map_err(|e| { + warp_utils::reject::custom_bad_request(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 ed7abead18a..1ee2a5abe0f 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 ac24b648e05..62e0cce967e 100644 --- a/beacon_node/network/src/network_beacon_processor/rpc_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/rpc_methods.rs @@ -108,11 +108,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. @@ -868,11 +864,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 bb2c6799f1d..e538da12e7e 100644 --- a/beacon_node/network/src/sync/custody_backfill_sync/mod.rs +++ b/beacon_node/network/src/sync/custody_backfill_sync/mod.rs @@ -183,6 +183,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 338f21ce987..d062979a165 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..a234e3bb6b9 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,8 +393,8 @@ 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 @@ -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 e4c7c6ff1fe..e23de4ad64e 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 26dd3b6642e..f4eec4f7b12 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, multiaddr::Protocol}; @@ -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/book/src/help_bn.md b/book/src/help_bn.md index 5f3c43a7e42..2df5e317ce5 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 b1a61ce00cc..a777724905a 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, @@ -1495,8 +1495,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 9a8cae0c365..0cd8c6a40a9 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}; @@ -62,12 +62,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, @@ -77,6 +73,7 @@ pub enum Error { }, UnrealizedVoteProcessing(state_processing::EpochProcessingError), ValidatorStatuses(BeaconStateError), + BadIrreversibleCheckpoint(String), } impl From for Error { @@ -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,8 +391,9 @@ where }); } - let finalized_block_slot = anchor_block.slot(); - let finalized_block_state_root = anchor_block.state_root(); + 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)?; @@ -388,10 +422,12 @@ where 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 +437,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 +446,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 +509,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 +519,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 +594,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 +618,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 +691,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 +709,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, @@ -909,8 +899,8 @@ 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(()) @@ -924,7 +914,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) @@ -932,7 +922,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); } @@ -1265,33 +1255,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 { @@ -1316,7 +1285,7 @@ where Ok(status.is_optimistic_or_invalid()) } else { Ok(self - .get_finalized_block()? + .local_irreversible_block()? .execution_status .is_optimistic_or_invalid()) } @@ -1338,14 +1307,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 { @@ -1356,6 +1332,35 @@ where *self.fc_store.unrealized_finalized_checkpoint() } + /// TODO: Document + pub fn set_local_irreversible_checkpoint( + &mut self, + checkpoint: Checkpoint, + ) -> Result<(), Error> { + // 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 + ))); + } + } 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)`. @@ -1396,7 +1401,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) @@ -1474,13 +1479,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, }; @@ -1511,9 +1509,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..511c1e1d475 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() { 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 207324ea33f..a97742edeff 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 8e9d438a243..1074512e019 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -789,13 +789,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); @@ -806,13 +807,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); @@ -824,13 +826,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);