diff --git a/Cargo.lock b/Cargo.lock index 0bec6c9e0..1b58b2dc8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2057,6 +2057,7 @@ dependencies = [ "hex", "leanvm", "postcard", + "rand 0.10.1", "rayon", "thiserror 2.0.18", "tracing", diff --git a/crates/blockchain/state_transition/src/beacon/block_production.rs b/crates/blockchain/state_transition/src/beacon/block_production.rs index 9c3d0f8ce..cf8519a9b 100644 --- a/crates/blockchain/state_transition/src/beacon/block_production.rs +++ b/crates/blockchain/state_transition/src/beacon/block_production.rs @@ -47,6 +47,7 @@ use super::helpers::electra::{ use super::helpers::misc::compute_signing_root; use super::helpers::predicates::is_slashable_validator; use super::lean_boundary::lean_state_unreachable; +use super::stf::signatures::SignatureBatch; use super::stf::{self, ExecutionEngine}; use ethlambda_storage::pools::single_committee; @@ -442,7 +443,7 @@ fn pack_each( candidates: Vec, max: usize, config: &Config, - apply: impl Fn(&mut BeaconState, &T, &Config) -> Result<()>, + apply: impl Fn(&mut BeaconState, &T, &Config, &mut SignatureBatch) -> Result<()>, ) -> Vec { let mut packed = Vec::new(); // Bounds the work if the pool is full of stale entries. @@ -450,7 +451,11 @@ fn pack_each( if packed.len() == max { break; } - if apply(scratch, &candidate, config).is_ok() { + // An immediate batch, so a bad signature rejects the candidate before + // it writes, which is what lets a rejected candidate leave `scratch` + // untouched (see above); a deferred batch would only fail at the end. + let mut signatures = SignatureBatch::immediate(); + if apply(scratch, &candidate, config, &mut signatures).is_ok() { packed.push(candidate); } } diff --git a/crates/blockchain/state_transition/src/beacon/gloas_block_production.rs b/crates/blockchain/state_transition/src/beacon/gloas_block_production.rs index b4621d542..eeb0ee7c2 100644 --- a/crates/blockchain/state_transition/src/beacon/gloas_block_production.rs +++ b/crates/blockchain/state_transition/src/beacon/gloas_block_production.rs @@ -403,7 +403,7 @@ pub fn assemble_gloas_block_on_bid( body, }; let mut post = state.clone(); - stf::gloas::process_block( + stf::gloas::process_block_and_verify( &mut post, &block, config, @@ -529,7 +529,7 @@ pub fn assemble_gloas_block( let BeaconState::Gloas(_) = &post else { unreachable!("checked above") }; - stf::gloas::process_block( + stf::gloas::process_block_and_verify( &mut post, &block, config, @@ -745,7 +745,7 @@ pub mod test_support { /// The state after `block` is applied to `state`, as the chain stores it. pub fn post_state(state: &BeaconState, block: &BeaconBlock) -> BeaconState { let mut post = state.clone(); - stf::gloas::process_block( + stf::gloas::process_block_and_verify( &mut post, block, &config(), @@ -1042,7 +1042,7 @@ mod gloas_block_production_tests { // Importing it with the same option lands on the state root it names. let mut post = state.clone(); - stf::gloas::process_block( + stf::gloas::process_block_and_verify( &mut post, &produced.block, &config(), diff --git a/crates/blockchain/state_transition/src/beacon/helpers/attestation.rs b/crates/blockchain/state_transition/src/beacon/helpers/attestation.rs index b225195c9..d52da7017 100644 --- a/crates/blockchain/state_transition/src/beacon/helpers/attestation.rs +++ b/crates/blockchain/state_transition/src/beacon/helpers/attestation.rs @@ -9,6 +9,8 @@ //! 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::constants; use crate::beacon::containers::BeaconState; use crate::beacon::containers::phase0::{Attestation, AttestingIndices, IndexedAttestation}; use crate::beacon::error::Result; @@ -16,7 +18,6 @@ use crate::beacon::helpers::accessors::{CommitteeCache, CommitteeCacheExt, get_d use crate::beacon::helpers::misc::{compute_epoch_at_slot, compute_signing_root}; use crate::beacon::helpers::predicates::are_indices_sorted_and_unique; use crate::beacon::primitives::{Domain, HashTreeRoot as _, ValidatorIndex}; -use crate::beacon::{bls, constants}; /// The committee members whose bit is set in `attestation`, in ascending order. /// @@ -79,12 +80,27 @@ pub fn is_valid_indexed_attestation( state: &BeaconState, indexed_attestation: &IndexedAttestation, ) -> bool { + indexed_attestation_signature_set(state, indexed_attestation).is_some_and(|set| set.verify()) +} + +/// The signature check [`is_valid_indexed_attestation`] makes, resolved but +/// not yet run, so a block can verify every attestation's signature in one +/// batch. +/// +/// Every structural check [`is_valid_indexed_attestation`] makes (the +/// attester set's shape and range) happens here, at once: `None` means the +/// attestation is invalid without needing a pairing. Only the pairing is left +/// to the caller. +pub fn indexed_attestation_signature_set( + state: &BeaconState, + indexed_attestation: &IndexedAttestation, +) -> Option { let domain = get_domain( state, constants::DOMAIN_BEACON_ATTESTER, Some(indexed_attestation.data.target.epoch), ); - is_valid_indexed_attestation_with_domain(state, indexed_attestation, domain) + indexed_attestation_signature_set_with_domain(state, indexed_attestation, domain) } /// [`is_valid_indexed_attestation`] under a signing domain the caller chose. @@ -97,19 +113,30 @@ pub fn is_valid_indexed_attestation_with_domain( indexed_attestation: &IndexedAttestation, domain: Domain, ) -> bool { + indexed_attestation_signature_set_with_domain(state, indexed_attestation, domain) + .is_some_and(|set| set.verify()) +} + +/// [`indexed_attestation_signature_set`] under a signing domain the caller +/// chose. +pub fn indexed_attestation_signature_set_with_domain( + state: &BeaconState, + indexed_attestation: &IndexedAttestation, + domain: Domain, +) -> Option { let indices: &[ValidatorIndex] = &indexed_attestation.attesting_indices; if indices.is_empty() || !are_indices_sorted_and_unique(indices) { - return false; + return None; } let mut pubkeys = Vec::with_capacity(indices.len()); for index in indices { match state.validator(*index) { Ok(validator) => pubkeys.push(validator.pubkey), - Err(_) => return false, + Err(_) => return None, } } let signing_root = compute_signing_root(indexed_attestation.data.hash_tree_root(), domain); - bls::fast_aggregate_verify(&pubkeys, signing_root, &indexed_attestation.signature) + SignatureSet::fast_aggregate(&pubkeys, signing_root, &indexed_attestation.signature) } diff --git a/crates/blockchain/state_transition/src/beacon/helpers/electra.rs b/crates/blockchain/state_transition/src/beacon/helpers/electra.rs index 94fa7d20b..6e6daf9c2 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::SignatureSet; use crate::beacon::config::Config; use crate::beacon::constants::{self, FAR_FUTURE_EPOCH}; use crate::beacon::containers::shared::Validator; @@ -280,12 +280,27 @@ pub fn is_valid_indexed_attestation( state: &BeaconState, indexed_attestation: &electra::IndexedAttestation, ) -> bool { + indexed_attestation_signature_set(state, indexed_attestation).is_some_and(|set| set.verify()) +} + +/// The signature check [`is_valid_indexed_attestation`] makes, resolved but +/// not yet run, so a block can verify every attestation's signature in one +/// batch. +/// +/// Every structural check [`is_valid_indexed_attestation`] makes (the +/// attester set's shape and range) happens here, at once: `None` means the +/// attestation is invalid without needing a pairing. Only the pairing is left +/// to the caller. +pub fn indexed_attestation_signature_set( + state: &BeaconState, + indexed_attestation: &electra::IndexedAttestation, +) -> Option { let domain = get_domain( state, constants::DOMAIN_BEACON_ATTESTER, Some(indexed_attestation.data.target.epoch), ); - is_valid_indexed_attestation_with_domain(state, indexed_attestation, domain) + indexed_attestation_signature_set_with_domain(state, indexed_attestation, domain) } /// [`is_valid_indexed_attestation`] under a signing domain the caller chose. @@ -297,21 +312,32 @@ pub fn is_valid_indexed_attestation_with_domain( indexed_attestation: &electra::IndexedAttestation, domain: Domain, ) -> bool { + indexed_attestation_signature_set_with_domain(state, indexed_attestation, domain) + .is_some_and(|set| set.verify()) +} + +/// [`indexed_attestation_signature_set`] under a signing domain the caller +/// chose. +pub fn indexed_attestation_signature_set_with_domain( + state: &BeaconState, + indexed_attestation: &electra::IndexedAttestation, + domain: Domain, +) -> Option { let indices: &[ValidatorIndex] = &indexed_attestation.attesting_indices; if indices.is_empty() || !are_indices_sorted_and_unique(indices) { - return false; + return None; } let mut pubkeys = Vec::with_capacity(indices.len()); for index in indices { match state.validator(*index) { Ok(validator) => pubkeys.push(validator.pubkey), - Err(_) => return false, + Err(_) => return None, } } let signing_root = compute_signing_root(indexed_attestation.data.hash_tree_root(), domain); - bls::fast_aggregate_verify(&pubkeys, signing_root, &indexed_attestation.signature) + SignatureSet::fast_aggregate(&pubkeys, signing_root, &indexed_attestation.signature) } // --------------------------------------------------------------------------- diff --git a/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs b/crates/blockchain/state_transition/src/beacon/helpers/gloas.rs index 3bb3d0094..a95b66f06 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; +use crate::beacon::bls::SignatureSet; use crate::beacon::config::Config; use crate::beacon::constants; use crate::beacon::containers::shared::AttestationData; @@ -124,12 +124,27 @@ pub fn is_valid_indexed_attestation( state: &BeaconState, indexed_attestation: &gloas::IndexedAttestation, ) -> bool { + indexed_attestation_signature_set(state, indexed_attestation).is_some_and(|set| set.verify()) +} + +/// The signature check [`is_valid_indexed_attestation`] makes, resolved but +/// not yet run, so a block can verify every attestation's signature in one +/// batch. +/// +/// Every structural check [`is_valid_indexed_attestation`] makes (the +/// attester set's shape and range) happens here, at once: `None` means the +/// attestation is invalid without needing a pairing. Only the pairing is left +/// to the caller. +pub fn indexed_attestation_signature_set( + state: &BeaconState, + indexed_attestation: &gloas::IndexedAttestation, +) -> Option { let domain = get_domain( state, constants::DOMAIN_BEACON_ATTESTER, Some(indexed_attestation.data.target.epoch), ); - is_valid_indexed_attestation_with_domain(state, indexed_attestation, domain) + indexed_attestation_signature_set_with_domain(state, indexed_attestation, domain) } /// [`is_valid_indexed_attestation`] under a signing domain the caller chose. @@ -141,24 +156,35 @@ pub fn is_valid_indexed_attestation_with_domain( indexed_attestation: &gloas::IndexedAttestation, domain: Domain, ) -> bool { + indexed_attestation_signature_set_with_domain(state, indexed_attestation, domain) + .is_some_and(|set| set.verify()) +} + +/// [`indexed_attestation_signature_set`] under a signing domain the caller +/// chose. +pub fn indexed_attestation_signature_set_with_domain( + state: &BeaconState, + indexed_attestation: &gloas::IndexedAttestation, + domain: Domain, +) -> Option { let indices: &[ValidatorIndex] = &indexed_attestation.attesting_indices; if indices.is_empty() || indices.len() > preset::MAX_VALIDATORS_PER_SLOT || !are_indices_sorted_and_unique(indices) { - return false; + return None; } let mut pubkeys = Vec::with_capacity(indices.len()); for index in indices { match state.validator(*index) { Ok(validator) => pubkeys.push(validator.pubkey), - Err(_) => return false, + Err(_) => return None, } } let signing_root = compute_signing_root(indexed_attestation.data.hash_tree_root(), domain); - bls::fast_aggregate_verify(&pubkeys, signing_root, &indexed_attestation.signature) + SignatureSet::fast_aggregate(&pubkeys, signing_root, &indexed_attestation.signature) } // --------------------------------------------------------------------------- @@ -284,16 +310,27 @@ pub fn is_valid_indexed_payload_attestation( state: &BeaconState, attestation: &gloas::IndexedPayloadAttestation, ) -> bool { + indexed_payload_attestation_signature_set(state, attestation).is_some_and(|set| set.verify()) +} + +/// The signature check [`is_valid_indexed_payload_attestation`] makes, +/// resolved but not yet run. The structural checks (non-empty, sorted, every +/// index in range) happen here, at once; `None` means the attestation is +/// invalid without needing a pairing. +pub fn indexed_payload_attestation_signature_set( + state: &BeaconState, + attestation: &gloas::IndexedPayloadAttestation, +) -> Option { let indices: &[ValidatorIndex] = &attestation.attesting_indices; if indices.is_empty() || !indices.windows(2).all(|pair| pair[0] <= pair[1]) { - return false; + return None; } let mut pubkeys = Vec::with_capacity(indices.len()); for index in indices { match state.validator(*index) { Ok(validator) => pubkeys.push(validator.pubkey), - Err(_) => return false, + Err(_) => return None, } } @@ -303,7 +340,7 @@ pub fn is_valid_indexed_payload_attestation( Some(compute_epoch_at_slot(attestation.data.slot)), ); let signing_root = compute_signing_root(attestation.data.hash_tree_root(), domain); - bls::fast_aggregate_verify(&pubkeys, signing_root, &attestation.signature) + SignatureSet::fast_aggregate(&pubkeys, signing_root, &attestation.signature) } // --------------------------------------------------------------------------- diff --git a/crates/blockchain/state_transition/src/beacon/stf/altair.rs b/crates/blockchain/state_transition/src/beacon/stf/altair.rs index 2b33e79be..ba926d667 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/altair.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/altair.rs @@ -26,7 +26,7 @@ //! two are the same number applied with opposite sign, not two independently //! derived quantities that happen to match. -use crate::beacon::bls; +use crate::beacon::bls::{self, SignatureSet}; use crate::beacon::constants; use crate::beacon::containers::{BeaconState, altair, phase0}; use crate::beacon::error::{Error, Result, verify}; @@ -38,11 +38,16 @@ use crate::beacon::helpers::accessors::{ use crate::beacon::helpers::altair::{ add_flag, base_reward_per_increment, get_attestation_participation_flag_indices, has_flag, }; -use crate::beacon::helpers::attestation::{get_indexed_attestation, is_valid_indexed_attestation}; +use crate::beacon::helpers::attestation::{ + get_indexed_attestation, indexed_attestation_signature_set, +}; use crate::beacon::helpers::misc::{compute_epoch_at_slot, compute_signing_root}; use crate::beacon::helpers::mutators::{decrease_balance, increase_balance}; use crate::beacon::preset; use crate::beacon::primitives::{BlsPubkey, Gwei, ParticipationFlags, ValidatorIndex}; +use crate::beacon::stf::signatures::SignatureBatch; +#[cfg(test)] +use crate::beacon::stf::signatures::checked; use std::collections::HashMap; // --------------------------------------------------------------------------- @@ -84,6 +89,7 @@ pub fn process_attestation( attestation: &phase0::Attestation, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + signatures: &mut SignatureBatch, ) -> Result<()> { let data = attestation.data; let current_epoch = get_current_epoch(state); @@ -137,8 +143,8 @@ pub fn process_attestation( get_attestation_participation_flag_indices(state, &data, inclusion_delay)?; let indexed_attestation = get_indexed_attestation(state, attestation, committees)?; - verify( - is_valid_indexed_attestation(state, &indexed_attestation), + signatures.push( + indexed_attestation_signature_set(state, &indexed_attestation), "is_valid_indexed_attestation(state, get_indexed_attestation(state, attestation))", )?; @@ -304,6 +310,7 @@ pub fn process_sync_aggregate( state: &mut BeaconState, sync_aggregate: &altair::SyncAggregate, active_balances: &ActiveBalanceCache, + signatures: &mut SignatureBatch, ) -> Result<()> { let committee_bits = &sync_aggregate.sync_committee_bits; @@ -327,14 +334,25 @@ pub fn process_sync_aggregate( Some(compute_epoch_at_slot(previous_slot)), ); let signing_root = compute_signing_root(get_block_root_at_slot(state, previous_slot)?, domain); - verify( - bls::eth_fast_aggregate_verify( - &participant_pubkeys, - signing_root, - &sync_aggregate.sync_committee_signature, - ), - "eth_fast_aggregate_verify(participant_pubkeys, signing_root, sync_aggregate.sync_committee_signature)", - )?; + // `eth_fast_aggregate_verify`'s one carve-out: with no participants the + // signature must be the point at infinity, and there is nothing to + // pair, so that case is decided here and defers nothing. + let eth_fast_aggregate_verify = "eth_fast_aggregate_verify(participant_pubkeys, signing_root, sync_aggregate.sync_committee_signature)"; + if participant_pubkeys.is_empty() { + verify( + sync_aggregate.sync_committee_signature.as_ref() == bls::G2_POINT_AT_INFINITY, + eth_fast_aggregate_verify, + )?; + } else { + signatures.push( + SignatureSet::fast_aggregate( + &participant_pubkeys, + signing_root, + &sync_aggregate.sync_committee_signature, + ), + eth_fast_aggregate_verify, + )?; + } // Resolve every committee seat's validator index up front; see this // function's documentation for why the mutation loop below cannot do its @@ -593,12 +611,15 @@ mod tests { let proposer_index = get_beacon_proposer_index(&state).unwrap(); let balance_before = state.balance(proposer_index).unwrap(); - process_attestation( - &mut state, - &attestation, - &CommitteeCache::default(), - &ActiveBalanceCache::default(), - ) + checked(|s| { + process_attestation( + &mut state, + &attestation, + &CommitteeCache::default(), + &ActiveBalanceCache::default(), + s, + ) + }) .unwrap(); let (previous_epoch_participation, _, _) = state.altair_validator_lists().unwrap(); @@ -629,12 +650,15 @@ mod tests { // so reprocessing the identical attestation must grant nothing new, and // the proposer's balance must not move. let balance_before_replay = state.balance(proposer_index).unwrap(); - process_attestation( - &mut state, - &attestation, - &CommitteeCache::default(), - &ActiveBalanceCache::default(), - ) + checked(|s| { + process_attestation( + &mut state, + &attestation, + &CommitteeCache::default(), + &ActiveBalanceCache::default(), + s, + ) + }) .unwrap(); let balance_after_replay = state.balance(proposer_index).unwrap(); assert_eq!( @@ -711,8 +735,15 @@ mod tests { .map(|index| state.balance(index).unwrap()) .collect(); - process_sync_aggregate(&mut state, &sync_aggregate, &ActiveBalanceCache::default()) - .unwrap(); + checked(|s| { + process_sync_aggregate( + &mut state, + &sync_aggregate, + &ActiveBalanceCache::default(), + s, + ) + }) + .unwrap(); // Independently derived expected rewards, sharing only the // already-tested building blocks (`get_total_active_balance`, @@ -777,8 +808,15 @@ mod tests { sync_committee_signature: BlsSignature::default(), }; assert!( - process_sync_aggregate(&mut state.clone(), &wrong, &ActiveBalanceCache::default()) - .is_err(), + checked(|s| { + process_sync_aggregate( + &mut state.clone(), + &wrong, + &ActiveBalanceCache::default(), + s, + ) + }) + .is_err(), "the all-zero signature is not the point at infinity, so this must be rejected" ); @@ -787,7 +825,10 @@ mod tests { sync_committee_signature: g2_point_at_infinity(), }; let balance_before = state.balance(0).unwrap(); - process_sync_aggregate(&mut state, &correct, &ActiveBalanceCache::default()).unwrap(); + checked(|s| { + process_sync_aggregate(&mut state, &correct, &ActiveBalanceCache::default(), s) + }) + .unwrap(); // The sole validator holds every seat and none of them participated, // so it is penalized once per seat and the proposer (itself, the only diff --git a/crates/blockchain/state_transition/src/beacon/stf/bellatrix.rs b/crates/blockchain/state_transition/src/beacon/stf/bellatrix.rs index e9287513e..e5fa269d4 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/bellatrix.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/bellatrix.rs @@ -40,6 +40,7 @@ use crate::beacon::preset; use crate::beacon::primitives::{ Bytes32, ExecutionAddress, ExecutionBlockHash, HashTreeRoot as _, Root, Slot, Uint256, }; +use crate::beacon::stf::signatures::SignatureBatch; use super::ExecutionEngine; @@ -63,6 +64,7 @@ use super::ExecutionEngine; /// the one fork where a block might still legitimately carry no real payload, /// if this chain's merge transition has not happened yet. Every later fork /// drops the condition outright, since by then it is always true. +#[allow(clippy::too_many_arguments)] pub fn process_block( state: &mut BeaconState, block: &bellatrix::BeaconBlock, @@ -71,6 +73,7 @@ pub fn process_block( committees: &CommitteeCache, active_balances: &ActiveBalanceCache, options: super::block::BlockProcessingOptions, + signatures: &mut SignatureBatch, ) -> Result<()> { super::block::process_block_header( state, @@ -82,7 +85,7 @@ pub fn process_block( if is_execution_enabled(state, &block.body.execution_payload)? { process_execution_payload(state, &block.body.execution_payload, config, engine)?; } - super::block::process_randao(state, &block.body.randao_reveal, options.randao)?; + super::block::process_randao(state, &block.body.randao_reveal, options.randao, signatures)?; super::block::process_eth1_data(state, &block.body.eth1_data)?; super::operations::process_operations( state, @@ -94,8 +97,14 @@ pub fn process_block( config, committees, active_balances, + signatures, + )?; + super::altair::process_sync_aggregate( + state, + &block.body.sync_aggregate, + active_balances, + signatures, )?; - super::altair::process_sync_aggregate(state, &block.body.sync_aggregate, active_balances)?; Ok(()) } diff --git a/crates/blockchain/state_transition/src/beacon/stf/block.rs b/crates/blockchain/state_transition/src/beacon/stf/block.rs index dcaa32684..af6057f3f 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; +use crate::beacon::bls::SignatureSet; use crate::beacon::config::Config; use crate::beacon::constants; use crate::beacon::containers::{self, BeaconBlockHeader, BeaconState, Eth1Data, altair, phase0}; @@ -31,16 +31,26 @@ use crate::beacon::helpers::math::xor; use crate::beacon::helpers::misc::compute_signing_root; use crate::beacon::preset; use crate::beacon::primitives::{BlsSignature, HashTreeRoot as _, Root, Slot, ValidatorIndex}; +use crate::beacon::stf::signatures::SignatureBatch; use super::{ExecutionEngine, bellatrix, capella, deneb, electra, fulu, gloas}; -/// Dispatches on the block's fork and runs that fork's own block processing. +/// Dispatches on the block's fork and runs that fork's own block processing, +/// then verifies every signature the processing deferred. /// /// Matches [`containers::SignedBeaconBlock`]'s variant once, then hands the /// unwrapped, fork-specific `BeaconBlock` to the function written for it. Called /// after [`super::process_slots`] has already advanced the state to the block's /// slot, so every arm validates and mutates a state already positioned at /// `signed_block`'s slot. +/// +/// Means what it always did, that the block is valid: it builds the +/// [`SignatureBatch`] the drivers push into and checks it before returning +/// `Ok`, so a caller needs no knowledge of it. +/// +/// An `Err` leaves `state` partly mutated, as it always could: an operation +/// whose signature is deferred has already written when a bad signature is +/// found at the end. Every caller discards the state after an error. pub fn process_block( state: &mut BeaconState, signed_block: &containers::SignedBeaconBlock, @@ -49,6 +59,33 @@ pub fn process_block( committees: &CommitteeCache, active_balances: &ActiveBalanceCache, options: BlockProcessingOptions, +) -> Result<()> { + SignatureBatch::deferred(|signatures| { + process_block_with_signatures( + state, + signed_block, + config, + engine, + committees, + active_balances, + options, + signatures, + ) + }) +} + +/// [`process_block`]'s dispatch: every signature the block's processing needs +/// is pushed into `signatures`, which [`process_block`] then verifies. +#[allow(clippy::too_many_arguments)] +fn process_block_with_signatures( + state: &mut BeaconState, + signed_block: &containers::SignedBeaconBlock, + config: &Config, + engine: &ExecutionEngine, + committees: &CommitteeCache, + active_balances: &ActiveBalanceCache, + options: BlockProcessingOptions, + signatures: &mut SignatureBatch, ) -> Result<()> { match signed_block { containers::SignedBeaconBlock::Phase0(signed) => process_block_phase0( @@ -58,6 +95,7 @@ pub fn process_block( committees, active_balances, options, + signatures, ), containers::SignedBeaconBlock::Altair(signed) => process_block_altair( state, @@ -66,6 +104,7 @@ pub fn process_block( committees, active_balances, options, + signatures, ), containers::SignedBeaconBlock::Bellatrix(signed) => bellatrix::process_block( state, @@ -75,6 +114,7 @@ pub fn process_block( committees, active_balances, options, + signatures, ), containers::SignedBeaconBlock::Capella(signed) => capella::process_block( state, @@ -84,6 +124,7 @@ pub fn process_block( committees, active_balances, options, + signatures, ), containers::SignedBeaconBlock::Deneb(signed) => deneb::process_block( state, @@ -93,6 +134,7 @@ pub fn process_block( committees, active_balances, options, + signatures, ), containers::SignedBeaconBlock::Electra(signed) => electra::process_block( state, @@ -102,6 +144,7 @@ pub fn process_block( committees, active_balances, options, + signatures, ), containers::SignedBeaconBlock::Fulu(signed) => fulu::process_block( state, @@ -111,6 +154,7 @@ pub fn process_block( committees, active_balances, options, + signatures, ), // ePBS (EIP-7732) restructures block processing around a builder's // bid rather than an embedded payload: a gloas block carries no @@ -122,6 +166,7 @@ pub fn process_block( committees, active_balances, options, + signatures, ), containers::SignedBeaconBlock::Lean(_) => { crate::beacon::lean_block_unreachable("process_block") @@ -138,6 +183,7 @@ pub fn process_block_phase0( committees: &CommitteeCache, active_balances: &ActiveBalanceCache, options: BlockProcessingOptions, + signatures: &mut SignatureBatch, ) -> Result<()> { process_block_header( state, @@ -146,7 +192,7 @@ pub fn process_block_phase0( block.parent_root, block.body.hash_tree_root(), )?; - process_randao(state, &block.body.randao_reveal, options.randao)?; + process_randao(state, &block.body.randao_reveal, options.randao, signatures)?; process_eth1_data(state, &block.body.eth1_data)?; crate::beacon::stf::operations::process_operations( state, @@ -158,6 +204,7 @@ pub fn process_block_phase0( config, committees, active_balances, + signatures, )?; Ok(()) } @@ -175,6 +222,7 @@ pub fn process_block_altair( committees: &CommitteeCache, active_balances: &ActiveBalanceCache, options: BlockProcessingOptions, + signatures: &mut SignatureBatch, ) -> Result<()> { process_block_header( state, @@ -183,7 +231,7 @@ pub fn process_block_altair( block.parent_root, block.body.hash_tree_root(), )?; - process_randao(state, &block.body.randao_reveal, options.randao)?; + process_randao(state, &block.body.randao_reveal, options.randao, signatures)?; process_eth1_data(state, &block.body.eth1_data)?; crate::beacon::stf::operations::process_operations( state, @@ -195,8 +243,14 @@ pub fn process_block_altair( config, committees, active_balances, + signatures, + )?; + super::altair::process_sync_aggregate( + state, + &block.body.sync_aggregate, + active_balances, + signatures, )?; - super::altair::process_sync_aggregate(state, &block.body.sync_aggregate, active_balances)?; Ok(()) } @@ -276,8 +330,9 @@ pub enum RandaoVerification { Skip, } -/// Verifies the proposer's RANDAO reveal (unless `verification` skips it) and -/// mixes it into the current epoch's randao mix. +/// Defers the check of the proposer's RANDAO reveal (unless `verification` +/// skips it) to `signatures`, and mixes the reveal into the current epoch's +/// randao mix. /// /// The reveal signs the epoch rather than the slot, so the same signature would /// verify for every slot of the epoch: the mix it feeds is per-epoch, not @@ -287,6 +342,7 @@ pub fn process_randao( state: &mut BeaconState, randao_reveal: &BlsSignature, verification: RandaoVerification, + signatures: &mut SignatureBatch, ) -> Result<()> { let epoch = get_current_epoch(state); let proposer_index = get_beacon_proposer_index(state)?; @@ -295,8 +351,8 @@ pub fn process_randao( let proposer = state.validator(proposer_index)?; if verification == RandaoVerification::Verify { - verify( - bls::verify(&proposer.pubkey, signing_root, randao_reveal), + signatures.push( + SignatureSet::single(&proposer.pubkey, signing_root, randao_reveal), "RANDAO reveal signature", )?; } @@ -333,7 +389,9 @@ pub fn process_eth1_data(state: &mut BeaconState, eth1_data: &Eth1Data) -> Resul #[cfg(test)] mod tests { use super::*; + use crate::beacon::bls; use crate::beacon::primitives::Bytes32; + use crate::beacon::stf::signatures::checked; /// An otherwise-empty block body, since the header, RANDAO, and eth1 checks /// below never look past its `hash_tree_root`. @@ -410,16 +468,73 @@ mod tests { let reveal = BlsSignature(bls::G2_POINT_AT_INFINITY); let expected = xor(get_randao_mix(&state, epoch), hash(&reveal.0)); - process_randao(&mut state, &reveal, RandaoVerification::Skip).unwrap(); + checked(|s| process_randao(&mut state, &reveal, RandaoVerification::Skip, s)).unwrap(); assert_eq!(get_randao_mix(&state, epoch), expected); } + /// A reveal signed by the right proposer over the wrong message. + fn reveal_over(state: &BeaconState, signed: Root) -> BlsSignature { + let proposer = get_beacon_proposer_index(state).unwrap(); + crate::beacon::helpers::test_state::sign_for(proposer as usize, signed) + } + + #[test] + fn a_bad_randao_reveal_is_deferred_and_rejected_by_the_batch() { + let mut state = crate::beacon::helpers::test_state::with_validators(8); + let reveal = reveal_over(&state, Root::repeat_byte(7)); + let mut signatures = SignatureBatch::new(); + + // Nothing is checked while processing; the batch is what rejects. + process_randao( + &mut state, + &reveal, + RandaoVerification::Verify, + &mut signatures, + ) + .unwrap(); + let err = signatures.verify().unwrap_err(); + assert!( + format!("{err:?}").contains("RANDAO reveal signature"), + "{err:?}" + ); + } + + #[test] + fn a_valid_randao_reveal_passes_the_batch() { + let mut state = crate::beacon::helpers::test_state::with_validators(8); + let epoch = get_current_epoch(&state); + let domain = get_domain(&state, constants::DOMAIN_RANDAO, None); + let signing_root = compute_signing_root(epoch.hash_tree_root(), domain); + let reveal = reveal_over(&state, signing_root); + + checked(|s| process_randao(&mut state, &reveal, RandaoVerification::Verify, s)).unwrap(); + } + + #[test] + fn skipping_randao_verification_pushes_nothing() { + let mut state = crate::beacon::helpers::test_state::with_validators(8); + let reveal = BlsSignature(bls::G2_POINT_AT_INFINITY); + let mut signatures = SignatureBatch::new(); + + process_randao( + &mut state, + &reveal, + RandaoVerification::Skip, + &mut signatures, + ) + .unwrap(); + assert!(signatures.is_empty()); + } + #[test] fn verifying_randao_rejects_the_infinity_reveal() { let mut state = crate::beacon::helpers::test_state::with_validators(8); let reveal = BlsSignature(bls::G2_POINT_AT_INFINITY); - assert!(process_randao(&mut state, &reveal, RandaoVerification::Verify).is_err()); + assert!( + checked(|s| process_randao(&mut state, &reveal, RandaoVerification::Verify, s)) + .is_err() + ); } } diff --git a/crates/blockchain/state_transition/src/beacon/stf/capella.rs b/crates/blockchain/state_transition/src/beacon/stf/capella.rs index f773d6da7..90958a744 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/capella.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/capella.rs @@ -41,7 +41,7 @@ //! fresh here, since capella's own [`capella::ExecutionPayloadHeader`] adds //! `withdrawals_root` and is a distinct type from bellatrix's. -use crate::beacon::bls; +use crate::beacon::bls::SignatureSet; use crate::beacon::config::Config; use crate::beacon::constants; use crate::beacon::containers::shared::{Deposit, ProposerSlashing, SignedVoluntaryExit}; @@ -58,6 +58,7 @@ use crate::beacon::helpers::misc::{compute_domain, compute_signing_root}; use crate::beacon::helpers::mutators::decrease_balance; use crate::beacon::preset; use crate::beacon::primitives::{H256, HashTreeRoot as _}; +use crate::beacon::stf::signatures::SignatureBatch; use super::ExecutionEngine; @@ -72,6 +73,7 @@ use super::ExecutionEngine; /// See this module's own documentation for why [`process_withdrawals`] runs /// before [`process_operations`], and so before this same block's own /// `bls_to_execution_changes` are processed. +#[allow(clippy::too_many_arguments)] pub fn process_block( state: &mut BeaconState, block: &capella::BeaconBlock, @@ -80,6 +82,7 @@ pub fn process_block( committees: &CommitteeCache, active_balances: &ActiveBalanceCache, options: super::block::BlockProcessingOptions, + signatures: &mut SignatureBatch, ) -> Result<()> { super::block::process_block_header( state, @@ -90,7 +93,7 @@ pub fn process_block( )?; process_withdrawals(state, &block.body.execution_payload)?; process_execution_payload(state, &block.body.execution_payload, config, engine)?; - super::block::process_randao(state, &block.body.randao_reveal, options.randao)?; + super::block::process_randao(state, &block.body.randao_reveal, options.randao, signatures)?; super::block::process_eth1_data(state, &block.body.eth1_data)?; process_operations( state, @@ -103,8 +106,14 @@ pub fn process_block( config, committees, active_balances, + signatures, + )?; + super::altair::process_sync_aggregate( + state, + &block.body.sync_aggregate, + active_balances, + signatures, )?; - super::altair::process_sync_aggregate(state, &block.body.sync_aggregate, active_balances)?; Ok(()) } @@ -369,6 +378,7 @@ pub fn process_operations( config: &Config, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + signatures: &mut SignatureBatch, ) -> Result<()> { super::operations::process_operations( state, @@ -380,9 +390,10 @@ pub fn process_operations( config, committees, active_balances, + signatures, )?; for signed_change in bls_to_execution_changes { - process_bls_to_execution_change(state, signed_change, config)?; + process_bls_to_execution_change(state, signed_change, config, signatures)?; } Ok(()) } @@ -414,6 +425,7 @@ pub fn process_bls_to_execution_change( state: &mut BeaconState, signed_change: &capella::SignedBLSToExecutionChange, config: &Config, + signatures: &mut SignatureBatch, ) -> Result<()> { let change = &signed_change.message; @@ -434,8 +446,8 @@ pub fn process_bls_to_execution_change( state.genesis_validators_root(), ); let signing_root = compute_signing_root(change.hash_tree_root(), domain); - verify( - bls::verify( + signatures.push( + SignatureSet::single( &change.from_bls_pubkey, signing_root, &signed_change.signature, @@ -748,7 +760,10 @@ mod tests { ); let signed_change = capella::SignedBLSToExecutionChange { message, signature }; - process_bls_to_execution_change(&mut state, &signed_change, &config).unwrap(); + crate::beacon::stf::signatures::checked(|s| { + process_bls_to_execution_change(&mut state, &signed_change, &config, s) + }) + .unwrap(); let validator = state.validator(0).unwrap(); assert_eq!( @@ -797,7 +812,12 @@ mod tests { ); let signed_change = capella::SignedBLSToExecutionChange { message, signature }; - assert!(process_bls_to_execution_change(&mut state, &signed_change, &config).is_err()); + assert!( + crate::beacon::stf::signatures::checked(|s| { + process_bls_to_execution_change(&mut state, &signed_change, &config, s) + }) + .is_err() + ); } #[test] @@ -818,6 +838,11 @@ mod tests { signature: BlsSignature::default(), }; - assert!(process_bls_to_execution_change(&mut state, &signed_change, &config).is_err()); + assert!( + crate::beacon::stf::signatures::checked(|s| { + process_bls_to_execution_change(&mut state, &signed_change, &config, s) + }) + .is_err() + ); } } diff --git a/crates/blockchain/state_transition/src/beacon/stf/deneb.rs b/crates/blockchain/state_transition/src/beacon/stf/deneb.rs index 72b5ac946..920f5d3d0 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; +use crate::beacon::bls::SignatureSet; use crate::beacon::config::Config; use crate::beacon::constants::{self, FAR_FUTURE_EPOCH}; use crate::beacon::containers::capella::SignedBLSToExecutionChange; @@ -41,7 +41,9 @@ use crate::beacon::helpers::accessors::{ get_previous_epoch, get_randao_mix, }; use crate::beacon::helpers::altair::{add_flag, base_reward_per_increment, has_flag}; -use crate::beacon::helpers::attestation::{get_indexed_attestation, is_valid_indexed_attestation}; +use crate::beacon::helpers::attestation::{ + get_indexed_attestation, indexed_attestation_signature_set, +}; use crate::beacon::helpers::math::integer_squareroot; use crate::beacon::helpers::misc::{compute_domain, compute_epoch_at_slot, compute_signing_root}; use crate::beacon::helpers::mutators::{ @@ -52,6 +54,7 @@ use crate::beacon::preset; use crate::beacon::primitives::{ Bytes32, Gwei, H256, HashTreeRoot as _, KzgCommitment, ParticipationFlags, ValidatorIndex, }; +use crate::beacon::stf::signatures::SignatureBatch; use super::ExecutionEngine; @@ -72,6 +75,7 @@ use super::ExecutionEngine; /// [`super::operations::process_operations`] has no fork check of its own to /// hang them on. See [`process_operations`]'s own documentation for why that /// is a full copy of the loop rather than a smaller patch. +#[allow(clippy::too_many_arguments)] pub fn process_block( state: &mut BeaconState, block: &deneb::BeaconBlock, @@ -80,6 +84,7 @@ pub fn process_block( committees: &CommitteeCache, active_balances: &ActiveBalanceCache, options: super::block::BlockProcessingOptions, + signatures: &mut SignatureBatch, ) -> Result<()> { super::block::process_block_header( state, @@ -96,7 +101,7 @@ pub fn process_block( config, engine, )?; - super::block::process_randao(state, &block.body.randao_reveal, options.randao)?; + super::block::process_randao(state, &block.body.randao_reveal, options.randao, signatures)?; super::block::process_eth1_data(state, &block.body.eth1_data)?; process_operations( state, @@ -109,8 +114,14 @@ pub fn process_block( config, committees, active_balances, + signatures, + )?; + super::altair::process_sync_aggregate( + state, + &block.body.sync_aggregate, + active_balances, + signatures, )?; - super::altair::process_sync_aggregate(state, &block.body.sync_aggregate, active_balances)?; Ok(()) } @@ -152,6 +163,7 @@ fn process_operations( config: &Config, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + signatures: &mut SignatureBatch, ) -> Result<()> { let outstanding = state .eth1_data() @@ -166,22 +178,22 @@ fn process_operations( )?; for proposer_slashing in proposer_slashings { - super::operations::process_proposer_slashing(state, proposer_slashing, config)?; + super::operations::process_proposer_slashing(state, proposer_slashing, config, signatures)?; } for attester_slashing in attester_slashings { - super::operations::process_attester_slashing(state, attester_slashing, config)?; + super::operations::process_attester_slashing(state, attester_slashing, config, signatures)?; } for attestation in attestations { - process_attestation(state, attestation, committees, active_balances)?; + process_attestation(state, attestation, committees, active_balances, signatures)?; } for deposit in deposits { super::operations::process_deposit(state, deposit, config)?; } for voluntary_exit in voluntary_exits { - process_voluntary_exit(state, voluntary_exit, config)?; + process_voluntary_exit(state, voluntary_exit, config, signatures)?; } for signed_change in bls_to_execution_changes { - super::capella::process_bls_to_execution_change(state, signed_change, config)?; + super::capella::process_bls_to_execution_change(state, signed_change, config, signatures)?; } Ok(()) @@ -287,6 +299,7 @@ pub fn process_attestation( attestation: &phase0::Attestation, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + signatures: &mut SignatureBatch, ) -> Result<()> { let data = attestation.data; let current_epoch = get_current_epoch(state); @@ -336,8 +349,8 @@ pub fn process_attestation( attestation_participation_flag_indices(state, &data, inclusion_delay)?; let indexed_attestation = get_indexed_attestation(state, attestation, committees)?; - verify( - is_valid_indexed_attestation(state, &indexed_attestation), + signatures.push( + indexed_attestation_signature_set(state, &indexed_attestation), "is_valid_indexed_attestation(state, get_indexed_attestation(state, attestation))", )?; @@ -660,6 +673,7 @@ pub fn process_voluntary_exit( state: &mut BeaconState, signed_voluntary_exit: &SignedVoluntaryExit, config: &Config, + signatures: &mut SignatureBatch, ) -> Result<()> { let voluntary_exit = signed_voluntary_exit.message; let current_epoch = get_current_epoch(state); @@ -696,8 +710,8 @@ pub fn process_voluntary_exit( state.genesis_validators_root(), ); let signing_root = compute_signing_root(voluntary_exit.hash_tree_root(), domain); - verify( - bls::verify( + signatures.push( + SignatureSet::single( &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 9b3519caf..53becb7bb 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; +use crate::beacon::bls::{self, SignatureSet}; use crate::beacon::config::Config; use crate::beacon::constants::{self, FAR_FUTURE_EPOCH}; use crate::beacon::containers::shared::{ @@ -65,8 +65,8 @@ use crate::beacon::helpers::electra::{ 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, - initiate_validator_exit as electra_initiate_validator_exit, is_fully_withdrawable_validator, - is_partially_withdrawable_validator, is_valid_indexed_attestation, pending_queue_fields, + indexed_attestation_signature_set, initiate_validator_exit as electra_initiate_validator_exit, + is_fully_withdrawable_validator, is_partially_withdrawable_validator, pending_queue_fields, pending_queue_fields_ref, }; use crate::beacon::helpers::math::integer_squareroot; @@ -81,6 +81,7 @@ use crate::beacon::primitives::{ BlsPubkey, BlsSignature, Bytes32, Gwei, HashTreeRoot as _, ParticipationFlags, ValidatorIndex, WithdrawalIndex, }; +use crate::beacon::stf::signatures::SignatureBatch; use super::ExecutionEngine; @@ -489,6 +490,7 @@ pub fn process_voluntary_exit( state: &mut BeaconState, signed_voluntary_exit: &SignedVoluntaryExit, config: &Config, + signatures: &mut SignatureBatch, ) -> Result<()> { let voluntary_exit = signed_voluntary_exit.message; let current_epoch = get_current_epoch(state); @@ -528,8 +530,8 @@ pub fn process_voluntary_exit( state.genesis_validators_root(), ); let signing_root = compute_signing_root(voluntary_exit.hash_tree_root(), domain); - verify( - bls::verify( + signatures.push( + SignatureSet::single( &validator.pubkey, signing_root, &signed_voluntary_exit.signature, @@ -560,8 +562,9 @@ pub fn process_bls_to_execution_change( state: &mut BeaconState, signed_change: &capella::SignedBLSToExecutionChange, config: &Config, + signatures: &mut SignatureBatch, ) -> Result<()> { - super::capella::process_bls_to_execution_change(state, signed_change, config) + super::capella::process_bls_to_execution_change(state, signed_change, config, signatures) } // --------------------------------------------------------------------------- @@ -586,6 +589,7 @@ pub fn process_attester_slashing( state: &mut BeaconState, attester_slashing: &electra::AttesterSlashing, config: &Config, + signatures: &mut SignatureBatch, ) -> Result<()> { let attestation_1 = &attester_slashing.attestation_1; let attestation_2 = &attester_slashing.attestation_2; @@ -594,12 +598,12 @@ pub fn process_attester_slashing( is_slashable_attestation_data(&attestation_1.data, &attestation_2.data), "is_slashable_attestation_data(attestation_1.data, attestation_2.data)", )?; - verify( - is_valid_indexed_attestation(state, attestation_1), + signatures.push( + indexed_attestation_signature_set(state, attestation_1), "is_valid_indexed_attestation(state, attestation_1)", )?; - verify( - is_valid_indexed_attestation(state, attestation_2), + signatures.push( + indexed_attestation_signature_set(state, attestation_2), "is_valid_indexed_attestation(state, attestation_2)", )?; @@ -641,6 +645,7 @@ pub fn process_attestation( attestation: &electra::Attestation, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + signatures: &mut SignatureBatch, ) -> Result<()> { let data = attestation.data; let current_epoch = get_current_epoch(state); @@ -710,8 +715,8 @@ pub fn process_attestation( attestation_participation_flag_indices(state, &data, inclusion_delay)?; let indexed_attestation = get_indexed_attestation(state, attestation, committees)?; - verify( - is_valid_indexed_attestation(state, &indexed_attestation), + signatures.push( + indexed_attestation_signature_set(state, &indexed_attestation), "is_valid_indexed_attestation(state, get_indexed_attestation(state, attestation))", )?; @@ -1785,6 +1790,7 @@ pub fn process_operations( config: &Config, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + signatures: &mut SignatureBatch, ) -> Result<()> { let deposit_requests_start_index = block_ref(state, "process_operations")?.deposit_requests_start_index(); @@ -1803,24 +1809,24 @@ pub fn process_operations( } for proposer_slashing in body.proposer_slashings.iter() { - super::operations::process_proposer_slashing(state, proposer_slashing, config)?; + super::operations::process_proposer_slashing(state, proposer_slashing, config, signatures)?; } for attester_slashing in body.attester_slashings.iter() { - process_attester_slashing(state, attester_slashing, config)?; + process_attester_slashing(state, attester_slashing, config, signatures)?; } // [Modified in Electra:EIP7549] for attestation in body.attestations.iter() { - process_attestation(state, attestation, committees, active_balances)?; + process_attestation(state, attestation, committees, active_balances, signatures)?; } for deposit in body.deposits.iter() { process_deposit(state, deposit, config)?; } // [Modified in Electra:EIP7251] for voluntary_exit in body.voluntary_exits.iter() { - process_voluntary_exit(state, voluntary_exit, config)?; + process_voluntary_exit(state, voluntary_exit, config, signatures)?; } for signed_change in body.bls_to_execution_changes.iter() { - process_bls_to_execution_change(state, signed_change, config)?; + process_bls_to_execution_change(state, signed_change, config, signatures)?; } // [New in Electra:EIP6110:EIP7002:EIP7251] process_execution_requests(state, &body.execution_requests, config)?; @@ -1845,6 +1851,7 @@ pub fn process_operations( /// them taking those fields directly rather than a whole body (see /// `crate::beacon::stf`'s module documentation). [`super::altair::process_sync_aggregate`] /// is reused the same way, for the same reason. +#[allow(clippy::too_many_arguments)] pub fn process_block( state: &mut BeaconState, block: &electra::BeaconBlock, @@ -1853,6 +1860,7 @@ pub fn process_block( committees: &CommitteeCache, active_balances: &ActiveBalanceCache, options: super::block::BlockProcessingOptions, + signatures: &mut SignatureBatch, ) -> Result<()> { super::block::process_block_header( state, @@ -1863,10 +1871,22 @@ pub fn process_block( )?; process_withdrawals(state, &block.body.execution_payload)?; process_execution_payload(state, &block.body, config, engine)?; - super::block::process_randao(state, &block.body.randao_reveal, options.randao)?; + super::block::process_randao(state, &block.body.randao_reveal, options.randao, signatures)?; super::block::process_eth1_data(state, &block.body.eth1_data)?; - process_operations(state, &block.body, config, committees, active_balances)?; - super::altair::process_sync_aggregate(state, &block.body.sync_aggregate, active_balances)?; + process_operations( + state, + &block.body, + config, + committees, + active_balances, + signatures, + )?; + super::altair::process_sync_aggregate( + state, + &block.body.sync_aggregate, + active_balances, + signatures, + )?; Ok(()) } diff --git a/crates/blockchain/state_transition/src/beacon/stf/fulu.rs b/crates/blockchain/state_transition/src/beacon/stf/fulu.rs index 1a8bc3b58..9f04facf6 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/fulu.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/fulu.rs @@ -49,6 +49,7 @@ use crate::beacon::helpers::accessors::{ use crate::beacon::helpers::electra::pending_queue_fields; use crate::beacon::helpers::fulu::{fulu_state, fulu_state_ref}; use crate::beacon::primitives::{Bytes32, HashTreeRoot as _}; +use crate::beacon::stf::signatures::SignatureBatch; use super::ExecutionEngine; @@ -61,6 +62,7 @@ use super::ExecutionEngine; /// for [`super::electra::process_execution_payload`] and /// [`super::electra::process_operations`]. See the module docs for why these /// are the only steps this fork's specification asks to change. +#[allow(clippy::too_many_arguments)] pub fn process_block( state: &mut BeaconState, block: &electra::BeaconBlock, @@ -69,6 +71,7 @@ pub fn process_block( committees: &CommitteeCache, active_balances: &ActiveBalanceCache, options: super::block::BlockProcessingOptions, + signatures: &mut SignatureBatch, ) -> Result<()> { super::block::process_block_header( state, @@ -79,10 +82,22 @@ pub fn process_block( )?; super::electra::process_withdrawals(state, &block.body.execution_payload)?; process_execution_payload(state, &block.body, config, engine)?; - super::block::process_randao(state, &block.body.randao_reveal, options.randao)?; + super::block::process_randao(state, &block.body.randao_reveal, options.randao, signatures)?; super::block::process_eth1_data(state, &block.body.eth1_data)?; - process_operations(state, &block.body, config, committees, active_balances)?; - super::altair::process_sync_aggregate(state, &block.body.sync_aggregate, active_balances)?; + process_operations( + state, + &block.body, + config, + committees, + active_balances, + signatures, + )?; + super::altair::process_sync_aggregate( + state, + &block.body.sync_aggregate, + active_balances, + signatures, + )?; Ok(()) } @@ -224,26 +239,33 @@ pub fn process_operations( config: &Config, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + signatures: &mut SignatureBatch, ) -> Result<()> { // [Modified in Fulu] verify(body.deposits.is_empty(), "len(body.deposits) == 0")?; for proposer_slashing in body.proposer_slashings.iter() { - super::operations::process_proposer_slashing(state, proposer_slashing, config)?; + super::operations::process_proposer_slashing(state, proposer_slashing, config, signatures)?; } for attester_slashing in body.attester_slashings.iter() { - super::electra::process_attester_slashing(state, attester_slashing, config)?; + super::electra::process_attester_slashing(state, attester_slashing, config, signatures)?; } for attestation in body.attestations.iter() { - super::electra::process_attestation(state, attestation, committees, active_balances)?; + super::electra::process_attestation( + state, + attestation, + committees, + active_balances, + signatures, + )?; } // [Modified in Fulu] // Removed `process_deposit` for voluntary_exit in body.voluntary_exits.iter() { - super::electra::process_voluntary_exit(state, voluntary_exit, config)?; + super::electra::process_voluntary_exit(state, voluntary_exit, config, signatures)?; } for signed_change in body.bls_to_execution_changes.iter() { - super::electra::process_bls_to_execution_change(state, signed_change, config)?; + super::electra::process_bls_to_execution_change(state, signed_change, config, signatures)?; } process_execution_requests(state, &body.execution_requests, config)?; diff --git a/crates/blockchain/state_transition/src/beacon/stf/gloas.rs b/crates/blockchain/state_transition/src/beacon/stf/gloas.rs index 0ef4c1c8c..45de2f56b 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/gloas.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/gloas.rs @@ -47,7 +47,7 @@ use libssz::SszEncode as _; -use crate::beacon::bls; +use crate::beacon::bls::{self, SignatureSet}; use crate::beacon::config::Config; use crate::beacon::constants; use crate::beacon::containers::shared::{DepositMessage, ProposerSlashing}; @@ -64,9 +64,9 @@ 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, is_valid_indexed_attestation, - is_valid_indexed_payload_attestation, settle_builder_payment, + 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}; @@ -76,6 +76,7 @@ use crate::beacon::primitives::{ Bytes32, ExecutionAddress, Gwei, HashTreeRoot as _, ParticipationFlags, Root, Slot, ValidatorIndex, WithdrawalIndex, }; +use crate::beacon::stf::signatures::SignatureBatch; use super::ExecutionEngine; @@ -105,6 +106,7 @@ pub fn process_block( committees: &CommitteeCache, active_balances: &ActiveBalanceCache, options: super::block::BlockProcessingOptions, + signatures: &mut SignatureBatch, ) -> Result<()> { // [New in Gloas:EIP7732] let parent_slot = state.latest_block_header().slot; @@ -122,8 +124,13 @@ pub fn process_block( process_withdrawals(state)?; // [Modified in Gloas:EIP7732] Removed `process_execution_payload`. // [New in Gloas:EIP7732] - process_execution_payload_bid(state, &block.body.signed_execution_payload_bid, config)?; - super::block::process_randao(state, &block.body.randao_reveal, options.randao)?; + process_execution_payload_bid( + state, + &block.body.signed_execution_payload_bid, + config, + signatures, + )?; + super::block::process_randao(state, &block.body.randao_reveal, options.randao, signatures)?; super::block::process_eth1_data(state, &block.body.eth1_data)?; // [Modified in Gloas:EIP7732] process_operations( @@ -133,11 +140,45 @@ pub fn process_block( config, committees, active_balances, + signatures, + )?; + super::altair::process_sync_aggregate( + state, + &block.body.sync_aggregate, + active_balances, + signatures, )?; - super::altair::process_sync_aggregate(state, &block.body.sync_aggregate, active_balances)?; Ok(()) } +/// [`process_block`] followed by the check of every signature it deferred: +/// the whole of what the specification's `process_block` means. +/// +/// For the callers that run a gloas block through its own driver rather than +/// through [`super::block::process_block`] (block production, which builds a +/// block's state root this way), and have no signature of their own to add to +/// the batch. +pub fn process_block_and_verify( + state: &mut BeaconState, + block: &gloas::BeaconBlock, + config: &Config, + committees: &CommitteeCache, + active_balances: &ActiveBalanceCache, + options: super::block::BlockProcessingOptions, +) -> Result<()> { + SignatureBatch::deferred(|signatures| { + process_block( + state, + block, + config, + committees, + active_balances, + options, + signatures, + ) + }) +} + // --------------------------------------------------------------------------- // Withdrawals // --------------------------------------------------------------------------- @@ -745,6 +786,17 @@ pub fn verify_execution_payload_bid_signature( state: &BeaconState, signed_bid: &gloas::SignedExecutionPayloadBid, ) -> Result { + Ok(execution_payload_bid_signature_set(state, signed_bid)?.is_some_and(|set| set.verify())) +} + +/// The signature check [`verify_execution_payload_bid_signature`] makes, +/// resolved but not yet run, so a block can verify it with the rest of its +/// signatures in one batch. `Ok(None)` is a builder key or a signature that +/// does not validate: the bid is invalid without needing a pairing. +pub fn execution_payload_bid_signature_set( + state: &BeaconState, + signed_bid: &gloas::SignedExecutionPayloadBid, +) -> Result> { let builder_index = signed_bid.message.builder_index; let inner = gloas_state_ref(state, "verify_execution_payload_bid_signature")?; let builder = inner @@ -756,7 +808,7 @@ pub fn verify_execution_payload_bid_signature( })?; let domain = get_domain(state, constants::DOMAIN_BEACON_BUILDER, None); let signing_root = compute_signing_root(signed_bid.message.hash_tree_root(), domain); - Ok(bls::verify( + Ok(SignatureSet::single( &builder.pubkey, signing_root, &signed_bid.signature, @@ -768,6 +820,7 @@ pub fn process_execution_payload_bid( state: &mut BeaconState, signed_bid: &gloas::SignedExecutionPayloadBid, config: &Config, + signatures: &mut SignatureBatch, ) -> Result<()> { let bid = signed_bid.message.clone(); let builder_index = bid.builder_index; @@ -816,8 +869,8 @@ pub fn process_execution_payload_bid( "process_execution_payload_bid: can_builder_cover_bid(state, builder_index, amount)", )?; // Verify that the bid signature is valid. - verify( - verify_execution_payload_bid_signature(state, signed_bid)?, + signatures.push( + execution_payload_bid_signature_set(state, signed_bid)?, "process_execution_payload_bid: verify_execution_payload_bid_signature(state, signed_bid)", )?; } @@ -1186,9 +1239,13 @@ pub fn process_proposer_slashing( state: &mut BeaconState, proposer_slashing: &ProposerSlashing, config: &Config, + signatures: &mut SignatureBatch, ) -> Result<()> { - let (proposer_index, current_epoch) = - crate::beacon::stf::operations::verify_proposer_slashing(state, proposer_slashing)?; + let (proposer_index, current_epoch) = crate::beacon::stf::operations::verify_proposer_slashing( + state, + proposer_slashing, + signatures, + )?; // [New in Gloas:EIP7732] Remove the `BuilderPendingPayment` corresponding // to this proposal if it is still in the 2-epoch window. @@ -1249,6 +1306,7 @@ pub fn process_attester_slashing( state: &mut BeaconState, attester_slashing: &gloas::AttesterSlashing, config: &Config, + signatures: &mut SignatureBatch, ) -> Result<()> { let attestation_1 = &attester_slashing.attestation_1; let attestation_2 = &attester_slashing.attestation_2; @@ -1257,12 +1315,12 @@ pub fn process_attester_slashing( is_slashable_attestation_data(&attestation_1.data, &attestation_2.data), "process_attester_slashing: is_slashable_attestation_data(attestation_1.data, attestation_2.data)", )?; - verify( - is_valid_indexed_attestation(state, attestation_1), + signatures.push( + indexed_attestation_signature_set(state, attestation_1), "process_attester_slashing: is_valid_indexed_attestation(state, attestation_1)", )?; - verify( - is_valid_indexed_attestation(state, attestation_2), + signatures.push( + indexed_attestation_signature_set(state, attestation_2), "process_attester_slashing: is_valid_indexed_attestation(state, attestation_2)", )?; @@ -1303,6 +1361,7 @@ pub fn process_attestation( parent_slot: Slot, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + signatures: &mut SignatureBatch, ) -> Result<()> { let data = attestation.data; let current_epoch = get_current_epoch(state); @@ -1363,8 +1422,8 @@ pub fn process_attestation( get_attestation_participation_flag_indices(state, &data, inclusion_delay, parent_slot)?; let indexed_attestation = get_indexed_attestation(state, attestation, committees)?; - verify( - is_valid_indexed_attestation(state, &indexed_attestation), + signatures.push( + indexed_attestation_signature_set(state, &indexed_attestation), "process_attestation: is_valid_indexed_attestation(state, get_indexed_attestation(state, attestation))", )?; @@ -1541,6 +1600,7 @@ pub fn process_payload_attestation( state: &mut BeaconState, payload_attestation: &gloas::PayloadAttestation, config: &Config, + signatures: &mut SignatureBatch, ) -> Result<()> { let data = payload_attestation.data; @@ -1561,8 +1621,8 @@ pub fn process_payload_attestation( // Verify signature. let indexed_payload_attestation = get_indexed_payload_attestation(state, payload_attestation, config)?; - verify( - is_valid_indexed_payload_attestation(state, &indexed_payload_attestation), + signatures.push( + indexed_payload_attestation_signature_set(state, &indexed_payload_attestation), "process_payload_attestation: is_valid_indexed_payload_attestation(state, indexed_payload_attestation)", )?; Ok(()) @@ -1588,6 +1648,7 @@ pub fn process_operations( config: &Config, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + signatures: &mut SignatureBatch, ) -> Result<()> { verify( body.deposits.is_empty(), @@ -1622,26 +1683,43 @@ pub fn process_operations( // [Modified in Gloas:EIP7732] for proposer_slashing in body.proposer_slashings.iter() { - process_proposer_slashing(state, proposer_slashing, config)?; + process_proposer_slashing(state, proposer_slashing, config, signatures)?; } for attester_slashing in body.attester_slashings.iter() { - process_attester_slashing(state, attester_slashing, config)?; + process_attester_slashing(state, attester_slashing, config, signatures)?; } // [Modified in Gloas:EIP7732] for attestation in body.attestations.iter() { - process_attestation(state, attestation, parent_slot, committees, active_balances)?; + process_attestation( + state, + attestation, + parent_slot, + committees, + active_balances, + signatures, + )?; } for voluntary_exit in body.voluntary_exits.iter() { - crate::beacon::stf::electra::process_voluntary_exit(state, voluntary_exit, config)?; + crate::beacon::stf::electra::process_voluntary_exit( + state, + voluntary_exit, + config, + signatures, + )?; } for signed_change in body.bls_to_execution_changes.iter() { - crate::beacon::stf::capella::process_bls_to_execution_change(state, signed_change, config)?; + crate::beacon::stf::capella::process_bls_to_execution_change( + state, + signed_change, + config, + signatures, + )?; } // [Modified in Gloas:EIP7732] Removed `process_deposit_request`, // `process_withdrawal_request`, and `process_consolidation_request`. // [New in Gloas:EIP7732] for payload_attestation in body.payload_attestations.iter() { - process_payload_attestation(state, payload_attestation, config)?; + process_payload_attestation(state, payload_attestation, config, signatures)?; } Ok(()) @@ -1677,7 +1755,10 @@ mod tests { signature: g2_point_at_infinity(), }; - process_execution_payload_bid(&mut state, &signed_bid, &config).unwrap(); + crate::beacon::stf::signatures::checked(|s| { + process_execution_payload_bid(&mut state, &signed_bid, &config, s) + }) + .unwrap(); assert_eq!( gloas_state_ref(&state, "test") .unwrap() @@ -1706,7 +1787,9 @@ mod tests { signature: g2_point_at_infinity(), }; - let result = process_execution_payload_bid(&mut state, &signed_bid, &config); + let result = crate::beacon::stf::signatures::checked(|s| { + process_execution_payload_bid(&mut state, &signed_bid, &config, s) + }); assert!( matches!( result, @@ -1922,14 +2005,17 @@ mod tests { for (expected_message, body) in cases { let mut state = state.clone(); let committees = CommitteeCache::default(); - let result = process_operations( - &mut state, - &body, - 0, - &config, - &committees, - &ActiveBalanceCache::default(), - ); + let result = crate::beacon::stf::signatures::checked(|s| { + process_operations( + &mut state, + &body, + 0, + &config, + &committees, + &ActiveBalanceCache::default(), + s, + ) + }); assert!( matches!(result, Err(Error::SpecAssert(message)) if message == expected_message), "expected {expected_message:?} to fail, got {result:?}" diff --git a/crates/blockchain/state_transition/src/beacon/stf/mod.rs b/crates/blockchain/state_transition/src/beacon/stf/mod.rs index 3dba6eef8..890c670bd 100644 --- a/crates/blockchain/state_transition/src/beacon/stf/mod.rs +++ b/crates/blockchain/state_transition/src/beacon/stf/mod.rs @@ -63,6 +63,7 @@ pub mod epoch; pub mod fulu; pub mod gloas; pub mod operations; +pub mod signatures; use crate::beacon::containers; use crate::beacon::containers::{BeaconState, phase0}; @@ -177,6 +178,12 @@ pub fn apply_block( "the block's fork matches the state's", )?; + // The proposer's signature on its own, before the body runs, rather than in + // the body's batch: a block nobody but its sender signed then costs one + // pairing to reject instead of a whole body's processing. Gossip checked it + // already (`precheck_block`), but a block fetched by request arrives here + // unchecked. `process_block` batches the body's own signatures and checks + // them before returning, so before the state root below is computed. if validate_result { verify( verify_block_signature(state, signed_block), @@ -212,12 +219,20 @@ pub fn verify_block_signature( state: &BeaconState, signed_block: &containers::SignedBeaconBlock, ) -> bool { - let Ok(proposer) = state.validator(signed_block.proposer_index()) else { - return false; - }; + block_signature_set(state, signed_block).is_some_and(|set| set.verify()) +} + +/// The proposer's signature check [`verify_block_signature`] makes, resolved +/// but not yet run. `None` is a proposer index outside the registry, or a key +/// or signature that does not validate. +pub fn block_signature_set( + state: &BeaconState, + signed_block: &containers::SignedBeaconBlock, +) -> Option { + let proposer = state.validator(signed_block.proposer_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::verify(&proposer.pubkey, signing_root, &signed_block.signature()) + bls::SignatureSet::single(&proposer.pubkey, 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 5f9add7cc..86d15dcb8 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; +use crate::beacon::bls::{self, SignatureSet}; use crate::beacon::config::Config; use crate::beacon::constants::{self, FAR_FUTURE_EPOCH}; use crate::beacon::containers::BeaconState; @@ -26,7 +26,9 @@ use crate::beacon::helpers::accessors::{ ActiveBalanceCache, CommitteeCache, CommitteeCacheExt, get_beacon_proposer_index, get_current_epoch, get_domain, get_previous_epoch, }; -use crate::beacon::helpers::attestation::{get_indexed_attestation, is_valid_indexed_attestation}; +use crate::beacon::helpers::attestation::{ + get_indexed_attestation, indexed_attestation_signature_set, +}; use crate::beacon::helpers::misc::{ compute_deposit_domain, compute_epoch_at_slot, compute_signing_root, is_valid_merkle_branch, }; @@ -39,6 +41,7 @@ use crate::beacon::helpers::predicates::{ use crate::beacon::lean_state_unreachable; use crate::beacon::preset; use crate::beacon::primitives::{Epoch, Gwei, HashTreeRoot as _, ValidatorIndex}; +use crate::beacon::stf::signatures::SignatureBatch; /// Runs every operation in a block, in the specification's order. /// @@ -74,6 +77,7 @@ pub fn process_operations( config: &Config, committees: &CommitteeCache, active_balances: &ActiveBalanceCache, + signatures: &mut SignatureBatch, ) -> Result<()> { // Checked before anything below can mutate `state`, and unconditionally // rather than only when `attestations` is non-empty: a refused fork must @@ -118,10 +122,10 @@ pub fn process_operations( )?; for proposer_slashing in proposer_slashings { - process_proposer_slashing(state, proposer_slashing, config)?; + process_proposer_slashing(state, proposer_slashing, config, signatures)?; } for attester_slashing in attester_slashings { - process_attester_slashing(state, attester_slashing, config)?; + process_attester_slashing(state, attester_slashing, config, signatures)?; } for attestation in attestations { // Phase0 defers an attestation's reward to the epoch boundary, so it @@ -139,16 +143,17 @@ pub fn process_operations( attestation, committees, active_balances, + signatures, )?; } else { - process_attestation(state, attestation, config, committees)?; + process_attestation(state, attestation, config, committees, signatures)?; } } for deposit in deposits { process_deposit(state, deposit, config)?; } for voluntary_exit in voluntary_exits { - process_voluntary_exit(state, voluntary_exit, config)?; + process_voluntary_exit(state, voluntary_exit, config, signatures)?; } Ok(()) @@ -160,7 +165,8 @@ pub fn process_operations( /// Checks a proposer slashing's evidence without slashing anyone: the two /// headers agree on slot and proposer, differ from each other, name a -/// still-slashable proposer, and both signatures check out. +/// still-slashable proposer. Both signatures are pushed to `signatures`, which +/// the block's driver checks once processing ends. /// /// Shared between phase0's own [`process_proposer_slashing`] below and /// `crate::beacon::stf::gloas::process_proposer_slashing`: gloas's own @@ -177,6 +183,7 @@ pub fn process_operations( pub(crate) fn verify_proposer_slashing( state: &BeaconState, proposer_slashing: &ProposerSlashing, + signatures: &mut SignatureBatch, ) -> Result<(ValidatorIndex, Epoch)> { let header_1 = &proposer_slashing.signed_header_1.message; let header_2 = &proposer_slashing.signed_header_2.message; @@ -210,8 +217,8 @@ pub(crate) fn verify_proposer_slashing( let epoch = compute_epoch_at_slot(signed_header.message.slot); let domain = get_domain(state, constants::DOMAIN_BEACON_PROPOSER, Some(epoch)); let signing_root = compute_signing_root(signed_header.message.hash_tree_root(), domain); - verify( - bls::verify(&pubkey, signing_root, &signed_header.signature), + signatures.push( + SignatureSet::single(&pubkey, signing_root, &signed_header.signature), "bls.Verify(proposer.pubkey, signing_root, signed_header.signature)", )?; } @@ -226,8 +233,10 @@ pub fn process_proposer_slashing( state: &mut BeaconState, proposer_slashing: &ProposerSlashing, config: &Config, + signatures: &mut SignatureBatch, ) -> Result<()> { - let (proposer_index, _current_epoch) = verify_proposer_slashing(state, proposer_slashing)?; + let (proposer_index, _current_epoch) = + verify_proposer_slashing(state, proposer_slashing, signatures)?; slash_validator(state, proposer_index, None, config)?; Ok(()) } @@ -295,6 +304,7 @@ pub fn process_attester_slashing( state: &mut BeaconState, attester_slashing: &phase0::AttesterSlashing, config: &Config, + signatures: &mut SignatureBatch, ) -> Result<()> { let attestation_1 = &attester_slashing.attestation_1; let attestation_2 = &attester_slashing.attestation_2; @@ -303,12 +313,12 @@ pub fn process_attester_slashing( is_slashable_attestation_data(&attestation_1.data, &attestation_2.data), "is_slashable_attestation_data(attestation_1.data, attestation_2.data)", )?; - verify( - is_valid_indexed_attestation(state, attestation_1), + signatures.push( + indexed_attestation_signature_set(state, attestation_1), "is_valid_indexed_attestation(state, attestation_1)", )?; - verify( - is_valid_indexed_attestation(state, attestation_2), + signatures.push( + indexed_attestation_signature_set(state, attestation_2), "is_valid_indexed_attestation(state, attestation_2)", )?; @@ -345,6 +355,7 @@ pub fn process_attestation( attestation: &phase0::Attestation, _config: &Config, committees: &CommitteeCache, + signatures: &mut SignatureBatch, ) -> Result<()> { let data = attestation.data; let current_epoch = get_current_epoch(state); @@ -419,8 +430,8 @@ pub fn process_attestation( } let indexed_attestation = get_indexed_attestation(state, attestation, committees)?; - verify( - is_valid_indexed_attestation(state, &indexed_attestation), + signatures.push( + indexed_attestation_signature_set(state, &indexed_attestation), "is_valid_indexed_attestation(state, get_indexed_attestation(state, attestation))", )?; @@ -621,6 +632,7 @@ pub fn process_voluntary_exit( state: &mut BeaconState, signed_voluntary_exit: &SignedVoluntaryExit, config: &Config, + signatures: &mut SignatureBatch, ) -> Result<()> { let voluntary_exit = signed_voluntary_exit.message; let current_epoch = get_current_epoch(state); @@ -659,8 +671,8 @@ pub fn process_voluntary_exit( Some(voluntary_exit.epoch), ); let signing_root = compute_signing_root(voluntary_exit.hash_tree_root(), domain); - verify( - bls::verify( + signatures.push( + SignatureSet::single( &validator.pubkey, signing_root, &signed_voluntary_exit.signature, @@ -705,7 +717,12 @@ mod tests { signed_header_2: signed_header, }; - assert!(process_proposer_slashing(&mut state, &slashing, &config).is_err()); + assert!( + crate::beacon::stf::signatures::checked(|s| { + process_proposer_slashing(&mut state, &slashing, &config, s) + }) + .is_err() + ); } #[test] @@ -790,6 +807,11 @@ mod tests { attestation_2, }; - assert!(process_attester_slashing(&mut state, &slashing, &config).is_err()); + assert!( + crate::beacon::stf::signatures::checked(|s| { + process_attester_slashing(&mut state, &slashing, &config, s) + }) + .is_err() + ); } } diff --git a/crates/blockchain/state_transition/src/beacon/stf/signatures.rs b/crates/blockchain/state_transition/src/beacon/stf/signatures.rs new file mode 100644 index 000000000..7717c944d --- /dev/null +++ b/crates/blockchain/state_transition/src/beacon/stf/signatures.rs @@ -0,0 +1,226 @@ +//! The signatures a block's processing defers, so they can be checked together. +//! +//! The specification checks every signature the moment its operation is +//! processed, and each check is a full pairing. A block carries a dozen or so +//! signature sets (the RANDAO reveal, every attestation, the sync aggregate, +//! any exits and slashings), and checking them in one random-linear-combination +//! batch costs a fraction of checking them one by one (see +//! [`ethlambda_crypto::bls::verify_signature_sets`]). So processing *builds* +//! each set where the specification verifies it, pushes it here, and the block +//! driver verifies the whole [`SignatureBatch`] once processing ends. +//! +//! # What may be deferred +//! +//! A signature may be deferred exactly when its failure rejects the block, since +//! then the late rejection and the early one end in the same place: the state +//! the block was applied to is discarded either way. Block import runs the +//! transition on a clone of the parent's state (`transition_block` in +//! `fork_choice.rs`) and never reads that clone after an error, so a state +//! left partly mutated by a block that is rejected a few steps later is never +//! observed. What must stay immediate is a signature whose failure does *not* +//! reject the block but changes what happens next: a deposit with a bad +//! signature is skipped and the block carries on, so that check cannot wait. +//! +//! # What a late failure names +//! +//! Every error is the check's own label (the same `&'static str` the immediate +//! check would have failed with), so a rejected block's error still says which +//! signature was bad. A batch that fails does not know which set did, so it +//! re-checks the sets one at a time in push order and reports the first that +//! fails, which is also the one the immediate checks would have stopped at. +//! The order between a bad signature and some *other* check later in the block +//! can differ from the immediate path: a block with two defects may now be +//! rejected for the later one, or the signature, depending on which the +//! driver reaches first. Which defect is named was never part of the +//! specification, only that the block is invalid. + +use ethlambda_crypto::bls::{SignatureSet, verify_signature_sets}; + +use crate::beacon::error::{Result, verify}; + +/// The signatures a block's processing defers, checked together once it ends. +/// +/// See the module documentation for what may be deferred and why. Outside +/// this crate there are two ways to get one, and both check every set pushed: +/// [`Self::deferred`] verifies the batch once its closure returns, and +/// [`Self::immediate`] checks each set as it is pushed, which is for the +/// callers that apply an operation to a scratch state and must learn at once +/// whether it was accepted. A deferring batch that could be dropped +/// unverified would turn every function taking one into "valid except for +/// its signatures". +#[derive(Default)] +pub struct SignatureBatch { + sets: Vec, + /// `sets[i]`'s label, apart from the sets so the batch check reads them + /// without a copy. + labels: Vec<&'static str>, + immediate: bool, +} + +impl SignatureBatch { + /// An empty batch that defers every set pushed to it. Crate-internal: the + /// caller must [`Self::verify`] it, which [`Self::deferred`] guarantees. + pub(crate) fn new() -> Self { + Self::default() + } + + /// Runs `f` with a batch that defers every set pushed to it, then verifies + /// them all: what a block's driver does, and the only way outside this + /// crate to defer signatures. An error from `f` is returned as is, since + /// it rejects whatever `f` was processing whatever the signatures say. + pub fn deferred(f: impl FnOnce(&mut SignatureBatch) -> Result) -> Result { + let mut batch = Self::new(); + let out = f(&mut batch)?; + batch.verify()?; + Ok(out) + } + + /// An empty batch that checks each set the moment it is pushed. + /// + /// Block production's operation packer applies candidates one after the + /// other to a scratch state and keeps the ones that are accepted. A + /// candidate rejected only at the end of a deferred batch would already + /// have written to the scratch state, which the packer relies on never + /// happening, so its calls check as they go and behave exactly as the + /// immediate checks did. + pub fn immediate() -> Self { + Self { + immediate: true, + ..Self::default() + } + } + + /// Defers `set`, or rejects now with `what` when the set could not be built + /// (an invalid key or signature, an empty signer list): the same moment and + /// the same label today's immediate check fails with. + pub fn push(&mut self, set: Option, what: &'static str) -> Result<()> { + let Some(set) = set else { + return verify(false, what); + }; + if self.immediate { + return verify(set.verify(), what); + } + self.sets.push(set); + self.labels.push(what); + Ok(()) + } + + /// One batch check. On failure, re-checks the sets one by one and rejects with + /// the first failing set's `what`, so the error names the same check it does + /// today. + pub(crate) fn verify(self) -> Result<()> { + if verify_signature_sets(&self.sets) { + return Ok(()); + } + // A batch of valid sets always passes, so one of these fails. Should + // none do (which would be a bug in the batch check), the block is + // still rejected, with the last label, rather than accepted. + let mut last = "block signatures"; + for (set, what) in self.sets.iter().zip(self.labels) { + last = what; + verify(set.verify(), what)?; + } + verify(false, last) + } + + /// Whether no set has been deferred. + #[cfg(test)] + pub(crate) fn is_empty(&self) -> bool { + self.sets.is_empty() + } +} + +/// [`SignatureBatch::deferred`], under the short name the single-operation +/// tests use: they only mean "accepted" once their deferred signatures have +/// been checked. +#[cfg(test)] +pub(crate) fn checked(f: impl FnOnce(&mut SignatureBatch) -> Result) -> Result { + SignatureBatch::deferred(f) +} + +#[cfg(test)] +mod tests { + use blst::min_pk::SecretKey; + + use super::*; + use crate::beacon::bls::DST; + use crate::beacon::primitives::{BlsPubkey, BlsSignature, Root}; + + fn set(seed: u8, valid: bool) -> SignatureSet { + let key = SecretKey::key_gen(&[seed + 1; 32], &[]).unwrap(); + let pubkey = BlsPubkey(key.sk_to_pk().to_bytes()); + let message = Root::repeat_byte(seed); + let signed = if valid { + message + } else { + Root::repeat_byte(seed ^ 0xff) + }; + let signature = BlsSignature(key.sign(signed.as_slice(), DST, &[]).to_bytes()); + SignatureSet::single(&pubkey, message, &signature).unwrap() + } + + #[test] + fn an_empty_batch_verifies() { + assert!(SignatureBatch::new().verify().is_ok()); + } + + #[test] + fn valid_sets_verify_together() { + let mut batch = SignatureBatch::new(); + for seed in 0..5 { + batch.push(Some(set(seed, true)), "valid").unwrap(); + } + assert!(batch.verify().is_ok()); + } + + #[test] + fn a_set_that_could_not_be_built_rejects_at_once_with_its_label() { + let mut batch = SignatureBatch::new(); + let err = batch.push(None, "unbuildable").unwrap_err(); + assert!(format!("{err:?}").contains("unbuildable"), "{err:?}"); + } + + #[test] + fn a_failing_batch_names_the_first_bad_set() { + let mut batch = SignatureBatch::new(); + batch.push(Some(set(0, true)), "first good").unwrap(); + batch.push(Some(set(1, false)), "first bad").unwrap(); + batch.push(Some(set(2, true)), "second good").unwrap(); + batch.push(Some(set(3, false)), "second bad").unwrap(); + let err = batch.verify().unwrap_err(); + assert!(format!("{err:?}").contains("first bad"), "{err:?}"); + } + + #[test] + fn a_deferred_failure_carries_the_label_an_immediate_one_does() { + let mut immediate = SignatureBatch::immediate(); + let early = immediate + .push(Some(set(1, false)), "the label") + .unwrap_err(); + + let mut deferred = SignatureBatch::new(); + deferred.push(Some(set(1, false)), "the label").unwrap(); + let late = deferred.verify().unwrap_err(); + + assert_eq!(format!("{early:?}"), format!("{late:?}")); + } + + #[test] + fn one_bad_signature_among_several_rejects_the_batch() { + let mut batch = SignatureBatch::new(); + for seed in 0..6 { + batch + .push(Some(set(seed, seed != 4)), "attestation") + .unwrap(); + } + assert!(batch.verify().is_err()); + } + + #[test] + fn an_immediate_batch_rejects_on_push() { + let mut batch = SignatureBatch::immediate(); + batch.push(Some(set(0, true)), "good").unwrap(); + let err = batch.push(Some(set(1, false)), "bad").unwrap_err(); + assert!(format!("{err:?}").contains("bad"), "{err:?}"); + } +} diff --git a/crates/blockchain/state_transition/tests/beacon_spec/operations.rs b/crates/blockchain/state_transition/tests/beacon_spec/operations.rs index e627daeba..2c2023283 100644 --- a/crates/blockchain/state_transition/tests/beacon_spec/operations.rs +++ b/crates/blockchain/state_transition/tests/beacon_spec/operations.rs @@ -140,6 +140,7 @@ use ethlambda_state_transition::beacon::stf::deneb as deneb_stf; use ethlambda_state_transition::beacon::stf::electra as electra_stf; use ethlambda_state_transition::beacon::stf::fulu as fulu_stf; use ethlambda_state_transition::beacon::stf::gloas as gloas_stf; +use ethlambda_state_transition::beacon::stf::signatures::SignatureBatch; use ethlambda_state_transition::beacon::stf::{ExecutionEngine, block, operations}; use libtest_mimic::{Failed, Trial}; @@ -173,6 +174,32 @@ fn apply( state: &mut BeaconState, config: &Config, ) -> Result<(), String> { + // Every signature an operation defers is checked once the operation has + // run, which is how a block's own driver treats them: a case whose only + // defect is a bad signature is still rejected, only later. + let mut unhandled = None; + let outcome = SignatureBatch::deferred(|signatures| { + run(handler, case, state, config, signatures).unwrap_or_else(|err| { + unhandled = Some(err); + Ok(()) + }) + }); + if let Some(err) = unhandled { + return Err(err); + } + outcome.map_err(|err| format!("{err:?}")) +} + +/// [`apply`]'s dispatch, pushing every deferred signature into `signatures`. +/// The outer `Err` is a case no handler here covers; the inner result is the +/// crate's own verdict. +fn run( + handler: &str, + case: &Case, + state: &mut BeaconState, + config: &Config, + signatures: &mut SignatureBatch, +) -> 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. @@ -274,27 +301,51 @@ fn apply( // Phase0 defers every attestation's reward to the epoch boundary. ForkName::Phase0 => { let attestation: phase0::Attestation = case.ssz("attestation"); - operations::process_attestation(state, &attestation, config, &committees) + operations::process_attestation( + state, + &attestation, + config, + &committees, + signatures, + ) } // Altair scores an attestation immediately instead, and neither // bellatrix's nor capella's specification lists a further change, // so altair's own function serves both. ForkName::Altair | ForkName::Bellatrix | ForkName::Capella => { let attestation: phase0::Attestation = case.ssz("attestation"); - altair_stf::process_attestation(state, &attestation, &committees, &active_balances) + altair_stf::process_attestation( + state, + &attestation, + &committees, + &active_balances, + signatures, + ) } // Deneb widens the inclusion window and the timely-target // condition (EIP-7045); the container is still phase0's. ForkName::Deneb => { let attestation: phase0::Attestation = case.ssz("attestation"); - deneb_stf::process_attestation(state, &attestation, &committees, &active_balances) + deneb_stf::process_attestation( + state, + &attestation, + &committees, + &active_balances, + signatures, + ) } // Electra reshapes the container itself (EIP-7549's // `committee_bits`), and fulu's specification makes no further // change to either the container or the function. ForkName::Electra | ForkName::Fulu => { let attestation: electra::Attestation = case.ssz("attestation"); - electra_stf::process_attestation(state, &attestation, &committees, &active_balances) + electra_stf::process_attestation( + state, + &attestation, + &committees, + &active_balances, + signatures, + ) } // Gloas reshapes the container a third time (EIP-7688's unbounded // `AttestingIndices`) and modifies the function again (the new @@ -311,6 +362,7 @@ fn apply( meta.parent_slot, &committees, &active_balances, + signatures, ) } ForkName::Lean => lean_is_not_a_fixture_fork("attestation"), @@ -323,7 +375,7 @@ fn apply( | ForkName::Capella | ForkName::Deneb => { let slashing: phase0::AttesterSlashing = case.ssz("attester_slashing"); - operations::process_attester_slashing(state, &slashing, config) + operations::process_attester_slashing(state, &slashing, config, signatures) } // Electra's container widens the same way `Attestation`'s does // (see the `attestation` arm above); the function itself is not @@ -331,14 +383,14 @@ fn apply( // Fulu's specification changes neither. ForkName::Electra | ForkName::Fulu => { let slashing: electra::AttesterSlashing = case.ssz("attester_slashing"); - electra_stf::process_attester_slashing(state, &slashing, config) + electra_stf::process_attester_slashing(state, &slashing, config, signatures) } // Gloas's container widens again (EIP-7688's unbounded // `AttestingIndices`); the function is still not behaviorally // modified, only transcribed against gloas's own type. ForkName::Gloas => { let slashing: gloas::AttesterSlashing = case.ssz("attester_slashing"); - gloas_stf::process_attester_slashing(state, &slashing, config) + gloas_stf::process_attester_slashing(state, &slashing, config, signatures) } ForkName::Lean => lean_is_not_a_fixture_fork("attester_slashing"), }, @@ -357,8 +409,12 @@ fn apply( | ForkName::Capella | ForkName::Deneb | ForkName::Electra - | ForkName::Fulu => operations::process_proposer_slashing(state, &slashing, config), - ForkName::Gloas => gloas_stf::process_proposer_slashing(state, &slashing, config), + | ForkName::Fulu => { + operations::process_proposer_slashing(state, &slashing, config, signatures) + } + ForkName::Gloas => { + gloas_stf::process_proposer_slashing(state, &slashing, config, signatures) + } ForkName::Lean => lean_is_not_a_fixture_fork("proposer_slashing"), } } @@ -405,14 +461,14 @@ fn apply( // Unchanged through capella. ForkName::Phase0 | ForkName::Altair | ForkName::Bellatrix | ForkName::Capella => { let exit: shared::SignedVoluntaryExit = case.ssz("voluntary_exit"); - operations::process_voluntary_exit(state, &exit, config) + operations::process_voluntary_exit(state, &exit, config, signatures) } // Deneb pins the signature to `CAPELLA_FORK_VERSION` (EIP-7044), // so an exit signed long before it takes effect never expires // out from under its own signer at a later fork boundary. ForkName::Deneb => { let exit: shared::SignedVoluntaryExit = case.ssz("voluntary_exit"); - deneb_stf::process_voluntary_exit(state, &exit, config) + deneb_stf::process_voluntary_exit(state, &exit, config, signatures) } // Electra adds a pending-partial-withdrawal check and its own // balance-churn exit-queue accounting (EIP-7251); neither fulu's @@ -422,7 +478,7 @@ fn apply( // `process_voluntary_exit`; see that function's own doc). ForkName::Electra | ForkName::Fulu | ForkName::Gloas => { let exit: shared::SignedVoluntaryExit = case.ssz("voluntary_exit"); - electra_stf::process_voluntary_exit(state, &exit, config) + electra_stf::process_voluntary_exit(state, &exit, config, signatures) } ForkName::Lean => lean_is_not_a_fixture_fork("voluntary_exit"), }, @@ -430,7 +486,7 @@ fn apply( // further fork match. "sync_aggregate" => { let sync_aggregate: altair::SyncAggregate = case.ssz("sync_aggregate"); - altair_stf::process_sync_aggregate(state, &sync_aggregate, &active_balances) + altair_stf::process_sync_aggregate(state, &sync_aggregate, &active_balances, signatures) } // The fixture's operation file is a whole `BeaconBlockBody`, not a // bare payload: checking one needs fields that live on the body @@ -516,7 +572,7 @@ fn apply( // from here on. "bls_to_execution_change" => { let signed_change: capella::SignedBLSToExecutionChange = case.ssz("address_change"); - capella_stf::process_bls_to_execution_change(state, &signed_change, config) + capella_stf::process_bls_to_execution_change(state, &signed_change, config, signatures) } "withdrawals" => match case.fork { ForkName::Capella => { @@ -556,7 +612,7 @@ fn apply( // builder's payload. "execution_payload_bid" => { let signed_bid: gloas::SignedExecutionPayloadBid = case.ssz("execution_payload_bid"); - gloas_stf::process_execution_payload_bid(state, &signed_bid, config) + gloas_stf::process_execution_payload_bid(state, &signed_bid, config, signatures) } // New in gloas (EIP-7732): the *next* block's processing of the // previous slot's payload, so the fixture's operation file is a whole @@ -627,12 +683,12 @@ fn apply( // whether the *previous* slot's payload showed up on time. "payload_attestation" => { let payload_attestation: gloas::PayloadAttestation = case.ssz("payload_attestation"); - gloas_stf::process_payload_attestation(state, &payload_attestation, config) + gloas_stf::process_payload_attestation(state, &payload_attestation, config, signatures) } other => return Err(format!("unhandled operation `{other}`")), }; - outcome.map_err(|err| format!("{err:?}")) + Ok(outcome) } pub fn trials() -> Vec { diff --git a/crates/common/crypto/Cargo.toml b/crates/common/crypto/Cargo.toml index bce0ef64f..c324d55e1 100644 --- a/crates/common/crypto/Cargo.toml +++ b/crates/common/crypto/Cargo.toml @@ -13,6 +13,7 @@ version.workspace = true blst = "0.3.16" ethlambda-metrics.workspace = true ethlambda-types.workspace = true +rand.workspace = true rayon.workspace = true leanvm.workspace = true diff --git a/crates/common/crypto/src/bls.rs b/crates/common/crypto/src/bls.rs index 066ace760..7a5a07631 100644 --- a/crates/common/crypto/src/bls.rs +++ b/crates/common/crypto/src/bls.rs @@ -105,6 +105,9 @@ use ethlambda_types::beacon::primitives::{ BLS_PUBKEY_SIZE, BLS_SIGNATURE_SIZE, BlsPubkey, BlsSignature, Root, }; +mod batch; +pub use batch::{SignatureSet, verify_signature_sets}; + /// The ciphersuite the specification pins BLS signatures to: the IETF BLS /// draft's proof-of-possession scheme over BLS12-381's G2, using SHA-256 in /// the XMD hash-to-curve construction. diff --git a/crates/common/crypto/src/bls/batch.rs b/crates/common/crypto/src/bls/batch.rs new file mode 100644 index 000000000..f9ac8bea0 --- /dev/null +++ b/crates/common/crypto/src/bls/batch.rs @@ -0,0 +1,412 @@ +//! Signature checks resolved ahead of time, so a block's worth of them can be +//! verified in one pairing batch instead of one pairing each. +//! +//! # Why a batch is cheaper +//! +//! A signature check is two parts: resolving its inputs (validating the keys, +//! summing them, decoding the signature) and the pairing equation. The pairing +//! is dominated by its final exponentiation, which every standalone check pays +//! in full. Checking `n` equations `e(pk_i, H(m_i)) = e(g1, sig_i)` together +//! needs one Miller loop per set but only one final exponentiation for all of +//! them, so a block with a dozen signature sets spends a fraction of what a +//! dozen separate checks do. +//! +//! # Why the weights are random +//! +//! Multiplying the equations together unweighted is unsound: an attacker who +//! can pick two invalid signatures `s1 * d` and `s2 / d` (for some group +//! element `d`) makes the product of the two checks hold although neither +//! check does. Each set is therefore weighted by its own random scalar +//! before the products are combined (the random linear combination of +//! ). +//! The weights must be unpredictable to whoever chose the signatures: with +//! independent uniform 64-bit weights, a forged batch passes with probability +//! about 2^-64, which is the soundness level every consensus client accepts. +//! They are drawn from `rand`'s thread-local CSPRNG, which is seeded from the +//! operating system's entropy and reseeded periodically, and a weight of zero +//! is redrawn, since it would drop its set from the equation altogether. +//! +//! # What a batch answers +//! +//! Only whether every set holds. When it fails it cannot say which set did; +//! callers that need to name one check the sets individually afterwards +//! ([`SignatureSet::verify`]), which is also the answer a standalone +//! [`super::verify`] or [`super::fast_aggregate_verify`] gives for the same +//! inputs. + +use blst::BLST_ERROR; +use blst::blst_scalar; +use blst::min_pk::{PublicKey, Signature}; +use rand::RngExt; + +use ethlambda_types::beacon::primitives::{BlsPubkey, BlsSignature, Root}; + +use super::{DST, validated_pubkey, validated_pubkeys}; + +/// How many bits of each random weight the batch check uses. +const RAND_BITS: usize = 64; + +/// The bit of a compressed G1 point's first byte that marks the point at +/// infinity. +const INFINITY_FLAG: u8 = 0b0100_0000; + +/// One signature check, resolved and ready, that may be verified alone or in a +/// batch. +/// +/// Holds the signer's key already validated (and, for an aggregate, summed), +/// the signed message and the signature already validated, so building one is +/// everything a check does short of the pairing. That split is what lets a +/// caller decide *when* to pay for the pairing: at once ([`Self::verify`]) or +/// together with the other sets of a block ([`verify_signature_sets`]). +#[derive(Clone, Copy)] +pub struct SignatureSet { + pubkey: PublicKey, + message: Root, + signature: Signature, + /// Whether `pubkey` is a sum of keys, checked the way + /// `bls.FastAggregateVerify` checks one, rather than a single registry key. + aggregated: bool, +} + +impl SignatureSet { + /// `bls.Verify`'s inputs. + /// + /// `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 { + let pubkey = validated_pubkey(pubkey)?; + let signature = Signature::sig_validate(signature.as_ref(), false).ok()?; + Some(Self { + pubkey, + message, + signature, + aggregated: false, + }) + } + + /// `bls.FastAggregateVerify`'s inputs. + /// + /// `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], + message: Root, + signature: &BlsSignature, + ) -> Option { + if pubkeys.is_empty() { + return None; + } + let signature = Signature::sig_validate(signature.as_ref(), false).ok()?; + let points = validated_pubkeys(pubkeys)?; + // Every point above already passed `key_validate`, so the sum does + // not re-check them. + let pubkey = super::sum_validated_points(&points)?; + Some(Self { + pubkey, + message, + signature, + aggregated: true, + }) + } + + /// The pairing check on its own: the same answer [`super::verify`] or + /// [`super::fast_aggregate_verify`] gives for these inputs. + pub fn verify(&self) -> bool { + let result = if self.aggregated { + self.signature.fast_aggregate_verify_pre_aggregated( + false, + self.message.as_slice(), + DST, + &self.pubkey, + ) + } else { + self.signature.verify( + false, + self.message.as_slice(), + DST, + &[], + &self.pubkey, + false, + ) + }; + result == BLST_ERROR::BLST_SUCCESS + } + + /// Whether the key is the identity, which no signature can validly be + /// made under. Only a sum of keys can be: a single registry key is + /// rejected by `key_validate` if it is. + /// + /// blst rejects the same input itself (`BLST_PK_IS_INFINITY`, from the + /// pairing aggregation both the single and the batch check go through), + /// so this makes the existing rule explicit and changes no behavior. + fn key_is_identity(&self) -> bool { + self.pubkey.to_bytes()[0] & INFINITY_FLAG != 0 + } +} + +/// A random weight in `1..2^64`, as the little-endian scalar `blst` expects. +fn random_weight(rng: &mut impl RngExt) -> blst_scalar { + let weight = loop { + let candidate: u64 = rng.random(); + if candidate != 0 { + break candidate; + } + }; + let mut scalar = blst_scalar::default(); + scalar.b[..8].copy_from_slice(&weight.to_le_bytes()); + scalar +} + +/// Whether every set verifies, in one random-linear-combination check: +/// `verify_multiple_aggregate_signatures` with 64-bit scalars from the +/// OS-seeded RNG and one final exponentiation. An empty slice verifies. +/// +/// See the module documentation for why the weights are random. A single set +/// skips them and is verified directly, since one equation needs no +/// combining. A set whose key is the identity fails the batch, as it fails +/// [`SignatureSet::verify`]. blst rejects that input itself, so the explicit +/// check only states the rule where a reader of the batch looks for it. +pub fn verify_signature_sets(sets: &[SignatureSet]) -> bool { + match sets { + [] => return true, + [only] => return only.verify(), + _ => {} + } + if sets.iter().any(SignatureSet::key_is_identity) { + return false; + } + let mut rng = rand::rng(); + let weights: Vec = sets.iter().map(|_| random_weight(&mut rng)).collect(); + let messages: Vec<&[u8]> = sets.iter().map(|set| set.message.as_slice()).collect(); + let pubkeys: Vec<&PublicKey> = sets.iter().map(|set| &set.pubkey).collect(); + let signatures: Vec<&Signature> = sets.iter().map(|set| &set.signature).collect(); + // Keys and signatures were validated when the sets were built. + let result = Signature::verify_multiple_aggregate_signatures( + &messages, + DST, + &pubkeys, + false, + &signatures, + false, + &weights, + RAND_BITS, + ); + result == BLST_ERROR::BLST_SUCCESS +} + +#[cfg(test)] +mod tests { + use blst::min_pk::SecretKey; + + use super::*; + use crate::bls; + + fn secret_key(seed: u8) -> SecretKey { + let mut ikm = [seed; 32]; + ikm[0] = ikm[0].wrapping_add(1); + SecretKey::key_gen(&ikm, &[]).unwrap() + } + + fn pubkey_of(key: &SecretKey) -> BlsPubkey { + BlsPubkey(key.sk_to_pk().to_bytes()) + } + + fn sign(key: &SecretKey, message: Root) -> BlsSignature { + BlsSignature(key.sign(message.as_slice(), DST, &[]).to_bytes()) + } + + fn message(byte: u8) -> Root { + Root::from([byte; 32]) + } + + /// `n` single-key sets over distinct messages, all valid. + fn valid_sets(n: u8) -> Vec { + (0..n) + .map(|i| { + let key = secret_key(i); + let msg = message(i); + SignatureSet::single(&pubkey_of(&key), msg, &sign(&key, msg)).unwrap() + }) + .collect() + } + + #[test] + fn a_valid_batch_of_ten_verifies() { + assert!(verify_signature_sets(&valid_sets(10))); + } + + #[test] + fn an_empty_batch_verifies() { + assert!(verify_signature_sets(&[])); + } + + #[test] + fn a_single_set_batch_verifies_or_fails_alone() { + assert!(verify_signature_sets(&valid_sets(1))); + let key = secret_key(1); + let wrong = + SignatureSet::single(&pubkey_of(&key), message(9), &sign(&key, message(8))).unwrap(); + assert!(!verify_signature_sets(&[wrong])); + } + + #[test] + fn one_bad_set_among_ten_fails() { + for bad in [0u8, 4, 9] { + let mut sets = valid_sets(10); + let key = secret_key(bad); + // A well-formed signature over a different message. + sets[usize::from(bad)] = + SignatureSet::single(&pubkey_of(&key), message(bad), &sign(&key, message(200))) + .unwrap(); + assert!(!verify_signature_sets(&sets), "bad set {bad}"); + } + } + + #[test] + fn swapped_signatures_fail() { + let (k0, k1) = (secret_key(0), secret_key(1)); + let (m0, m1) = (message(0), message(1)); + let (s0, s1) = (sign(&k0, m0), sign(&k1, m1)); + let a = SignatureSet::single(&pubkey_of(&k0), m0, &s1).unwrap(); + let b = SignatureSet::single(&pubkey_of(&k1), m1, &s0).unwrap(); + assert!(!a.verify()); + assert!(!b.verify()); + assert!(!verify_signature_sets(&[a, b])); + } + + #[test] + fn repeated_messages_across_sets_verify() { + let msg = message(7); + let sets: Vec = (0..6) + .map(|i| { + let key = secret_key(i); + SignatureSet::single(&pubkey_of(&key), msg, &sign(&key, msg)).unwrap() + }) + .collect(); + assert!(verify_signature_sets(&sets)); + } + + #[test] + fn single_and_aggregate_sets_mix_in_one_batch() { + let mut sets = valid_sets(3); + let keys: Vec = (10..14).map(secret_key).collect(); + let msg = message(99); + let signatures: Vec = keys.iter().map(|key| sign(key, msg)).collect(); + let aggregate = bls::aggregate(&signatures).unwrap(); + let pubkeys: Vec = keys.iter().map(pubkey_of).collect(); + sets.push(SignatureSet::fast_aggregate(&pubkeys, msg, &aggregate).unwrap()); + assert!(verify_signature_sets(&sets)); + } + + #[test] + fn verify_agrees_with_bls_verify() { + let key = secret_key(3); + let pubkey = pubkey_of(&key); + let msg = message(3); + let good = sign(&key, msg); + let wrong_message = sign(&key, message(4)); + let other_key = sign(&secret_key(5), msg); + for signature in [good, wrong_message, other_key] { + let set = SignatureSet::single(&pubkey, msg, &signature).unwrap(); + assert_eq!(set.verify(), bls::verify(&pubkey, msg, &signature)); + } + } + + #[test] + fn verify_agrees_with_fast_aggregate_verify() { + let keys: Vec = (0..5).map(secret_key).collect(); + let pubkeys: Vec = keys.iter().map(pubkey_of).collect(); + let msg = message(42); + let signatures: Vec = keys.iter().map(|key| sign(key, msg)).collect(); + let good = bls::aggregate(&signatures).unwrap(); + let short = bls::aggregate(&signatures[1..]).unwrap(); + for (signature, message) in [(good, msg), (short, msg), (good, message(43))] { + let set = SignatureSet::fast_aggregate(&pubkeys, message, &signature).unwrap(); + assert_eq!( + set.verify(), + bls::fast_aggregate_verify(&pubkeys, message, &signature) + ); + } + } + + #[test] + fn invalid_inputs_build_no_set() { + let key = secret_key(1); + let pubkey = pubkey_of(&key); + let msg = message(1); + let signature = sign(&key, msg); + let garbage_key = BlsPubkey([0xff; 48]); + let garbage_signature = BlsSignature([0xff; 96]); + + 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(&[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. + let infinity = BlsSignature(bls::G2_POINT_AT_INFINITY); + let set = SignatureSet::single(&pubkey, msg, &infinity).unwrap(); + assert!(!set.verify()); + } + + #[test] + fn an_infinity_signature_among_valid_sets_fails_the_batch() { + let mut sets = valid_sets(4); + let key = secret_key(1); + let infinity = BlsSignature(bls::G2_POINT_AT_INFINITY); + sets[1] = SignatureSet::single(&pubkey_of(&key), message(1), &infinity).unwrap(); + assert!(!verify_signature_sets(&sets)); + } + + /// `-P`, from `P`'s compressed encoding (the sort flag picks the other + /// `y`). Valid on its own, so a registry can hold both. + fn negated(pubkey: BlsPubkey) -> BlsPubkey { + let mut bytes = pubkey.0; + bytes[0] ^= 0b0010_0000; + BlsPubkey(bytes) + } + + #[test] + fn keys_that_cancel_never_verify_alone_or_in_a_batch() { + // `[P, -P]` sums to the identity, and with the identity signature that + // is the cancellation the rule against identity keys exists for. A + // real signature over the message must not pass either. + let key = secret_key(20); + let pubkey = pubkey_of(&key); + let pair = [pubkey, negated(pubkey)]; + let msg = message(20); + let infinity = BlsSignature(bls::G2_POINT_AT_INFINITY); + for signature in [infinity, sign(&key, msg)] { + let set = SignatureSet::fast_aggregate(&pair, msg, &signature).unwrap(); + assert!(set.key_is_identity()); + assert!(!set.verify()); + assert_eq!( + set.verify(), + bls::fast_aggregate_verify(&pair, msg, &signature) + ); + + let mut sets = valid_sets(3); + sets.push(set); + assert!(!verify_signature_sets(&sets)); + } + + // `P + P - P` is `P`, so `P`'s own signature verifies. + let three = [pubkey, pubkey, negated(pubkey)]; + let set = SignatureSet::fast_aggregate(&three, msg, &sign(&key, msg)).unwrap(); + assert!(!set.key_is_identity()); + let mut sets = valid_sets(3); + sets.push(set); + assert!(verify_signature_sets(&sets)); + } + + #[test] + fn random_weights_are_never_zero() { + let mut rng = rand::rng(); + for _ in 0..10_000 { + let weight = random_weight(&mut rng); + assert!(weight.b[..8].iter().any(|byte| *byte != 0)); + assert!(weight.b[8..].iter().all(|byte| *byte == 0)); + } + } +} diff --git a/crates/net/rpc/src/beacon/validator_client_tests.rs b/crates/net/rpc/src/beacon/validator_client_tests.rs index b62b61cdc..4de1e9957 100644 --- a/crates/net/rpc/src/beacon/validator_client_tests.rs +++ b/crates/net/rpc/src/beacon/validator_client_tests.rs @@ -1006,12 +1006,15 @@ fn assert_full_sync_aggregate( "every member signed the parent" ); let mut state = advanced.clone(); - ethlambda_state_transition::beacon::stf::altair::process_sync_aggregate( - &mut state, - aggregate, - ðlambda_state_transition::beacon::helpers::accessors::ActiveBalanceCache::default(), - ) - .expect("the packed aggregate verifies as the state transition checks it"); + ethlambda_state_transition::beacon::stf::signatures::SignatureBatch::deferred(|signatures| { + ethlambda_state_transition::beacon::stf::altair::process_sync_aggregate( + &mut state, + aggregate, + ðlambda_state_transition::beacon::helpers::accessors::ActiveBalanceCache::default(), + signatures, + ) + }) + .expect("the packed aggregate is well formed and verifies as the state transition checks it"); } /// A sync committee member's slot through this node: the duties and