Skip to content
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
9eb6f5b
Initial implementation of `ProgressiveTotalBalances` cache
jimmygchen Jun 1, 2023
c650a71
Use `ProgressiveTotalBalances` cache to process jutification and fina…
jimmygchen Jun 1, 2023
36ca37a
Add `ProgressiveTotalBalances` cache to `BeaconState` clone config
jimmygchen Jun 1, 2023
7e2212c
Revert incorrect change in per_epoch_processing and check progressive…
jimmygchen Jun 2, 2023
aeb086a
Initialize `ProgressiveTotalBalances` cache in `per_block_processing`
jimmygchen Jun 2, 2023
0703bcd
Passing fork choice tests \o/
jimmygchen Jun 2, 2023
d31b42e
Fix failing `per_block_processing` tests
jimmygchen Jun 2, 2023
f7cf5dd
Fix ef tests
jimmygchen Jun 2, 2023
787f67d
Fix progressive balances mismatch. Perform checks only in debug mode.…
jimmygchen Jun 6, 2023
4dbb04c
Rename to `progressive_balances_cache` and add some documentation.
jimmygchen Jun 7, 2023
17d7525
Add `--progressive-balances` flag and implement flag behaviour
jimmygchen Jun 7, 2023
33e6b70
Use `Balance` type in `ProgressiveBalancesCache` and remove balance c…
jimmygchen Jun 7, 2023
70f0fa1
Error handling cleanup. Use automatic error conversion.
jimmygchen Jun 7, 2023
3044a94
Merge branch 'unstable' into unrealized-ffg-progressive
jimmygchen Jun 13, 2023
21642f4
Add more logging to progressive cache mismatch
jimmygchen Jun 13, 2023
ec6f8fb
Add failing test for cached balances mismatch when previous epoch tar…
jimmygchen Jun 13, 2023
7dceb0c
Fix cached attester balances mismatch when previous epoch target atte…
jimmygchen Jun 13, 2023
3628c6d
Add failing test for cached balances mismatch when current epoch targ…
jimmygchen Jun 14, 2023
ecaa5ce
Fix cached balances mismatch when a current epoch target attester is …
jimmygchen Jun 15, 2023
a7c15bc
Merge branch 'unstable' into unrealized-ffg-progressive
jimmygchen Jun 16, 2023
cd57aac
Move progressive cache update logic to a separate file and add progre…
jimmygchen Jun 16, 2023
a9b151c
Add another progressive balacnes cache test for proposer slashing sce…
jimmygchen Jun 16, 2023
9bc83e5
Allow the default `ProgressiveBalancesCache::Checked` mode to fallbac…
jimmygchen Jun 16, 2023
0643a26
Merge branch 'unstable' into unrealized-ffg-progressive
jimmygchen Jun 16, 2023
0bddc9e
Add error logging when there is a balance mismatch in `Checked` mode
jimmygchen Jun 16, 2023
f31de13
Clean up: remove ProgressiveBalancesMode checks from Capella tests
jimmygchen Jun 16, 2023
5fcc4f2
Update help text.
jimmygchen Jun 16, 2023
132bf84
Fix tests compilation
jimmygchen Jun 16, 2023
9e29adf
Fix failing EF tests
jimmygchen Jun 16, 2023
93da1ed
Apply suggestions from code review
jimmygchen Jun 26, 2023
5fb3d09
Update `fork_choice.on_block` parameters order and remove some unnece…
jimmygchen Jun 26, 2023
2350ca3
Merge branch 'unstable' into unrealized-ffg-progressive
jimmygchen Jun 26, 2023
617bd3a
Fix compilation errors in tests
jimmygchen Jun 26, 2023
1da56b6
Rename `build_all_caches` as we're now only building 3/5 caches on `B…
jimmygchen Jun 26, 2023
2c9519e
Use "strict" progressive balances mode in EF tests so we start failin…
jimmygchen Jun 29, 2023
964f0bd
Update progressive balances cache metrics at the end of `per_block_pr…
jimmygchen Jun 29, 2023
72935e1
Falls back to the non-optimized unrealized FFG processing method if `…
jimmygchen Jun 29, 2023
f49daa0
Add missing checks when updating progressive balances metrics.
jimmygchen Jun 29, 2023
b12c091
Update `num_caches` to `5` in test.
jimmygchen Jun 29, 2023
90acdc7
Always fallback to the un-optimized processing method unless we're in…
jimmygchen Jun 30, 2023
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
1 change: 1 addition & 0 deletions beacon_node/store/src/partial_beacon_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ macro_rules! impl_try_into_beacon_state {

// Caching
total_active_balance: <_>::default(),
progressive_balances_cache: <_>::default(),
committee_caches: <_>::default(),
pubkey_cache: <_>::default(),
exit_cache: <_>::default(),
Expand Down
71 changes: 68 additions & 3 deletions consensus/fork_choice/src/fork_choice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ use proto_array::{
};
use slog::{crit, debug, warn, Logger};
use ssz_derive::{Decode, Encode};
use state_processing::per_epoch_processing::altair::ParticipationCache;
use state_processing::{
per_block_processing::errors::AttesterSlashingValidationError, per_epoch_processing,
};
use std::cmp::Ordering;
use std::collections::BTreeSet;
use std::marker::PhantomData;
use std::time::Duration;
use types::ProgressiveBalancesCache;
use types::{
consts::merge::INTERVALS_PER_SLOT, AbstractExecPayload, AttestationShufflingId,
AttesterSlashing, BeaconBlockRef, BeaconState, BeaconStateError, ChainSpec, Checkpoint, Epoch,
Expand Down Expand Up @@ -758,9 +760,15 @@ where
BeaconBlockRef::Capella(_)
| BeaconBlockRef::Merge(_)
| BeaconBlockRef::Altair(_) => {
let participation_cache =
per_epoch_processing::altair::ParticipationCache::new(state, spec)
.map_err(Error::ParticipationCacheBuild)?;
let participation_cache = ParticipationCache::new(state, spec)
.map_err(Error::ParticipationCacheBuild)?;

check_progressive_balances_cache(
state.progressive_balances_cache(),
&participation_cache,
spec,
);

per_epoch_processing::altair::process_justification_and_finalization(
state,
&participation_cache,
Expand Down Expand Up @@ -1520,6 +1528,63 @@ where
}
}

/// Check the `ProgressiveBalancesCache` values against the computed `ParticipationCache`.
///
/// ## Panics
///
/// In debug mode (`debug_assertions` set to `true`), this function panics if the cached progressive
/// balances doesn't match computed values from `ParticipationCache`, or if it fails to perform the
/// check.
fn check_progressive_balances_cache(
progressive_balances_cache: &ProgressiveBalancesCache,
participation_cache: &ParticipationCache,
spec: &ChainSpec,
) {
let do_check = || {
// FIXME[JC]: Converts value to 0 if it is the same as `EFFECTIVE_BALANCE_INCREMENT`.
// `ParticipationCache` methods return `EFFECTIVE_BALANCE_INCREMENT` (1,000,000,000)
// when the balance is 0, and this breaks our calculation.
let handle_zero_effective_balance = |val| {
if val == spec.effective_balance_increment {
0
} else {
val
}
};

let previous_target_balance = participation_cache
.previous_epoch_target_attesting_balance()
.map_err(|e| BeaconStateError::ParticipationCacheError(format!("{:?}", e)))?;
let cached_previous_target_balance =
progressive_balances_cache.previous_epoch_target_attesting_balance()?;

debug_assert!(
handle_zero_effective_balance(previous_target_balance) == cached_previous_target_balance,
"Previous epoch target attesting balance from progressive cache does not match computed value."
);

let current_target_balance = participation_cache
.current_epoch_target_attesting_balance()
.map_err(|e| BeaconStateError::ParticipationCacheError(format!("{:?}", e)))?;
let cached_current_target_balance =
progressive_balances_cache.current_epoch_target_attesting_balance()?;

debug_assert!(
handle_zero_effective_balance(current_target_balance) == cached_current_target_balance,
"Current epoch target attesting balance from progressive cache does not match computed value."
);

Ok(())
};

let result: Result<(), BeaconStateError> = do_check();
debug_assert!(
result.is_ok(),
"An error occurred while checking progressive balances: {:?}",
result.err()
);
}

/// Helper struct that is used to encode/decode the state of the `ForkChoice` as SSZ bytes.
///
/// This is used when persisting the state of the fork choice to disk.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
use crate::per_epoch_processing::altair::ParticipationCache;
use std::borrow::Cow;
use types::{BeaconState, BeaconStateError, ChainSpec, EthSpec};

/// Initializes the `ProgressiveBalancesCache` cache using balance values from the
/// `ParticipationCache`. If the optional `&ParticipationCache` is not supplied, it will be computed
/// from the `BeaconState`.
pub fn initialize_progressive_balances_cache<E: EthSpec>(
state: &mut BeaconState<E>,
maybe_participation_cache: Option<&ParticipationCache>,
spec: &ChainSpec,
) -> Result<(), BeaconStateError> {
if !is_progressive_balances_enabled(state)
|| state.progressive_balances_cache().is_initialized()
{
return Ok(());
}

let participation_cache = match maybe_participation_cache {
Some(cache) => Cow::Borrowed(cache),
None => Cow::Owned(ParticipationCache::new(state, spec)?),
};

// FIXME[JC]: Converts value to 0 if it is the same as `EFFECTIVE_BALANCE_INCREMENT`.
// `ParticipationCache` methods return `EFFECTIVE_BALANCE_INCREMENT` (1,000,000,000)
// when the balance is 0, and this breaks our calculation.
let handle_zero_effective_balance = |val| {
if val == spec.effective_balance_increment {
0
} else {
val
}
};

let previous_epoch_target_attesting_balance = participation_cache
.previous_epoch_target_attesting_balance()
.map(handle_zero_effective_balance)
.map_err(|e| BeaconStateError::ParticipationCacheError(format!("{:?}", e)))?;

let current_epoch_target_attesting_balance = participation_cache
.current_epoch_target_attesting_balance()
.map(handle_zero_effective_balance)
.map_err(|e| BeaconStateError::ParticipationCacheError(format!("{:?}", e)))?;

let current_epoch = state.current_epoch();
state.progressive_balances_cache_mut().initialize(
current_epoch,
previous_epoch_target_attesting_balance,
current_epoch_target_attesting_balance,
);

Ok(())
}

/// `ProgressiveBalancesCache` is only enabled from `Altair` as it requires `ParticipationCache`.
fn is_progressive_balances_enabled<E: EthSpec>(state: &BeaconState<E>) -> bool {
match state {
BeaconState::Base(_) => false,
BeaconState::Altair(_) | BeaconState::Merge(_) | BeaconState::Capella(_) => true,
}
}
1 change: 1 addition & 0 deletions consensus/state_processing/src/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ mod slash_validator;

pub mod altair;
pub mod base;
pub mod initialize_progressive_balances_cache;

pub use deposit_data_tree::DepositDataTree;
pub use get_attestation_participation::get_attestation_participation_flag_indices;
Expand Down
2 changes: 1 addition & 1 deletion consensus/state_processing/src/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ pub fn process_activations<T: EthSpec>(
state: &mut BeaconState<T>,
spec: &ChainSpec,
) -> Result<(), Error> {
let (validators, balances) = state.validators_and_balances_mut();
let (validators, balances, _) = state.validators_and_balances_and_progressive_balances_mut();
for (index, validator) in validators.iter_mut().enumerate() {
let balance = balances
.get(index)
Expand Down
3 changes: 3 additions & 0 deletions consensus/state_processing/src/per_block_processing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ mod verify_proposer_slashing;
use crate::common::decrease_balance;
use crate::StateProcessingStrategy;

use crate::common::initialize_progressive_balances_cache::initialize_progressive_balances_cache;
#[cfg(feature = "arbitrary-fuzz")]
use arbitrary::Arbitrary;

Expand Down Expand Up @@ -114,6 +115,8 @@ pub fn per_block_processing<T: EthSpec, Payload: AbstractExecPayload<T>>(
.fork_name(spec)
.map_err(BlockProcessingError::InconsistentStateFork)?;

initialize_progressive_balances_cache(state, None, spec)?;

let verify_signatures = match block_signature_strategy {
BlockSignatureStrategy::VerifyBulk => {
// Verify all signatures in the block at once.
Expand Down
9 changes: 9 additions & 0 deletions consensus/state_processing/src/per_block_processing/errors.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use super::signature_sets::Error as SignatureSetError;
use crate::per_epoch_processing::altair::participation_cache;
use crate::ContextError;
use merkle_proof::MerkleTreeError;
use participation_cache::Error as ParticipationCacheError;
use safe_arith::ArithError;
use ssz::DecodeError;
use types::*;
Expand Down Expand Up @@ -83,6 +85,7 @@ pub enum BlockProcessingError {
found: Hash256,
},
WithdrawalCredentialsInvalid,
ParticipationCacheError(ParticipationCacheError),
}

impl From<BeaconStateError> for BlockProcessingError {
Expand Down Expand Up @@ -140,6 +143,12 @@ impl From<BlockOperationError<HeaderInvalid>> for BlockProcessingError {
}
}

impl From<ParticipationCacheError> for BlockProcessingError {
fn from(e: ParticipationCacheError) -> Self {
BlockProcessingError::ParticipationCacheError(e)
}
}

/// A conversion that consumes `self` and adds an `index` variable to resulting struct.
///
/// Used here to allow converting an error into an upstream error that points to the object that
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ use crate::common::{
use crate::per_block_processing::errors::{BlockProcessingError, IntoWithIndex};
use crate::VerifySignatures;
use safe_arith::SafeArith;
use types::consts::altair::{PARTICIPATION_FLAG_WEIGHTS, PROPOSER_WEIGHT, WEIGHT_DENOMINATOR};
use types::consts::altair::{
PARTICIPATION_FLAG_WEIGHTS, PROPOSER_WEIGHT, TIMELY_TARGET_FLAG_INDEX, WEIGHT_DENOMINATOR,
};

pub fn process_operations<T: EthSpec, Payload: AbstractExecPayload<T>>(
state: &mut BeaconState<T>,
Expand Down Expand Up @@ -97,6 +99,7 @@ pub mod base {

pub mod altair {
use super::*;
use types::consts::altair::TIMELY_TARGET_FLAG_INDEX;

pub fn process_attestations<T: EthSpec>(
state: &mut BeaconState<T>,
Expand Down Expand Up @@ -163,6 +166,15 @@ pub mod altair {
get_base_reward(state, index, base_reward_per_increment, spec)?
.safe_mul(weight)?,
)?;
if flag_index == TIMELY_TARGET_FLAG_INDEX {
let validator_effective_balance = state.get_effective_balance(index)?;
state
.progressive_balances_cache_mut()
.on_new_target_attestation(
data.target.epoch,
validator_effective_balance,
)?;
}
}
}
}
Expand Down Expand Up @@ -230,11 +242,34 @@ pub fn process_attester_slashings<T: EthSpec>(

for i in slashable_indices {
slash_validator(state, i as usize, None, ctxt, spec)?;
update_progressive_balances_cache(state, i as usize)?;
}
}

Ok(())
}

fn update_progressive_balances_cache<T: EthSpec>(
state: &mut BeaconState<T>,
validator_index: usize,
) -> Result<(), BlockProcessingError> {
if let Ok(current_epoch_participation) = state.current_epoch_participation() {
let current_epoch_participation = current_epoch_participation;
let participation_flags = current_epoch_participation
.get(validator_index)
.ok_or(BeaconStateError::UnknownValidator(validator_index))?;
let is_current_epoch_target_attester =
participation_flags.has_flag(TIMELY_TARGET_FLAG_INDEX)?;
let validator_effective_balance = state.get_effective_balance(validator_index)?;
state.progressive_balances_cache_mut().on_slashing(
is_current_epoch_target_attester,
validator_effective_balance,
)?;
}

Ok(())
}

/// Wrapper function to handle calling the correct version of `process_attestations` based on
/// the fork.
pub fn process_attestations<T: EthSpec, Payload: AbstractExecPayload<T>>(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use super::{process_registry_updates, process_slashings, EpochProcessingSummary, Error};
use crate::common::initialize_progressive_balances_cache::initialize_progressive_balances_cache;
use crate::per_epoch_processing::{
effective_balance_updates::process_effective_balance_updates,
historical_roots_update::process_historical_roots_update,
Expand Down Expand Up @@ -31,6 +32,7 @@ pub fn process_epoch<T: EthSpec>(
// Pre-compute participating indices and total balances.
let participation_cache = ParticipationCache::new(state, spec)?;
let sync_committee = state.current_sync_committee()?.clone();
initialize_progressive_balances_cache::<T>(state, Some(&participation_cache), spec)?;

// Justification and finalization.
let justification_and_finalization_state =
Expand All @@ -56,7 +58,7 @@ pub fn process_epoch<T: EthSpec>(
process_eth1_data_reset(state)?;

// Update effective balances with hysteresis (lag).
process_effective_balance_updates(state, spec)?;
process_effective_balance_updates(state, Some(&participation_cache), spec)?;

// Reset slashings
process_slashings_reset(state)?;
Expand All @@ -75,6 +77,11 @@ pub fn process_epoch<T: EthSpec>(
// Rotate the epoch caches to suit the epoch transition.
state.advance_caches(spec)?;

// Update progressive total balances
state
.progressive_balances_cache_mut()
.on_epoch_transition()?;

Ok(EpochProcessingSummary::Altair {
participation_cache,
sync_committee,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use types::{
BeaconState, BeaconStateError, ChainSpec, Epoch, EthSpec, ParticipationFlags, RelativeEpoch,
};

#[derive(Debug, PartialEq)]
#[derive(Debug, PartialEq, Clone)]
pub enum Error {
InvalidFlagIndex(usize),
InvalidValidatorIndex(usize),
Expand Down Expand Up @@ -53,7 +53,7 @@ impl Balance {
}

/// Caches the participation values for one epoch (either the previous or current).
#[derive(PartialEq, Debug)]
#[derive(PartialEq, Debug, Clone)]
struct SingleEpochParticipationCache {
/// Maps an active validator index to their participation flags.
///
Expand Down Expand Up @@ -173,7 +173,7 @@ impl SingleEpochParticipationCache {
}

/// Maintains a cache to be used during `altair::process_epoch`.
#[derive(PartialEq, Debug)]
#[derive(PartialEq, Debug, Clone)]
pub struct ParticipationCache {
current_epoch: Epoch,
/// Caches information about active validators pertaining to `self.current_epoch`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub fn process_epoch<T: EthSpec>(
process_eth1_data_reset(state)?;

// Update effective balances with hysteresis (lag).
process_effective_balance_updates(state, spec)?;
process_effective_balance_updates(state, None, spec)?;

// Reset slashings
process_slashings_reset(state)?;
Expand Down
Loading