feat: add iroh-ice module for WebRTC ICE candidate extraction#741
Draft
feat: add iroh-ice module for WebRTC ICE candidate extraction#741
Conversation
Contributor
📝 WalkthroughWalkthroughAdds end-to-end ICE candidate support: TypeScript client/resolver APIs, new GraphQL types/resolver in Rust, and a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client App
participant TSClient as RuntimeClient
participant GraphQL as GraphQL Server
participant Resolver as runtime_ice_candidates
participant Holochain as Holochain Service
participant IrohIce as IrohIce Module
Client->>TSClient: call iceCandidates()
TSClient->>GraphQL: query runtimeIceCandidates
GraphQL->>Resolver: resolve runtime_ice_candidates()
Resolver->>Holochain: request local socket addresses
Holochain-->>Resolver: return Vec<SocketAddr>
Resolver->>IrohIce: construct with local_addrs
IrohIce->>IrohIce: classify_addr()/compute_priority()/format_candidate()
IrohIce-->>Resolver: return IceCandidateInfo list
Resolver->>GraphQL: map to IceCandidateInfo[]
GraphQL-->>TSClient: runtimeIceCandidates result
TSClient-->>Client: Promise<IceCandidateInfo[]>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Generic Iroh-to-ICE bridge that converts socket addresses from the Holochain/kitsune2 transport layer into WebRTC ICE candidate strings. - Classifies addresses as host (private/LAN) or srflx (public/reflexive) - Computes RFC 8445 compliant priorities - Formats ICE candidate strings for RTCPeerConnection.addIceCandidate() - Zero external dependencies (pure std::net) - 9 unit tests covering classification, formatting, priority ordering Usage: IrohIce::new(conductor.holochain_p2p().local_socket_addrs().await?) Part of: #719
HexaField
force-pushed
the
feat/iroh-ice
branch
from
9c645d8 to
d918f3b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
core/src/runtime/RuntimeResolver.ts (1)
47-57: Consider adding explicitInttype forportfield.The
portfield is anumberin TypeScript, but GraphQL distinguishes betweenIntandFloat. Without an explicit type annotation, type-graphql defaults toFloatfornumbertypes. For consistency with the Rust backend (which usesi32), consider specifyingInt:♻️ Proposed fix
`@ObjectType`() export class IceCandidateInfo { `@Field`() candidate: string; `@Field`() candidateType: string; `@Field`() address: string; - `@Field`() + `@Field`(type => Int) port: number; }Note: You'll need to import
Intfromtype-graphqlif not already imported.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/runtime/RuntimeResolver.ts` around lines 47 - 57, The IceCandidateInfo.port field is a TS number but GraphQL will treat it as Float by default; update the `@Field` decorator on the port property in class IceCandidateInfo to explicitly use GraphQL Int (e.g., `@Field`(() => Int)) and add an import for Int from type-graphql so the schema matches the Rust i32 semantics.rust-executor/src/iroh_ice/mod.rs (1)
104-126: Consider handling IPv4 CGNAT/shared address space (100.64.0.0/10).The
is_private()function doesn't account for the IPv4 shared address space (100.64.0.0/10, RFC 6598), commonly used by carrier-grade NAT (CGNAT). These addresses are not globally routable and would be misclassified as server-reflexive (public).While this may be uncommon in practice, it could lead to ICE candidates that won't work for external connections.
♻️ Proposed fix to handle CGNAT range
fn is_private(addr: &SocketAddr) -> bool { match addr { SocketAddr::V4(v4) => { let ip = v4.ip(); + let octets = ip.octets(); ip.is_private() || ip.is_loopback() || ip.is_link_local() || ip.is_unspecified() + // CGNAT/shared address space (100.64.0.0/10) per RFC 6598 + || (octets[0] == 100 && (octets[1] & 0xc0) == 64) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust-executor/src/iroh_ice/mod.rs` around lines 104 - 126, The is_private function currently misses IPv4 CGNAT/shared space (100.64.0.0/10); update the SocketAddr::V4 branch in fn is_private to detect this range by inspecting the IPv4 octets (e.g., let octets = ip.octets(); treat as private when octets[0] == 100 && (octets[1] & 0b1100_0000) == 0b0100_0000), and return true for that case so CGNAT addresses are classified as non-public; adjust or add unit tests around is_private to cover 100.64.0.0/10.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rust-executor/src/graphql/query_resolvers.rs`:
- Around line 795-821: Add a capability check at the start of
runtime_ice_candidates to prevent unauthorized access to local socket addresses:
call the same capability enforcement used in runtime_hc_agent_infos (e.g.
context.require_capability(RUNTIME_HC_AGENT_INFO_READ_CAPABILITY) or define and
use a new RUNTIME_ICE_CANDIDATES_READ_CAPABILITY constant) before calling
get_holochain_service(), and return a FieldError or propagate the existing error
path on failure so callers without the capability cannot retrieve ICE candidate
info.
---
Nitpick comments:
In `@core/src/runtime/RuntimeResolver.ts`:
- Around line 47-57: The IceCandidateInfo.port field is a TS number but GraphQL
will treat it as Float by default; update the `@Field` decorator on the port
property in class IceCandidateInfo to explicitly use GraphQL Int (e.g.,
`@Field`(() => Int)) and add an import for Int from type-graphql so the schema
matches the Rust i32 semantics.
In `@rust-executor/src/iroh_ice/mod.rs`:
- Around line 104-126: The is_private function currently misses IPv4
CGNAT/shared space (100.64.0.0/10); update the SocketAddr::V4 branch in fn
is_private to detect this range by inspecting the IPv4 octets (e.g., let octets
= ip.octets(); treat as private when octets[0] == 100 && (octets[1] &
0b1100_0000) == 0b0100_0000), and return true for that case so CGNAT addresses
are classified as non-public; adjust or add unit tests around is_private to
cover 100.64.0.0/10.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7042434f-94e0-48d1-a703-a7733453b8c2
📒 Files selected for processing (6)
core/src/runtime/RuntimeClient.tscore/src/runtime/RuntimeResolver.tsrust-executor/src/graphql/graphql_types.rsrust-executor/src/graphql/query_resolvers.rsrust-executor/src/iroh_ice/mod.rsrust-executor/src/lib.rs
HexaField
force-pushed
the
feat/iroh-ice
branch
from
a316e4b to
a18548c
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/src/runtime/RuntimeClient.ts`:
- Around line 81-91: The iceCandidates method is querying runtimeIceCandidates
via this.#apolloClient.query and currently relies on the default cache-first
policy, which can return stale ICE data; update the call in iceCandidates (the
invocation of this.#apolloClient.query that wraps unwrapApolloResult) to pass
fetchPolicy: "no-cache" so the query always fetches fresh ICE candidate info
from the network rather than from Apollo cache.
In `@rust-executor/src/iroh_ice/mod.rs`:
- Around line 63-75: The current mapping over self.local_addrs in
runtimeIceCandidates (and the similar block around lines 104-129) includes
unspecified/wildcard addresses (0.0.0.0 / ::), producing dead ICE candidates;
filter out addresses where addr.is_unspecified() before calling classify_addr,
compute_priority, format_ice_candidate, and constructing IceCandidate so only
concrete host addresses are serialized. Locate the iterator using
self.local_addrs and insert a .filter(|addr| !addr.is_unspecified()) (or
equivalent) prior to the .enumerate().map(...) to drop wildcard bindings.
- Around line 57-75: The code incorrectly infers srflx purely from IPs in
local_candidates by using classify_addr and then calling format_ice_candidate
without provenance; instead accept and propagate provenance/base-address
metadata into this module (or into the IceCandidate creation path) and use that
provenance when deciding candidate.typ and when calling format_ice_candidate, or
conservatively emit public addresses as host until the transport proves they are
server-reflexive; update the logic around local_candidates (and the equivalent
candidate-building sites that use classify_addr, compute_priority,
format_ice_candidate, and IceCandidate) to consult the new provenance field
rather than IP class alone so SDP serialization and downstream candidate_type
remain correct.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6765dd6a-95cb-4ace-b1f8-1ae0b82c0397
📒 Files selected for processing (5)
core/src/runtime/RuntimeClient.tscore/src/runtime/RuntimeResolver.tsrust-executor/src/graphql/graphql_types.rsrust-executor/src/graphql/query_resolvers.rsrust-executor/src/iroh_ice/mod.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- core/src/runtime/RuntimeResolver.ts
- rust-executor/src/graphql/graphql_types.rs
- rust-executor/src/graphql/query_resolvers.rs
Wires up the iroh-ice module to the executor GraphQL API and AD4M core TypeScript client, enabling Flux and other consumers to fetch Iroh-derived ICE candidates for WebRTC connections. Rust (executor): - IceCandidateInfo GraphQL type with candidate, candidateType, address, port - runtimeIceCandidates query: fetches socket addrs from conductor.holochain_p2p() and converts them to ICE candidate format via IrohIce TypeScript (core): - IceCandidateInfo class in RuntimeResolver.ts - runtime.iceCandidates() client method in RuntimeClient.ts Usage in Flux: const candidates = await ad4mClient.runtime.iceCandidates() // Inject as additional ICE candidates on RTCPeerConnection Part of: #719
HexaField
force-pushed
the
feat/iroh-ice
branch
2 times, most recently
from
a316e4b to
b9981dc
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rust-executor/src/iroh_ice/mod.rs`:
- Around line 151-171: The formatter format_ice_candidate is emitting "typ
srflx" without the required related-address fields; either stop advertising
srflx or extend the candidate type to carry the server-reflexive base
address/port and serialize raddr/rport. Concretely: update the IceCandidateType
to include the base SocketAddr for ServerReflexive (e.g.,
ServerReflexive(SocketAddr)) or add an explicit related_addr parameter to
format_ice_candidate, and when matching IceCandidateType::ServerReflexive
include " raddr {ip} rport {port}" in the formatted string; otherwise only emit
"typ host" for host candidates so tests and callers see a valid ICE candidate
string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8fc2d162-462c-4e35-b9e3-fba667aed8df
📒 Files selected for processing (5)
core/src/runtime/RuntimeClient.tscore/src/runtime/RuntimeResolver.tsrust-executor/src/graphql/graphql_types.rsrust-executor/src/graphql/query_resolvers.rsrust-executor/src/iroh_ice/mod.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- rust-executor/src/graphql/graphql_types.rs
- core/src/runtime/RuntimeResolver.ts
- core/src/runtime/RuntimeClient.ts
Comment on lines
+151
to
+171
| /// Format an ICE candidate string per RFC 8445. | ||
| /// | ||
| /// Format: candidate:{foundation} {component} udp {priority} {ip} {port} typ {type} | ||
| fn format_ice_candidate( | ||
| foundation: usize, | ||
| addr: &SocketAddr, | ||
| typ: &IceCandidateType, | ||
| priority: u32, | ||
| ) -> String { | ||
| let type_str = match typ { | ||
| IceCandidateType::Host => "host", | ||
| IceCandidateType::ServerReflexive => "srflx", | ||
| }; | ||
| format!( | ||
| "candidate:{foundation} 1 udp {priority} {ip} {port} typ {type_str}", | ||
| foundation = foundation, | ||
| priority = priority, | ||
| ip = addr.ip(), | ||
| port = addr.port(), | ||
| type_str = type_str, | ||
| ) |
Contributor
There was a problem hiding this comment.
Don't advertise srflx support with an incomplete candidate format.
A server-reflexive candidate needs its related/base address and port serialized as raddr / rport. Right now this formatter emits typ srflx only, and the test on Line 223 locks that invalid shape in. Either carry the base address through this type and append the related-address attributes, or keep the surface host-only until provenance exists.
Also applies to: 223-230
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust-executor/src/iroh_ice/mod.rs` around lines 151 - 171, The formatter
format_ice_candidate is emitting "typ srflx" without the required
related-address fields; either stop advertising srflx or extend the candidate
type to carry the server-reflexive base address/port and serialize raddr/rport.
Concretely: update the IceCandidateType to include the base SocketAddr for
ServerReflexive (e.g., ServerReflexive(SocketAddr)) or add an explicit
related_addr parameter to format_ice_candidate, and when matching
IceCandidateType::ServerReflexive include " raddr {ip} rport {port}" in the
formatted string; otherwise only emit "typ host" for host candidates so tests
and callers see a valid ICE candidate string.
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
1 participant
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.
Summary
Adds a complete Iroh-to-ICE bridge that exposes Holochain's transport-discovered socket addresses as WebRTC ICE candidates, replacing the need for external STUN servers. Provides both the low-level module and the full GraphQL API + TypeScript client integration.
Changes
rust-executor/src/iroh_ice/mod.rs— Core ModuleIrohIcestruct: takes socket addresses, produces RFC 8445 compliant ICE candidateshost(private/LAN) orsrflx(public/reflexive)RTCPeerConnection.addIceCandidate()std::net)rust-executor/src/graphql/— GraphQL APIIceCandidateInfoGraphQL type withcandidate,candidateType,address,portruntimeIceCandidatesquery: fetches socket addresses fromconductor.holochain_p2p().local_socket_addrs()and converts them viaIrohIcecore/src/runtime/— TypeScript ClientIceCandidateInfoclass inRuntimeResolver.tsruntime.iceCandidates()client method inRuntimeClient.tsUsage
Flux Integration (separate PR)
The
packages/webrtc/src/WebRTCManager.tsin Flux currently uses hardcoded STUN servers (stun:relay.ad4m.dev:3478,stun:stun.l.google.com:19302, etc.). The integration path:WebRTCManager.init(), callad4mClient.runtime.iceCandidates()to fetch Iroh-derived candidatesRTCPeerConnectionis created (inAD4MPeer.createPeer()), inject the Iroh candidates viapc.addIceCandidate()This works for both the existing P2P mesh calls and the future SFU system (#700), since
iroh-iceis transport-agnostic — it provides candidates that any WebRTC consumer can use.SFU Considerations
The
iroh-icemodule is deliberately generic — no knowledge of mesh vs SFU vs pipe transport. When the str0m SFU (#700) lands:IrohIceto get its own ICE candidates to share with clientsruntimeIceCandidatesAPI to get their local candidatesDependencies
local_socket_addrs()on Transport traitNote
The
ad4m-clientcrate has 327 pre-existing compile errors (GraphQL codegen issue) unrelated to this PR. Thead4m-executorcrate and theiroh-icemodule compile and test successfully.See: #719
Summary by CodeRabbit