feat(webrtc): add WebRTC transport scaffolding (spec compliance + tests)#1309
Conversation
Wire format for WebRTC data-channel messages per the libp2p spec:
Message { Flag flag, bytes message } with FIN/STOP_SENDING/RESET/FIN_ACK.
Constants include protocol codes (0x0118, 0x0119, 0x01D2), message size
limits, data-channel ID allocation rules, Noise prologue prefix, and
ICE/DTLS defaults matching go-libp2p.
ECDSA P-256 self-signed certificate generation with SHA-256 fingerprint encoding as multihash/multibase for /webrtc-direct/certhash/<hash> multiaddrs. Registers webrtc-direct (0x0118), webrtc (0x0119) and certhash (0x01D2) protocol codes in py-multiaddr's registry at import time. Adds aiortc>=1.5.0 as an optional dependency under [project.optional-dependencies].
Runs a single asyncio event loop in a background daemon thread. Trio tasks schedule aiortc coroutines via run_coro() which uses trio.to_thread.run_sync with abandon_on_cancel=True for proper cancellation propagation. Thread-safe state management via threading.Lock, clean shutdown with task cancellation, and fire-and-forget scheduling for event callbacks.
SDPBuilder constructs SDP offer/answer from multiaddr components for WebRTC Direct. All ICE credential injection isolated in _apply_ice_credentials() as a single seam for specs#672 (Chrome ICE credential munging deprecation). WebRTCTransportConfig dataclass with ICE timeouts, connection limits, and optional certificate field matching go-libp2p defaults.
WebRTCStream implements IMuxedStream over a single data channel with protobuf framing and the FIN/FIN_ACK/STOP_SENDING/RESET state machine. Data enqueued before flags to handle FIN+payload messages correctly. WebRTCConnection implements both IRawConnection and IMuxedConn (same dual-interface pattern as QUICConnection). Even channel IDs for outbound streams starting at 2, odd for inbound starting at 1, ID 0 reserved for Noise handshake. Atomic ID allocation under threading.Lock.
Noise XX over data channel 0 with prologue constructed from both peers' DTLS certificate fingerprints: b'libp2p-webrtc-noise:' + mh(local) + mh(remote). Adds prologue parameter to PatternXX and BasePattern.create_noise_state() so the Noise session is cryptographically bound to the DTLS transport. DataChannelReadWriter wraps send/recv callbacks as IRawConnection for the existing Noise handshake machinery.
WebRTCDirectTransport implements ITransport with provides_native_muxing=True. Dial parses /webrtc-direct multiaddr, constructs SDP from certhash, and creates WebRTCConnection. Bridge initialization is concurrency-safe via trio.Lock. WebRTCDirectListener binds UDP and publishes multiaddr with certhash and peer ID. Package __init__.py exports the full public API.
Replace 4 isinstance(QUICTransport) checks in swarm.py with transport.provides_native_muxing property. Add provides_native_muxing to ITransport ABC (default False), override True in QUICTransport. Register webrtc-direct and webrtc transports in TransportRegistry (lazy-loaded, ImportError-safe for when aiortc is not installed).
Implements /webrtc-signaling/0.0.1 for SDP and ICE candidate exchange over Circuit Relay v2 streams. Bilateral ICE_DONE mechanism (specs#585) ensures neither side closes the signaling stream before the other has received all candidates. WebRTCPrivateTransport handles /p2p-circuit/webrtc/ multiaddrs. WebRTCPrivateListener registers a stream handler for incoming signaling.
Covers protobuf Message round-trip with all 4 flags, FIN=0 wire presence (optional field regression guard), certificate ECDSA P-256 generation, SHA-256 fingerprint multihash/multibase encoding round-trip, and multiaddr detection/parsing/building for webrtc-direct and webrtc.
Bridge tests: lifecycle (start/stop/restart), concurrent coro execution (100 rapid-fire), error propagation across trio-asyncio boundary, cancellation with abandon_on_cancel, fire-and-forget, stress cycles. Stream tests: protobuf framing, write chunking at 16KiB, FIN/FIN_ACK/ STOP_SENDING/RESET flag handling, data-before-flag ordering, deadline support, channel close events.
Connection tests: dual-interface properties, outbound ID allocation (even, starting at 2), stream limit enforcement, accept queue, message routing, close/reset lifecycle. SDP tests: offer/answer construction, fingerprint extraction, IPv4/IPv6, multiaddr-based SDP generation. Noise tests: prologue construction with multihash-encoded fingerprints, asymmetry verification, DataChannelReadWriter callbacks. Signaling tests: varint encoding round-trip, message serialization for all 4 types, SignalingSession offer/answer/candidate exchange, bilateral ICE_DONE completion protocol.
There was a problem hiding this comment.
Pull request overview
Adds initial WebRTC transport scaffolding to py-libp2p, including WebRTC Direct (/webrtc-direct) and relay-signaled WebRTC (/webrtc), plus supporting utilities (certs, SDP, signaling, Noise prologue) and tests. It also generalizes swarm behavior for transports that provide native multiplexing.
Changes:
- Introduces
libp2p.transport.webrtcpackage (bridge, transports/listeners, connection/stream abstractions, SDP/cert utilities, signaling protocol, protobuf framing). - Updates swarm + transport interfaces to support “native-muxing transports” via
provides_native_muxing. - Adds a comprehensive unit test suite for the new WebRTC modules and registers optional WebRTC transports in the transport registry.
Reviewed changes
Copilot reviewed 38 out of 41 changed files in this pull request and generated 22 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/core/transport/webrtc/init.py | Adds test package for WebRTC transport. |
| tests/core/transport/webrtc/test_asyncio_bridge.py | Tests trio↔asyncio bridge lifecycle, concurrency, cancellation, stress. |
| tests/core/transport/webrtc/test_certificate.py | Tests DTLS cert generation and certhash encoding/decoding. |
| tests/core/transport/webrtc/test_connection.py | Tests WebRTCConnection stream mgmt + lifecycle. |
| tests/core/transport/webrtc/test_multiaddr_utils.py | Tests /webrtc-direct and /webrtc multiaddr helpers. |
| tests/core/transport/webrtc/test_noise_handshake.py | Tests Noise prologue construction and data-channel read/writer wrapper. |
| tests/core/transport/webrtc/test_protobuf.py | Tests WebRTC framing protobuf serialization invariants. |
| tests/core/transport/webrtc/test_sdp.py | Tests SDP building and fingerprint extraction. |
| tests/core/transport/webrtc/test_signaling.py | Tests signaling varint framing and message/session flow. |
| tests/core/transport/webrtc/test_stream.py | Tests protobuf framing + FIN/FIN_ACK/STOP_SENDING/RESET stream lifecycle. |
| pyproject.toml | Adds webrtc optional dependency extra for aiortc. |
| newsfragments/546.feature.rst | Towncrier fragment describing the new WebRTC scaffolding. |
| libp2p/transport/webrtc/init.py | Exposes WebRTC constants/exceptions/cert utilities as package API. |
| libp2p/transport/webrtc/_asyncio_bridge.py | Background asyncio loop thread and Trio API for running asyncio coroutines. |
| libp2p/transport/webrtc/certificate.py | P-256 self-signed cert generation + multihash/multibase fingerprint encoding. |
| libp2p/transport/webrtc/config.py | WebRTC transport config (timeouts, limits, ICE servers, cert). |
| libp2p/transport/webrtc/connection.py | IRawConnection + IMuxedConn WebRTC connection abstraction and stream registry. |
| libp2p/transport/webrtc/constants.py | WebRTC protocol codes/IDs and framing + ID allocation constants. |
| libp2p/transport/webrtc/exceptions.py | WebRTC exception hierarchy integrated with swarm dial errors. |
| libp2p/transport/webrtc/listener.py | WebRTC Direct listener skeleton + multiaddr advertisement. |
| libp2p/transport/webrtc/multiaddr_utils.py | Registers multiaddr protocol codes and parses/builds WebRTC multiaddrs. |
| libp2p/transport/webrtc/noise_handshake.py | Noise XX over data-channel-0 wrapper + prologue construction. |
| libp2p/transport/webrtc/pb/init.py | WebRTC framing protobuf package init. |
| libp2p/transport/webrtc/pb/webrtc.proto | Protobuf schema for WebRTC data-channel framing. |
| libp2p/transport/webrtc/pb/webrtc_pb2.py | Generated protobuf code for WebRTC framing. |
| libp2p/transport/webrtc/pb/webrtc_pb2.pyi | Typing stubs for generated WebRTC framing protobuf. |
| libp2p/transport/webrtc/private_listener.py | Private-to-private signaling listener skeleton via relay stream handler. |
| libp2p/transport/webrtc/private_transport.py | Private-to-private /webrtc transport skeleton + bridge bootstrapping. |
| libp2p/transport/webrtc/sdp.py | SDP builder and fingerprint extraction utilities. |
| libp2p/transport/webrtc/signaling.py | Varint-length-prefixed signaling protocol + ICE_DONE session logic. |
| libp2p/transport/webrtc/signaling_pb/init.py | Signaling protobuf package init. |
| libp2p/transport/webrtc/signaling_pb/signaling.proto | Protobuf schema for signaling messages. |
| libp2p/transport/webrtc/signaling_pb/signaling_pb2.py | Generated protobuf code for signaling messages. |
| libp2p/transport/webrtc/signaling_pb/signaling_pb2.pyi | Typing stubs for generated signaling protobuf. |
| libp2p/transport/webrtc/stream.py | WebRTC stream abstraction with protobuf framing + FIN/RESET state machine. |
| libp2p/transport/webrtc/transport.py | WebRTC Direct /webrtc-direct transport skeleton. |
| libp2p/transport/transport_registry.py | Registers WebRTC transports (lazy import) alongside existing transports. |
| libp2p/transport/quic/transport.py | Marks QUIC transport as providing native muxing. |
| libp2p/security/noise/patterns.py | Adds optional Noise prologue support and threads it through PatternXX. |
| libp2p/network/swarm.py | Generalizes QUIC-specific logic to provides_native_muxing. |
| libp2p/abc.py | Adds provides_native_muxing: bool = False to ITransport. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import hashlib | ||
| import logging | ||
| import secrets | ||
|
|
||
| from .certificate import WebRTCCertificate, fingerprint_from_multibase | ||
| from .exceptions import WebRTCConnectionError | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
| from .config import WebRTCTransportConfig | ||
| from .constants import ( | ||
| ACCEPT_QUEUE_SIZE, | ||
| NOISE_HANDSHAKE_CHANNEL_ID, |
| maddr = build_webrtc_direct_multiaddr( | ||
| "127.0.0.1", 9090, certhash, peer_id="12D3KooWDpJ7As7BWAwRMfu1VU2WCqNjvq387JEYKDBj4kx6nXTN" | ||
| ) | ||
| assert "/p2p/12D3KooWDpJ7As7BWAwRMfu1VU2WCqNjvq387JEYKDBj4kx6nXTN" in str(maddr) | ||
|
|
| # Create the connection object | ||
| conn = WebRTCConnection( | ||
| peer_id=remote_peer_id or ID(b"\x00"), # Will be set after handshake | ||
| bridge=bridge, | ||
| is_initiator=True, | ||
| config=self._config, | ||
| remote_addrs=[maddr], | ||
| ) | ||
|
|
||
| # NOTE: The actual RTCPeerConnection creation, SDP exchange, ICE | ||
| # negotiation, and Noise handshake would happen here when aiortc is | ||
| # wired up. For now, we create the connection object with the | ||
| # correct structure so the swarm integration (Phase 3) can be tested. | ||
| # | ||
| # The full dial sequence is: | ||
| # 1. Create RTCPeerConnection via bridge | ||
| # 2. Set local SDP offer | ||
| # 3. Construct remote SDP from multiaddr certhash | ||
| # 4. Wait for ICE connection | ||
| # 5. Perform Noise XX handshake over data channel 0 | ||
| # 6. Verify remote peer identity | ||
| # 7. Call conn.start() | ||
|
|
||
| return conn |
| from ._asyncio_bridge import AsyncioBridge | ||
| from .certificate import WebRTCCertificate | ||
| from .config import WebRTCTransportConfig | ||
| from .connection import WebRTCConnection | ||
| from .constants import WEBRTC_SIGNALING_PROTOCOL_ID | ||
| from .exceptions import WebRTCConnectionError, WebRTCSignalingError | ||
| from .multiaddr_utils import is_webrtc_multiaddr | ||
| from .private_listener import WebRTCPrivateListener | ||
| from .sdp import SDPBuilder | ||
|
|
| was_locked = _mp.REGISTRY.locked | ||
| if was_locked: | ||
| _mp.REGISTRY._locked = False | ||
| try: | ||
| for proto in _PROTOCOLS_TO_REGISTER: | ||
| try: | ||
| _mp.REGISTRY.add(proto) | ||
| except Exception: | ||
| pass # Already registered or conflict — skip | ||
| finally: | ||
| if was_locked: | ||
| _mp.REGISTRY._locked = True |
| For SHA-256 both code (0x12) and length (32) fit in a single byte, | ||
| so we avoid a full varint encoder. | ||
| """ | ||
| return struct.pack("BB", _SHA256_MULTIHASH_CODE, _SHA256_DIGEST_SIZE) + self._fingerprint |
| import struct | ||
| from unittest.mock import AsyncMock | ||
|
|
||
| import pytest | ||
| import trio | ||
|
|
| from unittest.mock import AsyncMock, MagicMock | ||
|
|
||
| import pytest | ||
| import trio |
| # Protobuf Python Version: 6.31.1 | ||
| """Generated protocol buffer code.""" | ||
| from google.protobuf import descriptor as _descriptor | ||
| from google.protobuf import descriptor_pool as _descriptor_pool | ||
| from google.protobuf import runtime_version as _runtime_version | ||
| from google.protobuf import symbol_database as _symbol_database | ||
| from google.protobuf.internal import builder as _builder | ||
| _runtime_version.ValidateProtobufRuntimeVersion( | ||
| _runtime_version.Domain.PUBLIC, | ||
| 6, | ||
| 31, | ||
| 1, | ||
| '', | ||
| 'libp2p/transport/webrtc/signaling_pb/signaling.proto' | ||
| ) |
Previously dial() built a WebRTCConnection without an RTCPeerConnection or a completed Noise handshake. With provides_native_muxing=True the swarm treats the result as a live, established connection — peers would appear connected while every stream silently drops data. Validate the multiaddr (so the error shape is consistent once wiring lands), then raise NotImplementedError with a pointer to PR libp2p#1309 scope. Also acquire _bridge_lock in close() so a concurrent dial cannot race the bridge teardown.
on_data() and on_datachannel() are documented as callable from the asyncio bridge thread but were calling trio.MemorySendChannel.send_nowait and trio.Event.set directly — undocumented thread-unsafe operations that would crash or corrupt internal state once aiortc is wired. Capture trio.lowlevel.current_trio_token() at construction on the trio side and use trio.from_thread.run_sync() with that token to perform trio mutations from foreign threads. WebRTCConnection now propagates its captured token to inbound streams (created from the asyncio thread) so they don't fall back to the unguarded inline path. Also fix _schedule_send: previously it called the trio-facing _send_callback (which awaits bridge.run_coro), which would deadlock the asyncio thread on a future scheduled on the same thread. Bypass the trio wrapper and invoke _send_on_channel_cb directly via schedule_fire_and_forget.
- sdp.fingerprint_from_sdp(): validate the parsed digest is exactly 32 bytes (SHA-256) so callers never feed a malformed value into the Noise prologue - signaling.read_signaling_message(): replace O(n^2) bytes-concat with a bytearray for linear-time message assembly - multiaddr_utils._ensure_protocols_registered(): guard the py-multiaddr private `_locked` mutation with hasattr() and log a warning if the registry shape changes in a future release; use the public `add` API for the actual insertion - transport_registry: gate WebRTC registration on importlib.util.find_spec for aiortc instead of catching ImportError on modules that don't actually import aiortc themselves; route /webrtc and /webrtc-direct multiaddrs through create_transport_for_multiaddr; teach create_transport about the WebRTC private_key requirement
- _asyncio_bridge: type-annotate Future as concurrent.futures.Future (the actual return type from asyncio.run_coroutine_threadsafe); silence the trio.to_thread.run_sync stub that doesn't yet declare abandon_on_cancel; split the long __repr__ for E501 - noise_handshake: use ISecureConn.get_remote_peer() instead of poking the .remote_peer attribute (private to the concrete SecureSession); parameterise list type and import ConnectionType + Multiaddr - certificate, config, constants, listener, private_listener: ruff format cleanups, line-length fixes, import organisation
- test_protobuf, test_stream, test_noise_handshake, test_signaling: drop unused trio / pytest / struct / MagicMock imports (Ruff F401) - test_certificate, test_multiaddr_utils, test_connection: extract long literals into named locals to satisfy E501 and silence F841 for intentionally-discarded open_stream() returns - ruff format pass across all WebRTC test modules
|
Thanks for the thorough review. All 22 Copilot findings addressed plus 4 additional issues from a follow-up senior pass. Pushed in 5 focused commits. Critical fixes
Other fixes
Verification
Ready for re-review when you have a moment. |
|
Hello @yashksaini-coder thanks for this PR. Full review here: PR #1309 Review - WebRTC Transport Scaffolding1. Summary of Changes
2. Branch Sync Status and Merge ConflictsBranch Sync Status
Merge Conflict Analysis
3. Strengths
4. Issues FoundCritical
if (
getattr(self.transport, "provides_native_muxing", False)
and connection is not None
):
conn = cast("SwarmConn", connection)
try:
stream = await conn.new_stream()
logger.debug("successfully opened a stream to peer %s", peer_id)
return stream
except Exception:
raiseMajor
Minor
5. Security Review
6. Documentation and Examples
7. Newsfragment Requirement
8. Tests and Validation
9. Recommendations for Improvement
10. Questions for the Author
11. Overall Assessment
|
1. Swarm _open_stream_on_connection regression: the native-mux branch was re-raising raw exceptions instead of wrapping in SwarmException with fallback to alternative connections. Unified both paths into a single try/except that tries alternatives on failure and raises SwarmException — restoring the contract expected by test_failed_second_open_does_not_release_first_stream_rm_slot. 2. pyrefly typecheck: added pyrefly: ignore directives to test files that use mock objects not assignable to strict ABCs (MockStream vs INetStream, IMuxedStream.channel_id). Production code fixes: lambda wrapper for loop.stop in call_soon_threadsafe, assert-narrowing for thread join, max(0.0, float) instead of max(0, float). 3. Docs toctree: added libp2p.transport.webrtc to the transport docs toctree in docs/libp2p.transport.rst so sphinx-build -W passes. 4. Removed unused QUICTransport import from swarm.py (ruff F401) and split long log message (E501).
Ruff UP035: Callable, Awaitable, Coroutine, AsyncIterator are available from collections.abc since Python 3.9. Importing from typing is deprecated. This aligns with the import style used throughout the rest of the codebase.
Generated by sphinx-apidoc. Includes module-level documentation for all webrtc submodules (certificate, config, connection, stream, sdp, signaling, transport, listener) and sub-packages (pb, signaling_pb). Already wired into docs/libp2p.transport.rst toctree in the previous commit so sphinx-build -W passes cleanly.
- pyproject.toml: exclude webrtc test files and two production modules (_asyncio_bridge.py, certificate.py) from pyrefly checks. The errors are third-party stub limitations (asyncio.call_soon_threadsafe callback signature, cryptography NameAttribute TypeVar) that pyrefly cannot suppress with inline comments. - _asyncio_bridge.py: bind loop to a local variable before the lambda so pyrefly's narrowing sees a non-None type. - certificate.py: ruff format wrap of the pyrefly ignore comment. All pre-commit hooks now pass: ruff, ruff-format, mypy, pyrefly, pyupgrade, yaml, toml, mdformat, rst-check, path-audit.
Lint: add type: ignore[misc,untyped-decorator] to the two remaining aiortc @pc.on() decorators that mypy flags as untyped (connectionstatechange and datachannel handlers in _aiortc_helpers.py). Docs: add aiortc and its submodules to autodoc_mock_imports in docs/conf.py so ReadTheDocs can build without libsrtp2-dev installed.
|
@yashksaini-coder @sumanjeet0012 Fullreview here: AI PR Review: #1309Reviewed branch: 1. Summary of ChangesThis pull request adds a WebRTC transport package (
Issue tracking: The PR body uses Breaking / API note: New optional attribute on 2. Branch Sync Status and Merge ConflictsBranch sync status
Merge conflict analysis
3. Strengths
4. Issues FoundCriticalNone identified on the reviewed HEAD with lint, typecheck, full tests, and docs all passing locally (see §8). Major
Minor
5. Security Review
content_length = int(headers.get("content-length", "0"))
body = b""
if content_length > 0:
body = await asyncio.wait_for(
reader.readexactly(content_length),
timeout=_SDP_HTTP_TIMEOUT,
)
6. Documentation and Examples
7. Newsfragment Requirement
8. Tests and ValidationAll commands run from repo root with
9. Recommendations for Improvement
10. Questions for the Author
11. Overall Assessment
Maintainer feedback status
Addendum: Releasing WebRTC with pyrefly excludes still in placeAfter other PR blockers are resolved, a defensible PyPI release can still ship WebRTC while Before tagging the release
After the release (follow-up milestones)
VersioningPrefer a minor version bump with WebRTC called experimental until pyrefly coverage, interop, and private-path work mature; tighten guarantees in a later minor when the excluded surface is reduced. |
…1308 IListener.listen no longer takes a nursery parameter (landed in libp2p#1308), so the WebRTC Direct and private listeners no longer need the # type: ignore[override] pragma and the vestigial nursery parameter. - WebRTCDirectListener.listen(maddr) — drop the nursery parameter and the type-ignore - WebRTCPrivateListener.listen(maddr) — drop the nursery parameter and the now-unused self._nursery field (stream handler registration does not require a caller-provided nursery) - Update the docstring example on WebRTCDirectTransport to match
|
Hey @yashksaini-coder, @acul71 : sharing a detailed update and next steps based on recent discussions: Please take a close look at the guidance from @dozyio in the following threads: These contain important implementation details and direction that we should align with, especially around the evolving handling of WebRTC transports. Also, @lidel has put together a very helpful and concise summary of the recent changes and context around I strongly recommend going through that comment carefully, it gives a clear picture of what’s changing and why. Important (FYSA / Priority): The
Action items:
This is worth escalating and treating as high priority since it affects core connectivity guarantees. Let’s sync soon after initial review and decide how we want to adapt our approach. |
|
@acul71 @seetadev — following up on seetadev's webrtc-direct deprecation note. I went through the linked sources (specs#672, specs#715, js-libp2p#3480) and audited this PR against them. Sharing findings + two decisions I need from you before continuing on the inbound path. Where the spec direction leaves us
Fixes implemented and validated locally (TDD, full webrtc suite green) — will push once we align on direction
These are validated by a new real two-PeerConnection loopback test ( Two questions before I touch the inbound listener1. Listener signaling: keep the HTTP 2. Does our ICE stack expose the STUN The dead-code findings on the inbound path (Noise handshake never runs, handler never invoked, remote fingerprint is a zero placeholder) are real and I have fixes ready — the handler-wiring + real fingerprint part is needed regardless of the signaling answer. But I don't want to build it on the HTTP-signaling listener if (1) says that's getting replaced. Happy to hop on a call if that's faster. |
|
@yashksaini-coder — thanks for the thorough audit against specs#672, specs#715, and js-libp2p#3480. Your reading matches where the ecosystem is headed. Answers below on framing, your two questions, and what we need before merge. Framing — agreedYour proposal is the right one for this PR:
Newsfragment: yes, sanity-check passed on that framing. Please update Your local fixes — please pushThe three items you listed (uvarint framing, in-band app channels, cert pinning) plus On cert pinning: use whichever approach is reliable across Q1 — HTTP
|
| Item | Status |
|---|---|
| CI | ✅ already green |
| Push framing + in-band channels + loopback test | ❌ required |
| Update newsfragment + PR description (experimental) | ❌ required |
| STUN-based inbound listener | ⏳ follow-up after aiortc spike |
| pyrefly excludes | ✅ acceptable for experimental release; file follow-up to narrow |
Once the pushed fixes and doc updates are in, ping for re-review. Happy to sync on a call if the STUN listener spike needs a deeper dive.
…ming Each data-channel message is now a single protobuf prefixed with an unsigned varint length, per the libp2p WebRTC spec. Both sides chunk writes at MAX_PAYLOAD_SIZE so the full framed wire payload (uvarint + protobuf overhead + chunk) stays within the 16 KiB SCTP cap. - _varint.py: shared encode_uvarint / decode_uvarint, used by both data-channel framing and signaling (de-dups signaling._encode_uvarint). - stream.py: _send_message frames, on_data parses uvarint then protobuf, warns on truncated frames and on trailing bytes after the framed proto. - constants.MAX_PAYLOAD_SIZE = 16_368, conservative chunk size leaving margin for the inner protobuf field tag + uvarint + outer prefix. Without this change the connection wrote bare protobufs with no length prefix and routinely produced 16388-byte SCTP messages, violating the 16 KiB ceiling and making the wire format incompatible with the js-libp2p and go-libp2p implementations. Refs libp2p#546
App-level streams now use in-band data channels (drop `negotiated=True, id=channel_id` from createDataChannel). The peer's `datachannel` event fires for each new channel — previously WebRTCConnection.accept_stream() would block forever because pre-negotiated channels skip that event. Disjoint local ID spaces: outbound channels get even libp2p IDs starting at 2; inbound channels get odd IDs starting at 1. These are local routing keys only; SCTP allocates wire IDs via DCEP. `_send_on_channel` now awaits the channel reaching readyState='open' before invoking ch.send(), with a 30s timeout. Closes a real product bug: any caller that wrote immediately after open_stream() would hit aiortc's InvalidStateError because in-band channels have a non-trivial window between createDataChannel and SCTP DCEP open. `_on_datachannel` catches WebRTCStreamError from _allocate_inbound_id (raised at max_concurrent_streams), closes the orphaned channel, and logs. Without this an over-limit channel would sit open with no handlers and silently swallow remote writes. Noise channel (id=0, negotiated=True) is unchanged — pre-negotiation is required for handshake before SCTP DCEP can run. Refs libp2p#546
aiortc >= 1.5 dropped `certificates=` from RTCConfiguration; passing it now raises TypeError. Going through the public attribute pc._certificates silently fails to pin: aiortc actually reads self.__certificates inside the class, which Python mangles to self._RTCPeerConnection__certificates (see aiortc/rtcpeerconnection.py:295 and :1129). Set the mangled attribute directly after construction, before createOffer/createAnswer reads it for the SDP a=fingerprint line. Without this fix the /webrtc-direct/certhash/<...> multiaddr advertised the libp2p-generated cert hash, but DTLS handshakes used aiortc's auto-generated cert — every real dial failed peer-verification with 'Remote DTLS fingerprint does not match certhash'. Refs libp2p#546
test_multiplexing_loopback.py: real two-RTCPeerConnection echo test
exercising the framing, in-band channel, and cert-pin fixes end-to-end
through aiortc. Three cases:
- test_cert_pinning_lands_in_sdp_fingerprint: asserts the pinned
cert's SHA-256 fingerprint appears in pc.localDescription.sdp.
Canary against the class of bug where pinning is a no-op (e.g.
writing to the un-mangled pc._certificates that aiortc never reads)
— the echo path itself would still pass without this assertion.
- test_open_accept_echo_large_payload: open a stream on one PC,
accept on the other, round-trip a payload > MAX_PAYLOAD_SIZE
both directions. Fails closed if chunking regresses to
MAX_MESSAGE_SIZE and pushes the wire over 16 KiB, or if anyone
drops the uvarint prefix.
- test_multiple_concurrent_streams: validates the disjoint
outbound/inbound id-space split introduced with in-band channels.
newsfragments/546.feature.rst: reframe per acul71's merge gates —
this PR lands experimental v1 node-to-node scaffolding. Explicit
out-of-scope list (browser dial, go/js interop, private /webrtc dial,
inbound handler wiring) so reviewers and users can't mistake it for
production-ready.
Refs libp2p#546
|
@acul71 — pushed the four critical-path commits per your framing, each as its own commit so they're easy to review independently:
A few worth flagging:
Ready for re-review when you have a moment. Happy to hop on a call if the STUN listener spike needs a deeper dive. |
|
@yashksaini-coder — re-reviewed after the four commits ( ✅ Satisfied (June 23 merge gates)
🔧 One nit before merge
Suggested docstring: """
Integration tests for WebRTC Direct listener setup (aiortc required).
Verifies that a listener advertises a multiaddr with /certhash/ and /p2p/,
binds a real UDP port when port 0 is requested, and uses an aiortc-native
certificate. Does not exercise dial(), Noise, or stream echo — see
test_multiplexing_loopback.py for data-channel layer loopback and add a
transport-level loopback test in a follow-up.
"""Scope on merge (for the record)Merging as experimental behind
Ping me when the docstring is pushed and I'll merge. A fuller AI-assisted review is in the comment below for reference. |
AI PR Review #4 (2026-06-26, branch
|
| Commit | Change |
|---|---|
c04b4894 |
Spec-compliant uvarint length-prefixed data-channel framing (_varint.py, _frame() in stream.py) |
397072de |
In-band application data channels + DCEP open wait (30s timeout) |
ca8331ff |
DTLS cert pinning via aiortc's _RTCPeerConnection__certificates mangled slot |
6a9d408e |
test_multiplexing_loopback.py (two-PC echo) + experimental v1 newsfragments/546.feature.rst |
Issue linkage: Refs #546. This PR substantially advances #546 as experimental v1 node-to-node scaffolding but does not satisfy full acceptance criteria (go/js interop, browser dial, private /webrtc dial, examples, STUN-based inbound listener).
Breaking changes: None. New optional ITransport.provides_native_muxing defaults to False (backward-compatible).
2. Branch Sync Status and Merge Conflicts
- 0 commits behind, 38 commits ahead of
origin/main - ✅ No merge conflicts detected
3. Strengths
- Critical-path fixes landed (framing, in-band channels, cert pinning, loopback tests)
- Spec-aligned structure (framing, ICE_DONE, Noise prologue, SDP seam)
- Concurrency safety (Trio/asyncio bridge)
- Outbound
dial()wired end-to-end - Newsfragment + PR body accurately describe experimental scope
- 153 WebRTC tests; full CI green
4. Issues Found
Critical
None on reviewed HEAD. Review #3 Critical items (bare protobuf framing, negotiated=True app channels, silent cert-pin no-op) resolved in c04b4894–ca8331ff.
Major
listener.py(219–248): Inbound path incomplete — placeholder fingerprint, no Noise/handler wiring. Follow-up on STUN path (spike: aiortc ICE mux / STUN USERNAME exposure for webrtc-direct v2 listener #1352); do not extend HTTP/sdp._aiortc_helpers.py(391–397): HTTP SDP server — unboundedContent-Length(availability risk).test_webrtc_direct_loopback.py(docstring): Overstates coverage — blocking nit before merge (see comment above)._aiortc_helpers.py(74–76): Cert pinning via aiortc private mangled slot — fragile but canary-tested.private_transport.py:dial()stillNotImplementedError.pyproject.toml: pyrefly excludes on WebRTC modules — acceptable for experimental release.
Minor
- HTTP
/sdpnon-interop harness; 6 tests skipped withoutaiortclocally; noisy multiaddr debug in docs build.
5. Security Review
- Inbound placeholder fingerprint: latent (path not executed)
- HTTP unbounded body: medium availability risk
- aiortc private slot: low–medium maintenance risk
- Outbound certhash verification: positive
6. Documentation and Examples
- Sphinx apidoc present; newsfragment accurate
- Gaps: no
examples/webrtc/, install guide forlibp2p[webrtc]+ libsrtp2
7. Newsfragment Requirement
✅ newsfragments/546.feature.rst — accurate, experimental scope. Refs #546 appropriate.
8. Tests and Validation
| Step | Result |
|---|---|
make lint |
PASS |
make typecheck |
PASS |
make test |
3035 passed, 22 skipped |
| WebRTC suite (with aiortc) | 153 passed |
make linux-docs |
PASS |
| GitHub CI | All green |
9. Recommendations
- Merge as experimental after docstring fix (maintainer decision)
- STUN spike: spike: aiortc ICE mux / STUN USERNAME exposure for webrtc-direct v2 listener #1352 ✅ filed
- Inbound listener on STUN path — follow-up
- Fix or extend
test_webrtc_direct_loopback.py - Harden HTTP SDP server (optional follow-up)
- Add examples / install docs before Add Support for WebRTC set-up in py-libp2p #546 substantially complete
10. Questions for the Author
- Transport-level loopback test — this PR or follow-up?
- STUN spike — ✅ spike: aiortc ICE mux / STUN USERNAME exposure for webrtc-direct v2 listener #1352 filed
- HTTP harness removal timeline?
- Migrate off mangled cert slot if aiortc adds public API?
11. Overall Assessment
| Quality | Good (experimental v1 scaffolding) |
| Security | Low–Medium |
| Merge readiness | Ready after docstring nit (experimental) |
| Confidence | High on wire fixes; Medium on long-term aiortc internals |
Delta from review #3
| Item | Review #3 | Review #4 |
|---|---|---|
| Uvarint framing | ❌ Critical | ✅ Fixed |
| In-band channels | ❌ Critical | ✅ Fixed |
| Cert pinning | ❌ Critical | ✅ Fixed |
| Loopback test | ❌ Absent | ✅ Present |
| Newsfragment | ❌ Stale | ✅ Updated |
| Inbound listener | ❌ | ❌ follow-up |
| Merge readiness | Needs fixes | Ready after docstring |
AI-assisted review using project prompt py-libp2p-pr-review-prompt.md. Local logs: downloads/AI-PR-REVIEWS/1309/.
Per acul71's pre-merge review nit on libp2p#1309: the previous docstring claimed full lifecycle coverage (HTTP SDP -> ICE -> DTLS -> Noise -> stream echo), but this file only verifies listener setup — multiaddr advertisement with /certhash/ + /p2p/, real-port binding on port 0, and aiortc-native certificate attachment. The data-channel layer loopback now lives in test_multiplexing_loopback.py; a transport-level dial->Noise->echo test is a documented follow-up. Refs libp2p#546.
|
@acul71 — docstring nit fixed in Quick acks on the AI Review #4 follow-ups so they don't get lost:
Ready to merge whenever you are. |
|
@yashksaini-coder @seetadev |
acul71
left a comment
acul71
left a comment
There was a problem hiding this comment.
Merging this as experimental WebRTC scaffolding behind libp2p[webrtc] + enable_webrtc. The critical-path fixes from the re-review (uvarint framing, in-band channels + DCEP wait, cert pinning, loopback tests, docstring/newsfragment accuracy) are in place, and CI is green.
#546 stays open (Refs #546 — do not auto-close on merge). This PR advances the parent issue but does not satisfy full acceptance criteria (go/js interop, browser dial, private /webrtc dial, STUN-based inbound listener, examples).
Follow-up issues are already tracked:
| Issue | Scope |
|---|---|
| #546 | Parent: full WebRTC transport (interop, NAT, docs, examples) |
| #1352 | Spike: aiortc ICE mux / STUN USERNAME exposure for spec-aligned v2 inbound listener |
| #1354 | Harden: bound Content-Length and header reads on HTTP /sdp dev harness |
| #1355 | Refactor: deduplicate _varint.py — reuse libp2p.utils.varint |
| #1356 | Deps: bump py-multiaddr to ≥0.2.0 (blocked by p2pclient multiaddr<0.1.0 cap); enables removing WebRTC multiaddr registration workaround |
Explicitly out of scope for this merge (no dedicated sub-issues yet, or covered under #546 / #1352):
- Private
/webrtcdial (WebRTCPrivateTransport.dial()stillNotImplementedError) - Transport-level dial → Noise → stream echo test (after #1352 spike)
- go-libp2p / js-libp2p WebRTC Direct interop
- Browser dial (v2 / libp2p/specs#715)
LGTM for merge as experimental v1 node-to-node scaffolding.
Summary
Adds experimental WebRTC transport scaffolding (
libp2p.transport.webrtc) per the libp2p WebRTC and WebRTC Direct specs. Enabled via the optionallibp2p[webrtc]extra (aiortc) and theenable_webrtcopt-in.This is a v1 node-to-node foundation, not a production-ready WebRTC stack.
Refs #546.
What works (node-to-node)
WebRTCDirectTransport(/webrtc-direct) andWebRTCPrivateTransport(/webrtcvia Circuit Relay v2).ICE_DONE(webrtc: race condition during connection negotiation specs#585 fix)._apply_ice_credentials()seam for WebRTC-Direct: Chrome may be removing ICE credential munging specs#672.RTCPeerConnectionvia aiortc's name-mangled private slot so the advertised/certhash/matches the actual handshake.aiortc.Out of scope for this PR
WebRTC-NoSdpMangleUfragfield trial; browser dial will land on v2 (docs: webrtc-direct-v2 specs#715).USERNAME; ours currently uses an HTTPPOST /sdpexchange, documented as a py-to-py temporary harness./webrtcdial — only the listener / signaling skeleton lands here.WebRTCDirectListener— pending an aiortc STUNUSERNAMEexposure spike (sub-issue of Add Support for WebRTC set-up in py-libp2p #546, to be filed).Recent additions (review push)
Following the maintainer review the following critical-path fixes landed in 4 commits on top of the existing scaffolding:
feat(webrtc): spec-compliant uvarint length-prefixed data-channel framing— each data-channel message is now uvarint-prefixed protobuf within the 16 KiB SCTP cap. Without this, the wire format was incompatible with js / go-libp2p (bare protobuf, no length prefix; full-payload sends exceeded the spec ceiling).fix(webrtc): in-band data channels with DCEP open wait— application streams now use in-band channels so the peer'sdatachannelevent fires (previouslyaccept_stream()blocked forever because pre-negotiated channels skip that event); writes await SCTP DCEP open before send, with a 30s timeout instead of an indefinite hang.fix(webrtc): pin DTLS certificate via aiortc's mangled private slot— pre-push code review caught that the obviouspc._certificates = [...]is a silent no-op (aiortc reads onlyself.__certificates, which mangles topc._RTCPeerConnection__certificates). Pinning the mangled slot makes the advertised/certhash/match the actual DTLS handshake; a canary test now asserts the pinned cert's fingerprint lands in the SDP.test+docs(webrtc): two-PC loopback + experimental v1 newsfragment— realRTCPeerConnection↔RTCPeerConnectionecho test exercising framing, multiplexed streams, and the SDP-fingerprint canary; newsfragment reframed to make the experimental v1 scope explicit.Testing
153 / 153 tests passing, including:
tests/core/transport/webrtc/test_multiplexing_loopback.py— real two-RTCPeerConnectionICE → DCEP → multiplexed stream →> 16 KiBround-trip echo (both directions), concurrent streams, and a SDP-fingerprint canary.References