crates/starknet-devnet-server/src/restrictive_mode.rs (4)

25-30: Prefer &str (and optionally a Set) for membership checks

Accepting &String needlessly constrains callers and causes extra allocations at call sites. Use &str and iterate, or switch to a HashSet for O(1) lookups.

-pub(crate) fn is_json_rpc_method_restricted(
-    json_rpc_method: &String,
-    restricted_methods: &[String],
-) -> bool {
-    restricted_methods.contains(json_rpc_method)
-}
+pub(crate) fn is_json_rpc_method_restricted(
+    json_rpc_method: &str,
+    restricted_methods: &[String],
+) -> bool {
+    restricted_methods.iter().any(|m| m == json_rpc_method)
+}

Optional follow-up: keep DEFAULT_RESTRICTED_JSON_RPC_METHODS as a HashSet<&'static str> (once_cell::sync::Lazy) for faster membership tests.

---

41-47: Fix method name typo in the ad-hoc list

"devnet_CreateBlocks" likely meant "devnet_createBlock". Avoid subtle test confusion from near-miss names.

-            &(["devnet_abortBlocks", "devnet_CreateBlocks", "devnet_mint"]
+            &(["devnet_abortBlocks", "devnet_createBlock", "devnet_mint"]

---

52-54: Non-restricted sample: consider a known valid method

Using a method that might not exist (e.g., devnet_getAccountBalance) can mislead future readers. Prefer a known, allowed RPC such as "starknet_chainId".

-        assert_is_not_restricted("devnet_getAccountBalance", &DEFAULT_RESTRICTED_JSON_RPC_METHODS);
+        assert_is_not_restricted("starknet_chainId", &DEFAULT_RESTRICTED_JSON_RPC_METHODS);

---

64-69: Remove gratuitous allocations in test helpers

With the fn signature change to &str, you can avoid to_string() in tests.

-        assert!(is_json_rpc_method_restricted(&method.to_string(), restricted_methods));
+        assert!(is_json_rpc_method_restricted(method, restricted_methods));
@@
-        assert!(!is_json_rpc_method_restricted(&method.to_string(), restricted_methods));
+        assert!(!is_json_rpc_method_restricted(method, restricted_methods));

crates/starknet-devnet-server/src/api/http/endpoints/dump_load.rs (1)

2-2: HTTP-typed result in a post-HTTP world

This impl still returns HttpApiResult/HttpApiError from a module under api/http, but REST wrappers are gone. Consider moving dump_impl to a transport-agnostic module (e.g., api/ops or api/internal) and return a common ApiError/Result so JSON-RPC can use it without HTTP coupling.

tests/integration/test_restrictive_mode.rs (1)

12-12: Strengthen the positive-case assertion (optional)

You can assert a known field from devnet_getConfig to prove functionality beyond mere success, e.g., check presence of "chainId" or configured flags. If not needed, current check is fine.

crates/starknet-devnet/src/main.rs (3)

333-349: Server startup flow LGTM, but minor ergonomics nit.

Spawning the server with with_graceful_shutdown is correct. Consider logging “JSON-RPC only” to make the breaking change explicit to users when Devnet starts.

-    info!("Starknet Devnet listening on {}", address);
+    info!("Starknet Devnet listening on {} (JSON-RPC only)", address);

---

370-399: Unify signal handling to a portable form.

The per-OS signal branching works, but you can simplify and avoid subtle platform quirks by using the cross-platform tokio::signal::ctrl_c() directly in select.

-    #[cfg(unix)]
-    let mut sigint = { signal(SignalKind::interrupt()).expect("Failed to setup SIGINT handler") };
-
-    #[cfg(windows)]
-    let mut sigint = {
-        let ctrl_c_signal = ctrl_c().expect("Failed to setup Ctrl+C handler");
-        Box::pin(ctrl_c_signal)
-    };
+    // Cross-platform ctrl-c listener (works on Unix and Windows)
+    let mut ctrlc = tokio::signal::ctrl_c();
@@
-            _ = sigint.recv() => {
+            _ = &mut ctrlc => {
                 return Ok(())
             }

---

379-393: Add a request timeout to the internal reqwest client.

Without a timeout, the periodic POST to self could hang indefinitely under transient issues. Use a small timeout (e.g., server_config.timeout) to fail fast.

-    let devnet_client = reqwest::Client::new();
+    let devnet_client = reqwest::Client::builder()
+        .timeout(std::time::Duration::from_secs(10))
+        .build()
+        .expect("Failed to build reqwest client");

If you prefer wiring the CLI timeout, pass it via ServerConfig and reuse here.

crates/starknet-devnet-server/src/api/json_rpc/endpoints.rs (1)

20-21: Decouple JSON-RPC from the http module type.

get_devnet_config constructs DevnetConfig from crate::api::http::endpoints::DevnetConfig. Since HTTP surface is being removed, consider relocating DevnetConfig to a transport-agnostic module (e.g., crate::api::models) to avoid coupling JSON-RPC to http::*.

-use crate::api::http::endpoints::DevnetConfig;
+use crate::api::models::DevnetConfig; // move the type to a shared/models module

crates/starknet-devnet/src/cli.rs (1)

303-327: Validation logic simplified to JSON-RPC only — good; small perf/readability nit.

For large lists, converting json_rpc_methods to a HashSet avoids O(n²) contains checks and clarifies intent.

-                let json_rpc_methods = JsonRpcRequest::all_variants_serde_renames();
+                let json_rpc_methods: std::collections::HashSet<_> =
+                    JsonRpcRequest::all_variants_serde_renames().into_iter().collect();
@@
-                    if !json_rpc_methods.contains(method) {
+                    if !json_rpc_methods.contains(method.as_str()) {
                         wrong_restricted_methods.push(method.clone());
                     }

crates/starknet-devnet-server/src/api/http/endpoints/accounts.rs (1)

10-12: Consider decoupling result/error types from “http” now that REST endpoints are gone

These impls return HttpApiResult/HttpApiError despite no remaining HTTP surface. Suggest introducing a crate-level ApiResult (and From conversions to RPC errors) to avoid leaking HTTP semantics into internal logic.

Also applies to: 74-95

crates/starknet-devnet-server/src/server.rs (2)

31-39: Harden /is_alive and support HEAD

Return an explicit status and add HEAD for common load balancers and k8s probes. Keeps GET body simple and content-type implicit via status.

-use axum::routing::{IntoMakeService, get, post};
+use axum::routing::{IntoMakeService, get, head, post};
@@
-    let mut routes = Router::new()
-        .route("/is_alive", get(|| async { "Alive!!!" }))// Only REST endpoint to simplify liveness probe
+    let mut routes = Router::new()
+        .route(
+            "/is_alive",
+            get(|| async { StatusCode::OK }).head(|| async { StatusCode::OK }),
+        ) // Only REST endpoint to simplify liveness probe
         .merge(json_rpc_routes(json_rpc_handler.clone()))
         .layer(TraceLayer::new_for_http());

---

101-113: Response/request body logging collects entire bodies; consider sampling

Collecting full bodies can be expensive and can break streaming in other contexts. Consider capping logged bytes, redacting large payloads, or making logging opt-in per request header.

crates/starknet-devnet-server/src/api/http/endpoints/mod.rs (1)

6-12: Module naming still implies HTTP; consider relocating helpers

With REST removed, keeping helper modules under api::http is misleading. Consider moving accounts/mint_token helpers to a shared/internal module (e.g., api::helpers or domain::accounts).

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro