Snode pool: 421 recovery on evidence, strike expiry, and reporting failed refreshes

[mpretty-cyro]

Five commits against dev, each independently reviewable and any of them droppable. They're together
because they all sit in src/network/ and touch the same few files, so splitting them into separate
PRs just moved the conflict resolution to merge time.

---

1–2. A 421 couldn't invalidate the mapping it proved was wrong

A 421 says authoritatively that the swarm we resolved for an account is wrong. _handle_421_retry
answered it with refresh_if_needed, which decides on cache_expiration (2h) and so declines for
any cache younger than that, then re-read the same _swarm_cache entry — so the retry went to another
node in the list that had just rejected us, redirect_retry_count (1) was exhausted, and every later
request for that account did the same until the cache aged out. A client could be unable to interact
with a swarm for up to two hours, recoverable only by waiting or a full clear_cache().

Evicting the swarm cache entry is a no-op, which is worth stating since it's the obvious-looking fix:
_swarm_cache only ever holds swarm::get_swarm(pubkey, _all_swarms), _all_swarms is
generate_swarms(_snode_cache), and both writers of _all_swarms replace _swarm_cache in the same
breath. It can never disagree with the pool. What a 421 disproves is the pool snapshot.

Commit 1 adds invalidate_swarm — evidence-driven, alongside the age-driven refresh_if_needed —
with a backoff that doubles each time another rejection arrives after a refresh we already ran for one
(immediate, 1m, 2m, 4m… capped at cache_expiration), so a node rejecting everything costs a handful
of refreshes over a couple of hours rather than one a minute.

Commit 2 makes the redirect the primary path, after review feedback that refreshing on every 421
means a swarm change has every client of that swarm fetch the full node list (51 bytes/node from each
of cache_num_nodes_to_use_for_refresh nodes) where nothing made them fetch it before. A rejecting
node usually names the swarm the account actually belongs to; get_swarm now prefers that until the
pool is refreshed, with the refresh kept as the fallback when a 421 carries no usable redirect.

Only pubkeys are read from the response, and only ones resolving against the pool we fetched
ourselves, so a redirect reaches registered service nodes we already know and nothing else. It can
still choose which of those, so: a redirect naming the swarm we already calculated is refused, fewer
than cache_min_swarm_size resolved names is refused, three in a row without an intervening refresh
is refused, and a pool refresh drops every override it was correcting. Overrides are kept out of
_swarm_cache, which stays a pure memo of _all_swarms.

⚠️ A redirect reports INVALID_SWARM_ID — it names members, not an id, and the new id isn't derivable
from a stale pool. Harmless today (the C API discards the swarm id, _handle_421_retry ignores it) but
it would matter if a consumer started reading it.

3. Strikes never expired where it counted

STRIKE_EXPIRY (48h) was applied in node_strike_count() — which nothing in this repo calls, it's
client-facing API — and on the disk load. Every place that actually decides something counted the
raw vector: refresh_if_needed's usable-node count, get_unused_nodes' skip, and get_swarm's
filter_by_strikes. Since record_node_failure only appends, a node struck three times during a
two-minute outage stayed out of path building, swarm answers and refresh candidates for the life of
the process, with its vector growing unbounded. Restarting was the only cure, because loading is the
one path that filtered.

Lopsided in a telling way: _perform_strikes_write already filtered by STRIKE_EXPIRY on the way out
and _load_from_disk filtered again on the way in. Both ends of persistence honoured expiry; only the
in-memory decision sites didn't. (So this changes nothing about what lands on disk.)

Two further defects fell out, both only reachable at cache_node_strike_threshold == 0 — which is what
tests/test_snode_pool.cpp has been running with — and both caught by the existing suite:

1. A bare >= against the threshold excludes every node in the pool, including ones that never
   failed (0 >= 0). The old code dodged it only by requiring the map lookup to succeed first.
2. A permanent failure recorded no strikes at all — for (i = 0; i < threshold; ++i) is zero
   iterations — leaving a node we know is gone in full rotation. Now max(1, threshold).

⚠️ Behaviour change worth a second opinion: an unreachable-but-registered node now returns to rotation
48h after its last strike instead of never. Intended — it may have recovered, and gets struck out again
on first use — but it is a change.

4. The cached edge node never had its strikes consulted

OnionRequestRouter keeps sticky edge nodes so a client's first hop is stable across sessions, and
hands one to _build_path as a forced first hop — so it's the one node in a path that never passes
the strike filter get_unused_nodes applies to the rest. Only edge_node_cache_duration (10 days)
ever dropped it, so a node we had evidence was unreachable got another path built onto it on every
launch and every resume.

It now loses the cached-edge role once struck out — not its place in the pool; nothing here touches
_snode_cache, so get_unused_nodes can pick it again when its strikes expire. Dropped rather than
skipped because _cached_edge_nodes is written once at load and never again, so a retained entry would
reclaim the role at expiry, by which point we'd been on a different edge node for two days.

Lowest-value commit of the five — the retry already excludes the failed edge node, so the real cost
is one wasted build plus a retry per launch. Easy to drop.

5. refresh_if_needed could silently never call back

Three ways to do nothing and say nothing — suspended pool, no candidate nodes, no fetcher. Callers set
their state up before calling, so "never" is permanent, not slow:

caller	cost
_resync_clock	sets _current_clock_resync_id first, so every later clock resync short-circuits as "already in progress" and its queue is stranded
both routers' setup	_finish_setup() never runs — the router never becomes usable
get_random_nodes, path build, proxy selection	the caller waits on something that never arrives

The callback now carries whether a refresh happened; "nothing needed refreshing" reports true, since
that's the answer the caller wanted. The flag does real work rather than being decoration — simply
always invoking the callback would turn two of these hangs into spins, as get_random_nodes re-enters
itself (and Loop::call runs inline on the loop thread, so that's stack recursion) and the path
builder rebuilds into the same branch.

SnodePool has no C API surface, so this is internal only. The judgement calls are the review-worthy
part: both routers still finish setup (not finishing leaves them permanently unusable), and
_resync_clock routes into the existing _on_clock_resync_complete(), which resets the in-progress id,
fails the queued requests, and leaves the existing offset alone — an old offset beats none.

---

Tests

Four new cases — [invalidate_swarm], [swarm_redirect], [strike_expiry],
[refresh_callback_contract], [cached_edge_nodes] — 139 total, all green. utils/format.sh verify clean.

Where a test couldn't fail against the pre-fix tree (it calls a function that didn't exist), the old
behaviour was reproduced by mutation instead and the failing assertion recorded. [strike_expiry] is a
genuine fail-before-fix: 2 == 4 and 4 == 1 against unfixed code.

Forward-port

client (b0a523fd) carries all of these. Commits 1–2 and 4 apply cleanly; 3 conflicts only on the
sysclock_now_s() → clock_now_s() rename and 5 on the _loop->call → _jq.call job-queue refactor.

[mpretty-cyro]

Pushed a second commit (329f8b9d) answering the aggregate-bandwidth feedback. It's a fast-forward — 60c6d799 is unchanged and still the first commit, so nothing already reviewed has been rewritten.

What changed. The point stands that refreshing the pool on a 421 means a swarm change has every client of that swarm fetch the full node list (51 bytes/node from each of cache_num_nodes_to_use_for_refresh nodes), where nothing made them fetch it before — and the backoff in the first commit bounds how often one client repeats that, not the aggregate.

So the redirect is now the primary path: a node rejecting a request for an account usually names the swarm it actually belongs to, and get_swarm prefers that over its own calculation until the pool is refreshed. The pool refresh stays as the fallback for a 421 carrying no usable redirect, so both paths are needed.

On safety — this was the objection I'd originally raised against using the 421 body, and taking only pubkeys and resolving them against our own pool answers it: a redirect can reach registered service nodes we already know about and nothing else. It can still choose which of those we talk to, so it's bounded:

• a redirect naming the swarm we already calculated is refused (the node contradicting itself)
• fewer than cache_min_swarm_size resolved names is refused
• three in a row without an intervening refresh is refused, then we fall back to refreshing
• a pool refresh drops every override it was correcting, so a claim can't outlive the ground truth that would overrule it

Overrides are kept out of _swarm_cache, which stays a pure memo of _all_swarms.

One thing worth a second opinion: a redirect reports INVALID_SWARM_ID, because a redirect names swarm members, not an id, and the new id isn't derivable from a stale pool. Harmless today — the C API discards the swarm id and the 421 handler ignores it — but it would matter if a consumer ever started reading it.

New [network][swarm_redirect] test covers the accept path and all four refusals, and asserts no refresh is started against a deliberately-aged pool, so "no refresh happened" means the redirect was taken rather than the pool being too fresh to bother. Full suite green (136 cases).