diff --git a/consensus/state_processing/src/epoch_cache.rs b/consensus/state_processing/src/epoch_cache.rs index be641d2398f..8f8006e2bfa 100644 --- a/consensus/state_processing/src/epoch_cache.rs +++ b/consensus/state_processing/src/epoch_cache.rs @@ -18,10 +18,10 @@ impl PreEpochCache { // State root should already have been filled in by `process_slot`, except in the case // of a `partial_state_advance`. - let decision_block_root = latest_block_header.canonical_root(); - if decision_block_root.is_zero() { - return Err(EpochCacheError::ZeroDecisionBlock); + if latest_block_header.state_root.is_zero() { + return Err(EpochCacheError::ZeroStateRoot); } + let decision_block_root = latest_block_header.canonical_root(); let epoch_key = EpochCacheKey { epoch: state.next_epoch()?, diff --git a/consensus/state_processing/src/per_epoch_processing/effective_balance_updates.rs b/consensus/state_processing/src/per_epoch_processing/effective_balance_updates.rs index d56f8367209..d894e456385 100644 --- a/consensus/state_processing/src/per_epoch_processing/effective_balance_updates.rs +++ b/consensus/state_processing/src/per_epoch_processing/effective_balance_updates.rs @@ -48,7 +48,7 @@ pub fn process_effective_balance_updates( } } - state.set_total_active_balance(next_epoch, new_total_active_balance); + state.set_total_active_balance(next_epoch, new_total_active_balance, spec); Ok(()) } diff --git a/consensus/state_processing/src/per_epoch_processing/registry_updates.rs b/consensus/state_processing/src/per_epoch_processing/registry_updates.rs index a7eaace29c6..0d2105bec2c 100644 --- a/consensus/state_processing/src/per_epoch_processing/registry_updates.rs +++ b/consensus/state_processing/src/per_epoch_processing/registry_updates.rs @@ -43,7 +43,7 @@ pub fn process_registry_updates( // Dequeue validators for activation up to churn limit let churn_limit = state.get_activation_churn_limit(spec)? as usize; - let epoch_cache = state.epoch_cache().clone(); + let epoch_cache = state.epoch_cache(); let activation_queue = epoch_cache .activation_queue()? .get_validators_eligible_for_activation(state.finalized_checkpoint().epoch, churn_limit); diff --git a/consensus/state_processing/src/per_epoch_processing/single_pass.rs b/consensus/state_processing/src/per_epoch_processing/single_pass.rs index b3ae0e5a9a5..9319d2941b5 100644 --- a/consensus/state_processing/src/per_epoch_processing/single_pass.rs +++ b/consensus/state_processing/src/per_epoch_processing/single_pass.rs @@ -278,7 +278,7 @@ pub fn process_epoch_single_pass( } if conf.effective_balance_updates { - state.set_total_active_balance(next_epoch, next_epoch_total_active_balance); + state.set_total_active_balance(next_epoch, next_epoch_total_active_balance, spec); *state.epoch_cache_mut() = next_epoch_cache.into_epoch_cache( next_epoch_total_active_balance, next_epoch_activation_queue, diff --git a/consensus/types/src/activation_queue.rs b/consensus/types/src/activation_queue.rs index 5ec4ef66895..09ffa5b85e7 100644 --- a/consensus/types/src/activation_queue.rs +++ b/consensus/types/src/activation_queue.rs @@ -5,10 +5,15 @@ use std::collections::BTreeSet; #[derive(Debug, PartialEq, Eq, Default, Clone, arbitrary::Arbitrary)] pub struct ActivationQueue { /// Validators represented by `(activation_eligibility_epoch, index)` in sorted order. + /// + /// These validators are not *necessarily* going to be activated. Their activation depends + /// on how finalization is updated, and the `churn_limit`. queue: BTreeSet<(Epoch, usize)>, } impl ActivationQueue { + /// Check if `validator` could be eligible for activation in the next epoch and add them to + /// the tentative activation queue if this is the case. pub fn add_if_could_be_eligible_for_activation( &mut self, index: usize, @@ -22,6 +27,7 @@ impl ActivationQueue { } } + /// Determine the final activation queue after accounting for finalization & the churn limit. pub fn get_validators_eligible_for_activation( &self, finalized_epoch: Epoch, diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index 601eeb077c2..e520a164181 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -373,7 +373,6 @@ where #[ssz(skip_serializing, skip_deserializing)] #[tree_hash(skip_hashing)] #[test_random(default)] - #[derivative(Clone(clone_with = "clone_default"))] pub epoch_cache: EpochCache, #[serde(skip_serializing, skip_deserializing)] #[ssz(skip_serializing, skip_deserializing)] @@ -1513,8 +1512,11 @@ impl BeaconState { /// /// This should only be called when the total active balance has been computed as part of /// single-pass epoch processing (or `process_rewards_and_penalties` for phase0). - pub fn set_total_active_balance(&mut self, epoch: Epoch, balance: u64) { - *self.total_active_balance_mut() = Some((epoch, balance)); + /// + /// This function will ensure the balance is never set to 0, thus conforming to the spec. + pub fn set_total_active_balance(&mut self, epoch: Epoch, balance: u64, spec: &ChainSpec) { + let safe_balance = std::cmp::max(balance, spec.effective_balance_increment); + *self.total_active_balance_mut() = Some((epoch, safe_balance)); } /// Build the total active balance cache for the current epoch if it is not already built. @@ -1884,9 +1886,6 @@ impl BeaconState { if config.slashings_cache { *res.slashings_cache_mut() = self.slashings_cache().clone(); } - if config.epoch_cache { - *res.epoch_cache_mut() = self.epoch_cache().clone(); - } if config.tree_hash_cache { *res.tree_hash_cache_mut() = self.tree_hash_cache().clone(); } diff --git a/consensus/types/src/beacon_state/clone_config.rs b/consensus/types/src/beacon_state/clone_config.rs index a43070a194b..27e066d5db6 100644 --- a/consensus/types/src/beacon_state/clone_config.rs +++ b/consensus/types/src/beacon_state/clone_config.rs @@ -5,7 +5,6 @@ pub struct CloneConfig { pub pubkey_cache: bool, pub exit_cache: bool, pub slashings_cache: bool, - pub epoch_cache: bool, pub tree_hash_cache: bool, pub progressive_balances_cache: bool, } @@ -17,7 +16,6 @@ impl CloneConfig { pubkey_cache: true, exit_cache: true, slashings_cache: true, - epoch_cache: true, tree_hash_cache: true, progressive_balances_cache: true, } diff --git a/consensus/types/src/beacon_state/committee_cache.rs b/consensus/types/src/beacon_state/committee_cache.rs index d8eb3e753e2..7e73fc46475 100644 --- a/consensus/types/src/beacon_state/committee_cache.rs +++ b/consensus/types/src/beacon_state/committee_cache.rs @@ -35,7 +35,7 @@ pub struct CommitteeCache { /// /// It can happen that states from different epochs computing the same cache have different /// numbers of validators in `state.validators()` due to recent deposits. These new validators -/// cannot be active however and will always be ommitted from the shuffling. This function checks +/// cannot be active however and will always be omitted from the shuffling. This function checks /// that two lists of shuffling positions are equivalent by ensuring that they are identical on all /// common entries, and that new entries at the end are all `None`. /// diff --git a/consensus/types/src/beacon_state/tests.rs b/consensus/types/src/beacon_state/tests.rs index d559b77516b..59295d1b785 100644 --- a/consensus/types/src/beacon_state/tests.rs +++ b/consensus/types/src/beacon_state/tests.rs @@ -223,15 +223,14 @@ async fn clone_config() { .update_tree_hash_cache() .expect("should update tree hash cache"); - let num_caches = 7; + let num_caches = 6; let all_configs = (0..2u8.pow(num_caches)).map(|i| CloneConfig { committee_caches: (i & 1) != 0, pubkey_cache: ((i >> 1) & 1) != 0, exit_cache: ((i >> 2) & 1) != 0, slashings_cache: ((i >> 3) & 1) != 0, - epoch_cache: ((i >> 4) & 1) != 0, - tree_hash_cache: ((i >> 5) & 1) != 0, - progressive_balances_cache: ((i >> 6) & 1) != 0, + tree_hash_cache: ((i >> 4) & 1) != 0, + progressive_balances_cache: ((i >> 5) & 1) != 0, }); for config in all_configs { diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index 3f2efc7d5d5..ff3ebe58a24 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -502,12 +502,6 @@ impl ChainSpec { epoch.safe_add(1)?.safe_add(self.max_seed_lookahead) } - #[allow(clippy::arithmetic_side_effects)] - pub const fn attestation_subnet_prefix_bits(&self) -> u32 { - let attestation_subnet_count_bits = self.attestation_subnet_count.ilog2(); - self.attestation_subnet_extra_bits as u32 + attestation_subnet_count_bits - } - pub fn maximum_gossip_clock_disparity(&self) -> Duration { Duration::from_millis(self.maximum_gossip_clock_disparity_millis) } diff --git a/consensus/types/src/epoch_cache.rs b/consensus/types/src/epoch_cache.rs index 60266fe2fb3..86ac7b45cca 100644 --- a/consensus/types/src/epoch_cache.rs +++ b/consensus/types/src/epoch_cache.rs @@ -40,7 +40,7 @@ pub struct EpochCacheKey { pub enum EpochCacheError { IncorrectEpoch { cache: Epoch, state: Epoch }, IncorrectDecisionBlock { cache: Hash256, state: Hash256 }, - ZeroDecisionBlock, + ZeroStateRoot, ValidatorIndexOutOfBounds { validator_index: usize }, EffectiveBalanceOutOfBounds { effective_balance_eth: usize }, InvalidSlot { slot: Slot },