Repository navigation
Conversation
A 421 tells us, 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 on any cache younger than that, and then re-read the same `_swarm_cache` entry - so the retry went to another node in the list that had just rejected us, and every later request for that account did the same until the cache aged out. Evicting the swarm cache entry alone cannot fix it: the entry is only ever a memo of `swarm::get_swarm(pubkey, _all_swarms)`, so it recomputes the identical answer. What the rejection disproves is the pool snapshot the swarms were generated from, and only a refresh replaces that. `invalidate_swarm` refreshes on that evidence, with a backoff that doubles each time another rejection arrives after a refresh we already ran for one, so a node rejecting everything costs a handful of refreshes over a couple of hours rather than one every minute for as long as it keeps it up.
Some responses (like 421s) don't include requested_unlock_height values, and when that happens we were leaving the value uninitialized. This fixes it to default to -1, which means "unknown" so that a missing value can't get misinterpreted as a node about to leave the network.
Refreshing on a 421 recovers the account, but it means a swarm change has every client of that swarm fetch the full node list - 51 bytes per node from each of `cache_num_nodes_to_use_for_refresh` nodes - where nothing made them fetch it at all before. The backoff added with `invalidate_swarm` bounds how often one client repeats that; it does nothing about the aggregate. A node rejecting a request for an account usually names the swarm it actually belongs to, which corrects the one mapping we know is wrong without fetching anything. `get_swarm` prefers such a redirect over its own calculation until the pool is refreshed, and the refresh stays as the fallback for a 421 that carries no usable redirect. Only the node pubkeys are read from the response, and only ones that resolve against the pool we fetched ourselves, so a redirect reaches registered service nodes we already know about and nothing else - it cannot invent a node or name an address of its own choosing. It can still choose which of those nodes we talk to, 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. The redirects are kept out of `_swarm_cache`, which stays a pure memo of `_all_swarms`.
`STRIKE_EXPIRY` was applied in `node_strike_count()` - which nothing in this repo calls - and on the disk load. Every place that actually decides something counted the raw vector instead, and `record_node_failure` only ever appends, so a node struck during a two-minute outage stayed out of path building, swarm answers and refresh candidates for the life of the process, and its timestamp vector grew without bound. Restarting was the only thing that cleared it, because loading is the one path that filtered. Counting is now in one place, used by all four, and a node's expired strikes are dropped when it collects a new one. Two things this uncovered, both only reachable with a strike threshold of 0, which is what `tests/test_snode_pool.cpp` has been running with: - comparing the count against the threshold with a bare `>=` excludes every node in the pool, including ones that have never failed, so a threshold of 0 has to keep meaning "drop a node on its first strike". - a permanent failure looped up to the threshold, recording no strikes at all and leaving a node we know is gone in rotation.
A cached edge node is handed to `_build_path` as a forced first hop, so it is the one node in a path that never passes the strike filter `get_unused_nodes` applies to the rest, and the only thing that dropped it was `edge_node_cache_duration` (10 days). So a node we already had evidence was unreachable got another path built onto it on every launch and every resume, and a path rotation carried it into the replacement path. It loses the cached-edge role rather than its place in the pool: it stays a node like any other and can be picked again once its strikes expire. Keeping the entry and merely skipping it would hand the role back at that expiry, by which point we have been running on a different edge node for two days - a second change of first hop, not a return to a stable one.
`refresh_if_needed` had three ways to do nothing and say nothing: a suspended pool, no candidate nodes, and no fetcher. The callback was simply never invoked, and every caller sets its own state up before calling, so "never" is not a delay - it is permanent: - `_resync_clock` sets `_current_clock_resync_id` first, so a dropped callback makes every later clock resync short-circuit as "already in progress" and strands everything queued behind it. - both routers finish setup from the callback, so they never finish it. - `get_random_nodes` and the path builder retry from the callback, so their callers wait on something that will not arrive. The callback now takes whether a refresh actually happened, which is the smallest thing that lets a caller tell "the pool is fine" from "we could not find out". Nothing needing a refresh reports true - that is the answer the caller wanted, not a failure to get one. Each caller had to be given a failing branch, and two of them would have spun rather than hung if the callback had simply always been invoked: `get_random_nodes` re-enters itself, and the path builder rebuilds into the same too-few-nodes branch. `SnodePool` has no C API surface, so this is internal only.
Issue: a batch or sequence request that gets a 421 carries its redirect inside each sub-response, but the redirect was only read from the top level of the body. Every batched 421 therefore fell back to a full pool refresh - the very thundering herd the redirect exists to prevent, on the path Session polls through. Separately, a batch that is rejected for some accounts and answered for others (as coalescing requests for several accounts into one batch to their shared swarm will produce) never reached the 421 handling at all, because its results aren't uniformly 421. Solution: the redirects are taken from every snode response, per account. `Request::swarm_pubkey` becomes `swarm_pubkeys`, one account per sub-request in order, and each 421 is attributed to the account at its position; the legacy C API fills this in by parsing the pregenerated batch body. Session storage server 2.12 also echoes the rejected account as `pubkey` in the 421 body, but that is the responding node's word, so it is only checked against the position: a 421 naming a different account is ignored. Trusting it instead would let any node the client talks to install a redirect for any account - a group everyone is in, say - rather than only the ones it was asked about. A rejection of a single account still retries the whole request on its (possibly redirected) swarm. A batch rejected for several accounts is not retried - a retry goes to one swarm - but its redirects are recorded, so the next request for each account goes to the right place. A rejection that names no usable swarm falls back to one pool refresh for the whole response. Because the redirects are now recorded before anything else happens to the response, a 421 on a request whose retry budget is already spent no longer discards its redirect either. The batch walk shared by `find_uniform_batch_error`, `_update_network_state` and the redirect extraction is now one `response::subresponses`, and the response body is parsed once rather than separately by each. Forward-porting to `client`: Core's `_swarm_request` needs to fill in `swarm_pubkeys` from the accounts it builds each body for.
Issue: an onion or proxied request's response is encrypted by the destination, and only a response that decrypts is the destination's answer. When decryption fails - as it does for anything a node along the path sends back on its own - the onion router and the session-router proxy both passed the outer HTTP status through unchanged. Network acts on some statuses as the destination's verdict: - 421 re-resolves the account's swarm, refreshing the whole pool when there is no usable redirect, and retries; - 406 resyncs the clock against the snode network; - 425 resyncs the clock against a server. So any node on the path could force a pool refresh or a clock resync just by answering with one of those statuses in plaintext. With 421 recovery now refreshing on evidence rather than on the pool's age, that is a cheap way to make a client fetch the full node list. Solution: when a response can't be decrypted, those three statuses are reported as a new ERROR_UNAUTHENTICATED_RESPONSE instead. Every other status passes through as before: the same path also carries timeouts and transport failures, and the onion router's path-failure handling matches on the statuses nodes along the path return, none of which is one of these three. The error message now carries the status the node sent, since the reported status no longer does. Requests sent to a storage server through session-router tunnels, and by the direct router, are unaffected: they aren't body-encrypted, their status is the destination's own over an authenticated connection, and they never reach either decryption path.
Issue: a redirect was refused as "the node contradicting itself" when it named the swarm calculated from our pool. That comparison measures a redirect against exactly the data the 421 says is out of date. Swarms can be rearranged any way at all - members join or leave, a swarm is disbanded and its members scattered, a new one is formed from nodes anywhere in the pool - so the account's actual swarm can relate to our grouping in any way: the same, a part of it, overlapping, or disjoint. How a redirect compares with it says nothing about whether the redirect is right. In particular, once a redirect had been followed to swarm T, a node in T correctly naming the calculated swarm - which is how a wrong redirect gets put right - was refused, and since refused redirects don't count towards the limit, the account stayed pinned to T until a pool refresh, which invalidate_swarm declines while the pool is under a minute old or its backoff is active. Solution: the comparison is gone. A redirect is followed whenever enough of the nodes it names resolve against our pool. What bounds a redirect is unchanged and never depended on the comparison: only nodes already in the pool can be named, and every redirect is dropped at the next pool refresh.
Issue: refresh_if_needed reports true both when a refresh ran and when nothing needed refreshing, and in the latter case calls back straight away - inline, since Network, the pool and the routers share one loop. Two callers retried on true without checking that they could now get what they needed: - get_random_nodes called itself again; - _build_path rebuilt the path. get_unused_nodes applies subnet diversity, exclusions and strikes, none of which refresh_if_needed's count of usable nodes does, so a pool can be healthy by refresh_if_needed's measure and still unable to supply the caller. Each retry then landed back in the same branch with nothing changed, recursing until the stack gave out: get_random_nodes from the C API with a count the pool can't meet, _build_path on a pool whose usable nodes sit in too few /24s to fill a path, such as a small testnet or devnet. This predates the refresh callback reporting a result - it only stopped the retry when a refresh could not be started at all. Solution: both check before retrying, as the session-router proxy selection already did, so each makes at most one refresh request. get_random_nodes calls back with an empty list, per its documented contract, and _build_path fails the queued requests for the category through the same path as a refresh that could not start, with messages that now say which it was. The onion router test's snode pool can now call back from refresh_if_needed, up to a limit so that code which asks again on every callback stops rather than overflowing the stack. Against the unfixed _build_path the new test sees 21 refresh requests and the queued request never answered; with the fix it sees one, and the request failed.
Issue: after MAX_CONSECUTIVE_SWARM_REDIRECTS (3) redirects for an account, the next was refused and the account's override erased, for the caller to fall back on refreshing the pool. But that refresh goes through invalidate_swarm's throttle, which declines while the pool is under a minute old or its backoff is active - exactly when a network-wide rearrangement makes hitting the limit likely. When it declined, the request failed, the account went back to the swarm calculated from the pool (the one the first 421 had already disproved), and with the entry erased the count started over from zero, so the limit never held. The count was also never reset by anything but a refresh, so "consecutive" was wrong, and legitimate changes to a swarm's membership spent it just as disagreeing nodes did. Solution: the limit only ever asks for a refresh; it never refuses a redirect, since refusing one only guarantees the request fails. Past it, each further redirect is followed as usual and also asks invalidate_swarm for a refresh, so the refresh happens at the first redirect its throttle allows, and until then the account stays on its latest redirect. Renamed to SWARM_REDIRECTS_BEFORE_REFRESH for what it now does, and raised to 8: a genuine bounce between disagreeing nodes still reaches it within a few requests, while ordinary rearrangements, which take one or two redirects an account, don't. The count saturates rather than wrapping, since it keeps climbing for as long as the refresh stays throttled.
Issue: a redirect was refused unless at least cache_min_swarm_size of the nodes it named were in our pool. That is the realistic way a redirect can't be followed in full: a swarm rearranged partly out of nodes that registered after our last pool refresh. The nodes we do know are still members of the account's actual swarm, but refusing the redirect left the account on the swarm that had just rejected it, with the request falling back to invalidate_swarm - which declines while the pool is under a minute old or its backoff is active, and when it declines fails the request outright. Solution: a redirect is followed with whichever of its nodes are in our pool, and refused only when none are. When some of the named nodes are unknown and that leaves fewer than cache_min_swarm_size, it also asks invalidate_swarm for a refresh, through the same throttle as the redirect limit, to fill in the rest. A small swarm whose members are all known doesn't count, since the pool isn't missing anything. Failing the request when the swarm can't be re-resolved is kept, and now says why: it is reached only when no node the redirect named is one we know, so a retry could only go back to the swarm that rejected us.
Issue: when a path couldn't be built, or the session router couldn't find a proxy, because the snode pool couldn't supply enough usable nodes, the failure was reported with a bare -1. Network failures are otherwise reported with the named ERROR_* codes, which is what lets a caller (through the C API included) tell what went wrong; -1 only says that something did. Solution: a new ERROR_INSUFFICIENT_NODES (-10013) for all three sites - the path builder failing its queued requests, and proxy selection after a pool refresh that failed or left no usable node.
Issue: QuicTransport::suspend() returned early unless the transport was
already suspended - the guard was inverted - so it never did anything and
_suspended was never set. In production that mostly didn't show, because
Network::suspend closed the transport's connections straight afterwards,
which resets its endpoint: requests during a suspension still failed at
once, just with -1 ("Network is invalid") rather than
ERROR_NETWORK_SUSPENDED. The defect was the wrong error code and a
suspended flag nothing could rely on. This is on dev as well.
Fixing the guard makes the transport's suspend() close its connections
itself, and exposes how that close treated the nodes it closed on:
- The connections are closed synchronously by the endpoint's destructor
when _close_connections() resets it (close_conns() only defers, and its
deferred job finds the endpoint gone). Each close runs the transport's
handler with code 0, and _fail_connection passes that code to every
pending edge verification. The router strikes an edge node whose
verification fails with an error code other than a handshake timeout, and
0 counts - so a close we asked for struck every edge node with a
verification in flight. Verifications with no connection yet were
cancelled with -1, to the same effect.
- _fail_connection also fires the transport's failure listeners, and the
router's listener retires the path and starts rebuilding it.
Both reach the router while it is still live, because Network::suspend
suspended the transport before the router, and Network::_close_connections
(the public close_connections() route, on dev too) closes the transport
before the router.
Solution:
- suspend() returns early only when already suspended.
- _close_connections() takes the pending verifications and requests out of
their maps before resetting the endpoint, so the destructor's closes find
nothing to fail, and answers the copies once the transport is fully
closed: verifications with no error code, since our own close is no
evidence against a node, and requests with ERROR_NETWORK_SUSPENDED when
suspended or ERROR_CONNECTION_CLOSED when not (they were told "suspended"
either way). Answering copies rather than walking the members also means
a callback that calls back into the transport - the router's path rebuild
calling verify_connectivity, say - can't insert into the maps being
walked; such a call finds no endpoint and fails cleanly, without an error
code. That is a reason to keep this shape even if the code-0 close is
ever avoided some other way.
- Network::suspend suspends the router before the transport, and
Network::_close_connections closes the router before the transport, so
the router has dropped its path builds and failure listeners before the
transport closes the connections they are attached to.
Tests: a request sent to a suspended transport is refused with
ERROR_NETWORK_SUSPENDED (with the inverted guard it went out and failed on
the network with -1), and a verification in flight when the transport is
closed is answered exactly once, with no error code (before, it got 0 from
the destructor's close).
Issue: SnodePool::suspend() set its flag and flushed strikes, and left any refresh in progress to carry on. With the transport and router both refusing requests while suspended, every fetch that refresh made failed at once, and its retry paths - a failed fetch moving to the next candidate, a rejected set of results being fetched again - don't check for suspension, so it worked its way through the candidate list at the retry delay (capped at 5s, three chains in parallel): about an hour for a mainnet-sized pool, waking the process to do nothing useful on Android and desktop (iOS freezes it, so there the timers never fire). Whatever waited on the refresh was answered only when the candidates ran out, and the failures it counted up slowed the first refreshes after resume. Solution: suspend() abandons a refresh in progress - resetting its id, candidates, results and failure count - and answers everything waiting on it with false, as the router and transport already do with their own work in flight. Treating a suspension as a fresh start drops genuine pre-suspend failures too; that is deliberate, since they say nothing about the network we resume on. Nothing can be in flight while suspended any more, so _refresh_snode_cache's suspended branch answers its waiters unconditionally. Retries belonging to an abandoned refresh can still fire afterwards, so: - _launch_next_refresh_request does nothing unless its refresh is still the current one; before, it only checked that some refresh was, and would take candidates from whichever refresh had replaced it; - the "ran out of candidates" retry captures its refresh id and does nothing when that refresh is gone. Before, it reset the id and started a new refresh unconditionally, which could wipe out a refresh started after resume. This was the one place a refresh restarted itself; after a suspension, the next refresh now comes from the pool's ordinary callers asking again - the router's path pre-build on resume, via get_unused_nodes, or a clock resync. A refresh invalidate_swarm started has already stepped its backoff when it is abandoned, so after a short suspension the first rejection-driven refresh can be declined, for at most the current backoff interval. The throttle resets itself after quiet for twice that interval, so a longer suspension carries nothing over.
Issue: [get_unused_nodes], [update_cache] and [refresh_min_cache_size] built their config::SnodePool positionally, with trailing comments naming fields the values no longer went into. Each list was one entry short of the struct and misaligned by two, so the value commented as cache_node_strike_threshold went two fields early and the trailing false filled a numeric field as 0: - in [get_unused_nodes] (both of its configs) and [update_cache], the 3 went into cache_num_nodes_to_use_for_refresh and the false into cache_min_num_refresh_presence_to_include_node; - in [refresh_min_cache_size], which also sets cache_min_size, the 3 went into cache_min_num_refresh_presence_to_include_node and the false into cache_node_strike_threshold. Every one of them therefore ran with a strike threshold of 0 - a node out on its first strike. None of the three tests' outcomes depended on that, or on the refresh settings the values landed in: TestSnodePool makes refresh_if_needed a no-op, none of them reaches a refresh through get_swarm on an empty pool, and [refresh_min_cache_size] feeds _on_refresh_complete a single response, for which the presence requirement doesn't apply. But the one strike check, in [get_unused_nodes], uses permanent failures, which strike a node out at any threshold, so nothing checked the threshold itself. Solution: all four configs are designated, with the values the comments meant - cache_node_strike_threshold 3, plus cache_min_size 12 where marked - and everything else they didn't annotate left 0. So besides the threshold, cache_num_nodes_to_use_for_refresh goes from 3 to 0 in the short lists and the presence requirement from 3 to 0 in [refresh_min_cache_size]; for the reasons above, neither has any effect on these tests. [get_unused_nodes] now also checks the threshold: a node with two ordinary strikes is still picked, and with a third it isn't. At the old threshold of 0 that check fails, since two strikes already exclude the node.
Issue: _rotate_path keeps a path's edge node across rotations - so a client's first hop stays stable - unless it has been in use longer than edge_node_cache_duration, or (since the cached edge node change) it has been struck out. Nothing tested the struck-out case; the existing test covers only the edge nodes cached from a previous run. Solution: [onion_request_router][rotate_path] rotates a path whose edge node was connected just now, so the ten-day rule can't be what decides, and checks what the rotation asks the pool for: the rest of a path keeping the edge node (path_length - 1 nodes) when it isn't struck, a whole new path (path_length) once it is. The mock pool now records the count of each get_unused_nodes call, since its canned result doesn't reflect it. Without the struck-out condition the second rotation asks for path_length - 1 and the test fails. TestOnionRequestRouter::set_paths used emplace, which leaves a category's paths alone if it already has some, so setting them twice on one router silently kept the first set. It now replaces them, which is what its name says and what the new test needs; no existing test set them twice.
Issue: get_swarm schedules a background refresh_if_needed, the check that refreshes a pool past cache_expiration, only on the path that works a swarm out from the pool. Answers from the swarm cache return before it, and so, since the redirect commits, do answers from an override. So each account triggered the check once per pool generation, on its first lookup, and never again while its swarm was cached - and an overridden account, whose override only a refresh clears, never did at all. Refreshes still happened, because path rotation and building call get_unused_nodes, which schedules the same check, but only thanks to that unrelated caller. The cached case predates this branch; the override case widened it. Solution: the check is scheduled at the top of get_swarm, so every call makes it. It costs nothing when the pool is fresh or a refresh is already running. A test counts the checks: one each for a swarm worked out from the pool, one answered from the cache and one answered from an override; before, the last two made none.
Issue: session_network_send_request takes each sub-request's account from a pregenerated batch or sequence body, and uses the caller's swarm_pubkey_hex only when the body can't be parsed at all. A sub-request whose params carry no pubkey got an empty entry even when the caller had said which account the request was for. With 421s attributed strictly by position in the request, a 421 for that sub-request could then be attributed to no account, and the redirect it carried was lost. Solution: batch_request_accounts takes a fallback account, which fills any sub-request whose params name none; accounts parsed from the body are kept, since they can differ per sub-request. The C API passes swarm_pubkey_hex as the fallback.
Issue: every accepted swarm redirect logged at info, and once an account is past SWARM_REDIRECTS_BEFORE_REFRESH every further redirect warned and asked invalidate_swarm for a refresh, whose "leaving it alone" line for a throttled refresh was also at info. During a bounce between disagreeing nodes that is several lines per request for as long as the backoff holds, up to its two-hour cap - useful once, noise after that. Solution: the per-redirect line and invalidate_swarm's decline line are debug. The limit warning fires only on the redirect that crosses it, and later ones log at debug. The warning for a redirect naming nodes our pool doesn't have is unchanged. It can repeat too - every request for that account produces one while the refresh it asks for is throttled - and the same first-then-debug treatment would apply, but that waits for evidence that it is noisy in practice.
A redirect asked for a pool refresh only when it named nodes we don't have *and* that left fewer than cache_min_swarm_size of them. The first half reads "our pool is behind", which is a fair guess for an honest redirect - a rearrangement pulls in nodes registered since our last refresh, so some names don't resolve - but it is exactly backwards for a dishonest one: a node naming only itself leaves nothing unresolved, so the pool looks fine and no refresh is asked for, while the account is pinned to a single node until one happens for another reason. Dropping that half changes one shape - a redirect naming fewer than cache_min_swarm_size nodes, all of which we know. Once enough resolve the condition is false either way, so a redirect naming a node or two we don't happen to have alongside a full swarm still asks for nothing. The redirect is followed either way; this only decides whether a refresh is also requested, and that request goes through invalidate_swarm's throttle.
The Linux CI stages build against the system liboxenquic, which deprecates quic::opt::disable_mtu_discovery in favour of max_udp_payload::minimum(), and -Werror makes that fatal. The vendored libquic this repo pins has no max_udp_payload at all, so the deprecated name is the only spelling that compiles against both - there is nothing to migrate to here yet. Moving the pin is not a pin change: the newer session-router defines oxen::quic through its own session-deps, which collides with the definition in external/CMakeLists.txt, so the dependency handling has to move onto session-deps first. That comes over with the Session client work, and the TODO goes with it.
tests/utils.hpp uses std::condition_variable, std::mutex and the lock guards without including <condition_variable> or <mutex>, and test_config_userprofile.cpp calls std::this_thread::sleep_for without <thread>. Newer standard libraries pull all three in through something else, so this builds everywhere except the older libstdc++ on the Debian 12 CI stage, which is where it fails. Both predate this branch. They surfaced now because every Linux stage used to fail compiling the network library, before it got as far as the tests.
…-invalidation-on-421 Snode pool: 421 recovery on evidence, strike expiry, and reporting failed refreshes
Forward-port of dev commit 348b778; line-for-line identical to dev's. from_json left requested_unlock_height uninitialized whenever the JSON omitted it, which swarm responses and 421 bodies always do, and an oxend node dump does for a node with no unlock requested. The field becomes int64_t defaulting to -1 so "we were not told" is distinguishable from "not requested", and the snode_pool filter tests `> 0` rather than truthiness.
Brings client's snode pool, network and routers to dev's session-foundation#154 behaviour: swarm redirects taken from a 421's own swarm list (resolved only against nodes we already know), evidence-driven refreshes with global exponential backoff, overrides kept apart from the calculated swarm and dropped on refresh, refresh callbacks that report whether a refresh happened, strike expiry applied everywhere strikes are counted, a stale check on every get_swarm, per-account rejection parsing for batch/sequence requests, undecryptable responses reported as ERROR_UNAUTHENTICATED_RESPONSE, ERROR_INSUFFICIENT_NODES, and the QUIC suspend/close fixes. The one piece of client's design kept is that 421 retries belong to Core, not Network. Network therefore corrects the swarm but does not retry: it reports ERROR_MISDIRECTED_REQUEST once re-resolving the swarm would give a better answer (immediately when a redirect was taken, after the refresh otherwise), and the new ERROR_SWARM_UNRESOLVED when it could not be corrected, so Core ends the operation instead of retrying against the swarm that just rejected it. This replaces client's set_swarm-based adoption of a 421's swarm. Tests are dev's, adapted to client's JobQueue/Loop& and std::byte idioms.
Marks dev through 3e92795 as merged without taking its tree: - session-foundation#154 (swarm invalidation on 421) is ported in 87a6bb9, adapted to client's Core-side 421 retry. - 348b778 (uninitialized requested_unlock_height) is ported in 4df438e. - 3e92795 (version bump) is 1.x-only and not taken.
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
2 participants
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.
Forward-ports dev's recent changes to client, and records dev (through 3e92795) as merged.
Merge this as a merge commit, not a squash: the last commit is a
-s oursmerge of dev, which is what lets a later merge of client into dev go through without conflicts.Commits
Fix uninitialized requested_unlock_height usage: dev's 348b778, line-for-line.
Port swarm invalidation on 421 from dev (Snode pool: 421 recovery on evidence, strike expiry, and reporting failed refreshes #154): brings client's snode pool, network and routers up to dev's Snode pool: 421 recovery on evidence, strike expiry, and reporting failed refreshes #154 behaviour:
get_swarmchecks whether the pool is staleERROR_UNAUTHENTICATED_RESPONSEERROR_INSUFFICIENT_NODESThe one piece of client's design that stays is that Core, not Network, retries a 421. So Network corrects the swarm but does not retry:
ERROR_MISDIRECTED_REQUESTonce re-resolving would give a better answer: straight away when a redirect was taken, otherwise after the refreshERROR_SWARM_UNRESOLVEDwhen the swarm couldn't be corrected, and Core then ends the operation rather than retrying against the swarm that just rejected itThis replaces client's
set_swarmhandling of a 421's swarm. Dev's tests come across, adapted to client's code (JobQueue,Loop&,std::byte).Merge dev into client: a
-s oursmarker covering the above. Dev's 3e92795 version bump is 1.x-only and not taken.Notes
[attachments],[configs][core],[send_dm], …) now log a lot of "Cannot refresh cache: No seed nodes are configured!":get_swarmnow checks for staleness on every call, as on dev, and those tests' pools have no seed nodes. Nothing fails.