Single-pass epoch processing and optimised block processing (#5279)

* Single-pass epoch processing (#4483, #4573)

Co-authored-by: Michael Sproul <michael@sigmaprime.io>

* Delete unused epoch processing code (#5170)

* Delete unused epoch processing code

* Compare total deltas

* Remove unnecessary apply_pending

* cargo fmt

* Remove newline

* Use epoch cache in block packing (#5223)

* Remove progressive balances mode (#5224)

* inline inactivity_penalty_quotient_for_state

* drop previous_epoch_total_active_balance

* fc lint

* spec compliant process_sync_aggregate (#15)

* spec compliant process_sync_aggregate

* Update consensus/state_processing/src/per_block_processing/altair/sync_committee.rs

Co-authored-by: Michael Sproul <micsproul@gmail.com>

---------

Co-authored-by: Michael Sproul <micsproul@gmail.com>

* Delete the participation cache (#16)

* update help

* Fix op_pool tests

* Fix fork choice tests

* Merge remote-tracking branch 'sigp/unstable' into epoch-single-pass

* Simplify exit cache (#5280)

* Fix clippy on exit cache

* Clean up single-pass a bit (#5282)

* Address Mark's review of single-pass (#5386)

* Merge remote-tracking branch 'origin/unstable' into epoch-single-pass

* Address Sean's review comments (#5414)

* Address most of Sean's review comments

* Simplify total balance cache building

* Clean up unused junk

* Merge remote-tracking branch 'origin/unstable' into epoch-single-pass

* More self-review

* Merge remote-tracking branch 'origin/unstable' into epoch-single-pass

* Merge branch 'unstable' into epoch-single-pass

* Fix imports for beta compiler

* Fix tests, probably

consensus/state_processing/src/common/altair.rs

@@ -24,14 +24,12 @@ impl BaseRewardPerIncrement {
 /// shown to be a significant optimisation.
 ///
 /// Spec v1.1.0
-pub fn get_base_reward<E: EthSpec>(
-    state: &BeaconState<E>,
-    index: usize,
+pub fn get_base_reward(
+    validator_effective_balance: u64,
     base_reward_per_increment: BaseRewardPerIncrement,
     spec: &ChainSpec,
 ) -> Result<u64, Error> {
-    state
-        .get_effective_balance(index)?
+    validator_effective_balance
         .safe_div(spec.effective_balance_increment)?
         .safe_mul(base_reward_per_increment.as_u64())
         .map_err(Into::into)

consensus/state_processing/src/common/base.rs

@@ -1,31 +1,30 @@
 use integer_sqrt::IntegerSquareRoot;
-use safe_arith::SafeArith;
+use safe_arith::{ArithError, SafeArith};
 use types::*;
 
-/// Returns the base reward for some validator.
-pub fn get_base_reward<E: EthSpec>(
-    state: &BeaconState<E>,
-    index: usize,
-    // Should be == get_total_active_balance(state, spec)
-    total_active_balance: u64,
-    spec: &ChainSpec,
-) -> Result<u64, BeaconStateError> {
-    state
-        .get_effective_balance(index)?
-        .safe_mul(spec.base_reward_factor)?
-        .safe_div(total_active_balance.integer_sqrt())?
-        .safe_div(spec.base_rewards_per_epoch)
-        .map_err(Into::into)
+/// This type exists to avoid confusing `total_active_balance` with `sqrt_total_active_balance`,
+/// since they are used in close proximity and have the same type (`u64`).
+#[derive(Copy, Clone)]
+pub struct SqrtTotalActiveBalance(u64);
+
+impl SqrtTotalActiveBalance {
+    pub fn new(total_active_balance: u64) -> Self {
+        Self(total_active_balance.integer_sqrt())
+    }
+
+    pub fn as_u64(&self) -> u64 {
+        self.0
+    }
 }
 
-pub fn get_base_reward_from_effective_balance<E: EthSpec>(
-    effective_balance: u64,
-    total_active_balance: u64,
+/// Returns the base reward for some validator.
+pub fn get_base_reward(
+    validator_effective_balance: u64,
+    sqrt_total_active_balance: SqrtTotalActiveBalance,
     spec: &ChainSpec,
-) -> Result<u64, BeaconStateError> {
-    effective_balance
+) -> Result<u64, ArithError> {
+    validator_effective_balance
         .safe_mul(spec.base_reward_factor)?
-        .safe_div(total_active_balance.integer_sqrt())?
+        .safe_div(sqrt_total_active_balance.as_u64())?
         .safe_div(spec.base_rewards_per_epoch)
-        .map_err(Into::into)
 }

consensus/state_processing/src/common/initiate_validator_exit.rs

@@ -8,10 +8,12 @@ pub fn initiate_validator_exit<E: EthSpec>(
     index: usize,
     spec: &ChainSpec,
 ) -> Result<(), Error> {
-    // Return if the validator already initiated exit
-    if state.get_validator(index)?.exit_epoch != spec.far_future_epoch {
-        return Ok(());
-    }
+    // We do things in a slightly different order to the spec here. Instead of immediately checking
+    // whether the validator has already exited, we instead prepare the exit cache and compute the
+    // cheap-to-calculate values from that. *Then* we look up the validator a single time in the
+    // validator tree (expensive), make the check and mutate as appropriate. Compared to the spec
+    // ordering, this saves us from looking up the validator in the validator registry multiple
+    // times.
 
     // Ensure the exit cache is built.
     state.build_exit_cache(spec)?;
@@ -28,12 +30,20 @@ pub fn initiate_validator_exit<E: EthSpec>(
         exit_queue_epoch.safe_add_assign(1)?;
     }
 
+    let validator = state.get_validator_mut(index)?;
+
+    // Return if the validator already initiated exit
+    if validator.exit_epoch != spec.far_future_epoch {
+        return Ok(());
+    }
+
+    validator.exit_epoch = exit_queue_epoch;
+    validator.withdrawable_epoch =
+        exit_queue_epoch.safe_add(spec.min_validator_withdrawability_delay)?;
+
     state
         .exit_cache_mut()
         .record_validator_exit(exit_queue_epoch)?;
-    state.get_validator_mut(index)?.exit_epoch = exit_queue_epoch;
-    state.get_validator_mut(index)?.withdrawable_epoch =
-        exit_queue_epoch.safe_add(spec.min_validator_withdrawability_delay)?;
 
     Ok(())
 }

consensus/state_processing/src/common/mod.rs

@@ -25,8 +25,7 @@ pub fn increase_balance<E: EthSpec>(
     index: usize,
     delta: u64,
 ) -> Result<(), BeaconStateError> {
-    state.get_balance_mut(index)?.safe_add_assign(delta)?;
-    Ok(())
+    increase_balance_directly(state.get_balance_mut(index)?, delta)
 }
 
 /// Decrease the balance of a validator, saturating upon overflow, as per the spec.
@@ -35,7 +34,17 @@ pub fn decrease_balance<E: EthSpec>(
     index: usize,
     delta: u64,
 ) -> Result<(), BeaconStateError> {
-    let balance = state.get_balance_mut(index)?;
+    decrease_balance_directly(state.get_balance_mut(index)?, delta)
+}
+
+/// Increase the balance of a validator, erroring upon overflow, as per the spec.
+pub fn increase_balance_directly(balance: &mut u64, delta: u64) -> Result<(), BeaconStateError> {
+    balance.safe_add_assign(delta)?;
+    Ok(())
+}
+
+/// Decrease the balance of a validator, saturating upon overflow, as per the spec.
+pub fn decrease_balance_directly(balance: &mut u64, delta: u64) -> Result<(), BeaconStateError> {
     *balance = balance.saturating_sub(delta);
     Ok(())
 }

consensus/state_processing/src/common/slash_validator.rs

@@ -20,6 +20,7 @@ pub fn slash_validator<E: EthSpec>(
     spec: &ChainSpec,
 ) -> Result<(), BlockProcessingError> {
     let epoch = state.current_epoch();
+    let latest_block_slot = state.latest_block_header().slot;
 
     initiate_validator_exit(state, slashed_index, spec)?;
 
@@ -44,7 +45,10 @@ pub fn slash_validator<E: EthSpec>(
             .safe_div(spec.min_slashing_penalty_quotient_for_state(state))?,
     )?;
 
-    update_progressive_balances_on_slashing(state, slashed_index)?;
+    update_progressive_balances_on_slashing(state, slashed_index, validator_effective_balance)?;
+    state
+        .slashings_cache_mut()
+        .record_validator_slashing(latest_block_slot, slashed_index)?;
 
     // Apply proposer and whistleblower rewards
     let proposer_index = ctxt.get_proposer_index(state, spec)? as usize;

consensus/state_processing/src/common/update_progressive_balances_cache.rs

@@ -3,23 +3,16 @@ use crate::metrics::{
     PARTICIPATION_CURR_EPOCH_TARGET_ATTESTING_GWEI_PROGRESSIVE_TOTAL,
     PARTICIPATION_PREV_EPOCH_TARGET_ATTESTING_GWEI_PROGRESSIVE_TOTAL,
 };
-use crate::per_epoch_processing::altair::ParticipationCache;
 use crate::{BlockProcessingError, EpochProcessingError};
 use lighthouse_metrics::set_gauge;
-use ssz_types::VariableList;
-use std::borrow::Cow;
-use types::consts::altair::TIMELY_TARGET_FLAG_INDEX;
 use types::{
-    is_progressive_balances_enabled, BeaconState, BeaconStateError, ChainSpec, Epoch, EthSpec,
-    ParticipationFlags, ProgressiveBalancesCache,
+    is_progressive_balances_enabled, BeaconState, BeaconStateError, ChainSpec, Epoch,
+    EpochTotalBalances, EthSpec, ParticipationFlags, ProgressiveBalancesCache, Validator,
 };
 
-/// Initializes the `ProgressiveBalancesCache` cache using balance values from the
-/// `ParticipationCache`. If the optional `&ParticipationCache` is not supplied, it will be computed
-/// from the `BeaconState`.
+/// Initializes the `ProgressiveBalancesCache` if it is unbuilt.
 pub fn initialize_progressive_balances_cache<E: EthSpec>(
     state: &mut BeaconState<E>,
-    maybe_participation_cache: Option<&ParticipationCache>,
     spec: &ChainSpec,
 ) -> Result<(), BeaconStateError> {
     if !is_progressive_balances_enabled(state)
@@ -28,24 +21,42 @@ pub fn initialize_progressive_balances_cache<E: EthSpec>(
         return Ok(());
     }
 
-    let participation_cache = match maybe_participation_cache {
-        Some(cache) => Cow::Borrowed(cache),
-        None => Cow::Owned(ParticipationCache::new(state, spec)?),
-    };
-
-    let previous_epoch_target_attesting_balance = participation_cache
-        .previous_epoch_target_attesting_balance_raw()
-        .map_err(|e| BeaconStateError::ParticipationCacheError(format!("{e:?}")))?;
-
-    let current_epoch_target_attesting_balance = participation_cache
-        .current_epoch_target_attesting_balance_raw()
-        .map_err(|e| BeaconStateError::ParticipationCacheError(format!("{e:?}")))?;
-
+    // Calculate the total flag balances for previous & current epoch in a single iteration.
+    // This calculates `get_total_balance(unslashed_participating_indices(..))` for each flag in
+    // the current and previous epoch.
     let current_epoch = state.current_epoch();
+    let previous_epoch = state.previous_epoch();
+    let mut previous_epoch_cache = EpochTotalBalances::new(spec);
+    let mut current_epoch_cache = EpochTotalBalances::new(spec);
+    for ((validator, current_epoch_flags), previous_epoch_flags) in state
+        .validators()
+        .iter()
+        .zip(state.current_epoch_participation()?)
+        .zip(state.previous_epoch_participation()?)
+    {
+        // Exclude slashed validators. We are calculating *unslashed* participating totals.
+        if validator.slashed {
+            continue;
+        }
+
+        // Update current epoch flag balances.
+        if validator.is_active_at(current_epoch) {
+            update_flag_total_balances(&mut current_epoch_cache, *current_epoch_flags, validator)?;
+        }
+        // Update previous epoch flag balances.
+        if validator.is_active_at(previous_epoch) {
+            update_flag_total_balances(
+                &mut previous_epoch_cache,
+                *previous_epoch_flags,
+                validator,
+            )?;
+        }
+    }
+
     state.progressive_balances_cache_mut().initialize(
         current_epoch,
-        previous_epoch_target_attesting_balance,
-        current_epoch_target_attesting_balance,
+        previous_epoch_cache,
+        current_epoch_cache,
     );
 
     update_progressive_balances_metrics(state.progressive_balances_cache())?;
@@ -53,20 +64,41 @@ pub fn initialize_progressive_balances_cache<E: EthSpec>(
     Ok(())
 }
 
+/// During the initialization of the progressive balances for a single epoch, add
+/// `validator.effective_balance` to the flag total, for each flag present in `participation_flags`.
+///
+/// Pre-conditions:
+///
+/// - `validator` must not be slashed
+/// - the `participation_flags` must be for `validator` in the same epoch as the `total_balances`
+fn update_flag_total_balances(
+    total_balances: &mut EpochTotalBalances,
+    participation_flags: ParticipationFlags,
+    validator: &Validator,
+) -> Result<(), BeaconStateError> {
+    for (flag, balance) in total_balances.total_flag_balances.iter_mut().enumerate() {
+        if participation_flags.has_flag(flag)? {
+            balance.safe_add_assign(validator.effective_balance)?;
+        }
+    }
+    Ok(())
+}
+
 /// Updates the `ProgressiveBalancesCache` when a new target attestation has been processed.
 pub fn update_progressive_balances_on_attestation<E: EthSpec>(
     state: &mut BeaconState<E>,
     epoch: Epoch,
-    validator_index: usize,
+    flag_index: usize,
+    validator_effective_balance: u64,
+    validator_slashed: bool,
 ) -> Result<(), BlockProcessingError> {
     if is_progressive_balances_enabled(state) {
-        let validator = state.get_validator(validator_index)?;
-        if !validator.slashed {
-            let validator_effective_balance = validator.effective_balance;
-            state
-                .progressive_balances_cache_mut()
-                .on_new_target_attestation(epoch, validator_effective_balance)?;
-        }
+        state.progressive_balances_cache_mut().on_new_attestation(
+            epoch,
+            validator_slashed,
+            flag_index,
+            validator_effective_balance,
+        )?;
     }
     Ok(())
 }
@@ -75,21 +107,22 @@ pub fn update_progressive_balances_on_attestation<E: EthSpec>(
 pub fn update_progressive_balances_on_slashing<E: EthSpec>(
     state: &mut BeaconState<E>,
     validator_index: usize,
+    validator_effective_balance: u64,
 ) -> Result<(), BlockProcessingError> {
     if is_progressive_balances_enabled(state) {
-        let previous_epoch_participation = state.previous_epoch_participation()?;
-        let is_previous_epoch_target_attester =
-            is_target_attester_in_epoch::<E>(previous_epoch_participation, validator_index)?;
+        let previous_epoch_participation = *state
+            .previous_epoch_participation()?
+            .get(validator_index)
+            .ok_or(BeaconStateError::UnknownValidator(validator_index))?;
 
-        let current_epoch_participation = state.current_epoch_participation()?;
-        let is_current_epoch_target_attester =
-            is_target_attester_in_epoch::<E>(current_epoch_participation, validator_index)?;
-
-        let validator_effective_balance = state.get_effective_balance(validator_index)?;
+        let current_epoch_participation = *state
+            .current_epoch_participation()?
+            .get(validator_index)
+            .ok_or(BeaconStateError::UnknownValidator(validator_index))?;
 
         state.progressive_balances_cache_mut().on_slashing(
-            is_previous_epoch_target_attester,
-            is_current_epoch_target_attester,
+            previous_epoch_participation,
+            current_epoch_participation,
             validator_effective_balance,
         )?;
     }
@@ -128,15 +161,3 @@ pub fn update_progressive_balances_metrics(
 
     Ok(())
 }
-
-fn is_target_attester_in_epoch<E: EthSpec>(
-    epoch_participation: &VariableList<ParticipationFlags, E::ValidatorRegistryLimit>,
-    validator_index: usize,
-) -> Result<bool, BlockProcessingError> {
-    let participation_flags = epoch_participation
-        .get(validator_index)
-        .ok_or(BeaconStateError::UnknownValidator(validator_index))?;
-    participation_flags
-        .has_flag(TIMELY_TARGET_FLAG_INDEX)
-        .map_err(|e| e.into())
-}