Skip to content

perf(beacon): verify a block's signatures in one random-weight batch - #738

Open
MegaRedHand wants to merge 6 commits into
perf/bls-batched-key-sumsfrom
perf/block-signature-batch
Open

MegaRedHand wants to merge 6 commits into
perf/bls-batched-key-sumsfrom
perf/block-signature-batch

Conversation

@MegaRedHand

@MegaRedHand MegaRedHand commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Stack 2/4 of the BLS verification work: (a) batched key sums -> (b) per-block signature batch -> (c) committee key sums -> (d) index-keyed pubkey cache. Based on (a).

What changed and why

Block import checked every signature with its own pairing: the proposer's, the RANDAO reveal, each attestation, the sync aggregate and any slashings, exits and BLS changes. A pairing check is dominated by its final exponentiation, which a batch pays once for all sets.

  • ethlambda-crypto: new bls/batch.rs with SignatureSet (single, fast_aggregate, verify) and verify_signature_sets. A set is the resolved inputs of a check (validated and summed key, message, validated signature); a batch is verify_multiple_aggregate_signatures with 64-bit non-zero weights from rand's OS-seeded CSPRNG. Random weights are required: without them two invalid signatures can be crafted to cancel; with unpredictable 64-bit weights a forger succeeds with probability about 2^-64. A single set skips the weights. A set whose summed key is the identity fails the batch explicitly, as it fails alone.
  • ethlambda-state-transition: new stf/signatures.rs with SignatureBatch. Operations build their set where the specification verifies it and push it with the same label the immediate check used; on a batch failure the sets are re-checked one by one and the first failing label is the rejection. Outside the crate there are only two ways to get a batch, and both check every set: SignatureBatch::deferred(|batch| ...) verifies once the closure returns, and immediate() checks at push time (see below). new() and verify() are crate-internal, so a caller cannot drop a deferring batch unverified.
  • stf::block::process_block keeps its signature and still means "the block is valid": it runs the body inside SignatureBatch::deferred, so the batch is verified before it returns. apply_block checks the proposer's block signature on its own first, as before this PR: a block fetched by request reaches the STF without gossip's precheck_block, so a block its proposer never signed is still rejected after one pairing rather than after the whole body. The body's batch is verified before the state root is computed.
  • The attestation validity helpers (phase0/altair, electra, gloas, payload attestations) are split into a set builder plus the existing bool function (builder(...).is_some_and(|set| set.verify())). Every structural check stays immediate; gossip, fork choice and block production keep calling the bool functions.

Stacked on part a (batched key sums): SignatureSet::fast_aggregate sums its keys with sum_validated_points. blst already rejects an infinity public key on both the single and the batch path; the explicit identity check in verify_signature_sets states that rule and changes no behavior.

Deferred and immediate signatures

Deferred (failure rejects the block): RANDAO reveal (block.rs), proposer slashing headers (operations.rs, shared by gloas), attester slashings (phase0, electra, gloas), attestations (phase0, altair, deneb, electra, gloas), voluntary exits (phase0, deneb, electra), BLS to execution changes (capella.rs), sync aggregate (altair.rs; empty participants keep the infinity-signature rule and push nothing), gloas execution payload bid, gloas payload attestations.

Immediate, unchanged (failure changes state but does not reject): every deposit signature (operations.rs, electra.rs, epoch/electra.rs) and the gloas builder deposit signature. The gloas envelope signature is not reachable from process_block.

Behavior notes

  • Every accepted input is still accepted and every rejected one rejected. A rejected block now reports the first failing signature's label, but another defect later in the block may be reported first or instead; the label of a bad signature is the one the immediate check used.
  • An Err now can leave the state partly mutated by operations whose signature was checked at the end. Import runs the transition on a clone (transition_block, fork_choice.rs) and every caller discards the state after an error.
  • The operation packer in block production applies candidates in place and relies on a rejected candidate writing nothing. It therefore passes SignatureBatch::immediate(), which checks at push time and keeps that guarantee.
  • gloas::process_block_and_verify is the helper for the direct gloas drivers in block production.

Immediate, as on main: the proposer's block signature (apply_block), so a forged block stays cheap to reject.

Measured

On a loaded M3, the pairing per block took 2.7-4x less wall time with one batch. That ratio mixes two effects: fewer final exponentiations, and blst's verify_multiple_aggregate_signatures spreading the Miller loops over its own pool where the old per-set check used one worker. CPU time drops too: profiling a Hoodi block replay on a Ryzen 9700X (the whole stack, warm caches) puts pairing and hash-to-curve at 7.9 ms CPU per block before and 2.4 ms after. That run predates this PR's review fix, which takes the proposer's signature back out of the batch, so it now costs about one more pairing per block than measured.

Tests

  • cargo test -p ethlambda-crypto --profile release-fast --lib: the batch tests cover a valid batch of ten, one bad among ten, swapped signatures, empty batch, single set, repeated messages, mixed single and aggregate, agreement with bls::verify and bls::fast_aggregate_verify, None for invalid inputs, weights never zero, an infinity signature among valid sets, and keys that cancel ([P, -P] never verifies, alone or in a batch, with the identity signature or a real one; [P, P, -P] verifies under P's signature).
  • cargo test -p ethlambda-state-transition --profile release-fast --lib --features beacon-spec-tests: 655 passed, 0 failed, rerun after the merge (includes the BLS fixture vectors and the new tests: a bad RANDAO reveal is deferred and rejected by the batch, a valid one passes, RandaoVerification::Skip pushes nothing, deferred and immediate failures carry the same label, one bad signature among several rejects the batch).
  • cargo test -p ethlambda-state-transition --profile release-fast --test beacon_spec_tests --features beacon-spec-tests (unfiltered, mainnet preset): 7493 passed, 0 failed, 977 ignored. The operation runner builds a batch around each operation and treats an Err from verify() as the rejection.
  • Filtered beacon_spec_tests after the merge: /attestation/ 356 passed, /sync_aggregate/ 182 passed, sanity/ 595 passed, 0 failed.
  • cargo clippy --profile release-fast -p ethlambda-crypto -p ethlambda-state-transition -p ethlambda-rpc --all-targets -D warnings clean, cargo fmt --all applied.

On this branch after its review fixes: crypto --lib 32 passed; state-transition --lib 655 passed; beacon_spec_tests operations/ 2141, sanity/ 595, finality/ 41, fork_choice/ 206, random/ 1969 (175 ignored), transition/ 172, sync/ 6 passed; the rpc validator-client tests pass.

Rerun after the review fixes, on the stack's tip (all four PRs, --profile release-fast): ethlambda-crypto --lib 47 passed, 14 ignored; ethlambda-state-transition --lib --features beacon-spec-tests (with the BLS fixture vectors) 658 passed; storage 213, blockchain 291, p2p 362, rpc 335 passed; full beacon_spec_tests 7493 passed, 977 ignored, 0 failed; make lint clean, also on a trial merge of current main.

Block import paid one pairing per signature: the proposer's, the RANDAO
reveal, every attestation, the sync aggregate and each slashing, exit and
BLS change. A pairing is dominated by its final exponentiation, which a
batch pays once, so a block with a dozen signature sets checks in a
fraction of the time.

ethlambda-crypto gains SignatureSet and verify_signature_sets: resolved
checks combined with unpredictable non-zero 64-bit weights from the
OS-seeded CSPRNG (without them two invalid signatures can cancel). The
state transition builds each set where the specification verifies it and
pushes it into a SignatureBatch that the block driver verifies once
processing ends. A failing batch re-checks sets one by one so the error
still names the first bad signature with its usual label.

Signatures whose failure rejects the block are deferred; deposit
signatures, whose failure only skips the deposit, stay immediate, as does
epoch processing. process_block keeps its meaning and signature, apply_block
adds the proposer's signature to the same batch. The operation packer in
block production uses an immediate batch so a rejected candidate still
writes nothing. The attestation validity helpers are split into a set
builder plus the existing bool function, so gossip and fork choice are
untouched.
SignatureSet::fast_aggregate now sums its keys with part a's batched
sum_validated_points instead of a sequential AggregatePublicKey::aggregate,
and the identity-key check in verify_signature_sets documents that blst
rejects the same input.
@MegaRedHand
MegaRedHand added this pull request to stack #744 October 30, 2026 16:08
MegaRedHand added a commit that referenced this pull request Oct 30, 2026
…738)

perf(beacon): verify a block's signatures in one random-weight batch

Block import paid one pairing per signature: the proposer's, the RANDAO
reveal, every attestation, the sync aggregate and each slashing, exit and
BLS change. A pairing is dominated by its final exponentiation, which a
batch pays once, so a block with a dozen signature sets checks in a
fraction of the time.

ethlambda-crypto gains SignatureSet and verify_signature_sets: resolved
checks combined with unpredictable non-zero 64-bit weights from the
OS-seeded CSPRNG (without them two invalid signatures can cancel). The
state transition builds each set where the specification verifies it and
pushes it into a SignatureBatch that the block driver verifies once
processing ends. A failing batch re-checks sets one by one so the error
still names the first bad signature with its usual label.

Signatures whose failure rejects the block are deferred; deposit
signatures, whose failure only skips the deposit, stay immediate, as does
epoch processing. process_block keeps its meaning and signature, apply_block
adds the proposer's signature to the same batch. The operation packer in
block production uses an immediate batch so a rejected candidate still
writes nothing. The attestation validity helpers are split into a set
builder plus the existing bool function, so gossip and fork choice are
untouched.

Cherry-picked from perf/block-signature-batch at 7736808.
…tch unverified

A block fetched by request reaches the state transition without gossip's
precheck, and with the proposer's signature in the body's batch a block its
proposer never signed was only rejected after the whole body ran. Check that
one signature on its own again, before the body, as before the batch existed.

A deferring SignatureBatch could also be built and dropped without its sets
ever being checked, turning any function that takes one into "valid except for
its signatures". Outside the crate, a deferring batch now only comes from
SignatureBatch::deferred, which verifies once its closure returns.

Also cover an infinity signature inside a batch and keys that cancel.
@github-actions

Copy link
Copy Markdown

🤖 Codex Code Review

Looks good overall. The PR is a sensible refactor toward reusable signature batching, and I did not spot a consensus-breaking issue in the changed paths.

  • crates/blockchain/state_transition/src/beacon/block_production.rs:457 is the most important correctness point, and it looks right: using SignatureBatch::immediate() preserves the old “reject before mutating scratch state” behavior for pool packing. That avoids a subtle block-production bug where invalid attestations/exits/slashings could otherwise leak mutations into scratch.

  • crates/blockchain/state_transition/src/beacon/gloas_block_production.rs:406 and crates/blockchain/state_transition/src/beacon/gloas_block_production.rs:532 are also correct in spirit: switching block production from process_block to process_block_and_verify closes a real validation gap. Without that, produced gloas blocks could compute a state root from a post-state that includes deferred-but-never-verified signatures.

  • crates/blockchain/state_transition/src/beacon/helpers/attestation.rs:94, crates/blockchain/state_transition/src/beacon/helpers/electra.rs:294, and crates/blockchain/state_transition/src/beacon/helpers/gloas.rs:138 preserve the old attestation validity checks while exposing an Option<SignatureSet>. Returning None for malformed index sets or out-of-range validator indices is the right shape for callers that batch signatures.

  • Performance is improved or neutral in the hot path: deferring attestation pairings into SignatureBatch remains the same model, and the new helpers avoid redoing structural checks. The extra allocation of Vec<Pubkey> was already present before in the old fast_aggregate_verify path.

  • Security/memory-safety wise, I don’t see unsafe behavior or error-swallowing regressions. The Option<SignatureSet> API is also safer than a boolean helper for callers that need to distinguish “cannot even build a valid signature set” from “built, but pairing failed”.

  • Readability is good. The doc comments around immediate vs deferred verification in crates/blockchain/state_transition/src/beacon/stf/signatures.rs:78 make the invariant explicit, which is important for consensus code.

No requested changes from me. If you want, I can also do a second pass specifically on whether PR 738 needs new regression tests around gloas block production and attestation batching.


Automated review by OpenAI Codex · gpt-5.4 · custom prompt

@github-actions

Copy link
Copy Markdown

🤖 Claude Code Review

Review: PR 738, per-block BLS signature batch

I read the new bls/batch.rs and stf/signatures.rs, and the changes to apply_block, process_block*, the operations and the attestation helpers. I did not build or run the tests.

Overall: the design is sound and I found no blocking issues.

Soundness of the batch

  • bls/batch.rs weights each set with a non-zero 64-bit scalar from the OS-seeded rand::rng(). That is the standard random-linear-combination construction, and the doc comment explains why the weights must be random.
  • Keys and signatures are validated when each set is built, so passing false for the group-check flags in verify_multiple_aggregate_signatures is correct.
  • The explicit identity-key check (0x40 infinity flag on byte 0 of the compressed key) is correct. A single set skips it and goes through verify(), which is also fine.
  • sync_committee_bits with no participants is still decided up front by comparing the signature to G2_POINT_AT_INFINITY. It never builds a set, which avoids an empty fast_aggregate.

Deferral semantics

  • Deferring is safe where the block is rejected anyway. The signatures.rs module doc argues this well.
  • Deposits correctly stay immediate, because a bad deposit signature skips the deposit instead of rejecting the block.
  • apply_block pushes the proposer's signature first, so a block whose only defect is that signature keeps its error label.
  • signatures.verify() runs before apply_pending_mutations and the state-root computation, so a bad block does not pay for hashing.
  • Making new() and verify() crate-internal, with only deferred/immediate public, rules out dropping an unverified batch. The test-only checked helper covers the unit tests.
  • Block production's pack_each uses SignatureBatch::immediate(), which keeps its "rejected candidate leaves scratch untouched" invariant.

Minor points (non-blocking)

  1. Cost of a failing batch (signatures.rs, SignatureBatch::verify). On failure it re-checks every set serially. A block whose last signature is bad now costs one batch plus n single pairings, where the old path cost about n. A bad block still costs bounded work, since the proposer signature is checked before the body, so this is probably acceptable. A comment saying so would help. Bisecting the batch would cut the cost if it ever matters.
  2. Block-level error order changed. A block with several defects can now be rejected for a later structural error before the bad signature is reported, or the reverse. The module doc already says this. The spec fixtures only assert that the block is rejected, so this should be fine, but it is worth confirming make test-beacon passes on both presets.
  3. Partly mutated state after Err (block.rs, process_block doc). Callers must discard state after an error. That holds today (transition_block clones the parent state), but nothing enforces it. A one-line pointer in process_block_with_signatures would help future callers.
  4. Duplicated set builders. The indexed_attestation_signature_set(_with_domain) code is copied across phase0, electra and gloas, and each copy builds its pubkey vector with the same loop. Part (c) of the stack, committee key sums, may remove this. If not, a shared generic helper would cut the duplication.
  5. SignatureSet is Copy (batch.rs). verify() copies every set into a fresh Vec (self.sets.iter().map(|(set, _)| *set)). This is cheap, but it could take a &[SignatureSet] view if the label were stored in a parallel vector.

LGTM, pending CI on the spec fixtures for both presets.


Automated review by Claude (Anthropic) · sonnet · custom prompt

@github-actions

Copy link
Copy Markdown

🤖 Kimi Code Review

I'll review this PR which implements BLS signature batching for block processing. This is a significant optimization and correctness change for consensus-layer signature verification.

High-Level Assessment

The PR introduces deferred signature batching across all beacon state transition forks, replacing individual pairing checks with a single random-linear-combination batch verification. This is a well-known optimization (2-3x faster for typical blocks), but requires careful handling because:

  1. Some signatures must remain immediate (deposits with bad signatures are skipped, not rejected)
  2. Deferred verification means state mutations happen before signature checks
  3. The batch failure must identify which signature failed for debugging/spec compliance

Detailed Review

1. Core Batching Logic (crates/common/crypto/src/bls/batch.rs)

Line 78-82: RAND_BITS and security analysis

const RAND_BITS: usize = 64;

The 64-bit random weights provide ~2^-64 soundness, which is acceptable for consensus. However, I note that blst's verify_multiple_aggregate_signatures with RAND_BITS uses a specific multi-scalar multiplication path. The documentation correctly cites the ethresear.ch post.

Line 183-195: key_is_identity check

fn key_is_identity(&self) -> bool {
    self.pubkey.to_bytes()[0] & INFINITY_FLAG != 0
}

This is correct but fragile—it assumes specific encoding details of blst's compressed G1 format. The INFINITY_FLAG constant is well-defined in the BLS12-381 serialization spec, but this should have a comment referencing the spec (Zcash BLS12-381 §1.2 or similar).

Line 206-217: random_weight

fn random_weight(rng: &mut impl RngExt) -> blst_scalar {
    let weight = loop {
        let candidate: u64 = rng.random();
        if candidate != 0 {
            break candidate;
        }
    };

Good: zero weights are explicitly rejected. The RngExt trait usage is correct for rand 0.10.

Line 237-250: verify_signature_sets

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;
    }

The single-set optimization is correct and important. The identity key check is necessary because blst's verify_multiple_aggregate_signatures may not reject identity pubkeys in all cases (the behavior is aggregation-dependent).

Potential issue: verify_multiple_aggregate_signatures parameters

Looking at line 253-261:

let result = Signature::verify_multiple_aggregate_signatures(
    &messages,
    DST,
    &pubkeys,
    false,
    &signatures,
    false,
    &weights,
    RAND_BITS,
);

The false parameters are sig_groupcheck and pks_validate. Since keys and signatures were validated at SignatureSet construction time, skipping re-validation is correct. However, I want to verify: does verify_multiple_aggregate_signatures perform the final exponentiation once? Yes, this is blst's documented behavior for this API.

2. SignatureBatch State Management (crates/blockchain/state_transition/src/beacon/stf/signatures.rs)

Line 88-96: SignatureBatch::deferred

pub fn deferred<T>(f: impl FnOnce(&mut SignatureBatch) -> Result<T>) -> Result<T> {
    let mut batch = Self::new();
    let out = f(&mut batch)?;
    batch.verify()?;
    Ok(out)
}

This is the critical API. The closure receives &mut SignatureBatch, and verification happens after f returns successfully. Important correctness property: if f returns Err, the batch is dropped unverified, which is correct because the block is already rejected.

Line 104-111: SignatureBatch::immediate

pub fn immediate() -> Self {
    Self {
        immediate: true,
        ..Self::default()
    }
}

Used in block production's pack_each for scratch-state candidate testing. This preserves the "reject before write" invariant.

Line 113-123: push

pub fn push(&mut self, set: Option<SignatureSet>, 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(())
}

Correct: None from SignatureSet::single/fast_aggregate means invalid key/signature/empty-signer-list, which fails immediately with the same label.

Line 128-142: verify with fallback

pub(crate) fn verify(self) -> Result<()> {
    if verify_signature_sets(&self.sets) {
        return Ok(());
    }
    // ... fallback to individual verification
    let mut last = "block signatures";
    for (set, what) in self.sets.iter().zip(self.labels) {
        last = what;
        verify(set.verify(), what)?;
    }
    verify(false, last)
}

The fallback is necessary for error attribution. However, there's a subtle issue: if the batch fails but no individual set fails (shouldn't happen with correct crypto, but defense in depth), the final verify(false, last) catches it.

Line 142: Potential panic source

verify(false, last)

This will panic via verify if somehow reached. Actually, looking at the loop: if all individual set.verify() pass, we reach verify(false, last) which will panic. This is unreachable with correct BLS math, but consider making this a hard error rather than panic, or document the invariant.

3. Fork-Specific Integration

Phase0/Altair (crates/blockchain/state_transition/src/beacon/stf/altair.rs)

Line 89-97: process_attestation now takes signatures: &mut SignatureBatch

Line 137-145: The attestation signature is deferred:

signatures.push(
    indexed_attestation_signature_set(state, &indexed_attestation),
    "is_valid_indexed_attestation(state, get_indexed_attestation(state, attestation))",
)?;

Line 310-345: process_sync_aggregate — critical special case handled correctly

The eth_fast_aggregate_verify with zero participants requires the signature to be the point at infinity. This is checked immediately (not deferred) because there's no pairing to batch:

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(...),
        ...
    )?;
}

This is correct per the spec and preserves the "no participants = infinity signature" rule.

Bellatrix through Fulu: #[allow(clippy::too_many_arguments)]

Multiple functions now have 8+ parameters due to the signatures parameter. The allow attributes are pragmatic but suggest these functions should perhaps be refactored into struct-based configuration. Not blocking for this PR.

4. Gloas-Specific Changes

Line 106-140: process_block_and_verify

New public function for gloas block production that wraps process_block with SignatureBatch::deferred. This is necessary because gloas block production calls process_block directly rather than through the generic process_block dispatch.

Line 786-808: execution_payload_bid_signature_set

Returns Result<Option<SignatureSet>> — the Result is for state access errors (builder index out of range), Option is for invalid key/signature. This is correctly propagated:

signatures.push(
    execution_payload_bid_signature_set(state, signed_bid)?,
    "process_execution_payload_bid: verify_execution_payload_bid_signature(state, signed_bid)",
)?;

Note: the ? propagates Result::Err (state access failure) immediately, while None from Ok(None) would be caught by push and converted to a verification failure. This is correct: a missing builder is a state error, not a signature failure.

5. Test Infrastructure

crates/blockchain/state_transition/tests/beacon_spec/operations.rs

The test wrapper apply now uses SignatureBatch::deferred:

let mut unhandled = None;
let outcome = SignatureBatch::deferred(|signatures| {
    run(handler, case, state, config, signatures).unwrap_or_else(|err| {
        unhandled = Some(err);
        Ok(())
    })
});

This is slightly awkward. The unhandled captures non-handler errors (unknown operation names). The unwrap_or_else converts the inner Result to always succeed for the closure, letting SignatureBatch::deferred verify signatures even for unknown operations... wait, no: if run returns Err(String) (unknown handler), the closure returns Ok(()) and signatures are verified on an empty batch, then unhandled is checked.

Actually, re-reading: run returns Result<Result<(), Error>, String> — outer Err is "no handler", inner Err is state transition failure. The closure must return Result<()> to match SignatureBatch::deferred's bound.

The pattern:

let outcome = SignatureBatch::deferred(|signatures| {
    run(...).unwrap_or_else(|err| { unhandled = Some(err); Ok(()) })
});

If run returns Err("unknown handler"), we set unhandled and return Ok(()). The deferred batch verifies (empty, so succeeds), then we check unhandled and return that error.

If run returns Ok(Err(state_transition_error)), the closure returns Err(state_transition_error), SignatureBatch::deferred short-circuits without verifying, and we return that error.

If run returns Ok(Ok(())), signatures are verified, and we return Ok(()).

This is correct but the control flow is subtle. A comment explaining this would help.

6. rand Dependency Addition

Cargo.lock and crates/common/crypto/Cargo.toml

rand 0.10.1 added to ethlambda-state-transition (lockfile) and rand.workspace = true in ethlambda-crypto. The workspace already has rand elsewhere; this is consistent.

7. Block Production (pack_each)

crates/blockchain/state_transition/src/beacon/block_production.rs

Line 443-462:

fn pack_each<T>(
    candidates: Vec<T>,
    max: usize,
    config: &Config,
    apply: impl Fn(&mut BeaconState, &T, &Config, &mut SignatureBatch) -> Result<()>,
) -> Vec<T> {
    // ...
    let mut signatures = SignatureBatch::immediate();
    if apply(scratch, &candidate, config, &mut signatures).is_ok() {
        packed.push(candidate);
    }
}

The comment explains why immediate is used: "a bad signature rejects the candidate before it writes, which is what lets a rejected candidate leave scratch untouched."

This is a critical invariant for block production. The immediate batch verifies on each push, so apply returns Err before any state mutation from a later step. Correct.

Issues Found

Issue 1: Potential for Unbatched Block Signature in apply_block

File: crates/blockchain/state_transition/src/beacon/stf/mod.rs
Lines: 177-192

if validate_result {
    verify(
        verify_block_signature(state, signed_block),
        "block signature",
    )?;
}
// ...
process_block(state, signed_block, ...)?;

The block proposer signature is verified before process_block, not batched with body signatures. The comment explains this is intentional (early rejection for gossip-unchecked blocks). However, this means:

  1. A block with an invalid proposer signature costs 1 pairing (early)
  2. A block with valid proposer signature but invalid body signature costs 1 + batch pairings

This is optimal for the common case (invalid blocks rejected early), but the comment should explicitly note that the proposer signature is intentionally not in the batch.

Severity: Documentation only. The logic is correct.

Issue 2: verify_signature_sets with Single Element vs. key_is_identity

File: crates/common/crypto/src/bls/batch.rs
Lines: 222-226

match sets {
    [] => return true,
    [only] => return only.verify(),
    _ => {}
}
if sets.iter().any(SignatureSet::key_is_identity) {
    return false;
}

The single-element optimization skips key_is_identity. But only.verify() will call self.signature.verify(...) or fast_aggregate_verify_pre_aggregated, which does reject identity pubkeys (BLST_PK_IS_INFINITY). So behavior is consistent.

However, for a single aggregated set with identity key (e.g., [P, -P] summing to identity), only.verify() returns false, same as the batch would. Correct.

Issue 3: Missing #[must_use] on SignatureBatch::deferred Return Value

File: crates/blockchain/state_transition/src/beacon/stf/signatures.rs
Line: 91

SignatureBatch::deferred returns Result<T>. If a caller ignores this, they might not notice batch verification failure. However, Result is already must-use-like in Rust. Not a real issue.

Issue 4: Test Helper checked Could Panic on Nested Use

File: crates/blockchain/state_transition/src/beacon/stf/signatures.rs
Lines: 155-158

#[cfg(test)]
pub(crate) fn checked<T>(f: impl FnOnce(&mut SignatureBatch) -> Result<T>) -> Result<T> {
    SignatureBatch::deferred(f)
}

If f internally calls SignatureBatch::deferred, we'd have nested deferred batches. This would work (each gets its own batch), but it's inefficient. No actual nested use in the PR.

Issue 5: process_block Name Collision in gloas

File: crates/blockchain/state_transition/src/beacon/stf/gloas.rs

There are now two process_block functions with different signatures:

  • gloas::process_block (takes &mut SignatureBatch, doesn't verify)
  • super::block::process_block (generic dispatch, verifies)

And gloas::process_block_and_verify wraps the former. The naming is clear in context but could confuse. The gloas block production calls use process_block_and_verify correctly.

Issue 6: Comment Typo

File: crates/blockchain/state_transition/src/beacon/stf/signatures.rs
Line: 0 (module doc)

"valid except for its signatures" — good warning. But also check line 85: "A deferring batch that could be dropped unverified would turn every function taking one into 'valid except for its signatures'." This is why SignatureBatch is not Clone and verify takes self. Good design.

Consensus Correctness Verification

Let me verify the critical property: deferred signature verification must not accept any block that immediate verification would reject.

For each signature type:

Operation Deferred? Immediate fallback? Correct?
Block proposer No N/A Yes (early reject)
RANDAO Yes process_randao Yes
Attestation (all forks) Yes indexed_attestation_signature_set → verify Yes
Sync aggregate (non-empty) Yes SignatureSet::verify Yes
Sync aggregate (empty) No (infinity check) N/A Yes
Proposer slashing Yes SignatureSet::verify Yes
Attester slashing Yes indexed_attestation_signature_set → verify Yes
Voluntary exit Yes SignatureSet::verify Yes
BLS to execution change Yes SignatureSet::verify Yes
Deposit No N/A Yes (bad deposit skipped, not rejected)
Execution payload bid (gloas) Yes SignatureSet::verify Yes
Payload attestation (gloas) Yes SignatureSet::verify Yes

All correct. Deposits remain immediate because a bad signature deposit is skipped (not rejected), which changes state transition behavior.

Performance Assessment

The batch verification uses blst's verify_multiple_aggregate_signatures with:

  • One Miller loop per signature set
  • One final exponentiation total
  • Random 64-bit weights

For a typical block with ~16 attestations + RANDAO + sync aggregate + exits:

  • Before: ~19 full pairings = ~19 final exponentiations
  • After: ~19 Miller loops + 1 final exponentiation ≈ 3-4x faster

The SignatureSet construction validates keys/signatures once, avoiding re-validation in the batch. Good.

Final Verdict

Approve with minor suggestions.

The PR is correct, well-tested, and implements a significant optimization safely. The deferred batch pattern with immediate fallback for block production is sound.

Suggested changes:

  1. Document INFINITY_FLAG source (crates/common/crypto/src/bls/batch.rs:75):

    /// The bit of a compressed G1 point's first byte that marks the point at
    /// infinity (Zcash BLS12-381 §1.2: "The most significant three bits of
    /// u[0] are used to unambiguously represent the underlying element").
    const INFINITY_FLAG: u8 = 0b0100_0000;
  2. Add safety comment (crates/blockchain/state_transition/src/beacon/stf/signatures.rs:142):

    // Unreachable with correct BLS batch verification: if the batch failed,
    // at least one individual set must fail. This is a defense-in-depth panic.
    verify(false, last)
  3. Clarify test wrapper (crates/blockchain/state_transition/tests/beacon_spec/operations.rs:174-192):
    Add comment explaining the unhandled pattern for operation-name errors vs. state-transition errors.

  4. Consider whether process_block_and_verify should be the pattern for all forks, not just gloas


Automated review by Kimi (Moonshot AI) · kimi-k2.6 · custom prompt

MegaRedHand added a commit that referenced this pull request Oct 30, 2026
…738): review fixes

fix(beacon): check the proposer's signature first and never drop a batch unverified

A block fetched by request reaches the state transition without gossip's
precheck, and with the proposer's signature in the body's batch a block its
proposer never signed was only rejected after the whole body ran. Check that
one signature on its own again, before the body, as before the batch existed.

A deferring SignatureBatch could also be built and dropped without its sets
ever being checked, turning any function that takes one into "valid except for
its signatures". Outside the crate, a deferring batch now only comes from
SignatureBatch::deferred, which verifies once its closure returns.

Also cover an infinity signature inside a batch and keys that cancel.

Updates nightly's #738 from perf/block-signature-batch at 7736808 to e6eeb5b.
MegaRedHand added a commit that referenced this pull request Oct 30, 2026
…and resources (#756)

The import replay reported `stf` as one number, which is most of a mainnet
block's import but does not say which step of the transition to attack, nor
whether an epoch-crossing block, a block after skipped slots and an ordinary
block differ. The beacon STF had no internal timing at all.

lean_beacon_stf_phase_seconds{context,phase} is observed around the steps of
the transition: process_slots (excluding epoch work), process_epoch, the block
signature, header (including the body hash it commits to), randao, eth1 data,
execution payload (gloas: parent payload, withdrawals, bid), each operation
list, the sync aggregate and the post-state root check. One Instant pair per
step, none per attestation; an empty list observes nothing. The phase list is
generated with the enum that the code observes through, so the two cannot
disagree.

The epoch precompute worker runs process_epoch on another thread while an
import runs, and the replay reads the histogram before and after each block, so
one series would charge the worker to whichever block was importing. The series
is split by a context label instead: only fork_choice's transition_block sets
`import` (a thread-local guard, since the steps are observed deep inside the
per-fork block processing), and every other caller writes `other`. The replay
reads only `import`.

The replay report (schema_version 2) gains per block: the STF steps, the stf
time outside any step, a class (epoch_boundary, after_skip, ordinary), process
CPU seconds and RSS; and per run: p95 and p99, a per-class summary and the
deltas of the cache lookup counters, so precompute hit rates appear in it.
Consensus behavior is unchanged.

Adapted onto nightly: block processing now defers every signature but the
proposer's into one `SignatureBatch` (#738), so the old inline checks the
timers wrapped no longer exist. Every call site keeps nightly's `signatures`
argument and order, with the timers wrapped around the same steps as before;
the proposer signature stays immediate and is still timed as
`block_signature`. A new phase `batch_signatures` times `SignatureBatch::verify`,
the one random-weight check of everything the steps deferred, so the verify
cost that used to sit inside `randao`, the operation lists and `sync_aggregate`
is now its own series. Those phases now cover building the signature sets and
the state changes, not verification, and docs/metrics.md and
docs/benchmarking.md say so. The phase is declared in the `StfPhase` enum, so
the label list stays generated from it. replay.rs keeps both
`--warm-committee-sums` (warm-up before the timers start) and the report
changes; the report test for warm_committee_sums gets the new `Caches`
argument. Verdicts, order and errors are nightly's. docs/benchmarking.md keeps
#755's paragraph on the overlapped transition ahead of this section.

Cherry-picked from perf/beacon-stf-phase-timing at c891134, adapted in
5c385e084.
@MegaRedHand

Copy link
Copy Markdown
Collaborator Author

Block import benchmark (offline replay of mainnet blocks, steady state)

leg STF per block vs main
main 012dc4f 157.7 ms
main + #737 150.0 ms -4.9%
main + #737 + #738 145.9 ms -7.5%
Method

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

beacon Ethereum Beacon Chain client performance Performance improvements or possible performance improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant