get_active_validator_indices() now has bound check (#1300)

beacon_node/genesis/src/eth1_genesis_service.rs

@@ -183,7 +183,10 @@ impl Eth1GenesisService {
                 info!(
                     log,
                     "Genesis ceremony complete";
-                    "genesis_validators" => genesis_state.get_active_validator_indices(E::genesis_epoch()).len(),
+                    "genesis_validators" => genesis_state
+                        .get_active_validator_indices(E::genesis_epoch(), &spec)
+                        .map_err(|e| format!("Genesis validators error: {:?}", e))?
+                        .len(),
                     "genesis_time" => genesis_state.genesis_time,
                 );
                 break Ok(genesis_state);
@@ -311,8 +314,10 @@ impl Eth1GenesisService {
             // Note: this state is fully valid, some fields have been bypassed to make verification
             // faster.
             let state = self.cheap_state_at_eth1_block::<E>(block, &spec)?;
-            let active_validator_count =
-                state.get_active_validator_indices(E::genesis_epoch()).len();
+            let active_validator_count = state
+                .get_active_validator_indices(E::genesis_epoch(), spec)
+                .map_err(|e| format!("Genesis validators error: {:?}", e))?
+                .len();
 
             self.stats
                 .active_validator_count

consensus/state_processing/src/genesis.rs

@@ -52,9 +52,12 @@ pub fn initialize_beacon_state_from_eth1<T: EthSpec>(
 ///
 /// Spec v0.12.1
 pub fn is_valid_genesis_state<T: EthSpec>(state: &BeaconState<T>, spec: &ChainSpec) -> bool {
+    state
+        .get_active_validator_indices(T::genesis_epoch(), spec)
+        .map_or(false, |active_validators| {
             state.genesis_time >= spec.min_genesis_time
-        && state.get_active_validator_indices(T::genesis_epoch()).len() as u64
-            >= spec.min_genesis_active_validator_count
+                && active_validators.len() as u64 >= spec.min_genesis_active_validator_count
+        })
 }
 
 /// Activate genesis validators, if their balance is acceptable.

consensus/types/src/beacon_state.rs

@@ -375,9 +375,16 @@ impl<T: EthSpec> BeaconState<T> {
     /// Does not utilize the cache, performs a full iteration over the validator registry.
     ///
     /// Spec v0.12.1
-    pub fn get_active_validator_indices(&self, epoch: Epoch) -> Vec<usize> {
-        // FIXME(sproul): put a bounds check on here based on the maximum lookahead
-        get_active_validator_indices(&self.validators, epoch)
+    pub fn get_active_validator_indices(
+        &self,
+        epoch: Epoch,
+        spec: &ChainSpec,
+    ) -> Result<Vec<usize>, Error> {
+        if epoch >= self.compute_activation_exit_epoch(self.current_epoch(), spec) {
+            Err(BeaconStateError::EpochOutOfBounds)
+        } else {
+            Ok(get_active_validator_indices(&self.validators, epoch))
+        }
     }
 
     /// Return the cached active validator indices at some epoch.
@@ -505,7 +512,7 @@ impl<T: EthSpec> BeaconState<T> {
     pub fn get_beacon_proposer_index(&self, slot: Slot, spec: &ChainSpec) -> Result<usize, Error> {
         let epoch = slot.epoch(T::slots_per_epoch());
         let seed = self.get_beacon_proposer_seed(slot, spec)?;
-        let indices = self.get_active_validator_indices(epoch);
+        let indices = self.get_active_validator_indices(epoch, spec)?;
 
         self.compute_proposer_index(&indices, &seed, spec)
     }

consensus/types/src/beacon_state/tests.rs

@@ -19,10 +19,10 @@ fn test_beacon_proposer_index<T: EthSpec>() {
     };
 
     // Get the i'th candidate proposer for the given state and slot
-    let ith_candidate = |state: &BeaconState<T>, slot: Slot, i: usize| {
+    let ith_candidate = |state: &BeaconState<T>, slot: Slot, i: usize, spec: &ChainSpec| {
         let epoch = slot.epoch(T::slots_per_epoch());
         let seed = state.get_beacon_proposer_seed(slot, &spec).unwrap();
-        let active_validators = state.get_active_validator_indices(epoch);
+        let active_validators = state.get_active_validator_indices(epoch, spec).unwrap();
         active_validators[compute_shuffled_index(
             i,
             active_validators.len(),
@@ -36,7 +36,7 @@ fn test_beacon_proposer_index<T: EthSpec>() {
     let test = |state: &BeaconState<T>, slot: Slot, candidate_index: usize| {
         assert_eq!(
             state.get_beacon_proposer_index(slot, &spec),
-            Ok(ith_candidate(state, slot, candidate_index))
+            Ok(ith_candidate(state, slot, candidate_index, &spec))
         );
     };
 
@@ -56,7 +56,7 @@ fn test_beacon_proposer_index<T: EthSpec>() {
 
     // Test with two validators per slot, first validator has zero balance.
     let mut state = build_state(T::slots_per_epoch() as usize * 2);
-    let slot0_candidate0 = ith_candidate(&state, Slot::new(0), 0);
+    let slot0_candidate0 = ith_candidate(&state, Slot::new(0), 0, &spec);
     state.validators[slot0_candidate0].effective_balance = 0;
     test(&state, Slot::new(0), 1);
     for i in 1..T::slots_per_epoch() {