Move get_active_validator_indices to state

eth2/fork_choice/src/bitwise_lmd_ghost.rs

@@ -10,10 +10,7 @@ use db::{
 use log::{debug, trace};
 use std::collections::HashMap;
 use std::sync::Arc;
-use types::{
+use types::{BeaconBlock, ChainSpec, Hash256, Slot, SlotHeight};
-    validator_registry::get_active_validator_indices, BeaconBlock, ChainSpec, Hash256, Slot,
-    SlotHeight,
-};
 
 //TODO: Pruning - Children
 //TODO: Handle Syncing
@@ -93,10 +90,8 @@ where
             .get_deserialized(&state_root)?
             .ok_or_else(|| ForkChoiceError::MissingBeaconState(*state_root))?;
 
-        let active_validator_indices = get_active_validator_indices(
+        let active_validator_indices =
-            &current_state.validator_registry[..],
+            current_state.get_active_validator_indices(block_slot.epoch(spec.slots_per_epoch));
-            block_slot.epoch(spec.slots_per_epoch),
-        );
 
         for index in active_validator_indices {
             let balance = std::cmp::min(

eth2/fork_choice/src/optimized_lmd_ghost.rs

@@ -10,10 +10,7 @@ use log::{debug, trace};
 use std::cmp::Ordering;
 use std::collections::HashMap;
 use std::sync::Arc;
-use types::{
+use types::{BeaconBlock, ChainSpec, Hash256, Slot, SlotHeight};
-    validator_registry::get_active_validator_indices, BeaconBlock, ChainSpec, Hash256, Slot,
-    SlotHeight,
-};
 
 //TODO: Pruning - Children
 //TODO: Handle Syncing
@@ -93,10 +90,8 @@ where
             .get_deserialized(&state_root)?
             .ok_or_else(|| ForkChoiceError::MissingBeaconState(*state_root))?;
 
-        let active_validator_indices = get_active_validator_indices(
+        let active_validator_indices =
-            &current_state.validator_registry[..],
+            current_state.get_active_validator_indices(block_slot.epoch(spec.slots_per_epoch));
-            block_slot.epoch(spec.slots_per_epoch),
-        );
 
         for index in active_validator_indices {
             let balance = std::cmp::min(

eth2/fork_choice/src/slow_lmd_ghost.rs

@@ -8,9 +8,7 @@ use db::{
 use log::{debug, trace};
 use std::collections::HashMap;
 use std::sync::Arc;
-use types::{
+use types::{BeaconBlock, ChainSpec, Hash256, Slot};
-    validator_registry::get_active_validator_indices, BeaconBlock, ChainSpec, Hash256, Slot,
-};
 
 //TODO: Pruning and syncing
 
@@ -61,10 +59,8 @@ where
             .get_deserialized(&state_root)?
             .ok_or_else(|| ForkChoiceError::MissingBeaconState(*state_root))?;
 
-        let active_validator_indices = get_active_validator_indices(
+        let active_validator_indices =
-            &current_state.validator_registry[..],
+            current_state.get_active_validator_indices(block_slot.epoch(spec.slots_per_epoch));
-            block_slot.epoch(spec.slots_per_epoch),
-        );
 
         for index in active_validator_indices {
             let balance = std::cmp::min(

eth2/fork_choice/tests/tests.rs

@@ -242,8 +242,9 @@ fn setup_inital_state(
 
     let spec = ChainSpec::foundation();
 
-    let state_builder =
+    let mut state_builder =
         TestingBeaconStateBuilder::from_single_keypair(num_validators, &Keypair::random(), &spec);
+    state_builder.build_caches(&spec).unwrap();
     let (state, _keypairs) = state_builder.build();
 
     let state_root = state.canonical_root();

eth2/state_processing/src/get_genesis_state.rs

@@ -34,7 +34,7 @@ pub fn get_genesis_state(
 
     // Set all the active index roots to be the genesis active index root.
     let active_validator_indices = state
-        .get_active_validator_indices(spec.genesis_epoch, spec)?
+        .get_cached_active_validator_indices(RelativeEpoch::Current, spec)?
         .to_vec();
     let genesis_active_index_root = Hash256::from_slice(&active_validator_indices.hash_tree_root());
     state.fill_active_index_roots_with(genesis_active_index_root, spec);

eth2/state_processing/src/per_epoch_processing.rs

@@ -7,7 +7,7 @@ use process_validator_registry::process_validator_registry;
 use rayon::prelude::*;
 use ssz::TreeHash;
 use std::collections::HashMap;
-use types::{validator_registry::get_active_validator_indices, *};
+use types::*;
 use validator_statuses::{TotalBalances, ValidatorStatuses};
 use winning_root::{winning_root, WinningRoot};
 
@@ -70,16 +70,6 @@ pub fn per_epoch_processing(state: &mut BeaconState, spec: &ChainSpec) -> Result
     Ok(())
 }
 
-/// Returns a list of active validator indices for the state's current epoch.
-///
-/// Spec v0.5.0
-pub fn calculate_active_validator_indices(state: &BeaconState, spec: &ChainSpec) -> Vec<usize> {
-    get_active_validator_indices(
-        &state.validator_registry,
-        state.slot.epoch(spec.slots_per_epoch),
-    )
-}
-
 /// Calculates various sets of attesters, including:
 ///
 /// - current epoch attesters
@@ -454,10 +444,9 @@ pub fn update_active_tree_index_roots(
 ) -> Result<(), Error> {
     let next_epoch = state.next_epoch(spec);
 
-    let active_tree_root = get_active_validator_indices(
+    let active_tree_root = state
-        &state.validator_registry,
+        .get_active_validator_indices(next_epoch + Epoch::from(spec.activation_exit_delay))
-        next_epoch + Epoch::from(spec.activation_exit_delay),
+        .to_vec()
-    )
         .hash_tree_root();
 
     state.set_active_index_root(next_epoch, Hash256::from_slice(&active_tree_root[..]), spec)?;

eth2/state_processing/src/per_epoch_processing/process_ejections.rs

@@ -9,7 +9,7 @@ pub fn process_ejections(state: &mut BeaconState, spec: &ChainSpec) -> Result<()
     // There is an awkward double (triple?) loop here because we can't loop across the borrowed
     // active validator indices and mutate state in the one loop.
     let exitable: Vec<usize> = state
-        .get_active_validator_indices(state.current_epoch(spec), spec)?
+        .get_cached_active_validator_indices(RelativeEpoch::Current, spec)?
         .iter()
         .filter_map(|&i| {
             if state.validator_balances[i as usize] < spec.ejection_balance {

eth2/state_processing/src/per_epoch_processing/process_slashings.rs

@@ -7,7 +7,8 @@ use types::{BeaconStateError as Error, *};
 /// Spec v0.4.0
 pub fn process_slashings(state: &mut BeaconState, spec: &ChainSpec) -> Result<(), Error> {
     let current_epoch = state.current_epoch(spec);
-    let active_validator_indices = state.get_active_validator_indices(current_epoch, spec)?;
+    let active_validator_indices =
+        state.get_cached_active_validator_indices(RelativeEpoch::Current, spec)?;
     let total_balance = state.get_total_balance(&active_validator_indices[..], spec)?;
 
     for (index, validator) in state.validator_registry.iter().enumerate() {

eth2/state_processing/src/per_epoch_processing/process_validator_registry.rs

@@ -21,7 +21,7 @@ pub fn process_validator_registry(state: &mut BeaconState, spec: &ChainSpec) ->
         state.current_shuffling_start_shard = (state.current_shuffling_start_shard
             + spec.get_epoch_committee_count(
                 state
-                    .get_active_validator_indices(current_epoch, spec)?
+                    .get_cached_active_validator_indices(RelativeEpoch::Current, spec)?
                     .len(),
             ) as u64)
             % spec.shard_count;
@@ -53,7 +53,7 @@ pub fn should_update_validator_registry(
     }
 
     let num_active_validators = state
-        .get_active_validator_indices(state.current_epoch(spec), spec)?
+        .get_cached_active_validator_indices(RelativeEpoch::Current, spec)?
         .len();
     let current_epoch_committee_count = spec.get_epoch_committee_count(num_active_validators);
 

eth2/state_processing/src/per_epoch_processing/update_validator_registry.rs

@@ -8,7 +8,8 @@ use types::{BeaconStateError as Error, *};
 /// Spec v0.4.0
 pub fn update_validator_registry(state: &mut BeaconState, spec: &ChainSpec) -> Result<(), Error> {
     let current_epoch = state.current_epoch(spec);
-    let active_validator_indices = state.get_active_validator_indices(current_epoch, spec)?;
+    let active_validator_indices =
+        state.get_cached_active_validator_indices(RelativeEpoch::Current, spec)?;
     let total_balance = state.get_total_balance(&active_validator_indices[..], spec)?;
 
     let max_balance_churn = std::cmp::max(

eth2/types/src/beacon_state.rs

@@ -1,4 +1,4 @@
-use self::epoch_cache::{EpochCache, Error as EpochCacheError};
+use self::epoch_cache::{get_active_validator_indices, EpochCache, Error as EpochCacheError};
 use crate::test_utils::TestRandom;
 use crate::*;
 use int_to_bytes::int_to_bytes32;
@@ -234,28 +234,31 @@ impl BeaconState {
     /// Returns the active validator indices for the given epoch, assuming there is no validator
     /// registry update in the next epoch.
     ///
+    /// This uses the cache, so it saves an iteration over the validator registry, however it can
+    /// not return a result for any epoch before the previous epoch.
+    ///
     /// Note: Utilizes the cache and will fail if the appropriate cache is not initialized.
     ///
     /// Spec v0.5.0
-    pub fn get_active_validator_indices(
+    pub fn get_cached_active_validator_indices(
         &self,
-        epoch: Epoch,
+        relative_epoch: RelativeEpoch,
         spec: &ChainSpec,
     ) -> Result<&[usize], Error> {
-        // If the slot is in the next epoch, assume there was no validator registry update.
-        let relative_epoch =
-            match RelativeEpoch::from_epoch(self.slot.epoch(spec.slots_per_epoch), epoch) {
-                Err(RelativeEpochError::AmbiguiousNextEpoch) => {
-                    Ok(RelativeEpoch::NextWithoutRegistryChange)
-                }
-                e => e,
-            }?;
-
         let cache = self.cache(relative_epoch, spec)?;
 
         Ok(&cache.active_validator_indices)
     }
 
+    /// Returns the active validator indices for the given epoch.
+    ///
+    /// Does not utilize the cache, performs a full iteration over the validator registry.
+    ///
+    /// Spec v0.5.0
+    pub fn get_active_validator_indices(&self, epoch: Epoch) -> Vec<usize> {
+        get_active_validator_indices(&self.validator_registry, epoch)
+    }
+
     /// Returns the crosslink committees for some slot.
     ///
     /// Note: Utilizes the cache and will fail if the appropriate cache is not initialized.

eth2/types/src/beacon_state/epoch_cache/tests.rs

@@ -7,15 +7,19 @@ use swap_or_not_shuffle::shuffle_list;
 fn do_sane_cache_test(
     state: BeaconState,
     epoch: Epoch,
+    relative_epoch: RelativeEpoch,
     validator_count: usize,
     expected_seed: Hash256,
     expected_shuffling_start: u64,
     spec: &ChainSpec,
 ) {
     let active_indices: Vec<usize> = (0..validator_count).collect();
+
     assert_eq!(
         &active_indices[..],
-        state.get_active_validator_indices(epoch, &spec).unwrap(),
+        state
+            .get_cached_active_validator_indices(relative_epoch, &spec)
+            .unwrap(),
         "Validator indices mismatch"
     );
 
@@ -101,6 +105,7 @@ fn builds_sane_current_epoch_cache() {
     do_sane_cache_test(
         state.clone(),
         state.current_epoch(&spec),
+        RelativeEpoch::Current,
         validator_count as usize,
         state.current_shuffling_seed,
         state.current_shuffling_start_shard,
@@ -117,6 +122,7 @@ fn builds_sane_previous_epoch_cache() {
     do_sane_cache_test(
         state.clone(),
         state.previous_epoch(&spec),
+        RelativeEpoch::Previous,
         validator_count as usize,
         state.previous_shuffling_seed,
         state.previous_shuffling_start_shard,
@@ -134,6 +140,7 @@ fn builds_sane_next_without_update_epoch_cache() {
     do_sane_cache_test(
         state.clone(),
         state.next_epoch(&spec),
+        RelativeEpoch::NextWithoutRegistryChange,
         validator_count as usize,
         state.current_shuffling_seed,
         state.current_shuffling_start_shard,

eth2/types/src/beacon_state/pubkey_cache.rs

@@ -6,13 +6,17 @@ type ValidatorIndex = usize;
 
 #[derive(Debug, PartialEq, Clone, Default, Serialize, Deserialize)]
 pub struct PubkeyCache {
+    /// Maintain the number of keys added to the map. It is not sufficient to just use the HashMap
+    /// len, as it does not increase when duplicate keys are added. Duplicate keys are used during
+    /// testing.
+    len: usize,
     map: HashMap<PublicKey, ValidatorIndex>,
 }
 
 impl PubkeyCache {
-    /// Returns the number of validator indices already in the map.
+    /// Returns the number of validator indices added to the map so far.
     pub fn len(&self) -> ValidatorIndex {
-        self.map.len()
+        self.len
     }
 
     /// Inserts a validator index into the map.
@@ -20,8 +24,9 @@ impl PubkeyCache {
     /// The added index must equal the number of validators already added to the map. This ensures
     /// that an index is never skipped.
     pub fn insert(&mut self, pubkey: PublicKey, index: ValidatorIndex) -> bool {
-        if index == self.map.len() {
+        if index == self.len {
             self.map.insert(pubkey, index);
+            self.len += 1;
             true
         } else {
             false

eth2/types/src/chain_spec.rs

@@ -117,7 +117,7 @@ pub struct ChainSpec {
 impl ChainSpec {
     /// Return the number of committees in one epoch.
     ///
-    /// Spec v0.4.0
+    /// Spec v0.5.0
     pub fn get_epoch_committee_count(&self, active_validator_count: usize) -> u64 {
         std::cmp::max(
             1,

eth2/types/src/lib.rs

@@ -34,7 +34,6 @@ pub mod relative_epoch;
 pub mod slot_epoch;
 pub mod slot_height;
 pub mod validator;
-pub mod validator_registry;
 
 use ethereum_types::{H160, H256, U256};
 use std::collections::HashMap;

eth2/types/src/transfer.rs

@@ -9,7 +9,7 @@ use test_random_derive::TestRandom;
 
 /// The data submitted to the deposit contract.
 ///
-/// Spec v0.4.0
+/// Spec v0.5.0
 #[derive(
     Debug,
     PartialEq,

eth2/types/src/validator_registry.rs

@@ -1,174 +0,0 @@
-/// Contains logic to manipulate a `&[Validator]`.
-/// For now, we avoid defining a newtype and just have flat functions here.
-use super::validator::*;
-use crate::Epoch;
-
-/// Given an indexed sequence of `validators`, return the indices corresponding to validators that are active at `epoch`.
-///
-/// Spec v0.4.0
-pub fn get_active_validator_indices(validators: &[Validator], epoch: Epoch) -> Vec<usize> {
-    let mut active = Vec::with_capacity(validators.len());
-
-    for (index, validator) in validators.iter().enumerate() {
-        if validator.is_active_at(epoch) {
-            active.push(index)
-        }
-    }
-
-    active.shrink_to_fit();
-
-    active
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use crate::test_utils::{SeedableRng, TestRandom, XorShiftRng};
-
-    #[test]
-    fn can_get_empty_active_validator_indices() {
-        let mut rng = XorShiftRng::from_seed([42; 16]);
-
-        let validators = vec![];
-        let some_epoch = Epoch::random_for_test(&mut rng);
-        let indices = get_active_validator_indices(&validators, some_epoch);
-        assert_eq!(indices, vec![]);
-    }
-
-    #[test]
-    fn can_get_no_active_validator_indices() {
-        let mut rng = XorShiftRng::from_seed([42; 16]);
-        let mut validators = vec![];
-        let count_validators = 10;
-        for _ in 0..count_validators {
-            validators.push(Validator::default())
-        }
-
-        let some_epoch = Epoch::random_for_test(&mut rng);
-        let indices = get_active_validator_indices(&validators, some_epoch);
-        assert_eq!(indices, vec![]);
-    }
-
-    #[test]
-    fn can_get_all_active_validator_indices() {
-        let mut rng = XorShiftRng::from_seed([42; 16]);
-        let count_validators = 10;
-        let some_epoch = Epoch::random_for_test(&mut rng);
-
-        let mut validators = (0..count_validators)
-            .into_iter()
-            .map(|_| {
-                let mut validator = Validator::default();
-
-                let activation_offset = u64::random_for_test(&mut rng);
-                let exit_offset = u64::random_for_test(&mut rng);
-
-                validator.activation_epoch = some_epoch - activation_offset;
-                validator.exit_epoch = some_epoch + exit_offset;
-
-                validator
-            })
-            .collect::<Vec<_>>();
-
-        // test boundary condition by ensuring that at least one validator in the list just activated
-        if let Some(validator) = validators.get_mut(0) {
-            validator.activation_epoch = some_epoch;
-        }
-
-        let indices = get_active_validator_indices(&validators, some_epoch);
-        assert_eq!(
-            indices,
-            (0..count_validators).into_iter().collect::<Vec<_>>()
-        );
-    }
-
-    fn set_validators_to_default_entry_exit(validators: &mut [Validator]) {
-        for validator in validators.iter_mut() {
-            validator.activation_epoch = Epoch::max_value();
-            validator.exit_epoch = Epoch::max_value();
-        }
-    }
-
-    // sets all `validators` to be active as of some epoch prior to `epoch`. returns the activation epoch.
-    fn set_validators_to_activated(validators: &mut [Validator], epoch: Epoch) -> Epoch {
-        let activation_epoch = epoch - 10;
-        for validator in validators.iter_mut() {
-            validator.activation_epoch = activation_epoch;
-        }
-        activation_epoch
-    }
-
-    // sets all `validators` to be exited as of some epoch before `epoch`.
-    fn set_validators_to_exited(
-        validators: &mut [Validator],
-        epoch: Epoch,
-        activation_epoch: Epoch,
-    ) {
-        assert!(activation_epoch < epoch);
-        let mut exit_epoch = activation_epoch + 10;
-        while exit_epoch >= epoch {
-            exit_epoch -= 1;
-        }
-        assert!(activation_epoch < exit_epoch && exit_epoch < epoch);
-
-        for validator in validators.iter_mut() {
-            validator.exit_epoch = exit_epoch;
-        }
-    }
-
-    #[test]
-    fn can_get_some_active_validator_indices() {
-        let mut rng = XorShiftRng::from_seed([42; 16]);
-        const COUNT_PARTITIONS: usize = 3;
-        const COUNT_VALIDATORS: usize = 3 * COUNT_PARTITIONS;
-        let some_epoch: Epoch = Epoch::random_for_test(&mut rng);
-
-        let mut validators = (0..COUNT_VALIDATORS)
-            .into_iter()
-            .map(|_| {
-                let mut validator = Validator::default();
-
-                let activation_offset = Epoch::random_for_test(&mut rng);
-                let exit_offset = Epoch::random_for_test(&mut rng);
-
-                validator.activation_epoch = some_epoch - activation_offset;
-                validator.exit_epoch = some_epoch + exit_offset;
-
-                validator
-            })
-            .collect::<Vec<_>>();
-
-        // we partition the set into partitions based on lifecycle:
-        for (i, chunk) in validators.chunks_exact_mut(COUNT_PARTITIONS).enumerate() {
-            match i {
-                0 => {
-                    // 1. not activated (Default::default())
-                    set_validators_to_default_entry_exit(chunk);
-                }
-                1 => {
-                    // 2. activated, but not exited
-                    set_validators_to_activated(chunk, some_epoch);
-                    // test boundary condition by ensuring that at least one validator in the list just activated
-                    if let Some(validator) = chunk.get_mut(0) {
-                        validator.activation_epoch = some_epoch;
-                    }
-                }
-                2 => {
-                    // 3. exited
-                    let activation_epoch = set_validators_to_activated(chunk, some_epoch);
-                    set_validators_to_exited(chunk, some_epoch, activation_epoch);
-                    // test boundary condition by ensuring that at least one validator in the list just exited
-                    if let Some(validator) = chunk.get_mut(0) {
-                        validator.exit_epoch = some_epoch;
-                    }
-                }
-                _ => unreachable!(
-                    "constants local to this test not in sync with generation of test case"
-                ),
-            }
-        }
-
-        let indices = get_active_validator_indices(&validators, some_epoch);
-        assert_eq!(indices, vec![3, 4, 5]);
-    }
-}