Skip to content

Commit

Permalink
Merge pull request #5940 from dapplion/electra_attestation_changes_li…
Browse files Browse the repository at this point in the history 
…onreview

Electra attestations #5712 review
  • Loading branch information
@realbigsean
realbigsean authored Jun 19, 2024
2 parents 9a01b6b + 3977b92 commit 6e44832
Show file tree
Hide file tree
Showing 38 changed files with 533 additions and 781 deletions.
117 changes: 58 additions & 59 deletions beacon_node/beacon_chain/src/attestation_verification.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,10 @@ use std::borrow::Cow;
use strum::AsRefStr;
use tree_hash::TreeHash;
use types::{
Attestation, AttestationRef, BeaconCommittee, BeaconStateError,
BeaconStateError::NoCommitteeFound, ChainSpec, CommitteeIndex, Epoch, EthSpec, ForkName,
Hash256, IndexedAttestation, SelectionProof, SignedAggregateAndProof, Slot, SubnetId,
Attestation, AttestationRef, BeaconCommittee,
BeaconStateError::{self, NoCommitteeFound},
ChainSpec, CommitteeIndex, Epoch, EthSpec, ForkName, Hash256, IndexedAttestation,
SelectionProof, SignedAggregateAndProof, Slot, SubnetId,
};

pub use batch::{batch_verify_aggregated_attestations, batch_verify_unaggregated_attestations};
Expand Down Expand Up @@ -519,7 +520,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
.tree_hash_root();

// [New in Electra:EIP7549]
verify_committee_index(attestation, &chain.spec)?;
verify_committee_index(attestation)?;

if chain
.observed_attestations
Expand Down Expand Up @@ -596,59 +597,62 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
))
}
};

// Committees must be sorted by ascending index order 0..committees_per_slot
let get_indexed_attestation_with_committee =
|(committees, _): (Vec<BeaconCommittee>, CommitteesPerSlot)| {
match signed_aggregate {
SignedAggregateAndProof::Base(signed_aggregate) => {
let att = &signed_aggregate.message.aggregate;
let aggregator_index = signed_aggregate.message.aggregator_index;
let committee = committees
.iter()
.filter(|&committee| committee.index == att.data.index)
.at_most_one()
.map_err(|_| Error::NoCommitteeForSlotAndIndex {
slot: att.data.slot,
index: att.data.index,
})?;

// TODO(electra):
let (index, aggregator_index, selection_proof, data) = match signed_aggregate {
SignedAggregateAndProof::Base(signed_aggregate) => (
signed_aggregate.message.aggregate.data.index,
signed_aggregate.message.aggregator_index,
// Note: this clones the signature which is known to be a relatively slow operation.
//
// Future optimizations should remove this clone.
if let Some(committee) = committee {
let selection_proof = SelectionProof::from(
signed_aggregate.message.selection_proof.clone(),
);

if !selection_proof
.is_aggregator(committee.committee.len(), &chain.spec)
.map_err(|e| Error::BeaconChainError(e.into()))?
{
return Err(Error::InvalidSelectionProof { aggregator_index });
}
signed_aggregate.message.selection_proof.clone(),
signed_aggregate.message.aggregate.data.clone(),
),
SignedAggregateAndProof::Electra(signed_aggregate) => (
signed_aggregate
.message
.aggregate
.committee_index()
.ok_or(Error::NotExactlyOneCommitteeBitSet(0))?,
signed_aggregate.message.aggregator_index,
signed_aggregate.message.selection_proof.clone(),
signed_aggregate.message.aggregate.data.clone(),
),
};
let slot = data.slot;

// Ensure the aggregator is a member of the committee for which it is aggregating.
if !committee.committee.contains(&(aggregator_index as usize)) {
return Err(Error::AggregatorNotInCommittee { aggregator_index });
}
let committee = committees
.get(index as usize)
.ok_or(Error::NoCommitteeForSlotAndIndex { slot, index })?;

attesting_indices_base::get_indexed_attestation(
committee.committee,
att,
)
.map_err(|e| BeaconChainError::from(e).into())
} else {
Err(Error::NoCommitteeForSlotAndIndex {
slot: att.data.slot,
index: att.data.index,
})
}
if !SelectionProof::from(selection_proof)
.is_aggregator(committee.committee.len(), &chain.spec)
.map_err(|e| Error::BeaconChainError(e.into()))?
{
return Err(Error::InvalidSelectionProof { aggregator_index });
}

// Ensure the aggregator is a member of the committee for which it is aggregating.
if !committee.committee.contains(&(aggregator_index as usize)) {
return Err(Error::AggregatorNotInCommittee { aggregator_index });
}

// p2p aggregates have a single committee, we can assert that aggregation_bits is always
// less then MaxValidatorsPerCommittee
match signed_aggregate {
SignedAggregateAndProof::Base(signed_aggregate) => {
attesting_indices_base::get_indexed_attestation(
committee.committee,
&signed_aggregate.message.aggregate,
)
.map_err(|e| BeaconChainError::from(e).into())
}
SignedAggregateAndProof::Electra(signed_aggregate) => {
attesting_indices_electra::get_indexed_attestation_from_signed_aggregate(
attesting_indices_electra::get_indexed_attestation(
&committees,
signed_aggregate,
&chain.spec,
&signed_aggregate.message.aggregate,
)
.map_err(|e| BeaconChainError::from(e).into())
}
Expand Down Expand Up @@ -837,7 +841,7 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> {
}

// [New in Electra:EIP7549]
verify_committee_index(attestation, &chain.spec)?;
verify_committee_index(attestation)?;

// Attestations must be for a known block. If the block is unknown, we simply drop the
// attestation and do not delay consideration for later.
Expand Down Expand Up @@ -1337,17 +1341,10 @@ pub fn verify_signed_aggregate_signatures<T: BeaconChainTypes>(

/// 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<E: EthSpec>(
attestation: AttestationRef<E>,
spec: &ChainSpec,
) -> Result<(), Error> {
if spec.fork_name_at_slot::<E>(attestation.data().slot) >= ForkName::Electra {
pub fn verify_committee_index<E: EthSpec>(attestation: AttestationRef<E>) -> Result<(), Error> {
if let Ok(committee_bits) = attestation.committee_bits() {
// Check to ensure that the attestation is for a single committee.
let num_committee_bits = get_committee_indices::<E>(
attestation
.committee_bits()
.map_err(|e| Error::BeaconChainError(e.into()))?,
);
let num_committee_bits = get_committee_indices::<E>(committee_bits);
if num_committee_bits.len() != 1 {
return Err(Error::NotExactlyOneCommitteeBitSet(
num_committee_bits.len(),
Expand Down Expand Up @@ -1424,6 +1421,8 @@ pub fn obtain_indexed_attestation_and_committees_per_slot<T: BeaconChainTypes>(
///
/// If the committees for an `attestation`'s slot isn't found in the `shuffling_cache`, we will read a state
/// from disk and then update the `shuffling_cache`.
///
/// Committees are sorted by ascending index order 0..committees_per_slot
fn map_attestation_committees<T, F, R>(
chain: &BeaconChain<T>,
attestation: AttestationRef<T::EthSpec>,
Expand Down
2 changes: 2 additions & 0 deletions beacon_node/beacon_chain/src/attester_cache.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use state_processing::state_advance::{partial_state_advance, Error as StateAdvan
use std::collections::HashMap;
use std::ops::Range;
use types::{
attestation::Error as AttestationError,
beacon_state::{
compute_committee_index_in_epoch, compute_committee_range_in_epoch, epoch_committee_count,
},
Expand Down Expand Up @@ -59,6 +60,7 @@ pub enum Error {
InverseRange {
range: Range<usize>,
},
AttestationError(AttestationError),
}

impl From<BeaconStateError> for Error {
Expand Down
40 changes: 9 additions & 31 deletions beacon_node/beacon_chain/src/beacon_chain.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ use store::{
use task_executor::{ShutdownReason, TaskExecutor};
use tokio_stream::Stream;
use tree_hash::TreeHash;
use types::attestation::AttestationBase;
use types::blob_sidecar::FixedBlobSidecarList;
use types::payload::BlockProductionVersion;
use types::*;
Expand Down Expand Up @@ -1994,36 +1993,15 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
};
drop(cache_timer);

if self.spec.fork_name_at_slot::<T::EthSpec>(request_slot) >= ForkName::Electra {
let mut committee_bits = BitVector::default();
if committee_len > 0 {
committee_bits.set(request_index as usize, true)?;
}
Ok(Attestation::Electra(AttestationElectra {
aggregation_bits: BitList::with_capacity(committee_len)?,
data: AttestationData {
slot: request_slot,
index: 0u64,
beacon_block_root,
source: justified_checkpoint,
target,
},
committee_bits,
signature: AggregateSignature::empty(),
}))
} else {
Ok(Attestation::Base(AttestationBase {
aggregation_bits: BitList::with_capacity(committee_len)?,
data: AttestationData {
slot: request_slot,
index: request_index,
beacon_block_root,
source: justified_checkpoint,
target,
},
signature: AggregateSignature::empty(),
}))
}
Ok(Attestation::<T::EthSpec>::empty_for_signing(
request_index,
committee_len,
request_slot,
beacon_block_root,
justified_checkpoint,
target,
&self.spec,
)?)
}

/// Performs the same validation as `Self::verify_unaggregated_attestation_for_gossip`, but for
Expand Down
45 changes: 10 additions & 35 deletions beacon_node/beacon_chain/src/early_attester_cache.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use crate::{
use parking_lot::RwLock;
use proto_array::Block as ProtoBlock;
use std::sync::Arc;
use types::attestation::AttestationBase;
use types::*;

pub struct CacheItem<E: EthSpec> {
Expand Down Expand Up @@ -123,40 +122,16 @@ impl<E: EthSpec> EarlyAttesterCache<E> {
item.committee_lengths
.get_committee_length::<E>(request_slot, request_index, spec)?;

let attestation = if spec.fork_name_at_slot::<E>(request_slot) >= ForkName::Electra {
let mut committee_bits = BitVector::default();
if committee_len > 0 {
committee_bits
.set(request_index as usize, true)
.map_err(BeaconStateError::from)?;
}
Attestation::Electra(AttestationElectra {
aggregation_bits: BitList::with_capacity(committee_len)
.map_err(BeaconStateError::from)?,
committee_bits,
data: AttestationData {
slot: request_slot,
index: 0u64,
beacon_block_root: item.beacon_block_root,
source: item.source,
target: item.target,
},
signature: AggregateSignature::empty(),
})
} else {
Attestation::Base(AttestationBase {
aggregation_bits: BitList::with_capacity(committee_len)
.map_err(BeaconStateError::from)?,
data: AttestationData {
slot: request_slot,
index: request_index,
beacon_block_root: item.beacon_block_root,
source: item.source,
target: item.target,
},
signature: AggregateSignature::empty(),
})
};
let attestation = Attestation::empty_for_signing(
request_index,
committee_len,
request_slot,
item.beacon_block_root,
item.source,
item.target,
spec,
)
.map_err(Error::AttestationError)?;

metrics::inc_counter(&metrics::BEACON_EARLY_ATTESTER_CACHE_HITS);

Expand Down
5 changes: 3 additions & 2 deletions beacon_node/beacon_chain/src/observed_aggregates.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -473,12 +473,13 @@ where
#[cfg(not(debug_assertions))]
mod tests {
use super::*;
use types::{test_utils::test_random_instance, Hash256};
use types::{test_utils::test_random_instance, AttestationBase, Hash256};

type E = types::MainnetEthSpec;

fn get_attestation(slot: Slot, beacon_block_root: u64) -> Attestation<E> {
let mut a: Attestation<E> = test_random_instance();
let a: AttestationBase<E> = test_random_instance();
let mut a = Attestation::Base(a);
a.data_mut().slot = slot;
a.data_mut().beacon_block_root = Hash256::from_low_u64_be(beacon_block_root);
a
Expand Down
Loading

0 comments on commit 6e44832

Please sign in to comment.