Skip to content

fix(send): gate SKDM redistribution on the primary device (WA Web parity) - #872

Merged
jlucaso1 merged 1 commit into
mainfrom
fix/skdm-primary-device-gate
Jun 15, 2026
Merged

jlucaso1 merged 1 commit into
mainfrom
fix/skdm-primary-device-gate

Conversation

@jlucaso1

@jlucaso1 jlucaso1 commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Problem

During a retry storm (mass PN->LID migration), each retrying device is marked has_key=false individually, but the SKDM filter redistributed the sender key to ALL of that user's devices via a per-user is_user_forgotten clause, including devices that already hold the key. That is extra outbound SKDM volume during exactly the storms that risk AccountLocked, and it diverges from WhatsApp Web.

Fix

Match WA Web's getGroupSenderKeyListFromParticipantRecord (ParticipantStore.js): a device is warm (skip SKDM) only when both it AND its primary device (device 0) hold the key. The filter predicate becomes "needs SKDM iff the device is cold OR its primary is cold", replacing the per-user is_user_forgotten clause with a primary-device gate.

Strictly more correct than both the old behavior and a naive per-device check:

  • Old (is_user_forgotten): a single cold companion redistributed the whole user. Broader than WA Web.
  • Naive per-device: would skip a warm companion when the primary is cold. Narrower than WA Web (drops a needed SKDM).
  • Primary-gate (this PR): a cold companion redistributes only itself; a cold primary redistributes the whole user. Exact WA Web parity.

The now-dead forgotten_users set and is_user_forgotten method are removed.

Tests

  • filter_skdm_targets_uses_primary_device_gate drives the real filter across the three distinguishing cases (cold companion + warm primary => only companion; cold primary + warm companion => whole user; warm companion + absent primary => companion). It fails for both the old over-broad and the naive under-broad behaviors.
  • Existing SKDM map tests updated to the primary-gate predicate.

Verify

  • cargo fmt --all, cargo clippy --all-targets -- -D warnings clean.
  • cargo test -p whatsapp-rust --lib (827 tests) pass.

Notes (pre-existing, out of scope)

  • SenderKeyDeviceMap indexes by user (not server+user). Safe in practice (PN and LID user-id spaces are disjoint, and a single send resolves one namespace).
  • Identity / raw-id clears delete peer sessions but not sender_key_devices rows. The retry path self-heals (a re-added device fails, gets marked cold, next send redistributes). Worth a separate look.

Review in cubic

…ity)

During a retry storm (mass PN->LID migration) each retrying device is marked has_key=false individually, but the SKDM filter redistributed the sender key to ALL of that user's devices via a per-user is_user_forgotten clause, including devices that already hold the key. That is extra outbound SKDM volume during the storms that risk AccountLocked, and it diverges from WhatsApp Web.

Match WA Web's getGroupSenderKeyListFromParticipantRecord (ParticipantStore.js): a device is warm (skip SKDM) only when both it AND its primary device (device 0) hold the key. The filter predicate becomes "needs SKDM iff the device is cold OR its primary is cold", replacing is_user_forgotten with a primary-device gate. A cold companion now redistributes only itself; a cold primary redistributes the whole user. This is exact WA Web parity, unlike the old over-broad behavior or a naive per-device check (which would drop an SKDM to a warm companion when the primary is cold).

The now-dead forgotten_users set and is_user_forgotten method are removed.
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 227b588e-6b19-4a50-992b-48f1be266e2b

📥 Commits

Reviewing files that changed from the base of the PR and between b23a9f7 and d67ee63.

📒 Files selected for processing (2)
  • src/send.rs
  • src/sender_key_device_cache.rs

📝 Walkthrough

Summary by CodeRabbit

  • Refactor
    • Updated sender key distribution recipient-device filtering to use a primary-device gate mechanism for determining companion device inclusion.
    • Simplified device key cache to track only per-user device key presence, removing unused user-level bookkeeping.

Walkthrough

SenderKeyDeviceMap drops the forgotten_users field and is_user_forgotten method. filter_skdm_targets in send.rs replaces the prior forgotten-user check with a primary-device gate: a companion device is warm only when both it and its user's device 0 have the sender key recorded. Tests are updated and a new dedicated gate test is added.

Changes

SKDM Primary-Device Gate Refactor

Layer / File(s) Summary
SenderKeyDeviceMap: remove forgotten_users and is_user_forgotten
src/sender_key_device_cache.rs
Drops the forgotten_users: HashSet<Arc<str>> field, removes the HashSet import, simplifies from_db_rows to only populate the devices map, and deletes the is_user_forgotten public accessor.
filter_skdm_targets: primary-device gate
src/send.rs
Removes the is_user_forgotten-based condition; a companion device is now warm only when both the companion's own key entry and the same user's device-0 key entry are present, otherwise it is included for SKDM redistribution.
Updated and new SKDM filter tests
src/send.rs
empty_sender_key_device_map_marks_all_devices_for_skdm and single_forgotten_row_keeps_full_distribution are updated for new gate expectations; new filter_skdm_targets_uses_primary_device_gate test covers companion-forgotten vs primary-warm, primary-forgotten vs companion-warm, and absent-primary-treated-as-cold.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • oxidezap/whatsapp-rust#445: Introduced the SenderKeyDeviceMap cache and is_user_forgotten APIs that this PR directly removes and replaces.
  • oxidezap/whatsapp-rust#603: Modified SKDM target resolution in src/send.rs using is_user_forgotten semantics that this PR supersedes with primary-device gating.
  • oxidezap/whatsapp-rust#678: Also changes SKDM recipient-device selection logic derived from the sender-key cache state in src/send.rs.

Suggested labels

breaking-change

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main change: gating SKDM redistribution on the primary device and achieving WhatsApp Web parity. It accurately reflects the core objective of the PR.
Description check ✅ Passed The description comprehensively explains the problem, solution, testing approach, and verification steps. It clearly relates to the changeset and provides proper context for understanding the fix.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/skdm-primary-device-gate

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codspeed

codspeed Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 172 untouched benchmarks


Comparing fix/skdm-primary-device-gate (d67ee63) with main (b23a9f7)

Open in CodSpeed

@github-actions

Copy link
Copy Markdown

📦 Binary size report

Metric main PR Δ
bin size (stripped) 10.59 MiB 10.59 MiB -1.31 KiB (-0.01%) 🔽
bin .text 8.68 MiB 8.68 MiB -1.31 KiB (-0.01%) 🔽
bin allocated (text+data+bss) 10.59 MiB 10.58 MiB -4.00 KiB (-0.04%) 🔽
llvm-lines wacore 644,384 644,384 0
llvm-lines wacore copies 17,673 17,673 0
llvm-lines whatsapp-rust lib 657,128 656,564 -564 (-0.09%) 🔽
llvm-lines whatsapp-rust lib copies 20,013 19,997 -16 (-0.08%) 🔽
deps crates (Cargo.lock) 354 354 0
.text per crate
Crate main PR Δ
.text whatsapp_rust 1.48 MiB 1.48 MiB -1.04 KiB (-0.07%) 🔽
.text wacore 543.77 KiB 543.77 KiB 0
.text wacore_binary 103.64 KiB 103.64 KiB 0
.text wacore_libsignal 168.32 KiB 168.32 KiB 0
.text wacore_appstate 35.26 KiB 35.26 KiB 0
.text wacore_noise 30.68 KiB 30.68 KiB 0
.text waproto 895.34 KiB 895.34 KiB 0
.text whatsapp_rust_sqlite_storage 206.21 KiB 206.21 KiB 0
.text whatsapp_rust_tokio_transport 33.09 KiB 33.09 KiB 0
.text whatsapp_rust_ureq_http_client 6.19 KiB 6.19 KiB 0
.text std 1.13 MiB 1.13 MiB -295 B (-0.02%) 🔽
.text other deps 4.02 MiB 4.02 MiB 0
Top movers (cargo-bloat attribution)
Crate main PR Δ
whatsapp_rust 1.48 MiB 1.48 MiB -1.04 KiB (-0.07%)

Baseline: b23a9f7f3 (latest main run) · Head: dec8939bd · Graphs

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Re-trigger cubic

@jlucaso1
jlucaso1 merged commit 09e3d97 into main Jun 15, 2026
18 of 20 checks passed
@jlucaso1
jlucaso1 deleted the fix/skdm-primary-device-gate branch June 15, 2026 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant