feat(webrtc-utils): move the offer SDP template alongside the answer - #6572
Open
yexiyue wants to merge 2 commits into
Open
feat(webrtc-utils): move the offer SDP template alongside the answer#6572yexiyue wants to merge 2 commits into
yexiyue wants to merge 2 commits into
Conversation
`libp2p-webrtc-utils` owns half of the SDP pair: `sdp::answer` and `SERVER_SESSION_DESCRIPTION` live here, while the offer template sits private in `transports/webrtc/src/tokio/sdp.rs`. The split is visible in the source -- `SERVER_SESSION_DESCRIPTION`'s doc comment opens with "See [`CLIENT_SESSION_DESCRIPTION`]", pointing at a constant in another crate. Move `CLIENT_SESSION_DESCRIPTION` here with its annotations and add `sdp::offer` mirroring `sdp::answer`, so both descriptions of the libp2p WebRTC handshake are in one place. The offer takes the fingerprint as an argument rather than hardcoding `Fingerprint::FF`, which keeps the two functions symmetric and leaves the "we don't verify the client certificate" decision at the call site where it is explained. `libp2p-webrtc`'s `sdp::offer` becomes the same thin `RTCSessionDescription` wrapper its `sdp::answer` already was. No behaviour change: the rendered offer is byte-for-byte what it was.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
libp2p-webrtc-utilsowns half of the SDP pair.sdp::answerandSERVER_SESSION_DESCRIPTIONlive there; the offer template sits private intransports/webrtc/src/tokio/sdp.rs. The split is visible in the source — the doc comment onSERVER_SESSION_DESCRIPTIONopens with:// See [`CLIENT_SESSION_DESCRIPTION`].pointing at a constant in a different crate, where the shared RFC annotations for both templates
actually live.
This moves
CLIENT_SESSION_DESCRIPTIONover with its annotations and addssdp::offermirroringsdp::answer, so both descriptions of the libp2p WebRTC handshake sit in one place:The offer takes the fingerprint as an argument rather than hardcoding
Fingerprint::FF. Thatkeeps the two symmetric and leaves the "we don't verify the client's certificate, its identity
comes from the Noise handshake" decision at the call site, where the comment explaining it already
is.
libp2p-webrtc'ssdp::offerbecomes the same thinRTCSessionDescriptionwrapper itssdp::answeralready was.No behaviour change — the rendered offer is byte-for-byte what it was.
AI Assistance Disclosure
Tools used (required — write
noneif no AI was used): Claude CodeAttestation (required):
Notes & open questions
This came out of building a WebRTC-Direct transport outside the tree.
libp2p-webrtc-utilsisotherwise exactly the crate you need for that — fingerprints, the Noise prologue, the stream
framing, the answer template — and the offer template was the one piece I had to copy out of
libp2p-webrtcverbatim.One thing this makes visible: with both templates rendered inside the crate,
render_descriptionno longer has any caller outside it. I left it
pubsince narrowing it would be a breaking changeunrelated to this PR, but happy to fold that in if you'd rather.
The changelog entries have placeholder PR numbers; I'll push the real one once this is assigned.
Change checklist