feat(iroh)!: make direct_addresses always be initialised#3505
feat(iroh)!: make direct_addresses always be initialised#3505dignifiedquire merged 4 commits intomainfrom
Conversation
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3505/docs/iroh/ Last updated: 2025-10-07T17:05:16Z |
Just let it return an empty addr? (I.e. a NodeAddr with only the node id set). That's much preferable than a different signature IMO |
flub
left a comment
There was a problem hiding this comment.
The effect of this is lovely! Amazing to see this work out. Though I don't understand how we make up the addresses, but that hasn't changed here so not a blocker.
Would love if the doc comment was updated though to explain what you can rely on. And maybe also what you can not rely on.
| /// - the endpoint changes its preferred relay server | ||
| /// - more addresses are discovered for this endpoint | ||
| pub fn node_addr(&self) -> NodeAddr { | ||
| let node_id = self.node_id(); |
There was a problem hiding this comment.
The first paragraph of the doc comment needs updating I think. It could describe what you'd expect to be in the direct addresses immediately, but that they -and should- still change as discovered direct addresses become available.
iroh/src/magicsock.rs
Outdated
| .ip_bind_addrs() | ||
| .iter() | ||
| .copied() | ||
| .zip(self.ip_local_addrs()) |
There was a problem hiding this comment.
I don't follow why these two iters form pairs? Are they even guaranteed to be the same length?
update: oh, this is moved code. Probably fine then, but I wouldn't mind more comments because I don't understand what it's trying to do.
There was a problem hiding this comment.
added some more comments
There was a problem hiding this comment.
thanks, they help! it makes sense now
iroh/src/magicsock.rs
Outdated
| fn collect_local_addresses( | ||
| &self, | ||
| netmon_watcher: &mut n0_watcher::Direct<netmon::State>, | ||
| addrs: &mut BTreeMap<SocketAddr, DirectAddrType>, |
There was a problem hiding this comment.
Whoa, I'm very not used to seeing an "out" parameter in rust code.
There was a problem hiding this comment.
it actually works out quite nice in rust if you need it, and makes your code go brr ;P
2cb927e to
91a706a
Compare
| relay_url, | ||
| direct_addresses, | ||
| } | ||
| self.watch_node_addr().get() |
There was a problem hiding this comment.
This kind of gets us back to the "why have two methods?" question. This one is now pretty trivial.
There was a problem hiding this comment.
Though I do appreciate this way people don't have to learn about the Watcher trait.
Description
Turns out we just need to call the init early enough.
Thanks to @flub for reminding me of this
Breaking Changes
iroh::Endpoint::watch_node_addrnow returns a watchable forNodeAddrnotOption<NodeAddr>Notes & open questions
Currently this means that in wasm we have a different signature for
watch_node_addr()because it can be empty, need some feedback on how to solve this