Superstruct AggregateAndProof (sigp#5715)

* Upgrade `superstruct` to `0.8.0`

* superstruct `AggregateAndProof`

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"

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<T: BeaconChainTypes>: Sized {
 
     // Inefficient default implementation. This is overridden for gossip verified attestations.
     fn into_attestation_and_indices(self) -> (Attestation<T::EthSpec>, Vec<u64>) {
-        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<T>,
     ) -> Result<Self, AttestationSlashInfo<'a, T, Error>> {
         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<BeaconCommittee>, 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<T: BeaconChainTypes>(
     attestation: AttestationRef<T::EthSpec>,
 ) -> Result<(IndexedAttestation<T::EthSpec>, 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)
     })

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<E: EthSpec> AggregateMap for AggregatedAttestationMap<E> {
         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::<Vec<_>>(),
-            Attestation::Electra(att) => att
+            AttestationRef::Electra(att) => att
                 .aggregation_bits
                 .iter()
                 .enumerate()
@@ -290,10 +287,8 @@ impl<E: EthSpec> AggregateMap for AggregatedAttestationMap<E> {
             }
 
             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 })
         }
     }
 

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
                                     },
                                 )

beacon_node/http_api/src/lib.rs

@@ -3368,9 +3368,9 @@ pub fn serve<T: BeaconChainTypes>(
                                     "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<T: BeaconChainTypes>(
                                     "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,
                                 );

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::<E>(&[aggregate])

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<E: EthSpec> PubsubMessage<E> {
                     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<E: EthSpec> std::fmt::Display for PubsubMessage<E> {
             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,

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<T: BeaconChainTypes> VerifiedAttestation<T> for VerifiedAggregate<T> {
 
     /// Efficient clone-free implementation that moves out of the `Box`.
     fn into_attestation_and_indices(self) -> (Attestation<T::EthSpec>, Vec<u64>) {
-        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<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
         reprocess_tx: Option<mpsc::Sender<ReprocessQueueMessage>>,
         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

beacon_node/network/src/network_beacon_processor/mod.rs

@@ -142,7 +142,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
             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 {

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());

consensus/state_processing/src/per_block_processing/signature_sets.rs

@@ -425,7 +425,7 @@ where
     E: EthSpec,
     F: Fn(usize) -> Option<Cow<'a, PublicKey>>,
 {
-    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<Cow<'a, PublicKey>>,
 {
     let target_epoch = signed_aggregate_and_proof
-        .message
-        .aggregate
+        .message()
+        .aggregate()
         .data()
         .target
         .epoch;