fix(iroh): always ping new paths, add test for reconnecting with changed addrs

[Frando]

Description

Adds a test that closes a client endpoint and recreates it right away with the same node id. It tries to connect to a server it was previously connected to to. The test fails on main.

The reason is that the server thinks the client's previous UDP send address is still valid. Even though it receives both disco pings and quic packets from the new address, it doesn't update the best_addr used for sending, therefore the server's QUIC replies are sent into nirvana, and the test times out.

The second commit fixes this behavior by always sending disco pings to new paths.

Breaking Changes

Notes & open questions

Change checklist

• Self-review.
• Documentation updates following the style guide, if relevant.
• Tests if relevant.
• All breaking changes documented.
  • List all breaking changes in the above "Breaking Changes" section.
  • Open an issue or PR on any number0 repos that are affected by this breaking change. Give guidance on how the updates should be handled or do the actual updates themselves. The major ones are:
    • quic-rpc
    • iroh-gossip
    • iroh-blobs
    • dumbpipe
    • sendme

[github-actions]

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

Last updated: 2025-06-30T09:24:39Z

[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: 38380b8

[flub]

Yes, this is a good fix! I can't think of any reason why this extra ping back would ever do much harm (famous last words).

My only concern is the 5s is asking for flakyness, but I can't really think of something better right away.

[flub]

How long does this take? Since both endpoints are still there this should be pretty fast, no? Feels a bit odd to spawn this.

[Frando]

Yea, I'll remove it again. It was a trial for the windows failure, but that's not it, see comment below

[flub]

My only concern is the 5s is asking for flakyness, but I can't really think of something better right away.

Oh, lol. Windows CI already fails because of this... Guess we'll have to figure this out right away.

[flub]

I think you might be able to set this to a 30s timeout because without a relay that would still fail after 30s in the old version? You can verify that.

[flub]

Alternatively you can mimic what #3350 does and measure the time it takes to connect initially. Then require the 2nd connection to succeed in 3 times that base time. Could be more robust?

[Frando]

The flakyness is not only due to slow CI I think, I got it to fail locally too from time to time. My current reasoning is that the fix applied here is not enough (pasting from discord):

so with the change from the PR, a new path is pinged. once we get the pong, we check if this path should be the new best_addr. which takes us to here:
https://github.com/n0-computer/iroh/blob/main/iroh/src/magicsock/node_map/best_addr.rs#L111
the new path will only become the best_addr (and thus the send addr for the next quic packet) if it has a lower latency than the current best_addr, or if the current best_addr exceeds its trust interval
in our test case, the previous addr is still trusted. the path is also physically identical, so it is purely random which of the two pingpongs had a lower latency
if the pingpong from the new client endpoint has a lower latency, the addr from the new client endpoint is used from then on. if the pingpong from the old client endpoint happened to have a lower latency, the best_addr is kept and future udp packets are sent into nirvana

[Frando]

I pushed a commit that makes the test pass reliably (locally for me at least):

a8ee257

It adds the following case to BestAddr::insert_if_better_or_reconfirm, which is called when a pong is received on a UDP path:

// If we receive a pong on a different port but the same IP address as the current best addr,
// we assume that the endpoint has rebound, and thus use the new port.
} else if state.addr.addr.ip() == addr.ip() {
    self.insert(addr, latency, source, confirmed_at)
}

So when we receive a pong from the same IP address as our current best_addr, but with a different port, we unconditionally switch our best addr to that new port, even if the latency is lower than the best addr's latency from the last pingpong.

So far this was the simplest I could come up with. Can you think of any scenarios where that would lead to undesired outcomes?

[flub]

My main issue with this logic is that I find it fairly arbitrary. It fixes an artificial test case, but it seems like a sub-set of the cases that need to be fixed. We're also not really sure we can send on this path at this point, but I'm not even sure if that's something we're always sure of anyway.