perf(jsonrpc): join batch response bytes instead of re-encoding

[NazariiDenha]

User description

handleBatchRequest collects each response as a json.RawMessage, then packs together with json.Marshal(responses). This is inefficient so this pr joins the bytes directly

Numbers

k6 against a local seoplia, batch requests at 100 VUs

method	avg batch response size	rps main	rps this pr
blockNumber	4 KB	378,544	403,702	+6.7%
getBlockWithTxHashes	132 KB	149,079	163,22	+9.5%
getBlockWithReceipts	570 KB	90,420	100,113	+10.7%
getClass (batch of 10)	6.6 MB	2,400	2,894	+20.6%

---

PR Type

Enhancement, Tests

---

Description

• Join batch JSON response bytes directly

• Avoid re-encoding batch responses in server

• Add byte-identical batch response test

---

File Walkthrough

	Relevant files
Enhancement	server.go` Concatenate batch response bytes manually                                jsonrpc/server.go Replace batch response json.Marshal with byte concatenation Add concatBatchResponses to build JSON array from raw messages
Tests	server_test.go` Test batch array matches JSON marshal bytes                            jsonrpc/server_test.go Add test asserting concatenated batch response matches json.Marshal Cover special JSON escaping and empty payload cases

---

[github-actions]

Claude finished @NazariiDenha's task in 3m 5s —— View job

---

Review: perf(jsonrpc): join batch response bytes instead of re-encoding

• Gather context (diff, CLAUDE.md, related code)
• Understand the request (code review)
• Review jsonrpc/server.go changes
• Review jsonrpc/server_test.go changes
• Post findings

Summary: This is a clean, low-risk optimization. concatBatchResponses replaces json.Marshal(responses) with direct byte concatenation, avoiding a redundant compact()/re-escape pass over already-marshaled JSON. Since every element in responses originates from json.Marshal(response) in addResponse (server.go:450), it's guaranteed to already be compact, HTML-escaped JSON — so skipping the second compact pass is safe and produces byte-identical output. Capacity pre-allocation (size := len(responses) + 1 + Σlen(response)) correctly matches the existing totalBytes accounting used for the batch-size limit, so no reallocation occurs during the build.

The new test TestBatchArrayIsByteIdenticalToJSONMarshal is a good addition — it specifically exercises the cases that would break if HTML-escaping/Unicode-line-separator handling diverged (<script>, &&, U+2028/U+2029, quotes/backslashes), by round-tripping the server's actual output through json.Marshal and asserting byte equality.

Nit (posted inline): minor grammar fix in the new doc comment, plus a suggestion to note the implicit invariant that concatBatchResponses depends on its inputs always being compact/pre-escaped json.RawMessage — not a bug today, just worth flagging for future maintainers who might reuse this helper with differently-sourced raw messages.

No blockers or important issues found.

Note: I wasn't able to run go test ./jsonrpc/... or golangci-lint in this sandbox (tool execution beyond read-only git commands required approval that wasn't available non-interactively). Review is based on static analysis of the diff and knowledge of encoding/json's RawMessage/compact semantics — you may want to confirm CI passes for the new test.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 89de865)

Here are some key observations to aid the review process:

Estimated effort to review: 2 🔵🔵⚪⚪⚪

🏅 Score: 93

🧪 PR contains tests

🔒 No security concerns identified

⚡ No major issues detected

[github-actions]

Claude finished @NazariiDenha's task in 1m 1s —— View job

---

Review: perf(jsonrpc): join batch response bytes instead of re-encoding

• Gather context (diff against origin/main, addResponse/handleBatchRequest in jsonrpc/server.go)
• Review correctness of concatBatchResponses
• Review capacity pre-allocation math
• Review error-handling change (dropped err from json.Marshal)
• Review new test TestBatchArrayIsByteIdenticalToJSONMarshal
• Post summary

Summary

No blockers or important issues. This is a correct, low-risk hot-path optimization.

Correctness:

• Every element in responses (jsonrpc/server.go:460) is produced by json.Marshal(response) inside addResponse, so each json.RawMessage is guaranteed compact and HTML-escaped before concatBatchResponses runs. That's exactly the invariant json.Marshal([]json.RawMessage)'s internal compact() pass would have been enforcing, so skipping it and just joining bytes is safe and produces byte-identical output.
• Capacity math (jsonrpc/server.go:570-573) is exact: for N elements it's N+1 (brackets + separators) plus Σlen(response), matching the existing totalBytes accounting used for the batch-size limit — no reallocation during the build.
• Dropping the error return (previously from json.Marshal) is safe: the only way that marshal could fail is if a RawMessage held invalid JSON, which can't happen here since every entry already round-tripped through json.Marshal successfully in addResponse.
• No changes to the concurrency/aggregation logic (wg.Wait(), mutex-guarded addResponse) — this only touches how the already-collected responses are serialized, so no new race conditions.

Tests:

• TestBatchArrayIsByteIdenticalToJSONMarshal (jsonrpc/server_test.go:1054) is a solid regression test — it specifically exercises <script>, &&, U+2028/U+2029, and quote/backslash payloads (the cases that would diverge if HTML-escaping ever differed between the per-element and batch-level marshal), and asserts byte-for-byte equality against json.Marshal.

Nit (already flagged by another review pass on this PR): the doc comment on concatBatchResponses has a small grammar issue and could note explicitly that correctness depends on inputs always being pre-compacted/escaped json.RawMessage — worth keeping in mind if this helper is ever reused with differently-sourced input, but not a functional issue today.

No security, panic, or resource-leak concerns identified in this diff.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.18%. Comparing base (f103674) to head (cc3a4ac).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3998      +/-   ##
==========================================
- Coverage   79.29%   79.18%   -0.12%     
==========================================
  Files         465      465              
  Lines       35785    35794       +9     
==========================================
- Hits        28377    28344      -33     
- Misses       7399     7441      +42     
  Partials        9        9              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Persistent review updated to latest commit 89de865

[github-actions]

Claude finished @NazariiDenha's task in 2m 50s —— View job

---

Review: perf(jsonrpc): join batch response bytes instead of re-encoding

• Gather context (diff vs origin/main, handleBatchRequest/addResponse in jsonrpc/server.go)
• Review concatBatchResponses correctness and capacity math
• Review dropped error return from batch marshal
• Review new test TestBatchArrayIsByteIdenticalToJSONMarshal
• Build (go build ./jsonrpc/...) and go vet ./jsonrpc/...
• Post summary

Verdict: clean, no blockers or important issues.

Correctness

• Every element of responses (jsonrpc/server.go:460) is produced by json.Marshal(response) inside addResponse, so each json.RawMessage is guaranteed compact + HTML-escaped before concatBatchResponses (jsonrpc/server.go:573) runs. That's exactly the invariant encoding/json's internal compact() pass enforced previously, so joining bytes directly is safe and byte-identical.
• Capacity pre-allocation is exact: size := len(responses) + 1 + Σlen(response) accounts for the 2 brackets + (n-1) separators = n+1 extra bytes, matching the existing totalBytes accounting used for the batch-size limit (server.go:446,459). No reallocation during the build.
• Dropping the error return is safe — the only way the old json.Marshal(responses) could fail is an invalid RawMessage, which can't happen since every entry already round-tripped through a successful json.Marshal in addResponse.
• The empty-batch guard (len(responses) == 0 → nil, finalHeaders, nil, server.go:534-536) still runs before concatBatchResponses, so it's never called with zero elements — no div-by-zero/edge case there.
• No changes to the concurrency model (mutex-guarded addResponse, wg.Wait() before use) — this only changes how already-collected responses are serialized.
• Confirmed locally: go build ./jsonrpc/... and go vet ./jsonrpc/... both pass clean. (Sandbox didn't allow running go test/golangci-lint non-interactively — worth confirming CI is green.)

Tests

• TestBatchArrayIsByteIdenticalToJSONMarshal (jsonrpc/server_test.go:1054) is a solid regression test: it exercises <script>/&& (HTML-escape chars), U+2028/U+2029 (line separators encoding/json normally escapes), quotes/backslashes, and an empty payload — then asserts the server's actual batch output is byte-identical to json.Marshal of the decoded responses. Good coverage of exactly the cases that would silently diverge if this optimization were wrong.

Nit (already flagged on this PR, still unresolved)

• jsonrpc/server.go:570-572 doc comment has a grammar slip ("elements that already valid JSON") and could spell out the invariant that correctness depends on inputs always being pre-compacted/escaped json.RawMessage (as produced by addResponse). Not a functional issue today — just worth tightening before this helper is reused elsewhere with differently-sourced input.

[github-actions]

Standalone PR Review

PR-Agent could not safely update the persistent review. This standalone result will not replace the canonical review.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🏅 Score: 85

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Silent Error Swallowing The previous implementation used json.Marshal(responses), which validates each json.RawMessage via compact() and returns an error if any element is malformed JSON. concatBatchResponses blindly concatenates bytes with no validation and the caller now always receives a nil error. If any individual response ever contains invalid/corrupted JSON (e.g. from a bug in a handler), the server will silently emit a broken JSON array to the client instead of surfacing an error. // concatBatchResponses builds the JSON array from elements that already valid // JSON, so it joins bytes instead of re-encoding. json.Marshal would run every // byte through compact() again, which is costly func concatBatchResponses(responses []json.RawMessage) []byte { size := len(responses) + 1 for _, response := range responses { size += len(response) } result := make([]byte, 0, size) result = append(result, '[') for i, response := range responses { if i > 0 { result = append(result, ',') } result = append(result, response...) } return append(result, ']') }

[github-actions]

Claude finished @NazariiDenha's task in 2m 39s —— View job

---

Review: perf(jsonrpc): join batch response bytes instead of re-encoding

• Gather context (diff vs origin/main, handleBatchRequest/addResponse in jsonrpc/server.go)
• Verify build/vet (go build ./jsonrpc/..., go vet ./jsonrpc/... — both pass, confirms server_test.go also compiles)
• Post final review summary

Verdict: clean, no blockers.

This code has already been reviewed multiple times on this thread; re-checked against the current HEAD (cc3a4ac, same diff as previously reviewed) and nothing new stands out.

Correctness (confirmed again):

• concatBatchResponses (jsonrpc/server.go:573) only ever receives elements appended by addResponse (server.go:449-461), which appends to responses only after json.Marshal(response) succeeds — a failed marshal is logged and the element is dropped, never added. So by construction every json.RawMessage passed in is already valid, compact, HTML-escaped JSON. The "Silent Error Swallowing" concern raised by the automated PR-Agent pass (that malformed elements could produce a broken array) doesn't hold — there's no code path that reaches concatBatchResponses with an unvalidated/invalid element.
• Capacity math (size := len(responses) + 1 + Σlen(response)) is exact (n+1 for brackets/separators), matching the existing totalBytes accounting — no reallocation during the build.
• No changes to concurrency/aggregation (mutex-guarded addResponse, wg.Wait() before use).

Tests: TestBatchArrayIsByteIdenticalToJSONMarshal (server_test.go:1054) is a solid regression test, covering HTML-escape chars, U+2028/U+2029, quotes/backslashes, and an empty payload, asserting byte-identical output vs json.Marshal.

Nit (already flagged inline, unaddressed, non-blocking): the doc comment grammar ("elements that already valid JSON") on server.go:570-572 is still unfixed — small polish item, not worth blocking on.

Design discussion (already addressed by author): infrmtcs's suggestion to wrap this in a custom RawSlice type with MarshalJSON was reasonably countered by the author — a custom MarshalJSON still goes through json.Marshal's reflection/interface-dispatch overhead, which is exactly what this PR avoids.

Note: sandbox permissions allow go build/go vet but not go test non-interactively in this run — build and vet both pass clean; recommend confirming CI test run is green before merge.

[github-actions]

Standalone PR Review

PR-Agent could not safely update the persistent review. This standalone result will not replace the canonical review.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🏅 Score: 85

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Lost Validation json.Marshal(responses) previously ran each json.RawMessage through compact(), which validates that the bytes are syntactically valid JSON and returns an error otherwise. concatBatchResponses skips this check and blindly concatenates the raw bytes. If any individual response ever contains malformed JSON (e.g. due to a bug in a handler's marshaling), the batch handler will now silently return an invalid/corrupted JSON payload to the client instead of surfacing an error. func concatBatchResponses(responses []json.RawMessage) []byte { size := len(responses) + 1 for _, response := range responses { size += len(response) } result := make([]byte, 0, size) result = append(result, '[') for i, response := range responses { if i > 0 { result = append(result, ',') } result = append(result, response...) } return append(result, ']') }