Skip to content

fix(signal): retain durability gates through deletes - #1042

Merged
jlucaso1 merged 1 commit into
mainfrom
fix/signal-tombstone-durability-gates
Jul 15, 2026
Merged

fix(signal): retain durability gates through deletes#1042
jlucaso1 merged 1 commit into
mainfrom
fix/signal-tombstone-durability-gates

Conversation

@jlucaso1

Copy link
Copy Markdown
Collaborator

Summary

  • Keep session reservation gates attached when an entry becomes a tombstone.
  • Do the same for sender-key wire gates.
  • Release each gate only after its matching backend delete succeeds.
  • Cover failed-delete retries for both stores with deterministic barrier tests.

Why

A concurrent invalidation could previously remove a durability gate before its delete reached storage. That allowed ciphertext onto the wire while the old chain state was still durable; after a crash, the old state could be loaded and outbound key material derived again.

The fix reuses the existing pending sets. It adds no cache structure or steady-state allocation, and a failed delete remains gated until a later flush makes the tombstone durable.

Closes #1036.

Validation

  • cargo fmt --all
  • cargo clippy --all --tests
  • cargo test -p wacore pre_wire_gate_tests — 9 passed
  • cargo test -p wacore — unit, integration, and doc tests passed
  • cargo test --workspace --exclude e2e-tests — passed
  • cargo test --all reached the E2E suite; its local mock server was not running, so those tests ended with the expected connection-refused timeout

@coderabbitai

coderabbitai Bot commented Jul 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 (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 83bbe814-d85f-477c-9df5-0072ef80d979

📥 Commits

Reviewing files that changed from the base of the PR and between a6e701e and 0f349fd.

📒 Files selected for processing (1)
  • wacore/src/store/signal_cache.rs

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved message protection during session and sender-key deletion.
    • Ensured deleted security data remains blocked from use until removal is confirmed durable.
    • Added safeguards for failed or delayed deletion operations to prevent premature gate release.

Walkthrough

The Signal cache now retains session and sender-key pre-wire gates while tombstones await durable deletion. Flush clears each gate only after successful backend deletion, with barrier-based tests covering failed, successful, and lossy-clear scenarios.

Changes

Tombstone durability gates

Layer / File(s) Summary
Preserve and release tombstone gates
wacore/src/store/signal_cache.rs
Session and sender-key deletion retains pending gates until flush confirms successful backend deletion, then clears the corresponding markers.
Validate gated tombstone flows
wacore/src/store/signal_cache.rs
Barrier-controlled backend tests verify failed deletes remain gated, successful deletes release gates, and clear() drops a pending tombstone gate.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Suggested reviewers: greptile-apps

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: keeping Signal durability gates attached through deletes.
Description check ✅ Passed The description is directly about the cache gate and tombstone durability fix in both stores.
Linked Issues check ✅ Passed The changes satisfy #1036 by retaining gates until successful backend deletes and adding barrier tests for both stores.
Out of Scope Changes check ✅ Passed The added test infrastructure and cache logic stay within the tombstone durability-gate fix scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/signal-tombstone-durability-gates

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.

@greptile-apps

greptile-apps Bot commented Jul 15, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a durability race where deleting a session or sender-key record would prematurely release the pre-wire flush gate even though the backend delete had not yet completed. On a crash after the gate was dropped but before the delete reached storage, the old chain state could be reloaded and outbound key material re-derived.

  • Session store: removes reservation_pending.remove(&addr) from SessionStoreState::delete and adds it instead to the flush path, immediately after backend.delete_session(address).await? succeeds.
  • Sender-key store: removes wire_gate_pending.remove(&addr) from SenderKeyStoreState::delete and adds it inside the flush loop's Some(None) arm, after backend.delete_sender_key(name).await? succeeds.
  • Tests: adds a DeleteBarrierBackend helper with cyclic barriers and a fail_delete atomic flag, then two new deterministic tests that verify the gate persists through a failed delete and lifts only on the subsequent successful retry.

Confidence Score: 5/5

Safe to merge — the production change is four lines confined to well-understood hot-path state transitions, and the new tests exercise both the failure and retry paths deterministically.

The production diff removes two premature gate-release calls and re-inserts them in the only code path that can confirm backend durability. Both are guarded by an existing ? early-return, so a failed delete can never silently clear the gate. The clear() path continues to drop the gate, which is correct because the transport has already been torn down at that point.

No files require special attention.

Important Files Changed

Filename Overview
wacore/src/store/signal_cache.rs Two-line removal + two-line addition moves gate release from the in-memory delete path to after a confirmed backend delete; new barrier-based tests cover failed-delete retry for both stores.

Reviews (1): Last reviewed commit: "fix(signal): retain durability gates thr..." | Re-trigger Greptile

@github-actions

Copy link
Copy Markdown

📦 Binary size report

Metric main PR Δ
bin size (stripped) 10.89 MiB 10.89 MiB -1.62 KiB (-0.01%) 🔽
bin .text 8.88 MiB 8.88 MiB -1.62 KiB (-0.02%) 🔽
bin allocated (text+data+bss) 10.89 MiB 10.88 MiB -4.00 KiB (-0.04%) 🔽
llvm-lines wacore 508,537 508,533 -4 (-0.00%) 🔽
llvm-lines wacore copies 17,438 17,438 0
llvm-lines whatsapp-rust lib 774,379 774,394 +15 (+0.00%) 🔺
llvm-lines whatsapp-rust lib copies 25,193 25,193 0
deps crates (Cargo.lock) 472 472 0
.text per crate
Crate main PR Δ
.text whatsapp_rust 1.67 MiB 1.67 MiB 0
.text wacore 530.46 KiB 528.84 KiB -1.61 KiB (-0.30%) 🔽
.text wacore_binary 148.45 KiB 148.45 KiB 0
.text wacore_libsignal 195.25 KiB 195.25 KiB 0
.text wacore_appstate 158.25 KiB 158.25 KiB 0
.text wacore_noise 26.03 KiB 26.03 KiB 0
.text waproto 1.60 MiB 1.60 MiB 0
.text whatsapp_rust_sqlite_storage 513.08 KiB 513.08 KiB 0
.text whatsapp_rust_tokio_transport 43.69 KiB 43.69 KiB 0
.text whatsapp_rust_ureq_http_client 10.47 KiB 10.47 KiB 0
.text std 1.01 MiB 1.01 MiB 0
.text other deps 2.95 MiB 2.95 MiB 0
Top movers (cargo-bloat attribution)
Crate main PR Δ
wacore 530.46 KiB 528.84 KiB -1.61 KiB (-0.30%)

Baseline: a6e701e31 (latest main run) · Head: e528a0924 · Graphs

@jlucaso1
jlucaso1 merged commit 042367e into main Jul 15, 2026
22 of 23 checks passed
@jlucaso1
jlucaso1 deleted the fix/signal-tombstone-durability-gates branch July 15, 2026 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keep outbound durability gates attached to Signal tombstones

1 participant