Attestation superstruct changes for EIP 7549 (sigp#5644)

* update

* experiment

* superstruct changes

* revert

* superstruct changes

* fix tests

* indexed attestation

* indexed attestation superstruct

* updated TODOs

beacon_node/beacon_chain/src/attestation_verification.rs

@@ -348,7 +348,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_as_attestation();
+        let attestation = self.attestation().clone();
         let attesting_indices = self.indexed_attestation().attesting_indices_to_vec();
         (attestation, attesting_indices)
     }
@@ -489,16 +489,9 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
             });
         }
 
-        let observed_attestation_key_root = ObservedAttestationKey {
-            committee_index: attestation
-                .committee_index()
-                .ok_or(Error::NotExactlyOneCommitteeBitSet(0))?,
-            attestation_data: attestation.data().clone(),
-        }
-        .tree_hash_root();
-
-        // [New in Electra:EIP7549]
-        verify_committee_index(attestation)?;
+        // Ensure the valid aggregated attestation has not already been seen locally.
+        let attestation_data = attestation.data();
+        let attestation_data_root = attestation_data.tree_hash_root();
 
         if chain
             .observed_attestations
@@ -842,8 +835,8 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> {
         subnet_id: Option<SubnetId>,
         chain: &BeaconChain<T>,
     ) -> Result<(u64, SubnetId), Error> {
-        let expected_subnet_id = SubnetId::compute_subnet_for_attestation::<T::EthSpec>(
-            attestation,
+        let expected_subnet_id = SubnetId::compute_subnet_for_attestation_data::<T::EthSpec>(
+            indexed_attestation.data(),
             committees_per_slot,
             &chain.spec,
         )
@@ -1433,12 +1426,12 @@ where
             let committees_per_slot = committee_cache.committees_per_slot();
 
             Ok(committee_cache
-                .get_beacon_committees_at_slot(attestation.data().slot)
-                .map(|committees| map_fn((committees, committees_per_slot)))
-                .unwrap_or_else(|_| {
+                .get_beacon_committee(attestation.data().slot, attestation.data().index)
+                .map(|committee| map_fn((committee, committees_per_slot)))
+                .unwrap_or_else(|| {
                     Err(Error::NoCommitteeForSlotAndIndex {
                         slot: attestation.data().slot,
-                        index: attestation.committee_index().unwrap_or(0),
+                        index: attestation.data().index,
                     })
                 }))
         })

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -122,6 +122,7 @@ use store::{
 use task_executor::{ShutdownReason, TaskExecutor};
 use tokio_stream::Stream;
 use tree_hash::TreeHash;
+use types::attestation::AttestationBase;
 use types::blob_sidecar::FixedBlobSidecarList;
 use types::payload::BlockProductionVersion;
 use types::*;
@@ -1993,15 +1994,18 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             };
         drop(cache_timer);
 
-        Ok(Attestation::<T::EthSpec>::empty_for_signing(
-            request_index,
-            committee_len,
-            request_slot,
-            beacon_block_root,
-            justified_checkpoint,
-            target,
-            &self.spec,
-        )?)
+        // TODO(electra) implement electra variant
+        Ok(Attestation::Base(AttestationBase {
+            aggregation_bits: BitList::with_capacity(committee_len)?,
+            data: AttestationData {
+                slot: request_slot,
+                index: request_index,
+                beacon_block_root,
+                source: justified_checkpoint,
+                target,
+            },
+            signature: AggregateSignature::empty(),
+        }))
     }
 
     /// Performs the same validation as `Self::verify_unaggregated_attestation_for_gossip`, but for
@@ -2215,7 +2219,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 self.log,
                 "Stored unaggregated attestation";
                 "outcome" => ?outcome,
-                "index" => attestation.committee_index(),
+                "index" => attestation.data().index,
                 "slot" => attestation.data().slot.as_u64(),
             ),
             Err(NaiveAggregationError::SlotTooLow {
@@ -2234,7 +2238,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                         self.log,
                         "Failed to store unaggregated attestation";
                         "error" => ?e,
-                        "index" => attestation.committee_index(),
+                        "index" => attestation.data().index,
                         "slot" => attestation.data().slot.as_u64(),
                 );
                 return Err(Error::from(e).into());

beacon_node/beacon_chain/src/block_reward.rs

@@ -88,6 +88,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             block
                 .body()
                 .attestations()
+                .iter()
                 .map(|a| a.data().clone())
                 .collect()
         } else {

beacon_node/beacon_chain/src/early_attester_cache.rs

@@ -6,6 +6,7 @@ use crate::{
 use parking_lot::RwLock;
 use proto_array::Block as ProtoBlock;
 use std::sync::Arc;
+use types::attestation::AttestationBase;
 use types::*;
 
 pub struct CacheItem<E: EthSpec> {
@@ -122,16 +123,19 @@ impl<E: EthSpec> EarlyAttesterCache<E> {
             item.committee_lengths
                 .get_committee_length::<E>(request_slot, request_index, spec)?;
 
-        let attestation = Attestation::empty_for_signing(
-            request_index,
-            committee_len,
-            request_slot,
-            item.beacon_block_root,
-            item.source,
-            item.target,
-            spec,
-        )
-        .map_err(Error::AttestationError)?;
+        // TODO(electra) make fork-agnostic
+        let attestation = Attestation::Base(AttestationBase {
+            aggregation_bits: BitList::with_capacity(committee_len)
+                .map_err(BeaconStateError::from)?,
+            data: AttestationData {
+                slot: request_slot,
+                index: request_index,
+                beacon_block_root: item.beacon_block_root,
+                source: item.source,
+                target: item.target,
+            },
+            signature: AggregateSignature::empty(),
+        });
 
         metrics::inc_counter(&metrics::BEACON_EARLY_ATTESTER_CACHE_HITS);
 

beacon_node/beacon_chain/src/naive_aggregation_pool.rs

@@ -241,19 +241,36 @@ impl<E: EthSpec> AggregateMap for AggregatedAttestationMap<E> {
     fn insert(&mut self, a: AttestationRef<E>) -> Result<InsertOutcome, Error> {
         let _timer = metrics::start_timer(&metrics::ATTESTATION_PROCESSING_AGG_POOL_CORE_INSERT);
 
-        let aggregation_bit = *a
-            .set_aggregation_bits()
-            .iter()
-            .at_most_one()
-            .map_err(|iter| Error::MoreThanOneAggregationBitSet(iter.count()))?
+        let set_bits = match a {
+            Attestation::Base(att) => att
+                .aggregation_bits
+                .iter()
+                .enumerate()
+                .filter(|(_i, bit)| *bit)
+                .map(|(i, _bit)| i)
+                .collect::<Vec<_>>(),
+            Attestation::Electra(att) => att
+                .aggregation_bits
+                .iter()
+                .enumerate()
+                .filter(|(_i, bit)| *bit)
+                .map(|(i, _bit)| i)
+                .collect::<Vec<_>>(),
+        };
+
+        let committee_index = set_bits
+            .first()
+            .copied()
             .ok_or(Error::NoAggregationBitsSet)?;
 
         let attestation_key = AttestationKey::from_attestation_ref(a)?;
         let attestation_key_root = attestation_key.tree_hash_root();
 
-        if let Some(existing_attestation) = self.map.get_mut(&attestation_key_root) {
+        let attestation_data_root = a.data().tree_hash_root();
+
+        if let Some(existing_attestation) = self.map.get_mut(&attestation_data_root) {
             if existing_attestation
-                .get_aggregation_bit(aggregation_bit)
+                .get_aggregation_bit(committee_index)
                 .map_err(|_| Error::InconsistentBitfieldLengths)?
             {
                 Ok(InsertOutcome::SignatureAlreadyKnown {
@@ -587,22 +604,12 @@ mod tests {
 
     type E = types::MainnetEthSpec;
 
-    fn get_attestation_base(slot: Slot) -> Attestation<E> {
-        let mut a: AttestationBase<E> = test_random_instance();
-        a.data.slot = slot;
-        a.aggregation_bits = BitList::with_capacity(4).expect("should create bitlist");
-        Attestation::Base(a)
-    }
-
-    fn get_attestation_electra(slot: Slot) -> Attestation<E> {
-        let mut a: AttestationElectra<E> = test_random_instance();
-        a.data.slot = slot;
-        a.aggregation_bits = BitList::with_capacity(4).expect("should create bitlist");
-        a.committee_bits = BitVector::new();
-        a.committee_bits
-            .set(0, true)
-            .expect("should set committee bit");
-        Attestation::Electra(a)
+    fn get_attestation(slot: Slot) -> Attestation<E> {
+        let mut a: Attestation<E> = test_random_instance();
+        a.data_mut().slot = slot;
+        *a.aggregation_bits_base_mut().unwrap() =
+            BitList::with_capacity(4).expect("should create bitlist");
+        a
     }
 
     fn get_sync_contribution(slot: Slot) -> SyncCommitteeContribution<E> {
@@ -645,16 +652,10 @@ mod tests {
     }
 
     fn unset_attestation_bit(a: &mut Attestation<E>, i: usize) {
-        match a {
-            Attestation::Base(ref mut att) => att
-                .aggregation_bits
-                .set(i, false)
-                .expect("should unset aggregation bit"),
-            Attestation::Electra(ref mut att) => att
-                .aggregation_bits
-                .set(i, false)
-                .expect("should unset aggregation bit"),
-        }
+        a.aggregation_bits_base_mut()
+            .unwrap()
+            .set(i, false)
+            .expect("should unset aggregation bit")
     }
 
     fn unset_sync_contribution_bit(a: &mut SyncCommitteeContribution<E>, i: usize) {
@@ -675,8 +676,8 @@ mod tests {
         a.data().beacon_block_root == block_root
     }
 
-    fn key_from_attestation(a: &Attestation<E>) -> AttestationKey {
-        AttestationKey::from_attestation_ref(a.to_ref()).expect("should create attestation key")
+    fn key_from_attestation(a: &Attestation<E>) -> AttestationData {
+        a.data().clone()
     }
 
     fn mutate_sync_contribution_block_root(

beacon_node/beacon_chain/src/observed_aggregates.rs

@@ -117,45 +117,32 @@ impl<'a, E: EthSpec> SubsetItem for AttestationRef<'a, E> {
     type Item = BitList<E::MaxValidatorsPerSlot>;
     fn is_subset(&self, other: &Self::Item) -> bool {
         match self {
-            Self::Base(att) => {
-                if let Ok(extended_aggregation_bits) = att.extend_aggregation_bits() {
-                    return extended_aggregation_bits.is_subset(other);
-                }
-                false
-            }
-            Self::Electra(att) => att.aggregation_bits.is_subset(other),
+            Attestation::Base(att) => att.aggregation_bits.is_subset(other),
+            // TODO(electra) implement electra variant
+            Attestation::Electra(_) => todo!(),
         }
     }
 
     fn is_superset(&self, other: &Self::Item) -> bool {
         match self {
-            Self::Base(att) => {
-                if let Ok(extended_aggregation_bits) = att.extend_aggregation_bits() {
-                    return other.is_subset(&extended_aggregation_bits);
-                }
-                false
-            }
-            Self::Electra(att) => other.is_subset(&att.aggregation_bits),
+            Attestation::Base(att) => other.is_subset(&att.aggregation_bits),
+            // TODO(electra) implement electra variant
+            Attestation::Electra(_) => todo!(),
         }
     }
 
     /// Returns the sync contribution aggregation bits.
-    fn get_item(&self) -> Result<Self::Item, Error> {
+    fn get_item(&self) -> Self::Item {
         match self {
-            Self::Base(att) => att
-                .extend_aggregation_bits()
-                .map_err(|_| Error::GetItemError),
-            Self::Electra(att) => Ok(att.aggregation_bits.clone()),
+            Attestation::Base(att) => att.aggregation_bits.clone(),
+            // TODO(electra) implement electra variant
+            Attestation::Electra(_) => todo!(),
         }
     }
 
-    /// Returns the hash tree root of the attestation data augmented with the committee index.
-    fn root(&self) -> Result<Hash256, Error> {
-        Ok(ObservedAttestationKey {
-            committee_index: self.committee_index().ok_or(Error::RootError)?,
-            attestation_data: self.data().clone(),
-        }
-        .tree_hash_root())
+    /// Returns the hash tree root of the attestation data.
+    fn root(&self) -> Hash256 {
+        self.data().tree_hash_root()
     }
 }
 

beacon_node/beacon_chain/src/observed_operations.rs

@@ -61,12 +61,12 @@ impl<E: EthSpec> ObservableOperation<E> for ProposerSlashing {
 impl<E: EthSpec> ObservableOperation<E> for AttesterSlashing<E> {
     fn observed_validators(&self) -> SmallVec<[u64; SMALL_VEC_SIZE]> {
         let attestation_1_indices = self
-            .attestation_1()
+            .attestation_1
             .attesting_indices_iter()
             .copied()
             .collect::<HashSet<u64>>();
         let attestation_2_indices = self
-            .attestation_2()
+            .attestation_2
             .attesting_indices_iter()
             .copied()
             .collect::<HashSet<u64>>();