From 5214f7fb48c12844c6d20fee07c0987bd46f3ec8 Mon Sep 17 00:00:00 2001 From: Mark Mackey Date: Fri, 3 May 2024 15:54:35 -0500 Subject: [PATCH] superstruct `AggregateAndProof` --- .../src/attestation_verification.rs | 82 +++++++++-------- beacon_node/beacon_chain/src/beacon_chain.rs | 10 +- .../src/naive_aggregation_pool.rs | 79 ++++++++++------ beacon_node/beacon_chain/src/test_utils.rs | 2 +- .../beacon_chain/src/validator_monitor.rs | 2 +- beacon_node/http_api/src/lib.rs | 8 +- .../http_api/src/publish_attestations.rs | 2 +- beacon_node/http_api/tests/tests.rs | 10 +- .../lighthouse_network/src/types/pubsub.rs | 34 +++++-- .../gossip_methods.rs | 29 +++--- .../src/network_beacon_processor/mod.rs | 2 +- beacon_node/operation_pool/src/lib.rs | 25 ++--- .../per_block_processing/signature_sets.rs | 16 ++-- consensus/types/src/aggregate_and_proof.rs | 92 ++++++++++++++----- consensus/types/src/attestation.rs | 49 ++++++---- consensus/types/src/attester_slashing.rs | 13 +-- consensus/types/src/lib.rs | 8 +- .../types/src/signed_aggregate_and_proof.rs | 71 ++++++++++---- testing/ef_tests/src/cases/fork_choice.rs | 18 ++-- validator_client/src/attestation_service.rs | 8 +- validator_client/src/signing_method.rs | 2 +- .../src/signing_method/web3signer.rs | 2 +- validator_client/src/validator_store.rs | 45 ++++++--- 23 files changed, 399 insertions(+), 210 deletions(-) diff --git a/beacon_node/beacon_chain/src/attestation_verification.rs b/beacon_node/beacon_chain/src/attestation_verification.rs index 1dd43b3c024..dff3fd6f2dd 100644 --- a/beacon_node/beacon_chain/src/attestation_verification.rs +++ b/beacon_node/beacon_chain/src/attestation_verification.rs @@ -55,8 +55,8 @@ use std::borrow::Cow; use strum::AsRefStr; use tree_hash::TreeHash; use types::{ - Attestation, BeaconCommittee, ChainSpec, CommitteeIndex, Epoch, EthSpec, ForkName, Hash256, - IndexedAttestation, SelectionProof, SignedAggregateAndProof, Slot, SubnetId, + Attestation, AttestationRef, BeaconCommittee, ChainSpec, CommitteeIndex, Epoch, EthSpec, + ForkName, Hash256, IndexedAttestation, SelectionProof, SignedAggregateAndProof, Slot, SubnetId, }; pub use batch::{batch_verify_aggregated_attestations, batch_verify_unaggregated_attestations}; @@ -274,7 +274,7 @@ struct IndexedAggregatedAttestation<'a, T: BeaconChainTypes> { /// /// These attestations have *not* undergone signature verification. struct IndexedUnaggregatedAttestation<'a, T: BeaconChainTypes> { - attestation: &'a Attestation, + attestation: AttestationRef<'a, T::EthSpec>, indexed_attestation: IndexedAttestation, subnet_id: SubnetId, validator_index: u64, @@ -295,7 +295,7 @@ impl<'a, T: BeaconChainTypes> VerifiedAggregatedAttestation<'a, T> { /// Wraps an `Attestation` that has been fully verified for propagation on the gossip network. pub struct VerifiedUnaggregatedAttestation<'a, T: BeaconChainTypes> { - attestation: &'a Attestation, + attestation: AttestationRef<'a, T::EthSpec>, indexed_attestation: IndexedAttestation, subnet_id: SubnetId, } @@ -322,20 +322,20 @@ impl<'a, T: BeaconChainTypes> Clone for IndexedUnaggregatedAttestation<'a, T> { /// A helper trait implemented on wrapper types that can be progressed to a state where they can be /// verified for application to fork choice. pub trait VerifiedAttestation: Sized { - fn attestation(&self) -> &Attestation; + fn attestation(&self) -> AttestationRef; fn indexed_attestation(&self) -> &IndexedAttestation; // Inefficient default implementation. This is overridden for gossip verified attestations. fn into_attestation_and_indices(self) -> (Attestation, Vec) { - let attestation = self.attestation().clone(); + let attestation = self.attestation().clone_as_attestation(); let attesting_indices = self.indexed_attestation().attesting_indices_to_vec(); (attestation, attesting_indices) } } impl<'a, T: BeaconChainTypes> VerifiedAttestation for VerifiedAggregatedAttestation<'a, T> { - fn attestation(&self) -> &Attestation { + fn attestation(&self) -> AttestationRef { self.attestation() } @@ -345,7 +345,7 @@ impl<'a, T: BeaconChainTypes> VerifiedAttestation for VerifiedAggregatedAttes } impl<'a, T: BeaconChainTypes> VerifiedAttestation for VerifiedUnaggregatedAttestation<'a, T> { - fn attestation(&self) -> &Attestation { + fn attestation(&self) -> AttestationRef { self.attestation } @@ -357,7 +357,7 @@ impl<'a, T: BeaconChainTypes> VerifiedAttestation for VerifiedUnaggregatedAtt /// Information about invalid attestations which might still be slashable despite being invalid. pub enum AttestationSlashInfo<'a, T: BeaconChainTypes, TErr> { /// The attestation is invalid, but its signature wasn't checked. - SignatureNotChecked(&'a Attestation, TErr), + SignatureNotChecked(AttestationRef<'a, T::EthSpec>, TErr), /// As for `SignatureNotChecked`, but we know the `IndexedAttestation`. SignatureNotCheckedIndexed(IndexedAttestation, TErr), /// The attestation's signature is invalid, so it will never be slashable. @@ -446,7 +446,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> { signed_aggregate: &SignedAggregateAndProof, chain: &BeaconChain, ) -> Result { - let attestation = &signed_aggregate.message.aggregate; + let attestation = signed_aggregate.message().aggregate(); // Ensure attestation is within the last ATTESTATION_PROPAGATION_SLOT_RANGE slots (within a // MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance). @@ -471,14 +471,14 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> { if chain .observed_attestations .write() - .is_known_subset(attestation.to_ref(), attestation_data_root) + .is_known_subset(attestation, attestation_data_root) .map_err(|e| Error::BeaconChainError(e.into()))? { metrics::inc_counter(&metrics::AGGREGATED_ATTESTATION_SUBSETS); return Err(Error::AttestationSupersetKnown(attestation_data_root)); } - let aggregator_index = signed_aggregate.message.aggregator_index; + let aggregator_index = signed_aggregate.message().aggregator_index(); // Ensure there has been no other observed aggregate for the given `aggregator_index`. // @@ -532,11 +532,16 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> { ) -> Result> { use AttestationSlashInfo::*; - let attestation = &signed_aggregate.message.aggregate; - let aggregator_index = signed_aggregate.message.aggregator_index; + let attestation = signed_aggregate.message().aggregate(); + let aggregator_index = signed_aggregate.message().aggregator_index(); let attestation_data_root = match Self::verify_early_checks(signed_aggregate, chain) { Ok(root) => root, - Err(e) => return Err(SignatureNotChecked(&signed_aggregate.message.aggregate, e)), + Err(e) => { + return Err(SignatureNotChecked( + signed_aggregate.message().aggregate(), + e, + )) + } }; let get_indexed_attestation_with_committee = @@ -545,7 +550,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> { // // Future optimizations should remove this clone. let selection_proof = - SelectionProof::from(signed_aggregate.message.selection_proof.clone()); + SelectionProof::from(signed_aggregate.message().selection_proof().clone()); if !selection_proof .is_aggregator(committee.committee.len(), &chain.spec) @@ -559,7 +564,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> { return Err(Error::AggregatorNotInCommittee { aggregator_index }); } - get_indexed_attestation(committee.committee, attestation.to_ref()) + get_indexed_attestation(committee.committee, attestation) .map_err(|e| BeaconChainError::from(e).into()) }; @@ -569,7 +574,12 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> { get_indexed_attestation_with_committee, ) { Ok(indexed_attestation) => indexed_attestation, - Err(e) => return Err(SignatureNotChecked(&signed_aggregate.message.aggregate, e)), + Err(e) => { + return Err(SignatureNotChecked( + signed_aggregate.message().aggregate(), + e, + )) + } }; Ok(IndexedAggregatedAttestation { @@ -587,8 +597,8 @@ impl<'a, T: BeaconChainTypes> VerifiedAggregatedAttestation<'a, T> { attestation_data_root: Hash256, chain: &BeaconChain, ) -> Result<(), Error> { - let attestation = &signed_aggregate.message.aggregate; - let aggregator_index = signed_aggregate.message.aggregator_index; + let attestation = signed_aggregate.message().aggregate(); + let aggregator_index = signed_aggregate.message().aggregator_index(); // Observe the valid attestation so we do not re-process it. // @@ -597,7 +607,7 @@ impl<'a, T: BeaconChainTypes> VerifiedAggregatedAttestation<'a, T> { if let ObserveOutcome::Subset = chain .observed_attestations .write() - .observe_item(attestation.to_ref(), Some(attestation_data_root)) + .observe_item(attestation, Some(attestation_data_root)) .map_err(|e| Error::BeaconChainError(e.into()))? { metrics::inc_counter(&metrics::AGGREGATED_ATTESTATION_SUBSETS); @@ -696,8 +706,8 @@ impl<'a, T: BeaconChainTypes> VerifiedAggregatedAttestation<'a, T> { } /// Returns the underlying `attestation` for the `signed_aggregate`. - pub fn attestation(&self) -> &Attestation { - &self.signed_aggregate.message.aggregate + pub fn attestation(&self) -> AttestationRef<'a, T::EthSpec> { + self.signed_aggregate.message().aggregate() } /// Returns the underlying `signed_aggregate`. @@ -709,7 +719,7 @@ impl<'a, T: BeaconChainTypes> VerifiedAggregatedAttestation<'a, T> { impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> { /// Run the checks that happen before an indexed attestation is constructed. pub fn verify_early_checks( - attestation: &Attestation, + attestation: AttestationRef, chain: &BeaconChain, ) -> Result<(), Error> { let attestation_epoch = attestation.data().slot.epoch(T::EthSpec::slots_per_epoch()); @@ -750,7 +760,7 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> { /// Run the checks that apply to the indexed attestation before the signature is checked. pub fn verify_middle_checks( - attestation: &Attestation, + attestation: AttestationRef, indexed_attestation: &IndexedAttestation, committees_per_slot: u64, subnet_id: Option, @@ -806,7 +816,7 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> { subnet_id: Option, chain: &BeaconChain, ) -> Result { - Self::verify_slashable(attestation, subnet_id, chain) + Self::verify_slashable(attestation.to_ref(), subnet_id, chain) .map(|verified_unaggregated| { if let Some(slasher) = chain.slasher.as_ref() { slasher.accept_attestation(verified_unaggregated.indexed_attestation.clone()); @@ -818,7 +828,7 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> { /// Verify the attestation, producing extra information about whether it might be slashable. pub fn verify_slashable( - attestation: &'a Attestation, + attestation: AttestationRef<'a, T::EthSpec>, subnet_id: Option, chain: &BeaconChain, ) -> Result> { @@ -867,7 +877,7 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> { impl<'a, T: BeaconChainTypes> VerifiedUnaggregatedAttestation<'a, T> { /// Run the checks that apply after the signature has been checked. fn verify_late_checks( - attestation: &Attestation, + attestation: AttestationRef, validator_index: u64, chain: &BeaconChain, ) -> Result<(), Error> { @@ -961,7 +971,7 @@ impl<'a, T: BeaconChainTypes> VerifiedUnaggregatedAttestation<'a, T> { } /// Returns the wrapped `attestation`. - pub fn attestation(&self) -> &Attestation { + pub fn attestation(&self) -> AttestationRef { self.attestation } @@ -991,7 +1001,7 @@ impl<'a, T: BeaconChainTypes> VerifiedUnaggregatedAttestation<'a, T> { /// already finalized. fn verify_head_block_is_known( chain: &BeaconChain, - attestation: &Attestation, + attestation: AttestationRef, max_skip_slots: Option, ) -> Result { let block_opt = chain @@ -1039,7 +1049,7 @@ fn verify_head_block_is_known( /// Accounts for `MAXIMUM_GOSSIP_CLOCK_DISPARITY`. pub fn verify_propagation_slot_range( slot_clock: &S, - attestation: &Attestation, + attestation: AttestationRef, spec: &ChainSpec, ) -> Result<(), Error> { let attestation_slot = attestation.data().slot; @@ -1124,7 +1134,7 @@ pub fn verify_attestation_signature( /// `attestation.data.beacon_block_root`. pub fn verify_attestation_target_root( head_block: &ProtoBlock, - attestation: &Attestation, + attestation: AttestationRef, ) -> Result<(), Error> { // Check the attestation target root. let head_block_epoch = head_block.slot.epoch(E::slots_per_epoch()); @@ -1193,7 +1203,7 @@ pub fn verify_signed_aggregate_signatures( .try_read_for(VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT) .ok_or(BeaconChainError::ValidatorPubkeyCacheLockTimeout)?; - let aggregator_index = signed_aggregate.message.aggregator_index; + let aggregator_index = signed_aggregate.message().aggregator_index(); if aggregator_index >= pubkey_cache.len() as u64 { return Err(Error::AggregatorPubkeyUnknown(aggregator_index)); } @@ -1240,10 +1250,10 @@ type CommitteesPerSlot = u64; /// public keys cached in the `chain`. pub fn obtain_indexed_attestation_and_committees_per_slot( chain: &BeaconChain, - attestation: &Attestation, + attestation: AttestationRef, ) -> Result<(IndexedAttestation, CommitteesPerSlot), Error> { map_attestation_committee(chain, attestation, |(committee, committees_per_slot)| { - get_indexed_attestation(committee.committee, attestation.to_ref()) + get_indexed_attestation(committee.committee, attestation) .map(|attestation| (attestation, committees_per_slot)) .map_err(Error::Invalid) }) @@ -1260,7 +1270,7 @@ pub fn obtain_indexed_attestation_and_committees_per_slot( /// from disk and then update the `shuffling_cache`. fn map_attestation_committee( chain: &BeaconChain, - attestation: &Attestation, + attestation: AttestationRef, map_fn: F, ) -> Result where diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index a99b5db6051..cd47068977b 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1966,8 +1966,9 @@ impl BeaconChain { // This method is called for API and gossip attestations, so this covers all unaggregated attestation events if let Some(event_handler) = self.event_handler.as_ref() { if event_handler.has_attestation_subscribers() { - event_handler - .register(EventKind::Attestation(Box::new(v.attestation().clone()))); + event_handler.register(EventKind::Attestation(Box::new( + v.attestation().clone_as_attestation(), + ))); } } metrics::inc_counter(&metrics::UNAGGREGATED_ATTESTATION_PROCESSING_SUCCESSES); @@ -2003,8 +2004,9 @@ impl BeaconChain { // This method is called for API and gossip attestations, so this covers all aggregated attestation events if let Some(event_handler) = self.event_handler.as_ref() { if event_handler.has_attestation_subscribers() { - event_handler - .register(EventKind::Attestation(Box::new(v.attestation().clone()))); + event_handler.register(EventKind::Attestation(Box::new( + v.attestation().clone_as_attestation(), + ))); } } metrics::inc_counter(&metrics::AGGREGATED_ATTESTATION_PROCESSING_SUCCESSES); diff --git a/beacon_node/beacon_chain/src/naive_aggregation_pool.rs b/beacon_node/beacon_chain/src/naive_aggregation_pool.rs index 0eab13a4c1d..a12521cd171 100644 --- a/beacon_node/beacon_chain/src/naive_aggregation_pool.rs +++ b/beacon_node/beacon_chain/src/naive_aggregation_pool.rs @@ -1,10 +1,13 @@ use crate::metrics; +use crate::observed_aggregates::AsReference; use std::collections::HashMap; use tree_hash::TreeHash; use types::consts::altair::SYNC_COMMITTEE_SUBNET_COUNT; use types::slot_data::SlotData; use types::sync_committee_contribution::SyncContributionData; -use types::{Attestation, AttestationData, EthSpec, Hash256, Slot, SyncCommitteeContribution}; +use types::{ + Attestation, AttestationData, AttestationRef, EthSpec, Hash256, Slot, SyncCommitteeContribution, +}; type AttestationDataRoot = Hash256; type SyncDataRoot = Hash256; @@ -59,12 +62,15 @@ pub enum Error { /// Implemented for items in the `NaiveAggregationPool`. Requires that items implement `SlotData`, /// which means they have an associated slot. This handles aggregation of items that are inserted. -pub trait AggregateMap { +pub trait AggregateMap +where + for<'a> ::Reference<'a>: SlotData, +{ /// `Key` should be a hash of `Data`. type Key; /// The item stored in the map - type Value: Clone + SlotData; + type Value: Clone + SlotData + AsReference; /// The unique fields of `Value`, hashed to create `Key`. type Data: SlotData; @@ -73,7 +79,10 @@ pub trait AggregateMap { fn new(initial_capacity: usize) -> Self; /// Insert a `Value` into `Self`, returning a result. - fn insert(&mut self, value: &Self::Value) -> Result; + fn insert( + &mut self, + value: ::Reference<'_>, + ) -> Result; /// Get a `Value` from `Self` based on `Data`. fn get(&self, data: &Self::Data) -> Option; @@ -121,18 +130,18 @@ impl AggregateMap for AggregatedAttestationMap { /// Insert an attestation into `self`, aggregating it into the pool. /// /// The given attestation (`a`) must only have one signature. - fn insert(&mut self, a: &Self::Value) -> Result { + fn insert(&mut self, a: AttestationRef) -> Result { let _timer = metrics::start_timer(&metrics::ATTESTATION_PROCESSING_AGG_POOL_CORE_INSERT); let set_bits = match a { - Attestation::Base(att) => att + AttestationRef::Base(att) => att .aggregation_bits .iter() .enumerate() .filter(|(_i, bit)| *bit) .map(|(i, _bit)| i) .collect::>(), - Attestation::Electra(att) => att + AttestationRef::Electra(att) => att .aggregation_bits .iter() .enumerate() @@ -169,7 +178,8 @@ impl AggregateMap for AggregatedAttestationMap { return Err(Error::ReachedMaxItemsPerSlot(MAX_ATTESTATIONS_PER_SLOT)); } - self.map.insert(attestation_data_root, a.clone()); + self.map + .insert(attestation_data_root, a.clone_as_attestation()); Ok(InsertOutcome::NewItemInserted { committee_index }) } } @@ -344,12 +354,20 @@ impl AggregateMap for SyncContributionAggregateMap { /// `current_slot - SLOTS_RETAINED` will be removed and any future item with a slot lower /// than that will also be refused. Pruning is done automatically based upon the items it /// receives and it can be triggered manually. -pub struct NaiveAggregationPool { +pub struct NaiveAggregationPool +where + T: AggregateMap, + for<'a> ::Reference<'a>: SlotData, +{ lowest_permissible_slot: Slot, maps: HashMap, } -impl Default for NaiveAggregationPool { +impl Default for NaiveAggregationPool +where + T: AggregateMap, + for<'a> ::Reference<'a>: SlotData, +{ fn default() -> Self { Self { lowest_permissible_slot: Slot::new(0), @@ -358,7 +376,11 @@ impl Default for NaiveAggregationPool { } } -impl NaiveAggregationPool { +impl NaiveAggregationPool +where + T: AggregateMap, + for<'a> ::Reference<'a>: SlotData, +{ /// Insert an item into `self`, aggregating it into the pool. /// /// The given item must only have one signature and have an @@ -366,7 +388,10 @@ impl NaiveAggregationPool { /// /// The pool may be pruned if the given item has a slot higher than any /// previously seen. - pub fn insert(&mut self, item: &T::Value) -> Result { + pub fn insert( + &mut self, + item: ::Reference<'_>, + ) -> Result { let _timer = T::start_insert_timer(); let slot = item.get_slot(); let lowest_permissible_slot = self.lowest_permissible_slot; @@ -602,10 +627,10 @@ mod tests { let mut a = $get_method_name(Slot::new(0)); let mut pool: NaiveAggregationPool<$map_type> = - NaiveAggregationPool::default(); + NaiveAggregationPool::<$map_type>::default(); assert_eq!( - pool.insert(&a), + pool.insert(a.as_reference()), Err(Error::NoAggregationBitsSet), "should not accept item without any signatures" ); @@ -613,12 +638,12 @@ mod tests { $sign_method_name(&mut a, 0, Hash256::random()); assert_eq!( - pool.insert(&a), + pool.insert(a.as_reference()), Ok(InsertOutcome::NewItemInserted { committee_index: 0 }), "should accept new item" ); assert_eq!( - pool.insert(&a), + pool.insert(a.as_reference()), Ok(InsertOutcome::SignatureAlreadyKnown { committee_index: 0 }), "should acknowledge duplicate signature" ); @@ -631,7 +656,7 @@ mod tests { $sign_method_name(&mut a, 1, Hash256::random()); assert_eq!( - pool.insert(&a), + pool.insert(a.as_reference()), Err(Error::MoreThanOneAggregationBitSet(2)), "should not accept item with multiple signatures" ); @@ -647,15 +672,15 @@ mod tests { $sign_method_name(&mut a_1, 1, genesis_validators_root); let mut pool: NaiveAggregationPool<$map_type> = - NaiveAggregationPool::default(); + NaiveAggregationPool::<$map_type>::default(); assert_eq!( - pool.insert(&a_0), + pool.insert(a_0.as_reference()), Ok(InsertOutcome::NewItemInserted { committee_index: 0 }), "should accept a_0" ); assert_eq!( - pool.insert(&a_1), + pool.insert(a_1.as_reference()), Ok(InsertOutcome::SignatureAggregated { committee_index: 1 }), "should accept a_1" ); @@ -665,7 +690,7 @@ mod tests { .expect("should not error while getting attestation"); let mut a_01 = a_0.clone(); - a_01.aggregate(&a_1); + a_01.aggregate(a_1.as_reference()); assert_eq!(retrieved, a_01, "retrieved item should be aggregated"); @@ -681,7 +706,7 @@ mod tests { $block_root_mutator(&mut a_different, different_root); assert_eq!( - pool.insert(&a_different), + pool.insert(a_different.as_reference()), Ok(InsertOutcome::NewItemInserted { committee_index: 2 }), "should accept a_different" ); @@ -700,7 +725,7 @@ mod tests { $sign_method_name(&mut base, 0, Hash256::random()); let mut pool: NaiveAggregationPool<$map_type> = - NaiveAggregationPool::default(); + NaiveAggregationPool::<$map_type>::default(); for i in 0..SLOTS_RETAINED * 2 { let slot = Slot::from(i); @@ -708,7 +733,7 @@ mod tests { $slot_mutator(&mut a, slot); assert_eq!( - pool.insert(&a), + pool.insert(a.as_reference()), Ok(InsertOutcome::NewItemInserted { committee_index: 0 }), "should accept new item" ); @@ -749,7 +774,7 @@ mod tests { $sign_method_name(&mut base, 0, Hash256::random()); let mut pool: NaiveAggregationPool<$map_type> = - NaiveAggregationPool::default(); + NaiveAggregationPool::<$map_type>::default(); for i in 0..=$item_limit { let mut a = base.clone(); @@ -757,13 +782,13 @@ mod tests { if i < $item_limit { assert_eq!( - pool.insert(&a), + pool.insert(a.as_reference()), Ok(InsertOutcome::NewItemInserted { committee_index: 0 }), "should accept item below limit" ); } else { assert_eq!( - pool.insert(&a), + pool.insert(a.as_reference()), Err(Error::ReachedMaxItemsPerSlot($item_limit)), "should not accept item above limit" ); diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 613411d8a2d..ff439742d11 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -1358,7 +1358,7 @@ where committee_attestations.iter().skip(1).fold( attestation.clone(), |mut agg, (att, _)| { - agg.aggregate(att); + agg.aggregate(att.to_ref()); agg }, ) diff --git a/beacon_node/beacon_chain/src/validator_monitor.rs b/beacon_node/beacon_chain/src/validator_monitor.rs index ea0fe46caeb..058da369bcb 100644 --- a/beacon_node/beacon_chain/src/validator_monitor.rs +++ b/beacon_node/beacon_chain/src/validator_monitor.rs @@ -1331,7 +1331,7 @@ impl ValidatorMonitor { slot_clock, ); - let aggregator_index = signed_aggregate_and_proof.message.aggregator_index; + let aggregator_index = signed_aggregate_and_proof.message().aggregator_index(); if let Some(validator) = self.get_validator(aggregator_index) { let id = &validator.id; diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 5580f820fbb..6cb8f6fe0b9 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -3368,9 +3368,9 @@ pub fn serve( "Failure verifying aggregate and proofs"; "error" => format!("{:?}", e), "request_index" => index, - "aggregator_index" => aggregate.message.aggregator_index, - "attestation_index" => aggregate.message.aggregate.data().index, - "attestation_slot" => aggregate.message.aggregate.data().slot, + "aggregator_index" => aggregate.message().aggregator_index(), + "attestation_index" => aggregate.message().aggregate().data().index, + "attestation_slot" => aggregate.message().aggregate().data().slot, ); failures.push(api_types::Failure::new(index, format!("Verification: {:?}", e))); } @@ -3389,7 +3389,7 @@ pub fn serve( "Failure applying verified aggregate attestation to fork choice"; "error" => format!("{:?}", e), "request_index" => index, - "aggregator_index" => verified_aggregate.aggregate().message.aggregator_index, + "aggregator_index" => verified_aggregate.aggregate().message().aggregator_index(), "attestation_index" => verified_aggregate.attestation().data().index, "attestation_slot" => verified_aggregate.attestation().data().slot, ); diff --git a/beacon_node/http_api/src/publish_attestations.rs b/beacon_node/http_api/src/publish_attestations.rs index 8eaee093c1a..541ba8b7871 100644 --- a/beacon_node/http_api/src/publish_attestations.rs +++ b/beacon_node/http_api/src/publish_attestations.rs @@ -87,7 +87,7 @@ fn verify_and_publish_attestation( .send(NetworkMessage::Publish { messages: vec![PubsubMessage::Attestation(Box::new(( attestation.subnet_id(), - attestation.attestation().clone(), + attestation.attestation().clone_as_attestation(), )))], }) .map_err(|_| Error::Publication)?; diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 790cc5b70b4..3653929bdab 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -3327,8 +3327,14 @@ impl ApiTester { pub async fn test_get_validator_aggregate_and_proofs_invalid(mut self) -> Self { let mut aggregate = self.get_aggregate().await; - - aggregate.message.aggregate.data_mut().slot += 1; + match &mut aggregate { + SignedAggregateAndProof::Base(ref mut aggregate) => { + aggregate.message.aggregate.data.slot += 1; + } + SignedAggregateAndProof::Electra(ref mut aggregate) => { + aggregate.message.aggregate.data.slot += 1; + } + } self.client .post_validator_aggregate_and_proof::(&[aggregate]) diff --git a/beacon_node/lighthouse_network/src/types/pubsub.rs b/beacon_node/lighthouse_network/src/types/pubsub.rs index 4af0f54f31e..8945c23a549 100644 --- a/beacon_node/lighthouse_network/src/types/pubsub.rs +++ b/beacon_node/lighthouse_network/src/types/pubsub.rs @@ -9,7 +9,8 @@ use std::sync::Arc; use types::{ Attestation, AttesterSlashing, AttesterSlashingBase, AttesterSlashingElectra, BlobSidecar, EthSpec, ForkContext, ForkName, LightClientFinalityUpdate, LightClientOptimisticUpdate, - ProposerSlashing, SignedAggregateAndProof, SignedBeaconBlock, SignedBeaconBlockAltair, + ProposerSlashing, SignedAggregateAndProof, SignedAggregateAndProofBase, + SignedAggregateAndProofElectra, SignedBeaconBlock, SignedBeaconBlockAltair, SignedBeaconBlockBase, SignedBeaconBlockBellatrix, SignedBeaconBlockCapella, SignedBeaconBlockDeneb, SignedBeaconBlockElectra, SignedBlsToExecutionChange, SignedContributionAndProof, SignedVoluntaryExit, SubnetId, SyncCommitteeMessage, SyncSubnetId, @@ -154,10 +155,29 @@ impl PubsubMessage { // the ssz decoders match gossip_topic.kind() { GossipKind::BeaconAggregateAndProof => { - let agg_and_proof = SignedAggregateAndProof::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?; + let signed_aggregate_and_proof = + match fork_context.from_context_bytes(gossip_topic.fork_digest) { + Some(ForkName::Base) + | Some(ForkName::Altair) + | Some(ForkName::Bellatrix) + | Some(ForkName::Capella) + | Some(ForkName::Deneb) => SignedAggregateAndProof::Base( + SignedAggregateAndProofBase::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ), + Some(ForkName::Electra) => SignedAggregateAndProof::Electra( + SignedAggregateAndProofElectra::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ), + None => { + return Err(format!( + "Unknown gossipsub fork digest: {:?}", + gossip_topic.fork_digest + )) + } + }; Ok(PubsubMessage::AggregateAndProofAttestation(Box::new( - agg_and_proof, + signed_aggregate_and_proof, ))) } GossipKind::Attestation(subnet_id) => { @@ -362,9 +382,9 @@ impl std::fmt::Display for PubsubMessage { PubsubMessage::AggregateAndProofAttestation(att) => write!( f, "Aggregate and Proof: slot: {}, index: {}, aggregator_index: {}", - att.message.aggregate.data().slot, - att.message.aggregate.data().index, - att.message.aggregator_index, + att.message().aggregate().data().slot, + att.message().aggregate().data().index, + att.message().aggregator_index(), ), PubsubMessage::Attestation(data) => write!( f, 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 6dc671243d5..d41233c903b 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -31,8 +31,8 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use store::hot_cold_store::HotColdDBError; use tokio::sync::mpsc; use types::{ - Attestation, AttesterSlashing, BlobSidecar, EthSpec, Hash256, IndexedAttestation, - LightClientFinalityUpdate, LightClientOptimisticUpdate, ProposerSlashing, + Attestation, AttestationRef, AttesterSlashing, BlobSidecar, EthSpec, Hash256, + IndexedAttestation, LightClientFinalityUpdate, LightClientOptimisticUpdate, ProposerSlashing, SignedAggregateAndProof, SignedBeaconBlock, SignedBlsToExecutionChange, SignedContributionAndProof, SignedVoluntaryExit, Slot, SubnetId, SyncCommitteeMessage, SyncSubnetId, @@ -62,8 +62,8 @@ struct VerifiedUnaggregate { /// This implementation allows `Self` to be imported to fork choice and other functions on the /// `BeaconChain`. impl VerifiedAttestation for VerifiedUnaggregate { - fn attestation(&self) -> &Attestation { - &self.attestation + fn attestation(&self) -> AttestationRef { + self.attestation.to_ref() } fn indexed_attestation(&self) -> &IndexedAttestation { @@ -95,8 +95,8 @@ struct VerifiedAggregate { /// This implementation allows `Self` to be imported to fork choice and other functions on the /// `BeaconChain`. impl VerifiedAttestation for VerifiedAggregate { - fn attestation(&self) -> &Attestation { - &self.signed_aggregate.message.aggregate + fn attestation(&self) -> AttestationRef { + self.signed_aggregate.message().aggregate() } fn indexed_attestation(&self) -> &IndexedAttestation { @@ -105,7 +105,12 @@ impl VerifiedAttestation for VerifiedAggregate { /// Efficient clone-free implementation that moves out of the `Box`. fn into_attestation_and_indices(self) -> (Attestation, Vec) { - let attestation = self.signed_aggregate.message.aggregate; + // TODO(electra): technically we shouldn't have to clone.. + let attestation = self + .signed_aggregate + .message() + .aggregate() + .clone_as_attestation(); let attesting_indices = self.indexed_attestation.attesting_indices_to_vec(); (attestation, attesting_indices) } @@ -143,10 +148,10 @@ impl FailedAtt { } } - pub fn attestation(&self) -> &Attestation { + pub fn attestation(&self) -> AttestationRef { match self { - FailedAtt::Unaggregate { attestation, .. } => attestation, - FailedAtt::Aggregate { attestation, .. } => &attestation.message.aggregate, + FailedAtt::Unaggregate { attestation, .. } => attestation.to_ref(), + FailedAtt::Aggregate { attestation, .. } => attestation.message().aggregate(), } } } @@ -412,7 +417,7 @@ impl NetworkBeaconProcessor { reprocess_tx: Option>, seen_timestamp: Duration, ) { - let beacon_block_root = aggregate.message.aggregate.data().beacon_block_root; + let beacon_block_root = aggregate.message().aggregate().data().beacon_block_root; let result = match self .chain @@ -2727,7 +2732,7 @@ impl NetworkBeaconProcessor { /// timely), propagate it on gossip. Otherwise, ignore it. fn propagate_attestation_if_timely( &self, - attestation: &Attestation, + attestation: AttestationRef, message_id: MessageId, peer_id: PeerId, ) { diff --git a/beacon_node/network/src/network_beacon_processor/mod.rs b/beacon_node/network/src/network_beacon_processor/mod.rs index b85be1b0c15..cc36e5cc905 100644 --- a/beacon_node/network/src/network_beacon_processor/mod.rs +++ b/beacon_node/network/src/network_beacon_processor/mod.rs @@ -144,7 +144,7 @@ impl NetworkBeaconProcessor { processor.process_gossip_aggregate_batch(aggregates, Some(reprocess_tx)) }; - let beacon_block_root = aggregate.message.aggregate.data().beacon_block_root; + let beacon_block_root = aggregate.message().aggregate().data().beacon_block_root; self.try_send(BeaconWorkEvent { drop_during_sync: true, work: Work::GossipAggregate { diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index f700eecf6bd..c2dc2732020 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -894,7 +894,7 @@ mod release_tests { ); for (atts, aggregate) in &attestations { - let att2 = aggregate.as_ref().unwrap().message.aggregate.clone(); + let att2 = aggregate.as_ref().unwrap().message().aggregate().clone(); let att1 = atts .into_iter() @@ -902,7 +902,7 @@ mod release_tests { .take(2) .fold::>, _>(None, |att, new_att| { if let Some(mut a) = att { - a.aggregate(&new_att); + a.aggregate(new_att.to_ref()); Some(a) } else { Some(new_att.clone()) @@ -911,9 +911,9 @@ mod release_tests { .unwrap(); let att1_indices = get_attesting_indices_from_state(&state, att1.to_ref()).unwrap(); - let att2_indices = get_attesting_indices_from_state(&state, att2.to_ref()).unwrap(); + let att2_indices = get_attesting_indices_from_state(&state, att2).unwrap(); let att1_split = SplitAttestation::new(att1.clone(), att1_indices); - let att2_split = SplitAttestation::new(att2.clone(), att2_indices); + let att2_split = SplitAttestation::new(att2.clone_as_attestation(), att2_indices); assert_eq!( att1.num_set_aggregation_bits(), @@ -1054,12 +1054,13 @@ mod release_tests { ); for (_, aggregate) in attestations { - let att = aggregate.unwrap().message.aggregate; - let attesting_indices = get_attesting_indices_from_state(&state, att.to_ref()).unwrap(); + let agg = aggregate.unwrap(); + let att = agg.message().aggregate(); + let attesting_indices = get_attesting_indices_from_state(&state, att).unwrap(); op_pool - .insert_attestation(att.clone(), attesting_indices.clone()) + .insert_attestation(att.clone_as_attestation(), attesting_indices.clone()) .unwrap(); - op_pool.insert_attestation(att, attesting_indices).unwrap(); + op_pool.insert_attestation(att.clone_as_attestation(), attesting_indices).unwrap(); } assert_eq!(op_pool.num_attestations(), committees.len()); @@ -1108,7 +1109,7 @@ mod release_tests { None, |att, new_att| { if let Some(mut a) = att { - a.aggregate(new_att); + a.aggregate(new_att.to_ref()); Some(a) } else { Some(new_att.clone()) @@ -1131,7 +1132,7 @@ mod release_tests { None, |att, new_att| { if let Some(mut a) = att { - a.aggregate(new_att); + a.aggregate(new_att.to_ref()); Some(a) } else { Some(new_att.clone()) @@ -1208,7 +1209,7 @@ mod release_tests { .fold::, _>( att_0.clone(), |mut att, new_att| { - att.aggregate(new_att); + att.aggregate(new_att.to_ref()); att }, ) @@ -1304,7 +1305,7 @@ mod release_tests { .fold::, _>( att_0.clone(), |mut att, new_att| { - att.aggregate(new_att); + att.aggregate(new_att.to_ref()); att }, ) diff --git a/consensus/state_processing/src/per_block_processing/signature_sets.rs b/consensus/state_processing/src/per_block_processing/signature_sets.rs index 9885daaab0a..a179583556f 100644 --- a/consensus/state_processing/src/per_block_processing/signature_sets.rs +++ b/consensus/state_processing/src/per_block_processing/signature_sets.rs @@ -425,7 +425,7 @@ where E: EthSpec, F: Fn(usize) -> Option>, { - let slot = signed_aggregate_and_proof.message.aggregate.data().slot; + let slot = signed_aggregate_and_proof.message().aggregate().data().slot; let domain = spec.get_domain( slot.epoch(E::slots_per_epoch()), @@ -434,8 +434,8 @@ where genesis_validators_root, ); let message = slot.signing_root(domain); - let signature = &signed_aggregate_and_proof.message.selection_proof; - let validator_index = signed_aggregate_and_proof.message.aggregator_index; + let signature = signed_aggregate_and_proof.message().selection_proof(); + let validator_index = signed_aggregate_and_proof.message().aggregator_index(); Ok(SignatureSet::single_pubkey( signature, @@ -456,8 +456,8 @@ where F: Fn(usize) -> Option>, { let target_epoch = signed_aggregate_and_proof - .message - .aggregate + .message() + .aggregate() .data() .target .epoch; @@ -468,9 +468,9 @@ where fork, genesis_validators_root, ); - let message = signed_aggregate_and_proof.message.signing_root(domain); - let signature = &signed_aggregate_and_proof.signature; - let validator_index = signed_aggregate_and_proof.message.aggregator_index; + let message = signed_aggregate_and_proof.message().signing_root(domain); + let signature = signed_aggregate_and_proof.signature(); + let validator_index = signed_aggregate_and_proof.message().aggregator_index(); Ok(SignatureSet::single_pubkey( signature, diff --git a/consensus/types/src/aggregate_and_proof.rs b/consensus/types/src/aggregate_and_proof.rs index 0297c6d6845..b0dbdadb952 100644 --- a/consensus/types/src/aggregate_and_proof.rs +++ b/consensus/types/src/aggregate_and_proof.rs @@ -1,41 +1,79 @@ +use super::{Attestation, AttestationBase, AttestationElectra, AttestationRef}; use super::{ - Attestation, ChainSpec, Domain, EthSpec, Fork, Hash256, PublicKey, SecretKey, SelectionProof, - Signature, SignedRoot, + ChainSpec, Domain, EthSpec, Fork, Hash256, PublicKey, SecretKey, SelectionProof, Signature, + SignedRoot, }; use crate::test_utils::TestRandom; use serde::{Deserialize, Serialize}; use ssz_derive::{Decode, Encode}; +use superstruct::superstruct; use test_random_derive::TestRandom; use tree_hash_derive::TreeHash; -/// A Validators aggregate attestation and selection proof. -/// -/// Spec v0.12.1 +#[superstruct( + variants(Base, Electra), + variant_attributes( + derive( + arbitrary::Arbitrary, + Debug, + Clone, + PartialEq, + Serialize, + Deserialize, + Encode, + Decode, + TestRandom, + TreeHash, + ), + serde(bound = "E: EthSpec"), + arbitrary(bound = "E: EthSpec"), + ), + ref_attributes( + derive(Debug, PartialEq, TreeHash, Serialize,), + serde(bound = "E: EthSpec"), + tree_hash(enum_behaviour = "transparent") + ) +)] #[derive( - arbitrary::Arbitrary, - Debug, - Clone, - PartialEq, - Serialize, - Deserialize, - Encode, - Decode, - TestRandom, - TreeHash, + arbitrary::Arbitrary, Debug, Clone, PartialEq, Serialize, Deserialize, Encode, TreeHash, )] -#[serde(bound = "E: EthSpec")] +#[serde(untagged)] +#[tree_hash(enum_behaviour = "transparent")] +#[ssz(enum_behaviour = "transparent")] +#[serde(bound = "E: EthSpec", deny_unknown_fields)] #[arbitrary(bound = "E: EthSpec")] pub struct AggregateAndProof { /// The index of the validator that created the attestation. #[serde(with = "serde_utils::quoted_u64")] + #[superstruct(getter(copy))] pub aggregator_index: u64, /// The aggregate attestation. + #[superstruct(flatten)] pub aggregate: Attestation, /// A proof provided by the validator that permits them to publish on the /// `beacon_aggregate_and_proof` gossipsub topic. pub selection_proof: Signature, } +impl<'a, E: EthSpec> AggregateAndProofRef<'a, E> { + /// Returns `true` if `validator_pubkey` signed over `self.aggregate.data.slot`. + pub fn aggregate(self) -> AttestationRef<'a, E> { + match self { + AggregateAndProofRef::Base(a) => AttestationRef::Base(&a.aggregate), + AggregateAndProofRef::Electra(a) => AttestationRef::Electra(&a.aggregate), + } + } +} +impl AggregateAndProof { + /// Returns `true` if `validator_pubkey` signed over `self.aggregate.data.slot`. + pub fn aggregate(&self) -> AttestationRef { + match self { + AggregateAndProof::Base(a) => AttestationRef::Base(&a.aggregate), + AggregateAndProof::Electra(a) => AttestationRef::Electra(&a.aggregate), + } + } +} + impl AggregateAndProof { /// Produces a new `AggregateAndProof` with a `selection_proof` generated by signing /// `aggregate.data.slot` with `secret_key`. @@ -62,10 +100,17 @@ impl AggregateAndProof { }) .into(); - Self { - aggregator_index, - aggregate, - selection_proof, + match aggregate { + Attestation::Base(attestation) => Self::Base(AggregateAndProofBase { + aggregator_index, + aggregate: attestation, + selection_proof, + }), + Attestation::Electra(attestation) => Self::Electra(AggregateAndProofElectra { + aggregator_index, + aggregate: attestation, + selection_proof, + }), } } @@ -77,16 +122,17 @@ impl AggregateAndProof { genesis_validators_root: Hash256, spec: &ChainSpec, ) -> bool { - let target_epoch = self.aggregate.data().slot.epoch(E::slots_per_epoch()); + let target_epoch = self.aggregate().data().slot.epoch(E::slots_per_epoch()); let domain = spec.get_domain( target_epoch, Domain::SelectionProof, fork, genesis_validators_root, ); - let message = self.aggregate.data().slot.signing_root(domain); - self.selection_proof.verify(validator_pubkey, message) + let message = self.aggregate().data().slot.signing_root(domain); + self.selection_proof().verify(validator_pubkey, message) } } impl SignedRoot for AggregateAndProof {} +impl<'a, E: EthSpec> SignedRoot for AggregateAndProofRef<'a, E> {} diff --git a/consensus/types/src/attestation.rs b/consensus/types/src/attestation.rs index 0d3f29ae289..4676a100be6 100644 --- a/consensus/types/src/attestation.rs +++ b/consensus/types/src/attestation.rs @@ -42,7 +42,8 @@ pub enum Error { derivative(PartialEq, Hash(bound = "E: EthSpec")), serde(bound = "E: EthSpec", deny_unknown_fields), arbitrary(bound = "E: EthSpec"), - ) + ), + ref_attributes(derive(TreeHash), tree_hash(enum_behaviour = "transparent")) )] #[derive( Debug, @@ -123,22 +124,24 @@ impl Attestation { /// Aggregate another Attestation into this one. /// /// The aggregation bitfields must be disjoint, and the data must be the same. - pub fn aggregate(&mut self, other: &Self) { + pub fn aggregate(&mut self, other: AttestationRef) { match self { - Attestation::Base(att) => { - debug_assert!(other.as_base().is_ok()); - - if let Ok(other) = other.as_base() { - att.aggregate(other) + Attestation::Base(att) => match other { + AttestationRef::Base(oth) => { + att.aggregate(oth); } - } - Attestation::Electra(att) => { - debug_assert!(other.as_electra().is_ok()); - - if let Ok(other) = other.as_electra() { - att.aggregate(other) + AttestationRef::Electra(_) => { + debug_assert!(false, "Cannot aggregate base and electra attestations"); + } + }, + Attestation::Electra(att) => match other { + AttestationRef::Base(_) => { + debug_assert!(false, "Cannot aggregate base and electra attestations"); } - } + AttestationRef::Electra(oth) => { + att.aggregate(oth); + } + }, } } @@ -215,8 +218,22 @@ impl Attestation { impl<'a, E: EthSpec> AttestationRef<'a, E> { pub fn clone_as_attestation(self) -> Attestation { match self { - AttestationRef::Base(att) => Attestation::Base(att.clone()), - AttestationRef::Electra(att) => Attestation::Electra(att.clone()), + Self::Base(att) => Attestation::Base(att.clone()), + Self::Electra(att) => Attestation::Electra(att.clone()), + } + } + + pub fn is_aggregation_bits_zero(self) -> bool { + match self { + Self::Base(att) => att.aggregation_bits.is_zero(), + Self::Electra(att) => att.aggregation_bits.is_zero(), + } + } + + pub fn num_set_aggregation_bits(&self) -> usize { + match self { + Self::Base(att) => att.aggregation_bits.num_set_bits(), + Self::Electra(att) => att.aggregation_bits.num_set_bits(), } } } diff --git a/consensus/types/src/attester_slashing.rs b/consensus/types/src/attester_slashing.rs index 4ad19f477ca..6668f809b82 100644 --- a/consensus/types/src/attester_slashing.rs +++ b/consensus/types/src/attester_slashing.rs @@ -39,15 +39,10 @@ use tree_hash_derive::TreeHash; #[ssz(enum_behaviour = "transparent")] #[tree_hash(enum_behaviour = "transparent")] pub struct AttesterSlashing { - // TODO(electra) change this to `#[superstruct(flatten)]` when 0.8 is out.. - #[superstruct(only(Base), partial_getter(rename = "attestation_1_base"))] - pub attestation_1: IndexedAttestationBase, - #[superstruct(only(Electra), partial_getter(rename = "attestation_1_electra"))] - pub attestation_1: IndexedAttestationElectra, - #[superstruct(only(Base), partial_getter(rename = "attestation_2_base"))] - pub attestation_2: IndexedAttestationBase, - #[superstruct(only(Electra), partial_getter(rename = "attestation_2_electra"))] - pub attestation_2: IndexedAttestationElectra, + #[superstruct(flatten)] + pub attestation_1: IndexedAttestation, + #[superstruct(flatten)] + pub attestation_2: IndexedAttestation, } /// This is a copy of the `AttesterSlashing` enum but with `Encode` and `Decode` derived diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index ebfc4ab8bb0..3a1c0b9a52c 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -114,7 +114,9 @@ pub mod runtime_var_list; use ethereum_types::{H160, H256}; pub use crate::activation_queue::ActivationQueue; -pub use crate::aggregate_and_proof::AggregateAndProof; +pub use crate::aggregate_and_proof::{ + AggregateAndProof, AggregateAndProofBase, AggregateAndProofElectra, AggregateAndProofRef, +}; pub use crate::attestation::{ Attestation, AttestationBase, AttestationElectra, AttestationRef, AttestationRefMut, Error as AttestationError, @@ -218,7 +220,9 @@ pub use crate::relative_epoch::{Error as RelativeEpochError, RelativeEpoch}; pub use crate::runtime_var_list::RuntimeVariableList; pub use crate::selection_proof::SelectionProof; pub use crate::shuffling_id::AttestationShufflingId; -pub use crate::signed_aggregate_and_proof::SignedAggregateAndProof; +pub use crate::signed_aggregate_and_proof::{ + SignedAggregateAndProof, SignedAggregateAndProofBase, SignedAggregateAndProofElectra, +}; pub use crate::signed_beacon_block::{ ssz_tagged_signed_beacon_block, ssz_tagged_signed_beacon_block_arc, SignedBeaconBlock, SignedBeaconBlockAltair, SignedBeaconBlockBase, SignedBeaconBlockBellatrix, diff --git a/consensus/types/src/signed_aggregate_and_proof.rs b/consensus/types/src/signed_aggregate_and_proof.rs index 491edb3cb0d..649b5fc6fac 100644 --- a/consensus/types/src/signed_aggregate_and_proof.rs +++ b/consensus/types/src/signed_aggregate_and_proof.rs @@ -1,10 +1,14 @@ use super::{ - AggregateAndProof, Attestation, ChainSpec, Domain, EthSpec, Fork, Hash256, SecretKey, - SelectionProof, Signature, SignedRoot, + AggregateAndProof, AggregateAndProofBase, AggregateAndProofElectra, AggregateAndProofRef, +}; +use super::{ + Attestation, ChainSpec, Domain, EthSpec, Fork, Hash256, SecretKey, SelectionProof, Signature, + SignedRoot, }; use crate::test_utils::TestRandom; use serde::{Deserialize, Serialize}; use ssz_derive::{Decode, Encode}; +use superstruct::superstruct; use test_random_derive::TestRandom; use tree_hash_derive::TreeHash; @@ -12,22 +16,36 @@ use tree_hash_derive::TreeHash; /// gossipsub topic. /// /// Spec v0.12.1 +#[superstruct( + variants(Base, Electra), + variant_attributes( + derive( + arbitrary::Arbitrary, + Debug, + Clone, + PartialEq, + Serialize, + Deserialize, + Encode, + Decode, + TestRandom, + TreeHash, + ), + serde(bound = "E: EthSpec"), + arbitrary(bound = "E: EthSpec"), + ) +)] #[derive( - Debug, - Clone, - PartialEq, - Serialize, - Deserialize, - Encode, - Decode, - TestRandom, - TreeHash, - arbitrary::Arbitrary, + arbitrary::Arbitrary, Debug, Clone, PartialEq, Serialize, Deserialize, Encode, TreeHash, )] -#[serde(bound = "E: EthSpec")] +#[serde(untagged)] +#[tree_hash(enum_behaviour = "transparent")] +#[ssz(enum_behaviour = "transparent")] +#[serde(bound = "E: EthSpec", deny_unknown_fields)] #[arbitrary(bound = "E: EthSpec")] pub struct SignedAggregateAndProof { /// The `AggregateAndProof` that was signed. + #[superstruct(flatten)] pub message: AggregateAndProof, /// The aggregate attestation. pub signature: Signature, @@ -57,7 +75,7 @@ impl SignedAggregateAndProof { spec, ); - let target_epoch = message.aggregate.data().slot.epoch(E::slots_per_epoch()); + let target_epoch = message.aggregate().data().slot.epoch(E::slots_per_epoch()); let domain = spec.get_domain( target_epoch, Domain::AggregateAndProof, @@ -66,9 +84,28 @@ impl SignedAggregateAndProof { ); let signing_message = message.signing_root(domain); - SignedAggregateAndProof { - message, - signature: secret_key.sign(signing_message), + match message { + AggregateAndProof::Base(message) => { + SignedAggregateAndProof::Base(SignedAggregateAndProofBase { + message, + signature: secret_key.sign(signing_message), + }) + } + AggregateAndProof::Electra(message) => { + SignedAggregateAndProof::Electra(SignedAggregateAndProofElectra { + message, + signature: secret_key.sign(signing_message), + }) + } + } + } + + pub fn message(&self) -> AggregateAndProofRef { + match self { + SignedAggregateAndProof::Base(message) => AggregateAndProofRef::Base(&message.message), + SignedAggregateAndProof::Electra(message) => { + AggregateAndProofRef::Electra(&message.message) + } } } } diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 7b60e331666..676bf7cae2e 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -24,8 +24,8 @@ use std::future::Future; use std::sync::Arc; use std::time::Duration; use types::{ - Attestation, AttesterSlashing, AttesterSlashingRef, BeaconBlock, BeaconState, BlobSidecar, - BlobsList, Checkpoint, ExecutionBlockHash, Hash256, IndexedAttestation, KzgProof, + Attestation, AttestationRef, AttesterSlashing, AttesterSlashingRef, BeaconBlock, BeaconState, + BlobSidecar, BlobsList, Checkpoint, ExecutionBlockHash, Hash256, IndexedAttestation, KzgProof, ProposerPreparationData, SignedBeaconBlock, Slot, Uint256, }; @@ -589,11 +589,11 @@ impl Tester { } pub fn process_attestation(&self, attestation: &Attestation) -> Result<(), Error> { - let (indexed_attestation, _) = - obtain_indexed_attestation_and_committees_per_slot(&self.harness.chain, attestation) - .map_err(|e| { - Error::InternalError(format!("attestation indexing failed with {:?}", e)) - })?; + let (indexed_attestation, _) = obtain_indexed_attestation_and_committees_per_slot( + &self.harness.chain, + attestation.to_ref(), + ) + .map_err(|e| Error::InternalError(format!("attestation indexing failed with {:?}", e)))?; let verified_attestation: ManuallyVerifiedAttestation> = ManuallyVerifiedAttestation { attestation, @@ -865,8 +865,8 @@ pub struct ManuallyVerifiedAttestation<'a, T: BeaconChainTypes> { } impl<'a, T: BeaconChainTypes> VerifiedAttestation for ManuallyVerifiedAttestation<'a, T> { - fn attestation(&self) -> &Attestation { - self.attestation + fn attestation(&self) -> AttestationRef { + self.attestation.to_ref() } fn indexed_attestation(&self) -> &IndexedAttestation { diff --git a/validator_client/src/attestation_service.rs b/validator_client/src/attestation_service.rs index 653d829d685..5b7f31867bb 100644 --- a/validator_client/src/attestation_service.rs +++ b/validator_client/src/attestation_service.rs @@ -605,11 +605,11 @@ impl AttestationService { { Ok(()) => { for signed_aggregate_and_proof in signed_aggregate_and_proofs { - let attestation = &signed_aggregate_and_proof.message.aggregate; + let attestation = signed_aggregate_and_proof.message().aggregate(); info!( log, "Successfully published attestation"; - "aggregator" => signed_aggregate_and_proof.message.aggregator_index, + "aggregator" => signed_aggregate_and_proof.message().aggregator_index(), "signatures" => attestation.num_set_aggregation_bits(), "head_block" => format!("{:?}", attestation.data().beacon_block_root), "committee_index" => attestation.data().index, @@ -620,12 +620,12 @@ impl AttestationService { } Err(e) => { for signed_aggregate_and_proof in signed_aggregate_and_proofs { - let attestation = &signed_aggregate_and_proof.message.aggregate; + let attestation = &signed_aggregate_and_proof.message().aggregate(); crit!( log, "Failed to publish attestation"; "error" => %e, - "aggregator" => signed_aggregate_and_proof.message.aggregator_index, + "aggregator" => signed_aggregate_and_proof.message().aggregator_index(), "committee_index" => attestation.data().index, "slot" => attestation.data().slot.as_u64(), "type" => "aggregated", diff --git a/validator_client/src/signing_method.rs b/validator_client/src/signing_method.rs index fe520e11f5f..19824d76fb0 100644 --- a/validator_client/src/signing_method.rs +++ b/validator_client/src/signing_method.rs @@ -38,7 +38,7 @@ pub enum SignableMessage<'a, E: EthSpec, Payload: AbstractExecPayload = FullP RandaoReveal(Epoch), BeaconBlock(&'a BeaconBlock), AttestationData(&'a AttestationData), - SignedAggregateAndProof(&'a AggregateAndProof), + SignedAggregateAndProof(AggregateAndProofRef<'a, E>), SelectionProof(Slot), SyncSelectionProof(&'a SyncAggregatorSelectionData), SyncCommitteeSignature { diff --git a/validator_client/src/signing_method/web3signer.rs b/validator_client/src/signing_method/web3signer.rs index 8ad37a1620a..86e7015ad35 100644 --- a/validator_client/src/signing_method/web3signer.rs +++ b/validator_client/src/signing_method/web3signer.rs @@ -43,7 +43,7 @@ pub enum Web3SignerObject<'a, E: EthSpec, Payload: AbstractExecPayload> { AggregationSlot { slot: Slot, }, - AggregateAndProof(&'a AggregateAndProof), + AggregateAndProof(AggregateAndProofRef<'a, E>), Attestation(&'a AttestationData), BeaconBlock { version: ForkName, diff --git a/validator_client/src/validator_store.rs b/validator_client/src/validator_store.rs index 8e69cb440a4..a173e5da12b 100644 --- a/validator_client/src/validator_store.rs +++ b/validator_client/src/validator_store.rs @@ -18,12 +18,16 @@ use std::sync::Arc; use task_executor::TaskExecutor; use types::{ attestation::Error as AttestationError, graffiti::GraffitiString, AbstractExecPayload, Address, - AggregateAndProof, Attestation, BeaconBlock, BlindedPayload, ChainSpec, ContributionAndProof, - Domain, Epoch, EthSpec, Fork, ForkName, Graffiti, Hash256, PublicKeyBytes, SelectionProof, - Signature, SignedAggregateAndProof, SignedBeaconBlock, SignedContributionAndProof, SignedRoot, - SignedValidatorRegistrationData, SignedVoluntaryExit, Slot, SyncAggregatorSelectionData, - SyncCommitteeContribution, SyncCommitteeMessage, SyncSelectionProof, SyncSubnetId, - ValidatorRegistrationData, VoluntaryExit, + Attestation, BeaconBlock, BlindedPayload, ChainSpec, ContributionAndProof, Domain, Epoch, + EthSpec, Fork, ForkName, Graffiti, Hash256, PublicKeyBytes, SelectionProof, Signature, + SignedBeaconBlock, SignedContributionAndProof, SignedRoot, SignedValidatorRegistrationData, + SignedVoluntaryExit, Slot, SyncAggregatorSelectionData, SyncCommitteeContribution, + SyncCommitteeMessage, SyncSelectionProof, SyncSubnetId, ValidatorRegistrationData, + VoluntaryExit, +}; +use types::{ + AggregateAndProof, AggregateAndProofBase, AggregateAndProofElectra, SignedAggregateAndProof, + SignedAggregateAndProofBase, SignedAggregateAndProofElectra, }; pub use crate::doppelganger_service::DoppelgangerStatus; @@ -801,16 +805,23 @@ impl ValidatorStore { let signing_epoch = aggregate.data().target.epoch; let signing_context = self.signing_context(Domain::AggregateAndProof, signing_epoch); - let message = AggregateAndProof { - aggregator_index, - aggregate, - selection_proof: selection_proof.into(), + let message = match aggregate { + Attestation::Base(att) => AggregateAndProof::Base(AggregateAndProofBase { + aggregator_index, + aggregate: att, + selection_proof: selection_proof.into(), + }), + Attestation::Electra(att) => AggregateAndProof::Electra(AggregateAndProofElectra { + aggregator_index, + aggregate: att, + selection_proof: selection_proof.into(), + }), }; let signing_method = self.doppelganger_checked_signing_method(validator_pubkey)?; let signature = signing_method .get_signature::>( - SignableMessage::SignedAggregateAndProof(&message), + SignableMessage::SignedAggregateAndProof(message.to_ref()), signing_context, &self.spec, &self.task_executor, @@ -819,7 +830,17 @@ impl ValidatorStore { metrics::inc_counter_vec(&metrics::SIGNED_AGGREGATES_TOTAL, &[metrics::SUCCESS]); - Ok(SignedAggregateAndProof { message, signature }) + match message { + AggregateAndProof::Base(message) => { + Ok(SignedAggregateAndProof::Base(SignedAggregateAndProofBase { + message, + signature, + })) + } + AggregateAndProof::Electra(message) => Ok(SignedAggregateAndProof::Electra( + SignedAggregateAndProofElectra { message, signature }, + )), + } } /// Produces a `SelectionProof` for the `slot`, signed by with corresponding secret key to