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 95fde28f5b..8fc771aa7d 100644 --- a/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs +++ b/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs @@ -172,11 +172,13 @@ where 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 - .slot() - .as_u64() - .is_multiple_of(E::slots_per_epoch()) + // Pre-gloas the anchor state MUST be on an epoch boundary (it should be advanced by the caller). + // Post-gloas this requirement is relaxed. + if !anchor_state.fork_name_unchecked().gloas_enabled() + && !anchor_state + .slot() + .as_u64() + .is_multiple_of(E::slots_per_epoch()) { return Err(Error::UnalignedCheckpoint { block_slot: anchor_block_header.slot, diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index f848b48f05..5920243c50 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -42,7 +42,7 @@ use store::{Error as StoreError, HotColdDB, ItemStore, KeyValueStoreOp}; use task_executor::{ShutdownReason, TaskExecutor}; use tracing::{debug, error, info, warn}; use tree_hash::TreeHash; -use types::SignedExecutionPayloadEnvelope; +use types::StatePayloadStatus; use types::data::CustodyIndex; use types::{ BeaconBlock, BeaconState, BlobSidecarList, ChainSpec, ColumnIndex, DataColumnSidecarList, @@ -427,7 +427,6 @@ where mut weak_subj_state: BeaconState, weak_subj_block: SignedBeaconBlock, weak_subj_blobs: Option>, - weak_subj_payload: Option>, genesis_state: BeaconState, ) -> Result { let store = self @@ -435,9 +434,15 @@ where .clone() .ok_or("weak_subjectivity_state requires a store")?; - // Ensure the state is advanced to an epoch boundary. + // Pre-gloas ensure the state is advanced to an epoch boundary. + // Post-gloas checkpoint states are always pending (post-block) and cannot + // be advanced across epoch boundaries without first checking for a payload + // envelope. let slots_per_epoch = E::slots_per_epoch(); - if weak_subj_state.slot() % slots_per_epoch != 0 { + + if !weak_subj_state.fork_name_unchecked().gloas_enabled() + && weak_subj_state.slot() % slots_per_epoch != 0 + { debug!( state_slot = %weak_subj_state.slot(), block_slot = %weak_subj_block.slot(), @@ -570,7 +575,7 @@ where // Write the state, block and blobs non-atomically, it doesn't matter if they're forgotten // about on a crash restart. store - .update_finalized_state( + .set_initial_finalized_state( weak_subj_state_root, weak_subj_block_root, weak_subj_state.clone(), @@ -603,13 +608,6 @@ where .map_err(|e| format!("Failed to store weak subjectivity blobs: {e:?}"))?; } } - if let Some(ref envelope) = weak_subj_payload { - store - .put_payload_envelope(&weak_subj_block_root, envelope.clone()) - .map_err(|e| { - format!("Failed to store weak subjectivity payload envelope: {e:?}") - })?; - } // Stage the database's metadata fields for atomic storage when `build` is called. // This prevents the database from restarting in an inconsistent state if the anchor @@ -626,25 +624,18 @@ where .map_err(|e| format!("Failed to initialize data column info: {:?}", e))?, ); - if self - .spec - .fork_name_at_slot::(weak_subj_slot) - .gloas_enabled() + if weak_subj_state.fork_name_unchecked().gloas_enabled() + && weak_subj_state.payload_status() != StatePayloadStatus::Pending { - let envelope = weak_subj_payload.as_ref().ok_or_else(|| { - "Gloas checkpoint sync requires an execution payload envelope".to_string() - })?; - if envelope.message.beacon_block_root != weak_subj_block_root { - return Err(format!( - "Envelope beacon_block_root {:?} does not match block root {:?}", - envelope.message.beacon_block_root, weak_subj_block_root - )); - } + return Err(format!( + "Checkpoint sync state must be Pending (post-block) for Gloas, got {:?}", + weak_subj_state.payload_status() + )); } - // TODO(gloas): add check that checkpoint state is Pending + let snapshot = BeaconSnapshot { beacon_block_root: weak_subj_block_root, - execution_envelope: weak_subj_payload.map(Arc::new), + execution_envelope: None, beacon_block: Arc::new(weak_subj_block), beacon_state: weak_subj_state, }; diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index a0b0b77ec4..b98a3f4cff 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -2938,7 +2938,6 @@ async fn reproduction_unaligned_checkpoint_sync_pruned_payload() { wss_state, wss_block.clone(), wss_blobs_opt.clone(), - None, genesis_state, ) .unwrap() @@ -3089,7 +3088,6 @@ async fn weak_subjectivity_sync_test( } else { None }, - None, genesis_state, ) .unwrap() @@ -5467,6 +5465,150 @@ fn check_finalization(harness: &TestHarness, expected_slot: u64) { ); } +// Verify that post-gloas checkpoint sync accepts a non-epoch aligned state and builds +// the chain. +// +// Since post-gloas checkpoint sync states are always the post block state, if the epoch boundary +// slot is skipped, we'll receive a checkpoint state that is not epoch aligned. +// +// Example: slot `n` is the epoch boundary slot and is skipped. We'll receive the post block state for +// slot `n - 1`. This is the state before the payload for slot `n - 1` was processed. +#[tokio::test] +async fn weak_subjectivity_sync_gloas_pending_non_aligned() { + if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { + return; + } + + let spec = test_spec::(); + + // Build a chain with a skipped slot at the epoch boundary. + let epoch_boundary_slot = E::slots_per_epoch(); + let num_initial_slots = E::slots_per_epoch() * 4; + let checkpoint_slot = Slot::new(epoch_boundary_slot); + + let slots = (1..num_initial_slots) + .map(Slot::new) + .filter(|&slot| { + // Skip the epoch boundary slot + slot.as_u64() != epoch_boundary_slot + }) + .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 the checkpoint block and its Pending state. + let wss_block_root = harness + .chain + .block_root_at_slot(checkpoint_slot, WhenSlotSkipped::Prev) + .unwrap() + .unwrap(); + let wss_block = harness + .chain + .store + .get_full_block(&wss_block_root) + .unwrap() + .unwrap(); + + // The block's state_root points to the Pending state in Gloas. + let wss_state_root = wss_block.state_root(); + let wss_state = full_store + .get_state( + &wss_state_root, + Some(wss_block.slot()), + CACHE_STATE_IN_TESTS, + ) + .unwrap() + .unwrap(); + + assert_eq!( + wss_state.payload_status(), + StatePayloadStatus::Pending, + "Checkpoint state should be Pending" + ); + assert_ne!( + wss_state.slot() % E::slots_per_epoch(), + 0, + "Checkpoint state is epoch-aligned, expected non-aligned" + ); + + let wss_blobs_opt = harness + .chain + .get_or_reconstruct_blobs(&wss_block_root) + .unwrap(); + + // Build a new chain from the non-aligned Pending checkpoint state. + let temp2 = tempdir().unwrap(); + let store = get_store_generic(&temp2, StoreConfig::default(), spec.clone()); + + let slot_clock = TestingSlotClock::new( + Slot::new(0), + Duration::from_secs(harness.chain.genesis_time), + spec.get_slot_duration(), + ); + slot_clock.set_slot(harness.get_current_slot().as_u64()); + + let chain_config = ChainConfig { + archive: 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 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, wss_blobs_opt, 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(generate_data_column_indices_rand_order::()) + .rng(Box::new(StdRng::seed_from_u64(42))) + .build(); + + assert!( + beacon_chain.is_ok(), + "Beacon chain should build from non-aligned Gloas Pending checkpoint state. Error: {:?}", + beacon_chain.err() + ); + + let chain = beacon_chain.unwrap(); + + // The head state should be at the block's slot + assert_eq!( + chain.head_snapshot().beacon_state.slot(), + Slot::new(epoch_boundary_slot - 1), + "Head state should be at the checkpoint block's slot" + ); + assert_eq!( + chain.head_snapshot().beacon_state.payload_status(), + StatePayloadStatus::Pending, + "Head state should be Pending after checkpoint sync" + ); +} + // ===================== Gloas Store Tests ===================== /// Test basic Gloas block + envelope storage and retrieval. diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index f4c8689b1e..865599b9bd 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -372,7 +372,6 @@ where anchor_state, anchor_block, anchor_blobs, - None, genesis_state, )? } @@ -446,21 +445,6 @@ where None }; - let envelope = if spec - .fork_name_at_slot::(finalized_block_slot) - .gloas_enabled() - { - debug!("Downloading payload"); - remote - .get_beacon_execution_payload_envelope(BlockId::Slot(finalized_block_slot)) - .await - .map_err(|e| format!("Error fetching finalized blobs from remote: {e:?}"))? - .map(|resp| resp.into_data()) - } else { - None - }; - debug!("Downloaded finalized payload"); - let genesis_state = genesis_state(&runtime_context, &config).await?; info!( @@ -470,7 +454,7 @@ where "Loaded checkpoint block and state" ); - builder.weak_subjectivity_state(state, block, blobs, envelope, genesis_state)? + builder.weak_subjectivity_state(state, block, blobs, genesis_state)? } ClientGenesis::DepositContract => { return Err("Loading genesis from deposit contract no longer supported".to_string()); diff --git a/beacon_node/store/src/errors.rs b/beacon_node/store/src/errors.rs index a07cc83886..e403df483a 100644 --- a/beacon_node/store/src/errors.rs +++ b/beacon_node/store/src/errors.rs @@ -101,6 +101,7 @@ pub enum Error { from_state_slot: Slot, target_slot: Slot, }, + FinalizedStateAlreadySet, } pub trait HandleUnavailable { diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index 9a99b56348..e0fd0c48f8 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -474,6 +474,25 @@ impl, Cold: ItemStore> HotColdDB } } + /// See [`StateCache::set_initial_finalized_state`](crate::state_cache::StateCache::set_initial_finalized_state). + pub fn set_initial_finalized_state( + &self, + state_root: Hash256, + block_root: Hash256, + state: BeaconState, + ) -> Result<(), Error> { + let start_slot = self.get_anchor_info().anchor_slot; + let pre_finalized_slots_to_retain = self + .hierarchy + .closest_layer_points(state.slot(), start_slot); + self.state_cache.lock().set_initial_finalized_state( + state_root, + block_root, + state, + &pre_finalized_slots_to_retain, + ) + } + pub fn update_finalized_state( &self, state_root: Hash256, diff --git a/beacon_node/store/src/state_cache.rs b/beacon_node/store/src/state_cache.rs index d016922ade..288b0a7d69 100644 --- a/beacon_node/store/src/state_cache.rs +++ b/beacon_node/store/src/state_cache.rs @@ -124,6 +124,34 @@ impl StateCache { roots } + /// Used by checkpoint sync to initialize the finalized state in the state cache. + /// + /// Post-gloas the checkpoint state may not be epoch-aligned, e.g when the epoch boundary + /// slot is skipped. Regular finalization updates should use `update_finalized_state`. + pub fn set_initial_finalized_state( + &mut self, + state_root: Hash256, + block_root: Hash256, + state: BeaconState, + pre_finalized_slots_to_retain: &[Slot], + ) -> Result<(), Error> { + if self.finalized_state.is_some() { + return Err(Error::FinalizedStateAlreadySet); + } + + if !state.fork_name_unchecked().gloas_enabled() && state.slot() % E::slots_per_epoch() != 0 + { + return Err(Error::FinalizedStateUnaligned); + } + + self.update_finalized_state_inner( + state_root, + block_root, + state, + pre_finalized_slots_to_retain, + ) + } + pub fn update_finalized_state( &mut self, state_root: Hash256, @@ -135,6 +163,21 @@ impl StateCache { return Err(Error::FinalizedStateUnaligned); } + self.update_finalized_state_inner( + state_root, + block_root, + state, + pre_finalized_slots_to_retain, + ) + } + + fn update_finalized_state_inner( + &mut self, + state_root: Hash256, + block_root: Hash256, + state: BeaconState, + pre_finalized_slots_to_retain: &[Slot], + ) -> Result<(), Error> { if self .finalized_state .as_ref() diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 92fd4c1faf..5f2c3fc861 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -396,8 +396,11 @@ where current_slot: Option, spec: &ChainSpec, ) -> Result> { - // Sanity check: the anchor must lie on an epoch boundary. - if anchor_state.slot() % E::slots_per_epoch() != 0 { + // Pre-gloas sanity check: the anchor must lie on an epoch boundary. + // Post-gloas we relax this requirement + if !anchor_state.fork_name_unchecked().gloas_enabled() + && anchor_state.slot() % E::slots_per_epoch() != 0 + { return Err(Error::InvalidAnchor { block_slot: anchor_block.slot(), state_slot: anchor_state.slot(),