Skip to content

feat(l1): [2] add unary ws JSON-RPC transport - #3877

Draft
brbrr wants to merge 1 commit into
feat/l1-client-enablersfrom
feat/l1-ws-transport
Draft

feat(l1): [2] add unary ws JSON-RPC transport#3877
brbrr wants to merge 1 commit into
feat/l1-client-enablersfrom
feat/l1-ws-transport

Conversation

@brbrr

@brbrr brbrr commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI review requested due to automatic review settings July 28, 2026 15:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a WebSocket JSON-RPC transport for the L1 Ethereum client, including subscription multiplexing (unary calls + eth_subscribe notifications), plus a shared internal test server and comprehensive transport/filter tests. The PR also exposes a jsonrpc.Response type and makes jsonrpc.Error implement error to support response decoding in the new transport.

Changes:

  • Introduce wsTransport with ping loop, request/reply routing, and subscription notification dispatch.
  • Add internal JSON-RPC test server supporting HTTP POST and WS upgrade, plus extensive WS transport tests (including failure/race scenarios).
  • Add FilterQuery JSON marshaling tailored for eth_subscribe log filters; export jsonrpc.Response and implement Error() on jsonrpc.Error.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
l1/internal/clienttest/server.go Internal minimal JSON-RPC server for shared WS/HTTP client tests, including ping control and raw frame injection.
l1/eth/client/transport_ws.go New WS transport implementation (dial, read/ping loops, routing, subscription lifecycle).
l1/eth/client/transport_ws_test.go Black-box tests covering unary calls, subscriptions, malformed frames, ping behavior, and concurrency.
l1/eth/client/transport_ws_internal_test.go White-box tests for error classification and bounded orphan tracking.
l1/eth/client/subscribe.go Subscription implementation (wsLogSub) and subscribeLogs using the WS transport.
l1/eth/client/options.go Client options extended for WS ping and dial timeouts + logger injection.
l1/eth/client/filter_query.go FilterQuery with custom MarshalJSON to omit unset block range keys.
l1/eth/client/filter_query_test.go Tests verifying FilterQuery marshaling shapes (block range omission, topics/address encoding).
jsonrpc/server.go Export Response and add Error() string on jsonrpc.Error to make it an error (plus minor formatting).

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

Comment thread jsonrpc/server.go Outdated
@brbrr
brbrr force-pushed the feat/l1-ws-transport branch from f412544 to 69c7c01 Compare July 28, 2026 15:21
Copilot AI review requested due to automatic review settings July 28, 2026 15:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comments suppressed due to low confidence (2)

l1/internal/clienttest/server.go:198

  • WebSocket connections are appended to ts.wsConns but never removed when the handler exits. This can cause WSConnCount() to over-report and PushNotification/PushRawFrame to attempt writes to closed conns (returning spurious errors) after clients disconnect/reconnect.
	ts.mu.Lock()
	ts.wsConns = append(ts.wsConns, conn)
	ts.mu.Unlock()
	defer func() { _ = conn.CloseNow() }()

l1/eth/client/transport_ws.go:158

  • Logging the method as a ByteString is misleading here because probe.Method is already a Go string (and zap will encode ByteString differently). Prefer zap.String for clearer logs.
		t.logger.Trace(
			"drop frame with no id and no recognised method",
			zap.ByteString("method", []byte(probe.Method)),
		)

@brbrr
brbrr changed the base branch from main to feat/l1-client-enablers July 28, 2026 15:27
@brbrr brbrr changed the title feat(l1): add one-shot ws JSON-RPC transport feat(l1): add unary ws JSON-RPC transport Jul 28, 2026
@brbrr brbrr changed the title feat(l1): add unary ws JSON-RPC transport feat(l1): [2] add unary ws JSON-RPC transport Jul 28, 2026
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.15183% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.28%. Comparing base (d7e2746) to head (0c0f876).

Files with missing lines Patch % Lines
l1/eth/client/transport_ws.go 82.51% 23 Missing and 9 partials ⚠️
l1/eth/client/options.go 50.00% 4 Missing ⚠️
Additional details and impacted files
@@                     Coverage Diff                     @@
##           feat/l1-client-enablers    #3877      +/-   ##
===========================================================
+ Coverage                    75.13%   75.28%   +0.14%     
===========================================================
  Files                          443      445       +2     
  Lines                        39974    40143     +169     
===========================================================
+ Hits                         30035    30220     +185     
+ Misses                        7855     7814      -41     
- Partials                      2084     2109      +25     

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

@brbrr
brbrr force-pushed the feat/l1-client-enablers branch from 7cab580 to e29ba56 Compare July 28, 2026 17:31
@brbrr
brbrr force-pushed the feat/l1-ws-transport branch from 69c7c01 to b94a5ac Compare July 28, 2026 17:31
@brbrr
brbrr force-pushed the feat/l1-client-enablers branch from e29ba56 to a33c2b3 Compare July 28, 2026 18:51
@brbrr
brbrr force-pushed the feat/l1-ws-transport branch 2 times, most recently from 7b1a4eb to e39a4f5 Compare July 28, 2026 18:54
@brbrr
brbrr force-pushed the feat/l1-client-enablers branch 2 times, most recently from 592fb3e to 7903a3e Compare July 28, 2026 19:15
@brbrr
brbrr force-pushed the feat/l1-ws-transport branch from e39a4f5 to e657702 Compare July 28, 2026 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants