Clean up single-pass a bit (#5282)

beacon_node/beacon_chain/src/attestation_rewards.rs

@@ -193,8 +193,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         };
 
         for &validator_index in &validators {
-            // Return 0s for unknown/inactive validator indices. This is a bit different from stable
-            // where we error for unknown pubkeys.
+            // Return 0s for unknown/inactive validator indices.
             let Ok(validator) = state.get_validator(validator_index) else {
                 debug!(
                     self.log,

beacon_node/genesis/src/interop.rs

@@ -178,14 +178,13 @@ mod test {
         }
 
         for v in state.validators() {
-            let creds = v.withdrawal_credentials;
+            let creds = v.withdrawal_credentials.as_bytes();
             assert_eq!(
-                creds.as_bytes()[0],
-                spec.bls_withdrawal_prefix_byte,
+                creds[0], spec.bls_withdrawal_prefix_byte,
                 "first byte of withdrawal creds should be bls prefix"
             );
             assert_eq!(
-                &creds.as_bytes()[1..],
+                &creds[1..],
                 &hash(&v.pubkey.as_ssz_bytes())[1..],
                 "rest of withdrawal creds should be pubkey hash"
             )
@@ -241,8 +240,7 @@ mod test {
         }
 
         for (index, v) in state.validators().iter().enumerate() {
-            let withdrawal_credientials = v.withdrawal_credentials;
-            let creds = withdrawal_credientials.as_bytes();
+            let creds = v.withdrawal_credentials.as_bytes();
             if index % 2 == 0 {
                 assert_eq!(
                     creds[0], spec.bls_withdrawal_prefix_byte,

book/src/validator-inclusion.md

@@ -56,7 +56,6 @@ The following fields are returned:
 	able to vote) during the current epoch.
 - `current_epoch_target_attesting_gwei`: the total staked gwei that attested to
 	the majority-elected Casper FFG target epoch during the current epoch.
-- `previous_epoch_active_gwei`: as per `current_epoch_active_gwei`, but during the previous epoch.
 - `previous_epoch_target_attesting_gwei`: see `current_epoch_target_attesting_gwei`.
 - `previous_epoch_head_attesting_gwei`: the total staked gwei that attested to a
 	head beacon block that is in the canonical chain.
@@ -65,7 +64,7 @@ From this data you can calculate:
 
 #### Justification/Finalization Rate
 
-`previous_epoch_target_attesting_gwei / previous_epoch_active_gwei`
+`previous_epoch_target_attesting_gwei / current_epoch_active_gwei`
 
 When this value is greater than or equal to `2/3` it is possible that the
 beacon chain may justify and/or finalize the epoch.
@@ -80,7 +79,6 @@ curl -X GET "http://localhost:5052/lighthouse/validator_inclusion/0/global" -H
 {
   "data": {
     "current_epoch_active_gwei": 642688000000000,
-    "previous_epoch_active_gwei": 642688000000000,
     "current_epoch_target_attesting_gwei": 366208000000000,
     "previous_epoch_target_attesting_gwei": 1000000000,
     "previous_epoch_head_attesting_gwei": 1000000000

consensus/state_processing/src/all_caches.rs

@@ -9,14 +9,12 @@ use types::{BeaconState, ChainSpec, EpochCacheError, EthSpec, Hash256, RelativeE
 pub trait AllCaches {
     /// Build all caches.
     ///
-    /// Note that this excludes milhouse's intrinsic tree-hash cache. That needs to be managed
-    /// separately.
+    /// Note that this excludes the tree-hash cache. That needs to be managed separately.
     fn build_all_caches(&mut self, spec: &ChainSpec) -> Result<(), EpochCacheError>;
 
     /// Return true if all caches are built.
     ///
-    /// Note that this excludes milhouse's intrinsic tree-hash cache. That needs to be managed
-    /// separately.
+    /// Note that this excludes the tree-hash cache. That needs to be managed separately.
     fn all_caches_built(&self) -> bool;
 }
 

consensus/state_processing/src/common/base.rs

@@ -3,7 +3,7 @@ use safe_arith::{ArithError, SafeArith};
 use types::*;
 
 /// This type exists to avoid confusing `total_active_balance` with `sqrt_total_active_balance`,
-/// since they are used in close proximity and the same type (`u64`).
+/// since they are used in close proximity and have the same type (`u64`).
 #[derive(Copy, Clone)]
 pub struct SqrtTotalActiveBalance(u64);
 

consensus/state_processing/src/per_block_processing/process_operations.rs

@@ -113,7 +113,6 @@ pub mod altair_deneb {
             })
     }
 
-    #[allow(clippy::too_many_arguments)]
     pub fn process_attestation<T: EthSpec>(
         state: &mut BeaconState<T>,
         attestation: &Attestation<T>,

consensus/state_processing/src/per_epoch_processing/effective_balance_updates.rs

@@ -5,6 +5,7 @@ use types::beacon_state::BeaconState;
 use types::chain_spec::ChainSpec;
 use types::{BeaconStateError, EthSpec};
 
+/// This implementation is now only used in phase0. Later hard forks use single-pass.
 pub fn process_effective_balance_updates<T: EthSpec>(
     state: &mut BeaconState<T>,
     spec: &ChainSpec,
@@ -52,6 +53,7 @@ pub fn process_effective_balance_updates<T: EthSpec>(
     Ok(())
 }
 
+/// Only used to test the effective balance part of single-pass in isolation.
 pub fn process_effective_balance_updates_slow<T: EthSpec>(
     state: &mut BeaconState<T>,
     spec: &ChainSpec,

consensus/types/src/beacon_state.rs

@@ -1480,24 +1480,10 @@ impl<T: EthSpec> BeaconState<T> {
         Ok(cache.get_attestation_duties(validator_index))
     }
 
-    /// Implementation of `get_total_balance`, matching the spec.
+    /// Build the total active balance cache from scratch.
     ///
-    /// Returns minimum `EFFECTIVE_BALANCE_INCREMENT`, to avoid div by 0.
-    pub fn get_total_balance<'a, I: IntoIterator<Item = &'a usize>>(
-        &'a self,
-        validator_indices: I,
-        spec: &ChainSpec,
-    ) -> Result<u64, Error> {
-        let total_balance = validator_indices.into_iter().try_fold(0_u64, |acc, i| {
-            self.get_effective_balance(*i)
-                .and_then(|bal| Ok(acc.safe_add(bal)?))
-        })?;
-        Ok(std::cmp::max(
-            total_balance,
-            spec.effective_balance_increment,
-        ))
-    }
-
+    /// This method should rarely be invoked because single-pass epoch processing keeps the total
+    /// active balance cache up to date.
     pub fn compute_total_active_balance_slow(
         &self,
         epoch: Epoch,
@@ -1530,6 +1516,7 @@ impl<T: EthSpec> BeaconState<T> {
         self.get_total_active_balance_at_epoch(self.current_epoch())
     }
 
+    /// Get the cached total active balance while checking that it is for the correct `epoch`.
     pub fn get_total_active_balance_at_epoch(&self, epoch: Epoch) -> Result<u64, Error> {
         let (initialized_epoch, balance) = self
             .total_active_balance()
@@ -1545,6 +1532,10 @@ impl<T: EthSpec> BeaconState<T> {
         }
     }
 
+    /// Manually set the total active balance.
+    ///
+    /// 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));
     }
@@ -1561,6 +1552,7 @@ impl<T: EthSpec> BeaconState<T> {
         Ok(())
     }
 
+    /// Build the total active balance cache, even if it is already built.
     pub fn force_build_total_active_balance_cache_at(
         &mut self,
         epoch: Epoch,