Electra minor refactorings (#6839)

N/A


  Fix some typos and other minor refactorings in the electra code. Thanks @jtraglia for bringing them up.

Note to reviewiers: 4780349 is the commit that needs looking into in detail. The rest are very minor refactorings

Author: pawanjay176

beacon_node/operation_pool/src/attestation_storage.rs

@@ -214,7 +214,7 @@ impl<E: EthSpec> CompactIndexedAttestationElectra<E> {
                 .is_zero()
     }
 
-    /// Returns `true` if aggregated, otherwise `false`.
+    /// Returns `true` if aggregated, otherwise `false`.
     pub fn aggregate_same_committee(&mut self, other: &Self) -> bool {
         if self.committee_bits != other.committee_bits {
             return false;

consensus/state_processing/src/per_block_processing.rs

@@ -523,9 +523,9 @@ pub fn get_expected_withdrawals<E: EthSpec>(
     // [New in Electra:EIP7251]
     // Consume pending partial withdrawals
     let processed_partial_withdrawals_count =
-        if let Ok(partial_withdrawals) = state.pending_partial_withdrawals() {
+        if let Ok(pending_partial_withdrawals) = state.pending_partial_withdrawals() {
             let mut processed_partial_withdrawals_count = 0;
-            for withdrawal in partial_withdrawals {
+            for withdrawal in pending_partial_withdrawals {
                 if withdrawal.withdrawable_epoch > epoch
                     || withdrawals.len() == spec.max_pending_partials_per_withdrawals_sweep as usize
                 {
@@ -552,7 +552,7 @@ pub fn get_expected_withdrawals<E: EthSpec>(
                         validator_index: withdrawal.validator_index,
                         address: validator
                             .get_execution_withdrawal_address(spec)
-                            .ok_or(BeaconStateError::NonExecutionAddresWithdrawalCredential)?,
+                            .ok_or(BeaconStateError::NonExecutionAddressWithdrawalCredential)?,
                         amount: withdrawable_balance,
                     });
                     withdrawal_index.safe_add_assign(1)?;
@@ -583,7 +583,7 @@ pub fn get_expected_withdrawals<E: EthSpec>(
                 validator_index as usize,
             ))?
             .safe_sub(partially_withdrawn_balance)?;
-        if validator.is_fully_withdrawable_at(balance, epoch, spec, fork_name) {
+        if validator.is_fully_withdrawable_validator(balance, epoch, spec, fork_name) {
             withdrawals.push(Withdrawal {
                 index: withdrawal_index,
                 validator_index,
@@ -600,9 +600,7 @@ pub fn get_expected_withdrawals<E: EthSpec>(
                 address: validator
                     .get_execution_withdrawal_address(spec)
                     .ok_or(BlockProcessingError::WithdrawalCredentialsInvalid)?,
-                amount: balance.safe_sub(
-                    validator.get_max_effective_balance(spec, state.fork_name_unchecked()),
-                )?,
+                amount: balance.safe_sub(validator.get_max_effective_balance(spec, fork_name))?,
             });
             withdrawal_index.safe_add_assign(1)?;
         }
@@ -624,7 +622,7 @@ pub fn process_withdrawals<E: EthSpec, Payload: AbstractExecPayload<E>>(
     spec: &ChainSpec,
 ) -> Result<(), BlockProcessingError> {
     if state.fork_name_unchecked().capella_enabled() {
-        let (expected_withdrawals, partial_withdrawals_count) =
+        let (expected_withdrawals, processed_partial_withdrawals_count) =
             get_expected_withdrawals(state, spec)?;
         let expected_root = expected_withdrawals.tree_hash_root();
         let withdrawals_root = payload.withdrawals_root()?;
@@ -645,14 +643,10 @@ pub fn process_withdrawals<E: EthSpec, Payload: AbstractExecPayload<E>>(
         }
 
         // Update pending partial withdrawals [New in Electra:EIP7251]
-        if let Some(partial_withdrawals_count) = partial_withdrawals_count {
-            // TODO(electra): Use efficient pop_front after milhouse release https://github.com/sigp/milhouse/pull/38
-            let new_partial_withdrawals = state
-                .pending_partial_withdrawals()?
-                .iter_from(partial_withdrawals_count)?
-                .cloned()
-                .collect::<Vec<_>>();
-            *state.pending_partial_withdrawals_mut()? = List::new(new_partial_withdrawals)?;
+        if let Some(processed_partial_withdrawals_count) = processed_partial_withdrawals_count {
+            state
+                .pending_partial_withdrawals_mut()?
+                .pop_front(processed_partial_withdrawals_count)?;
         }
 
         // Update the next withdrawal index if this block contained withdrawals

consensus/state_processing/src/per_epoch_processing/single_pass.rs

@@ -1075,13 +1075,9 @@ fn process_pending_consolidations<E: EthSpec>(
         next_pending_consolidation.safe_add_assign(1)?;
     }
 
-    let new_pending_consolidations = List::try_from_iter(
-        state
-            .pending_consolidations()?
-            .iter_from(next_pending_consolidation)?
-            .cloned(),
-    )?;
-    *state.pending_consolidations_mut()? = new_pending_consolidations;
+    state
+        .pending_consolidations_mut()?
+        .pop_front(next_pending_consolidation)?;
 
     // the spec tests require we don't perform effective balance updates when testing pending_consolidations
     if !perform_effective_balance_updates {

consensus/state_processing/src/upgrade/electra.rs

@@ -47,10 +47,11 @@ pub fn upgrade_to_electra<E: EthSpec>(
         .enumerate()
         .filter(|(_, validator)| validator.activation_epoch == spec.far_future_epoch)
         .sorted_by_key(|(index, validator)| (validator.activation_eligibility_epoch, *index))
+        .map(|(index, _)| index)
         .collect::<Vec<_>>();
 
     // Process validators to queue entire balance and reset them
-    for (index, _) in pre_activation {
+    for index in pre_activation {
         let balance = post
             .balances_mut()
             .get_mut(index)

consensus/types/src/beacon_state.rs

@@ -161,7 +161,7 @@ pub enum Error {
     InvalidFlagIndex(usize),
     MerkleTreeError(merkle_proof::MerkleTreeError),
     PartialWithdrawalCountInvalid(usize),
-    NonExecutionAddresWithdrawalCredential,
+    NonExecutionAddressWithdrawalCredential,
     NoCommitteeFound(CommitteeIndex),
     InvalidCommitteeIndex(CommitteeIndex),
     InvalidSelectionProof {
@@ -2214,7 +2214,7 @@ impl<E: EthSpec> BeaconState<E> {
 
     // ******* Electra accessors *******
 
-    /// Return the churn limit for the current epoch.
+    /// Return the churn limit for the current epoch.
     pub fn get_balance_churn_limit(&self, spec: &ChainSpec) -> Result<u64, Error> {
         let total_active_balance = self.get_total_active_balance()?;
         let churn = std::cmp::max(
@@ -2329,21 +2329,12 @@ impl<E: EthSpec> BeaconState<E> {
             | BeaconState::Bellatrix(_)
             | BeaconState::Capella(_)
             | BeaconState::Deneb(_) => Err(Error::IncorrectStateVariant),
-            BeaconState::Electra(_) => {
-                let state = self.as_electra_mut()?;
-
+            BeaconState::Electra(_) | BeaconState::Fulu(_) => {
                 // Consume the balance and update state variables
-                state.exit_balance_to_consume = exit_balance_to_consume.safe_sub(exit_balance)?;
-                state.earliest_exit_epoch = earliest_exit_epoch;
-                Ok(state.earliest_exit_epoch)
-            }
-            BeaconState::Fulu(_) => {
-                let state = self.as_fulu_mut()?;
-
-                // Consume the balance and update state variables
-                state.exit_balance_to_consume = exit_balance_to_consume.safe_sub(exit_balance)?;
-                state.earliest_exit_epoch = earliest_exit_epoch;
-                Ok(state.earliest_exit_epoch)
+                *self.exit_balance_to_consume_mut()? =
+                    exit_balance_to_consume.safe_sub(exit_balance)?;
+                *self.earliest_exit_epoch_mut()? = earliest_exit_epoch;
+                self.earliest_exit_epoch()
             }
         }
     }
@@ -2385,23 +2376,12 @@ impl<E: EthSpec> BeaconState<E> {
             | BeaconState::Bellatrix(_)
             | BeaconState::Capella(_)
             | BeaconState::Deneb(_) => Err(Error::IncorrectStateVariant),
-            BeaconState::Electra(_) => {
-                let state = self.as_electra_mut()?;
-
-                // Consume the balance and update state variables.
-                state.consolidation_balance_to_consume =
-                    consolidation_balance_to_consume.safe_sub(consolidation_balance)?;
-                state.earliest_consolidation_epoch = earliest_consolidation_epoch;
-                Ok(state.earliest_consolidation_epoch)
-            }
-            BeaconState::Fulu(_) => {
-                let state = self.as_fulu_mut()?;
-
+            BeaconState::Electra(_) | BeaconState::Fulu(_) => {
                 // Consume the balance and update state variables.
-                state.consolidation_balance_to_consume =
+                *self.consolidation_balance_to_consume_mut()? =
                     consolidation_balance_to_consume.safe_sub(consolidation_balance)?;
-                state.earliest_consolidation_epoch = earliest_consolidation_epoch;
-                Ok(state.earliest_consolidation_epoch)
+                *self.earliest_consolidation_epoch_mut()? = earliest_consolidation_epoch;
+                self.earliest_consolidation_epoch()
             }
         }
     }

consensus/types/src/validator.rs

@@ -56,7 +56,7 @@ impl Validator {
         };
 
         let max_effective_balance = validator.get_max_effective_balance(spec, fork_name);
-        // safe math is unnecessary here since the spec.effecive_balance_increment is never <= 0
+        // safe math is unnecessary here since the spec.effective_balance_increment is never <= 0
         validator.effective_balance = std::cmp::min(
             amount - (amount % spec.effective_balance_increment),
             max_effective_balance,
@@ -195,22 +195,22 @@ impl Validator {
     /// Returns `true` if the validator is fully withdrawable at some epoch.
     ///
     /// Calls the correct function depending on the provided `fork_name`.
-    pub fn is_fully_withdrawable_at(
+    pub fn is_fully_withdrawable_validator(
         &self,
         balance: u64,
         epoch: Epoch,
         spec: &ChainSpec,
         current_fork: ForkName,
     ) -> bool {
         if current_fork.electra_enabled() {
-            self.is_fully_withdrawable_at_electra(balance, epoch, spec)
+            self.is_fully_withdrawable_validator_electra(balance, epoch, spec)
         } else {
-            self.is_fully_withdrawable_at_capella(balance, epoch, spec)
+            self.is_fully_withdrawable_validator_capella(balance, epoch, spec)
         }
     }
 
     /// Returns `true` if the validator is fully withdrawable at some epoch.
-    fn is_fully_withdrawable_at_capella(
+    fn is_fully_withdrawable_validator_capella(
         &self,
         balance: u64,
         epoch: Epoch,
@@ -222,7 +222,7 @@ impl Validator {
     /// Returns `true` if the validator is fully withdrawable at some epoch.
     ///
     /// Modified in electra as part of EIP 7251.
-    fn is_fully_withdrawable_at_electra(
+    fn is_fully_withdrawable_validator_electra(
         &self,
         balance: u64,
         epoch: Epoch,