refactor(iroh)!: remove Endpoint::add_node_addr

[dignifiedquire]

Description

Simplifies the public API. If you need to provide static information about node addresses, please use StaticProvider from now on.

Closes #3329

Breaking Changes

• removed
  • iroh::Endpoint::add_node_addr
  • iroh::Endpoint::remote_info
  • iroh::Endpoint::remote_info_iter
  • iroh::RemoteInfo
  • iroh::discovery::DiscoveryItem
• added
  • iroh::discovery::mdns::DiscoveryItem
  • iroh::Endpoint::latency

Notes & open questions

Is it okay to have these all removed now, given the replacements for connection details have not yet landed (will only with multipath)?

[flub]

seems reasonable to me.

Wrt removal of remote_info, the conn_type API still exists so might be usable? We're also unlikely to release in this state? Though it could happen I guess.

[github-actions]

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: 3dd9383

[dignifiedquire]

the conn_type API still exists so might be usable?

It is, but it doesn't provide latency information, which is something I know some users do care about

[rklaehn]

I thought this might break big-block-syc, but it seems like in the latest version of big-block-sync I only use Connection::rtt(). So I guess it's fine. I will have to refactor a lot of code to get rid of add_node_addr, e.g. in the dht experiment, but it should be fine.

Not a fan of having any kind of mutable default StaticDiscovery in the endpoint. Just accept the inconvenience instead of creating chaos later when multiple places try to modify it.

https://github.com/n0-computer/big-block-sync/blob/main/src/lib.rs

[dignifiedquire]

This currently breaks the advertised way to see (passively) discovered MDNS nodes, need to figure this part out.

[dignifiedquire]

Latency should be fine, .rtt() on the connection is preferrable anway

[Frando]

Nous started using Endpoint::remote_info in psyche just now to sort provider candidates for iroh blobs downloads by latency:
https://github.com/PsycheFoundation/psyche/blob/4a54db82929a86612423ea537c3b65106ab76a99/shared/network/src/latency_sorted.rs#L42

This is a valid use case IMO, and it only works easily this way if this info is available for any node directly from the endpoint, not from Connection only, so this might need some more thought? Or what would our alternative suggestion be?

[ramfox]

This currently breaks the advertised way to see (passively) discovered MDNS nodes, need to figure this part out.

Yea, I think the only option is to change the example to show you need to build the mdns discovery service, subscribe to the stream, and THEN add the discovery service to the endpoint, now that we can add discovery services after the endpoint has been built.

Then the example can show a loop through the stream, which will all be locally discovered nodes, rather than relying on a provenance string.

[github-actions]

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

Last updated: 2025-10-03T01:56:08Z

[ramfox]

Hmmm, to @Frando 's point, I think as long as the docs specify that the remote_info and remote_info_iter returns information for remote endpoints that we have called called connect on or have accepted connections from, then it's valid to add those methods back in.

[ramfox]

Cleaned things up so tests are passing:

• adjusted doc comments and examples
• removed discovery_subscriber method and DiscoverySubscriber struct

API thoughts:

1. The current way to add a discovery service after the endpoint has been created is:
   ep.discovery().clone().add(new_discovery_service)
   The clone is a bit of a footgun, because without it the error that the complier gives you is about the ConcurrentDiscovery being behind a &, so it can't be mutable and it's not obvious from the docs that you won't get penalized by cloning because there is an internal Arc in ConcurrentDiscovery.
   Now that ConcurrentDiscovery has an Arc, should we just return a ConcurrentDiscovery rather than a &ConcurrentDiscovery, from ep.discovery()?

2. Now that subscribe is only an MdnsDiscovery method, we should likely change the return types from DiscoveryEvent to MdnsEvent. We also no longer need to return DiscoveryItems, and can likely just simplify to returning NodeAddrs for MdnsEvent::Discovery and NodeIds for MdnsEvent::Expiry events.

Also, we now have to including tokio::pin-ing the stream, why did we not have to do that before?

3. I do think we might be foot-gunning ourselves by removing remote_info/remote_info_iter too early until we have a valid replacement.

[dignifiedquire]

Also, we now have to including tokio::pin-ing the stream, why did we not have to do that before?

yeah, a bit annoying, I want to try and fix it

[ramfox]

Added no_run to the mdns doc example, since there is no logical end to the example anymore, and it can theoretically run forever (which was causing problems in CI)

[dignifiedquire]

The current way to add a discovery service after the endpoint has been created is:
ep.discovery().clone().add(new_discovery_service)

fixed

Now that subscribe is only an MdnsDiscovery method, we should likely change the return types from DiscoveryEvent to MdnsEvent

fixed

Also, we now have to including tokio::pin-ing the stream, why did we not have to do that before?

fixed

[flub]

🎉