feat(rpc): add GET /lean/v0/config/spec#456
Conversation
Split the monolithic lib.rs API router into focused modules: core.rs holds the finalized-state/finalized-block/justified-checkpoint handlers and shared response helpers; blocks.rs, fork_choice.rs, and admin.rs each expose pub(crate) routes() -> Router<Store>. build_api_router merges them with .with_state(store). No behavior change.
🤖 Kimi Code ReviewFile: Critical: Hardcoded Fork Digest
Safety: Unchecked Type Cast
Maintenance Burden
Testing Gap
Minor: Unnecessary Allocation
Architecture Note File:
Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Codex Code Review
No consensus-path code was changed here, so I did not find issues in fork choice, attestation validation, STF, XMSS, or SSZ logic from this diff itself. I could not run the targeted Rust test in this sandbox because Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Greptile SummaryThis PR adds the
Confidence Score: 3/5The endpoint is broadly correct, but the duplicated FORK_DIGEST constant has no compile-time guard and will silently diverge from the P2P layer when the value is eventually derived dynamically. The duplicated FORK_DIGEST without an import-based sync guard is the key concern: the whole purpose of this endpoint is for clients to discover the fork digest reliably, but a future update to the P2P crate will leave the spec endpoint returning a stale value with no compile-time or test-time warning. The rest of the implementation — serialisation, routing, test structure — is straightforward and correct. crates/net/rpc/src/spec.rs — the FORK_DIGEST duplication and the incomplete test assertions both need attention before this can serve as a trustworthy discovery endpoint.
|
| Filename | Overview |
|---|---|
| crates/net/rpc/src/spec.rs | New file adding GET /lean/v0/config/spec; duplicates FORK_DIGEST without a compile-time sync guard, creating silent divergence risk when the P2P crate updates it. |
| crates/net/rpc/src/lib.rs | One-line module declaration and router merge for spec::routes(); change is minimal and correct. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Client
participant RPC as ethlambda-rpc
participant BC as ethlambda-blockchain
participant Types as ethlambda-types
Client->>RPC: GET /lean/v0/config/spec
RPC->>BC: read MILLISECONDS_PER_SLOT, INTERVALS_PER_SLOT, MILLISECONDS_PER_INTERVAL
RPC->>Types: read HISTORICAL_ROOTS_LIMIT
Note over RPC: FORK_DIGEST read from local hardcoded constant (duplicated from ethlambda-p2p)
RPC-->>Client: "200 OK { MILLISECONDS_PER_SLOT, INTERVALS_PER_SLOT, MILLISECONDS_PER_INTERVAL, HISTORICAL_ROOTS_LIMIT, FORK_DIGEST }"
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Client
participant RPC as ethlambda-rpc
participant BC as ethlambda-blockchain
participant Types as ethlambda-types
Client->>RPC: GET /lean/v0/config/spec
RPC->>BC: read MILLISECONDS_PER_SLOT, INTERVALS_PER_SLOT, MILLISECONDS_PER_INTERVAL
RPC->>Types: read HISTORICAL_ROOTS_LIMIT
Note over RPC: FORK_DIGEST read from local hardcoded constant (duplicated from ethlambda-p2p)
RPC-->>Client: "200 OK { MILLISECONDS_PER_SLOT, INTERVALS_PER_SLOT, MILLISECONDS_PER_INTERVAL, HISTORICAL_ROOTS_LIMIT, FORK_DIGEST }"
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/net/rpc/src/spec.rs:9-10
**Duplicated constant with no compile-time guard**
`FORK_DIGEST` is manually copied from `ethlambda_p2p::gossipsub::messages::FORK_DIGEST` with a prose comment asking authors to "keep in sync." Because `ethlambda-p2p` is not a dependency of `ethlambda-rpc`, there is no import that would force a compile error on divergence. The `messages.rs` doc already notes that this value will eventually be derived from the fork version and genesis validators root. When that change lands, the spec endpoint will silently return the old placeholder while the network uses the new digest — exactly the out-of-band coordination problem this endpoint is meant to eliminate. The fix is to move the constant into a shared crate (`ethlambda-types` or `ethlambda-blockchain`) and import it in both `ethlambda-p2p` and `ethlambda-rpc`.
### Issue 2 of 2
crates/net/rpc/src/spec.rs:72-75
The test never asserts `HISTORICAL_ROOTS_LIMIT`, so a wrong cast or accidental removal of that field would go undetected. Adding the assertion keeps the test in sync with the full response shape.
```suggestion
assert_eq!(json["MILLISECONDS_PER_SLOT"], MILLISECONDS_PER_SLOT);
assert_eq!(json["INTERVALS_PER_SLOT"], INTERVALS_PER_SLOT);
assert_eq!(json["MILLISECONDS_PER_INTERVAL"], MILLISECONDS_PER_INTERVAL);
assert_eq!(
json["HISTORICAL_ROOTS_LIMIT"],
ethlambda_types::state::HISTORICAL_ROOTS_LIMIT as u64
);
assert_eq!(json["FORK_DIGEST"], FORK_DIGEST);
```
Reviews (1): Last reviewed commit: "feat(rpc): add GET /lean/v0/config/spec" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Dummy fork digest; keep in sync with ethlambda_p2p::gossipsub::messages::FORK_DIGEST until it's centralized in a shared crate. | ||
| const FORK_DIGEST: &str = "12345678"; |
There was a problem hiding this comment.
FORK_DIGEST is manually copied from ethlambda_p2p::gossipsub::messages::FORK_DIGEST with a prose comment asking authors to "keep in sync." Because ethlambda-p2p is not a dependency of ethlambda-rpc, there is no import that would force a compile error on divergence. The messages.rs doc already notes that this value will eventually be derived from the fork version and genesis validators root. When that change lands, the spec endpoint will silently return the old placeholder while the network uses the new digest — exactly the out-of-band coordination problem this endpoint is meant to eliminate. The fix is to move the constant into a shared crate (ethlambda-types or ethlambda-blockchain) and import it in both ethlambda-p2p and ethlambda-rpc.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/net/rpc/src/spec.rs
Line: 9-10
Comment:
**Duplicated constant with no compile-time guard**
`FORK_DIGEST` is manually copied from `ethlambda_p2p::gossipsub::messages::FORK_DIGEST` with a prose comment asking authors to "keep in sync." Because `ethlambda-p2p` is not a dependency of `ethlambda-rpc`, there is no import that would force a compile error on divergence. The `messages.rs` doc already notes that this value will eventually be derived from the fork version and genesis validators root. When that change lands, the spec endpoint will silently return the old placeholder while the network uses the new digest — exactly the out-of-band coordination problem this endpoint is meant to eliminate. The fix is to move the constant into a shared crate (`ethlambda-types` or `ethlambda-blockchain`) and import it in both `ethlambda-p2p` and `ethlambda-rpc`.
How can I resolve this? If you propose a fix, please make it concise.| assert_eq!(json["MILLISECONDS_PER_SLOT"], MILLISECONDS_PER_SLOT); | ||
| assert_eq!(json["INTERVALS_PER_SLOT"], INTERVALS_PER_SLOT); | ||
| assert_eq!(json["MILLISECONDS_PER_INTERVAL"], MILLISECONDS_PER_INTERVAL); | ||
| assert_eq!(json["FORK_DIGEST"], FORK_DIGEST); |
There was a problem hiding this comment.
HISTORICAL_ROOTS_LIMIT, so a wrong cast or accidental removal of that field would go undetected. Adding the assertion keeps the test in sync with the full response shape.
| assert_eq!(json["MILLISECONDS_PER_SLOT"], MILLISECONDS_PER_SLOT); | |
| assert_eq!(json["INTERVALS_PER_SLOT"], INTERVALS_PER_SLOT); | |
| assert_eq!(json["MILLISECONDS_PER_INTERVAL"], MILLISECONDS_PER_INTERVAL); | |
| assert_eq!(json["FORK_DIGEST"], FORK_DIGEST); | |
| assert_eq!(json["MILLISECONDS_PER_SLOT"], MILLISECONDS_PER_SLOT); | |
| assert_eq!(json["INTERVALS_PER_SLOT"], INTERVALS_PER_SLOT); | |
| assert_eq!(json["MILLISECONDS_PER_INTERVAL"], MILLISECONDS_PER_INTERVAL); | |
| assert_eq!( | |
| json["HISTORICAL_ROOTS_LIMIT"], | |
| ethlambda_types::state::HISTORICAL_ROOTS_LIMIT as u64 | |
| ); | |
| assert_eq!(json["FORK_DIGEST"], FORK_DIGEST); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/net/rpc/src/spec.rs
Line: 72-75
Comment:
The test never asserts `HISTORICAL_ROOTS_LIMIT`, so a wrong cast or accidental removal of that field would go undetected. Adding the assertion keeps the test in sync with the full response shape.
```suggestion
assert_eq!(json["MILLISECONDS_PER_SLOT"], MILLISECONDS_PER_SLOT);
assert_eq!(json["INTERVALS_PER_SLOT"], INTERVALS_PER_SLOT);
assert_eq!(json["MILLISECONDS_PER_INTERVAL"], MILLISECONDS_PER_INTERVAL);
assert_eq!(
json["HISTORICAL_ROOTS_LIMIT"],
ethlambda_types::state::HISTORICAL_ROOTS_LIMIT as u64
);
assert_eq!(json["FORK_DIGEST"], FORK_DIGEST);
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
🤖 Claude Code ReviewHere is the review: Review:
|
1 participant
Adds the
GET /lean/v0/config/specendpoint, which exposes chain configuration parameters (slot duration, committee sizes, finality constants, etc.) as JSON.Allows external tools and cross-client integrations to discover runtime config without out-of-band coordination. Has unit tests and passed clippy.
Stacked on #454.