superstruct the AttesterSlashing (sigp#5636)

* `superstruct` Attester Fork Variants

* Push a little further

* Deal with Encode / Decode of AttesterSlashing

* not so sure about this..

* Stop Encode/Decode Bounds from Propagating Out

* Tons of Changes..

* More Conversions to AttestationRef

* Add AsReference trait (#15)

* Add AsReference trait

* Fix some snafus

* Got it Compiling! :D

* Got Tests Building

* Get beacon chain tests compiling

---------

Co-authored-by: Michael Sproul <micsproul@gmail.com>

beacon_node/beacon_chain/src/attestation_verification.rs

@@ -496,7 +496,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
         if chain
             .observed_attestations
             .write()
-            .is_known_subset(attestation, observed_attestation_key_root)
+            .is_known_subset(attestation.to_ref(), attestation_data_root)
             .map_err(|e| Error::BeaconChainError(e.into()))?
         {
             metrics::inc_counter(&metrics::AGGREGATED_ATTESTATION_SUBSETS);
@@ -610,24 +610,8 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
                     return Err(Error::AggregatorNotInCommittee { aggregator_index });
                 }
 
-                // p2p aggregates have a single committee, we can assert that aggregation_bits is always
-                // less then MaxValidatorsPerCommittee
-                match signed_aggregate {
-                    SignedAggregateAndProof::Base(signed_aggregate) => {
-                        attesting_indices_base::get_indexed_attestation(
-                            committee.committee,
-                            &signed_aggregate.message.aggregate,
-                        )
-                        .map_err(|e| BeaconChainError::from(e).into())
-                    }
-                    SignedAggregateAndProof::Electra(signed_aggregate) => {
-                        attesting_indices_electra::get_indexed_attestation(
-                            &committees,
-                            &signed_aggregate.message.aggregate,
-                        )
-                        .map_err(|e| BeaconChainError::from(e).into())
-                    }
-                }
+                get_indexed_attestation(committee.committee, attestation.to_ref())
+                    .map_err(|e| BeaconChainError::from(e).into())
             };
 
         let attestation = signed_aggregate.message().aggregate();
@@ -669,7 +653,7 @@ impl<'a, T: BeaconChainTypes> VerifiedAggregatedAttestation<'a, T> {
         if let ObserveOutcome::Subset = chain
             .observed_attestations
             .write()
-            .observe_item(attestation, Some(observed_attestation_key_root))
+            .observe_item(attestation.to_ref(), Some(attestation_data_root))
             .map_err(|e| Error::BeaconChainError(e.into()))?
         {
             metrics::inc_counter(&metrics::AGGREGATED_ATTESTATION_SUBSETS);
@@ -1341,44 +1325,10 @@ pub fn obtain_indexed_attestation_and_committees_per_slot<T: BeaconChainTypes>(
     chain: &BeaconChain<T>,
     attestation: AttestationRef<T::EthSpec>,
 ) -> Result<(IndexedAttestation<T::EthSpec>, CommitteesPerSlot), Error> {
-    map_attestation_committees(chain, attestation, |(committees, committees_per_slot)| {
-        match attestation {
-            AttestationRef::Base(att) => {
-                let committee = committees
-                    .iter()
-                    .filter(|&committee| committee.index == att.data.index)
-                    .at_most_one()
-                    .map_err(|_| Error::NoCommitteeForSlotAndIndex {
-                        slot: att.data.slot,
-                        index: att.data.index,
-                    })?;
-
-                if let Some(committee) = committee {
-                    attesting_indices_base::get_indexed_attestation(committee.committee, att)
-                        .map(|attestation| (attestation, committees_per_slot))
-                        .map_err(Error::Invalid)
-                } else {
-                    Err(Error::NoCommitteeForSlotAndIndex {
-                        slot: att.data.slot,
-                        index: att.data.index,
-                    })
-                }
-            }
-            AttestationRef::Electra(att) => {
-                attesting_indices_electra::get_indexed_attestation(&committees, att)
-                    .map(|attestation| (attestation, committees_per_slot))
-                    .map_err(|e| {
-                        if let BlockOperationError::BeaconStateError(NoCommitteeFound(index)) = e {
-                            Error::NoCommitteeForSlotAndIndex {
-                                slot: att.data.slot,
-                                index,
-                            }
-                        } else {
-                            Error::Invalid(e)
-                        }
-                    })
-            }
-        }
+    map_attestation_committee(chain, attestation, |(committee, committees_per_slot)| {
+        get_indexed_attestation(committee.committee, attestation.to_ref())
+            .map(|attestation| (attestation, committees_per_slot))
+            .map_err(Error::Invalid)
     })
 }
 

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -2556,14 +2556,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             .fork_choice_write_lock()
             .on_attester_slashing(attester_slashing.as_inner().to_ref());
 
-        if let Some(event_handler) = self.event_handler.as_ref() {
-            if event_handler.has_attester_slashing_subscribers() {
-                event_handler.register(EventKind::AttesterSlashing(Box::new(
-                    attester_slashing.clone().into_inner(),
-                )));
-            }
-        }
-
         // Add to the op pool (if we have the ability to propose blocks).
         if self.eth1_chain.is_some() {
             self.op_pool.insert_attester_slashing(attester_slashing)
@@ -3966,7 +3958,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         if block.slot() + 2 * T::EthSpec::slots_per_epoch() >= current_slot {
             metrics::observe(
                 &metrics::OPERATIONS_PER_BLOCK_ATTESTATION,
-                block.body().attestations_len() as f64,
+                block.body().attestations().count() as f64,
             );
 
             if let Ok(sync_aggregate) = block.body().sync_aggregate() {
@@ -5184,6 +5176,50 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             },
         );
 
+        // TODO(electra): figure out what should *actually* be done here when we have attestations / attester_slashings of the wrong type
+        match &state {
+            BeaconState::Base(_)
+            | BeaconState::Altair(_)
+            | BeaconState::Merge(_)
+            | BeaconState::Capella(_)
+            | BeaconState::Deneb(_) => {
+                if !attestations_electra.is_empty() {
+                    error!(
+                        self.log,
+                        "Tried to produce block with attestations of the wrong type";
+                        "slot" => slot,
+                        "attestations" => attestations_electra.len(),
+                    );
+                }
+                if !attester_slashings_electra.is_empty() {
+                    error!(
+                        self.log,
+                        "Tried to produce block with attester slashings of the wrong type";
+                        "slot" => slot,
+                        "attester_slashings" => attester_slashings_electra.len(),
+                    );
+                }
+            }
+            BeaconState::Electra(_) => {
+                if !attestations_base.is_empty() {
+                    error!(
+                        self.log,
+                        "Tried to produce block with attestations of the wrong type";
+                        "slot" => slot,
+                        "attestations" => attestations_base.len(),
+                    );
+                }
+                if !attester_slashings_base.is_empty() {
+                    error!(
+                        self.log,
+                        "Tried to produce block with attester slashings of the wrong type";
+                        "slot" => slot,
+                        "attester_slashings" => attester_slashings_base.len(),
+                    );
+                }
+            }
+        };
+
         let (inner_block, maybe_blobs_and_proofs, execution_payload_value) = match &state {
             BeaconState::Base(_) => (
                 BeaconBlock::Base(BeaconBlockBase {

beacon_node/beacon_chain/src/block_reward.rs

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

beacon_node/beacon_chain/src/observed_aggregates.rs

@@ -11,9 +11,7 @@ use types::consts::altair::{
     SYNC_COMMITTEE_SUBNET_COUNT, TARGET_AGGREGATORS_PER_SYNC_SUBCOMMITTEE,
 };
 use types::slot_data::SlotData;
-use types::{
-    Attestation, AttestationData, AttestationRef, EthSpec, Hash256, Slot, SyncCommitteeContribution,
-};
+use types::{Attestation, AttestationRef, EthSpec, Hash256, Slot, SyncCommitteeContribution};
 
 pub type ObservedSyncContributions<E> = ObservedAggregates<
     SyncCommitteeContribution<E>,
@@ -114,29 +112,29 @@ pub trait SubsetItem {
 }
 
 impl<'a, E: EthSpec> SubsetItem for AttestationRef<'a, E> {
-    type Item = BitList<E::MaxValidatorsPerSlot>;
+    type Item = BitList<E::MaxValidatorsPerCommittee>;
     fn is_subset(&self, other: &Self::Item) -> bool {
         match self {
-            Attestation::Base(att) => att.aggregation_bits.is_subset(other),
+            Self::Base(att) => att.aggregation_bits.is_subset(other),
             // TODO(electra) implement electra variant
-            Attestation::Electra(_) => todo!(),
+            Self::Electra(_) => todo!(),
         }
     }
 
     fn is_superset(&self, other: &Self::Item) -> bool {
         match self {
-            Attestation::Base(att) => other.is_subset(&att.aggregation_bits),
+            Self::Base(att) => other.is_subset(&att.aggregation_bits),
             // TODO(electra) implement electra variant
-            Attestation::Electra(_) => todo!(),
+            Self::Electra(_) => todo!(),
         }
     }
 
     /// Returns the sync contribution aggregation bits.
     fn get_item(&self) -> Self::Item {
         match self {
-            Attestation::Base(att) => att.aggregation_bits.clone(),
+            Self::Base(att) => att.aggregation_bits.clone(),
             // TODO(electra) implement electra variant
-            Attestation::Electra(_) => todo!(),
+            Self::Electra(_) => todo!(),
         }
     }
 
@@ -465,8 +463,7 @@ mod tests {
     type E = types::MainnetEthSpec;
 
     fn get_attestation(slot: Slot, beacon_block_root: u64) -> Attestation<E> {
-        let a: AttestationBase<E> = test_random_instance();
-        let mut a = Attestation::Base(a);
+        let mut a: Attestation<E> = test_random_instance();
         a.data_mut().slot = slot;
         a.data_mut().beacon_block_root = Hash256::from_low_u64_be(beacon_block_root);
         a
@@ -494,10 +491,7 @@ mod tests {
 
                     for a in &items {
                         assert_eq!(
-                            store.is_known_subset(
-                                a.as_reference(),
-                                a.as_reference().root().unwrap()
-                            ),
+                            store.is_known_subset(a.as_reference(), a.as_reference().root()),
                             Ok(false),
                             "should indicate an unknown attestation is unknown"
                         );
@@ -510,18 +504,12 @@ mod tests {
 
                     for a in &items {
                         assert_eq!(
-                            store.is_known_subset(
-                                a.as_reference(),
-                                a.as_reference().root().unwrap()
-                            ),
+                            store.is_known_subset(a.as_reference(), a.as_reference().root()),
                             Ok(true),
                             "should indicate a known attestation is known"
                         );
                         assert_eq!(
-                            store.observe_item(
-                                a.as_reference(),
-                                Some(a.as_reference().root().unwrap())
-                            ),
+                            store.observe_item(a.as_reference(), Some(a.as_reference().root())),
                             Ok(ObserveOutcome::Subset),
                             "should acknowledge an existing attestation"
                         );

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