Simplify Validator Creation and Align with Spec (#6515)

* Simplify Validator Creation and Align with Spec

* clippy

* Bug Fix

consensus/state_processing/src/per_block_processing/process_operations.rs

@@ -475,50 +475,13 @@ pub fn apply_deposit<E: EthSpec>(
             return Ok(());
         }
 
-        let new_validator_index = state.validators().len();
-
-        // [Modified in Electra:EIP7251]
-        let (effective_balance, state_balance) = if state.fork_name_unchecked() >= ForkName::Electra
-        {
-            (0, 0)
-        } else {
-            (
-                std::cmp::min(
-                    amount.safe_sub(amount.safe_rem(spec.effective_balance_increment)?)?,
-                    spec.max_effective_balance,
-                ),
-                amount,
-            )
-        };
-        // Create a new validator.
-        let validator = Validator {
-            pubkey: deposit_data.pubkey,
-            withdrawal_credentials: deposit_data.withdrawal_credentials,
-            activation_eligibility_epoch: spec.far_future_epoch,
-            activation_epoch: spec.far_future_epoch,
-            exit_epoch: spec.far_future_epoch,
-            withdrawable_epoch: spec.far_future_epoch,
-            effective_balance,
-            slashed: false,
-        };
-        state.validators_mut().push(validator)?;
-        state.balances_mut().push(state_balance)?;
-
-        // Altair or later initializations.
-        if let Ok(previous_epoch_participation) = state.previous_epoch_participation_mut() {
-            previous_epoch_participation.push(ParticipationFlags::default())?;
-        }
-        if let Ok(current_epoch_participation) = state.current_epoch_participation_mut() {
-            current_epoch_participation.push(ParticipationFlags::default())?;
-        }
-        if let Ok(inactivity_scores) = state.inactivity_scores_mut() {
-            inactivity_scores.push(0)?;
-        }
+        state.add_validator_to_registry(&deposit_data, spec)?;
+        let new_validator_index = state.validators().len().safe_sub(1)? as u64;
 
         // [New in Electra:EIP7251]
         if let Ok(pending_balance_deposits) = state.pending_balance_deposits_mut() {
             pending_balance_deposits.push(PendingBalanceDeposit {
-                index: new_validator_index as u64,
+                index: new_validator_index,
                 amount,
             })?;
         }

consensus/types/src/beacon_state.rs

@@ -1548,6 +1548,35 @@ impl<E: EthSpec> BeaconState<E> {
             .ok_or(Error::UnknownValidator(validator_index))
     }
 
+    pub fn add_validator_to_registry(
+        &mut self,
+        deposit_data: &DepositData,
+        spec: &ChainSpec,
+    ) -> Result<(), Error> {
+        let fork = self.fork_name_unchecked();
+        let amount = if fork.electra_enabled() {
+            0
+        } else {
+            deposit_data.amount
+        };
+        self.validators_mut()
+            .push(Validator::from_deposit(deposit_data, amount, fork, spec))?;
+        self.balances_mut().push(amount)?;
+
+        // Altair or later initializations.
+        if let Ok(previous_epoch_participation) = self.previous_epoch_participation_mut() {
+            previous_epoch_participation.push(ParticipationFlags::default())?;
+        }
+        if let Ok(current_epoch_participation) = self.current_epoch_participation_mut() {
+            current_epoch_participation.push(ParticipationFlags::default())?;
+        }
+        if let Ok(inactivity_scores) = self.inactivity_scores_mut() {
+            inactivity_scores.push(0)?;
+        }
+
+        Ok(())
+    }
+
     /// Safe copy-on-write accessor for the `validators` list.
     pub fn get_validator_cow(
         &mut self,

consensus/types/src/validator.rs

@@ -1,6 +1,6 @@
 use crate::{
-    test_utils::TestRandom, Address, BeaconState, ChainSpec, Checkpoint, Epoch, EthSpec,
-    FixedBytesExtended, ForkName, Hash256, PublicKeyBytes,
+    test_utils::TestRandom, Address, BeaconState, ChainSpec, Checkpoint, DepositData, Epoch,
+    EthSpec, FixedBytesExtended, ForkName, Hash256, PublicKeyBytes,
 };
 use serde::{Deserialize, Serialize};
 use ssz_derive::{Decode, Encode};
@@ -35,6 +35,34 @@ pub struct Validator {
 }
 
 impl Validator {
+    #[allow(clippy::arithmetic_side_effects)]
+    pub fn from_deposit(
+        deposit_data: &DepositData,
+        amount: u64,
+        fork_name: ForkName,
+        spec: &ChainSpec,
+    ) -> Self {
+        let mut validator = Validator {
+            pubkey: deposit_data.pubkey,
+            withdrawal_credentials: deposit_data.withdrawal_credentials,
+            activation_eligibility_epoch: spec.far_future_epoch,
+            activation_epoch: spec.far_future_epoch,
+            exit_epoch: spec.far_future_epoch,
+            withdrawable_epoch: spec.far_future_epoch,
+            effective_balance: 0,
+            slashed: false,
+        };
+
+        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
+        validator.effective_balance = std::cmp::min(
+            amount - (amount % spec.effective_balance_increment),
+            max_effective_balance,
+        );
+
+        validator
+    }
+
     /// Returns `true` if the validator is considered active at some epoch.
     pub fn is_active_at(&self, epoch: Epoch) -> bool {
         self.activation_epoch <= epoch && epoch < self.exit_epoch