beacon chain tests are passing

beacon_node/beacon_chain/src/attestation_verification.rs

@@ -60,7 +60,7 @@ use strum::AsRefStr;
 use tree_hash::TreeHash;
 use tree_hash_derive::TreeHash;
 use types::{
-    Attestation, AttestationData, AttestationRef, BeaconCommittee,
+    Attestation, AttestationData, AttestationRef, BeaconCommittee, BeaconStateError,
     BeaconStateError::NoCommitteeFound, ChainSpec, CommitteeIndex, Epoch, EthSpec, ForkName,
     Hash256, IndexedAttestation, SelectionProof, SignedAggregateAndProof, Slot, SubnetId,
 };
@@ -264,9 +264,30 @@ pub enum Error {
     BeaconChainError(BeaconChainError),
 }
 
+// TODO(electra) the error conversion changes here are to get a test case to pass
+// this could easily be cleaned up
 impl From<BeaconChainError> for Error {
     fn from(e: BeaconChainError) -> Self {
-        Error::BeaconChainError(e)
+        match &e {
+            BeaconChainError::BeaconStateError(beacon_state_error) => {
+                if let BeaconStateError::AggregatorNotInCommittee { aggregator_index } =
+                    beacon_state_error
+                {
+                    Self::AggregatorNotInCommittee {
+                        aggregator_index: *aggregator_index,
+                    }
+                } else if let BeaconStateError::InvalidSelectionProof { aggregator_index } =
+                    beacon_state_error
+                {
+                    Self::InvalidSelectionProof {
+                        aggregator_index: *aggregator_index,
+                    }
+                } else {
+                    Error::BeaconChainError(e)
+                }
+            }
+            _ => Error::BeaconChainError(e),
+        }
     }
 }
 
@@ -404,7 +425,6 @@ fn process_slash_info<T: BeaconChainTypes>(
     use AttestationSlashInfo::*;
 
     if let Some(slasher) = chain.slasher.as_ref() {
-
         let (indexed_attestation, check_signature, err) = match slash_info {
             SignatureNotChecked(attestation, err) => {
                 match obtain_indexed_attestation_and_committees_per_slot(chain, attestation) {
@@ -436,7 +456,6 @@ fn process_slash_info<T: BeaconChainTypes>(
             }
         }
 
-
         // Supply to slasher.
         slasher.accept_attestation(indexed_attestation);
 
@@ -565,9 +584,6 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
         chain: &BeaconChain<T>,
     ) -> Result<Self, AttestationSlashInfo<'a, T, Error>> {
         use AttestationSlashInfo::*;
-
-        let attestation = signed_aggregate.message().aggregate();
-        let aggregator_index = signed_aggregate.message().aggregator_index();
         let observed_attestation_key_root = match Self::verify_early_checks(signed_aggregate, chain)
         {
             Ok(root) => root,
@@ -580,8 +596,10 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
         };
         let get_indexed_attestation_with_committee =
             |(committees, _): (Vec<BeaconCommittee>, CommitteesPerSlot)| {
-                match attestation {
-                    AttestationRef::Base(att) => {
+                match signed_aggregate {
+                    SignedAggregateAndProof::Base(signed_aggregate) => {
+                        let att = &signed_aggregate.message.aggregate;
+                        let aggregator_index = signed_aggregate.message.aggregator_index;
                         let committee = committees
                             .iter()
                             .filter(|&committee| committee.index == att.data.index)
@@ -591,13 +609,13 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
                                 index: att.data.index,
                             })?;
 
+                        // TODO(electra):
+                        // Note: this clones the signature which is known to be a relatively slow operation.
+                        //
+                        // Future optimizations should remove this clone.
                         if let Some(committee) = committee {
-                            // TODO(electra):
-                            // 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(),
+                                signed_aggregate.message.selection_proof.clone(),
                             );
 
                             if !selection_proof
@@ -611,7 +629,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
                             if !committee.committee.contains(&(aggregator_index as usize)) {
                                 return Err(Error::AggregatorNotInCommittee { aggregator_index });
                             }
-                            
+
                             attesting_indices_base::get_indexed_attestation(
                                 committee.committee,
                                 att,
@@ -624,27 +642,18 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
                             })
                         }
                     }
-                    AttestationRef::Electra(att) => {
-                        for committee in committees.iter() {
-                            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 });
-                            }
-                        }
-
-                        attesting_indices_electra::get_indexed_attestation(&committees, att)
-                            .map_err(|e| BeaconChainError::from(e).into())
+                    SignedAggregateAndProof::Electra(signed_aggregate) => {
+                        attesting_indices_electra::get_indexed_attestation_from_signed_aggregate(
+                            &committees,
+                            signed_aggregate,
+                            &chain.spec,
+                        )
+                        .map_err(|e| BeaconChainError::from(e).into())
                     }
                 }
             };
 
-
+        let attestation = signed_aggregate.message().aggregate();
         let indexed_attestation = match map_attestation_committees(
             chain,
             attestation,

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -2063,7 +2063,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             // This method is called for API and gossip attestations, so this covers all aggregated attestation events
             if let Some(event_handler) = self.event_handler.as_ref() {
                 if event_handler.has_attestation_subscribers() {
-
                     event_handler.register(EventKind::Attestation(Box::new(
                         v.attestation().clone_as_attestation(),
                     )));

beacon_node/beacon_chain/tests/attestation_verification.rs

@@ -423,6 +423,7 @@ impl GossipTester {
         /*
          * Batch verification
          */
+        println!("desc {}", desc);
         let mut results = self
             .harness
             .chain
@@ -771,6 +772,7 @@ async fn aggregated_gossip_verification() {
                 }
             },
             |_, err| {
+                println!("{:?}", err);
                 assert!(matches!(
                     err,
                     // Naively we should think this condition would trigger this error:
@@ -780,12 +782,7 @@ async fn aggregated_gossip_verification() {
                     // However, the following error is triggered first:
                     AttnError::AggregatorNotInCommittee {
                         aggregator_index
-                    } |
-                    // unless were working with electra attestations
-                    // in which case this error is triggered instead:
-                    AttnError::AggregatorPubkeyUnknown(
-                        aggregator_index
-                    )
+                    }
                     if aggregator_index == VALIDATOR_COUNT as u64
                 ))
             },

consensus/state_processing/src/common/get_attesting_indices.rs

@@ -51,6 +51,100 @@ pub mod attesting_indices_electra {
     use safe_arith::SafeArith;
     use types::*;
 
+    // TODO(electra) remove duplicate code
+    // get_indexed_attestation is almost an exact duplicate
+    // the only differences are the invalid selection proof 
+    // and aggregator not in committee checks
+    pub fn get_indexed_attestation_from_signed_aggregate<E: EthSpec>(
+        committees: &[BeaconCommittee],
+        signed_aggregate: &SignedAggregateAndProofElectra<E>,
+        spec: &ChainSpec,
+    ) -> Result<IndexedAttestation<E>, BeaconStateError> {
+        let mut output: HashSet<u64> = HashSet::new();
+
+        let committee_bits = &signed_aggregate.message.aggregate.committee_bits;
+        let aggregation_bits = &signed_aggregate.message.aggregate.aggregation_bits;
+        let aggregator_index = signed_aggregate.message.aggregator_index;
+        let attestation = &signed_aggregate.message.aggregate;
+
+        let committee_indices = get_committee_indices::<E>(committee_bits);
+
+        let mut committee_offset = 0;
+
+        let committees_map: HashMap<u64, &BeaconCommittee> = committees
+            .iter()
+            .map(|committee| (committee.index, committee))
+            .collect();
+
+        let committee_count_per_slot = committees.len() as u64;
+        let mut participant_count = 0;
+
+        // TODO(electra):
+        // 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());
+
+        for index in committee_indices {
+            if let Some(&beacon_committee) = committees_map.get(&index) {
+                if !selection_proof
+                    .is_aggregator(beacon_committee.committee.len(), spec)
+                    .map_err(BeaconStateError::ArithError)?
+                {
+                    return Err(BeaconStateError::InvalidSelectionProof { aggregator_index });
+                }
+
+                if !beacon_committee
+                    .committee
+                    .contains(&(aggregator_index as usize))
+                {
+                    return Err(BeaconStateError::AggregatorNotInCommittee { aggregator_index });
+                }
+
+                // This check is new to the spec's `process_attestation` in Electra.
+                if index >= committee_count_per_slot {
+                    return Err(BeaconStateError::InvalidCommitteeIndex(index));
+                }
+
+                participant_count.safe_add_assign(beacon_committee.committee.len() as u64)?;
+                let committee_attesters = beacon_committee
+                    .committee
+                    .iter()
+                    .enumerate()
+                    .filter_map(|(i, &index)| {
+                        if let Ok(aggregation_bit_index) = committee_offset.safe_add(i) {
+                            if aggregation_bits.get(aggregation_bit_index).unwrap_or(false) {
+                                return Some(index as u64);
+                            }
+                        }
+                        None
+                    })
+                    .collect::<HashSet<u64>>();
+
+                output.extend(committee_attesters);
+
+                committee_offset.safe_add_assign(beacon_committee.committee.len())?;
+            } else {
+                return Err(Error::NoCommitteeFound(index));
+            }
+        }
+
+        // This check is new to the spec's `process_attestation` in Electra.
+        if participant_count as usize != aggregation_bits.len() {
+            return Err(Error::InvalidBitfield);
+        }
+
+        let mut indices = output.into_iter().collect_vec();
+        indices.sort_unstable();
+
+        Ok(IndexedAttestation::Electra(IndexedAttestationElectra {
+            attesting_indices: VariableList::new(indices)?,
+            data: attestation.data.clone(),
+            signature: attestation.signature.clone(),
+        }))
+    }
+
     pub fn get_indexed_attestation<E: EthSpec>(
         committees: &[BeaconCommittee],
         attestation: &AttestationElectra<E>,

consensus/state_processing/src/per_block_processing/signature_sets.rs

@@ -437,7 +437,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();
-
+    println!("validator index, {}", validator_index);
     Ok(SignatureSet::single_pubkey(
         signature,
         get_pubkey(validator_index as usize).ok_or(Error::ValidatorUnknown(validator_index))?,

consensus/types/src/beacon_state.rs

@@ -163,6 +163,12 @@ pub enum Error {
     NonExecutionAddresWithdrawalCredential,
     NoCommitteeFound(CommitteeIndex),
     InvalidCommitteeIndex(CommitteeIndex),
+    InvalidSelectionProof {
+        aggregator_index: u64,
+    },
+    AggregatorNotInCommittee {
+        aggregator_index: u64,
+    },
 }
 
 /// Control whether an epoch-indexed field can be indexed at the next epoch or not.

consensus/types/src/signed_aggregate_and_proof.rs

@@ -74,7 +74,6 @@ impl<E: EthSpec> SignedAggregateAndProof<E> {
             genesis_validators_root,
             spec,
         );
-
         let target_epoch = message.aggregate().data().slot.epoch(E::slots_per_epoch());
         let domain = spec.get_domain(
             target_epoch,