Skip to content

Allow Checkpoint Sync on State Beyond Finalization #5128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions beacon_node/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ clap = { workspace = true }
slog = { workspace = true }
dirs = { workspace = true }
directory = { workspace = true }
eth2 = { workspace = true }
futures = { workspace = true }
environment = { workspace = true }
task_executor = { workspace = true }
Expand Down
8 changes: 4 additions & 4 deletions beacon_node/beacon_chain/src/beacon_fork_choice_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,9 +389,9 @@ pub struct PersistedForkChoiceStore {
pub equivocating_indices: BTreeSet<u64>,
}

impl Into<PersistedForkChoiceStore> for PersistedForkChoiceStoreV11 {
fn into(self) -> PersistedForkChoiceStore {
PersistedForkChoiceStore {
impl Into<PersistedForkChoiceStoreV17> for PersistedForkChoiceStoreV11 {
fn into(self) -> PersistedForkChoiceStoreV17 {
PersistedForkChoiceStoreV17 {
balances_cache: self.balances_cache,
time: self.time,
finalized_checkpoint: self.finalized_checkpoint,
Expand All @@ -405,7 +405,7 @@ impl Into<PersistedForkChoiceStore> for PersistedForkChoiceStoreV11 {
}
}

impl Into<PersistedForkChoiceStoreV11> for PersistedForkChoiceStore {
impl Into<PersistedForkChoiceStoreV11> for PersistedForkChoiceStoreV17 {
fn into(self) -> PersistedForkChoiceStoreV11 {
PersistedForkChoiceStoreV11 {
balances_cache: self.balances_cache,
Expand Down
7 changes: 6 additions & 1 deletion beacon_node/beacon_chain/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::{
};
use eth1::Config as Eth1Config;
use execution_layer::ExecutionLayer;
use fork_choice::{ForkChoice, ResetPayloadStatuses};
use fork_choice::{AnchorState, ForkChoice, ResetPayloadStatuses};
use futures::channel::mpsc::Sender;
use kzg::{Kzg, TrustedSetup};
use operation_pool::{OperationPool, PersistedOperationPool};
Expand Down Expand Up @@ -412,6 +412,7 @@ where
genesis.beacon_block_root,
&genesis.beacon_block,
&genesis.beacon_state,
AnchorState::Finalized,
current_slot,
&self.spec,
)
Expand All @@ -428,6 +429,7 @@ where
mut weak_subj_state: BeaconState<TEthSpec>,
weak_subj_block: SignedBeaconBlock<TEthSpec>,
genesis_state: BeaconState<TEthSpec>,
anchor_state: AnchorState,
) -> Result<Self, String> {
let store = self
.store
Expand Down Expand Up @@ -552,6 +554,7 @@ where
snapshot.beacon_block_root,
&snapshot.beacon_block,
&snapshot.beacon_state,
anchor_state,
Some(weak_subj_slot),
&self.spec,
)
Expand Down Expand Up @@ -689,6 +692,7 @@ where
None
};

let anchor_state = fork_choice.anchor_state();
let initial_head_block_root = fork_choice
.get_head(current_slot, &self.spec)
.map_err(|e| format!("Unable to get fork choice head: {:?}", e))?;
Expand Down Expand Up @@ -738,6 +742,7 @@ where
Some(current_slot),
&self.spec,
self.chain_config.progressive_balances_mode,
anchor_state,
&log,
)?;
}
Expand Down
22 changes: 20 additions & 2 deletions beacon_node/beacon_chain/src/canonical_head.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ use crate::{
};
use eth2::types::{EventKind, SseChainReorg, SseFinalizedCheckpoint, SseHead, SseLateHead};
use fork_choice::{
ExecutionStatus, ForkChoiceStore, ForkChoiceView, ForkchoiceUpdateParameters, ProtoBlock,
ResetPayloadStatuses,
AnchorState, ExecutionStatus, ForkChoiceStore, ForkChoiceView, ForkchoiceUpdateParameters,
ProtoBlock, ResetPayloadStatuses,
};
use itertools::process_results;
use parking_lot::{Mutex, RwLock, RwLockReadGuard, RwLockWriteGuard};
Expand Down Expand Up @@ -109,6 +109,8 @@ pub struct CachedHead<E: EthSpec> {
justified_hash: Option<ExecutionBlockHash>,
/// The `execution_payload.block_hash` of the finalized block. Set to `None` before Bellatrix.
finalized_hash: Option<ExecutionBlockHash>,
/// The state of the finalized checkpoint
anchor_state: AnchorState,
}

impl<E: EthSpec> CachedHead<E> {
Expand Down Expand Up @@ -226,6 +228,11 @@ impl<E: EthSpec> CachedHead<E> {
finalized_hash: self.finalized_hash,
}
}

/// Returns the anchor state
pub fn anchor_state(&self) -> AnchorState {
self.anchor_state
}
}

/// Represents the "canonical head" of the beacon chain.
Expand Down Expand Up @@ -266,6 +273,7 @@ impl<T: BeaconChainTypes> CanonicalHead<T> {
head_hash: forkchoice_update_params.head_hash,
justified_hash: forkchoice_update_params.justified_hash,
finalized_hash: forkchoice_update_params.finalized_hash,
anchor_state: fork_choice_view.anchor_state,
};

Self {
Expand Down Expand Up @@ -318,6 +326,7 @@ impl<T: BeaconChainTypes> CanonicalHead<T> {
head_hash: forkchoice_update_params.head_hash,
justified_hash: forkchoice_update_params.justified_hash,
finalized_hash: forkchoice_update_params.finalized_hash,
anchor_state: fork_choice_view.anchor_state,
};

*fork_choice_write_lock = fork_choice;
Expand Down Expand Up @@ -447,6 +456,12 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
self.canonical_head.cached_head_read_lock().snapshot.clone()
}

/// Returns the `AnchorState` of the finalized checkpoint
/// TODO: determine if this puts a burdon on the fork choice lock
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is used every time we add a peer who's finalized is lower than ours. Might not be a big deal in practice. But if it is, we could track "has our anchor state ever been set to AnchorState::Finalized" because it will never go from AnchorState::NonRevertible => AnchorState::Finalized. If we track that, we won't need to bother fork choice as soon as finalization advances at any point after startup

pub fn finalized_checkpoint_anchor_state(&self) -> AnchorState {
self.canonical_head.cached_head_read_lock().anchor_state()
}

/// Returns the beacon block at the head of the canonical chain.
///
/// See `Self::head` for more information.
Expand Down Expand Up @@ -576,6 +591,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
head_block_root: old_cached_head.head_block_root(),
justified_checkpoint: old_cached_head.justified_checkpoint(),
finalized_checkpoint: old_cached_head.finalized_checkpoint(),
anchor_state: old_cached_head.anchor_state(),
};

let mut fork_choice_write_lock = self.canonical_head.fork_choice_write_lock();
Expand Down Expand Up @@ -702,6 +718,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
head_hash: new_forkchoice_update_parameters.head_hash,
justified_hash: new_forkchoice_update_parameters.justified_hash,
finalized_hash: new_forkchoice_update_parameters.finalized_hash,
anchor_state: new_view.anchor_state,
};

let new_head = {
Expand Down Expand Up @@ -729,6 +746,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
head_hash: new_forkchoice_update_parameters.head_hash,
justified_hash: new_forkchoice_update_parameters.justified_hash,
finalized_hash: new_forkchoice_update_parameters.finalized_hash,
anchor_state: new_view.anchor_state,
};

let mut cached_head_write_lock = self.canonical_head.cached_head_write_lock();
Expand Down
5 changes: 4 additions & 1 deletion beacon_node/beacon_chain/src/fork_revert.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{BeaconForkChoiceStore, BeaconSnapshot};
use fork_choice::{ForkChoice, PayloadVerificationStatus};
use fork_choice::{AnchorState, ForkChoice, PayloadVerificationStatus};
use itertools::process_results;
use slog::{info, warn, Logger};
use state_processing::state_advance::complete_state_advance;
Expand Down Expand Up @@ -97,13 +97,15 @@ pub fn revert_to_fork_boundary<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>
/// WARNING: this function is destructive and causes fork choice to permanently forget all
/// chains other than the chain leading to `head_block_root`. It should only be used in extreme
/// circumstances when there is no better alternative.
#[allow(clippy::too_many_arguments)]
pub fn reset_fork_choice_to_finalization<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>>(
head_block_root: Hash256,
head_state: &BeaconState<E>,
store: Arc<HotColdDB<E, Hot, Cold>>,
current_slot: Option<Slot>,
spec: &ChainSpec,
progressive_balances_mode: ProgressiveBalancesMode,
anchor_state: AnchorState,
log: &Logger,
) -> Result<ForkChoice<BeaconForkChoiceStore<E, Hot, Cold>, E>, String> {
// Fetch finalized block.
Expand Down Expand Up @@ -157,6 +159,7 @@ pub fn reset_fork_choice_to_finalization<E: EthSpec, Hot: ItemStore<E>, Cold: It
finalized_block_root,
&finalized_snapshot.beacon_block,
&finalized_snapshot.beacon_state,
anchor_state,
current_slot,
spec,
)
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/beacon_chain/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ pub use eth1_chain::{Eth1Chain, Eth1ChainBackend};
pub use events::ServerSentEventHandler;
pub use execution_layer::EngineState;
pub use execution_payload::NotifyExecutionLayer;
pub use fork_choice::{ExecutionStatus, ForkchoiceUpdateParameters};
pub use fork_choice::{AnchorState, ExecutionStatus, ForkchoiceUpdateParameters};
pub use kzg::TrustedSetup;
pub use metrics::scrape_for_metrics;
pub use migrate::MigratorConfig;
Expand Down
38 changes: 30 additions & 8 deletions beacon_node/beacon_chain/src/persisted_fork_choice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,34 @@ use store::{DBColumn, Error, StoreItem};
use superstruct::superstruct;

// If adding a new version you should update this type alias and fix the breakages.
pub type PersistedForkChoice = PersistedForkChoiceV17;
pub type PersistedForkChoice = PersistedForkChoiceV20;

#[superstruct(
variants(V11, V17),
variants(V11, V17, V20),
variant_attributes(derive(Encode, Decode)),
no_enum
)]
pub struct PersistedForkChoice {
pub fork_choice: fork_choice::PersistedForkChoice,
#[superstruct(only(V11, V17))]
pub fork_choice: fork_choice::PersistedForkChoiceV19,
#[superstruct(only(V20))]
pub fork_choice: fork_choice::PersistedForkChoiceV20,
#[superstruct(only(V11))]
pub fork_choice_store: PersistedForkChoiceStoreV11,
#[superstruct(only(V17))]
#[superstruct(only(V17, V20))]
pub fork_choice_store: PersistedForkChoiceStoreV17,
}

impl Into<PersistedForkChoice> for PersistedForkChoiceV11 {
fn into(self) -> PersistedForkChoice {
PersistedForkChoice {
impl Into<PersistedForkChoiceV17> for PersistedForkChoiceV11 {
fn into(self) -> PersistedForkChoiceV17 {
PersistedForkChoiceV17 {
fork_choice: self.fork_choice,
fork_choice_store: self.fork_choice_store.into(),
}
}
}

impl Into<PersistedForkChoiceV11> for PersistedForkChoice {
impl Into<PersistedForkChoiceV11> for PersistedForkChoiceV17 {
fn into(self) -> PersistedForkChoiceV11 {
PersistedForkChoiceV11 {
fork_choice: self.fork_choice,
Expand All @@ -38,6 +41,24 @@ impl Into<PersistedForkChoiceV11> for PersistedForkChoice {
}
}

impl Into<PersistedForkChoiceV20> for PersistedForkChoiceV17 {
fn into(self) -> PersistedForkChoiceV20 {
PersistedForkChoiceV20 {
fork_choice: self.fork_choice.into(),
fork_choice_store: self.fork_choice_store,
}
}
}

impl Into<PersistedForkChoiceV17> for PersistedForkChoiceV20 {
fn into(self) -> PersistedForkChoiceV17 {
PersistedForkChoiceV17 {
fork_choice: self.fork_choice.into(),
fork_choice_store: self.fork_choice_store,
}
}
}

macro_rules! impl_store_item {
($type:ty) => {
impl StoreItem for $type {
Expand All @@ -58,3 +79,4 @@ macro_rules! impl_store_item {

impl_store_item!(PersistedForkChoiceV11);
impl_store_item!(PersistedForkChoiceV17);
impl_store_item!(PersistedForkChoiceV20);
9 changes: 9 additions & 0 deletions beacon_node/beacon_chain/src/schema_change.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
mod migration_schema_v17;
mod migration_schema_v18;
mod migration_schema_v19;
mod migration_schema_v20;

use crate::beacon_chain::BeaconChainTypes;
use crate::types::ChainSpec;
Expand Down Expand Up @@ -78,6 +79,14 @@ pub fn migrate_schema<T: BeaconChainTypes>(
let ops = migration_schema_v19::downgrade_from_v19::<T>(db.clone(), log)?;
db.store_schema_version_atomically(to, ops)
}
(SchemaVersion(19), SchemaVersion(20)) => {
let ops = migration_schema_v20::upgrade_to_v20::<T>(db.clone(), log)?;
db.store_schema_version_atomically(to, ops)
}
(SchemaVersion(20), SchemaVersion(19)) => {
let ops = migration_schema_v20::downgrade_from_v20::<T>(db.clone(), log)?;
db.store_schema_version_atomically(to, ops)
}
// Anything else is an error.
(_, _) => Err(HotColdDBError::UnsupportedSchemaVersion {
target_version: to,
Expand Down
35 changes: 35 additions & 0 deletions beacon_node/beacon_chain/src/schema_change/migration_schema_v20.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
use crate::beacon_chain::{BeaconChainTypes, FORK_CHOICE_DB_KEY};
use crate::persisted_fork_choice::{PersistedForkChoiceV17, PersistedForkChoiceV20};
use slog::{debug, Logger};
use std::sync::Arc;
use store::{Error, HotColdDB, KeyValueStoreOp, StoreItem};

pub fn upgrade_to_v20<T: BeaconChainTypes>(
db: Arc<HotColdDB<T::EthSpec, T::HotStore, T::ColdStore>>,
log: Logger,
) -> Result<Vec<KeyValueStoreOp>, Error> {
let v17 = db
.get_item::<PersistedForkChoiceV17>(&FORK_CHOICE_DB_KEY)?
.ok_or_else(|| Error::SchemaMigrationError("fork choice missing from database".into()))?;

let v20: PersistedForkChoiceV20 = v17.into();

debug!(log, "Adding anchor_state to fork choice");

Ok(vec![v20.as_kv_store_op(FORK_CHOICE_DB_KEY)])
}

pub fn downgrade_from_v20<T: BeaconChainTypes>(
db: Arc<HotColdDB<T::EthSpec, T::HotStore, T::ColdStore>>,
log: Logger,
) -> Result<Vec<KeyValueStoreOp>, Error> {
let v20 = db
.get_item::<PersistedForkChoiceV20>(&FORK_CHOICE_DB_KEY)?
.ok_or_else(|| Error::SchemaMigrationError("fork choice missing from database".into()))?;

let v17: PersistedForkChoiceV17 = v20.into();

debug!(log, "Dropping anchor_state from fork choice.");

Ok(vec![v17.as_kv_store_op(FORK_CHOICE_DB_KEY)])
}
12 changes: 9 additions & 3 deletions beacon_node/beacon_chain/tests/store_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ use beacon_chain::test_utils::{
};
use beacon_chain::{
data_availability_checker::MaybeAvailableBlock, historical_blocks::HistoricalBlockError,
migrate::MigratorConfig, BeaconChain, BeaconChainError, BeaconChainTypes, BeaconSnapshot,
BlockError, ChainConfig, NotifyExecutionLayer, ServerSentEventHandler, WhenSlotSkipped,
migrate::MigratorConfig, AnchorState, BeaconChain, BeaconChainError, BeaconChainTypes,
BeaconSnapshot, BlockError, ChainConfig, NotifyExecutionLayer, ServerSentEventHandler,
WhenSlotSkipped,
};
use eth2_network_config::TRUSTED_SETUP_BYTES;
use kzg::TrustedSetup;
Expand Down Expand Up @@ -2437,7 +2438,12 @@ async fn weak_subjectivity_sync_test(slots: Vec<Slot>, checkpoint_slot: Slot) {
.custom_spec(test_spec::<E>())
.task_executor(harness.chain.task_executor.clone())
.logger(log.clone())
.weak_subjectivity_state(wss_state, wss_block.clone(), genesis_state)
.weak_subjectivity_state(
wss_state,
wss_block.clone(),
genesis_state,
AnchorState::Finalized,
)
.unwrap()
.store_migrator_config(MigratorConfig::default().blocking())
.dummy_eth1_backend()
Expand Down
Loading