Skip to content

Electra minor refactorings #6839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion beacon_node/operation_pool/src/attestation_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
26 changes: 10 additions & 16 deletions consensus/state_processing/src/per_block_processing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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)?;
Expand Down Expand Up @@ -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,
Expand All @@ -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)?;
}
Expand All @@ -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()?;
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion consensus/state_processing/src/upgrade/electra.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
42 changes: 11 additions & 31 deletions consensus/types/src/beacon_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ pub enum Error {
InvalidFlagIndex(usize),
MerkleTreeError(merkle_proof::MerkleTreeError),
PartialWithdrawalCountInvalid(usize),
NonExecutionAddresWithdrawalCredential,
NonExecutionAddressWithdrawalCredential,
NoCommitteeFound(CommitteeIndex),
InvalidCommitteeIndex(CommitteeIndex),
InvalidSelectionProof {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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()
}
}
}
Expand Down Expand Up @@ -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()
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions consensus/types/src/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
Loading