daemon: event-driven path reset on rekey give-up (corrects #415, verified live)#416
Merged
Merged
Conversation
…fied live) #415 tried to recover the T2 desync by polling the rekey-gave-up state from the path watchdog. A LIVE test (2026-07-22) proved it never fired: a desynced peer has no usable key, so it is not Ready, so ReadyPeerIDs() — the set the watchdog scans — excludes it entirely. 0 firings across dozens of real give-ups. (The unit test passed only because it injected a Ready peer + gave-up state, a combination that can't occur.) Correct fix: event-driven. Add an onGaveUp hook fired from the keyexchange retransmit loop at the exact give-up site, wired through the tunnel to a daemon handler (onRekeyGaveUp) that runs resetPeerPath — the only thing that recovers a desync, because it re-resolves a FRESH endpoint and re-punches (plain rekey-retransmit just resends to the same stale cached endpoint forever). Per-peer 30s cooldown prevents a persistently-offline peer from storming resolves; reset runs async (registry I/O) off the rekey loop goroutine. Reverts #415's ineffective path-watchdog poll. Verified LIVE on a real NAT'd laptop: give-up -> full path reset (fresh resolve + punch + PILA) in ~1.2s, reproduced across peers 17461 and 16392, vs the prior indefinite silent-drop. This is the 'no reply on whatever' fix. Tests: TestOnRekeyGaveUpResetsWithCooldown (hook resets + cooldown suppresses a second give-up). pkg/daemon + keyexchange green under -race. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
2 participants
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.
#415 didn't work — a live test proved it. It polled the rekey-gave-up state from the path watchdog, but a desynced peer isn't Ready, so
ReadyPeerIDs()(what the watchdog scans) never includes it → 0 firings across dozens of real give-ups. The unit test passed only on an impossible injected combination.This is the correct fix: an event-driven
onGaveUphook fired from the keyexchange retransmit loop at the give-up site → daemonresetPeerPath(fresh resolve + re-punch + PILA — the only thing that recovers a desync; plain rekey-retransmit resends to the same stale endpoint forever). Per-peer 30s cooldown; async off the rekey goroutine. Reverts #415's ineffective poll.Verified LIVE on a real NAT'd laptop: give-up → full path reset in ~1.2s, reproduced across peers 17461 and 16392 — vs the prior indefinite silent drop (the 'no reply' symptom).
-racegreen.🤖 Generated with Claude Code