Fix: onion path rotation could never escape a bad guard node

[mpretty-cyro]

Issue #2216. Two halves of one defect — neither works alone — plus the follow-ups to #2221, now that it is on dev.

The path test always targeted the same node

PathManager.testPath took the first eligible member of the snode pool as its destination, so every path test in a process talked to one node. A node that rejects onion payloads therefore failed every path test on that device for as long as it stayed in the pool, however healthy the candidate paths were. The field evidence on the issue is two disjoint candidate paths failing to one constant destination.

The destination is now picked with secureRandom() over the eligible members, matching the rest of the file.

Rotation could never replace a bad guard

Rotation rebuilds its candidates around the existing guards, and a rotation can only be committed whole — Phase 3 requires the new guard set to match the current one. So the candidate built on a bad first hop fails verification and the whole rotation is discarded, whether it was the only candidate that failed or all of them. Failures of that shape strike nothing either: a reachable guard returning 400, or any code with no specific rule, maps to PathError and penalises neither node nor path, which is the agreed design. Nothing else broke the loop.

Three rotations in a row that fail to verify every candidate now drop the paths via the existing clearPaths(). The next getPath() rebuilds with no reusable guards and draws fresh ones, replacing both. A successful commit resets the count, so a flaky network cannot walk a working client into repeated rebuilds.

Escalation is skipped while the network is down: every path test fails for a reason that says nothing about the guards, and counting those would hand an offline device a fresh guard every rotation interval for as long as it stayed offline.

It counts attempts rather than elapsed time because a failed rotation does not advance the rotation timestamp — a wedged client re-enters rotation on every getPath, so three failures is seconds apart, not thirty minutes.

Follow-ups to #2221

• Exclusion now identifies a node by its ed25519 key. Snode equality is address and port, so the destination exclusion missed a node whose pool record and swarm record were fetched either side of an IP or port change — the same node under two addresses, one of them the path's. Kept local to path selection rather than changing Snode.equals, which the pool, the paths and the swarm all rely on.
• Dropped a stale cross-repo claim from the comment explaining the exclusion: it said quic-to-quic "refuses it outright", which describes service-node behaviour that has since changed, and nothing here can notice when it changes again. The local obligation is the durable half.

Deliberate, not oversights

• getGuardSnodes draws from the whole pool without excluding struck nodes, so a bad guard can be re-drawn by chance (roughly one in pool-size). A further escalation moves off it.
• Rebuild frequency changes only in the failure path — a healthy or offline client never reaches the counter.
• No new penalty rule for 400s, and no strike for a failed path test. Non-penalising defaults are the agreed paradigm, not a gap.
• No persisted state and no schema change; Path is still a bare List<Snode>.

Tests

Seven in PathManagerTest, all on synthetic pools and paths. Each was verified to fail with only its own production change reverted:

test	reverting
path tests do not all target the same snode	the random destination
rotation commits even though the first pool member rejects every request	the random destination
rotation that never verifies drops the paths and rebuilds onto fresh guards	the escalation
a single bad guard escalates to a rebuild	escalating on a partial failure
a rotation that verifies keeps a flaky client off the rebuild path	the reset on commit
an offline client keeps its guards however many rotations fail	the network gate
a snode is excluded after its address changes	identity by key

The escalation tests assert getGuardSnodes is called with existingGuards = emptySet(), so the assumption that clearing the paths actually reaches a guard-replacing rebuild is checked by CI rather than by reading.

Rebased on dev d29053ab48. Unit suite: 318 pass, 0 fail.