SnodePool: don't let an undersized refresh wipe a good cache

[mpretty-cyro]

Follow-up to #105. That PR fixed the use-after-free in the post-refresh callbacks; this fixes the thing that made it reachable.

The bug

_on_refresh_complete handed whatever it computed straight to _update_cache with no size check:

// Update the cache with the combined nodes
_update_cache(refresh_id, std::move(nodes));

So a refresh that succeeded but produced too few nodes replaced a perfectly good cache. The multi-request path is the easy way in — it keeps only nodes present in at least cache_min_num_refresh_presence_to_include_node of the responses, and that intersection can come back near-empty or empty. The client is then left with nothing to route through until the next refresh completes.

This is also what made the #105 use-after-free reachable: an empty _snode_cache is what sent the deferred get_swarm back round to re-register itself mid-iteration, reallocating the vector being walked.

The fix

Gate on cache_min_size before updating, and treat a shortfall as a failed refresh — keep the existing cache and retry with the same backoff already used for an unparseable response.

cache_min_size is already this codebase's "too small to be usable" line, so the gate is consistent rather than a new policy:

• the fallback-pool loader rejects a pool below it (snode_pool.cpp:510)
• refresh_if_needed triggers a refresh below it (snode_pool.cpp:1064)

Empty is rejected outright as well, so a client that opts out with cache_min_size = 0 still can't be left cacheless.

The retry block is hoisted into a discard_and_retry lambda rather than duplicated — it's the same clear-results / bump-failure-count / exponential-backoff / relaunch sequence in both places.

Trade-off worth a look

On a network with genuinely fewer than cache_min_size nodes we now retry indefinitely instead of serving from an undersized pool. I think that's acceptable because refresh_if_needed already spins in that situation, so such a deployment needs cache_min_size lowered either way — but it is a behaviour change and worth a second opinion, particularly for small local devnets.

Testing

New [network][refresh_min_cache_size] case driving _on_refresh_complete with encoded responses. Verified it fails without the gate:

CHECK( snode_pool->size() == 20 )   // undersized refresh
with expansion: 11 == 20
CHECK( snode_pool->size() == 20 )   // empty refresh
with expansion:  0 == 20

and passes with it. It also asserts the test's own encoder round-trips through process_snode_cache_bin first, so a fixture bug can't masquerade as a pass.

Full suite green: 130 cases, 25,755,856 assertions.

[jagerman]

One possible change here (although I'm not fully convinced either way) would be for this to be a ratio than than an absolute number. For example, 0.5, so that you don't accept an update if it drops by more than 50%.

One downside of that approach is that if some service node maliciously fed you a huge (> double the size) fake list, would you move to it and then stop accepting the legitimate list?