feat: Add wasm support#1
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRemoves monolithic defaults and replaces them with platform-specific default implementations; introduces JS/WASM WebTransport implementations and stubs for WebRTC, and applies build tags to gate WebRTC/WebTransport files between js and non-js builds. ChangesDefaults split
WebRTC: non-js gating
WebRTC: js stub
WebTransport: non-js gating
WebTransport: js implementations
Tests gating
Sequence Diagram(s)sequenceDiagram
participant Client as JS Client
participant T as WebTransport Transport
participant S as WebTransport Session
participant N as Noise
participant C as libp2p Connection
Client->>T: Dial(multiaddr with certhashes)
T->>T: parse SNI, build HTTPS URL
T->>S: Open WebTransport session (URL, expected certs)
S-->>T: session ready
T->>S: openStream()
S-->>T: bidirectional stream
T->>N: SecureOutbound(stream) with early-data handler
N->>T: request early data
T-->>N: provide cert hashes (from multiaddr / protobuf)
N->>N: complete handshake & verify certs
N-->>T: secure stream
T->>C: wrap secure stream into CapableConn
T-->>Client: return CapableConn
sequenceDiagram
participant App as Application
participant Config as libp2p Config
participant Defaults as Platform Defaults
participant Stack as Transport Stack
App->>Config: create Config
Config->>Defaults: Apply defaults (build-tagged)
alt js build
Defaults->>Stack: WebTransport only
else non-js build
Defaults->>Stack: TCP, QUIC, WS, WebTransport, WebRTC
end
Defaults-->>Config: set ListenAddrs / Transports
Config-->>App: configured node
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
|
Suggested tag: This is the first release of this module. Cutting a Release (and modifying code files)This PR is modifying both Automatically created GitHub ReleasePre-creating GitHub Releases on release PRs initiated from forks is not supported. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
go.mod (1)
3-3:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCI is blocked by Go toolchain mismatch against the module directive.
go.moddeclaresgo 1.25.7, but your pipeline reports a max supported version of 1.16, sogo mod tidycannot pass in current CI. Please align CI Go version (preferred) or lower the module directive if that compatibility is intentional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 3, The module directive in go.mod currently reads "go 1.25.7" which is incompatible with CI's Go toolchain; either update your CI toolchain to a Go version that supports 1.25.7 or change the go directive in go.mod to the maximum supported CI version (e.g., "go 1.16") so commands like go mod tidy succeed; locate the go directive line in go.mod and edit it to the chosen supported version, then re-run the CI steps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@p2p/transport/webtransport/listener_js.go`:
- Around line 15-16: The Accept method currently returns the package-local
sentinel errListenerClosed instead of the shared sentinel tpt.ErrListenerClosed;
update Accept (and any other returns using errListenerClosed on lines around the
Accept implementation) to return tpt.ErrListenerClosed so callers receive the
standard listener-closed sentinel; remove or stop using the local var
errListenerClosed in favor of the shared tpt.ErrListenerClosed in
listener_js.go.
In `@p2p/transport/webtransport/stream_js.go`:
- Around line 47-63: In Read, after calling s.read(b) (the call to s.read), mark
s.readAny = true if n > 0 immediately before handling err so that any delivered
bytes are recorded even when s.read returns both bytes and errInputStream; then
continue the existing error handling (if err != nil and err.Error() ==
errInputStream set s.done = true and return io.EOF, else return n, err). Keep
the rest of the logic (the s.done precheck and final return) unchanged and only
move the readAny assignment to occur right after observing n > 0.
In `@p2p/transport/webtransport/transport_js.go`:
- Around line 154-159: The current code defers s.Close() immediately after
calling sess.openStream, which closes the underlying stream before upgrade
returns and thus returns a ConnSecurity backed by a closed stream; remove the
unconditional defer s.Close() in the upgrade path and instead ensure the stream
is closed only on error (e.g., close s if sess.openStream or subsequent upgrade
steps fail) so that when upgrade returns successfully the returned ConnSecurity
still owns an open stream; look for usages of sess.openStream, s.Close(), and
the upgrade function/ConnSecurity return to implement this change.
- Around line 161-163: The callback passed to t.noise.WithSessionOptions via
noise.EarlyData and newEarlyDataReceiver dereferences the protobuf extensions
(pb.NoiseExtensions) without checking for nil, which can panic when Received
supplies a nil payload; update the early-data handler inside
newEarlyDataReceiver (the function receiving *pb.NoiseExtensions) to first guard
`if b == nil` (or similarly check b.WebtransportCerthashes) and return a
controlled error or nil to cleanly abort the handshake, otherwise proceed to
call decodeCertHashesFromProtobuf on b.WebtransportCerthashes.
- Around line 137-143: The dial function builds the URL using sni but currently
drops any port from host; update the url construction in transport.dial to
preserve the advertised port when sni is used by extracting the port from host
(e.g. via net.SplitHostPort or equivalent) and appending it to the sni
(sni:port) before formatting the URL with webtransportHTTPEndpoint, falling back
to sni alone if no port present; leave the host branch unchanged and ensure the
resulting URL still includes "?type=noise".
---
Outside diff comments:
In `@go.mod`:
- Line 3: The module directive in go.mod currently reads "go 1.25.7" which is
incompatible with CI's Go toolchain; either update your CI toolchain to a Go
version that supports 1.25.7 or change the go directive in go.mod to the maximum
supported CI version (e.g., "go 1.16") so commands like go mod tidy succeed;
locate the go directive line in go.mod and edit it to the chosen supported
version, then re-run the CI steps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b0e677b5-ddc8-402b-ae19-b8ab8a26beb7
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (40)
defaults.godefaults_js.godefaults_other.gogo.modp2p/test/webtransport/webtransport_test.gop2p/transport/webrtc/connection.gop2p/transport/webrtc/fingerprint.gop2p/transport/webrtc/hex.gop2p/transport/webrtc/hex_test.gop2p/transport/webrtc/listener.gop2p/transport/webrtc/logger.gop2p/transport/webrtc/sdp.gop2p/transport/webrtc/sdp_test.gop2p/transport/webrtc/stream.gop2p/transport/webrtc/stream_read.gop2p/transport/webrtc/stream_test.gop2p/transport/webrtc/stream_write.gop2p/transport/webrtc/stub_js.gop2p/transport/webrtc/transport.gop2p/transport/webrtc/transport_test.gop2p/transport/webtransport/cert_manager.gop2p/transport/webtransport/cert_manager_test.gop2p/transport/webtransport/conn.gop2p/transport/webtransport/conn_js.gop2p/transport/webtransport/crypto.gop2p/transport/webtransport/crypto_test.gop2p/transport/webtransport/listener.gop2p/transport/webtransport/listener_js.gop2p/transport/webtransport/mock_connection_gater_test.gop2p/transport/webtransport/multiaddr.gop2p/transport/webtransport/multiaddr_js.gop2p/transport/webtransport/multiaddr_test.gop2p/transport/webtransport/noise_early_data.gop2p/transport/webtransport/noise_early_data_js.gop2p/transport/webtransport/session_js.gop2p/transport/webtransport/stream.gop2p/transport/webtransport/stream_js.gop2p/transport/webtransport/transport.gop2p/transport/webtransport/transport_js.gop2p/transport/webtransport/transport_test.go
💤 Files with no reviewable changes (1)
- defaults.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: go-check / All
- GitHub Check: go-test / ubuntu (go 1.26.x)
- GitHub Check: go-test / ubuntu (go 1.25.x)
- GitHub Check: go-test / macos (go 1.25.x)
- GitHub Check: go-test / macos (go 1.26.x)
- GitHub Check: go-test / windows (go 1.25.x)
- GitHub Check: go-test / windows (go 1.26.x)
- GitHub Check: Run transport interoperability tests
🧰 Additional context used
🪛 GitHub Actions: Upstream Test
go.mod
[error] 1-1: go mod tidy failed: go.mod file indicates go 1.25, but maximum supported version is 1.16.
🔇 Additional comments (34)
p2p/transport/webrtc/logger.go (1)
1-2: Build constraint is correctly scoped for wasm/js compatibility.
//go:build !jscleanly prevents native WebRTC logger code from being compiled into js/wasm targets, which aligns with the PR’s transport split strategy.go.mod (1)
57-57:gojidirect dependency looks correctly scoped for wasm/js transport paths.This addition is consistent with the js-gated imports in
p2p/transport/webtransport/session_js.go,p2p/transport/webtransport/transport_js.go, andp2p/transport/webtransport/stream_js.go.p2p/transport/webtransport/mock_connection_gater_test.go (1)
1-1: Build constraint is correctly scoped.Good call adding the
!jsguard for this generated test mock so js/wasm builds don’t pull in non-applicable test code.p2p/transport/webrtc/sdp.go (1)
1-1:!jstag placement looks correct.This keeps the native SDP path out of js/wasm builds and matches the introduced platform separation.
p2p/transport/webtransport/cert_manager.go (1)
1-1: Build gating here is appropriate.Restricting cert-manager implementation to non-js targets is consistent with wasm/browser constraints.
p2p/transport/webtransport/multiaddr_js.go (2)
18-53: Helper conversion/parsing path looks solid.UDP enforcement plus explicit multibase/multihash decode errors is a good, defensive shape for JS WebTransport address handling.
55-89: State-machine validation is clear and maintainable.The ordered protocol progression (
udp -> quic-v1 -> webtransport) with certhash counting is straightforward and easy to reason about.p2p/transport/webtransport/multiaddr.go (1)
1-1: Non-js guard is correct.This cleanly avoids symbol collision with the new js-specific multiaddr implementation.
p2p/transport/webrtc/stub_js.go (1)
3-52: Stub design is well structured for js/wasm compatibility.Matching the non-js constructor/type signatures while returning a consistent unsupported error is the right tradeoff for platform-limited builds.
p2p/transport/webrtc/fingerprint.go (1)
1-1: Build tag update is correct here.Excluding this fingerprint implementation from js aligns with the new js stub strategy.
p2p/transport/webrtc/listener.go (1)
1-1:!jsconstraint is appropriate for this listener implementation.This cleanly routes js/wasm builds to the stub transport and avoids native runtime assumptions.
p2p/transport/webrtc/transport_test.go (1)
1-2: LGTM! Build constraint correctly excludes WebRTC tests from JS builds.The
//go:build !jsconstraint properly gates this test file from JS/WASM compilation, aligning with the PR's strategy of providing separate JS-specific implementations.p2p/transport/webrtc/sdp_test.go (1)
1-2: LGTM! Consistent build constraint pattern.The build tag correctly excludes SDP tests from JS builds, maintaining consistency with the broader PR pattern.
p2p/transport/webrtc/stream.go (1)
1-2: LGTM! Build constraint correctly applied to implementation.The
!jstag properly excludes the non-JS WebRTC stream implementation, allowing for the JS-specific alternative mentioned in the PR summary.p2p/transport/webtransport/crypto_test.go (1)
1-2: LGTM! WebTransport crypto tests appropriately gated.The build constraint correctly excludes crypto tests from JS builds, consistent with the PR's platform-specific implementation strategy.
p2p/transport/webtransport/transport_test.go (1)
1-2: LGTM! Consistent gating of WebTransport tests.The build tag properly excludes transport tests from JS compilation, maintaining the PR's clean separation between platform implementations.
p2p/transport/webtransport/stream.go (1)
1-2: LGTM! WebTransport stream implementation correctly gated.The
!jsconstraint appropriately excludes the non-JS stream implementation, aligning with the PR's platform-specific architecture.p2p/transport/webtransport/multiaddr_test.go (1)
1-2: LGTM! Multiaddr tests appropriately excluded from JS builds.The build constraint correctly gates these tests from JS compilation, consistent with the overall PR pattern.
p2p/transport/webtransport/listener.go (1)
1-2: LGTM! WebTransport listener correctly gated from JS builds.The
!jsbuild constraint properly excludes the non-JS listener implementation. This completes a consistent pattern across all 8 files in this review, cleanly separating platform-specific implementations for WASM support.p2p/transport/webrtc/connection.go (1)
1-2: LGTM! Build constraint correctly gates WebRTC for non-JS platforms.The
//go:build !jsconstraint appropriately excludes this Pion-based WebRTC connection implementation from JavaScript/WASM builds, where the browser's native WebRTC APIs would be used instead.p2p/transport/webrtc/transport.go (1)
1-2: LGTM! Build constraint properly isolates platform-specific WebRTC transport.The build tag ensures this implementation is excluded from JS builds, complementing the stub implementation in
stub_js.go.p2p/transport/webrtc/stream_test.go (1)
1-2: LGTM! Test gating aligns with implementation platform constraints.The build tag correctly prevents these Pion WebRTC-dependent tests from running on JS/WASM targets.
p2p/transport/webtransport/conn.go (1)
1-2: LGTM! Build constraint enables platform-specific WebTransport connection implementations.The constraint appropriately separates the quic-go-based connection (non-JS) from the browser API-based implementation (JS).
p2p/transport/webtransport/cert_manager_test.go (1)
1-2: LGTM! Certificate manager tests appropriately excluded from browser builds.Browser-based WebTransport handles certificates differently, making these tests inapplicable to JS environments.
p2p/transport/webtransport/transport.go (1)
1-2: LGTM! Transport build constraint enables platform-appropriate WebTransport implementations.The constraint correctly separates the full-featured transport (with Listen support) from the browser-based client-only implementation.
p2p/transport/webtransport/noise_early_data.go (1)
1-2: LGTM! Build constraint appropriately splits server and client-only early data handling.The constraint correctly separates the full implementation (with
newEarlyDataSenderfor servers) from the JS client-only version that omits server-side functionality.p2p/test/webtransport/webtransport_test.go (1)
1-2: LGTM! Integration test correctly excluded from browser environments.The test exercises Listen and certificate management features unavailable in browsers, making the build constraint appropriate.
p2p/transport/webrtc/hex_test.go (1)
1-1:!jstest gating looks correct.This build constraint cleanly keeps these tests aligned with the non-JS WebRTC hex implementation.
p2p/transport/webrtc/stream_write.go (1)
1-1: Build tag split is appropriate here.Keeping this write path out of JS builds matches the platform-specific transport split.
p2p/transport/webrtc/stream_read.go (1)
1-1:!jsguard is a good fit.This prevents non-applicable WebRTC stream-read code from entering JS/WASM builds.
p2p/transport/webrtc/hex.go (1)
1-1: Non-JS build constraint is consistent.No concerns with scoping this implementation to non-JS targets.
p2p/transport/webtransport/crypto.go (1)
1-1:!jsgating is correct for this crypto path.This cleanly separates non-JS certificate logic from JS/WASM transport code.
p2p/transport/webtransport/conn_js.go (1)
39-95: CapableConn implementation shape looks solid.The delegation pattern to
sessionplus explicit interface assertion is clean and consistent for the JS transport path.p2p/transport/webtransport/session_js.go (1)
16-69: Session lifecycle and stream wiring are clear and well-scoped.The split between
ready,openStream,acceptStream, andcloseis easy to follow and fits the JS transport abstraction.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@p2p/transport/webtransport/stream_js.go`:
- Around line 68-85: The current read and Write methods snapshot deadlines and
call reader.ReadContext / writer.WriteContext on a context that won't be
canceled if SetReadDeadline/SetWriteDeadline/SetDeadline is called while an I/O
is in flight; change to per-direction cancellable context management: add
per-direction cancellation state (e.g., readCancel and writeCancel fields,
protected by a mutex or atomic) and have
SetReadDeadline/SetWriteDeadline/SetDeadline call the existing cancel to abort
any in-progress ReadContext/WriteContext before installing the new deadline;
update stream.read and stream.Write to create a context whose cancel func is
stored in the per-direction field so deadline updates can cancel the in-flight
operation via the stored cancel and then cleared appropriately.
In `@p2p/transport/webtransport/transport_js.go`:
- Around line 87-89: CanDial currently returns true for WebTransport multiaddrs
even when Dial will fail due to unsupported certhashes; update CanDial (in
transport.CanDial) to perform the same certhash support check that Dial uses
(i.e., call IsWebtransportMultiaddr and then verify the multiaddr contains at
least one JS-supported certhash) and return false if no supported certhashes are
present so that undialable addresses are rejected up front.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d36e151a-f48a-424d-8788-1ad09fe6f4cb
📒 Files selected for processing (3)
p2p/transport/webtransport/listener_js.gop2p/transport/webtransport/stream_js.gop2p/transport/webtransport/transport_js.go
🚧 Files skipped from review as they are similar to previous changes (1)
- p2p/transport/webtransport/listener_js.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: go-check / All
- GitHub Check: go-test / ubuntu (go 1.25.x)
- GitHub Check: go-test / macos (go 1.26.x)
- GitHub Check: go-test / macos (go 1.25.x)
- GitHub Check: go-test / windows (go 1.25.x)
- GitHub Check: go-test / windows (go 1.26.x)
- GitHub Check: go-test / ubuntu (go 1.26.x)
- GitHub Check: Run transport interoperability tests
| func (s *stream) read(b []byte) (int, error) { | ||
| deadline := s.readDeadline.Load() | ||
| if deadline == nil { | ||
| return s.reader.Read(b) | ||
| } | ||
| ctx, cancel := context.WithDeadline(context.Background(), *deadline) | ||
| defer cancel() | ||
| return s.reader.ReadContext(ctx, b) | ||
| } | ||
|
|
||
| func (s *stream) Write(b []byte) (int, error) { | ||
| deadline := s.writeDeadline.Load() | ||
| if deadline == nil { | ||
| return s.writer.Write(b) | ||
| } | ||
| ctx, cancel := context.WithDeadline(context.Background(), *deadline) | ||
| defer cancel() | ||
| return s.writer.WriteContext(ctx, b) |
There was a problem hiding this comment.
Deadline changes only affect future I/O, not blocked calls.
read and Write snapshot the current deadline once and then block on that context. If another goroutine calls SetReadDeadline, SetWriteDeadline, or SetDeadline while a Read/Write is already in progress, the in-flight operation keeps waiting on the old context and may never unblock when the caller expects it to.
This needs per-direction cancellation state so deadline updates can cancel the currently running ReadContext / WriteContext, not just the next call.
Also applies to: 91-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@p2p/transport/webtransport/stream_js.go` around lines 68 - 85, The current
read and Write methods snapshot deadlines and call reader.ReadContext /
writer.WriteContext on a context that won't be canceled if
SetReadDeadline/SetWriteDeadline/SetDeadline is called while an I/O is in
flight; change to per-direction cancellable context management: add
per-direction cancellation state (e.g., readCancel and writeCancel fields,
protected by a mutex or atomic) and have
SetReadDeadline/SetWriteDeadline/SetDeadline call the existing cancel to abort
any in-progress ReadContext/WriteContext before installing the new deadline;
update stream.read and stream.Write to create a context whose cancel func is
stored in the per-direction field so deadline updates can cancel the in-flight
operation via the stored cancel and then cleared appropriately.
| func (t *transport) CanDial(addr ma.Multiaddr) bool { | ||
| ok, _ := IsWebtransportMultiaddr(addr) | ||
| return ok |
There was a problem hiding this comment.
Make CanDial reject undialable JS WebTransport addresses.
Dial hard-fails when the multiaddr has no supported certhashes, but CanDial still returns true for the same address. That violates the transport contract here and lets address selection pick this transport for dials that can never succeed on wasm.
Suggested fix
func (t *transport) CanDial(addr ma.Multiaddr) bool {
ok, _ := IsWebtransportMultiaddr(addr)
- return ok
+ if !ok {
+ return false
+ }
+ certHashes, err := extractCertHashes(addr)
+ return err == nil && len(certHashes) > 0
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@p2p/transport/webtransport/transport_js.go` around lines 87 - 89, CanDial
currently returns true for WebTransport multiaddrs even when Dial will fail due
to unsupported certhashes; update CanDial (in transport.CanDial) to perform the
same certhash support check that Dial uses (i.e., call IsWebtransportMultiaddr
and then verify the multiaddr contains at least one JS-supported certhash) and
return false if no supported certhashes are present so that undialable addresses
are rejected up front.
2 participants
This PR adds wasm build support. It also adds a wasm compatible version of the webtransport implementation.