Add committee_index to aggregator

Fixes a bug where the validator index bit was set on the bitfield,
instead of the committee index

beacon_node/beacon_chain/src/attestation_aggregator.rs

@@ -1,7 +1,7 @@
 use std::collections::{HashMap, HashSet};
 use types::{
-    AggregateSignature, Attestation, AttestationData, BeaconState, Bitfield, ChainSpec,
-    FreeAttestation, Signature,
+    beacon_state::CommitteesError, AggregateSignature, Attestation, AttestationData, BeaconState,
+    Bitfield, ChainSpec, FreeAttestation, Signature,
 };
 
 const PHASE_0_CUSTODY_BIT: bool = false;
@@ -21,6 +21,9 @@ pub enum ProcessOutcome {
 pub enum ProcessError {
     BadValidatorIndex,
     BadSignature,
+    BadSlot,
+    BadShard,
+    CommitteesError(CommitteesError),
 }
 
 impl AttestationAggregator {
@@ -34,13 +37,25 @@ impl AttestationAggregator {
         &mut self,
         state: &BeaconState,
         free_attestation: &FreeAttestation,
+        spec: &ChainSpec,
     ) -> Result<ProcessOutcome, ProcessError> {
-        let validator_index = free_attestation.validator_index as usize;
+        // let validator_index = free_attestation.validator_index as usize;
+        let (slot, shard, committee_index) = state.attestation_slot_and_shard_for_validator(
+            free_attestation.validator_index as usize,
+            spec,
+        )?;
+
+        if free_attestation.data.slot != slot {
+            return Err(ProcessError::BadSlot);
+        }
+        if free_attestation.data.shard != shard {
+            return Err(ProcessError::BadShard);
+        }
 
         let signable_message = free_attestation.data.signable_message(PHASE_0_CUSTODY_BIT);
         let validator_pubkey = &state
             .validator_registry
-            .get(validator_index)
+            .get(free_attestation.validator_index as usize)
             .ok_or_else(|| ProcessError::BadValidatorIndex)?
             .pubkey;
 
@@ -55,7 +70,7 @@ impl AttestationAggregator {
             if let Some(updated_attestation) = aggregate_attestation(
                 existing_attestation,
                 &free_attestation.signature,
-                validator_index,
+                committee_index as usize,
             ) {
                 self.store.insert(signable_message, updated_attestation);
                 Ok(ProcessOutcome::Aggregated)
@@ -66,7 +81,7 @@ impl AttestationAggregator {
             let mut aggregate_signature = AggregateSignature::new();
             aggregate_signature.add(&free_attestation.signature);
             let mut aggregation_bitfield = Bitfield::new();
-            aggregation_bitfield.set(validator_index, true);
+            aggregation_bitfield.set(committee_index as usize, true);
             let new_attestation = Attestation {
                 data: free_attestation.data.clone(),
                 aggregation_bitfield,
@@ -113,18 +128,18 @@ impl AttestationAggregator {
 fn aggregate_attestation(
     existing_attestation: &Attestation,
     signature: &Signature,
-    validator_index: usize,
+    committee_index: usize,
 ) -> Option<Attestation> {
     let already_signed = existing_attestation
         .aggregation_bitfield
-        .get(validator_index)
+        .get(committee_index)
         .unwrap_or(false);
 
     if already_signed {
         None
     } else {
         let mut aggregation_bitfield = existing_attestation.aggregation_bitfield.clone();
-        aggregation_bitfield.set(validator_index, true);
+        aggregation_bitfield.set(committee_index, true);
         let mut aggregate_signature = existing_attestation.aggregate_signature.clone();
         aggregate_signature.add(&signature);
 
@@ -135,3 +150,9 @@ fn aggregate_attestation(
         })
     }
 }
+
+impl From<CommitteesError> for ProcessError {
+    fn from(e: CommitteesError) -> ProcessError {
+        ProcessError::CommitteesError(e)
+    }
+}

beacon_node/beacon_chain/src/attestation_processing.rs

@@ -27,7 +27,7 @@ where
         self.attestation_aggregator
             .write()
             .expect("Aggregator unlock failed.")
-            .process_free_attestation(&state, &free_attestation)
+            .process_free_attestation(&state, &free_attestation, &self.spec)
             .map_err(|e| e.into())
     }
 }

beacon_node/beacon_chain/src/info.rs

@@ -67,9 +67,10 @@ where
         let present_slot = self.present_slot()?;
         let state = self.state(present_slot).ok()?;
 
-        state
+        let (slot, shard, _committee) = state
             .attestation_slot_and_shard_for_validator(validator_index, &self.spec)
-            .ok()
+            .ok()?;
+        Some((slot, shard))
     }
 }
 

eth2/types/src/beacon_state/attestation_participants.rs

@@ -1,6 +1,8 @@
 use crate::{
-    beacon_state::CommitteesError, PendingAttestation, AttestationData, BeaconState, Bitfield, ChainSpec,
+    beacon_state::CommitteesError, AttestationData, BeaconState, Bitfield, ChainSpec,
+    PendingAttestation,
 };
+use log::debug;
 
 #[derive(Debug, PartialEq)]
 pub enum Error {
@@ -74,9 +76,16 @@ impl BeaconState {
         let mut participants = vec![];
         for (i, validator_index) in crosslink_committee.iter().enumerate() {
             if aggregation_bitfield.get(i).unwrap() {
+                debug!(
+                    "committee index {} found in attestation on slot {}",
+                    i, attestation_data.slot
+                );
                 participants.push(*validator_index);
             } else {
-                debug!("get_attestation_participants: validator missing.");
+                debug!(
+                    "committee index {} not found in attestation on slot {}",
+                    i, attestation_data.slot
+                );
             }
         }
         Ok(participants)

eth2/types/src/beacon_state/committees.rs

@@ -82,13 +82,6 @@ impl BeaconState {
         slot: u64,
         spec: &ChainSpec,
     ) -> Result<Vec<(Vec<usize>, u64)>> {
-        /*
-        let previous_epoch_range = self.get_current_epoch_boundaries(spec.epoch_length);
-        let current_epoch_range = self.get_current_epoch_boundaries(spec.epoch_length);
-        if !range_contains(&current_epoch_range, slot) {
-            return Err(Error::InvalidEpoch(slot, current_epoch_range));
-        }
-        */
         let epoch = slot / spec.epoch_length;
         let current_epoch = self.slot / spec.epoch_length;
         let previous_epoch = if current_epoch == spec.genesis_slot {
@@ -98,13 +91,6 @@ impl BeaconState {
         };
         let next_epoch = current_epoch + 1;
 
-        /*
-        debug!(
-            "state.slot: {}, slot: {}, current_epoch: {}, previous_epoch: {}, next_epoch: {}",
-            self.slot, slot, current_epoch, previous_epoch, next_epoch
-        );
-        */
-
         ensure!(
             (previous_epoch <= epoch) & (epoch < next_epoch),
             Error::InvalidEpoch(slot, previous_epoch..current_epoch)

eth2/types/src/beacon_state/slot_processing.rs

@@ -29,12 +29,12 @@ impl BeaconState {
         &self,
         validator_index: usize,
         spec: &ChainSpec,
-    ) -> Result<(u64, u64), CommitteesError> {
+    ) -> Result<(u64, u64, u64), CommitteesError> {
         let mut result = None;
         for slot in self.get_current_epoch_boundaries(spec.epoch_length) {
             for (committee, shard) in self.get_crosslink_committees_at_slot(slot, spec)? {
-                if committee.iter().find(|i| **i == validator_index).is_some() {
-                    result = Some(Ok((slot, shard)));
+                if let Some(committee_index) = committee.iter().find(|i| **i == validator_index) {
+                    result = Some(Ok((slot, shard, *committee_index as u64)));
                 }
             }
         }