diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index 834f83cde..c7c530f46 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -498,17 +498,14 @@ impl HotColdDB { /// Replay `blocks` on top of `state` until `target_slot` is reached. /// - /// Will skip slots as necessary. + /// Will skip slots as necessary. The returned state is not guaranteed + /// to have any caches built, beyond those immediately required by block processing. fn replay_blocks( &self, mut state: BeaconState, blocks: Vec>, target_slot: Slot, ) -> Result, Error> { - state - .build_all_caches(&self.spec) - .map_err(HotColdDBError::BlockReplayBeaconError)?; - let state_root_from_prev_block = |i: usize, state: &BeaconState| { if i > 0 { let prev_block = &blocks[i - 1]; diff --git a/eth2/state_processing/src/common/initiate_validator_exit.rs b/eth2/state_processing/src/common/initiate_validator_exit.rs index f8d342863..479424fbd 100644 --- a/eth2/state_processing/src/common/initiate_validator_exit.rs +++ b/eth2/state_processing/src/common/initiate_validator_exit.rs @@ -18,19 +18,22 @@ pub fn initiate_validator_exit( return Ok(()); } + // Ensure the exit cache is built. + state.exit_cache.build(&state.validators, spec)?; + // Compute exit queue epoch let delayed_epoch = state.compute_activation_exit_epoch(state.current_epoch(), spec); let mut exit_queue_epoch = state .exit_cache - .max_epoch() + .max_epoch()? .map_or(delayed_epoch, |epoch| max(epoch, delayed_epoch)); - let exit_queue_churn = state.exit_cache.get_churn_at(exit_queue_epoch); + let exit_queue_churn = state.exit_cache.get_churn_at(exit_queue_epoch)?; if exit_queue_churn >= state.get_churn_limit(spec)? { exit_queue_epoch += 1; } - state.exit_cache.record_validator_exit(exit_queue_epoch); + state.exit_cache.record_validator_exit(exit_queue_epoch)?; state.validators[index].exit_epoch = exit_queue_epoch; state.validators[index].withdrawable_epoch = exit_queue_epoch + spec.min_validator_withdrawability_delay; diff --git a/eth2/types/src/beacon_state.rs b/eth2/types/src/beacon_state.rs index 43886d4f5..ba29d4565 100644 --- a/eth2/types/src/beacon_state.rs +++ b/eth2/types/src/beacon_state.rs @@ -58,6 +58,7 @@ pub enum Error { PreviousCommitteeCacheUninitialized, CurrentCommitteeCacheUninitialized, RelativeEpochError(RelativeEpochError), + ExitCacheUninitialized, CommitteeCacheUninitialized(Option), SszTypesError(ssz_types::Error), CachedTreeHashError(cached_tree_hash::Error), @@ -779,13 +780,19 @@ impl BeaconState { /// Build all the caches, if they need to be built. pub fn build_all_caches(&mut self, spec: &ChainSpec) -> Result<(), Error> { + self.build_all_committee_caches(spec)?; + self.update_pubkey_cache()?; + self.build_tree_hash_cache()?; + self.exit_cache.build(&self.validators, spec)?; + + Ok(()) + } + + /// Build all committee caches, if they need to be built. + pub fn build_all_committee_caches(&mut self, spec: &ChainSpec) -> Result<(), Error> { self.build_committee_cache(RelativeEpoch::Previous, spec)?; self.build_committee_cache(RelativeEpoch::Current, spec)?; self.build_committee_cache(RelativeEpoch::Next, spec)?; - self.update_pubkey_cache()?; - self.build_tree_hash_cache()?; - self.exit_cache.build_from_registry(&self.validators, spec); - Ok(()) } diff --git a/eth2/types/src/beacon_state/exit_cache.rs b/eth2/types/src/beacon_state/exit_cache.rs index 475dab3d6..4908194fa 100644 --- a/eth2/types/src/beacon_state/exit_cache.rs +++ b/eth2/types/src/beacon_state/exit_cache.rs @@ -1,35 +1,68 @@ -use super::{ChainSpec, Epoch, Validator}; +use super::{BeaconStateError, ChainSpec, Epoch, Validator}; use serde_derive::{Deserialize, Serialize}; use std::collections::HashMap; /// Map from exit epoch to the number of validators known to be exiting/exited at that epoch. #[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)] -pub struct ExitCache(HashMap); +pub struct ExitCache { + initialized: bool, + exits_per_epoch: HashMap, +} impl ExitCache { + /// Ensure the cache is built, and do nothing if it's already initialized. + pub fn build( + &mut self, + validators: &[Validator], + spec: &ChainSpec, + ) -> Result<(), BeaconStateError> { + if self.initialized { + Ok(()) + } else { + self.force_build(validators, spec) + } + } + /// Add all validators with a non-trivial exit epoch to the cache. - pub fn build_from_registry(&mut self, validators: &[Validator], spec: &ChainSpec) { + pub fn force_build( + &mut self, + validators: &[Validator], + spec: &ChainSpec, + ) -> Result<(), BeaconStateError> { + self.initialized = true; validators .iter() .filter(|validator| validator.exit_epoch != spec.far_future_epoch) - .for_each(|validator| self.record_validator_exit(validator.exit_epoch)); + .try_for_each(|validator| self.record_validator_exit(validator.exit_epoch)) + } + + /// Check that the cache is initialized and return an error if it isn't. + pub fn check_initialized(&self) -> Result<(), BeaconStateError> { + if self.initialized { + Ok(()) + } else { + Err(BeaconStateError::ExitCacheUninitialized) + } } /// Record the exit of a single validator in the cache. /// /// Must only be called once per exiting validator. - pub fn record_validator_exit(&mut self, exit_epoch: Epoch) { - *self.0.entry(exit_epoch).or_insert(0) += 1; + pub fn record_validator_exit(&mut self, exit_epoch: Epoch) -> Result<(), BeaconStateError> { + self.check_initialized()?; + *self.exits_per_epoch.entry(exit_epoch).or_insert(0) += 1; + Ok(()) } /// Get the greatest epoch for which validator exits are known. - pub fn max_epoch(&self) -> Option { - // This could probably be made even faster by caching the maximum. - self.0.keys().max().cloned() + pub fn max_epoch(&self) -> Result, BeaconStateError> { + self.check_initialized()?; + Ok(self.exits_per_epoch.keys().max().cloned()) } /// Get the number of validators exiting/exited at a given epoch, or zero if not known. - pub fn get_churn_at(&self, epoch: Epoch) -> u64 { - self.0.get(&epoch).cloned().unwrap_or(0) + pub fn get_churn_at(&self, epoch: Epoch) -> Result { + self.check_initialized()?; + Ok(self.exits_per_epoch.get(&epoch).cloned().unwrap_or(0)) } } diff --git a/tests/ef_tests/src/cases/epoch_processing.rs b/tests/ef_tests/src/cases/epoch_processing.rs index 975130b23..07d679f13 100644 --- a/tests/ef_tests/src/cases/epoch_processing.rs +++ b/tests/ef_tests/src/cases/epoch_processing.rs @@ -133,8 +133,8 @@ impl> Case for EpochProcessing { let spec = &E::default_spec(); let mut result = (|| { - // Processing requires the epoch cache. - state.build_all_caches(spec)?; + // Processing requires the committee caches. + state.build_all_committee_caches(spec)?; T::run(&mut state, spec).map(|_| state) })(); diff --git a/tests/ef_tests/src/cases/operations.rs b/tests/ef_tests/src/cases/operations.rs index 692108681..1d5a85c6f 100644 --- a/tests/ef_tests/src/cases/operations.rs +++ b/tests/ef_tests/src/cases/operations.rs @@ -174,8 +174,10 @@ impl> Case for Operations { let mut state = self.pre.clone(); let mut expected = self.post.clone(); - // Processing requires the epoch cache. - state.build_all_caches(spec).unwrap(); + // Processing requires the committee caches. + state + .build_all_committee_caches(spec) + .expect("committee caches OK"); let mut result = self.operation.apply_to(&mut state, spec).map(|()| state);