From 721323f0451e4b0c8c94f2bc57de9b4c69502b61 Mon Sep 17 00:00:00 2001 From: ethDreamer <37123614+ethDreamer@users.noreply.github.com> Date: Mon, 29 Jun 2020 04:21:51 -0400 Subject: [PATCH] get_active_validator_indices() now has bound check (#1300) --- beacon_node/genesis/src/eth1_genesis_service.rs | 11 ++++++++--- consensus/state_processing/src/genesis.rs | 9 ++++++--- consensus/types/src/beacon_state.rs | 15 +++++++++++---- consensus/types/src/beacon_state/tests.rs | 8 ++++---- 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/beacon_node/genesis/src/eth1_genesis_service.rs b/beacon_node/genesis/src/eth1_genesis_service.rs index 540c96be7..a1f665850 100644 --- a/beacon_node/genesis/src/eth1_genesis_service.rs +++ b/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::(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 diff --git a/consensus/state_processing/src/genesis.rs b/consensus/state_processing/src/genesis.rs index e38b1eef4..0f4369a86 100644 --- a/consensus/state_processing/src/genesis.rs +++ b/consensus/state_processing/src/genesis.rs @@ -52,9 +52,12 @@ pub fn initialize_beacon_state_from_eth1( /// /// Spec v0.12.1 pub fn is_valid_genesis_state(state: &BeaconState, spec: &ChainSpec) -> bool { - state.genesis_time >= spec.min_genesis_time - && state.get_active_validator_indices(T::genesis_epoch()).len() as u64 - >= spec.min_genesis_active_validator_count + state + .get_active_validator_indices(T::genesis_epoch(), spec) + .map_or(false, |active_validators| { + state.genesis_time >= spec.min_genesis_time + && active_validators.len() as u64 >= spec.min_genesis_active_validator_count + }) } /// Activate genesis validators, if their balance is acceptable. diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index e3d628b57..bf25fb45f 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -375,9 +375,16 @@ impl BeaconState { /// 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 { - // 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, 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 BeaconState { pub fn get_beacon_proposer_index(&self, slot: Slot, spec: &ChainSpec) -> Result { 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) } diff --git a/consensus/types/src/beacon_state/tests.rs b/consensus/types/src/beacon_state/tests.rs index 2fd8f27ca..918feda74 100644 --- a/consensus/types/src/beacon_state/tests.rs +++ b/consensus/types/src/beacon_state/tests.rs @@ -19,10 +19,10 @@ fn test_beacon_proposer_index() { }; // Get the i'th candidate proposer for the given state and slot - let ith_candidate = |state: &BeaconState, slot: Slot, i: usize| { + let ith_candidate = |state: &BeaconState, 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() { let test = |state: &BeaconState, 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() { // 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() {