Skip to content

daemon: relayed trust handshakes complete in seconds instead of two minutes - #500

Merged
TeoSlayer merged 4 commits into
mainfrom
feat/fast-relayed-handshake
Oct 5, 2026
Merged

TeoSlayer merged 4 commits into
mainfrom
feat/fast-relayed-handshake

Conversation

@TeoSlayer

Copy link
Copy Markdown
Collaborator

Pull Request

Summary

Trust between two private nodes took about two minutes. A requester cannot resolve a private node before trust exists, so the handshake is relayed through the registry, and each side only found the relayed message in handshakePollLoop, which ticks every 60 seconds: ~57s for the request to reach the target, ~59s for the approval to come back. The direct path (to a public node) was already instant.

Changes

  1. The requester polls quickly only while it is waiting. After a handshake request is sent, poll every 2s until the peer answers or becomes trusted, for at most 2 minutes. One extra timer in the existing loop; no goroutine per request.
  2. Poll on demand. pending, trust, approve, reject and wait-for-trust check the registry first, at most once per 2s (3s bound if the registry is slow), so an operator asking "anything pending?" sees a relayed request at once.
  3. Handle the beacon's notify. A two-byte [0x0A][0x01] from the beacon's address triggers a poll. It needs beacon: NotifyNode tells a node the registry holds a relayed handshake for it beacon#59 and registry: tell the beacon which node a relayed handshake was parked for rendezvous#129 on the server side; without them nothing sends it.

Load

The 60-second baseline is unchanged: two idle nodes made 32 registry requests in 240s before and after. Worst-case handshake polls per node per minute (today: 1):

State Polls/min
Idle 1
Waiting on a request it sent up to 31, for at most 2 minutes
Local client hammering pending at most 31
Flood of notifies at most 16 in the first minute, then 13
All at once about 31 — every trigger shares one gate

A notify never triggers a poll sooner than 2s after the previous one (deferred, not dropped). Automatic, dial-driven handshakes cannot restart the fast window for the same peer within 10 minutes.

Measured (Docker, private requester and target, manual approval)

Requester / target / servers Request at target Approval at requester First message
old / old / old 57.2s 59.5s 117.0s
new / new / new 0.1s 0.0s 0.4s
new / new / old servers 54.4s 0.0s 54.6s
new / new / old servers, operator runs pending 0.1s 0.0s 0.3s
old requester / new target / new servers 0.1s 57.5s 57.8s
new requester / old target / new servers 54.3s 0.0s 54.6s

The un-updated side keeps working at the old pace in every mix. Baseline approval time is either ~57s or near zero depending on how the two nodes' 60s ticks fall.

Notes for review

  • Two existing tests were changed (separate commit): TestHandshakeTrustPersistence and TestHandshakeTrustLoadVerify read trust.json the instant the peer appears in the trusted list, but the handshake plugin writes the file just after. The new poll-before-listing made that gap show every time; the tests now wait up to 2s for the file. Product behaviour is unchanged.
  • The notify is trusted by source address, like punch commands. Someone who can spoof the beacon's address gains only the rate-limited polls.
  • TestRelayedHandshakeReachesUnattendedTarget skips until the beacon and rendezvous modules are bumped to releases with the notify.
  • Not tested: multiple beacons, and the compat (WSS) transport; both fall back to polling.

Test Plan

  • go build ./..., go vet ./..., unit suite with GOWORK=off
  • Scheduler unit tests on an injected clock; TestBeaconNotifyOnlyFromBeacon; TestRelayedHandshakeCompletesInSeconds (fails on main: "relayed request not in the target's pending list after 20s")
  • Full go test -parallel 4 -count=1 ./tests/: one failure, TestManualSnapshotTrigger, which binds fixed port 127.0.0.1:18080 held by another process on the test machine

Checklist

  • New code includes the SPDX license header
  • go.mod / go.sum unchanged
  • CHANGELOG updated

🤖 Generated with Claude Code

@TeoSlayer
TeoSlayer force-pushed the feat/fast-relayed-handshake branch from 0663a08 to 29e9b2a Compare October 1, 2026 18:56
@TeoSlayer

Copy link
Copy Markdown
Collaborator Author

Rebased onto current main (CHANGELOG conflict only, both sides kept). Build, vet and the unit suite pass after the rebase; the handshake integration tests pass. The full integration suite was not re-run after the rebase.

Comment thread pkg/daemon/daemon.go Fixed
Teo Calin and others added 3 commits October 2, 2026 12:42
…inutes

Between two private nodes the handshake request and its answer are parked at
the registry and each side learns of them only by polling, once per
keepalive interval (60s): ~59s for the request to show up on the target,
~57s more for the approval to reach the requester.

Keep the 60s poll as the baseline and add three bounded triggers
(handshakepoll.go):

- After sending a handshake request the node polls every 2s until the peer
  answers or becomes trusted, for at most 2 minutes. Automatic (dial-driven)
  requests cannot restart the window for the same peer within 10 minutes.
- The IPC handlers behind pending / trust / approve / reject / wait-for-trust
  poll first, at most once per 2s and bounded to 3s when the registry is
  slow.
- A beacon notify ([0x0A][kind], accepted only from the beacon's address)
  triggers one poll from a token bucket (burst 3, one per 5s), never sooner
  than 2s after the previous poll. Released beacons do not send it yet.

One extra timer in the existing poll loop, armed only while a request is
outstanding or a notify is owed a poll; no goroutine per request. The
loop's startup jitter no longer delays these triggers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TestHandshakeTrustPersistence and TestHandshakeTrustLoadVerify stat/read
trust.json the moment the peer shows up in the trusted list. The handshake
plugin marks the peer trusted in memory and writes the store just after, so
the tests depended on losing that race. The trust-list request now does a
registry poll first, which lined the read up with the gap every time.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…llows the one deferred poll

A burst of pokes is served by one poll at once and, if any poke lands
after that poll began, one more after handshakeOnDemandGap. The test
required exactly one and failed in CI when the scheduler let part of the
burst land late (registry polls = 2, want 1). It now checks the bound that
holds: one or two polls for the burst, and none after.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@TeoSlayer
TeoSlayer force-pushed the feat/fast-relayed-handshake branch from 29e9b2a to e3d54a0 Compare October 2, 2026 09:44
…poll

Fixes from an independent review of this branch.

- An on-demand poll gave up after 3s but had already asked the registry,
  which empties the node's handshake inbox as it answers: a reply that
  came later was thrown away, and with it the requests and approvals it
  carried. A poll now runs on its own goroutine and always completes; a
  caller only stops waiting for it.
- At most one poll is in flight. A caller that arrived behind a stuck poll
  used to wait 3s and then start another with its own 3s; now it waits for
  the one in flight, counted from when that poll started, and starts
  nothing. One slow registry call occupies one pooled connection, not one
  per caller.
- Wait-for-trust polled before checking trust. pilotctl calls it with a
  zero timeout before every send, connect and ping, so a busy node made up
  to 30 polls a minute with no handshake in flight and stalled 3s per
  command when the registry hung. It polls only while a request this node
  sent that peer is unanswered.
- fastActive asked the handshake plugin about trust with the scheduler's
  lock held; the plugin can hold its own lock across a registry lookup and
  the tunnel read loop takes the scheduler's lock for every notify. The
  question is now asked with the lock released.
- A beacon notify must be exactly [0x0A][0x01]; a bare type byte or an
  unknown kind no longer triggers a poll.
- The extra timer polls only if a poke or an unanswered request is still
  owed one, and timer-driven polls keep to the 2s gap.
- Peers kept only for their rearm time no longer fill the 64-entry table
  and deny a new request its fast polling.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@TeoSlayer
TeoSlayer merged commit 5cfdbd3 into main Oct 5, 2026
14 checks passed
TeoSlayer pushed a commit that referenced this pull request Oct 7, 2026
#500 made TestHandshakeTrustLoadVerify and TestHandshakeTrustPersistence
wait up to 2s for trust.json to exist. The handshake manager writes the
file from a goroutine after the in-memory state changes, and its first
write can still hold only the pending request, so LoadVerify could read a
file with no trusted peer ("trust file should have at least one trusted
peer"). It now waits for a write that lists one, and both tests allow 5s,
which is what failed under load.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TeoSlayer added a commit that referenced this pull request Oct 7, 2026
… appears (#508)

tests: wait for a trust file that lists the peer, not just for the file

#500 made TestHandshakeTrustLoadVerify and TestHandshakeTrustPersistence
wait up to 2s for trust.json to exist. The handshake manager writes the
file from a goroutine after the in-memory state changes, and its first
write can still hold only the pending request, so LoadVerify could read a
file with no trusted peer ("trust file should have at least one trusted
peer"). It now waits for a write that lists one, and both tests allow 5s,
which is what failed under load.

Co-authored-by: Teo Calin <calinteodor@Teos-MacBook-Pro.local>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
TeoSlayer pushed a commit that referenced this pull request Oct 7, 2026
…-ups to #500

From the second review of #500.

- Shutdown closed the registry client without waiting for a poll in
  flight. The registry empties the node's handshake inbox as it answers,
  so a restart during a poll lost what it carried. doStop now waits up to
  5s for the poll (waitIdle), and pollHandshakes starts none once the
  daemon is stopping.
- At the 64-peer cap, only the closed-window peer whose rearm ends
  soonest is evicted; the others keep their hold-off against automatic
  handshakes reopening a window.
- A beacon notify with bytes after the handshake kind is acted on, so a
  later beacon can extend the message without older daemons ignoring it.
- Comments: the package doc describes the wait-for-trust condition, and
  pollRelayedHandshakes no longer documents the removed timeout.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TeoSlayer added a commit that referenced this pull request Oct 7, 2026
…500 (#514)

* daemon: shutdown waits for a handshake poll in flight; smaller follow-ups to #500

From the second review of #500.

- Shutdown closed the registry client without waiting for a poll in
  flight. The registry empties the node's handshake inbox as it answers,
  so a restart during a poll lost what it carried. doStop now waits up to
  5s for the poll (waitIdle), and pollHandshakes starts none once the
  daemon is stopping.
- At the 64-peer cap, only the closed-window peer whose rearm ends
  soonest is evicted; the others keep their hold-off against automatic
  handshakes reopening a window.
- A beacon notify with bytes after the handshake kind is acted on, so a
  later beacon can extend the message without older daemons ignoring it.
- Comments: the package doc describes the wait-for-trust condition, and
  pollRelayedHandshakes no longer documents the removed timeout.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* daemon: shutdown waits for the handshake poll before the manager stops

Review fixes.

- Shutdown waited for a poll in flight only after the handshake manager had
  stopped, so the poll's reply was processed by a stopped manager: requests
  and approvals were neither saved nor answered, and the registry had
  already emptied the inbox. The wait now comes before the manager stops.
- No poll can start once shutdown has begun: the scheduler is closed under
  the same lock that starting a poll takes. Before, a caller past the
  stopping check could start one after the wait had returned.
- The wait shares the 5s deadline shutdown already gives its background
  goroutines instead of adding 5s of its own.
- The shutdown test now calls Stop and checks the order; tests cover the
  closed scheduler and which settled peer is let go. Each fails against
  the matching revert.
- Changelog entries for the eviction and notify changes; the notify
  comments say v1.16.0 still requires exactly two bytes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Teo Calin <calinteodor@Teos-MacBook-Pro.local>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants