Skip to content
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.

Sign up for GitHub

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

Delete unused epoch processing code #5170

Merged
merged 5 commits into from
Feb 7, 2024
Merged

Delete unused epoch processing code #5170

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0ca51de
Delete unused epoch processing code
michaelsproul Feb 2, 2024
2f32fa6
Compare total deltas
michaelsproul Feb 5, 2024
7cc1506
Remove unnecessary apply_pending
michaelsproul Feb 5, 2024
81181d1
cargo fmt
michaelsproul Feb 5, 2024
9057a97
Remove newline
michaelsproul Feb 5, 2024
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
82 changes: 4 additions & 78 deletions consensus/state_processing/src/per_epoch_processing/altair/rewards_and_penalties.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
use super::ParticipationCache;

use crate::per_epoch_processing::{
single_pass::{process_epoch_single_pass, SinglePassConfig},
Delta, Error,
single_pass::{process_epoch_single_pass, SinglePassConfig}, Error,
};
use safe_arith::SafeArith;

use types::consts::altair::{
PARTICIPATION_FLAG_WEIGHTS, TIMELY_HEAD_FLAG_INDEX, TIMELY_TARGET_FLAG_INDEX,
WEIGHT_DENOMINATOR,
PARTICIPATION_FLAG_WEIGHTS,
};
use types::{BeaconState, ChainSpec, EthSpec};

Expand All @@ -28,82 +26,10 @@ pub fn process_rewards_and_penalties_slow<T: EthSpec>(
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<T: EthSpec>(
deltas: &mut [Delta],
state: &BeaconState<T>,
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<u64, Error> {
PARTICIPATION_FLAG_WEIGHTS
.get(flag_index)
.copied()
.ok_or(Error::InvalidFlagIndex(flag_index))
}

pub fn get_inactivity_penalty_deltas<T: EthSpec>(
deltas: &mut [Delta],
state: &BeaconState<T>,
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(())
}
102 changes: 19 additions & 83 deletions testing/ef_tests/src/cases/rewards.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,13 @@ 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},
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 {
Expand Down Expand Up @@ -108,53 +104,30 @@ impl<E: EthSpec> Case for RewardsTest<E> {

fn result(&self, _case_index: usize, fork_name: ForkName) -> Result<(), Error> {
let mut state = self.pre.clone();

// NOTE: We cannot run these tests for forks other than phase0 because single-pass epoch
// processing cannot expose these individual deltas. There is no point maintaining a
// separate implementation of rewards processing that will not be used in prod.
if fork_name != ForkName::Base {
return Ok(());
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For post-base forks, assert the sum of all deltas instead of skipping

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea, implemented in the latest commit. Just had to skip 1 test due to ethereum/consensus-specs#3593.


let spec = &testing_spec::<E>(fork_name);

let deltas: Result<AllDeltas, EpochProcessingError> = (|| {
// 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)?;

let deltas = base::rewards_and_penalties::get_attestation_deltas_all(
&state,
&validator_statuses,
spec,
)?;

Ok(convert_all_base_deltas(&deltas))
} else {
let total_active_balance = state.get_total_active_balance()?;

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,
})
}
let mut validator_statuses = ValidatorStatuses::new(&state, spec)?;
validator_statuses.process_attestations(&state)?;

let deltas = base::rewards_and_penalties::get_attestation_deltas_all(
&state,
&validator_statuses,
spec,
)?;

Ok(convert_all_base_deltas(&deltas))
})();

compare_result_detailed(&deltas, &Some(self.deltas.clone()))?;
Expand All @@ -181,40 +154,3 @@ fn convert_base_deltas(attestation_deltas: &[AttestationDelta], accessor: Access
.unzip();
Deltas { rewards, penalties }
}

fn compute_altair_flag_deltas<E: EthSpec>(
state: &BeaconState<E>,
flag_index: usize,
total_active_balance: u64,
spec: &ChainSpec,
) -> Result<Deltas, EpochProcessingError> {
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 compute_altair_inactivity_deltas<E: EthSpec>(
state: &BeaconState<E>,
spec: &ChainSpec,
) -> Result<Deltas, EpochProcessingError> {
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 convert_altair_deltas(deltas: Vec<Delta>) -> Deltas {
let (rewards, penalties) = deltas.into_iter().map(|d| (d.rewards, d.penalties)).unzip();
Deltas { rewards, penalties }
}
Loading