docs: reflect sender-chain counter-lease batching (whatsapp-rust#1026) - #403
Conversation
📝 WalkthroughWalkthroughUpdated Signal durability documentation to describe batched sender-chain counter leases for direct messages, synchronous persistence for lease exhaustion and group/status sends, coalesced receive flushing, persistence failures, and unsafe downgrade behavior. ChangesSignal durability documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@advanced/signal-protocol.mdx`:
- Around line 1140-1144: Rewrite the updated durability documentation at
advanced/signal-protocol.mdx:1140-1144, api/client.mdx:1466-1466, and
api/send.mdx:47-47 into concise, active-voice, second-person sentences or
bullets. At the signal protocol site, separate lease behavior, flushing timing,
encoding scope, recovery, and failure guarantees; at the client site, separate
send, receive, success, timeout, and failure behavior; at the send site,
separate lease, group/status, counter-reuse, and persistence-failure guarantees.
Preserve all existing technical guarantees while limiting each sentence to one
idea.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 49a17cb2-9d53-4e4d-86e7-c5e845af35ab
📒 Files selected for processing (3)
advanced/signal-protocol.mdxapi/client.mdxapi/send.mdx
| - **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.
📐 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.
advanced/signal-protocol.mdx#L1140-L1144: Separate lease, flushing, encoding, recovery, and failure guarantees.api/client.mdx#L1466-L1466: Separate send-path, receive-path, success, timeout, and failure behavior.api/send.mdx#L47-L47: Separate lease, group/status, counter-reuse, and persistence-failure guarantees.
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
advanced/signal-protocol.mdx#L1140-L1144(this comment)api/client.mdx#L1466-L1466api/send.mdx#L47-L47
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@advanced/signal-protocol.mdx` around lines 1140 - 1144, Rewrite the updated
durability documentation at advanced/signal-protocol.mdx:1140-1144,
api/client.mdx:1466-1466, and api/send.mdx:47-47 into concise, active-voice,
second-person sentences or bullets. At the signal protocol site, separate lease
behavior, flushing timing, encoding scope, recovery, and failure guarantees; at
the client site, separate send, receive, success, timeout, and failure behavior;
at the send site, separate lease, group/status, counter-reuse, and
persistence-failure guarantees. Preserve all existing technical guarantees while
limiting each sentence to one idea.
Source: Coding guidelines
|
| Filename | Overview |
|---|---|
| advanced/signal-protocol.mdx | Rewrote "Flush scheduling: send vs. receive" section; DM counter-lease mechanism is well-documented, but the DM bullet omits the global pre-wire flush gate and the group/status bullet uses ambiguous "status sends" that doesn't carve out status reactions (which follow the DM lease path). |
| api/client.mdx | Updated flush_pending_signal_state() description to cover DM lease batching, group/status synchronous sends, status-reaction exception, and the global pre-wire flush gate; consistent with api/send.mdx. |
| api/send.mdx | Note on send_message durability updated to correctly distinguish DM lease behavior from synchronous group/status sends, with explicit status-reaction exception; terminology is consistent with api/client.mdx. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[send_message called] --> B{Message type?}
B -->|Group send| C[Flush synchronously before wire]
B -->|Status post via client.status| C
B -->|Status reaction send_reaction| D
B -->|DM 1:1| D[Counter within current lease?]
D -->|Yes lease still valid| E[Schedule coalesced write-behind 25ms]
D -->|No lease exhausted 1 in 64| F[Reserve next 64 counters Flush synchronously before wire]
E --> G{Global pre-wire flush pending?}
G -->|Yes| H[Force synchronous flush before wire]
G -->|No| I[Transmit stanza]
F -->|Flush fails| J[Abort send return Err]
F -->|Flush succeeds| I
C -->|Flush fails| J
C -->|Flush succeeds| I
H --> I
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[send_message called] --> B{Message type?}
B -->|Group send| C[Flush synchronously before wire]
B -->|Status post via client.status| C
B -->|Status reaction send_reaction| D
B -->|DM 1:1| D[Counter within current lease?]
D -->|Yes lease still valid| E[Schedule coalesced write-behind 25ms]
D -->|No lease exhausted 1 in 64| F[Reserve next 64 counters Flush synchronously before wire]
E --> G{Global pre-wire flush pending?}
G -->|Yes| H[Force synchronous flush before wire]
G -->|No| I[Transmit stanza]
F -->|Flush fails| J[Abort send return Err]
F -->|Flush succeeds| I
C -->|Flush fails| J
C -->|Flush succeeds| I
H --> I
Reviews (4): Last reviewed commit: "docs: correct global pre-wire gate and s..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7679a3b701
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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. | ||
|
|
||
| <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.
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 👍 / 👎.
| *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. |
There was a problem hiding this comment.
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 global needs_pre_wire_flush() predicate, so this otherwise lease-covered DM send will synchronously flush and can return Err. 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 👍 / 👎.
|
|
||
| <Note> | ||
| For DMs, group, and status sends, the outbound Signal ratchet advance is persisted to the backend **synchronously, before the stanza is transmitted** — reusing an outbound counter would reuse its message key and IV, so the advance must be durable before anyone can act on the ciphertext. If that persistence write fails, `send_message` returns `Err` instead of transmitting an advance that couldn't be saved. See [Signal Protocol — flush scheduling](/advanced/signal-protocol#flush-scheduling-send-vs-receive) for the full durability model. | ||
| For DMs, the outbound Signal ratchet advance is persisted through a batched counter lease: the sender-chain counter is reserved 64 at a time, so most sends are already covered by a durable lease and only schedule a coalesced write-behind, while the send that raises the lease (roughly 1 in 64) is persisted to the backend **synchronously, before the stanza is transmitted**. Group and status sends always persist their sender-key ratchet advance synchronously before the stanza is transmitted. Either way, reusing an outbound counter would reuse its message key and IV, so the advance is durable before it can be reused, and if a required persistence write fails, `send_message` returns `Err` instead of transmitting an advance that couldn't be saved. See [Signal Protocol — flush scheduling](/advanced/signal-protocol#flush-scheduling-send-vs-receive) for the full durability model. |
There was a problem hiding this comment.
For send_reaction() calls targeting status@broadcast, the send path fans out pairwise to the status author's devices and uses the DM branch, so those sends follow the DM counter-lease behavior rather than persisting a status sender-key ratchet on every send. This note now says status sends always use synchronous sender-key persistence, which misleads callers about the latency and failure behavior of status reactions documented later on this same page.
Useful? React with 👍 / 👎.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
1 participant
Summary
whatsapp-rust#1026 ("perf(signal): lease outbound counters in batches instead of flushing every send") changed the documented Signal-protocol send-flush durability model for 1:1 (DM) sessions: instead of flushing the outbound ratchet advance synchronously on every send,
SessionRecordnow reserves its sender-chain counter in batches of 64 (SessionRecord::reserve_sender_chain_counters). Only the send that exhausts the lease (~1 in 64) still flushes synchronously before the stanza hits the wire; the rest schedule the same coalesced write-behind used by the receive path. Group and status sends are unaffected — they still flush synchronously on every send.This updates the three docs pages that documented the old "every send flushes synchronously" behavior:
advanced/signal-protocol.mdx— rewrote the "Flush scheduling: send vs. receive" section to describe the counter-lease mechanism (batch size, local-only wire field, fast-forward on load), split the DM vs. group/status send behavior into separate bullets, and added a downgrade caveat as a<Warning>.api/send.mdx— updated thesend_messagedurability note to describe the batched-lease behavior for DMs vs. the unchanged synchronous behavior for group/status sends.api/client.mdx— updated theflush_pending_signal_state()description, which explained what the method is closing the gap on, to match the new mechanics.No changelog entry included per standing instructions (changelog entries are added by a human).
Source
🤖 Generated with Claude Code
Generated by Claude Code
Summary by cubic
Update Signal Protocol durability docs to cover DM sender‑chain counter leasing (64-counter batches). Clarifies that the pre‑wire flush check is global (an unrelated pending flush can force a sync flush) and that status reactions follow the DM lease; group/status posts still flush per send.
Written for commit 722b680. Summary will update on new commits.
Summary by CodeRabbit