diff --git a/consensus/state_processing/src/per_epoch_processing/altair.rs b/consensus/state_processing/src/per_epoch_processing/altair.rs index 1011abe28f..9403ebfbaf 100644 --- a/consensus/state_processing/src/per_epoch_processing/altair.rs +++ b/consensus/state_processing/src/per_epoch_processing/altair.rs @@ -29,7 +29,7 @@ pub fn process_epoch( state.build_committee_cache(RelativeEpoch::Next, spec)?; // Pre-compute participating indices and total balances. - let participation_cache = ParticipationCache::new(state, spec)?; + let mut participation_cache = ParticipationCache::new(state, spec)?; let sync_committee = state.current_sync_committee()?.clone(); // Justification and finalization. @@ -46,6 +46,7 @@ pub fn process_epoch( // Slashings. process_slashings( state, + Some(participation_cache.process_slashings_indices()), participation_cache.current_epoch_total_active_balance(), spec, )?; diff --git a/consensus/state_processing/src/per_epoch_processing/altair/inactivity_updates.rs b/consensus/state_processing/src/per_epoch_processing/altair/inactivity_updates.rs index d637e84676..57cd223b3d 100644 --- a/consensus/state_processing/src/per_epoch_processing/altair/inactivity_updates.rs +++ b/consensus/state_processing/src/per_epoch_processing/altair/inactivity_updates.rs @@ -2,7 +2,6 @@ use super::ParticipationCache; use crate::EpochProcessingError; use safe_arith::SafeArith; use std::cmp::min; -use std::cmp::Ordering; use types::beacon_state::BeaconState; use types::chain_spec::ChainSpec; use types::consts::altair::TIMELY_TARGET_FLAG_INDEX; @@ -19,54 +18,36 @@ pub fn process_inactivity_updates( } let is_in_inactivity_leak = state.is_in_inactivity_leak(spec); - let unslashed_indices = participation_cache - .get_unslashed_participating_indices(TIMELY_TARGET_FLAG_INDEX, state.previous_epoch())?; - - let mut eligible_validators_iter = participation_cache - .eligible_validator_indices() - .iter() - .peekable(); - - // FIXME(sproul): this is a really ugly hack - let mut is_eligible = |index: usize| -> bool { - while let Some(&eligible_index) = eligible_validators_iter.peek() { - match eligible_index.cmp(&index) { - // Should visit every - Ordering::Less => { - unreachable!("should have already visited {}", eligible_index) - } - Ordering::Equal => { - eligible_validators_iter.next(); - return true; - } - Ordering::Greater => { - return false; - } - } - } - false - }; - let mut inactivity_scores = state.inactivity_scores_mut()?.iter_cow(); while let Some((index, inactivity_score)) = inactivity_scores.next_cow() { - if !is_eligible(index) { - continue; - } + let validator = match participation_cache.get_validator(index) { + Ok(val) if val.is_eligible => val, + _ => continue, + }; - let inactivity_score = inactivity_score.to_mut(); + let inactivity_score_mut; // Increase inactivity score of inactive validators - if unslashed_indices.contains(index)? { - inactivity_score.safe_sub_assign(min(1, *inactivity_score))?; + if validator.is_unslashed_participating_index(TIMELY_TARGET_FLAG_INDEX)? { + // Avoid mutating when the inactivity score is 0 and can't go any lower -- the common + // case. + if *inactivity_score == 0 { + continue; + } + inactivity_score_mut = inactivity_score.to_mut(); + inactivity_score_mut.safe_sub_assign(1)?; } else { - inactivity_score.safe_add_assign(spec.inactivity_score_bias)?; + inactivity_score_mut = inactivity_score.to_mut(); + inactivity_score_mut.safe_add_assign(spec.inactivity_score_bias)?; } // Decrease the score of all validators for forgiveness when not during a leak if !is_in_inactivity_leak { - inactivity_score - .safe_sub_assign(min(spec.inactivity_score_recovery_rate, *inactivity_score))?; + inactivity_score_mut.safe_sub_assign(min( + spec.inactivity_score_recovery_rate, + *inactivity_score_mut, + ))?; } } Ok(()) diff --git a/consensus/state_processing/src/per_epoch_processing/altair/justification_and_finalization.rs b/consensus/state_processing/src/per_epoch_processing/altair/justification_and_finalization.rs index f47d9c0e68..88f524ec67 100644 --- a/consensus/state_processing/src/per_epoch_processing/altair/justification_and_finalization.rs +++ b/consensus/state_processing/src/per_epoch_processing/altair/justification_and_finalization.rs @@ -2,7 +2,6 @@ use super::ParticipationCache; use crate::per_epoch_processing::weigh_justification_and_finalization; use crate::per_epoch_processing::Error; use safe_arith::SafeArith; -use types::consts::altair::TIMELY_TARGET_FLAG_INDEX; use types::{BeaconState, EthSpec}; /// Update the justified and finalized checkpoints for matching target attestations. @@ -14,15 +13,9 @@ pub fn process_justification_and_finalization( return Ok(()); } - let previous_epoch = state.previous_epoch(); - let current_epoch = state.current_epoch(); - let previous_indices = participation_cache - .get_unslashed_participating_indices(TIMELY_TARGET_FLAG_INDEX, previous_epoch)?; - let current_indices = participation_cache - .get_unslashed_participating_indices(TIMELY_TARGET_FLAG_INDEX, current_epoch)?; let total_active_balance = participation_cache.current_epoch_total_active_balance(); - let previous_target_balance = previous_indices.total_balance()?; - let current_target_balance = current_indices.total_balance()?; + let previous_target_balance = participation_cache.previous_epoch_target_attesting_balance()?; + let current_target_balance = participation_cache.current_epoch_target_attesting_balance()?; weigh_justification_and_finalization( state, total_active_balance, diff --git a/consensus/state_processing/src/per_epoch_processing/altair/participation_cache.rs b/consensus/state_processing/src/per_epoch_processing/altair/participation_cache.rs index 22238a2c4d..a092d58b64 100644 --- a/consensus/state_processing/src/per_epoch_processing/altair/participation_cache.rs +++ b/consensus/state_processing/src/per_epoch_processing/altair/participation_cache.rs @@ -11,6 +11,7 @@ //! Additionally, this cache is returned from the `altair::process_epoch` function and can be used //! to get useful summaries about the validator participation in an epoch. +use crate::common::altair::get_base_reward; use rustc_hash::FxHashMap as HashMap; use safe_arith::{ArithError, SafeArith}; use types::{ @@ -19,14 +20,14 @@ use types::{ TIMELY_TARGET_FLAG_INDEX, }, BeaconState, BeaconStateError, ChainSpec, Epoch, EthSpec, ParticipationFlags, RelativeEpoch, - Validator, + Unsigned, Validator, }; #[derive(Debug, PartialEq)] pub enum Error { InvalidFlagIndex(usize), NoUnslashedParticipatingIndices, - MissingEffectiveBalance(usize), + MissingValidator(usize), } /// A balance which will never be below the specified `minimum`. @@ -58,16 +59,6 @@ impl Balance { /// Caches the participation values for one epoch (either the previous or current). #[derive(PartialEq, Debug)] struct SingleEpochParticipationCache { - /// Maps an active validator index to their participation flags. - /// - /// To reiterate, only active and unslashed validator indices are stored in this map. - /// - /// ## Note - /// - /// It would be ideal to maintain a reference to the `BeaconState` here rather than copying the - /// `ParticipationFlags`, however that would cause us to run into mutable reference limitations - /// upstream. - unslashed_participating_indices: Option>, /// Stores the sum of the balances for all validators in `self.unslashed_participating_indices` /// for all flags in `NUM_FLAG_INDICES`. /// @@ -79,12 +70,10 @@ struct SingleEpochParticipationCache { } impl SingleEpochParticipationCache { - fn new(hashmap_len: Option, spec: &ChainSpec) -> Self { + fn new(spec: &ChainSpec) -> Self { let zero_balance = Balance::zero(spec.effective_balance_increment); Self { - unslashed_participating_indices: hashmap_len - .map(|len| HashMap::with_capacity_and_hasher(len, Default::default())), total_flag_balances: [zero_balance; NUM_FLAG_INDICES], total_active_balance: zero_balance, } @@ -98,29 +87,6 @@ impl SingleEpochParticipationCache { .ok_or(Error::InvalidFlagIndex(flag_index)) } - /// Returns `true` if `val_index` is active, unslashed and has `flag_index` set. - /// - /// ## Errors - /// - /// May return an error if `flag_index` is out-of-bounds. - fn has_flag(&self, val_index: usize, flag_index: usize) -> Result { - if let Some(participation_flags) = self.unslashed_participating_indices()?.get(&val_index) { - participation_flags - .has_flag(flag_index) - .map_err(|_| Error::InvalidFlagIndex(flag_index)) - } else { - Ok(false) - } - } - - fn unslashed_participating_indices( - &self, - ) -> Result<&HashMap, Error> { - self.unslashed_participating_indices - .as_ref() - .ok_or(Error::NoUnslashedParticipatingIndices) - } - /// Process an **active** validator, reading from the `state` with respect to the /// `relative_epoch`. /// @@ -152,12 +118,6 @@ impl SingleEpochParticipationCache { return Ok(()); } - // Add their `ParticipationFlags` to the map *if* we need them (for the previous epoch). - if let Some(ref mut unslashed_participating_indices) = self.unslashed_participating_indices - { - unslashed_participating_indices.insert(val_index, *epoch_participation); - } - // Iterate through all the flags and increment the total flag balances for whichever flags // are set for `val_index`. for (flag, balance) in self.total_flag_balances.iter_mut().enumerate() { @@ -170,6 +130,42 @@ impl SingleEpochParticipationCache { } } +#[derive(Debug, PartialEq)] +pub struct ValidatorInfo { + pub effective_balance: u64, + pub base_reward: u64, + pub is_eligible: bool, + pub is_slashed: bool, + pub is_active_current_epoch: bool, + pub is_active_previous_epoch: bool, + pub previous_epoch_participation: ParticipationFlags, +} + +impl ValidatorInfo { + pub fn is_unslashed_participating_index(&self, flag_index: usize) -> Result { + Ok(self.is_active_previous_epoch + && !self.is_slashed + && self + .previous_epoch_participation + .has_flag(flag_index) + .map_err(|_| Error::InvalidFlagIndex(flag_index))?) + } +} + +/// Single `HashMap` for validator info relevant to `process_epoch`. +#[derive(Debug, PartialEq)] +struct ValidatorInfoCache { + info: HashMap, +} + +impl ValidatorInfoCache { + pub fn new(capacity: usize) -> Self { + Self { + info: HashMap::with_capacity_and_hasher(capacity, Default::default()), + } + } +} + /// Maintains a cache to be used during `altair::process_epoch`. #[derive(PartialEq, Debug)] pub struct ParticipationCache { @@ -179,10 +175,13 @@ pub struct ParticipationCache { previous_epoch: Epoch, /// Caches information about active validators pertaining to `self.previous_epoch`. previous_epoch_participation: SingleEpochParticipationCache, - /// Caches validator effective balances from the start of `process_epoch`. - effective_balances: HashMap, + /// Caches validator information relevant to `process_epoch`. + validators: ValidatorInfoCache, /// Caches the result of the `get_eligible_validator_indices` function. eligible_indices: Vec, + /// Caches the indices and effective balances of validators that need to be processed by + /// `process_slashings`. + process_slashings_indices: Vec<(usize, u64)>, } impl ParticipationCache { @@ -204,12 +203,12 @@ impl ParticipationCache { // Both the current/previous epoch participations are set to a capacity that is slightly // larger than required. The difference will be due slashed-but-active validators. - let mut current_epoch_participation = SingleEpochParticipationCache::new(None, spec); - let mut previous_epoch_participation = - SingleEpochParticipationCache::new(Some(num_previous_epoch_active_vals), spec); + let mut current_epoch_participation = SingleEpochParticipationCache::new(spec); + let mut previous_epoch_participation = SingleEpochParticipationCache::new(spec); - let mut effective_balances = - HashMap::with_capacity_and_hasher(num_previous_epoch_active_vals, Default::default()); + let mut validators = ValidatorInfoCache::new(num_previous_epoch_active_vals); + + let current_epoch_total_active_balance = state.get_total_active_balance()?; // Contains the set of validators which are either: // @@ -220,6 +219,8 @@ impl ParticipationCache { // reallocations. let mut eligible_indices = Vec::with_capacity(state.validators().len()); + let mut process_slashings_indices = vec![]; + // Iterate through all validators, updating: // // 1. Validator participation for current and previous epochs. @@ -234,7 +235,11 @@ impl ParticipationCache { .zip(state.previous_epoch_participation()?) .enumerate(); for (val_index, ((val, curr_epoch_flags), prev_epoch_flags)) in iter { - if val.is_active_at(current_epoch) { + let is_active_current_epoch = val.is_active_at(current_epoch); + let is_active_previous_epoch = val.is_active_at(previous_epoch); + let is_eligible = state.is_eligible_validator(val); + + if is_active_current_epoch { current_epoch_participation.process_active_validator( val_index, val, @@ -244,7 +249,9 @@ impl ParticipationCache { )?; } - if val.is_active_at(previous_epoch) { + if is_active_previous_epoch { + assert!(is_eligible); + previous_epoch_participation.process_active_validator( val_index, val, @@ -254,21 +261,55 @@ impl ParticipationCache { )?; } + if val.slashed + && current_epoch.safe_add(T::EpochsPerSlashingsVector::to_u64().safe_div(2)?)? + == val.withdrawable_epoch + { + process_slashings_indices.push((val_index, val.effective_balance)); + } + // Note: a validator might still be "eligible" whilst returning `false` to - // `Validator::is_active_at`. - if state.is_eligible_validator(val) { + // `Validator::is_active_at`. It's also possible for a validator to be active + // in the current epoch without being eligible (if it was just activated). + if is_eligible { eligible_indices.push(val_index); - effective_balances.insert(val_index, val.effective_balance); + } + + if is_eligible || is_active_current_epoch { + let effective_balance = val.effective_balance; + let base_reward = + get_base_reward(effective_balance, current_epoch_total_active_balance, spec)?; + + validators.info.insert( + val_index, + ValidatorInfo { + effective_balance, + base_reward, + is_eligible, + is_slashed: val.slashed, + is_active_current_epoch, + is_active_previous_epoch, + previous_epoch_participation: *prev_epoch_flags, + }, + ); } } + // Sanity check total active balance. + // FIXME(sproul): assert + assert_eq!( + current_epoch_participation.total_active_balance.get(), + current_epoch_total_active_balance + ); + Ok(Self { current_epoch, current_epoch_participation, previous_epoch, previous_epoch_participation, - effective_balances, + validators, eligible_indices, + process_slashings_indices, }) } @@ -277,24 +318,8 @@ impl ParticipationCache { &self.eligible_indices } - /// Equivalent to the `get_unslashed_participating_indices` function in the specification. - pub fn get_unslashed_participating_indices( - &self, - flag_index: usize, - epoch: Epoch, - ) -> Result { - let participation = if epoch == self.current_epoch { - &self.current_epoch_participation - } else if epoch == self.previous_epoch { - &self.previous_epoch_participation - } else { - return Err(BeaconStateError::EpochOutOfBounds); - }; - - Ok(UnslashedParticipatingIndices { - participation, - flag_index, - }) + pub fn process_slashings_indices(&mut self) -> Vec<(usize, u64)> { + std::mem::take(&mut self.process_slashings_indices) } /* @@ -315,28 +340,28 @@ impl ParticipationCache { } pub fn previous_epoch_target_attesting_balance(&self) -> Result { - self.previous_epoch_participation - .total_flag_balance(TIMELY_TARGET_FLAG_INDEX) + self.previous_epoch_flag_attesting_balance(TIMELY_TARGET_FLAG_INDEX) } pub fn previous_epoch_source_attesting_balance(&self) -> Result { - self.previous_epoch_participation - .total_flag_balance(TIMELY_SOURCE_FLAG_INDEX) + self.previous_epoch_flag_attesting_balance(TIMELY_SOURCE_FLAG_INDEX) } pub fn previous_epoch_head_attesting_balance(&self) -> Result { + self.previous_epoch_flag_attesting_balance(TIMELY_HEAD_FLAG_INDEX) + } + + pub fn previous_epoch_flag_attesting_balance(&self, flag_index: usize) -> Result { self.previous_epoch_participation - .total_flag_balance(TIMELY_HEAD_FLAG_INDEX) + .total_flag_balance(flag_index) } /* * Active/Unslashed */ - pub fn is_active_unslashed_in_previous_epoch(&self, val_index: usize) -> bool { - self.previous_epoch_participation - .unslashed_participating_indices() - .map_or(false, |indices| indices.contains_key(&val_index)) + pub fn is_active_unslashed_in_previous_epoch(&self, _val_index: usize) -> bool { + false } pub fn is_active_unslashed_in_current_epoch(&self, _val_index: usize) -> bool { @@ -344,88 +369,52 @@ impl ParticipationCache { false } - pub fn get_effective_balance(&self, val_index: usize) -> Result { - self.effective_balances + pub fn get_validator(&self, val_index: usize) -> Result<&ValidatorInfo, Error> { + self.validators + .info .get(&val_index) - .copied() - .ok_or(Error::MissingEffectiveBalance(val_index)) + .ok_or(Error::MissingValidator(val_index)) } /* * Flags */ + // FIXME(sproul): broken /// Always returns false for a slashed validator. pub fn is_previous_epoch_timely_source_attester( &self, - val_index: usize, + _val_index: usize, ) -> Result { - self.previous_epoch_participation - .has_flag(val_index, TIMELY_SOURCE_FLAG_INDEX) + Ok(false) } /// Always returns false for a slashed validator. pub fn is_previous_epoch_timely_target_attester( &self, - val_index: usize, + _val_index: usize, ) -> Result { - self.previous_epoch_participation - .has_flag(val_index, TIMELY_TARGET_FLAG_INDEX) + Ok(false) } /// Always returns false for a slashed validator. - pub fn is_previous_epoch_timely_head_attester(&self, val_index: usize) -> Result { - self.previous_epoch_participation - .has_flag(val_index, TIMELY_HEAD_FLAG_INDEX) + pub fn is_previous_epoch_timely_head_attester(&self, _val_index: usize) -> Result { + Ok(false) } /// Always returns false for a slashed validator. - pub fn is_current_epoch_timely_source_attester(&self, val_index: usize) -> Result { - self.current_epoch_participation - .has_flag(val_index, TIMELY_SOURCE_FLAG_INDEX) + pub fn is_current_epoch_timely_source_attester( + &self, + _val_index: usize, + ) -> Result { + Ok(false) } /// Always returns false for a slashed validator. - pub fn is_current_epoch_timely_target_attester(&self, val_index: usize) -> Result { - self.current_epoch_participation - .has_flag(val_index, TIMELY_TARGET_FLAG_INDEX) - } - - /// Always returns false for a slashed validator. - pub fn is_current_epoch_timely_head_attester(&self, val_index: usize) -> Result { - self.current_epoch_participation - .has_flag(val_index, TIMELY_HEAD_FLAG_INDEX) - } -} - -/// Imitates the return value of the `get_unslashed_participating_indices` in the -/// specification. -/// -/// This struct exists to help make the Lighthouse code read more like the specification. -pub struct UnslashedParticipatingIndices<'a> { - participation: &'a SingleEpochParticipationCache, - flag_index: usize, -} - -impl<'a> UnslashedParticipatingIndices<'a> { - /// Returns `Ok(true)` if the given `val_index` is both: - /// - /// - An active validator. - /// - Has `self.flag_index` set. - pub fn contains(&self, val_index: usize) -> Result { - self.participation.has_flag(val_index, self.flag_index) - } - - /// Returns the sum of all balances of validators which have `self.flag_index` set. - /// - /// ## Notes - /// - /// Respects the `EFFECTIVE_BALANCE_INCREMENT` minimum. - pub fn total_balance(&self) -> Result { - self.participation - .total_flag_balances - .get(self.flag_index) - .ok_or(Error::InvalidFlagIndex(self.flag_index)) - .map(Balance::get) + pub fn is_current_epoch_timely_target_attester( + &self, + _val_index: usize, + ) -> Result { + Ok(false) } } 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 20bab811bd..2d74fcedfc 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 @@ -6,9 +6,7 @@ use types::consts::altair::{ }; use types::{BeaconState, BeaconStateError, ChainSpec, EthSpec}; -use crate::common::{ - altair::get_base_reward, decrease_balance_directly, increase_balance_directly, -}; +use crate::common::{decrease_balance_directly, increase_balance_directly}; use crate::per_epoch_processing::{Delta, Error}; /// Apply attester and proposer rewards. @@ -72,24 +70,20 @@ pub fn get_flag_index_deltas( participation_cache: &ParticipationCache, spec: &ChainSpec, ) -> Result<(), Error> { - let previous_epoch = state.previous_epoch(); - let unslashed_participating_indices = - participation_cache.get_unslashed_participating_indices(flag_index, previous_epoch)?; let weight = get_flag_weight(flag_index)?; - let unslashed_participating_balance = unslashed_participating_indices.total_balance()?; + 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)?; for &index in participation_cache.eligible_validator_indices() { - let base_reward = get_base_reward( - participation_cache.get_effective_balance(index as usize)?, - total_active_balance, - spec, - )?; + let validator = participation_cache.get_validator(index)?; + let base_reward = validator.base_reward; + let mut delta = Delta::default(); - if unslashed_participating_indices.contains(index as usize)? { + if validator.is_unslashed_participating_index(flag_index)? { if !state.is_in_inactivity_leak(spec) { let reward_numerator = base_reward .safe_mul(weight)? @@ -102,8 +96,8 @@ pub fn get_flag_index_deltas( delta.penalize(base_reward.safe_mul(weight)?.safe_div(WEIGHT_DENOMINATOR)?)?; } deltas - .get_mut(index as usize) - .ok_or(Error::DeltaOutOfBounds(index as usize))? + .get_mut(index) + .ok_or(Error::DeltaOutOfBounds(index))? .combine(delta)?; } Ok(()) @@ -123,15 +117,13 @@ pub fn get_inactivity_penalty_deltas( participation_cache: &ParticipationCache, spec: &ChainSpec, ) -> Result<(), Error> { - let previous_epoch = state.previous_epoch(); - let matching_target_indices = participation_cache - .get_unslashed_participating_indices(TIMELY_TARGET_FLAG_INDEX, previous_epoch)?; for &index in participation_cache.eligible_validator_indices() { + let validator = participation_cache.get_validator(index)?; let mut delta = Delta::default(); - if !matching_target_indices.contains(index)? { - let penalty_numerator = participation_cache - .get_effective_balance(index)? + 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 diff --git a/consensus/state_processing/src/per_epoch_processing/base.rs b/consensus/state_processing/src/per_epoch_processing/base.rs index 4ae2207ff2..24e60abe76 100644 --- a/consensus/state_processing/src/per_epoch_processing/base.rs +++ b/consensus/state_processing/src/per_epoch_processing/base.rs @@ -42,6 +42,7 @@ pub fn process_epoch( // Slashings. process_slashings( state, + None, validator_statuses.total_balances.current_epoch(), spec, )?; diff --git a/consensus/state_processing/src/per_epoch_processing/effective_balance_updates.rs b/consensus/state_processing/src/per_epoch_processing/effective_balance_updates.rs index c87871f1d8..ba6f86c248 100644 --- a/consensus/state_processing/src/per_epoch_processing/effective_balance_updates.rs +++ b/consensus/state_processing/src/per_epoch_processing/effective_balance_updates.rs @@ -8,6 +8,11 @@ pub fn process_effective_balance_updates( state: &mut BeaconState, spec: &ChainSpec, ) -> Result<(), EpochProcessingError> { + // Compute new total active balance for the next epoch as a side-effect of iterating the + // effective balances. + let next_epoch = state.next_epoch()?; + let mut new_total_active_balance = 0; + let hysteresis_increment = spec .effective_balance_increment .safe_div(spec.hysteresis_quotient)?; @@ -15,20 +20,35 @@ pub fn process_effective_balance_updates( let upward_threshold = hysteresis_increment.safe_mul(spec.hysteresis_upward_multiplier)?; let (validators, balances) = state.validators_and_balances_mut(); let mut validators_iter = validators.iter_cow(); + while let Some((index, validator)) = validators_iter.next_cow() { let balance = balances .get(index) .copied() .ok_or(BeaconStateError::BalancesOutOfBounds(index))?; - if balance.safe_add(downward_threshold)? < validator.effective_balance + let new_effective_balance = if balance.safe_add(downward_threshold)? + < validator.effective_balance || validator.effective_balance.safe_add(upward_threshold)? < balance { - validator.to_mut().effective_balance = std::cmp::min( + std::cmp::min( balance.safe_sub(balance.safe_rem(spec.effective_balance_increment)?)?, spec.max_effective_balance, - ); + ) + } else { + validator.effective_balance + }; + + if validator.is_active_at(next_epoch) { + new_total_active_balance.safe_add_assign(new_effective_balance)?; + } + + if new_effective_balance != validator.effective_balance { + validator.to_mut().effective_balance = new_effective_balance; } } + + state.set_total_active_balance(next_epoch, new_total_active_balance); + Ok(()) } diff --git a/consensus/state_processing/src/per_epoch_processing/slashings.rs b/consensus/state_processing/src/per_epoch_processing/slashings.rs index e1632280f4..8d7838700a 100644 --- a/consensus/state_processing/src/per_epoch_processing/slashings.rs +++ b/consensus/state_processing/src/per_epoch_processing/slashings.rs @@ -1,10 +1,12 @@ +use crate::common::decrease_balance; use crate::per_epoch_processing::Error; use safe_arith::{SafeArith, SafeArithIter}; -use types::{BeaconState, BeaconStateError, ChainSpec, EthSpec, Unsigned}; +use types::{BeaconState, ChainSpec, EthSpec, Unsigned}; /// Process slashings. pub fn process_slashings( state: &mut BeaconState, + indices: Option>, total_balance: u64, spec: &ChainSpec, ) -> Result<(), Error> { @@ -16,28 +18,30 @@ pub fn process_slashings( total_balance, ); - let (validators, balances) = state.validators_and_balances_mut(); - let mut validators_iter = validators.iter_cow(); - while let Some((index, validator)) = validators_iter.next_cow() { - if validator.slashed - && epoch.safe_add(T::EpochsPerSlashingsVector::to_u64().safe_div(2)?)? - == validator.withdrawable_epoch - { - let increment = spec.effective_balance_increment; - let penalty_numerator = validator - .effective_balance - .safe_div(increment)? - .safe_mul(adjusted_total_slashing_balance)?; - let penalty = penalty_numerator - .safe_div(total_balance)? - .safe_mul(increment)?; + let target_withdrawable_epoch = + epoch.safe_add(T::EpochsPerSlashingsVector::to_u64().safe_div(2)?)?; + let indices = indices.unwrap_or_else(|| { + state + .validators() + .iter() + .enumerate() + .filter(|(_, validator)| { + validator.slashed && target_withdrawable_epoch == validator.withdrawable_epoch + }) + .map(|(index, validator)| (index, validator.effective_balance)) + .collect() + }); - // Equivalent to `decrease_balance(state, index, penalty)`, but avoids borrowing `state`. - let balance = balances - .get_mut(index) - .ok_or(BeaconStateError::BalancesOutOfBounds(index))?; - *balance = balance.saturating_sub(penalty); - } + for (index, validator_effective_balance) in indices { + let increment = spec.effective_balance_increment; + let penalty_numerator = validator_effective_balance + .safe_div(increment)? + .safe_mul(adjusted_total_slashing_balance)?; + let penalty = penalty_numerator + .safe_div(total_balance)? + .safe_mul(increment)?; + + decrease_balance(state, index, penalty)?; } Ok(()) diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index 5a4fe3c043..70666cc432 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -1327,6 +1327,10 @@ impl BeaconState { } } + pub fn set_total_active_balance(&mut self, epoch: Epoch, balance: u64) { + *self.total_active_balance_mut() = Some((epoch, balance)); + } + /// Build the total active balance cache. fn build_total_active_balance_cache(&mut self, spec: &ChainSpec) -> Result<(), Error> { let current_epoch = self.current_epoch(); @@ -1473,33 +1477,9 @@ impl BeaconState { /// /// Note: this function will not build any new committee caches, but will build the total /// balance cache if the (new) current epoch cache is initialized. - pub fn advance_caches(&mut self, spec: &ChainSpec) -> Result<(), Error> { + pub fn advance_caches(&mut self, _spec: &ChainSpec) -> Result<(), Error> { self.committee_caches_mut().rotate_left(1); - // Re-compute total active balance for current epoch. - // - // This can only be computed once the state's effective balances have been updated - // for the current epoch. I.e. it is not possible to know this value with the same - // lookahead as the committee shuffling. - let curr = Self::committee_cache_index(RelativeEpoch::Current); - let curr_cache = mem::take(self.committee_cache_at_index_mut(curr)?); - - // If current epoch cache is initialized, compute the total active balance from its - // indices. We check that the cache is initialized at the _next_ epoch because the slot has - // not yet been advanced. - let new_current_epoch = self.next_epoch()?; - if curr_cache.is_initialized_at(new_current_epoch) { - let total_active_balance = - self.compute_total_active_balance(new_current_epoch, spec)?; - *self.total_active_balance_mut() = Some((new_current_epoch, total_active_balance)); - } - // If the cache is not initialized, then the previous cached value for the total balance is - // wrong, so delete it. - else { - self.drop_total_active_balance_cache(); - } - *self.committee_cache_at_index_mut(curr)? = curr_cache; - let next = Self::committee_cache_index(RelativeEpoch::Next); *self.committee_cache_at_index_mut(next)? = Arc::new(CommitteeCache::default()); Ok(()) diff --git a/lcli/src/transition_blocks.rs b/lcli/src/transition_blocks.rs index 527294035b..64b9e9c467 100644 --- a/lcli/src/transition_blocks.rs +++ b/lcli/src/transition_blocks.rs @@ -45,9 +45,13 @@ pub fn run_transition_blocks( load_from_ssz_with(&block_path, spec, SignedBeaconBlock::from_ssz_bytes)?; let t = std::time::Instant::now(); - let post_state = do_transition(pre_state.clone(), block, spec)?; + let mut post_state = do_transition(pre_state.clone(), block.clone(), spec)?; println!("Total transition time: {}ms", t.elapsed().as_millis()); + if post_state.update_tree_hash_cache().unwrap() != block.state_root() { + return Err("state root mismatch".into()); + } + let mut output_file = File::create(output_path).map_err(|e| format!("Unable to create output file: {:?}", e))?;