From 8e2f7c5fb9233b72abaebea9a9f5bd82ef5afd40 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 8 Feb 2024 09:20:28 +1100 Subject: [PATCH] Delete unused epoch processing code (#5170) * Delete unused epoch processing code * Compare total deltas * Remove unnecessary apply_pending * cargo fmt * Remove newline --- .../altair/rewards_and_penalties.rs | 81 +--------- testing/ef_tests/src/cases/rewards.rs | 149 +++++++++--------- 2 files changed, 79 insertions(+), 151 deletions(-) diff --git a/consensus/state_processing/src/per_epoch_processing/altair/rewards_and_penalties.rs b/consensus/state_processing/src/per_epoch_processing/altair/rewards_and_penalties.rs index 9510f2eaec4..6b36346847b 100644 --- a/consensus/state_processing/src/per_epoch_processing/altair/rewards_and_penalties.rs +++ b/consensus/state_processing/src/per_epoch_processing/altair/rewards_and_penalties.rs @@ -1,13 +1,8 @@ -use super::ParticipationCache; use crate::per_epoch_processing::{ single_pass::{process_epoch_single_pass, SinglePassConfig}, - Delta, Error, -}; -use safe_arith::SafeArith; -use types::consts::altair::{ - PARTICIPATION_FLAG_WEIGHTS, TIMELY_HEAD_FLAG_INDEX, TIMELY_TARGET_FLAG_INDEX, - WEIGHT_DENOMINATOR, + Error, }; +use types::consts::altair::PARTICIPATION_FLAG_WEIGHTS; use types::{BeaconState, ChainSpec, EthSpec}; /// Apply attester and proposer rewards. @@ -28,51 +23,6 @@ pub fn process_rewards_and_penalties_slow( Ok(()) } -/// Return the deltas for a given flag index by scanning through the participation flags. -/// -/// Spec v1.1.0 -pub fn get_flag_index_deltas( - deltas: &mut [Delta], - state: &BeaconState, - flag_index: usize, - total_active_balance: u64, - participation_cache: &ParticipationCache, - spec: &ChainSpec, -) -> Result<(), Error> { - let weight = get_flag_weight(flag_index)?; - let unslashed_participating_balance = - participation_cache.previous_epoch_flag_attesting_balance(flag_index)?; - let unslashed_participating_increments = - unslashed_participating_balance.safe_div(spec.effective_balance_increment)?; - let active_increments = total_active_balance.safe_div(spec.effective_balance_increment)?; - let previous_epoch = state.previous_epoch(); - - for &index in participation_cache.eligible_validator_indices() { - let validator = participation_cache.get_validator(index)?; - let base_reward = validator.base_reward; - - let mut delta = Delta::default(); - - if validator.is_unslashed_participating_index(flag_index)? { - if !state.is_in_inactivity_leak(previous_epoch, spec)? { - let reward_numerator = base_reward - .safe_mul(weight)? - .safe_mul(unslashed_participating_increments)?; - delta.reward( - reward_numerator.safe_div(active_increments.safe_mul(WEIGHT_DENOMINATOR)?)?, - )?; - } - } else if flag_index != TIMELY_HEAD_FLAG_INDEX { - delta.penalize(base_reward.safe_mul(weight)?.safe_div(WEIGHT_DENOMINATOR)?)?; - } - deltas - .get_mut(index) - .ok_or(Error::DeltaOutOfBounds(index))? - .combine(delta)?; - } - Ok(()) -} - /// Get the weight for a `flag_index` from the constant list of all weights. pub fn get_flag_weight(flag_index: usize) -> Result { PARTICIPATION_FLAG_WEIGHTS @@ -80,30 +30,3 @@ pub fn get_flag_weight(flag_index: usize) -> Result { .copied() .ok_or(Error::InvalidFlagIndex(flag_index)) } - -pub fn get_inactivity_penalty_deltas( - deltas: &mut [Delta], - state: &BeaconState, - participation_cache: &ParticipationCache, - spec: &ChainSpec, -) -> Result<(), Error> { - for &index in participation_cache.eligible_validator_indices() { - let validator = participation_cache.get_validator(index)?; - let mut delta = Delta::default(); - - if !validator.is_unslashed_participating_index(TIMELY_TARGET_FLAG_INDEX)? { - let penalty_numerator = validator - .effective_balance - .safe_mul(state.get_inactivity_score(index)?)?; - let penalty_denominator = spec - .inactivity_score_bias - .safe_mul(spec.inactivity_penalty_quotient_for_state(state))?; - delta.penalize(penalty_numerator.safe_div(penalty_denominator)?)?; - } - deltas - .get_mut(index) - .ok_or(Error::DeltaOutOfBounds(index))? - .combine(delta)?; - } - Ok(()) -} diff --git a/testing/ef_tests/src/cases/rewards.rs b/testing/ef_tests/src/cases/rewards.rs index bb41f6fe12f..1a8d5b0f539 100644 --- a/testing/ef_tests/src/cases/rewards.rs +++ b/testing/ef_tests/src/cases/rewards.rs @@ -7,17 +7,14 @@ use ssz::four_byte_option_impl; use ssz_derive::{Decode, Encode}; use state_processing::{ per_epoch_processing::{ - altair::{self, rewards_and_penalties::get_flag_index_deltas, ParticipationCache}, + altair, base::{self, rewards_and_penalties::AttestationDelta, ValidatorStatuses}, Delta, }, EpochProcessingError, }; use std::path::{Path, PathBuf}; -use types::{ - consts::altair::{TIMELY_HEAD_FLAG_INDEX, TIMELY_SOURCE_FLAG_INDEX, TIMELY_TARGET_FLAG_INDEX}, - BeaconState, EthSpec, ForkName, -}; +use types::{BeaconState, EthSpec, ForkName}; #[derive(Debug, Clone, PartialEq, Decode, Encode, CompareFields)] pub struct Deltas { @@ -41,6 +38,11 @@ pub struct AllDeltas { inactivity_penalty_deltas: Deltas, } +#[derive(Debug, Clone, PartialEq, CompareFields)] +pub struct TotalDeltas { + deltas: Vec, +} + #[derive(Debug, Clone, Default, Deserialize)] pub struct Metadata { pub description: Option, @@ -110,11 +112,19 @@ impl Case for RewardsTest { let mut state = self.pre.clone(); let spec = &testing_spec::(fork_name); - let deltas: Result = (|| { - // Processing requires the committee caches. - state.build_all_committee_caches(spec)?; + // Single-pass epoch processing doesn't compute rewards in the genesis epoch because that's + // what the spec for `process_rewards_and_penalties` says to do. We skip these tests for now. + // + // See: https://github.com/ethereum/consensus-specs/issues/3593 + if fork_name != ForkName::Base && state.current_epoch() == 0 { + return Err(Error::SkippedKnownFailure); + } + + if let BeaconState::Base(_) = state { + let deltas: Result = (|| { + // Processing requires the committee caches. + state.build_all_committee_caches(spec)?; - if let BeaconState::Base(_) = state { let mut validator_statuses = ValidatorStatuses::new(&state, spec)?; validator_statuses.process_attestations(&state)?; @@ -125,39 +135,19 @@ impl Case for RewardsTest { )?; Ok(convert_all_base_deltas(&deltas)) - } else { - let total_active_balance = state.get_total_active_balance()?; + })(); + compare_result_detailed(&deltas, &Some(self.deltas.clone()))?; + } else { + let deltas: Result = (|| { + // Processing requires the committee caches. + state.build_all_committee_caches(spec)?; + compute_altair_deltas(&mut state, spec) + })(); - let source_deltas = compute_altair_flag_deltas( - &state, - TIMELY_SOURCE_FLAG_INDEX, - total_active_balance, - spec, - )?; - let target_deltas = compute_altair_flag_deltas( - &state, - TIMELY_TARGET_FLAG_INDEX, - total_active_balance, - spec, - )?; - let head_deltas = compute_altair_flag_deltas( - &state, - TIMELY_HEAD_FLAG_INDEX, - total_active_balance, - spec, - )?; - let inactivity_penalty_deltas = compute_altair_inactivity_deltas(&state, spec)?; - Ok(AllDeltas { - source_deltas, - target_deltas, - head_deltas, - inclusion_delay_deltas: None, - inactivity_penalty_deltas, - }) - } - })(); - - compare_result_detailed(&deltas, &Some(self.deltas.clone()))?; + let expected = all_deltas_to_total_deltas(&self.deltas); + + compare_result_detailed(&deltas, &Some(expected))?; + }; Ok(()) } @@ -182,39 +172,54 @@ fn convert_base_deltas(attestation_deltas: &[AttestationDelta], accessor: Access Deltas { rewards, penalties } } -fn compute_altair_flag_deltas( - state: &BeaconState, - flag_index: usize, - total_active_balance: u64, - spec: &ChainSpec, -) -> Result { - let mut deltas = vec![Delta::default(); state.validators().len()]; - get_flag_index_deltas( - &mut deltas, - state, - flag_index, - total_active_balance, - &ParticipationCache::new(state, spec).unwrap(), - spec, - )?; - Ok(convert_altair_deltas(deltas)) +fn deltas_to_total_deltas(d: &Deltas) -> impl Iterator + '_ { + d.rewards + .iter() + .zip(&d.penalties) + .map(|(&reward, &penalty)| reward as i64 - penalty as i64) } -fn compute_altair_inactivity_deltas( - state: &BeaconState, - spec: &ChainSpec, -) -> Result { - let mut deltas = vec![Delta::default(); state.validators().len()]; - altair::rewards_and_penalties::get_inactivity_penalty_deltas( - &mut deltas, - state, - &ParticipationCache::new(state, spec).unwrap(), - spec, - )?; - Ok(convert_altair_deltas(deltas)) +fn optional_deltas_to_total_deltas(d: &Option, len: usize) -> TotalDeltas { + let deltas = if let Some(d) = d { + deltas_to_total_deltas(d).collect() + } else { + vec![0i64; len] + }; + TotalDeltas { deltas } } -fn convert_altair_deltas(deltas: Vec) -> Deltas { - let (rewards, penalties) = deltas.into_iter().map(|d| (d.rewards, d.penalties)).unzip(); - Deltas { rewards, penalties } +fn all_deltas_to_total_deltas(d: &AllDeltas) -> TotalDeltas { + let len = d.source_deltas.rewards.len(); + let deltas = deltas_to_total_deltas(&d.source_deltas) + .zip(deltas_to_total_deltas(&d.target_deltas)) + .zip(deltas_to_total_deltas(&d.head_deltas)) + .zip(optional_deltas_to_total_deltas(&d.inclusion_delay_deltas, len).deltas) + .zip(deltas_to_total_deltas(&d.inactivity_penalty_deltas)) + .map( + |((((source, target), head), inclusion_delay), inactivity_penalty)| { + source + target + head + inclusion_delay + inactivity_penalty + }, + ) + .collect::>(); + TotalDeltas { deltas } +} + +fn compute_altair_deltas( + state: &mut BeaconState, + spec: &ChainSpec, +) -> Result { + // Initialise deltas to pre-state balances. + let mut deltas = state + .balances() + .iter() + .map(|x| *x as i64) + .collect::>(); + altair::process_rewards_and_penalties_slow(state, spec)?; + + for (delta, new_balance) in deltas.iter_mut().zip(state.balances()) { + let old_balance = *delta; + *delta = *new_balance as i64 - old_balance; + } + + Ok(TotalDeltas { deltas }) }