Skip to content

Rename SignedBlockWithAttestation.message to .block to match leanSpec#250

Merged
MegaRedHand merged 1 commit intomainfrom
rename-signed-block-message-to-block
Mar 25, 2026
Merged

Rename SignedBlockWithAttestation.message to .block to match leanSpec#250
MegaRedHand merged 1 commit intomainfrom
rename-signed-block-message-to-block

Conversation

@pablodeymo
Copy link
Collaborator

Motivation

leanSpec#465 renamed the SignedBlock container field from message to block. As noted in that PR:

This PR changes the ssz container field name from SignedBlock.message to SignedBlock.block. It doesn't affect ssz compatibility because ssz is schemaless, but I believe we rely on the same container namings across all client impls, so it will be hard to sync specs vs impls if not renamed.

This PR makes the corresponding rename in ethlambda to stay aligned with the spec and other client implementations.

Changes

Renamed SignedBlockWithAttestation.messageSignedBlockWithAttestation.block across 9 files (39 insertions, 40 deletions):

Core type definition

  • crates/common/types/src/block.rs: Renamed the struct field, updated the manual Debug impl, from_signed_block(), to_signed_block(), and doc comments that referenced message.

Blockchain crate

  • crates/blockchain/src/lib.rs: Updated block construction in propose_block(), field access in process_or_pend_block(), and pending block cascade in process_pending_children().
  • crates/blockchain/src/store.rs: Updated on_block_core() (block extraction and cloning), verify_signatures() (block and proposer attestation access), and a test helper struct construction.

Storage crate

  • crates/storage/src/store.rs: Updated the destructuring pattern in write_signed_block().

Networking crate

  • crates/net/p2p/src/gossipsub/handler.rs: Updated all 10 field accesses in handle_gossipsub_message() (block receive logging) and publish_block() (block publish logging).
  • crates/net/p2p/src/req_resp/handlers.rs: Updated block root computation in handle_blocks_by_root_response().

Test files

  • crates/blockchain/tests/forkchoice_spectests.rs: Updated build_signed_block() construction and slot access.
  • crates/blockchain/tests/signature_spectests.rs: Updated slot access for block timing.
  • crates/blockchain/tests/signature_types.rs: Updated local variable name and struct construction in From<TestSignedBlockWithAttestation> impl.

Not changed

  • TestSignedBlockWithAttestation.message (in signature_types.rs): This test deserialization type still uses message to match the current YAML test fixture field names. It will need updating when LEAN_SPEC_COMMIT_HASH is bumped to a commit that includes leanSpec#465.

Notes

  • SSZ compatibility: SSZ is schemaless, so this rename has zero impact on wire format or cross-client interop.
  • No behavioral changes: This is a pure rename — no logic was added, removed, or modified.
  • The resulting access pattern signed_block.block.block (outer .block is BlockWithAttestation, inner .block is Block) is slightly redundant but matches the spec structure.

How to Test

make fmt    # ✅ passes
make lint   # ✅ passes
make test   # ✅ passes

Aligns with leanEthereum/leanSpec#465 which renamed SignedBlock.message
to SignedBlock.block. SSZ compatibility is unaffected since SSZ is
schemaless; this is purely a naming convention change to keep all client
implementations consistent with the spec.
@github-actions
Copy link

🤖 Kimi Code Review

This is a straightforward field rename from message to block in SignedBlockWithAttestation. The change is consistent across all call sites and appears correct.

Review Notes:

  1. SSZ Compatibility: The rename is safe for SSZ serialization. SSZ encodes struct fields by order, not by name, so changing the Rust field identifier doesn't affect the wire format or tree hashing (as verified by the preservation of field order in block.rs:16-28).

  2. Naming Stutter: The change maintains the existing double-block access pattern (signed_block.block.block.slot). While slightly awkward, this accurately reflects the type hierarchy (SignedBlockWithAttestationBlockWithAttestationBlock). This is pre-existing and not introduced by this PR.

  3. Spec Alignment: Note that Ethereum consensus specs typically use message as the field name in signed containers (e.g., SignedBeaconBlock.message). This rename moves away from that convention, though given the specific type name (SignedBlockWithAttestation), using block may actually improve clarity.

  4. Pattern Matching: The nested destructuring in crates/storage/src/store.rs:1128-1133 is correctly updated.

Verdict: LGTM. No correctness, security, or performance issues identified.


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

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 25, 2026

Greptile Summary

This PR is a pure mechanical rename of SignedBlockWithAttestation.messageSignedBlockWithAttestation.block across 9 files, aligning the implementation with the upstream leanSpec change (leanSpec#465). There are no behavioural or SSZ wire-format changes.

Key observations:

  • The rename is complete and consistent across all production code (block.rs, lib.rs, store.rs, gossipsub handler, req-resp handler, storage).
  • The intentional carve-out — keeping TestSignedBlockWithAttestation.message to match existing YAML test fixture field names — is clearly documented both in the PR description and in the signature_types.rs conversion impl, which correctly bridges the two names.
  • The resulting signed_block.block.block chaining is acknowledged as slightly redundant but faithfully mirrors the spec's layered container structure (SignedBlockWithAttestation.blockBlockWithAttestation.block).
  • All compilation, linting, and tests pass per the PR notes.

Confidence Score: 5/5

  • This PR is safe to merge — it is a pure, mechanical rename with no behavioural changes, confirmed passing by fmt/lint/test.
  • The change is a straightforward field rename driven by an upstream spec alignment. Every access site across production code, networking, storage, and tests has been updated. The one deliberate exception (keeping TestSignedBlockWithAttestation.message for YAML fixture compatibility) is well-reasoned and documented. No logic was added or removed, and the Rust compiler would have caught any missed usages, which is corroborated by the passing test suite.
  • No files require special attention.

Important Files Changed

Filename Overview
crates/common/types/src/block.rs Core struct field renamed from message to block in SignedBlockWithAttestation; Debug impl, doc comments, from_signed_block(), and to_signed_block() all updated consistently.
crates/blockchain/src/lib.rs All three call sites (propose_block, process_or_pend_block, process_pending_children) updated from .message.block.* to .block.block.*.
crates/blockchain/src/store.rs All usages in on_block_core and verify_signatures updated; test helper struct construction also updated correctly.
crates/storage/src/store.rs Destructuring pattern in write_signed_block updated from message: BlockWithAttestation { ... } to block: BlockWithAttestation { ... }; formatting improved as a minor bonus.
crates/net/p2p/src/gossipsub/handler.rs All 10 field accesses across handle_gossipsub_message and publish_block updated consistently.
crates/net/p2p/src/req_resp/handlers.rs Single field access in handle_blocks_by_root_response updated correctly.
crates/blockchain/tests/forkchoice_spectests.rs build_signed_block helper and slot access updated; correct field name used in SignedBlockWithAttestation construction.
crates/blockchain/tests/signature_spectests.rs Single slot access updated from .message.block.slot to .block.block.slot.
crates/blockchain/tests/signature_types.rs Local variable renamed to block and struct field updated in From impl; TestSignedBlockWithAttestation.message deliberately kept for YAML fixture compatibility — intentional and well-documented.

Class Diagram

%%{init: {'theme': 'neutral'}}%%
classDiagram
    class SignedBlockWithAttestation {
        +BlockWithAttestation block
        +BlockSignatures signature
    }
    class BlockWithAttestation {
        +Block block
        +Attestation proposer_attestation
    }
    class Block {
        +u64 slot
        +u64 proposer_index
        +Hash parent_root
        +BlockBody body
    }
    class BlockSignatures {
        +VariableList attestation_signatures
        +Signature proposer_signature
    }

    SignedBlockWithAttestation --> BlockWithAttestation : block\n(renamed from message)
    SignedBlockWithAttestation --> BlockSignatures : signature
    BlockWithAttestation --> Block : block
    note for SignedBlockWithAttestation "Field renamed: message → block\nAccess pattern: signed_block.block.block"
Loading

Reviews (1): Last reviewed commit: "Rename SignedBlockWithAttestation.messag..." | Re-trigger Greptile

@github-actions
Copy link

🤖 Claude Code Review

Review: Rename SignedBlockWithAttestation.message.block

Summary

This is a clean, mechanical rename aligned with leanSpec#465. The change is propagated consistently across 9 files. No logic is modified — only field access paths.


Correctness

SSZ compatibility: Field renaming has no effect on SSZ encoding since SSZ is schemaless (position-based). This is correctly noted in the spec PR and implicitly relied upon here. No concern.

Storage destructuring (crates/storage/src/store.rs:1128): The nested destructuring pattern is valid and correct:

let SignedBlockWithAttestation {
    block: BlockWithAttestation {
        block,
        proposer_attestation,
    },
    signature,
} = signed_block;

The outer block: matches the renamed field; the inner block binds the Block inside BlockWithAttestation. Slightly subtle but idiomatic Rust.

signature_types.rs From impl: The local variable and struct field are correctly updated to block, while still reading from value.message.* (the test type's field, intentionally preserved). The intentional omission is well-documented in the PR description.


Minor Concerns

Double .block.block access pattern: Many call sites now read signed_block.block.block.slot, signed_block.block.block.tree_hash_root(), etc. This is inherently awkward — SignedBlockWithAttestation.block (a BlockWithAttestation) contains .block (a Block). This is a spec-driven naming choice, not a bug, but worth acknowledging as a readability trade-off. No action needed here unless the team wants to consider a method like fn inner_block(&self) -> &Block on the type — but that would be a separate PR.

Test fixture divergence in signature_types.rs: The PR correctly documents that TestSignedBlockWithAttestation.message must stay as-is until LEAN_SPEC_COMMIT_HASH is bumped to include leanSpec#465. It would be worth adding an inline comment to that field (e.g., // Still "message" to match current YAML fixture field names; update when bumping LEAN_SPEC_COMMIT_HASH) to avoid confusion for future contributors who see an inconsistency between the two types.


Verdict

The rename is complete, correct, and well-scoped. No logic changes, no correctness issues, no SSZ impact. The intentional asymmetry in test types is properly handled and documented. Approved with a suggestion to add a short inline comment on TestSignedBlockWithAttestation.message to explain why it differs from the production type.


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

@github-actions
Copy link

🤖 Codex Code Review

  1. Medium: SignedBlockWithAttestation’s public field rename from message to block is a source-breaking API change for any downstream consumer of ethlambda-types. SSZ wire/storage layout should remain unchanged because field order is unchanged, but external code that constructs, pattern-matches, or accesses .message will stop compiling. If this crate is intended to be consumed outside this workspace, this should be treated as a breaking change or staged behind a compatibility layer.

No correctness, fork-choice, attestation-validation, state-transition, XMSS verification, or SSZ-layout regressions stood out in the diff itself. The change appears to be a mechanical rename, and the updated call sites are consistent.

I could not run cargo test in this environment because the sandbox blocks writes under /home/runner/.cargo and /home/runner/.rustup, so the review is based on static inspection only.


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

@MegaRedHand MegaRedHand merged commit 7cc7851 into main Mar 25, 2026
7 checks passed
@MegaRedHand MegaRedHand deleted the rename-signed-block-message-to-block branch March 25, 2026 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants