feat: support batched json rpc requests #675
Conversation
Manual Deploy AvailableYou can trigger a manual deploy of this PR branch to testnet: Alternative: Comment
Comment updated automatically when the PR is synchronized. |
📝 WalkthroughWalkthroughThis pull request adds support for batch JSON-RPC requests to the HTTP dispatcher. A new Possibly related PRs
Suggested reviewers
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
Cargo.toml(1 hunks)magicblock-aperture/Cargo.toml(1 hunks)magicblock-aperture/src/requests/http/mod.rs(3 hunks)magicblock-aperture/src/requests/mod.rs(1 hunks)magicblock-aperture/src/server/http/dispatch.rs(4 hunks)magicblock-aperture/tests/batches.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-21T14:00:54.642Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-aperture/src/requests/websocket/account_subscribe.rs:18-27
Timestamp: 2025-10-21T14:00:54.642Z
Learning: In magicblock-aperture account_subscribe handler (src/requests/websocket/account_subscribe.rs), the RpcAccountInfoConfig fields data_slice, commitment, and min_context_slot are currently ignored—only encoding is applied. This is tracked as technical debt in issue #579: https://github.com/magicblock-labs/magicblock-validator/issues/579
Applied to files:
magicblock-aperture/src/server/http/dispatch.rs
🧬 Code graph analysis (1)
magicblock-aperture/src/server/http/dispatch.rs (2)
magicblock-aperture/src/requests/payload.rs (3)
encode(61-79)encode(105-115)encode(120-133)magicblock-aperture/src/requests/http/mod.rs (2)
extract_bytes(67-100)parse_body(49-64)
🔇 Additional comments (2)
magicblock-aperture/Cargo.toml (1)
72-72: LGTM!The addition of
reqwestas a dev-dependency is appropriate for the new batch request tests intests/batches.rs.magicblock-aperture/src/server/http/dispatch.rs (1)
110-112: LGTM! Access control headers on error responses.Good catch to ensure CORS headers are set on error responses. This prevents CORS-related failures in browser clients when requests fail.
| quote = "1.0" | ||
| rand = "0.8.5" | ||
| rayon = "1.10.0" | ||
| reqwest = "0.11" |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the reqwest version is current.
The addition of reqwest to workspace dependencies is appropriate for the new HTTP client functionality in tests.
🌐 Web query:
What is the latest stable version of the reqwest crate?
💡 Result:
The latest stable version is reqwest 0.12.24 (as of Nov 22, 2025). (docs.rs)
Citations:
Update reqwest to the latest stable version.
The latest stable version is reqwest 0.12.24 (as of Nov 22, 2025). The current 0.11 is outdated and should be updated in Cargo.toml.
🤖 Prompt for AI Agents
In Cargo.toml around line 145, the reqwest dependency is pinned to "0.11";
update the version to the latest stable "0.12.24" by replacing the version
string, then run cargo update and cargo build to refresh the lockfile and
compile; if compilation fails address any reqwest 0.12 API changes (adjust
imports, async runtime features, and feature flags such as tls or json) until
the project builds and tests pass.
| pub(crate) fn parse_body(body: Data) -> RpcResult<RpcRequest> { | ||
| let body_bytes = match &body { | ||
| Data::Empty => { | ||
| return Err(RpcError::invalid_request("missing request body")) | ||
| } | ||
| Data::SingleChunk(slice) => slice.as_ref(), | ||
| Data::MultiChunk(vec) => vec.as_ref(), | ||
| }; | ||
| json::from_slice(body_bytes).map_err(Into::into) | ||
| // Hacky/cheap way to detect single request vs an array of requests | ||
| if body_bytes.first().map(|&b| b == b'{').unwrap_or_default() { | ||
| json::from_slice(body_bytes).map(RpcRequest::Single) | ||
| } else { | ||
| json::from_slice(body_bytes).map(RpcRequest::Multi) | ||
| } | ||
| .map_err(Into::into) | ||
| } |
There was a problem hiding this comment.
Leading whitespace breaks the single/batch detection.
The first-byte check (body_bytes.first().map(|&b| b == b'{')) doesn't handle leading whitespace, which is valid in JSON per RFC 8259. A request like {"jsonrpc":"2.0",...} would incorrectly be parsed as a batch request.
Consider using a more robust approach:
- // Hacky/cheap way to detect single request vs an array of requests
- if body_bytes.first().map(|&b| b == b'{').unwrap_or_default() {
- json::from_slice(body_bytes).map(RpcRequest::Single)
- } else {
- json::from_slice(body_bytes).map(RpcRequest::Multi)
- }
- .map_err(Into::into)
+ // Peek at the first non-whitespace character to determine single vs batch
+ let first_non_ws = body_bytes.iter()
+ .find(|&&b| !b.is_ascii_whitespace())
+ .copied()
+ .unwrap_or_default();
+
+ if first_non_ws == b'{' {
+ json::from_slice(body_bytes).map(RpcRequest::Single)
+ } else if first_non_ws == b'[' {
+ json::from_slice(body_bytes).map(RpcRequest::Multi)
+ } else {
+ Err(RpcError::invalid_request("expected JSON object or array"))
+ }
+ .map_err(Into::into)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub(crate) fn parse_body(body: Data) -> RpcResult<RpcRequest> { | |
| let body_bytes = match &body { | |
| Data::Empty => { | |
| return Err(RpcError::invalid_request("missing request body")) | |
| } | |
| Data::SingleChunk(slice) => slice.as_ref(), | |
| Data::MultiChunk(vec) => vec.as_ref(), | |
| }; | |
| json::from_slice(body_bytes).map_err(Into::into) | |
| // Hacky/cheap way to detect single request vs an array of requests | |
| if body_bytes.first().map(|&b| b == b'{').unwrap_or_default() { | |
| json::from_slice(body_bytes).map(RpcRequest::Single) | |
| } else { | |
| json::from_slice(body_bytes).map(RpcRequest::Multi) | |
| } | |
| .map_err(Into::into) | |
| } | |
| pub(crate) fn parse_body(body: Data) -> RpcResult<RpcRequest> { | |
| let body_bytes = match &body { | |
| Data::Empty => { | |
| return Err(RpcError::invalid_request("missing request body")) | |
| } | |
| Data::SingleChunk(slice) => slice.as_ref(), | |
| Data::MultiChunk(vec) => vec.as_ref(), | |
| }; | |
| // Peek at the first non-whitespace character to determine single vs batch | |
| let first_non_ws = body_bytes.iter() | |
| .find(|&&b| !b.is_ascii_whitespace()) | |
| .copied() | |
| .unwrap_or_default(); | |
| if first_non_ws == b'{' { | |
| json::from_slice(body_bytes).map(RpcRequest::Single) | |
| } else if first_non_ws == b'[' { | |
| json::from_slice(body_bytes).map(RpcRequest::Multi) | |
| } else { | |
| Err(RpcError::invalid_request("expected JSON object or array")) | |
| } | |
| .map_err(Into::into) | |
| } |
| pub enum RpcRequest { | ||
| Single(JsonHttpRequest), | ||
| Multi(Vec<JsonHttpRequest>), | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add documentation and derives for the public enum.
The RpcRequest enum is a key type in the public API but lacks documentation and useful derives.
Apply this diff to add documentation and derives:
+/// Represents either a single JSON-RPC request or a batch of multiple requests.
+#[derive(Debug)]
pub enum RpcRequest {
+ /// A single JSON-RPC request.
Single(JsonHttpRequest),
+ /// A batch of multiple JSON-RPC requests.
Multi(Vec<JsonHttpRequest>),
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub enum RpcRequest { | |
| Single(JsonHttpRequest), | |
| Multi(Vec<JsonHttpRequest>), | |
| } | |
| /// Represents either a single JSON-RPC request or a batch of multiple requests. | |
| #[derive(Debug)] | |
| pub enum RpcRequest { | |
| /// A single JSON-RPC request. | |
| Single(JsonHttpRequest), | |
| /// A batch of multiple JSON-RPC requests. | |
| Multi(Vec<JsonHttpRequest>), | |
| } |
🤖 Prompt for AI Agents
In magicblock-aperture/src/requests/mod.rs around lines 19 to 22, the public
enum RpcRequest lacks documentation and useful derives; add a short doc comment
describing the enum's purpose and variants, and add derives such as Clone,
Debug, PartialEq, Eq and serde::Serialize/serde::Deserialize (or
serde::{Serialize, Deserialize}) so it can be debugged, cloned and serialized
for the public API.
| RpcRequest::Multi(requests) => { | ||
| let mut jobs = FuturesOrdered::new(); | ||
| for mut r in requests { | ||
| let j = async { | ||
| let response = self.process(&mut r).await; | ||
| (response, r) | ||
| }; | ||
| jobs.push_back(j); | ||
| } | ||
| let mut body = vec![b'[']; | ||
| while let Some((response, request)) = jobs.next().await { | ||
| let response = unwrap!(response, Some(&request.id)); | ||
| body.extend_from_slice(&response.into_body().0); | ||
| body.push(b','); | ||
| } | ||
| if let Some(b) = body.last_mut() { | ||
| *b = b']' | ||
| } | ||
| (Ok(Response::new(JsonBody(body))), None) | ||
| } | ||
| }; |
There was a problem hiding this comment.
Batch error handling violates JSON-RPC 2.0 specification.
The unwrap! macro on line 139 returns early on the first error in a batch, but JSON-RPC 2.0 requires that batch requests return an array containing responses for all requests, whether they succeed or fail. Each request should be processed independently.
Apply this diff to collect all responses (success and error):
let mut body = vec![b'['];
while let Some((response, request)) = jobs.next().await {
- let response = unwrap!(response, Some(&request.id));
- body.extend_from_slice(&response.into_body().0);
+ let resp = match response {
+ Ok(r) => r,
+ Err(error) => ResponseErrorPayload::encode(Some(&request.id), error)
+ };
+ body.extend_from_slice(&resp.into_body().0);
body.push(b',');
}
if body.len() > 1 {
*body.last_mut().unwrap() = b']';
} else {
body.push(b']');
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In magicblock-aperture/src/server/http/dispatch.rs around lines 128 to 148, the
current batch branch returns early on the first error via unwrap!, violating
JSON-RPC 2.0 which requires an array of responses for all requests; change the
loop to handle each job result independently: for each completed (response,
request) await the response result and if it's Ok use its body, otherwise
convert the error into a JSON-RPC error response using the request.id (or null
if none), then append that serialized response to the body vector with commas
between entries; finally ensure the trailing comma is replaced by the closing
']' (or build entries into a Vec and join) so the method returns a single JSON
array containing one entry per request (success or error).
| let mut body = vec![b'[']; | ||
| while let Some((response, request)) = jobs.next().await { | ||
| let response = unwrap!(response, Some(&request.id)); | ||
| body.extend_from_slice(&response.into_body().0); | ||
| body.push(b','); | ||
| } | ||
| if let Some(b) = body.last_mut() { | ||
| *b = b']' | ||
| } |
There was a problem hiding this comment.
Empty batch produces invalid JSON.
If the batch contains zero requests, the code produces invalid JSON:
- Starts with
body = vec![b'['] - While loop doesn't execute
body.last_mut()returns the opening[- Sets it to
], resulting in a single]character (invalid JSON)
The correct empty batch response should be [].
Apply this diff:
let mut body = vec![b'['];
while let Some((response, request)) = jobs.next().await {
let response = unwrap!(response, Some(&request.id));
body.extend_from_slice(&response.into_body().0);
body.push(b',');
}
- if let Some(b) = body.last_mut() {
- *b = b']'
+ if body.len() > 1 {
+ // Replace trailing comma with closing bracket
+ *body.last_mut().unwrap() = b']';
+ } else {
+ // Empty batch case
+ body.push(b']');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let mut body = vec![b'[']; | |
| while let Some((response, request)) = jobs.next().await { | |
| let response = unwrap!(response, Some(&request.id)); | |
| body.extend_from_slice(&response.into_body().0); | |
| body.push(b','); | |
| } | |
| if let Some(b) = body.last_mut() { | |
| *b = b']' | |
| } | |
| let mut body = vec![b'[']; | |
| while let Some((response, request)) = jobs.next().await { | |
| let response = unwrap!(response, Some(&request.id)); | |
| body.extend_from_slice(&response.into_body().0); | |
| body.push(b','); | |
| } | |
| if body.len() > 1 { | |
| // Replace trailing comma with closing bracket | |
| *body.last_mut().unwrap() = b']'; | |
| } else { | |
| // Empty batch case | |
| body.push(b']'); | |
| } |
🤖 Prompt for AI Agents
In magicblock-aperture/src/server/http/dispatch.rs around lines 137 to 145, the
current logic turns an initial '[' into ']' for empty batches producing invalid
JSON; change the finalization to handle the empty case explicitly: after the
loop, if body.len() == 1 (only the initial '[') push a ']' to make "[]",
otherwise replace the trailing comma with ']' as now; ensure you only replace
the comma when there was at least one element.
| #[tokio::test] | ||
| async fn test_batch_requests() { | ||
| let env = RpcTestEnv::new().await; | ||
| let client = reqwest::Client::new(); | ||
| let rpc_url = env.rpc.url(); | ||
|
|
||
| // Construct a batch request using serde_json macro | ||
| let batch_request = json::json!([ | ||
| {"jsonrpc": "2.0", "method": "getVersion", "id": 1}, | ||
| {"jsonrpc": "2.0", "method": "getIdentity", "id": 2} | ||
| ]); | ||
|
|
||
| let response = client | ||
| .post(rpc_url) | ||
| .json(&batch_request) | ||
| .send() | ||
| .await | ||
| .expect("Failed to send batch request"); | ||
|
|
||
| assert!( | ||
| response.status().is_success(), | ||
| "HTTP request failed status: {}", | ||
| response.status() | ||
| ); | ||
| let text = response.text().await.unwrap(); | ||
| let body: Value = json::from_str(&text).unwrap(); | ||
|
|
||
| assert!(body.is_array(), "Response should be a JSON array"); | ||
| let results = body.as_array().unwrap(); | ||
| assert_eq!(results.len(), 2, "Should return exactly 2 results"); | ||
|
|
||
| // Helper to find result by ID since batch responses can be out of order | ||
| let get_result = |id: u64| { | ||
| results | ||
| .iter() | ||
| .find(|v| v["id"] == id) | ||
| .expect("Result for id not found") | ||
| }; | ||
|
|
||
| // Verify getVersion result (ID 1) | ||
| let res1 = get_result(1); | ||
| assert!( | ||
| res1.get("result").is_some(), | ||
| "Should contain a result object" | ||
| ); | ||
| assert!( | ||
| res1["result"]["solana-core"].is_str(), | ||
| "Should contain solana-core version" | ||
| ); | ||
|
|
||
| // Verify getIdentity result (ID 2) | ||
| let res2 = get_result(2); | ||
| assert!( | ||
| res2["result"].is_object(), | ||
| "getIdentity should return adn object" | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding error handling test cases.
The test validates successful batch requests but doesn't cover error scenarios. Consider adding tests for:
- Mixed success/error responses in a batch
- Invalid method names in batch
- Empty batch array
[]
Would you like me to generate additional test cases for error handling scenarios?
🤖 Prompt for AI Agents
In magicblock-aperture/tests/batches.rs around lines 6 to 62, the existing test
only checks successful batch responses; add additional test functions that
exercise error scenarios: (1) a mixed success/error batch where one request is
valid (e.g., getVersion) and one uses an invalid method name—send the batch,
assert HTTP success, verify response is an array of the same length, locate
items by id and assert the valid id contains a result and the invalid id
contains an error object with numeric "code" and string "message"; (2) a batch
where all methods are invalid—assert each item contains an error object with
proper structure; and (3) an empty batch array []—send the request and assert
the server returns the expected behavior (either HTTP 400 or a JSON-RPC error
per your server spec); for each new test, use the same RpcTestEnv and reqwest
client, preserve id-based lookup because order may vary, and include explicit
failure messages on assertions.
| let res2 = get_result(2); | ||
| assert!( | ||
| res2["result"].is_object(), | ||
| "getIdentity should return adn object" |
There was a problem hiding this comment.
Fix typo in comment.
Minor typo: "adn" should be "an".
Apply this diff:
- "getIdentity should return adn object"
+ "getIdentity should return an object"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "getIdentity should return adn object" | |
| "getIdentity should return an object" |
🤖 Prompt for AI Agents
In magicblock-aperture/tests/batches.rs around line 60, the test description
contains a typo: replace "getIdentity should return adn object" with
"getIdentity should return an object" by changing "adn" to "an" in the string
literal so the comment/test name reads correctly.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.