Skip to content

Conversation

@ramfox
Copy link
Member

@ramfox ramfox commented Nov 21, 2025

Description

Adds the ability to resolve and publish relay URLs over mDNS. This is off by default.

RelayUrls are typically only used for hole punching, which likely is not needed when using mDNS. However, we have seen some test cases where mDNS packets get through local firewalls, but our iroh packets are blocked. In these cases, it may be prudent to also publish your relay url.

  • Adds MdnsDiscoveryBuilder::publish_relay_url method

@github-actions
Copy link

github-actions bot commented Nov 21, 2025

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: 8d00074

@n0bot n0bot bot added this to iroh Nov 21, 2025
@github-project-automation github-project-automation bot moved this to 🏗 In progress in iroh Nov 21, 2025
@github-actions
Copy link

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3691/docs/iroh/

Last updated: 2025-11-22T09:15:04Z

@ramfox
Copy link
Member Author

ramfox commented Nov 23, 2025

TODOS

  • Add API to mDNS discovery to add a mapping function that let's users filter the kinds of addrs they want to publish, as well as order them. (takes a transport::Addr slice returns a transport::Addr vec?)
  • adjust mDNS discovery publishing to map the addrs before publishing
  • use match on addr to know how to format -> if IP do normal, if relay add to TXT attribute, only allow 4 txt attributes, any that don't get added log a warn with the addr
  • add test that ensures we can get multiple relays
  • add test that ensures we cannot add too many
  • add tests that shows we respect the map filter


/// Sets whether this endpoint should advertise its [`RelayUrl`].
///
/// Overridden by the `MdnsDiscoveryBuilder::advertise` method. If this is
Copy link
Contributor

Choose a reason for hiding this comment

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

"Overridden" threw me off, I think it's the wrong language. Maybe something longer like "Only if mDNS advertising is enabled using [Self::advertise] will relay URLs be advertised." But even that's not great.

Maybe the whole thing can be described differently:

/// Enables advertising this endpoint's [`RelayUrl`] in addition to IP addresses.
///
/// Enabling [`Self::advertise`] will only advertise IP addresses, enabling this will additionally advertise the endpoint's [`RelayUrl`]. Note that if [`Self::advertise`] is disabled no advertising happens at all, neither of IP addresses or relay URLs.

Or maybe something better even :)

Also, I think it's worth updating the doc comment of advertise itself as well to make clear it only advertises IP addresses but is also a master switch.

}

/// Add the subscriber to the list of subscribers
/// Add the subscriber to the list of subscribers.
Copy link
Contributor

Choose a reason for hiding this comment

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

😂

if publish_relay_url {
if let Some(relay) = data.relay_urls().next() {
if let Err(err) = discovery.set_txt_attribute(RELAY_URL_ATTRIBUTE.to_string(), Some(relay.to_string())) {
warn!("Failed to set the relay url in mDNS: {err:?}");
Copy link
Contributor

Choose a reason for hiding this comment

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

This can happen if the RelayUrl is too long right? I wonder if this could get very noisy. But maybe that's the point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🏗 In progress

Development

Successfully merging this pull request may close these issues.

3 participants