diff --git a/eth2/fork_choice/src/bitwise_lmd_ghost.rs b/eth2/fork_choice/src/bitwise_lmd_ghost.rs index 9410fd203..8ae0251d2 100644 --- a/eth2/fork_choice/src/bitwise_lmd_ghost.rs +++ b/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::{ - validator_registry::get_active_validator_indices, BeaconBlock, ChainSpec, Hash256, Slot, - SlotHeight, -}; +use types::{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( - ¤t_state.validator_registry[..], - block_slot.epoch(spec.slots_per_epoch), - ); + let active_validator_indices = + current_state.get_active_validator_indices(block_slot.epoch(spec.slots_per_epoch)); for index in active_validator_indices { let balance = std::cmp::min( diff --git a/eth2/fork_choice/src/optimized_lmd_ghost.rs b/eth2/fork_choice/src/optimized_lmd_ghost.rs index e1b8914a6..ee2919e85 100644 --- a/eth2/fork_choice/src/optimized_lmd_ghost.rs +++ b/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::{ - validator_registry::get_active_validator_indices, BeaconBlock, ChainSpec, Hash256, Slot, - SlotHeight, -}; +use types::{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( - ¤t_state.validator_registry[..], - block_slot.epoch(spec.slots_per_epoch), - ); + let active_validator_indices = + current_state.get_active_validator_indices(block_slot.epoch(spec.slots_per_epoch)); for index in active_validator_indices { let balance = std::cmp::min( diff --git a/eth2/fork_choice/src/slow_lmd_ghost.rs b/eth2/fork_choice/src/slow_lmd_ghost.rs index af58aa7b8..25d137089 100644 --- a/eth2/fork_choice/src/slow_lmd_ghost.rs +++ b/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::{ - validator_registry::get_active_validator_indices, BeaconBlock, ChainSpec, Hash256, Slot, -}; +use types::{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( - ¤t_state.validator_registry[..], - block_slot.epoch(spec.slots_per_epoch), - ); + let active_validator_indices = + current_state.get_active_validator_indices(block_slot.epoch(spec.slots_per_epoch)); for index in active_validator_indices { let balance = std::cmp::min( diff --git a/eth2/fork_choice/tests/tests.rs b/eth2/fork_choice/tests/tests.rs index 80fbbbe20..3ce63eeb7 100644 --- a/eth2/fork_choice/tests/tests.rs +++ b/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(); diff --git a/eth2/state_processing/src/get_genesis_state.rs b/eth2/state_processing/src/get_genesis_state.rs index bfcf82216..7c4d4cafd 100644 --- a/eth2/state_processing/src/get_genesis_state.rs +++ b/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); diff --git a/eth2/state_processing/src/per_epoch_processing.rs b/eth2/state_processing/src/per_epoch_processing.rs index 97a0e9987..8e03457d3 100644 --- a/eth2/state_processing/src/per_epoch_processing.rs +++ b/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 { - get_active_validator_indices( - &state.validator_registry, - state.slot.epoch(spec.slots_per_epoch), - ) -} - /// Calculates various sets of attesters, including: /// /// - current epoch attesters @@ -454,11 +444,10 @@ pub fn update_active_tree_index_roots( ) -> Result<(), Error> { let next_epoch = state.next_epoch(spec); - let active_tree_root = get_active_validator_indices( - &state.validator_registry, - next_epoch + Epoch::from(spec.activation_exit_delay), - ) - .hash_tree_root(); + let active_tree_root = state + .get_active_validator_indices(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)?; diff --git a/eth2/state_processing/src/per_epoch_processing/process_ejections.rs b/eth2/state_processing/src/per_epoch_processing/process_ejections.rs index 27dd37479..a60d92187 100644 --- a/eth2/state_processing/src/per_epoch_processing/process_ejections.rs +++ b/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 = 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 { diff --git a/eth2/state_processing/src/per_epoch_processing/process_slashings.rs b/eth2/state_processing/src/per_epoch_processing/process_slashings.rs index b14a9ee37..19c1e519b 100644 --- a/eth2/state_processing/src/per_epoch_processing/process_slashings.rs +++ b/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() { diff --git a/eth2/state_processing/src/per_epoch_processing/process_validator_registry.rs b/eth2/state_processing/src/per_epoch_processing/process_validator_registry.rs index 2eb39711d..85d6c37f6 100644 --- a/eth2/state_processing/src/per_epoch_processing/process_validator_registry.rs +++ b/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); diff --git a/eth2/state_processing/src/per_epoch_processing/update_validator_registry.rs b/eth2/state_processing/src/per_epoch_processing/update_validator_registry.rs index 8b612c346..ecf05ce6f 100644 --- a/eth2/state_processing/src/per_epoch_processing/update_validator_registry.rs +++ b/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( diff --git a/eth2/types/src/beacon_state.rs b/eth2/types/src/beacon_state.rs index 30f95c02c..22e7c6ecf 100644 --- a/eth2/types/src/beacon_state.rs +++ b/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 { + 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. diff --git a/eth2/types/src/beacon_state/epoch_cache/tests.rs b/eth2/types/src/beacon_state/epoch_cache/tests.rs index 10df635f2..5643776e2 100644 --- a/eth2/types/src/beacon_state/epoch_cache/tests.rs +++ b/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 = (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, diff --git a/eth2/types/src/beacon_state/pubkey_cache.rs b/eth2/types/src/beacon_state/pubkey_cache.rs index 340bdb311..4632a2d9c 100644 --- a/eth2/types/src/beacon_state/pubkey_cache.rs +++ b/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, } 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 diff --git a/eth2/types/src/chain_spec.rs b/eth2/types/src/chain_spec.rs index e9ade2c91..cfb88bcb8 100644 --- a/eth2/types/src/chain_spec.rs +++ b/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, diff --git a/eth2/types/src/lib.rs b/eth2/types/src/lib.rs index 05f8254d5..30b0e4a13 100644 --- a/eth2/types/src/lib.rs +++ b/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; diff --git a/eth2/types/src/transfer.rs b/eth2/types/src/transfer.rs index 1c9968702..2570d7b3f 100644 --- a/eth2/types/src/transfer.rs +++ b/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, diff --git a/eth2/types/src/validator_registry.rs b/eth2/types/src/validator_registry.rs deleted file mode 100644 index db35ae993..000000000 --- a/eth2/types/src/validator_registry.rs +++ /dev/null @@ -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 { - 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::>(); - - // 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::>() - ); - } - - 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::>(); - - // 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]); - } -}