From 1af3f0f9d8aaa86a9f4156a0e65b8705c23c93e9 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Fri, 3 May 2024 15:24:01 +0530 Subject: [PATCH] Make modified helpers in electra fork aware (#5698) * Make modified helpers in electra fork aware * Make more functions fork aware * formatting fixes only. --- .../src/per_block_processing.rs | 5 +- .../per_epoch_processing/registry_updates.rs | 5 +- .../src/per_epoch_processing/single_pass.rs | 2 +- consensus/types/src/validator.rs | 96 +++++++++++++++++-- 4 files changed, 96 insertions(+), 12 deletions(-) diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index 2efa1218829..98671f82b9f 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -508,6 +508,7 @@ pub fn get_expected_withdrawals( let mut withdrawal_index = state.next_withdrawal_index()?; let mut validator_index = state.next_withdrawal_validator_index()?; let mut withdrawals = vec![]; + let fork_name = state.fork_name_unchecked(); let bound = std::cmp::min( state.validators().len() as u64, @@ -518,7 +519,7 @@ pub fn get_expected_withdrawals( let balance = *state.balances().get(validator_index as usize).ok_or( BeaconStateError::BalancesOutOfBounds(validator_index as usize), )?; - if validator.is_fully_withdrawable_at(balance, epoch, spec) { + if validator.is_fully_withdrawable_at(balance, epoch, spec, fork_name) { withdrawals.push(Withdrawal { index: withdrawal_index, validator_index, @@ -528,7 +529,7 @@ pub fn get_expected_withdrawals( amount: balance, }); withdrawal_index.safe_add_assign(1)?; - } else if validator.is_partially_withdrawable_validator(balance, spec) { + } else if validator.is_partially_withdrawable_validator(balance, spec, fork_name) { withdrawals.push(Withdrawal { index: withdrawal_index, validator_index, diff --git a/consensus/state_processing/src/per_epoch_processing/registry_updates.rs b/consensus/state_processing/src/per_epoch_processing/registry_updates.rs index 4b2f940e5f8..3d02d797366 100644 --- a/consensus/state_processing/src/per_epoch_processing/registry_updates.rs +++ b/consensus/state_processing/src/per_epoch_processing/registry_updates.rs @@ -19,19 +19,20 @@ pub fn process_registry_updates( validator.is_active_at(current_epoch) && validator.effective_balance <= spec.ejection_balance }; + let fork_name = state.fork_name_unchecked(); let indices_to_update: Vec<_> = state .validators() .iter() .enumerate() .filter(|(_, validator)| { - validator.is_eligible_for_activation_queue(spec) || is_ejectable(validator) + validator.is_eligible_for_activation_queue(spec, fork_name) || is_ejectable(validator) }) .map(|(idx, _)| idx) .collect(); for index in indices_to_update { let validator = state.get_validator_mut(index)?; - if validator.is_eligible_for_activation_queue(spec) { + if validator.is_eligible_for_activation_queue(spec, fork_name) { validator.activation_eligibility_epoch = current_epoch.safe_add(1)?; } if is_ejectable(validator) { diff --git a/consensus/state_processing/src/per_epoch_processing/single_pass.rs b/consensus/state_processing/src/per_epoch_processing/single_pass.rs index 7a95de3317e..a9629e73e40 100644 --- a/consensus/state_processing/src/per_epoch_processing/single_pass.rs +++ b/consensus/state_processing/src/per_epoch_processing/single_pass.rs @@ -466,7 +466,7 @@ fn process_single_registry_update( ) -> Result<(), Error> { let current_epoch = state_ctxt.current_epoch; - if validator.is_eligible_for_activation_queue(spec) { + if validator.is_eligible_for_activation_queue(spec, state_ctxt.fork_name) { validator.make_mut()?.activation_eligibility_epoch = current_epoch.safe_add(1)?; } diff --git a/consensus/types/src/validator.rs b/consensus/types/src/validator.rs index 8ed449ec8a7..9e26d1eeca6 100644 --- a/consensus/types/src/validator.rs +++ b/consensus/types/src/validator.rs @@ -1,5 +1,5 @@ use crate::{ - test_utils::TestRandom, Address, BeaconState, ChainSpec, Epoch, EthSpec, Hash256, + test_utils::TestRandom, Address, BeaconState, ChainSpec, Epoch, EthSpec, ForkName, Hash256, PublicKeyBytes, }; use serde::{Deserialize, Serialize}; @@ -57,8 +57,31 @@ impl Validator { /// Returns `true` if the validator is eligible to join the activation queue. /// - /// Modified in electra - pub fn is_eligible_for_activation_queue(&self, spec: &ChainSpec) -> bool { + /// Calls the correct function depending on the provided `fork_name`. + pub fn is_eligible_for_activation_queue( + &self, + spec: &ChainSpec, + current_fork: ForkName, + ) -> bool { + if current_fork >= ForkName::Electra { + self.is_eligible_for_activation_queue_electra(spec) + } else { + self.is_eligible_for_activation_queue_base(spec) + } + } + + /// Returns `true` if the validator is eligible to join the activation queue. + /// + /// Spec v0.12.1 + fn is_eligible_for_activation_queue_base(&self, spec: &ChainSpec) -> bool { + self.activation_eligibility_epoch == spec.far_future_epoch + && self.effective_balance == spec.max_effective_balance + } + + /// Returns `true` if the validator is eligible to join the activation queue. + /// + /// Modified in electra as part of EIP 7251. + fn is_eligible_for_activation_queue_electra(&self, spec: &ChainSpec) -> bool { self.activation_eligibility_epoch == spec.far_future_epoch && self.effective_balance >= spec.min_activation_balance } @@ -131,8 +154,40 @@ impl Validator { /// Returns `true` if the validator is fully withdrawable at some epoch. /// - /// Note: Modified in electra. - pub fn is_fully_withdrawable_at(&self, balance: u64, epoch: Epoch, spec: &ChainSpec) -> bool { + /// Calls the correct function depending on the provided `fork_name`. + pub fn is_fully_withdrawable_at( + &self, + balance: u64, + epoch: Epoch, + spec: &ChainSpec, + current_fork: ForkName, + ) -> bool { + if current_fork >= ForkName::Electra { + self.is_fully_withdrawable_at_electra(balance, epoch, spec) + } else { + self.is_fully_withdrawable_at_capella(balance, epoch, spec) + } + } + + /// Returns `true` if the validator is fully withdrawable at some epoch. + fn is_fully_withdrawable_at_capella( + &self, + balance: u64, + epoch: Epoch, + spec: &ChainSpec, + ) -> bool { + self.has_eth1_withdrawal_credential(spec) && self.withdrawable_epoch <= epoch && balance > 0 + } + + /// 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( + &self, + balance: u64, + epoch: Epoch, + spec: &ChainSpec, + ) -> bool { self.has_execution_withdrawal_credential(spec) && self.withdrawable_epoch <= epoch && balance > 0 @@ -140,8 +195,35 @@ impl Validator { /// Returns `true` if the validator is partially withdrawable. /// - /// Note: Modified in electra. - pub fn is_partially_withdrawable_validator(&self, balance: u64, spec: &ChainSpec) -> bool { + /// Calls the correct function depending on the provided `fork_name`. + pub fn is_partially_withdrawable_validator( + &self, + balance: u64, + spec: &ChainSpec, + current_fork: ForkName, + ) -> bool { + if current_fork >= ForkName::Electra { + self.is_partially_withdrawable_validator_electra(balance, spec) + } else { + self.is_partially_withdrawable_validator_capella(balance, spec) + } + } + + /// Returns `true` if the validator is partially withdrawable. + fn is_partially_withdrawable_validator_capella(&self, balance: u64, spec: &ChainSpec) -> bool { + self.has_eth1_withdrawal_credential(spec) + && self.effective_balance == spec.max_effective_balance + && balance > spec.max_effective_balance + } + + /// Returns `true` if the validator is partially withdrawable. + /// + /// Modified in electra as part of EIP 7251. + pub fn is_partially_withdrawable_validator_electra( + &self, + balance: u64, + spec: &ChainSpec, + ) -> bool { let max_effective_balance = self.get_validator_max_effective_balance(spec); let has_max_effective_balance = self.effective_balance == max_effective_balance; let has_excess_balance = balance > max_effective_balance;