From 5c787c6cc7490be0f43a92e30f1a9e469c64bd3a 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, 30 Oct 2026 10:54:04 -0300 Subject: [PATCH 1/5] perf(beacon): committee key sums, warmed off the import path An attestation aggregate usually covers most of its committee, so its verification key is cheaper to derive as the committee's key sum minus the non-attesters than as the sum of the attesters. The result is the same group element, so the pairing check is unchanged; only the number of point additions drops. Each resident shuffling in the committee cache gets a second slot for its committee key sums, so eviction and head pinning stay in one place. Block import only looks the sums up: building them costs a pass over the whole active set, which is more than they save for committees attested once. The epoch precompute worker builds them instead, after it has handed the advanced state to the actor, so the epoch's first block never waits. Sums are keyed by the shuffling key. That is sound because the registry is append-only, a pubkey is never reassigned, and a validator active at epoch E was activated at least MAX_SEED_LOOKAHEAD + 1 epochs earlier, so it is already in the history the decision root commits to. Nothing in block processing calls the new key builder yet. --- crates/blockchain/src/epoch_precompute.rs | 9 + .../src/beacon/helpers/accessors.rs | 113 ++++++- .../src/beacon/helpers/electra.rs | 262 +++++++++++++++- .../state_transition/src/metrics.rs | 15 + crates/common/crypto/src/bls.rs | 3 + .../common/crypto/src/bls/committee_sums.rs | 291 ++++++++++++++++++ crates/storage/src/committee_cache.rs | 146 ++++++++- docs/metrics.md | 1 + 8 files changed, 823 insertions(+), 17 deletions(-) create mode 100644 crates/common/crypto/src/bls/committee_sums.rs diff --git a/crates/blockchain/src/epoch_precompute.rs b/crates/blockchain/src/epoch_precompute.rs index 9c77e137a..78ac11e2e 100644 --- a/crates/blockchain/src/epoch_precompute.rs +++ b/crates/blockchain/src/epoch_precompute.rs @@ -25,6 +25,7 @@ use std::sync::Arc; use std::time::Duration; use ethlambda_state_transition::beacon::fork_choice; +use ethlambda_state_transition::beacon::helpers::accessors::CommitteeCacheExt as _; use ethlambda_state_transition::beacon::helpers::misc::compute_start_slot_at_epoch; use ethlambda_state_transition::metrics as stf_metrics; use ethlambda_storage::{CacheKey, Chain}; @@ -220,13 +221,21 @@ impl BlockChainServer { let config = self.store.config(); let actor = ctx.actor_ref(); + let committee_cache = self.store.committee_cache(); tokio::task::spawn_blocking(move || { let result = { let _timing = stf_metrics::time_epoch_precompute(); fork_choice::advance_to_epoch_start(&head_state, key.epoch, &config) }; let result = result.map(Arc::new).map_err(|err| err.to_string()); + let advanced = result.as_ref().ok().map(Arc::clone); + // The state goes to the actor first: the epoch's first block must + // not wait for key sums, which only make its attestations cheaper. let _ = actor.send(EpochPrecomputed { key, result }); + if let Some(state) = advanced { + let _timing = stf_metrics::time_key_sums_warm(); + committee_cache.warm_key_sums(&state, key.epoch); + } }); } } diff --git a/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs b/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs index 60cdcc7a1..cfb5413ab 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs @@ -7,6 +7,8 @@ use std::sync::Arc; +use rayon::prelude::*; + // Re-exported at this old path (`crate::beacon::helpers::accessors::CommitteeCache`, // and so on), so every existing import of the type this module used to define // is unaffected by its move to `ethlambda-storage`; see [`CommitteeCacheExt`] @@ -15,6 +17,7 @@ pub use ethlambda_storage::{ ActiveBalanceCache, ActiveBalanceKey, CommitteeCache, Lookup, ShufflingKey, }; +use crate::beacon::bls::{AggregatePubkey, CommitteeKeySums}; use crate::beacon::config::Config; use crate::beacon::constants; use crate::beacon::containers::BeaconState; @@ -24,7 +27,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::{ @@ -278,6 +281,59 @@ pub trait CommitteeCacheExt { /// the previous head pinned stays resident until an insertion evicts it /// on epoch like any other entry. fn update_head(&self, head_root: Root, head_state: &BeaconState); + + /// `epoch`'s committee key sums as `state` names them, if they have been + /// warmed. + /// + /// Lookup only, and it never builds: building costs a pass over every + /// active validator's key, which is more than the sums save for + /// committees attested once, so block import must never be the one to pay + /// it. `None` (cold, evicted, or an epoch `state` cannot key) means the + /// caller sums its attesters directly. Warmed by [`Self::warm_key_sums`]. + /// + /// # Why the sums can be shared on the shuffling's key + /// + /// The sums are filed under the same [`ShufflingKey`] as the committees, + /// so two states with one key must agree on every member's *public key* as + /// well as on who the members are. They do, for three reasons, each + /// checked against this crate's spec code: + /// + /// - A validator's index and public key are fixed when it is appended to + /// the registry and never change after: the registry only grows + /// (`process_deposit`, and electra's pending-deposit path through + /// `add_validator_to_registry`, push; nothing removes or reorders), and + /// no code assigns `validator.pubkey` after construction. + /// - A new validator is appended with `activation_epoch` at + /// `FAR_FUTURE_EPOCH`. The only writer that gives it a real one is + /// `process_registry_updates`, running in epoch `N`'s epoch processing, + /// and it writes `compute_activation_exit_epoch(N)`, which is + /// `N + 1 + MAX_SEED_LOOKAHEAD`. (Genesis validators are active from + /// `GENESIS_EPOCH` and are in every state's history.) + /// - A validator in `epoch`'s committees is active there, so its + /// `activation_epoch <= epoch`, so it was activated by epoch processing + /// at some `N <= epoch - 1 - MAX_SEED_LOOKAHEAD`, and appended no later + /// than that. That is before the end of `epoch - 2`, so the block the + /// [`ShufflingKey`] names as the decision root already has it in its + /// registry, at that index, with that key. Every state descending from + /// that block inherits it. + /// + /// So the member list and each member's key are functions of the key, + /// and so is their sum. This is a stronger claim than the committees' + /// own (which need only the activation and exit epochs to be fixed), but + /// it follows from the same lookahead and from the registry being + /// append-only. + fn key_sums(&self, state: &BeaconState, epoch: Epoch) -> Option>; + + /// Builds `epoch`'s committee key sums as `state` names them, unless they + /// are already there. Meant for a background task: it derives the + /// committees if they are not resident, then spends one pass over the + /// registry and one key sum per committee, parallel across committees. + /// + /// A committee with a member whose key fails validation gets no sum + /// (`None` in its slot), so lookups of it fall back to the direct path + /// rather than to a sum that omits the member. Does nothing for an epoch + /// `state` cannot key, since there is nowhere to file the result. + fn warm_key_sums(&self, state: &BeaconState, epoch: Epoch); } impl CommitteeCacheExt for CommitteeCache { @@ -303,6 +359,61 @@ impl CommitteeCacheExt for CommitteeCache { let keys = epochs.map(|epoch| shuffling_key(head_state, epoch)); self.pin_head(head_root, keys); } + + fn key_sums(&self, state: &BeaconState, epoch: Epoch) -> Option> { + self.resident_key_sums(shuffling_key(state, epoch)?) + } + + fn warm_key_sums(&self, state: &BeaconState, epoch: Epoch) { + let Some(key) = shuffling_key(state, epoch) else { + return; + }; + if self.resident_key_sums(key).is_some() { + return; + } + // Resident before the sums are filed into it; a no-op when the import + // path already built them. + let committees = self.committees(state, epoch); + self.get_or_init_key_sums(key, || build_committee_key_sums(state, &committees, epoch)); + } +} + +/// Every committee of `committees`' epoch summed, indexed by committee number. +/// +/// One `state.iter_validators()` pass copies each validator's key bytes into a +/// flat list (a per-index `state.validator(i)` is a tree descent, and the +/// whole active set is read), then committees are summed in parallel. An entry +/// is `None` for a committee with a member whose key fails validation or whose +/// index is outside the registry, and for an empty committee. +fn build_committee_key_sums( + state: &BeaconState, + committees: &EpochCommittees, + epoch: Epoch, +) -> CommitteeKeySums { + let pubkeys: Vec = state.iter_validators().map(|v| v.pubkey).collect(); + let per_slot = committees.committees_per_slot(); + let first_slot = compute_start_slot_at_epoch(epoch); + + let members: Vec<&[ValidatorIndex]> = (0..preset::SLOTS_PER_EPOCH) + .flat_map(|offset| (0..per_slot).map(move |index| (first_slot + offset, index))) + .map(|(slot, index)| { + committees + .committee(slot, index) + .expect("slot is in the epoch and index is below the committee count") + }) + .collect(); + + let sums = members + .par_iter() + .map(|committee| { + let keys: Option> = committee + .iter() + .map(|index| pubkeys.get(*index as usize).copied()) + .collect(); + AggregatePubkey::sum(&keys?) + }) + .collect(); + CommitteeKeySums::new(sums) } /// The key `state`'s total active balance is cached under, or `None` if diff --git a/crates/blockchain/state_transition/src/beacon/helpers/electra.rs b/crates/blockchain/state_transition/src/beacon/helpers/electra.rs index 94fa7d20b..668e21365 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/electra.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/electra.rs @@ -123,7 +123,7 @@ //! single easiest mistake to make copying that shape without also copying //! the reasoning behind it. -use crate::beacon::bls; +use crate::beacon::bls::{self, AggregatePubkey}; use crate::beacon::config::Config; use crate::beacon::constants::{self, FAR_FUTURE_EPOCH}; use crate::beacon::containers::shared::Validator; @@ -139,8 +139,8 @@ use crate::beacon::primitives::{ }; use super::accessors::{ - CommitteeCache, CommitteeCacheExt, get_active_validator_indices, get_current_epoch, get_domain, - get_seed, get_total_active_balance, + CommitteeCache, CommitteeCacheExt, committee_number, get_active_validator_indices, + get_current_epoch, get_domain, get_seed, get_total_active_balance, }; use super::math::bytes_to_uint64; use super::misc::{compute_activation_exit_epoch, compute_epoch_at_slot, compute_signing_root}; @@ -513,6 +513,93 @@ pub(crate) fn attesting_indices_from_committee_bits( Ok(indices) } +/// The aggregate key of the attesters an electra-shaped attestation names: +/// the sum of their public keys, which is what `bls.FastAggregateVerify` +/// checks the signature against. `Ok(None)` when a key fails validation, a +/// named attester is outside the registry, or no attester is set. +/// +/// Walks the committees the way [`attesting_indices_from_committee_bits`] +/// does, `is_set` being the same closure over the aggregation bits (gloas +/// passes its progressive bitlist through it, so one builder serves both). +/// Per committee, when the shuffling's key sums are warm +/// ([`CommitteeCacheExt::key_sums`], lookup only: import never builds them) +/// and more than half of the committee attested, the committee's part is its +/// sum minus the keys of the non-attesters; otherwise it is the sum of the +/// attesters. A committee whose sum is `None` (a member's key fails +/// validation) takes the second branch, so only the attesters' own keys decide +/// it, exactly as with cold sums. The parts are then added. +/// Subtracting is the same group element as summing the attesters, so the +/// pairing check cannot tell the two apart; only the point additions differ, +/// absentees' instead of attesters'. +/// +/// Phase0 through deneb attestations stay on the direct path: each names one +/// committee, and the live chain is past electra. +#[allow( + dead_code, + reason = "wired into process_attestation by the next change" +)] +pub(crate) fn attesters_aggregate_pubkey( + state: &BeaconState, + slot: Slot, + committee_bits: &electra::CommitteeBits, + is_set: impl Fn(usize) -> bool, + committees: &CommitteeCache, +) -> Result> { + let epoch = compute_epoch_at_slot(slot); + let epoch_committees = committees.committees(state, epoch); + let key_sums = committees.key_sums(state, epoch); + let committees_per_slot = epoch_committees.committees_per_slot(); + + let pubkey_of = |index: ValidatorIndex| state.validator(index).ok().map(|v| v.pubkey); + + let mut total: Option = None; + let mut committee_offset = 0usize; + for committee_index in get_committee_indices(committee_bits) { + let committee = epoch_committees.committee(slot, committee_index)?; + let first_bit = committee_offset; + committee_offset += committee.len(); + + let attested: Vec = (0..committee.len()) + .map(|position| is_set(first_bit + position)) + .collect(); + let attester_count = attested.iter().filter(|set| **set).count(); + if attester_count == 0 { + continue; + } + + let number = committee_number(slot, committees_per_slot, committee_index)?; + let sum = key_sums.as_ref().and_then(|sums| sums.get(number as usize)); + let part = match sum { + Some(sum) if attester_count * 2 > committee.len() => { + let absent: Option> = committee + .iter() + .zip(&attested) + .filter(|(_, set)| !**set) + .map(|(index, _)| pubkey_of(*index)) + .collect(); + absent.and_then(|absent| sum.minus(&absent)) + } + _ => { + let present: Option> = committee + .iter() + .zip(&attested) + .filter(|(_, set)| **set) + .map(|(index, _)| pubkey_of(*index)) + .collect(); + present.and_then(|present| AggregatePubkey::sum(&present)) + } + }; + let Some(part) = part else { + return Ok(None); + }; + total = Some(match total { + Some(total) => total.plus(&part), + None => part, + }); + } + Ok(total) +} + /// The committee members whose bit is set in `attestation`, in ascending /// order. See [`attesting_indices_from_committee_bits`] for the walk itself; /// this is a thin wrapper over it, closing over `attestation`'s own @@ -1990,4 +2077,173 @@ mod tests { ); assert_ne!(electra_limit, gloas_limit); } + + // -- attesters_aggregate_pubkey ------------------------------------------ + + use crate::beacon::helpers::accessors::get_current_epoch; + use crate::beacon::helpers::misc::compute_start_slot_at_epoch; + use crate::beacon::helpers::test_state::{sign_for, with_signing_validators_at}; + use crate::beacon::primitives::Root; + + /// Enough validators for two committees per slot, so one attestation can + /// name two committees; mainnet's preset is the only one these run under. + fn two_committees_per_slot_state() -> BeaconState { + let count = (preset::SLOTS_PER_EPOCH * preset::TARGET_COMMITTEE_SIZE * 2) as usize; + with_signing_validators_at(ForkName::Electra, count) + } + + /// A reproducible xorshift stream, so a failing mask can be replayed. + fn next(seed: &mut u64) -> u64 { + *seed ^= *seed << 13; + *seed ^= *seed >> 7; + *seed ^= *seed << 17; + *seed + } + + fn bits_naming(indices: &[usize]) -> electra::CommitteeBits { + let mut bits = electra::CommitteeBits::default(); + for index in indices { + bits.set(*index, true).unwrap(); + } + bits + } + + /// Whether `key` is exactly the sum of the validators `attesters` names, by + /// the only check available from outside the crypto crate: it verifies the + /// aggregate of those validators' signatures over one message, which a + /// different group element would not. + fn is_sum_of(key: &AggregatePubkey, attesters: &[ValidatorIndex], message: Root) -> bool { + let signatures: Vec<_> = attesters + .iter() + .map(|index| sign_for(*index as usize, message)) + .collect(); + let aggregate = bls::aggregate(&signatures).unwrap(); + key.verify(message, &aggregate) + } + + /// For random committee sizes and masks of every shape the majority rule + /// distinguishes, the builder's key is the attesters' sum, with the sums + /// cold and with them warm. + #[test] + fn the_builder_key_is_the_attesters_sum_with_cold_and_warm_sums() { + let state = two_committees_per_slot_state(); + let epoch = get_current_epoch(&state); + let message = Root::repeat_byte(0x42); + let mut seed = 0x9e37_79b9_7f4a_7c15u64; + + let cold = CommitteeCache::default(); + let warm = CommitteeCache::default(); + warm.warm_key_sums(&state, epoch); + assert!(warm.key_sums(&state, epoch).is_some()); + assert!(cold.key_sums(&state, epoch).is_none()); + + for slot_offset in [0u64, 7, 31] { + let slot = compute_start_slot_at_epoch(epoch) + slot_offset; + let committee_sets: [&[usize]; 3] = [&[0], &[1], &[0, 1]]; + for named in committee_sets { + let bits = bits_naming(named); + let epoch_committees = warm.committees(&state, epoch); + let members: Vec = named + .iter() + .flat_map(|c| { + epoch_committees + .committee(slot, *c as u64) + .unwrap() + .to_vec() + }) + .collect(); + let len = members.len(); + + let mut masks: Vec> = vec![ + vec![false; len], + vec![true; len], + (0..len).map(|p| p == len / 3).collect(), + (0..len).map(|p| p < len / 2).collect(), + (0..len).map(|p| p <= len / 2).collect(), + ]; + for _ in 0..4 { + masks.push( + (0..len) + .map(|_| !next(&mut seed).is_multiple_of(5)) + .collect(), + ); + masks.push( + (0..len) + .map(|_| next(&mut seed).is_multiple_of(2)) + .collect(), + ); + } + + for mask in masks { + let attesters: Vec = members + .iter() + .zip(&mask) + .filter(|(_, set)| **set) + .map(|(index, _)| *index) + .collect(); + for cache in [&cold, &warm] { + let key = attesters_aggregate_pubkey( + &state, + slot, + &bits, + |bit| mask.get(bit).copied().unwrap_or(false), + cache, + ) + .unwrap(); + match key { + None => assert!(attesters.is_empty(), "slot {slot} {named:?}"), + Some(key) => assert!( + is_sum_of(&key, &attesters, message), + "slot {slot} committees {named:?} with {} of {len} set", + attesters.len() + ), + } + } + } + } + } + } + + /// A committee with a member whose key fails validation gets no sum, its + /// neighbours keep theirs, and the builder falls back to the attesters: + /// valid when the bad member did not attest, refused when it did. + #[test] + fn a_committee_with_an_invalid_member_key_falls_back() { + let mut state = two_committees_per_slot_state(); + let epoch = get_current_epoch(&state); + let slot = compute_start_slot_at_epoch(epoch) + 3; + let message = Root::repeat_byte(0x17); + + let probe = CommitteeCache::default(); + let members = probe + .committees(&state, epoch) + .committee(slot, 0) + .unwrap() + .to_vec(); + let bad = members[5]; + state.validator_mut(bad).unwrap().pubkey = Default::default(); + + let committees = CommitteeCache::default(); + committees.warm_key_sums(&state, epoch); + let sums = committees.key_sums(&state, epoch).expect("warmed"); + let per_slot = committees.committees(&state, epoch).committees_per_slot(); + let number = |index| committee_number(slot, per_slot, index).unwrap() as usize; + assert!( + sums.get(number(0)).is_none(), + "the bad committee has no sum" + ); + assert!(sums.get(number(1)).is_some(), "its neighbour keeps one"); + + let bits = bits_naming(&[0]); + let without_bad: Vec = members.iter().map(|m| *m != bad).collect(); + let key = attesters_aggregate_pubkey(&state, slot, &bits, |b| without_bad[b], &committees) + .unwrap() + .expect("the bad member did not attest"); + let attesters: Vec = + members.iter().copied().filter(|m| *m != bad).collect(); + assert!(is_sum_of(&key, &attesters, message)); + + let everyone = attesters_aggregate_pubkey(&state, slot, &bits, |_| true, &committees); + assert!(everyone.unwrap().is_none(), "the bad member attested"); + } } diff --git a/crates/blockchain/state_transition/src/metrics.rs b/crates/blockchain/state_transition/src/metrics.rs index 63c029166..1cd9eb560 100644 --- a/crates/blockchain/state_transition/src/metrics.rs +++ b/crates/blockchain/state_transition/src/metrics.rs @@ -145,6 +145,21 @@ pub fn time_epoch_precompute() -> TimingGuard { TimingGuard::new(&LEAN_BEACON_EPOCH_PRECOMPUTE_SECONDS) } +static LEAN_BEACON_COMMITTEE_KEY_SUMS_WARM_SECONDS: LazyLock = LazyLock::new(|| { + register_histogram!( + "lean_beacon_committee_key_sums_warm_seconds", + "Duration of warming one epoch's committee key sums, off the import path", + vec![0.05, 0.1, 0.25, 0.5, 1.0, 2.0, 4.0, 8.0] + ) + .unwrap() +}); + +/// Start timing one committee key-sum warm-up. Records duration when the +/// guard is dropped. +pub fn time_key_sums_warm() -> TimingGuard { + TimingGuard::new(&LEAN_BEACON_COMMITTEE_KEY_SUMS_WARM_SECONDS) +} + static LEAN_BEACON_EPOCH_PRECOMPUTE_STARTED_TOTAL: LazyLock = LazyLock::new(|| { register_int_counter_vec!( "lean_beacon_epoch_precompute_started_total", diff --git a/crates/common/crypto/src/bls.rs b/crates/common/crypto/src/bls.rs index fb9dd2005..1267c1e91 100644 --- a/crates/common/crypto/src/bls.rs +++ b/crates/common/crypto/src/bls.rs @@ -439,6 +439,9 @@ pub fn key_validate(pubkey: &BlsPubkey) -> bool { validated_pubkey(pubkey).is_some() } +mod committee_sums; +pub use committee_sums::{AggregatePubkey, CommitteeKeySums}; + #[cfg(test)] mod tests { use super::*; diff --git a/crates/common/crypto/src/bls/committee_sums.rs b/crates/common/crypto/src/bls/committee_sums.rs new file mode 100644 index 000000000..f7d3e6b9c --- /dev/null +++ b/crates/common/crypto/src/bls/committee_sums.rs @@ -0,0 +1,291 @@ +//! Committee key sums: a committee's aggregate public key, kept as a curve +//! point so an attestation's key can be derived from it instead of re-summed. +//! +//! An attestation aggregate usually covers most of its committee, and its +//! verification key is the sum of its attesters' keys. Summing every attester +//! costs one point addition per signer; summing the *non*-attesters and +//! subtracting from the committee's own sum costs one per absentee, which on a +//! healthy network is a small fraction. The two are the same group element, so +//! the pairing check is unchanged and only the number of additions moves. +//! +//! This module holds the crypto half: [`AggregatePubkey`], a sum that can be +//! added to, subtracted from and verified against without ever being +//! serialized, and [`CommitteeKeySums`], one shuffling's committee sums +//! indexed by committee number. Which committees there are, who is in them and +//! when a sum is still valid belongs to `ethlambda-storage`'s `CommitteeCache` +//! and `ethlambda-state-transition`'s accessors; this module knows only keys. +//! +//! Every key reaches a sum through `validated_pubkeys`, so the guarantees of +//! the module documentation in [`super`] hold here too: a key that fails +//! `key_validate` is never summed, and a failure is reported as `None` rather +//! than as a sum that silently omits the key. + +use blst::BLST_ERROR; +use blst::MultiPoint; +use blst::min_pk::{AggregatePublicKey, PublicKey, Signature}; + +use ethlambda_types::beacon::primitives::{BlsPubkey, BlsSignature, Root}; + +use super::{DST, validated_pubkeys}; + +/// The sum of `points`, or `None` for an empty slice. +/// +/// `blst`'s batched addition indexes its first element, so an empty input has +/// to be refused here rather than reaching it. Every point passed already +/// cleared `key_validate`, so nothing is re-checked. Equal points are fine: the +/// addition doubles where it must, which matters because committees drawn with +/// replacement (sync and payload timeliness committees) can hold one key twice. +fn sum_points(points: &[PublicKey]) -> Option { + if points.is_empty() { + return None; + } + Some(points.add()) +} + +/// A sum of validated public keys, kept as a curve point. Never serialized. +/// +/// Not a [`BlsPubkey`] on purpose: compressing a point costs a field inversion, +/// and the only consumer is a pairing check in this process, so the point stays +/// as it is. The sum may be the identity (every key subtracted away), which is +/// representable here and refused by [`Self::verify`], exactly as `blst` +/// refuses an infinite key at pairing time. +#[derive(Clone, Copy)] +pub struct AggregatePubkey(AggregatePublicKey); + +impl AggregatePubkey { + /// The sum of `pubkeys`, each validated through the pubkey cache. `None` if + /// `pubkeys` is empty or any key fails validation. + pub fn sum(pubkeys: &[BlsPubkey]) -> Option { + let points = validated_pubkeys(pubkeys)?; + sum_points(&points).map(Self) + } + + /// `self - sum(pubkeys)`, with each key validated through the pubkey + /// cache. `None` if any key fails validation; an empty `pubkeys` is `self`. + /// + /// Negation goes through `blst`'s own `sub_aggregate`, so no `unsafe` is + /// needed here. Subtracting every key `self` was built from yields the + /// identity, not an error: see [`Self::verify`]. + pub fn minus(&self, pubkeys: &[BlsPubkey]) -> Option { + if pubkeys.is_empty() { + return Some(*self); + } + let points = validated_pubkeys(pubkeys)?; + let removed = sum_points(&points)?; + let mut result = self.0; + result.sub_aggregate(&removed); + Some(Self(result)) + } + + /// `self + other`. + pub fn plus(&self, other: &Self) -> Self { + let mut result = self.0; + result.add_aggregate(&other.0); + Self(result) + } + + /// `bls.FastAggregateVerify` with this already-summed key: false for an + /// invalid signature, and false for the identity key, as the path that sums + /// the keys itself is. + /// + /// The signature is validated here for the same reason + /// [`super::fast_aggregate_verify`] validates it: nothing upstream + /// guarantees a subgroup-correct point. The key needs no check, since a sum + /// of validated points is in the subgroup; the identity is the one value + /// that is not a valid key, and `blst` rejects it at pairing time. + pub fn verify(&self, message: Root, signature: &BlsSignature) -> bool { + let Ok(signature) = Signature::sig_validate(signature.as_ref(), false) else { + return false; + }; + let key = self.0.to_public_key(); + signature.fast_aggregate_verify_pre_aggregated(false, message.as_slice(), DST, &key) + == BLST_ERROR::BLST_SUCCESS + } + + /// The sum compressed, for comparing two sums in tests. + #[cfg(test)] + fn to_bytes(self) -> [u8; 48] { + self.0.to_public_key().to_bytes() + } +} + +impl std::fmt::Debug for AggregatePubkey { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("AggregatePubkey(..)") + } +} + +/// One shuffling's committee key sums, by committee number within the epoch +/// (`slot_offset * committees_per_slot + committee_index`, the position +/// `committee_number` computes). An entry is `None` when some member's key +/// fails validation, so that committee falls back to summing its attesters. +pub struct CommitteeKeySums { + sums: Vec>, +} + +impl CommitteeKeySums { + /// Wraps `sums`, one entry per committee of the epoch in committee-number order. + pub fn new(sums: Vec>) -> Self { + Self { sums } + } + + /// The sum of committee `committee_number`'s members' keys, or `None` when + /// the number is out of range or that committee has no usable sum. + pub fn get(&self, committee_number: usize) -> Option<&AggregatePubkey> { + self.sums.get(committee_number)?.as_ref() + } +} + +impl std::fmt::Debug for CommitteeKeySums { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("CommitteeKeySums") + .field("committees", &self.sums.len()) + .finish() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use blst::min_pk::SecretKey; + + fn secret(seed: u64) -> SecretKey { + let mut ikm = [0x3cu8; 32]; + ikm[..8].copy_from_slice(&seed.to_le_bytes()); + SecretKey::key_gen(&ikm, &[]).expect("32 bytes of input material") + } + + fn pubkeys(count: usize) -> Vec { + (0..count) + .map(|i| BlsPubkey(secret(i as u64 + 1000).sk_to_pk().to_bytes())) + .collect() + } + + /// The sum straight through `blst`, no cache and no helper of this module. + fn direct(keys: &[BlsPubkey]) -> [u8; 48] { + let points: Vec = keys + .iter() + .map(|k| PublicKey::key_validate(k.as_ref()).unwrap()) + .collect(); + let refs: Vec<&PublicKey> = points.iter().collect(); + AggregatePublicKey::aggregate(&refs, false) + .unwrap() + .to_public_key() + .to_bytes() + } + + #[test] + fn sum_matches_the_direct_sum_including_duplicates() { + let keys = pubkeys(8); + let mut with_duplicates = keys.clone(); + with_duplicates.push(keys[0]); + with_duplicates.push(keys[0]); + for set in [&keys[..1], &keys[..2], &keys[..], &with_duplicates[..]] { + assert_eq!(AggregatePubkey::sum(set).unwrap().to_bytes(), direct(set)); + } + let twice = [keys[0], keys[0]]; + assert_eq!( + AggregatePubkey::sum(&twice).unwrap().to_bytes(), + direct(&twice) + ); + } + + #[test] + fn sum_of_nothing_or_of_an_invalid_key_is_none() { + assert!(AggregatePubkey::sum(&[]).is_none()); + let mut keys = pubkeys(3); + keys.push(BlsPubkey::default()); + assert!(AggregatePubkey::sum(&keys).is_none()); + } + + /// `committee - absentees` is the attesters' sum for every mask shape. + #[test] + fn minus_and_plus_agree_with_the_direct_sum() { + let keys = pubkeys(16); + let total = AggregatePubkey::sum(&keys).unwrap(); + for absent in [0usize, 1, 8, 9, 15] { + let attesters = &keys[absent..]; + let absentees = &keys[..absent]; + let derived = total.minus(absentees).unwrap(); + assert_eq!(derived.to_bytes(), direct(attesters), "absent {absent}"); + } + let left = AggregatePubkey::sum(&keys[..5]).unwrap(); + let right = AggregatePubkey::sum(&keys[5..]).unwrap(); + assert_eq!(left.plus(&right).to_bytes(), total.to_bytes()); + assert_eq!(total.minus(&[]).unwrap().to_bytes(), total.to_bytes()); + } + + #[test] + fn minus_with_an_invalid_key_is_none() { + let keys = pubkeys(4); + let total = AggregatePubkey::sum(&keys).unwrap(); + assert!(total.minus(&[BlsPubkey::default()]).is_none()); + } + + fn signed(count: usize, seed: u64, message: Root) -> (Vec, Vec) { + let secrets: Vec = (0..count as u64).map(|i| secret(i + seed)).collect(); + let keys = secrets + .iter() + .map(|s| BlsPubkey(s.sk_to_pk().to_bytes())) + .collect(); + let sigs = secrets + .iter() + .map(|s| BlsSignature(s.sign(message.as_slice(), DST, &[]).to_bytes())) + .collect(); + (keys, sigs) + } + + #[test] + fn verify_agrees_with_fast_aggregate_verify_on_real_signatures() { + let message = Root::repeat_byte(9); + let (keys, sigs) = signed(6, 50, message); + let good = crate::bls::aggregate(&sigs).unwrap(); + let partial = crate::bls::aggregate(&sigs[1..]).unwrap(); + + let total = AggregatePubkey::sum(&keys).unwrap(); + assert!(total.verify(message, &good)); + assert!(crate::bls::fast_aggregate_verify(&keys, message, &good)); + + // The same key from `total - absentee`, against the matching signature. + let derived = total.minus(&keys[..1]).unwrap(); + assert!(derived.verify(message, &partial)); + assert!(crate::bls::fast_aggregate_verify( + &keys[1..], + message, + &partial + )); + + // A wrong signature, a wrong message and a missing signature fail on both paths. + assert!(!derived.verify(message, &good)); + assert!(!crate::bls::fast_aggregate_verify( + &keys[1..], + message, + &good + )); + assert!(!total.verify(Root::repeat_byte(10), &good)); + assert!(!total.verify(message, &BlsSignature::default())); + } + + #[test] + fn the_identity_aggregate_never_verifies() { + let message = Root::repeat_byte(1); + let (keys, sigs) = signed(3, 90, message); + let signature = crate::bls::aggregate(&sigs).unwrap(); + let identity = AggregatePubkey::sum(&keys).unwrap().minus(&keys).unwrap(); + assert!(!identity.verify(message, &signature)); + } + + #[test] + fn committee_key_sums_index_by_committee_number() { + let keys = pubkeys(2); + let sums = CommitteeKeySums::new(vec![ + AggregatePubkey::sum(&keys[..1]), + None, + AggregatePubkey::sum(&keys), + ]); + assert!(sums.get(0).is_some()); + assert!(sums.get(1).is_none()); + assert!(sums.get(2).is_some()); + assert!(sums.get(3).is_none()); + } +} diff --git a/crates/storage/src/committee_cache.rs b/crates/storage/src/committee_cache.rs index 35c079d9c..a2361b9c8 100644 --- a/crates/storage/src/committee_cache.rs +++ b/crates/storage/src/committee_cache.rs @@ -13,6 +13,11 @@ //! runner, a one-off lookup, a unit test) holds a fresh //! [`CommitteeCache::default`] for as long as that work lasts instead. //! +//! Each resident shuffling also carries a second, independently filled slot +//! for its [`CommitteeKeySums`] (the sum of every committee's public keys), +//! built off the import path and only ever looked up on it; see +//! [`CommitteeCache::get_or_init_key_sums`]. +//! //! This module knows nothing about a `BeaconState`: it is handed a //! [`ShufflingKey`] and, on a miss, runs a builder closure that hands back //! the finished [`EpochCommittees`]. The consensus logic that derives both @@ -25,6 +30,7 @@ use std::sync::{Arc, Mutex, OnceLock}; +use ethlambda_crypto::bls::CommitteeKeySums; use ethlambda_types::beacon::committees::EpochCommittees; use ethlambda_types::beacon::primitives::{Epoch, Root}; @@ -107,16 +113,29 @@ pub enum Lookup { Miss, } -/// One resident shuffling's slot: empty until some call's builder fills it. +/// One resident shuffling's slot: its committees and their key sums, each +/// empty until some call's builder fills it. /// -/// An `Arc` around the `OnceLock` (rather than the `OnceLock` sitting -/// directly in the entry list) is what lets [`CommitteeCache::evict_one`] -/// drop the entry list's own reference to a slot while a waiter elsewhere -/// still holds a clone of the same `Arc`, taken before eviction ran: the -/// waiter's `get_or_init` call keeps running against a slot that is no -/// longer reachable from the cache at all, exactly as safely as if it had -/// never been evicted. -type Slot = Arc>>; +/// The key sums sit beside the committees, rather than in a second cache, so +/// that eviction and head pinning stay in one place and a shuffling's sums die +/// with it: nothing can hold sums for a shuffling the cache no longer holds. +/// The two fill independently. The committees are built on the import path as +/// soon as any block needs them; the sums are built off it by +/// [`CommitteeCache::get_or_init_key_sums`] and only ever read on it, since +/// building them costs a pass over the whole active set. +#[derive(Debug, Default)] +struct Resident { + committees: OnceLock>, + key_sums: OnceLock>, +} + +/// A [`Resident`] behind an `Arc`, which is what lets +/// [`CommitteeCache::evict_one`] drop the entry list's own reference to a slot +/// while a waiter elsewhere still holds a clone of the same `Arc`, taken +/// before eviction ran: the waiter's `get_or_init` call keeps running against +/// a slot that is no longer reachable from the cache at all, exactly as safely +/// as if it had never been evicted. +type Slot = Arc; /// The state one [`Mutex`] in [`CommitteeCache`] guards: the resident entries /// and the current head's pins. One lock over both, rather than two, because @@ -215,7 +234,7 @@ impl CommitteeCache { { Arc::clone(slot) } else { - let slot: Slot = Arc::new(OnceLock::new()); + let slot: Slot = Arc::new(Resident::default()); if state.entries.len() >= COMMITTEE_CACHE_CAPACITY { state.evict_one(); } @@ -230,12 +249,12 @@ impl CommitteeCache { // just inserted, or one a concurrent call inserted a moment ago and // has not finished building yet, is a `Miss` regardless of which of // the racing calls ends up actually running `build`. - let lookup = if slot.get().is_some() { + let lookup = if slot.committees.get().is_some() { Lookup::Hit } else { Lookup::Miss }; - let committees = Arc::clone(slot.get_or_init(|| Arc::new(build()))); + let committees = Arc::clone(slot.committees.get_or_init(|| Arc::new(build()))); (committees, lookup) } @@ -285,7 +304,55 @@ impl CommitteeCache { let (_, keys) = state.head.as_ref()?; let current = keys[HEAD_CURRENT_EPOCH]?; let (_, slot) = state.entries.iter().find(|(key, _)| *key == current)?; - slot.get().cloned() + slot.committees.get().cloned() + } + + /// `key`'s committee key sums, if that shuffling is resident and its sums + /// have been built. + /// + /// Never builds, and never inserts: block import must not pay for a pass + /// over the active set that costs more than it saves for committees + /// attested once. A cold lookup is an answer, and the caller falls back to + /// summing the attesters directly. Named apart from + /// `CommitteeCacheExt::key_sums` for the reason [`Self::pin_head`] gives. + pub fn resident_key_sums(&self, key: ShufflingKey) -> Option> { + let slot = self.resident_slot(key)?; + slot.key_sums.get().cloned() + } + + /// `key`'s committee key sums, running `build` if they are not there yet + /// and sharing the result with every call naming the same `key`. + /// + /// `None`, without running `build`, when the shuffling is not resident: + /// the sums belong to a resident entry and this never creates one, so a + /// caller warms the committees first (the entry's own `get_or_init`) and + /// the sums second. An entry evicted between the two simply loses the + /// warm-up, which costs a later import the direct sum and nothing else. + /// + /// The same concurrency rule as [`Self::get_or_init`]: the entry-list lock + /// is held only to find the slot, never while `build` runs, since `build` + /// is a pass over every active validator's key. Concurrent callers race + /// [`OnceLock::get_or_init`] on the slot: one runs `build`, the rest wait + /// on it. A slot evicted while `build` runs is built into regardless and + /// dropped with its last holder. + pub fn get_or_init_key_sums( + &self, + key: ShufflingKey, + build: impl FnOnce() -> CommitteeKeySums, + ) -> Option> { + let slot = self.resident_slot(key)?; + Some(Arc::clone(slot.key_sums.get_or_init(|| Arc::new(build())))) + } + + /// `key`'s slot, if resident, found under the entry-list lock and handed + /// back as an owned `Arc` so the lock is released before it is used. + fn resident_slot(&self, key: ShufflingKey) -> Option { + let state = self.state.lock().unwrap(); + state + .entries + .iter() + .find(|(entry_key, _)| *entry_key == key) + .map(|(_, slot)| Arc::clone(slot)) } } @@ -469,6 +536,59 @@ mod tests { } } + /// `resident_key_sums` is a lookup: it never creates an entry and never + /// builds sums, however often it is asked. + #[test] + fn key_sums_lookup_never_builds_or_inserts() { + let cache = CommitteeCache::default(); + let k = key(3, 3); + assert!(cache.resident_key_sums(k).is_none(), "not resident"); + assert!(cache.state.lock().unwrap().entries.is_empty()); + + cache.get_or_init(k, || dummy_committees(3)); + assert!(cache.resident_key_sums(k).is_none(), "resident but cold"); + } + + /// Sums are built into a resident shuffling only, once, and then served. + #[test] + fn key_sums_are_built_once_into_a_resident_shuffling() { + let cache = CommitteeCache::default(); + let k = key(3, 3); + let refused = cache.get_or_init_key_sums(k, || panic!("no entry to build into")); + assert!(refused.is_none()); + assert!(cache.state.lock().unwrap().entries.is_empty()); + + cache.get_or_init(k, || dummy_committees(3)); + let first = cache + .get_or_init_key_sums(k, || CommitteeKeySums::new(vec![None])) + .expect("resident"); + let second = cache + .get_or_init_key_sums(k, || panic!("should not rebuild")) + .expect("resident"); + assert!(Arc::ptr_eq(&first, &second)); + assert!(Arc::ptr_eq(&first, &cache.resident_key_sums(k).unwrap())); + } + + /// The sums leave with their shuffling: once the entry is evicted, a + /// lookup finds nothing, and a fresh entry for the key starts cold. + #[test] + fn key_sums_are_evicted_with_their_shuffling() { + let cache = CommitteeCache::default(); + let oldest = key(0, 0); + cache.get_or_init(oldest, || dummy_committees(0)); + cache.get_or_init_key_sums(oldest, || CommitteeKeySums::new(Vec::new())); + assert!(cache.resident_key_sums(oldest).is_some()); + + for n in 1..=COMMITTEE_CACHE_CAPACITY { + let k = key(n as Epoch, n as u8); + cache.get_or_init(k, move || dummy_committees(k.epoch)); + } + assert!(cache.resident_key_sums(oldest).is_none()); + + cache.get_or_init(oldest, || dummy_committees(0)); + assert!(cache.resident_key_sums(oldest).is_none(), "starts cold"); + } + /// Every clone of a `CommitteeCache` shares one underlying cache: a /// lookup through one clone is a hit through another. `CommitteeCache` /// does not itself derive `Clone` (its only owner, `Store`, holds it diff --git a/docs/metrics.md b/docs/metrics.md index 1ea07953f..9178a702f 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -544,6 +544,7 @@ leanMetrics spec. |------|------|-------|-------------------------|--------|---------| | `lean_beacon_epoch_precompute_lookups_total` | Counter | Epoch-crossing imports, by whether a precomputed boundary state was cached | On each import whose parent is before the block's epoch start and whose block is at or after it | result=hit,miss | | | `lean_beacon_epoch_precompute_seconds` | Histogram | Worker time to advance, flush and hash one state | When a precompute worker finishes | | 0.1, 0.25, 0.5, 0.75, 1, 1.5, 2, 3, 4, 8 | +| `lean_beacon_committee_key_sums_warm_seconds` | Histogram | Worker time to sum every committee's keys for one epoch, after its precomputed state was handed to the actor | When a warm-up finishes | | 0.05, 0.1, 0.25, 0.5, 1, 2, 4, 8 | | `lean_beacon_epoch_precompute_started_total` | Counter | Workers started, by trigger | When a worker is spawned | trigger=head,timer | | **Read the hit ratio against epochs.** On a synced follower nearly every From dd3c975208dcd82b9cf15d0891cf78bdd5ea4631 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, 30 Oct 2026 12:11:03 -0300 Subject: [PATCH 2/5] perf(beacon): verify attestations against committee key sums Electra, fulu and gloas process_attestation now build the attestation's signature set from attesters_aggregate_pubkey, which derives the key as the warmed committee sum minus the non-attesters when most of the committee attested, and sums the attesters otherwise. The key is the same group element either way, so the verdict is unchanged; only the point additions differ. Import still never builds the sums: with them cold, the builder sums the attesters directly. The indexed attestation is still built for the participation flags, and the structural checks on it (non-empty, sorted and unique, gloas's length bound) and the push label are the ones the indexed path made. The spec runners use a fresh committee cache, which only ever runs the cold branch, so electra-and-later attestation cases and every block run now run a second time with the sums warmed. Both runs must reach the same verdict and post-state. --- .../src/beacon/helpers/electra.rs | 64 ++++++++++++- .../src/beacon/helpers/gloas.rs | 24 +++++ .../src/beacon/stf/electra.rs | 96 ++++++++++++++++++- .../state_transition/src/beacon/stf/gloas.rs | 11 ++- .../state_transition/tests/beacon_spec/mod.rs | 22 +++++ .../tests/beacon_spec/operations.rs | 39 ++++++-- .../tests/beacon_spec/sanity.rs | 45 +++++++-- .../common/crypto/src/bls/committee_sums.rs | 2 +- 8 files changed, 274 insertions(+), 29 deletions(-) diff --git a/crates/blockchain/state_transition/src/beacon/helpers/electra.rs b/crates/blockchain/state_transition/src/beacon/helpers/electra.rs index 51f516a55..bb880f1f1 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/electra.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/electra.rs @@ -126,7 +126,7 @@ use crate::beacon::bls::{AggregatePubkey, SignatureSet}; use crate::beacon::config::Config; use crate::beacon::constants::{self, FAR_FUTURE_EPOCH}; -use crate::beacon::containers::shared::Validator; +use crate::beacon::containers::shared::{AttestationData, Validator}; use crate::beacon::containers::{BeaconState, electra, fulu, gloas}; use crate::beacon::error::{Error, Result}; use crate::beacon::fork::ForkName; @@ -560,10 +560,6 @@ pub(crate) fn attesting_indices_from_committee_bits( /// /// Phase0 through deneb attestations stay on the direct path: each names one /// committee, and the live chain is past electra. -#[allow( - dead_code, - reason = "wired into process_attestation by the next change" -)] pub(crate) fn attesters_aggregate_pubkey( state: &BeaconState, slot: Slot, @@ -626,6 +622,64 @@ pub(crate) fn attesters_aggregate_pubkey( Ok(total) } +/// The signature check for an electra-shaped attestation, with the key taken +/// from the shuffling's committee key sums when they are warm: the counterpart +/// of [`indexed_attestation_signature_set`] for a caller that still holds the +/// bit-encoded attestation next to the indexed one it built. +/// +/// Makes the structural checks [`indexed_attestation_signature_set_with_domain`] +/// makes on `attesting_indices` (non-empty, sorted and unique) and signs over +/// the same root under the same domain (`DOMAIN_BEACON_ATTESTER` at the +/// target epoch). The key comes from [`attesters_aggregate_pubkey`], the same +/// group element as the sum of `attesting_indices`' keys: the indices are +/// read off the same bits, and a committee member is drawn from the state's own +/// active set, so every index is in the registry by construction. +/// +/// `None` means the attestation is invalid without needing a pairing, as for +/// [`indexed_attestation_signature_set`]. A bound on the number of indices +/// (gloas's `MAX_VALIDATORS_PER_SLOT`) is the caller's to check first. +pub(crate) fn attestation_signature_set( + state: &BeaconState, + data: &AttestationData, + signature: &BlsSignature, + attesting_indices: &[ValidatorIndex], + committee_bits: &electra::CommitteeBits, + is_set: impl Fn(usize) -> bool, + committees: &CommitteeCache, +) -> Option { + if attesting_indices.is_empty() || !are_indices_sorted_and_unique(attesting_indices) { + return None; + } + let key = + attesters_aggregate_pubkey(state, data.slot, committee_bits, is_set, committees).ok()??; + let domain = get_domain( + state, + constants::DOMAIN_BEACON_ATTESTER, + Some(data.target.epoch), + ); + let signing_root = compute_signing_root(data.hash_tree_root(), domain); + SignatureSet::from_aggregate(key, signing_root, signature) +} + +/// [`attestation_signature_set`] for an electra or fulu attestation and the +/// indexed attestation built from it. +pub fn electra_attestation_signature_set( + state: &BeaconState, + attestation: &electra::Attestation, + indexed_attestation: &electra::IndexedAttestation, + committees: &CommitteeCache, +) -> Option { + attestation_signature_set( + state, + &indexed_attestation.data, + &indexed_attestation.signature, + &indexed_attestation.attesting_indices, + &attestation.committee_bits, + |bit| attestation.aggregation_bits.get(bit).unwrap_or(false), + committees, + ) +} + /// The committee members whose bit is set in `attestation`, in ascending /// order. See [`attesting_indices_from_committee_bits`] for the walk itself; /// this is a thin wrapper over it, closing over `attestation`'s own diff --git a/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs b/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs index a95b66f06..f3c15394d 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs @@ -187,6 +187,30 @@ pub fn indexed_attestation_signature_set_with_domain( SignatureSet::fast_aggregate(&pubkeys, signing_root, &indexed_attestation.signature) } +/// [`super::electra::electra_attestation_signature_set`] for a gloas +/// attestation: the same checks, plus the length bound the container no longer +/// enforces (`MAX_VALIDATORS_PER_SLOT`), checked first as +/// [`indexed_attestation_signature_set_with_domain`] does. +pub fn gloas_attestation_signature_set( + state: &BeaconState, + attestation: &gloas::Attestation, + indexed_attestation: &gloas::IndexedAttestation, + committees: &CommitteeCache, +) -> Option { + if indexed_attestation.attesting_indices.len() > preset::MAX_VALIDATORS_PER_SLOT { + return None; + } + super::electra::attestation_signature_set( + state, + &indexed_attestation.data, + &indexed_attestation.signature, + &indexed_attestation.attesting_indices, + &attestation.committee_bits, + |bit| attestation.aggregation_bits.get(bit).unwrap_or(false), + committees, + ) +} + // --------------------------------------------------------------------------- // Attestations // --------------------------------------------------------------------------- diff --git a/crates/blockchain/state_transition/src/beacon/stf/electra.rs b/crates/blockchain/state_transition/src/beacon/stf/electra.rs index 53becb7bb..c0e138775 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/electra.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/electra.rs @@ -61,7 +61,7 @@ use crate::beacon::helpers::accessors::{ use crate::beacon::helpers::altair::{add_flag, base_reward_per_increment, has_flag}; use crate::beacon::helpers::capella::withdrawal_address; use crate::beacon::helpers::electra::{ - compute_exit_epoch_and_update_churn, get_committee_indices, + compute_exit_epoch_and_update_churn, electra_attestation_signature_set, get_committee_indices, get_consolidation_churn_limit_for_fork, get_indexed_attestation, get_max_effective_balance, get_pending_balance_to_withdraw, has_compounding_withdrawal_credential, has_eth1_withdrawal_credential, has_execution_withdrawal_credential, @@ -716,7 +716,7 @@ pub fn process_attestation( let indexed_attestation = get_indexed_attestation(state, attestation, committees)?; signatures.push( - indexed_attestation_signature_set(state, &indexed_attestation), + electra_attestation_signature_set(state, attestation, &indexed_attestation, committees), "is_valid_indexed_attestation(state, get_indexed_attestation(state, attestation))", )?; @@ -2431,4 +2431,96 @@ mod tests { assert!(list[0].len() > 1); assert!(list[1].len() > 1); } + + // -- process_attestation with warmed committee key sums ------------------- + + const ATTESTATION_LABEL: &str = + "is_valid_indexed_attestation(state, get_indexed_attestation(state, attestation))"; + + /// An attestation for slot `SLOTS_PER_EPOCH - 1` of a state one epoch in, + /// signed by every member of its committee but the first two, so that the + /// majority rule subtracts when the sums are warm. `signed_message` is the + /// root the committee actually signs; a root other than the attestation's + /// own makes the signature bad. + fn attestation_for_the_last_slot_of_epoch_zero( + state: &BeaconState, + signed_message: Option, + ) -> electra::Attestation { + use crate::beacon::helpers::accessors::{get_beacon_committee, get_domain}; + use crate::beacon::helpers::misc::compute_signing_root; + use crate::beacon::helpers::test_state::sign_for; + + let slot = preset::SLOTS_PER_EPOCH - 1; + let committee = get_beacon_committee(state, slot, 0).unwrap(); + let data = AttestationData { + slot, + ..Default::default() + }; + let domain = get_domain(state, constants::DOMAIN_BEACON_ATTESTER, Some(0)); + let root = + signed_message.unwrap_or_else(|| compute_signing_root(data.hash_tree_root(), domain)); + + let mut aggregation_bits = electra::AggregationBits::with_length(committee.len()).unwrap(); + let mut signatures = Vec::new(); + for (position, member) in committee.iter().enumerate().skip(2) { + aggregation_bits.set(position, true).unwrap(); + signatures.push(sign_for(*member as usize, root)); + } + let mut committee_bits = electra::CommitteeBits::default(); + committee_bits.set(0, true).unwrap(); + electra::Attestation { + aggregation_bits, + data, + signature: bls::aggregate(&signatures).unwrap(), + committee_bits, + } + } + + fn run_attestation( + mut state: BeaconState, + attestation: &electra::Attestation, + committees: &CommitteeCache, + ) -> Result { + let mut signatures = SignatureBatch::new(); + process_attestation( + &mut state, + attestation, + committees, + &ActiveBalanceCache::default(), + &mut signatures, + )?; + signatures.verify()?; + state.apply_pending_mutations(); + Ok(state) + } + + /// A good attestation reaches the same post-state whether the key sums are + /// cold or warm, and a bad signature is refused under the attestation's + /// own label either way, so the warm path rejects what the cold one does. + #[test] + fn warm_key_sums_change_no_verdict_and_no_post_state() { + let state = + crate::beacon::helpers::test_state::with_signing_validators_at(ForkName::Electra, 1024); + let epoch = get_previous_epoch(&state); + let warm = CommitteeCache::default(); + warm.warm_key_sums(&state, epoch); + assert!(warm.key_sums(&state, epoch).is_some(), "sums are warm"); + let cold = CommitteeCache::default(); + + let good = attestation_for_the_last_slot_of_epoch_zero(&state, None); + let cold_state = run_attestation(state.clone(), &good, &cold).unwrap(); + let warm_state = run_attestation(state.clone(), &good, &warm).unwrap(); + assert_eq!(cold_state.hash_tree_root(), warm_state.hash_tree_root()); + assert_ne!(cold_state.hash_tree_root(), state.hash_tree_root()); + + let wrong_root = crate::beacon::primitives::Root::repeat_byte(0x5a); + let bad = attestation_for_the_last_slot_of_epoch_zero(&state, Some(wrong_root)); + for committees in [&cold, &warm] { + let err = run_attestation(state.clone(), &bad, committees).unwrap_err(); + assert!( + format!("{err:?}").contains(ATTESTATION_LABEL), + "unexpected rejection {err:?}" + ); + } + } } diff --git a/crates/blockchain/state_transition/src/beacon/stf/gloas.rs b/crates/blockchain/state_transition/src/beacon/stf/gloas.rs index 26835b970..2c0bf772c 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/gloas.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/gloas.rs @@ -63,10 +63,11 @@ use crate::beacon::helpers::electra::{g2_point_at_infinity, get_committee_indice use crate::beacon::helpers::gloas::{ add_builder_to_registry, can_builder_cover_bid, convert_builder_index_to_validator_index, convert_validator_index_to_builder_index, get_attestation_participation_flag_indices, - get_indexed_attestation, get_indexed_payload_attestation, gloas_state, gloas_state_ref, - indexed_attestation_signature_set, indexed_payload_attestation_signature_set, - initiate_builder_exit, is_active_builder, is_attestation_same_slot, is_builder_index, - is_builder_withdrawal_credential, settle_builder_payment, + get_indexed_attestation, get_indexed_payload_attestation, gloas_attestation_signature_set, + gloas_state, gloas_state_ref, indexed_attestation_signature_set, + indexed_payload_attestation_signature_set, initiate_builder_exit, is_active_builder, + is_attestation_same_slot, is_builder_index, is_builder_withdrawal_credential, + settle_builder_payment, }; use crate::beacon::helpers::misc::{compute_domain, compute_epoch_at_slot, compute_signing_root}; use crate::beacon::helpers::mutators::{decrease_balance, increase_balance, slash_validator}; @@ -1423,7 +1424,7 @@ pub fn process_attestation( let indexed_attestation = get_indexed_attestation(state, attestation, committees)?; signatures.push( - indexed_attestation_signature_set(state, &indexed_attestation), + gloas_attestation_signature_set(state, attestation, &indexed_attestation, committees), "process_attestation: is_valid_indexed_attestation(state, get_indexed_attestation(state, attestation))", )?; diff --git a/crates/blockchain/state_transition/tests/beacon_spec/mod.rs b/crates/blockchain/state_transition/tests/beacon_spec/mod.rs index f84ae0d5b..878e028a3 100644 --- a/crates/blockchain/state_transition/tests/beacon_spec/mod.rs +++ b/crates/blockchain/state_transition/tests/beacon_spec/mod.rs @@ -595,3 +595,25 @@ pub fn check_transition( (false, Err(_)) => Ok(()), } } + +/// Warms `committees`' key sums for the epochs a block applied to `state` can +/// reach: its previous, current and next epochs, the same three the node pins +/// for its head. A case that crosses an epoch boundary asks the next epoch's +/// shuffling after `process_slots`, whose key `state` already names. +/// +/// The fixture runners run an electra-or-later attestation twice, once with a +/// fresh cache and once after this, because a fresh cache only ever exercises +/// the cold branch of the attestation key builder. Both runs must reach the +/// same verdict and the same post-state. +pub fn warm_key_sums_around( + committees: ðlambda_state_transition::beacon::helpers::accessors::CommitteeCache, + state: &BeaconState, +) { + use ethlambda_state_transition::beacon::helpers::accessors::{ + CommitteeCacheExt as _, get_current_epoch, get_previous_epoch, + }; + let current = get_current_epoch(state); + for epoch in [get_previous_epoch(state), current, current + 1] { + committees.warm_key_sums(state, epoch); + } +} diff --git a/crates/blockchain/state_transition/tests/beacon_spec/operations.rs b/crates/blockchain/state_transition/tests/beacon_spec/operations.rs index 835380139..a47cf02f7 100644 --- a/crates/blockchain/state_transition/tests/beacon_spec/operations.rs +++ b/crates/blockchain/state_transition/tests/beacon_spec/operations.rs @@ -173,11 +173,11 @@ fn apply( case: &Case, state: &mut BeaconState, config: &Config, + committees: &CommitteeCache, ) -> Result<(), String> { - // Only the `attestation` handler reads this. Bound up here so each of its - // calls fits on one line; a case processes a single attestation, so there - // is nothing for the cache to share between calls. - let committees = CommitteeCache::default(); + // `committees` is only read by the `attestation` handler. The caller owns + // it so that an electra-or-later attestation can be run once with a cold + // cache and once with a warmed one; see [`trials`]. let active_balances = ActiveBalanceCache::default(); // Every signature an operation defers lands here and is checked once the // operation has run, which is how a block's own driver treats them: a @@ -283,7 +283,7 @@ fn apply( state, &attestation, config, - &committees, + committees, &mut signatures, ) } @@ -295,7 +295,7 @@ fn apply( altair_stf::process_attestation( state, &attestation, - &committees, + committees, &active_balances, &mut signatures, ) @@ -307,7 +307,7 @@ fn apply( deneb_stf::process_attestation( state, &attestation, - &committees, + committees, &active_balances, &mut signatures, ) @@ -320,7 +320,7 @@ fn apply( electra_stf::process_attestation( state, &attestation, - &committees, + committees, &active_balances, &mut signatures, ) @@ -338,7 +338,7 @@ fn apply( state, &attestation, meta.parent_slot, - &committees, + committees, &active_balances, &mut signatures, ) @@ -696,7 +696,26 @@ pub fn trials() -> Vec { .map_err(|err| format!("the fixture's pre-state does not decode: {err:?}"))?; let config = super::case_config(case); - let outcome = apply(&handler, case, &mut state, &config); + // An electra-or-later attestation picks its aggregate key from the + // shuffling's committee key sums when they are warm, and a fresh + // cache is never warm, so the case runs a second time on a copy + // of the pre-state with the sums built. Both runs must agree. + let warm_run = (handler == "attestation" && case.fork >= ForkName::Electra) + .then(|| state.clone()); + + let outcome = apply(&handler, case, &mut state, &config, &CommitteeCache::default()); + if let Some(mut warm_state) = warm_run { + let committees = CommitteeCache::default(); + super::warm_key_sums_around(&committees, &warm_state); + let warm = apply(&handler, case, &mut warm_state, &config, &committees); + if warm != outcome { + return Err(format!( + "the warm-sums run answered {warm:?} where the cold run answered {outcome:?}" + )); + } + super::check_transition(case, warm, &warm_state) + .map_err(|err| format!("warm sums: {err}"))?; + } super::check_transition(case, outcome, &state) })); } diff --git a/crates/blockchain/state_transition/tests/beacon_spec/sanity.rs b/crates/blockchain/state_transition/tests/beacon_spec/sanity.rs index 7d85ee935..a3f456130 100644 --- a/crates/blockchain/state_transition/tests/beacon_spec/sanity.rs +++ b/crates/blockchain/state_transition/tests/beacon_spec/sanity.rs @@ -13,6 +13,7 @@ //! Unlike the `operations` suite, these run the full [`state_transition`], so //! they check the proposer signature and the committed `state_root` too. +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}; @@ -27,7 +28,17 @@ struct BlocksMeta { } /// Applies every block in the case, stopping at the first one rejected. -fn apply_blocks(case: &Case, state: &mut BeaconState, config: &Config) -> Result<(), String> { +/// +/// With `warm`, the shuffling's committee key sums are built before each block, +/// from the state it applies to, the way the node's epoch precompute worker +/// builds them ahead of the epoch's first block. A fresh cache would otherwise +/// only ever run the attestation key builder's cold branch. +fn apply_blocks( + case: &Case, + state: &mut BeaconState, + config: &Config, + warm: bool, +) -> Result<(), String> { let meta: BlocksMeta = case.yaml("meta"); // `sanity/blocks`, `finality`, and `random` ship no `execution.yaml` @@ -59,6 +70,10 @@ fn apply_blocks(case: &Case, state: &mut BeaconState, config: &Config) -> Result state.latest_block_header_mut().state_root = root; } + if warm { + super::warm_key_sums_around(&committees, state); + } + stf::state_transition( state, &block, @@ -96,11 +111,29 @@ fn run(runner: &str, handler: &str, slots: bool) -> Vec { .map_err(|err| format!("the fixture's pre-state does not decode: {err:?}"))?; let config = super::case_config(case); - let outcome = if slots { - apply_slots(case, &mut state, &config) - } else { - apply_blocks(case, &mut state, &config) - }; + if slots { + let outcome = apply_slots(case, &mut state, &config); + return super::check_transition(case, outcome, &state); + } + + // Electra and later pick an attestation's aggregate key from + // warmed committee key sums when there are any, so those cases run + // a second time with the sums built. Both runs must reach the same + // verdict and the same post-state. + if case.fork >= ForkName::Electra { + let mut warm_state = state.clone(); + let warm = apply_blocks(case, &mut warm_state, &config, true); + let cold = apply_blocks(case, &mut state, &config, false); + if warm != cold { + return Err(format!( + "the warm-sums run answered {warm:?} where the cold run answered {cold:?}" + )); + } + super::check_transition(case, warm, &warm_state) + .map_err(|err| format!("warm sums: {err}"))?; + return super::check_transition(case, cold, &state); + } + let outcome = apply_blocks(case, &mut state, &config, false); super::check_transition(case, outcome, &state) })); } diff --git a/crates/common/crypto/src/bls/committee_sums.rs b/crates/common/crypto/src/bls/committee_sums.rs index 3d5b6371f..df30eee9b 100644 --- a/crates/common/crypto/src/bls/committee_sums.rs +++ b/crates/common/crypto/src/bls/committee_sums.rs @@ -131,8 +131,8 @@ impl std::fmt::Debug for CommitteeKeySums { #[cfg(test)] mod tests { - use super::*; use super::super::DST; + use super::*; use blst::min_pk::SecretKey; fn secret(seed: u64) -> SecretKey { From 508c43ea03a5c4afdb94003616262c245e6efd70 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, 30 Oct 2026 13:06:22 -0300 Subject: [PATCH 3/5] feat(benchmark): add --warm-committee-sums to the import replay The replay drives import_block directly with no tick loop, so the epoch precompute worker never runs and committee key sums stay cold; the committee-sum saving is invisible in a replay although a live node gets it. The flag warms each epoch's sums before its first block, outside every measured span, so the warm-up is in no phase and no wall_seconds. It is recorded in the report params and the human header. --- bin/ethlambda/src/benchmark/import/mod.rs | 12 ++++ bin/ethlambda/src/benchmark/import/replay.rs | 71 +++++++++++++++++++- bin/ethlambda/src/benchmark/report/import.rs | 20 +++++- docs/benchmarking.md | 13 ++++ 4 files changed, 112 insertions(+), 4 deletions(-) diff --git a/bin/ethlambda/src/benchmark/import/mod.rs b/bin/ethlambda/src/benchmark/import/mod.rs index f8344b803..eb513e4c5 100644 --- a/bin/ethlambda/src/benchmark/import/mod.rs +++ b/bin/ethlambda/src/benchmark/import/mod.rs @@ -91,6 +91,12 @@ pub(crate) struct ReplayOptions { /// sleep is outside every measured span. 0 feeds blocks back to back. #[arg(long, value_name = "MILLISECONDS", default_value_t = 0)] pub block_delay: u64, + /// Warm each epoch's committee key sums before its first block is + /// imported, outside every measured span. A live node does this on a + /// worker between slots (the epoch precompute); a replay has no tick + /// loop, so without this flag part of the import never sees warm sums. + #[arg(long)] + pub warm_committee_sums: bool, /// Report format printed to stdout. Logs go to stderr, so JSON output can /// be piped directly (e.g. into jq). #[arg(long, value_enum, default_value_t = OutputFormat::Human)] @@ -194,6 +200,12 @@ mod tests { .replay } + #[test] + fn warm_committee_sums_is_off_unless_asked_for() { + assert!(!parse(&[]).warm_committee_sums); + assert!(parse(&["--warm-committee-sums"]).warm_committee_sums); + } + #[test] fn block_delay_defaults_to_zero_and_parses_milliseconds() { assert_eq!(parse(&[]).block_delay, 0); diff --git a/bin/ethlambda/src/benchmark/import/replay.rs b/bin/ethlambda/src/benchmark/import/replay.rs index f0365571f..1915e59ee 100644 --- a/bin/ethlambda/src/benchmark/import/replay.rs +++ b/bin/ethlambda/src/benchmark/import/replay.rs @@ -24,6 +24,7 @@ use ethlambda_blockchain::metrics::{BLOCK_ARRIVAL_PHASES, BLOCK_IMPORT_PHASES}; use ethlambda_blockchain::{BlockChainServer, ImportOutcome}; use ethlambda_p2p::beacon::decode; use ethlambda_state_transition::beacon::fork_choice; +use ethlambda_state_transition::beacon::helpers::accessors::CommitteeCacheExt as _; use ethlambda_storage::Store; use ethlambda_storage::backend::RocksDBBackend; use ethlambda_types::ShortRoot; @@ -123,10 +124,17 @@ pub(crate) async fn replay_corpus(dir: &Path, options: &ReplayOptions) -> eyre:: BlockChainServer::for_replay(store, None, options.safe_slots_to_import_optimistically); let mut samples = Vec::with_capacity(manifest.slots.len()); + // The epoch of the block imported before the one in hand, for + // `--warm-committee-sums`. + let mut previous_epoch: Option = None; let warmup_total = manifest.warmup_slots.len(); for (position, &slot) in manifest.warmup_slots.iter().enumerate() { let (block, block_root) = load_block(dir, &config, &mut clock, slot)?; + if options.warm_committee_sums { + warm_committee_sums(&clock, &block, previous_epoch)?; + } + previous_epoch = Some(slot / preset::SLOTS_PER_EPOCH); let wall_seconds = import(&mut server, block, slot, block_root).await?; eprintln!( "warm-up {}/{warmup_total}: slot {slot} imported in {}", @@ -139,6 +147,12 @@ pub(crate) async fn replay_corpus(dir: &Path, options: &ReplayOptions) -> eyre:: let total = manifest.slots.len(); for (iteration, &slot) in manifest.slots.iter().enumerate() { let (block, block_root) = load_block(dir, &config, &mut clock, slot)?; + // Before the timer starts, so the warm-up is in no phase and no + // `wall_seconds`. + if options.warm_committee_sums { + warm_committee_sums(&clock, &block, previous_epoch)?; + } + previous_epoch = Some(slot / preset::SLOTS_PER_EPOCH); let timer = PhaseTimer::start(IMPORT_PHASE_HISTOGRAM); let wall_seconds = import(&mut server, block, slot, block_root).await?; @@ -164,7 +178,12 @@ pub(crate) async fn replay_corpus(dir: &Path, options: &ReplayOptions) -> eyre:: Ok(Report::new( Environment::collect(), - params_from(&manifest, dir, options.block_delay), + params_from( + &manifest, + dir, + options.block_delay, + options.warm_committee_sums, + ), samples, )) } @@ -198,6 +217,48 @@ fn load_block( Ok((block, block_root)) } +/// Warms the committee key sums a live node's epoch precompute worker would +/// have warmed before `block` arrives: its epoch's, when that epoch is later +/// than `previous_epoch`'s, and, before the first block of the replay +/// (`previous_epoch` is `None`), the epoch before as well. +/// +/// The worker advances the head's post-state to the epoch start and warms +/// from that state; this does the same from the block's parent post-state, so +/// the shuffling key it files the sums under is the one the import looks up. +/// Nothing else the worker does is reproduced: in a replay the import +/// already produces the epoch's state itself, inside the measured span. +fn warm_committee_sums( + store: &Store, + block: &SignedBeaconBlock, + previous_epoch: Option, +) -> eyre::Result<()> { + let epoch = block.slot() / preset::SLOTS_PER_EPOCH; + let epochs: Vec = match previous_epoch { + None => epoch.checked_sub(1).into_iter().chain([epoch]).collect(), + Some(previous) if epoch > previous => vec![epoch], + Some(_) => return Ok(()), + }; + let parent_root = block.parent_root(); + let parent_state = store.get_state(&parent_root)?.ok_or_else(|| { + eyre::eyre!( + "no post-state for parent {} of the block at slot {}", + ShortRoot(&parent_root.0), + block.slot() + ) + })?; + let config = store.config(); + let committee_cache = store.committee_cache(); + for epoch in epochs { + if parent_state.slot() < epoch * preset::SLOTS_PER_EPOCH { + let advanced = fork_choice::advance_to_epoch_start(&parent_state, epoch, &config)?; + committee_cache.warm_key_sums(&advanced, epoch); + } else { + committee_cache.warm_key_sums(&parent_state, epoch); + } + } + Ok(()) +} + /// Import one block, returning the wall time of the `import_block` call /// alone, and fail unless it imported. async fn import( @@ -240,7 +301,12 @@ fn read_anchor(dir: &Path, config: &Config) -> eyre::Result<(BeaconState, Signed /// The report parameters a manifest already carries, so the loop above never /// has to reconstruct them from samples. -fn params_from(manifest: &Manifest, dir: &Path, block_delay_ms: u64) -> Params { +fn params_from( + manifest: &Manifest, + dir: &Path, + block_delay_ms: u64, + warm_committee_sums: bool, +) -> Params { Params { mode: "import", corpus: dir.display().to_string(), @@ -252,6 +318,7 @@ fn params_from(manifest: &Manifest, dir: &Path, block_delay_ms: u64) -> Params { range_end: manifest.range_end, blocks: manifest.slots.len(), block_delay_ms, + warm_committee_sums, } } diff --git a/bin/ethlambda/src/benchmark/report/import.rs b/bin/ethlambda/src/benchmark/report/import.rs index 74bb20e8e..2a744e869 100644 --- a/bin/ethlambda/src/benchmark/report/import.rs +++ b/bin/ethlambda/src/benchmark/report/import.rs @@ -24,6 +24,9 @@ pub(crate) struct Params { /// Milliseconds slept after each block's import, outside every measured /// span. 0 means blocks were fed back to back. pub block_delay_ms: u64, + /// Whether each epoch's committee key sums were warmed before its first + /// block, outside every measured span (`--warm-committee-sums`). + pub warm_committee_sums: bool, } #[derive(Debug, Serialize)] @@ -107,7 +110,7 @@ impl Report { let _ = writeln!(out, "Block-import benchmark — {} workload", params.mode); let _ = writeln!( out, - " corpus={} network={} anchor_slot={} warmup_blocks={} range=[{}, {}] blocks={} block_delay_ms={}", + " corpus={} network={} anchor_slot={} warmup_blocks={} range=[{}, {}] blocks={} block_delay_ms={} warm_committee_sums={}", params.corpus, params.network, params.anchor_slot, @@ -115,7 +118,8 @@ impl Report { params.range_start, params.range_end, params.blocks, - params.block_delay_ms + params.block_delay_ms, + params.warm_committee_sums ); let _ = writeln!( out, @@ -217,6 +221,7 @@ mod tests { range_end: 3, blocks: 2, block_delay_ms: 0, + warm_committee_sums: false, } } @@ -231,6 +236,17 @@ mod tests { assert!(report.human_table().contains("block_delay_ms=750")); } + #[test] + fn warm_committee_sums_is_recorded_in_the_json_and_the_header() { + let mut params = params(); + params.warm_committee_sums = true; + let report = Report::new(Environment::collect(), params, Vec::new()); + + let json: serde_json::Value = serde_json::from_str(&report.to_json().unwrap()).unwrap(); + assert_eq!(json["params"]["warm_committee_sums"], true); + assert!(report.human_table().contains("warm_committee_sums=true")); + } + #[test] fn the_range_prints_inclusive_as_fetch_takes_it() { let report = Report::new(Environment::collect(), params(), Vec::new()); diff --git a/docs/benchmarking.md b/docs/benchmarking.md index ae50db389..648bfc4b6 100644 --- a/docs/benchmarking.md +++ b/docs/benchmarking.md @@ -249,6 +249,19 @@ has no end-to-end figure to adjust. The value is recorded as `params.block_delay_ms` in the JSON report and printed in the human header, so compare only reports made with the same delay. +**`--warm-committee-sums`** (default off) warms each epoch's committee key +sums before the epoch's first block is imported. A live node does this on the +epoch precompute worker between slots; a replay drives `import_block` +directly with no tick loop, so that worker never runs and the sums stay cold, +which hides the committee-sum saving. This flag reproduces only the worker's +committee-sum warm-up (not its state advance, which the import still does +inside the measured span). It runs before a block whose epoch is later than the +previous block's, and for both the first block's epoch and the one before it +ahead of the first block. The warm-up runs outside every measured span, so it +is in no phase and no `wall_seconds`. The flag is recorded as +`params.warm_committee_sums` in the JSON report and printed in the human +header; compare only reports made with the same setting. + ### What is measured The measured span is one `BlockChainServer::import_block` call per block, the From 562b3691b148bfddfc50074b32d635728f92d786 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, 30 Oct 2026 18:35:14 -0300 Subject: [PATCH 4/5] perf(beacon): keep cold committee sums at the plain path's cost and warm missed epochs With the sums cold, every committee was summed on its own, and each sum of a few hundred keys paid blst's pool dispatch separately where the plain path made one sum over all attesters: a cold multi-committee aggregate got slower than before this change. Every directly summed committee now goes into one sum, so the cold path is that plain sum again. Warming only ran for the epoch a precompute worker advanced into. The worker now also warms the previous epoch (after a restart mid-epoch, or a boundary whose worker did not run), and a boundary state someone else already cached (a gloas bid for the epoch's first slot) gets a warm-only task instead of no warm-up at all. Pre-electra states skip it, since nothing reads the sums there, and the warm-up histogram now times the sums alone, only when they are built. --- crates/blockchain/src/epoch_precompute.rs | 45 ++++++++++++---- .../src/beacon/helpers/accessors.rs | 14 ++++- .../src/beacon/helpers/electra.rs | 51 ++++++++++++------- .../common/crypto/src/bls/committee_sums.rs | 27 ++++++++++ docs/metrics.md | 2 +- 5 files changed, 108 insertions(+), 31 deletions(-) diff --git a/crates/blockchain/src/epoch_precompute.rs b/crates/blockchain/src/epoch_precompute.rs index 78ac11e2e..30f5f69d0 100644 --- a/crates/blockchain/src/epoch_precompute.rs +++ b/crates/blockchain/src/epoch_precompute.rs @@ -183,13 +183,10 @@ impl BlockChainServer { Trigger::Head => head_slot / preset::SLOTS_PER_EPOCH + 1, Trigger::Timer { slot } => slot / preset::SLOTS_PER_EPOCH + 1, }; - let already_cached = self - .store - .cached_state(CacheKey::CheckpointState { - epoch: candidate_epoch, - root: head_root, - }) - .is_some(); + let cached = self.store.cached_state(CacheKey::CheckpointState { + epoch: candidate_epoch, + root: head_root, + }); let inputs = DecisionInputs { trigger, syncing: self.sync_status.is_syncing(), @@ -197,9 +194,24 @@ impl BlockChainServer { head_root, current_slot: self.wall_clock_slot(), in_flight: self.epoch_precompute_in_flight, - already_cached, + already_cached: cached.is_some(), }; let Some(key) = decide(&inputs) else { + // Someone else advanced the state already (a gloas bid for the + // epoch's first slot does), so no worker runs: warm the sums from + // that state instead, or the epoch would import without them. + if let Some(state) = cached + && decide(&DecisionInputs { + already_cached: false, + ..inputs + }) + .is_some() + { + let committee_cache = self.store.committee_cache(); + tokio::task::spawn_blocking(move || { + warm_epoch_key_sums(&committee_cache, &state, candidate_epoch) + }); + } return; }; // The head's post-state is resident (it was just imported); a read that @@ -233,13 +245,26 @@ impl BlockChainServer { // not wait for key sums, which only make its attestations cheaper. let _ = actor.send(EpochPrecomputed { key, result }); if let Some(state) = advanced { - let _timing = stf_metrics::time_key_sums_warm(); - committee_cache.warm_key_sums(&state, key.epoch); + warm_epoch_key_sums(&committee_cache, &state, key.epoch); } }); } } +/// Warms the key sums `epoch`'s blocks read: `epoch`'s own, and the previous +/// epoch's for the attestations those blocks carry from it. The previous +/// epoch's are normally warm already, from the last boundary; after a restart +/// mid-epoch, or a boundary whose worker did not run, they are not. Both calls +/// return at once when the sums are resident. +fn warm_epoch_key_sums( + committee_cache: ðlambda_storage::CommitteeCache, + state: &BeaconState, + epoch: u64, +) { + committee_cache.warm_key_sums(state, epoch); + committee_cache.warm_key_sums(state, epoch.saturating_sub(1)); +} + impl Handler for BlockChainServer { async fn handle(&mut self, msg: EpochPrecomputeCheck, ctx: &Context) { self.maybe_start_epoch_precompute(Trigger::Timer { slot: msg.slot }, ctx); diff --git a/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs b/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs index cfb5413ab..ef4f2aec4 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs @@ -332,7 +332,9 @@ pub trait CommitteeCacheExt { /// A committee with a member whose key fails validation gets no sum /// (`None` in its slot), so lookups of it fall back to the direct path /// rather than to a sum that omits the member. Does nothing for an epoch - /// `state` cannot key, since there is nowhere to file the result. + /// `state` cannot key, since there is nowhere to file the result, nor + /// before electra, whose attestations never read the sums. Idempotent, so + /// a caller unsure whether an epoch is warm can just ask again. fn warm_key_sums(&self, state: &BeaconState, epoch: Epoch); } @@ -365,6 +367,9 @@ impl CommitteeCacheExt for CommitteeCache { } fn warm_key_sums(&self, state: &BeaconState, epoch: Epoch) { + if state.fork_name() < ForkName::Electra { + return; + } let Some(key) = shuffling_key(state, epoch) else { return; }; @@ -374,7 +379,12 @@ impl CommitteeCacheExt for CommitteeCache { // Resident before the sums are filed into it; a no-op when the import // path already built them. let committees = self.committees(state, epoch); - self.get_or_init_key_sums(key, || build_committee_key_sums(state, &committees, epoch)); + self.get_or_init_key_sums(key, || { + // The sums alone: not the committees above, nor a lookup that + // found the work already done. + let _timing = crate::metrics::time_key_sums_warm(); + build_committee_key_sums(state, &committees, epoch) + }); } } diff --git a/crates/blockchain/state_transition/src/beacon/helpers/electra.rs b/crates/blockchain/state_transition/src/beacon/helpers/electra.rs index bb880f1f1..471d538d6 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/electra.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/electra.rs @@ -550,13 +550,18 @@ pub(crate) fn attesting_indices_from_committee_bits( /// Per committee, when the shuffling's key sums are warm /// ([`CommitteeCacheExt::key_sums`], lookup only: import never builds them) /// and more than half of the committee attested, the committee's part is its -/// sum minus the keys of the non-attesters; otherwise it is the sum of the -/// attesters. A committee whose sum is `None` (a member's key fails -/// validation) takes the second branch, so only the attesters' own keys decide -/// it, exactly as with cold sums. The parts are then added. -/// Subtracting is the same group element as summing the attesters, so the -/// pairing check cannot tell the two apart; only the point additions differ, -/// absentees' instead of attesters'. +/// sum minus the keys of the non-attesters; otherwise its attesters' keys are +/// summed directly. A committee whose sum is `None` (a member's key fails +/// validation) is summed directly, so only the attesters' own keys decide it, +/// exactly as with cold sums. Subtracting is the same group element as summing +/// the attesters, so the pairing check cannot tell the two apart; only the +/// point additions differ, absentees' instead of attesters'. +/// +/// Every directly summed attester, across all committees, goes into one sum +/// rather than one per committee. With cold sums that is the single sum of +/// every attester the plain path makes, at its cost: blst splits a large sum +/// over its pool, and one sum per committee would pay that pool's dispatch +/// once per committee for a few hundred keys each. /// /// Phase0 through deneb attestations stay on the direct path: each names one /// committee, and the live chain is past electra. @@ -574,7 +579,10 @@ pub(crate) fn attesters_aggregate_pubkey( let pubkey_of = |index: ValidatorIndex| state.validator(index).ok().map(|v| v.pubkey); + // Committees derived from their sums, added as they come, and the keys of + // every committee summed directly, summed once at the end. let mut total: Option = None; + let mut direct = Vec::new(); let mut committee_offset = 0usize; for committee_index in get_committee_indices(committee_bits) { let committee = epoch_committees.committee(slot, committee_index)?; @@ -591,7 +599,7 @@ pub(crate) fn attesters_aggregate_pubkey( let number = committee_number(slot, committees_per_slot, committee_index)?; let sum = key_sums.as_ref().and_then(|sums| sums.get(number as usize)); - let part = match sum { + match sum { Some(sum) if attester_count * 2 > committee.len() => { let absent: Option> = committee .iter() @@ -599,19 +607,26 @@ pub(crate) fn attesters_aggregate_pubkey( .filter(|(_, set)| !**set) .map(|(index, _)| pubkey_of(*index)) .collect(); - absent.and_then(|absent| sum.minus(&absent)) + let Some(part) = absent.and_then(|absent| sum.minus(&absent)) else { + return Ok(None); + }; + total = Some(match total { + Some(total) => total.plus(&part), + None => part, + }); } _ => { - let present: Option> = committee - .iter() - .zip(&attested) - .filter(|(_, set)| **set) - .map(|(index, _)| pubkey_of(*index)) - .collect(); - present.and_then(|present| AggregatePubkey::sum(&present)) + for (index, _) in committee.iter().zip(&attested).filter(|(_, set)| **set) { + let Some(pubkey) = pubkey_of(*index) else { + return Ok(None); + }; + direct.push(pubkey); + } } - }; - let Some(part) = part else { + } + } + if !direct.is_empty() { + let Some(part) = AggregatePubkey::sum(&direct) else { return Ok(None); }; total = Some(match total { diff --git a/crates/common/crypto/src/bls/committee_sums.rs b/crates/common/crypto/src/bls/committee_sums.rs index df30eee9b..a59000a66 100644 --- a/crates/common/crypto/src/bls/committee_sums.rs +++ b/crates/common/crypto/src/bls/committee_sums.rs @@ -201,6 +201,33 @@ mod tests { assert_eq!(total.minus(&[]).unwrap().to_bytes(), total.to_bytes()); } + /// The same over random masks of a committee-sized run, with the attesters + /// scattered rather than a prefix: `sum - absentees` and the direct sum of + /// the attesters are the same bytes. + #[test] + fn minus_agrees_with_the_direct_sum_over_random_masks() { + let keys = pubkeys(64); + let total = AggregatePubkey::sum(&keys).unwrap(); + // xorshift64, fixed seed: reproducible without a seeded-RNG dependency. + let mut state = 0x9e37_79b9_7f4a_7c15u64; + for round in 0..40 { + state ^= state << 13; + state ^= state >> 7; + state ^= state << 17; + let (attesters, absentees): (Vec<_>, Vec<_>) = keys + .iter() + .enumerate() + .partition(|(position, _)| state >> position & 1 == 1); + let attesters: Vec = attesters.into_iter().map(|(_, key)| *key).collect(); + let absentees: Vec = absentees.into_iter().map(|(_, key)| *key).collect(); + if attesters.is_empty() { + continue; + } + let derived = total.minus(&absentees).unwrap(); + assert_eq!(derived.to_bytes(), direct(&attesters), "round {round}"); + } + } + #[test] fn minus_with_an_invalid_key_is_none() { let keys = pubkeys(4); diff --git a/docs/metrics.md b/docs/metrics.md index 9178a702f..eeb9393e6 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -544,7 +544,7 @@ leanMetrics spec. |------|------|-------|-------------------------|--------|---------| | `lean_beacon_epoch_precompute_lookups_total` | Counter | Epoch-crossing imports, by whether a precomputed boundary state was cached | On each import whose parent is before the block's epoch start and whose block is at or after it | result=hit,miss | | | `lean_beacon_epoch_precompute_seconds` | Histogram | Worker time to advance, flush and hash one state | When a precompute worker finishes | | 0.1, 0.25, 0.5, 0.75, 1, 1.5, 2, 3, 4, 8 | -| `lean_beacon_committee_key_sums_warm_seconds` | Histogram | Worker time to sum every committee's keys for one epoch, after its precomputed state was handed to the actor | When a warm-up finishes | | 0.05, 0.1, 0.25, 0.5, 1, 2, 4, 8 | +| `lean_beacon_committee_key_sums_warm_seconds` | Histogram | Worker time to sum one epoch's committee keys, off the import path: the sums alone, not the committees they are built from | When a warm-up that built sums finishes; a warm-up that finds them resident records nothing | | 0.05, 0.1, 0.25, 0.5, 1, 2, 4, 8 | | `lean_beacon_epoch_precompute_started_total` | Counter | Workers started, by trigger | When a worker is spawned | trigger=head,timer | | **Read the hit ratio against epochs.** On a synced follower nearly every From 944e1512d6c32771c8842feea616452dc325c290 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Wed, 11 Nov 2026 15:39:40 -0300 Subject: [PATCH 5/5] test(beacon): make the committee key-sum tests preset-agnostic The two attesters_aggregate_pubkey tests added in #742 assumed the mainnet preset, but `make test-beacon-minimal` (and the minimal-preset test job) runs the crate's `--lib` target with `preset-minimal` too, so both failed there: - the_builder_key_is_the_attesters_sum_with_cold_and_warm_sums used slot offset 31, past minimal's 8-slot epoch (SpecAssert "compute_epoch_at_slot(slot) == epoch"). - a_committee_with_an_invalid_member_key_falls_back took members[5] of a committee that holds 4 members under minimal (index out of bounds). Take the last offset from preset::SLOTS_PER_EPOCH and the bad member from the committee's middle, so both run under either preset. --- .../state_transition/src/beacon/helpers/electra.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/blockchain/state_transition/src/beacon/helpers/electra.rs b/crates/blockchain/state_transition/src/beacon/helpers/electra.rs index 471d538d6..51a649895 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/electra.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/electra.rs @@ -2181,7 +2181,10 @@ mod tests { use crate::beacon::primitives::Root; /// Enough validators for two committees per slot, so one attestation can - /// name two committees; mainnet's preset is the only one these run under. + /// name two committees. These tests run under both presets (`make + /// test-beacon-minimal` runs `--lib` with `preset-minimal`), so slots and + /// member positions are derived from the preset rather than hardcoded: + /// minimal's epoch is 8 slots and its committees here hold 4 members. fn two_committees_per_slot_state() -> BeaconState { let count = (preset::SLOTS_PER_EPOCH * preset::TARGET_COMMITTEE_SIZE * 2) as usize; with_signing_validators_at(ForkName::Electra, count) @@ -2232,7 +2235,7 @@ mod tests { assert!(warm.key_sums(&state, epoch).is_some()); assert!(cold.key_sums(&state, epoch).is_none()); - for slot_offset in [0u64, 7, 31] { + for slot_offset in [0u64, 7, preset::SLOTS_PER_EPOCH - 1] { let slot = compute_start_slot_at_epoch(epoch) + slot_offset; let committee_sets: [&[usize]; 3] = [&[0], &[1], &[0, 1]]; for named in committee_sets { @@ -2315,7 +2318,7 @@ mod tests { .committee(slot, 0) .unwrap() .to_vec(); - let bad = members[5]; + let bad = members[members.len() / 2]; state.validator_mut(bad).unwrap().pubkey = Default::default(); let committees = CommitteeCache::default();