From f5eb33d5482c95f3f23aa2fad97886a5f07e1f95 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Thu, 9 May 2024 11:25:14 -0400 Subject: [PATCH] add new gossip conditions --- .../src/attestation_verification.rs | 65 ++++++++++++++++--- beacon_node/beacon_chain/src/errors.rs | 2 + .../gossip_methods.rs | 34 ++++++++++ .../src/common/get_attesting_indices.rs | 2 +- consensus/types/src/attestation.rs | 5 +- 5 files changed, 98 insertions(+), 10 deletions(-) diff --git a/beacon_node/beacon_chain/src/attestation_verification.rs b/beacon_node/beacon_chain/src/attestation_verification.rs index 3d722a534b..c9742d5468 100644 --- a/beacon_node/beacon_chain/src/attestation_verification.rs +++ b/beacon_node/beacon_chain/src/attestation_verification.rs @@ -45,7 +45,10 @@ use proto_array::Block as ProtoBlock; use slog::debug; use slot_clock::SlotClock; use state_processing::{ - common::{attesting_indices_base, attesting_indices_electra}, + common::{ + attesting_indices_base, + attesting_indices_electra::{self, get_committee_indices}, + }, per_block_processing::errors::{AttestationValidationError, BlockOperationError}, signature_sets::{ indexed_attestation_signature_set_from_pubkeys, @@ -139,6 +142,12 @@ pub enum Error { /// /// The peer has sent an invalid message. ValidatorIndexTooHigh(usize), + /// The validator index is not set to zero after Electra. + /// + /// ## Peer scoring + /// + /// The peer has sent an invalid message. + CommitteeIndexNonZero(usize), /// The `attestation.data.beacon_block_root` block is unknown. /// /// ## Peer scoring @@ -187,6 +196,12 @@ pub enum Error { /// /// The peer has sent an invalid message. NotExactlyOneAggregationBitSet(usize), + /// The attestation doesn't have only one aggregation bit set. + /// + /// ## Peer scoring + /// + /// The peer has sent an invalid message. + NotExactlyOneCommitteeBitSet(usize), /// We have already observed an attestation for the `validator_index` and refuse to process /// another. /// @@ -466,6 +481,9 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> { }); } + // [New in Electra:EIP7549] + verify_committee_index(attestation, &chain.spec)?; + // Ensure the valid aggregated attestation has not already been seen locally. let attestation_data = attestation.data(); let attestation_data_root = attestation_data.tree_hash_root(); @@ -775,6 +793,9 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> { return Err(Error::NotExactlyOneAggregationBitSet(num_aggregation_bits)); } + // [New in Electra:EIP7549] + verify_committee_index(attestation, &chain.spec)?; + // Attestations must be for a known block. If the block is unknown, we simply drop the // attestation and do not delay consideration for later. // @@ -1101,14 +1122,13 @@ pub fn verify_propagation_slot_range( let current_fork = spec.fork_name_at_slot::(slot_clock.now().ok_or(BeaconChainError::UnableToReadSlot)?); - let earliest_permissible_slot = match current_fork { - ForkName::Base | ForkName::Altair | ForkName::Bellatrix | ForkName::Capella => { - one_epoch_prior - } - // EIP-7045 - ForkName::Deneb | ForkName::Electra => one_epoch_prior + let earliest_permissible_slot = if current_fork < ForkName::Deneb { + one_epoch_prior + // EIP-7045 + } else { + one_epoch_prior .epoch(E::slots_per_epoch()) - .start_slot(E::slots_per_epoch()), + .start_slot(E::slots_per_epoch()) }; if attestation_slot < earliest_permissible_slot { @@ -1273,6 +1293,35 @@ pub fn verify_signed_aggregate_signatures( Ok(verify_signature_sets(signature_sets.iter())) } +/// Verify that the `attestation` committee index is properly set for the attestation's fork. +/// This function will only apply verification post-Electra. +pub fn verify_committee_index( + attestation: AttestationRef, + spec: &ChainSpec, +) -> Result<(), Error> { + if spec.fork_name_at_slot::(attestation.data().slot) >= ForkName::Electra { + // Check to ensure that the attestation is for a single committee. + let num_committee_bits = get_committee_indices::( + attestation + .committee_bits() + .map_err(|e| Error::BeaconChainError(e.into()))?, + ); + if num_committee_bits.len() != 1 { + return Err(Error::NotExactlyOneCommitteeBitSet( + num_committee_bits.len(), + )); + } + + // Ensure the attestation index is set to zero post Electra. + if attestation.data().index != 0 { + return Err(Error::CommitteeIndexNonZero( + attestation.data().index as usize, + )); + } + } + Ok(()) +} + /// Assists in readability. type CommitteesPerSlot = u64; diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index 340f1f9f79..b210536a62 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -226,6 +226,7 @@ pub enum BeaconChainError { LightClientError(LightClientError), UnsupportedFork, MilhouseError(MilhouseError), + AttestationError(AttestationError), } easy_from_to!(SlotProcessingError, BeaconChainError); @@ -256,6 +257,7 @@ easy_from_to!(AvailabilityCheckError, BeaconChainError); easy_from_to!(EpochCacheError, BeaconChainError); easy_from_to!(LightClientError, BeaconChainError); easy_from_to!(MilhouseError, BeaconChainError); +easy_from_to!(AttestationError, BeaconChainError); #[derive(Debug)] pub enum BlockProductionError { 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 d41233c903..d1e7f54abf 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -2038,6 +2038,27 @@ impl NetworkBeaconProcessor { "attn_val_index_too_high", ); } + AttnError::CommitteeIndexNonZero(index) => { + /* + * The validator index is not set to zero after Electra. + * + * The peer has published an invalid consensus message. + */ + debug!( + self.log, + "Committee index non zero"; + "peer_id" => %peer_id, + "block" => ?beacon_block_root, + "type" => ?attestation_type, + "committee_index" => index, + ); + self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Reject); + self.gossip_penalize_peer( + peer_id, + PeerAction::LowToleranceError, + "attn_comm_index_non_zero", + ); + } AttnError::UnknownHeadBlock { beacon_block_root } => { trace!( self.log, @@ -2193,6 +2214,19 @@ impl NetworkBeaconProcessor { "attn_too_many_agg_bits", ); } + AttnError::NotExactlyOneCommitteeBitSet(_) => { + /* + * The attestation doesn't have only one committee bit set. + * + * The peer has published an invalid consensus message. + */ + self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Reject); + self.gossip_penalize_peer( + peer_id, + PeerAction::LowToleranceError, + "attn_too_many_comm_bits", + ); + } AttnError::AttestsToFutureBlock { .. } => { /* * The beacon_block_root is from a higher slot than the attestation. diff --git a/consensus/state_processing/src/common/get_attesting_indices.rs b/consensus/state_processing/src/common/get_attesting_indices.rs index 9848840e96..1c59edf20e 100644 --- a/consensus/state_processing/src/common/get_attesting_indices.rs +++ b/consensus/state_processing/src/common/get_attesting_indices.rs @@ -155,7 +155,7 @@ pub mod attesting_indices_electra { Ok(indices) } - fn get_committee_indices( + pub fn get_committee_indices( committee_bits: &BitVector, ) -> Vec { committee_bits diff --git a/consensus/types/src/attestation.rs b/consensus/types/src/attestation.rs index bcecfde10e..8f836810a7 100644 --- a/consensus/types/src/attestation.rs +++ b/consensus/types/src/attestation.rs @@ -22,6 +22,7 @@ pub enum Error { SszTypesError(ssz_types::Error), AlreadySigned(usize), SubnetCountIsZero(ArithError), + IncorrectStateVariant, } #[superstruct( @@ -43,7 +44,9 @@ pub enum Error { serde(bound = "E: EthSpec", deny_unknown_fields), arbitrary(bound = "E: EthSpec"), ), - ref_attributes(derive(TreeHash), tree_hash(enum_behaviour = "transparent")) + ref_attributes(derive(TreeHash), tree_hash(enum_behaviour = "transparent")), + cast_error(ty = "Error", expr = "Error::IncorrectStateVariant"), + partial_getter_error(ty = "Error", expr = "Error::IncorrectStateVariant") )] #[derive( Debug,