From 46b6c7519cabdb9eb4ba27a70343cb318cdaec0c Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Sun, 31 May 2026 22:04:52 +0300 Subject: [PATCH] make sure proposer cache stores by dep root --- .../gossip_verified_bid.rs | 11 ++++- .../gossip_verified_proposer_preferences.rs | 9 ++-- .../proposer_preferences_verification/mod.rs | 2 - .../proposer_preference_cache.rs | 47 ++++++++++++++++--- .../tests.rs | 6 ++- .../gossip_methods.rs | 3 +- 6 files changed, 59 insertions(+), 19 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 354705b92c..1f7a3e6b86 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/proposer_preferences_verification/gossip_verified_proposer_preferences.rs b/beacon_node/beacon_chain/src/proposer_preferences_verification/gossip_verified_proposer_preferences.rs index 99ea0f29ba..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 @@ -60,7 +60,6 @@ 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() @@ -82,15 +81,13 @@ impl GossipVerifiedProposerPreferences { ctx.spec, )?; - // Get the block at dependent_root from fork choice to verify canonicity and get state_root + // 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 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); 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 8c85f024c8..cd05853ae9 100644 --- a/beacon_node/beacon_chain/src/proposer_preferences_verification/mod.rs +++ b/beacon_node/beacon_chain/src/proposer_preferences_verification/mod.rs @@ -40,8 +40,6 @@ pub enum ProposerPreferencesError { 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, 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 e547594f31..73e94ff9a9 100644 --- a/beacon_node/beacon_chain/src/proposer_preferences_verification/tests.rs +++ b/beacon_node/beacon_chain/src/proposer_preferences_verification/tests.rs @@ -256,7 +256,11 @@ fn correct_proposer_bad_signature() { !ctx.preferences_cache .get_seen_validator(&slot, ctx.head_block_root, actual_proposer) ); - assert!(ctx.preferences_cache.get_preferences(&slot).is_none()); + assert!( + ctx.preferences_cache + .get_preferences(&slot, ctx.head_block_root) + .is_none() + ); } #[test] 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 488c9915fe..57689612a9 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -4114,8 +4114,7 @@ impl NetworkBeaconProcessor { | ProposerPreferencesError::BeaconChainError(_) | ProposerPreferencesError::BeaconStateError(_) | ProposerPreferencesError::UnableToReadSlot - | ProposerPreferencesError::DependentRootUnknown { .. } - | ProposerPreferencesError::DependentRootNotCanonical { .. }, + | ProposerPreferencesError::DependentRootUnknown { .. }, ) => { self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore); }