Detect configs expired from the swarm and re-store them - #1973
Open
mpretty-cyro wants to merge 14 commits into
Open
Detect configs expired from the swarm and re-store them#1973mpretty-cyro wants to merge 14 commits into
mpretty-cyro wants to merge 14 commits into
Conversation
Bilb
reviewed
Aug 5, 2026
Comment on lines
+34
to
+37
| * Note: group configs are detected but NOT recovered here. The NodeJS wrapper's | ||
| * `MetaGroupWrapper::push()` returns null for any sub-config whose `needs_push()` is false, and | ||
| * §4.2 only lets us re-store a config that is clean — i.e. exactly when it returns null. There is | ||
| * no way to reach the bytes until the wrapper exposes an unconditional serialise. |
Collaborator
There was a problem hiding this comment.
this PR goal is not to help with recovering for groups specifically?
Collaborator
Author
There was a problem hiding this comment.
Ugh... of course Claude would explicitly tell me the implementations are consistent and they wouldn't be and I just missed that there were no group recovery functions in the file as both iOS and Android have a single "restore" function that handles both so I just assumed that restoreUserConfigs was correctly doing both 😞
mpretty-cyro
marked this pull request as draft
August 16, 2026 23:26
…tore them Config messages have a 30-day TTL that is refreshed by the expire bump already piggybacked on every poll. A device offline past the TTL loses its config from the swarm and nothing noticed. This reads the bump's own answer to detect that case and re-stores from local state. Reasoning for each rule is in the code. - Detect missing config hashes from the `unchanged` array the server returns for an extend-only expire; send shortenOrExtend: 'extend' rather than '' so the array is returned at all. An absent key means detection is unavailable, not that every config is gone. - Re-store on a later poll, only once local state is level with the swarm, so a stale local config can never overwrite a newer remote one. - Chunk the recovery batch at 20 sub-requests inclusive of the obsolete-hash delete and stop at the first failing batch. The server rejects the whole sequence above 20, and a single config can split into ~66 parts. - Bar a settled hash for an hour rather than for the session; a desktop session can run for days and outlive the TTL. - Issue the obsolete-hash delete only for restores that fully landed. - Retrieve now requires exactly one response length rather than accepting N or N+1. The old tolerance let a response missing one retrieve result pass, after which every namespace took its neighbour's messages and the last one was handed the expire result. iOS and Android both reject a short response already.
Group configs were detected but never recovered — recoverIfNeeded returned at `!isUsFromCache` before even the level-with-swarm check. The guards and bookkeeping are now shared and only the inspect-and-restore step branches. Three things about the group path that read like bugs and are not: - GroupKeys is detect-only. A keys message cannot be re-emitted; only an admin rekey replaces one. A missing keys hash is reported and settled, never retried. - a member gets an EMPTY obsolete-hash list. push() hands the superseded hashes back only if (!is_readonly()) while clearing them either way, so empty is the expected result rather than a failure. - a member could not act on a non-empty list anyway: its subaccount token carries Read+Write but not Delete. Member-driven recovery re-stores and never prunes. Kicked and destroyed are checked separately on purpose: libsession sets kicked=false when a group is destroyed, so checking one reads as complete and silently misses the other population. StoreGroupConfigSubRequest now takes optional authData and requires EITHER credential rather than specifically the admin key. Member recovery could not have worked without this: the constructor threw on an empty secretKey while its own build() comment described subaccount signing, so it forbade a capability it claimed to have. The signer already supported it. This is a shared request type on the normal push path — an admin push that omitted its secretKey previously threw and will now attempt subaccount signing. Review items: - recoverIfNeeded is no longer awaited by the poller; a recovery round is up to 20 sub-requests per batch and was holding the shared poll loop. It cannot produce an unhandled rejection because it catches its own errors. Voiding it makes overlapping rounds possible, so rounds are now guarded per pubkey and cleared in a finally — the guard holds the round's promise rather than a marker, which is also what lets a test await a round the poller does not. - per-swarm state is typed AccountPubkey (PubkeyType | GroupPubkeyType) rather than string; it is never a snode key. Narrowed at the detection boundary with a real is03/is05 check rather than a cast, since something that is neither names no swarm we could recover. - removed all references to the design document and its section numbers. It exists in no repo, so every one was a dangling pointer; the section numbers were worse, reading like a reference to something in this tree. One comment was a bare section number with no prose and has had its substance written in. - the retrieve length check now admits exactly one length instead of two. Sub-responses are paired to sub-requests BY INDEX, and the old check accepted either N or N+1, so a response missing one retrieve result passed and shifted every later namespace onto its neighbour's messages — with the last one handed the expire result as its message list. Behaviour change on the polling path: a short response now fails the poll (retries in seconds) instead of being silently misattributed (never self-corrects). Read from the request builder, not observed on the wire. Tests: 14 new, covering V16/V16a/V16b/V19/V20/V21, kicked, destroyed, clean-only, level-with-swarm, and the in-flight guard including its release on failure. All mutation-verified. One earlier test asserting "group swarms are not re-stored on Desktop" was removed: it had stopped being true, and it had only kept passing because that file never stubbed the group wrappers, so the lookup threw and the early return did the work.
…node check The failed sub-responses in these two vectors carried no `unchanged` array, so the eligibility check excluded them as unreadable and never consulted `failed`. The `failed` term could have been deleted at any point with the whole suite green — verified by doing exactly that: nothing died until these fixtures gained the array, and then precisely V5 and V6 died. Carrying the array also makes them the shape the term exists for. A node that reports failure while still returning arrays is the dangerous input: read as usable, its empty arrays become authority and every requested hash is reported missing — a false positive that re-stores configs the swarm still holds, on the word of a node that told us it failed, and then prunes the older copy. Found via the shared vector table, whose V5 row omitted the array and propagated the blind spot to whoever implemented it faithfully.
…ne twice V8c is the mixed case: unreadable sub-responses alongside a readable one. It had two unreadable nodes that were both excluded by the SAME guard — the failed node carried no `unchanged` array, so the readability check excluded it and its `failed` flag was decorative. Each unreadable node now trips only its own guard: the failed one carries full arrays, the other omits `unchanged`. Verified by mutation in both directions — V8c now dies when either guard is deleted, where before it was insensitive to `failed` entirely. Same shape as the V5/V6 fix in 80f60ad62: a vector naming only the field that must be wrong gets a fixture that trips some other guard first.
libSession now retains each active keys message verbatim, so a keys message that expired from the swarm can be pushed back unchanged. That is what makes this work for a MEMBER: a keys message is admin-signed with padding derived from the group secret key, so nobody can regenerate one, but bytes already held land on the same hash. We never reconstruct and never re-sign. Three consequences that are not obvious from the rule: - ALL retained keys messages go back, not only the ones reported missing. A generation is the full rekey plus every supplemental issued against it, and a member receiving only part of one does not get the key. The accessor is hash-keyed and carries no generation, so grouping by generation is not expressible; re-storing everything is a superset of the affected generation and the extras are byte-identical no-op TTL refreshes. - no delete on this path. A keys message supersedes nothing, so there is never an obsolete-hash list to prune. - pushForRecovery() is now only called when groupInfo or groupMember actually needs restoring. It drains obsolete hashes as a side effect, and a keys-only recovery has no reason to pay that. The expired flag is handled at both ends, because one alone is insufficient: - the poller DEFERS it. Flagging a group expired while we hold the bytes to repair it would tell the user it is gone at the moment we can fix it, and raising-then-clearing is a visible flicker. - a landed keys re-store CLEARS an existing flag eagerly rather than relying on the reactive clear. That path fires when config messages are received, but we just re-stored messages we already hold and may never re-handle them, so the flag could sit set forever over keys that are back on the swarm. Vectors V23, V23a, V23b, V23c, V23d, plus a member variant and a failure counterpart for the flag clear. All four rules mutation-verified. Also corrects comments this change falsified — "a keys message cannot be re-emitted" was true when written and is now the opposite of the behaviour. Requires libsession_util_nodejs to expose activeKeyMessages(), which needs the unmerged libsession-util change. Built and tested here against a local wrapper build; package.json pins a release that does not carry it yet, and must be bumped once one does.
…ogress
Cold review found a re-push storm. The backoff reset was gated on
`anyPartLanded` — any store sub-request returning 200 — while barring is gated on
a config landing in FULL. Every part of a multipart config goes back on every
attempt, so a config whose parts half-land sends the identical request next
round. One part that always succeeds beside one that always fails therefore reset
the counter forever: nothing barred, backoffMsFor(0) is 0, full re-send on every
poll, indefinitely. Measured before the fix at 10 polls -> 10 sends, 0 barred.
The doc comment asserted the opposite of what the file did ("the parts that
stored are barred and the next round is strictly smaller"), and the test asserted
only that a retry happened — which is equally true of the storm. That is why it
survived: with the all-parts rule, "it retried" cannot distinguish progress from
a loop.
The predicate is now "did anything become BARRED", not "did anything return 200".
Only the first makes the next round smaller. Note it is still not simply `stored`
— with several configs in flight, one landing in full genuinely shrinks the next
round, and backing off there would penalise a converging swarm. The new tests
bracket both sides: the storm test dies if the predicate widens back to any-200,
the converging test dies if it narrows to total-success.
Also from the same review:
- keys recovery is no longer gated on `needsPush` (ruling v139). That gate exists
so local state cannot overwrite newer remote state; keys recovery replays the
exact bytes the swarm already had, so it cannot overwrite anything, and a
pending rekey produces a new message at a new generation. Gating keys on a
dirty groupInfo excluded groups in active use — the population most likely to
need them. Info and members stay gated, and a test pins that.
- the expired flag is now set from detection. Nothing set it before: the only
path was the poller's empty-fetch branch, which requires holding NO config
hashes and so cannot fire for a device whose hashes are exactly what reported
them missing. V16, V16a and V23c had no implementation. Raised when every keys
hash is gone and no bytes are held, and after a keys re-store we attempted and
failed; still deferred while a repair is in hand, still cleared eagerly on
success. The comment claiming this waited on wrapper attribution was stale —
that blocker went with activeHashesByConfig.
- `missingHashesByPubkey` no longer grows without bound. Same leak
pruneExpiredBars was written for, one map over, against the same long-lived
sessions. The two pruners are order-dependent: pruneSettledDetections reads
hashSettledAt and pruneExpiredBars empties it, so run the other way round the
leak survives with both present and looking correct. Commented at the call site.
- a rationale citing MAX_RECOVERY_ROUNDS_PER_SWARM, which was replaced by the
backoff and no longer exists. Behaviour was right, stated reason was not.
- mergeMultipleRetrieveResults set each namespace's code from `find(...)?.code ||
200`: the first entry across all polled snodes, so the verdict depended on
result ordering and was arbitrary in both directions, and `|| 200` defaulted a
missing code to success. Its only consumer decides whether we are level with
the swarm, where "unanswered read as answered" is the failure that matters. Now
a namespace answered if any snode returned 200, with no success default.
1002 passing / 0 failing. Every fix mutation-verified.
…e for The comment above these declarations recorded that the scoping is non-uniform — Sets session-scoped, hashSettledAt time-bounded — and stopped there. Recording a difference without saying what it constrains reads as trivia, so a reader takes it as background rather than as a rule, and reaches for the predicate for a purpose it does not suit. That already happened: localStateIsLevelWithSwarm was proposed as the guard on a force-rekey. It cannot be one. Session-scoped here means PROCESS-LIFETIME, which on Desktop is weeks — nothing ages these Sets out, so the predicate answers "was level at some point since startup", never "is level now", and the staleness it permits is unbounded. That is correct for its current consumer: recovery is a cheap idempotent re-store, so acting on a stale verdict costs a redundant request. It is wrong for anything irreversible. A rekey encrypts to THIS DEVICE'S view of the members (libsession keys.cpp:376-386 builds member_xpks from the passed Members config), so a stale "level" verdict authorises a write from a members list we already know may be behind, silently dropping anyone added since. It fails OPEN. The general rule, at the declarations rather than any call site because that is where the next reader arrives: check a value's lifetime against what YOU are about to do with it, not against what its existing caller does. Note hashSettledAt was made time-bounded for exactly this reason, and the same reasoning was never applied one declaration up. The lesson and its counterexample sat eleven lines apart in this file, both written by me — a rule filed under the mechanism that produced it does not generalise; filed under the property it protects, it does. Doc-only. No behaviour change.
… fetch libSession retains the raw bytes of a keys message when it loads one, and only then. A device that merged a group's keys before retention existed holds the key but not the bytes, so it cannot put that message back if it ever expires — and it has no way to notice. This re-fetches the keys namespace and re-merges it, which captures the bytes. Re-loading a keys message we already hold the key for is a no-op for key state (insert_key early-returns) but not for retention: that path still stores the bytes and flags a dump. It runs PROACTIVELY, beside recovery rather than inside it. Recovery acts on a hash the swarm has LOST; this acts on one the swarm still HAS. Hanging it off the detection path would look correct and be nearly useless, because by the time detection fires the message it needed to fetch is gone. Three things that are easy to get wrong and are pinned by tests: - IT MUST PERSIST THE DUMP. Retention lives in the config dump, so a merge that captures bytes without persisting loses them on restart — the back-fill appears to work and silently does not, and every in-process assertion passes either way. Worse here than elsewhere: saveDumpsToDb is stubbed in the suite's beforeEach for an unrelated reason, so an implementation that never persists would pass the whole file. The test therefore asserts the merge ran as a separate PREMISE before asserting the persist, or "it was called" is satisfied by a path that exited early. - THE ATTEMPT RECORD MEANS "ATTEMPTED AND THE BYTES ARE STILL ABSENT", not "the fetch came back empty". A fetch returning messages that still do not restore the bytes is equally a failed attempt; recording only the empty case leaves the group looking un-attempted forever while re-fetching the same useless messages. The two are indistinguishable in any fixture where the swarm holds nothing — which is the first fixture anyone writes — so there is a test with a swarm that returns something and still leaves the bytes missing. Credit to the iOS client, which shipped this wrong and said so. - IT CALLS THE RETRIEVE LAYER DIRECTLY, never the poll wrapper. pollNodeForKey writes the namespace cursor from whatever it fetched, and this asks with no last_hash, so routing through it would advance the cursor past messages the poll never consumed. Verified the retrieve is pure: the only writers of that cursor are that one call site and the Data helper it calls. The record is in memory deliberately. Persisting it would be a sticky negative, letting a later force-rekey fire on evidence gathered weeks ago after the swarm had changed underneath it. Forgetting on restart delays by one poll cycle, which is the safe direction. It is cleared on success, because it is read as "this device cannot repair this group" and a device that just retained the bytes can. A throw records nothing: we never learned whether the bytes are obtainable, and a failure recorded there would let the rekey act on evidence we do not have. 1043 passing / 0 failing, tsc 0. Measured against libSession 97aafbbd via a local wrapper build (config-recovery-keys-messages @ 4ed2d44); package.json still pins v0.7.4, which does not carry activeKeyMessages yet.
…the keys Last resort for a group whose keys messages are gone from the swarm and whose bytes nothing here retained: an admin mints a new generation so the group becomes usable again. This is the only irreversible, universally visible write in config recovery. Everything else replays bytes the swarm already had — byte-identical, same hash, invisible to other clients. This creates new state every member on every version must process, and it cannot be undone. Almost all of the code is about not doing it when it was not needed. Preconditions, each with a test that starts from a fixture which WOULD rekey and changes exactly one thing: - our members view must be level with the swarm AS OF THE CALLING POLL. The rekey encrypts to this device's view of the members, and it fires precisely on devices whose config state is known to be degraded, so a member added while we were away and not yet merged would be silently dropped. The caller computes the value and passes it; refusing on it here rather than trusting the caller is what makes the refusal testable at all. - a back-fill must have RUN and still come up short. "We hold no bytes" cannot distinguish that from "no back-fill has ever run" — identical on a fresh install, a restored backup, or before the first poll completes, and only one of them justifies this. - every keys hash must be beyond us. One surviving keys message still admits a new device, so the group is not stuck. - admin only, and not for a kicked or destroyed group. - once per group per session, with a cooldown. Without it every poll that still sees the old preconditions mints another generation. Lives in its own file with one entry point and no dependency from the back-fill to it, so it can be removed by deleting the file. Verified by doing exactly that rather than by arguing it: with this file, its test and its call site deleted, the suite returns to 1043 passing and all eight back-fill tests still pass. The freshness value passed to it is the same expression the enclosing branch computes, not a literal `true` written inside that branch — a literal would be correct only until someone moved the call. 1050 passing / 0 failing, tsc 0, measured against libSession 97aafbbd via a local wrapper build (config-recovery-keys-messages @ 4ed2d44).
… in no repo Both cited the shared design doc — one a section number, one a version — in comments that already carried their own reasoning, so removing the citation costs nothing. A bare section number is the worse of the two: it reads like a reference to something in this repository. Found by widening the sweep from this round's files to every file the branch touches. The earlier sweep was scoped to the four files I had just edited, which is why these survived it: they predate that round.
…ent permits Four comments referred to a design document that exists in no repository. Two were bare citations that cost nothing to drop. Two were worse than citations, because they outsourced the obligation itself and left the reader nothing local to act on: - allConfigNamespacesAnswered said we gate the whole swarm rather than the individual namespace because "the spec allows either, and the coarser one cannot be got subtly wrong". A reader cannot check that. It now gives the local reason: gating per namespace means mapping each hash back to the namespace it came from and keeping that mapping correct as either side changes, and getting it wrong means acting on a config we are ignorant about, silently. Gating the whole swarm is coarser — we skip a repair we could safely have made — but it fails by doing nothing, which the next poll fixes. - the detection vectors' header said "don't relax one to make an implementation pass, change the spec", which is an instruction the reader cannot follow. It now says what to do: a vector failing here is a disagreement between clients, so the other two implementations are the thing to check, and relaxing it removes the only evidence they have diverged. Found by widening the sweep pattern rather than the file set. The file set was already every file this branch touches; the previous pattern searched for section symbols and "spec v", and these use neither. A sweep has two independent dimensions and a count over files says nothing about the regex — so the pattern belongs in the report beside the count. Retained deliberately: "until v0.6.20 exposed" names a real released artefact anyone can look up, which is the opposite of the problem here.
The force rekey took its freshness precondition as a parameter: the poller computed "our members view is level as of this poll" and passed it in. That made the refusal testable, but nothing stopped a caller passing whatever it liked, and the value the poller computed was correct only for as long as the call stayed inside the branch that computed it. The store now stamps each level mark with a token that changes when a poll begins for that swarm, and the rekey asks the store rather than the caller. One field serves both readers, deliberately not two: has(pubkey) were we EVER level this session — recovery stored === current token are we level AS OF THIS POLL — the force rekey Recovery is a cheap idempotent re-store, so a stale verdict costs a redundant request and the sticky reading suits it. A rekey encrypts to this device's view of the members and cannot be undone, so it needs the poll-scoped one. Two fields would drift; one value read two ways cannot. The token is minted when a poll STARTS. Taken at the end it would name the poll that just finished, so the rekey would compare a mark against its own poll and always agree — the check would typecheck, read correctly and never refuse. The poll-scoped reader fails closed: a swarm never polled, never marked, or withdrawn answers false, because its only consumer is an irreversible write and "we do not know" must not read as "yes". The sticky merge-incomplete withdrawal still deletes the entry outright, which correctly makes both readings false. Checked the writer list rather than assuming: both production writers are poll completions inside pollOnceForKey. Neither stamps a token outside a poll. V25e pins it, asserting on the rekey count rather than the return value so a refusal cannot be confused with a throw: begun-but-unmarked refuses, marked-this- poll proceeds, and a new poll makes the earlier mark stale. It also asserts the STICKY reading is still true at that last step — otherwise the test would pass if both readings had gone false together, which is the wrong reason. Still in memory. A persisted stamp would be the sticky negative this design has already ruled against once. 1051 passing / 0 failing, tsc 0.
… it blocks The 24h cooldown had no test on either side. Its reachable job is throttling retries after a rekey that threw — on success the once-per-session set refuses first, so the cooldown never gets asked. A stuck cooldown would therefore strand a group whose rekey failed, and for a full day would be indistinguishable from a working one. Three steps: repeated attempts inside the window stay at one, two hours later still one, a further twenty-three and it tries again. The middle step is what pins the interval — without it a silent regression to an hour passes.
…doing Nine comments described a state the code is not in — an old predicate name, a bound that was once looser, an assertion this test once made. A reader a year from now never saw the change, so the prose spends their attention without telling them what to preserve. Each now states the contract and what breaks if it is broken, which is the part that stops the old thing coming back. Also drops the 25 warning markers this branch added to comments. The base repo uses none in the files touched here; the emphasis was doing work a bus message needs and a source file does not.
mpretty-cyro
force-pushed
the
feature/config-recovery
branch
from
September 8, 2026 06:30
9bf26a3 to
776a14a
Compare
mpretty-cyro
marked this pull request as ready for review
September 8, 2026 06:33
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.
Config recovery: put back config messages that expired from the swarm
A config message has a 30-day TTL. A client that is offline long enough — or whose TTL refresh
never landed — comes back to a swarm that has dropped it. Today nothing notices and nothing
repairs it: the device still holds the config locally, so it looks healthy, while a new device
restoring from seed gets nothing.
This detects that and puts the message back. It rests on config encryption being deterministic:
re-storing an unchanged config reproduces its original message hash, so this is the same message
returning to where it was, not new state competing with old.
What changes
Extend-only TTL refresh. The refresh piggybacked on every poll now sets
extendrather thanleaving the flag unset. That is a fix in its own right — the previous code built a request whose
signature and wire flag could disagree — and it is also what makes the server return the
unchangedarray everything below reads.Detection. The
expiresub-response names which hashes the swarm still holds. A hash absentfrom both its arrays is missing. One responsive snode reporting absence is enough: re-storing is
idempotent, so a false positive costs one redundant request, while waiting for consensus leans on
the replication that is itself the unreliable part.
Recovery, for our own configs and for groups. Clean configs only, active hashes only, and only
once we know our local state is level with the swarm — otherwise we would re-upload a view we
already know is behind. Group members are covered, not just admins.
GroupKeysrecovery from retained bytes. libSession now keeps the raw bytes of each activekeys message, so an expired one can be pushed back verbatim. That is what makes it work for a
member: a keys message is admin-signed and padded from the group secret key, so nobody can
regenerate one — but bytes already held land on the same hash. Every retained message goes back,
not just the missing one, because a member holding part of a generation does not get the key.
A back-fill for the bytes we never captured. Retention happens when libSession loads a keys
message. A device that merged a group's keys before retention existed holds the key but not the
bytes, and cannot repair that group later. The back-fill re-fetches and re-merges so the bytes are
captured. It runs proactively rather than on detection — by the time detection fires, the message
it needed to fetch is gone.
A force rekey, as a last resort. When every keys message is gone from the swarm and nothing
here can restore it, an admin mints a new generation. This is the only irreversible, universally
visible write in the change, and most of its code is about not doing it when it was not needed.
Two changes outside the feature's own files
Both are flagged because a reviewer should decide on them separately.
StoreGroupConfigSubRequestnow accepts either credential. It threw on an emptysecretKeywhile its own
build()said it would sign with the subaccount key — so it forbade a capability itclaimed to have, and member recovery could not have worked. The signer already supported it. An
admin push that omitted its secret key previously threw and will now attempt subaccount signing.
The retrieve length check admits exactly one length. It accepted
NorN+1because theexpiresub-request is conditional. Sub-responses are paired to sub-requests by index, so aresponse missing one retrieve result passed the lower bound and shifted every later namespace onto
its neighbour's messages — with the last one handed the
expireresult as its message list. Thisis on the polling hot path: a short response now fails the poll and retries in seconds, rather than
being silently misattributed, which never self-corrects. Read from the request builder, not
observed on the wire.
What this cannot do
back-fill. That is the population the force rekey exists for.
GroupKeyscannot be re-serialised, only replayed. A device holding no bytes for a keys hashcannot repair it, and reports the group as unavailable to this device rather than as broken.
the swarm — retention is on load, not on create. Nothing here treats admins as privileged for
keys recovery.
What cannot go wrong here
No config can be lost by this change. A re-store is byte-identical, lands on the same message
hash and does not bump the seqno, so it cannot overwrite or compete with newer state — that is the
property the whole feature rests on. The only deletion is of hashes
push()itself hands back assuperseded, and only for configs whose replacement is confirmed stored.
A reviewer's instinct here is to worry about config loss. The thing actually worth scrutinising is
the force rekey, whose worst case is below.
Known gaps
No poller-level test drives the back-fill or the force rekey through a real poll. Both are
unit-tested through their own entry points; the caller's half — that the poller computes the
freshness signal correctly and calls these at the right moment — is asserted nowhere. iOS and
Android have the same gap; it is a shared limitation rather than a Desktop shortfall.
The force rekey's worst case is a member quietly losing access to a group. The rekey encrypts
the new key to this device's view of the members. Issued from a stale view, it silently excludes
anyone added since that view was last updated — no error, no failed send, and the excluded member
simply stops being able to read the group. And it fires precisely on devices whose config state is
known to be degraded, so "stale view" is the expected condition rather than the unlucky one.
What guards it: the rekey asks the recovery store whether this swarm was marked level during the
poll running now, not merely at some point since the process started. The store stamps each mark
with the poll it came from, so a mark that predates the current poll is rejected. A caller cannot
pass a value it prefers.
The residual, honestly: the marking site is still trusted. If the poller ever marks a swarm level
when it should not, this believes it. That reduces the failure class — a stale mark can no longer
authorise a rekey — rather than removing it.
The force rekey's storm guard is in memory, so a restart clears it. It bounds an admin to one
rekey per group per 24 hours per running process, not absolutely. In practice the chain is
self-limiting — the precondition requires a back-fill to have run and failed, that record is also
cleared by a restart, and after a successful rekey the back-fill finds the new keys message on the
swarm and succeeds, which blocks the rekey. The gap is the window where the just-pushed keys
message is not yet retrievable: there, a restart could produce a second rekey.
Nothing here has run against a real swarm. Everything is mocked at the network boundary. In
particular, that
extend: truemakes the storage server returnunchangedis read fromrequest_handler.cppand never observed, and the server's hash derivation is read-verified only.An end-to-end pass is worth having before shipping.
Dependencies
activeKeyMessages()needs alibsession-util change that is open but not yet merged — libsession-util#123 — and a
libsession_util_nodejsrelease that does not exist.package.jsonstill pins v0.7.4, which does not carry it.The chain is: libsession-util#123 merged → wrapper released → pin bumped. Until then the numbers below
are measured against a locally built wrapper.
Testing
1052 passing, 0 failing,
tsc0 errors.Measured against:
776a14a69libsession_util_nodejsconfig-recovery-keys-messages @ 4ed2d44(local build)libsession-util97aafbbdBoth wrapper trees clean apart from the submodule pointer.
Every behavioural rule is mutation-verified: change a guard and a named test fails.
The force rekey can be dropped by deleting one file. Verified by doing it — removing
configRecoveryForceRekey.ts, its test and its single call site returns the suite to exactly itspre-rekey number with the back-fill series still green. Nothing in the back-fill depends on it.