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

beacon_node/beacon_chain/src/attestation_rewards.rs

@@ -1,14 +1,27 @@
 use crate::{BeaconChain, BeaconChainError, BeaconChainTypes};
 use eth2::lighthouse::attestation_rewards::{IdealAttestationRewards, TotalAttestationRewards};
 use eth2::lighthouse::StandardAttestationRewards;
+use eth2::types::ValidatorId;
 use safe_arith::SafeArith;
 use serde_utils::quoted_u64::Quoted;
 use slog::debug;
+use state_processing::common::base::get_base_reward_from_effective_balance;
 use state_processing::per_epoch_processing::altair::{
     process_inactivity_updates_slow, process_justification_and_finalization,
 };
+use state_processing::per_epoch_processing::base::rewards_and_penalties::{
+    get_attestation_component_delta, get_attestation_deltas_all, get_attestation_deltas_subset,
+    get_inactivity_penalty_delta, get_inclusion_delay_delta,
+};
+use state_processing::per_epoch_processing::base::validator_statuses::InclusionInfo;
+use state_processing::per_epoch_processing::base::{
+    process_justification_and_finalization as process_justification_and_finalization_base,
+    TotalBalances, ValidatorStatus, ValidatorStatuses,
+};
 use state_processing::{
     common::altair::BaseRewardPerIncrement,
+    common::update_progressive_balances_cache::initialize_progressive_balances_cache,
+    epoch_cache::initialize_epoch_cache,
     per_epoch_processing::altair::rewards_and_penalties::get_flag_weight,
 };
 use std::collections::HashMap;
@@ -17,20 +30,7 @@ use store::consts::altair::{
     TIMELY_TARGET_FLAG_INDEX,
 };
 use types::consts::altair::WEIGHT_DENOMINATOR;
-
-use types::{BeaconState, Epoch, EthSpec};
-
-use eth2::types::ValidatorId;
-use state_processing::common::base::get_base_reward_from_effective_balance;
-use state_processing::per_epoch_processing::base::rewards_and_penalties::{
-    get_attestation_component_delta, get_attestation_deltas_all, get_attestation_deltas_subset,
-    get_inactivity_penalty_delta, get_inclusion_delay_delta,
-};
-use state_processing::per_epoch_processing::base::validator_statuses::InclusionInfo;
-use state_processing::per_epoch_processing::base::{
-    process_justification_and_finalization as process_justification_and_finalization_base,
-    TotalBalances, ValidatorStatus, ValidatorStatuses,
-};
+use types::{BeaconState, Epoch, EthSpec, RelativeEpoch};
 
 impl<T: BeaconChainTypes> BeaconChain<T> {
     pub fn compute_attestation_rewards(
@@ -132,6 +132,13 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     ) -> Result<StandardAttestationRewards, BeaconChainError> {
         let spec = &self.spec;
 
+        // Build required caches.
+        initialize_epoch_cache(&mut state, spec)?;
+        initialize_progressive_balances_cache(&mut state, spec)?;
+        state.build_exit_cache(spec)?;
+        state.build_committee_cache(RelativeEpoch::Previous, spec)?;
+        state.build_committee_cache(RelativeEpoch::Current, spec)?;
+
         // Calculate ideal_rewards
         process_justification_and_finalization(&state)?.apply_changes_to_state(&mut state);
         process_inactivity_updates_slow(&mut state, spec)?;

consensus/state_processing/src/common/initiate_validator_exit.rs

@@ -11,7 +11,9 @@ pub fn initiate_validator_exit<T: EthSpec>(
     // 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.
+    // 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)?;

consensus/state_processing/src/per_block_processing.rs

@@ -163,7 +163,7 @@ pub fn per_block_processing<T: EthSpec, Payload: AbstractExecPayload<T>>(
     } else {
         verify_signatures
     };
-    // Ensure the current and previous epoch caches are built.
+    // Ensure the current and previous epoch committee caches are built.
     state.build_committee_cache(RelativeEpoch::Previous, spec)?;
     state.build_committee_cache(RelativeEpoch::Current, spec)?;
 

consensus/state_processing/src/per_block_processing/process_operations.rs

@@ -54,8 +54,12 @@ pub mod base {
         ctxt: &mut ConsensusContext<T>,
         spec: &ChainSpec,
     ) -> Result<(), BlockProcessingError> {
-        // Ensure the previous epoch cache exists.
+        // Ensure required caches are all built. These should be no-ops during regular operation.
+        state.build_committee_cache(RelativeEpoch::Current, spec)?;
         state.build_committee_cache(RelativeEpoch::Previous, spec)?;
+        initialize_epoch_cache(state, spec)?;
+        initialize_progressive_balances_cache(state, spec)?;
+        state.build_slashings_cache()?;
 
         let proposer_index = ctxt.get_proposer_index(state, spec)?;
 
@@ -121,9 +125,6 @@ pub mod altair_deneb {
         verify_signatures: VerifySignatures,
         spec: &ChainSpec,
     ) -> Result<(), BlockProcessingError> {
-        state.build_committee_cache(RelativeEpoch::Previous, spec)?;
-        state.build_committee_cache(RelativeEpoch::Current, spec)?;
-
         let proposer_index = ctxt.get_proposer_index(state, spec)?;
         let previous_epoch = ctxt.previous_epoch;
         let current_epoch = ctxt.current_epoch;

consensus/state_processing/src/per_epoch_processing/altair/inactivity_updates.rs

@@ -4,9 +4,9 @@ use types::beacon_state::BeaconState;
 use types::chain_spec::ChainSpec;
 use types::eth_spec::EthSpec;
 
-/// Slow version of `process_inactivity_updates`.
+/// Slow version of `process_inactivity_updates` that runs a subset of single-pass processing.
 ///
-/// Should only be used for testing.
+/// Should not be used for block processing, but is useful for testing & analytics.
 pub fn process_inactivity_updates_slow<T: EthSpec>(
     state: &mut BeaconState<T>,
     spec: &ChainSpec,

consensus/state_processing/src/per_epoch_processing/single_pass.rs

@@ -13,7 +13,7 @@ use types::{
         TIMELY_TARGET_FLAG_INDEX, WEIGHT_DENOMINATOR,
     },
     ActivationQueue, BeaconState, BeaconStateError, ChainSpec, Epoch, EthSpec, ExitCache, ForkName,
-    ParticipationFlags, ProgressiveBalancesCache, Unsigned, Validator,
+    ParticipationFlags, ProgressiveBalancesCache, RelativeEpoch, Unsigned, Validator,
 };
 
 pub struct SinglePassConfig {
@@ -112,6 +112,8 @@ pub fn process_epoch_single_pass<E: EthSpec>(
     initialize_epoch_cache(state, spec)?;
     initialize_progressive_balances_cache(state, spec)?;
     state.build_exit_cache(spec)?;
+    state.build_committee_cache(RelativeEpoch::Previous, spec)?;
+    state.build_committee_cache(RelativeEpoch::Current, spec)?;
 
     let previous_epoch = state.previous_epoch();
     let current_epoch = state.current_epoch();

consensus/types/src/beacon_state.rs

@@ -1433,9 +1433,7 @@ impl<T: EthSpec> BeaconState<T> {
 
     /// Return the churn limit for the current epoch (number of validators who can leave per epoch).
     ///
-    /// Uses the epoch cache, and will error if it isn't initialized.
-    ///
-    /// Spec v0.12.1
+    /// Uses the current epoch committee cache, and will error if it isn't initialized.
     pub fn get_churn_limit(&self, spec: &ChainSpec) -> Result<u64, Error> {
         Ok(std::cmp::max(
             spec.min_per_epoch_churn_limit,
@@ -1448,9 +1446,7 @@ impl<T: EthSpec> BeaconState<T> {
 
     /// Return the activation churn limit for the current epoch (number of validators who can enter per epoch).
     ///
-    /// Uses the epoch cache, and will error if it isn't initialized.
-    ///
-    /// Spec v1.4.0
+    /// Uses the current epoch committee cache, and will error if it isn't initialized.
     pub fn get_activation_churn_limit(&self, spec: &ChainSpec) -> Result<u64, Error> {
         Ok(match self {
             BeaconState::Base(_)
@@ -1673,7 +1669,7 @@ impl<T: EthSpec> BeaconState<T> {
         })
     }
 
-    /// Build an epoch cache, unless it is has already been built.
+    /// Build a committee cache, unless it is has already been built.
     pub fn build_committee_cache(
         &mut self,
         relative_epoch: RelativeEpoch,
@@ -1694,7 +1690,7 @@ impl<T: EthSpec> BeaconState<T> {
         Ok(())
     }
 
-    /// Always builds the previous epoch cache, even if it is already initialized.
+    /// Always builds the requested committee cache, even if it is already initialized.
     pub fn force_build_committee_cache(
         &mut self,
         relative_epoch: RelativeEpoch,
@@ -1724,7 +1720,7 @@ impl<T: EthSpec> BeaconState<T> {
     /// This should be used if the `slot` of this state is advanced beyond an epoch boundary.
     ///
     /// Note: this function will not build any new committee caches, but will build the total
-    /// balance cache if the (new) current epoch cache is initialized.
+    /// balance cache if the (new) current committee cache is initialized.
     pub fn advance_caches(&mut self, _spec: &ChainSpec) -> Result<(), Error> {
         self.committee_caches_mut().rotate_left(1);
 
@@ -1970,6 +1966,9 @@ impl<T: EthSpec> BeaconState<T> {
         Ok(sync_committee)
     }
 
+    /// Get the base reward for `validator_index` from the epoch cache.
+    ///
+    /// This function will error if the epoch cache is not initialized.
     pub fn get_base_reward(&self, validator_index: usize) -> Result<u64, EpochCacheError> {
         self.epoch_cache().get_base_reward(validator_index)
     }

consensus/types/src/chain_spec.rs

@@ -495,6 +495,9 @@ impl ChainSpec {
         Hash256::from(domain)
     }
 
+    /// Compute the epoch used for activations prior to Deneb, and for exits under all forks.
+    ///
+    /// Spec: https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/beacon-chain.md#compute_activation_exit_epoch
     pub fn compute_activation_exit_epoch(&self, epoch: Epoch) -> Result<Epoch, ArithError> {
         epoch.safe_add(1)?.safe_add(self.max_seed_lookahead)
     }