Merge branch 'gloas-fix-proposer-pref-gossip-verification' into glamsterdam-devnet-6

beacon_node/beacon_chain/src/proposer_preferences_verification/gossip_verified_proposer_preferences.rs

@@ -1,7 +1,7 @@
 use std::sync::Arc;
 
 use crate::{
-    BeaconChain, BeaconChainTypes, CanonicalHead,
+    BeaconChain, BeaconChainTypes, BeaconStore, CanonicalHead,
     proposer_preferences_verification::{
         ProposerPreferencesError, proposer_preference_cache::GossipVerifiedProposerPreferenceCache,
     },
@@ -9,20 +9,17 @@ use crate::{
 use eth2::types::{EventKind, ForkVersionedResponse};
 use slot_clock::SlotClock;
 use state_processing::signature_sets::{get_pubkey_from_state, proposer_preferences_signature_set};
+use state_processing::state_advance::partial_state_advance;
 use tracing::debug;
-use types::{
+use types::{ChainSpec, EthSpec, ProposerPreferences, SignedProposerPreferences, Slot};
-    BeaconState, ChainSpec, EthSpec, ProposerPreferences, SignedProposerPreferences, Slot,
-};
 
 /// Verify that proposer preferences are consistent with the current chain state
 pub(crate) fn verify_preferences_consistency<E: EthSpec>(
     preferences: &ProposerPreferences,
     current_slot: Slot,
-    head_state: &BeaconState<E>,
     spec: &ChainSpec,
 ) -> Result<(), ProposerPreferencesError> {
     let proposal_slot = preferences.proposal_slot;
-    let validator_index = preferences.validator_index;
     let current_epoch = current_slot.epoch(E::slots_per_epoch());
     let proposal_epoch = proposal_slot.epoch(E::slots_per_epoch());
 
@@ -39,13 +36,6 @@ pub(crate) fn verify_preferences_consistency<E: EthSpec>(
         });
     }
 
-    if !head_state.is_valid_proposal_slot(preferences, spec)? {
-        return Err(ProposerPreferencesError::InvalidProposalSlot {
-            validator_index,
-            proposal_slot,
-        });
-    }
-
     Ok(())
 }
 
@@ -54,6 +44,7 @@ pub struct GossipVerificationContext<'a, T: BeaconChainTypes> {
     pub gossip_verified_proposer_preferences_cache: &'a GossipVerifiedProposerPreferenceCache,
     pub slot_clock: &'a T::SlotClock,
     pub spec: &'a ChainSpec,
+    pub store: &'a BeaconStore<T>,
 }
 
 /// A wrapper around `SignedProposerPreferences` that has been verified for gossip propagation.
@@ -75,7 +66,6 @@ impl GossipVerifiedProposerPreferences {
             .slot_clock
             .now()
             .ok_or(ProposerPreferencesError::UnableToReadSlot)?;
-        let head_state = &cached_head.snapshot.beacon_state;
 
         if ctx
             .gossip_verified_proposer_preferences_cache
@@ -87,17 +77,51 @@ impl GossipVerifiedProposerPreferences {
             });
         }
 
-        verify_preferences_consistency(
+        verify_preferences_consistency::<T::EthSpec>(
             &signed_preferences.message,
             current_slot,
-            head_state,
             ctx.spec,
         )?;
 
+        // Get the block at dependent_root from fork choice to verify canonicity and get state_root
+        let fork_choice = ctx.canonical_head.fork_choice_read_lock();
+        let dependent_block = fork_choice
+            .get_block(&dependent_root)
+            .ok_or(ProposerPreferencesError::DependentRootUnknown { dependent_root })?;
+        let head_root = cached_head.head_block_root();
+        if !fork_choice.is_descendant(dependent_root, head_root) {
+            return Err(ProposerPreferencesError::DependentRootNotCanonical { dependent_root });
+        }
+        let dependent_state_root = dependent_block.state_root;
+        drop(fork_choice);
+
+        // We need a state at `target_epoch` so we have the correct proposer lookahead.
+        let proposal_epoch = proposal_slot.epoch(T::EthSpec::slots_per_epoch());
+        let target_epoch = proposal_epoch.saturating_sub(ctx.spec.min_seed_lookahead);
+        let target_slot = target_epoch.start_slot(T::EthSpec::slots_per_epoch());
+
+        let (state_root, mut state) = ctx
+            .store
+            .get_advanced_hot_state(dependent_root, target_slot, dependent_state_root)
+            .map_err(crate::BeaconChainError::DBError)?
+            .ok_or(ProposerPreferencesError::DependentRootUnknown { dependent_root })?;
+
+        if state.current_epoch() < target_epoch {
+            partial_state_advance(&mut state, Some(state_root), target_slot, ctx.spec)
+                .map_err(crate::BeaconChainError::StateAdvanceError)?;
+        }
+
+        if !state.is_valid_proposal_slot(&signed_preferences.message, ctx.spec)? {
+            return Err(ProposerPreferencesError::InvalidProposalSlot {
+                validator_index,
+                proposal_slot,
+            });
+        }
+
         // Verify signature
         proposer_preferences_signature_set(
-            head_state,
+            &state,
-            |i| get_pubkey_from_state(head_state, i),
+            |i| get_pubkey_from_state(&state, i),
             &signed_preferences,
             ctx.spec,
         )
@@ -128,6 +152,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 .gossip_verified_proposer_preferences_cache,
             slot_clock: &self.slot_clock,
             spec: &self.spec,
+            store: &self.store,
         }
     }
 
@@ -176,10 +201,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
 
 #[cfg(test)]
 mod tests {
-    use types::{
+    use types::{Address, ChainSpec, EthSpec, Hash256, MinimalEthSpec, ProposerPreferences, Slot};
-        Address, BeaconState, ChainSpec, EthSpec, Hash256, MinimalEthSpec, ProposerPreferences,
-        Slot,
-    };
 
     use super::verify_preferences_consistency;
     use crate::proposer_preferences_verification::ProposerPreferencesError;
@@ -197,11 +219,6 @@ mod tests {
         }
     }
 
-    fn state() -> BeaconState<E> {
-        let spec = spec();
-        BeaconState::new(0, <_>::default(), &spec)
-    }
-
     fn spec() -> ChainSpec {
         test_spec::<E>()
     }
@@ -214,7 +231,7 @@ mod tests {
         let current_slot = Slot::new(2 * E::slots_per_epoch());
         let prefs = make_preferences(Slot::new(3), 0);
 
-        let result = verify_preferences_consistency::<E>(&prefs, current_slot, &state(), &spec());
+        let result = verify_preferences_consistency::<E>(&prefs, current_slot, &spec());
         assert!(matches!(
             result,
             Err(ProposerPreferencesError::InvalidProposalEpoch { .. })
@@ -229,7 +246,7 @@ mod tests {
         let current_slot = Slot::new(E::slots_per_epoch());
         let prefs = make_preferences(Slot::new(3 * E::slots_per_epoch() + 1), 0);
 
-        let result = verify_preferences_consistency::<E>(&prefs, current_slot, &state(), &spec());
+        let result = verify_preferences_consistency::<E>(&prefs, current_slot, &spec());
         assert!(matches!(
             result,
             Err(ProposerPreferencesError::InvalidProposalEpoch { .. })
@@ -244,7 +261,7 @@ mod tests {
         let current_slot = Slot::new(10);
         let prefs = make_preferences(Slot::new(9), 0);
 
-        let result = verify_preferences_consistency::<E>(&prefs, current_slot, &state(), &spec());
+        let result = verify_preferences_consistency::<E>(&prefs, current_slot, &spec());
         assert!(matches!(
             result,
             Err(ProposerPreferencesError::ProposalSlotAlreadyPassed { .. })
@@ -259,7 +276,7 @@ mod tests {
         let current_slot = Slot::new(10);
         let prefs = make_preferences(Slot::new(10), 0);
 
-        let result = verify_preferences_consistency::<E>(&prefs, current_slot, &state(), &spec());
+        let result = verify_preferences_consistency::<E>(&prefs, current_slot, &spec());
         assert!(matches!(
             result,
             Err(ProposerPreferencesError::ProposalSlotAlreadyPassed { .. })

beacon_node/beacon_chain/src/proposer_preferences_verification/mod.rs

@@ -12,7 +12,7 @@
 
 use std::sync::Arc;
 
-use types::{BeaconStateError, Epoch, Slot};
+use types::{BeaconStateError, Epoch, Hash256, Slot};
 
 use crate::BeaconChainError;
 
@@ -38,6 +38,10 @@ pub enum ProposerPreferencesError {
     },
     /// The slot clock cannot be read.
     UnableToReadSlot,
+    /// The block with root `dependent_root` has not been seen.
+    DependentRootUnknown { dependent_root: Hash256 },
+    /// The block with root `dependent_root` is not canonical.
+    DependentRootNotCanonical { dependent_root: Hash256 },
     /// A valid message from this validator for this slot has already been seen.
     AlreadySeen {
         validator_index: u64,

beacon_node/beacon_chain/src/proposer_preferences_verification/tests.rs

@@ -6,7 +6,7 @@ use fork_choice::ForkChoice;
 use genesis::{generate_deterministic_keypairs, interop_genesis_state};
 use proto_array::PayloadStatus;
 use slot_clock::{SlotClock, TestingSlotClock};
-use store::{HotColdDB, StoreConfig};
+use store::{HotColdDB, MemoryStore, StoreConfig};
 use types::{
     Address, BeaconBlock, ChainSpec, Checkpoint, Epoch, EthSpec, Hash256, MinimalEthSpec,
     ProposerPreferences, SignedBeaconBlock, SignedProposerPreferences, Slot,
@@ -36,6 +36,8 @@ struct TestContext {
     preferences_cache: GossipVerifiedProposerPreferenceCache,
     slot_clock: TestingSlotClock,
     spec: ChainSpec,
+    store: Arc<HotColdDB<E, MemoryStore, MemoryStore>>,
+    head_block_root: Hash256,
 }
 
 impl TestContext {
@@ -57,12 +59,27 @@ impl TestContext {
             root: Hash256::ZERO,
         };
 
-        let mut genesis_block = BeaconBlock::empty(&spec);
+        let genesis_state_root = state
-        *genesis_block.state_root_mut() = state
             .update_tree_hash_cache()
             .expect("should hash genesis state");
+
+        let mut anchor_header = state.latest_block_header().clone();
+        if anchor_header.state_root.is_zero() {
+            anchor_header.state_root = genesis_state_root;
+        }
+        let block_root = anchor_header.canonical_root();
+
+        // Build a signed block with the correct state root for the snapshot.
+        let mut genesis_block = BeaconBlock::empty(&spec);
+        *genesis_block.state_root_mut() = genesis_state_root;
         let signed_block = SignedBeaconBlock::from_block(genesis_block, Signature::empty());
-        let block_root = signed_block.canonical_root();
+
+        let _ = store
+            .init_anchor_info(Hash256::ZERO, Slot::new(0), Slot::new(0), false)
+            .expect("should init anchor info");
+        store
+            .put_state(&genesis_state_root, &state)
+            .expect("should persist genesis state");
 
         let snapshot = BeaconSnapshot::new(
             Arc::new(signed_block.clone()),
@@ -91,6 +108,8 @@ impl TestContext {
             preferences_cache: GossipVerifiedProposerPreferenceCache::default(),
             slot_clock,
             spec,
+            store,
+            head_block_root: block_root,
         }
     }
 
@@ -100,6 +119,7 @@ impl TestContext {
             gossip_verified_proposer_preferences_cache: &self.preferences_cache,
             slot_clock: &self.slot_clock,
             spec: &self.spec,
+            store: &self.store,
         }
     }
 
@@ -124,10 +144,11 @@ impl TestContext {
 fn make_signed_preferences(
     proposal_slot: Slot,
     validator_index: u64,
+    dependent_root: Hash256,
 ) -> Arc<SignedProposerPreferences> {
     Arc::new(SignedProposerPreferences {
         message: ProposerPreferences {
-            dependent_root: Hash256::ZERO,
+            dependent_root,
             proposal_slot,
             validator_index,
             fee_recipient: Address::ZERO,
@@ -147,11 +168,11 @@ fn already_seen_validator() {
     let slot = Slot::new(1);
 
     let verified = GossipVerifiedProposerPreferences {
-        signed_preferences: make_signed_preferences(slot, 42),
+        signed_preferences: make_signed_preferences(slot, 42, Hash256::ZERO),
     };
     ctx.preferences_cache.insert_seen_validator(&verified);
 
-    let prefs = make_signed_preferences(slot, 42);
+    let prefs = make_signed_preferences(slot, 42, Hash256::ZERO);
     let result = GossipVerifiedProposerPreferences::new(prefs, &gossip);
     assert!(matches!(
         result,
@@ -171,7 +192,7 @@ fn invalid_epoch_too_far_ahead() {
     let gossip = ctx.gossip_ctx();
 
     let far_slot = Slot::new(3 * E::slots_per_epoch());
-    let prefs = make_signed_preferences(far_slot, 0);
+    let prefs = make_signed_preferences(far_slot, 0, Hash256::ZERO);
     let result = GossipVerifiedProposerPreferences::new(prefs, &gossip);
     assert!(matches!(
         result,
@@ -187,7 +208,7 @@ fn proposal_slot_already_passed() {
     let ctx = TestContext::new();
     let gossip = ctx.gossip_ctx();
 
-    let prefs = make_signed_preferences(Slot::new(0), 0);
+    let prefs = make_signed_preferences(Slot::new(0), 0, Hash256::ZERO);
     let result = GossipVerifiedProposerPreferences::new(prefs, &gossip);
     assert!(matches!(
         result,
@@ -207,7 +228,7 @@ fn wrong_proposer_for_slot() {
     let actual_proposer = ctx.proposer_at_slot(slot);
     let wrong_validator = if actual_proposer == 0 { 1 } else { 0 };
 
-    let prefs = make_signed_preferences(slot, wrong_validator);
+    let prefs = make_signed_preferences(slot, wrong_validator, ctx.head_block_root);
     let result = GossipVerifiedProposerPreferences::new(prefs, &gossip);
     assert!(matches!(
         result,
@@ -225,7 +246,7 @@ fn correct_proposer_bad_signature() {
     let slot = Slot::new(1);
 
     let actual_proposer = ctx.proposer_at_slot(slot);
-    let prefs = make_signed_preferences(slot, actual_proposer);
+    let prefs = make_signed_preferences(slot, actual_proposer, ctx.head_block_root);
     let result = GossipVerifiedProposerPreferences::new(prefs, &gossip);
     assert!(matches!(
         result,
@@ -233,7 +254,7 @@ fn correct_proposer_bad_signature() {
     ));
     assert!(
         !ctx.preferences_cache
-            .get_seen_validator(&slot, Hash256::ZERO, actual_proposer)
+            .get_seen_validator(&slot, ctx.head_block_root, actual_proposer)
     );
     assert!(ctx.preferences_cache.get_preferences(&slot).is_none());
 }
@@ -247,7 +268,7 @@ fn validator_index_out_of_bounds() {
     let gossip = ctx.gossip_ctx();
     let slot = Slot::new(1);
 
-    let prefs = make_signed_preferences(slot, u64::MAX);
+    let prefs = make_signed_preferences(slot, u64::MAX, ctx.head_block_root);
     let result = GossipVerifiedProposerPreferences::new(prefs, &gossip);
     assert!(matches!(
         result,
@@ -290,6 +311,43 @@ fn same_validator_different_dependent_root_not_deduplicated() {
     );
 }
 
+#[test]
+fn dependent_root_unknown() {
+    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 unknown_root = Hash256::repeat_byte(0xff);
+    let prefs = make_signed_preferences(slot, 0, unknown_root);
+    let result = GossipVerifiedProposerPreferences::new(prefs, &gossip);
+    assert!(matches!(
+        result,
+        Err(ProposerPreferencesError::DependentRootUnknown { .. })
+    ));
+}
+
+#[test]
+fn invalid_epoch_too_old() {
+    if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) {
+        return;
+    }
+    let ctx = TestContext::new();
+    // Advance the clock so that epoch 0 slots are too old.
+    ctx.slot_clock.set_slot(3 * E::slots_per_epoch());
+    let gossip = ctx.gossip_ctx();
+
+    let old_slot = Slot::new(1);
+    let prefs = make_signed_preferences(old_slot, 0, Hash256::ZERO);
+    let result = GossipVerifiedProposerPreferences::new(prefs, &gossip);
+    assert!(matches!(
+        result,
+        Err(ProposerPreferencesError::InvalidProposalEpoch { .. })
+    ));
+}
+
 // TODO(gloas) add successful proposer preferences check once we have proposer preferences signing logic
 
 #[test]
@@ -304,7 +362,7 @@ fn preferences_for_next_epoch_slot() {
     let next_epoch_slot = Slot::new(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);
+    let prefs = make_signed_preferences(next_epoch_slot, actual_proposer, ctx.head_block_root);
     let result = GossipVerifiedProposerPreferences::new(prefs, &gossip);
     // Should pass consistency checks but fail on signature (empty sig).
     assert!(

beacon_node/http_api/tests/tests.rs

@@ -2925,7 +2925,7 @@ impl ApiTester {
             .expect("slot index should be in lookahead") as usize;
 
         let preferences = ProposerPreferences {
-            dependent_root: Hash256::ZERO,
+            dependent_root: head.beacon_block_root,
             proposal_slot,
             validator_index: validator_index as u64,
             fee_recipient: Address::repeat_byte(0xaa),

beacon_node/network/src/network_beacon_processor/gossip_methods.rs

@@ -4080,7 +4080,9 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
                 | ProposerPreferencesError::ProposalSlotAlreadyPassed { .. }
                 | ProposerPreferencesError::BeaconChainError(_)
                 | ProposerPreferencesError::BeaconStateError(_)
-                | ProposerPreferencesError::UnableToReadSlot,
+                | ProposerPreferencesError::UnableToReadSlot
+                | ProposerPreferencesError::DependentRootUnknown { .. }
+                | ProposerPreferencesError::DependentRootNotCanonical { .. },
             ) => {
                 self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore);
             }