From 46be05f7280c3afdfbd614dbc8f6a9b3985ab821 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Fri, 30 Jun 2023 01:13:06 +0000 Subject: [PATCH] Cache target attester balances for unrealized FFG progression calculation (#4362) ## Issue Addressed #4118 ## Proposed Changes This PR introduces a "progressive balances" cache on the `BeaconState`, which keeps track of the accumulated target attestation balance for the current & previous epochs. The cached values are utilised by fork choice to calculate unrealized justification and finalization (instead of converting epoch participation arrays to balances for each block we receive). This optimization will be rolled out gradually to allow for more testing. A new `--progressive-balances disabled|checked|strict|fast` flag is introduced to support this: - `checked`: enabled with checks against participation cache, and falls back to the existing epoch processing calculation if there is a total target attester balance mismatch. There is no performance gain from this as the participation cache still needs to be computed. **This is the default mode for now.** - `strict`: enabled with checks against participation cache, returns error if there is a mismatch. **Used for testing only**. - `fast`: enabled with no comparative checks and without computing the participation cache. This mode gives us the performance gains from the optimization. This is still experimental and not currently recommended for production usage, but will become the default mode in a future release. - `disabled`: disable the usage of progressive cache, and use the existing method for FFG progression calculation. This mode may be useful if we find a bug and want to stop the frequent error logs. ### Tasks - [x] Initial cache implementation in `BeaconState` - [x] Perform checks in fork choice to compare the progressive balances cache against results from `ParticipationCache` - [x] Add CLI flag, and disable the optimization by default - [x] Testing on Goerli & Benchmarking - [x] Move caching logic from state processing to the `ProgressiveBalancesCache` (see [this comment](https://github.com/sigp/lighthouse/pull/4362#discussion_r1230877001)) - [x] Add attesting balance metrics Co-authored-by: Jimmy Chen --- Cargo.lock | 1 + beacon_node/beacon_chain/src/beacon_chain.rs | 2 + beacon_node/beacon_chain/src/builder.rs | 8 +- beacon_node/beacon_chain/src/chain_config.rs | 5 +- beacon_node/beacon_chain/src/fork_revert.rs | 9 +- beacon_node/beacon_chain/src/test_utils.rs | 38 +++- beacon_node/beacon_chain/tests/capella.rs | 18 +- .../tests/payload_invalidation.rs | 3 +- beacon_node/http_api/src/block_rewards.rs | 2 +- beacon_node/src/cli.rs | 14 ++ beacon_node/src/config.rs | 6 + beacon_node/store/src/partial_beacon_state.rs | 1 + beacon_node/store/src/reconstruct.rs | 2 +- consensus/fork_choice/src/fork_choice.rs | 209 +++++++++++++++--- consensus/fork_choice/tests/tests.rs | 106 ++++++++- consensus/state_processing/src/common/mod.rs | 1 + .../src/common/slash_validator.rs | 3 + .../update_progressive_balances_cache.rs | 142 ++++++++++++ consensus/state_processing/src/genesis.rs | 4 +- consensus/state_processing/src/metrics.rs | 11 + .../src/per_block_processing.rs | 9 + .../src/per_block_processing/errors.rs | 9 + .../process_operations.rs | 11 + .../src/per_epoch_processing/altair.rs | 8 +- .../altair/participation_cache.rs | 54 ++--- .../src/per_epoch_processing/base.rs | 2 +- .../src/per_epoch_processing/capella.rs | 8 +- .../effective_balance_updates.rs | 41 +++- .../src/per_epoch_processing/slashings.rs | 2 +- .../state_processing/src/upgrade/altair.rs | 4 + .../state_processing/src/upgrade/capella.rs | 1 + .../state_processing/src/upgrade/merge.rs | 1 + consensus/types/Cargo.toml | 1 + consensus/types/benches/benches.rs | 2 +- consensus/types/src/beacon_state.rs | 55 ++++- consensus/types/src/beacon_state/balance.rs | 33 +++ .../types/src/beacon_state/clone_config.rs | 2 + .../progressive_balances_cache.rs | 184 +++++++++++++++ consensus/types/src/beacon_state/tests.rs | 5 +- lcli/src/new_testnet.rs | 2 +- lcli/src/skip_slots.rs | 2 +- lcli/src/transition_blocks.rs | 6 +- lighthouse/tests/beacon_node.rs | 30 ++- .../ef_tests/src/cases/epoch_processing.rs | 4 +- testing/ef_tests/src/cases/fork_choice.rs | 5 +- testing/ef_tests/src/cases/operations.rs | 4 + testing/ef_tests/src/cases/sanity_blocks.rs | 2 +- testing/ef_tests/src/cases/sanity_slots.rs | 2 +- 48 files changed, 953 insertions(+), 121 deletions(-) create mode 100644 consensus/state_processing/src/common/update_progressive_balances_cache.rs create mode 100644 consensus/types/src/beacon_state/balance.rs create mode 100644 consensus/types/src/beacon_state/progressive_balances_cache.rs diff --git a/Cargo.lock b/Cargo.lock index 02922b2d7e..efc6a5d6ab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8935,6 +8935,7 @@ dependencies = [ "smallvec", "ssz_types", "state_processing", + "strum", "superstruct 0.6.0", "swap_or_not_shuffle", "tempfile", diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 772e4c1529..01343ff3b1 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -2898,7 +2898,9 @@ impl BeaconChain { block_delay, &state, payload_verification_status, + self.config.progressive_balances_mode, &self.spec, + &self.log, ) .map_err(|e| BlockError::BeaconChainError(e.into()))?; } diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 9bb3939632..044391c415 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -338,7 +338,7 @@ where let beacon_block = genesis_block(&mut beacon_state, &self.spec)?; beacon_state - .build_all_caches(&self.spec) + .build_caches(&self.spec) .map_err(|e| format!("Failed to build genesis state caches: {:?}", e))?; let beacon_state_root = beacon_block.message().state_root(); @@ -437,7 +437,7 @@ where // Prime all caches before storing the state in the database and computing the tree hash // root. weak_subj_state - .build_all_caches(&self.spec) + .build_caches(&self.spec) .map_err(|e| format!("Error building caches on checkpoint state: {e:?}"))?; let computed_state_root = weak_subj_state @@ -687,6 +687,8 @@ where store.clone(), Some(current_slot), &self.spec, + self.chain_config.progressive_balances_mode, + &log, )?; } @@ -700,7 +702,7 @@ where head_snapshot .beacon_state - .build_all_caches(&self.spec) + .build_caches(&self.spec) .map_err(|e| format!("Failed to build state caches: {:?}", e))?; // Perform a check to ensure that the finalization points of the head and fork choice are diff --git a/beacon_node/beacon_chain/src/chain_config.rs b/beacon_node/beacon_chain/src/chain_config.rs index 34a5c9a4ec..cc7a957ecc 100644 --- a/beacon_node/beacon_chain/src/chain_config.rs +++ b/beacon_node/beacon_chain/src/chain_config.rs @@ -1,7 +1,7 @@ pub use proto_array::{DisallowedReOrgOffsets, ReOrgThreshold}; use serde_derive::{Deserialize, Serialize}; use std::time::Duration; -use types::{Checkpoint, Epoch}; +use types::{Checkpoint, Epoch, ProgressiveBalancesMode}; pub const DEFAULT_RE_ORG_THRESHOLD: ReOrgThreshold = ReOrgThreshold(20); pub const DEFAULT_RE_ORG_MAX_EPOCHS_SINCE_FINALIZATION: Epoch = Epoch::new(2); @@ -81,6 +81,8 @@ pub struct ChainConfig { pub always_prepare_payload: bool, /// Whether backfill sync processing should be rate-limited. pub enable_backfill_rate_limiting: bool, + /// Whether to use `ProgressiveBalancesCache` in unrealized FFG progression calculation. + pub progressive_balances_mode: ProgressiveBalancesMode, } impl Default for ChainConfig { @@ -111,6 +113,7 @@ impl Default for ChainConfig { genesis_backfill: false, always_prepare_payload: false, enable_backfill_rate_limiting: true, + progressive_balances_mode: ProgressiveBalancesMode::Checked, } } } diff --git a/beacon_node/beacon_chain/src/fork_revert.rs b/beacon_node/beacon_chain/src/fork_revert.rs index 084ae95e09..dc0e34277c 100644 --- a/beacon_node/beacon_chain/src/fork_revert.rs +++ b/beacon_node/beacon_chain/src/fork_revert.rs @@ -10,7 +10,10 @@ use state_processing::{ use std::sync::Arc; use std::time::Duration; use store::{iter::ParentRootBlockIterator, HotColdDB, ItemStore}; -use types::{BeaconState, ChainSpec, EthSpec, ForkName, Hash256, SignedBeaconBlock, Slot}; +use types::{ + BeaconState, ChainSpec, EthSpec, ForkName, Hash256, ProgressiveBalancesMode, SignedBeaconBlock, + Slot, +}; const CORRUPT_DB_MESSAGE: &str = "The database could be corrupt. Check its file permissions or \ consider deleting it by running with the --purge-db flag."; @@ -100,6 +103,8 @@ pub fn reset_fork_choice_to_finalization, Cold: It store: Arc>, current_slot: Option, spec: &ChainSpec, + progressive_balances_mode: ProgressiveBalancesMode, + log: &Logger, ) -> Result, E>, String> { // Fetch finalized block. let finalized_checkpoint = head_state.finalized_checkpoint(); @@ -197,7 +202,9 @@ pub fn reset_fork_choice_to_finalization, Cold: It Duration::from_secs(0), &state, payload_verification_status, + progressive_balances_mode, spec, + log, ) .map_err(|e| format!("Error applying replayed block to fork choice: {:?}", e))?; } diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 21f7248cee..6520c9ba9c 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -754,9 +754,7 @@ where complete_state_advance(&mut state, None, slot, &self.spec) .expect("should be able to advance state to slot"); - state - .build_all_caches(&self.spec) - .expect("should build caches"); + state.build_caches(&self.spec).expect("should build caches"); let proposer_index = state.get_beacon_proposer_index(slot, &self.spec).unwrap(); @@ -803,9 +801,7 @@ where complete_state_advance(&mut state, None, slot, &self.spec) .expect("should be able to advance state to slot"); - state - .build_all_caches(&self.spec) - .expect("should build caches"); + state.build_caches(&self.spec).expect("should build caches"); let proposer_index = state.get_beacon_proposer_index(slot, &self.spec).unwrap(); @@ -1523,6 +1519,36 @@ where .sign(sk, &fork, genesis_validators_root, &self.chain.spec) } + pub fn add_proposer_slashing(&self, validator_index: u64) -> Result<(), String> { + let propposer_slashing = self.make_proposer_slashing(validator_index); + if let ObservationOutcome::New(verified_proposer_slashing) = self + .chain + .verify_proposer_slashing_for_gossip(propposer_slashing) + .expect("should verify proposer slashing for gossip") + { + self.chain + .import_proposer_slashing(verified_proposer_slashing); + Ok(()) + } else { + Err("should observe new proposer slashing".to_string()) + } + } + + pub fn add_attester_slashing(&self, validator_indices: Vec) -> Result<(), String> { + let attester_slashing = self.make_attester_slashing(validator_indices); + if let ObservationOutcome::New(verified_attester_slashing) = self + .chain + .verify_attester_slashing_for_gossip(attester_slashing) + .expect("should verify attester slashing for gossip") + { + self.chain + .import_attester_slashing(verified_attester_slashing); + Ok(()) + } else { + Err("should observe new attester slashing".to_string()) + } + } + pub fn add_bls_to_execution_change( &self, validator_index: u64, diff --git a/beacon_node/beacon_chain/tests/capella.rs b/beacon_node/beacon_chain/tests/capella.rs index e910e8134f..f0b799ec9f 100644 --- a/beacon_node/beacon_chain/tests/capella.rs +++ b/beacon_node/beacon_chain/tests/capella.rs @@ -133,13 +133,8 @@ async fn base_altair_merge_capella() { for _ in (merge_fork_slot.as_u64() + 3)..capella_fork_slot.as_u64() { harness.extend_slots(1).await; let block = &harness.chain.head_snapshot().beacon_block; - let full_payload: FullPayload = block - .message() - .body() - .execution_payload() - .unwrap() - .clone() - .into(); + let full_payload: FullPayload = + block.message().body().execution_payload().unwrap().into(); // pre-capella shouldn't have withdrawals assert!(full_payload.withdrawals_root().is_err()); execution_payloads.push(full_payload); @@ -151,13 +146,8 @@ async fn base_altair_merge_capella() { for _ in 0..16 { harness.extend_slots(1).await; let block = &harness.chain.head_snapshot().beacon_block; - let full_payload: FullPayload = block - .message() - .body() - .execution_payload() - .unwrap() - .clone() - .into(); + let full_payload: FullPayload = + block.message().body().execution_payload().unwrap().into(); // post-capella should have withdrawals assert!(full_payload.withdrawals_root().is_ok()); execution_payloads.push(full_payload); diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index 018defd2f0..9a8c324d09 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -1064,8 +1064,9 @@ async fn invalid_parent() { Duration::from_secs(0), &state, PayloadVerificationStatus::Optimistic, + rig.harness.chain.config.progressive_balances_mode, &rig.harness.chain.spec, - + rig.harness.logger() ), Err(ForkChoiceError::ProtoArrayStringError(message)) if message.contains(&format!( diff --git a/beacon_node/http_api/src/block_rewards.rs b/beacon_node/http_api/src/block_rewards.rs index 828be8e576..299bc019c4 100644 --- a/beacon_node/http_api/src/block_rewards.rs +++ b/beacon_node/http_api/src/block_rewards.rs @@ -49,7 +49,7 @@ pub fn get_block_rewards( .map_err(beacon_chain_error)?; state - .build_all_caches(&chain.spec) + .build_caches(&chain.spec) .map_err(beacon_state_error)?; let mut reward_cache = Default::default(); diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 206cd3c72f..646356b6cb 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -1,5 +1,6 @@ use clap::{App, Arg}; use strum::VariantNames; +use types::ProgressiveBalancesMode; pub fn cli_app<'a, 'b>() -> App<'a, 'b> { App::new("beacon_node") @@ -1117,4 +1118,17 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { developers. This directory is not pruned, users should be careful to avoid \ filling up their disks.") ) + .arg( + Arg::with_name("progressive-balances") + .long("progressive-balances") + .value_name("MODE") + .help("Options to enable or disable the progressive balances cache for \ + unrealized FFG progression calculation. The default `checked` mode compares \ + the progressive balances from the cache against results from the existing \ + method. If there is a mismatch, it falls back to the existing method. The \ + optimized mode (`fast`) is faster but is still experimental, and is \ + not recommended for mainnet usage at this time.") + .takes_value(true) + .possible_values(ProgressiveBalancesMode::VARIANTS) + ) } diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index c59b297c1b..948c70dd41 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -800,6 +800,12 @@ pub fn get_config( client_config.network.invalid_block_storage = Some(path); } + if let Some(progressive_balances_mode) = + clap_utils::parse_optional(cli_args, "progressive-balances")? + { + client_config.chain.progressive_balances_mode = progressive_balances_mode; + } + Ok(client_config) } diff --git a/beacon_node/store/src/partial_beacon_state.rs b/beacon_node/store/src/partial_beacon_state.rs index cd923da40d..9f2532d0a7 100644 --- a/beacon_node/store/src/partial_beacon_state.rs +++ b/beacon_node/store/src/partial_beacon_state.rs @@ -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(), diff --git a/beacon_node/store/src/reconstruct.rs b/beacon_node/store/src/reconstruct.rs index cd50babdb0..bac5d3cc82 100644 --- a/beacon_node/store/src/reconstruct.rs +++ b/beacon_node/store/src/reconstruct.rs @@ -63,7 +63,7 @@ where .load_cold_state_by_slot(lower_limit_slot)? .ok_or(HotColdDBError::MissingLowerLimitState(lower_limit_slot))?; - state.build_all_caches(&self.spec)?; + state.build_caches(&self.spec)?; process_results(block_root_iter, |iter| -> Result<(), Error> { let mut io_batch = vec![]; diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 5d86f99f1a..e60774fc86 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1,10 +1,15 @@ use crate::{ForkChoiceStore, InvalidationOperation}; +use per_epoch_processing::altair::participation_cache::Error as ParticipationCacheError; use proto_array::{ Block as ProtoBlock, DisallowedReOrgOffsets, ExecutionStatus, ProposerHeadError, ProposerHeadInfo, ProtoArrayForkChoice, ReOrgThreshold, }; -use slog::{crit, debug, warn, Logger}; +use slog::{crit, debug, error, warn, Logger}; use ssz_derive::{Decode, Encode}; +use state_processing::per_epoch_processing::altair::ParticipationCache; +use state_processing::per_epoch_processing::{ + weigh_justification_and_finalization, JustificationAndFinalizationState, +}; use state_processing::{ per_block_processing::errors::AttesterSlashingValidationError, per_epoch_processing, }; @@ -18,6 +23,7 @@ use types::{ EthSpec, ExecPayload, ExecutionBlockHash, Hash256, IndexedAttestation, RelativeEpoch, SignedBeaconBlock, Slot, }; +use types::{ProgressiveBalancesCache, ProgressiveBalancesMode}; #[derive(Debug)] pub enum Error { @@ -72,7 +78,9 @@ pub enum Error { }, UnrealizedVoteProcessing(state_processing::EpochProcessingError), ParticipationCacheBuild(BeaconStateError), + ParticipationCacheError(ParticipationCacheError), ValidatorStatuses(BeaconStateError), + ProgressiveBalancesCacheCheckFailed(String), } impl From for Error { @@ -93,6 +101,18 @@ impl From for Error { } } +impl From for Error { + fn from(e: BeaconStateError) -> Self { + Error::BeaconStateError(e) + } +} + +impl From for Error { + fn from(e: ParticipationCacheError) -> Self { + Error::ParticipationCacheError(e) + } +} + #[derive(Debug, Clone, Copy)] /// Controls how fork choice should behave when restoring from a persisted fork choice. pub enum ResetPayloadStatuses { @@ -643,7 +663,9 @@ where block_delay: Duration, state: &BeaconState, payload_verification_status: PayloadVerificationStatus, + progressive_balances_mode: ProgressiveBalancesMode, spec: &ChainSpec, + log: &Logger, ) -> Result<(), Error> { // If this block has already been processed we do not need to reprocess it. // We check this immediately in case re-processing the block mutates some property of the @@ -737,43 +759,84 @@ where parent_justified.epoch == block_epoch && parent_finalized.epoch + 1 >= block_epoch }); - let (unrealized_justified_checkpoint, unrealized_finalized_checkpoint) = - if let Some((parent_justified, parent_finalized)) = parent_checkpoints { - (parent_justified, parent_finalized) - } else { - let justification_and_finalization_state = match block { - BeaconBlockRef::Capella(_) - | BeaconBlockRef::Merge(_) - | BeaconBlockRef::Altair(_) => { - let participation_cache = - per_epoch_processing::altair::ParticipationCache::new(state, spec) - .map_err(Error::ParticipationCacheBuild)?; + let (unrealized_justified_checkpoint, unrealized_finalized_checkpoint) = if let Some(( + parent_justified, + parent_finalized, + )) = + parent_checkpoints + { + (parent_justified, parent_finalized) + } else { + let justification_and_finalization_state = match block { + BeaconBlockRef::Capella(_) + | BeaconBlockRef::Merge(_) + | BeaconBlockRef::Altair(_) => match progressive_balances_mode { + ProgressiveBalancesMode::Disabled => { + let participation_cache = ParticipationCache::new(state, spec) + .map_err(Error::ParticipationCacheBuild)?; per_epoch_processing::altair::process_justification_and_finalization( state, &participation_cache, )? } - BeaconBlockRef::Base(_) => { - let mut validator_statuses = - per_epoch_processing::base::ValidatorStatuses::new(state, spec) - .map_err(Error::ValidatorStatuses)?; - validator_statuses - .process_attestations(state) - .map_err(Error::ValidatorStatuses)?; - per_epoch_processing::base::process_justification_and_finalization( - state, - &validator_statuses.total_balances, - spec, - )? - } - }; + ProgressiveBalancesMode::Fast + | ProgressiveBalancesMode::Checked + | ProgressiveBalancesMode::Strict => { + let maybe_participation_cache = progressive_balances_mode + .perform_comparative_checks() + .then(|| { + ParticipationCache::new(state, spec) + .map_err(Error::ParticipationCacheBuild) + }) + .transpose()?; - ( - justification_and_finalization_state.current_justified_checkpoint(), - justification_and_finalization_state.finalized_checkpoint(), - ) + process_justification_and_finalization_from_progressive_cache::( + state, + maybe_participation_cache.as_ref(), + ) + .or_else(|e| { + if progressive_balances_mode != ProgressiveBalancesMode::Strict { + error!( + log, + "Processing with progressive balances cache failed"; + "info" => "falling back to the non-optimized processing method", + "error" => ?e, + ); + let participation_cache = maybe_participation_cache + .map(Ok) + .unwrap_or_else(|| ParticipationCache::new(state, spec)) + .map_err(Error::ParticipationCacheBuild)?; + per_epoch_processing::altair::process_justification_and_finalization( + state, + &participation_cache, + ).map_err(Error::from) + } else { + Err(e) + } + })? + } + }, + BeaconBlockRef::Base(_) => { + let mut validator_statuses = + per_epoch_processing::base::ValidatorStatuses::new(state, spec) + .map_err(Error::ValidatorStatuses)?; + validator_statuses + .process_attestations(state) + .map_err(Error::ValidatorStatuses)?; + per_epoch_processing::base::process_justification_and_finalization( + state, + &validator_statuses.total_balances, + spec, + )? + } }; + ( + justification_and_finalization_state.current_justified_checkpoint(), + justification_and_finalization_state.finalized_checkpoint(), + ) + }; + // Update best known unrealized justified & finalized checkpoints if unrealized_justified_checkpoint.epoch > self.fc_store.unrealized_justified_checkpoint().epoch @@ -1499,6 +1562,92 @@ where } } +/// Process justification and finalization using progressive cache. Also performs a comparative +/// check against the `ParticipationCache` if it is supplied. +/// +/// Returns an error if the cache is not initialized or if there is a mismatch on the comparative check. +fn process_justification_and_finalization_from_progressive_cache( + state: &BeaconState, + maybe_participation_cache: Option<&ParticipationCache>, +) -> Result, Error> +where + E: EthSpec, + T: ForkChoiceStore, +{ + let justification_and_finalization_state = JustificationAndFinalizationState::new(state); + if state.current_epoch() <= E::genesis_epoch() + 1 { + return Ok(justification_and_finalization_state); + } + + // Load cached balances + let progressive_balances_cache: &ProgressiveBalancesCache = state.progressive_balances_cache(); + let previous_target_balance = + progressive_balances_cache.previous_epoch_target_attesting_balance()?; + let current_target_balance = + progressive_balances_cache.current_epoch_target_attesting_balance()?; + let total_active_balance = state.get_total_active_balance()?; + + if let Some(participation_cache) = maybe_participation_cache { + check_progressive_balances::( + state, + participation_cache, + previous_target_balance, + current_target_balance, + total_active_balance, + )?; + } + + weigh_justification_and_finalization( + justification_and_finalization_state, + total_active_balance, + previous_target_balance, + current_target_balance, + ) + .map_err(Error::from) +} + +/// Perform comparative checks against `ParticipationCache`, will return error if there's a mismatch. +fn check_progressive_balances( + state: &BeaconState, + participation_cache: &ParticipationCache, + cached_previous_target_balance: u64, + cached_current_target_balance: u64, + cached_total_active_balance: u64, +) -> Result<(), Error> +where + E: EthSpec, + T: ForkChoiceStore, +{ + let slot = state.slot(); + let epoch = state.current_epoch(); + + // Check previous epoch target balances + let previous_target_balance = participation_cache.previous_epoch_target_attesting_balance()?; + if previous_target_balance != cached_previous_target_balance { + return Err(Error::ProgressiveBalancesCacheCheckFailed( + format!("Previous epoch target attesting balance mismatch, slot: {}, epoch: {}, actual: {}, cached: {}", slot, epoch, previous_target_balance, cached_previous_target_balance) + )); + } + + // Check current epoch target balances + let current_target_balance = participation_cache.current_epoch_target_attesting_balance()?; + if current_target_balance != cached_current_target_balance { + return Err(Error::ProgressiveBalancesCacheCheckFailed( + format!("Current epoch target attesting balance mismatch, slot: {}, epoch: {}, actual: {}, cached: {}", slot, epoch, current_target_balance, cached_current_target_balance) + )); + } + + // Check current epoch total balances + let total_active_balance = participation_cache.current_epoch_total_active_balance(); + if total_active_balance != cached_total_active_balance { + return Err(Error::ProgressiveBalancesCacheCheckFailed( + format!("Current epoch total active balance mismatch, slot: {}, epoch: {}, actual: {}, cached: {}", slot, epoch, total_active_balance, cached_total_active_balance) + )); + } + + Ok(()) +} + /// 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. diff --git a/consensus/fork_choice/tests/tests.rs b/consensus/fork_choice/tests/tests.rs index ef262b58c0..d28210aa1b 100644 --- a/consensus/fork_choice/tests/tests.rs +++ b/consensus/fork_choice/tests/tests.rs @@ -17,12 +17,13 @@ use fork_choice::{ use store::MemoryStore; use types::{ test_utils::generate_deterministic_keypair, BeaconBlockRef, BeaconState, ChainSpec, Checkpoint, - Epoch, EthSpec, Hash256, IndexedAttestation, MainnetEthSpec, SignedBeaconBlock, Slot, SubnetId, + Epoch, EthSpec, ForkName, Hash256, IndexedAttestation, MainnetEthSpec, ProgressiveBalancesMode, + RelativeEpoch, SignedBeaconBlock, Slot, SubnetId, }; pub type E = MainnetEthSpec; -pub const VALIDATOR_COUNT: usize = 32; +pub const VALIDATOR_COUNT: usize = 64; /// Defines some delay between when an attestation is created and when it is mutated. pub enum MutationDelay { @@ -68,6 +69,24 @@ impl ForkChoiceTest { Self { harness } } + /// Creates a new tester with the specified `ProgressiveBalancesMode` and genesis from latest fork. + fn new_with_progressive_balances_mode(mode: ProgressiveBalancesMode) -> ForkChoiceTest { + // genesis with latest fork (at least altair required to test the cache) + let spec = ForkName::latest().make_genesis_spec(ChainSpec::default()); + let harness = BeaconChainHarness::builder(MainnetEthSpec) + .spec(spec) + .chain_config(ChainConfig { + progressive_balances_mode: mode, + ..ChainConfig::default() + }) + .deterministic_keypairs(VALIDATOR_COUNT) + .fresh_ephemeral_store() + .mock_execution_layer() + .build(); + + Self { harness } + } + /// Get a value from the `ForkChoice` instantiation. fn get(&self, func: T) -> U where @@ -212,6 +231,39 @@ impl ForkChoiceTest { self } + /// Slash a validator from the previous epoch committee. + pub async fn add_previous_epoch_attester_slashing(self) -> Self { + let state = self.harness.get_current_state(); + let previous_epoch_shuffling = state.get_shuffling(RelativeEpoch::Previous).unwrap(); + let validator_indices = previous_epoch_shuffling + .iter() + .map(|idx| *idx as u64) + .take(1) + .collect(); + + self.harness + .add_attester_slashing(validator_indices) + .unwrap(); + + self + } + + /// Slash the proposer of a block in the previous epoch. + pub async fn add_previous_epoch_proposer_slashing(self, slots_per_epoch: u64) -> Self { + let previous_epoch_slot = self.harness.get_current_slot() - slots_per_epoch; + let previous_epoch_block = self + .harness + .chain + .block_at_slot(previous_epoch_slot, WhenSlotSkipped::None) + .unwrap() + .unwrap(); + let proposer_index: u64 = previous_epoch_block.message().proposer_index(); + + self.harness.add_proposer_slashing(proposer_index).unwrap(); + + self + } + /// Apply `count` blocks to the chain (without attestations). pub async fn apply_blocks_without_new_attestations(self, count: usize) -> Self { self.harness.advance_slot(); @@ -286,7 +338,9 @@ impl ForkChoiceTest { Duration::from_secs(0), &state, PayloadVerificationStatus::Verified, + self.harness.chain.config.progressive_balances_mode, &self.harness.chain.spec, + self.harness.logger(), ) .unwrap(); self @@ -328,7 +382,9 @@ impl ForkChoiceTest { Duration::from_secs(0), &state, PayloadVerificationStatus::Verified, + self.harness.chain.config.progressive_balances_mode, &self.harness.chain.spec, + self.harness.logger(), ) .err() .expect("on_block did not return an error"); @@ -1287,3 +1343,49 @@ async fn weak_subjectivity_check_epoch_boundary_is_skip_slot_failure() { .assert_finalized_epoch_is_less_than(checkpoint.epoch) .assert_shutdown_signal_sent(); } + +/// Checks that `ProgressiveBalancesCache` is updated correctly after an attester slashing event, +/// where the slashed validator is a target attester in previous / current epoch. +#[tokio::test] +async fn progressive_balances_cache_attester_slashing() { + ForkChoiceTest::new_with_progressive_balances_mode(ProgressiveBalancesMode::Strict) + // first two epochs + .apply_blocks_while(|_, state| state.finalized_checkpoint().epoch == 0) + .await + .unwrap() + .add_previous_epoch_attester_slashing() + .await + // expect fork choice to import blocks successfully after a previous epoch attester is + // slashed, i.e. the slashed attester's balance is correctly excluded from + // the previous epoch total balance in `ProgressiveBalancesCache`. + .apply_blocks(1) + .await + // expect fork choice to import another epoch of blocks successfully - the slashed + // attester's balance should be excluded from the current epoch total balance in + // `ProgressiveBalancesCache` as well. + .apply_blocks(MainnetEthSpec::slots_per_epoch() as usize) + .await; +} + +/// Checks that `ProgressiveBalancesCache` is updated correctly after a proposer slashing event, +/// where the slashed validator is a target attester in previous / current epoch. +#[tokio::test] +async fn progressive_balances_cache_proposer_slashing() { + ForkChoiceTest::new_with_progressive_balances_mode(ProgressiveBalancesMode::Strict) + // first two epochs + .apply_blocks_while(|_, state| state.finalized_checkpoint().epoch == 0) + .await + .unwrap() + .add_previous_epoch_proposer_slashing(MainnetEthSpec::slots_per_epoch()) + .await + // expect fork choice to import blocks successfully after a previous epoch proposer is + // slashed, i.e. the slashed proposer's balance is correctly excluded from + // the previous epoch total balance in `ProgressiveBalancesCache`. + .apply_blocks(1) + .await + // expect fork choice to import another epoch of blocks successfully - the slashed + // proposer's balance should be excluded from the current epoch total balance in + // `ProgressiveBalancesCache` as well. + .apply_blocks(MainnetEthSpec::slots_per_epoch() as usize) + .await; +} diff --git a/consensus/state_processing/src/common/mod.rs b/consensus/state_processing/src/common/mod.rs index 8a2e2439bb..ffe8be3a04 100644 --- a/consensus/state_processing/src/common/mod.rs +++ b/consensus/state_processing/src/common/mod.rs @@ -7,6 +7,7 @@ mod slash_validator; pub mod altair; pub mod base; +pub mod update_progressive_balances_cache; pub use deposit_data_tree::DepositDataTree; pub use get_attestation_participation::get_attestation_participation_flag_indices; diff --git a/consensus/state_processing/src/common/slash_validator.rs b/consensus/state_processing/src/common/slash_validator.rs index d4675f5ef5..d54da43a04 100644 --- a/consensus/state_processing/src/common/slash_validator.rs +++ b/consensus/state_processing/src/common/slash_validator.rs @@ -1,3 +1,4 @@ +use crate::common::update_progressive_balances_cache::update_progressive_balances_on_slashing; use crate::{ common::{decrease_balance, increase_balance, initiate_validator_exit}, per_block_processing::errors::BlockProcessingError, @@ -43,6 +44,8 @@ pub fn slash_validator( .safe_div(spec.min_slashing_penalty_quotient_for_state(state))?, )?; + update_progressive_balances_on_slashing(state, slashed_index)?; + // Apply proposer and whistleblower rewards let proposer_index = ctxt.get_proposer_index(state, spec)? as usize; let whistleblower_index = opt_whistleblower_index.unwrap_or(proposer_index); diff --git a/consensus/state_processing/src/common/update_progressive_balances_cache.rs b/consensus/state_processing/src/common/update_progressive_balances_cache.rs new file mode 100644 index 0000000000..45b5d657a6 --- /dev/null +++ b/consensus/state_processing/src/common/update_progressive_balances_cache.rs @@ -0,0 +1,142 @@ +/// A collection of all functions that mutates the `ProgressiveBalancesCache`. +use crate::metrics::{ + PARTICIPATION_CURR_EPOCH_TARGET_ATTESTING_GWEI_PROGRESSIVE_TOTAL, + PARTICIPATION_PREV_EPOCH_TARGET_ATTESTING_GWEI_PROGRESSIVE_TOTAL, +}; +use crate::per_epoch_processing::altair::ParticipationCache; +use crate::{BlockProcessingError, EpochProcessingError}; +use lighthouse_metrics::set_gauge; +use ssz_types::VariableList; +use std::borrow::Cow; +use types::consts::altair::TIMELY_TARGET_FLAG_INDEX; +use types::{ + is_progressive_balances_enabled, BeaconState, BeaconStateError, ChainSpec, Epoch, EthSpec, + ParticipationFlags, ProgressiveBalancesCache, +}; + +/// 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( + state: &mut BeaconState, + 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)?), + }; + + let previous_epoch_target_attesting_balance = participation_cache + .previous_epoch_target_attesting_balance_raw() + .map_err(|e| BeaconStateError::ParticipationCacheError(format!("{e:?}")))?; + + let current_epoch_target_attesting_balance = participation_cache + .current_epoch_target_attesting_balance_raw() + .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, + ); + + update_progressive_balances_metrics(state.progressive_balances_cache())?; + + Ok(()) +} + +/// Updates the `ProgressiveBalancesCache` when a new target attestation has been processed. +pub fn update_progressive_balances_on_attestation( + state: &mut BeaconState, + epoch: Epoch, + validator_index: usize, +) -> Result<(), BlockProcessingError> { + if is_progressive_balances_enabled(state) { + let validator = state.get_validator(validator_index)?; + if !validator.slashed { + let validator_effective_balance = validator.effective_balance; + state + .progressive_balances_cache_mut() + .on_new_target_attestation(epoch, validator_effective_balance)?; + } + } + Ok(()) +} + +/// Updates the `ProgressiveBalancesCache` when a target attester has been slashed. +pub fn update_progressive_balances_on_slashing( + state: &mut BeaconState, + validator_index: usize, +) -> Result<(), BlockProcessingError> { + if is_progressive_balances_enabled(state) { + let previous_epoch_participation = state.previous_epoch_participation()?; + let is_previous_epoch_target_attester = + is_target_attester_in_epoch::(previous_epoch_participation, validator_index)?; + + let current_epoch_participation = state.current_epoch_participation()?; + let is_current_epoch_target_attester = + is_target_attester_in_epoch::(current_epoch_participation, validator_index)?; + + let validator_effective_balance = state.get_effective_balance(validator_index)?; + + state.progressive_balances_cache_mut().on_slashing( + is_previous_epoch_target_attester, + is_current_epoch_target_attester, + validator_effective_balance, + )?; + } + + Ok(()) +} + +/// Updates the `ProgressiveBalancesCache` on epoch transition. +pub fn update_progressive_balances_on_epoch_transition( + state: &mut BeaconState, + spec: &ChainSpec, +) -> Result<(), EpochProcessingError> { + if is_progressive_balances_enabled(state) { + state + .progressive_balances_cache_mut() + .on_epoch_transition(spec)?; + + update_progressive_balances_metrics(state.progressive_balances_cache())?; + } + + Ok(()) +} + +pub fn update_progressive_balances_metrics( + cache: &ProgressiveBalancesCache, +) -> Result<(), BeaconStateError> { + set_gauge( + &PARTICIPATION_PREV_EPOCH_TARGET_ATTESTING_GWEI_PROGRESSIVE_TOTAL, + cache.previous_epoch_target_attesting_balance()? as i64, + ); + + set_gauge( + &PARTICIPATION_CURR_EPOCH_TARGET_ATTESTING_GWEI_PROGRESSIVE_TOTAL, + cache.current_epoch_target_attesting_balance()? as i64, + ); + + Ok(()) +} + +fn is_target_attester_in_epoch( + epoch_participation: &VariableList, + validator_index: usize, +) -> Result { + let participation_flags = epoch_participation + .get(validator_index) + .ok_or(BeaconStateError::UnknownValidator(validator_index))?; + participation_flags + .has_flag(TIMELY_TARGET_FLAG_INDEX) + .map_err(|e| e.into()) +} diff --git a/consensus/state_processing/src/genesis.rs b/consensus/state_processing/src/genesis.rs index 68f04b554e..ebbc8f9f31 100644 --- a/consensus/state_processing/src/genesis.rs +++ b/consensus/state_processing/src/genesis.rs @@ -92,7 +92,7 @@ pub fn initialize_beacon_state_from_eth1( } // Now that we have our validators, initialize the caches (including the committees) - state.build_all_caches(spec)?; + state.build_caches(spec)?; // Set genesis validators root for domain separation and chain versioning *state.genesis_validators_root_mut() = state.update_validators_tree_hash_cache()?; @@ -115,7 +115,7 @@ pub fn process_activations( state: &mut BeaconState, 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) diff --git a/consensus/state_processing/src/metrics.rs b/consensus/state_processing/src/metrics.rs index ddfaae5640..360b007678 100644 --- a/consensus/state_processing/src/metrics.rs +++ b/consensus/state_processing/src/metrics.rs @@ -23,4 +23,15 @@ lazy_static! { "beacon_participation_prev_epoch_active_gwei_total", "Total effective balance (gwei) of validators active in the previous epoch" ); + /* + * Participation Metrics (progressive balances) + */ + pub static ref PARTICIPATION_PREV_EPOCH_TARGET_ATTESTING_GWEI_PROGRESSIVE_TOTAL: Result = try_create_int_gauge( + "beacon_participation_prev_epoch_target_attesting_gwei_progressive_total", + "Progressive total effective balance (gwei) of validators who attested to the target in the previous epoch" + ); + pub static ref PARTICIPATION_CURR_EPOCH_TARGET_ATTESTING_GWEI_PROGRESSIVE_TOTAL: Result = try_create_int_gauge( + "beacon_participation_curr_epoch_target_attesting_gwei_progressive_total", + "Progressive total effective balance (gwei) of validators who attested to the target in the current epoch" + ); } diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index 124fdf6500..b8b76a499d 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -41,6 +41,9 @@ mod verify_proposer_slashing; use crate::common::decrease_balance; use crate::StateProcessingStrategy; +use crate::common::update_progressive_balances_cache::{ + initialize_progressive_balances_cache, update_progressive_balances_metrics, +}; #[cfg(feature = "arbitrary-fuzz")] use arbitrary::Arbitrary; @@ -114,6 +117,8 @@ pub fn per_block_processing>( .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. @@ -182,6 +187,10 @@ pub fn per_block_processing>( )?; } + if is_progressive_balances_enabled(state) { + update_progressive_balances_metrics(state.progressive_balances_cache())?; + } + Ok(()) } diff --git a/consensus/state_processing/src/per_block_processing/errors.rs b/consensus/state_processing/src/per_block_processing/errors.rs index 1aaf298d69..0aba1d83fa 100644 --- a/consensus/state_processing/src/per_block_processing/errors.rs +++ b/consensus/state_processing/src/per_block_processing/errors.rs @@ -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::*; @@ -83,6 +85,7 @@ pub enum BlockProcessingError { found: Hash256, }, WithdrawalCredentialsInvalid, + ParticipationCacheError(ParticipationCacheError), } impl From for BlockProcessingError { @@ -140,6 +143,12 @@ impl From> for BlockProcessingError { } } +impl From 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 diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index 4bee596615..1dbcb7fb8f 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -97,6 +97,8 @@ pub mod base { pub mod altair { use super::*; + use crate::common::update_progressive_balances_cache::update_progressive_balances_on_attestation; + use types::consts::altair::TIMELY_TARGET_FLAG_INDEX; pub fn process_attestations( state: &mut BeaconState, @@ -163,6 +165,14 @@ pub mod altair { get_base_reward(state, index, base_reward_per_increment, spec)? .safe_mul(weight)?, )?; + + if flag_index == TIMELY_TARGET_FLAG_INDEX { + update_progressive_balances_on_attestation( + state, + data.target.epoch, + index, + )?; + } } } } @@ -235,6 +245,7 @@ pub fn process_attester_slashings( Ok(()) } + /// Wrapper function to handle calling the correct version of `process_attestations` based on /// the fork. pub fn process_attestations>( diff --git a/consensus/state_processing/src/per_epoch_processing/altair.rs b/consensus/state_processing/src/per_epoch_processing/altair.rs index d5df2fc975..0abbd16a98 100644 --- a/consensus/state_processing/src/per_epoch_processing/altair.rs +++ b/consensus/state_processing/src/per_epoch_processing/altair.rs @@ -1,4 +1,7 @@ use super::{process_registry_updates, process_slashings, EpochProcessingSummary, Error}; +use crate::common::update_progressive_balances_cache::{ + initialize_progressive_balances_cache, update_progressive_balances_on_epoch_transition, +}; use crate::per_epoch_processing::{ effective_balance_updates::process_effective_balance_updates, historical_roots_update::process_historical_roots_update, @@ -31,6 +34,7 @@ pub fn process_epoch( // 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::(state, Some(&participation_cache), spec)?; // Justification and finalization. let justification_and_finalization_state = @@ -56,7 +60,7 @@ pub fn process_epoch( 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)?; @@ -75,6 +79,8 @@ pub fn process_epoch( // Rotate the epoch caches to suit the epoch transition. state.advance_caches(spec)?; + update_progressive_balances_on_epoch_transition(state, spec)?; + Ok(EpochProcessingSummary::Altair { participation_cache, sync_committee, 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 004726923e..a5caddd045 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,49 +11,23 @@ //! 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 safe_arith::{ArithError, SafeArith}; use types::{ consts::altair::{ NUM_FLAG_INDICES, TIMELY_HEAD_FLAG_INDEX, TIMELY_SOURCE_FLAG_INDEX, TIMELY_TARGET_FLAG_INDEX, }, - BeaconState, BeaconStateError, ChainSpec, Epoch, EthSpec, ParticipationFlags, RelativeEpoch, + Balance, BeaconState, BeaconStateError, ChainSpec, Epoch, EthSpec, ParticipationFlags, + RelativeEpoch, }; -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Clone)] pub enum Error { InvalidFlagIndex(usize), InvalidValidatorIndex(usize), } -/// A balance which will never be below the specified `minimum`. -/// -/// This is an effort to ensure the `EFFECTIVE_BALANCE_INCREMENT` minimum is always respected. -#[derive(PartialEq, Debug, Clone, Copy)] -struct Balance { - raw: u64, - minimum: u64, -} - -impl Balance { - /// Initialize the balance to `0`, or the given `minimum`. - pub fn zero(minimum: u64) -> Self { - Self { raw: 0, minimum } - } - - /// Returns the balance with respect to the initialization `minimum`. - pub fn get(&self) -> u64 { - std::cmp::max(self.raw, self.minimum) - } - - /// Add-assign to the balance. - pub fn safe_add_assign(&mut self, other: u64) -> Result<(), ArithError> { - self.raw.safe_add_assign(other) - } -} - /// 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. /// @@ -95,6 +69,14 @@ impl SingleEpochParticipationCache { .ok_or(Error::InvalidFlagIndex(flag_index)) } + /// Returns the raw total balance of attesters who have `flag_index` set. + fn total_flag_balance_raw(&self, flag_index: usize) -> Result { + self.total_flag_balances + .get(flag_index) + .copied() + .ok_or(Error::InvalidFlagIndex(flag_index)) + } + /// Returns `true` if `val_index` is active, unslashed and has `flag_index` set. /// /// ## Errors @@ -173,7 +155,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`. @@ -291,6 +273,11 @@ impl ParticipationCache { .total_flag_balance(TIMELY_TARGET_FLAG_INDEX) } + pub fn current_epoch_target_attesting_balance_raw(&self) -> Result { + self.current_epoch_participation + .total_flag_balance_raw(TIMELY_TARGET_FLAG_INDEX) + } + pub fn previous_epoch_total_active_balance(&self) -> u64 { self.previous_epoch_participation.total_active_balance.get() } @@ -300,6 +287,11 @@ impl ParticipationCache { .total_flag_balance(TIMELY_TARGET_FLAG_INDEX) } + pub fn previous_epoch_target_attesting_balance_raw(&self) -> Result { + self.previous_epoch_participation + .total_flag_balance_raw(TIMELY_TARGET_FLAG_INDEX) + } + pub fn previous_epoch_source_attesting_balance(&self) -> Result { self.previous_epoch_participation .total_flag_balance(TIMELY_SOURCE_FLAG_INDEX) diff --git a/consensus/state_processing/src/per_epoch_processing/base.rs b/consensus/state_processing/src/per_epoch_processing/base.rs index cb7e7d4b30..680563ce74 100644 --- a/consensus/state_processing/src/per_epoch_processing/base.rs +++ b/consensus/state_processing/src/per_epoch_processing/base.rs @@ -52,7 +52,7 @@ pub fn process_epoch( 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)?; diff --git a/consensus/state_processing/src/per_epoch_processing/capella.rs b/consensus/state_processing/src/per_epoch_processing/capella.rs index aaf301f29e..911510ed0c 100644 --- a/consensus/state_processing/src/per_epoch_processing/capella.rs +++ b/consensus/state_processing/src/per_epoch_processing/capella.rs @@ -11,6 +11,9 @@ use crate::per_epoch_processing::{ }; use types::{BeaconState, ChainSpec, EthSpec, RelativeEpoch}; +use crate::common::update_progressive_balances_cache::{ + initialize_progressive_balances_cache, update_progressive_balances_on_epoch_transition, +}; pub use historical_summaries_update::process_historical_summaries_update; mod historical_summaries_update; @@ -27,6 +30,7 @@ pub fn process_epoch( // 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(state, Some(&participation_cache), spec)?; // Justification and finalization. let justification_and_finalization_state = @@ -52,7 +56,7 @@ pub fn process_epoch( 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)?; @@ -71,6 +75,8 @@ pub fn process_epoch( // Rotate the epoch caches to suit the epoch transition. state.advance_caches(spec)?; + update_progressive_balances_on_epoch_transition(state, spec)?; + Ok(EpochProcessingSummary::Altair { participation_cache, sync_committee, 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 c166667b5a..1759f7e140 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 @@ -1,11 +1,13 @@ use super::errors::EpochProcessingError; +use crate::per_epoch_processing::altair::ParticipationCache; use safe_arith::SafeArith; use types::beacon_state::BeaconState; use types::chain_spec::ChainSpec; -use types::{BeaconStateError, EthSpec}; +use types::{BeaconStateError, EthSpec, ProgressiveBalancesCache}; pub fn process_effective_balance_updates( state: &mut BeaconState, + maybe_participation_cache: Option<&ParticipationCache>, spec: &ChainSpec, ) -> Result<(), EpochProcessingError> { let hysteresis_increment = spec @@ -13,7 +15,8 @@ pub fn process_effective_balance_updates( .safe_div(spec.hysteresis_quotient)?; let downward_threshold = hysteresis_increment.safe_mul(spec.hysteresis_downward_multiplier)?; let upward_threshold = hysteresis_increment.safe_mul(spec.hysteresis_upward_multiplier)?; - let (validators, balances) = state.validators_and_balances_mut(); + let (validators, balances, progressive_balances_cache) = + state.validators_and_balances_and_progressive_balances_mut(); for (index, validator) in validators.iter_mut().enumerate() { let balance = balances .get(index) @@ -23,11 +26,43 @@ pub fn process_effective_balance_updates( if balance.safe_add(downward_threshold)? < validator.effective_balance || validator.effective_balance.safe_add(upward_threshold)? < balance { - validator.effective_balance = std::cmp::min( + let old_effective_balance = validator.effective_balance; + let new_effective_balance = std::cmp::min( balance.safe_sub(balance.safe_rem(spec.effective_balance_increment)?)?, spec.max_effective_balance, ); + + if let Some(participation_cache) = maybe_participation_cache { + update_progressive_balances( + participation_cache, + progressive_balances_cache, + index, + old_effective_balance, + new_effective_balance, + )?; + } + + validator.effective_balance = new_effective_balance; } } Ok(()) } + +fn update_progressive_balances( + participation_cache: &ParticipationCache, + progressive_balances_cache: &mut ProgressiveBalancesCache, + index: usize, + old_effective_balance: u64, + new_effective_balance: u64, +) -> Result<(), EpochProcessingError> { + if old_effective_balance != new_effective_balance { + let is_current_epoch_target_attester = + participation_cache.is_current_epoch_timely_target_attester(index)?; + progressive_balances_cache.on_effective_balance_change( + is_current_epoch_target_attester, + old_effective_balance, + new_effective_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 6d5342cd36..2d595491c1 100644 --- a/consensus/state_processing/src/per_epoch_processing/slashings.rs +++ b/consensus/state_processing/src/per_epoch_processing/slashings.rs @@ -16,7 +16,7 @@ pub fn process_slashings( total_balance, ); - 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().enumerate() { if validator.slashed && epoch.safe_add(T::EpochsPerSlashingsVector::to_u64().safe_div(2)?)? diff --git a/consensus/state_processing/src/upgrade/altair.rs b/consensus/state_processing/src/upgrade/altair.rs index 176f1af15c..26b1192bc1 100644 --- a/consensus/state_processing/src/upgrade/altair.rs +++ b/consensus/state_processing/src/upgrade/altair.rs @@ -1,3 +1,4 @@ +use crate::common::update_progressive_balances_cache::initialize_progressive_balances_cache; use crate::common::{get_attestation_participation_flag_indices, get_attesting_indices}; use std::mem; use std::sync::Arc; @@ -101,6 +102,7 @@ pub fn upgrade_to_altair( next_sync_committee: temp_sync_committee, // not read // Caches total_active_balance: pre.total_active_balance, + progressive_balances_cache: mem::take(&mut pre.progressive_balances_cache), committee_caches: mem::take(&mut pre.committee_caches), pubkey_cache: mem::take(&mut pre.pubkey_cache), exit_cache: mem::take(&mut pre.exit_cache), @@ -110,6 +112,8 @@ pub fn upgrade_to_altair( // Fill in previous epoch participation from the pre state's pending attestations. translate_participation(&mut post, &pre.previous_epoch_attestations, spec)?; + initialize_progressive_balances_cache(&mut post, None, spec)?; + // Fill in sync committees // Note: A duplicate committee is assigned for the current and next committee at the fork // boundary diff --git a/consensus/state_processing/src/upgrade/capella.rs b/consensus/state_processing/src/upgrade/capella.rs index 3b933fac37..5153e35f44 100644 --- a/consensus/state_processing/src/upgrade/capella.rs +++ b/consensus/state_processing/src/upgrade/capella.rs @@ -62,6 +62,7 @@ pub fn upgrade_to_capella( historical_summaries: VariableList::default(), // Caches total_active_balance: pre.total_active_balance, + progressive_balances_cache: mem::take(&mut pre.progressive_balances_cache), committee_caches: mem::take(&mut pre.committee_caches), pubkey_cache: mem::take(&mut pre.pubkey_cache), exit_cache: mem::take(&mut pre.exit_cache), diff --git a/consensus/state_processing/src/upgrade/merge.rs b/consensus/state_processing/src/upgrade/merge.rs index c172466248..eb74450107 100644 --- a/consensus/state_processing/src/upgrade/merge.rs +++ b/consensus/state_processing/src/upgrade/merge.rs @@ -60,6 +60,7 @@ pub fn upgrade_to_bellatrix( latest_execution_payload_header: >::default(), // Caches total_active_balance: pre.total_active_balance, + progressive_balances_cache: mem::take(&mut pre.progressive_balances_cache), committee_caches: mem::take(&mut pre.committee_caches), pubkey_cache: mem::take(&mut pre.pubkey_cache), exit_cache: mem::take(&mut pre.exit_cache), diff --git a/consensus/types/Cargo.toml b/consensus/types/Cargo.toml index 583b940d5f..ba15f6d488 100644 --- a/consensus/types/Cargo.toml +++ b/consensus/types/Cargo.toml @@ -52,6 +52,7 @@ serde_json = "1.0.74" smallvec = "1.8.0" serde_with = "1.13.0" maplit = "1.0.2" +strum = { version = "0.24.0", features = ["derive"] } [dev-dependencies] criterion = "0.3.3" diff --git a/consensus/types/benches/benches.rs b/consensus/types/benches/benches.rs index 28f57e7080..bb2b527109 100644 --- a/consensus/types/benches/benches.rs +++ b/consensus/types/benches/benches.rs @@ -51,7 +51,7 @@ fn all_benches(c: &mut Criterion) { let spec = Arc::new(MainnetEthSpec::default_spec()); let mut state = get_state::(validator_count); - state.build_all_caches(&spec).expect("should build caches"); + state.build_caches(&spec).expect("should build caches"); let state_bytes = state.as_ssz_bytes(); let inner_state = state.clone(); diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index 4a9da36404..1fa4dee3a0 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -26,6 +26,8 @@ pub use self::committee_cache::{ compute_committee_index_in_epoch, compute_committee_range_in_epoch, epoch_committee_count, CommitteeCache, }; +pub use crate::beacon_state::balance::Balance; +pub use crate::beacon_state::progressive_balances_cache::*; use crate::historical_summary::HistoricalSummary; pub use clone_config::CloneConfig; pub use eth_spec::*; @@ -34,9 +36,11 @@ pub use tree_hash_cache::BeaconTreeHashCache; #[macro_use] mod committee_cache; +mod balance; mod clone_config; mod exit_cache; mod iter; +mod progressive_balances_cache; mod pubkey_cache; mod tests; mod tree_hash_cache; @@ -101,6 +105,9 @@ pub enum Error { SszTypesError(ssz_types::Error), TreeHashCacheNotInitialized, NonLinearTreeHashCacheHistory, + ParticipationCacheError(String), + ProgressiveBalancesCacheNotInitialized, + ProgressiveBalancesCacheInconsistent, TreeHashCacheSkippedSlot { cache: Slot, state: Slot, @@ -317,6 +324,12 @@ where #[tree_hash(skip_hashing)] #[test_random(default)] #[derivative(Clone(clone_with = "clone_default"))] + pub progressive_balances_cache: ProgressiveBalancesCache, + #[serde(skip_serializing, skip_deserializing)] + #[ssz(skip_serializing, skip_deserializing)] + #[tree_hash(skip_hashing)] + #[test_random(default)] + #[derivative(Clone(clone_with = "clone_default"))] pub committee_caches: [CommitteeCache; CACHED_EPOCHS], #[serde(skip_serializing, skip_deserializing)] #[ssz(skip_serializing, skip_deserializing)] @@ -393,6 +406,7 @@ impl BeaconState { // Caching (not in spec) total_active_balance: None, + progressive_balances_cache: <_>::default(), committee_caches: [ CommitteeCache::default(), CommitteeCache::default(), @@ -757,7 +771,7 @@ impl BeaconState { Ok(signature_hash_int.safe_rem(modulo)? == 0) } - /// Returns the beacon proposer index for the `slot` in the given `relative_epoch`. + /// Returns the beacon proposer index for the `slot` in `self.current_epoch()`. /// /// Spec v0.12.1 pub fn get_beacon_proposer_index(&self, slot: Slot, spec: &ChainSpec) -> Result { @@ -1150,12 +1164,30 @@ impl BeaconState { } /// Convenience accessor for validators and balances simultaneously. - pub fn validators_and_balances_mut(&mut self) -> (&mut [Validator], &mut [u64]) { + pub fn validators_and_balances_and_progressive_balances_mut( + &mut self, + ) -> (&mut [Validator], &mut [u64], &mut ProgressiveBalancesCache) { match self { - BeaconState::Base(state) => (&mut state.validators, &mut state.balances), - BeaconState::Altair(state) => (&mut state.validators, &mut state.balances), - BeaconState::Merge(state) => (&mut state.validators, &mut state.balances), - BeaconState::Capella(state) => (&mut state.validators, &mut state.balances), + BeaconState::Base(state) => ( + &mut state.validators, + &mut state.balances, + &mut state.progressive_balances_cache, + ), + BeaconState::Altair(state) => ( + &mut state.validators, + &mut state.balances, + &mut state.progressive_balances_cache, + ), + BeaconState::Merge(state) => ( + &mut state.validators, + &mut state.balances, + &mut state.progressive_balances_cache, + ), + BeaconState::Capella(state) => ( + &mut state.validators, + &mut state.balances, + &mut state.progressive_balances_cache, + ), } } @@ -1380,7 +1412,7 @@ impl BeaconState { } /// Build all caches (except the tree hash cache), if they need to be built. - pub fn build_all_caches(&mut self, spec: &ChainSpec) -> Result<(), Error> { + pub fn build_caches(&mut self, spec: &ChainSpec) -> Result<(), Error> { self.build_all_committee_caches(spec)?; self.update_pubkey_cache()?; self.build_exit_cache(spec)?; @@ -1412,6 +1444,7 @@ impl BeaconState { self.drop_committee_cache(RelativeEpoch::Next)?; self.drop_pubkey_cache(); self.drop_tree_hash_cache(); + self.drop_progressive_balances_cache(); *self.exit_cache_mut() = ExitCache::default(); Ok(()) } @@ -1608,6 +1641,11 @@ impl BeaconState { *self.pubkey_cache_mut() = PubkeyCache::default() } + /// Completely drops the `progressive_balances_cache` cache, replacing it with a new, empty cache. + fn drop_progressive_balances_cache(&mut self) { + *self.progressive_balances_cache_mut() = ProgressiveBalancesCache::default(); + } + /// Initialize but don't fill the tree hash cache, if it isn't already initialized. pub fn initialize_tree_hash_cache(&mut self) { if !self.tree_hash_cache().is_initialized() { @@ -1679,6 +1717,9 @@ impl BeaconState { if config.tree_hash_cache { *res.tree_hash_cache_mut() = self.tree_hash_cache().clone(); } + if config.progressive_balances_cache { + *res.progressive_balances_cache_mut() = self.progressive_balances_cache().clone(); + } res } diff --git a/consensus/types/src/beacon_state/balance.rs b/consensus/types/src/beacon_state/balance.rs new file mode 100644 index 0000000000..e537a5b984 --- /dev/null +++ b/consensus/types/src/beacon_state/balance.rs @@ -0,0 +1,33 @@ +use arbitrary::Arbitrary; +use safe_arith::{ArithError, SafeArith}; + +/// A balance which will never be below the specified `minimum`. +/// +/// This is an effort to ensure the `EFFECTIVE_BALANCE_INCREMENT` minimum is always respected. +#[derive(PartialEq, Debug, Clone, Copy, Arbitrary)] +pub struct Balance { + raw: u64, + minimum: u64, +} + +impl Balance { + /// Initialize the balance to `0`, or the given `minimum`. + pub fn zero(minimum: u64) -> Self { + Self { raw: 0, minimum } + } + + /// Returns the balance with respect to the initialization `minimum`. + pub fn get(&self) -> u64 { + std::cmp::max(self.raw, self.minimum) + } + + /// Add-assign to the balance. + pub fn safe_add_assign(&mut self, other: u64) -> Result<(), ArithError> { + self.raw.safe_add_assign(other) + } + + /// Sub-assign to the balance. + pub fn safe_sub_assign(&mut self, other: u64) -> Result<(), ArithError> { + self.raw.safe_sub_assign(other) + } +} diff --git a/consensus/types/src/beacon_state/clone_config.rs b/consensus/types/src/beacon_state/clone_config.rs index e5f050aee6..c6e7f47421 100644 --- a/consensus/types/src/beacon_state/clone_config.rs +++ b/consensus/types/src/beacon_state/clone_config.rs @@ -5,6 +5,7 @@ pub struct CloneConfig { pub pubkey_cache: bool, pub exit_cache: bool, pub tree_hash_cache: bool, + pub progressive_balances_cache: bool, } impl CloneConfig { @@ -14,6 +15,7 @@ impl CloneConfig { pubkey_cache: true, exit_cache: true, tree_hash_cache: true, + progressive_balances_cache: true, } } diff --git a/consensus/types/src/beacon_state/progressive_balances_cache.rs b/consensus/types/src/beacon_state/progressive_balances_cache.rs new file mode 100644 index 0000000000..9f5c223d57 --- /dev/null +++ b/consensus/types/src/beacon_state/progressive_balances_cache.rs @@ -0,0 +1,184 @@ +use crate::beacon_state::balance::Balance; +use crate::{BeaconState, BeaconStateError, ChainSpec, Epoch, EthSpec}; +use arbitrary::Arbitrary; +use safe_arith::SafeArith; +use serde_derive::{Deserialize, Serialize}; +use strum::{Display, EnumString, EnumVariantNames}; + +/// This cache keeps track of the accumulated target attestation balance for the current & previous +/// epochs. The cached values can be utilised by fork choice to calculate unrealized justification +/// and finalization instead of converting epoch participation arrays to balances for each block we +/// process. +#[derive(Default, Debug, PartialEq, Arbitrary, Clone)] +pub struct ProgressiveBalancesCache { + inner: Option, +} + +#[derive(Debug, PartialEq, Arbitrary, Clone)] +struct Inner { + pub current_epoch: Epoch, + pub previous_epoch_target_attesting_balance: Balance, + pub current_epoch_target_attesting_balance: Balance, +} + +impl ProgressiveBalancesCache { + pub fn initialize( + &mut self, + current_epoch: Epoch, + previous_epoch_target_attesting_balance: Balance, + current_epoch_target_attesting_balance: Balance, + ) { + self.inner = Some(Inner { + current_epoch, + previous_epoch_target_attesting_balance, + current_epoch_target_attesting_balance, + }); + } + + pub fn is_initialized(&self) -> bool { + self.inner.is_some() + } + + /// When a new target attestation has been processed, we update the cached + /// `current_epoch_target_attesting_balance` to include the validator effective balance. + /// If the epoch is neither the current epoch nor the previous epoch, an error is returned. + pub fn on_new_target_attestation( + &mut self, + epoch: Epoch, + validator_effective_balance: u64, + ) -> Result<(), BeaconStateError> { + let cache = self.get_inner_mut()?; + + if epoch == cache.current_epoch { + cache + .current_epoch_target_attesting_balance + .safe_add_assign(validator_effective_balance)?; + } else if epoch.safe_add(1)? == cache.current_epoch { + cache + .previous_epoch_target_attesting_balance + .safe_add_assign(validator_effective_balance)?; + } else { + return Err(BeaconStateError::ProgressiveBalancesCacheInconsistent); + } + + Ok(()) + } + + /// When a validator is slashed, we reduce the `current_epoch_target_attesting_balance` by the + /// validator's effective balance to exclude the validator weight. + pub fn on_slashing( + &mut self, + is_previous_epoch_target_attester: bool, + is_current_epoch_target_attester: bool, + effective_balance: u64, + ) -> Result<(), BeaconStateError> { + let cache = self.get_inner_mut()?; + if is_previous_epoch_target_attester { + cache + .previous_epoch_target_attesting_balance + .safe_sub_assign(effective_balance)?; + } + if is_current_epoch_target_attester { + cache + .current_epoch_target_attesting_balance + .safe_sub_assign(effective_balance)?; + } + Ok(()) + } + + /// When a current epoch target attester has its effective balance changed, we adjust the + /// its share of the target attesting balance in the cache. + pub fn on_effective_balance_change( + &mut self, + is_current_epoch_target_attester: bool, + old_effective_balance: u64, + new_effective_balance: u64, + ) -> Result<(), BeaconStateError> { + let cache = self.get_inner_mut()?; + if is_current_epoch_target_attester { + if new_effective_balance > old_effective_balance { + cache + .current_epoch_target_attesting_balance + .safe_add_assign(new_effective_balance.safe_sub(old_effective_balance)?)?; + } else { + cache + .current_epoch_target_attesting_balance + .safe_sub_assign(old_effective_balance.safe_sub(new_effective_balance)?)?; + } + } + Ok(()) + } + + /// On epoch transition, the balance from current epoch is shifted to previous epoch, and the + /// current epoch balance is reset to 0. + pub fn on_epoch_transition(&mut self, spec: &ChainSpec) -> Result<(), BeaconStateError> { + let cache = self.get_inner_mut()?; + cache.current_epoch.safe_add_assign(1)?; + cache.previous_epoch_target_attesting_balance = + cache.current_epoch_target_attesting_balance; + cache.current_epoch_target_attesting_balance = + Balance::zero(spec.effective_balance_increment); + Ok(()) + } + + pub fn previous_epoch_target_attesting_balance(&self) -> Result { + Ok(self + .get_inner()? + .previous_epoch_target_attesting_balance + .get()) + } + + pub fn current_epoch_target_attesting_balance(&self) -> Result { + Ok(self + .get_inner()? + .current_epoch_target_attesting_balance + .get()) + } + + fn get_inner_mut(&mut self) -> Result<&mut Inner, BeaconStateError> { + self.inner + .as_mut() + .ok_or(BeaconStateError::ProgressiveBalancesCacheNotInitialized) + } + + fn get_inner(&self) -> Result<&Inner, BeaconStateError> { + self.inner + .as_ref() + .ok_or(BeaconStateError::ProgressiveBalancesCacheNotInitialized) + } +} + +#[derive( + Debug, PartialEq, Eq, Clone, Copy, Deserialize, Serialize, Display, EnumString, EnumVariantNames, +)] +#[strum(serialize_all = "lowercase")] +pub enum ProgressiveBalancesMode { + /// Disable the usage of progressive cache, and use the existing `ParticipationCache` calculation. + Disabled, + /// Enable the usage of progressive cache, with checks against the `ParticipationCache` and falls + /// back to the existing calculation if there is a balance mismatch. + Checked, + /// Enable the usage of progressive cache, with checks against the `ParticipationCache`. Errors + /// if there is a balance mismatch. Used in testing only. + Strict, + /// Enable the usage of progressive cache, with no comparative checks against the + /// `ParticipationCache`. This is fast but an experimental mode, use with caution. + Fast, +} + +impl ProgressiveBalancesMode { + pub fn perform_comparative_checks(&self) -> bool { + match self { + Self::Disabled | Self::Fast => false, + Self::Checked | Self::Strict => true, + } + } +} + +/// `ProgressiveBalancesCache` is only enabled from `Altair` as it requires `ParticipationCache`. +pub fn is_progressive_balances_enabled(state: &BeaconState) -> bool { + match state { + BeaconState::Base(_) => false, + BeaconState::Altair(_) | BeaconState::Merge(_) | BeaconState::Capella(_) => true, + } +} diff --git a/consensus/types/src/beacon_state/tests.rs b/consensus/types/src/beacon_state/tests.rs index d63eaafc4b..6cd9c1dbf8 100644 --- a/consensus/types/src/beacon_state/tests.rs +++ b/consensus/types/src/beacon_state/tests.rs @@ -219,17 +219,18 @@ async fn clone_config() { let mut state = build_state::(16).await; - state.build_all_caches(&spec).unwrap(); + state.build_caches(&spec).unwrap(); state .update_tree_hash_cache() .expect("should update tree hash cache"); - let num_caches = 4; + let num_caches = 5; let all_configs = (0..2u8.pow(num_caches)).map(|i| CloneConfig { committee_caches: (i & 1) != 0, pubkey_cache: ((i >> 1) & 1) != 0, exit_cache: ((i >> 2) & 1) != 0, tree_hash_cache: ((i >> 3) & 1) != 0, + progressive_balances_cache: ((i >> 4) & 1) != 0, }); for config in all_configs { diff --git a/lcli/src/new_testnet.rs b/lcli/src/new_testnet.rs index aa5f52eef8..01a44cabef 100644 --- a/lcli/src/new_testnet.rs +++ b/lcli/src/new_testnet.rs @@ -303,7 +303,7 @@ fn initialize_state_with_validators( } // Now that we have our validators, initialize the caches (including the committees) - state.build_all_caches(spec).unwrap(); + state.build_caches(spec).unwrap(); // Set genesis validators root for domain separation and chain versioning *state.genesis_validators_root_mut() = state.update_validators_tree_hash_cache().unwrap(); diff --git a/lcli/src/skip_slots.rs b/lcli/src/skip_slots.rs index 49d1dd424d..e3b2a5acbf 100644 --- a/lcli/src/skip_slots.rs +++ b/lcli/src/skip_slots.rs @@ -109,7 +109,7 @@ pub fn run(env: Environment, matches: &ArgMatches) -> Result<(), let target_slot = initial_slot + slots; state - .build_all_caches(spec) + .build_caches(spec) .map_err(|e| format!("Unable to build caches: {:?}", e))?; let state_root = if let Some(root) = cli_state_root.or(state_root) { diff --git a/lcli/src/transition_blocks.rs b/lcli/src/transition_blocks.rs index cf971c69f0..34a4560761 100644 --- a/lcli/src/transition_blocks.rs +++ b/lcli/src/transition_blocks.rs @@ -205,7 +205,7 @@ pub fn run(env: Environment, matches: &ArgMatches) -> Result<(), if config.exclude_cache_builds { pre_state - .build_all_caches(spec) + .build_caches(spec) .map_err(|e| format!("Unable to build caches: {:?}", e))?; let state_root = pre_state .update_tree_hash_cache() @@ -303,7 +303,7 @@ fn do_transition( if !config.exclude_cache_builds { let t = Instant::now(); pre_state - .build_all_caches(spec) + .build_caches(spec) .map_err(|e| format!("Unable to build caches: {:?}", e))?; debug!("Build caches: {:?}", t.elapsed()); @@ -335,7 +335,7 @@ fn do_transition( let t = Instant::now(); pre_state - .build_all_caches(spec) + .build_caches(spec) .map_err(|e| format!("Unable to build caches: {:?}", e))?; debug!("Build all caches (again): {:?}", t.elapsed()); diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index 65d7bd08b2..ac0780015f 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -16,7 +16,10 @@ use std::str::FromStr; use std::string::ToString; use std::time::Duration; use tempfile::TempDir; -use types::{Address, Checkpoint, Epoch, ExecutionBlockHash, ForkName, Hash256, MainnetEthSpec}; +use types::{ + Address, Checkpoint, Epoch, ExecutionBlockHash, ForkName, Hash256, MainnetEthSpec, + ProgressiveBalancesMode, +}; use unused_port::{unused_tcp4_port, unused_tcp6_port, unused_udp4_port, unused_udp6_port}; const DEFAULT_ETH1_ENDPOINT: &str = "http://localhost:8545/"; @@ -2284,3 +2287,28 @@ fn invalid_gossip_verified_blocks_path() { ) }); } + +#[test] +fn progressive_balances_default() { + CommandLineTest::new() + .run_with_zero_port() + .with_config(|config| { + assert_eq!( + config.chain.progressive_balances_mode, + ProgressiveBalancesMode::Checked + ) + }); +} + +#[test] +fn progressive_balances_fast() { + CommandLineTest::new() + .flag("progressive-balances", Some("fast")) + .run_with_zero_port() + .with_config(|config| { + assert_eq!( + config.chain.progressive_balances_mode, + ProgressiveBalancesMode::Fast + ) + }); +} diff --git a/testing/ef_tests/src/cases/epoch_processing.rs b/testing/ef_tests/src/cases/epoch_processing.rs index 6095e1be6b..31542ba447 100644 --- a/testing/ef_tests/src/cases/epoch_processing.rs +++ b/testing/ef_tests/src/cases/epoch_processing.rs @@ -6,9 +6,9 @@ use crate::type_name; use crate::type_name::TypeName; use serde_derive::Deserialize; use state_processing::per_epoch_processing::capella::process_historical_summaries_update; +use state_processing::per_epoch_processing::effective_balance_updates::process_effective_balance_updates; use state_processing::per_epoch_processing::{ altair, base, - effective_balance_updates::process_effective_balance_updates, historical_roots_update::process_historical_roots_update, process_registry_updates, process_slashings, resets::{process_eth1_data_reset, process_randao_mixes_reset, process_slashings_reset}, @@ -173,7 +173,7 @@ impl EpochTransition for Eth1DataReset { impl EpochTransition for EffectiveBalanceUpdates { fn run(state: &mut BeaconState, spec: &ChainSpec) -> Result<(), EpochProcessingError> { - process_effective_balance_updates(state, spec) + process_effective_balance_updates(state, None, spec) } } diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 65528de175..9627d2cde0 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -18,7 +18,8 @@ use std::sync::Arc; use std::time::Duration; use types::{ Attestation, AttesterSlashing, BeaconBlock, BeaconState, Checkpoint, EthSpec, - ExecutionBlockHash, ForkName, Hash256, IndexedAttestation, SignedBeaconBlock, Slot, Uint256, + ExecutionBlockHash, ForkName, Hash256, IndexedAttestation, ProgressiveBalancesMode, + SignedBeaconBlock, Slot, Uint256, }; #[derive(Default, Debug, PartialEq, Clone, Deserialize, Decode)] @@ -440,7 +441,9 @@ impl Tester { block_delay, &state, PayloadVerificationStatus::Irrelevant, + ProgressiveBalancesMode::Strict, &self.harness.chain.spec, + self.harness.logger(), ); if result.is_ok() { diff --git a/testing/ef_tests/src/cases/operations.rs b/testing/ef_tests/src/cases/operations.rs index 5fd00285aa..21a56dcf2a 100644 --- a/testing/ef_tests/src/cases/operations.rs +++ b/testing/ef_tests/src/cases/operations.rs @@ -4,6 +4,7 @@ use crate::case_result::compare_beacon_state_results_without_caches; use crate::decode::{ssz_decode_file, ssz_decode_file_with, ssz_decode_state, yaml_decode_file}; use crate::testing_spec; use serde_derive::Deserialize; +use state_processing::common::update_progressive_balances_cache::initialize_progressive_balances_cache; use state_processing::{ per_block_processing::{ errors::BlockProcessingError, @@ -96,6 +97,7 @@ impl Operation for Attestation { spec, ), BeaconState::Altair(_) | BeaconState::Merge(_) | BeaconState::Capella(_) => { + initialize_progressive_balances_cache(state, None, spec)?; altair::process_attestation(state, self, 0, &mut ctxt, VerifySignatures::True, spec) } } @@ -118,6 +120,7 @@ impl Operation for AttesterSlashing { _: &Operations, ) -> Result<(), BlockProcessingError> { let mut ctxt = ConsensusContext::new(state.slot()); + initialize_progressive_balances_cache(state, None, spec)?; process_attester_slashings( state, &[self.clone()], @@ -168,6 +171,7 @@ impl Operation for ProposerSlashing { _: &Operations, ) -> Result<(), BlockProcessingError> { let mut ctxt = ConsensusContext::new(state.slot()); + initialize_progressive_balances_cache(state, None, spec)?; process_proposer_slashings( state, &[self.clone()], diff --git a/testing/ef_tests/src/cases/sanity_blocks.rs b/testing/ef_tests/src/cases/sanity_blocks.rs index e51fed1907..191b45c33a 100644 --- a/testing/ef_tests/src/cases/sanity_blocks.rs +++ b/testing/ef_tests/src/cases/sanity_blocks.rs @@ -67,7 +67,7 @@ impl Case for SanityBlocks { let spec = &testing_spec::(fork_name); // Processing requires the epoch cache. - bulk_state.build_all_caches(spec).unwrap(); + bulk_state.build_caches(spec).unwrap(); // Spawning a second state to call the VerifyIndiviual strategy to avoid bitrot. // See https://github.com/sigp/lighthouse/issues/742. diff --git a/testing/ef_tests/src/cases/sanity_slots.rs b/testing/ef_tests/src/cases/sanity_slots.rs index a38a8930a0..dd385d13f4 100644 --- a/testing/ef_tests/src/cases/sanity_slots.rs +++ b/testing/ef_tests/src/cases/sanity_slots.rs @@ -61,7 +61,7 @@ impl Case for SanitySlots { let spec = &testing_spec::(fork_name); // Processing requires the epoch cache. - state.build_all_caches(spec).unwrap(); + state.build_caches(spec).unwrap(); let mut result = (0..self.slots) .try_for_each(|_| per_slot_processing(&mut state, None, spec).map(|_| ()))