refactor(appstate): scan instead of HashSet for index-mac dedup in the patch path

[jlucaso1]

What

Two HashSet<&[u8]> over index_mac blobs in the patch-validation path are swapped for linear-scan Vec<&[u8]>:

1. detect_duplicate_index_in_patch (processor.rs) — the in-patch duplicate-index guard (Set/Remove deduped independently).
2. HashState::update_hash's removed_in_patch (hash.rs) — the membership set that gates the SET-also-REMOVEd double-subtract.

Why

The keys are index_mac blobs — HMAC outputs, i.e. uniformly random bytes. SipHash's distribution/DoS resistance buys nothing over a trivial byte compare; it only costs the per-key hash setup and a table allocation. This is the same trade-off the project already validated for the sibling collect_unique_index_macs in #856, where a HashSet measured 6–120% worse than a linear scan at the patch sizes seen in practice (N≈10–50). Applying the same scan keeps the three functions consistent, and uses zero new dependencies (no fast-hash crate — consistent with the recent dependency trimming, e.g. #860).

Honest measurement — this is NOT a measurable speedup

I profiled it with the CodSpeed MCP before claiming anything. CodSpeed reports no performance change across all 172 benchmarks, and a direct run-to-run compare came back at −0.05% overall, attributed to an environment difference (AVX-512 flags), not the code. In the bench_process_patch_50_validated flamegraph the dedup doesn't even surface as a ≥1% line: the 2 ms benchmark is crypto-bound (SHA-256 compress256 66%, lthash HKDF 59%, decode_record 32%, MAC validation). Deduping ~50 random 32-byte MACs is microscopic next to ~15 SHA compressions.

So this is reframed from perf to refactor: the concrete, measured win is binary size — the prior binary-size report showed wacore_appstate .text dropping −1.1 KiB (−3.03%) from dropping the monomorphized HashSet/SipHash machinery — plus one fewer allocation per patch and simpler code. No latency claim attached.

Deliberately left as-is

• in_patch value-lookup HashMap (processor.rs) — an intentional O(1) map replacing an O(n²) reverse scan; converting it would regress.
• collect_key_ids_from_patch_list's seen (decode.rs) — dedups key IDs (not MACs), tiny N, off the hot path.

Correctness

Behaviour is byte-for-byte identical. Set and Remove stay deduped independently, and removed_in_patch is only queried via .contains(), so unconditional push is membership-equivalent. Existing tests unchanged and cover both paths: process_patch_rejects_duplicate_set_index, process_patch_allows_same_index_across_set_and_remove, plus the update_hash index-mode tests.

Verification

• cargo fmt --all — clean (run locally)
• clippy + tests + CodSpeed run in CI

https://claude.ai/code/session_01XHsbPwjaCRDHDL69HbEgR8

[coderabbitai]

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: fcc92c08-e0d0-45a9-bcf4-75adc92aee84

📥 Commits

Reviewing files that changed from the base of the PR and between 826882b and 41fae2c.

📒 Files selected for processing (2)

• wacore/appstate/src/hash.rs
• wacore/appstate/src/processor.rs

---

📝 Walkthrough

Summary by CodeRabbit

• Refactor
  • Optimized internal state tracking and mutation processing mechanisms.

Walkthrough

Two appstate patch-processing functions replace HashSet<&[u8]> containers with Vec<&[u8]> for small per-patch index tracking: removed_in_patch in HashState::update_hash and seen_set/seen_remove in detect_duplicate_index_in_patch. The HashSet import is removed from processor.rs. Error and suppression behaviors are unchanged.

Changes

AppState Patch Index Tracking: HashSet → Vec

Layer / File(s)	Summary
removed_in_patch Vec in update_hash wacore/appstate/src/hash.rs	removed_in_patch changes from HashSet<&[u8]> to Vec<&[u8]>. REMOVE mutations append into the Vec, and the SET suppression check uses removed_in_patch.contains(&index_mac) against the new type.
detect_duplicate_index_in_patch Vec refactor wacore/appstate/src/processor.rs	HashSet import removed. seen_set and seen_remove switch from HashSet to Vec<&[u8]> with explicit contains + push membership logic. Inline documentation updated to reflect the trade-off. AppStateError::DuplicateIndexInPatch behavior unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• oxidezap/whatsapp-rust#752: Directly connected — adds duplicate-index detection via per-operation index_mac sets in processor.rs, which this PR refactors from HashSet to Vec membership checks.
• oxidezap/whatsapp-rust#829: Modifies the same HashState::update_hash index-mode removed-index tracking and SET suppression logic that this PR refactors.
• oxidezap/whatsapp-rust#239: Touches HashState::update_hash REMOVE mutation handling and previous-value logic, overlapping with the same code path changed here.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and concisely describes the main change: replacing HashSet with linear scanning for index-MAC deduplication in the patch validation path.
Description check	✅ Passed	The description comprehensively explains what changed, why it matters (correctness and binary size), and includes honest performance data showing no latency impact while reducing binary size by 3.03%.
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 claude/whatsapp-rust-pr-review-mz0gyy

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[cubic-dev-ai]

No issues found across 1 file

_{Re-trigger cubic}

[github-actions]

📦 Binary size report

Metric	main	PR	Δ
bin size (stripped)	10.70 MiB	10.70 MiB	-2.50 KiB (-0.02%) 🔽
bin .text	8.79 MiB	8.79 MiB	-2.44 KiB (-0.03%) 🔽
bin allocated (text+data+bss)	10.70 MiB	10.70 MiB	-3.97 KiB (-0.04%) 🔽
llvm-lines wacore	654,355	654,355	0
llvm-lines wacore copies	17,627	17,627	0
llvm-lines whatsapp-rust lib	645,497	645,001	-496 (-0.08%) 🔽
llvm-lines whatsapp-rust lib copies	19,642	19,623	-19 (-0.10%) 🔽
deps crates (Cargo.lock)	354	354	0

.text per crate

Crate	main	PR	Δ
.text whatsapp_rust	1.51 MiB	1.51 MiB	+5 B (+0.00%) 🔺
.text wacore	553.25 KiB	551.94 KiB	-1.30 KiB (-0.24%) 🔽
.text wacore_binary	103.64 KiB	103.64 KiB	0
.text wacore_libsignal	168.32 KiB	168.32 KiB	0
.text wacore_appstate	36.36 KiB	35.26 KiB	-1.10 KiB (-3.03%) 🎉
.text wacore_noise	30.68 KiB	30.68 KiB	0
.text waproto	971.41 KiB	971.41 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.12 MiB	1.12 MiB	0
.text other deps	4.03 MiB	4.03 MiB	0

Top movers (cargo-bloat attribution)

Crate	main	PR	Δ
regex_automata	2.03 KiB	4.16 KiB	+2.13 KiB (+104.86%)
prost	479.94 KiB	477.81 KiB	-2.13 KiB (-0.44%)
wacore	553.25 KiB	551.94 KiB	-1.30 KiB (-0.24%)
wacore_appstate	36.36 KiB	35.26 KiB	-1.10 KiB (-3.03%)

Baseline: 826882b68 (latest main run) · Head: 938d7f87e · Graphs

[codspeed-hq]

Merging this PR will not alter performance

✅ 172 untouched benchmarks

---

_{Comparing claude/whatsapp-rust-pr-review-mz0gyy (41fae2c) with main (826882b)}