diff --git a/beacon_node/beacon_chain/src/proposer_preferences_verification/gossip_verified_proposer_preferences.rs b/beacon_node/beacon_chain/src/proposer_preferences_verification/gossip_verified_proposer_preferences.rs index e8a2a92edee..f8578c54ba5 100644 --- a/beacon_node/beacon_chain/src/proposer_preferences_verification/gossip_verified_proposer_preferences.rs +++ b/beacon_node/beacon_chain/src/proposer_preferences_verification/gossip_verified_proposer_preferences.rs @@ -86,8 +86,42 @@ impl GossipVerifiedProposerPreferences { ctx.spec, )?; - // Resolve the proposer for `proposal_slot` via the proposer shuffling cache. let proposal_epoch = proposal_slot.epoch(T::EthSpec::slots_per_epoch()); + + let lookahead_epoch = proposal_epoch.saturating_sub(ctx.spec.min_seed_lookahead); + let head_block_root = ctx.canonical_head.cached_head().head_block_root(); + let fork_choice_read = ctx.canonical_head.fork_choice_read_lock(); + + let dependent_block = fork_choice_read + .get_block(&dependent_root) + .ok_or(ProposerPreferencesError::DependentRootUnknown { dependent_root })?; + + // Ensure the dependent root block is before the start of the lookahead epoch. + // Skip this check for gloas at genesis/epoch 1. + if dependent_block.slot >= lookahead_epoch.start_slot(T::EthSpec::slots_per_epoch()) + && dependent_block.slot != 0 + { + return Err(ProposerPreferencesError::DependentRootTooRecent { + dependent_root, + block_slot: dependent_block.slot, + epoch_start_slot: lookahead_epoch.start_slot(T::EthSpec::slots_per_epoch()), + }); + } + + // Look for at least one child that crosses the epoch boundary directly after `dependent_block`. + let has_qualifying_child = fork_choice_read + .get_children(&dependent_root) + .iter() + .any(|child| child.slot >= lookahead_epoch.start_slot(T::EthSpec::slots_per_epoch())); + + // Head is exempt from this check, it may eventually have a child that crosses the epoch boundary. + if !has_qualifying_child && dependent_root != head_block_root { + return Err(ProposerPreferencesError::InvalidDependentRoot { dependent_root }); + } + + drop(fork_choice_read); + + // Resolve the proposer for `proposal_slot` via the proposer shuffling cache. let proposer = with_proposer_cache( ctx.beacon_proposer_cache, dependent_root, diff --git a/beacon_node/beacon_chain/src/proposer_preferences_verification/mod.rs b/beacon_node/beacon_chain/src/proposer_preferences_verification/mod.rs index cd05853ae91..53fd94a9271 100644 --- a/beacon_node/beacon_chain/src/proposer_preferences_verification/mod.rs +++ b/beacon_node/beacon_chain/src/proposer_preferences_verification/mod.rs @@ -47,6 +47,18 @@ pub enum ProposerPreferencesError { }, /// The signature is invalid. BadSignature, + + /// The block with root `dependent_root` is not before the start of the lookahead epoch. + DependentRootTooRecent { + dependent_root: Hash256, + block_slot: Slot, + epoch_start_slot: Slot, + }, + + /// The block with root `dependent_root` is not a possible dependent block + /// for the given epoch. + InvalidDependentRoot { dependent_root: Hash256 }, + /// Some Beacon Chain Error BeaconChainError(Arc), /// Some Beacon State error diff --git a/beacon_node/beacon_chain/src/proposer_preferences_verification/tests.rs b/beacon_node/beacon_chain/src/proposer_preferences_verification/tests.rs index 9c61d84a705..7d2579aa3fe 100644 --- a/beacon_node/beacon_chain/src/proposer_preferences_verification/tests.rs +++ b/beacon_node/beacon_chain/src/proposer_preferences_verification/tests.rs @@ -7,15 +7,16 @@ use genesis::{generate_deterministic_keypairs, interop_genesis_state}; use parking_lot::{Mutex, RwLock}; use proto_array::PayloadStatus; use slot_clock::{SlotClock, TestingSlotClock}; +use state_processing::AllCaches; use store::{HotColdDB, MemoryStore, StoreConfig}; use types::{ - Address, BeaconBlock, ChainSpec, Checkpoint, Epoch, EthSpec, Hash256, MinimalEthSpec, - ProposerPreferences, SignedBeaconBlock, SignedProposerPreferences, Slot, + Address, BeaconBlock, ChainSpec, EthSpec, Hash256, MinimalEthSpec, ProposerPreferences, + SignedBeaconBlock, SignedProposerPreferences, Slot, }; use crate::{ beacon_fork_choice_store::BeaconForkChoiceStore, - beacon_proposer_cache::BeaconProposerCache, + beacon_proposer_cache::{BeaconProposerCache, ensure_state_can_determine_proposers_for_epoch}, beacon_snapshot::BeaconSnapshot, canonical_head::CanonicalHead, chain_config::FastConfirmationMode, @@ -61,11 +62,6 @@ impl TestContext { interop_genesis_state::(&keypairs, 0, Hash256::repeat_byte(0x42), None, &spec) .expect("should build genesis state"); - *state.finalized_checkpoint_mut() = Checkpoint { - epoch: Epoch::new(1), - root: Hash256::ZERO, - }; - let genesis_state_root = state .update_tree_hash_cache() .expect("should hash genesis state"); @@ -80,6 +76,9 @@ impl TestContext { let _ = store .init_anchor_info(Hash256::ZERO, Slot::new(0), Slot::new(0), false) .expect("should init anchor info"); + state + .build_all_caches(&spec) + .expect("should build state caches"); store .put_state(&genesis_state_root, &state) .expect("should persist genesis state"); @@ -147,19 +146,36 @@ impl TestContext { fn proposer_at_slot(&self, slot: Slot) -> u64 { let head = self.canonical_head.cached_head(); - let state = &head.snapshot.beacon_state; - let lookahead = state - .proposer_lookahead() - .expect("Gloas state has lookahead"); - let slot_in_epoch = slot.as_usize() % E::slots_per_epoch() as usize; + let mut state = head.snapshot.beacon_state.clone(); + let state_root = head.snapshot.beacon_block.message().state_root(); let epoch = slot.epoch(E::slots_per_epoch()); - let current_epoch = state.slot().epoch(E::slots_per_epoch()); - let index = if epoch == current_epoch.saturating_add(self.spec.min_seed_lookahead) { - E::slots_per_epoch() as usize + slot_in_epoch - } else { - slot_in_epoch - }; - *lookahead.get(index).expect("index in range") + state + .build_all_caches(&self.spec) + .expect("should build state caches"); + ensure_state_can_determine_proposers_for_epoch(&mut state, state_root, epoch, &self.spec) + .expect("should advance state to determine proposers"); + let proposers = state + .get_beacon_proposer_indices(epoch, &self.spec) + .expect("should compute proposer indices"); + let slot_in_epoch = slot.as_usize() % E::slots_per_epoch() as usize; + *proposers.get(slot_in_epoch).expect("slot within epoch") as u64 + } + + /// Insert a block into fork choice by cloning the parent's proto node with a new + /// root and slot. The block has no state in the store, so verification can only + /// proceed as far as the fork-choice-based checks. + fn add_block(&self, parent_root: Hash256, root: Hash256, slot: Slot) { + let mut fork_choice = self.canonical_head.fork_choice_write_lock(); + let mut block = fork_choice + .get_block(&parent_root) + .expect("parent block should be in fork choice"); + block.root = root; + block.parent_root = Some(parent_root); + block.slot = slot; + fork_choice + .proto_array_mut() + .process_block::(block, slot, &self.spec, Duration::ZERO) + .expect("should insert block into fork choice"); } } @@ -244,8 +260,11 @@ fn wrong_proposer_for_slot() { return; } let ctx = TestContext::new(); + // Advance to epoch 1 so the genesis head is a valid dependent root for epoch-2 + // proposals (it is strictly before the start of lookahead epoch 1). + ctx.slot_clock.set_slot(E::slots_per_epoch()); let gossip = ctx.gossip_ctx(); - let slot = Slot::new(1); + let slot = Slot::new(2 * E::slots_per_epoch()); let actual_proposer = ctx.proposer_at_slot(slot); let wrong_validator = if actual_proposer == 0 { 1 } else { 0 }; @@ -264,8 +283,11 @@ fn correct_proposer_bad_signature() { return; } let ctx = TestContext::new(); + // Clock and proposal both in epoch 2: the childless genesis head is only a + // valid dependent root via the head exemption. + ctx.slot_clock.set_slot(2 * E::slots_per_epoch()); let gossip = ctx.gossip_ctx(); - let slot = Slot::new(1); + let slot = Slot::new(2 * E::slots_per_epoch() + 1); let actual_proposer = ctx.proposer_at_slot(slot); let prefs = make_signed_preferences(slot, actual_proposer, ctx.head_block_root); @@ -291,8 +313,9 @@ fn validator_index_out_of_bounds() { return; } let ctx = TestContext::new(); + ctx.slot_clock.set_slot(E::slots_per_epoch()); let gossip = ctx.gossip_ctx(); - let slot = Slot::new(1); + let slot = Slot::new(2 * E::slots_per_epoch()); let prefs = make_signed_preferences(slot, u64::MAX, ctx.head_block_root); let result = GossipVerifiedProposerPreferences::new(prefs, &gossip); @@ -382,10 +405,11 @@ fn preferences_for_next_epoch_slot() { return; } let ctx = TestContext::new(); + // Clock in epoch 1, proposal in epoch 2 (the next epoch). + ctx.slot_clock.set_slot(E::slots_per_epoch()); let gossip = ctx.gossip_ctx(); - // Head is at slot 0 (epoch 0). Pick a slot in epoch 1. - let next_epoch_slot = Slot::new(E::slots_per_epoch() + 1); + let next_epoch_slot = Slot::new(2 * E::slots_per_epoch() + 1); let actual_proposer = ctx.proposer_at_slot(next_epoch_slot); let prefs = make_signed_preferences(next_epoch_slot, actual_proposer, ctx.head_block_root); @@ -397,3 +421,148 @@ fn preferences_for_next_epoch_slot() { result ); } + +/// For proposal epochs at or before the lookahead there is no block strictly before the +/// boundary; the genesis block is exempt so genesis-gloas networks can gossip preferences +/// during the first epochs. +#[test] +fn genesis_dependent_root_valid_for_early_epochs() { + if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { + return; + } + let ctx = TestContext::new(); + let gossip = ctx.gossip_ctx(); + let slot = Slot::new(1); + + let actual_proposer = ctx.proposer_at_slot(slot); + let prefs = make_signed_preferences(slot, actual_proposer, ctx.head_block_root); + let result = GossipVerifiedProposerPreferences::new(prefs, &gossip); + assert!( + matches!(result, Err(ProposerPreferencesError::BadSignature)), + "expected BadSignature for genesis dependent root, got: {:?}", + result + ); +} + +/// The genesis exemption only admits the block at slot 0: any other block is still too +/// recent for early proposal epochs. +#[test] +fn non_genesis_dependent_root_too_recent_for_early_epochs() { + if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { + return; + } + let ctx = TestContext::new(); + let gossip = ctx.gossip_ctx(); + + let recent_root = Hash256::repeat_byte(0xab); + ctx.add_block(ctx.head_block_root, recent_root, Slot::new(1)); + + let prefs = make_signed_preferences(Slot::new(2), 0, recent_root); + let result = GossipVerifiedProposerPreferences::new(prefs, &gossip); + assert!(matches!( + result, + Err(ProposerPreferencesError::DependentRootTooRecent { .. }) + )); +} + +#[test] +fn dependent_root_too_recent() { + if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { + return; + } + let ctx = TestContext::new(); + ctx.slot_clock.set_slot(E::slots_per_epoch()); + let gossip = ctx.gossip_ctx(); + + // A block after the start of lookahead epoch 1 cannot be a dependent root for + // epoch-2 proposals. + let recent_root = Hash256::repeat_byte(0xaa); + ctx.add_block( + ctx.head_block_root, + recent_root, + Slot::new(E::slots_per_epoch() + 4), + ); + + let proposal_slot = Slot::new(2 * E::slots_per_epoch() + 1); + let prefs = make_signed_preferences(proposal_slot, 0, recent_root); + let result = GossipVerifiedProposerPreferences::new(prefs, &gossip); + assert!(matches!( + result, + Err(ProposerPreferencesError::DependentRootTooRecent { .. }) + )); +} + +/// A known, old-enough block that has no children and is not the head is not a +/// plausible dependent root on any branch. +#[test] +fn dependent_root_childless_non_head() { + if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { + return; + } + let ctx = TestContext::new(); + ctx.slot_clock.set_slot(E::slots_per_epoch()); + let gossip = ctx.gossip_ctx(); + + let branch_root = Hash256::repeat_byte(0xbb); + ctx.add_block(ctx.head_block_root, branch_root, Slot::new(4)); + + let proposal_slot = Slot::new(2 * E::slots_per_epoch() + 1); + let prefs = make_signed_preferences(proposal_slot, 0, branch_root); + let result = GossipVerifiedProposerPreferences::new(prefs, &gossip); + assert!(matches!( + result, + Err(ProposerPreferencesError::InvalidDependentRoot { .. }) + )); +} + +/// A child that is itself before the lookahead boundary does not qualify: the block is +/// not the latest one before the boundary on that branch. +#[test] +fn dependent_root_child_before_boundary() { + if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { + return; + } + let ctx = TestContext::new(); + ctx.slot_clock.set_slot(E::slots_per_epoch()); + let gossip = ctx.gossip_ctx(); + + let parent_root = Hash256::repeat_byte(0xcc); + let child_root = Hash256::repeat_byte(0xdd); + ctx.add_block(ctx.head_block_root, parent_root, Slot::new(2)); + ctx.add_block(parent_root, child_root, Slot::new(4)); + + let proposal_slot = Slot::new(2 * E::slots_per_epoch() + 1); + let prefs = make_signed_preferences(proposal_slot, 0, parent_root); + let result = GossipVerifiedProposerPreferences::new(prefs, &gossip); + assert!(matches!( + result, + Err(ProposerPreferencesError::InvalidDependentRoot { .. }) + )); +} + +/// A non-head block with a child crossing the lookahead boundary passes dependent-root +/// validation. Verification still fails later (the fabricated block has no state in the +/// store), but not with a dependent-root error. +#[test] +fn dependent_root_valid_via_boundary_crossing_child() { + if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { + return; + } + let ctx = TestContext::new(); + ctx.slot_clock.set_slot(E::slots_per_epoch()); + let gossip = ctx.gossip_ctx(); + + let parent_root = Hash256::repeat_byte(0xee); + let child_root = Hash256::repeat_byte(0xef); + ctx.add_block(ctx.head_block_root, parent_root, Slot::new(4)); + ctx.add_block(parent_root, child_root, Slot::new(E::slots_per_epoch() + 4)); + + let proposal_slot = Slot::new(2 * E::slots_per_epoch() + 1); + let prefs = make_signed_preferences(proposal_slot, 0, parent_root); + let result = GossipVerifiedProposerPreferences::new(prefs, &gossip); + assert!( + matches!(&result, Err(ProposerPreferencesError::BeaconChainError(_))), + "expected verification to pass dependent-root checks and fail at proposer resolution, got: {:?}", + result + ); +} diff --git a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs index 9701aed5f76..3480cf80653 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -4275,13 +4275,15 @@ impl NetworkBeaconProcessor { | ProposerPreferencesError::BeaconChainError(_) | ProposerPreferencesError::BeaconStateError(_) | ProposerPreferencesError::UnableToReadSlot - | ProposerPreferencesError::DependentRootUnknown { .. }, + | ProposerPreferencesError::DependentRootUnknown { .. } + | ProposerPreferencesError::InvalidDependentRoot { .. }, ) => { self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore); } Err( ProposerPreferencesError::InvalidProposalSlot { .. } - | ProposerPreferencesError::BadSignature, + | ProposerPreferencesError::BadSignature + | ProposerPreferencesError::DependentRootTooRecent { .. }, ) => { self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Reject); self.gossip_penalize_peer( diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index ea9c7b1b32d..5556baca3a5 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1595,6 +1595,18 @@ where } } + /// Returns the blocks of the direct children of the block with `block_root`. + /// + /// Returns an empty vec if the block is unknown, has no children, or is not a + /// descendant of the finalized root. + pub fn get_children(&self, block_root: &Hash256) -> Vec { + if self.is_finalized_checkpoint_or_descendant(*block_root) { + self.proto_array.get_children(block_root) + } else { + Vec::new() + } + } + /// Returns whether the execution payload for a block has been received. /// /// Returns `false` for unknown blocks and pre-Gloas nodes. diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 7b2e1c150df..8c4f07c5aa2 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -951,12 +951,34 @@ impl ProtoArrayForkChoice { pub fn get_block(&self, block_root: &Hash256) -> Option { let block = self.get_proto_node(block_root)?; + Some(self.proto_node_to_block(block)) + } + + /// Returns the blocks of the direct children of the block with `block_root`. + /// + /// Returns an empty vec if the block is unknown or has no children. + pub fn get_children(&self, block_root: &Hash256) -> Vec { + self.proto_array + .indices + .get(block_root) + .and_then(|&index| self.proto_array.children.get(index)) + .map(|children| { + children + .iter() + .filter_map(|&child_index| self.proto_array.nodes.get(child_index)) + .map(|child| self.proto_node_to_block(child)) + .collect() + }) + .unwrap_or_default() + } + + fn proto_node_to_block(&self, block: &ProtoNode) -> Block { let parent_root = block .parent() .and_then(|i| self.proto_array.nodes.get(i)) .map(|parent| parent.root()); - Some(Block { + Block { slot: block.slot(), root: block.root(), parent_root, @@ -975,7 +997,7 @@ impl ProtoArrayForkChoice { execution_payload_block_hash: block.execution_payload_block_hash().ok(), proposer_index: block.proposer_index().ok(), payload_received: block.payload_received().unwrap_or(false), - }) + } } /// Called by the proposer to decide whether to build on the full or empty