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

59-102: Verify block number stability when querying origin.

Past review comments raised a concern about using block numbers vs hashes when querying the origin. If the origin chain has been reorganized or pruned, block number N on the origin might represent a different block than expected. Consider whether BlockId::Number should be restricted or validated when fetching from origin.

Based on past review comments.

crates/starknet-devnet-types/src/rpc/block.rs (1)

134-138: Straightforward conversion looks correct

Field-to-field mapping matches the types; no ownership pitfalls. Consider also implementing From<&starknet_rs_core::types::ResourcePrice> for flexibility, but not required.

crates/starknet-devnet-core/src/blocks/mod.rs (1)

411-429: Hash inputs/order are explicit; timestamp zeroing is intentional — add a brief doc/test

The array ordering is clear and stable; zeroing timestamp for reproducibility is sensible. Two small asks:

• Document this rationale at the function (or type) level to avoid regressions.
• Add a unit test asserting stable hash for identical headers across runs (esp. timestamp).

Optional micro‑refactor for readability:

-        let hash = Poseidon::hash_array(&[
+        // Compose Poseidon inputs in a single place for clarity.
+        let hash_inputs = [
             felt!(self.header.block_header_without_hash.block_number.0), // block number
             self.header.block_header_without_hash.state_root.0,          // global_state_root
             *self.header.block_header_without_hash.sequencer.0.key(),    // sequencer_address
             Felt::ZERO,                                                  /* block_timestamp;
                                                                           * would normally be
                                                                           * felt!(self.header.
                                                                           * timestamp.0), but
                                                                           * is modified to enable replicability
                                                                           * in re-execution on
                                                                           * loading on dump */
             felt!(self.transaction_hashes.len() as u64), // transaction_count
             Felt::ZERO,                                  // transaction_commitment
             Felt::ZERO,                                  // event_count
             Felt::ZERO,                                  // event_commitment
             Felt::ZERO,                                  // protocol_version
             Felt::ZERO,                                  // extra_data
             self.header.block_header_without_hash.parent_hash.0, // parent_block_hash
-        ]);
+        ];
+        let hash = Poseidon::hash_array(&hash_inputs);

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

461-506: Make prefix handling explicit; consider an origin call timeout

• Use strip_prefix to express intent and avoid accidental multi-strip semantics.
• Wrap the origin fetch in a timeout to prevent request-thread starvation if the origin stalls (duration can be crate-configurable).

-        let origin_continuation_token = continuation_token
-            .map(|token| token.trim_start_matches(CONTINUATION_TOKEN_ORIGIN_PREFIX).to_string());
+        let origin_continuation_token = continuation_token
+            .as_deref()
+            .and_then(|t| t.strip_prefix(CONTINUATION_TOKEN_ORIGIN_PREFIX))
+            .map(str::to_string);

Optional (illustrative):

// let origin_events_chunk = tokio::time::timeout(Duration::from_secs(10), origin_caller
//     .starknet_client
//     .get_events(...))
//     .await
//     .map_err(|_| ApiError::StarknetDevnetError(Error::UnexpectedInternalError { msg: "Origin events timeout".into() }))??
//     .into();

tests/integration/test_subscription_to_events.rs (1)

556-629: Good end‑to‑end coverage of forked origin + local events

Solid flow: origin emission, fork, subscribe, then local emissions using origin account to avoid nonce clash. Optional: also assert finality_status for origin and fork emissions to tighten guarantees.

tests/integration/test_subscription_to_blocks.rs (2)

261-331: Block notifications across fork boundaries look correct

Nice sequencing: origin 0..n, expected empty block after fork, pre‑subscription fork blocks, then live blocks. Optional: assert parent_hash linkage for a sampled pair to detect ordering regressions.

---

332-392: Subscription starting by origin block hash works; assertions are clear

Covers mid‑range start and post‑fork progression. Optional: include an assertion that block hashes are unique and strictly increase by number for the notified sequence.

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

176-187: Consider parallel fetching for better performance.

The method fetches origin block headers sequentially, which could be slow for large ranges (up to 1024 blocks). Consider using parallel requests (e.g., with futures::future::join_all) to improve performance, especially since the headers are pre-fetched before notifying subscribers.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro