Improve attester errors, move info -> helpers

- Ensured one can distingush between a committee error and an invalid
validator index when using `validator_attesation_slot_and_shard`.
- Renamed the `info.rs` file to `getters.rs`, for clarity.

beacon_node/beacon_chain/src/attestation_aggregator.rs

@@ -67,10 +67,12 @@ impl AttestationAggregator {
         free_attestation: &FreeAttestation,
         spec: &ChainSpec,
     ) -> Result<ProcessOutcome, Error> {
-        let (slot, shard, committee_index) = state.attestation_slot_and_shard_for_validator(
+        let (slot, shard, committee_index) = state
-            free_attestation.validator_index as usize,
+            .attestation_slot_and_shard_for_validator(
-            spec,
+                free_attestation.validator_index as usize,
-        )?;
+                spec,
+            )?
+            .ok_or_else(|| Error::BadValidatorIndex)?;
 
         if free_attestation.data.slot != slot {
             return Err(Error::BadSlot);

beacon_node/beacon_chain/src/getters.rs

@@ -0,0 +1,100 @@
+use super::{BeaconChain, ClientDB, SlotClock};
+use types::{beacon_state::CommitteesError, PublicKey};
+
+impl<T, U> BeaconChain<T, U>
+where
+    T: ClientDB,
+    U: SlotClock,
+{
+    /// Returns the the validator index (if any) for the given public key.
+    ///
+    /// Information is retrieved from the present `beacon_state.validator_registry`.
+    pub fn validator_index(&self, pubkey: &PublicKey) -> Option<usize> {
+        for (i, validator) in self
+            .head()
+            .beacon_state
+            .validator_registry
+            .iter()
+            .enumerate()
+        {
+            if validator.pubkey == *pubkey {
+                return Some(i);
+            }
+        }
+        None
+    }
+
+    /// Returns the number of slots the validator has been required to propose.
+    ///
+    /// Returns `None` if the `validator_index` is invalid.
+    ///
+    /// Information is retrieved from the present `beacon_state.validator_registry`.
+    pub fn proposer_slots(&self, validator_index: usize) -> Option<u64> {
+        if let Some(validator) = self.state.read().validator_registry.get(validator_index) {
+            Some(validator.proposer_slots)
+        } else {
+            None
+        }
+    }
+
+    /// Reads the slot clock, returns `None` if the slot is unavailable.
+    ///
+    /// The slot might be unavailable due to an error with the system clock, or if the present time
+    /// is before genesis (i.e., a negative slot).
+    ///
+    /// This is distinct to `present_slot`, which simply reads the latest state. If a
+    /// call to `read_slot_clock` results in a higher slot than a call to `present_slot`,
+    /// `self.state` should undergo per slot processing.
+    pub fn read_slot_clock(&self) -> Option<u64> {
+        match self.slot_clock.present_slot() {
+            Ok(some_slot) => some_slot,
+            _ => None,
+        }
+    }
+
+    /// Returns slot of the present state.
+    ///
+    /// This is distinct to `read_slot_clock`, which reads from the actual system clock. If
+    /// `self.state` has not been transitioned it is possible for the system clock to be on a
+    /// different slot to what is returned from this call.
+    pub fn present_slot(&self) -> u64 {
+        self.state.read().slot
+    }
+
+    /// Returns the block proposer for a given slot.
+    ///
+    /// Information is read from the present `beacon_state` shuffling, so only information from the
+    /// present and prior epoch is available.
+    pub fn block_proposer(&self, slot: u64) -> Result<usize, CommitteesError> {
+        let index = self
+            .state
+            .read()
+            .get_beacon_proposer_index(slot, &self.spec)?;
+
+        Ok(index)
+    }
+
+    /// Returns the justified slot for the present state.
+    pub fn justified_slot(&self) -> u64 {
+        self.state.read().justified_slot
+    }
+
+    /// Returns the attestation slot and shard for a given validator index.
+    ///
+    /// Information is read from the current state, so only information from the present and prior
+    /// epoch is available.
+    pub fn validator_attestion_slot_and_shard(
+        &self,
+        validator_index: usize,
+    ) -> Result<Option<(u64, u64)>, CommitteesError> {
+        if let Some((slot, shard, _committee)) = self
+            .state
+            .read()
+            .attestation_slot_and_shard_for_validator(validator_index, &self.spec)?
+        {
+            Ok(Some((slot, shard)))
+        } else {
+            Ok(None)
+        }
+    }
+}

beacon_node/beacon_chain/src/info.rs

@@ -1,76 +0,0 @@
-use super::{BeaconChain, ClientDB, SlotClock};
-use types::{beacon_state::CommitteesError, PublicKey};
-
-#[derive(Debug, PartialEq)]
-pub enum Error {
-    SlotClockError,
-    CommitteesError(CommitteesError),
-}
-
-impl<T, U> BeaconChain<T, U>
-where
-    T: ClientDB,
-    U: SlotClock,
-{
-    pub fn validator_index(&self, pubkey: &PublicKey) -> Option<usize> {
-        for (i, validator) in self
-            .head()
-            .beacon_state
-            .validator_registry
-            .iter()
-            .enumerate()
-        {
-            if validator.pubkey == *pubkey {
-                return Some(i);
-            }
-        }
-        None
-    }
-
-    pub fn proposer_slots(&self, validator_index: usize) -> Option<u64> {
-        if let Some(validator) = self.state.read().validator_registry.get(validator_index) {
-            Some(validator.proposer_slots)
-        } else {
-            None
-        }
-    }
-
-    pub fn read_slot_clock(&self) -> Option<u64> {
-        match self.slot_clock.present_slot() {
-            Ok(some_slot) => some_slot,
-            _ => None,
-        }
-    }
-
-    pub fn present_slot(&self) -> u64 {
-        self.state.read().slot
-    }
-
-    pub fn block_proposer(&self, slot: u64) -> Result<usize, CommitteesError> {
-        let index = self
-            .state
-            .read()
-            .get_beacon_proposer_index(slot, &self.spec)?;
-
-        Ok(index)
-    }
-
-    pub fn justified_slot(&self) -> u64 {
-        self.state.read().justified_slot
-    }
-
-    pub fn validator_attestion_slot_and_shard(&self, validator_index: usize) -> Option<(u64, u64)> {
-        let (slot, shard, _committee) = self
-            .state
-            .read()
-            .attestation_slot_and_shard_for_validator(validator_index, &self.spec)
-            .ok()?;
-        Some((slot, shard))
-    }
-}
-
-impl From<CommitteesError> for Error {
-    fn from(e: CommitteesError) -> Error {
-        Error::CommitteesError(e)
-    }
-}

beacon_node/beacon_chain/src/lib.rs

@@ -9,7 +9,7 @@ mod canonical_head;
 mod checkpoint;
 pub mod dump;
 mod finalized_head;
-mod info;
+mod getters;
 mod lmd_ghost;
 mod state;
 

beacon_node/beacon_chain/test_harness/src/validator/direct_duties.rs

@@ -53,9 +53,12 @@ impl<T: ClientDB, U: SlotClock> AttesterDutiesReader for DirectDuties<T, U> {
                 .beacon_chain
                 .validator_attestion_slot_and_shard(validator_index as usize)
             {
-                Some((attest_slot, attest_shard)) if attest_slot == slot => Ok(Some(attest_shard)),
+                Ok(Some((attest_slot, attest_shard))) if attest_slot == slot => {
-                Some(_) => Ok(None),
+                    Ok(Some(attest_shard))
-                None => Err(AttesterDutiesReaderError::UnknownEpoch),
+                }
+                Ok(Some(_)) => Ok(None),
+                Ok(None) => Err(AttesterDutiesReaderError::UnknownEpoch),
+                Err(_) => panic!("Error when getting validator attestation shard."),
             }
         } else {
             Err(AttesterDutiesReaderError::UnknownValidator)

eth2/types/src/beacon_state/slot_processing.rs

@@ -2,7 +2,6 @@ use crate::{
     beacon_state::{CommitteesError, EpochProcessingError},
     BeaconState, ChainSpec, Hash256,
 };
-use log::debug;
 
 #[derive(Debug, PartialEq)]
 pub enum Error {
@@ -43,17 +42,17 @@ impl BeaconState {
         &self,
         validator_index: usize,
         spec: &ChainSpec,
-    ) -> Result<(u64, u64, u64), CommitteesError> {
+    ) -> Result<Option<(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 let Some(committee_index) = committee.iter().position(|&i| i == validator_index)
                 {
-                    result = Some(Ok((slot, shard, committee_index as u64)));
+                    result = Some((slot, shard, committee_index as u64));
                 }
             }
         }
-        result.unwrap()
+        Ok(result)
     }
 }