diff --git a/beacon_node/beacon_chain/src/payload_attestation_verification/gossip_verified_payload_attestation.rs b/beacon_node/beacon_chain/src/payload_attestation_verification/gossip_verified_payload_attestation.rs index 6ff5b55ae5..8eadc395df 100644 --- a/beacon_node/beacon_chain/src/payload_attestation_verification/gossip_verified_payload_attestation.rs +++ b/beacon_node/beacon_chain/src/payload_attestation_verification/gossip_verified_payload_attestation.rs @@ -9,12 +9,12 @@ use bls::AggregateSignature; use educe::Educe; use eth2::types::{EventKind, ForkVersionedResponse}; use parking_lot::RwLock; -use safe_arith::SafeArith; use slot_clock::SlotClock; -use state_processing::per_block_processing::signature_sets::indexed_payload_attestation_signature_set; -use state_processing::state_advance::partial_state_advance; +use state_processing::per_block_processing::signature_sets::indexed_payload_attestation_signature_set_from_pubkeys; use std::borrow::Cow; -use types::{ChainSpec, EthSpec, IndexedPayloadAttestation, PTC, PayloadAttestationMessage, Slot}; +use types::{ + ChainSpec, EthSpec, Hash256, IndexedPayloadAttestation, PTC, PayloadAttestationMessage, Slot, +}; pub struct GossipVerificationContext<'a, T: BeaconChainTypes> { pub slot_clock: &'a T::SlotClock, @@ -24,6 +24,7 @@ pub struct GossipVerificationContext<'a, T: BeaconChainTypes> { pub shuffling_cache: &'a RwLock>, pub validator_pubkey_cache: &'a RwLock>, pub store: &'a BeaconStore, + pub genesis_validators_root: Hash256, } /// A `PayloadAttestationMessage` that has been verified for propagation on the gossip network. @@ -98,53 +99,6 @@ impl VerifiedPayloadAttestationMessage { }); } - // Get a state for signature verification. If the cached head state is too stale - // (e.g. during liveness failures with many skipped slots), fall back to loading - // a more recent state from the store and advancing it if necessary. - let head = ctx.canonical_head.cached_head(); - let head_state = &head.snapshot.beacon_state; - - let state_epoch = head_state.current_epoch(); - - // get_ptc can serve epochs in [state_epoch - 1, state_epoch + min_seed_lookahead]. - // If the message epoch is beyond that range, the head state is stale. - let advanced_state = if message_epoch - > state_epoch - .safe_add(ctx.spec.min_seed_lookahead) - .map_err(BeaconChainError::from)? - { - let head_block_root = head.head_block_root(); - let target_slot = message_epoch.start_slot(T::EthSpec::slots_per_epoch()); - - let (state_root, mut state) = ctx - .store - .get_advanced_hot_state( - head_block_root, - target_slot, - head.snapshot.beacon_state_root(), - ) - .map_err(BeaconChainError::from)? - .ok_or(BeaconChainError::MissingBeaconState( - head.snapshot.beacon_state_root(), - ))?; - - if state - .current_epoch() - .safe_add(ctx.spec.min_seed_lookahead) - .map_err(BeaconChainError::from)? - < message_epoch - { - partial_state_advance(&mut state, Some(state_root), target_slot, ctx.spec) - .map_err(BeaconChainError::from)?; - } - - Some(state) - } else { - None - }; - - let state = advanced_state.as_ref().unwrap_or(head_state); - // Build the indexed form for signature verification and downstream fork choice. let indexed_payload_attestation = IndexedPayloadAttestation { attesting_indices: vec![validator_index] @@ -157,11 +111,13 @@ impl VerifiedPayloadAttestationMessage { { // [REJECT] The signature is valid with respect to the `validator_index`. let pubkey_cache = ctx.validator_pubkey_cache.read(); - let signature_set = indexed_payload_attestation_signature_set( - state, + let fork = ctx.spec.fork_at_epoch(message_epoch); + let signature_set = indexed_payload_attestation_signature_set_from_pubkeys( |validator_index| pubkey_cache.get(validator_index).map(Cow::Borrowed), &indexed_payload_attestation.signature, &indexed_payload_attestation, + &fork, + ctx.genesis_validators_root, ctx.spec, ) .map_err(|_| Error::UnknownValidatorIndex(validator_index))?; @@ -219,6 +175,7 @@ impl BeaconChain { shuffling_cache: &self.shuffling_cache, validator_pubkey_cache: &self.validator_pubkey_cache, store: &self.store, + genesis_validators_root: self.genesis_validators_root, } } diff --git a/beacon_node/beacon_chain/src/payload_attestation_verification/mod.rs b/beacon_node/beacon_chain/src/payload_attestation_verification/mod.rs index f32e264a90..3c0efce6ed 100644 --- a/beacon_node/beacon_chain/src/payload_attestation_verification/mod.rs +++ b/beacon_node/beacon_chain/src/payload_attestation_verification/mod.rs @@ -9,7 +9,7 @@ use crate::BeaconChainError; use strum::AsRefStr; -use types::{BeaconStateError, Hash256, Slot}; +use types::{Hash256, Slot}; pub mod gossip_verified_payload_attestation; @@ -92,12 +92,6 @@ pub enum Error { /// We were unable to process this message due to an internal error. It's unclear if the /// message is valid. BeaconChainError(Box), - /// An error reading beacon state. - /// - /// ## Peer scoring - /// - /// We were unable to process this message due to an internal error. - BeaconStateError(BeaconStateError), } impl From for Error { @@ -106,11 +100,5 @@ impl From for Error { } } -impl From for Error { - fn from(e: BeaconStateError) -> Self { - Error::BeaconStateError(e) - } -} - #[cfg(test)] mod tests; diff --git a/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs b/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs index 4c44eb88f5..ee9ba0f8ca 100644 --- a/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs +++ b/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs @@ -260,20 +260,16 @@ fn duplicate_after_valid() { )); } -/// Exercises the `partial_state_advance` fallback in gossip verification when -/// the head state is too stale to compute PTC membership (e.g., during a -/// network liveness failure with many missed slots). +/// Exercises payload attestation gossip verification when the message epoch is ahead of the +/// canonical head due to many missed slots. #[tokio::test] -async fn stale_head_with_partial_advance() { +async fn stale_head_payload_attestation() { if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { return; } let slots_per_epoch = E::slots_per_epoch(); - // Head at epoch 1, message at epoch 5 — 4 epochs of missed slots. - // This exceeds min_seed_lookahead (1), triggering the fallback path: - // get_advanced_hot_state loads the stored state, then partial_state_advance - // advances it through epoch boundaries to populate ptc_window. + // Head at epoch 1, message at epoch 5: 4 epochs of missed slots. let head_slot = Slot::new(slots_per_epoch); let missed_epochs = 4; let target_slot = Slot::new(slots_per_epoch * (1 + missed_epochs)); @@ -292,7 +288,7 @@ async fn stale_head_with_partial_advance() { let head_epoch = head.snapshot.beacon_state.current_epoch(); assert!( target_epoch > head_epoch + harness.spec.min_seed_lookahead, - "precondition: message epoch must exceed head + min_seed_lookahead to trigger fallback" + "precondition: message epoch must exceed head + min_seed_lookahead" ); // GIVEN a slot clock advanced to epoch 5 without producing blocks @@ -317,7 +313,9 @@ async fn stale_head_with_partial_advance() { .expect("should get PTC from reference state"); let validator_index = *ptc.0.first().expect("PTC should have at least one member") as u64; - // WHEN a properly-signed payload attestation from a PTC member is verified. + // WHEN a properly-signed payload attestation from a PTC member is verified. The signature + // domain should come from the spec fork schedule and genesis validators root, not a loaded + // state in the verifier. let domain = harness.spec.get_domain( target_epoch, Domain::PTCAttester, 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 a564dc1090..d53cd91614 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -4242,8 +4242,7 @@ impl NetworkBeaconProcessor { ); } PayloadAttestationError::MissingPTC { .. } - | PayloadAttestationError::BeaconChainError(_) - | PayloadAttestationError::BeaconStateError(_) => { + | PayloadAttestationError::BeaconChainError(_) => { debug!( %peer_id, %message_slot, diff --git a/consensus/state_processing/src/per_block_processing/signature_sets.rs b/consensus/state_processing/src/per_block_processing/signature_sets.rs index 0686c4d605..ef39e4a17b 100644 --- a/consensus/state_processing/src/per_block_processing/signature_sets.rs +++ b/consensus/state_processing/src/per_block_processing/signature_sets.rs @@ -363,6 +363,30 @@ pub fn indexed_payload_attestation_signature_set<'a, 'b, E, F>( indexed_payload_attestation: &'b IndexedPayloadAttestation, spec: &'a ChainSpec, ) -> Result> +where + E: EthSpec, + F: Fn(usize) -> Option>, +{ + let fork = state.fork(); + + indexed_payload_attestation_signature_set_from_pubkeys( + get_pubkey, + signature, + indexed_payload_attestation, + &fork, + state.genesis_validators_root(), + spec, + ) +} + +pub fn indexed_payload_attestation_signature_set_from_pubkeys<'a, 'b, E, F>( + get_pubkey: F, + signature: &'a AggregateSignature, + indexed_payload_attestation: &'b IndexedPayloadAttestation, + fork: &Fork, + genesis_validators_root: Hash256, + spec: &'a ChainSpec, +) -> Result> where E: EthSpec, F: Fn(usize) -> Option>, @@ -378,12 +402,7 @@ where .data .slot .epoch(E::slots_per_epoch()); - let domain = spec.get_domain( - epoch, - Domain::PTCAttester, - &state.fork(), - state.genesis_validators_root(), - ); + let domain = spec.get_domain(epoch, Domain::PTCAttester, fork, genesis_validators_root); let message = indexed_payload_attestation.data.signing_root(domain);