fix(coord): persist vpn_ip in NVS to survive elided Node.Addresses - #73
Merged
Conversation
On repeat registration of an already-known node, the control plane may omit Node.Addresses in both RegisterResponse and the initial MapResponse — the client's own VPN IP is only assigned once and never changes, so the server doesn't resend it. vpn_ip lived only in RAM and had no fallback, so every reboot after the node's first-ever registration silently regressed to 0.0.0.0 while DERP/DISCO/WireGuard kept working fine, since none of that traffic depends on the node knowing its own address. Persist vpn_ip in the "microlink" NVS namespace (same as the machine/ WG/DISCO keys) and restore it at init as a fallback, mirroring the existing ml_peer_nvs.c pattern for the peer cache. Verified on hardware (LilyGo T-Display-S3): after a factory reset, first registration captured "Our VPN IP: 100.105.164.125" and persisted it. A second, non-factory-reset reboot reproduced the bug (Node.Addresses omitted again on both RegisterResponse and MapResponse) but vpn_ip stayed 100.105.164.125 throughout, loaded from NVS before registration even started. Fixes #72 Co-Authored-By: Claude Sonnet 5 <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
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.
Summary
Node.Addresses(andHomeDERP) in bothRegisterResponseand the initialMapResponse— the client's own VPN IP is assigned once and never changes, so the server just doesn't resend it.vpn_iponly ever lived in RAM with no fallback, so every reboot after the node's first-ever registration silently regressed to0.0.0.0while DERP/DISCO/WireGuard kept working fine (none of that traffic depends on the node knowing its own address) — matching the exact symptom in 3.1.0: vpn_ip stays 0.0.0.0 while state=CONNECTED, control-plane last-seen never advances #72 (state=CONNECTED, DERP/DISCO healthy,vpn_ipstuck at 0, Tailscale admin "last seen" never advancing).vpn_ipin the"microlink"NVS namespace (same namespace as the machine/WG/DISCO keys) and restore it as a fallback during init, mirroring the existingml_peer_nvs.cpattern already used for the peer cache. Save on every site that can setvpn_ip(do_register, thedo_fetch_peersMapResponse fallback, and the long-poll update path), gated on the value actually changing.Closes #72
Test plan
Verified on real hardware (LilyGo T-Display-S3), by pinning a local
microlinkoverride into a consumer project and driving two boots:Node.Addresses→ log showsOur VPN IP: 100.105.164.125, DERP connects, 6 peers handshake successfully. Value gets persisted to NVS.Node.AddressesandHomeDERPare both omitted again (Home DERP region: 9 (default)fires, noOur VPN IP:line) — butmicrolink: VPN IP restored from NVS: 100.105.164.125fires at init, before registration even starts, andvpn_ipstays correct throughout the session (DERP connects, DISCO PONGs succeed, no crashes).Fetching peers...runs, noRegisterResponse error/not authorized) — only theAddressesfield is selectively omitted, consistent with delta-style server behavior for a known node.TESTING_GUIDE.md) — this PR relies on the hardware repro above as the regression check.🤖 Generated with Claude Code