From 920b35fa80da6a8f311e4a49214b3196153e74a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Mon, 2 Nov 2026 16:38:21 -0300 Subject: [PATCH 1/2] perf(beacon): resolve pubkeys through a cached index and reuse the total active balance in churn On mainnet (~2.4M validators) the block STF spent 72 ms per block in process_sync_aggregate and 29 ms in process_execution_requests. The first walked the whole registry probing a SipHash map per validator to turn 512 sync committee pubkeys into indices; the second repeated a linear pubkey search per withdrawal and consolidation request, and recomputed the total active balance (another O(n) pass) several times per request through the churn helpers. Lighthouse keeps a pubkey -> index cache on the state (append-only registry, extended incrementally) and a per-epoch total active balance. This ports both ideas: - PubkeyIndexCache in ethlambda-storage, held by Store like the active balance cache: a sorted base of u64 key prefixes (bytes 1..9 of the compressed pubkey) and u32 indices, 12 bytes per validator, plus a small hash tail merged past a threshold. The table is process-wide, so every hit is verified against the asking state's own validator at that index, and a miss falls back to the old scan for that query alone; the table is never rebuilt on a miss, so a request naming an unregistered pubkey costs one scan. - The sync aggregate resolves seats through the cache and keeps the single-pass inverted scan as the fallback. Withdrawal and consolidation requests (electra, fulu, and gloas's parent-payload requests) use it too. - The exit and consolidation churn helpers take an optional ActiveBalanceCache and use the *_for(total, config) formulas, so a block shares one registry pass. Epoch processing and spec-shaped callers pass None and keep the uncached total, since the cache key is not sound after the epoch's effective-balance update. Slashings still use the uncached total, and epoch pending-deposit processing is untouched. --- bin/ethlambda/src/era/replay.rs | 7 +- .../src/beacon/block_production.rs | 16 +- .../src/beacon/fork_choice.rs | 10 +- .../src/beacon/gloas_block_production.rs | 19 +- .../src/beacon/helpers/accessors.rs | 189 ++++++++++- .../src/beacon/helpers/electra.rs | 97 ++++-- .../src/beacon/helpers/gloas.rs | 24 +- .../src/beacon/helpers/mutators.rs | 2 +- .../state_transition/src/beacon/stf/altair.rs | 126 ++++--- .../src/beacon/stf/bellatrix.rs | 11 +- .../state_transition/src/beacon/stf/block.rs | 21 +- .../src/beacon/stf/capella.rs | 11 +- .../state_transition/src/beacon/stf/deneb.rs | 11 +- .../src/beacon/stf/electra.rs | 215 +++++++++--- .../src/beacon/stf/epoch/electra.rs | 2 +- .../state_transition/src/beacon/stf/fulu.rs | 49 ++- .../state_transition/src/beacon/stf/gloas.rs | 41 ++- .../state_transition/src/beacon/stf/mod.rs | 10 +- .../state_transition/src/metrics.rs | 18 + .../tests/beacon_spec/operations.rs | 38 ++- .../tests/beacon_spec/sanity.rs | 6 +- .../tests/beacon_spec/transition.rs | 6 +- .../common/types/src/beacon/containers/mod.rs | 11 + .../rpc/src/beacon/validator_client_tests.rs | 1 + crates/storage/src/lib.rs | 2 + crates/storage/src/pubkey_index_cache.rs | 318 ++++++++++++++++++ crates/storage/src/store.rs | 14 + docs/metrics.md | 3 +- 28 files changed, 1111 insertions(+), 167 deletions(-) create mode 100644 crates/storage/src/pubkey_index_cache.rs diff --git a/bin/ethlambda/src/era/replay.rs b/bin/ethlambda/src/era/replay.rs index 7ead6df14..7128e7085 100644 --- a/bin/ethlambda/src/era/replay.rs +++ b/bin/ethlambda/src/era/replay.rs @@ -18,7 +18,9 @@ use std::path::Path; use std::time::Instant; -use ethlambda_state_transition::beacon::helpers::accessors::{ActiveBalanceCache, CommitteeCache}; +use ethlambda_state_transition::beacon::helpers::accessors::{ + ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, +}; use ethlambda_state_transition::beacon::stf::{self, ExecutionEngine}; use ethlambda_types::beacon::config::Config; use ethlambda_types::beacon::containers::{BeaconState, SignedBeaconBlock}; @@ -54,6 +56,7 @@ pub(super) struct Replay<'a> { /// One for the whole run, as the node holds one across its imports. committees: CommitteeCache, active_balances: ActiveBalanceCache, + pubkey_indices: PubkeyIndexCache, max_blocks: Option, carried: Option, } @@ -66,6 +69,7 @@ impl<'a> Replay<'a> { engine: ExecutionEngine::valid(), committees: CommitteeCache::default(), active_balances: ActiveBalanceCache::default(), + pubkey_indices: PubkeyIndexCache::default(), max_blocks, carried: None, } @@ -169,6 +173,7 @@ impl<'a> Replay<'a> { &self.engine, &self.committees, &self.active_balances, + &self.pubkey_indices, ) .map_err(|err| { eyre!( diff --git a/crates/blockchain/state_transition/src/beacon/block_production.rs b/crates/blockchain/state_transition/src/beacon/block_production.rs index 9c3d0f8ce..a710a1fe5 100644 --- a/crates/blockchain/state_transition/src/beacon/block_production.rs +++ b/crates/blockchain/state_transition/src/beacon/block_production.rs @@ -38,8 +38,9 @@ use super::bls; use super::config::Config; use super::error::{Error, Result, verify}; use super::helpers::accessors::{ - ActiveBalanceCache, CommitteeCache, get_beacon_proposer_index, get_block_root, - get_block_root_at_slot, get_current_epoch, get_domain, get_previous_epoch, get_randao_mix, + ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, get_beacon_proposer_index, + get_block_root, get_block_root_at_slot, get_current_epoch, get_domain, get_previous_epoch, + get_randao_mix, }; use super::helpers::electra::{ get_attesting_indices, get_indexed_attestation, is_valid_indexed_attestation, @@ -402,12 +403,18 @@ pub fn pack_operations(state: &BeaconState, candidates: Operations, config: &Con stf::electra::process_attester_slashing, ); + // One table for the whole scratch pass: the scratch state is at the + // block's own slot, so its total active balance cannot change between + // candidates. + let active_balances = ActiveBalanceCache::default(); let voluntary_exits = pack_each( &mut scratch, candidates.voluntary_exits, preset::MAX_VOLUNTARY_EXITS, config, - stf::electra::process_voluntary_exit, + |state, exit, config| { + stf::electra::process_voluntary_exit(state, exit, config, &active_balances) + }, ); let bls_to_execution_changes = pack_each( &mut scratch, @@ -592,6 +599,7 @@ pub fn assemble_block( &ExecutionEngine::valid(), &CommitteeCache::default(), &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), options, )?; // Fold the block's writes into their trees first, so the root is computed @@ -702,6 +710,7 @@ mod tests { &ExecutionEngine::valid(), &CommitteeCache::default(), &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), BlockProcessingOptions::default(), ) .unwrap(); @@ -766,6 +775,7 @@ mod tests { &ExecutionEngine::valid(), &CommitteeCache::default(), &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), skip_randao(), ) .unwrap(); diff --git a/crates/blockchain/state_transition/src/beacon/fork_choice.rs b/crates/blockchain/state_transition/src/beacon/fork_choice.rs index 02c959376..dff9f293e 100644 --- a/crates/blockchain/state_transition/src/beacon/fork_choice.rs +++ b/crates/blockchain/state_transition/src/beacon/fork_choice.rs @@ -1060,6 +1060,10 @@ fn transition_block( committees: &CommitteeCache, active_balances: &ActiveBalanceCache, ) -> (BeaconState, Result<()>) { + // The store's own table, shared across forks; every hit is verified + // against the state in hand (see `PubkeyIndexCacheExt`). + let pubkey_cache = store.pubkey_index_cache(); + let pubkey_indices = &*pubkey_cache; let block_slot = signed_block.slot(); let epoch = compute_epoch_at_slot(block_slot); let epoch_start = compute_start_slot_at_epoch(epoch); @@ -1085,6 +1089,7 @@ fn transition_block( engine, committees, active_balances, + pubkey_indices, ); return (state, outcome); } @@ -1102,6 +1107,7 @@ fn transition_block( engine, committees, active_balances, + pubkey_indices, ) }); return (state, outcome); @@ -1127,6 +1133,7 @@ fn transition_block( engine, committees, active_balances, + pubkey_indices, ) }); (state, outcome) @@ -5634,7 +5641,7 @@ mod tests { use super::*; use crate::beacon::bls; use crate::beacon::containers::BeaconBlockHeader; - use crate::beacon::helpers::accessors::{self, ActiveBalanceCache}; + use crate::beacon::helpers::accessors::{self, ActiveBalanceCache, PubkeyIndexCache}; use crate::beacon::helpers::misc::compute_signing_root; use crate::beacon::helpers::test_state; use crate::beacon::primitives::BlsSignature; @@ -7186,6 +7193,7 @@ mod tests { &stf::ExecutionEngine::valid(), &trial_committees, &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), ) .expect("the drafted block must transition cleanly"); let state_root = trial_state.hash_tree_root(); diff --git a/crates/blockchain/state_transition/src/beacon/gloas_block_production.rs b/crates/blockchain/state_transition/src/beacon/gloas_block_production.rs index b4621d542..8f3438224 100644 --- a/crates/blockchain/state_transition/src/beacon/gloas_block_production.rs +++ b/crates/blockchain/state_transition/src/beacon/gloas_block_production.rs @@ -26,8 +26,8 @@ use super::bls; use super::config::Config; use super::error::{Error, Result, verify}; use super::helpers::accessors::{ - ActiveBalanceCache, CommitteeCache, get_beacon_proposer_index, get_current_epoch, - get_randao_mix, + ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, get_beacon_proposer_index, + get_current_epoch, get_randao_mix, }; use super::helpers::gloas::{ can_builder_cover_bid, get_indexed_payload_attestation, get_ptc, gloas_state_ref, @@ -91,7 +91,15 @@ pub fn gloas_payload_inputs( let parent_bid = &inner.latest_execution_payload_bid; let (withdrawals, head_block_hash) = if build_on_full { let mut applied = state.clone(); - stf::gloas::apply_parent_execution_payload(&mut applied, parent_requests, config)?; + // A scratch copy used once: fresh caches, as for the rest of block + // production. + stf::gloas::apply_parent_execution_payload( + &mut applied, + parent_requests, + config, + &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), + )?; ( stf::gloas::get_expected_withdrawals(&applied)?.withdrawals, parent_bid.block_hash, @@ -409,6 +417,7 @@ pub fn assemble_gloas_block_on_bid( config, &CommitteeCache::default(), &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), options, )?; block.state_root = post.hash_tree_root(); @@ -535,6 +544,7 @@ pub fn assemble_gloas_block( config, &CommitteeCache::default(), &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), options, )?; block.state_root = post.hash_tree_root(); @@ -751,6 +761,7 @@ pub mod test_support { &config(), &CommitteeCache::default(), &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), BlockProcessingOptions::default(), ) .unwrap(); @@ -915,6 +926,7 @@ mod gloas_block_production_tests { &ExecutionEngine::valid(), &CommitteeCache::default(), &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), BlockProcessingOptions::default(), ) .unwrap(); @@ -1048,6 +1060,7 @@ mod gloas_block_production_tests { &config(), &CommitteeCache::default(), &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), skip, ) .unwrap(); diff --git a/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs b/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs index 60cdcc7a1..875257d70 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs @@ -12,7 +12,7 @@ use std::sync::Arc; // is unaffected by its move to `ethlambda-storage`; see [`CommitteeCacheExt`] // below for what this crate still contributes. pub use ethlambda_storage::{ - ActiveBalanceCache, ActiveBalanceKey, CommitteeCache, Lookup, ShufflingKey, + ActiveBalanceCache, ActiveBalanceKey, CommitteeCache, Lookup, PubkeyIndexCache, ShufflingKey, }; use crate::beacon::config::Config; @@ -24,7 +24,7 @@ use crate::beacon::hash::hash; use crate::beacon::lean_state_unreachable; use crate::beacon::preset; use crate::beacon::primitives::{ - Bytes32, CommitteeIndex, Domain, DomainType, Epoch, Gwei, Root, Slot, ValidatorIndex, + BlsPubkey, Bytes32, CommitteeIndex, Domain, DomainType, Epoch, Gwei, Root, Slot, ValidatorIndex, }; use super::misc::{ @@ -395,6 +395,88 @@ impl ActiveBalanceCacheExt for ActiveBalanceCache { } } +/// `state`'s total active balance for block processing: through `cache` when +/// the caller holds one, the spec-shaped uncached pass when not. +/// +/// The churn helpers take an `Option` so that block processing, which holds an +/// [`ActiveBalanceCache`], shares one registry pass across a block's exits, +/// consolidations and slashings, while epoch processing and the spec-shaped +/// unit tests keep the uncached function. Passing `Some` is sound only for the +/// callers [`active_balance_key`] documents: states advanced to their block's +/// slot, never mid epoch processing. +pub fn total_active_balance_or_compute( + state: &BeaconState, + cache: Option<&ActiveBalanceCache>, +) -> Result { + match cache { + Some(cache) => cache.total_active_balance(state), + None => get_total_active_balance(state), + } +} + +/// Extends `ethlambda-storage`'s [`PubkeyIndexCache`] with the consensus logic +/// that feeds it a registry and checks its answers, for the same reason +/// [`CommitteeCacheExt`] lives here and not in `ethlambda-storage`. +/// +/// # Soundness +/// +/// Pubkeys are unique within any reachable registry (a deposit for a pubkey +/// already registered tops the existing validator up instead of appending), +/// and a registry only ever grows by appending, so on one chain a pubkey's +/// index is fixed from the block that registers it. The table is process-wide +/// and so also holds positions from other forks, where an unfinalized index can +/// differ. Every candidate is therefore checked against the asking state's own +/// validator at that index, which is what makes a stale, foreign or +/// prefix-colliding entry cost a scan instead of a wrong answer. +pub trait PubkeyIndexCacheExt { + /// The index holding `pubkey` in `state`, from the cache only: `None` if + /// no candidate survives verification, which is *not* proof the pubkey is + /// absent. Brings the table up to `state`'s registry length first, then + /// does no scan. + fn verified_index(&self, state: &BeaconState, pubkey: &BlsPubkey) -> Option; + + /// The index holding `pubkey` in `state`, exactly as a position search of + /// the registry would answer: the cache when it verifies, otherwise that + /// search. A pubkey that is not registered costs one search, never a + /// rebuild of the table. Counted in `lean_beacon_pubkey_index_lookups_total`. + fn validator_index(&self, state: &BeaconState, pubkey: &BlsPubkey) -> Option; +} + +impl PubkeyIndexCacheExt for PubkeyIndexCache { + fn verified_index(&self, state: &BeaconState, pubkey: &BlsPubkey) -> Option { + let count = state.validator_count(); + let covered = self.covered_len(); + if count > covered { + // Scanned outside the table's lock; `extend` skips whatever a + // racing caller already ingested. The first call on a fresh table + // ingests the whole registry once. + let tail: Vec = state + .iter_validators_from(covered) + .map(|validator| PubkeyIndexCache::key_of(&validator.pubkey)) + .collect(); + self.extend(covered, &tail); + } + self.candidates(pubkey).into_iter().find(|index| { + *index < count as u64 + && state + .validator(*index) + .is_ok_and(|validator| validator.pubkey == *pubkey) + }) + } + + fn validator_index(&self, state: &BeaconState, pubkey: &BlsPubkey) -> Option { + if let Some(index) = self.verified_index(state, pubkey) { + crate::metrics::inc_pubkey_index_lookups("hit", 1); + return Some(index); + } + crate::metrics::inc_pubkey_index_lookups("fallback", 1); + state + .iter_validators() + .position(|validator| validator.pubkey == *pubkey) + .map(|index| index as ValidatorIndex) + } +} + /// The committee at `slot` with index `index`. /// /// One epoch's active set is shuffled once and then split across every slot and @@ -1005,4 +1087,107 @@ mod tests { config.min_per_epoch_churn_limit ); } + + // -- PubkeyIndexCache ---------------------------------------------------- + + fn pubkey_at(state: &BeaconState, index: u64) -> BlsPubkey { + state.validator(index).unwrap().pubkey + } + + fn append_validator(state: &mut BeaconState, pubkey: BlsPubkey) { + let mut validator = state.validator(0).unwrap().clone(); + validator.pubkey = pubkey; + state.push_validator(validator, 0).unwrap(); + } + + #[test] + fn a_pubkey_resolves_from_the_cache_after_one_ingest() { + let state = with_validators(20); + let cache = PubkeyIndexCache::default(); + assert_eq!(cache.covered_len(), 0); + + assert_eq!(cache.verified_index(&state, &pubkey_at(&state, 7)), Some(7)); + assert_eq!(cache.covered_len(), 20); + assert_eq!( + cache.verified_index(&state, &pubkey_at(&state, 19)), + Some(19) + ); + } + + #[test] + fn appended_validators_are_ingested_without_a_rebuild() { + let mut state = with_validators(10); + let cache = PubkeyIndexCache::default(); + assert_eq!(cache.verified_index(&state, &pubkey_at(&state, 3)), Some(3)); + + let fresh = BlsPubkey([0xAB; 48]); + append_validator(&mut state, fresh); + assert_eq!(cache.verified_index(&state, &fresh), Some(10)); + assert_eq!(cache.covered_len(), 11); + } + + #[test] + fn a_wrong_index_from_another_fork_fails_verification() { + let ours = with_validators(10); + let cache = PubkeyIndexCache::default(); + // The table is fed by a fork where index 4 holds a different key. + let mut other_fork = ours.clone(); + let foreign = BlsPubkey([0xCD; 48]); + other_fork.validator_mut(4).unwrap().pubkey = foreign; + assert_eq!(cache.verified_index(&other_fork, &foreign), Some(4)); + + // On our fork index 4 does not hold it: the candidate is refused, and + // the public lookup falls back to a scan that finds nothing. + assert_eq!(cache.verified_index(&ours, &foreign), None); + assert_eq!(cache.validator_index(&ours, &foreign), None); + // Our own key at that index, absent from the table's fork, is found by + // the scan. + let own = pubkey_at(&ours, 4); + assert_eq!(cache.validator_index(&ours, &own), Some(4)); + } + + #[test] + fn an_absent_pubkey_costs_a_scan_and_does_not_rebuild_the_table() { + let state = with_validators(10); + let cache = PubkeyIndexCache::default(); + cache.verified_index(&state, &pubkey_at(&state, 0)); + let covered = cache.covered_len(); + + let missing = BlsPubkey([0xEE; 48]); + assert_eq!(cache.validator_index(&state, &missing), None); + assert_eq!(cache.covered_len(), covered); + assert!(cache.candidates(&missing).is_empty()); + } + + #[test] + fn a_key_prefix_collision_falls_back_to_the_right_validator() { + let mut state = with_validators(10); + let cache = PubkeyIndexCache::default(); + // Two distinct pubkeys sharing the eight bytes the table keys on. + let first = pubkey_at(&state, 2); + let mut twin = first; + twin.0[47] ^= 0xFF; + assert_eq!( + PubkeyIndexCache::key_of(&first), + PubkeyIndexCache::key_of(&twin) + ); + append_validator(&mut state, twin); + + assert_eq!(cache.validator_index(&state, &first), Some(2)); + assert_eq!(cache.validator_index(&state, &twin), Some(10)); + } + + #[test] + fn the_cache_agrees_with_a_registry_scan_for_every_validator() { + let state = with_validators(64); + let cache = PubkeyIndexCache::default(); + for index in 0..64u64 { + let pubkey = pubkey_at(&state, index); + let scanned = state + .iter_validators() + .position(|validator| validator.pubkey == pubkey) + .map(|position| position as u64); + assert_eq!(cache.validator_index(&state, &pubkey), scanned); + } + } } diff --git a/crates/blockchain/state_transition/src/beacon/helpers/electra.rs b/crates/blockchain/state_transition/src/beacon/helpers/electra.rs index 94fa7d20b..0e98c3115 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/electra.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/electra.rs @@ -139,8 +139,9 @@ use crate::beacon::primitives::{ }; use super::accessors::{ - CommitteeCache, CommitteeCacheExt, get_active_validator_indices, get_current_epoch, get_domain, - get_seed, get_total_active_balance, + ActiveBalanceCache, CommitteeCache, CommitteeCacheExt, get_active_validator_indices, + get_current_epoch, get_domain, get_seed, get_total_active_balance, + total_active_balance_or_compute, }; use super::math::bytes_to_uint64; use super::misc::{compute_activation_exit_epoch, compute_epoch_at_slot, compute_signing_root}; @@ -409,11 +410,20 @@ pub(crate) fn activation_exit_churn_limit_for(total_active_balance: Gwei, config /// The portion of [`get_balance_churn_limit`] left over for consolidations /// once [`get_activation_exit_churn_limit`] has taken its share. pub fn get_consolidation_churn_limit(state: &BeaconState, config: &Config) -> Result { - let balance_churn_limit = get_balance_churn_limit(state, config)?; - let activation_exit_churn_limit = get_activation_exit_churn_limit(state, config)?; - // `get_activation_exit_churn_limit` is a `min` against `balance_churn_limit`, + Ok(consolidation_churn_limit_for( + get_total_active_balance(state)?, + config, + )) +} + +/// [`get_consolidation_churn_limit`] for a caller that already holds the total +/// active balance: one registry pass for both halves instead of one each. +pub(crate) fn consolidation_churn_limit_for(total_active_balance: Gwei, config: &Config) -> Gwei { + let balance_churn_limit = balance_churn_limit_for(total_active_balance, config); + let activation_exit_churn_limit = activation_exit_churn_limit_for(total_active_balance, config); + // `activation_exit_churn_limit_for` is a `min` against `balance_churn_limit`, // so it can never exceed it, and this subtraction cannot underflow. - Ok(balance_churn_limit - activation_exit_churn_limit) + balance_churn_limit - activation_exit_churn_limit } /// The total amount queued in [`electra::PendingPartialWithdrawal`]s for @@ -604,17 +614,23 @@ pub fn get_next_sync_committee_indices(state: &BeaconState) -> Result, ) -> Result<()> { if state.validator(index)?.exit_epoch != FAR_FUTURE_EPOCH { return Ok(()); } let effective_balance = state.validator(index)?.effective_balance; - let exit_queue_epoch = compute_exit_epoch_and_update_churn(state, effective_balance, config)?; + let exit_queue_epoch = + compute_exit_epoch_and_update_churn(state, effective_balance, config, active_balances)?; // See `crate::beacon::helpers::mutators::initiate_validator_exit` for why this is // checked rather than left to wrap: a validator already queued with an @@ -656,15 +672,34 @@ pub fn initiate_validator_exit( /// ejections) nor `process_withdrawal_request`, so calling this from any /// of them already picks up gloas's churn automatically, the same way /// pyspec's unmodified callers do. +/// +/// # The total active balance +/// +/// Both churn limits are fractions of the total active balance, which costs a +/// registry pass. Block processing passes its [`ActiveBalanceCache`] so a +/// block's exits and withdrawal requests share one pass. Sound there because +/// the total cannot change within an epoch: effective balances move only in +/// epoch processing, and every activation or exit epoch assigned in a block is +/// at least `MAX_SEED_LOOKAHEAD` epochs ahead, so none reaches the current +/// epoch's active set (see `accessors::active_balance_key`). Epoch processing +/// passes `None`, because the cache key is not sound once the epoch's +/// effective-balance update has run. pub fn compute_exit_epoch_and_update_churn( state: &mut BeaconState, exit_balance: Gwei, config: &Config, + active_balances: Option<&ActiveBalanceCache>, ) -> Result { let current_epoch = get_current_epoch(state); let per_epoch_churn = match state.fork_name() { - ForkName::Electra | ForkName::Fulu => get_activation_exit_churn_limit(state, config)?, - ForkName::Gloas => crate::beacon::helpers::gloas::get_exit_churn_limit(state, config)?, + ForkName::Electra | ForkName::Fulu => activation_exit_churn_limit_for( + total_active_balance_or_compute(state, active_balances)?, + config, + ), + ForkName::Gloas => crate::beacon::helpers::gloas::exit_churn_limit_for( + total_active_balance_or_compute(state, active_balances)?, + config, + ), fork @ (ForkName::Phase0 | ForkName::Altair | ForkName::Bellatrix @@ -794,15 +829,26 @@ impl ExitChurnCursor { /// [`compute_consolidation_epoch_and_update_churn`] and /// [`crate::beacon::stf::electra::process_consolidation_request`] both go /// through this rather than either raw formula directly. +/// +/// `active_balances` is block processing's cache (see +/// [`compute_exit_epoch_and_update_churn`] for why that is sound), or `None` +/// for the uncached total. pub fn get_consolidation_churn_limit_for_fork( state: &BeaconState, config: &Config, + active_balances: Option<&ActiveBalanceCache>, ) -> Result { match state.fork_name() { - ForkName::Electra | ForkName::Fulu => get_consolidation_churn_limit(state, config), - ForkName::Gloas => { - crate::beacon::helpers::gloas::get_consolidation_churn_limit(state, config) - } + ForkName::Electra | ForkName::Fulu => Ok(consolidation_churn_limit_for( + total_active_balance_or_compute(state, active_balances)?, + config, + )), + ForkName::Gloas => Ok( + crate::beacon::helpers::gloas::consolidation_churn_limit_for( + total_active_balance_or_compute(state, active_balances)?, + config, + ), + ), fork @ (ForkName::Phase0 | ForkName::Altair | ForkName::Bellatrix @@ -835,9 +881,10 @@ pub fn compute_consolidation_epoch_and_update_churn( state: &mut BeaconState, consolidation_balance: Gwei, config: &Config, + active_balances: Option<&ActiveBalanceCache>, ) -> Result { let current_epoch = get_current_epoch(state); - let per_epoch_churn = get_consolidation_churn_limit_for_fork(state, config)?; + let per_epoch_churn = get_consolidation_churn_limit_for_fork(state, config, active_balances)?; let mut fields = churn_cursors_mut(state, "compute_consolidation_epoch_and_update_churn")?; @@ -1760,7 +1807,7 @@ mod tests { let config = Config::mainnet(); let mut state = electra_state_with_validators(4); - initiate_validator_exit(&mut state, 0, &config).unwrap(); + initiate_validator_exit(&mut state, 0, &config, None).unwrap(); let first_exit_epoch = state.validator(0).unwrap().exit_epoch; assert_ne!(first_exit_epoch, FAR_FUTURE_EPOCH); assert_eq!( @@ -1768,7 +1815,7 @@ mod tests { first_exit_epoch + config.min_validator_withdrawability_delay, ); - initiate_validator_exit(&mut state, 0, &config).unwrap(); + initiate_validator_exit(&mut state, 0, &config, None).unwrap(); assert_eq!(state.validator(0).unwrap().exit_epoch, first_exit_epoch); } @@ -1781,7 +1828,8 @@ mod tests { let baseline = compute_activation_exit_epoch(get_current_epoch(&state)); let exit_epoch = - compute_exit_epoch_and_update_churn(&mut state, per_epoch_churn * 2, &config).unwrap(); + compute_exit_epoch_and_update_churn(&mut state, per_epoch_churn * 2, &config, None) + .unwrap(); assert!(exit_epoch > baseline); } @@ -1792,9 +1840,11 @@ mod tests { let per_epoch_churn = get_activation_exit_churn_limit(&state, &config).unwrap(); let first_epoch = - compute_exit_epoch_and_update_churn(&mut state, per_epoch_churn / 4, &config).unwrap(); + compute_exit_epoch_and_update_churn(&mut state, per_epoch_churn / 4, &config, None) + .unwrap(); let second_epoch = - compute_exit_epoch_and_update_churn(&mut state, per_epoch_churn / 4, &config).unwrap(); + compute_exit_epoch_and_update_churn(&mut state, per_epoch_churn / 4, &config, None) + .unwrap(); // Together they are still under one epoch's churn limit, so the // cursor lets the second exit spend what the first left over instead @@ -1932,7 +1982,7 @@ mod tests { let mut state = fulu_state_with_validators(4); let baseline = compute_activation_exit_epoch(get_current_epoch(&state)); - let exit_epoch = compute_exit_epoch_and_update_churn(&mut state, 1, &config).unwrap(); + let exit_epoch = compute_exit_epoch_and_update_churn(&mut state, 1, &config, None).unwrap(); assert_eq!(exit_epoch, baseline); } @@ -1957,7 +2007,7 @@ mod tests { crate::beacon::helpers::gloas::get_consolidation_churn_limit(&state, &config).unwrap(); assert!(gloas_limit > 0, "the fixture must exercise a real budget"); - compute_consolidation_epoch_and_update_churn(&mut state, 1, &config).unwrap(); + compute_consolidation_epoch_and_update_churn(&mut state, 1, &config, None).unwrap(); let fields = churn_cursors_mut(&mut state, "test").unwrap(); assert_eq!(fields.consolidation_balance_to_consume(), gloas_limit - 1); @@ -1976,8 +2026,9 @@ mod tests { crate::beacon::helpers::test_state::with_validators_at(ForkName::Gloas, 64); let electra_limit = - get_consolidation_churn_limit_for_fork(&electra_state, &config).unwrap(); - let gloas_limit = get_consolidation_churn_limit_for_fork(&gloas_state, &config).unwrap(); + get_consolidation_churn_limit_for_fork(&electra_state, &config, None).unwrap(); + let gloas_limit = + get_consolidation_churn_limit_for_fork(&gloas_state, &config, None).unwrap(); assert_eq!( electra_limit, diff --git a/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs b/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs index 3bb3d0094..f48b45c3c 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs @@ -821,11 +821,19 @@ pub fn get_activation_churn_limit(state: &BeaconState, config: &Config) -> Resul /// draws from this on a gloas state /// rather than electra's combined budget. pub fn get_exit_churn_limit(state: &BeaconState, config: &Config) -> Result { - let total_active_balance = get_total_active_balance(state)?; + Ok(exit_churn_limit_for( + get_total_active_balance(state)?, + config, + )) +} + +/// [`get_exit_churn_limit`] for a caller that already holds the total active +/// balance, so block processing can take it from the `ActiveBalanceCache`. +pub(crate) fn exit_churn_limit_for(total_active_balance: Gwei, config: &Config) -> Gwei { let churn = config .min_per_epoch_churn_limit_electra .max(total_active_balance / config.churn_limit_quotient_gloas); - Ok(churn - churn % preset::EFFECTIVE_BALANCE_INCREMENT) + churn - churn % preset::EFFECTIVE_BALANCE_INCREMENT } /// The per-epoch churn limit reserved for consolidations, in Gwei. @@ -839,9 +847,17 @@ pub fn get_exit_churn_limit(state: &BeaconState, config: &Config) -> Result Result { - let total_active_balance = get_total_active_balance(state)?; + Ok(consolidation_churn_limit_for( + get_total_active_balance(state)?, + config, + )) +} + +/// [`get_consolidation_churn_limit`] for a caller that already holds the total +/// active balance. +pub(crate) fn consolidation_churn_limit_for(total_active_balance: Gwei, config: &Config) -> Gwei { let churn = total_active_balance / config.consolidation_churn_limit_quotient; - Ok(churn - churn % preset::EFFECTIVE_BALANCE_INCREMENT) + churn - churn % preset::EFFECTIVE_BALANCE_INCREMENT } // --------------------------------------------------------------------------- diff --git a/crates/blockchain/state_transition/src/beacon/helpers/mutators.rs b/crates/blockchain/state_transition/src/beacon/helpers/mutators.rs index c9ae7ff45..72efd03bc 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/mutators.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/mutators.rs @@ -181,7 +181,7 @@ fn initiate_validator_exit_for_fork( // draws through `compute_exit_epoch_and_update_churn` is already // gloas's own by the time it gets here. ForkName::Electra | ForkName::Fulu | ForkName::Gloas => { - crate::beacon::helpers::electra::initiate_validator_exit(state, index, config) + crate::beacon::helpers::electra::initiate_validator_exit(state, index, config, None) } ForkName::Phase0 | ForkName::Altair diff --git a/crates/blockchain/state_transition/src/beacon/stf/altair.rs b/crates/blockchain/state_transition/src/beacon/stf/altair.rs index 2b33e79be..2b0bcb2d9 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/altair.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/altair.rs @@ -31,9 +31,9 @@ use crate::beacon::constants; use crate::beacon::containers::{BeaconState, altair, phase0}; use crate::beacon::error::{Error, Result, verify}; use crate::beacon::helpers::accessors::{ - ActiveBalanceCache, ActiveBalanceCacheExt, CommitteeCache, CommitteeCacheExt, - get_beacon_proposer_index, get_block_root_at_slot, get_current_epoch, get_domain, - get_previous_epoch, + ActiveBalanceCache, ActiveBalanceCacheExt, CommitteeCache, CommitteeCacheExt, PubkeyIndexCache, + PubkeyIndexCacheExt, get_beacon_proposer_index, get_block_root_at_slot, get_current_epoch, + get_domain, get_previous_epoch, }; use crate::beacon::helpers::altair::{ add_flag, base_reward_per_increment, get_attestation_participation_flag_indices, has_flag, @@ -284,9 +284,10 @@ pub fn process_attestation( /// there is no independently-derived penalty formula that could drift out of /// sync with the reward one. /// -/// Resolving each committee seat's validator index needs a linear scan of -/// `state.validators` (sync committee membership is recorded as pubkeys, not -/// indices), and the specification does that scan inside the very loop that +/// Resolving each committee seat's validator index is, in the specification, a +/// linear scan of `state.validators` (sync committee membership is recorded as +/// pubkeys, not indices); here it goes through the [`PubkeyIndexCache`], with +/// that scan as the fallback. The specification does the scan inside the very loop that /// also calls `get_beacon_proposer_index` and adjusts balances. Doing the scan /// there would need `state` borrowed immutably (to search) and mutably (to /// pay or penalize) at the same time, which does not borrow-check; this @@ -304,6 +305,7 @@ pub fn process_sync_aggregate( state: &mut BeaconState, sync_aggregate: &altair::SyncAggregate, active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, ) -> Result<()> { let committee_bits = &sync_aggregate.sync_committee_bits; @@ -341,37 +343,25 @@ pub fn process_sync_aggregate( // own scan of `state.validators` the way the specification's single loop // does. // - // One pass over the registry, not one per seat. The specification writes - // this as a search per committee member, which is `SYNC_COMMITTEE_SIZE` - // scans of the whole validator set: at mainnet's ~1M validators that is - // hundreds of millions of 48-byte pubkey comparisons per block, and it was - // measured as the single largest cost in a live import, dwarfing the - // signature verification above it. Inverting the loop makes it one scan - // against a committee-sized map. - // - // A pubkey may hold more than one seat, since sync-committee selection - // samples with replacement, so each key maps to every seat it occupies - // rather than to one. - let mut seats_by_pubkey: HashMap<&BlsPubkey, Vec> = HashMap::new(); - for (seat, pubkey) in committee_pubkeys.iter().enumerate() { - seats_by_pubkey.entry(pubkey).or_default().push(seat); - } - let mut resolved: Vec> = vec![None; committee_pubkeys.len()]; - for (index, validator) in state.iter_validators().enumerate() { - if let Some(seats) = seats_by_pubkey.get(&validator.pubkey) { - for &seat in seats { - resolved[seat] = Some(index as ValidatorIndex); - } + // Through the pubkey index cache: the specification writes this as a + // search per committee member, which at mainnet's registry size was the + // largest single cost of a live import (a hash probe per validator, every + // block), where the cache answers each seat with a lookup and one + // verifying tree descent. + let cached: Option> = committee_pubkeys + .iter() + .map(|pubkey| pubkey_indices.verified_index(state, pubkey)) + .collect(); + let committee_indices = match cached { + Some(indices) => { + crate::metrics::inc_pubkey_index_lookups("hit", indices.len() as u64); + indices } - } - let committee_indices: Vec = resolved - .into_iter() - .map(|index| { - index.ok_or(Error::SpecAssert( - "state.current_sync_committee.pubkeys[i] in [v.pubkey for v in state.validators]", - )) - }) - .collect::>>()?; + None => { + crate::metrics::inc_pubkey_index_lookups("fallback", committee_pubkeys.len() as u64); + resolve_sync_committee_by_scan(state, committee_pubkeys)? + } + }; let total_active_balance = active_balances.total_active_balance(state)?; let total_active_increments = total_active_balance / preset::EFFECTIVE_BALANCE_INCREMENT; @@ -415,6 +405,44 @@ pub fn process_sync_aggregate( Ok(()) } +/// Every committee seat's validator index, by one pass over the registry: +/// the fallback for a seat the pubkey index cache could not verify. +/// +/// One pass, not one per seat. The specification writes this as a search per +/// committee member, which is `SYNC_COMMITTEE_SIZE` scans of the whole +/// validator set: at mainnet's ~1M validators that is hundreds of millions of +/// 48-byte pubkey comparisons per block. Inverting the loop makes it one scan +/// against a committee-sized map. +/// +/// A pubkey may hold more than one seat, since sync-committee selection +/// samples with replacement, so each key maps to every seat it occupies +/// rather than to one. +fn resolve_sync_committee_by_scan( + state: &BeaconState, + committee_pubkeys: &[BlsPubkey], +) -> Result> { + let mut seats_by_pubkey: HashMap<&BlsPubkey, Vec> = HashMap::new(); + for (seat, pubkey) in committee_pubkeys.iter().enumerate() { + seats_by_pubkey.entry(pubkey).or_default().push(seat); + } + let mut resolved: Vec> = vec![None; committee_pubkeys.len()]; + for (index, validator) in state.iter_validators().enumerate() { + if let Some(seats) = seats_by_pubkey.get(&validator.pubkey) { + for &seat in seats { + resolved[seat] = Some(index as ValidatorIndex); + } + } + } + resolved + .into_iter() + .map(|index| { + index.ok_or(Error::SpecAssert( + "state.current_sync_committee.pubkeys[i] in [v.pubkey for v in state.validators]", + )) + }) + .collect() +} + #[cfg(test)] mod tests { use blst::min_pk::SecretKey; @@ -711,8 +739,13 @@ mod tests { .map(|index| state.balance(index).unwrap()) .collect(); - process_sync_aggregate(&mut state, &sync_aggregate, &ActiveBalanceCache::default()) - .unwrap(); + process_sync_aggregate( + &mut state, + &sync_aggregate, + &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), + ) + .unwrap(); // Independently derived expected rewards, sharing only the // already-tested building blocks (`get_total_active_balance`, @@ -777,8 +810,13 @@ mod tests { sync_committee_signature: BlsSignature::default(), }; assert!( - process_sync_aggregate(&mut state.clone(), &wrong, &ActiveBalanceCache::default()) - .is_err(), + process_sync_aggregate( + &mut state.clone(), + &wrong, + &ActiveBalanceCache::default(), + &PubkeyIndexCache::default() + ) + .is_err(), "the all-zero signature is not the point at infinity, so this must be rejected" ); @@ -787,7 +825,13 @@ mod tests { sync_committee_signature: g2_point_at_infinity(), }; let balance_before = state.balance(0).unwrap(); - process_sync_aggregate(&mut state, &correct, &ActiveBalanceCache::default()).unwrap(); + process_sync_aggregate( + &mut state, + &correct, + &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), + ) + .unwrap(); // The sole validator holds every seat and none of them participated, // so it is penalized once per seat and the proposer (itself, the only diff --git a/crates/blockchain/state_transition/src/beacon/stf/bellatrix.rs b/crates/blockchain/state_transition/src/beacon/stf/bellatrix.rs index e9287513e..d24358b3c 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/bellatrix.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/bellatrix.rs @@ -34,7 +34,7 @@ use crate::beacon::constants; use crate::beacon::containers::{BeaconState, bellatrix}; use crate::beacon::error::{Error, Result, verify}; use crate::beacon::helpers::accessors::{ - ActiveBalanceCache, CommitteeCache, get_current_epoch, get_randao_mix, + ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, get_current_epoch, get_randao_mix, }; use crate::beacon::preset; use crate::beacon::primitives::{ @@ -63,6 +63,7 @@ use super::ExecutionEngine; /// the one fork where a block might still legitimately carry no real payload, /// if this chain's merge transition has not happened yet. Every later fork /// drops the condition outright, since by then it is always true. +#[allow(clippy::too_many_arguments)] pub fn process_block( state: &mut BeaconState, block: &bellatrix::BeaconBlock, @@ -70,6 +71,7 @@ pub fn process_block( engine: &ExecutionEngine, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, options: super::block::BlockProcessingOptions, ) -> Result<()> { super::block::process_block_header( @@ -95,7 +97,12 @@ pub fn process_block( committees, active_balances, )?; - super::altair::process_sync_aggregate(state, &block.body.sync_aggregate, active_balances)?; + super::altair::process_sync_aggregate( + state, + &block.body.sync_aggregate, + active_balances, + pubkey_indices, + )?; Ok(()) } diff --git a/crates/blockchain/state_transition/src/beacon/stf/block.rs b/crates/blockchain/state_transition/src/beacon/stf/block.rs index dcaa32684..f3bb60d92 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/block.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/block.rs @@ -24,8 +24,8 @@ use crate::beacon::containers::{self, BeaconBlockHeader, BeaconState, Eth1Data, use crate::beacon::error::{Result, verify}; use crate::beacon::hash::hash; use crate::beacon::helpers::accessors::{ - ActiveBalanceCache, CommitteeCache, get_beacon_proposer_index, get_current_epoch, get_domain, - get_randao_mix, + ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, get_beacon_proposer_index, + get_current_epoch, get_domain, get_randao_mix, }; use crate::beacon::helpers::math::xor; use crate::beacon::helpers::misc::compute_signing_root; @@ -41,6 +41,7 @@ use super::{ExecutionEngine, bellatrix, capella, deneb, electra, fulu, gloas}; /// after [`super::process_slots`] has already advanced the state to the block's /// slot, so every arm validates and mutates a state already positioned at /// `signed_block`'s slot. +#[allow(clippy::too_many_arguments)] pub fn process_block( state: &mut BeaconState, signed_block: &containers::SignedBeaconBlock, @@ -48,6 +49,7 @@ pub fn process_block( engine: &ExecutionEngine, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, options: BlockProcessingOptions, ) -> Result<()> { match signed_block { @@ -65,6 +67,7 @@ pub fn process_block( config, committees, active_balances, + pubkey_indices, options, ), containers::SignedBeaconBlock::Bellatrix(signed) => bellatrix::process_block( @@ -74,6 +77,7 @@ pub fn process_block( engine, committees, active_balances, + pubkey_indices, options, ), containers::SignedBeaconBlock::Capella(signed) => capella::process_block( @@ -83,6 +87,7 @@ pub fn process_block( engine, committees, active_balances, + pubkey_indices, options, ), containers::SignedBeaconBlock::Deneb(signed) => deneb::process_block( @@ -92,6 +97,7 @@ pub fn process_block( engine, committees, active_balances, + pubkey_indices, options, ), containers::SignedBeaconBlock::Electra(signed) => electra::process_block( @@ -101,6 +107,7 @@ pub fn process_block( engine, committees, active_balances, + pubkey_indices, options, ), containers::SignedBeaconBlock::Fulu(signed) => fulu::process_block( @@ -110,6 +117,7 @@ pub fn process_block( engine, committees, active_balances, + pubkey_indices, options, ), // ePBS (EIP-7732) restructures block processing around a builder's @@ -121,6 +129,7 @@ pub fn process_block( config, committees, active_balances, + pubkey_indices, options, ), containers::SignedBeaconBlock::Lean(_) => { @@ -174,6 +183,7 @@ pub fn process_block_altair( config: &Config, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, options: BlockProcessingOptions, ) -> Result<()> { process_block_header( @@ -196,7 +206,12 @@ pub fn process_block_altair( committees, active_balances, )?; - super::altair::process_sync_aggregate(state, &block.body.sync_aggregate, active_balances)?; + super::altair::process_sync_aggregate( + state, + &block.body.sync_aggregate, + active_balances, + pubkey_indices, + )?; Ok(()) } diff --git a/crates/blockchain/state_transition/src/beacon/stf/capella.rs b/crates/blockchain/state_transition/src/beacon/stf/capella.rs index f773d6da7..7aafdbd11 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/capella.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/capella.rs @@ -49,7 +49,7 @@ use crate::beacon::containers::{BeaconState, capella, phase0}; use crate::beacon::error::{Error, Result, verify}; use crate::beacon::hash::hash; use crate::beacon::helpers::accessors::{ - ActiveBalanceCache, CommitteeCache, get_current_epoch, get_randao_mix, + ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, get_current_epoch, get_randao_mix, }; use crate::beacon::helpers::capella::{ is_fully_withdrawable_validator, is_partially_withdrawable_validator, withdrawal_address, @@ -72,6 +72,7 @@ use super::ExecutionEngine; /// See this module's own documentation for why [`process_withdrawals`] runs /// before [`process_operations`], and so before this same block's own /// `bls_to_execution_changes` are processed. +#[allow(clippy::too_many_arguments)] pub fn process_block( state: &mut BeaconState, block: &capella::BeaconBlock, @@ -79,6 +80,7 @@ pub fn process_block( engine: &ExecutionEngine, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, options: super::block::BlockProcessingOptions, ) -> Result<()> { super::block::process_block_header( @@ -104,7 +106,12 @@ pub fn process_block( committees, active_balances, )?; - super::altair::process_sync_aggregate(state, &block.body.sync_aggregate, active_balances)?; + super::altair::process_sync_aggregate( + state, + &block.body.sync_aggregate, + active_balances, + pubkey_indices, + )?; Ok(()) } diff --git a/crates/blockchain/state_transition/src/beacon/stf/deneb.rs b/crates/blockchain/state_transition/src/beacon/stf/deneb.rs index 72b5ac946..a1e111964 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/deneb.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/deneb.rs @@ -36,7 +36,7 @@ use crate::beacon::containers::{BeaconState, deneb, phase0}; use crate::beacon::error::{Error, Result, verify}; use crate::beacon::hash::hash; use crate::beacon::helpers::accessors::{ - ActiveBalanceCache, ActiveBalanceCacheExt, CommitteeCache, CommitteeCacheExt, + ActiveBalanceCache, ActiveBalanceCacheExt, CommitteeCache, CommitteeCacheExt, PubkeyIndexCache, get_beacon_proposer_index, get_block_root, get_block_root_at_slot, get_current_epoch, get_previous_epoch, get_randao_mix, }; @@ -72,6 +72,7 @@ use super::ExecutionEngine; /// [`super::operations::process_operations`] has no fork check of its own to /// hang them on. See [`process_operations`]'s own documentation for why that /// is a full copy of the loop rather than a smaller patch. +#[allow(clippy::too_many_arguments)] pub fn process_block( state: &mut BeaconState, block: &deneb::BeaconBlock, @@ -79,6 +80,7 @@ pub fn process_block( engine: &ExecutionEngine, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, options: super::block::BlockProcessingOptions, ) -> Result<()> { super::block::process_block_header( @@ -110,7 +112,12 @@ pub fn process_block( committees, active_balances, )?; - super::altair::process_sync_aggregate(state, &block.body.sync_aggregate, active_balances)?; + super::altair::process_sync_aggregate( + state, + &block.body.sync_aggregate, + active_balances, + pubkey_indices, + )?; Ok(()) } diff --git a/crates/blockchain/state_transition/src/beacon/stf/electra.rs b/crates/blockchain/state_transition/src/beacon/stf/electra.rs index 9b3519caf..847c50b2e 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/electra.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/electra.rs @@ -54,9 +54,9 @@ use crate::beacon::containers::shared::{ use crate::beacon::containers::{BeaconState, capella, deneb, electra, fulu}; use crate::beacon::error::{Error, Result, verify}; use crate::beacon::helpers::accessors::{ - ActiveBalanceCache, ActiveBalanceCacheExt, CommitteeCache, CommitteeCacheExt, - get_beacon_proposer_index, get_block_root, get_block_root_at_slot, get_current_epoch, - get_previous_epoch, get_randao_mix, + ActiveBalanceCache, ActiveBalanceCacheExt, CommitteeCache, CommitteeCacheExt, PubkeyIndexCache, + PubkeyIndexCacheExt, get_beacon_proposer_index, get_block_root, get_block_root_at_slot, + get_current_epoch, get_previous_epoch, get_randao_mix, }; use crate::beacon::helpers::altair::{add_flag, base_reward_per_increment, has_flag}; use crate::beacon::helpers::capella::withdrawal_address; @@ -489,6 +489,7 @@ pub fn process_voluntary_exit( state: &mut BeaconState, signed_voluntary_exit: &SignedVoluntaryExit, config: &Config, + active_balances: &ActiveBalanceCache, ) -> Result<()> { let voluntary_exit = signed_voluntary_exit.message; let current_epoch = get_current_epoch(state); @@ -538,7 +539,12 @@ pub fn process_voluntary_exit( )?; // [Modified in Electra:EIP7251] - electra_initiate_validator_exit(state, voluntary_exit.validator_index, config)?; + electra_initiate_validator_exit( + state, + voluntary_exit.validator_index, + config, + Some(active_balances), + )?; Ok(()) } @@ -1188,15 +1194,23 @@ pub fn process_execution_requests( state: &mut BeaconState, requests: &electra::ExecutionRequests, config: &Config, + active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, ) -> Result<()> { for deposit in requests.deposits.iter() { process_deposit_request(state, deposit)?; } for withdrawal in requests.withdrawals.iter() { - process_withdrawal_request(state, withdrawal, config)?; + process_withdrawal_request(state, withdrawal, config, active_balances, pubkey_indices)?; } for consolidation in requests.consolidations.iter() { - process_consolidation_request(state, consolidation, config)?; + process_consolidation_request( + state, + consolidation, + config, + active_balances, + pubkey_indices, + )?; } Ok(()) } @@ -1307,6 +1321,8 @@ pub fn process_withdrawal_request( state: &mut BeaconState, request: &electra::WithdrawalRequest, config: &Config, + active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, ) -> Result<()> { let amount = request.amount; let is_full_exit_request = amount == constants::FULL_EXIT_REQUEST_AMOUNT; @@ -1321,11 +1337,7 @@ pub fn process_withdrawal_request( return Ok(()); } - let Some(index) = state - .iter_validators() - .position(|validator| validator.pubkey == request.validator_pubkey) - .map(|index| index as ValidatorIndex) - else { + let Some(index) = pubkey_indices.validator_index(state, &request.validator_pubkey) else { return Ok(()); }; let validator = state.validator(index)?; @@ -1360,7 +1372,7 @@ pub fn process_withdrawal_request( // queue; either way, a full exit request never falls through to the // partial-withdrawal logic below. if pending_balance_to_withdraw == 0 { - electra_initiate_validator_exit(state, index, config)?; + electra_initiate_validator_exit(state, index, config, Some(active_balances))?; } return Ok(()); } @@ -1388,7 +1400,8 @@ pub fn process_withdrawal_request( "state.balances[index] - MIN_ACTIVATION_BALANCE - pending_balance_to_withdraw", ))? .min(amount); - let exit_queue_epoch = compute_exit_epoch_and_update_churn(state, to_withdraw, config)?; + let exit_queue_epoch = + compute_exit_epoch_and_update_churn(state, to_withdraw, config, Some(active_balances))?; let withdrawable_epoch = exit_queue_epoch .checked_add(config.min_validator_withdrawability_delay) .ok_or(Error::ArithmeticOverflow( @@ -1425,17 +1438,16 @@ pub fn process_withdrawal_request( pub fn is_valid_switch_to_compounding_request( state: &BeaconState, request: &electra::ConsolidationRequest, + pubkey_indices: &PubkeyIndexCache, ) -> Result { if request.source_pubkey != request.target_pubkey { return Ok(false); } - let Some(source_validator) = state - .iter_validators() - .find(|validator| validator.pubkey == request.source_pubkey) - else { + let Some(source_index) = pubkey_indices.validator_index(state, &request.source_pubkey) else { return Ok(false); }; + let source_validator = state.validator(source_index)?; if withdrawal_address(source_validator) != request.source_address { return Ok(false); @@ -1476,14 +1488,14 @@ pub fn process_consolidation_request( state: &mut BeaconState, request: &electra::ConsolidationRequest, config: &Config, + active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, ) -> Result<()> { - if is_valid_switch_to_compounding_request(state, request)? { + if is_valid_switch_to_compounding_request(state, request, pubkey_indices)? { // Already known to resolve, by the check just above; state has not // been mutated since. - let source_index = state - .iter_validators() - .position(|validator| validator.pubkey == request.source_pubkey) - .map(|index| index as ValidatorIndex) + let source_index = pubkey_indices + .validator_index(state, &request.source_pubkey) .ok_or(Error::SpecAssert( "is_valid_switch_to_compounding_request already resolved consolidation_request.source_pubkey", ))?; @@ -1504,22 +1516,16 @@ pub fn process_consolidation_request( if pending_consolidations_len == preset::PENDING_CONSOLIDATIONS_LIMIT { return Ok(()); } - if get_consolidation_churn_limit_for_fork(state, config)? <= preset::MIN_ACTIVATION_BALANCE { + if get_consolidation_churn_limit_for_fork(state, config, Some(active_balances))? + <= preset::MIN_ACTIVATION_BALANCE + { return Ok(()); } - let Some(source_index) = state - .iter_validators() - .position(|validator| validator.pubkey == request.source_pubkey) - .map(|index| index as ValidatorIndex) - else { + let Some(source_index) = pubkey_indices.validator_index(state, &request.source_pubkey) else { return Ok(()); }; - let Some(target_index) = state - .iter_validators() - .position(|validator| validator.pubkey == request.target_pubkey) - .map(|index| index as ValidatorIndex) - else { + let Some(target_index) = pubkey_indices.validator_index(state, &request.target_pubkey) else { return Ok(()); }; @@ -1568,6 +1574,7 @@ pub fn process_consolidation_request( state, source_effective_balance, config, + Some(active_balances), )?; let withdrawable_epoch = exit_epoch .checked_add(config.min_validator_withdrawability_delay) @@ -1785,6 +1792,7 @@ pub fn process_operations( config: &Config, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, ) -> Result<()> { let deposit_requests_start_index = block_ref(state, "process_operations")?.deposit_requests_start_index(); @@ -1817,13 +1825,19 @@ pub fn process_operations( } // [Modified in Electra:EIP7251] for voluntary_exit in body.voluntary_exits.iter() { - process_voluntary_exit(state, voluntary_exit, config)?; + process_voluntary_exit(state, voluntary_exit, config, active_balances)?; } for signed_change in body.bls_to_execution_changes.iter() { process_bls_to_execution_change(state, signed_change, config)?; } // [New in Electra:EIP6110:EIP7002:EIP7251] - process_execution_requests(state, &body.execution_requests, config)?; + process_execution_requests( + state, + &body.execution_requests, + config, + active_balances, + pubkey_indices, + )?; Ok(()) } @@ -1845,6 +1859,7 @@ pub fn process_operations( /// them taking those fields directly rather than a whole body (see /// `crate::beacon::stf`'s module documentation). [`super::altair::process_sync_aggregate`] /// is reused the same way, for the same reason. +#[allow(clippy::too_many_arguments)] pub fn process_block( state: &mut BeaconState, block: &electra::BeaconBlock, @@ -1852,6 +1867,7 @@ pub fn process_block( engine: &ExecutionEngine, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, options: super::block::BlockProcessingOptions, ) -> Result<()> { super::block::process_block_header( @@ -1865,8 +1881,20 @@ pub fn process_block( process_execution_payload(state, &block.body, config, engine)?; super::block::process_randao(state, &block.body.randao_reveal, options.randao)?; super::block::process_eth1_data(state, &block.body.eth1_data)?; - process_operations(state, &block.body, config, committees, active_balances)?; - super::altair::process_sync_aggregate(state, &block.body.sync_aggregate, active_balances)?; + process_operations( + state, + &block.body, + config, + committees, + active_balances, + pubkey_indices, + )?; + super::altair::process_sync_aggregate( + state, + &block.body.sync_aggregate, + active_balances, + pubkey_indices, + )?; Ok(()) } @@ -1944,8 +1972,14 @@ mod tests { validator_pubkey: BlsPubkey([0xff; 48]), amount: constants::FULL_EXIT_REQUEST_AMOUNT, }; - process_withdrawal_request(&mut state, &request, &config) - .expect("an unresolved pubkey is dropped silently, not rejected"); + process_withdrawal_request( + &mut state, + &request, + &config, + &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), + ) + .expect("an unresolved pubkey is dropped silently, not rejected"); assert_eq!( state, before, "no validator should have been touched for a pubkey nobody holds" @@ -1966,8 +2000,14 @@ mod tests { validator_pubkey: pubkey, amount: constants::FULL_EXIT_REQUEST_AMOUNT, }; - process_withdrawal_request(&mut state, &request, &config) - .expect("a mismatched source address is dropped silently, not rejected"); + process_withdrawal_request( + &mut state, + &request, + &config, + &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), + ) + .expect("a mismatched source address is dropped silently, not rejected"); assert_eq!(state, before, "no validator should have been touched"); } @@ -1982,7 +2022,14 @@ mod tests { validator_pubkey: pubkey, amount: constants::FULL_EXIT_REQUEST_AMOUNT, }; - process_withdrawal_request(&mut state, &request, &config).unwrap(); + process_withdrawal_request( + &mut state, + &request, + &config, + &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), + ) + .unwrap(); assert_ne!( state.validator(0).unwrap().exit_epoch, @@ -2040,8 +2087,14 @@ mod tests { validator_pubkey: pubkey, amount: 1, }; - process_withdrawal_request(&mut state, &partial_request, &config) - .expect("a full queue drops a partial request silently, not with an error"); + process_withdrawal_request( + &mut state, + &partial_request, + &config, + &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), + ) + .expect("a full queue drops a partial request silently, not with an error"); assert_eq!( block_mut(&mut state, "test assertion") .unwrap() @@ -2058,7 +2111,14 @@ mod tests { validator_pubkey: pubkey, amount: constants::FULL_EXIT_REQUEST_AMOUNT, }; - process_withdrawal_request(&mut state, &full_exit_request, &config).unwrap(); + process_withdrawal_request( + &mut state, + &full_exit_request, + &config, + &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), + ) + .unwrap(); assert_ne!( state.validator(1).unwrap().exit_epoch, FAR_FUTURE_EPOCH, @@ -2092,7 +2152,14 @@ mod tests { validator_pubkey: pubkey, amount: constants::FULL_EXIT_REQUEST_AMOUNT, }; - process_withdrawal_request(&mut state, &full_exit_request, &config).unwrap(); + process_withdrawal_request( + &mut state, + &full_exit_request, + &config, + &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), + ) + .unwrap(); assert_ne!( state.validator(1).unwrap().exit_epoch, FAR_FUTURE_EPOCH, @@ -2117,7 +2184,14 @@ mod tests { validator_pubkey: pubkey, amount: 500_000_000, }; - process_withdrawal_request(&mut state, &request, &config).unwrap(); + process_withdrawal_request( + &mut state, + &request, + &config, + &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), + ) + .unwrap(); let queued = crate::beacon::helpers::electra::get_pending_balance_to_withdraw(&state, 0).unwrap(); @@ -2144,7 +2218,14 @@ mod tests { validator_pubkey: pubkey, amount: 500_000_000, }; - process_withdrawal_request(&mut state, &request, &config).unwrap(); + process_withdrawal_request( + &mut state, + &request, + &config, + &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), + ) + .unwrap(); let queued = crate::beacon::helpers::electra::get_pending_balance_to_withdraw(&state, 0).unwrap(); @@ -2167,7 +2248,14 @@ mod tests { source_pubkey: pubkey, target_pubkey: pubkey, }; - process_consolidation_request(&mut state, &request, &config).unwrap(); + process_consolidation_request( + &mut state, + &request, + &config, + &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), + ) + .unwrap(); assert!(has_compounding_withdrawal_credential( state.validator(0).unwrap() @@ -2196,8 +2284,14 @@ mod tests { source_pubkey: pubkey, target_pubkey: pubkey, }; - process_consolidation_request(&mut state, &request, &config) - .expect("neither branch applies, so this must be a silent no-op, not an error"); + process_consolidation_request( + &mut state, + &request, + &config, + &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), + ) + .expect("neither branch applies, so this must be a silent no-op, not an error"); assert_eq!(state, before, "no validator should have been touched"); } @@ -2232,8 +2326,14 @@ mod tests { source_pubkey, target_pubkey, }; - process_consolidation_request(&mut state, &request, &config) - .expect("a full queue drops the request silently, not with an error"); + process_consolidation_request( + &mut state, + &request, + &config, + &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), + ) + .expect("a full queue drops the request silently, not with an error"); assert_eq!( pending_queue_fields(&mut state, "test assertion") @@ -2280,7 +2380,14 @@ mod tests { source_pubkey, target_pubkey, }; - process_consolidation_request(&mut state, &request, &config).unwrap(); + process_consolidation_request( + &mut state, + &request, + &config, + &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), + ) + .unwrap(); assert_ne!( state.validator(0).unwrap().exit_epoch, diff --git a/crates/blockchain/state_transition/src/beacon/stf/epoch/electra.rs b/crates/blockchain/state_transition/src/beacon/stf/epoch/electra.rs index 672d78663..b0f4fc679 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/epoch/electra.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/epoch/electra.rs @@ -220,7 +220,7 @@ pub fn process_registry_updates(state: &mut BeaconState, config: &Config) -> Res Some(RegistryAction::QueueForActivation) => { state.validator_mut(index)?.activation_eligibility_epoch = current_epoch + 1; } - Some(RegistryAction::Eject) => initiate_validator_exit(state, index, config)?, + Some(RegistryAction::Eject) => initiate_validator_exit(state, index, config, None)?, Some(RegistryAction::Activate) => { state.validator_mut(index)?.activation_epoch = activation_epoch; } diff --git a/crates/blockchain/state_transition/src/beacon/stf/fulu.rs b/crates/blockchain/state_transition/src/beacon/stf/fulu.rs index 1a8bc3b58..0a299d36e 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/fulu.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/fulu.rs @@ -44,7 +44,7 @@ use crate::beacon::config::Config; use crate::beacon::containers::{BeaconState, deneb, electra}; use crate::beacon::error::{Result, verify}; use crate::beacon::helpers::accessors::{ - ActiveBalanceCache, CommitteeCache, get_current_epoch, get_randao_mix, + ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, get_current_epoch, get_randao_mix, }; use crate::beacon::helpers::electra::pending_queue_fields; use crate::beacon::helpers::fulu::{fulu_state, fulu_state_ref}; @@ -61,6 +61,7 @@ use super::ExecutionEngine; /// for [`super::electra::process_execution_payload`] and /// [`super::electra::process_operations`]. See the module docs for why these /// are the only steps this fork's specification asks to change. +#[allow(clippy::too_many_arguments)] pub fn process_block( state: &mut BeaconState, block: &electra::BeaconBlock, @@ -68,6 +69,7 @@ pub fn process_block( engine: &ExecutionEngine, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, options: super::block::BlockProcessingOptions, ) -> Result<()> { super::block::process_block_header( @@ -81,8 +83,20 @@ pub fn process_block( process_execution_payload(state, &block.body, config, engine)?; super::block::process_randao(state, &block.body.randao_reveal, options.randao)?; super::block::process_eth1_data(state, &block.body.eth1_data)?; - process_operations(state, &block.body, config, committees, active_balances)?; - super::altair::process_sync_aggregate(state, &block.body.sync_aggregate, active_balances)?; + process_operations( + state, + &block.body, + config, + committees, + active_balances, + pubkey_indices, + )?; + super::altair::process_sync_aggregate( + state, + &block.body.sync_aggregate, + active_balances, + pubkey_indices, + )?; Ok(()) } @@ -224,6 +238,7 @@ pub fn process_operations( config: &Config, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, ) -> Result<()> { // [Modified in Fulu] verify(body.deposits.is_empty(), "len(body.deposits) == 0")?; @@ -240,12 +255,18 @@ pub fn process_operations( // [Modified in Fulu] // Removed `process_deposit` for voluntary_exit in body.voluntary_exits.iter() { - super::electra::process_voluntary_exit(state, voluntary_exit, config)?; + super::electra::process_voluntary_exit(state, voluntary_exit, config, active_balances)?; } for signed_change in body.bls_to_execution_changes.iter() { super::electra::process_bls_to_execution_change(state, signed_change, config)?; } - process_execution_requests(state, &body.execution_requests, config)?; + process_execution_requests( + state, + &body.execution_requests, + config, + active_balances, + pubkey_indices, + )?; Ok(()) } @@ -262,15 +283,29 @@ pub fn process_execution_requests( state: &mut BeaconState, requests: &electra::ExecutionRequests, config: &Config, + active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, ) -> Result<()> { for deposit in requests.deposits.iter() { process_deposit_request(state, deposit)?; } for withdrawal in requests.withdrawals.iter() { - super::electra::process_withdrawal_request(state, withdrawal, config)?; + super::electra::process_withdrawal_request( + state, + withdrawal, + config, + active_balances, + pubkey_indices, + )?; } for consolidation in requests.consolidations.iter() { - super::electra::process_consolidation_request(state, consolidation, config)?; + super::electra::process_consolidation_request( + state, + consolidation, + config, + active_balances, + pubkey_indices, + )?; } Ok(()) } diff --git a/crates/blockchain/state_transition/src/beacon/stf/gloas.rs b/crates/blockchain/state_transition/src/beacon/stf/gloas.rs index 0ef4c1c8c..d0cbc0298 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/gloas.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/gloas.rs @@ -54,7 +54,7 @@ use crate::beacon::containers::shared::{DepositMessage, ProposerSlashing}; use crate::beacon::containers::{BeaconState, capella, gloas}; use crate::beacon::error::{Error, Result, verify}; use crate::beacon::helpers::accessors::{ - ActiveBalanceCache, ActiveBalanceCacheExt, CommitteeCache, CommitteeCacheExt, + ActiveBalanceCache, ActiveBalanceCacheExt, CommitteeCache, CommitteeCacheExt, PubkeyIndexCache, get_beacon_proposer_index, get_block_root_at_slot, get_current_epoch, get_domain, get_previous_epoch, get_randao_mix, }; @@ -104,13 +104,14 @@ pub fn process_block( config: &Config, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, options: super::block::BlockProcessingOptions, ) -> Result<()> { // [New in Gloas:EIP7732] let parent_slot = state.latest_block_header().slot; // [New in Gloas:EIP7732] - process_parent_execution_payload(state, block, config)?; + process_parent_execution_payload(state, block, config, active_balances, pubkey_indices)?; super::block::process_block_header( state, block.slot, @@ -134,7 +135,12 @@ pub fn process_block( committees, active_balances, )?; - super::altair::process_sync_aggregate(state, &block.body.sync_aggregate, active_balances)?; + super::altair::process_sync_aggregate( + state, + &block.body.sync_aggregate, + active_balances, + pubkey_indices, + )?; Ok(()) } @@ -904,6 +910,8 @@ pub fn apply_parent_execution_payload( state: &mut BeaconState, requests: &gloas::ExecutionRequests, config: &Config, + active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, ) -> Result<()> { let (parent_bid, parent_slot, parent_epoch) = { let inner = gloas_state_ref(state, "apply_parent_execution_payload")?; @@ -943,10 +951,22 @@ pub fn apply_parent_execution_payload( crate::beacon::stf::fulu::process_deposit_request(state, deposit)?; } for withdrawal in requests.withdrawals.iter() { - crate::beacon::stf::electra::process_withdrawal_request(state, withdrawal, config)?; + crate::beacon::stf::electra::process_withdrawal_request( + state, + withdrawal, + config, + active_balances, + pubkey_indices, + )?; } for consolidation in requests.consolidations.iter() { - crate::beacon::stf::electra::process_consolidation_request(state, consolidation, config)?; + crate::beacon::stf::electra::process_consolidation_request( + state, + consolidation, + config, + active_balances, + pubkey_indices, + )?; } for builder_deposit in requests.builder_deposits.iter() { process_builder_deposit_request(state, builder_deposit, config)?; @@ -1010,6 +1030,8 @@ pub fn process_parent_execution_payload( state: &mut BeaconState, block: &gloas::BeaconBlock, config: &Config, + active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, ) -> Result<()> { let bid = &block.body.signed_execution_payload_bid.message; let parent_bid = gloas_state_ref(state, "process_parent_execution_payload")? @@ -1031,7 +1053,7 @@ pub fn process_parent_execution_payload( requests.hash_tree_root() == parent_bid.execution_requests_root, "process_parent_execution_payload: hash_tree_root(requests) == parent_bid.execution_requests_root", )?; - apply_parent_execution_payload(state, requests, config) + apply_parent_execution_payload(state, requests, config, active_balances, pubkey_indices) } // --------------------------------------------------------------------------- @@ -1632,7 +1654,12 @@ pub fn process_operations( process_attestation(state, attestation, parent_slot, committees, active_balances)?; } for voluntary_exit in body.voluntary_exits.iter() { - crate::beacon::stf::electra::process_voluntary_exit(state, voluntary_exit, config)?; + crate::beacon::stf::electra::process_voluntary_exit( + state, + voluntary_exit, + config, + active_balances, + )?; } for signed_change in body.bls_to_execution_changes.iter() { crate::beacon::stf::capella::process_bls_to_execution_change(state, signed_change, config)?; diff --git a/crates/blockchain/state_transition/src/beacon/stf/mod.rs b/crates/blockchain/state_transition/src/beacon/stf/mod.rs index 3dba6eef8..f87ffdc6c 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/mod.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/mod.rs @@ -67,7 +67,9 @@ pub mod operations; use crate::beacon::containers; use crate::beacon::containers::{BeaconState, phase0}; use crate::beacon::error::{Error, Result, verify}; -use crate::beacon::helpers::accessors::{ActiveBalanceCache, CommitteeCache, get_domain}; +use crate::beacon::helpers::accessors::{ + ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, get_domain, +}; use crate::beacon::helpers::misc::compute_signing_root; use crate::beacon::lean_state_unreachable; use crate::beacon::preset; @@ -127,6 +129,7 @@ impl ExecutionEngine { /// specification requires instead of failing on it. Rejecting the mismatch /// before any of that runs is what keeps the failure legible: "wrong fork," not /// some unrelated-looking assertion three steps later. +#[allow(clippy::too_many_arguments)] pub fn state_transition( state: &mut BeaconState, signed_block: &containers::SignedBeaconBlock, @@ -135,6 +138,7 @@ pub fn state_transition( engine: &ExecutionEngine, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, ) -> Result<()> { process_slots(state, signed_block.slot(), config)?; apply_block( @@ -145,6 +149,7 @@ pub fn state_transition( engine, committees, active_balances, + pubkey_indices, ) } @@ -157,6 +162,7 @@ pub fn state_transition( /// when the block sits exactly on the boundary. A state at the wrong slot is /// still refused: the block header check in `process_block` compares the /// block's slot with the state's. +#[allow(clippy::too_many_arguments)] pub fn apply_block( state: &mut BeaconState, signed_block: &containers::SignedBeaconBlock, @@ -165,6 +171,7 @@ pub fn apply_block( engine: &ExecutionEngine, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + pubkey_indices: &PubkeyIndexCache, ) -> Result<()> { // After `process_slots`, never before it. A block proposed at the first slot // of a fork's activation epoch is the *post*-fork shape while the state @@ -191,6 +198,7 @@ pub fn apply_block( engine, committees, active_balances, + pubkey_indices, block::BlockProcessingOptions::default(), )?; // As in `process_slot`: the post-state's root, checked below and needed diff --git a/crates/blockchain/state_transition/src/metrics.rs b/crates/blockchain/state_transition/src/metrics.rs index 63c029166..2b2cabb8a 100644 --- a/crates/blockchain/state_transition/src/metrics.rs +++ b/crates/blockchain/state_transition/src/metrics.rs @@ -80,6 +80,24 @@ pub fn inc_total_active_balance_lookups(result: &str) { .inc(); } +static LEAN_BEACON_PUBKEY_INDEX_LOOKUPS_TOTAL: LazyLock = LazyLock::new(|| { + register_int_counter_vec!( + "lean_beacon_pubkey_index_lookups_total", + "Pubkey to validator-index lookups in block processing, by whether the pubkey index cache served them", + &["result"] + ) + .unwrap() +}); + +/// Count `count` `PubkeyIndexCache` lookups: `hit` (a verified cache entry) or +/// `fallback` (the registry was searched, whether or not the pubkey was +/// found). +pub fn inc_pubkey_index_lookups(result: &str, count: u64) { + LEAN_BEACON_PUBKEY_INDEX_LOOKUPS_TOTAL + .with_label_values(&[result]) + .inc_by(count); +} + static LEAN_BEACON_JUSTIFIED_BALANCES_LOOKUPS_TOTAL: LazyLock = LazyLock::new( || { register_int_counter_vec!( diff --git a/crates/blockchain/state_transition/tests/beacon_spec/operations.rs b/crates/blockchain/state_transition/tests/beacon_spec/operations.rs index e627daeba..49fa4d8c8 100644 --- a/crates/blockchain/state_transition/tests/beacon_spec/operations.rs +++ b/crates/blockchain/state_transition/tests/beacon_spec/operations.rs @@ -131,7 +131,9 @@ use ethlambda_state_transition::beacon::config::Config; use ethlambda_state_transition::beacon::containers::{ BeaconState, altair, bellatrix, capella, deneb, electra, gloas, phase0, shared, }; -use ethlambda_state_transition::beacon::helpers::accessors::{ActiveBalanceCache, CommitteeCache}; +use ethlambda_state_transition::beacon::helpers::accessors::{ + ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, +}; use ethlambda_state_transition::beacon::primitives::{HashTreeRoot as _, Slot}; use ethlambda_state_transition::beacon::stf::altair as altair_stf; use ethlambda_state_transition::beacon::stf::bellatrix as bellatrix_stf; @@ -178,6 +180,7 @@ fn apply( // is nothing for the cache to share between calls. let committees = CommitteeCache::default(); let active_balances = ActiveBalanceCache::default(); + let pubkey_indices = PubkeyIndexCache::default(); let outcome = match handler { // Every fork's own `BeaconBlock` carries a different concrete body // (altair's adds a sync aggregate, bellatrix's an execution payload, @@ -422,7 +425,7 @@ fn apply( // `process_voluntary_exit`; see that function's own doc). ForkName::Electra | ForkName::Fulu | ForkName::Gloas => { let exit: shared::SignedVoluntaryExit = case.ssz("voluntary_exit"); - electra_stf::process_voluntary_exit(state, &exit, config) + electra_stf::process_voluntary_exit(state, &exit, config, &active_balances) } ForkName::Lean => lean_is_not_a_fixture_fork("voluntary_exit"), }, @@ -430,7 +433,12 @@ fn apply( // further fork match. "sync_aggregate" => { let sync_aggregate: altair::SyncAggregate = case.ssz("sync_aggregate"); - altair_stf::process_sync_aggregate(state, &sync_aggregate, &active_balances) + altair_stf::process_sync_aggregate( + state, + &sync_aggregate, + &active_balances, + &pubkey_indices, + ) } // The fixture's operation file is a whole `BeaconBlockBody`, not a // bare payload: checking one needs fields that live on the body @@ -563,7 +571,13 @@ fn apply( // `BeaconBlock`, named `block` like `block_header`'s. "parent_execution_payload" => { let block: gloas::BeaconBlock = case.ssz("block"); - gloas_stf::process_parent_execution_payload(state, &block, config) + gloas_stf::process_parent_execution_payload( + state, + &block, + config, + &active_balances, + &pubkey_indices, + ) } // `consolidation_request` and `withdrawal_request` are new in // electra, alongside `withdrawals`; neither fulu's nor gloas's own @@ -576,7 +590,13 @@ fn apply( // match is needed for either the type or the call. "consolidation_request" => { let request: electra::ConsolidationRequest = case.ssz("consolidation_request"); - electra_stf::process_consolidation_request(state, &request, config) + electra_stf::process_consolidation_request( + state, + &request, + config, + &active_balances, + &pubkey_indices, + ) } // `deposit_request` is new in electra too, but fulu's specification // does list a modified version (`beacon-chain.md`'s "Modified @@ -609,7 +629,13 @@ fn apply( }, "withdrawal_request" => { let request: electra::WithdrawalRequest = case.ssz("withdrawal_request"); - electra_stf::process_withdrawal_request(state, &request, config) + electra_stf::process_withdrawal_request( + state, + &request, + config, + &active_balances, + &pubkey_indices, + ) } // New in gloas (EIP-8282): the builder registry's own onboarding and // exit requests, applied by `apply_parent_execution_payload` the same diff --git a/crates/blockchain/state_transition/tests/beacon_spec/sanity.rs b/crates/blockchain/state_transition/tests/beacon_spec/sanity.rs index 7d85ee935..bd615f24a 100644 --- a/crates/blockchain/state_transition/tests/beacon_spec/sanity.rs +++ b/crates/blockchain/state_transition/tests/beacon_spec/sanity.rs @@ -15,7 +15,9 @@ use ethlambda_state_transition::beacon::config::Config; use ethlambda_state_transition::beacon::containers::{BeaconState, SignedBeaconBlock}; -use ethlambda_state_transition::beacon::helpers::accessors::{ActiveBalanceCache, CommitteeCache}; +use ethlambda_state_transition::beacon::helpers::accessors::{ + ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, +}; use ethlambda_state_transition::beacon::stf::{self, ExecutionEngine}; use libtest_mimic::Trial; @@ -49,6 +51,7 @@ fn apply_blocks(case: &Case, state: &mut BeaconState, config: &Config) -> Result // imports, so consecutive blocks of an epoch share its shuffling. let committees = CommitteeCache::default(); let active_balances = ActiveBalanceCache::default(); + let pubkey_indices = PubkeyIndexCache::default(); for index in 0..meta.blocks_count { let bytes = case.ssz_bytes_indexed("blocks", index); @@ -67,6 +70,7 @@ fn apply_blocks(case: &Case, state: &mut BeaconState, config: &Config) -> Result &engine, &committees, &active_balances, + &pubkey_indices, ) .map_err(|err| format!("block {index} rejected: {err:?}"))?; diff --git a/crates/blockchain/state_transition/tests/beacon_spec/transition.rs b/crates/blockchain/state_transition/tests/beacon_spec/transition.rs index dc6d5a2f1..ca6125181 100644 --- a/crates/blockchain/state_transition/tests/beacon_spec/transition.rs +++ b/crates/blockchain/state_transition/tests/beacon_spec/transition.rs @@ -69,7 +69,9 @@ use std::sync::Arc; use ethlambda_state_transition::beacon::ForkName; use ethlambda_state_transition::beacon::config::Config; use ethlambda_state_transition::beacon::containers::{BeaconState, SignedBeaconBlock}; -use ethlambda_state_transition::beacon::helpers::accessors::{ActiveBalanceCache, CommitteeCache}; +use ethlambda_state_transition::beacon::helpers::accessors::{ + ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, +}; use ethlambda_state_transition::beacon::helpers::misc::compute_start_slot_at_epoch; use ethlambda_state_transition::beacon::primitives::Epoch; use ethlambda_state_transition::beacon::stf::{self, ExecutionEngine}; @@ -139,6 +141,7 @@ fn apply_blocks( // imports, so consecutive blocks of an epoch share its shuffling. let committees = CommitteeCache::default(); let active_balances = ActiveBalanceCache::default(); + let pubkey_indices = PubkeyIndexCache::default(); for index in 0..meta.blocks_count { let fork = if index < first_post_fork_index { @@ -163,6 +166,7 @@ fn apply_blocks( &engine, &committees, &active_balances, + &pubkey_indices, ) .map_err(|err| format!("block {index} rejected: {err:?}"))?; diff --git a/crates/common/types/src/beacon/containers/mod.rs b/crates/common/types/src/beacon/containers/mod.rs index a7859de2e..c8b980768 100644 --- a/crates/common/types/src/beacon/containers/mod.rs +++ b/crates/common/types/src/beacon/containers/mod.rs @@ -799,6 +799,17 @@ impl BeaconState { } } + /// The validators from index `start` on, in index order, pending writes + /// included; empty if `start` is past the end. For a pass over a + /// registry's appended tail that must not walk the part before it. + pub fn iter_validators_from(&self, start: usize) -> impl ExactSizeIterator { + match self.registry() { + Some(Registry::Bounded(v, _)) => RegistryIter::Bounded(v.iter_from(start)), + Some(Registry::Progressive(v, _)) => RegistryIter::Progressive(v.iter_from(start)), + None => lean_state_unreachable("iter_validators_from"), + } + } + /// Every balance, in index order, pending writes included. See /// [`Self::iter_validators`] for why this is an iterator. pub fn iter_balances(&self) -> impl ExactSizeIterator { diff --git a/crates/net/rpc/src/beacon/validator_client_tests.rs b/crates/net/rpc/src/beacon/validator_client_tests.rs index 693450ebf..a6e323df5 100644 --- a/crates/net/rpc/src/beacon/validator_client_tests.rs +++ b/crates/net/rpc/src/beacon/validator_client_tests.rs @@ -1010,6 +1010,7 @@ fn assert_full_sync_aggregate( &mut state, aggregate, ðlambda_state_transition::beacon::helpers::accessors::ActiveBalanceCache::default(), + ðlambda_state_transition::beacon::helpers::accessors::PubkeyIndexCache::default(), ) .expect("the packed aggregate verifies as the state transition checks it"); } diff --git a/crates/storage/src/lib.rs b/crates/storage/src/lib.rs index 85ad30c5a..1b91825d9 100644 --- a/crates/storage/src/lib.rs +++ b/crates/storage/src/lib.rs @@ -7,6 +7,7 @@ mod error; mod liveness; mod metrics; pub mod pools; +mod pubkey_index_cache; mod state_codec; mod state_diff; mod state_writer; @@ -21,6 +22,7 @@ pub use committee_cache::{CommitteeCache, Lookup, ShufflingKey}; /// callers can match on it (e.g. to distinguish [`Error::DbVersionMismatch`]). pub use error::Error; pub use liveness::ObservedLiveness; +pub use pubkey_index_cache::{PubkeyIndexCache, TAIL_MERGE_THRESHOLD}; // `CacheKey` lives in `state_writer` (beside the `StateCache` it keys), not // `store`; re-exported here so the public path (`ethlambda_storage::CacheKey`) // is unaffected by which module owns it. diff --git a/crates/storage/src/pubkey_index_cache.rs b/crates/storage/src/pubkey_index_cache.rs new file mode 100644 index 000000000..4972165c0 --- /dev/null +++ b/crates/storage/src/pubkey_index_cache.rs @@ -0,0 +1,318 @@ +//! The pubkey to validator-index cache: a compact table answering "which +//! registry position holds this pubkey?" without walking the registry. +//! +//! A sibling of [`crate::active_balance_cache::ActiveBalanceCache`], held by +//! the `Store` for the same reasons (one table shared by the chain actor's +//! state transition and anything else holding a `Store` clone). A caller with +//! no `Store` of its own (a spec runner, a unit test, block production's +//! scratch state) holds a fresh [`PubkeyIndexCache::default`] for as long as +//! that work lasts. +//! +//! This module knows nothing about a `BeaconState`. It stores positions it is +//! told about and answers *candidates*; the consensus logic that feeds it a +//! state's registry and verifies a candidate against that state lives in +//! `ethlambda-state-transition`'s `beacon::helpers::accessors`, in the +//! `PubkeyIndexCacheExt` trait this type implements there. +//! +//! # Why a candidate, not an answer +//! +//! The table is process-wide, so it is fed by whichever state asked first. The +//! registry is append-only along one chain, so a pubkey's index never changes +//! there; across forks an unfinalized index may differ between branches, and +//! two keys can share a [`PubkeyIndexCache::key_of`] prefix. Both only ever +//! make a candidate wrong, never make a right one disappear, so the caller +//! checks every candidate against its own state and falls back to a scan when +//! none passes. A missing or wrong entry is a slower lookup, never a wrong +//! validator. +//! +//! # Layout +//! +//! A pubkey is keyed by eight bytes of itself ([`PubkeyIndexCache::key_of`]), +//! not all 48: that is 12 bytes per validator in the sorted base (an 8 byte +//! key and a 4 byte index) instead of 52, and eight bytes taken from the +//! uniformly distributed part of a compressed point make a collision between +//! two real keys negligible. +//! +//! Entries live in a sorted base (two parallel arrays, binary searched) plus +//! a small hash map holding positions appended since the base was last built. +//! The registry grows by a handful of validators a block at most, so the tail +//! absorbs that growth cheaply; when it reaches [`TAIL_MERGE_THRESHOLD`] it is +//! merged into the base in one linear pass. + +use std::collections::HashMap; +use std::sync::RwLock; + +use ethlambda_types::beacon::primitives::BlsPubkey; + +/// How many appended entries the hash map tail holds before it is merged into +/// the sorted base. +/// +/// A merge is linear in the whole table, so it must be rare: this many appends +/// is on the order of the validators a mainnet day adds. It is also small +/// enough that the tail's per-entry cost (a hash map slot, several times a +/// base entry) stays a rounding error next to the base. +pub const TAIL_MERGE_THRESHOLD: usize = 8192; + +/// Pubkey-prefix to validator-index table. See the module documentation. +/// +/// Internally synchronized, so every method takes `&self`. The write lock is +/// held only to splice entries in, never while a state is scanned: callers +/// collect a state's tail into a buffer first and hand it over. +#[derive(Debug, Default)] +pub struct PubkeyIndexCache { + inner: RwLock, +} + +#[derive(Debug, Default)] +struct Inner { + /// Sorted ascending by `(key, index)`; `indices[i]` belongs to `keys[i]`. + /// Equal keys (a prefix collision, or one pubkey seen at two indices on + /// two forks) sit next to each other and are all returned as candidates. + keys: Vec, + indices: Vec, + /// Positions appended since the base was built. First writer wins for a + /// key; a second candidate for the same key just costs the caller a scan. + tail: HashMap, + /// How many registry positions have been ingested: positions + /// `0..covered_len` of some chain's registry, in order. + covered_len: usize, +} + +impl PubkeyIndexCache { + /// The table key for `pubkey`: bytes `1..9` as a big-endian integer. + /// + /// Byte 0 carries the compressed-point flag bits, which are the same for + /// nearly every key, so it is skipped; the next eight bytes are uniformly + /// distributed. + pub fn key_of(pubkey: &BlsPubkey) -> u64 { + let mut prefix = [0u8; 8]; + prefix.copy_from_slice(&pubkey.0[1..9]); + u64::from_be_bytes(prefix) + } + + /// How many registry positions have been ingested. + pub fn covered_len(&self) -> usize { + self.inner.read().unwrap().covered_len + } + + /// The indices recorded under `pubkey`'s key: usually one, empty for a + /// pubkey the table has never seen. Each must be verified against the + /// caller's own registry. + pub fn candidates(&self, pubkey: &BlsPubkey) -> Vec { + let key = Self::key_of(pubkey); + let inner = self.inner.read().unwrap(); + let start = inner.keys.partition_point(|candidate| *candidate < key); + let mut found: Vec = inner.keys[start..] + .iter() + .zip(&inner.indices[start..]) + .take_while(|(candidate, _)| **candidate == key) + .map(|(_, index)| u64::from(*index)) + .collect(); + if let Some(index) = inner.tail.get(&key) { + found.push(u64::from(*index)); + } + found + } + + /// Records the pubkeys of registry positions `start..start + keys.len()`, + /// where `keys` are [`Self::key_of`] values in position order. + /// + /// Positions already covered are skipped, so two callers that both saw the + /// table short and both scanned the same tail do not double-insert. A + /// `start` past `covered_len` would leave a gap that nothing could later + /// fill in order, so it is refused (returns `false`); callers pass the + /// `covered_len` they read, which only grows. + /// + /// A position whose index does not fit the table's `u32` is counted as + /// covered but not stored: such a lookup finds no candidate and takes the + /// caller's scan, which is correct and unreachable in practice. + pub fn extend(&self, start: usize, keys: &[u64]) -> bool { + let mut inner = self.inner.write().unwrap(); + if start > inner.covered_len { + return false; + } + let skip = (inner.covered_len - start).min(keys.len()); + let fresh = &keys[skip..]; + let first_position = inner.covered_len; + let entries = fresh.iter().enumerate().filter_map(|(offset, key)| { + u32::try_from(first_position + offset) + .ok() + .map(|index| (*key, index)) + }); + + if fresh.len() >= TAIL_MERGE_THRESHOLD { + // A bulk load (the first ingest of a whole registry): sorting it + // directly beats funnelling millions of entries through the map. + let mut batch: Vec<(u64, u32)> = entries.collect(); + batch.extend(inner.tail.drain()); + inner.merge(batch); + } else { + for (key, index) in entries { + inner.tail.entry(key).or_insert(index); + } + if inner.tail.len() >= TAIL_MERGE_THRESHOLD { + let batch: Vec<(u64, u32)> = inner.tail.drain().collect(); + inner.merge(batch); + } + } + inner.covered_len += fresh.len(); + true + } + + /// Heap bytes held by the table: capacity of the base arrays plus the + /// tail's slots. + pub fn heap_bytes(&self) -> usize { + let inner = self.inner.read().unwrap(); + inner.keys.capacity() * size_of::() + + inner.indices.capacity() * size_of::() + + inner.tail.capacity() * (size_of::() + size_of::()) + } +} + +impl Inner { + /// Merges `batch` into the sorted base, ordering ties by index so the + /// earlier registry position is returned first. + fn merge(&mut self, mut batch: Vec<(u64, u32)>) { + batch.sort_unstable(); + let total = self.keys.len() + batch.len(); + let mut keys = Vec::with_capacity(total); + let mut indices = Vec::with_capacity(total); + let (mut base, mut fresh) = (0, 0); + while base < self.keys.len() || fresh < batch.len() { + let take_base = fresh == batch.len() + || (base < self.keys.len() + && (self.keys[base], self.indices[base]) <= batch[fresh]); + if take_base { + keys.push(self.keys[base]); + indices.push(self.indices[base]); + base += 1; + } else { + keys.push(batch[fresh].0); + indices.push(batch[fresh].1); + fresh += 1; + } + } + self.keys = keys; + self.indices = indices; + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn pubkey(seed: u64) -> BlsPubkey { + let mut bytes = [0u8; 48]; + bytes[0] = 0x80; + bytes[1..9].copy_from_slice(&seed.to_be_bytes()); + BlsPubkey(bytes) + } + + fn keys(seeds: std::ops::Range) -> Vec { + seeds + .map(|seed| PubkeyIndexCache::key_of(&pubkey(seed))) + .collect() + } + + #[test] + fn a_small_extend_is_served_from_the_tail() { + let cache = PubkeyIndexCache::default(); + assert!(cache.extend(0, &keys(10..20))); + assert_eq!(cache.covered_len(), 10); + assert_eq!(cache.candidates(&pubkey(13)), vec![3]); + assert!(cache.candidates(&pubkey(99)).is_empty()); + } + + #[test] + fn a_bulk_extend_and_a_later_tail_both_resolve() { + let cache = PubkeyIndexCache::default(); + let bulk = TAIL_MERGE_THRESHOLD as u64 * 2; + assert!(cache.extend(0, &keys(1..bulk + 1))); + assert!(cache.extend(bulk as usize, &keys(bulk + 1..bulk + 4))); + assert_eq!(cache.covered_len(), bulk as usize + 3); + assert_eq!(cache.candidates(&pubkey(1)), vec![0]); + assert_eq!(cache.candidates(&pubkey(bulk)), vec![bulk - 1]); + assert_eq!(cache.candidates(&pubkey(bulk + 2)), vec![bulk + 1]); + } + + #[test] + fn a_tail_reaching_the_threshold_merges_into_the_base() { + let cache = PubkeyIndexCache::default(); + let step = TAIL_MERGE_THRESHOLD as u64 - 1; + assert!(cache.extend(0, &keys(0..step))); + assert_eq!(cache.inner.read().unwrap().keys.len(), 0); + assert!(cache.extend(step as usize, &keys(step..step + 5))); + { + let inner = cache.inner.read().unwrap(); + assert_eq!(inner.keys.len(), step as usize + 5); + assert!(inner.tail.is_empty()); + assert!(inner.keys.is_sorted()); + } + assert_eq!(cache.candidates(&pubkey(step + 4)), vec![step + 4]); + } + + #[test] + fn already_covered_positions_are_skipped_and_gaps_refused() { + let cache = PubkeyIndexCache::default(); + assert!(cache.extend(0, &keys(0..5))); + // A second caller that scanned the same tail. + assert!(cache.extend(3, &keys(3..8))); + assert_eq!(cache.covered_len(), 8); + assert_eq!(cache.candidates(&pubkey(4)), vec![4]); + assert!(!cache.extend(20, &keys(20..22))); + assert_eq!(cache.covered_len(), 8); + } + + #[test] + fn equal_keys_return_every_candidate_in_index_order() { + let cache = PubkeyIndexCache::default(); + let bulk = TAIL_MERGE_THRESHOLD as u64; + let mut all = keys(100..100 + bulk); + all[7] = all[2]; + assert!(cache.extend(0, &all)); + assert_eq!(cache.candidates(&pubkey(102)), vec![2, 7]); + } + + #[test] + fn the_base_costs_twelve_bytes_an_entry() { + let cache = PubkeyIndexCache::default(); + let count = TAIL_MERGE_THRESHOLD as u64 * 4; + assert!(cache.extend(0, &keys(0..count))); + assert_eq!(cache.heap_bytes(), count as usize * 12); + } + + /// Sizing check for a mainnet-scale registry; prints, asserts nothing + /// about time. Run with `-- --ignored --nocapture`. + #[test] + #[ignore = "measurement, not a check"] + fn a_mainnet_scale_ingest_measured() { + const COUNT: u64 = 2_400_000; + let mut state = 0x9E37_79B9_7F4A_7C15u64; + let keys: Vec = (0..COUNT) + .map(|_| { + state = state + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + state + }) + .collect(); + let cache = PubkeyIndexCache::default(); + let started = std::time::Instant::now(); + cache.extend(0, &keys); + let ingest = started.elapsed(); + let started = std::time::Instant::now(); + let hits = (0..COUNT as usize) + .step_by(1000) + .filter(|position| { + let key = keys[*position]; + let inner = cache.inner.read().unwrap(); + inner.keys.binary_search(&key).is_ok() + }) + .count(); + println!( + "ingest {COUNT} keys: {ingest:?}; {hits} lookups: {:?}; heap {} bytes", + started.elapsed(), + cache.heap_bytes() + ); + } +} diff --git a/crates/storage/src/store.rs b/crates/storage/src/store.rs index 14ac9116f..37b298022 100644 --- a/crates/storage/src/store.rs +++ b/crates/storage/src/store.rs @@ -12,6 +12,7 @@ use crate::committee_cache::CommitteeCache; use crate::error::Error; use crate::liveness::ObservedLiveness; use crate::pools::{AttestationPool, OperationPool}; +use crate::pubkey_index_cache::PubkeyIndexCache; use ethlambda_crypto::signature::ValidatorSignature; use ethlambda_types::{ @@ -1203,6 +1204,11 @@ pub struct Store { /// `beacon::helpers::accessors::ActiveBalanceCacheExt` for the /// state-aware half. Always empty on lean. active_balance_cache: Arc, + /// Pubkey to validator index, shared like [`Self::active_balance_cache`]. + /// See [`PubkeyIndexCache`], and `ethlambda-state-transition`'s + /// `beacon::helpers::accessors::PubkeyIndexCacheExt` for the state-aware + /// half. Always empty on lean. + pubkey_index_cache: Arc, /// Beacon fork-choice scratch. Empty and untouched on a lean chain. pub(crate) beacon: Arc>, /// The background writer, joined when the last clone of this `Store` @@ -1847,6 +1853,7 @@ impl Store { committee_cache: Arc::new(CommitteeCache::default()), observed_liveness: Arc::new(ObservedLiveness::default()), active_balance_cache: Arc::new(ActiveBalanceCache::default()), + pubkey_index_cache: Arc::new(PubkeyIndexCache::default()), beacon: Default::default(), state_writer, state_handoff: Default::default(), @@ -3301,6 +3308,13 @@ impl Store { Arc::clone(&self.active_balance_cache) } + /// The pubkey-index cache shared by every clone of this store. + /// + /// An owned `Arc` for the same reason as [`Self::committee_cache`]. + pub fn pubkey_index_cache(&self) -> Arc { + Arc::clone(&self.pubkey_index_cache) + } + /// Returns whether a state is available for the given block root. /// /// True if the state cache or `pending_states` holds the state, a snapshot diff --git a/docs/metrics.md b/docs/metrics.md index 7107778d1..5858fd0c8 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -514,7 +514,8 @@ not part of the leanMetrics spec. | Name | Type | Usage | Sample collection event | Labels | |------|------|-------|-------------------------|--------| -| `lean_beacon_total_active_balance_lookups_total` | Counter | Total active balance lookups, by whether the cache served them | On every `ActiveBalanceCacheExt::total_active_balance` call: block processing's attestations and sync aggregate | result=hit,miss,unkeyable | +| `lean_beacon_total_active_balance_lookups_total` | Counter | Total active balance lookups, by whether the cache served them | On every `ActiveBalanceCacheExt::total_active_balance` call: block processing's attestations, sync aggregate and exit/consolidation churn | result=hit,miss,unkeyable | +| `lean_beacon_pubkey_index_lookups_total` | Counter | Pubkey to validator-index lookups in block processing, by whether the pubkey index cache served them | Per sync committee seat, and per pubkey of a withdrawal or consolidation request; `fallback` is a registry search (also counted when the pubkey is not registered) | result=hit,fallback | Expect one miss per epoch per chain, and hits otherwise. `unkeyable` is the genesis state asking about its own epoch, and should be zero on a From bbec4ba27615752d0a376f7efc4366033f5b960c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Fri, 6 Nov 2026 15:07:12 -0300 Subject: [PATCH 2/2] perf(beacon): address review of the pubkey index cache Block production now resolves pubkeys through the store's shared PubkeyIndexCache. assemble_block, assemble_gloas_block(_on_bid) and gloas_payload_inputs take the table, and the RPC passes store.pubkey_index_cache(), so a proposal no longer ingests and sorts the whole registry into a fresh table on every state transition run. A fallback scan now repairs the table. validator_index records the index it found (PubkeyIndexCache::insert), process_sync_aggregate scans only the seats the cache could not verify and records them, and the tail keeps every index under a key (a BTreeSet instead of a first-writer-wins map), so a prefix twin or a position taken in from another fork costs one scan rather than one per block. extend builds and sorts its batch before taking the write lock; under the lock it only re-checks covered_len, drops what a racing caller ingested, and splices. Slashings thread the ActiveBalanceCache into slash_validator's exit, so a block slashing many validators reads the total active balance once, and pack_operations shares one table with its slashings. The churn helpers pick the per-fork formula first and compute the total once. The STF entry points (state_transition, apply_block, every fork's process_block, transition_block) take one stf::BlockCaches instead of three cache arguments, with OwnedBlockCaches for callers with no Store. That drops the eight too_many_arguments allows the PR added. switch_to_compounding_source replaces is_valid_switch_to_compounding_request and returns the source index, so the request resolves its pubkey once. The soundness notes on active_balance_key and in docs/beacon_stf.md now name every cached caller. TAIL_MERGE_THRESHOLD is private and heap_bytes is gone (a test helper measures the base instead). Claude-Session: https://claude.ai/code/session_011vghNj5DNZw778LTBJPURC --- bin/ethlambda/src/era/replay.rs | 17 +- .../src/beacon/block_production.rs | 57 +++-- .../src/beacon/fork_choice.rs | 55 ++--- .../src/beacon/gloas_block_production.rs | 155 ++++++++---- .../src/beacon/helpers/accessors.rs | 64 ++++- .../src/beacon/helpers/electra.rs | 57 ++--- .../src/beacon/helpers/gloas.rs | 12 +- .../src/beacon/helpers/mutators.rs | 79 +++--- .../state_transition/src/beacon/stf/altair.rs | 121 ++++++--- .../src/beacon/stf/bellatrix.rs | 19 +- .../state_transition/src/beacon/stf/block.rs | 129 +++------- .../src/beacon/stf/capella.rs | 17 +- .../state_transition/src/beacon/stf/deneb.rs | 31 ++- .../src/beacon/stf/electra.rs | 66 ++--- .../state_transition/src/beacon/stf/fulu.rs | 31 ++- .../state_transition/src/beacon/stf/gloas.rs | 31 ++- .../state_transition/src/beacon/stf/mod.rs | 64 +++-- .../src/beacon/stf/operations.rs | 32 ++- .../tests/beacon_spec/operations.rs | 17 +- .../tests/beacon_spec/sanity.rs | 22 +- .../tests/beacon_spec/transition.rs | 22 +- crates/net/rpc/src/beacon/gloas_proposal.rs | 65 ++++- crates/net/rpc/src/beacon/proposal.rs | 5 + crates/storage/src/lib.rs | 2 +- crates/storage/src/pubkey_index_cache.rs | 231 +++++++++++++----- docs/beacon_stf.md | 25 +- 26 files changed, 871 insertions(+), 555 deletions(-) diff --git a/bin/ethlambda/src/era/replay.rs b/bin/ethlambda/src/era/replay.rs index 7128e7085..25f7cedd7 100644 --- a/bin/ethlambda/src/era/replay.rs +++ b/bin/ethlambda/src/era/replay.rs @@ -18,10 +18,7 @@ use std::path::Path; use std::time::Instant; -use ethlambda_state_transition::beacon::helpers::accessors::{ - ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, -}; -use ethlambda_state_transition::beacon::stf::{self, ExecutionEngine}; +use ethlambda_state_transition::beacon::stf::{self, ExecutionEngine, OwnedBlockCaches}; use ethlambda_types::beacon::config::Config; use ethlambda_types::beacon::containers::{BeaconState, SignedBeaconBlock}; use ethlambda_types::beacon::preset; @@ -54,9 +51,7 @@ pub(super) struct Replay<'a> { config: &'a Config, engine: ExecutionEngine, /// One for the whole run, as the node holds one across its imports. - committees: CommitteeCache, - active_balances: ActiveBalanceCache, - pubkey_indices: PubkeyIndexCache, + caches: OwnedBlockCaches, max_blocks: Option, carried: Option, } @@ -67,9 +62,7 @@ impl<'a> Replay<'a> { network, config, engine: ExecutionEngine::valid(), - committees: CommitteeCache::default(), - active_balances: ActiveBalanceCache::default(), - pubkey_indices: PubkeyIndexCache::default(), + caches: OwnedBlockCaches::default(), max_blocks, carried: None, } @@ -171,9 +164,7 @@ impl<'a> Replay<'a> { true, self.config, &self.engine, - &self.committees, - &self.active_balances, - &self.pubkey_indices, + self.caches.caches(), ) .map_err(|err| { eyre!( diff --git a/crates/blockchain/state_transition/src/beacon/block_production.rs b/crates/blockchain/state_transition/src/beacon/block_production.rs index a710a1fe5..1d56bb56a 100644 --- a/crates/blockchain/state_transition/src/beacon/block_production.rs +++ b/crates/blockchain/state_transition/src/beacon/block_production.rs @@ -48,7 +48,7 @@ use super::helpers::electra::{ use super::helpers::misc::compute_signing_root; use super::helpers::predicates::is_slashable_validator; use super::lean_boundary::lean_state_unreachable; -use super::stf::{self, ExecutionEngine}; +use super::stf::{self, BlockCaches, ExecutionEngine, OwnedBlockCaches}; use ethlambda_storage::pools::single_committee; /// `state` advanced through empty slots to `slot`, as a block for `slot` is @@ -378,12 +378,18 @@ impl Operations { /// maximum. pub fn pack_operations(state: &BeaconState, candidates: Operations, config: &Config) -> Operations { let mut scratch = state.clone(); + // One table for the whole scratch pass, slashings included (a slashing + // starts an exit too): the scratch state is at the block's own slot, so + // its total active balance cannot change between candidates. + let active_balances = ActiveBalanceCache::default(); let proposer_slashings = pack_each( &mut scratch, candidates.proposer_slashings, preset::MAX_PROPOSER_SLASHINGS, config, - stf::operations::process_proposer_slashing, + |state, slashing, config| { + stf::operations::process_proposer_slashing(state, slashing, config, &active_balances) + }, ); let epoch = get_current_epoch(&scratch); @@ -400,13 +406,11 @@ pub fn pack_operations(state: &BeaconState, candidates: Operations, config: &Con ranked.into_iter().map(|(_, slashing)| slashing).collect(), preset::MAX_ATTESTER_SLASHINGS_ELECTRA, config, - stf::electra::process_attester_slashing, + |state, slashing, config| { + stf::electra::process_attester_slashing(state, slashing, config, &active_balances) + }, ); - // One table for the whole scratch pass: the scratch state is at the - // block's own slot, so its total active balance cannot change between - // candidates. - let active_balances = ActiveBalanceCache::default(); let voluntary_exits = pack_each( &mut scratch, candidates.voluntary_exits, @@ -515,10 +519,17 @@ pub struct BlockInputs { /// the network would, before anything is signed. `options` is what that run may /// leave unchecked: only a caller that set `skip_randao_verification` passes /// anything but the default. +/// +/// `pubkey_indices` is the pubkey table that run resolves execution requests +/// and the sync committee through. A node passes its `Store`'s shared one: a +/// fresh table would first ingest the whole registry, which at mainnet's size +/// costs more than the registry scans the table replaces. The run's committee +/// and total-active-balance tables are fresh, as they always were here. pub fn assemble_block( state: &BeaconState, inputs: BlockInputs, config: &Config, + pubkey_indices: &PubkeyIndexCache, options: stf::block::BlockProcessingOptions, ) -> Result { let body = BeaconBlockBody { @@ -592,14 +603,17 @@ pub fn assemble_block( BeaconState::Lean(_) => lean_state_unreachable("assemble_block"), }; let mut post = state.clone(); + let scratch = OwnedBlockCaches::default(); + let caches = BlockCaches { + pubkey_indices, + ..scratch.caches() + }; stf::block::process_block( &mut post, &wrapped, config, &ExecutionEngine::valid(), - &CommitteeCache::default(), - &ActiveBalanceCache::default(), - &PubkeyIndexCache::default(), + caches, options, )?; // Fold the block's writes into their trees first, so the root is computed @@ -687,6 +701,7 @@ mod tests { &state, inputs, &Config::mainnet(), + &PubkeyIndexCache::default(), BlockProcessingOptions::default(), ) .unwrap(); @@ -708,9 +723,7 @@ mod tests { &signed, &Config::mainnet(), &ExecutionEngine::valid(), - &CommitteeCache::default(), - &ActiveBalanceCache::default(), - &PubkeyIndexCache::default(), + OwnedBlockCaches::default().caches(), BlockProcessingOptions::default(), ) .unwrap(); @@ -745,6 +758,7 @@ mod tests { &state, inputs_with_infinity_reveal(&state), &config, + &PubkeyIndexCache::default(), BlockProcessingOptions::default(), ) .is_err() @@ -754,6 +768,7 @@ mod tests { &state, inputs_with_infinity_reveal(&state), &config, + &PubkeyIndexCache::default(), skip_randao(), ) .unwrap(); @@ -773,9 +788,7 @@ mod tests { &signed, &config, &ExecutionEngine::valid(), - &CommitteeCache::default(), - &ActiveBalanceCache::default(), - &PubkeyIndexCache::default(), + OwnedBlockCaches::default().caches(), skip_randao(), ) .unwrap(); @@ -802,6 +815,7 @@ mod tests { &state, inputs, &Config::mainnet(), + &PubkeyIndexCache::default(), BlockProcessingOptions::default(), ) .is_err() @@ -1171,8 +1185,14 @@ mod tests { execution_requests: ExecutionRequests::default(), sync_aggregate: empty_sync_aggregate(), }; - let block = - assemble_block(&state, inputs, &config, BlockProcessingOptions::default()).unwrap(); + let block = assemble_block( + &state, + inputs, + &config, + &PubkeyIndexCache::default(), + BlockProcessingOptions::default(), + ) + .unwrap(); assert_eq!(block.body.voluntary_exits.len(), 1); assert_eq!(block.body.voluntary_exits[0].message.validator_index, 4); } @@ -1246,6 +1266,7 @@ mod tests { &state, inputs, &Config::mainnet(), + &PubkeyIndexCache::default(), BlockProcessingOptions::default(), ) .unwrap(); diff --git a/crates/blockchain/state_transition/src/beacon/fork_choice.rs b/crates/blockchain/state_transition/src/beacon/fork_choice.rs index 9ab52109f..6fc064ac8 100644 --- a/crates/blockchain/state_transition/src/beacon/fork_choice.rs +++ b/crates/blockchain/state_transition/src/beacon/fork_choice.rs @@ -182,8 +182,7 @@ use crate::beacon::containers::{bellatrix, deneb, electra, fulu, gloas, phase0}; use crate::beacon::error::{Error, Result, verify}; use crate::beacon::fork::ForkName; use crate::beacon::helpers::accessors::{ - ActiveBalanceCache, CommitteeCache, CommitteeCacheExt, get_active_validator_indices, - get_current_epoch, + CommitteeCache, CommitteeCacheExt, get_active_validator_indices, get_current_epoch, }; use crate::beacon::helpers::attestation as phase0_attestation; use crate::beacon::helpers::electra as electra_helpers; @@ -1060,13 +1059,8 @@ fn transition_block( validate_result: bool, config: &Config, engine: &stf::ExecutionEngine, - committees: &CommitteeCache, - active_balances: &ActiveBalanceCache, + caches: stf::BlockCaches<'_>, ) -> (BeaconState, Result<()>) { - // The store's own table, shared across forks; every hit is verified - // against the state in hand (see `PubkeyIndexCacheExt`). - let pubkey_cache = store.pubkey_index_cache(); - let pubkey_indices = &*pubkey_cache; let block_slot = signed_block.slot(); let epoch = compute_epoch_at_slot(block_slot); let epoch_start = compute_start_slot_at_epoch(epoch); @@ -1090,9 +1084,7 @@ fn transition_block( validate_result, config, engine, - committees, - active_balances, - pubkey_indices, + caches, ); return (state, outcome); } @@ -1108,9 +1100,7 @@ fn transition_block( validate_result, config, engine, - committees, - active_balances, - pubkey_indices, + caches, ) }); return (state, outcome); @@ -1134,9 +1124,7 @@ fn transition_block( validate_result, config, engine, - committees, - active_balances, - pubkey_indices, + caches, ) }); (state, outcome) @@ -4965,9 +4953,17 @@ pub fn on_block( // applies, after `process_slots`, so no second parent state exists). // Either way `state` is an independent clone, so a failing block cannot // corrupt a cached entry. - // The store's own cache, so every block of this chain shares one entry per - // epoch; see `ActiveBalanceCacheExt`. + // The store's own tables: every block of this chain shares one total + // active balance per epoch (see `ActiveBalanceCacheExt`), and one pubkey + // table, shared across forks since every hit is verified against the state + // in hand (see `PubkeyIndexCacheExt`). let active_balances = store.active_balance_cache(); + let pubkey_indices = store.pubkey_index_cache(); + let caches = stf::BlockCaches { + committees, + active_balances: &active_balances, + pubkey_indices: &pubkey_indices, + }; let (mut state, transition) = transition_block( store, &parent_state, @@ -4976,8 +4972,7 @@ pub fn on_block( true, config, &engine, - committees, - &active_balances, + caches, ); // `optimistic-sync.md`: a block deemed `INVALIDATED` MUST NOT be included @@ -5653,7 +5648,7 @@ mod tests { use super::*; use crate::beacon::bls; use crate::beacon::containers::BeaconBlockHeader; - use crate::beacon::helpers::accessors::{self, ActiveBalanceCache, PubkeyIndexCache}; + use crate::beacon::helpers::accessors; use crate::beacon::helpers::misc::compute_signing_root; use crate::beacon::helpers::test_state; use crate::beacon::primitives::BlsSignature; @@ -6323,8 +6318,7 @@ mod tests { false, &config, &stf::ExecutionEngine::valid(), - &CommitteeCache::default(), - &ActiveBalanceCache::default(), + stf::OwnedBlockCaches::default().caches(), ); outcome.expect("applies"); post.apply_pending_mutations(); @@ -7196,16 +7190,13 @@ mod tests { // the transition once, unvalidated, the way a proposer decides // everything but the signature and the state root before signing. let mut trial_state = anchor_state.clone(); - let trial_committees = CommitteeCache::default(); stf::state_transition( &mut trial_state, &draft, false, &config, &stf::ExecutionEngine::valid(), - &trial_committees, - &ActiveBalanceCache::default(), - &PubkeyIndexCache::default(), + stf::OwnedBlockCaches::default().caches(), ) .expect("the drafted block must transition cleanly"); let state_root = trial_state.hash_tree_root(); @@ -10399,7 +10390,9 @@ mod tests { BlockInputs, advance_to_slot, assemble_block, empty_sync_aggregate, payload_inputs, }; use crate::beacon::containers::electra; - use crate::beacon::helpers::accessors::{get_beacon_proposer_index, get_domain}; + use crate::beacon::helpers::accessors::{ + PubkeyIndexCache, get_beacon_proposer_index, get_domain, + }; use crate::beacon::helpers::test_state::{sign_for, with_signing_validators_at}; use crate::beacon::primitives::Bytes32; use ethlambda_types::beacon::containers::deneb::ExecutionPayload; @@ -10456,6 +10449,7 @@ mod tests { &advanced, inputs, &config, + &PubkeyIndexCache::default(), crate::beacon::stf::block::BlockProcessingOptions::default(), ) .expect("assemble"); @@ -10481,8 +10475,7 @@ mod tests { false, &Config::mainnet(), &stf::ExecutionEngine::valid(), - &CommitteeCache::default(), - &ActiveBalanceCache::default(), + stf::OwnedBlockCaches::default().caches(), ); outcome.expect("the block applies"); state.apply_pending_mutations(); diff --git a/crates/blockchain/state_transition/src/beacon/gloas_block_production.rs b/crates/blockchain/state_transition/src/beacon/gloas_block_production.rs index 8f3438224..a1b9dd13f 100644 --- a/crates/blockchain/state_transition/src/beacon/gloas_block_production.rs +++ b/crates/blockchain/state_transition/src/beacon/gloas_block_production.rs @@ -26,14 +26,14 @@ use super::bls; use super::config::Config; use super::error::{Error, Result, verify}; use super::helpers::accessors::{ - ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, get_beacon_proposer_index, - get_current_epoch, get_randao_mix, + ActiveBalanceCache, PubkeyIndexCache, get_beacon_proposer_index, get_current_epoch, + get_randao_mix, }; use super::helpers::gloas::{ can_builder_cover_bid, get_indexed_payload_attestation, get_ptc, gloas_state_ref, is_active_builder, is_attestation_same_slot, is_valid_indexed_payload_attestation, }; -use super::stf; +use super::stf::{self, BlockCaches, OwnedBlockCaches}; use ethlambda_types::beacon::{ constants, containers::{ @@ -81,24 +81,29 @@ pub struct GloasPayloadInputs { /// the parent's payload is skipped and the withdrawals are the ones the state /// already expects. The target gas limit is the parent bid's own (contract /// decision: this node expresses no preference of its own). +/// +/// `pubkey_indices` resolves the parent's withdrawal and consolidation +/// requests; pass the `Store`'s shared table, as for [`assemble_gloas_block`]. pub fn gloas_payload_inputs( state: &BeaconState, build_on_full: bool, parent_requests: &ExecutionRequests, config: &Config, + pubkey_indices: &PubkeyIndexCache, ) -> Result { let inner = gloas_state_ref(state, "gloas_payload_inputs")?; let parent_bid = &inner.latest_execution_payload_bid; let (withdrawals, head_block_hash) = if build_on_full { let mut applied = state.clone(); - // A scratch copy used once: fresh caches, as for the rest of block - // production. + // A scratch copy used once: a fresh total-active-balance table, but + // the caller's pubkey table, which a withdrawal or consolidation + // request resolves through (a fresh one would ingest the registry). stf::gloas::apply_parent_execution_payload( &mut applied, parent_requests, config, &ActiveBalanceCache::default(), - &PubkeyIndexCache::default(), + pubkey_indices, )?; ( stf::gloas::get_expected_withdrawals(&applied)?.withdrawals, @@ -380,12 +385,13 @@ pub struct GloasBidBlockInputs { /// a copy fills `state_root` and enforces `process_execution_payload_bid`: /// the builder is active, covers the bid and signed it, and the bid's slot, /// parent hash and root and randao are the state's. There is no envelope: the -/// builder reveals the payload itself. `options` is what that run may leave -/// unchecked, as in [`assemble_gloas_block`]. +/// builder reveals the payload itself. `options` and `pubkey_indices` are as +/// in [`assemble_gloas_block`]. pub fn assemble_gloas_block_on_bid( state: &BeaconState, inputs: GloasBidBlockInputs, config: &Config, + pubkey_indices: &PubkeyIndexCache, options: stf::block::BlockProcessingOptions, ) -> Result { verify( @@ -411,15 +417,12 @@ pub fn assemble_gloas_block_on_bid( body, }; let mut post = state.clone(); - stf::gloas::process_block( - &mut post, - &block, - config, - &CommitteeCache::default(), - &ActiveBalanceCache::default(), - &PubkeyIndexCache::default(), - options, - )?; + let scratch = OwnedBlockCaches::default(); + let caches = BlockCaches { + pubkey_indices, + ..scratch.caches() + }; + stf::gloas::process_block(&mut post, &block, config, caches, options)?; block.state_root = post.hash_tree_root(); Ok(block) } @@ -485,10 +488,17 @@ pub struct GloasProduced { /// malformed build fails here and not on the network. `options` is what the /// `process_block` run may leave unchecked: only a caller that set /// `skip_randao_verification` passes anything but the default. +/// +/// `pubkey_indices` is the pubkey table that run resolves the parent's +/// execution requests and the sync committee through. A node passes its +/// `Store`'s shared one: a fresh table would first ingest the whole registry, +/// which at mainnet's size costs more than the registry scans it replaces. The +/// run's committee and total-active-balance tables are fresh. pub fn assemble_gloas_block( state: &BeaconState, inputs: GloasBlockInputs, config: &Config, + pubkey_indices: &PubkeyIndexCache, options: stf::block::BlockProcessingOptions, ) -> Result { verify( @@ -538,15 +548,12 @@ pub fn assemble_gloas_block( let BeaconState::Gloas(_) = &post else { unreachable!("checked above") }; - stf::gloas::process_block( - &mut post, - &block, - config, - &CommitteeCache::default(), - &ActiveBalanceCache::default(), - &PubkeyIndexCache::default(), - options, - )?; + let scratch = OwnedBlockCaches::default(); + let caches = BlockCaches { + pubkey_indices, + ..scratch.caches() + }; + stf::gloas::process_block(&mut post, &block, config, caches, options)?; block.state_root = post.hash_tree_root(); let envelope = ExecutionPayloadEnvelope { @@ -633,7 +640,7 @@ pub mod test_support { use super::*; use crate::beacon::ForkName; use crate::beacon::block_production::{advance_to_slot, empty_sync_aggregate}; - use crate::beacon::helpers::accessors::get_domain; + use crate::beacon::helpers::accessors::{CommitteeCache, get_domain}; use crate::beacon::helpers::fulu::initialize_proposer_lookahead; use crate::beacon::helpers::gloas::compute_ptc; use crate::beacon::helpers::misc::compute_signing_root; @@ -733,7 +740,13 @@ pub mod test_support { payload_attestations: Vec, ) -> Result { let requests = ExecutionRequests::default(); - let inputs = gloas_payload_inputs(state, build_on_full, &requests, &config())?; + let inputs = gloas_payload_inputs( + state, + build_on_full, + &requests, + &config(), + &PubkeyIndexCache::default(), + )?; assemble_gloas_block( state, GloasBlockInputs { @@ -748,6 +761,7 @@ pub mod test_support { execution_requests: ExecutionRequests::default(), }, &config(), + &PubkeyIndexCache::default(), BlockProcessingOptions::default(), ) } @@ -759,9 +773,7 @@ pub mod test_support { &mut post, block, &config(), - &CommitteeCache::default(), - &ActiveBalanceCache::default(), - &PubkeyIndexCache::default(), + OwnedBlockCaches::default().caches(), BlockProcessingOptions::default(), ) .unwrap(); @@ -875,7 +887,14 @@ mod gloas_block_production_tests { ); let requests = ExecutionRequests::default(); - let inputs = gloas_payload_inputs(&state, true, &requests, &config()).unwrap(); + let inputs = gloas_payload_inputs( + &state, + true, + &requests, + &config(), + &PubkeyIndexCache::default(), + ) + .unwrap(); let produced = assemble_gloas_block( &state, GloasBlockInputs { @@ -890,6 +909,7 @@ mod gloas_block_production_tests { execution_requests: ExecutionRequests::default(), }, &config(), + &PubkeyIndexCache::default(), BlockProcessingOptions::default(), ) .unwrap(); @@ -924,9 +944,7 @@ mod gloas_block_production_tests { &wrapped, &config(), &ExecutionEngine::valid(), - &CommitteeCache::default(), - &ActiveBalanceCache::default(), - &PubkeyIndexCache::default(), + OwnedBlockCaches::default().caches(), BlockProcessingOptions::default(), ) .unwrap(); @@ -986,8 +1004,22 @@ mod gloas_block_production_tests { fn building_on_full_extends_the_parent_payload_and_on_empty_skips_it() { let state = state_to_build_on(); let requests = ExecutionRequests::default(); - let full = gloas_payload_inputs(&state, true, &requests, &config()).unwrap(); - let empty = gloas_payload_inputs(&state, false, &requests, &config()).unwrap(); + let full = gloas_payload_inputs( + &state, + true, + &requests, + &config(), + &PubkeyIndexCache::default(), + ) + .unwrap(); + let empty = gloas_payload_inputs( + &state, + false, + &requests, + &config(), + &PubkeyIndexCache::default(), + ) + .unwrap(); assert_eq!( full.head_block_hash, ExecutionBlockHash::repeat_byte(PARENT_BLOCK_HASH) @@ -1026,7 +1058,14 @@ mod gloas_block_production_tests { }; let produce_with = |options| { let requests = ExecutionRequests::default(); - let inputs = gloas_payload_inputs(&state, true, &requests, &config()).unwrap(); + let inputs = gloas_payload_inputs( + &state, + true, + &requests, + &config(), + &PubkeyIndexCache::default(), + ) + .unwrap(); assemble_gloas_block( &state, GloasBlockInputs { @@ -1041,6 +1080,7 @@ mod gloas_block_production_tests { execution_requests: ExecutionRequests::default(), }, &config(), + &PubkeyIndexCache::default(), options, ) }; @@ -1058,9 +1098,7 @@ mod gloas_block_production_tests { &mut post, &produced.block, &config(), - &CommitteeCache::default(), - &ActiveBalanceCache::default(), - &PubkeyIndexCache::default(), + OwnedBlockCaches::default().caches(), skip, ) .unwrap(); @@ -1071,7 +1109,14 @@ mod gloas_block_production_tests { fn a_payload_on_the_wrong_parent_is_refused_before_signing() { let state = state_to_build_on(); let requests = ExecutionRequests::default(); - let inputs = gloas_payload_inputs(&state, true, &requests, &config()).unwrap(); + let inputs = gloas_payload_inputs( + &state, + true, + &requests, + &config(), + &PubkeyIndexCache::default(), + ) + .unwrap(); let mut payload = payload_for(&inputs); payload.parent_hash = ExecutionBlockHash::repeat_byte(9); let result = assemble_gloas_block( @@ -1088,6 +1133,7 @@ mod gloas_block_production_tests { execution_requests: ExecutionRequests::default(), }, &config(), + &PubkeyIndexCache::default(), BlockProcessingOptions::default(), ); assert!(result.is_err()); @@ -1097,7 +1143,14 @@ mod gloas_block_production_tests { fn a_withdrawal_set_the_state_does_not_expect_is_refused() { let state = state_to_build_on(); let requests = ExecutionRequests::default(); - let inputs = gloas_payload_inputs(&state, true, &requests, &config()).unwrap(); + let inputs = gloas_payload_inputs( + &state, + true, + &requests, + &config(), + &PubkeyIndexCache::default(), + ) + .unwrap(); let mut payload = payload_for(&inputs); payload.withdrawals = vec![Withdrawal { index: 99, @@ -1120,6 +1173,7 @@ mod gloas_block_production_tests { execution_requests: ExecutionRequests::default(), }, &config(), + &PubkeyIndexCache::default(), BlockProcessingOptions::default(), ); assert!(produced.is_err()); @@ -1260,7 +1314,14 @@ mod gloas_block_production_tests { // The packed vote rides in a block that passes the full STF. let packed = pack_gloas_attestations(&state, vec![vote(&state, parent_root, 0)]); let requests = ExecutionRequests::default(); - let inputs = gloas_payload_inputs(&state, true, &requests, &config()).unwrap(); + let inputs = gloas_payload_inputs( + &state, + true, + &requests, + &config(), + &PubkeyIndexCache::default(), + ) + .unwrap(); assemble_gloas_block( &state, GloasBlockInputs { @@ -1275,6 +1336,7 @@ mod gloas_block_production_tests { execution_requests: ExecutionRequests::default(), }, &config(), + &PubkeyIndexCache::default(), BlockProcessingOptions::default(), ) .unwrap(); @@ -1405,6 +1467,7 @@ mod bid_assembly_tests { &state, inputs(&state, bid.clone()), &config(), + &PubkeyIndexCache::default(), BlockProcessingOptions::default(), ) .unwrap(); @@ -1440,6 +1503,7 @@ mod bid_assembly_tests { &state, inputs(&state, bid), &config(), + &PubkeyIndexCache::default(), BlockProcessingOptions::default(), ) .is_err() @@ -1462,6 +1526,7 @@ mod bid_assembly_tests { &state, inputs(&state, bid), &config(), + &PubkeyIndexCache::default(), BlockProcessingOptions::default(), ) .is_err() @@ -1482,6 +1547,7 @@ mod bid_assembly_tests { &state, inputs(&state, bid), &config(), + &PubkeyIndexCache::default(), BlockProcessingOptions::default(), ) .is_err() @@ -1503,6 +1569,7 @@ mod bid_assembly_tests { &state, inputs(&state, bid), &config(), + &PubkeyIndexCache::default(), BlockProcessingOptions::default(), ) .is_err() diff --git a/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs b/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs index 875257d70..e3ebce7d3 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs @@ -344,11 +344,20 @@ impl CommitteeCacheExt for CommitteeCache { /// Between `process_effective_balance_updates` and the slot increment that /// follows it, a state is still in epoch `E - 1` but already carries epoch /// `E`'s effective balances, so its total would not match a key rooted at -/// `E - 2`'s end. The only callers routed through the cache are block -/// processing's (attestations, sync aggregate), which never run in that -/// window: a block is processed on a state that `process_slots` has already -/// advanced to the block's slot. Epoch processing keeps calling the -/// uncached [`get_total_active_balance`]. +/// `E - 2`'s end. Every caller routed through the cache is block processing, +/// which never runs in that window: a block is processed on a state that +/// `process_slots` has already advanced to the block's slot. Those callers +/// are attestations and the sync aggregate (base rewards), and the churn +/// limits of everything a block can make exit or consolidate (voluntary +/// exits, proposer and attester slashings, withdrawal and consolidation +/// requests, gloas's `apply_parent_execution_payload` included), which reach +/// it through [`total_active_balance_or_compute`] with `Some`. Block +/// production's scratch runs of those same functions count too: their state +/// is advanced to the block's slot first. +/// +/// Epoch processing keeps calling the uncached [`get_total_active_balance`], +/// and its ejections pass `None` to the same churn helpers. So does any other +/// caller of those helpers that is not processing a block at its slot. fn active_balance_key(state: &BeaconState) -> Option { let epoch = get_current_epoch(state); let decision_slot = compute_start_slot_at_epoch(epoch).saturating_sub(1); @@ -402,8 +411,8 @@ impl ActiveBalanceCacheExt for ActiveBalanceCache { /// [`ActiveBalanceCache`], shares one registry pass across a block's exits, /// consolidations and slashings, while epoch processing and the spec-shaped /// unit tests keep the uncached function. Passing `Some` is sound only for the -/// callers [`active_balance_key`] documents: states advanced to their block's -/// slot, never mid epoch processing. +/// callers [`active_balance_key`] lists: states advanced to their block's +/// slot, never mid epoch processing. A new `Some` caller belongs on that list. pub fn total_active_balance_or_compute( state: &BeaconState, cache: Option<&ActiveBalanceCache>, @@ -437,8 +446,10 @@ pub trait PubkeyIndexCacheExt { /// The index holding `pubkey` in `state`, exactly as a position search of /// the registry would answer: the cache when it verifies, otherwise that - /// search. A pubkey that is not registered costs one search, never a - /// rebuild of the table. Counted in `lean_beacon_pubkey_index_lookups_total`. + /// search, whose answer is then recorded in the table + /// ([`PubkeyIndexCache::insert`]) so it is searched for once. A pubkey + /// that is not registered costs one search, never a rebuild of the table. + /// Counted in `lean_beacon_pubkey_index_lookups_total`. fn validator_index(&self, state: &BeaconState, pubkey: &BlsPubkey) -> Option; } @@ -470,10 +481,15 @@ impl PubkeyIndexCacheExt for PubkeyIndexCache { return Some(index); } crate::metrics::inc_pubkey_index_lookups("fallback", 1); - state + let found = state .iter_validators() .position(|validator| validator.pubkey == *pubkey) - .map(|index| index as ValidatorIndex) + .map(|index| index as ValidatorIndex); + // Recorded so the next lookup of this pubkey on this chain is a hit. + if let Some(index) = found { + self.insert(pubkey, index); + } + found } } @@ -1143,7 +1159,28 @@ mod tests { // Our own key at that index, absent from the table's fork, is found by // the scan. let own = pubkey_at(&ours, 4); + assert_eq!(cache.verified_index(&ours, &own), None); + assert_eq!(cache.validator_index(&ours, &own), Some(4)); + } + + #[test] + fn a_fallback_scan_records_what_it_found() { + let ours = with_validators(10); + let cache = PubkeyIndexCache::default(); + // The table is fed by a fork where index 4 holds a different key, so + // our own key at index 4 is nowhere in it. + let mut other_fork = ours.clone(); + other_fork.validator_mut(4).unwrap().pubkey = BlsPubkey([0xCD; 48]); + cache.verified_index(&other_fork, &pubkey_at(&other_fork, 0)); + let own = pubkey_at(&ours, 4); + assert_eq!(cache.verified_index(&ours, &own), None); + + // One scan, and from then on the cache answers by itself, without + // having covered another position. + let covered = cache.covered_len(); assert_eq!(cache.validator_index(&ours, &own), Some(4)); + assert_eq!(cache.verified_index(&ours, &own), Some(4)); + assert_eq!(cache.covered_len(), covered); } #[test] @@ -1160,7 +1197,7 @@ mod tests { } #[test] - fn a_key_prefix_collision_falls_back_to_the_right_validator() { + fn a_key_prefix_collision_resolves_both_validators() { let mut state = with_validators(10); let cache = PubkeyIndexCache::default(); // Two distinct pubkeys sharing the eight bytes the table keys on. @@ -1173,6 +1210,9 @@ mod tests { ); append_validator(&mut state, twin); + // Both are kept under the shared key, so neither needs a scan. + assert_eq!(cache.verified_index(&state, &first), Some(2)); + assert_eq!(cache.verified_index(&state, &twin), Some(10)); assert_eq!(cache.validator_index(&state, &first), Some(2)); assert_eq!(cache.validator_index(&state, &twin), Some(10)); } diff --git a/crates/blockchain/state_transition/src/beacon/helpers/electra.rs b/crates/blockchain/state_transition/src/beacon/helpers/electra.rs index 0e98c3115..9ea0cb873 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/electra.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/electra.rs @@ -369,10 +369,8 @@ pub fn get_max_effective_balance(validator: &Validator) -> Gwei { /// `EFFECTIVE_BALANCE_INCREMENT` so the budget always divides evenly into the /// unit every balance change is already rounded to. pub fn get_balance_churn_limit(state: &BeaconState, config: &Config) -> Result { - Ok(balance_churn_limit_for( - get_total_active_balance(state)?, - config, - )) + let total_active_balance = get_total_active_balance(state)?; + Ok(balance_churn_limit_for(total_active_balance, config)) } /// [`get_balance_churn_limit`] for a caller that already holds the total @@ -393,8 +391,9 @@ pub(crate) fn balance_churn_limit_for(total_active_balance: Gwei, config: &Confi /// large validator set, activations and exits cannot alone consume the whole /// churn budget and starve consolidations of any share at all. pub fn get_activation_exit_churn_limit(state: &BeaconState, config: &Config) -> Result { + let total_active_balance = get_total_active_balance(state)?; Ok(activation_exit_churn_limit_for( - get_total_active_balance(state)?, + total_active_balance, config, )) } @@ -410,10 +409,8 @@ pub(crate) fn activation_exit_churn_limit_for(total_active_balance: Gwei, config /// The portion of [`get_balance_churn_limit`] left over for consolidations /// once [`get_activation_exit_churn_limit`] has taken its share. pub fn get_consolidation_churn_limit(state: &BeaconState, config: &Config) -> Result { - Ok(consolidation_churn_limit_for( - get_total_active_balance(state)?, - config, - )) + let total_active_balance = get_total_active_balance(state)?; + Ok(consolidation_churn_limit_for(total_active_balance, config)) } /// [`get_consolidation_churn_limit`] for a caller that already holds the total @@ -691,15 +688,9 @@ pub fn compute_exit_epoch_and_update_churn( active_balances: Option<&ActiveBalanceCache>, ) -> Result { let current_epoch = get_current_epoch(state); - let per_epoch_churn = match state.fork_name() { - ForkName::Electra | ForkName::Fulu => activation_exit_churn_limit_for( - total_active_balance_or_compute(state, active_balances)?, - config, - ), - ForkName::Gloas => crate::beacon::helpers::gloas::exit_churn_limit_for( - total_active_balance_or_compute(state, active_balances)?, - config, - ), + let churn_limit_for: fn(Gwei, &Config) -> Gwei = match state.fork_name() { + ForkName::Electra | ForkName::Fulu => activation_exit_churn_limit_for, + ForkName::Gloas => crate::beacon::helpers::gloas::exit_churn_limit_for, fork @ (ForkName::Phase0 | ForkName::Altair | ForkName::Bellatrix @@ -712,6 +703,8 @@ pub fn compute_exit_epoch_and_update_churn( } ForkName::Lean => lean_state_unreachable("compute_exit_epoch_and_update_churn"), }; + let total_active_balance = total_active_balance_or_compute(state, active_balances)?; + let per_epoch_churn = churn_limit_for(total_active_balance, config); let mut fields = churn_cursors_mut(state, "compute_exit_epoch_and_update_churn")?; @@ -838,27 +831,23 @@ pub fn get_consolidation_churn_limit_for_fork( config: &Config, active_balances: Option<&ActiveBalanceCache>, ) -> Result { - match state.fork_name() { - ForkName::Electra | ForkName::Fulu => Ok(consolidation_churn_limit_for( - total_active_balance_or_compute(state, active_balances)?, - config, - )), - ForkName::Gloas => Ok( - crate::beacon::helpers::gloas::consolidation_churn_limit_for( - total_active_balance_or_compute(state, active_balances)?, - config, - ), - ), + let churn_limit_for: fn(Gwei, &Config) -> Gwei = match state.fork_name() { + ForkName::Electra | ForkName::Fulu => consolidation_churn_limit_for, + ForkName::Gloas => crate::beacon::helpers::gloas::consolidation_churn_limit_for, fork @ (ForkName::Phase0 | ForkName::Altair | ForkName::Bellatrix | ForkName::Capella - | ForkName::Deneb) => Err(Error::UnsupportedForFork { - function: "get_consolidation_churn_limit_for_fork", - fork, - }), + | ForkName::Deneb) => { + return Err(Error::UnsupportedForFork { + function: "get_consolidation_churn_limit_for_fork", + fork, + }); + } ForkName::Lean => lean_state_unreachable("get_consolidation_churn_limit_for_fork"), - } + }; + let total_active_balance = total_active_balance_or_compute(state, active_balances)?; + Ok(churn_limit_for(total_active_balance, config)) } /// Advances the consolidation-queue cursor for a consolidation moving diff --git a/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs b/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs index f48b45c3c..bb00e05bf 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs @@ -821,10 +821,8 @@ pub fn get_activation_churn_limit(state: &BeaconState, config: &Config) -> Resul /// draws from this on a gloas state /// rather than electra's combined budget. pub fn get_exit_churn_limit(state: &BeaconState, config: &Config) -> Result { - Ok(exit_churn_limit_for( - get_total_active_balance(state)?, - config, - )) + let total_active_balance = get_total_active_balance(state)?; + Ok(exit_churn_limit_for(total_active_balance, config)) } /// [`get_exit_churn_limit`] for a caller that already holds the total active @@ -847,10 +845,8 @@ pub(crate) fn exit_churn_limit_for(total_active_balance: Gwei, config: &Config) /// [`Config::min_per_epoch_churn_limit_electra`] the way the activation and /// exit limits are. pub fn get_consolidation_churn_limit(state: &BeaconState, config: &Config) -> Result { - Ok(consolidation_churn_limit_for( - get_total_active_balance(state)?, - config, - )) + let total_active_balance = get_total_active_balance(state)?; + Ok(consolidation_churn_limit_for(total_active_balance, config)) } /// [`get_consolidation_churn_limit`] for a caller that already holds the total diff --git a/crates/blockchain/state_transition/src/beacon/helpers/mutators.rs b/crates/blockchain/state_transition/src/beacon/helpers/mutators.rs index 72efd03bc..e4daa0bf3 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/mutators.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/mutators.rs @@ -12,7 +12,9 @@ use crate::beacon::lean_state_unreachable; use crate::beacon::preset; use crate::beacon::primitives::{Epoch, Gwei, ValidatorIndex}; -use super::accessors::{get_beacon_proposer_index, get_current_epoch, get_validator_churn_limit}; +use super::accessors::{ + ActiveBalanceCache, get_beacon_proposer_index, get_current_epoch, get_validator_churn_limit, +}; use super::misc::compute_activation_exit_epoch; /// Adds `delta` to a validator's balance. @@ -127,31 +129,6 @@ pub fn initiate_validator_exit( Ok(()) } -/// Slashes a validator: exits it, penalizes it, and pays the reporter. -/// -/// The immediate penalty is only a fraction of the effective balance. The rest of -/// the punishment is applied at the epoch boundary and scales with how much of -/// the validator set was slashed around the same time, which is what makes a -/// coordinated attack far more expensive than an isolated mistake. Recording the -/// balance in `slashings` here is what lets that later computation see it. -/// -/// `whistleblower_index` defaults to the current proposer when not given. -/// -/// One copy of this function serves every fork. The immediate penalty's divisor -/// falls at altair, bellatrix, and electra, and the reporter's divisor rises at -/// electra, all without changing anything else here, so both are selected by -/// fork through [`preset::retuned`] rather than by redefining the function per -/// fork the way the specification does. -/// -/// Altair does also restate `proposer_reward` as -/// `whistleblower_reward * PROPOSER_WEIGHT / WEIGHT_DENOMINATOR` in place of -/// phase0's division by `PROPOSER_REWARD_QUOTIENT`. Those two agree exactly for -/// every input, since `PROPOSER_WEIGHT / WEIGHT_DENOMINATOR` reduces to the same -/// fraction, so this keeps phase0's spelling for all forks rather than selecting -/// between two expressions that cannot disagree. -/// -/// The exit it starts, however, is genuinely fork-dependent, which is what -/// [`initiate_validator_exit_for_fork`] exists to resolve. /// Starts `index`'s exit under the rules of the state's own fork. /// /// Electra replaces [`initiate_validator_exit`] outright @@ -167,10 +144,16 @@ pub fn initiate_validator_exit( /// slashing at electra would set a different `exit_epoch`, and leave the /// state's churn cursor unadvanced, so every subsequent exit in the same epoch /// would queue wrongly too. +/// +/// `active_balances` reaches electra's churn limit, which reads the total +/// active balance (see +/// [`crate::beacon::helpers::electra::compute_exit_epoch_and_update_churn`]); +/// phase0's count-based limit has no use for it. fn initiate_validator_exit_for_fork( state: &mut BeaconState, index: ValidatorIndex, config: &Config, + active_balances: Option<&ActiveBalanceCache>, ) -> Result<()> { match state.fork_name() { // Gloas is not a redefinition of `initiate_validator_exit` (EIP-8061 @@ -181,7 +164,12 @@ fn initiate_validator_exit_for_fork( // draws through `compute_exit_epoch_and_update_churn` is already // gloas's own by the time it gets here. ForkName::Electra | ForkName::Fulu | ForkName::Gloas => { - crate::beacon::helpers::electra::initiate_validator_exit(state, index, config, None) + crate::beacon::helpers::electra::initiate_validator_exit( + state, + index, + config, + active_balances, + ) } ForkName::Phase0 | ForkName::Altair @@ -192,14 +180,45 @@ fn initiate_validator_exit_for_fork( } } +/// Slashes a validator: exits it, penalizes it, and pays the reporter. +/// +/// The immediate penalty is only a fraction of the effective balance. The rest of +/// the punishment is applied at the epoch boundary and scales with how much of +/// the validator set was slashed around the same time, which is what makes a +/// coordinated attack far more expensive than an isolated mistake. Recording the +/// balance in `slashings` here is what lets that later computation see it. +/// +/// `whistleblower_index` defaults to the current proposer when not given. +/// +/// One copy of this function serves every fork. The immediate penalty's divisor +/// falls at altair, bellatrix, and electra, and the reporter's divisor rises at +/// electra, all without changing anything else here, so both are selected by +/// fork through [`preset::retuned`] rather than by redefining the function per +/// fork the way the specification does. +/// +/// Altair does also restate `proposer_reward` as +/// `whistleblower_reward * PROPOSER_WEIGHT / WEIGHT_DENOMINATOR` in place of +/// phase0's division by `PROPOSER_REWARD_QUOTIENT`. Those two agree exactly for +/// every input, since `PROPOSER_WEIGHT / WEIGHT_DENOMINATOR` reduces to the same +/// fraction, so this keeps phase0's spelling for all forks rather than selecting +/// between two expressions that cannot disagree. +/// +/// The exit it starts, however, is genuinely fork-dependent, which is what +/// [`initiate_validator_exit_for_fork`] exists to resolve. +/// +/// `active_balances` is block processing's cache, handed to that exit's churn +/// limit: every caller is a slashing operation in a block, so a block slashing +/// many validators reads the total active balance once rather than once per +/// validator. `None` computes it uncached. pub fn slash_validator( state: &mut BeaconState, slashed_index: ValidatorIndex, whistleblower_index: Option, config: &Config, + active_balances: Option<&ActiveBalanceCache>, ) -> Result<()> { let epoch = get_current_epoch(state); - initiate_validator_exit_for_fork(state, slashed_index, config)?; + initiate_validator_exit_for_fork(state, slashed_index, config, active_balances)?; let effective_balance = state.validator(slashed_index)?.effective_balance; @@ -294,7 +313,7 @@ mod tests { let effective_balance = state.validator(3).unwrap().effective_balance; let balance_before = state.balance(3).unwrap(); - slash_validator(&mut state, 3, None, &config).unwrap(); + slash_validator(&mut state, 3, None, &config, None).unwrap(); let validator = state.validator(3).unwrap(); assert!(validator.slashed); @@ -340,7 +359,7 @@ mod tests { ); let effective_balance = state.validator(0).unwrap().effective_balance; - initiate_validator_exit_for_fork(&mut state, 0, &config).unwrap(); + initiate_validator_exit_for_fork(&mut state, 0, &config, None).unwrap(); let BeaconState::Gloas(inner) = &state else { unreachable!("built as Gloas"); diff --git a/crates/blockchain/state_transition/src/beacon/stf/altair.rs b/crates/blockchain/state_transition/src/beacon/stf/altair.rs index 2b0bcb2d9..b8ed4e916 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/altair.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/altair.rs @@ -348,20 +348,24 @@ pub fn process_sync_aggregate( // largest single cost of a live import (a hash probe per validator, every // block), where the cache answers each seat with a lookup and one // verifying tree descent. - let cached: Option> = committee_pubkeys + let mut resolved: Vec> = committee_pubkeys .iter() .map(|pubkey| pubkey_indices.verified_index(state, pubkey)) .collect(); - let committee_indices = match cached { - Some(indices) => { - crate::metrics::inc_pubkey_index_lookups("hit", indices.len() as u64); - indices - } - None => { - crate::metrics::inc_pubkey_index_lookups("fallback", committee_pubkeys.len() as u64); - resolve_sync_committee_by_scan(state, committee_pubkeys)? - } - }; + let missing = resolved.iter().filter(|index| index.is_none()).count(); + crate::metrics::inc_pubkey_index_lookups("hit", (resolved.len() - missing) as u64); + if missing > 0 { + crate::metrics::inc_pubkey_index_lookups("fallback", missing as u64); + resolve_missing_seats_by_scan(state, committee_pubkeys, &mut resolved, pubkey_indices); + } + let committee_indices = resolved + .into_iter() + .map(|index| { + index.ok_or(Error::SpecAssert( + "state.current_sync_committee.pubkeys[i] in [v.pubkey for v in state.validators]", + )) + }) + .collect::>>()?; let total_active_balance = active_balances.total_active_balance(state)?; let total_active_increments = total_active_balance / preset::EFFECTIVE_BALANCE_INCREMENT; @@ -405,42 +409,49 @@ pub fn process_sync_aggregate( Ok(()) } -/// Every committee seat's validator index, by one pass over the registry: -/// the fallback for a seat the pubkey index cache could not verify. +/// Fills every seat of `resolved` the pubkey index cache could not verify, by +/// one pass over the registry, and records each index it finds in the cache +/// so the next block's lookup of that seat is a hit. +/// +/// Only the missing seats are searched for: the cache vouched for the rest, +/// so one unconfirmable member costs one scan, not a scan for the whole +/// committee. /// /// One pass, not one per seat. The specification writes this as a search per /// committee member, which is `SYNC_COMMITTEE_SIZE` scans of the whole /// validator set: at mainnet's ~1M validators that is hundreds of millions of /// 48-byte pubkey comparisons per block. Inverting the loop makes it one scan -/// against a committee-sized map. +/// against a map of the missing seats, which stops once every one is found. /// /// A pubkey may hold more than one seat, since sync-committee selection /// samples with replacement, so each key maps to every seat it occupies -/// rather than to one. -fn resolve_sync_committee_by_scan( +/// rather than to one. A key's first position is taken, as the +/// specification's `.index()` does; a seat whose pubkey is not registered is +/// left `None` for the caller to reject. +fn resolve_missing_seats_by_scan( state: &BeaconState, committee_pubkeys: &[BlsPubkey], -) -> Result> { + resolved: &mut [Option], + pubkey_indices: &PubkeyIndexCache, +) { let mut seats_by_pubkey: HashMap<&BlsPubkey, Vec> = HashMap::new(); for (seat, pubkey) in committee_pubkeys.iter().enumerate() { - seats_by_pubkey.entry(pubkey).or_default().push(seat); + if resolved[seat].is_none() { + seats_by_pubkey.entry(pubkey).or_default().push(seat); + } } - let mut resolved: Vec> = vec![None; committee_pubkeys.len()]; for (index, validator) in state.iter_validators().enumerate() { - if let Some(seats) = seats_by_pubkey.get(&validator.pubkey) { - for &seat in seats { - resolved[seat] = Some(index as ValidatorIndex); + if seats_by_pubkey.is_empty() { + break; + } + if let Some(seats) = seats_by_pubkey.remove(&validator.pubkey) { + let index = index as ValidatorIndex; + for seat in seats { + resolved[seat] = Some(index); } + pubkey_indices.insert(&validator.pubkey, index); } } - resolved - .into_iter() - .map(|index| { - index.ok_or(Error::SpecAssert( - "state.current_sync_committee.pubkeys[i] in [v.pubkey for v in state.validators]", - )) - }) - .collect() } #[cfg(test)] @@ -781,6 +792,56 @@ mod tests { } } + /// A seat the shared pubkey cache cannot verify (here, because the table + /// was fed by a fork whose registry holds another key at that index) is + /// scanned for, gives the same post-state as a fresh cache, and is + /// recorded so the next block's lookup is a hit. + #[test] + fn process_sync_aggregate_scans_for_an_unverifiable_seat_and_records_it() { + let (mut state, secret_keys) = altair_state_with_keypairs(3); + let pubkeys_vec: Vec = (0..preset::SYNC_COMMITTEE_SIZE) + .map(|seat| BlsPubkey(secret_keys[seat % 3].sk_to_pk().to_bytes())) + .collect(); + let aggregate_pubkey = bls::eth_aggregate_pubkeys(&pubkeys_vec).unwrap(); + { + let (current_sync_committee, _) = state.sync_committees_mut().unwrap(); + *current_sync_committee = SyncCommittee { + pubkeys: pubkeys_vec.try_into().unwrap(), + aggregate_pubkey, + }; + } + let sync_aggregate = altair::SyncAggregate { + sync_committee_bits: altair::SyncCommitteeBits::default(), + sync_committee_signature: g2_point_at_infinity(), + }; + + let cache = PubkeyIndexCache::default(); + let mut other_fork = state.clone(); + other_fork.validator_mut(1).unwrap().pubkey = BlsPubkey([0xCD; 48]); + cache.verified_index(&other_fork, &BlsPubkey([0xCD; 48])); + let member = state.validator(1).unwrap().pubkey; + assert_eq!(cache.verified_index(&state, &member), None); + + let mut with_fresh = state.clone(); + process_sync_aggregate( + &mut with_fresh, + &sync_aggregate, + &ActiveBalanceCache::default(), + &PubkeyIndexCache::default(), + ) + .unwrap(); + process_sync_aggregate( + &mut state, + &sync_aggregate, + &ActiveBalanceCache::default(), + &cache, + ) + .unwrap(); + + assert_eq!(state, with_fresh); + assert_eq!(cache.verified_index(&state, &member), Some(1)); + } + #[test] fn process_sync_aggregate_rejects_a_non_infinity_signature_for_zero_participants() { let (mut state, secret_keys) = altair_state_with_keypairs(1); diff --git a/crates/blockchain/state_transition/src/beacon/stf/bellatrix.rs b/crates/blockchain/state_transition/src/beacon/stf/bellatrix.rs index d24358b3c..d1143bd26 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/bellatrix.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/bellatrix.rs @@ -33,15 +33,13 @@ use crate::beacon::config::Config; use crate::beacon::constants; use crate::beacon::containers::{BeaconState, bellatrix}; use crate::beacon::error::{Error, Result, verify}; -use crate::beacon::helpers::accessors::{ - ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, get_current_epoch, get_randao_mix, -}; +use crate::beacon::helpers::accessors::{get_current_epoch, get_randao_mix}; use crate::beacon::preset; use crate::beacon::primitives::{ Bytes32, ExecutionAddress, ExecutionBlockHash, HashTreeRoot as _, Root, Slot, Uint256, }; -use super::ExecutionEngine; +use super::{BlockCaches, ExecutionEngine}; // --------------------------------------------------------------------------- // Block processing @@ -63,15 +61,12 @@ use super::ExecutionEngine; /// the one fork where a block might still legitimately carry no real payload, /// if this chain's merge transition has not happened yet. Every later fork /// drops the condition outright, since by then it is always true. -#[allow(clippy::too_many_arguments)] pub fn process_block( state: &mut BeaconState, block: &bellatrix::BeaconBlock, config: &Config, engine: &ExecutionEngine, - committees: &CommitteeCache, - active_balances: &ActiveBalanceCache, - pubkey_indices: &PubkeyIndexCache, + caches: BlockCaches<'_>, options: super::block::BlockProcessingOptions, ) -> Result<()> { super::block::process_block_header( @@ -94,14 +89,14 @@ pub fn process_block( &block.body.deposits, &block.body.voluntary_exits, config, - committees, - active_balances, + caches.committees, + caches.active_balances, )?; super::altair::process_sync_aggregate( state, &block.body.sync_aggregate, - active_balances, - pubkey_indices, + caches.active_balances, + caches.pubkey_indices, )?; Ok(()) } diff --git a/crates/blockchain/state_transition/src/beacon/stf/block.rs b/crates/blockchain/state_transition/src/beacon/stf/block.rs index f3bb60d92..6b5967b7f 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/block.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/block.rs @@ -24,15 +24,14 @@ use crate::beacon::containers::{self, BeaconBlockHeader, BeaconState, Eth1Data, use crate::beacon::error::{Result, verify}; use crate::beacon::hash::hash; use crate::beacon::helpers::accessors::{ - ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, get_beacon_proposer_index, - get_current_epoch, get_domain, get_randao_mix, + get_beacon_proposer_index, get_current_epoch, get_domain, get_randao_mix, }; use crate::beacon::helpers::math::xor; use crate::beacon::helpers::misc::compute_signing_root; use crate::beacon::preset; use crate::beacon::primitives::{BlsSignature, HashTreeRoot as _, Root, Slot, ValidatorIndex}; -use super::{ExecutionEngine, bellatrix, capella, deneb, electra, fulu, gloas}; +use super::{BlockCaches, ExecutionEngine, bellatrix, capella, deneb, electra, fulu, gloas}; /// Dispatches on the block's fork and runs that fork's own block processing. /// @@ -41,97 +40,42 @@ use super::{ExecutionEngine, bellatrix, capella, deneb, electra, fulu, gloas}; /// after [`super::process_slots`] has already advanced the state to the block's /// slot, so every arm validates and mutates a state already positioned at /// `signed_block`'s slot. -#[allow(clippy::too_many_arguments)] pub fn process_block( state: &mut BeaconState, signed_block: &containers::SignedBeaconBlock, config: &Config, engine: &ExecutionEngine, - committees: &CommitteeCache, - active_balances: &ActiveBalanceCache, - pubkey_indices: &PubkeyIndexCache, + caches: BlockCaches<'_>, options: BlockProcessingOptions, ) -> Result<()> { match signed_block { - containers::SignedBeaconBlock::Phase0(signed) => process_block_phase0( - state, - &signed.message, - config, - committees, - active_balances, - options, - ), - containers::SignedBeaconBlock::Altair(signed) => process_block_altair( - state, - &signed.message, - config, - committees, - active_balances, - pubkey_indices, - options, - ), - containers::SignedBeaconBlock::Bellatrix(signed) => bellatrix::process_block( - state, - &signed.message, - config, - engine, - committees, - active_balances, - pubkey_indices, - options, - ), - containers::SignedBeaconBlock::Capella(signed) => capella::process_block( - state, - &signed.message, - config, - engine, - committees, - active_balances, - pubkey_indices, - options, - ), - containers::SignedBeaconBlock::Deneb(signed) => deneb::process_block( - state, - &signed.message, - config, - engine, - committees, - active_balances, - pubkey_indices, - options, - ), - containers::SignedBeaconBlock::Electra(signed) => electra::process_block( - state, - &signed.message, - config, - engine, - committees, - active_balances, - pubkey_indices, - options, - ), - containers::SignedBeaconBlock::Fulu(signed) => fulu::process_block( - state, - &signed.message, - config, - engine, - committees, - active_balances, - pubkey_indices, - options, - ), + containers::SignedBeaconBlock::Phase0(signed) => { + process_block_phase0(state, &signed.message, config, caches, options) + } + containers::SignedBeaconBlock::Altair(signed) => { + process_block_altair(state, &signed.message, config, caches, options) + } + containers::SignedBeaconBlock::Bellatrix(signed) => { + bellatrix::process_block(state, &signed.message, config, engine, caches, options) + } + containers::SignedBeaconBlock::Capella(signed) => { + capella::process_block(state, &signed.message, config, engine, caches, options) + } + containers::SignedBeaconBlock::Deneb(signed) => { + deneb::process_block(state, &signed.message, config, engine, caches, options) + } + containers::SignedBeaconBlock::Electra(signed) => { + electra::process_block(state, &signed.message, config, engine, caches, options) + } + containers::SignedBeaconBlock::Fulu(signed) => { + fulu::process_block(state, &signed.message, config, engine, caches, options) + } // ePBS (EIP-7732) restructures block processing around a builder's // bid rather than an embedded payload: a gloas block carries no // payload for `engine` to answer for, so this arm never reaches it. - containers::SignedBeaconBlock::Gloas(signed) => gloas::process_block( - state, - &signed.message, - config, - committees, - active_balances, - pubkey_indices, - options, - ), + containers::SignedBeaconBlock::Gloas(signed) => { + gloas::process_block(state, &signed.message, config, caches, options) + } containers::SignedBeaconBlock::Lean(_) => { crate::beacon::lean_block_unreachable("process_block") } @@ -144,8 +88,7 @@ pub fn process_block_phase0( state: &mut BeaconState, block: &phase0::BeaconBlock, config: &Config, - committees: &CommitteeCache, - active_balances: &ActiveBalanceCache, + caches: BlockCaches<'_>, options: BlockProcessingOptions, ) -> Result<()> { process_block_header( @@ -165,8 +108,8 @@ pub fn process_block_phase0( &block.body.deposits, &block.body.voluntary_exits, config, - committees, - active_balances, + caches.committees, + caches.active_balances, )?; Ok(()) } @@ -181,9 +124,7 @@ pub fn process_block_altair( state: &mut BeaconState, block: &altair::BeaconBlock, config: &Config, - committees: &CommitteeCache, - active_balances: &ActiveBalanceCache, - pubkey_indices: &PubkeyIndexCache, + caches: BlockCaches<'_>, options: BlockProcessingOptions, ) -> Result<()> { process_block_header( @@ -203,14 +144,14 @@ pub fn process_block_altair( &block.body.deposits, &block.body.voluntary_exits, config, - committees, - active_balances, + caches.committees, + caches.active_balances, )?; super::altair::process_sync_aggregate( state, &block.body.sync_aggregate, - active_balances, - pubkey_indices, + caches.active_balances, + caches.pubkey_indices, )?; Ok(()) } diff --git a/crates/blockchain/state_transition/src/beacon/stf/capella.rs b/crates/blockchain/state_transition/src/beacon/stf/capella.rs index 7aafdbd11..3de310e1e 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/capella.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/capella.rs @@ -49,7 +49,7 @@ use crate::beacon::containers::{BeaconState, capella, phase0}; use crate::beacon::error::{Error, Result, verify}; use crate::beacon::hash::hash; use crate::beacon::helpers::accessors::{ - ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, get_current_epoch, get_randao_mix, + ActiveBalanceCache, CommitteeCache, get_current_epoch, get_randao_mix, }; use crate::beacon::helpers::capella::{ is_fully_withdrawable_validator, is_partially_withdrawable_validator, withdrawal_address, @@ -59,7 +59,7 @@ use crate::beacon::helpers::mutators::decrease_balance; use crate::beacon::preset; use crate::beacon::primitives::{H256, HashTreeRoot as _}; -use super::ExecutionEngine; +use super::{BlockCaches, ExecutionEngine}; // --------------------------------------------------------------------------- // Block processing @@ -72,15 +72,12 @@ use super::ExecutionEngine; /// See this module's own documentation for why [`process_withdrawals`] runs /// before [`process_operations`], and so before this same block's own /// `bls_to_execution_changes` are processed. -#[allow(clippy::too_many_arguments)] pub fn process_block( state: &mut BeaconState, block: &capella::BeaconBlock, config: &Config, engine: &ExecutionEngine, - committees: &CommitteeCache, - active_balances: &ActiveBalanceCache, - pubkey_indices: &PubkeyIndexCache, + caches: BlockCaches<'_>, options: super::block::BlockProcessingOptions, ) -> Result<()> { super::block::process_block_header( @@ -103,14 +100,14 @@ pub fn process_block( &block.body.voluntary_exits, &block.body.bls_to_execution_changes, config, - committees, - active_balances, + caches.committees, + caches.active_balances, )?; super::altair::process_sync_aggregate( state, &block.body.sync_aggregate, - active_balances, - pubkey_indices, + caches.active_balances, + caches.pubkey_indices, )?; Ok(()) } diff --git a/crates/blockchain/state_transition/src/beacon/stf/deneb.rs b/crates/blockchain/state_transition/src/beacon/stf/deneb.rs index a1e111964..65e6e9026 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/deneb.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/deneb.rs @@ -36,7 +36,7 @@ use crate::beacon::containers::{BeaconState, deneb, phase0}; use crate::beacon::error::{Error, Result, verify}; use crate::beacon::hash::hash; use crate::beacon::helpers::accessors::{ - ActiveBalanceCache, ActiveBalanceCacheExt, CommitteeCache, CommitteeCacheExt, PubkeyIndexCache, + ActiveBalanceCache, ActiveBalanceCacheExt, CommitteeCache, CommitteeCacheExt, get_beacon_proposer_index, get_block_root, get_block_root_at_slot, get_current_epoch, get_previous_epoch, get_randao_mix, }; @@ -53,7 +53,7 @@ use crate::beacon::primitives::{ Bytes32, Gwei, H256, HashTreeRoot as _, KzgCommitment, ParticipationFlags, ValidatorIndex, }; -use super::ExecutionEngine; +use super::{BlockCaches, ExecutionEngine}; // --------------------------------------------------------------------------- // Block processing @@ -72,15 +72,12 @@ use super::ExecutionEngine; /// [`super::operations::process_operations`] has no fork check of its own to /// hang them on. See [`process_operations`]'s own documentation for why that /// is a full copy of the loop rather than a smaller patch. -#[allow(clippy::too_many_arguments)] pub fn process_block( state: &mut BeaconState, block: &deneb::BeaconBlock, config: &Config, engine: &ExecutionEngine, - committees: &CommitteeCache, - active_balances: &ActiveBalanceCache, - pubkey_indices: &PubkeyIndexCache, + caches: BlockCaches<'_>, options: super::block::BlockProcessingOptions, ) -> Result<()> { super::block::process_block_header( @@ -109,14 +106,14 @@ pub fn process_block( &block.body.voluntary_exits, &block.body.bls_to_execution_changes, config, - committees, - active_balances, + caches.committees, + caches.active_balances, )?; super::altair::process_sync_aggregate( state, &block.body.sync_aggregate, - active_balances, - pubkey_indices, + caches.active_balances, + caches.pubkey_indices, )?; Ok(()) } @@ -173,10 +170,20 @@ fn process_operations( )?; for proposer_slashing in proposer_slashings { - super::operations::process_proposer_slashing(state, proposer_slashing, config)?; + super::operations::process_proposer_slashing( + state, + proposer_slashing, + config, + active_balances, + )?; } for attester_slashing in attester_slashings { - super::operations::process_attester_slashing(state, attester_slashing, config)?; + super::operations::process_attester_slashing( + state, + attester_slashing, + config, + active_balances, + )?; } for attestation in attestations { process_attestation(state, attestation, committees, active_balances)?; diff --git a/crates/blockchain/state_transition/src/beacon/stf/electra.rs b/crates/blockchain/state_transition/src/beacon/stf/electra.rs index 847c50b2e..f73433d47 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/electra.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/electra.rs @@ -82,7 +82,7 @@ use crate::beacon::primitives::{ WithdrawalIndex, }; -use super::ExecutionEngine; +use super::{BlockCaches, ExecutionEngine}; // --------------------------------------------------------------------------- // Local state projection @@ -592,6 +592,7 @@ pub fn process_attester_slashing( state: &mut BeaconState, attester_slashing: &electra::AttesterSlashing, config: &Config, + active_balances: &ActiveBalanceCache, ) -> Result<()> { let attestation_1 = &attester_slashing.attestation_1; let attestation_2 = &attester_slashing.attestation_2; @@ -614,6 +615,7 @@ pub fn process_attester_slashing( &attestation_1.attesting_indices, &attestation_2.attesting_indices, config, + active_balances, ) } @@ -1423,9 +1425,12 @@ pub fn process_withdrawal_request( // Execution-layer consolidation requests // --------------------------------------------------------------------------- -/// Whether `request` is really a request to switch its source (and only -/// source) validator from an eth1 to a compounding withdrawal credential, -/// disguised as a self-consolidation. +/// The specification's `is_valid_switch_to_compounding_request`: whether +/// `request` is really a request to switch its source (and only source) +/// validator from an eth1 to a compounding withdrawal credential, disguised as +/// a self-consolidation. Answers with the source's index when it is (`None` +/// where the specification returns `False`), so the caller acting on it does +/// not resolve the same pubkey a second time. /// /// A validator that already holds an eth1 credential has no other way to /// become compounding: [`crate::beacon::helpers::electra::switch_to_compounding_validator`] @@ -1435,39 +1440,39 @@ pub fn process_withdrawal_request( /// functions; nothing inside can actually fail, since every condition here /// is itself one of the specification's own `return False` guards rather /// than an assertion. -pub fn is_valid_switch_to_compounding_request( +pub fn switch_to_compounding_source( state: &BeaconState, request: &electra::ConsolidationRequest, pubkey_indices: &PubkeyIndexCache, -) -> Result { +) -> Result> { if request.source_pubkey != request.target_pubkey { - return Ok(false); + return Ok(None); } let Some(source_index) = pubkey_indices.validator_index(state, &request.source_pubkey) else { - return Ok(false); + return Ok(None); }; let source_validator = state.validator(source_index)?; if withdrawal_address(source_validator) != request.source_address { - return Ok(false); + return Ok(None); } if !has_eth1_withdrawal_credential(source_validator) { - return Ok(false); + return Ok(None); } if !is_active_validator(source_validator, get_current_epoch(state)) { - return Ok(false); + return Ok(None); } if source_validator.exit_epoch != FAR_FUTURE_EPOCH { - return Ok(false); + return Ok(None); } - Ok(true) + Ok(Some(source_index)) } /// Honors (or quietly drops) an execution-layer-triggered consolidation /// (EIP-7251): either a switch-to-compounding request in disguise (see -/// [`is_valid_switch_to_compounding_request`]), or a real merge of one +/// [`switch_to_compounding_source`]), or a real merge of one /// validator's balance into another's. /// /// Every early return here, in both branches, is the specification's own @@ -1491,14 +1496,7 @@ pub fn process_consolidation_request( active_balances: &ActiveBalanceCache, pubkey_indices: &PubkeyIndexCache, ) -> Result<()> { - if is_valid_switch_to_compounding_request(state, request, pubkey_indices)? { - // Already known to resolve, by the check just above; state has not - // been mutated since. - let source_index = pubkey_indices - .validator_index(state, &request.source_pubkey) - .ok_or(Error::SpecAssert( - "is_valid_switch_to_compounding_request already resolved consolidation_request.source_pubkey", - ))?; + if let Some(source_index) = switch_to_compounding_source(state, request, pubkey_indices)? { crate::beacon::helpers::electra::switch_to_compounding_validator(state, source_index)?; return Ok(()); } @@ -1811,10 +1809,15 @@ pub fn process_operations( } for proposer_slashing in body.proposer_slashings.iter() { - super::operations::process_proposer_slashing(state, proposer_slashing, config)?; + super::operations::process_proposer_slashing( + state, + proposer_slashing, + config, + active_balances, + )?; } for attester_slashing in body.attester_slashings.iter() { - process_attester_slashing(state, attester_slashing, config)?; + process_attester_slashing(state, attester_slashing, config, active_balances)?; } // [Modified in Electra:EIP7549] for attestation in body.attestations.iter() { @@ -1859,15 +1862,12 @@ pub fn process_operations( /// them taking those fields directly rather than a whole body (see /// `crate::beacon::stf`'s module documentation). [`super::altair::process_sync_aggregate`] /// is reused the same way, for the same reason. -#[allow(clippy::too_many_arguments)] pub fn process_block( state: &mut BeaconState, block: &electra::BeaconBlock, config: &Config, engine: &ExecutionEngine, - committees: &CommitteeCache, - active_balances: &ActiveBalanceCache, - pubkey_indices: &PubkeyIndexCache, + caches: BlockCaches<'_>, options: super::block::BlockProcessingOptions, ) -> Result<()> { super::block::process_block_header( @@ -1885,15 +1885,15 @@ pub fn process_block( state, &block.body, config, - committees, - active_balances, - pubkey_indices, + caches.committees, + caches.active_balances, + caches.pubkey_indices, )?; super::altair::process_sync_aggregate( state, &block.body.sync_aggregate, - active_balances, - pubkey_indices, + caches.active_balances, + caches.pubkey_indices, )?; Ok(()) } diff --git a/crates/blockchain/state_transition/src/beacon/stf/fulu.rs b/crates/blockchain/state_transition/src/beacon/stf/fulu.rs index 0a299d36e..bbd4d3624 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/fulu.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/fulu.rs @@ -50,7 +50,7 @@ use crate::beacon::helpers::electra::pending_queue_fields; use crate::beacon::helpers::fulu::{fulu_state, fulu_state_ref}; use crate::beacon::primitives::{Bytes32, HashTreeRoot as _}; -use super::ExecutionEngine; +use super::{BlockCaches, ExecutionEngine}; // --------------------------------------------------------------------------- // Block processing @@ -61,15 +61,12 @@ use super::ExecutionEngine; /// for [`super::electra::process_execution_payload`] and /// [`super::electra::process_operations`]. See the module docs for why these /// are the only steps this fork's specification asks to change. -#[allow(clippy::too_many_arguments)] pub fn process_block( state: &mut BeaconState, block: &electra::BeaconBlock, config: &Config, engine: &ExecutionEngine, - committees: &CommitteeCache, - active_balances: &ActiveBalanceCache, - pubkey_indices: &PubkeyIndexCache, + caches: BlockCaches<'_>, options: super::block::BlockProcessingOptions, ) -> Result<()> { super::block::process_block_header( @@ -87,15 +84,15 @@ pub fn process_block( state, &block.body, config, - committees, - active_balances, - pubkey_indices, + caches.committees, + caches.active_balances, + caches.pubkey_indices, )?; super::altair::process_sync_aggregate( state, &block.body.sync_aggregate, - active_balances, - pubkey_indices, + caches.active_balances, + caches.pubkey_indices, )?; Ok(()) } @@ -244,10 +241,20 @@ pub fn process_operations( verify(body.deposits.is_empty(), "len(body.deposits) == 0")?; for proposer_slashing in body.proposer_slashings.iter() { - super::operations::process_proposer_slashing(state, proposer_slashing, config)?; + super::operations::process_proposer_slashing( + state, + proposer_slashing, + config, + active_balances, + )?; } for attester_slashing in body.attester_slashings.iter() { - super::electra::process_attester_slashing(state, attester_slashing, config)?; + super::electra::process_attester_slashing( + state, + attester_slashing, + config, + active_balances, + )?; } for attestation in body.attestations.iter() { super::electra::process_attestation(state, attestation, committees, active_balances)?; diff --git a/crates/blockchain/state_transition/src/beacon/stf/gloas.rs b/crates/blockchain/state_transition/src/beacon/stf/gloas.rs index d0cbc0298..e182ee8d2 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/gloas.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/gloas.rs @@ -77,7 +77,7 @@ use crate::beacon::primitives::{ ValidatorIndex, WithdrawalIndex, }; -use super::ExecutionEngine; +use super::{BlockCaches, ExecutionEngine}; // --------------------------------------------------------------------------- // Block processing @@ -102,16 +102,20 @@ pub fn process_block( state: &mut BeaconState, block: &gloas::BeaconBlock, config: &Config, - committees: &CommitteeCache, - active_balances: &ActiveBalanceCache, - pubkey_indices: &PubkeyIndexCache, + caches: BlockCaches<'_>, options: super::block::BlockProcessingOptions, ) -> Result<()> { // [New in Gloas:EIP7732] let parent_slot = state.latest_block_header().slot; // [New in Gloas:EIP7732] - process_parent_execution_payload(state, block, config, active_balances, pubkey_indices)?; + process_parent_execution_payload( + state, + block, + config, + caches.active_balances, + caches.pubkey_indices, + )?; super::block::process_block_header( state, block.slot, @@ -132,14 +136,14 @@ pub fn process_block( &block.body, parent_slot, config, - committees, - active_balances, + caches.committees, + caches.active_balances, )?; super::altair::process_sync_aggregate( state, &block.body.sync_aggregate, - active_balances, - pubkey_indices, + caches.active_balances, + caches.pubkey_indices, )?; Ok(()) } @@ -1208,6 +1212,7 @@ pub fn process_proposer_slashing( state: &mut BeaconState, proposer_slashing: &ProposerSlashing, config: &Config, + active_balances: &ActiveBalanceCache, ) -> Result<()> { let (proposer_index, current_epoch) = crate::beacon::stf::operations::verify_proposer_slashing(state, proposer_slashing)?; @@ -1228,7 +1233,7 @@ pub fn process_proposer_slashing( clear_builder_pending_payment_if_owned_by(state, payment_index, proposer_index)?; } - slash_validator(state, proposer_index, None, config)?; + slash_validator(state, proposer_index, None, config, Some(active_balances))?; Ok(()) } @@ -1271,6 +1276,7 @@ pub fn process_attester_slashing( state: &mut BeaconState, attester_slashing: &gloas::AttesterSlashing, config: &Config, + active_balances: &ActiveBalanceCache, ) -> Result<()> { let attestation_1 = &attester_slashing.attestation_1; let attestation_2 = &attester_slashing.attestation_2; @@ -1293,6 +1299,7 @@ pub fn process_attester_slashing( &attestation_1.attesting_indices, &attestation_2.attesting_indices, config, + active_balances, ) } @@ -1644,10 +1651,10 @@ pub fn process_operations( // [Modified in Gloas:EIP7732] for proposer_slashing in body.proposer_slashings.iter() { - process_proposer_slashing(state, proposer_slashing, config)?; + process_proposer_slashing(state, proposer_slashing, config, active_balances)?; } for attester_slashing in body.attester_slashings.iter() { - process_attester_slashing(state, attester_slashing, config)?; + process_attester_slashing(state, attester_slashing, config, active_balances)?; } // [Modified in Gloas:EIP7732] for attestation in body.attestations.iter() { diff --git a/crates/blockchain/state_transition/src/beacon/stf/mod.rs b/crates/blockchain/state_transition/src/beacon/stf/mod.rs index f87ffdc6c..cc2020fa1 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/mod.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/mod.rs @@ -105,6 +105,45 @@ impl ExecutionEngine { } } +/// The tables block processing reads through instead of recomputing per +/// block: committees, the total active balance, and pubkey to index. Borrowed +/// together, so a block-processing entry point takes one argument for all of +/// them, and a new table is a field here rather than a parameter on every +/// fork's `process_block`. +/// +/// Which tables a caller passes is its own decision: the chain actor's fork +/// choice passes the `Store`'s shared ones, so every import reuses what earlier +/// ones built; a caller with no `Store` holds an [`OwnedBlockCaches`]. Every +/// table is sound to share across forks (each is keyed or verified per state; +/// see each type), so mixing shared and fresh ones is a cost decision only. +#[derive(Debug, Clone, Copy)] +pub struct BlockCaches<'a> { + pub committees: &'a CommitteeCache, + pub active_balances: &'a ActiveBalanceCache, + pub pubkey_indices: &'a PubkeyIndexCache, +} + +/// A fresh set of the tables [`BlockCaches`] borrows, for a caller with no +/// `Store` (a spec runner, a unit test, the era replay) to hold for as long +/// as its work lasts. +#[derive(Debug, Default)] +pub struct OwnedBlockCaches { + pub committees: CommitteeCache, + pub active_balances: ActiveBalanceCache, + pub pubkey_indices: PubkeyIndexCache, +} + +impl OwnedBlockCaches { + /// Borrows every table. + pub fn caches(&self) -> BlockCaches<'_> { + BlockCaches { + committees: &self.committees, + active_balances: &self.active_balances, + pubkey_indices: &self.pubkey_indices, + } + } +} + /// Applies a signed block to the state. /// /// With `validate_result` set, the proposer's signature and the block's committed @@ -129,28 +168,16 @@ impl ExecutionEngine { /// specification requires instead of failing on it. Rejecting the mismatch /// before any of that runs is what keeps the failure legible: "wrong fork," not /// some unrelated-looking assertion three steps later. -#[allow(clippy::too_many_arguments)] pub fn state_transition( state: &mut BeaconState, signed_block: &containers::SignedBeaconBlock, validate_result: bool, config: &Config, engine: &ExecutionEngine, - committees: &CommitteeCache, - active_balances: &ActiveBalanceCache, - pubkey_indices: &PubkeyIndexCache, + caches: BlockCaches<'_>, ) -> Result<()> { process_slots(state, signed_block.slot(), config)?; - apply_block( - state, - signed_block, - validate_result, - config, - engine, - committees, - active_balances, - pubkey_indices, - ) + apply_block(state, signed_block, validate_result, config, engine, caches) } /// [`state_transition`] from the point where `state` is already at the block's @@ -162,16 +189,13 @@ pub fn state_transition( /// when the block sits exactly on the boundary. A state at the wrong slot is /// still refused: the block header check in `process_block` compares the /// block's slot with the state's. -#[allow(clippy::too_many_arguments)] pub fn apply_block( state: &mut BeaconState, signed_block: &containers::SignedBeaconBlock, validate_result: bool, config: &Config, engine: &ExecutionEngine, - committees: &CommitteeCache, - active_balances: &ActiveBalanceCache, - pubkey_indices: &PubkeyIndexCache, + caches: BlockCaches<'_>, ) -> Result<()> { // After `process_slots`, never before it. A block proposed at the first slot // of a fork's activation epoch is the *post*-fork shape while the state @@ -196,9 +220,7 @@ pub fn apply_block( signed_block, config, engine, - committees, - active_balances, - pubkey_indices, + caches, block::BlockProcessingOptions::default(), )?; // As in `process_slot`: the post-state's root, checked below and needed diff --git a/crates/blockchain/state_transition/src/beacon/stf/operations.rs b/crates/blockchain/state_transition/src/beacon/stf/operations.rs index 5f9add7cc..7f76caf4f 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/operations.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/operations.rs @@ -118,10 +118,10 @@ pub fn process_operations( )?; for proposer_slashing in proposer_slashings { - process_proposer_slashing(state, proposer_slashing, config)?; + process_proposer_slashing(state, proposer_slashing, config, active_balances)?; } for attester_slashing in attester_slashings { - process_attester_slashing(state, attester_slashing, config)?; + process_attester_slashing(state, attester_slashing, config, active_balances)?; } for attestation in attestations { // Phase0 defers an attestation's reward to the epoch boundary, so it @@ -226,9 +226,10 @@ pub fn process_proposer_slashing( state: &mut BeaconState, proposer_slashing: &ProposerSlashing, config: &Config, + active_balances: &ActiveBalanceCache, ) -> Result<()> { let (proposer_index, _current_epoch) = verify_proposer_slashing(state, proposer_slashing)?; - slash_validator(state, proposer_index, None, config)?; + slash_validator(state, proposer_index, None, config, Some(active_balances))?; Ok(()) } @@ -266,6 +267,7 @@ pub(crate) fn slash_attesting_index_intersection( attesting_indices_1: &[ValidatorIndex], attesting_indices_2: &[ValidatorIndex], config: &Config, + active_balances: &ActiveBalanceCache, ) -> Result<()> { let current_epoch = get_current_epoch(state); let indices_2: HashSet = attesting_indices_2.iter().copied().collect(); @@ -276,7 +278,7 @@ pub(crate) fn slash_attesting_index_intersection( continue; } if is_slashable_validator(state.validator(index)?, current_epoch) { - slash_validator(state, index, None, config)?; + slash_validator(state, index, None, config, Some(active_balances))?; slashed_any = true; } } @@ -295,6 +297,7 @@ pub fn process_attester_slashing( state: &mut BeaconState, attester_slashing: &phase0::AttesterSlashing, config: &Config, + active_balances: &ActiveBalanceCache, ) -> Result<()> { let attestation_1 = &attester_slashing.attestation_1; let attestation_2 = &attester_slashing.attestation_2; @@ -317,6 +320,7 @@ pub fn process_attester_slashing( &attestation_1.attesting_indices, &attestation_2.attesting_indices, config, + active_balances, ) } @@ -705,7 +709,15 @@ mod tests { signed_header_2: signed_header, }; - assert!(process_proposer_slashing(&mut state, &slashing, &config).is_err()); + assert!( + process_proposer_slashing( + &mut state, + &slashing, + &config, + &ActiveBalanceCache::default() + ) + .is_err() + ); } #[test] @@ -790,6 +802,14 @@ mod tests { attestation_2, }; - assert!(process_attester_slashing(&mut state, &slashing, &config).is_err()); + assert!( + process_attester_slashing( + &mut state, + &slashing, + &config, + &ActiveBalanceCache::default() + ) + .is_err() + ); } } diff --git a/crates/blockchain/state_transition/tests/beacon_spec/operations.rs b/crates/blockchain/state_transition/tests/beacon_spec/operations.rs index 49fa4d8c8..553c7aa7b 100644 --- a/crates/blockchain/state_transition/tests/beacon_spec/operations.rs +++ b/crates/blockchain/state_transition/tests/beacon_spec/operations.rs @@ -326,7 +326,7 @@ fn apply( | ForkName::Capella | ForkName::Deneb => { let slashing: phase0::AttesterSlashing = case.ssz("attester_slashing"); - operations::process_attester_slashing(state, &slashing, config) + operations::process_attester_slashing(state, &slashing, config, &active_balances) } // Electra's container widens the same way `Attestation`'s does // (see the `attestation` arm above); the function itself is not @@ -334,14 +334,14 @@ fn apply( // Fulu's specification changes neither. ForkName::Electra | ForkName::Fulu => { let slashing: electra::AttesterSlashing = case.ssz("attester_slashing"); - electra_stf::process_attester_slashing(state, &slashing, config) + electra_stf::process_attester_slashing(state, &slashing, config, &active_balances) } // Gloas's container widens again (EIP-7688's unbounded // `AttestingIndices`); the function is still not behaviorally // modified, only transcribed against gloas's own type. ForkName::Gloas => { let slashing: gloas::AttesterSlashing = case.ssz("attester_slashing"); - gloas_stf::process_attester_slashing(state, &slashing, config) + gloas_stf::process_attester_slashing(state, &slashing, config, &active_balances) } ForkName::Lean => lean_is_not_a_fixture_fork("attester_slashing"), }, @@ -360,8 +360,15 @@ fn apply( | ForkName::Capella | ForkName::Deneb | ForkName::Electra - | ForkName::Fulu => operations::process_proposer_slashing(state, &slashing, config), - ForkName::Gloas => gloas_stf::process_proposer_slashing(state, &slashing, config), + | ForkName::Fulu => operations::process_proposer_slashing( + state, + &slashing, + config, + &active_balances, + ), + ForkName::Gloas => { + gloas_stf::process_proposer_slashing(state, &slashing, config, &active_balances) + } ForkName::Lean => lean_is_not_a_fixture_fork("proposer_slashing"), } } diff --git a/crates/blockchain/state_transition/tests/beacon_spec/sanity.rs b/crates/blockchain/state_transition/tests/beacon_spec/sanity.rs index bd615f24a..c94ffd03d 100644 --- a/crates/blockchain/state_transition/tests/beacon_spec/sanity.rs +++ b/crates/blockchain/state_transition/tests/beacon_spec/sanity.rs @@ -15,10 +15,7 @@ use ethlambda_state_transition::beacon::config::Config; use ethlambda_state_transition::beacon::containers::{BeaconState, SignedBeaconBlock}; -use ethlambda_state_transition::beacon::helpers::accessors::{ - ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, -}; -use ethlambda_state_transition::beacon::stf::{self, ExecutionEngine}; +use ethlambda_state_transition::beacon::stf::{self, ExecutionEngine, OwnedBlockCaches}; use libtest_mimic::Trial; use super::{Case, PRESET, collect}; @@ -49,9 +46,7 @@ fn apply_blocks(case: &Case, state: &mut BeaconState, config: &Config) -> Result // One cache across the case's blocks, as the node holds one across its // imports, so consecutive blocks of an epoch share its shuffling. - let committees = CommitteeCache::default(); - let active_balances = ActiveBalanceCache::default(); - let pubkey_indices = PubkeyIndexCache::default(); + let caches = OwnedBlockCaches::default(); for index in 0..meta.blocks_count { let bytes = case.ssz_bytes_indexed("blocks", index); @@ -62,17 +57,8 @@ fn apply_blocks(case: &Case, state: &mut BeaconState, config: &Config) -> Result state.latest_block_header_mut().state_root = root; } - stf::state_transition( - state, - &block, - true, - config, - &engine, - &committees, - &active_balances, - &pubkey_indices, - ) - .map_err(|err| format!("block {index} rejected: {err:?}"))?; + stf::state_transition(state, &block, true, config, &engine, caches.caches()) + .map_err(|err| format!("block {index} rejected: {err:?}"))?; previous_state_root = Some(block.state_root()); } diff --git a/crates/blockchain/state_transition/tests/beacon_spec/transition.rs b/crates/blockchain/state_transition/tests/beacon_spec/transition.rs index ca6125181..268cd4a98 100644 --- a/crates/blockchain/state_transition/tests/beacon_spec/transition.rs +++ b/crates/blockchain/state_transition/tests/beacon_spec/transition.rs @@ -69,12 +69,9 @@ use std::sync::Arc; use ethlambda_state_transition::beacon::ForkName; use ethlambda_state_transition::beacon::config::Config; use ethlambda_state_transition::beacon::containers::{BeaconState, SignedBeaconBlock}; -use ethlambda_state_transition::beacon::helpers::accessors::{ - ActiveBalanceCache, CommitteeCache, PubkeyIndexCache, -}; use ethlambda_state_transition::beacon::helpers::misc::compute_start_slot_at_epoch; use ethlambda_state_transition::beacon::primitives::Epoch; -use ethlambda_state_transition::beacon::stf::{self, ExecutionEngine}; +use ethlambda_state_transition::beacon::stf::{self, ExecutionEngine, OwnedBlockCaches}; use libtest_mimic::Trial; use super::{Case, PRESET, collect}; @@ -139,9 +136,7 @@ fn apply_blocks( // One cache across the case's blocks, as the node holds one across its // imports, so consecutive blocks of an epoch share its shuffling. - let committees = CommitteeCache::default(); - let active_balances = ActiveBalanceCache::default(); - let pubkey_indices = PubkeyIndexCache::default(); + let caches = OwnedBlockCaches::default(); for index in 0..meta.blocks_count { let fork = if index < first_post_fork_index { @@ -158,17 +153,8 @@ fn apply_blocks( state.latest_block_header_mut().state_root = root; } - stf::state_transition( - state, - &block, - true, - &config, - &engine, - &committees, - &active_balances, - &pubkey_indices, - ) - .map_err(|err| format!("block {index} rejected: {err:?}"))?; + stf::state_transition(state, &block, true, &config, &engine, caches.caches()) + .map_err(|err| format!("block {index} rejected: {err:?}"))?; previous_state_root = Some(block.state_root()); } diff --git a/crates/net/rpc/src/beacon/gloas_proposal.rs b/crates/net/rpc/src/beacon/gloas_proposal.rs index 51b57d76e..c675b3404 100644 --- a/crates/net/rpc/src/beacon/gloas_proposal.rs +++ b/crates/net/rpc/src/beacon/gloas_proposal.rs @@ -77,7 +77,7 @@ use ethlambda_state_transition::beacon::{ }, sync_committee_pool::SharedSyncCommitteePool, }; -use ethlambda_storage::Store; +use ethlambda_storage::{PubkeyIndexCache, Store}; use ethlambda_types::{ beacon::{ config::Config, @@ -434,6 +434,10 @@ struct Prepared { /// What the block's state transition may leave unchecked: the reveal, when /// the caller set `skip_randao_verification`. options: BlockProcessingOptions, + /// The store's shared pubkey table, which every state transition of the + /// proposal resolves execution requests and the sync committee through + /// (a fresh one would ingest the whole registry each time). + pubkey_indices: Arc, } #[allow(clippy::too_many_arguments)] @@ -686,8 +690,15 @@ fn prepare( if randao == RandaoVerification::Verify { verify_randao_reveal(&state, proposer, randao_reveal)?; } - let mut inputs = gloas_payload_inputs(&state, build_on_full, &parent_requests, &config) - .map_err(|_| ApiError::Internal("computing the payload attributes failed"))?; + let pubkey_indices = store.pubkey_index_cache(); + let mut inputs = gloas_payload_inputs( + &state, + build_on_full, + &parent_requests, + &config, + &pubkey_indices, + ) + .map_err(|_| ApiError::Internal("computing the payload attributes failed"))?; // The proposer's signed preferences for this slot, under the dependent root // the head's chain gives it. They name the fee recipient and gas target the @@ -707,6 +718,7 @@ fn prepare( parent_requests, preferences, options: BlockProcessingOptions { randao }, + pubkey_indices, }) } @@ -865,6 +877,7 @@ fn assemble( head_slot, parent_requests, options, + pubkey_indices, .. } = prepared; let attestations = pack_gloas_attestations(state, candidates); @@ -891,7 +904,7 @@ fn assemble( }; let operations = attestations.len() + payload_attestations.len(); let attempt = inputs(attestations, payload_attestations, sync_aggregate); - match assemble_gloas_block(state, attempt, config, *options) { + match assemble_gloas_block(state, attempt, config, pubkey_indices, *options) { Ok(produced) => Ok(produced), // The packers check every operation's signature against this state, and // the sync aggregate was verified above, so this should not happen; but @@ -899,7 +912,7 @@ fn assemble( Err(err) if operations > 0 || has_sync_aggregate => { warn!(slot = state.slot(), %err, "Block with operations failed to build; retrying without"); let bare = inputs(Vec::new(), Vec::new(), empty_sync_aggregate()); - assemble_gloas_block(state, bare, config, *options) + assemble_gloas_block(state, bare, config, pubkey_indices, *options) .map_err(|_| ApiError::Internal("the block failed to build")) } Err(_) => Err(ApiError::Internal("the block failed to build")), @@ -923,6 +936,7 @@ fn assemble_on_bid( head_slot, parent_requests, options, + pubkey_indices, .. } = prepared; let attestations = pack_gloas_attestations(state, candidates); @@ -941,13 +955,20 @@ fn assemble_on_bid( state, inputs(attestations, payload_attestations), config, + pubkey_indices, *options, ) { Ok(block) => Ok(block), Err(err) if operations > 0 => { warn!(slot = state.slot(), %err, "Bid block with operations failed to build; retrying without"); - assemble_gloas_block_on_bid(state, inputs(Vec::new(), Vec::new()), config, *options) - .map_err(|_| ApiError::Internal("the block failed to build")) + assemble_gloas_block_on_bid( + state, + inputs(Vec::new(), Vec::new()), + config, + pubkey_indices, + *options, + ) + .map_err(|_| ApiError::Internal("the block failed to build")) } Err(_) => Err(ApiError::Internal("the block failed to build")), } @@ -1517,7 +1538,14 @@ mod tests { test_support::{payload_for, randao_reveal}, }; let requests = ExecutionRequests::default(); - let inputs = gloas_payload_inputs(state, true, &requests, &config()).unwrap(); + let inputs = gloas_payload_inputs( + state, + true, + &requests, + &config(), + &PubkeyIndexCache::default(), + ) + .unwrap(); assemble_gloas_block( state, GloasBlockInputs { @@ -1533,6 +1561,7 @@ mod tests { execution_requests: ExecutionRequests::default(), }, &config(), + &PubkeyIndexCache::default(), ethlambda_state_transition::beacon::stf::block::BlockProcessingOptions::default(), ) .unwrap() @@ -1571,7 +1600,14 @@ mod tests { payload_for, randao_reveal, }; let requests = ExecutionRequests::default(); - let inputs = gloas_payload_inputs(state, true, &requests, &config()).unwrap(); + let inputs = gloas_payload_inputs( + state, + true, + &requests, + &config(), + &PubkeyIndexCache::default(), + ) + .unwrap(); let built = BuiltGloasPayload { execution_payload: payload_for(&inputs), block_value: Default::default(), @@ -1588,6 +1624,7 @@ mod tests { parent_requests: requests.clone(), preferences: None, options: BlockProcessingOptions::default(), + pubkey_indices: Default::default(), }; assemble( &config(), @@ -1854,7 +1891,14 @@ mod tests { let commitment = blob_to_kzg_commitment(&blob).unwrap(); let (_, proofs) = compute_cells_and_kzg_proofs(&blob).unwrap(); let requests = ExecutionRequests::default(); - let inputs = gloas_payload_inputs(&state, true, &requests, &config()).unwrap(); + let inputs = gloas_payload_inputs( + &state, + true, + &requests, + &config(), + &PubkeyIndexCache::default(), + ) + .unwrap(); let produced = assemble_gloas_block( &state, GloasBlockInputs { @@ -1870,6 +1914,7 @@ mod tests { execution_requests: ExecutionRequests::default(), }, &config(), + &PubkeyIndexCache::default(), ethlambda_state_transition::beacon::stf::block::BlockProcessingOptions::default(), ) .unwrap(); diff --git a/crates/net/rpc/src/beacon/proposal.rs b/crates/net/rpc/src/beacon/proposal.rs index 8dcd6115d..a50212883 100644 --- a/crates/net/rpc/src/beacon/proposal.rs +++ b/crates/net/rpc/src/beacon/proposal.rs @@ -572,10 +572,14 @@ async fn produce( }; let attestation_count = attestations.len(); let has_operations = !operations.is_empty(); + // The store's shared pubkey table: a fresh one would ingest the whole + // registry on the block's first lookup, every proposal. + let pubkey_indices = store.pubkey_index_cache(); let block = match assemble_block( &state, inputs(attestations, operations, sync_aggregate), &config, + &pubkey_indices, options, ) { Ok(block) => block, @@ -589,6 +593,7 @@ async fn produce( &state, inputs(Vec::new(), Operations::default(), empty_sync_aggregate()), &config, + &pubkey_indices, options, ) .map_err(|_| ApiError::Internal("the block failed to build"))? diff --git a/crates/storage/src/lib.rs b/crates/storage/src/lib.rs index 1b91825d9..e752cf868 100644 --- a/crates/storage/src/lib.rs +++ b/crates/storage/src/lib.rs @@ -22,7 +22,7 @@ pub use committee_cache::{CommitteeCache, Lookup, ShufflingKey}; /// callers can match on it (e.g. to distinguish [`Error::DbVersionMismatch`]). pub use error::Error; pub use liveness::ObservedLiveness; -pub use pubkey_index_cache::{PubkeyIndexCache, TAIL_MERGE_THRESHOLD}; +pub use pubkey_index_cache::PubkeyIndexCache; // `CacheKey` lives in `state_writer` (beside the `StateCache` it keys), not // `store`; re-exported here so the public path (`ethlambda_storage::CacheKey`) // is unaffected by which module owns it. diff --git a/crates/storage/src/pubkey_index_cache.rs b/crates/storage/src/pubkey_index_cache.rs index 4972165c0..afeb3c7b3 100644 --- a/crates/storage/src/pubkey_index_cache.rs +++ b/crates/storage/src/pubkey_index_cache.rs @@ -3,10 +3,10 @@ //! //! A sibling of [`crate::active_balance_cache::ActiveBalanceCache`], held by //! the `Store` for the same reasons (one table shared by the chain actor's -//! state transition and anything else holding a `Store` clone). A caller with -//! no `Store` of its own (a spec runner, a unit test, block production's -//! scratch state) holds a fresh [`PubkeyIndexCache::default`] for as long as -//! that work lasts. +//! state transition, block production, and anything else holding a `Store` +//! clone). A caller with no `Store` of its own (a spec runner, a unit test, the +//! era replay) holds a fresh [`PubkeyIndexCache::default`] for as long as that +//! work lasts. //! //! This module knows nothing about a `BeaconState`. It stores positions it is //! told about and answers *candidates*; the consensus logic that feeds it a @@ -23,7 +23,8 @@ //! make a candidate wrong, never make a right one disappear, so the caller //! checks every candidate against its own state and falls back to a scan when //! none passes. A missing or wrong entry is a slower lookup, never a wrong -//! validator. +//! validator, and the index a fallback scan finds is recorded +//! ([`PubkeyIndexCache::insert`]), so it costs one scan, not one per lookup. //! //! # Layout //! @@ -34,30 +35,34 @@ //! two real keys negligible. //! //! Entries live in a sorted base (two parallel arrays, binary searched) plus -//! a small hash map holding positions appended since the base was last built. +//! a small ordered set holding entries added since the base was last built. //! The registry grows by a handful of validators a block at most, so the tail //! absorbs that growth cheaply; when it reaches [`TAIL_MERGE_THRESHOLD`] it is -//! merged into the base in one linear pass. +//! merged into the base in one linear pass. Both halves keep every index +//! recorded under a key, so equal keys (a prefix collision, or one pubkey at +//! two indices on two forks) are all returned as candidates. -use std::collections::HashMap; +use std::collections::BTreeSet; use std::sync::RwLock; use ethlambda_types::beacon::primitives::BlsPubkey; -/// How many appended entries the hash map tail holds before it is merged into -/// the sorted base. +/// How many entries the tail holds before it is merged into the sorted base. /// /// A merge is linear in the whole table, so it must be rare: this many appends /// is on the order of the validators a mainnet day adds. It is also small -/// enough that the tail's per-entry cost (a hash map slot, several times a -/// base entry) stays a rounding error next to the base. -pub const TAIL_MERGE_THRESHOLD: usize = 8192; +/// enough that the tail's per-entry cost (a B-tree slot, a few times a base +/// entry) stays a rounding error next to the base. +const TAIL_MERGE_THRESHOLD: usize = 8192; /// Pubkey-prefix to validator-index table. See the module documentation. /// -/// Internally synchronized, so every method takes `&self`. The write lock is -/// held only to splice entries in, never while a state is scanned: callers -/// collect a state's tail into a buffer first and hand it over. +/// Internally synchronized, so every method takes `&self`. Nothing slow runs +/// under the write lock except a merge: callers collect a state's keys into a +/// buffer first, and [`Self::extend`] builds and sorts its batch before taking +/// the lock. What it holds the lock for is the splice: a tail insert per +/// appended position, or one linear merge into the base, which happens on the +/// first (bulk) ingest and once every [`TAIL_MERGE_THRESHOLD`] entries after. #[derive(Debug, Default)] pub struct PubkeyIndexCache { inner: RwLock, @@ -66,13 +71,12 @@ pub struct PubkeyIndexCache { #[derive(Debug, Default)] struct Inner { /// Sorted ascending by `(key, index)`; `indices[i]` belongs to `keys[i]`. - /// Equal keys (a prefix collision, or one pubkey seen at two indices on - /// two forks) sit next to each other and are all returned as candidates. keys: Vec, indices: Vec, - /// Positions appended since the base was built. First writer wins for a - /// key; a second candidate for the same key just costs the caller a scan. - tail: HashMap, + /// `(key, index)` entries added since the base was built: appended + /// registry positions, and indices a fallback scan found. Ordered, so a + /// key's entries are one range and a merge needs no sort. + tail: BTreeSet<(u64, u32)>, /// How many registry positions have been ingested: positions /// `0..covered_len` of some chain's registry, in order. covered_len: usize, @@ -101,16 +105,16 @@ impl PubkeyIndexCache { pub fn candidates(&self, pubkey: &BlsPubkey) -> Vec { let key = Self::key_of(pubkey); let inner = self.inner.read().unwrap(); - let start = inner.keys.partition_point(|candidate| *candidate < key); - let mut found: Vec = inner.keys[start..] - .iter() - .zip(&inner.indices[start..]) - .take_while(|(candidate, _)| **candidate == key) - .map(|(_, index)| u64::from(*index)) + let mut found: Vec = inner + .base_range(key) + .map(|position| u64::from(inner.indices[position])) .collect(); - if let Some(index) = inner.tail.get(&key) { - found.push(u64::from(*index)); - } + found.extend( + inner + .tail + .range((key, 0)..=(key, u32::MAX)) + .map(|(_, index)| u64::from(*index)), + ); found } @@ -127,53 +131,95 @@ impl PubkeyIndexCache { /// covered but not stored: such a lookup finds no candidate and takes the /// caller's scan, which is correct and unreachable in practice. pub fn extend(&self, start: usize, keys: &[u64]) -> bool { + // Every position follows from `start`, so the batch is built, and a + // bulk one sorted, before the lock is taken: the first ingest of a + // whole registry sorts millions of entries, which no reader should + // wait on. + let mut batch: Vec<(u64, u32)> = keys + .iter() + .enumerate() + .filter_map(|(offset, key)| { + u32::try_from(start + offset) + .ok() + .map(|index| (*key, index)) + }) + .collect(); + let bulk = batch.len() >= TAIL_MERGE_THRESHOLD; + if bulk { + // Sorting directly beats funnelling millions of entries through + // the tail. + batch.sort_unstable(); + } + let mut inner = self.inner.write().unwrap(); - if start > inner.covered_len { + let covered = inner.covered_len; + if start > covered { return false; } - let skip = (inner.covered_len - start).min(keys.len()); - let fresh = &keys[skip..]; - let first_position = inner.covered_len; - let entries = fresh.iter().enumerate().filter_map(|(offset, key)| { - u32::try_from(first_position + offset) - .ok() - .map(|index| (*key, index)) - }); - - if fresh.len() >= TAIL_MERGE_THRESHOLD { - // A bulk load (the first ingest of a whole registry): sorting it - // directly beats funnelling millions of entries through the map. - let mut batch: Vec<(u64, u32)> = entries.collect(); - batch.extend(inner.tail.drain()); + let end = start + keys.len(); + if end <= covered { + return true; + } + if covered > start { + // A racing caller ingested part of this range first. + batch.retain(|(_, index)| *index as usize >= covered); + } + if bulk { inner.merge(batch); } else { - for (key, index) in entries { - inner.tail.entry(key).or_insert(index); - } - if inner.tail.len() >= TAIL_MERGE_THRESHOLD { - let batch: Vec<(u64, u32)> = inner.tail.drain().collect(); - inner.merge(batch); - } + inner.tail.extend(batch); + inner.merge_tail_if_full(); } - inner.covered_len += fresh.len(); + inner.covered_len = end; true } - /// Heap bytes held by the table: capacity of the base arrays plus the - /// tail's slots. - pub fn heap_bytes(&self) -> usize { - let inner = self.inner.read().unwrap(); - inner.keys.capacity() * size_of::() - + inner.indices.capacity() * size_of::() - + inner.tail.capacity() * (size_of::() + size_of::()) + /// Records that `pubkey` sits at `index`, as a scan of some registry + /// found: a candidate like any other, verified by whoever asks next. + /// + /// For the lookups the table could not answer from what it ingested: a + /// second key under a shared prefix, or a position the table took in from + /// another fork's registry. Without this such a pubkey would cost a scan on + /// every lookup for as long as the process runs, since ingestion only ever + /// appends positions past `covered_len`. Recording an entry already held is + /// a no-op; an index past `u32` is not stored, as in [`Self::extend`]. + pub fn insert(&self, pubkey: &BlsPubkey, index: u64) { + let Ok(index) = u32::try_from(index) else { + return; + }; + let key = Self::key_of(pubkey); + let mut inner = self.inner.write().unwrap(); + let in_base = inner + .base_range(key) + .any(|position| inner.indices[position] == index); + if !in_base && inner.tail.insert((key, index)) { + inner.merge_tail_if_full(); + } } } impl Inner { - /// Merges `batch` into the sorted base, ordering ties by index so the - /// earlier registry position is returned first. - fn merge(&mut self, mut batch: Vec<(u64, u32)>) { - batch.sort_unstable(); + /// Positions in the base whose key is `key`. + fn base_range(&self, key: u64) -> std::ops::Range { + let start = self.keys.partition_point(|candidate| *candidate < key); + let end = start + self.keys[start..].partition_point(|candidate| *candidate == key); + start..end + } + + /// Merges the tail into the base once it holds [`TAIL_MERGE_THRESHOLD`] + /// entries. + fn merge_tail_if_full(&mut self) { + if self.tail.len() >= TAIL_MERGE_THRESHOLD { + let batch: Vec<(u64, u32)> = std::mem::take(&mut self.tail).into_iter().collect(); + self.merge(batch); + } + } + + /// Merges `batch`, sorted ascending, into the sorted base in one linear + /// pass, ordering ties by index so the earlier registry position is + /// returned first. + fn merge(&mut self, batch: Vec<(u64, u32)>) { + debug_assert!(batch.is_sorted()); let total = self.keys.len() + batch.len(); let mut keys = Vec::with_capacity(total); let mut indices = Vec::with_capacity(total); @@ -214,6 +260,12 @@ mod tests { .collect() } + /// Heap bytes held by the sorted base: the capacity of its two arrays. + fn base_bytes(cache: &PubkeyIndexCache) -> usize { + let inner = cache.inner.read().unwrap(); + inner.keys.capacity() * size_of::() + inner.indices.capacity() * size_of::() + } + #[test] fn a_small_extend_is_served_from_the_tail() { let cache = PubkeyIndexCache::default(); @@ -263,6 +315,18 @@ mod tests { assert_eq!(cache.covered_len(), 8); } + #[test] + fn a_racing_bulk_extend_skips_what_was_ingested_first() { + let cache = PubkeyIndexCache::default(); + let bulk = TAIL_MERGE_THRESHOLD as u64 * 2; + assert!(cache.extend(0, &keys(0..10))); + // A caller that read `covered_len == 0` and scanned the whole registry. + assert!(cache.extend(0, &keys(0..bulk))); + assert_eq!(cache.covered_len(), bulk as usize); + assert_eq!(cache.candidates(&pubkey(4)), vec![4]); + assert_eq!(cache.candidates(&pubkey(bulk - 1)), vec![bulk - 1]); + } + #[test] fn equal_keys_return_every_candidate_in_index_order() { let cache = PubkeyIndexCache::default(); @@ -273,12 +337,45 @@ mod tests { assert_eq!(cache.candidates(&pubkey(102)), vec![2, 7]); } + #[test] + fn equal_keys_in_the_tail_are_all_kept() { + let cache = PubkeyIndexCache::default(); + let mut few = keys(100..110); + few[7] = few[2]; + assert!(cache.extend(0, &few)); + assert_eq!(cache.candidates(&pubkey(102)), vec![2, 7]); + } + + #[test] + fn an_inserted_index_becomes_a_candidate_without_covering_a_position() { + let cache = PubkeyIndexCache::default(); + assert!(cache.extend(0, &keys(0..5))); + // A scan elsewhere found pubkey 3 at index 9 (another fork's layout). + cache.insert(&pubkey(3), 9); + assert_eq!(cache.covered_len(), 5); + assert_eq!(cache.candidates(&pubkey(3)), vec![3, 9]); + // Recording it again, or recording what the base holds, adds nothing. + cache.insert(&pubkey(3), 9); + cache.insert(&pubkey(3), 3); + assert_eq!(cache.candidates(&pubkey(3)), vec![3, 9]); + } + + #[test] + fn an_insert_matching_the_base_is_not_duplicated() { + let cache = PubkeyIndexCache::default(); + let bulk = TAIL_MERGE_THRESHOLD as u64; + assert!(cache.extend(0, &keys(0..bulk))); + cache.insert(&pubkey(5), 5); + assert!(cache.inner.read().unwrap().tail.is_empty()); + assert_eq!(cache.candidates(&pubkey(5)), vec![5]); + } + #[test] fn the_base_costs_twelve_bytes_an_entry() { let cache = PubkeyIndexCache::default(); let count = TAIL_MERGE_THRESHOLD as u64 * 4; assert!(cache.extend(0, &keys(0..count))); - assert_eq!(cache.heap_bytes(), count as usize * 12); + assert_eq!(base_bytes(&cache), count as usize * 12); } /// Sizing check for a mainnet-scale registry; prints, asserts nothing @@ -310,9 +407,9 @@ mod tests { }) .count(); println!( - "ingest {COUNT} keys: {ingest:?}; {hits} lookups: {:?}; heap {} bytes", + "ingest {COUNT} keys: {ingest:?}; {hits} lookups: {:?}; base {} bytes", started.elapsed(), - cache.heap_bytes() + base_bytes(&cache) ); } } diff --git a/docs/beacon_stf.md b/docs/beacon_stf.md index b2df5a0f2..16ae4b2ba 100644 --- a/docs/beacon_stf.md +++ b/docs/beacon_stf.md @@ -327,10 +327,27 @@ activation or exit lands at least `MAX_SEED_LOOKAHEAD` epochs ahead, so no block inside the epoch can move the total, and two states agreeing on that root agree on it. The cache lives outside the state, so it adds no field to `BeaconState` and no SSZ concern; callers with no store (spec runners, block production, -tests) hold a fresh `ActiveBalanceCache::default()`. Epoch processing, churn -helpers and fork choice keep calling the uncached one-pass -`get_total_active_balance`. In debug builds every hit is cross-checked against a -fresh computation. +tests) hold a fresh `ActiveBalanceCache::default()`. Block processing reads it +for base rewards (attestations, the sync aggregate) and for the churn limits +of everything a block can make exit or consolidate: voluntary exits, proposer +and attester slashings, withdrawal and consolidation requests (gloas's +`apply_parent_execution_payload` included). The churn helpers take it as an +`Option`, and epoch processing (its ejections included) passes `None` and keeps +calling the uncached one-pass `get_total_active_balance`, because the key is +unsound once that epoch's effective-balance update has run. In debug builds +every hit is cross-checked against a fresh computation. + +A block-processing entry point (`state_transition`, `apply_block`, every +fork's `process_block`) takes its tables as one `stf::BlockCaches`: the +committee cache, the active-balance cache, and the pubkey-index cache, which +answers `validator_index(pubkey)` (the sync committee's seats, withdrawal and +consolidation requests) without walking the registry. Fork choice passes the +`Store`'s shared tables; block production passes a fresh committee and +active-balance table but the store's pubkey table, since a fresh one would +ingest the whole registry; a caller with no store holds an +`stf::OwnedBlockCaches`. Every pubkey-table answer is verified against the +asking state, and an index found by a fallback scan is recorded, so a key the +table cannot confirm costs one scan rather than one per block. Epoch steps 1-3 (justification, inactivity updates, rewards) follow that rule through `helpers::participation`. One walk of `iter_validators()`, zipped with