From f9620090da8e7dd381411c70d6593d445705adb7 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Sat, 4 Jul 2026 00:18:16 -0700 Subject: [PATCH] Gloas fix proposer preferences gossip verification (#9337) Ensure we are using the correct state when validating proposer preferences over gossip. Previously we were only using the head state. At epoch boundaries the head state could be at `current_epoch - 1`. Peers submitting proposer preferences for `current_epoch + 1` would be penalized because our head states lookahead did not have proposer duties for `current_epoch + 1` Co-Authored-By: Eitan Seri-Levi --- .../gossip_verified_bid.rs | 11 +- .../src/payload_bid_verification/tests.rs | 22 +++- .../gossip_verified_proposer_preferences.rs | 122 ++++++++++++------ .../proposer_preferences_verification/mod.rs | 4 +- .../proposer_preference_cache.rs | 47 ++++++- .../tests.rs | 106 ++++++++++++--- beacon_node/beacon_chain/tests/events.rs | 14 +- beacon_node/http_api/tests/tests.rs | 10 +- .../gossip_methods.rs | 3 +- .../types/src/builder/proposer_preferences.rs | 23 +++- consensus/types/src/state/beacon_state.rs | 39 +----- 11 files changed, 290 insertions(+), 111 deletions(-) diff --git a/beacon_node/beacon_chain/src/payload_bid_verification/gossip_verified_bid.rs b/beacon_node/beacon_chain/src/payload_bid_verification/gossip_verified_bid.rs index 2a0c49aff3..c88ec5813a 100644 --- a/beacon_node/beacon_chain/src/payload_bid_verification/gossip_verified_bid.rs +++ b/beacon_node/beacon_chain/src/payload_bid_verification/gossip_verified_bid.rs @@ -142,9 +142,18 @@ impl GossipVerifiedPayloadBid { .ok_or(PayloadBidError::UnableToReadSlot)?; let head_state = &cached_head.snapshot.beacon_state; + // Look up the preferences keyed by the dependent root that is canonical from our head's + // perspective, so we don't pick up preferences cached for a competing branch's proposer. + let proposal_epoch = bid_slot.epoch(T::EthSpec::slots_per_epoch()); + let dependent_root = head_state.proposer_shuffling_decision_root_at_epoch( + proposal_epoch, + cached_head.head_block_root(), + ctx.spec, + )?; + let Some(proposer_preferences) = ctx .gossip_verified_proposer_preferences_cache - .get_preferences(&bid_slot) + .get_preferences(&bid_slot, dependent_root) else { return Err(PayloadBidError::NoProposerPreferences { slot: bid_slot }); }; diff --git a/beacon_node/beacon_chain/src/payload_bid_verification/tests.rs b/beacon_node/beacon_chain/src/payload_bid_verification/tests.rs index cf132c2a97..d198b56d1b 100644 --- a/beacon_node/beacon_chain/src/payload_bid_verification/tests.rs +++ b/beacon_node/beacon_chain/src/payload_bid_verification/tests.rs @@ -276,10 +276,11 @@ fn make_signed_preferences( validator_index: u64, fee_recipient: Address, target_gas_limit: u64, + dependent_root: Hash256, ) -> Arc { Arc::new(SignedProposerPreferences { message: ProposerPreferences { - dependent_root: Hash256::ZERO, + dependent_root, proposal_slot, validator_index, fee_recipient, @@ -290,8 +291,25 @@ fn make_signed_preferences( } fn seed_preferences(ctx: &TestContext, slot: Slot, fee_recipient: Address, gas_limit: u64) { + // Key the preferences by the same dependent root that gossip verification will compute from + // the head state, otherwise the lookup misses and verification returns `NoProposerPreferences`. + let cached_head = ctx.canonical_head.cached_head(); + let head_state = &cached_head.snapshot.beacon_state; + let dependent_root = head_state + .proposer_shuffling_decision_root_at_epoch( + slot.epoch(E::slots_per_epoch()), + cached_head.head_block_root(), + &ctx.spec, + ) + .expect("should compute proposer shuffling decision root"); let prefs = GossipVerifiedProposerPreferences { - signed_preferences: make_signed_preferences(slot, 0, fee_recipient, gas_limit), + signed_preferences: make_signed_preferences( + slot, + 0, + fee_recipient, + gas_limit, + dependent_root, + ), }; ctx.preferences_cache.insert_preferences(prefs); } 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 4dc1646ec4..e8a2a92ede 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 @@ -1,28 +1,26 @@ use std::sync::Arc; use crate::{ - BeaconChain, BeaconChainTypes, CanonicalHead, + BeaconChain, BeaconChainError, BeaconChainTypes, BeaconStore, CanonicalHead, + beacon_proposer_cache::{BeaconProposerCache, with_proposer_cache}, proposer_preferences_verification::{ ProposerPreferencesError, proposer_preference_cache::GossipVerifiedProposerPreferenceCache, }, + validator_pubkey_cache::ValidatorPubkeyCache, }; use eth2::types::{EventKind, ForkVersionedResponse}; +use parking_lot::{Mutex, RwLock}; use slot_clock::SlotClock; -use state_processing::signature_sets::{get_pubkey_from_state, proposer_preferences_signature_set}; use tracing::debug; -use types::{ - BeaconState, ChainSpec, EthSpec, ProposerPreferences, SignedProposerPreferences, Slot, -}; +use types::{ChainSpec, EthSpec, Hash256, ProposerPreferences, SignedProposerPreferences, Slot}; /// Verify that proposer preferences are consistent with the current chain state pub(crate) fn verify_preferences_consistency( preferences: &ProposerPreferences, current_slot: Slot, - head_state: &BeaconState, 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 +37,6 @@ pub(crate) fn verify_preferences_consistency( }); } - if !head_state.is_valid_proposal_slot(preferences, spec)? { - return Err(ProposerPreferencesError::InvalidProposalSlot { - validator_index, - proposal_slot, - }); - } - Ok(()) } @@ -54,6 +45,10 @@ 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, + pub beacon_proposer_cache: &'a Mutex, + pub validator_pubkey_cache: &'a RwLock>, + pub genesis_validators_root: Hash256, } /// A wrapper around `SignedProposerPreferences` that has been verified for gossip propagation. @@ -70,12 +65,10 @@ impl GossipVerifiedProposerPreferences { let proposal_slot = signed_preferences.message.proposal_slot; let dependent_root = signed_preferences.message.dependent_root; let validator_index = signed_preferences.message.validator_index; - let cached_head = ctx.canonical_head.cached_head(); let current_slot = ctx .slot_clock .now() .ok_or(ProposerPreferencesError::UnableToReadSlot)?; - let head_state = &cached_head.snapshot.beacon_state; if ctx .gossip_verified_proposer_preferences_cache @@ -87,24 +80,75 @@ impl GossipVerifiedProposerPreferences { }); } - verify_preferences_consistency( + verify_preferences_consistency::( &signed_preferences.message, current_slot, - head_state, ctx.spec, )?; - // Verify signature - proposer_preferences_signature_set( - head_state, - |i| get_pubkey_from_state(head_state, i), - &signed_preferences, + // Resolve the proposer for `proposal_slot` via the proposer shuffling cache. + let proposal_epoch = proposal_slot.epoch(T::EthSpec::slots_per_epoch()); + let proposer = with_proposer_cache( + ctx.beacon_proposer_cache, + dependent_root, + proposal_epoch, + |proposers| proposers.get_slot::(proposal_slot), + || { + debug!( + ?dependent_root, + %proposal_slot, + "Proposer shuffling cache miss for proposer preferences verification" + ); + + // Fetch the dependent block's state root from fork choice. The block need not be + // canonical: preferences for a non-canonical branch are still verifiable, and the + // dependent state lives in the hot DB. + let dependent_state_root = ctx + .canonical_head + .fork_choice_read_lock() + .get_block(&dependent_root) + .ok_or(ProposerPreferencesError::DependentRootUnknown { dependent_root })? + .state_root; + + // Load a state at `target_slot` so it has the correct proposer lookahead. + let target_slot = proposal_epoch + .saturating_sub(ctx.spec.min_seed_lookahead) + .start_slot(T::EthSpec::slots_per_epoch()); + + let (state_root, state) = ctx + .store + .get_advanced_hot_state(dependent_root, target_slot, dependent_state_root) + .map_err(BeaconChainError::DBError)? + .ok_or(ProposerPreferencesError::DependentRootUnknown { dependent_root })?; + + Ok::<_, ProposerPreferencesError>((state_root, state)) + }, ctx.spec, - ) - .map_err(|_| ProposerPreferencesError::BadSignature)? - .verify() - .then_some(()) - .ok_or(ProposerPreferencesError::BadSignature)?; + )?; + + if proposer.index as u64 != validator_index { + return Err(ProposerPreferencesError::InvalidProposalSlot { + validator_index, + proposal_slot, + }); + } + + // Verify the signature using the proposer's pubkey from the validator pubkey cache + let signature_is_valid = { + let pubkey_cache = ctx.validator_pubkey_cache.read(); + let pubkey = pubkey_cache + .get(validator_index as usize) + .ok_or(ProposerPreferencesError::BadSignature)?; + signed_preferences.verify_signature::( + pubkey, + &proposer.fork, + ctx.genesis_validators_root, + ctx.spec, + ) + }; + if !signature_is_valid { + return Err(ProposerPreferencesError::BadSignature); + } let gossip_verified = GossipVerifiedProposerPreferences { signed_preferences }; @@ -128,6 +172,10 @@ impl BeaconChain { .gossip_verified_proposer_preferences_cache, slot_clock: &self.slot_clock, spec: &self.spec, + store: &self.store, + beacon_proposer_cache: &self.beacon_proposer_cache, + validator_pubkey_cache: &self.validator_pubkey_cache, + genesis_validators_root: self.genesis_validators_root, } } @@ -176,10 +224,7 @@ impl BeaconChain { #[cfg(test)] mod tests { - use types::{ - Address, BeaconState, ChainSpec, EthSpec, Hash256, MinimalEthSpec, ProposerPreferences, - Slot, - }; + use types::{Address, ChainSpec, EthSpec, Hash256, MinimalEthSpec, ProposerPreferences, Slot}; use super::verify_preferences_consistency; use crate::proposer_preferences_verification::ProposerPreferencesError; @@ -197,11 +242,6 @@ mod tests { } } - fn state() -> BeaconState { - let spec = spec(); - BeaconState::new(0, <_>::default(), &spec) - } - fn spec() -> ChainSpec { test_spec::() } @@ -214,7 +254,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::(&prefs, current_slot, &state(), &spec()); + let result = verify_preferences_consistency::(&prefs, current_slot, &spec()); assert!(matches!( result, Err(ProposerPreferencesError::InvalidProposalEpoch { .. }) @@ -229,7 +269,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::(&prefs, current_slot, &state(), &spec()); + let result = verify_preferences_consistency::(&prefs, current_slot, &spec()); assert!(matches!( result, Err(ProposerPreferencesError::InvalidProposalEpoch { .. }) @@ -244,7 +284,7 @@ mod tests { let current_slot = Slot::new(10); let prefs = make_preferences(Slot::new(9), 0); - let result = verify_preferences_consistency::(&prefs, current_slot, &state(), &spec()); + let result = verify_preferences_consistency::(&prefs, current_slot, &spec()); assert!(matches!( result, Err(ProposerPreferencesError::ProposalSlotAlreadyPassed { .. }) @@ -259,7 +299,7 @@ mod tests { let current_slot = Slot::new(10); let prefs = make_preferences(Slot::new(10), 0); - let result = verify_preferences_consistency::(&prefs, current_slot, &state(), &spec()); + let result = verify_preferences_consistency::(&prefs, current_slot, &spec()); assert!(matches!( result, Err(ProposerPreferencesError::ProposalSlotAlreadyPassed { .. }) 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 6c79e56733..cd05853ae9 100644 --- a/beacon_node/beacon_chain/src/proposer_preferences_verification/mod.rs +++ b/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,8 @@ pub enum ProposerPreferencesError { }, /// The slot clock cannot be read. UnableToReadSlot, + /// The block with root `dependent_root` has not been seen. + DependentRootUnknown { dependent_root: Hash256 }, /// A valid message from this validator for this slot has already been seen. AlreadySeen { validator_index: u64, diff --git a/beacon_node/beacon_chain/src/proposer_preferences_verification/proposer_preference_cache.rs b/beacon_node/beacon_chain/src/proposer_preferences_verification/proposer_preference_cache.rs index c423418fbc..2793449e7f 100644 --- a/beacon_node/beacon_chain/src/proposer_preferences_verification/proposer_preference_cache.rs +++ b/beacon_node/beacon_chain/src/proposer_preferences_verification/proposer_preference_cache.rs @@ -8,7 +8,9 @@ use parking_lot::RwLock; use types::{Hash256, SignedProposerPreferences, Slot}; pub struct GossipVerifiedProposerPreferenceCache { - preferences: RwLock>, + /// Mapping of `(proposal_slot, dependent_root)` to `GossipVerifiedProposerPreferences` + preferences: RwLock>, + /// Mapping of `proposal_slot` to `(dependent_root, validator_index)` seen: RwLock>>, } @@ -22,16 +24,23 @@ impl Default for GossipVerifiedProposerPreferenceCache { } impl GossipVerifiedProposerPreferenceCache { - pub fn get_preferences(&self, slot: &Slot) -> Option> { + pub fn get_preferences( + &self, + slot: &Slot, + dependent_root: Hash256, + ) -> Option> { self.preferences .read() - .get(slot) + .get(&(*slot, dependent_root)) .map(|p| p.signed_preferences.clone()) } pub fn insert_preferences(&self, preferences: GossipVerifiedProposerPreferences) { let slot = preferences.signed_preferences.message.proposal_slot; - self.preferences.write().insert(slot, preferences); + let dependent_root = preferences.signed_preferences.message.dependent_root; + self.preferences + .write() + .insert((slot, dependent_root), preferences); } pub fn get_seen_validator( @@ -60,7 +69,7 @@ impl GossipVerifiedProposerPreferenceCache { pub fn prune(&self, current_slot: Slot) { self.preferences .write() - .retain(|&slot, _| slot >= current_slot); + .retain(|&(slot, _), _| slot >= current_slot); self.seen.write().retain(|&slot, _| slot >= current_slot); } } @@ -108,15 +117,39 @@ mod tests { cache.prune(Slot::new(8)); for slot in [1, 2, 3, 7] { - assert!(cache.get_preferences(&Slot::new(slot)).is_none()); + assert!(cache.get_preferences(&Slot::new(slot), root).is_none()); assert!(!cache.get_seen_validator(&Slot::new(slot), root, slot)); } for slot in [8, 9, 10] { - assert!(cache.get_preferences(&Slot::new(slot)).is_some()); + assert!(cache.get_preferences(&Slot::new(slot), root).is_some()); assert!(cache.get_seen_validator(&Slot::new(slot), root, slot)); } } + #[test] + fn different_dependent_roots_not_overwritten() { + let cache = GossipVerifiedProposerPreferenceCache::default(); + let slot = Slot::new(5); + let root_a = Hash256::repeat_byte(0xaa); + let root_b = Hash256::repeat_byte(0xbb); + + // Two competing branches each have a (different) proposer for the same slot. + let verified_a = make_gossip_verified(slot, 1, root_a); + let verified_b = make_gossip_verified(slot, 2, root_b); + cache.insert_preferences(verified_a); + cache.insert_preferences(verified_b); + + // Neither branch's preferences overwrite the other; each is retrievable by its dependent root. + let prefs_a = cache + .get_preferences(&slot, root_a) + .expect("root_a present"); + let prefs_b = cache + .get_preferences(&slot, root_b) + .expect("root_b present"); + assert_eq!(prefs_a.message.validator_index, 1); + assert_eq!(prefs_b.message.validator_index, 2); + } + #[test] fn different_dependent_roots_not_deduped() { let cache = GossipVerifiedProposerPreferenceCache::default(); 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 53c1c4ded3..cd8752591c 100644 --- a/beacon_node/beacon_chain/src/proposer_preferences_verification/tests.rs +++ b/beacon_node/beacon_chain/src/proposer_preferences_verification/tests.rs @@ -4,9 +4,10 @@ use std::time::Duration; use bls::Signature; use fork_choice::ForkChoice; use genesis::{generate_deterministic_keypairs, interop_genesis_state}; +use parking_lot::{Mutex, RwLock}; 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, @@ -14,6 +15,7 @@ use types::{ use crate::{ beacon_fork_choice_store::BeaconForkChoiceStore, + beacon_proposer_cache::BeaconProposerCache, beacon_snapshot::BeaconSnapshot, canonical_head::CanonicalHead, proposer_preferences_verification::{ @@ -24,6 +26,7 @@ use crate::{ proposer_preference_cache::GossipVerifiedProposerPreferenceCache, }, test_utils::{EphemeralHarnessType, fork_name_from_env, test_spec}, + validator_pubkey_cache::ValidatorPubkeyCache, }; type E = MinimalEthSpec; @@ -36,6 +39,11 @@ struct TestContext { preferences_cache: GossipVerifiedProposerPreferenceCache, slot_clock: TestingSlotClock, spec: ChainSpec, + store: Arc>, + head_block_root: Hash256, + beacon_proposer_cache: Mutex, + validator_pubkey_cache: RwLock>, + genesis_validators_root: Hash256, } impl TestContext { @@ -57,12 +65,23 @@ impl TestContext { root: Hash256::ZERO, }; - let mut genesis_block = BeaconBlock::empty(&spec); - *genesis_block.state_root_mut() = state + let genesis_state_root = state .update_tree_hash_cache() .expect("should hash genesis state"); + + let block_root = state.get_latest_block_root(genesis_state_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()), @@ -86,11 +105,22 @@ impl TestContext { spec.get_slot_duration(), ); + let genesis_validators_root = state.genesis_validators_root(); + let validator_pubkey_cache = RwLock::new( + ValidatorPubkeyCache::new(&state, store.clone()) + .expect("should build validator pubkey cache"), + ); + Self { canonical_head, preferences_cache: GossipVerifiedProposerPreferenceCache::default(), slot_clock, spec, + store, + head_block_root: block_root, + beacon_proposer_cache: Mutex::new(BeaconProposerCache::default()), + validator_pubkey_cache, + genesis_validators_root, } } @@ -100,6 +130,10 @@ impl TestContext { gossip_verified_proposer_preferences_cache: &self.preferences_cache, slot_clock: &self.slot_clock, spec: &self.spec, + store: &self.store, + beacon_proposer_cache: &self.beacon_proposer_cache, + validator_pubkey_cache: &self.validator_pubkey_cache, + genesis_validators_root: self.genesis_validators_root, } } @@ -124,10 +158,11 @@ impl TestContext { fn make_signed_preferences( proposal_slot: Slot, validator_index: u64, + dependent_root: Hash256, ) -> Arc { Arc::new(SignedProposerPreferences { message: ProposerPreferences { - dependent_root: Hash256::ZERO, + dependent_root, proposal_slot, validator_index, fee_recipient: Address::ZERO, @@ -147,11 +182,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 +206,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 +222,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 +242,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 +260,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,9 +268,13 @@ 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, ctx.head_block_root) + .is_none() ); - assert!(ctx.preferences_cache.get_preferences(&slot).is_none()); } #[test] @@ -247,7 +286,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 +329,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 +380,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!( diff --git a/beacon_node/beacon_chain/tests/events.rs b/beacon_node/beacon_chain/tests/events.rs index 537e8f40ee..6e61f19e7f 100644 --- a/beacon_node/beacon_chain/tests/events.rs +++ b/beacon_node/beacon_chain/tests/events.rs @@ -8,7 +8,7 @@ use std::sync::Arc; use types::data::FixedBlobSidecarList; use types::{ Address, BlobSidecar, DataColumnSidecar, DataColumnSidecarFulu, DataColumnSidecarGloas, Domain, - EthSpec, Hash256, MinimalEthSpec, PayloadAttestationData, PayloadAttestationMessage, + EthSpec, MinimalEthSpec, PayloadAttestationData, PayloadAttestationMessage, ProposerPreferences, SignedExecutionPayloadBid, SignedProposerPreferences, SignedRoot, Slot, }; @@ -440,9 +440,19 @@ async fn proposer_preferences_event_on_gossip_verification() { .get(lookahead_index) .expect("lookahead index should be in range"); + // The dependent root must be the proposer shuffling decision block for the proposal epoch, so + // gossip verification can resolve the proposer shuffling from it. + let dependent_root = head_state + .proposer_shuffling_decision_root_at_epoch( + proposal_slot.epoch(E::slots_per_epoch()), + head.head_block_root(), + &harness.spec, + ) + .expect("should compute proposer shuffling decision root"); + // Build and sign proposer preferences for the proposer of `proposal_slot`. let preferences = ProposerPreferences { - dependent_root: Hash256::ZERO, + dependent_root, proposal_slot, validator_index, fee_recipient: Address::repeat_byte(0xaa), diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 4867852645..bd2a6f1755 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -2924,8 +2924,16 @@ impl ApiTester { .get(lookahead_index) .expect("slot index should be in lookahead") as usize; + let dependent_root = head_state + .proposer_shuffling_decision_root_at_epoch( + proposal_slot.epoch(E::slots_per_epoch()), + head.beacon_block_root, + &self.chain.spec, + ) + .expect("should compute proposer shuffling decision root"); + let preferences = ProposerPreferences { - dependent_root: Hash256::ZERO, + dependent_root, proposal_slot, validator_index: validator_index as u64, fee_recipient: Address::repeat_byte(0xaa), 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 c20673b79b..7957b56cbf 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -4076,7 +4076,8 @@ impl NetworkBeaconProcessor { | ProposerPreferencesError::ProposalSlotAlreadyPassed { .. } | ProposerPreferencesError::BeaconChainError(_) | ProposerPreferencesError::BeaconStateError(_) - | ProposerPreferencesError::UnableToReadSlot, + | ProposerPreferencesError::UnableToReadSlot + | ProposerPreferencesError::DependentRootUnknown { .. }, ) => { self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore); } diff --git a/consensus/types/src/builder/proposer_preferences.rs b/consensus/types/src/builder/proposer_preferences.rs index 97ba980d6c..cc470f77e7 100644 --- a/consensus/types/src/builder/proposer_preferences.rs +++ b/consensus/types/src/builder/proposer_preferences.rs @@ -1,5 +1,5 @@ -use crate::{Address, ForkName, Hash256, SignedRoot, Slot}; -use bls::Signature; +use crate::{Address, ChainSpec, Domain, EthSpec, Fork, ForkName, Hash256, SignedRoot, Slot}; +use bls::{PublicKey, Signature}; use context_deserialize::context_deserialize; use educe::Educe; use serde::{Deserialize, Serialize}; @@ -40,6 +40,25 @@ impl SignedProposerPreferences { signature: Signature::empty(), } } + + /// Verify `self.signature` against the given `pubkey`. + pub fn verify_signature( + &self, + pubkey: &PublicKey, + fork: &Fork, + genesis_validators_root: Hash256, + spec: &ChainSpec, + ) -> bool { + let proposal_epoch = self.message.proposal_slot.epoch(E::slots_per_epoch()); + let domain = spec.get_domain( + proposal_epoch, + Domain::ProposerPreferences, + fork, + genesis_validators_root, + ); + let message = self.message.signing_root(domain); + self.signature.verify(pubkey, message) + } } #[cfg(test)] diff --git a/consensus/types/src/state/beacon_state.rs b/consensus/types/src/state/beacon_state.rs index d25d3fc150..181245c747 100644 --- a/consensus/types/src/state/beacon_state.rs +++ b/consensus/types/src/state/beacon_state.rs @@ -23,7 +23,7 @@ use tree_hash_derive::TreeHash; use typenum::Unsigned; use crate::{ - Address, ExecutionBlockHash, ExecutionPayloadBid, ProposerPreferences, Withdrawal, + Address, ExecutionBlockHash, ExecutionPayloadBid, Withdrawal, attestation::{ AttestationData, AttestationDuty, BeaconCommittee, Checkpoint, CommitteeIndex, PTC, ParticipationFlags, PendingAttestation, @@ -1341,43 +1341,6 @@ impl BeaconState { } } - /// Check if the validator is the proposer for the given slot in the current or next epoch. - pub fn is_valid_proposal_slot( - &self, - preferences: &ProposerPreferences, - spec: &ChainSpec, - ) -> Result { - let current_epoch = self.current_epoch(); - let proposal_epoch = preferences.proposal_slot.epoch(E::slots_per_epoch()); - - if proposal_epoch < current_epoch { - return Ok(false); - } - - if proposal_epoch > current_epoch.saturating_add(spec.min_seed_lookahead) { - return Ok(false); - } - - let epoch_offset = proposal_epoch.as_u64().safe_sub(current_epoch.as_u64())?; - - let slot_in_epoch = preferences - .proposal_slot - .as_u64() - .safe_rem(E::slots_per_epoch())?; - - let index = epoch_offset - .safe_mul(E::slots_per_epoch()) - .and_then(|v| v.safe_add(slot_in_epoch))?; - - let proposer_lookahead = self.proposer_lookahead()?; - - let proposer = proposer_lookahead - .get(index as usize) - .ok_or(BeaconStateError::ProposerLookaheadOutOfBounds { i: index as usize })?; - - Ok(*proposer == preferences.validator_index) - } - /// Returns the beacon proposer index for each `slot` in `epoch`. /// /// The returned `Vec` contains one proposer index for each slot in the epoch.