Fix three consensus bugs!

michaelsproul · michaelsproul · commit d4f87ef1a1bf · 2023-10-11T11:18:22.000+11:00
diff --git a/consensus/state_processing/src/epoch_cache.rs b/consensus/state_processing/src/epoch_cache.rs
@@ -79,22 +79,23 @@ pub fn initialize_epoch_cache<E: EthSpec>(
     state: &mut BeaconState<E>,
     spec: &ChainSpec,
 ) -> Result<(), EpochCacheError> {
-    let epoch = state.current_epoch();
+    let current_epoch = state.current_epoch();
+    let next_epoch = state.next_epoch().map_err(EpochCacheError::BeaconState)?;
     let epoch_cache: &EpochCache = state.epoch_cache();
     let decision_block_root = state
         .proposer_shuffling_decision_root(Hash256::zero())
         .map_err(EpochCacheError::BeaconState)?;
 
     if epoch_cache
-        .check_validity::<E>(epoch, decision_block_root)
+        .check_validity::<E>(current_epoch, decision_block_root)
         .is_ok()
     {
         // `EpochCache` has already been initialized and is valid, no need to initialize.
         return Ok(());
     }
 
-    state.build_total_active_balance_cache_at(epoch, spec)?;
-    let total_active_balance = state.get_total_active_balance_at_epoch(epoch)?;
+    state.build_total_active_balance_cache_at(current_epoch, spec)?;
+    let total_active_balance = state.get_total_active_balance_at_epoch(current_epoch)?;
 
     // Collect effective balances and compute activation queue.
     let mut effective_balances = Vec::with_capacity(state.validators().len());
@@ -104,13 +105,14 @@ pub fn initialize_epoch_cache<E: EthSpec>(
         effective_balances.push(validator.effective_balance());
 
         // Add to speculative activation queue.
-        activation_queue.add_if_could_be_eligible_for_activation(index, validator, epoch, spec);
+        activation_queue
+            .add_if_could_be_eligible_for_activation(index, validator, next_epoch, spec);
     }
 
     // Compute base rewards.
     let pre_epoch_cache = PreEpochCache {
         epoch_key: EpochCacheKey {
-            epoch,
+            epoch: current_epoch,
             decision_block_root,
         },
         effective_balances,
diff --git a/consensus/state_processing/src/per_epoch_processing/single_pass.rs b/consensus/state_processing/src/per_epoch_processing/single_pass.rs
@@ -120,6 +120,7 @@ pub fn process_epoch_single_pass<E: EthSpec>(
     let is_in_inactivity_leak = state.is_in_inactivity_leak(previous_epoch, spec)?;
     let total_active_balance = state.get_total_active_balance()?;
     let churn_limit = state.get_churn_limit(spec)?;
+    let activation_churn_limit = state.get_activation_churn_limit(spec)?;
     let finalized_checkpoint = state.finalized_checkpoint();
     let fork_name = state.fork_name_unchecked();
 
@@ -164,7 +165,10 @@ pub fn process_epoch_single_pass<E: EthSpec>(
     let rewards_ctxt = &RewardsAndPenaltiesContext::new(progressive_balances, state_ctxt, spec)?;
     let activation_queue = &epoch_cache
         .activation_queue()?
-        .get_validators_eligible_for_activation(finalized_checkpoint.epoch, churn_limit as usize);
+        .get_validators_eligible_for_activation(
+            finalized_checkpoint.epoch,
+            activation_churn_limit as usize,
+        );
     let effective_balances_ctxt = &EffectiveBalancesContext::new(spec)?;
 
     // Iterate over the validators and related fields in one pass.
@@ -619,6 +623,7 @@ fn process_single_effective_balance_update(
         // Update progressive balances cache for the *current* epoch, which will soon become the
         // previous epoch once the epoch transition completes.
         progressive_balances.on_effective_balance_change(
+            validator.slashed(),
             validator_info.current_epoch_participation,
             old_effective_balance,
             new_effective_balance,
diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs
@@ -351,6 +351,7 @@ where
     #[test_random(default)]
     pub validators: VList<GenericValidator, T::ValidatorRegistryLimit>,
     #[serde(with = "ssz_types::serde_utils::quoted_u64_var_list")]
+    #[compare_fields(as_iter)]
     #[test_random(default)]
     pub balances: VList<u64, T::ValidatorRegistryLimit>,
 
diff --git a/consensus/types/src/beacon_state/progressive_balances_cache.rs b/consensus/types/src/beacon_state/progressive_balances_cache.rs
@@ -93,10 +93,16 @@ impl EpochTotalBalances {
 
     pub fn on_effective_balance_change(
         &mut self,
+        is_slashed: bool,
         current_epoch_participation_flags: ParticipationFlags,
         old_effective_balance: u64,
         new_effective_balance: u64,
     ) -> Result<(), BeaconStateError> {
+        // If the validator is slashed then we should not update the effective balance, because this
+        // validator's effective balance has already been removed from the totals.
+        if is_slashed {
+            return Ok(());
+        }
         for flag_index in 0..NUM_FLAG_INDICES {
             if current_epoch_participation_flags.has_flag(flag_index)? {
                 let total = self
@@ -188,12 +194,14 @@ impl ProgressiveBalancesCache {
     /// its share of the target attesting balance in the cache.
     pub fn on_effective_balance_change(
         &mut self,
+        is_slashed: bool,
         current_epoch_participation: ParticipationFlags,
         old_effective_balance: u64,
         new_effective_balance: u64,
     ) -> Result<(), BeaconStateError> {
         let cache = self.get_inner_mut()?;
         cache.current_epoch_cache.on_effective_balance_change(
+            is_slashed,
             current_epoch_participation,
             old_effective_balance,
             new_effective_balance,
