[MegaRedHand]

Why

beacon-APIs (v5.0.0-beta.0) defines skip_randao_verification on produceBlockV3 and produceBlockV4 as a flag with no value: the caller sends the point-at-infinity reveal and asks the node not to check it. We accepted the flag on v4 and ignored it, and v3 did not read it, so a request carrying it was refused (v4: 400 invalid randao_reveal, v3: 500 the block failed to build). Lighthouse and Prysm build the block. The assertoor gloas-api-check playbook (patched to v5.0.0-beta.0) hit this: lighthouse and prysm answered 200, ethlambda 400.

What

Request	Before	After
no flag	reveal verified	unchanged
skip_randao_verification / skip_randao_verification= + infinity reveal	v4 400, v3 500	block built; its state root mixes the unchecked reveal in
flag with a value (=true, ...)	ignored	400 skip_randao_verification does not take a value
flag + a reveal other than infinity	ignored	400 randao_reveal must be the point at infinity when skip_randao_verification is set

Parsing follows Lighthouse (get_randao_verification), which matches the spec's wording; Prysm instead accepts =true and silently replaces any reveal with infinity.

• STF: BlockProcessingOptions { randao: RandaoVerification } (stf/block.rs), last parameter of the process_block dispatcher, every fork's process_block and process_randao. Skip only drops the reveal's BLS check; the mix is updated as before. state_transition always passes the default, so import never skips.
• Production: assemble_block, assemble_gloas_block, assemble_gloas_block_on_bid take the options and forward them.
• RPC: one shared randao_verification helper in beacon/proposal.rs, called right after the query parses (a 400 on any node, ahead of the engine check). v4 still gives its early invalid randao_reveal when not skipping.

Tests

• STF: Skip mixes in the infinity reveal; Verify rejects it.
• Production (pre-gloas and gloas): the infinity reveal builds only with Skip, and re-applying the block with Skip lands on its state_root.
• RPC v3 and v4: 200 with both flag spellings and the infinity reveal in the returned block; 400 for a value on the flag and for a non-infinity reveal.

Ran: cargo test --profile release-fast --lib for ethlambda-state-transition (548 passed) and ethlambda-rpc (238 passed), clippy -D warnings on both, cargo fmt --check. The beacon spec-test suites were not run locally.

Open question

v3 without the flag still answers 500 for a reveal that does not verify (the STF rejects it), where v4 answers 400 early. Not changed here.

[github-actions]

🤖 Kimi Code Review

I'll review this PR which adds skip_randao_verification support for block production, matching Lighthouse's API. Let me analyze the changes systematically.

Security & Correctness

Critical: skip_randao_verification bypasses signature verification for block production only, not import

crates/blockchain/state_transition/src/beacon/stf/block.rs:207-246

The design correctly limits RandaoVerification::Skip to block production paths. The state_transition function at line 155 hardcodes BlockProcessingOptions::default(), ensuring all imported blocks verify RANDAO. This is the correct security boundary.

However, I notice a potential issue in the test at crates/blockchain/state_transition/src/beacon/block_production.rs:469-516:

fn skip_randao() -> BlockProcessingOptions {
    BlockProcessingOptions {
        randao: RandaoVerification::Skip,
    }
}

This helper creates options with only randao set, but BlockProcessingOptions may gain more fields in the future. The #[derive(Default)] on RandaoVerification means BlockProcessingOptions::default() is the safe choice for production. The explicit struct literal here is fine for tests but could be brittle if fields are added. Consider using BlockProcessingOptions { randao: RandaoVerification::Skip, ..Default::default() } or ensure BlockProcessingOptions stays a single-field struct.

Verification: Infinity point validation in RPC layer

crates/net/rpc/src/beacon/proposal.rs:284-302

pub(super) fn randao_verification(
    skip: Option<&str>,
    randao_reveal: &BlsSignature,
) -> Result<RandaoVerification, ApiError> {

This correctly validates that skip_randao_verification only works with G2_POINT_AT_INFINITY. The logic:

• None → Verify
• Some("") or Some("=") → Skip (but only if reveal is infinity)
• Some(other) → error

Wait—looking more carefully at the test in crates/net/rpc/src/beacon/builder_market_tests.rs:1259-1276:

for flag in ["&skip_randao_verification=", "&skip_randao_verification"] {

And in crates/net/rpc/src/beacon/proposal.rs:288-295:

Some("") => {
    if randao_reveal.0 == bls::G2_POINT_AT_INFINITY {
        Ok(RandaoVerification::Skip)
    } else {
        Err(ApiError::BadRequest(...))
    }
}

The Some("") case handles ?skip_randao_verification= (empty string value). But what about ?skip_randao_verification with no =? Serde's Option<String> with #[serde(default)] will parse this as Some("") too? Let me verify...

Actually, looking at serde_urlencoded behavior: ?skip_randao_verification (no =) typically deserializes to Some("") for Option<String>. The test confirms this works. But this is subtle—add a comment explaining this serde behavior.

API Consistency Issue

assemble_gloas_block_on_bid lacks infinity reveal validation

crates/blockchain/state_transition/src/beacon/gloas_block_production.rs:367-399

pub fn assemble_gloas_block_on_bid(
    state: &BeaconState,
    inputs: GloasBidBlockInputs,
    config: &Config,
    options: stf::block::BlockProcessingOptions,
) -> Result<BeaconBlock> {

Unlike assemble_block and assemble_gloas_block, this function doesn't validate that when skip_randao() is used, the reveal is actually infinity. The validation happens in the RPC layer's randao_verification(), but assemble_gloas_block_on_bid is a public function that could be called directly.

Compare with prepare() in crates/net/rpc/src/beacon/gloas_proposal.rs:610-682 which does validate:

if randao == RandaoVerification::Verify {
    verify_randao_reveal(&state, proposer, randao_reveal)?;
}

But assemble_gloas_block_on_bid itself doesn't check. If someone calls it directly with skip_randao() and a real signature, they'd produce an invalid block. Consider adding a debug assertion or validation in the assembly functions themselves, or document that validation is the caller's responsibility.

Actually, looking more carefully—the assemble_* functions don't take randao_reveal directly; they take BlockInputs/GloasBlockInputs which contain the reveal. The options parameter is separate. So the caller could pass skip_randao() with a real reveal. This is the same pattern as assemble_block and assemble_gloas_block.

The test at line 1260 in gloas_block_production.rs tests assemble_gloas_block with infinity, but not assemble_gloas_block_on_bid. Consider adding a similar test for assemble_gloas_block_on_bid.

Code Quality & Maintainability

BlockProcessingOptions should use #[non_exhaustive] or be pub(crate)

crates/blockchain/state_transition/src/beacon/stf/block.rs:209-213

#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
pub struct BlockProcessingOptions {
    pub randao: RandaoVerification,
}

This is pub and not #[non_exhaustive]. If you add fields, this is a breaking change for external callers. Given this is a consensus client, external API stability matters. Consider:

#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
#[non_exhaustive]
pub struct BlockProcessingOptions {
    pub randao: RandaoVerification,
}

Or add a constructor: impl BlockProcessingOptions { pub const fn new() -> Self { ... } }

Duplicate skip_randao() test helper

Both block_production.rs:469 and gloas_block_production.rs:780 define similar helpers. Consider extracting to a shared test utility, though this is minor.

randao_verification function naming

crates/net/rpc/src/beacon/proposal.rs:284

The name randao_verification returns a RandaoVerification, but the function name is a noun phrase, not a verb. Consider parse_randao_verification or determine_randao_verification for clarity.

Error Handling

Inconsistent error messages for infinity reveal without flag

In crates/net/rpc/src/beacon/proposal.rs, when skip_randao_verification is not set and the reveal is infinity:

• For fulu (V3): Returns StatusCode::INTERNAL_SERVER_ERROR (from assemble_block failing)
• For gloas (V4): Returns StatusCode::BAD_REQUEST with "invalid randao_reveal"

Wait, let me re-check. In builder_market_tests.rs:1273-1276:

let reply = send(&app, produce_request_with(infinity, "")).await;
assert_eq!(reply.status, StatusCode::BAD_REQUEST);
assert_eq!(reply.json()["message"], "invalid randao_reveal");

And in proposal.rs the test at line 665:

let (status, _) = respond(app, Request::get(uri).body(Body::empty()).unwrap()).await;
assert_eq!(status, StatusCode::INTERNAL_SERVER_ERROR);

This inconsistency is because the fulu path goes through assemble_block which fails with a generic error, while the gloas path has explicit validation in verify_randao_reveal. The gloas path gives a better error. Consider making the fulu path also return BAD_REQUEST with a clear message.

Actually, looking at the gloas code in gloas_proposal.rs:660-662:

if randao == RandaoVerification::Verify {
    verify_randao_reveal(&state, proposer, randao_reveal)?;
}

And verify_randao_reveal returns ApiError::BadRequest("invalid randao_reveal"). But in the fulu path (proposal.rs), the RANDAO verification happens inside assemble_block → process_block → process_randao, which returns a generic Result::Err that becomes INTERNAL_SERVER_ERROR.

This is a real inconsistency in user-facing error behavior. The fulu path should pre-validate like gloas does, or the STF error should be mapped to a better HTTP status.

Performance

No performance concerns. The changes add:

• One BlockProcessingOptions parameter pass-through (zero-cost)
• One enum comparison in process_randao (negligible)
• Optional skip of BLS verification (saves time when used)

Testing

Good coverage

• Unit tests for process_randao with both Skip and Verify
• Integration tests for both fulu and gloas block production
• RPC-level tests for flag parsing and error cases
• Test that produced block imports correctly with same options

Missing test: assemble_gloas_block_on_bid with infinity reveal

As noted above, no test covers assemble_gloas_block_on_bid with skip_randao(). Add one in bid_assembly_tests.

Minor Issues

#[allow(dead_code)] removed but field still somewhat unused

crates/net/rpc/src/beacon/gloas_proposal.rs:205-208

#[serde(default)]
skip_randao_verification: Option<String>,

The field is "used" for deserialization but not directly accessed—it's passed to randao_verification(). The comment explains this well.

super::super::validator_client_tests::fake_execution_client()

crates/net/rpc/src/beacon/proposal.rs:636

This path is awkward. The pub(super) visibility added in the diff suggests this is a test utility leaking across module boundaries. Consider a cleaner test helper organization.

Summary

Priority	Issue	Location
Medium	Inconsistent HTTP error status for invalid infinity reveal (fulu: 500, gloas: 400)	proposal.rs vs gloas_proposal.rs
Low	assemble_gloas_block_on_bid lacks infinity reveal test	gloas_block_production.rs
Low	BlockProcessingOptions should be #[non_exhaustive]	stf/block.rs:209
Low	randao_verification naming	proposal.rs:284
Nit	super::super:: test import path	proposal.rs:636

The security-critical design is correct: state_transition always verifies, production paths can skip with proper validation. No critical bugs found.

---

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

[github-actions]

🤖 Codex Code Review

Looks good overall. The change is narrowly scoped, preserves the default verification path, and adds good tests around the new flag behavior.

• No correctness bug stood out in fork choice, attestation flow, justification/finalization, or SSZ/XMSS handling from this diff.
• The security-sensitive part is the new RANDAO bypass. It is constrained to explicit block-production options and the RPC layer only allows it for the point-at-infinity reveal, which is an important safety guard in crates/net/rpc/src/beacon/proposal.rs:285 and reused by gloas at crates/net/rpc/src/beacon/gloas_proposal.rs:243.
• Performance impact is negligible; this mostly adds a branch and avoids one BLS verify only when explicitly requested.

Findings

• No blocking issues found.

Notes

• The split between request parsing and state transition is clean: RandaoVerification defaults to safe behavior in crates/blockchain/state_transition/src/beacon/stf/block.rs:213, and callers must opt into Skip.
• assemble_block now computes state_root under the same processing options it was built with, which is necessary for consistency when skip_randao_verification is enabled in crates/blockchain/state_transition/src/beacon/block_production.rs:313.
• The pre-validation in the RPC layer avoids returning opaque internal errors for bad caller input, especially in crates/net/rpc/src/beacon/proposal.rs:336 and crates/net/rpc/src/beacon/gloas_proposal.rs:241.
• Readability is solid; comments clearly explain why the infinity reveal is the only allowed bypass case.

If you want, I can turn this into a ready-to-post GitHub review comment.

---

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

[github-actions]

🤖 Claude Code Review

Review: PR 678, honor skip_randao_verification on block production

I read the diff only and did not build it or run the tests. I found no blocking issues.

What it does

• Adds BlockProcessingOptions { randao: RandaoVerification }, with Verify as the default, and threads it through every per-fork process_block, the assemble_* functions and the RPC producers.
• state_transition and block import pass BlockProcessingOptions::default(), so consensus validation is unchanged. The new option is reachable only through block production.

Correctness and security

• Skipping is guarded in randao_verification (proposal.rs). It is accepted only for the exact G2_POINT_AT_INFINITY reveal and only for the value-less flag form. A real reveal sent with the flag gets a 400, so a caller cannot get an unverified reveal mixed into a produced block by mistake.
• The flag is parsed ahead of the engine check, so a bad flag is a 400 on any node, as the comments describe.
• process_randao still mixes in hash(reveal) when skipping. The produced state_root therefore matches what an importer gets from the same reveal, and the tests check this by re-running process_block with the same option.
• A block produced with the infinity reveal is not valid on the network. That is the intended Lighthouse-compatible behavior, because the caller is expected to replace the reveal.
• In gloas_proposal.rs, prepare skips verify_randao_reveal only under Skip. The same options reach both assemble_gloas_block and assemble_gloas_block_on_bid, including their empty-attestation retry paths, so the two stay consistent.

Minor notes (non-blocking)

1. proposal.rs: #[allow(clippy::too_many_arguments)] on produce works, but a small params struct would be tidier if this function grows again.
2. process_randao computes the signing root and looks up the proposer even when skipping. This is harmless. Moving that work inside the Verify branch would save a little, but keeping it makes the proposer-index lookup still fail on a bad state, so I'd leave it.
3. BlockProcessingOptions is a public API in state_transition, and its doc says import uses only the default. A debug assertion or a pub(crate) constructor for non-default values would make that harder to misuse later. This is optional.

Tests
Coverage is good. It includes the unit tests for process_randao, assembly across fulu and gloas, and RPC tests for both flag forms (= and bare), the rejection of a value or a real reveal, and the no-flag 400/500 behavior.

The change looks good to merge.

---

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