fix: serve genesis block on BlocksByRoot with structurally-valid blank XMSS signature#371
fix: serve genesis block on BlocksByRoot with structurally-valid blank XMSS signature#371MegaRedHand wants to merge 2 commits into
Conversation
`get_signed_block` short-circuited to `None` whenever the `BlockSignatures` row was missing. That is the by-design state for genesis-style anchor blocks (no proposer signed them, no attestations exist) but it also means BlocksByRoot silently drops the genesis chunk: the leanSpec response is `List[SignedBlock]` and peers (notably the ethereum/hive lean simulator's `blocks_by_root/multiple_known_blocks` test) expect one chunk per requested root. Synthesize an empty `BlockSignatures` instead — zero-filled XMSS proposer signature, empty `AttestationSignatures` list — so fork-choice and BlocksByRoot agree on what the node will serve.
`empty_block_signatures()` and the handful of test/RPC stubs that
construct placeholder `SignedBlock`s filled the proposer signature with
2536 raw zero bytes. That worked on the wire (parent containers treat
`XmssSignature` as an opaque fixed-size blob per leanSpec
`xmss/containers.py::Signature.is_fixed_size`), but the bytes are not a
valid SSZ encoding of the inner `Signature` container — the leading
offsets are all zero. Any consumer that round-trips the placeholder
through the inner-container decoder (e.g. leanSpec API-output
validators) would fail.
Match ream's `Signature::blank()` (post devnet4 alignment): write the
three SSZ offsets at fixed positions so the 2536-byte blob decodes back
to `Signature { path = HashTreeOpening { siblings = [0; 32] }, rho = 0,
hashes = [0; 46] }`. Centralize the construction in a single
`attestation::blank_xmss_signature()` helper and point every existing
call site at it; also fix the stale `(3112 bytes)` doc on
`XmssSignature` and update the HTTP test that asserted 404-on-genesis
to reflect the post-`bbe165c` behavior (the endpoint now serves the
genesis block with this placeholder, matching what peers see on
BlocksByRoot).
🤖 Claude Code ReviewPR #371 — fix: serve genesis block on BlocksByRoot with structurally-valid blank XMSS signatureOverviewTwo coupled fixes: (1) CorrectnessSSZ offset math — verified correct:
The stale Potential Issues1. Synthesis applies to ALL blocks with missing
let signature = match view.get(Table::BlockSignatures, &key).expect("get") {
Some(sig_bytes) => BlockSignatures::from_ssz_bytes(&sig_bytes).expect("valid signatures"),
None => empty_block_signatures(),
};CLAUDE.md states: "All other blocks must have entries in all three tables." If a non-genesis block somehow has a missing Suggestion: Guard the synthesis on the anchor/genesis block specifically, e.g. check None if header.slot == 0 => empty_block_signatures(),
None => return None, // or log a warningThis makes the invariant explicit and avoids masking potential storage corruption. 2. The diff inserts the function above the existing 3. No direct unit test for The SSZ offset logic is subtle — a one-off in any constant silently produces a blob that looks valid at the outer-container level but decodes incorrectly at the inner- #[test]
fn blank_xmss_signature_has_valid_ssz_offsets() {
let sig = blank_xmss_signature();
let b = sig.as_ssz_bytes();
assert_eq!(u32::from_le_bytes(b[0..4].try_into().unwrap()), 36);
assert_eq!(u32::from_le_bytes(b[32..36].try_into().unwrap()), 1064);
assert_eq!(u32::from_le_bytes(b[36..40].try_into().unwrap()), 4);
}4. No unit test for The synthesis path is tested indirectly via the RPC // store round-trip: genesis block has no BlockSignatures row, synthesis returns Some
let signed = store.get_signed_block(&genesis_root);
assert!(signed.is_some());
assert_eq!(signed.unwrap().signature.proposer_signature, blank_xmss_signature());Minor / Style
SummaryThe wire-format math is correct and the consolidation to Automated review by Claude (Anthropic) · sonnet · custom prompt |
🤖 Kimi Code ReviewOverall Assessment: Good change that consolidates genesis signature handling into a single, well-documented helper. The structurally-valid SSZ approach for placeholder signatures is safer than opaque zero-bytes. However, there are correctness, performance, and maintainability issues to address. Critical IssuesNone identified. The blank signature correctly uses zeroed cryptographic material that will fail verification against any real public key, which is the required behavior for unsignable genesis blocks. Bugs & Correctness1. Incorrect documentation of hash array size
** Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
Greptile SummaryThis PR fixes two related gaps that prevented the genesis block from being served via BlocksByRoot and
Confidence Score: 4/5Safe to merge; the wire format for already-signed blocks is unchanged and the genesis placeholder now matches ream's blank() byte-for-byte. The SSZ offset arithmetic in blank_xmss_signature() is correct and the storage change is narrowly scoped. Two non-blocking observations: the get_signed_block fallback silently covers any block with a missing signature row rather than enforcing the genesis-only invariant structurally, and the new RPC test stops at a status-code check without verifying the response body. crates/storage/src/store.rs (broadened fallback) and crates/net/rpc/src/lib.rs (incomplete test assertions on the genesis path)
|
| Filename | Overview |
|---|---|
| crates/common/types/src/attestation.rs | Introduces blank_xmss_signature() with hardcoded SSZ offsets (36, 1064, 4); math checks out but offset correctness relies on spec constants remaining stable. |
| crates/storage/src/store.rs | get_signed_block now synthesizes empty BlockSignatures for any block missing a signature row, not just genesis; intentional but broadens fallback scope beyond the documented case. |
| crates/net/rpc/src/lib.rs | Genesis test updated from 404 to 200; new test only asserts status code, skipping body/content-type checks present in the neighbouring test. |
| crates/blockchain/src/store.rs | Test stubs migrated from inline zero-filled vec to blank_xmss_signature(); straightforward call-site update with no logic changes. |
| crates/common/test-fixtures/src/fork_choice.rs | Migrates fork-choice spec-test fixture builder to blank_xmss_signature(); trivially correct. |
| crates/net/p2p/src/req_resp/handlers.rs | Test helper migrated to blank_xmss_signature(); no logic changes, clean substitution. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["BlocksByRoot / GET /lean/v0/blocks/finalized"] --> B["get_signed_block(root)"]
B --> C{"BlockHeaders row?"}
C -- No --> D["return None → 404"]
C -- Yes --> E{"BlockBodies row?\n(if non-empty body)"}
E -- No --> D
E -- Yes --> F{"BlockSignatures row?"}
F -- Yes --> G["Deserialize real BlockSignatures"]
F -- No --> H["synthesize empty_block_signatures()\nblank_xmss_signature() + empty attestations"]
G --> I["Assemble SignedBlock"]
H --> I
I --> J["Serialize to SSZ → 200 OK"]
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
crates/storage/src/store.rs:1005-1010
**Fallback applies to any block missing signatures, not just genesis**
The synthesized-signature path triggers whenever `Table::BlockSignatures` has no row for a given root — including non-genesis blocks whose signature write was skipped or failed silently. The doc comment scopes the intent to "genesis-style anchor blocks," but the code has no guard (e.g., `header.slot == 0`) to enforce that. A non-genesis block with a missing signature row would now be served to peers with a blank XMSS placeholder rather than being treated as absent, making the gap invisible to callers. In the current write path this can't happen, but the invariant is implicit rather than structural.
### Issue 2 of 2
crates/net/rpc/src/lib.rs:560-584
**New genesis test only asserts status code**
`test_get_latest_finalized_block_serves_genesis_with_placeholder_signature` verifies `StatusCode::OK` but does not check the response body or `Content-Type` header. The adjacent test (`test_get_latest_finalized_block`) asserts both, and their symmetry is the best guard that the serialisation path actually works end-to-end for the genesis case. Without a body check, a regression that returns 200 with empty or malformed SSZ bytes would pass this test silently.
Reviews (1): Last reviewed commit: "fix(types): use SSZ-structurally-valid b..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let signature = match view.get(Table::BlockSignatures, &key).expect("get") { | ||
| Some(sig_bytes) => { | ||
| BlockSignatures::from_ssz_bytes(&sig_bytes).expect("valid signatures") | ||
| } | ||
| None => empty_block_signatures(), | ||
| }; |
There was a problem hiding this comment.
The synthesized-signature path triggers whenever Table::BlockSignatures has no row for a given root — including non-genesis blocks whose signature write was skipped or failed silently. The doc comment scopes the intent to "genesis-style anchor blocks," but the code has no guard (e.g., header.slot == 0) to enforce that. A non-genesis block with a missing signature row would now be served to peers with a blank XMSS placeholder rather than being treated as absent, making the gap invisible to callers. In the current write path this can't happen, but the invariant is implicit rather than structural.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/storage/src/store.rs
Line: 1005-1010
Comment:
**Fallback applies to any block missing signatures, not just genesis**
The synthesized-signature path triggers whenever `Table::BlockSignatures` has no row for a given root — including non-genesis blocks whose signature write was skipped or failed silently. The doc comment scopes the intent to "genesis-style anchor blocks," but the code has no guard (e.g., `header.slot == 0`) to enforce that. A non-genesis block with a missing signature row would now be served to peers with a blank XMSS placeholder rather than being treated as absent, making the gap invisible to callers. In the current write path this can't happen, but the invariant is implicit rather than structural.
How can I resolve this? If you propose a fix, please make it concise.| @@ -579,6 +580,6 @@ mod tests { | |||
| .await | |||
| .unwrap(); | |||
|
|
|||
| assert_eq!(response.status(), StatusCode::NOT_FOUND); | |||
| assert_eq!(response.status(), StatusCode::OK); | |||
| } | |||
There was a problem hiding this comment.
test_get_latest_finalized_block_serves_genesis_with_placeholder_signature verifies StatusCode::OK but does not check the response body or Content-Type header. The adjacent test (test_get_latest_finalized_block) asserts both, and their symmetry is the best guard that the serialisation path actually works end-to-end for the genesis case. Without a body check, a regression that returns 200 with empty or malformed SSZ bytes would pass this test silently.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/net/rpc/src/lib.rs
Line: 560-584
Comment:
**New genesis test only asserts status code**
`test_get_latest_finalized_block_serves_genesis_with_placeholder_signature` verifies `StatusCode::OK` but does not check the response body or `Content-Type` header. The adjacent test (`test_get_latest_finalized_block`) asserts both, and their symmetry is the best guard that the serialisation path actually works end-to-end for the genesis case. Without a body check, a regression that returns 200 with empty or malformed SSZ bytes would pass this test silently.
How can I resolve this? If you propose a fix, please make it concise.
🤖 Codex Code Review
Static review only. I couldn’t run the targeted cargo tests here because the environment blocks writes under Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
1 participant
Summary
Two related fixes that let peers (and our HTTP API) receive the genesis block in response to BlocksByRoot / `/lean/v0/blocks/finalized`.
1. `fix(storage): synthesize empty BlockSignatures for genesis on read`
`store.get_signed_block(root)` previously short-circuited to `None` when the `BlockSignatures` row was missing. That row is intentionally absent for genesis-style anchor blocks (no proposer ever signed them, no attestations exist) — but it also meant we silently dropped the genesis chunk from BlocksByRoot responses. The lean spec response container is `List[SignedBlock]` and peers (notably the ethereum/hive lean simulator's `blocks_by_root/multiple_known_blocks` test) expect one chunk per requested root.
Synthesize an empty `BlockSignatures` when the header exists but no signature row was written, so the fork-choice view and BlocksByRoot agree on what the node will serve. Same shape ream pre-materializes at write time (`bin/ream/src/main.rs:301-306`); we synthesize on read because our split storage layout (`BlockHeaders` / `BlockBodies` / `BlockSignatures`) would otherwise pin a 2536-byte zero blob per anchor.
2. `fix(types): use SSZ-structurally-valid blank XMSS signature`
The placeholder `proposer_signature` in (1) — and a handful of test / RPC stubs scattered around the workspace — was built as 2536 raw zero bytes. That works on the wire because parent containers treat `XmssSignature` as an opaque fixed-size blob (leanSpec `xmss/containers.py::Signature.is_fixed_size` returns `True`), but the bytes are not a valid SSZ encoding of the inner `Signature` container: all offsets are zero. Any consumer that round-trips the placeholder through the inner-container decoder (e.g. leanSpec API-output validators) would reject it.
Match ream's `Signature::blank()` (post devnet4 alignment): write the three SSZ offsets at fixed positions (36, 1064, 4) so the same 2536-byte blob decodes back to:
```
Signature {
path: HashTreeOpening { siblings = [0; 32] },
rho: 0,
hashes: [0; 46],
}
```
Centralize the construction in a single `attestation::blank_xmss_signature()` helper and point every existing call site at it (storage helper, blockchain/p2p/rpc test stubs, fork-choice spec-test fixture builder). Also:
Why bundled
(2) depends on (1) — the `empty_block_signatures()` helper it touches is introduced by (1). Keeping them in one PR avoids a half-step where genesis is served on the wire with bytes that no one can decode back to a `Signature` container.
Test plan
Cross-client context
ream pre-materializes the same placeholder at write time when constructing the anchor SignedBlock (`bin/ream/src/main.rs:283-310` calls `Signature::blank()`). Lighthouse does the equivalent for the beacon chain — see `beacon_node/beacon_chain/src/builder.rs::genesis_block` which wraps the genesis block in `SignedBeaconBlock::from_block(genesis_block, Signature::empty())` with the inline comment "Empty signature, which should NEVER be read. This isn't to-spec, but makes the genesis block consistent with every other block." The Ethereum consensus-specs are silent on what shape this placeholder takes (`specs/phase0/p2p-interface.md` `BeaconBlocksByRoot` allows peers to silently skip out-of-range roots), so this is by-convention across clients rather than a spec requirement.
Risk