-
Notifications
You must be signed in to change notification settings - Fork 0
docs: reflect retry pre-wire Signal persistence (whatsapp-rust#1041) #405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
da30551
232ad0d
501c223
1935c11
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1138,16 +1138,18 @@ impl SessionStore for SessionAdapter { | |
| *When* the dirty Signal cache reaches the backend differs by direction, because the two directions have different recovery properties: | ||
|
|
||
| - **Send (DM/1:1 sessions)** persists through a batched **counter lease**. `SessionRecord` reserves its outbound sender-chain counter `SENDER_CHAIN_RESERVATION_BATCH` (64) values at a time, via `SessionRecord::reserve_sender_chain_counters`. A send covered by an unexhausted lease is already durable — it only schedules the same coalesced write-behind as the receive path below. The send that exhausts the lease, roughly 1 in 64, raises the ceiling and flushes **synchronously, before the stanza reaches the wire**. If that flush fails, the send aborts instead of transmitting an advance it couldn't save. Reusing an outbound counter reuses its message key and IV, so no counter can ever be used before its lease is durable. The lease field is local-only: it's field 100 in the encoded `SessionRecord`, outside the vendored `whatsapp.proto`. On every load, `SessionRecord::deserialize` fast-forwards the sender chain to the lease ceiling, so a crash or reconnect mid-lease can never re-derive a possibly-spent counter. | ||
| - **Send (group and status sends)** ignores the lease. It still flushes **synchronously, before the stanza reaches the wire**, on every send. Sender-key leasing is a potential follow-up, not implemented yet. | ||
| - **Send (group and status sends)** persists through the same batched-lease pattern as DM sends: `SenderKeyRecord` reserves its outbound chain iteration in batches of `SENDER_CHAIN_RESERVATION_BATCH` (64), via `SenderKeyRecord::reserve_iterations` (mirroring `SessionRecord::reserve_sender_chain_counters`). A send covered by an unexhausted lease only schedules the coalesced write-behind; the send that exhausts the lease raises the ceiling and flushes **synchronously, before the stanza reaches the wire**, and a failed flush aborts the send instead of transmitting an advance it couldn't save. | ||
| - **Receive** (live traffic, outside the offline-drain batcher) routes through a single-flight coalescing scheduler (`src/signal_flush.rs`) instead of flushing per stanza: a burst of receives folds into one flush per ~25ms window, retried with exponential backoff (up to a 5s cap) on backend failure. This is safe because a lost receive-side advance simply re-derives forward on the next message (the receiving chain derives `CK_n → CK_n+1`), and a consumed one-time prekey stays buffered until its session is durable — a crash inside the window is recoverable. | ||
|
|
||
| <Warning> | ||
| Downgrading to a version that predates the counter lease after running a leased version: the older version ignores the lease field and could reuse counters that were only reserved (not yet actually sent) by the lease. Avoid downgrading a device's local state across this boundary. | ||
| Downgrading to a version that predates the counter lease after running a leased version: the older version ignores the lease fields on `SessionRecord` and `SenderKeyRecord` and could reuse counters/iterations that were only reserved (not yet actually sent) by the lease. Avoid downgrading a device's local state across this boundary. | ||
| </Warning> | ||
|
|
||
| The scheduler is generation-scoped (embeds the connection generation in its atomic state), so a reconnect during an in-flight flush needs no explicit reset: a stale worker from the previous connection cannot mutate the new generation's state, and stands down when it observes a foreign generation. | ||
|
|
||
| The offline drain, retry-receipt recovery, identity-change recovery, and teardown all keep their own **synchronous** flushes — they gate acks, receipts, or follow-up reads on durability and are not routed through the receive coalescer. See [Inbound Durability Hook](/advanced/inbound-durability) for the drain-batch commit ordering, which this coalescing does not change. | ||
| The offline drain, identity-change recovery, and teardown all keep their own **unconditional synchronous** flushes — they gate acks, receipts, or follow-up reads on durability and are not routed through the receive coalescer. See [Inbound Durability Hook](/advanced/inbound-durability) for the drain-batch commit ordering, which this coalescing does not change. | ||
|
|
||
| Retry-receipt recovery (`handle_retry_receipt` resending to a DM or group requester, `src/retry.rs`) instead shares the DM/group send durability rule above through one helper, `send_retry_stanza`: the session lock is released first, then `persist_signal_state_pre_wire()` flushes synchronously only if the retry's Signal advance crossed an unpersisted lease boundary, and the stanza is written to the wire only after that flush succeeds. A failed pre-wire flush aborts the retry instead of transmitting an advance it couldn't save. This replaced an earlier unconditional full flush that ran only *after* the retry stanza had already reached the wire — a crash or persistence failure in that window could reload the old chain state after the ciphertext was already sent. | ||
|
greptile-apps[bot] marked this conversation as resolved.
Outdated
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I checked the merged Useful? React with 👍 / 👎. |
||
|
|
||
| Call [`Client::flush_pending_signal_state()`](/api/client#flush_pending_signal_state) to force a deterministic settle — e.g. before reading persisted Signal state directly, or ahead of a non-graceful shutdown. Never call it from inside an `InboundDurabilityHook` or a synchronous, inline `EventHandler::handle_event` implementation, since settling re-enters the processing permit those run under and would deadlock during an offline-sync drain. Ordinary `Bot` closure handlers are unaffected — both default delivery modes run the callback in a detached task off the permit. | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For teardown during an active or stalled offline drain,
teardown_inbound_commits_boundeddoes not unconditionally flush: it only flushes after a durable drain commit with no restored entries, and on timeout or non-durable entries it clears or preserves state so the server can redeliver. The newunconditional synchronouswording can make operators think disconnect/teardown always gates Signal state on a flush, which is exactly the case the teardown path intentionally avoids to prevent rowless ratchet advances.Useful? React with 👍 / 👎.