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 1687760d25..bc81df1579 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 @@ -134,9 +134,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 5a2dd56828..644b9f0065 100644 --- a/beacon_node/beacon_chain/src/payload_bid_verification/tests.rs +++ b/beacon_node/beacon_chain/src/payload_bid_verification/tests.rs @@ -260,10 +260,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, @@ -274,8 +275,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 586721d8c1..ac81b42479 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,27 +1,24 @@ use std::sync::Arc; use crate::{ - BeaconChain, BeaconChainTypes, CanonicalHead, + BeaconChain, BeaconChainTypes, BeaconStore, CanonicalHead, proposer_preferences_verification::{ ProposerPreferencesError, proposer_preference_cache::GossipVerifiedProposerPreferenceCache, }, }; 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::{ - BeaconState, ChainSpec, EthSpec, ProposerPreferences, SignedProposerPreferences, Slot, -}; +use types::{ChainSpec, EthSpec, 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()); @@ -38,13 +35,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(()) } @@ -53,6 +43,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, } /// A wrapper around `SignedProposerPreferences` that has been verified for gossip propagation. @@ -69,12 +60,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 @@ -86,17 +75,49 @@ impl GossipVerifiedProposerPreferences { }); } - verify_preferences_consistency( + verify_preferences_consistency::( &signed_preferences.message, current_slot, - head_state, ctx.spec, )?; + // Get the block at dependent_root from fork choice to fetch its state_root. 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 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 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, - |i| get_pubkey_from_state(head_state, i), + &state, + |i| get_pubkey_from_state(&state, i), &signed_preferences, ctx.spec, ) @@ -127,6 +148,7 @@ impl BeaconChain { .gossip_verified_proposer_preferences_cache, slot_clock: &self.slot_clock, spec: &self.spec, + store: &self.store, } } @@ -162,10 +184,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; @@ -183,11 +202,6 @@ mod tests { } } - fn state() -> BeaconState { - let spec = spec(); - BeaconState::new(0, <_>::default(), &spec) - } - fn spec() -> ChainSpec { test_spec::() } @@ -200,7 +214,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 { .. }) @@ -215,7 +229,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 { .. }) @@ -230,7 +244,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 { .. }) @@ -245,7 +259,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..73e94ff9a9 100644 --- a/beacon_node/beacon_chain/src/proposer_preferences_verification/tests.rs +++ b/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>, + head_block_root: Hash256, } impl TestContext { @@ -57,12 +59,27 @@ 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 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 { 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,9 +254,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 +272,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 +315,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 +366,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/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 40cb2e592f..1195752ade 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/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), 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 b76f144887..4531faa553 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -3915,7 +3915,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); }