From eef1bf6bb3a0c28c3d06895f8eace080ce4f69fb Mon Sep 17 00:00:00 2001 From: hopinheimer Date: Sun, 19 Apr 2026 01:19:12 -0400 Subject: [PATCH] shifting `payload_attestation_verification` to separate module --- beacon_node/beacon_chain/src/beacon_chain.rs | 18 +- .../src/payload_attestation_verification.rs | 328 ------------------ .../gossip_verified_payload_attestation.rs | 181 ++++++++++ .../payload_attestation_verification/mod.rs | 107 ++++++ .../gossip_methods.rs | 2 +- consensus/fork_choice/src/fork_choice.rs | 2 +- 6 files changed, 305 insertions(+), 333 deletions(-) delete mode 100644 beacon_node/beacon_chain/src/payload_attestation_verification.rs create mode 100644 beacon_node/beacon_chain/src/payload_attestation_verification/gossip_verified_payload_attestation.rs create mode 100644 beacon_node/beacon_chain/src/payload_attestation_verification/mod.rs diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 6718b33958..38122a03f3 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -56,7 +56,8 @@ use crate::observed_data_sidecars::ObservedDataSidecars; use crate::observed_operations::{ObservationOutcome, ObservedOperations}; use crate::observed_slashable::ObservedSlashable; use crate::payload_attestation_verification::{ - Error as PayloadAttestationError, VerifiedPayloadAttestationMessage, + Error as PayloadAttestationError, GossipVerificationContext as PayloadAttestationGossipContext, + VerifiedPayloadAttestationMessage, }; use crate::payload_bid_verification::payload_bid_cache::GossipVerifiedPayloadBidCache; #[cfg(not(test))] @@ -2201,14 +2202,25 @@ impl BeaconChain { }) } + pub fn payload_attestation_gossip_context(&self) -> PayloadAttestationGossipContext<'_, T> { + PayloadAttestationGossipContext { + slot_clock: &self.slot_clock, + spec: &self.spec, + observed_payload_attesters: &self.observed_payload_attesters, + canonical_head: &self.canonical_head, + validator_pubkey_cache: &self.validator_pubkey_cache, + } + } + pub fn verify_payload_attestation_message_for_gossip( &self, payload_attestation_message: PayloadAttestationMessage, - ) -> Result, PayloadAttestationError> { + ) -> Result, PayloadAttestationError> { metrics::inc_counter(&metrics::PAYLOAD_ATTESTATION_PROCESSING_REQUESTS); let _timer = metrics::start_timer(&metrics::PAYLOAD_ATTESTATION_GOSSIP_VERIFICATION_TIMES); - VerifiedPayloadAttestationMessage::verify(payload_attestation_message, self).inspect(|_| { + let ctx = self.payload_attestation_gossip_context(); + VerifiedPayloadAttestationMessage::new(payload_attestation_message, &ctx).inspect(|_| { metrics::inc_counter(&metrics::PAYLOAD_ATTESTATION_PROCESSING_SUCCESSES); }) } diff --git a/beacon_node/beacon_chain/src/payload_attestation_verification.rs b/beacon_node/beacon_chain/src/payload_attestation_verification.rs deleted file mode 100644 index 1ee6ae149a..0000000000 --- a/beacon_node/beacon_chain/src/payload_attestation_verification.rs +++ /dev/null @@ -1,328 +0,0 @@ -//! Provides verification for `PayloadAttestationMessage` received from the gossip network. -//! -//! Similar to `crate::attestation_verification`, we try to avoid doing duplicate verification -//! work as a payload attestation message passes through different stages of verification. -//! -//! ```ignore -//! types::PayloadAttestationMessage -//! | -//! ▼ -//! IndexedPayloadAttestationMessage -//! | -//! ▼ -//! VerifiedPayloadAttestationMessage -//! ``` - -use crate::{BeaconChain, BeaconChainError, BeaconChainTypes}; -use bls::AggregateSignature; -use slot_clock::SlotClock; -use state_processing::per_block_processing::signature_sets::indexed_payload_attestation_signature_set; -use std::borrow::Cow; -use strum::AsRefStr; -use types::{ - BeaconState, BeaconStateError, ChainSpec, EthSpec, Hash256, IndexedPayloadAttestation, PTC, - PayloadAttestationMessage, Slot, -}; - -/// Returned when a payload attestation message was not successfully verified. It might not have -/// been verified for two reasons: -/// -/// - The message is malformed or inappropriate for the context (indicated by all variants -/// other than `BeaconChainError`). -/// - The application encountered an internal error whilst attempting to determine validity -/// (the `BeaconChainError` variant) -#[derive(Debug, AsRefStr)] -pub enum Error { - /// The payload attestation message is from a slot that is later than the current slot - /// (with respect to the gossip clock disparity). - /// - /// ## Peer scoring - /// - /// Assuming the local clock is correct, the peer has sent an invalid message. - FutureSlot { - message_slot: Slot, - latest_permissible_slot: Slot, - }, - /// The payload attestation message is from a slot that is prior to the earliest - /// permissible slot (with respect to the gossip clock disparity). - /// - /// ## Peer scoring - /// - /// Assuming the local clock is correct, the peer has sent an invalid message. - PastSlot { - message_slot: Slot, - earliest_permissible_slot: Slot, - }, - /// We have already observed a valid payload attestation message from this validator - /// for this slot. - /// - /// ## Peer scoring - /// - /// The peer is not necessarily faulty. - PriorPayloadAttestationMessageKnown { validator_index: u64, slot: Slot }, - /// The beacon block referenced by the payload attestation message is not known. - /// - /// ## Peer scoring - /// - /// The attestation points to a block we have not yet imported. It's unclear if the - /// attestation is valid or not. - UnknownHeadBlock { beacon_block_root: Hash256 }, - /// The validator index is not a member of the PTC for this slot. - /// - /// ## Peer scoring - /// - /// The peer has sent an invalid message. - NotInPTC { validator_index: u64, slot: Slot }, - /// The validator index is unknown. - /// - /// ## Peer scoring - /// - /// The peer has sent an invalid message. - UnknownValidatorIndex(u64), - /// The signature on the payload attestation message is invalid. - /// - /// ## Peer scoring - /// - /// The peer has sent an invalid message. - InvalidSignature, - /// There was an error whilst processing the payload attestation message. It is not known - /// if it is valid or invalid. - /// - /// ## Peer scoring - /// - /// 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 { - fn from(e: BeaconChainError) -> Self { - Error::BeaconChainError(Box::new(e)) - } -} - -impl From for Error { - fn from(e: BeaconStateError) -> Self { - Error::BeaconStateError(e) - } -} - -/// Wraps a `PayloadAttestationMessage` that has passed early and middle checks but has *not* -/// undergone signature verification. -struct IndexedPayloadAttestationMessage { - payload_attestation_message: PayloadAttestationMessage, - indexed_payload_attestation: IndexedPayloadAttestation, - ptc: PTC, -} - -/// Wraps a `PayloadAttestationMessage` that has been fully verified for propagation on the -/// gossip network. -#[derive(Clone, Debug)] -pub struct VerifiedPayloadAttestationMessage { - payload_attestation_message: PayloadAttestationMessage, - indexed_payload_attestation: IndexedPayloadAttestation, - ptc: PTC, -} - -impl IndexedPayloadAttestationMessage { - fn verify_early_checks( - payload_attestation_message: &PayloadAttestationMessage, - chain: &BeaconChain, - ) -> Result<(), Error> { - let slot = payload_attestation_message.data.slot; - let validator_index = payload_attestation_message.validator_index; - - verify_propagation_slot_range(&chain.slot_clock, slot, &chain.spec)?; - - if chain - .observed_payload_attesters - .read() - .validator_has_been_observed(slot, validator_index as usize) - .map_err(BeaconChainError::from)? - { - return Err(Error::PriorPayloadAttestationMessageKnown { - validator_index, - slot, - }); - } - - let beacon_block_root = payload_attestation_message.data.beacon_block_root; - if chain - .canonical_head - .fork_choice_read_lock() - .get_block(&beacon_block_root) - .is_none() - { - return Err(Error::UnknownHeadBlock { beacon_block_root }); - } - - Ok(()) - } - - #[allow(clippy::type_complexity)] - fn verify_middle_checks( - payload_attestation_message: &PayloadAttestationMessage, - chain: &BeaconChain, - state: &BeaconState, - ) -> Result<(IndexedPayloadAttestation, PTC), Error> { - let slot = payload_attestation_message.data.slot; - let validator_index = payload_attestation_message.validator_index; - - let ptc = state.get_ptc(slot, &chain.spec)?; - if !ptc.0.contains(&(validator_index as usize)) { - return Err(Error::NotInPTC { - validator_index, - slot, - }); - } - - let indexed_payload_attestation = IndexedPayloadAttestation { - attesting_indices: vec![validator_index] - .try_into() - .map_err(|_| Error::UnknownValidatorIndex(validator_index))?, - data: payload_attestation_message.data.clone(), - signature: AggregateSignature::from(&payload_attestation_message.signature), - }; - - Ok((indexed_payload_attestation, ptc)) - } - - /// Runs early and middle checks, producing an intermediate verified form. - fn verify( - payload_attestation_message: PayloadAttestationMessage, - chain: &BeaconChain, - state: &BeaconState, - ) -> Result { - Self::verify_early_checks(&payload_attestation_message, chain)?; - let (indexed_payload_attestation, ptc) = - Self::verify_middle_checks(&payload_attestation_message, chain, state)?; - - Ok(Self { - payload_attestation_message, - indexed_payload_attestation, - ptc, - }) - } -} - -impl VerifiedPayloadAttestationMessage { - /// Returns `Ok(Self)` if the `payload_attestation_message` is valid to be (re)published on - /// the gossip network. - pub fn verify>( - payload_attestation_message: PayloadAttestationMessage, - chain: &BeaconChain, - ) -> Result { - // TODO(manas): i think we can have a shuffling cache. but this an interim solution - // can be discussed. - let head_snapshot = chain.head_snapshot(); - let head_state = &head_snapshot.beacon_state; - - let indexed = IndexedPayloadAttestationMessage::verify( - payload_attestation_message, - chain, - head_state, - )?; - - Self::from_indexed(indexed, chain, head_state) - } - - fn from_indexed>( - indexed: IndexedPayloadAttestationMessage, - chain: &BeaconChain, - state: &BeaconState, - ) -> Result { - let slot = indexed.payload_attestation_message.data.slot; - let validator_index = indexed.payload_attestation_message.validator_index; - - let pubkey_cache = chain.validator_pubkey_cache.read(); - - let signature_set = indexed_payload_attestation_signature_set( - state, - |validator_index| pubkey_cache.get(validator_index).map(Cow::Borrowed), - &indexed.indexed_payload_attestation.signature, - &indexed.indexed_payload_attestation, - &chain.spec, - ) - .map_err(|_| Error::UnknownValidatorIndex(validator_index))?; - - if !signature_set.verify() { - return Err(Error::InvalidSignature); - } - - // Now that the message has been fully verified, store that we have received a valid - // payload attestation message from this validator. - // - // Double check with the write lock to handle race conditions. - if chain - .observed_payload_attesters - .write() - .observe_validator(slot, validator_index as usize, ()) - .map_err(BeaconChainError::from)? - { - return Err(Error::PriorPayloadAttestationMessageKnown { - validator_index, - slot, - }); - } - - Ok(Self { - payload_attestation_message: indexed.payload_attestation_message, - indexed_payload_attestation: indexed.indexed_payload_attestation, - ptc: indexed.ptc, - }) - } - - pub fn payload_attestation_message(&self) -> &PayloadAttestationMessage { - &self.payload_attestation_message - } - - pub fn indexed_payload_attestation(&self) -> &IndexedPayloadAttestation { - &self.indexed_payload_attestation - } - - pub fn ptc(&self) -> &PTC { - &self.ptc - } - - pub fn into_payload_attestation_message(self) -> PayloadAttestationMessage { - self.payload_attestation_message - } -} - -/// Verify that the `slot` is within the acceptable gossip propagation range, with reference -/// to the current slot of the clock. -/// -/// Accounts for `MAXIMUM_GOSSIP_CLOCK_DISPARITY`. -fn verify_propagation_slot_range( - slot_clock: &S, - message_slot: Slot, - spec: &ChainSpec, -) -> Result<(), Error> { - let latest_permissible_slot = slot_clock - .now_with_future_tolerance(spec.maximum_gossip_clock_disparity()) - .ok_or(BeaconChainError::UnableToReadSlot)?; - if message_slot > latest_permissible_slot { - return Err(Error::FutureSlot { - message_slot, - latest_permissible_slot, - }); - } - - let earliest_permissible_slot = slot_clock - .now_with_past_tolerance(spec.maximum_gossip_clock_disparity()) - .ok_or(BeaconChainError::UnableToReadSlot)?; - if message_slot < earliest_permissible_slot { - return Err(Error::PastSlot { - message_slot, - earliest_permissible_slot, - }); - } - - Ok(()) -} 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 new file mode 100644 index 0000000000..f04144c754 --- /dev/null +++ b/beacon_node/beacon_chain/src/payload_attestation_verification/gossip_verified_payload_attestation.rs @@ -0,0 +1,181 @@ +use super::Error; +use crate::canonical_head::CanonicalHead; +use crate::observed_attesters::ObservedPayloadAttesters; +use crate::validator_pubkey_cache::ValidatorPubkeyCache; +use crate::{BeaconChainError, BeaconChainTypes}; +use bls::AggregateSignature; +use educe::Educe; +use parking_lot::RwLock; +use slot_clock::SlotClock; +use state_processing::per_block_processing::signature_sets::indexed_payload_attestation_signature_set; +use std::borrow::Cow; +use types::{ChainSpec, IndexedPayloadAttestation, PTC, PayloadAttestationMessage, Slot}; + +/// Bundles only the dependencies needed for gossip verification of payload attestation messages, +/// decoupling verification from the full `BeaconChain`. +pub struct GossipVerificationContext<'a, T: BeaconChainTypes> { + pub slot_clock: &'a T::SlotClock, + pub spec: &'a ChainSpec, + pub observed_payload_attesters: &'a RwLock>, + pub canonical_head: &'a CanonicalHead, + pub validator_pubkey_cache: &'a RwLock>, +} + +/// A `PayloadAttestationMessage` that has been verified for propagation on the gossip network. +#[derive(Educe)] +#[educe(Clone, Debug)] +pub struct VerifiedPayloadAttestationMessage { + payload_attestation_message: PayloadAttestationMessage, + indexed_payload_attestation: IndexedPayloadAttestation, + ptc: PTC, +} + +impl VerifiedPayloadAttestationMessage { + pub fn new( + payload_attestation_message: PayloadAttestationMessage, + ctx: &GossipVerificationContext<'_, T>, + ) -> Result { + let slot = payload_attestation_message.data.slot; + let validator_index = payload_attestation_message.validator_index; + + // [IGNORE] `data.slot` is within the `MAXIMUM_GOSSIP_CLOCK_DISPARITY` allowance. + verify_propagation_slot_range(ctx.slot_clock, slot, ctx.spec)?; + + // [IGNORE] There has been no other valid payload attestation message for this + // validator index. + if ctx + .observed_payload_attesters + .read() + .validator_has_been_observed(slot, validator_index as usize) + .map_err(BeaconChainError::from)? + { + return Err(Error::PriorPayloadAttestationMessageKnown { + validator_index, + slot, + }); + } + + // [IGNORE] `data.beacon_block_root` has been seen (via gossip or non-optimistic RPC). + // [REJECT] `data.beacon_block_root` passes validation. + // + // TODO(EIP-7732): These two conditions are conflated. We need a status table to + // differentiate between: + // 1. Blocks we haven't seen (IGNORE), and + // 2. Blocks we've seen that are invalid (REJECT). + // Presently both cases return IGNORE. + let beacon_block_root = payload_attestation_message.data.beacon_block_root; + if ctx + .canonical_head + .fork_choice_read_lock() + .get_block(&beacon_block_root) + .is_none() + { + return Err(Error::UnknownHeadBlock { beacon_block_root }); + } + + // Get head state for PTC computation. + let head = ctx.canonical_head.cached_head(); + let head_state = &head.snapshot.beacon_state; + + // [REJECT] `validator_index` is within `get_ptc(state, data.slot)`. + let ptc = head_state.get_ptc(slot, ctx.spec)?; + if !ptc.0.contains(&(validator_index as usize)) { + return Err(Error::NotInPTC { + validator_index, + slot, + }); + } + + // Build the indexed form for signature verification and downstream fork choice. + let indexed_payload_attestation = IndexedPayloadAttestation { + attesting_indices: vec![validator_index] + .try_into() + .map_err(|_| Error::UnknownValidatorIndex(validator_index))?, + data: payload_attestation_message.data.clone(), + signature: AggregateSignature::from(&payload_attestation_message.signature), + }; + + // [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( + head_state, + |validator_index| pubkey_cache.get(validator_index).map(Cow::Borrowed), + &indexed_payload_attestation.signature, + &indexed_payload_attestation, + ctx.spec, + ) + .map_err(|_| Error::UnknownValidatorIndex(validator_index))?; + + if !signature_set.verify() { + return Err(Error::InvalidSignature); + } + + // Record that we have received a valid payload attestation message from this + // validator. Double check with the write lock to handle race conditions. + if ctx + .observed_payload_attesters + .write() + .observe_validator(slot, validator_index as usize, ()) + .map_err(BeaconChainError::from)? + { + return Err(Error::PriorPayloadAttestationMessageKnown { + validator_index, + slot, + }); + } + + Ok(Self { + payload_attestation_message, + indexed_payload_attestation, + ptc, + }) + } + + pub fn payload_attestation_message(&self) -> &PayloadAttestationMessage { + &self.payload_attestation_message + } + + pub fn indexed_payload_attestation(&self) -> &IndexedPayloadAttestation { + &self.indexed_payload_attestation + } + + pub fn ptc(&self) -> &PTC { + &self.ptc + } + + pub fn into_payload_attestation_message(self) -> PayloadAttestationMessage { + self.payload_attestation_message + } +} + +/// Verify that the `slot` is within the acceptable gossip propagation range, with reference +/// to the current slot of the clock. +/// +/// Accounts for `MAXIMUM_GOSSIP_CLOCK_DISPARITY`. +fn verify_propagation_slot_range( + slot_clock: &S, + message_slot: Slot, + spec: &ChainSpec, +) -> Result<(), Error> { + let latest_permissible_slot = slot_clock + .now_with_future_tolerance(spec.maximum_gossip_clock_disparity()) + .ok_or(BeaconChainError::UnableToReadSlot)?; + if message_slot > latest_permissible_slot { + return Err(Error::FutureSlot { + message_slot, + latest_permissible_slot, + }); + } + + let earliest_permissible_slot = slot_clock + .now_with_past_tolerance(spec.maximum_gossip_clock_disparity()) + .ok_or(BeaconChainError::UnableToReadSlot)?; + if message_slot < earliest_permissible_slot { + return Err(Error::PastSlot { + message_slot, + earliest_permissible_slot, + }); + } + + Ok(()) +} diff --git a/beacon_node/beacon_chain/src/payload_attestation_verification/mod.rs b/beacon_node/beacon_chain/src/payload_attestation_verification/mod.rs new file mode 100644 index 0000000000..38cd36d75e --- /dev/null +++ b/beacon_node/beacon_chain/src/payload_attestation_verification/mod.rs @@ -0,0 +1,107 @@ +//! Provides verification for `PayloadAttestationMessage` received from the gossip network. +//! +//! ```ignore +//! types::PayloadAttestationMessage +//! | +//! ▼ +//! VerifiedPayloadAttestationMessage +//! ``` + +use crate::BeaconChainError; +use strum::AsRefStr; +use types::{BeaconStateError, Hash256, Slot}; + +pub mod gossip_verified_payload_attestation; + +pub use gossip_verified_payload_attestation::{ + GossipVerificationContext, VerifiedPayloadAttestationMessage, +}; + +/// Returned when a payload attestation message was not successfully verified. It might not have +/// been verified for two reasons: +/// +/// - The message is malformed or inappropriate for the context (indicated by all variants +/// other than `BeaconChainError`). +/// - The application encountered an internal error whilst attempting to determine validity +/// (the `BeaconChainError` variant) +#[derive(Debug, AsRefStr)] +pub enum Error { + /// The payload attestation message is from a slot that is later than the current slot + /// (with respect to the gossip clock disparity). + /// + /// ## Peer scoring + /// + /// Assuming the local clock is correct, the peer has sent an invalid message. + FutureSlot { + message_slot: Slot, + latest_permissible_slot: Slot, + }, + /// The payload attestation message is from a slot that is prior to the earliest + /// permissible slot (with respect to the gossip clock disparity). + /// + /// ## Peer scoring + /// + /// Assuming the local clock is correct, the peer has sent an invalid message. + PastSlot { + message_slot: Slot, + earliest_permissible_slot: Slot, + }, + /// We have already observed a valid payload attestation message from this validator + /// for this slot. + /// + /// ## Peer scoring + /// + /// The peer is not necessarily faulty. + PriorPayloadAttestationMessageKnown { validator_index: u64, slot: Slot }, + /// The beacon block referenced by the payload attestation message is not known. + /// + /// ## Peer scoring + /// + /// The attestation points to a block we have not yet imported. It's unclear if the + /// attestation is valid or not. + UnknownHeadBlock { beacon_block_root: Hash256 }, + /// The validator index is not a member of the PTC for this slot. + /// + /// ## Peer scoring + /// + /// The peer has sent an invalid message. + NotInPTC { validator_index: u64, slot: Slot }, + /// The validator index is unknown. + /// + /// ## Peer scoring + /// + /// The peer has sent an invalid message. + UnknownValidatorIndex(u64), + /// The signature on the payload attestation message is invalid. + /// + /// ## Peer scoring + /// + /// The peer has sent an invalid message. + InvalidSignature, + /// There was an error whilst processing the payload attestation message. It is not known + /// if it is valid or invalid. + /// + /// ## Peer scoring + /// + /// 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 { + fn from(e: BeaconChainError) -> Self { + Error::BeaconChainError(Box::new(e)) + } +} + +impl From for Error { + fn from(e: BeaconStateError) -> Self { + Error::BeaconStateError(e) + } +} 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 edeb191bb6..bdaff57688 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -3687,7 +3687,7 @@ impl NetworkBeaconProcessor { fn process_gossip_payload_attestation_result( self: &Arc, - result: Result, RejectedPayloadAttestation>, + result: Result, RejectedPayloadAttestation>, message_id: MessageId, peer_id: PeerId, ) { diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 92fd4c1faf..e7d0c1e9fc 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1337,7 +1337,7 @@ where let ptc_indices: Vec = attestation .attesting_indices .iter() - .filter_map(|vi| ptc.iter().position(|&p| p == *vi as usize)) + .filter_map(|validator_index| ptc.iter().position(|&p| p == *validator_index as usize)) .collect(); // Check that all the attesters are in the PTC