-
Notifications
You must be signed in to change notification settings - Fork 0
docs: document inline phash checks and ack-worker memory fields (whatsapp-rust#1116) #441
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 1 commit
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 |
|---|---|---|
|
|
@@ -861,10 +861,10 @@ When sending group, status, or DM messages, the library validates the participan | |
| **How it works:** | ||
|
|
||
| 1. Before sending, the client obtains the locally computed `phash` — from the stanza `phash` attribute for group/status messages, or from `PreparedDmStanza.phash` for DMs | ||
| 2. A oneshot ack waiter is registered for the message ID via `register_ack_waiter` | ||
| 2. A `PhashWaiter` (expected hash, target JID, whether to also invalidate the group cache) is registered for the message ID via `register_phash_waiter` — a map entry, not a channel or a task | ||
| 3. The message stanza is sent to the server | ||
| 4. A background task (`spawn_phash_validation`) awaits the server's ack (with a 10-second timeout) | ||
| 5. The server's ack includes its own `phash` — if it differs from the local value, the client invalidates caches | ||
| 4. When the server's ack for that message ID arrives, the read loop compares its `phash` attribute against the expected value inline, with no task involved | ||
| 5. On a match the entry is just dropped; on a mismatch the client spawns a task to invalidate caches — so a send only pays for a task in the uncommon case, not on every send ([#1116](https://github.com/oxidezap/whatsapp-rust/pull/1116)) | ||
|
|
||
| **On mismatch, the following caches are invalidated:** | ||
|
|
||
|
|
@@ -895,7 +895,7 @@ if !jid.is_group() && !jid.is_status_broadcast() { | |
| ``` | ||
|
|
||
| <Note> | ||
| The phash validation runs asynchronously in the background and does not block the send path. If the server ack times out (after 10 seconds) or the oneshot channel is dropped, the validation is silently skipped. This matches WhatsApp Web's approach of using phash as a best-effort staleness detector rather than a hard requirement. | ||
| The phash check never blocks the send path. If the server's ack never arrives, nothing polls the waiter directly — it is swept out on the keepalive tick, which drops any phash waiter that has lived through a full sweep since it registered (a window of one keepalive tick, 15–30 seconds, rather than the old fixed 10-second timeout). The sweep runs before keepalive's own idle early-return, so a connection with steady inbound traffic — which skips sending pings — still gets its stale waiters cleared; a stranded waiter would otherwise read as an outstanding IQ and suppress pings for the life of the connection. This matches WhatsApp Web's approach of using phash as a best-effort staleness detector rather than a hard requirement. | ||
|
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.
The revised note combines waiter cleanup timing, sweep ordering, ping suppression, and WhatsApp Web parity into several long compound sentences. Split these concepts into shorter sentences so readers can distinguish the cleanup behavior from its rationale, as required by the repository's documentation style. AGENTS.md reference: AGENTS.md:L24-L25 Useful? React with 👍 / 👎. 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.
A waiter can be registered at any point during an already-running 15–30-second keepalive interval, so the time until the next tick is not itself bounded to 15–30 seconds and may be nearly zero. If the waiter must survive one complete sweep interval before removal, cleanup instead occurs on a later tick and can span portions of two intervals. Replace the stated 15–30-second lifetime with the actual bounds or describe the sweep semantics without claiming this range. Useful? React with 👍 / 👎. 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.
This change says phash waiters are no longer polled through a fixed timeout, but Useful? React with 👍 / 👎. 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.
The new text says the phash sweep runs before keepalive's idle early-return, while Useful? React with 👍 / 👎. |
||
| </Note> | ||
|
|
||
| #### WA Web phash parity (v0.6) | ||
|
|
||
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.
This step now says the comparison runs in the read loop, but the section's unchanged
Locationline still names onlysrc/send.rsandsrc/client.rs. The commit itself identifiessrc/client/node_io.rsandsrc/keepalive.rsas the files implementing the inline check and waiter sweep, so readers following the documented source locations will miss the behavior described here.Useful? React with 👍 / 👎.