feat(l1): [1] add jsonrpc client enablers and ws test server

[brbrr]

This is a perparation for #3877. Test server will be used in the follow up PR.

[Copilot]

Pull request overview

This PR adds a small internal JSON-RPC test server for L1 client/provider tests and slightly enhances the JSON-RPC server package by exporting the response type and making jsonrpc.Error implement the error interface.

Changes:

• Introduce l1/internal/clienttest.TestServer, supporting both HTTP POST JSON-RPC and WebSocket JSON-RPC on the same endpoint, plus helpers for pushing notifications/raw frames.
• Export jsonrpc.Response (previously unexported) and implement Error() string on jsonrpc.Error.
• Minor logging formatting adjustment in jsonrpc/server.go.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
l1/internal/clienttest/server.go	Adds an internal HTTP+WebSocket JSON-RPC test server with helpers for test-driven behaviors (notifications, malformed frames, ping/pong controls).
jsonrpc/server.go	Exposes Response and adds Error.Error() for improved error interoperability/logging.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov]

Codecov Report

❌ Patch coverage is 4.82759% with 138 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.13%. Comparing base (2c10d48) to head (d7e2746).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
l1/internal/clienttest/server.go	0.00%	137 Missing ⚠️
jsonrpc/server.go	87.50%	1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (4.82%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3878      +/-   ##
==========================================
- Coverage   75.43%   75.13%   -0.30%     
==========================================
  Files         442      443       +1     
  Lines       39862    39974     +112     
==========================================
- Hits        30068    30035      -33     
- Misses       7713     7855     +142     
- Partials     2081     2084       +3     

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

[Copilot]

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

l1/internal/clienttest/server.go:253

• rawRPCRequest is documented as being duplicated from a client rpcRequest and able to read malformed frames without client constraints, but there is no rpcRequest type in the repo and the current struct still constrains id to uint64 (string/null IDs will fail unmarshal and be dropped). Consider making the request id a json.RawMessage and parsing it best-effort for TestRequest.ID, and update the comment accordingly.

// rawRPCRequest is duplicated from the client's rpcRequest so the server can
// read malformed frames without the client package's constraints.
type rawRPCRequest struct {
	JSONRPC string            `json:"jsonrpc"`
	ID      uint64            `json:"id"`

[github-actions]

Claude finished @brbrr's task in 5m 26s —— View job

---

Review summary

• Read repo guidelines
• Review jsonrpc/server.go changes
• Review l1/internal/clienttest/server.go
• Review .gitignore
• Post inline comments

No blockers. go vet ./l1/internal/... ./jsonrpc/... is clean. The concurrency in the test server holds up: wsConns is consistently guarded by mu, both push helpers snapshot the slice before writing (so no lock is held across a blocking Write), coder/websocket's Write is internally serialized so a push racing the read-loop's reply is safe, and the handler goroutine does deregister its conn on exit (Copilot's earlier comment on that is already addressed by the defer at server.go:198-208). Close/KillWSConns unblock a handler parked in Read, and httptest.Server.Close is idempotent, so the t.Cleanup + manual-Close path is fine.

Important

• clienttest/server.go:216 — the ws loop silently drops frames it can't unmarshal, and rawRPCRequest.ID is uint64, so a string id or a batch is dropped with no reply and the client under test hangs until its own timeout. Make the id json.RawMessage and reply with an error frame. (Same input on the POST path becomes a bare 400.)
• jsonrpc/server.go:79 — *Error now satisfies error, so the typed-nil trap (var e *jsonrpc.Error; return e → non-nil error, nil pointer) becomes reachable across the ~100 handlers returning that type, and Error() nil-derefs. Add a nil-receiver guard. Also changes %v rendering of *jsonrpc.Error.
• jsonrpc/server.go:66 — exporting the server's response type for the client to decode into is lossy: Result any yields map[string]any/float64, and float64 mangles large numeric ids/block numbers. A client-side type with Result json.RawMessage is safer than widening the server API. Both Response and Error() are unused in this PR (the 0% patch coverage), so the choice is cheap to revisit now.

Nit

• No batch support on the POST path (:156) — handle […] or document it.
• PushNotification duplicates PushRawFrame's broadcast loop; also worth documenting the broadcast-to-all-conns semantics and guarding against a deadline-less ctx hanging the test.
• NewTestServer could take testing.TB instead of *testing.T.
• .gitignore: CLAUDE.md is unrelated to this PR and better placed in ~/.config/git/ignore; add a note on what produces snapshots/.

One process note: I could not run go test in this environment (command approval), so the findings above are from reading and go vet only — the uint64 id and batch behaviours are inferred from encoding/json semantics rather than executed. Since the package has no consumers yet, a single smoke test exercising POST, ws request/response, a pushed notification and SetDropPings would make the helper self-verifying and address the coverage gate.
• branch feat/l1-client-enablers

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

Review in progress

• Read repo guidelines (CLAUDE.md)
• Re-review jsonrpc/server.go
• Re-review l1/internal/clienttest/server.go
• Check whether prior review findings were addressed
• Post summary

View job run • branch feat/l1-client-enablers

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

Claude finished @brbrr's task in 4m 57s —— View job

---

Re-review (push 592fb3e)

• Read repo guidelines (no root CLAUDE.md in this tree)
• Diff current head against the previously reviewed head
• Re-review jsonrpc/server.go
• Re-review l1/internal/clienttest/server.go
• Check which prior findings were addressed
• Post inline comments

Addressed since the last push ✅

• .gitignore dropped from the PR — down to 2 files.
• PushNotification/PushRawFrame now share a single broadcast helper (server.go:133); the snapshot-under-lock-then-write pattern is correct, no lock held across a blocking Write.
• Log typo fixed → "Failed handling RPC request".
• Conn deregistration on handler exit was already handled by the defer at server.go:194-204.

Concurrency re-verified on the current code: wsConns is consistently guarded by mu; coder/websocket documents "All methods may be called concurrently except for Reader and Read" (conn.go:30), so a broadcast racing the read-loop's reply is safe; the in-place slice splice in the cleanup defer can't corrupt Close/KillWSConns' snapshot because both nil out wsConns under the same lock; SetDropPings matches AcceptOptions.OnPingReceived's documented false → no pong contract (accept.go:67-72); httptest.Server.Close is idempotent and untracks hijacked conns, so the t.Cleanup + manual-Close path neither deadlocks nor double-closes. go vet ./l1/... ./jsonrpc/... is clean.

Still open

Important

• clienttest/server.go:212 — unparseable frames are dropped with no reply → client tests hang instead of failing. ID uint64 makes the struct stricter than the wire format (rejects string ids, batches, by-name params) and silently answers notifications with "id":0. inline
• jsonrpc/server.go:79 — *Error now satisfies error, making the typed-nil trap reachable across ~100 rpc/v* handlers; Error() nil-derefs on a nil receiver. inline
• jsonrpc/server.go:66 — exporting the server's Response for client-side decoding is lossy: Result any yields map[string]any/float64, and float64 mangles large numeric ids and block numbers. A client-side type with Result json.RawMessage avoids widening the server API. Still unused in this PR, so still cheap to revisit. (prior inline)

Nit

• No batch support on the POST path (:170) — handle […] or say so in the package comment.
• broadcast (:133) fans out to every live conn (a reconnecting client can briefly see a notification twice) and conn.Write blocks until the peer reads — a context.Background() caller with a non-draining client hangs the test rather than failing it. Worth documenting both, and deriving a short default deadline when the ctx has none.
• NewTestServer (:53) could take testing.TB.
• The package has zero tests, which is what the failing patch-coverage gate is reporting. One smoke test over POST, a ws request/response, a pushed notification and SetDropPings would make the helper self-verifying — and would have caught the id/notification behaviour above.

Process note: go test isn't approved in this environment, so I wrote a probe test to confirm the id/batch/notification cases empirically, couldn't run it, and deleted it — those four cases are derived from encoding/json struct-decoding semantics, not executed. Everything else above is from reading the code, the vendored coder/websocket docs, and go vet.
• branch feat/l1-client-enablers

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

Claude finished @brbrr's task in 4m 8s —— View job

---

Re-review (head 7903a3e, force-push)

• Fetch latest head and diff against origin/main
• Read repo guidelines (no CLAUDE.md in this tree)
• Re-review jsonrpc/server.go
• Re-review l1/internal/clienttest/server.go
• Check which prior findings were addressed
• Post inline comments

The branch was force-pushed to a single squashed commit, so line numbers moved. Diff is 2 files, +258/−8. go vet ./l1/... ./jsonrpc/... is clean.

Addressed since the last review ✅

• (*Error) Error() string is gone. This resolves my main blocker-adjacent finding: *jsonrpc.Error no longer satisfies error, so the typed-nil trap across the ~100 rpc/v* handlers returning that type stays unreachable, and there's no nil-receiver deref to guard. The %v rendering of *jsonrpc.Error is also unchanged, so nothing asserting on the old format breaks. Good call dropping it rather than patching around it.
• PushNotification/PushRawFrame now share one broadcast helper (server.go:133).
• Log typo fixed → "Failed handling RPC request".
• .gitignore dropped from the PR.
• Conn deregistration on handler exit is handled by the defer at server.go:194-204.

Concurrency re-verified on this head: wsConns is consistently guarded by mu; broadcast copies the slice under the lock so no lock is held across a blocking Write; coder/websocket documents "All methods may be called concurrently except for Reader and Read", so a broadcast racing the read-loop's reply is safe; the in-place splice in the cleanup defer can't corrupt Close/KillWSConns' snapshot because both nil out wsConns under the same lock; SetDropPings matches AcceptOptions.OnPingReceived's documented false → no pong contract; httptest.Server.Close is idempotent and untracks hijacked conns, and Close unblocks parked Reads before calling it, so the t.Cleanup + manual-Close path neither deadlocks nor double-closes.

Still open

Important

• clienttest/server.go:213 — unparseable frames are dropped with no reply, so a client bug becomes a hang at the package timeout instead of an assertion failure. ID uint64 + Params []json.RawMessage make the struct stricter than the wire format (rejects string ids, batches, by-name params) and silently answer notifications with "id":0. inline
• clienttest/server.go:133 (new) — broadcast returns nil when there are zero live conns, so require.NoError(t, ts.PushNotification(...)) passes when nobody is listening and the test then blocks on its receive channel. Same comment covers the fan-out-to-all-conns and blocking-Write semantics. inline
• jsonrpc/server.go:66 — exporting the server's Response for client-side decoding is lossy: Result any yields map[string]any/float64, and float64 mangles large numeric ids and block numbers. A client-side type with Result json.RawMessage avoids widening the server API. grep confirms jsonrpc.Response has no consumer anywhere in the tree yet, so this is still free to revisit. (prior inline — not re-posting)

Nit

• No batch support on the POST path (:169) — a JSON array body fails Decode and 400s. Handle […] or say so in the package comment.
• NewTestServer (:53) could take testing.TB; Helper() and Cleanup() are both on TB.
• The package still has zero tests, which is what the failing patch-coverage gate reports (4.96%). One smoke test over POST, a ws request/response, a pushed notification and SetDropPings would make the helper self-verifying — and would have caught both open clienttest findings above.

Process note: go test isn't approved in this environment, so the four decode cases in the first inline are derived from encoding/json struct-decoding semantics rather than executed. The zero-conns finding is read directly off the len(conns) == 0 path in broadcast. Everything else is from reading the code, the vendored coder/websocket docs, and go vet.
• branch feat/l1-client-enablers

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

Claude finished @brbrr's task in 6m 12s —— View job

---

Re-review (head d7e2746, force-push)

• Diff current head against base
• Read repo guidelines (no CLAUDE.md in this tree)
• Re-review jsonrpc/server.go
• Re-review l1/internal/clienttest/server.go
• Check which prior findings were addressed
• Post inline comments

2 files, +278/−8. go vet ./jsonrpc/... ./l1/internal/... is clean.

Addressed since the last review ✅

• ID is now json.RawMessage, echoed verbatim (server.go:244, :255) — string ids and large numeric ids round-trip correctly.
• Notifications get no reply — respondTo returns (nil, false) on a missing id (:252-254), and the POST path answers 204 (:180-183). The "id":0 bug is gone. "id":null still gets a reply with "id":null, which is the right call for a test server.
• broadcast errors on zero live conns (:139-141) with a comment explaining why — a push that reaches nobody is a test-ordering bug, not success.
• NewTestServer(tb testing.TB) (:53).
• (*Error) Error() string stayed dropped — *jsonrpc.Error doesn't satisfy error, so the typed-nil trap across the ~100 rpc/v* handlers remains unreachable and %v rendering is unchanged.
• Response has a doc comment on the pre-seed contract; the pre-seed trick does work as described (verified against encoding/json's indirect()).

Concurrency re-verified on this head: wsConns is consistently guarded by mu; broadcast copies the slice under the lock so no lock is held across a blocking Write; coder/websocket documents "All methods may be called concurrently except for Reader and Read", so a broadcast racing the read loop's reply is safe; the in-place splice in the cleanup defer can't corrupt Close/KillWSConns' snapshot because both nil out wsConns under the same lock; callHandler copies the handler under the lock and invokes it unlocked, so a handler that pushes can't deadlock. httptest.Server.Close untracks hijacked conns (httptest/server.go:389-397) and hijackLocked does not cancel the request context (net/http/server.go:317-337), so r.Context() stays live for the ws loop and Close doesn't block on it.

Still open

Important

• clienttest/server.go:221-232 — undecodable frames are still dropped with no reply, so a client bug hangs to the package timeout instead of failing. Params []json.RawMessage rejects by-name params, any batch fails Unmarshal, and an unmarshalable handler result hits the second continue. inline
• clienttest/server.go:135 (new) — passing a ctx with a deadline to broadcast doesn't just fail the push, it closes the conn: coder/websocket's writeFrame → setupWriteTimeout registers context.AfterFunc(ctx, func(){ ...; c.close() }) (conn.go:171-182). The defensive context.WithTimeout a test would naturally write therefore severs the connection against a non-draining client, and the single reused ctx kills every conn once it fires. inline

Nit

• jsonrpc/server.go:66 — the pre-seed contract is opt-in and silent when forgotten, and Result != nil isn't a usable "has result" check under it. Response still has zero consumers in-tree, so a client-side type with Result json.RawMessage remains cheaper than exporting the server envelope. inline
• No batch support on the POST path (:174) — a JSON array body 400s. Handle […] or say so in the package comment.
• Close() snapshots wsConns before srv.Close(); a conn that finishes its upgrade after the snapshot is never closed, and since httptest untracks hijacked conns its handler goroutine parks in Read for the rest of the test binary. Test-only and bounded, but a closed flag checked before the append at :201 would close the window.
• The package still has zero tests — that's the failing patch-coverage gate (5.1%). A smoke test over POST, a ws request/response, a pushed notification and SetDropPings would make the helper self-verifying, and would have caught both open findings above.

Process note: go test isn't approved in this environment, so the decode cases in the first inline are derived from encoding/json struct-decoding semantics rather than executed. The setupWriteTimeout finding and the hijack/httptest.Close claims are read directly off the vendored coder/[email protected] and Go 1.26.5 stdlib sources cited above.
• branch feat/l1-client-enablers

[rodrodros]

Why do we need to define all this test utility and not make it part of a _test.go file?

[brbrr]

It is explained in the package doc comment, but tl;dr:

It (will be) used in both eth/client and eth_l1_state_provider tests. And from my understanding go won't allow importing from _test.go into another package.