diff --git a/Cargo.lock b/Cargo.lock index 90196ea5b31..064b1412600 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8036,8 +8036,7 @@ checksum = "81cdd64d312baedb58e21336b31bc043b77e01cc99033ce76ef539f78e965ebc" [[package]] name = "superstruct" version = "0.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bf0f31f730ad9e579364950e10d6172b4a9bd04b447edf5988b066a860cc340e" +source = "git+https://github.com/sigp/superstruct?rev=45eecbfb9708c9fe11dbb6a6a5bd8d618f02269e#45eecbfb9708c9fe11dbb6a6a5bd8d618f02269e" dependencies = [ "darling", "itertools", diff --git a/Cargo.toml b/Cargo.toml index d67f6edf1dd..1059c746253 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -162,7 +162,7 @@ smallvec = "1.11.2" snap = "1" ssz_types = "0.6" strum = { version = "0.24", features = ["derive"] } -superstruct = "0.8" +superstruct = { git = "https://github.com/sigp/superstruct", rev = "45eecbfb9708c9fe11dbb6a6a5bd8d618f02269e" } syn = "1" sysinfo = "0.26" tempfile = "3" diff --git a/beacon_node/beacon_chain/src/attestation_verification.rs b/beacon_node/beacon_chain/src/attestation_verification.rs index 851f945502c..b42f6fac834 100644 --- a/beacon_node/beacon_chain/src/attestation_verification.rs +++ b/beacon_node/beacon_chain/src/attestation_verification.rs @@ -61,9 +61,8 @@ use std::borrow::Cow; use strum::AsRefStr; use tree_hash::TreeHash; use types::{ - Attestation, AttestationRef, BeaconCommittee, BeaconStateError::NoCommitteeFound, ChainSpec, - CommitteeIndex, Epoch, EthSpec, 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}; @@ -348,7 +347,7 @@ pub trait VerifiedAttestation: Sized { // 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) } @@ -496,7 +495,7 @@ 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); @@ -558,8 +557,10 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> { chain: &BeaconChain, ) -> Result> { use AttestationSlashInfo::*; - let observed_attestation_key_root = match Self::verify_early_checks(signed_aggregate, chain) - { + + 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( @@ -571,28 +572,12 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> { // Committees must be sorted by ascending index order 0..committees_per_slot let get_indexed_attestation_with_committee = - |(committees, _): (Vec, CommitteesPerSlot)| { - let (index, aggregator_index, selection_proof, data) = match signed_aggregate { - SignedAggregateAndProof::Base(signed_aggregate) => ( - signed_aggregate.message.aggregate.data.index, - signed_aggregate.message.aggregator_index, - // Note: this clones the signature which is known to be a relatively slow operation. - // Future optimizations should remove this clone. - signed_aggregate.message.selection_proof.clone(), - signed_aggregate.message.aggregate.data.clone(), - ), - SignedAggregateAndProof::Electra(signed_aggregate) => ( - signed_aggregate - .message - .aggregate - .committee_index() - .ok_or(Error::NotExactlyOneCommitteeBitSet(0))?, - signed_aggregate.message.aggregator_index, - signed_aggregate.message.selection_proof.clone(), - signed_aggregate.message.aggregate.data.clone(), - ), - }; - let slot = data.slot; + |(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) @@ -610,7 +595,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()) }; @@ -653,7 +638,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); @@ -1326,7 +1311,7 @@ pub fn obtain_indexed_attestation_and_committees_per_slot( 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) }) diff --git a/beacon_node/beacon_chain/src/naive_aggregation_pool.rs b/beacon_node/beacon_chain/src/naive_aggregation_pool.rs index 7b729a1f56a..13b22df2fed 100644 --- a/beacon_node/beacon_chain/src/naive_aggregation_pool.rs +++ b/beacon_node/beacon_chain/src/naive_aggregation_pool.rs @@ -1,15 +1,12 @@ use crate::metrics; use crate::observed_aggregates::AsReference; -use itertools::Itertools; -use smallvec::SmallVec; use std::collections::HashMap; use tree_hash::{MerkleHasher, TreeHash, TreeHashType}; use types::consts::altair::SYNC_COMMITTEE_SUBNET_COUNT; use types::slot_data::SlotData; use types::sync_committee_contribution::SyncContributionData; use types::{ - Attestation, AttestationData, AttestationRef, CommitteeIndex, EthSpec, Hash256, Slot, - SyncCommitteeContribution, + Attestation, AttestationData, AttestationRef, EthSpec, Hash256, Slot, SyncCommitteeContribution, }; type AttestationKeyRoot = Hash256; @@ -242,14 +239,14 @@ impl AggregateMap for AggregatedAttestationMap { 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() @@ -290,10 +287,8 @@ impl AggregateMap for AggregatedAttestationMap { } self.map - .insert(attestation_key_root, a.clone_as_attestation()); - Ok(InsertOutcome::NewItemInserted { - committee_index: aggregation_bit, - }) + .insert(attestation_data_root, a.clone_as_attestation()); + Ok(InsertOutcome::NewItemInserted { committee_index }) } } diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 4184b8a8af8..23b8bb38016 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -1382,7 +1382,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/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 890331b94ae..8043a42c9cf 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/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 8ea564c386d..e01fdea7a46 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -3334,8 +3334,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 ce5e94d0cba..4d33455b1cb 100644 --- a/beacon_node/lighthouse_network/src/types/pubsub.rs +++ b/beacon_node/lighthouse_network/src/types/pubsub.rs @@ -9,9 +9,10 @@ use std::sync::Arc; use types::{ Attestation, AttesterSlashing, AttesterSlashingBase, AttesterSlashingElectra, BlobSidecar, EthSpec, ForkContext, ForkName, LightClientFinalityUpdate, LightClientOptimisticUpdate, - ProposerSlashing, SignedAggregateAndProof, SignedBeaconBlock, SignedBeaconBlockAltair, - SignedBeaconBlockBase, SignedBeaconBlockCapella, SignedBeaconBlockDeneb, - SignedBeaconBlockElectra, SignedBeaconBlockMerge, SignedBlsToExecutionChange, + ProposerSlashing, SignedAggregateAndProof, SignedAggregateAndProofBase, + SignedAggregateAndProofElectra, SignedBeaconBlock, SignedBeaconBlockAltair, + SignedBeaconBlockBase, SignedBeaconBlockBellatrix, SignedBeaconBlockCapella, + SignedBeaconBlockDeneb, SignedBeaconBlockElectra, SignedBlsToExecutionChange, SignedContributionAndProof, SignedVoluntaryExit, SubnetId, SyncCommitteeMessage, SyncSubnetId, }; @@ -156,19 +157,18 @@ impl PubsubMessage { GossipKind::BeaconAggregateAndProof => { let signed_aggregate_and_proof = match fork_context.from_context_bytes(gossip_topic.fork_digest) { - Some(&fork_name) => { - if fork_name.electra_enabled() { - SignedAggregateAndProof::Electra( - SignedAggregateAndProofElectra::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ) - } else { - SignedAggregateAndProof::Base( - SignedAggregateAndProofBase::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ) - } - } + 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: {:?}", @@ -402,9 +402,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 c00b5565a36..147cac5a973 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -31,9 +31,9 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use store::hot_cold_store::HotColdDBError; use tokio::sync::mpsc; use types::{ - beacon_block::BlockImportSource, Attestation, AttestationRef, AttesterSlashing, BlobSidecar, - EthSpec, Hash256, IndexedAttestation, LightClientFinalityUpdate, LightClientOptimisticUpdate, - ProposerSlashing, SignedAggregateAndProof, SignedBeaconBlock, SignedBlsToExecutionChange, + Attestation, AttestationRef, AttesterSlashing, BlobSidecar, EthSpec, Hash256, + IndexedAttestation, LightClientFinalityUpdate, LightClientOptimisticUpdate, ProposerSlashing, + SignedAggregateAndProof, SignedBeaconBlock, SignedBlsToExecutionChange, SignedContributionAndProof, SignedVoluntaryExit, Slot, SubnetId, SyncCommitteeMessage, SyncSubnetId, }; @@ -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) } @@ -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 diff --git a/beacon_node/network/src/network_beacon_processor/mod.rs b/beacon_node/network/src/network_beacon_processor/mod.rs index a37b8b99bfb..ccdbb10720c 100644 --- a/beacon_node/network/src/network_beacon_processor/mod.rs +++ b/beacon_node/network/src/network_beacon_processor/mod.rs @@ -142,7 +142,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 7bb9a4ec7bf..125e6a8cee9 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -921,7 +921,7 @@ 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_as_attestation(), att2_indices); @@ -1064,14 +1064,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_as_attestation(), attesting_indices.clone()) .unwrap(); - op_pool - .insert_attestation(att.clone_as_attestation(), attesting_indices) - .unwrap(); + op_pool.insert_attestation(att.clone_as_attestation(), attesting_indices).unwrap(); } assert_eq!(op_pool.num_attestations(), committees.len()); 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 d8e83202334..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()), @@ -436,6 +436,7 @@ where 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(); + Ok(SignatureSet::single_pubkey( signature, get_pubkey(validator_index as usize).ok_or(Error::ValidatorUnknown(validator_index))?, @@ -455,8 +456,8 @@ where F: Fn(usize) -> Option>, { let target_epoch = signed_aggregate_and_proof - .message - .aggregate + .message() + .aggregate() .data() .target .epoch; diff --git a/consensus/types/src/aggregate_and_proof.rs b/consensus/types/src/aggregate_and_proof.rs index 54a4a1aed7f..a48bf91eb87 100644 --- a/consensus/types/src/aggregate_and_proof.rs +++ b/consensus/types/src/aggregate_and_proof.rs @@ -1,4 +1,4 @@ -use super::{AttestationBase, AttestationElectra, AttestationRef}; +use super::{Attestation, AttestationBase, AttestationElectra, AttestationRef}; use super::{ ChainSpec, Domain, EthSpec, Fork, Hash256, PublicKey, SecretKey, SelectionProof, Signature, SignedRoot, @@ -31,10 +31,9 @@ use tree_hash_derive::TreeHash; ), ref_attributes( derive(Debug, PartialEq, TreeHash, Serialize,), - serde(untagged, bound = "E: EthSpec"), + serde(bound = "E: EthSpec"), tree_hash(enum_behaviour = "transparent") - ), - map_ref_into(AttestationRef) + ) )] #[derive( arbitrary::Arbitrary, Debug, Clone, PartialEq, Serialize, Deserialize, Encode, TreeHash, @@ -60,17 +59,19 @@ pub struct AggregateAndProof { 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> { - map_aggregate_and_proof_ref_into_attestation_ref!(&'a _, self, |inner, cons| { - cons(&inner.aggregate) - }) + 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<'a>(&'a self) -> AttestationRef<'a, E> { - map_aggregate_and_proof_ref_into_attestation_ref!(&'a _, self.to_ref(), |inner, cons| { - cons(&inner.aggregate) - }) + pub fn aggregate(&self) -> AttestationRef { + match self { + AggregateAndProof::Base(a) => AttestationRef::Base(&a.aggregate), + AggregateAndProof::Electra(a) => AttestationRef::Electra(&a.aggregate), + } } } @@ -100,29 +101,16 @@ impl AggregateAndProof { }) .into(); - Self::from_attestation( - aggregator_index, - aggregate.clone_as_attestation(), - selection_proof, - ) - } - - /// Produces a new `AggregateAndProof` given a `selection_proof` - pub fn from_attestation( - aggregator_index: u64, - aggregate: Attestation, - selection_proof: SelectionProof, - ) -> Self { match aggregate { - Attestation::Base(aggregate) => Self::Base(AggregateAndProofBase { + Attestation::Base(attestation) => Self::Base(AggregateAndProofBase { aggregator_index, - aggregate, - selection_proof: selection_proof.into(), + aggregate: attestation, + selection_proof, }), - Attestation::Electra(aggregate) => Self::Electra(AggregateAndProofElectra { + Attestation::Electra(attestation) => Self::Electra(AggregateAndProofElectra { aggregator_index, - aggregate, - selection_proof: selection_proof.into(), + aggregate: attestation, + selection_proof, }), } } @@ -135,15 +123,15 @@ 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) } } diff --git a/consensus/types/src/attestation.rs b/consensus/types/src/attestation.rs index 02899fcdbd5..c0588c99632 100644 --- a/consensus/types/src/attestation.rs +++ b/consensus/types/src/attestation.rs @@ -45,7 +45,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, @@ -140,22 +141,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); + } + }, } } @@ -232,8 +235,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 089190fc705..c778b7d9fe8 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 83c606ace0d..6a98d7ade96 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -112,7 +112,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, diff --git a/consensus/types/src/signed_aggregate_and_proof.rs b/consensus/types/src/signed_aggregate_and_proof.rs index 360e36e8479..e7ffccab60c 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::{ - AttestationRef, ChainSpec, Domain, EthSpec, Fork, Hash256, SecretKey, SelectionProof, - Signature, SignedRoot, + Attestation, ChainSpec, Domain, EthSpec, Fork, Hash256, SecretKey, SelectionProof, Signature, + SignedRoot, }; use crate::test_utils::TestRandom; use crate::Attestation; @@ -34,9 +34,7 @@ use tree_hash_derive::TreeHash; ), serde(bound = "E: EthSpec"), arbitrary(bound = "E: EthSpec"), - ), - map_into(Attestation), - map_ref_into(AggregateAndProofRef) + ) )] #[derive( arbitrary::Arbitrary, Debug, Clone, PartialEq, Serialize, Deserialize, Encode, TreeHash, @@ -78,7 +76,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, @@ -87,24 +85,31 @@ impl SignedAggregateAndProof { ); let signing_message = message.signing_root(domain); - Self::from_aggregate_and_proof(message, secret_key.sign(signing_message)) - } - - /// Produces a new `SignedAggregateAndProof` given a `signature` of `aggregate` - pub fn from_aggregate_and_proof(aggregate: AggregateAndProof, signature: Signature) -> Self { - match aggregate { + match message { AggregateAndProof::Base(message) => { - SignedAggregateAndProof::Base(SignedAggregateAndProofBase { message, signature }) + SignedAggregateAndProof::Base(SignedAggregateAndProofBase { + message, + signature: secret_key.sign(signing_message), + }) } AggregateAndProof::Electra(message) => { SignedAggregateAndProof::Electra(SignedAggregateAndProofElectra { message, - signature, + 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) + } + } + } + pub fn message<'a>(&'a self) -> AggregateAndProofRef<'a, E> { map_signed_aggregate_and_proof_ref_into_aggregate_and_proof_ref!( &'a _, diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 5e3e5ced250..f5d770f157b 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, }; diff --git a/validator_client/src/attestation_service.rs b/validator_client/src/attestation_service.rs index 9570f9499ba..84ee882eb69 100644 --- a/validator_client/src/attestation_service.rs +++ b/validator_client/src/attestation_service.rs @@ -604,7 +604,7 @@ impl AttestationService { 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,7 +620,7 @@ impl AttestationService { 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/validator_store.rs b/validator_client/src/validator_store.rs index e6e19b6e06c..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,8 +805,18 @@ impl ValidatorStore { let signing_epoch = aggregate.data().target.epoch; let signing_context = self.signing_context(Domain::AggregateAndProof, signing_epoch); - let message = - AggregateAndProof::from_attestation(aggregator_index, aggregate, selection_proof); + 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 @@ -816,9 +830,17 @@ impl ValidatorStore { metrics::inc_counter_vec(&metrics::SIGNED_AGGREGATES_TOTAL, &[metrics::SUCCESS]); - Ok(SignedAggregateAndProof::from_aggregate_and_proof( - 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