refactor(platform-wallet-storage)!: delete removed identities instead of tombstoning

[Claudius-Maginificent]

TL;DR: Removing an identity from a wallet now actually deletes it and everything tied to it, instead of just hiding it while its keys, contacts, and metadata sit in the database forever.

User story

As a developer integrating this SDK (DET, iOS/Android), I want removing an identity from a wallet to fully clean up its data, so that a database file doesn't accumulate orphaned keys, contacts, and metadata for identities the user already removed.

Scenario

Base flow

A host app calls remove_identity on a wallet-owned or out-of-wallet identity — a deliberate, user-facing action (not an internal sync artifact).

Actual behavior

Today, removal only flips a tombstoned flag on the identities row. The row, its public keys (identity_keys), contacts, token balances, DashPay profile, and per-identity metadata all stay on disk indefinitely — cleaned up only if the entire wallet is later deleted. In-memory, the removed identity's keys/contacts are already gone (ManagedIdentity is dropped in full), so storage and memory silently disagree about what "removed" means, and a re-added identity used to inherit its old on-disk keys back without anyone asking for that.

Expected behavior

Removal issues a real DELETE FROM identities. Existing foreign-key cascades clean up token_balances, dashpay_profiles, and dashpay_payments_overlay immediately. A new trigger closes the one place cascades don't reach — SQLite's MATCH SIMPLE skips FK enforcement entirely for out-of-wallet (wallet_id IS NULL) rows — by explicitly brooming identity_keys, contacts, and ignored_senders by identity_id. A re-added identity now starts from zero, matching what already happens in memory.

Detailed discussion

What was done

• New migration V015__identity_hard_delete.rs: adds cascade_children_on_identity_delete (broom trigger for the FK-dormant/FK-less children), two new indexes (idx_contacts_owner, idx_ignored_senders_owner) so the broom isn't a full scan, an explicit backfill purge of every already-tombstoned identity's dependents, then drops every already-tombstoned row and the tombstoned column itself.
• identities::apply split into apply_upserts (unchanged position) and apply_removals, moved to run after every identity-scoped child writer in the same transaction — a merged changeset can legitimately carry both a removal and a child-row upsert for the same identity in one flush, and running the DELETE first would hit a live FK violation against a just-deleted parent.
• LoadCtx's TombstonedIdentityOrphan tolerance site is rebased to MissingIdentityOwner: with no tombstone flag left to discriminate on, LoadPolicy::Recovery now forgives any missing identity owner (still counted and logged, never silent); LoadPolicy::Strict still aborts.
• Docs (SCHEMA.md, rustdoc) updated in the same commits to state the new contract, including why the trigger exists (the MATCH SIMPLE dormancy) rather than leaving the old "retained for cascade integrity" rationale stale.
• Stacked on feat(platform-wallet-storage): embeddable SQLite persistence backend with seedless rehydration #3968: this branch also carries feat(platform-wallet-storage): embeddable SQLite persistence backend with seedless rehydration #3968's own review-triage fixes (identity-index load-path cross-check, orphaned-key re-scoping on wallet promotion, Recovery-mode txid handling, a NULL-scope trigger tightening, public_key_hash cross-check, foreign_key_check on wallet open) since it forked from and was reconciled against that branch's latest state.

Consumer-facing impact

Surveyed every FFI/Swift/Kotlin/JNI call site: identities.tombstoned has zero consumer-facing exposure — every other "tombstone" hit in those layers is an unrelated contact-profile concept. remove_identity has exactly one production call site (the FFI-exposed manager method); no host surface offers "show removed identities" or "undo remove". No consumer contract is broken by this change.

Testing

• -p platform-wallet-storage: 784 tests passing (0 failed), both default and shielded feature configs.
• -p platform-wallet: 783 tests passing (0 failed).
• clippy --lib --all-features -D warnings and clippy on every touched test target: clean.
• New RED-first suite (tests/sqlite_identity_hard_delete.rs, 8 tests) proven failing before implementation, each for its own distinct reason — including one that reproduced the out-of-wallet FK-dormancy defect end-to-end through the public load() API (OrphanedIdentityEntry), not just in isolation.
• Existing tombstone-era tests rewritten with equivalent assertions against the new shape, not deleted (sqlite_qa_identity_tombstone.rs → sqlite_qa_identity_removal.rs, plus updates across sqlite_identity_index_uniqueness.rs, sqlite_identity_index_concurrency.rs, sqlite_contacts_keys_rehydration.rs, sqlite_compile_time.rs, sqlite_schema_pinning.rs).
• One pre-existing, unrelated intermittent noted for visibility: sqlite_restore_cross_process_exclusion::restore_excludes_peer_creating_missing_destination failed once under full-suite parallel contention, passed reliably in isolation and in a clean re-run — reads as a timing-sensitive cross-process SQLite-locking flake, not something this diff touches.
• Three pre-existing clippy failures on the base branch (fixture_gen.rs type-complexity, manager/mod.rs items-after-test-module, a feature-gated test file's unused imports under default features) confirmed present at the fork point via a throwaway baseline worktree and left untouched — candidates for a separate follow-up, not blockers here.

Breaking changes

This is a schema and behavioral break, hence the !:

• The identities.tombstoned column is dropped by V015; any code reading it directly (outside this crate) breaks.
• remove_identity is now genuinely terminal — a removed identity's keys, contacts, token balances, and DashPay data are gone, not recoverable by re-adding the same identity. (Previously this was silently possible via upsert-driven "resurrection," which no consumer relied on per the survey above, and which already contradicted in-memory behavior.)
• LoadPolicy::Recovery now tolerates any missing identity owner, not only a previously-tombstoned one — a deliberate widening, still counted/logged rather than silent.

Checklist

• Self-review performed
• Hard-to-understand areas commented (migration rationale, trigger dormancy explanation, transaction-ordering fix)
• Unit/integration tests added/updated
• Title carries ! and breaking changes described above
• Documentation updated (SCHEMA.md, rustdoc)

Prior work

Stacked on #3968 (feat/platform-wallet-storage-rehydration) — this PR's base branch; depends on it merging first.

Attribution

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

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.}

[github-actions]

📖 Book Preview built successfully.

Download the preview from the workflow artifacts.
To view locally: download the artifact, unzip, and open index.html.

Updated at 2026-08-27T11:40:12.318Z