From 38d24a6ed9b3475fbd288ba914d8e72cec42b156 Mon Sep 17 00:00:00 2001 From: Mark Mackey Date: Fri, 18 Oct 2024 13:19:05 -0500 Subject: [PATCH 1/3] Simplify Validator Creation and Align with Spec --- .../process_operations.rs | 41 +------------------ consensus/types/src/beacon_state.rs | 28 +++++++++++++ consensus/types/src/validator.rs | 27 +++++++++++- 3 files changed, 55 insertions(+), 41 deletions(-) diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index fb1c5c7eee2..3fa9bf1e509 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -475,45 +475,8 @@ pub fn apply_deposit( 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() as u64 - 1; // [New in Electra:EIP7251] if let Ok(pending_balance_deposits) = state.pending_balance_deposits_mut() { diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index 8eed790a02b..ea01b0f6fcd 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -1548,6 +1548,34 @@ impl BeaconState { .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(); + self.validators_mut() + .push(Validator::from_deposit(deposit_data, fork, spec))?; + if fork.electra_enabled() { + self.balances_mut().push(0)?; + } else { + self.balances_mut().push(deposit_data.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, diff --git a/consensus/types/src/validator.rs b/consensus/types/src/validator.rs index 298604d4f3e..aa4a8de9b12 100644 --- a/consensus/types/src/validator.rs +++ b/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,29 @@ pub struct Validator { } impl Validator { + pub fn from_deposit(deposit_data: &DepositData, 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 amount = deposit_data.amount; + 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 From 12854bce4e2187a1c6b006672bd3a1cabf1da643 Mon Sep 17 00:00:00 2001 From: Mark Mackey Date: Fri, 18 Oct 2024 13:38:41 -0500 Subject: [PATCH 2/3] clippy --- .../src/per_block_processing/process_operations.rs | 4 ++-- consensus/types/src/validator.rs | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index 3fa9bf1e509..a53dc15126f 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -476,12 +476,12 @@ pub fn apply_deposit( } state.add_validator_to_registry(&deposit_data, spec)?; - let new_validator_index = state.validators().len() as u64 - 1; + 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, })?; } diff --git a/consensus/types/src/validator.rs b/consensus/types/src/validator.rs index aa4a8de9b12..27b266dd159 100644 --- a/consensus/types/src/validator.rs +++ b/consensus/types/src/validator.rs @@ -35,6 +35,7 @@ pub struct Validator { } impl Validator { + #[allow(clippy::arithmetic_side_effects)] pub fn from_deposit(deposit_data: &DepositData, fork_name: ForkName, spec: &ChainSpec) -> Self { let mut validator = Validator { pubkey: deposit_data.pubkey, From 462b1eb438ee90571b7005253fafb1f86cbb6ee3 Mon Sep 17 00:00:00 2001 From: Mark Mackey Date: Fri, 18 Oct 2024 14:21:31 -0500 Subject: [PATCH 3/3] Bug Fix --- consensus/types/src/beacon_state.rs | 11 ++++++----- consensus/types/src/validator.rs | 8 ++++++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index ea01b0f6fcd..d772cb23b3d 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -1554,13 +1554,14 @@ impl BeaconState { spec: &ChainSpec, ) -> Result<(), Error> { let fork = self.fork_name_unchecked(); - self.validators_mut() - .push(Validator::from_deposit(deposit_data, fork, spec))?; - if fork.electra_enabled() { - self.balances_mut().push(0)?; + let amount = if fork.electra_enabled() { + 0 } else { - self.balances_mut().push(deposit_data.amount)?; + 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() { diff --git a/consensus/types/src/validator.rs b/consensus/types/src/validator.rs index 27b266dd159..8cf118eea59 100644 --- a/consensus/types/src/validator.rs +++ b/consensus/types/src/validator.rs @@ -36,7 +36,12 @@ pub struct Validator { impl Validator { #[allow(clippy::arithmetic_side_effects)] - pub fn from_deposit(deposit_data: &DepositData, fork_name: ForkName, spec: &ChainSpec) -> Self { + 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, @@ -48,7 +53,6 @@ impl Validator { slashed: false, }; - let amount = deposit_data.amount; 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(