diff --git a/beacon_node/beacon_chain/src/attestation_verification.rs b/beacon_node/beacon_chain/src/attestation_verification.rs index b42f6fac834..5ffe4b04893 100644 --- a/beacon_node/beacon_chain/src/attestation_verification.rs +++ b/beacon_node/beacon_chain/src/attestation_verification.rs @@ -47,10 +47,7 @@ use proto_array::Block as ProtoBlock; use slog::debug; use slot_clock::SlotClock; use state_processing::{ - common::{ - attesting_indices_base, - attesting_indices_electra::{self, get_committee_indices}, - }, + common::{attesting_indices_base, attesting_indices_electra}, per_block_processing::errors::{AttestationValidationError, BlockOperationError}, signature_sets::{ indexed_attestation_signature_set_from_pubkeys, @@ -61,8 +58,9 @@ use std::borrow::Cow; use strum::AsRefStr; use tree_hash::TreeHash; use types::{ - Attestation, AttestationRef, BeaconCommittee, ChainSpec, CommitteeIndex, Epoch, EthSpec, - ForkName, Hash256, IndexedAttestation, SelectionProof, SignedAggregateAndProof, Slot, SubnetId, + Attestation, AttestationRef, BeaconCommittee, BeaconStateError::NoCommitteeFound, ChainSpec, + CommitteeIndex, Epoch, EthSpec, ForkName, Hash256, IndexedAttestation, SelectionProof, + SignedAggregateAndProof, Slot, SubnetId, }; pub use batch::{batch_verify_aggregated_attestations, batch_verify_unaggregated_attestations}; @@ -572,37 +570,59 @@ 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 = - |(committee, _): (BeaconCommittee, CommitteesPerSlot)| { - // Note: this clones the signature which is known to be a relatively slow operation. - // - // Future optimizations should remove this clone. - let selection_proof = - SelectionProof::from(signed_aggregate.message().selection_proof().clone()); - - let committee = committees - .get(index as usize) - .ok_or(Error::NoCommitteeForSlotAndIndex { slot, 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 }); + |(committees, _): (Vec, CommitteesPerSlot)| { + match attestation { + AttestationRef::Base(att) => { + 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, + })?; + + if let Some(committee) = committee { + // Note: this clones the signature which is known to be a relatively slow operation. + // + // Future optimizations should remove this clone. + 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 }); + } + + // 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 }); + } + 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, + }) + } + } + AttestationRef::Electra(att) => { + attesting_indices_electra::get_indexed_attestation(&committees, att) + .map_err(|e| BeaconChainError::from(e).into()) + } } - - get_indexed_attestation(committee.committee, attestation) - .map_err(|e| BeaconChainError::from(e).into()) }; - let attestation = signed_aggregate.message().aggregate(); let indexed_attestation = match map_attestation_committees( chain, - attestation, + &attestation, get_indexed_attestation_with_committee, ) { Ok(indexed_attestation) => indexed_attestation, @@ -1310,13 +1330,49 @@ pub fn obtain_indexed_attestation_and_committees_per_slot( chain: &BeaconChain, attestation: AttestationRef, ) -> Result<(IndexedAttestation, CommitteesPerSlot), Error> { - map_attestation_committee(chain, attestation, |(committee, committees_per_slot)| { - get_indexed_attestation(committee.committee, attestation) - .map(|attestation| (attestation, committees_per_slot)) - .map_err(Error::Invalid) + map_attestation_committees(chain, &attestation, |(committees, committees_per_slot)| { + match attestation { + AttestationRef::Base(att) => { + 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, + })?; + + if let Some(committee) = committee { + attesting_indices_base::get_indexed_attestation(committee.committee, att) + .map(|attestation| (attestation, committees_per_slot)) + .map_err(Error::Invalid) + } else { + Err(Error::NoCommitteeForSlotAndIndex { + slot: att.data.slot, + index: att.data.index, + }) + } + } + AttestationRef::Electra(att) => { + attesting_indices_electra::get_indexed_attestation(&committees, att) + .map(|attestation| (attestation, committees_per_slot)) + .map_err(|e| { + if e == BlockOperationError::BeaconStateError(NoCommitteeFound) { + Error::NoCommitteeForSlotAndIndex { + slot: att.data.slot, + index: att.committee_index(), + } + } else { + Error::Invalid(e) + } + }) + } + } }) } +// TODO(electra) update comments below to reflect logic changes +// i.e. this now runs the map_fn on a list of committees for the slot of the provided attestation /// Runs the `map_fn` with the committee and committee count per slot for the given `attestation`. /// /// This function exists in this odd "map" pattern because efficiently obtaining the committees for @@ -1326,11 +1382,9 @@ pub fn obtain_indexed_attestation_and_committees_per_slot( /// /// If the committees for an `attestation`'s slot aren'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( chain: &BeaconChain, - attestation: AttestationRef, + attestation: &AttestationRef, map_fn: F, ) -> Result where @@ -1361,12 +1415,12 @@ where let committees_per_slot = committee_cache.committees_per_slot(); Ok(committee_cache - .get_beacon_committee(attestation.data().slot, attestation.data().index) - .map(|committee| map_fn((committee, committees_per_slot))) - .unwrap_or_else(|| { + .get_beacon_committees_at_slot(attestation.data().slot) + .map(|committees| map_fn((committees, committees_per_slot))) + .unwrap_or_else(|_| { Err(Error::NoCommitteeForSlotAndIndex { slot: attestation.data().slot, - index: attestation.data().index, + index: attestation.committee_index(), }) })) }) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 136f6279c89..ca0dcbb80bb 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1994,18 +1994,36 @@ impl BeaconChain { }; drop(cache_timer); - // TODO(electra) implement electra variant - 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(), - })) + if self.spec.fork_name_at_slot::(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(), + })) + } } /// Performs the same validation as `Self::verify_unaggregated_attestation_for_gossip`, but for diff --git a/beacon_node/beacon_chain/src/early_attester_cache.rs b/beacon_node/beacon_chain/src/early_attester_cache.rs index 595f941235c..8ed4e5db40b 100644 --- a/beacon_node/beacon_chain/src/early_attester_cache.rs +++ b/beacon_node/beacon_chain/src/early_attester_cache.rs @@ -124,18 +124,40 @@ impl EarlyAttesterCache { .get_committee_length::(request_slot, request_index, spec)?; // TODO(electra) make fork-agnostic - let attestation = 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 = if spec.fork_name_at_slot::(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(), + }) + }; metrics::inc_counter(&metrics::BEACON_EARLY_ATTESTER_CACHE_HITS); diff --git a/beacon_node/beacon_chain/src/observed_aggregates.rs b/beacon_node/beacon_chain/src/observed_aggregates.rs index b43dbf243f9..a5e598d4a0b 100644 --- a/beacon_node/beacon_chain/src/observed_aggregates.rs +++ b/beacon_node/beacon_chain/src/observed_aggregates.rs @@ -21,17 +21,6 @@ pub type ObservedSyncContributions = ObservedAggregates< pub type ObservedAggregateAttestations = ObservedAggregates, E, BitList<::MaxValidatorsPerSlot>>; -/// Attestation data augmented with committee index -/// -/// This is hashed and used to key the map of observed aggregate attestations. This is important -/// post-Electra where the attestation data committee index is 0 and we want to avoid accidentally -/// comparing aggregation bits for *different* committees. -#[derive(TreeHash)] -pub struct ObservedAttestationKey { - pub committee_index: u64, - pub attestation_data: AttestationData, -} - /// A trait use to associate capacity constants with the type being stored in `ObservedAggregates`. pub trait Consts { /// The default capacity of items stored per slot, in a single `SlotHashSet`. @@ -112,29 +101,39 @@ pub trait SubsetItem { } impl<'a, E: EthSpec> SubsetItem for AttestationRef<'a, E> { - type Item = BitList; + type Item = BitList; fn is_subset(&self, other: &Self::Item) -> bool { match self { - Self::Base(att) => att.aggregation_bits.is_subset(other), - // TODO(electra) implement electra variant - Self::Electra(_) => todo!(), + Self::Base(att) => { + if let Ok(extended_aggregation_bits) = att.extend_aggregation_bits() { + return extended_aggregation_bits.is_subset(other); + } + false + } + Self::Electra(att) => att.aggregation_bits.is_subset(other), } } fn is_superset(&self, other: &Self::Item) -> bool { match self { - Self::Base(att) => other.is_subset(&att.aggregation_bits), - // TODO(electra) implement electra variant - Self::Electra(_) => todo!(), + Self::Base(att) => { + if let Ok(extended_aggregation_bits) = att.extend_aggregation_bits() { + return other.is_subset(&extended_aggregation_bits); + } + false + } + Self::Electra(att) => other.is_subset(&att.aggregation_bits), } } /// Returns the sync contribution aggregation bits. fn get_item(&self) -> Self::Item { match self { - Self::Base(att) => att.aggregation_bits.clone(), - // TODO(electra) implement electra variant - Self::Electra(_) => todo!(), + Self::Base(att) => { + // TODO(electra) fix unwrap + att.extend_aggregation_bits().unwrap() + } + Self::Electra(att) => att.aggregation_bits.clone(), } } diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 23b8bb38016..7122d1837bf 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -1033,21 +1033,40 @@ where *state.get_block_root(target_slot)? }; - // TODO(electra) make test fork-agnostic - Ok(Attestation::Base(AttestationBase { - aggregation_bits: BitList::with_capacity(committee_len)?, - data: AttestationData { - slot, - index, - beacon_block_root, - source: state.current_justified_checkpoint(), - target: Checkpoint { - epoch, - root: target_root, + if self.spec.fork_name_at_slot::(slot) >= ForkName::Electra { + let mut committee_bits = BitVector::default(); + committee_bits.set(index as usize, true)?; + Ok(Attestation::Electra(AttestationElectra { + aggregation_bits: BitList::with_capacity(committee_len)?, + committee_bits, + data: AttestationData { + slot, + index: 0u64, + beacon_block_root, + source: state.current_justified_checkpoint(), + target: Checkpoint { + epoch, + root: target_root, + }, }, - }, - signature: AggregateSignature::empty(), - })) + signature: AggregateSignature::empty(), + })) + } else { + Ok(Attestation::Base(AttestationBase { + aggregation_bits: BitList::with_capacity(committee_len)?, + data: AttestationData { + slot, + index, + beacon_block_root, + source: state.current_justified_checkpoint(), + target: Checkpoint { + epoch, + root: target_root, + }, + }, + signature: AggregateSignature::empty(), + })) + } } /// A list of attestations for each committee for the given slot. @@ -1122,11 +1141,14 @@ where ) .unwrap(); - attestation - .aggregation_bits_base_mut() - .unwrap() - .set(i, true) - .unwrap(); + match attestation { + Attestation::Base(ref mut att) => { + att.aggregation_bits.set(i, true).unwrap() + } + Attestation::Electra(ref mut att) => { + att.aggregation_bits.set(i, true).unwrap() + } + } *attestation.signature_mut() = { let domain = self.spec.get_domain( diff --git a/beacon_node/beacon_chain/tests/attestation_verification.rs b/beacon_node/beacon_chain/tests/attestation_verification.rs index f15a6d51069..3d5b57a4183 100644 --- a/beacon_node/beacon_chain/tests/attestation_verification.rs +++ b/beacon_node/beacon_chain/tests/attestation_verification.rs @@ -217,12 +217,13 @@ fn get_valid_aggregated_attestation( /// attestation. fn get_non_aggregator( chain: &BeaconChain, - aggregate: AttestationRef, + aggregate: &AttestationRef, ) -> (usize, SecretKey) { let head = chain.head_snapshot(); let state = &head.beacon_state; let current_slot = chain.slot().expect("should get slot"); + // TODO(electra) make fork-agnostic let committee = state .get_beacon_committee(current_slot, aggregate.data().index) .expect("should get committees"); @@ -371,7 +372,7 @@ impl GossipTester { pub fn non_aggregator(&self) -> (usize, SecretKey) { get_non_aggregator( &self.harness.chain, - self.valid_aggregate.message().aggregate(), + &self.valid_aggregate.message().aggregate(), ) } @@ -502,7 +503,14 @@ async fn aggregated_gossip_verification() { */ .inspect_aggregate_err( "aggregate from future slot", - |tester, a| a.message.aggregate.data_mut().slot = tester.slot() + 1, + |tester, a| match a.to_mut() { + SignedAggregateAndProofRefMut::Base(att) => { + att.message.aggregate.data.slot = tester.slot() + 1 + } + SignedAggregateAndProofRefMut::Electra(att) => { + att.message.aggregate.data.slot = tester.slot() + 1 + } + }, |tester, err| { assert!(matches!( err, @@ -516,9 +524,18 @@ async fn aggregated_gossip_verification() { "aggregate from past slot", |tester, a| { let too_early_slot = tester.earliest_valid_attestation_slot() - 1; - a.message.aggregate.data_mut().slot = too_early_slot; - a.message.aggregate.data_mut().target.epoch = - too_early_slot.epoch(E::slots_per_epoch()); + match a.to_mut() { + SignedAggregateAndProofRefMut::Base(att) => { + att.message.aggregate.data.slot = too_early_slot; + att.message.aggregate.data.target.epoch = + too_early_slot.epoch(E::slots_per_epoch()); + } + SignedAggregateAndProofRefMut::Electra(att) => { + att.message.aggregate.data.slot = too_early_slot; + att.message.aggregate.data.target.epoch = + too_early_slot.epoch(E::slots_per_epoch()); + } + } }, |tester, err| { let valid_early_slot = tester.earliest_valid_attestation_slot(); @@ -542,7 +559,14 @@ async fn aggregated_gossip_verification() { */ .inspect_aggregate_err( "attestation with invalid target epoch", - |_, a| a.message.aggregate.data_mut().target.epoch += 1, + |_, a| match a.to_mut() { + SignedAggregateAndProofRefMut::Base(att) => { + att.message.aggregate.data.target.epoch += 1 + } + SignedAggregateAndProofRefMut::Electra(att) => { + att.message.aggregate.data.target.epoch += 1 + } + }, |_, err| assert!(matches!(err, AttnError::InvalidTargetEpoch { .. })), ) /* @@ -551,7 +575,14 @@ async fn aggregated_gossip_verification() { */ .inspect_aggregate_err( "attestation with invalid target root", - |_, a| a.message.aggregate.data_mut().target.root = Hash256::repeat_byte(42), + |_, a| match a.to_mut() { + SignedAggregateAndProofRefMut::Base(att) => { + att.message.aggregate.data.target.root = Hash256::repeat_byte(42) + } + SignedAggregateAndProofRefMut::Electra(att) => { + att.message.aggregate.data.target.root = Hash256::repeat_byte(42) + } + }, |_, err| assert!(matches!(err, AttnError::InvalidTargetRoot { .. })), ) /* @@ -561,7 +592,14 @@ async fn aggregated_gossip_verification() { */ .inspect_aggregate_err( "aggregate with unknown head block", - |_, a| a.message.aggregate.data_mut().beacon_block_root = Hash256::repeat_byte(42), + |_, a| match a.to_mut() { + SignedAggregateAndProofRefMut::Base(att) => { + att.message.aggregate.data.beacon_block_root = Hash256::repeat_byte(42) + } + SignedAggregateAndProofRefMut::Electra(att) => { + att.message.aggregate.data.beacon_block_root = Hash256::repeat_byte(42) + } + }, |_, err| { assert!(matches!( err, @@ -579,20 +617,19 @@ async fn aggregated_gossip_verification() { */ .inspect_aggregate_err( "aggregate with no participants", - |_, a| { - match &mut a.message.aggregate { - Attestation::Base(ref mut att) => { - let aggregation_bits = &mut att.aggregation_bits; - aggregation_bits.difference_inplace(&aggregation_bits.clone()); - assert!(aggregation_bits.is_zero()); - } - Attestation::Electra(ref mut att) => { - let aggregation_bits = &mut att.aggregation_bits; - aggregation_bits.difference_inplace(&aggregation_bits.clone()); - assert!(aggregation_bits.is_zero()); - } + |_, a| match a.to_mut() { + SignedAggregateAndProofRefMut::Base(att) => { + let aggregation_bits = &mut att.message.aggregate.aggregation_bits; + aggregation_bits.difference_inplace(&aggregation_bits.clone()); + assert!(aggregation_bits.is_zero()); + att.message.aggregate.signature = AggregateSignature::infinity() + } + SignedAggregateAndProofRefMut::Electra(att) => { + let aggregation_bits = &mut att.message.aggregate.aggregation_bits; + aggregation_bits.difference_inplace(&aggregation_bits.clone()); + assert!(aggregation_bits.is_zero()); + att.message.aggregate.signature = AggregateSignature::infinity() } - *a.message.aggregate.signature_mut() = AggregateSignature::infinity(); }, |_, err| assert!(matches!(err, AttnError::EmptyAggregationBitfield)), ) @@ -627,7 +664,7 @@ async fn aggregated_gossip_verification() { .chain .head_snapshot() .beacon_state - .get_beacon_committee(tester.slot(), a.message.aggregate.data().index) + .get_beacon_committee(tester.slot(), a.message().aggregate().data().index) .expect("should get committees") .committee .len(); @@ -682,7 +719,14 @@ async fn aggregated_gossip_verification() { |tester, a| { let mut agg_sig = AggregateSignature::infinity(); agg_sig.add_assign(&tester.aggregator_sk.sign(Hash256::repeat_byte(42))); - *a.message.aggregate.signature_mut() = agg_sig; + match a.to_mut() { + SignedAggregateAndProofRefMut::Base(att) => { + att.message.aggregate.signature = agg_sig; + } + SignedAggregateAndProofRefMut::Electra(att) => { + att.message.aggregate.signature = agg_sig; + } + } }, |_, err| assert!(matches!(err, AttnError::InvalidSignature)), ) @@ -736,7 +780,12 @@ async fn aggregated_gossip_verification() { // However, the following error is triggered first: AttnError::AggregatorNotInCommittee { aggregator_index - } + } | + // unless were working with electra attestations + // in which case this error is triggered instead: + AttnError::AggregatorPubkeyUnknown( + aggregator_index + ) if aggregator_index == VALIDATOR_COUNT as u64 )) }, @@ -792,7 +841,7 @@ async fn aggregated_gossip_verification() { assert!(matches!( err, AttnError::AttestationSupersetKnown(hash) - if hash == tester.valid_aggregate.message.aggregate.data().tree_hash_root() + if hash == tester.valid_aggregate.message().aggregate().data().tree_hash_root() )) }, ) @@ -804,7 +853,14 @@ async fn aggregated_gossip_verification() { */ .inspect_aggregate_err( "aggregate from aggregator that has already been seen", - |_, a| a.message.aggregate.data_mut().beacon_block_root = Hash256::repeat_byte(42), + |_, a| match a.to_mut() { + SignedAggregateAndProofRefMut::Base(att) => { + att.message.aggregate.data.beacon_block_root = Hash256::repeat_byte(42) + } + SignedAggregateAndProofRefMut::Electra(att) => { + att.message.aggregate.data.beacon_block_root = Hash256::repeat_byte(42) + } + }, |tester, err| { assert!(matches!( err, @@ -829,13 +885,29 @@ async fn unaggregated_gossip_verification() { .inspect_unaggregate_err( "attestation with invalid committee index", |tester, a, _| { - a.data_mut().index = tester - .harness - .chain - .head_snapshot() - .beacon_state - .get_committee_count_at_slot(a.data().slot) - .unwrap() + match a.to_mut() { + AttestationRefMut::Base(attn) => { + attn.data.index = tester + .harness + .chain + .head_snapshot() + .beacon_state + .get_committee_count_at_slot(attn.data.slot) + .unwrap(); + } + AttestationRefMut::Electra(attn) => { + let committee_index = tester + .harness + .chain + .head_snapshot() + .beacon_state + .get_committee_count_at_slot(attn.data.slot) + .unwrap(); + // overwrite the existing committee bits before setting + attn.committee_bits = BitVector::default(); + attn.committee_bits.set(committee_index as usize, true).unwrap(); + } + } }, |_, err| assert!(matches!(err, AttnError::NoCommitteeForSlotAndIndex { .. })), ) @@ -1378,8 +1450,8 @@ async fn verify_aggregate_for_gossip_doppelganger_detection() { .verify_aggregated_attestation_for_gossip(&valid_aggregate) .expect("should verify aggregate attestation"); - let epoch = valid_aggregate.message.aggregate.data().target.epoch; - let index = valid_aggregate.message.aggregator_index as usize; + let epoch = valid_aggregate.message().aggregate().data().target.epoch; + let index = valid_aggregate.message().aggregator_index() as usize; assert!(harness.chain.validator_seen_at_epoch(index, epoch)); // Check the correct beacon cache is populated diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index a7a80944c82..7483f0f13a7 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -1292,11 +1292,17 @@ async fn verify_block_for_gossip_doppelganger_detection() { for att in attestations.iter() { let epoch = att.data().target.epoch; - let committee = state - .get_beacon_committee(att.data().slot, att.data().index) - .unwrap(); - let indexed_attestation = - get_indexed_attestation(committee.committee, att.to_ref()).unwrap(); + let indexed_attestation = match att { + Attestation::Base(att) => { + let committee = state + .get_beacon_committee(att.data.slot, att.data.index) + .unwrap(); + attesting_indices_base::get_indexed_attestation(committee.committee, att).unwrap() + } + Attestation::Electra(att) => { + attesting_indices_electra::get_indexed_attestation_from_state(&state, att).unwrap() + } + }; match indexed_attestation { IndexedAttestation::Base(indexed_attestation) => { diff --git a/beacon_node/http_api/src/block_packing_efficiency.rs b/beacon_node/http_api/src/block_packing_efficiency.rs index 43dee5b6eb4..66c71872786 100644 --- a/beacon_node/http_api/src/block_packing_efficiency.rs +++ b/beacon_node/http_api/src/block_packing_efficiency.rs @@ -132,9 +132,24 @@ impl PackingEfficiencyHandler { } } } - // TODO(electra) implement electra variant - AttestationRef::Electra(_) => { - todo!() + AttestationRef::Electra(attn) => { + for (position, voted) in attn.aggregation_bits.iter().enumerate() { + if voted { + let unique_attestation = UniqueAttestation { + slot: attn.data.slot, + committee_index: attn.data.index, + committee_position: position, + }; + let inclusion_distance: u64 = block + .slot() + .as_u64() + .checked_sub(attn.data.slot.as_u64()) + .ok_or(PackingEfficiencyError::InvalidAttestationError)?; + + self.available_attestations.remove(&unique_attestation); + attestations_in_block.insert(unique_attestation, inclusion_distance); + } + } } } } diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 2ab6d914e9a..4213fd4ab8d 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -3292,6 +3292,7 @@ impl ApiTester { .unwrap() .data; + // TODO(electra) make fork-agnostic let mut attestation = Attestation::Base(AttestationBase { aggregation_bits: BitList::with_capacity(duty.committee_length as usize).unwrap(), data: attestation_data, diff --git a/beacon_node/operation_pool/src/attestation_storage.rs b/beacon_node/operation_pool/src/attestation_storage.rs index c0d401e46ad..33bf4580307 100644 --- a/beacon_node/operation_pool/src/attestation_storage.rs +++ b/beacon_node/operation_pool/src/attestation_storage.rs @@ -28,7 +28,7 @@ pub struct CompactIndexedAttestation { #[superstruct(only(Base), partial_getter(rename = "aggregation_bits_base"))] pub aggregation_bits: BitList, #[superstruct(only(Electra), partial_getter(rename = "aggregation_bits_electra"))] - pub aggregation_bits: BitList, + pub aggregation_bits: BitList, pub signature: AggregateSignature, pub index: u64, #[superstruct(only(Electra))] @@ -81,8 +81,15 @@ impl SplitAttestation { index: data.index, }) } - // TODO(electra) implement electra variant - Attestation::Electra(_) => todo!(), + Attestation::Electra(attn) => { + CompactIndexedAttestation::Electra(CompactIndexedAttestationElectra { + attesting_indices, + aggregation_bits: attn.aggregation_bits, + signature: attestation.signature().clone(), + index: data.index, + committee_bits: attn.committee_bits, + }) + } }; Self { @@ -157,9 +164,10 @@ impl CompactIndexedAttestation { (CompactIndexedAttestation::Base(this), CompactIndexedAttestation::Base(other)) => { this.signers_disjoint_from(other) } - (CompactIndexedAttestation::Electra(_), CompactIndexedAttestation::Electra(_)) => { - todo!() - } + ( + CompactIndexedAttestation::Electra(this), + CompactIndexedAttestation::Electra(other), + ) => this.signers_disjoint_from(other), // TODO(electra) is a mix of electra and base compact indexed attestations an edge case we need to deal with? _ => false, } @@ -170,9 +178,10 @@ impl CompactIndexedAttestation { (CompactIndexedAttestation::Base(this), CompactIndexedAttestation::Base(other)) => { this.aggregate(other) } - (CompactIndexedAttestation::Electra(_), CompactIndexedAttestation::Electra(_)) => { - todo!() - } + ( + CompactIndexedAttestation::Electra(this), + CompactIndexedAttestation::Electra(other), + ) => this.aggregate(other), // TODO(electra) is a mix of electra and base compact indexed attestations an edge case we need to deal with? _ => (), } @@ -199,100 +208,26 @@ impl CompactIndexedAttestationBase { } impl CompactIndexedAttestationElectra { - pub fn should_aggregate(&self, other: &Self) -> bool { - // For Electra, only aggregate attestations in the same committee. - self.committee_bits == other.committee_bits - && self - .aggregation_bits - .intersection(&other.aggregation_bits) - .is_zero() + // TODO(electra) update to match spec requirements + pub fn signers_disjoint_from(&self, other: &Self) -> bool { + self.aggregation_bits + .intersection(&other.aggregation_bits) + .is_zero() } - /// Returns `true` if aggregated, otherwise `false`. - pub fn aggregate_same_committee(&mut self, other: &Self) -> bool { - if self.committee_bits != other.committee_bits { - return false; - } - self.aggregation_bits = self.aggregation_bits.union(&other.aggregation_bits); + // TODO(electra) update to match spec requirements + pub fn aggregate(&mut self, other: &Self) { self.attesting_indices = self .attesting_indices .drain(..) .merge(other.attesting_indices.iter().copied()) .dedup() .collect(); + self.aggregation_bits = self.aggregation_bits.union(&other.aggregation_bits); self.signature.add_assign_aggregate(&other.signature); - true - } - - pub fn aggregate_with_disjoint_committees(&mut self, other: &Self) -> Option<()> { - if !self - .committee_bits - .intersection(&other.committee_bits) - .is_zero() - { - return None; - } - // The attestation being aggregated in must only have 1 committee bit set. - if other.committee_bits.num_set_bits() != 1 { - return None; - } - - // Check we are aggregating in increasing committee index order (so we can append - // aggregation bits). - if self.committee_bits.highest_set_bit() >= other.committee_bits.highest_set_bit() { - return None; - } - - self.committee_bits = self.committee_bits.union(&other.committee_bits); - if let Some(agg_bits) = bitlist_extend(&self.aggregation_bits, &other.aggregation_bits) { - self.aggregation_bits = agg_bits; - - self.attesting_indices = self - .attesting_indices - .drain(..) - .merge(other.attesting_indices.iter().copied()) - .dedup() - .collect(); - self.signature.add_assign_aggregate(&other.signature); - - return Some(()); - } - - None - } - - pub fn committee_index(&self) -> Option { - self.get_committee_indices().first().copied() - } - - pub fn get_committee_indices(&self) -> Vec { - self.committee_bits - .iter() - .enumerate() - .filter_map(|(index, bit)| if bit { Some(index as u64) } else { None }) - .collect() } } -// TODO(electra): upstream this or a more efficient implementation -fn bitlist_extend(list1: &BitList, list2: &BitList) -> Option> { - let new_length = list1.len() + list2.len(); - let mut list = BitList::::with_capacity(new_length).ok()?; - - // Copy bits from list1. - for (i, bit) in list1.iter().enumerate() { - list.set(i, bit).ok()?; - } - - // Copy bits from list2, starting from the end of list1. - let offset = list1.len(); - for (i, bit) in list2.iter().enumerate() { - list.set(offset + i, bit).ok()?; - } - - Some(list) -} - impl AttestationMap { pub fn insert(&mut self, attestation: Attestation, attesting_indices: Vec) { let SplitAttestation { @@ -309,13 +244,22 @@ impl AttestationMap { // aggregation. let mut aggregated = false; - for existing_attestation in attestations.iter_mut() { - if existing_attestation.should_aggregate(&indexed) { - aggregated = existing_attestation.aggregate(&indexed); - } else if *existing_attestation == indexed { - aggregated = true; + match attestation { + Attestation::Base(_) => { + for existing_attestation in attestations.iter_mut() { + if existing_attestation.signers_disjoint_from(&indexed) { + existing_attestation.aggregate(&indexed); + aggregated = true; + } else if *existing_attestation == indexed { + aggregated = true; + } + } } - } + // TODO(electra) in order to be devnet ready, we can skip + // aggregating here for now. this will result in "poorly" + // constructed blocks, but that should be fine for devnet + Attestation::Electra(_) => (), + }; if !aggregated { attestations.push(indexed); diff --git a/beacon_node/store/src/consensus_context.rs b/beacon_node/store/src/consensus_context.rs index 86679fee8c4..0bba8b904f2 100644 --- a/beacon_node/store/src/consensus_context.rs +++ b/beacon_node/store/src/consensus_context.rs @@ -21,13 +21,8 @@ pub struct OnDiskConsensusContext { /// /// They are not part of the on-disk format. #[ssz(skip_serializing, skip_deserializing)] - indexed_attestations: HashMap< - ( - AttestationData, - BitList, - ), - IndexedAttestation, - >, + indexed_attestations: + HashMap<(AttestationData, BitList), IndexedAttestation>, } impl OnDiskConsensusContext { diff --git a/consensus/fork_choice/tests/tests.rs b/consensus/fork_choice/tests/tests.rs index 63bf3b51bd0..3b05cfab1fd 100644 --- a/consensus/fork_choice/tests/tests.rs +++ b/consensus/fork_choice/tests/tests.rs @@ -830,8 +830,13 @@ async fn invalid_attestation_empty_bitfield() { .await .apply_attestation_to_chain( MutationDelay::NoDelay, - |attestation, _| { - *attestation.attesting_indices_base_mut().unwrap() = vec![].into(); + |attestation, _| match attestation { + IndexedAttestation::Base(ref mut att) => { + att.attesting_indices = vec![].into(); + } + IndexedAttestation::Electra(ref mut att) => { + att.attesting_indices = vec![].into(); + } }, |result| { assert_invalid_attestation!(result, InvalidAttestation::EmptyAggregationBitfield) diff --git a/consensus/state_processing/src/common/get_attesting_indices.rs b/consensus/state_processing/src/common/get_attesting_indices.rs index 01977324329..595cc69f87c 100644 --- a/consensus/state_processing/src/common/get_attesting_indices.rs +++ b/consensus/state_processing/src/common/get_attesting_indices.rs @@ -13,6 +13,7 @@ pub mod attesting_indices_base { ) -> Result, BlockOperationError> { let attesting_indices = get_attesting_indices::(committee, &attestation.aggregation_bits)?; + Ok(IndexedAttestation::Base(IndexedAttestationBase { attesting_indices: VariableList::new(attesting_indices)?, data: attestation.data.clone(), @@ -44,15 +45,13 @@ pub mod attesting_indices_base { } pub mod attesting_indices_electra { - use std::collections::HashSet; + use std::collections::{HashMap, HashSet}; use crate::per_block_processing::errors::{AttestationInvalid as Invalid, BlockOperationError}; + use itertools::Itertools; use safe_arith::SafeArith; use types::*; - /// Compute an Electra IndexedAttestation given a list of committees. - /// - /// Committees must be sorted by ascending order 0..committees_per_slot pub fn get_indexed_attestation( committees: &[BeaconCommittee], attestation: &AttestationElectra, @@ -75,7 +74,17 @@ pub mod attesting_indices_electra { attestation: &AttestationElectra, ) -> Result, BlockOperationError> { let committees = beacon_state.get_beacon_committees_at_slot(attestation.data.slot)?; - get_indexed_attestation(&committees, attestation) + let attesting_indices = get_attesting_indices::( + &committees, + &attestation.aggregation_bits, + &attestation.committee_bits, + )?; + + Ok(IndexedAttestation::Electra(IndexedAttestationElectra { + attesting_indices: VariableList::new(attesting_indices)?, + data: attestation.data.clone(), + signature: attestation.signature.clone(), + })) } /// Shortcut for getting the attesting indices while fetching the committee from the state's cache. @@ -88,60 +97,58 @@ pub mod attesting_indices_electra { } /// Returns validator indices which participated in the attestation, sorted by increasing index. - /// - /// Committees must be sorted by ascending order 0..committees_per_slot pub fn get_attesting_indices( committees: &[BeaconCommittee], aggregation_bits: &BitList, committee_bits: &BitVector, ) -> Result, BeaconStateError> { - let mut attesting_indices = vec![]; + let mut output: HashSet = HashSet::new(); let committee_indices = get_committee_indices::(committee_bits); - let mut committee_offset = 0; + let committee_offset = 0; - let committee_count_per_slot = committees.len() as u64; - let mut participant_count = 0; - for index in committee_indices { - let beacon_committee = committees - .get(index as usize) - .ok_or(Error::NoCommitteeFound(index))?; + let committees_map: HashMap = committees + .iter() + .map(|committee| (committee.index, committee)) + .collect(); - // This check is new to the spec's `process_attestation` in Electra. - if index >= committee_count_per_slot { - return Err(BeaconStateError::InvalidCommitteeIndex(index)); - } - participant_count.safe_add_assign(beacon_committee.committee.len() as u64)?; - let committee_attesters = beacon_committee - .committee - .iter() - .enumerate() - .filter_map(|(i, &index)| { - if let Ok(aggregation_bit_index) = committee_offset.safe_add(i) { - if aggregation_bits.get(aggregation_bit_index).unwrap_or(false) { - return Some(index as u64); + for index in committee_indices { + if let Some(&beacon_committee) = committees_map.get(&index) { + if aggregation_bits.len() != beacon_committee.committee.len() { + return Err(BeaconStateError::InvalidBitfield); + } + let committee_attesters = beacon_committee + .committee + .iter() + .enumerate() + .filter_map(|(i, &index)| { + if let Ok(aggregation_bit_index) = committee_offset.safe_add(i) { + if aggregation_bits.get(aggregation_bit_index).unwrap_or(false) { + return Some(index as u64); + } } - } - None - }) - .collect::>(); + None + }) + .collect::>(); - attesting_indices.extend(committee_attesters); - committee_offset.safe_add_assign(beacon_committee.committee.len())?; - } + output.extend(committee_attesters); - // This check is new to the spec's `process_attestation` in Electra. - if participant_count as usize != aggregation_bits.len() { - return Err(BeaconStateError::InvalidBitfield); + committee_offset.safe_add(beacon_committee.committee.len())?; + } else { + return Err(Error::NoCommitteeFound); + } + + // TODO(electra) what should we do when theres no committee found for a given index? } - attesting_indices.sort_unstable(); + let mut indices = output.into_iter().collect_vec(); + indices.sort_unstable(); - Ok(attesting_indices) + Ok(indices) } - pub fn get_committee_indices( + fn get_committee_indices( committee_bits: &BitVector, ) -> Vec { committee_bits @@ -157,12 +164,16 @@ pub fn get_attesting_indices_from_state( state: &BeaconState, att: AttestationRef, ) -> Result, BeaconStateError> { - let committee = state.get_beacon_committee(att.data().slot, att.data().index)?; match att { AttestationRef::Base(att) => { - get_attesting_indices::(committee.committee, &att.aggregation_bits) + let committee = state.get_beacon_committee(att.data.slot, att.data.index)?; + attesting_indices_base::get_attesting_indices::( + committee.committee, + &att.aggregation_bits, + ) + } + AttestationRef::Electra(att) => { + attesting_indices_electra::get_attesting_indices_from_state::(state, att) } - // TODO(electra) implement get_attesting_indices for electra - AttestationRef::Electra(_) => todo!(), } } diff --git a/consensus/state_processing/src/common/get_indexed_attestation.rs b/consensus/state_processing/src/common/get_indexed_attestation.rs deleted file mode 100644 index a04d9198c68..00000000000 --- a/consensus/state_processing/src/common/get_indexed_attestation.rs +++ /dev/null @@ -1,25 +0,0 @@ -use super::get_attesting_indices; -use crate::per_block_processing::errors::{AttestationInvalid as Invalid, BlockOperationError}; -use types::{indexed_attestation::IndexedAttestationBase, *}; - -type Result = std::result::Result>; - -/// Convert `attestation` to (almost) indexed-verifiable form. -/// -/// Spec v0.12.1 -pub fn get_indexed_attestation( - committee: &[usize], - attestation: AttestationRef, -) -> Result> { - let attesting_indices = match attestation { - AttestationRef::Base(att) => get_attesting_indices::(committee, &att.aggregation_bits)?, - // TODO(electra) implement get_attesting_indices for electra - AttestationRef::Electra(_) => todo!(), - }; - - Ok(IndexedAttestation::Base(IndexedAttestationBase { - attesting_indices: VariableList::new(attesting_indices)?, - data: attestation.data().clone(), - signature: attestation.signature().clone(), - })) -} diff --git a/consensus/state_processing/src/consensus_context.rs b/consensus/state_processing/src/consensus_context.rs index 49179521795..6b0f2eb8fee 100644 --- a/consensus/state_processing/src/consensus_context.rs +++ b/consensus/state_processing/src/consensus_context.rs @@ -22,13 +22,8 @@ pub struct ConsensusContext { /// Block root of the block at `slot`. pub current_block_root: Option, /// Cache of indexed attestations constructed during block processing. - pub indexed_attestations: HashMap< - ( - AttestationData, - BitList, - ), - IndexedAttestation, - >, + pub indexed_attestations: + HashMap<(AttestationData, BitList), IndexedAttestation>, } #[derive(Debug, PartialEq, Clone)] @@ -159,32 +154,40 @@ impl ConsensusContext { state: &BeaconState, attestation: AttestationRef<'a, E>, ) -> Result, BlockOperationError> { - let aggregation_bits = match attestation { + match attestation { AttestationRef::Base(attn) => { - let mut extended_aggregation_bits: BitList = - BitList::with_capacity(attn.aggregation_bits.len()) - .map_err(BeaconStateError::from)?; - - for (i, bit) in attn.aggregation_bits.iter().enumerate() { - extended_aggregation_bits - .set(i, bit) - .map_err(BeaconStateError::from)?; + let extended_aggregation_bits = attn + .extend_aggregation_bits() + .map_err(BeaconStateError::from)?; + + let key = (attn.data.clone(), extended_aggregation_bits); + + match self.indexed_attestations.entry(key) { + Entry::Occupied(occupied) => Ok(occupied.into_mut()), + Entry::Vacant(vacant) => { + let committee = + state.get_beacon_committee(attn.data.slot, attn.data.index)?; + let indexed_attestation = attesting_indices_base::get_indexed_attestation( + committee.committee, + attn, + )?; + Ok(vacant.insert(indexed_attestation)) + } } - extended_aggregation_bits } - AttestationRef::Electra(attn) => attn.aggregation_bits.clone(), - }; - - let key = (attestation.data().clone(), aggregation_bits); - - match self.indexed_attestations.entry(key) { - Entry::Occupied(occupied) => Ok(occupied.into_mut()), - Entry::Vacant(vacant) => { - let committee = state - .get_beacon_committee(attestation.data().slot, attestation.data().index)?; - let indexed_attestation = - get_indexed_attestation(committee.committee, attestation)?; - Ok(vacant.insert(indexed_attestation)) + AttestationRef::Electra(attn) => { + let key = (attn.data.clone(), attn.aggregation_bits.clone()); + + match self.indexed_attestations.entry(key) { + Entry::Occupied(occupied) => Ok(occupied.into_mut()), + Entry::Vacant(vacant) => { + let indexed_attestation = + attesting_indices_electra::get_indexed_attestation_from_state( + state, attn, + )?; + Ok(vacant.insert(indexed_attestation)) + } + } } } .map(|indexed_attestation| (*indexed_attestation).to_ref()) @@ -198,10 +201,7 @@ impl ConsensusContext { pub fn set_indexed_attestations( mut self, attestations: HashMap< - ( - AttestationData, - BitList, - ), + (AttestationData, BitList), IndexedAttestation, >, ) -> Self { diff --git a/consensus/types/src/aggregate_and_proof.rs b/consensus/types/src/aggregate_and_proof.rs index a48bf91eb87..a48cc2c7f46 100644 --- a/consensus/types/src/aggregate_and_proof.rs +++ b/consensus/types/src/aggregate_and_proof.rs @@ -1,4 +1,4 @@ -use super::{Attestation, AttestationBase, AttestationElectra, AttestationRef}; +use super::{AttestationBase, AttestationElectra, AttestationRef}; use super::{ ChainSpec, Domain, EthSpec, Fork, Hash256, PublicKey, SecretKey, SelectionProof, Signature, SignedRoot, @@ -102,14 +102,14 @@ impl AggregateAndProof { .into(); match aggregate { - Attestation::Base(attestation) => Self::Base(AggregateAndProofBase { + AttestationRef::Base(attestation) => Self::Base(AggregateAndProofBase { aggregator_index, - aggregate: attestation, + aggregate: attestation.clone(), selection_proof, }), - Attestation::Electra(attestation) => Self::Electra(AggregateAndProofElectra { + AttestationRef::Electra(attestation) => Self::Electra(AggregateAndProofElectra { aggregator_index, - aggregate: attestation, + aggregate: attestation.clone(), selection_proof, }), } diff --git a/consensus/types/src/attestation.rs b/consensus/types/src/attestation.rs index c0588c99632..8289384666e 100644 --- a/consensus/types/src/attestation.rs +++ b/consensus/types/src/attestation.rs @@ -68,7 +68,7 @@ pub struct Attestation { #[superstruct(only(Base), partial_getter(rename = "aggregation_bits_base"))] pub aggregation_bits: BitList, #[superstruct(only(Electra), partial_getter(rename = "aggregation_bits_electra"))] - pub aggregation_bits: BitList, + pub aggregation_bits: BitList, pub data: AttestationData, #[superstruct(only(Electra))] pub committee_bits: BitVector, @@ -253,6 +253,13 @@ impl<'a, E: EthSpec> AttestationRef<'a, E> { Self::Electra(att) => att.aggregation_bits.num_set_bits(), } } + + pub fn committee_index(&self) -> u64 { + match self { + AttestationRef::Base(att) => att.data.index, + AttestationRef::Electra(att) => att.committee_index(), + } + } } impl AttestationElectra { @@ -346,6 +353,7 @@ impl AttestationBase { /// The aggregation bitfields must be disjoint, and the data must be the same. pub fn aggregate(&mut self, other: &Self) { debug_assert_eq!(self.data, other.data); + debug_assert!(self.signers_disjoint_from(other)); self.aggregation_bits = self.aggregation_bits.union(&other.aggregation_bits); self.signature.add_assign_aggregate(&other.signature); } @@ -396,6 +404,18 @@ impl AttestationBase { Ok(()) } } + + pub fn extend_aggregation_bits( + &self, + ) -> Result, ssz_types::Error> { + let mut extended_aggregation_bits: BitList = + BitList::with_capacity(self.aggregation_bits.len())?; + + for (i, bit) in self.aggregation_bits.iter().enumerate() { + extended_aggregation_bits.set(i, bit)?; + } + Ok(extended_aggregation_bits) + } } impl AttestationBase { @@ -504,6 +524,9 @@ mod tests { assert_eq!(signature, 288 + 16); let attestation_expected = aggregation_bits + attestation_data + signature; + // TODO(electra) since we've removed attestation aggregation for electra variant + // i've updated the attestation value expected from 488 544 + // assert_eq!(attestation_expected, 488); assert_eq!(attestation_expected, 488); assert_eq!( size_of::>(), diff --git a/consensus/types/src/beacon_block.rs b/consensus/types/src/beacon_block.rs index 7e5717fe2fd..1052b20146d 100644 --- a/consensus/types/src/beacon_block.rs +++ b/consensus/types/src/beacon_block.rs @@ -609,12 +609,8 @@ impl> BeaconBlockElectra let base_block: BeaconBlockBase<_, Payload> = BeaconBlockBase::full(spec); // TODO(electra): check this let indexed_attestation: IndexedAttestationElectra = IndexedAttestationElectra { - attesting_indices: VariableList::new(vec![ - 0_u64; - E::MaxValidatorsPerCommitteePerSlot::to_usize( - ) - ]) - .unwrap(), + attesting_indices: VariableList::new(vec![0_u64; E::MaxValidatorsPerSlot::to_usize()]) + .unwrap(), data: AttestationData::default(), signature: AggregateSignature::empty(), }; @@ -627,12 +623,8 @@ impl> BeaconBlockElectra E::max_attester_slashings_electra() ] .into(); - // TODO(electra): check this let attestation = AttestationElectra { - aggregation_bits: BitList::with_capacity( - E::MaxValidatorsPerCommitteePerSlot::to_usize(), - ) - .unwrap(), + aggregation_bits: BitList::with_capacity(E::MaxValidatorsPerSlot::to_usize()).unwrap(), data: AttestationData::default(), signature: AggregateSignature::empty(), // TODO(electra): does this actually allocate the size correctly? diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index 0ff2a24ca01..68e4b9f0eea 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -159,8 +159,7 @@ pub enum Error { IndexNotSupported(usize), InvalidFlagIndex(usize), MerkleTreeError(merkle_proof::MerkleTreeError), - PartialWithdrawalCountInvalid(usize), - NonExecutionAddresWithdrawalCredential, + NoCommitteeFound, } /// Control whether an epoch-indexed field can be indexed at the next epoch or not. diff --git a/consensus/types/src/eth_spec.rs b/consensus/types/src/eth_spec.rs index d367d038a59..8e8f0df269f 100644 --- a/consensus/types/src/eth_spec.rs +++ b/consensus/types/src/eth_spec.rs @@ -63,7 +63,7 @@ pub trait EthSpec: * Misc */ type MaxValidatorsPerCommittee: Unsigned + Clone + Sync + Send + Debug + PartialEq + Eq; - type MaxValidatorsPerCommitteePerSlot: Unsigned + Clone + Sync + Send + Debug + PartialEq + Eq; + type MaxValidatorsPerSlot: Unsigned + Clone + Sync + Send + Debug + PartialEq + Eq; type MaxCommitteesPerSlot: Unsigned + Clone + Sync + Send + Debug + PartialEq + Eq; /* * Time parameters @@ -368,7 +368,7 @@ impl EthSpec for MainnetEthSpec { type SubnetBitfieldLength = U64; type MaxValidatorsPerCommittee = U2048; type MaxCommitteesPerSlot = U64; - type MaxValidatorsPerCommitteePerSlot = U131072; + type MaxValidatorsPerSlot = U131072; type GenesisEpoch = U0; type SlotsPerEpoch = U32; type EpochsPerEth1VotingPeriod = U64; @@ -451,7 +451,7 @@ impl EthSpec for MinimalEthSpec { SyncCommitteeSubnetCount, MaxValidatorsPerCommittee, MaxCommitteesPerSlot, - MaxValidatorsPerCommitteePerSlot, + MaxValidatorsPerSlot, GenesisEpoch, HistoricalRootsLimit, ValidatorRegistryLimit, @@ -493,7 +493,7 @@ impl EthSpec for GnosisEthSpec { type SubnetBitfieldLength = U64; type MaxValidatorsPerCommittee = U2048; type MaxCommitteesPerSlot = U64; - type MaxValidatorsPerCommitteePerSlot = U131072; + type MaxValidatorsPerSlot = U131072; type GenesisEpoch = U0; type SlotsPerEpoch = U16; type EpochsPerEth1VotingPeriod = U64; diff --git a/consensus/types/src/indexed_attestation.rs b/consensus/types/src/indexed_attestation.rs index 206897c581a..0dd805f1cb8 100644 --- a/consensus/types/src/indexed_attestation.rs +++ b/consensus/types/src/indexed_attestation.rs @@ -59,7 +59,7 @@ pub struct IndexedAttestation { pub attesting_indices: VariableList, #[superstruct(only(Electra), partial_getter(rename = "attesting_indices_electra"))] #[serde(with = "quoted_variable_list_u64")] - pub attesting_indices: VariableList, + pub attesting_indices: VariableList, pub data: AttestationData, pub signature: AggregateSignature, } diff --git a/consensus/types/src/signed_aggregate_and_proof.rs b/consensus/types/src/signed_aggregate_and_proof.rs index e7ffccab60c..9f94849b421 100644 --- a/consensus/types/src/signed_aggregate_and_proof.rs +++ b/consensus/types/src/signed_aggregate_and_proof.rs @@ -2,8 +2,8 @@ use super::{ AggregateAndProof, AggregateAndProofBase, AggregateAndProofElectra, AggregateAndProofRef, }; use super::{ - Attestation, ChainSpec, Domain, EthSpec, Fork, Hash256, SecretKey, SelectionProof, Signature, - SignedRoot, + AttestationRef, ChainSpec, Domain, EthSpec, Fork, Hash256, SecretKey, SelectionProof, + Signature, SignedRoot, }; use crate::test_utils::TestRandom; use crate::Attestation; diff --git a/lcli/src/indexed_attestations.rs b/lcli/src/indexed_attestations.rs index 83252df2049..ccc14171128 100644 --- a/lcli/src/indexed_attestations.rs +++ b/lcli/src/indexed_attestations.rs @@ -33,9 +33,14 @@ pub fn run(matches: &ArgMatches) -> Result<(), String> { let indexed_attestations = attestations .into_iter() - .map(|att| { - let committee = state.get_beacon_committee(att.data().slot, att.data().index)?; - get_indexed_attestation(committee.committee, att.to_ref()) + .map(|att| match att { + Attestation::Base(att) => { + let committee = state.get_beacon_committee(att.data.slot, att.data.index)?; + attesting_indices_base::get_indexed_attestation(committee.committee, &att) + } + Attestation::Electra(att) => { + attesting_indices_electra::get_indexed_attestation_from_state(&state, &att) + } }) .collect::, _>>() .map_err(|e| format!("Error constructing indexed attestation: {:?}", e))?; diff --git a/slasher/src/attester_record.rs b/slasher/src/attester_record.rs index 58d62273194..1cd4ba7d4e0 100644 --- a/slasher/src/attester_record.rs +++ b/slasher/src/attester_record.rs @@ -80,7 +80,7 @@ impl IndexedAttesterRecord { #[derive(Debug, Clone, Encode, Decode, TreeHash)] struct IndexedAttestationHeader { - pub attesting_indices: VariableList, + pub attesting_indices: VariableList, pub data_root: Hash256, pub signature: AggregateSignature, } diff --git a/slasher/src/test_utils.rs b/slasher/src/test_utils.rs index 9bd6e7ab7fb..a5ca34c694c 100644 --- a/slasher/src/test_utils.rs +++ b/slasher/src/test_utils.rs @@ -1,9 +1,10 @@ use std::collections::HashSet; use std::sync::Arc; use types::{ - indexed_attestation::IndexedAttestationBase, AggregateSignature, AttestationData, - AttesterSlashing, AttesterSlashingBase, AttesterSlashingElectra, BeaconBlockHeader, Checkpoint, - Epoch, Hash256, IndexedAttestation, MainnetEthSpec, Signature, SignedBeaconBlockHeader, Slot, + indexed_attestation::{IndexedAttestationBase, IndexedAttestationElectra}, + AggregateSignature, AttestationData, AttesterSlashing, AttesterSlashingBase, + AttesterSlashingElectra, BeaconBlockHeader, Checkpoint, Epoch, Hash256, IndexedAttestation, + MainnetEthSpec, Signature, SignedBeaconBlockHeader, Slot, }; pub type E = MainnetEthSpec; @@ -13,6 +14,31 @@ pub fn indexed_att_electra( source_epoch: u64, target_epoch: u64, target_root: u64, +) -> IndexedAttestation { + IndexedAttestation::Electra(IndexedAttestationElectra { + attesting_indices: attesting_indices.as_ref().to_vec().into(), + data: AttestationData { + slot: Slot::new(0), + index: 0, + beacon_block_root: Hash256::zero(), + source: Checkpoint { + epoch: Epoch::new(source_epoch), + root: Hash256::from_low_u64_be(0), + }, + target: Checkpoint { + epoch: Epoch::new(target_epoch), + root: Hash256::from_low_u64_be(target_root), + }, + }, + signature: AggregateSignature::empty(), + }) +} + +pub fn indexed_att( + attesting_indices: impl AsRef<[u64]>, + source_epoch: u64, + target_epoch: u64, + target_root: u64, ) -> IndexedAttestation { // TODO(electra) make fork-agnostic IndexedAttestation::Base(IndexedAttestationBase { diff --git a/testing/ef_tests/src/type_name.rs b/testing/ef_tests/src/type_name.rs index f886431b400..f4da3f35c5d 100644 --- a/testing/ef_tests/src/type_name.rs +++ b/testing/ef_tests/src/type_name.rs @@ -115,8 +115,8 @@ type_name_generic!(LightClientUpdateDeneb, "LightClientUpdate"); type_name_generic!(PendingAttestation); type_name!(ProposerSlashing); type_name_generic!(SignedAggregateAndProof); -type_name_generic!(SignedAggregateAndProofBase, "SignedAggregateAndProof"); -type_name_generic!(SignedAggregateAndProofElectra, "SignedAggregateAndProof"); +type_name_generic!(SignedAggregateAndProofBase, "SignedAggregateAndProofBase"); +type_name_generic!(SignedAggregateAndProofElectra, "SignedAggregateAndProofElectra"); type_name_generic!(SignedBeaconBlock); type_name!(SignedBeaconBlockHeader); type_name_generic!(SignedContributionAndProof); diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index 60685140100..14cc815cf66 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -245,54 +245,19 @@ mod ssz_static { ssz_static_test!(validator, Validator); ssz_static_test!(voluntary_exit, VoluntaryExit); - #[test] - fn attestation() { - SszStaticHandler::, MinimalEthSpec>::pre_electra().run(); - SszStaticHandler::, MainnetEthSpec>::pre_electra().run(); - SszStaticHandler::, MinimalEthSpec>::electra_only() - .run(); - SszStaticHandler::, MainnetEthSpec>::electra_only() - .run(); - } - - #[test] - fn attester_slashing() { - SszStaticHandler::, MinimalEthSpec>::pre_electra() - .run(); - SszStaticHandler::, MainnetEthSpec>::pre_electra() - .run(); - SszStaticHandler::, MinimalEthSpec>::electra_only() - .run(); - SszStaticHandler::, MainnetEthSpec>::electra_only() - .run(); - } #[test] fn signed_aggregate_and_proof() { - SszStaticHandler::, MinimalEthSpec>::pre_electra( - ) - .run(); - SszStaticHandler::, MainnetEthSpec>::pre_electra( + SszStaticHandler::, MinimalEthSpec>::pre_electra( ) .run(); - SszStaticHandler::, MinimalEthSpec>::electra_only( + SszStaticHandler::, MainnetEthSpec>::pre_electra( ) .run(); - SszStaticHandler::, MainnetEthSpec>::electra_only( - ) - .run(); - } - - #[test] - fn aggregate_and_proof() { - SszStaticHandler::, MinimalEthSpec>::pre_electra() - .run(); - SszStaticHandler::, MainnetEthSpec>::pre_electra() - .run(); - SszStaticHandler::, MinimalEthSpec>::electra_only( + SszStaticHandler::, MinimalEthSpec>::electra_only( ) .run(); - SszStaticHandler::, MainnetEthSpec>::electra_only( + SszStaticHandler::, MainnetEthSpec>::electra_only( ) .run(); } @@ -320,6 +285,22 @@ mod ssz_static { .run(); } + #[test] + fn signed_aggregate_and_proof() { + SszStaticHandler::, MinimalEthSpec>::pre_electra( + ) + .run(); + SszStaticHandler::, MainnetEthSpec>::pre_electra( + ) + .run(); + SszStaticHandler::, MinimalEthSpec>::electra_only( + ) + .run(); + SszStaticHandler::, MainnetEthSpec>::electra_only( + ) + .run(); + } + // Altair and later #[test] fn contribution_and_proof() {