Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/net/rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ mod blocks;
mod fork_choice;
mod heap_profiling;
pub mod metrics;
mod spec;
pub mod test_driver;

pub(crate) use base::json_response;
Expand Down Expand Up @@ -100,6 +101,7 @@ fn build_api_router(store: Store) -> Router {
.merge(blocks::routes())
.merge(fork_choice::routes())
.merge(admin::routes())
.merge(spec::routes())
.with_state(store)
}

Expand Down
82 changes: 82 additions & 0 deletions crates/net/rpc/src/spec.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
use axum::{Router, response::IntoResponse, routing::get};
use ethlambda_blockchain::{INTERVALS_PER_SLOT, MILLISECONDS_PER_INTERVAL, MILLISECONDS_PER_SLOT};
use ethlambda_storage::Store;
use ethlambda_types::state::HISTORICAL_ROOTS_LIMIT;
use serde::Serialize;

use crate::json_response;

// 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";
Comment on lines +9 to +10

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

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.


#[derive(Serialize)]
struct SpecResponse {
#[serde(rename = "MILLISECONDS_PER_SLOT")]
ms_per_slot: u64,
#[serde(rename = "INTERVALS_PER_SLOT")]
intervals_per_slot: u64,
#[serde(rename = "MILLISECONDS_PER_INTERVAL")]
ms_per_interval: u64,
#[serde(rename = "HISTORICAL_ROOTS_LIMIT")]
historical_roots_limit: u64,
#[serde(rename = "FORK_DIGEST")]
fork_digest: &'static str,
}

async fn get_spec() -> impl IntoResponse {
json_response(SpecResponse {
ms_per_slot: MILLISECONDS_PER_SLOT,
intervals_per_slot: INTERVALS_PER_SLOT,
ms_per_interval: MILLISECONDS_PER_INTERVAL,
historical_roots_limit: HISTORICAL_ROOTS_LIMIT as u64,
fork_digest: FORK_DIGEST,
})
}

pub(crate) fn routes() -> Router<Store> {
Router::new().route("/lean/v0/config/spec", get(get_spec))
}

#[cfg(test)]
mod tests {
use super::FORK_DIGEST;
use crate::test_utils::create_test_state;
use axum::{
body::Body,
http::{Request, StatusCode},
};
use ethlambda_blockchain::{
INTERVALS_PER_SLOT, MILLISECONDS_PER_INTERVAL, MILLISECONDS_PER_SLOT,
};
use ethlambda_storage::{Store, backend::InMemoryBackend};
use ethlambda_types::state::HISTORICAL_ROOTS_LIMIT;
use http_body_util::BodyExt;
use std::sync::Arc;
use tower::ServiceExt;

#[tokio::test]
async fn spec_returns_lean_constants() {
let store = Store::from_anchor_state(Arc::new(InMemoryBackend::new()), create_test_state());
let app = crate::build_api_router(store);
let resp = app
.oneshot(
Request::builder()
.uri("/lean/v0/config/spec")
.body(Body::empty())
.unwrap(),
)
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
let body = resp.into_body().collect().await.unwrap().to_bytes();
let json: serde_json::Value = serde_json::from_slice(&body).unwrap();
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"],
HISTORICAL_ROOTS_LIMIT as u64
);
assert_eq!(json["FORK_DIGEST"], FORK_DIGEST);
Comment on lines +73 to +80

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Suggested change
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!

}
}