EIP7549 get_attestation_indices (sigp#5657)

* get attesting indices electra impl

* fmt

* get tests to pass

* fmt

* fix some beacon chain tests

* fmt

* fix slasher test

* fmt got me again

* fix more tests

* fix tests

Author: eserilev

beacon_node/beacon_chain/src/attestation_verification.rs

@@ -47,10 +47,7 @@ use proto_array::Block as ProtoBlock;
 use slog::debug;
 use slot_clock::SlotClock;
 use state_processing::{
-    common::{
-        attesting_indices_base,
-        attesting_indices_electra::{self, get_committee_indices},
-    },
+    common::{attesting_indices_base, attesting_indices_electra},
     per_block_processing::errors::{AttestationValidationError, BlockOperationError},
     signature_sets::{
         indexed_attestation_signature_set_from_pubkeys,
@@ -61,8 +58,9 @@ use std::borrow::Cow;
 use strum::AsRefStr;
 use tree_hash::TreeHash;
 use types::{
-    Attestation, AttestationRef, BeaconCommittee, ChainSpec, CommitteeIndex, Epoch, EthSpec,
-    ForkName, Hash256, IndexedAttestation, SelectionProof, SignedAggregateAndProof, Slot, SubnetId,
+    Attestation, AttestationRef, BeaconCommittee, BeaconStateError::NoCommitteeFound, ChainSpec,
+    CommitteeIndex, Epoch, EthSpec, ForkName, Hash256, IndexedAttestation, SelectionProof,
+    SignedAggregateAndProof, Slot, SubnetId,
 };
 
 pub use batch::{batch_verify_aggregated_attestations, batch_verify_unaggregated_attestations};
@@ -572,37 +570,59 @@ 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 =
-            |(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)
-                    .ok_or(Error::NoCommitteeForSlotAndIndex { slot, index })?;
-
-                if !SelectionProof::from(selection_proof)
-                    .is_aggregator(committee.committee.len(), &chain.spec)
-                    .map_err(|e| Error::BeaconChainError(e.into()))?
-                {
-                    return Err(Error::InvalidSelectionProof { aggregator_index });
-                }
-
-                // Ensure the aggregator is a member of the committee for which it is aggregating.
-                if !committee.committee.contains(&(aggregator_index as usize)) {
-                    return Err(Error::AggregatorNotInCommittee { aggregator_index });
+            |(committees, _): (Vec<BeaconCommittee>, CommitteesPerSlot)| {
+                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 {
+                            // 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(),
+                            );
+
+                            if !selection_proof
+                                .is_aggregator(committee.committee.len(), &chain.spec)
+                                .map_err(|e| Error::BeaconChainError(e.into()))?
+                            {
+                                return Err(Error::InvalidSelectionProof { aggregator_index });
+                            }
+
+                            // Ensure the aggregator is a member of the committee for which it is aggregating.
+                            if !committee.committee.contains(&(aggregator_index as usize)) {
+                                return Err(Error::AggregatorNotInCommittee { aggregator_index });
+                            }
+                            attesting_indices_base::get_indexed_attestation(
+                                committee.committee,
+                                att,
+                            )
+                            .map_err(|e| BeaconChainError::from(e).into())
+                        } else {
+                            Err(Error::NoCommitteeForSlotAndIndex {
+                                slot: att.data.slot,
+                                index: att.data.index,
+                            })
+                        }
+                    }
+                    AttestationRef::Electra(att) => {
+                        attesting_indices_electra::get_indexed_attestation(&committees, att)
+                            .map_err(|e| BeaconChainError::from(e).into())
+                    }
                 }
-
-                get_indexed_attestation(committee.committee, attestation)
-                    .map_err(|e| BeaconChainError::from(e).into())
             };
 
-        let attestation = signed_aggregate.message().aggregate();
         let indexed_attestation = match map_attestation_committees(
             chain,
-            attestation,
+            &attestation,
             get_indexed_attestation_with_committee,
         ) {
             Ok(indexed_attestation) => indexed_attestation,
@@ -1310,13 +1330,49 @@ 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_committee(chain, attestation, |(committee, committees_per_slot)| {
-        get_indexed_attestation(committee.committee, attestation)
-            .map(|attestation| (attestation, committees_per_slot))
-            .map_err(Error::Invalid)
+    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 e == BlockOperationError::BeaconStateError(NoCommitteeFound) {
+                            Error::NoCommitteeForSlotAndIndex {
+                                slot: att.data.slot,
+                                index: att.committee_index(),
+                            }
+                        } else {
+                            Error::Invalid(e)
+                        }
+                    })
+            }
+        }
     })
 }
 
+// TODO(electra) update comments below to reflect logic changes
+// i.e. this now runs the map_fn on a list of committees for the slot of the provided attestation
 /// Runs the `map_fn` with the committee and committee count per slot for the given `attestation`.
 ///
 /// This function exists in this odd "map" pattern because efficiently obtaining the committees for
@@ -1326,11 +1382,9 @@ pub fn obtain_indexed_attestation_and_committees_per_slot<T: BeaconChainTypes>(
 ///
 /// If the committees for an `attestation`'s slot aren't found in the `shuffling_cache`, we will read a state
 /// from disk and then update the `shuffling_cache`.
-///
-/// Committees are sorted by ascending index order 0..committees_per_slot
 fn map_attestation_committees<T, F, R>(
     chain: &BeaconChain<T>,
-    attestation: AttestationRef<T::EthSpec>,
+    attestation: &AttestationRef<T::EthSpec>,
     map_fn: F,
 ) -> Result<R, Error>
 where
@@ -1361,12 +1415,12 @@ where
             let committees_per_slot = committee_cache.committees_per_slot();
 
             Ok(committee_cache
-                .get_beacon_committee(attestation.data().slot, attestation.data().index)
-                .map(|committee| map_fn((committee, committees_per_slot)))
-                .unwrap_or_else(|| {
+                .get_beacon_committees_at_slot(attestation.data().slot)
+                .map(|committees| map_fn((committees, committees_per_slot)))
+                .unwrap_or_else(|_| {
                     Err(Error::NoCommitteeForSlotAndIndex {
                         slot: attestation.data().slot,
-                        index: attestation.data().index,
+                        index: attestation.committee_index(),
                     })
                 }))
         })

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -1994,18 +1994,36 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             };
         drop(cache_timer);
 
-        // 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(),
-        }))
+        if self.spec.fork_name_at_slot::<T::EthSpec>(request_slot) >= ForkName::Electra {
+            let mut committee_bits = BitVector::default();
+            if committee_len > 0 {
+                committee_bits.set(request_index as usize, true)?;
+            }
+            Ok(Attestation::Electra(AttestationElectra {
+                aggregation_bits: BitList::with_capacity(committee_len)?,
+                data: AttestationData {
+                    slot: request_slot,
+                    index: 0u64,
+                    beacon_block_root,
+                    source: justified_checkpoint,
+                    target,
+                },
+                committee_bits,
+                signature: AggregateSignature::empty(),
+            }))
+        } else {
+            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

beacon_node/beacon_chain/src/early_attester_cache.rs

@@ -124,18 +124,40 @@ impl<E: EthSpec> EarlyAttesterCache<E> {
                 .get_committee_length::<E>(request_slot, request_index, spec)?;
 
         // 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(),
-        });
+        let attestation = if spec.fork_name_at_slot::<E>(request_slot) >= ForkName::Electra {
+            let mut committee_bits = BitVector::default();
+            if committee_len > 0 {
+                committee_bits
+                    .set(request_index as usize, true)
+                    .map_err(BeaconStateError::from)?;
+            }
+            Attestation::Electra(AttestationElectra {
+                aggregation_bits: BitList::with_capacity(committee_len)
+                    .map_err(BeaconStateError::from)?,
+                committee_bits,
+                data: AttestationData {
+                    slot: request_slot,
+                    index: 0u64,
+                    beacon_block_root: item.beacon_block_root,
+                    source: item.source,
+                    target: item.target,
+                },
+                signature: AggregateSignature::empty(),
+            })
+        } else {
+            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/observed_aggregates.rs

@@ -21,17 +21,6 @@ pub type ObservedSyncContributions<E> = ObservedAggregates<
 pub type ObservedAggregateAttestations<E> =
     ObservedAggregates<Attestation<E>, E, BitList<<E as types::EthSpec>::MaxValidatorsPerSlot>>;
 
-/// Attestation data augmented with committee index
-///
-/// This is hashed and used to key the map of observed aggregate attestations. This is important
-/// post-Electra where the attestation data committee index is 0 and we want to avoid accidentally
-/// comparing aggregation bits for *different* committees.
-#[derive(TreeHash)]
-pub struct ObservedAttestationKey {
-    pub committee_index: u64,
-    pub attestation_data: AttestationData,
-}
-
 /// A trait use to associate capacity constants with the type being stored in `ObservedAggregates`.
 pub trait Consts {
     /// The default capacity of items stored per slot, in a single `SlotHashSet`.
@@ -112,29 +101,39 @@ pub trait SubsetItem {
 }
 
 impl<'a, E: EthSpec> SubsetItem for AttestationRef<'a, E> {
-    type Item = BitList<E::MaxValidatorsPerCommittee>;
+    type Item = BitList<E::MaxValidatorsPerSlot>;
     fn is_subset(&self, other: &Self::Item) -> bool {
         match self {
-            Self::Base(att) => att.aggregation_bits.is_subset(other),
-            // TODO(electra) implement electra variant
-            Self::Electra(_) => todo!(),
+            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),
         }
     }
 
     fn is_superset(&self, other: &Self::Item) -> bool {
         match self {
-            Self::Base(att) => other.is_subset(&att.aggregation_bits),
-            // TODO(electra) implement electra variant
-            Self::Electra(_) => todo!(),
+            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),
         }
     }
 
     /// Returns the sync contribution aggregation bits.
     fn get_item(&self) -> Self::Item {
         match self {
-            Self::Base(att) => att.aggregation_bits.clone(),
-            // TODO(electra) implement electra variant
-            Self::Electra(_) => todo!(),
+            Self::Base(att) => {
+                // TODO(electra) fix unwrap
+                att.extend_aggregation_bits().unwrap()
+            }
+            Self::Electra(att) => att.aggregation_bits.clone(),
         }
     }