feat(rpc): add GET /lean/v0/genesis#455
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 ReviewReview of PR 455: Add genesis RPC endpoint The implementation is clean and follows existing patterns, but there's a critical semantic issue regarding what constitutes "genesis" data. Critical Issue
Medium Concerns
Minor Suggestions
Code Quality
Recommendation: Address Item 1 (semantic mismatch) before merging. The current implementation will confuse users expecting immutable genesis data while the node is synced past genesis. Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Codex Code Review
No correctness, fork-choice, attestation, finalization, XMSS, or SSZ issues are introduced by this PR beyond that performance concern; the change is otherwise straightforward route wiring plus a focused test. I could not run the Rust test suite in this sandbox: Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Greptile SummaryAdds a
Confidence Score: 3/5The endpoint's genesis_time field is correct, but validator_count reads from the live head state rather than the genesis state, making it semantically wrong for any chain that changes its validator set after genesis. The validator_count field is sourced from store.head_state(), which evolves with every block; the endpoint claims to expose genesis-stable data but will return different values depending on when it is called. The test does not detect this because it only runs against a freshly initialized store where head and genesis happen to be the same. crates/net/rpc/src/genesis.rs — specifically the validator_count derivation on line 15.
|
| Filename | Overview |
|---|---|
| crates/net/rpc/src/genesis.rs | New genesis endpoint; genesis_time is read correctly from config, but validator_count reads from the live head state instead of the genesis state, mismatching the stated semantics and making the field unstable over the chain's lifetime. |
| crates/net/rpc/src/lib.rs | Straightforward module declaration and router merge for the new genesis module; no issues. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Client
participant RPC as GET /lean/v0/genesis
participant Store
Client->>RPC: GET /lean/v0/genesis
RPC->>Store: store.config()
Store-->>RPC: "ChainConfig { genesis_time }"
RPC->>Store: store.head_state()
Store-->>RPC: State (full clone of current head)
RPC->>RPC: .validators.len() as u64
RPC-->>Client: "{ genesis_time, validator_count }"
note over Store,RPC: head_state() returns the live current state, not genesis state
%%{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 GET /lean/v0/genesis
participant Store
Client->>RPC: GET /lean/v0/genesis
RPC->>Store: store.config()
Store-->>RPC: "ChainConfig { genesis_time }"
RPC->>Store: store.head_state()
Store-->>RPC: State (full clone of current head)
RPC->>RPC: .validators.len() as u64
RPC-->>Client: "{ genesis_time, validator_count }"
note over Store,RPC: head_state() returns the live current state, not genesis state
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/genesis.rs:14-15
**Head state used instead of genesis state for validator count**
`store.head_state()` returns the current, evolving head state, not the genesis state. The PR description says this endpoint returns the "genesis validator set," but as the chain progresses and validators are added or removed, `head_state().validators.len()` will diverge from the genesis validator count. A client relying on this to "verify they are on the correct network" could observe different counts depending on when they query, making the field unreliable for its stated purpose. The genesis validator set should be read from the anchor/genesis state, not from the live head.
### Issue 2 of 2
crates/net/rpc/src/genesis.rs:15
**Full state clone on every request to read one field**
`store.head_state()` deserializes and clones the entire `State` object just to call `.validators.len()`. Since genesis data is immutable by definition, this full-state clone on every GET is wasteful. Consider caching the genesis validator count at startup, or reading only the relevant field rather than materializing the whole state.
Reviews (1): Last reviewed commit: "feat(rpc): add GET /lean/v0/genesis" | Re-trigger Greptile
| let genesis_time = store.config().genesis_time; | ||
| let validator_count = store.head_state().validators.len() as u64; |
There was a problem hiding this comment.
Head state used instead of genesis state for validator count
store.head_state() returns the current, evolving head state, not the genesis state. The PR description says this endpoint returns the "genesis validator set," but as the chain progresses and validators are added or removed, head_state().validators.len() will diverge from the genesis validator count. A client relying on this to "verify they are on the correct network" could observe different counts depending on when they query, making the field unreliable for its stated purpose. The genesis validator set should be read from the anchor/genesis state, not from the live head.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/net/rpc/src/genesis.rs
Line: 14-15
Comment:
**Head state used instead of genesis state for validator count**
`store.head_state()` returns the current, evolving head state, not the genesis state. The PR description says this endpoint returns the "genesis validator set," but as the chain progresses and validators are added or removed, `head_state().validators.len()` will diverge from the genesis validator count. A client relying on this to "verify they are on the correct network" could observe different counts depending on when they query, making the field unreliable for its stated purpose. The genesis validator set should be read from the anchor/genesis state, not from the live head.
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| async fn get_genesis(State(store): State<Store>) -> impl IntoResponse { | ||
| let genesis_time = store.config().genesis_time; | ||
| let validator_count = store.head_state().validators.len() as u64; |
There was a problem hiding this comment.
Full state clone on every request to read one field
store.head_state() deserializes and clones the entire State object just to call .validators.len(). Since genesis data is immutable by definition, this full-state clone on every GET is wasteful. Consider caching the genesis validator count at startup, or reading only the relevant field rather than materializing the whole state.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/net/rpc/src/genesis.rs
Line: 15
Comment:
**Full state clone on every request to read one field**
`store.head_state()` deserializes and clones the entire `State` object just to call `.validators.len()`. Since genesis data is immutable by definition, this full-state clone on every GET is wasteful. Consider caching the genesis validator count at startup, or reading only the relevant field rather than materializing the whole state.
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 ReviewNow I have all the information I need. Here is the review: PR Review:
|
…ount test) Build a 3-validator anchor state in the genesis test so asserting validator_count == 3 actually exercises the handler. Also adds a clarifying comment explaining why head_state().validators.len() equals the genesis count (no churn in lean), and switches the test router to routes().with_state(store) for isolation.
Adds the
GET /lean/v0/genesisendpoint, which returns the genesis time and genesis validator set from the node's configuration.Useful for clients that need to bootstrap their slot clock or verify they are on the correct network. Has unit tests and passed clippy.
Stacked on #454.