-
Notifications
You must be signed in to change notification settings - Fork 0
docs: reflect sender-chain counter-lease batching (whatsapp-rust#1026) #403
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 4 commits
cc6414d
3799840
7679a3b
67adef2
9c10ac6
d908bd4
722b680
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 |
|---|---|---|
|
|
@@ -1137,11 +1137,16 @@ 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, group, and status sends) flushes **synchronously, before the stanza reaches the wire**, and propagates a persistence failure by aborting the send. Reusing an outbound counter reuses its message key and IV, so the ratchet advance must be durable before anyone can act on the ciphertext — the send must not transmit an advance it couldn't save. | ||
| - **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 (`SessionRecord::reserve_sender_chain_counters`). A send covered by an unexhausted lease is already durable, so it just schedules the same coalesced write-behind as the receive path below. Only the send that exhausts the lease — roughly 1 in 64 — raises the ceiling and flushes **synchronously, before the stanza reaches the wire**, propagating a persistence failure by aborting the send. Either way, 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 (field 100 in the encoded `SessionRecord`, outside the vendored `whatsapp.proto`), and `SessionRecord::deserialize` fast-forwards the sender chain to the lease ceiling on every load, so a crash or reconnect mid-lease can never re-derive a possibly-spent counter. | ||
| - **Send (group and status sends)** is unaffected by the lease and still flushes **synchronously, before the stanza reaches the wire**, on every send — sender-key leasing is a potential follow-up, not implemented yet. | ||
| - **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. | ||
|
|
||
| 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. | ||
|
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. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Apply the documentation style rules consistently across the updated pages. The new durability explanations combine too many independent guarantees into single dense sentences and frequently use passive phrasing. Split each explanation into concise, reader-directed statements or bullets.
As per coding guidelines, MDX documentation must use active voice and second person, and keep sentences concise with one idea per sentence. 📍 Affects 3 files
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
| <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. | ||
|
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.
In the crash/downgrade scenario, the stale chain snapshot can be behind counters that were already transmitted after the durable lease was written; an older reader ignores the lease and resumes from that stale snapshot, so it can re-derive already-spent counters, not merely counters that were reserved but unsent. This warning understates the cryptographic risk and could lead operators to accept an unsafe rollback after a crash. Useful? React with 👍 / 👎. |
||
| </Warning> | ||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
|
|
||
| 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. | ||
|
|
||
| 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.
When a different session or sender-key chain has an unpersisted wire gate, such as after a backend failure while raising a lease,
Client::persist_signal_state_pre_wire()uses a globalneeds_pre_wire_flush()predicate, so this otherwise lease-covered DM send will synchronously flush and can returnErr. Saying it “just schedules” the coalescer and that only the lease-raising send flushes misdocuments an observable send failure mode that callers may need to handle.Useful? React with 👍 / 👎.