diff --git a/crates/blockchain/state_transition/src/beacon/gossip/aggregate.rs b/crates/blockchain/state_transition/src/beacon/gossip/aggregate.rs index d95914a61..3b7feaa1a 100644 --- a/crates/blockchain/state_transition/src/beacon/gossip/aggregate.rs +++ b/crates/blockchain/state_transition/src/beacon/gossip/aggregate.rs @@ -387,12 +387,16 @@ pub fn stateful_checks( let Ok(aggregator) = state.validator(aggregator_index) else { return Err(Outcome::Reject(RejectReason::UnknownValidator)); }; + let aggregator_pubkey = bls::IndexedPubkey { + index: aggregator_index, + pubkey: aggregator.pubkey, + }; // [REJECT] The selection proof selects the validator as an aggregator. let selection_proof = aggregate.selection_proof(); let selection_domain = get_domain_from_schedule(&config, &state, DOMAIN_SELECTION_PROOF, target_epoch); let selection_signing_root = compute_signing_root(data.slot.hash_tree_root(), selection_domain); - if !bls::verify(&aggregator.pubkey, selection_signing_root, &selection_proof) { + if !bls::verify(&aggregator_pubkey, selection_signing_root, &selection_proof) { return Err(Outcome::Reject(RejectReason::SelectionProof)); } // [REJECT] The aggregator's own signature, over the whole envelope. @@ -401,7 +405,7 @@ pub fn stateful_checks( let aggregator_signing_root = compute_signing_root(aggregate_and_proof_root(aggregate), aggregator_domain); if !bls::verify( - &aggregator.pubkey, + &aggregator_pubkey, aggregator_signing_root, &aggregate.signature(), ) { diff --git a/crates/blockchain/state_transition/src/beacon/gossip/attestation.rs b/crates/blockchain/state_transition/src/beacon/gossip/attestation.rs index 7ea4cb48c..711d2acac 100644 --- a/crates/blockchain/state_transition/src/beacon/gossip/attestation.rs +++ b/crates/blockchain/state_transition/src/beacon/gossip/attestation.rs @@ -190,7 +190,11 @@ pub fn stateful_checks(store: &Store, attestation: &SingleAttestation, subnet_id }; let domain = get_domain_from_schedule(&config, &state, DOMAIN_BEACON_ATTESTER, target_epoch); let signing_root = compute_signing_root(data.hash_tree_root(), domain); - if !bls::verify(&attester.pubkey, signing_root, &attestation.signature) { + let attester = bls::IndexedPubkey { + index: attestation.attester_index, + pubkey: attester.pubkey, + }; + if !bls::verify(&attester, signing_root, &attestation.signature) { return Outcome::Reject(RejectReason::BadSignature); } diff --git a/crates/blockchain/state_transition/src/beacon/gossip/column.rs b/crates/blockchain/state_transition/src/beacon/gossip/column.rs index 900d83353..e4f9cbd8e 100644 --- a/crates/blockchain/state_transition/src/beacon/gossip/column.rs +++ b/crates/blockchain/state_transition/src/beacon/gossip/column.rs @@ -585,8 +585,12 @@ fn header_signature_is_valid( state.genesis_validators_root(), ); let signing_root = compute_signing_root(header.hash_tree_root(), domain); + let proposer = bls::IndexedPubkey { + index: header.proposer_index, + pubkey: proposer.pubkey, + }; if !bls::verify( - &proposer.pubkey, + &proposer, signing_root, &sidecar.signed_block_header.signature, ) { diff --git a/crates/blockchain/state_transition/src/beacon/gossip/operations.rs b/crates/blockchain/state_transition/src/beacon/gossip/operations.rs index 2175911dc..fa096e666 100644 --- a/crates/blockchain/state_transition/src/beacon/gossip/operations.rs +++ b/crates/blockchain/state_transition/src/beacon/gossip/operations.rs @@ -248,7 +248,11 @@ fn voluntary_exit(state: &BeaconState, signed: &SignedVoluntaryExit, config: &Co state.genesis_validators_root(), ); let signing_root = compute_signing_root(exit.hash_tree_root(), domain); - if !bls::verify(&validator.pubkey, signing_root, &signed.signature) { + let validator = bls::IndexedPubkey { + index: exit.validator_index, + pubkey: validator.pubkey, + }; + if !bls::verify(&validator, signing_root, &signed.signature) { return Err(Outcome::Reject(RejectReason::BadSignature)); } Ok(()) @@ -264,6 +268,10 @@ fn proposer_slashing(state: &BeaconState, slashing: &ProposerSlashing) -> Verdic if !is_slashable_validator(proposer, get_current_epoch(state)) { return Err(Outcome::Reject(RejectReason::InvalidOperation)); } + let proposer = bls::IndexedPubkey { + index: proposer_index, + pubkey: proposer.pubkey, + }; // [REJECT] Both signatures are valid. for signed_header in [&slashing.signed_header_1, &slashing.signed_header_2] { let header = &signed_header.message; @@ -273,7 +281,7 @@ fn proposer_slashing(state: &BeaconState, slashing: &ProposerSlashing) -> Verdic Some(compute_epoch_at_slot(header.slot)), ); let signing_root = compute_signing_root(header.hash_tree_root(), domain); - if !bls::verify(&proposer.pubkey, signing_root, &signed_header.signature) { + if !bls::verify(&proposer, signing_root, &signed_header.signature) { return Err(Outcome::Reject(RejectReason::BadSignature)); } } diff --git a/crates/blockchain/state_transition/src/beacon/gossip/payload_attestation.rs b/crates/blockchain/state_transition/src/beacon/gossip/payload_attestation.rs index 1ba89616a..0b186b85c 100644 --- a/crates/blockchain/state_transition/src/beacon/gossip/payload_attestation.rs +++ b/crates/blockchain/state_transition/src/beacon/gossip/payload_attestation.rs @@ -132,7 +132,11 @@ pub fn stateful_checks(store: &Store, message: &gloas::PayloadAttestationMessage Some(compute_epoch_at_slot(data.slot)), ); let signing_root = compute_signing_root(data.hash_tree_root(), domain); - if !bls::verify(&validator.pubkey, signing_root, &message.signature) { + let validator = bls::IndexedPubkey { + index: validator_index, + pubkey: validator.pubkey, + }; + if !bls::verify(&validator, signing_root, &message.signature) { return Outcome::Reject(RejectReason::BadSignature); } Outcome::Accept diff --git a/crates/blockchain/state_transition/src/beacon/gossip/proposer_preferences.rs b/crates/blockchain/state_transition/src/beacon/gossip/proposer_preferences.rs index f278ea62e..3976e4d04 100644 --- a/crates/blockchain/state_transition/src/beacon/gossip/proposer_preferences.rs +++ b/crates/blockchain/state_transition/src/beacon/gossip/proposer_preferences.rs @@ -119,10 +119,13 @@ fn stateful_rules(store: &Store, signed: &gloas::SignedProposerPreferences) -> R return Err(Outcome::Reject(RejectReason::WrongProposer)); } // [REJECT] The signature is valid, under any candidate domain. - let pubkey = state - .validator(preferences.validator_index) - .map_err(|_| Outcome::Reject(RejectReason::WrongProposer))? - .pubkey; + let pubkey = bls::IndexedPubkey { + index: preferences.validator_index, + pubkey: state + .validator(preferences.validator_index) + .map_err(|_| Outcome::Reject(RejectReason::WrongProposer))? + .pubkey, + }; let message_root = preferences.hash_tree_root(); let valid = proposer_preferences_domains(&state, &config, proposal_epoch) .into_iter() diff --git a/crates/blockchain/state_transition/src/beacon/gossip/sync_committee.rs b/crates/blockchain/state_transition/src/beacon/gossip/sync_committee.rs index 0481c768a..919341bb4 100644 --- a/crates/blockchain/state_transition/src/beacon/gossip/sync_committee.rs +++ b/crates/blockchain/state_transition/src/beacon/gossip/sync_committee.rs @@ -245,7 +245,11 @@ pub fn check_message( message.slot, message.beacon_block_root, ); - if !bls::verify(&validator.pubkey, signing_root, &message.signature) { + let validator = bls::IndexedPubkey { + index: message.validator_index, + pubkey: validator.pubkey, + }; + if !bls::verify(&validator, signing_root, &message.signature) { return Err(Outcome::Reject(RejectReason::BadSignature)); } Ok(seats) @@ -353,12 +357,16 @@ pub fn check_contribution( contribution.slot, contribution.subcommittee_index, ); - if !bls::verify(&aggregator.pubkey, selection_root, &message.selection_proof) { + let aggregator_pubkey = bls::IndexedPubkey { + index: message.aggregator_index, + pubkey: aggregator.pubkey, + }; + if !bls::verify(&aggregator_pubkey, selection_root, &message.selection_proof) { return Outcome::Reject(RejectReason::SelectionProof); } // [REJECT] The aggregator's signature over the envelope is valid. let envelope_root = contribution_and_proof_signing_root(config, gvr, message); - if !bls::verify(&aggregator.pubkey, envelope_root, &signed.signature) { + if !bls::verify(&aggregator_pubkey, envelope_root, &signed.signature) { return Outcome::Reject(RejectReason::AggregatorSignature); } // [REJECT] The aggregate signature is valid over the participants. diff --git a/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs b/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs index ef4f2aec4..18d8a66ed 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/accessors.rs @@ -17,7 +17,7 @@ pub use ethlambda_storage::{ ActiveBalanceCache, ActiveBalanceKey, CommitteeCache, Lookup, ShufflingKey, }; -use crate::beacon::bls::{AggregatePubkey, CommitteeKeySums}; +use crate::beacon::bls::{AggregatePubkey, CommitteeKeySums, IndexedPubkey}; use crate::beacon::config::Config; use crate::beacon::constants; use crate::beacon::containers::BeaconState; @@ -416,9 +416,14 @@ fn build_committee_key_sums( let sums = members .par_iter() .map(|committee| { - let keys: Option> = committee + // The index is attached per committee rather than stored in the + // flat list, which spans the whole registry. + let keys: Option> = committee .iter() - .map(|index| pubkeys.get(*index as usize).copied()) + .map(|&index| { + let pubkey = *pubkeys.get(index as usize)?; + Some(IndexedPubkey { index, pubkey }) + }) .collect(); AggregatePubkey::sum(&keys?) }) diff --git a/crates/blockchain/state_transition/src/beacon/helpers/attestation.rs b/crates/blockchain/state_transition/src/beacon/helpers/attestation.rs index d52da7017..62c4c5746 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/attestation.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/attestation.rs @@ -9,7 +9,7 @@ //! the aggregation bits from one committee to a whole slot's worth and adding a //! committee bitfield, so it gets its own versions rather than reusing these. -use crate::beacon::bls::SignatureSet; +use crate::beacon::bls::{IndexedPubkey, SignatureSet}; use crate::beacon::constants; use crate::beacon::containers::BeaconState; use crate::beacon::containers::phase0::{Attestation, AttestingIndices, IndexedAttestation}; @@ -132,7 +132,10 @@ pub fn indexed_attestation_signature_set_with_domain( let mut pubkeys = Vec::with_capacity(indices.len()); for index in indices { match state.validator(*index) { - Ok(validator) => pubkeys.push(validator.pubkey), + Ok(validator) => pubkeys.push(IndexedPubkey { + index: *index, + pubkey: validator.pubkey, + }), Err(_) => return None, } } diff --git a/crates/blockchain/state_transition/src/beacon/helpers/electra.rs b/crates/blockchain/state_transition/src/beacon/helpers/electra.rs index 471d538d6..612cdd61a 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::{AggregatePubkey, SignatureSet}; +use crate::beacon::bls::{AggregatePubkey, IndexedPubkey, SignatureSet}; use crate::beacon::config::Config; use crate::beacon::constants::{self, FAR_FUTURE_EPOCH}; use crate::beacon::containers::shared::{AttestationData, Validator}; @@ -331,7 +331,10 @@ pub fn indexed_attestation_signature_set_with_domain( let mut pubkeys = Vec::with_capacity(indices.len()); for index in indices { match state.validator(*index) { - Ok(validator) => pubkeys.push(validator.pubkey), + Ok(validator) => pubkeys.push(IndexedPubkey { + index: *index, + pubkey: validator.pubkey, + }), Err(_) => return None, } } @@ -577,7 +580,12 @@ pub(crate) fn attesters_aggregate_pubkey( 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 pubkey_of = |index: ValidatorIndex| { + state.validator(index).ok().map(|v| IndexedPubkey { + index, + pubkey: v.pubkey, + }) + }; // Committees derived from their sums, added as they come, and the keys of // every committee summed directly, summed once at the end. diff --git a/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs b/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs index f3c15394d..9cc5fcd9f 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs @@ -86,7 +86,7 @@ //! doc), read through [`super::fulu::get_beacon_proposer_index`] rather //! than a copy of it here. -use crate::beacon::bls::SignatureSet; +use crate::beacon::bls::{IndexedPubkey, SignatureSet}; use crate::beacon::config::Config; use crate::beacon::constants; use crate::beacon::containers::shared::AttestationData; @@ -178,7 +178,10 @@ pub fn indexed_attestation_signature_set_with_domain( let mut pubkeys = Vec::with_capacity(indices.len()); for index in indices { match state.validator(*index) { - Ok(validator) => pubkeys.push(validator.pubkey), + Ok(validator) => pubkeys.push(IndexedPubkey { + index: *index, + pubkey: validator.pubkey, + }), Err(_) => return None, } } @@ -353,7 +356,10 @@ pub fn indexed_payload_attestation_signature_set( let mut pubkeys = Vec::with_capacity(indices.len()); for index in indices { match state.validator(*index) { - Ok(validator) => pubkeys.push(validator.pubkey), + Ok(validator) => pubkeys.push(IndexedPubkey { + index: *index, + pubkey: validator.pubkey, + }), Err(_) => return None, } } diff --git a/crates/blockchain/state_transition/src/beacon/precheck.rs b/crates/blockchain/state_transition/src/beacon/precheck.rs index c34511a17..5ab99dada 100644 --- a/crates/blockchain/state_transition/src/beacon/precheck.rs +++ b/crates/blockchain/state_transition/src/beacon/precheck.rs @@ -111,7 +111,10 @@ pub fn precheck_block( }; let pubkey = match (state.validator(proposer), reference) { - (Ok(validator), _) => &validator.pubkey, + (Ok(validator), _) => bls::IndexedPubkey { + index: proposer, + pubkey: validator.pubkey, + }, (Err(_), Reference::Parent(_)) => { return Err(PrecheckError::UnknownProposer { proposer }); } @@ -125,7 +128,7 @@ pub fn precheck_block( state.genesis_validators_root(), ); let signing_root = compute_signing_root(block_root, domain); - if !bls::verify(pubkey, signing_root, &signed_block.signature()) { + if !bls::verify(&pubkey, signing_root, &signed_block.signature()) { return Err(PrecheckError::BadSignature); } Ok(()) diff --git a/crates/blockchain/state_transition/src/beacon/stf/block.rs b/crates/blockchain/state_transition/src/beacon/stf/block.rs index af6057f3f..c86c84885 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/block.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/block.rs @@ -17,7 +17,7 @@ //! every driver, present and future, call the identical function regardless of //! what else that fork's body carries. -use crate::beacon::bls::SignatureSet; +use crate::beacon::bls::{IndexedPubkey, SignatureSet}; use crate::beacon::config::Config; use crate::beacon::constants; use crate::beacon::containers::{self, BeaconBlockHeader, BeaconState, Eth1Data, altair, phase0}; @@ -352,7 +352,14 @@ pub fn process_randao( let proposer = state.validator(proposer_index)?; if verification == RandaoVerification::Verify { signatures.push( - SignatureSet::single(&proposer.pubkey, signing_root, randao_reveal), + SignatureSet::single( + &IndexedPubkey { + index: proposer_index, + pubkey: proposer.pubkey, + }, + signing_root, + randao_reveal, + ), "RANDAO reveal signature", )?; } diff --git a/crates/blockchain/state_transition/src/beacon/stf/deneb.rs b/crates/blockchain/state_transition/src/beacon/stf/deneb.rs index 920f5d3d0..08fab3cf6 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/deneb.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/deneb.rs @@ -25,7 +25,7 @@ //! caches two new blob-gas fields for EIP-4844, and [`process_voluntary_exit`] //! signs under a fixed fork version for EIP-7044. -use crate::beacon::bls::SignatureSet; +use crate::beacon::bls::{IndexedPubkey, SignatureSet}; use crate::beacon::config::Config; use crate::beacon::constants::{self, FAR_FUTURE_EPOCH}; use crate::beacon::containers::capella::SignedBLSToExecutionChange; @@ -712,7 +712,10 @@ pub fn process_voluntary_exit( let signing_root = compute_signing_root(voluntary_exit.hash_tree_root(), domain); signatures.push( SignatureSet::single( - &validator.pubkey, + &IndexedPubkey { + index: voluntary_exit.validator_index, + pubkey: validator.pubkey, + }, signing_root, &signed_voluntary_exit.signature, ), diff --git a/crates/blockchain/state_transition/src/beacon/stf/electra.rs b/crates/blockchain/state_transition/src/beacon/stf/electra.rs index c0e138775..a58f7b7c8 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/electra.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/electra.rs @@ -44,7 +44,7 @@ use libssz::SszEncode as _; -use crate::beacon::bls::{self, SignatureSet}; +use crate::beacon::bls::{self, IndexedPubkey, SignatureSet}; use crate::beacon::config::Config; use crate::beacon::constants::{self, FAR_FUTURE_EPOCH}; use crate::beacon::containers::shared::{ @@ -532,7 +532,10 @@ pub fn process_voluntary_exit( let signing_root = compute_signing_root(voluntary_exit.hash_tree_root(), domain); signatures.push( SignatureSet::single( - &validator.pubkey, + &IndexedPubkey { + index: voluntary_exit.validator_index, + pubkey: validator.pubkey, + }, signing_root, &signed_voluntary_exit.signature, ), diff --git a/crates/blockchain/state_transition/src/beacon/stf/mod.rs b/crates/blockchain/state_transition/src/beacon/stf/mod.rs index 890c670bd..dbcbc2447 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/mod.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/mod.rs @@ -229,10 +229,15 @@ pub fn block_signature_set( state: &BeaconState, signed_block: &containers::SignedBeaconBlock, ) -> Option { - let proposer = state.validator(signed_block.proposer_index()).ok()?; + let index = signed_block.proposer_index(); + let proposer = state.validator(index).ok()?; let domain = get_domain(state, constants::DOMAIN_BEACON_PROPOSER, None); let signing_root = compute_signing_root(signed_block.message_hash_tree_root(), domain); - bls::SignatureSet::single(&proposer.pubkey, signing_root, &signed_block.signature()) + let proposer = bls::IndexedPubkey { + index, + pubkey: proposer.pubkey, + }; + bls::SignatureSet::single(&proposer, signing_root, &signed_block.signature()) } /// Advances the state to `slot`, running epoch processing at each boundary and diff --git a/crates/blockchain/state_transition/src/beacon/stf/operations.rs b/crates/blockchain/state_transition/src/beacon/stf/operations.rs index 86d15dcb8..be221518a 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/operations.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/operations.rs @@ -12,7 +12,7 @@ use std::collections::HashSet; -use crate::beacon::bls::{self, SignatureSet}; +use crate::beacon::bls::{self, IndexedPubkey, SignatureSet}; use crate::beacon::config::Config; use crate::beacon::constants::{self, FAR_FUTURE_EPOCH}; use crate::beacon::containers::BeaconState; @@ -208,7 +208,10 @@ pub(crate) fn verify_proposer_slashing( // Copied out rather than re-borrowed per iteration below: both signatures // are checked against the same proposer, and holding the borrow across // the loop would conflict with `get_domain`'s borrow of `state`. - let pubkey = proposer.pubkey; + let pubkey = IndexedPubkey { + index: proposer_index, + pubkey: proposer.pubkey, + }; for signed_header in [ &proposer_slashing.signed_header_1, @@ -673,7 +676,10 @@ pub fn process_voluntary_exit( let signing_root = compute_signing_root(voluntary_exit.hash_tree_root(), domain); signatures.push( SignatureSet::single( - &validator.pubkey, + &IndexedPubkey { + index: voluntary_exit.validator_index, + pubkey: validator.pubkey, + }, signing_root, &signed_voluntary_exit.signature, ), diff --git a/crates/common/crypto/src/bls.rs b/crates/common/crypto/src/bls.rs index 7e1ea6ba1..ae57ef6d0 100644 --- a/crates/common/crypto/src/bls.rs +++ b/crates/common/crypto/src/bls.rs @@ -34,17 +34,46 @@ //! key validation, and because every active validator's key signs again each //! epoch, almost all of it was repeated work. //! -//! So [`PublicKey::key_validate`]'s answer is memoized, keyed by the exact -//! compressed bytes it was asked about (see [`PubkeyCache`]). That keeps the -//! guarantee above intact rather than trading it away: `key_validate` is a -//! pure function of those bytes, so a hit hands back precisely the point a -//! fresh call would have produced, and a byte string only ever enters the -//! cache by passing it. Keying by the bytes rather than by validator index is -//! what makes that true without further argument: an index names whatever key -//! the state at hand says it does, which two forks need not agree on, while a -//! byte string names one point everywhere. A key that fails is never cached, -//! so an invalid key costs a full check every time, exactly as before, and -//! cannot grow the cache. +//! So [`PublicKey::key_validate`]'s answer is memoized in [`PubkeyCache`], +//! which holds it in two structures. Both are keyed, in the end, by the exact +//! compressed bytes the answer was asked about: +//! +//! - a byte-keyed map, sharded across locks, for a key with no index to its +//! name (a gossiped aggregate's, a deposit's, a sync committee's); +//! - an index-keyed table for a key a caller read from the registry, given as +//! an [`IndexedPubkey`]: one set-once slot per validator index, read without +//! a lock or a hash, holding the bytes next to the point. +//! +//! That keeps the guarantee above intact rather than trading it away: +//! `key_validate` is a pure function of those bytes, so a hit hands back +//! precisely the point a fresh call would have produced, and a byte string only +//! ever enters either structure by passing it. The table does not weaken that +//! by naming slots by index. An index names whatever key the state at hand says +//! it does, which two forks (or two spec-fixture states in one process) need +//! not agree on, so a slot is never trusted for its index alone: a lookup hits +//! only when the bytes stored in the slot equal the bytes the caller just read +//! from its own state, and then the stored point is the point for those bytes. +//! A slot holding other bytes, or an index past the table's bound, sends the +//! lookup to the byte-keyed map, which needs no argument about indices at all. +//! A slot is written once and never replaced, so a reader never sees a torn or +//! changing entry; a key that lost the race for its slot goes to the map. +//! A key first validated through the byte path and later looked up by index is +//! copied into its slot, and stays in the map as well: byte-path callers cannot +//! see the table, so dropping the map entry would make them validate it again. +//! A key reached through both paths is therefore held twice. That is why every +//! caller that read a key from the registry by index passes the index, gossip +//! validation included: the byte path is left to keys with no registry index +//! (deposits, BLS-to-execution changes, builders) and to the sync committee's +//! own key list, so the keys held twice stay few. +//! +//! The table's memory follows the span of indices it serves, not the number of +//! keys in it: a chunk is allocated whole on its first key, and a registry's +//! exited validators are spread through every chunk, so the table costs one +//! slot per registry index, filled or not. A slot is smaller than a map entry, +//! so on a registry where about half the validators have exited the table costs +//! somewhat more than a map of the active keys alone would. +//! A key that fails is never cached in either structure, so an invalid key +//! costs a full check every time, exactly as before, and cannot grow either. //! //! # `bool` versus `Result` //! @@ -94,7 +123,7 @@ //! validated. use std::collections::HashMap; -use std::sync::{LazyLock, RwLock}; +use std::sync::{LazyLock, OnceLock, RwLock}; use blst::min_pk::{AggregatePublicKey, AggregateSignature, PublicKey, Signature}; use blst::{BLST_ERROR, MultiPoint}; @@ -102,7 +131,7 @@ use rayon::prelude::*; use ethlambda_types::beacon::error::{Error, Result}; use ethlambda_types::beacon::primitives::{ - BLS_PUBKEY_SIZE, BLS_SIGNATURE_SIZE, BlsPubkey, BlsSignature, Root, + BLS_PUBKEY_SIZE, BLS_SIGNATURE_SIZE, BlsPubkey, BlsSignature, Root, ValidatorIndex, }; mod batch; @@ -143,8 +172,9 @@ const KEY_VALIDATE_MIN_PAR_LEN: usize = 16; /// those reads on the same cache line; a shard per lock spreads them out. const PUBKEY_CACHE_SHARDS: usize = 64; -/// The most validated public keys [`VALIDATED_PUBKEYS`] holds, across all of -/// its shards. +/// The most validated public keys each of [`VALIDATED_PUBKEYS`]'s two +/// structures holds: the map across all of its shards, and the index table, +/// whose bound on indices it serves is this same number. /// /// Far above any real validator registry: the bound is not there to evict /// anything a registry needs, but so that inputs which are not a registry @@ -165,17 +195,104 @@ const MAX_CACHED_PUBKEYS: usize = 1 << 22; static VALIDATED_PUBKEYS: LazyLock = LazyLock::new(|| PubkeyCache::new(MAX_CACHED_PUBKEYS)); -/// Validated, decompressed public keys, keyed by their compressed encoding. +/// How many validator indices one lazily allocated chunk of the index table +/// covers. +/// +/// Registry indices are dense, so a chunk fills as the registry is walked, and +/// a chunk is the unit of allocation: small enough that a test or a small +/// devnet pays for a few of them, large enough that a mainnet registry is a few +/// hundred. +const INDEX_CHUNK_LEN: usize = 1 << 12; + +/// A validated key as the index table stores it: the bytes it was validated +/// from, next to the point, so a lookup can check the bytes before trusting +/// the point. +type IndexedEntry = (BlsPubkey, PublicKey); + +/// One lazily allocated run of [`INDEX_CHUNK_LEN`] set-once slots. +type IndexChunk = Box<[OnceLock]>; + +/// What a key-taking function accepts: a bare [`BlsPubkey`], resolved through +/// the byte-keyed map, or an [`IndexedPubkey`], resolved through the index +/// table first. +/// +/// Sealed, because the cache's soundness argument (see the module +/// documentation) is about exactly these two shapes. Both carry the bytes the +/// caller read; the index is only a hint about where to look for them. +pub trait PubkeySource: sealed::Sealed {} + +impl PubkeySource for BlsPubkey {} +impl PubkeySource for IndexedPubkey {} + +mod sealed { + use super::*; + + /// The accessors the cache needs; kept out of [`PubkeySource`] so that + /// nothing outside this crate can implement it. + pub trait Sealed: Sync { + /// The compressed bytes the caller read from its state. + fn pubkey(&self) -> &BlsPubkey; + /// The registry index to try first, if the caller knows one. + fn index(&self) -> Option; + } + + impl Sealed for BlsPubkey { + fn pubkey(&self) -> &BlsPubkey { + self + } + fn index(&self) -> Option { + None + } + } + + impl Sealed for IndexedPubkey { + fn pubkey(&self) -> &BlsPubkey { + &self.pubkey + } + fn index(&self) -> Option { + Some(self.index) + } + } +} + +/// A public key together with the validator index the caller read it at. +/// +/// The index is a hint that lets the cache answer from its index-keyed table +/// instead of hashing the bytes. It is never trusted on its own: the table +/// hits only when the bytes held for that index equal `pubkey`, so a caller +/// whose state names a different key at that index than another state did +/// still gets the point for the bytes it passed. See the module documentation. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct IndexedPubkey { + /// The validator's index in the registry the key was read from. + pub index: ValidatorIndex, + /// The key's compressed bytes, as that registry holds them. + pub pubkey: BlsPubkey, +} + +/// Validated, decompressed public keys, by their compressed encoding or by the +/// registry index they were read at. /// /// A bounded, append-only memo of [`PublicKey::key_validate`]: an entry is /// only ever added for a key that passed, never changed, and never removed, /// since its answer can never change. See the module documentation for why /// that is sound. +/// +/// The index table holds a key resolved through an [`IndexedPubkey`] and the +/// map holds every other. A key reached through both paths is held in both, +/// since byte-path callers cannot see the table, which is why registry callers +/// pass the index. The table's memory is its allocated chunks', not its +/// entries'; see the module documentation. struct PubkeyCache { shards: [RwLock>; PUBKEY_CACHE_SHARDS], /// The most entries one shard takes, so the whole cache stays within the /// capacity it was built with. shard_capacity: usize, + /// Set-once slots by validator index, in chunks allocated on first write. + /// Its length times [`INDEX_CHUNK_LEN`] is the bound on indices it serves. + chunks: Box<[OnceLock]>, + /// The first index the table does not serve. + index_bound: usize, } impl PubkeyCache { @@ -183,6 +300,10 @@ impl PubkeyCache { Self { shards: std::array::from_fn(|_| RwLock::new(HashMap::new())), shard_capacity: capacity.div_ceil(PUBKEY_CACHE_SHARDS), + chunks: (0..capacity.div_ceil(INDEX_CHUNK_LEN)) + .map(|_| OnceLock::new()) + .collect(), + index_bound: capacity, } } @@ -194,40 +315,122 @@ impl PubkeyCache { &self.shards[byte % PUBKEY_CACHE_SHARDS] } - /// The validated point for `pubkey`, if it has passed before. - fn get(&self, pubkey: &BlsPubkey) -> Option { + /// The chunk cell covering `index` and the slot's offset in it, if the + /// index is inside the bound. + fn chunk_cell(&self, index: ValidatorIndex) -> Option<(&OnceLock, usize)> { + let index = usize::try_from(index) + .ok() + .filter(|&i| i < self.index_bound)?; + Some(( + &self.chunks[index / INDEX_CHUNK_LEN], + index % INDEX_CHUNK_LEN, + )) + } + + /// The table slot for `index`, if the index is inside the bound and its + /// chunk exists. A lookup never allocates. + fn slot(&self, index: ValidatorIndex) -> Option<&OnceLock> { + let (cell, offset) = self.chunk_cell(index)?; + Some(&cell.get()?[offset]) + } + + /// The table slot for `index`, allocating its chunk if this is the + /// chunk's first key, if the index is inside the bound. + fn slot_or_allocate(&self, index: ValidatorIndex) -> Option<&OnceLock> { + let (cell, offset) = self.chunk_cell(index)?; + let chunk = cell.get_or_init(|| { + crate::metrics::inc_pubkey_cache_index_chunks(); + (0..INDEX_CHUNK_LEN).map(|_| OnceLock::new()).collect() + }); + Some(&chunk[offset]) + } + + fn get_by_bytes(&self, pubkey: &BlsPubkey) -> Option { self.shard(pubkey).read().unwrap().get(pubkey).copied() } - /// Records that `pubkey` validated to `point`, unless its shard is full. - fn insert(&self, pubkey: BlsPubkey, point: PublicKey) { + /// The validated point for `key`, if its bytes have passed before. + /// + /// The index table answers only when the slot's stored bytes equal the + /// bytes in `key`; anything else, including an index outside the bound, + /// goes to the byte-keyed map. + fn get(&self, key: &K) -> Option { + let pubkey = key.pubkey(); + if let Some(index) = key.index() + && let Some((bytes, point)) = self.slot(index).and_then(OnceLock::get) + && bytes == pubkey + { + return Some(*point); + } + let point = self.get_by_bytes(pubkey)?; + // A key first seen through the byte path (gossip, sync committee + // messages) is copied into its empty slot, so later lookups by index + // hit the table. The map keeps its entry: byte-path callers cannot see + // the table and would have to validate again. Set-once, first writer + // wins; a slot holding other bytes is left alone. + if let Some(index) = key.index() + && let Some(slot) = self.slot_or_allocate(index) + && slot.set((*pubkey, point)).is_ok() + { + crate::metrics::inc_pubkey_cache_table_entries(); + } + Some(point) + } + + /// Records that `key`'s bytes validated to `point`: in the index table + /// when `key` has an index whose slot is free (or already holds these + /// bytes), otherwise in the map, unless that shard is full. + fn insert(&self, key: &K, point: PublicKey) { + let pubkey = *key.pubkey(); + if let Some(index) = key.index() + && let Some(slot) = self.slot_or_allocate(index) + { + match slot.set((pubkey, point)) { + Ok(()) => { + crate::metrics::inc_pubkey_cache_table_entries(); + return; + } + // First writer wins; a racing writer of the same bytes is done. + Err(_) if slot.get().is_some_and(|(bytes, _)| *bytes == pubkey) => return, + // The slot holds other bytes: this key lives in the map. + Err(_) => {} + } + } let mut shard = self.shard(&pubkey).write().unwrap(); if shard.len() < self.shard_capacity && shard.insert(pubkey, point).is_none() { - crate::metrics::inc_pubkey_cache_entries(); + crate::metrics::inc_pubkey_cache_map_entries(); } } - /// The number of keys held, across every shard. + /// The number of keys held, across the map and the table. #[cfg(test)] fn len(&self) -> usize { - self.shards + let mapped: usize = self + .shards .iter() .map(|shard| shard.read().unwrap().len()) - .sum() + .sum(); + let indexed: usize = self + .chunks + .iter() + .filter_map(|chunk| chunk.get()) + .map(|chunk| chunk.iter().filter(|slot| slot.get().is_some()).count()) + .sum(); + mapped + indexed } } /// `pubkey` as a validated point, or `None` if it is not a valid, /// subgroup-correct, non-identity key: [`PublicKey::key_validate`], through /// [`VALIDATED_PUBKEYS`]. -fn validated_pubkey(pubkey: &BlsPubkey) -> Option { +fn validated_pubkey(pubkey: &K) -> Option { if let Some(point) = VALIDATED_PUBKEYS.get(pubkey) { crate::metrics::inc_pubkey_cache_lookups(1, 0); return Some(point); } crate::metrics::inc_pubkey_cache_lookups(0, 1); - let point = PublicKey::key_validate(pubkey.as_ref()).ok()?; - VALIDATED_PUBKEYS.insert(*pubkey, point); + let point = PublicKey::key_validate(pubkey.pubkey().as_ref()).ok()?; + VALIDATED_PUBKEYS.insert(pubkey, point); Some(point) } @@ -237,7 +440,7 @@ fn validated_pubkey(pubkey: &BlsPubkey) -> Option { /// Hits resolve on the calling thread and only the misses go to rayon; see /// the module documentation for why. Each miss that validates is added to /// [`VALIDATED_PUBKEYS`] before returning. -fn validated_pubkeys(pubkeys: &[BlsPubkey]) -> Option> { +fn validated_pubkeys(pubkeys: &[K]) -> Option> { let mut points: Vec> = pubkeys .iter() .map(|pubkey| VALIDATED_PUBKEYS.get(pubkey)) @@ -254,10 +457,10 @@ fn validated_pubkeys(pubkeys: &[BlsPubkey]) -> Option> { let fresh: Vec = misses .par_iter() .with_min_len(KEY_VALIDATE_MIN_PAR_LEN) - .map(|&index| PublicKey::key_validate(pubkeys[index].as_ref()).ok()) + .map(|&index| PublicKey::key_validate(pubkeys[index].pubkey().as_ref()).ok()) .collect::>()?; for (&index, point) in misses.iter().zip(fresh) { - VALIDATED_PUBKEYS.insert(pubkeys[index], point); + VALIDATED_PUBKEYS.insert(&pubkeys[index], point); points[index] = Some(point); } } @@ -336,7 +539,7 @@ pub const G2_POINT_AT_INFINITY: [u8; BLS_SIGNATURE_SIZE] = g2_point_at_infinity( /// here rather than panicking or returning an error: an unparseable or /// invalid-point key is treated exactly like a valid key over the wrong /// signature, since the specification never distinguishes the two. -pub fn verify(pubkey: &BlsPubkey, message: Root, signature: &BlsSignature) -> bool { +pub fn verify(pubkey: &K, message: Root, signature: &BlsSignature) -> bool { let Some(pubkey) = validated_pubkey(pubkey) else { return false; }; @@ -410,8 +613,8 @@ pub fn aggregate_verify( /// Electra block's attestations, where a single aggregate can carry /// thousands of signers behind one shared message; keys go through /// [`PubkeyCache`], see the module documentation for why that is safe. -pub fn fast_aggregate_verify( - pubkeys: &[BlsPubkey], +pub fn fast_aggregate_verify( + pubkeys: &[K], message: Root, signature: &BlsSignature, ) -> bool { @@ -443,7 +646,7 @@ pub fn fast_aggregate_verify( /// contribute nothing (or something unintended) to the sum instead of failing /// the aggregation outright, which is why this returns [`Result`] rather /// than substituting a default. -pub fn eth_aggregate_pubkeys(pubkeys: &[BlsPubkey]) -> Result { +pub fn eth_aggregate_pubkeys(pubkeys: &[K]) -> Result { ethlambda_types::beacon::error::verify(!pubkeys.is_empty(), "len(pubkeys) > 0")?; let points = validated_pubkeys(pubkeys).ok_or(Error::SpecAssert( "all(bls.KeyValidate(pubkey) for pubkey in pubkeys)", @@ -466,8 +669,8 @@ pub fn eth_aggregate_pubkeys(pubkeys: &[BlsPubkey]) -> Result { /// the identity element by convention rather than "no signature was /// provided"; [`fast_aggregate_verify`] itself has no such case, since the /// underlying IETF ciphersuite it wraps was never given one. -pub fn eth_fast_aggregate_verify( - pubkeys: &[BlsPubkey], +pub fn eth_fast_aggregate_verify( + pubkeys: &[K], message: Root, signature: &BlsSignature, ) -> bool { @@ -484,7 +687,7 @@ pub fn eth_fast_aggregate_verify( /// Exposed standalone because the spec calls `KeyValidate` directly in more /// than one place (deposit processing, [`eth_aggregate_pubkeys`]'s own /// assertion), not only as a step inside a signature check. -pub fn key_validate(pubkey: &BlsPubkey) -> bool { +pub fn key_validate(pubkey: &K) -> bool { validated_pubkey(pubkey).is_some() } @@ -767,18 +970,228 @@ mod tests { .collect(); for pubkey in &pubkeys { let point = PublicKey::key_validate(pubkey.as_ref()).unwrap(); - cache.insert(*pubkey, point); + cache.insert(pubkey, point); // Inserting the same key again must not take a second slot. - cache.insert(*pubkey, point); + cache.insert(pubkey, point); } assert!(cache.len() <= PUBKEY_CACHE_SHARDS); let held = pubkeys .iter() - .filter(|pubkey| cache.get(pubkey).is_some()) + .filter(|pubkey| cache.get(*pubkey).is_some()) .count(); assert_eq!(held, cache.len()); } + /// The table slot for `index` in `cache`, if it holds anything. + fn held_at(cache: &PubkeyCache, index: ValidatorIndex) -> Option { + cache + .slot(index) + .and_then(OnceLock::get) + .map(|(bytes, _)| *bytes) + } + + fn indexed(index: ValidatorIndex, seed: u64) -> IndexedPubkey { + IndexedPubkey { + index, + pubkey: fresh_pubkey(seed), + } + } + + #[test] + fn an_indexed_key_hits_the_table_after_warming_and_stays_out_of_the_map() { + let cache = PubkeyCache::new(2 * INDEX_CHUNK_LEN); + let key = indexed(INDEX_CHUNK_LEN as u64 + 3, 9001); + assert!(cache.get(&key).is_none()); + + let point = PublicKey::key_validate(key.pubkey.as_ref()).unwrap(); + cache.insert(&key, point); + assert_eq!( + cache.get(&key).map(|point| point.to_bytes()), + Some(point.to_bytes()) + ); + assert_eq!(held_at(&cache, key.index), Some(key.pubkey)); + assert!(cache.get_by_bytes(&key.pubkey).is_none()); + assert_eq!(cache.len(), 1); + // The same bytes with no index are not in the byte map, and the + // answer through the global API is the same as a fresh validation. + assert_eq!( + validated_pubkey(&key).map(|point| point.to_bytes()), + uncached(&key.pubkey) + ); + } + + /// `fresh_pubkey(seed)`'s signature over `message`. + fn fresh_signature(seed: u64, message: Root) -> BlsSignature { + let mut ikm = [0xa5u8; 32]; + ikm[..8].copy_from_slice(&seed.to_le_bytes()); + let secret = blst::min_pk::SecretKey::key_gen(&ikm, &[]).unwrap(); + BlsSignature(secret.sign(message.as_slice(), DST, &[]).to_bytes()) + } + + #[test] + fn a_different_key_at_a_warmed_index_falls_back_and_verifies() { + let index = 3_000_001; + let message = ethlambda_types::beacon::primitives::H256([5u8; 32]); + let first = indexed(index, 9201); + let second = indexed(index, 9202); + assert!(key_validate(&first)); + assert_eq!(held_at(&VALIDATED_PUBKEYS, index), Some(first.pubkey)); + + // Another state names a different key at the same index: the slot's + // bytes differ, so it must not answer. + assert!(VALIDATED_PUBKEYS.get(&second).is_none()); + assert!(verify(&second, message, &fresh_signature(9202, message))); + assert!(!verify(&second, message, &fresh_signature(9201, message))); + assert!(verify(&first, message, &fresh_signature(9201, message))); + assert!(!verify(&first, message, &fresh_signature(9202, message))); + + // The slot kept its first writer; the loser lives in the byte map. + assert_eq!(held_at(&VALIDATED_PUBKEYS, index), Some(first.pubkey)); + assert!(VALIDATED_PUBKEYS.get_by_bytes(&second.pubkey).is_some()); + assert!(VALIDATED_PUBKEYS.get_by_bytes(&first.pubkey).is_none()); + + // An aggregate mixing the two states' keys verifies too. + let signature = aggregate(&[ + fresh_signature(9201, message), + fresh_signature(9202, message), + ]) + .unwrap(); + assert!(fast_aggregate_verify(&[first, second], message, &signature)); + } + + #[test] + fn a_cold_lookup_allocates_nothing() { + let cache = PubkeyCache::new(2 * INDEX_CHUNK_LEN); + let key = indexed(INDEX_CHUNK_LEN as u64 + 5, 9401); + assert!(cache.get(&key).is_none()); + assert!(cache.chunks.iter().all(|chunk| chunk.get().is_none())); + + // Only a key that validated allocates its chunk, and only that one. + let point = PublicKey::key_validate(key.pubkey.as_ref()).unwrap(); + cache.insert(&key, point); + assert!(cache.chunks[0].get().is_none()); + assert!(cache.chunks[1].get().is_some()); + } + + #[test] + fn a_byte_path_key_is_copied_into_the_table_on_its_first_index_lookup() { + let cache = PubkeyCache::new(INDEX_CHUNK_LEN); + let key = indexed(17, 9301); + let point = PublicKey::key_validate(key.pubkey.as_ref()).unwrap(); + cache.insert(&key.pubkey, point); + assert!(held_at(&cache, key.index).is_none()); + + assert_eq!( + cache.get(&key).map(|point| point.to_bytes()), + Some(point.to_bytes()) + ); + assert_eq!(held_at(&cache, key.index), Some(key.pubkey)); + // Held in both: byte-path callers still hit. + assert!(cache.get_by_bytes(&key.pubkey).is_some()); + assert_eq!(cache.len(), 2); + + // A slot holding other bytes is not overwritten by a promotion. + let other = indexed(17, 9302); + let other_point = PublicKey::key_validate(other.pubkey.as_ref()).unwrap(); + cache.insert(&other.pubkey, other_point); + assert!(cache.get(&other).is_some()); + assert_eq!(held_at(&cache, 17), Some(key.pubkey)); + } + + #[test] + fn an_index_past_the_bound_uses_the_map() { + for index in [MAX_CACHED_PUBKEYS as u64, u64::MAX] { + let key = indexed(index, 9100); + assert!(VALIDATED_PUBKEYS.slot_or_allocate(index).is_none()); + for _ in 0..2 { + assert_eq!( + validated_pubkey(&key).map(|point| point.to_bytes()), + uncached(&key.pubkey) + ); + } + assert!(VALIDATED_PUBKEYS.get_by_bytes(&key.pubkey).is_some()); + } + // The last index inside the bound is served by the table. + let last = MAX_CACHED_PUBKEYS as u64 - 1; + let key = indexed(last, 9101); + assert!(key_validate(&key)); + assert_eq!(held_at(&VALIDATED_PUBKEYS, last), Some(key.pubkey)); + } + + #[test] + fn an_invalid_indexed_key_is_cached_nowhere() { + let cache = PubkeyCache::new(INDEX_CHUNK_LEN); + for (offset, invalid) in [BlsPubkey::default(), identity_pubkey()] + .into_iter() + .enumerate() + { + let key = IndexedPubkey { + index: 3_100_000 + offset as u64, + pubkey: invalid, + }; + for _ in 0..2 { + assert!(!key_validate(&key)); + assert!(!verify( + &key, + ethlambda_types::beacon::primitives::H256([0u8; 32]), + &BlsSignature::default() + )); + assert!(eth_aggregate_pubkeys(&[key]).is_err()); + } + assert!(held_at(&VALIDATED_PUBKEYS, key.index).is_none()); + assert!(VALIDATED_PUBKEYS.get_by_bytes(&invalid).is_none()); + assert!(cache.get(&key).is_none()); + } + } + + #[test] + fn concurrent_readers_and_writers_agree_with_key_validate() { + const KEYS: u64 = 48; + const BASE: u64 = 3_200_000; + let pubkeys: Vec = (0..KEYS).map(|seed| fresh_pubkey(20_000 + seed)).collect(); + let expected: Vec<_> = pubkeys.iter().map(uncached).collect(); + + std::thread::scope(|scope| { + for thread in 0..8u64 { + let pubkeys = &pubkeys; + let expected = &expected; + scope.spawn(move || { + for round in 0..20 { + for (position, pubkey) in pubkeys.iter().enumerate() { + // Even threads agree on one index per key; odd + // threads rotate, so every slot is contested by + // different bytes, and some pass no index at all. + let index = BASE + (position as u64 + thread * (thread % 2)) % KEYS; + let got = match (thread + round) % 3 { + 0 => validated_pubkey(pubkey), + _ => validated_pubkey(&IndexedPubkey { + index, + pubkey: *pubkey, + }), + }; + assert_eq!(got.map(|point| point.to_bytes()), expected[position]); + } + } + }); + } + }); + + // Whoever won each slot, it holds a key that validates to its own point. + for index in BASE..BASE + KEYS { + if let Some(bytes) = held_at(&VALIDATED_PUBKEYS, index) { + assert_eq!( + VALIDATED_PUBKEYS + .get(&IndexedPubkey { + index, + pubkey: bytes + }) + .map(|point| point.to_bytes()), + uncached(&bytes) + ); + } + } + } + /// Builds a realistic attestation aggregate of `count` independent /// signers over one shared message: each signer gets its own /// `key_gen`-derived keypair and signs [`message`](Root) under this diff --git a/crates/common/crypto/src/bls/batch.rs b/crates/common/crypto/src/bls/batch.rs index 39e97b20a..adda00f62 100644 --- a/crates/common/crypto/src/bls/batch.rs +++ b/crates/common/crypto/src/bls/batch.rs @@ -39,9 +39,9 @@ use blst::blst_scalar; use blst::min_pk::{PublicKey, Signature}; use rand::RngExt; -use ethlambda_types::beacon::primitives::{BlsPubkey, BlsSignature, Root}; +use ethlambda_types::beacon::primitives::{BlsSignature, Root}; -use super::{AggregatePubkey, DST, validated_pubkey, validated_pubkeys}; +use super::{AggregatePubkey, DST, PubkeySource, validated_pubkey, validated_pubkeys}; /// How many bits of each random weight the batch check uses. const RAND_BITS: usize = 64; @@ -73,7 +73,11 @@ impl SignatureSet { /// /// `None` when the key or the signature fails validation, which is exactly /// when [`super::verify`] returns false before reaching the pairing. - pub fn single(pubkey: &BlsPubkey, message: Root, signature: &BlsSignature) -> Option { + pub fn single( + pubkey: &K, + message: Root, + signature: &BlsSignature, + ) -> Option { let pubkey = validated_pubkey(pubkey)?; let signature = Signature::sig_validate(signature.as_ref(), false).ok()?; Some(Self { @@ -89,8 +93,8 @@ impl SignatureSet { /// `None` for an empty `pubkeys`, any key that fails `key_validate`, or a /// signature that fails `sig_validate`: the cases /// [`super::fast_aggregate_verify`] answers false for before the pairing. - pub fn fast_aggregate( - pubkeys: &[BlsPubkey], + pub fn fast_aggregate( + pubkeys: &[K], message: Root, signature: &BlsSignature, ) -> Option { @@ -222,6 +226,7 @@ mod tests { use super::*; use crate::bls; + use ethlambda_types::beacon::primitives::BlsPubkey; fn secret_key(seed: u8) -> SecretKey { let mut ikm = [seed; 32]; @@ -362,7 +367,7 @@ mod tests { assert!(SignatureSet::single(&garbage_key, msg, &signature).is_none()); assert!(SignatureSet::single(&pubkey, msg, &garbage_signature).is_none()); - assert!(SignatureSet::fast_aggregate(&[], msg, &signature).is_none()); + assert!(SignatureSet::fast_aggregate::(&[], msg, &signature).is_none()); assert!(SignatureSet::fast_aggregate(&[pubkey, garbage_key], msg, &signature).is_none()); assert!(SignatureSet::fast_aggregate(&[pubkey], msg, &garbage_signature).is_none()); // The identity signature is a valid encoding: it builds a set that fails. diff --git a/crates/common/crypto/src/bls/committee_sums.rs b/crates/common/crypto/src/bls/committee_sums.rs index a59000a66..7416c7347 100644 --- a/crates/common/crypto/src/bls/committee_sums.rs +++ b/crates/common/crypto/src/bls/committee_sums.rs @@ -22,9 +22,9 @@ use blst::min_pk::{AggregatePublicKey, PublicKey}; -use ethlambda_types::beacon::primitives::{BlsPubkey, BlsSignature, Root}; +use ethlambda_types::beacon::primitives::{BlsSignature, Root}; -use super::{SignatureSet, sum_validated_projective, validated_pubkeys}; +use super::{PubkeySource, SignatureSet, sum_validated_projective, validated_pubkeys}; /// A sum of validated public keys, kept as a curve point. Never serialized. /// @@ -39,7 +39,7 @@ 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 { + pub fn sum(pubkeys: &[K]) -> Option { let points = validated_pubkeys(pubkeys)?; sum_validated_projective(&points).map(Self) } @@ -50,7 +50,7 @@ impl AggregatePubkey { /// 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 { + pub fn minus(&self, pubkeys: &[K]) -> Option { if pubkeys.is_empty() { return Some(*self); } @@ -134,6 +134,7 @@ mod tests { use super::super::DST; use super::*; use blst::min_pk::SecretKey; + use ethlambda_types::beacon::primitives::BlsPubkey; fn secret(seed: u64) -> SecretKey { let mut ikm = [0x3cu8; 32]; @@ -178,7 +179,7 @@ mod tests { #[test] fn sum_of_nothing_or_of_an_invalid_key_is_none() { - assert!(AggregatePubkey::sum(&[]).is_none()); + assert!(AggregatePubkey::sum::(&[]).is_none()); let mut keys = pubkeys(3); keys.push(BlsPubkey::default()); assert!(AggregatePubkey::sum(&keys).is_none()); @@ -198,7 +199,10 @@ mod tests { 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()); + assert_eq!( + total.minus::(&[]).unwrap().to_bytes(), + total.to_bytes() + ); } /// The same over random masks of a committee-sized run, with the attesters diff --git a/crates/common/crypto/src/metrics.rs b/crates/common/crypto/src/metrics.rs index 43cf7c20a..997ef3fa2 100644 --- a/crates/common/crypto/src/metrics.rs +++ b/crates/common/crypto/src/metrics.rs @@ -21,10 +21,26 @@ static LEAN_BEACON_PUBKEY_CACHE_HITS: LazyLock = static LEAN_BEACON_PUBKEY_CACHE_MISSES: LazyLock = LazyLock::new(|| LEAN_BEACON_PUBKEY_CACHE_LOOKUPS_TOTAL.with_label_values(&["miss"])); -static LEAN_BEACON_PUBKEY_CACHE_ENTRIES: LazyLock = LazyLock::new(|| { - register_int_gauge!( +static LEAN_BEACON_PUBKEY_CACHE_ENTRIES: LazyLock = LazyLock::new(|| { + register_int_gauge_vec!( "lean_beacon_pubkey_cache_entries", - "Validated BLS public keys held by the process-wide pubkey cache" + "Validated BLS public keys held by the process-wide pubkey cache, by the structure \ + holding them (a key reached both by index and by bytes is held in both)", + &["structure"] + ) + .unwrap() +}); + +static LEAN_BEACON_PUBKEY_CACHE_MAP_ENTRIES: LazyLock = + LazyLock::new(|| LEAN_BEACON_PUBKEY_CACHE_ENTRIES.with_label_values(&["map"])); +static LEAN_BEACON_PUBKEY_CACHE_TABLE_ENTRIES: LazyLock = + LazyLock::new(|| LEAN_BEACON_PUBKEY_CACHE_ENTRIES.with_label_values(&["table"])); + +static LEAN_BEACON_PUBKEY_CACHE_INDEX_CHUNKS: LazyLock = LazyLock::new(|| { + register_int_gauge!( + "lean_beacon_pubkey_cache_index_chunks", + "Chunks of the pubkey cache's index table allocated so far; the table's memory follows \ + this, not its entry count" ) .unwrap() }); @@ -40,7 +56,19 @@ pub(crate) fn inc_pubkey_cache_lookups(hits: u64, misses: u64) { } } -/// Count one key added to the pubkey cache. The cache never removes one. -pub(crate) fn inc_pubkey_cache_entries() { - LEAN_BEACON_PUBKEY_CACHE_ENTRIES.inc(); +/// Count one key added to the pubkey cache's byte-keyed map. The cache never +/// removes one. +pub(crate) fn inc_pubkey_cache_map_entries() { + LEAN_BEACON_PUBKEY_CACHE_MAP_ENTRIES.inc(); +} + +/// Count one key added to the pubkey cache's index table. The cache never +/// removes one. +pub(crate) fn inc_pubkey_cache_table_entries() { + LEAN_BEACON_PUBKEY_CACHE_TABLE_ENTRIES.inc(); +} + +/// Count one chunk of the index table allocated. Chunks are never freed. +pub(crate) fn inc_pubkey_cache_index_chunks() { + LEAN_BEACON_PUBKEY_CACHE_INDEX_CHUNKS.inc(); } diff --git a/crates/net/rpc/src/beacon/gloas_proposal.rs b/crates/net/rpc/src/beacon/gloas_proposal.rs index 51b57d76e..919570a5a 100644 --- a/crates/net/rpc/src/beacon/gloas_proposal.rs +++ b/crates/net/rpc/src/beacon/gloas_proposal.rs @@ -720,10 +720,13 @@ fn verify_randao_reveal( let epoch = compute_epoch_at_slot(state.slot()); let domain = get_domain(state, constants::DOMAIN_RANDAO, Some(epoch)); let signing_root = compute_signing_root(epoch.hash_tree_root(), domain); - let pubkey = state - .validator(proposer) - .map_err(|_| ApiError::Internal("the proposer is not in the registry"))? - .pubkey; + let pubkey = bls::IndexedPubkey { + index: proposer, + pubkey: state + .validator(proposer) + .map_err(|_| ApiError::Internal("the proposer is not in the registry"))? + .pubkey, + }; if bls::verify(&pubkey, signing_root, &randao_reveal) { Ok(()) } else { diff --git a/crates/net/rpc/src/beacon/pool.rs b/crates/net/rpc/src/beacon/pool.rs index 7131b67f4..c2ae5bcd1 100644 --- a/crates/net/rpc/src/beacon/pool.rs +++ b/crates/net/rpc/src/beacon/pool.rs @@ -243,10 +243,13 @@ fn validate( // [REJECT] The signature is valid, under the attester domain at the target // epoch. The schedule's domain, not the head state's: the head may still be // the previous fork's while the target epoch's first slots are empty. - let pubkey = state - .validator(attestation.attester_index) - .map_err(|_| "attester index is unknown")? - .pubkey; + let pubkey = bls::IndexedPubkey { + index: attestation.attester_index, + pubkey: state + .validator(attestation.attester_index) + .map_err(|_| "attester index is unknown")? + .pubkey, + }; let domain = get_domain_from_schedule(&config, state, DOMAIN_BEACON_ATTESTER, data.target.epoch); let signing_root = compute_signing_root(data.hash_tree_root(), domain); diff --git a/docs/metrics.md b/docs/metrics.md index dcb660e78..59c047302 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -564,21 +564,25 @@ A `timer` start with no matching `head` start means the last slot was skipped. Every BLS signature check `ethlambda beacon` runs (block import, fork choice, gossip validation) resolves its signers' compressed public keys to validated -curve points through one process-wide cache, keyed by the compressed bytes. See -`PubkeyCache` in `crates/blockchain/state_transition/src/beacon/bls.rs`. This is +curve points through one process-wide cache: an index table for keys read from +the registry by validator index, and a map keyed by the compressed bytes for +every other key. See `PubkeyCache` in `crates/common/crypto/src/bls.rs`. This is ethlambda-specific, not part of the leanMetrics spec. | Name | Type | Usage | Sample collection event | Labels | |------|------|-------|-------------------------|--------| | `lean_beacon_pubkey_cache_lookups_total` | Counter | Public keys resolved for a signature check, by whether the cache already held them | Once per signature check, counting every signer | result=hit,miss | -| `lean_beacon_pubkey_cache_entries` | Gauge | Validated public keys the cache holds | On each key added; the cache never removes one | | +| `lean_beacon_pubkey_cache_entries` | Gauge | Validated public keys the cache holds, by structure: `table` for keys looked up by validator index, `map` for keys looked up by bytes. A key reached both ways is counted in both | On each key added; the cache never removes one | structure=map,table | +| `lean_beacon_pubkey_cache_index_chunks` | Gauge | Chunks of the index table allocated. The table's memory follows this rather than its entry count, since a chunk is allocated in full on its first key | On each chunk allocated; chunks are never freed | | **Misses should fall to near zero within an epoch of a start**, once every active validator has signed something the node checked. A steady miss rate after that means keys the cache is refusing to hold: either the entry bound is reached (the gauge stops rising), or the keys are invalid, which are never -cached and pay the full check every time. `entries` is also the cache's memory -footprint, roughly a decompressed point plus its compressed key per entry. +cached and pay the full check every time. `entries{structure="map"}` is the +map's memory footprint, roughly a decompressed point plus its compressed key per +entry. The table's is `index_chunks` times a chunk's size, since a chunk is +allocated whole and a registry's exited validators leave slots in it empty. ### Data Column Sidecars (Fulu DAS)