merge queue: checking unstable (3ea7c47) and #9760 together

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,

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<BeaconChainError>),
     /// Some Beacon State error

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::<E>(&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::<E>(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
+    );
+}

beacon_node/network/src/network_beacon_processor/gossip_methods.rs

@@ -4275,13 +4275,15 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
                 | 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(

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<ProtoBlock> {
+        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.