feat(platform-wallet-storage): embeddable SQLite persistence backend with seedless rehydration

[Claudius-Maginificent]

Why this PR exists

• Problem: platform-wallet defines the persistence trait (PlatformWalletPersistence) and the manager-side load_from_persistor() entry point, but ships no production storage backend. Without one, a wallet's Platform state — identities, contacts, identity keys, tracked asset locks, balances, and core sync watermarks — has nowhere durable to live.
• What breaks without it: Restart the node/app and its Platform identities and contacts are gone until re-derived, the address-reuse guard resets, and core sync restarts from scratch — there is no on-disk durability, no backup/restore, and no schema-migration path for wallet state.
• Blocking relationship: Originally split from feat(platform-wallet): load watch-only wallet state from local storage at startup #3692; now rebased directly on v4.1-dev, which already carries the shared rehydration scaffolding (ClientStartState, load_from_persistor, the WalletType::ExternalSignable model). This PR is the storage half — the net change against v4.1-dev is almost entirely the new crate.

What was done

Adds rs-platform-wallet-storage — a self-contained, embeddable SQLite persistence backend implementing PlatformWalletPersistence. One .db file holds many wallets, durable across restarts, with online backup/restore and automatic schema migration, under a hard contract: no private-key material is ever written to the database (signing material stays in the OS keyring or an encrypted vault).

Persister & seedless rehydration

• SqlitePersister (usable as Arc<dyn PlatformWalletPersistence>, Send + Sync, object-safe) with configurable journal / synchronous / flush modes, a retention policy, and auto-backup.
• Seedless load() reconstructs each wallet external-signable from its persisted account manifest (Wallet::new_external_signable, no seed required), then layers the persisted core-state projection — UTXOs, sync watermarks, chainlock, used-address pool depth — via apply_persisted_core_state. Prekeyed identity/contact joins mean signing works immediately post-load with no key re-sync.
• Per-domain readers back the load path: accounts, asset locks, core state, core address pool, identities (prekeyed), identity keys, platform addresses, and version watermarks.

Schema & migrations (refinery; additive; version-pinned with golden schema-freeze fingerprints)

• V001 __initial — per-wallet tables keyed by wallet_id, native FOREIGN KEY … ON DELETE CASCADE; identity-owned tables cascade via identities.wallet_id.
• V002 __address_height_pin — adds platform_addresses.as_of_height (address double-count fix).
• V003 __unified (additive, sequenced after V002) — core_address_pool (first-class per-index address-pool rows with a used flag, giving real per-account UTXO attribution in place of an account-0 approximation), meta_data_versions (per-(wallet_id, domain) monotonic sequence for cache invalidation), and meta_store_generation (restore-regenerated store token).
• V004 __invitations (additive, from the v4.1-dev merge) — adds the invitations table for DIP-13 DashPay invitations (inviter-side records; no key material stored, the voucher key is HD-re-derivable from funding_index).

Trust boundary & robustness (the .db is untrusted input at load)

• Fail-hard load(): any row that fails to decode, or an out-of-range wallet_id, aborts the whole call with a typed WalletStorageError — no silent per-row skip, no partial Ok.
• Layered size limits: a 16 MiB (SIZE_LIMIT_BYTES) cap on KV values and BLOB decode, per-column length() pre-read gates before materialization, a bincode decode bounded by a Limit config that rejects trailing bytes, and a 32 MiB SQLITE_LIMIT_LENGTH connection backstop.
• Typed columns are cross-checked against the decoded BLOB on every reader (outpoint / account / identity / key / wallet id); any mismatch hard-errors.

Secrets

• Encrypted-vault + OS-keyring secret store (SecretStore / EncryptedFileStore: Argon2id KDF + XChaCha20-Poly1305 AEAD envelope, zeroized, over keyring-core), so signing material never touches the wallet .db.

Changes outside the storage crate (deliberately minimal — the design intent was to leave platform-wallet unchanged and move persistence into its own crate)

• platform-wallet: doc-comment only (new_watch_only → new_external_signable).
• swift-sdk: the Platform-wallet load FFI consumer reconciled to the shipped 1-arg platform_wallet_manager_load_from_persistor contract; one real fix in SendTransactionView (stop funding core-to-core sends from a Platform-Payment index); the rest are doc-comment renames.
• Infra: rust-dashcore dependency rev bump (key-wallet out-of-order UTXO spend fix). Known interim state, tracked via the TODO(pin) comment at the pin site: the rev currently points at the live head of an open, unmerged upstream draft PR (fix(key-wallet): out-of-order UTXO spend causes history divergence (#649) rust-dashcore#851) rather than a merged/tagged commit — needed before this lands on a base branch, since the pinned object lives only on a mutable, unprotected branch. Root .cargo/audit.toml acknowledges RUSTSEC-2025-0141 (bincode unmaintained — an informational advisory, mitigated by the size caps + fail-hard load() above).

Deferred (TODO-marked, no regression)

• Manifest authentication (a MAC binding the persisted manifest to its wallet_id) — tracked in feat(platform-wallet): manifest integrity checksum (Risk-6/R12.5 follow-up) #3992.
• Long-term recovery for orphaned wallet rows (a crash between wallet-row creation and first-account registration leaves a permanently-empty manifest; it rehydrates harmlessly as an empty external-signable wallet today, but there is no eviction / re-registration path) — an open product decision, TODO left at the exact branch.

Test-only fast KDF for SecretStore (closes #4111)

• Downstream integration suites (e.g. dash-evo-tool's wallet_backend tests) drove real SecretStore::{set,get,set_secret,get_secret,reprotect} flows and paid a production-strength Argon2id derivation (64 MiB) on every call — individually 5-23s, with no way to opt out.
• Added SecretStore::file_mock / EncryptedFileStore::open_mock, gated behind #[cfg(any(test, feature = "test-util"))] (new test-util Cargo feature). A mock-constructed store derives at the enforced floor params instead of the shipped default — the fastest configuration enforce_bounds still accepts — for both the vault-unlock derivation and the per-secret Tier-2 wrap, with no change to any public method signature. A caller swaps file → file_mock at construction; every subsequent call is transparently fast.
• No KDF parameter type is exposed outside the crate (KdfParams stays pub(crate)); the fix is entirely crate-side so no downstream consumer implements its own fast-KDF logic. A single shared KdfParams::floor_target() helper is the crate's only definition of "fastest legal Argon2id params" — the three previously-duplicated private #[cfg(test)] copies were deleted and their ~31 call sites rewritten against it.
• A cfg!(debug_assertions)-based runtime guard (a runtime value, present in every profile — unlike debug_assert!) lives on floor_target() itself, the single choke point every caller (mock constructors and internal tests alike) goes through. If test-util ever leaks into a release build via feature unification, the guard panics loudly instead of silently handing back weak crypto; open_mock evaluates it before the passphrase check so a blank passphrase can't mask the panic.
• Perf follow-through: argon2 is the only workspace crate consumer of the argon2 dependency, so added it to the root Cargo.toml's existing [profile.dev.package.*] opt-level = 3 list (alongside halo2_proofs/orchard/pasta_curves/etc.) — narrow in practice despite being a workspace-level stanza, since nothing else in the tree compiles argon2. Composes multiplicatively with the mock-KDF floor params: measured 149.19s → 13.51s (~11x) on platform-wallet-storage's 266 KDF-heavy lib unit tests.

How Has This Been Tested?

• cargo clippy --package platform-wallet --package platform-wallet-storage --package platform-wallet-ffi --package rs-unified-sdk-ffi --all-features --locked -- --no-deps -D warnings — clean (matches this repo's CI invocation exactly).
• cargo nextest run --package platform-wallet --package platform-wallet-storage --package platform-wallet-ffi --all-features --locked -E 'not test(~shield)' — 1299 passed / 0 failed (136 skipped), after rebasing onto v4.1-dev (which brought in DIP-13 invitations / V004, provider-key persistence, and the QA-002/003/004 Drop-release + InstantLock fixes) and re-pinning rust-dashcore.
• Swift compilation is verified by CI (Swift SDK build) — FFI symbols were matched by hand against the Rust extern "C" surface.
• Independently reviewed via a 14-agent panel (security/structural/QA passes across the storage core, secrets subsystem, tests+docs, and cross-cutting integration, plus an independent Codex Sol pass over the full diff): 0 CRITICAL/HIGH, 17 MEDIUM findings after deduplication — see review threads for detail. Top items worth a look before merge: the rust-dashcore pin above, and two data-durability edge cases (identities table missing a uniqueness constraint on (wallet_id, identity_index), and load_from_persistor's failure path not being recoverable by the shipped Swift reference caller).

Breaking Changes

None in this PR's net diff. The storage crate is purely additive; the platform-wallet / swift-sdk touches are comment renames plus one localized send-funding fix. (The WalletType::ExternalSignable model this crate consumes already lives in v4.1-dev.)

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas (trust-boundary and migration paths)
• I have added or updated relevant unit/integration/functional/e2e tests
• I have added "!" to the title and described breaking changes — n/a, no breaking changes
• I have made corresponding changes to the documentation (README / SCHEMA / SECRETS)

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

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

Summary by CodeRabbit

• New Features

  • Added support for a safer, more explicit secret-storage flow, including unprotected vaults, password-protected secrets, and stricter size limits.
  • Wallet data loading now restores more account, identity, contact, and balance information automatically.

• Bug Fixes

  • Improved database consistency during wallet delete, backup, restore, and migration operations.
  • Fixed several read/load paths to reject corrupted, oversized, or mismatched data instead of failing silently.
  • Strengthened key and secret handling to better prevent data leakage and invalid input issues.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR adds a Tier-2 secret-envelope format and hardens secret storage, while renaming the SQLite wallet root to wallets, adding typed per-area rehydration readers, and wiring SqlitePersister::load() to rebuild keyless wallets from persisted state.

Changes

Secrets

Layer / File(s)	Summary
Wire format and AAD types src/secrets/wire/*.rs	Adds the envelope, typed AAD structs, KDF wire encoding, and bincode wrap/unwrap logic.
Secret error taxonomy src/secrets/error.rs	Adds tier-2 error variants and updates keyring SPI projection.
Store API and file-vault hardening src/secrets/store.rs, src/secrets/file/*	Adds file_unprotected, refactors read/write flows, and hardens blank-passphrase, size, fsync, and permission handling.
Secrets docs, keyring docs, and config SECRETS.md, src/secrets/mod.rs, src/secrets/keyring.rs, Cargo.toml, tests/secrets_*	Updates docs, feature flags, compile-time checks, and secrets-focused integration tests.

SQLite

Layer / File(s)	Summary
Migrations, blob sealing, and shared helpers migrations/V001__initial.rs, migrations/V003__unified.rs, src/sqlite/schema/blob.rs, src/sqlite/schema/wallets.rs, src/kv.rs, src/lib.rs	Re-roots wallet FKs to wallets, adds address-pool and metadata-version tables, seals blob persistence, and updates shared size/cast helpers.
Per-area readers and load_state wiring src/sqlite/schema/*.rs	Adds typed readers for accounts, identities, keys, contacts, asset locks, core state, pools, and platform addresses.
Persister open/load/delete/backup/restore src/sqlite/persister.rs, src/sqlite/error.rs, src/sqlite/backup.rs, src/sqlite/conn.rs, src/sqlite/migrations.rs, src/sqlite/util/wallet.rs	Adds open-path guarding, schema/application checks, keyless load/rebuild, and hardened persistence flows.
Tests and fixtures tests/*.rs	Extensive coverage updates for the new schema, load path, and rehydration behavior.
Docs SCHEMA.md, README.md	Updates the schema and load() documentation to match the new root anchor and keyless rehydration model.

Estimated code review effort: 5 (Critical) | ~150 minutes

Possibly related issues

• rs-platform-wallet: own and expose the per-address platform nonce (advance-on-submit + local reader) #3825: The new wallet rehydration path and platform_addresses handling align with the issue’s platform-state exposure and reconstruction scope.

Possibly related PRs

• dashpay/platform#3062: Both PRs touch workspace dependency wiring and git revision pins.
• dashpay/platform#3625: This PR extends the same rs-platform-wallet-storage crate that was introduced there.
• dashpay/platform#3828: Both PRs change the core-state/account-index attribution path around UTXO persistence.

Suggested labels: Client Only

Suggested reviewers: shumkov, thepastaclaw

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Clearly summarizes the new platform-wallet-storage SQLite backend and seedless rehydration work.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/platform-wallet-storage-rehydration

---

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

[Claudius-Maginificent]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[thepastaclaw]

⛔ Blockers found — Sonnet deferred (commit 7956bb8)
Canonical validated blockers: 1

[thepastaclaw]

Code Review

The PR adds the storage-side keyless load readers, but it also replaces two externally reachable restore paths with unconditional panics. The new rehydration readers are mostly wired, but several fail-hard corruption checks are missing where typed SQLite columns can disagree with decoded blobs.

🔴 2 blocking | 🟡 6 suggestion(s)

Findings not posted inline (2)

These findings could not be anchored to the current diff, but they are still part of this review.

• [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/identities.rs:143-150: Identity reader trusts blob identity over the row key — load_state() selects identity_id but discards it, then decodes entry_blob and routes the restored identity using entry.id. The writer rejects IdentityEntry values whose blob ID disagrees with the typed column, but a restored or corrupted SQLite row can bypass the writer. The reader shou...
• [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/contacts.rs:245-266: Contact reader does not validate request IDs against row keys — The contacts reader keys pending rows from (owner_id, contact_id) but stores the decoded ContactRequest without checking its sender and recipient IDs. During apply, sent requests are inserted under entry.request.recipient_id and incoming requests under entry.request.sender_id, so a row wh...

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet/src/manager/load.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/manager/load.rs:13-15: Public manager restore API now panics
  `load_from_persistor()` is a public restore entry point returning `Result<(), PlatformWalletError>`, but this PR replaces the previous implementation with `todo!()`. The exported C ABI function `platform_wallet_manager_load_from_persistor` calls this method directly, and the Swift `loadFromPersistor()` wrapper calls that exported function, so any app invoking persisted wallet restore aborts instead of receiving a typed error. If this branch intentionally defers keyless manager rehydration to #3692, the public API still needs to fail closed with an error rather than panic across the FFI boundary.

In `packages/rs-platform-wallet-ffi/src/persistence.rs`:
- [BLOCKING] packages/rs-platform-wallet-ffi/src/persistence.rs:3389-3390: FFI persister load panics after receiving restore rows
  `FFIPersister::load()` calls `build_wallet_start_state()` for every wallet returned by the Swift `on_load_wallet_list_fn` callback, and this function now reaches an unconditional `todo!()` after partially reconstructing the entry. This path is externally reachable through restore and shielded binding flows that call `persister.load()`. A panic here can unwind toward `extern "C"` callers and abort the process instead of returning the existing `PersistenceError`/`PlatformWalletFFIResult` failure path.

In `packages/rs-platform-wallet-storage/src/sqlite/schema/identities.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/identities.rs:143-150: Identity reader trusts blob identity over the row key
  `load_state()` selects `identity_id` but discards it, then decodes `entry_blob` and routes the restored identity using `entry.id`. The writer rejects `IdentityEntry` values whose blob ID disagrees with the typed column, but a restored or corrupted SQLite row can bypass the writer. The reader should enforce the same column-vs-blob check, including wallet scope when `entry.wallet_id` is set, so semantic corruption fails the load instead of hydrating the wrong identity.
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/identities.rs:143-150: Identity reader trusts blob identity over the row key
  `load_state()` selects `identity_id` but discards it, then decodes `entry_blob` and routes the restored identity using `entry.id`. The writer rejects `IdentityEntry` values whose blob ID disagrees with the typed column, but a restored or corrupted SQLite row can bypass the writer. The reader should enforce the same column-vs-blob check, including wallet scope when `entry.wallet_id` is set, so semantic corruption fails the load instead of hydrating the wrong identity.

In `packages/rs-platform-wallet-storage/src/sqlite/schema/identity_keys.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/identity_keys.rs:168-169: Identity-key reader does not verify decoded entries match row columns
  `load_state()` reconstructs `(identity_id, key_id)` from the SQL row, decodes `public_key_blob`, and inserts the decoded entry without checking that the blob carries the same identity, key id, wallet id, or public-key hash. The apply path later ignores the changeset map key and routes by fields from the decoded `IdentityKeyEntry`, so a semantically inconsistent row can attach a public key to the wrong identity or carry a hash that disagrees with the indexed column. Mirror the writer-side consistency checks on read before inserting into the changeset.

In `packages/rs-platform-wallet-storage/src/sqlite/schema/contacts.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/contacts.rs:245-266: Contact reader does not validate request IDs against row keys
  The contacts reader keys pending rows from `(owner_id, contact_id)` but stores the decoded `ContactRequest` without checking its sender and recipient IDs. During apply, sent requests are inserted under `entry.request.recipient_id` and incoming requests under `entry.request.sender_id`, so a row whose blob disagrees with the typed columns rehydrates under a different counterparty and later tombstones for the row key will not clear it. Established rows should also verify their outgoing and incoming requests match the same `(owner, contact)` relationship before accepting the row.
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/contacts.rs:245-266: Contact reader does not validate request IDs against row keys
  The contacts reader keys pending rows from `(owner_id, contact_id)` but stores the decoded `ContactRequest` without checking its sender and recipient IDs. During apply, sent requests are inserted under `entry.request.recipient_id` and incoming requests under `entry.request.sender_id`, so a row whose blob disagrees with the typed columns rehydrates under a different counterparty and later tombstones for the row key will not clear it. Established rows should also verify their outgoing and incoming requests match the same `(owner, contact)` relationship before accepting the row.

In `packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs:316-325: Oversized BLOB rows are materialized before the size cap runs
  The new load readers fetch BLOB columns directly into `Vec<u8>` and only then call `blob::decode()`, whose 16 MiB cap runs after rusqlite has already allocated and copied the value. A restored or locally modified SQLite DB can therefore store a huge `record_blob` or other `*_blob` value that passes SQLite integrity checks and forces large process allocations on startup before returning `BlobTooLarge`. Use a shared bounded read helper or select `length(blob_column)` first, as the KV path already does, before materializing BLOB contents.

[coderabbitai]

Actionable comments posted: 16

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (7)

packages/rs-platform-wallet-storage/tests/sqlite_migrations.rs (1)

165-180: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Count identity_keys by wallet_id now that the table is wallet-scoped.

identity_keys moved onto (wallet_id, identity_id, key_id), but this smoke test still routes it through the via_identity path. That means the assertion would still pass if the row were written with the wrong wallet_id as long as identity_id matched, so the new schema contract is not actually being exercised here.

Suggested fix

     let via_identity = [
-        "identity_keys",
         "token_balances",
         "dashpay_profiles",
         "dashpay_payments_overlay",
     ];

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/rs-platform-wallet-storage/tests/sqlite_migrations.rs` around lines
165 - 180, The smoke test still treats identity_keys as identity-scoped, but the
schema now scopes it by wallet_id. Update the test logic in sqlite_migrations.rs
so identity_keys uses the wallet_id COUNT query path instead of the via_identity
branch, while keeping the other tables that still depend on identities routed
through identity_id. Use the existing via_identity handling in the loop over
cases to locate and adjust the count_sql selection.

packages/rs-platform-wallet-storage/SCHEMA.md (1)

507-513: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

The soft-cascade note overstates cleanup for identity-scoped metadata.

meta_identity and meta_token do not carry wallet_id, so a wallet delete only reaches them through existing identities rows. If metadata was written before an identities row ever existed, that cleanup path never fires; the orphan-metadata section above already documents exactly that case.

Suggested wording

-`wallets` row fires a wallet-rooted `AFTER DELETE` trigger that
-brooms the wallet-scoped tables (`meta_wallet`, `meta_contact`,
-`meta_platform_address`) by `wallet_id`, and the FK cascade through
-`identities` fires a per-identity trigger that brooms `meta_identity` +
-`meta_token` by `identity_id`. Both legs key on the id alone, so a wallet
-delete cleans its metadata transitively whether or not the typed parent
-was ever written and regardless of any contact's lifecycle state.
+`wallets` row fires a wallet-rooted `AFTER DELETE` trigger that
+brooms the wallet-scoped tables (`meta_wallet`, `meta_contact`,
+`meta_platform_address`) by `wallet_id`, and the FK cascade through
+existing `identities` rows fires a per-identity trigger that brooms
+`meta_identity` + `meta_token` by `identity_id`. That means wallet-scoped
+metadata is cleaned regardless of typed-parent existence, while
+identity-scoped metadata still requires an `identities` row to exist.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/rs-platform-wallet-storage/SCHEMA.md` around lines 507 - 513, The
soft-cascade description in SCHEMA.md overstates what a wallet delete cleans up
for identity-scoped metadata. Update the note near the wallet/identity trigger
flow to say that `wallets` deletion only reaches `meta_identity` and
`meta_token` through existing `identities` rows and that orphan metadata written
before an `identities` row exists is not covered; align the wording with the
existing orphan-metadata section and reference the `wallets` trigger and the
`identities` FK cascade path.

packages/rs-platform-wallet-storage/src/sqlite/schema/accounts.rs (1)

27-36: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Fail closed on corrupted platform-payment registration rows.

This helper trusts the typed account_index column but never verifies that the decoded AccountRegistrationEntry is actually a PlatformPayment entry for that same index. all_platform_payment_registrations() feeds platform_addrs::load_all(), so a tampered row will currently rehydrate under the typed index with the blob's xpub instead of tripping AccountRegistrationEntryMismatch.

Suggested fix

 fn decode_platform_payment_row(
     account_index: i64,
     xpub_bytes: &[u8],
 ) -> Result<PlatformPaymentRegistration, WalletStorageError> {
     let account_index = crate::sqlite::util::safe_cast::i64_to_u32(
         "account_registrations.account_index",
         account_index,
     )?;
     let entry: AccountRegistrationEntry = blob::decode(xpub_bytes)?;
+    if account_type_db_label(&entry.account_type) != "platform_payment"
+        || account_index(&entry.account_type) != account_index
+    {
+        return Err(WalletStorageError::AccountRegistrationEntryMismatch);
+    }
     Ok((account_index, entry.account_xpub))
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/rs-platform-wallet-storage/src/sqlite/schema/accounts.rs` around
lines 27 - 36, `decode_platform_payment_row` currently decodes the blob and
returns the typed `account_index` without checking that the
`AccountRegistrationEntry` is a `PlatformPayment` for that same index. Update
this helper to validate the decoded `AccountRegistrationEntry` matches the
expected `PlatformPayment` variant and index, and return
`AccountRegistrationEntryMismatch` if it does not. Keep the existing
`safe_cast::i64_to_u32` conversion, but make
`all_platform_payment_registrations()` fail closed by rejecting any corrupted or
mismatched row instead of rehydrating it.

packages/rs-platform-wallet-storage/src/sqlite/backup.rs (2)

243-263: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift

Do not delete WAL/SHM before the replacement is guaranteed.

If sibling removal succeeds and tmp.persist(dest_db_path) then fails, the original main DB remains but its WAL/SHM may be gone, losing committed WAL-mode state. The restore path needs a rollback-safe swap strategy or a SQLite-native restore that does not destructively unlink siblings before the main replacement succeeds.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/rs-platform-wallet-storage/src/sqlite/backup.rs` around lines 243 -
263, The restore flow in `backup.rs` removes `-wal`/`-shm` siblings before
`tmp.persist(dest_db_path)`, which can leave the original DB intact but its
WAL-mode state lost if persist fails. Change the `restore` logic to use a
rollback-safe replacement strategy: do not unlink siblings until the destination
swap is guaranteed, or replace the whole SQLite set atomically via a
SQLite-native restore path. Keep the fix localized around the sibling cleanup
and `tmp.persist` sequence so the operation remains all-or-nothing.

---

361-374: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Apply keep_last_n as a floor, not a ceiling.

With both keep_last_n and max_age set, line 373 still requires pass_count, so backups beyond the newest N are deleted even when they are within max_age. That contradicts the new floor semantics.

Proposed fix

-        let pass_count = match policy.keep_last_n {
-            Some(n) => idx < n,
-            None => true,
-        };
         let pass_age = match policy.max_age {
             Some(max) => now.duration_since(ts).map(|d| d <= max).unwrap_or(true),
-            None => true,
+            None => policy.keep_last_n.is_none(),
         };
-        if within_floor || (pass_count && pass_age) {
+        if within_floor || pass_age {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/rs-platform-wallet-storage/src/sqlite/backup.rs` around lines 361 -
374, In backup pruning logic in the `retain_backups` flow, `keep_last_n` is
still being treated like a ceiling because the deletion condition requires
`pass_count` even when `max_age` is also set. Update the condition around
`within_floor`, `pass_count`, and `pass_age` so that the newest N backups are
always kept as a floor and any backup within the age limit is also retained,
using the existing `policy.keep_last_n` and `policy.max_age` checks in this
block.

packages/rs-platform-wallet-storage/src/sqlite/schema/identities.rs (1)

143-150: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Validate typed identity columns against the blob during load.

load_state ignores the selected identity_id, so a corrupted row whose typed column and entry_blob.id diverge is silently rehydrated under the blob value. Also reject a blob wallet_id that disagrees with the scoped wallet.

Proposed fix

-        let _identity_id: Vec<u8> = row.get(0)?;
+        let identity_id: Vec<u8> = row.get(0)?;
         let payload: Vec<u8> = row.get(1)?;
         let tombstoned: i64 = row.get(2)?;
         if tombstoned != 0 {
             continue;
         }
+        let typed_id = <[u8; 32]>::try_from(identity_id.as_slice())
+            .map_err(|_| WalletStorageError::blob_decode("identities.identity_id is not 32 bytes"))?;
         let entry: IdentityEntry = blob::decode(&payload)?;
+        if entry.id.as_bytes() != &typed_id {
+            return Err(WalletStorageError::IdentityEntryIdMismatch);
+        }
+        if let Some(entry_wallet_id) = entry.wallet_id {
+            if entry_wallet_id != *wallet_id {
+                return Err(WalletStorageError::WalletIdMismatch {
+                    expected: *wallet_id,
+                    found: entry_wallet_id,
+                });
+            }
+        }
         let managed = managed_identity_from_entry(&entry, wallet_id);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/rs-platform-wallet-storage/src/sqlite/schema/identities.rs` around
lines 143 - 150, The load path in load_state is trusting the blob too much and
currently ignores the selected identity_id, so mismatched typed columns can be
silently rehydrated under the blob value. Update the row handling in load_state
to validate that the typed identity_id matches entry_blob.id before decoding
into IdentityEntry, and also verify the blob wallet_id matches the wallet_id
scope passed into managed_identity_from_entry. If either check fails, reject the
row instead of continuing.

packages/rs-platform-wallet-storage/src/sqlite/persister.rs (1)

299-326: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Enforce the open-path registry before restore.

restore_from_inner can replace dest_db_path while a live SqlitePersister in this process still owns the same DB. Check the registry up front and return AlreadyOpen; otherwise the live handle/buffer can diverge from the restored file.

Proposed fix outline

+        let registered_path = dest_db_path
+            .canonicalize()
+            .unwrap_or_else(|_| dest_db_path.to_path_buf());
+        if open_path_registry()
+            .lock()
+            .unwrap_or_else(|p| p.into_inner())
+            .contains(&registered_path)
+        {
+            return Err(WalletStorageError::AlreadyOpen {
+                path: registered_path,
+            });
+        }
+
         if !skip_backup && dest_db_path.exists() {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/rs-platform-wallet-storage/src/sqlite/persister.rs` around lines 299
- 326, restore_from_inner currently restores the database without checking
whether the destination path is already owned by a live SqlitePersister, which
can leave an in-memory handle out of sync with the replaced file. Add an upfront
registry lookup in restore_from_inner for dest_db_path and return
WalletStorageError::AlreadyOpen when the path is already registered, before any
backup or restore work begins. Keep the change localized around
restore_from_inner and the open-path registry used by SqlitePersister so
existing live handles are protected from restore-time replacement.

🧹 Nitpick comments (4)

packages/rs-platform-wallet-storage/tests/sqlite_load_wiring.rs (1)

91-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert synced_height as well as last_processed_height.

This test writes both fields, but only validates one of them. If load() stops wiring synced_height, the round-trip still passes.

Suggested assertion

     assert_eq!(slice.core_state.new_utxos.len(), 1);
     assert_eq!(slice.core_state.new_utxos[0].value(), 777_000);
+    assert_eq!(slice.core_state.synced_height, Some(50));
     assert_eq!(slice.core_state.last_processed_height, Some(50));

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/rs-platform-wallet-storage/tests/sqlite_load_wiring.rs` around lines
91 - 127, The round-trip test in `sqlite_load_wiring.rs` only verifies
`last_processed_height` from `state.wallets.get(&w).core_state` even though
`synced_height` is also written into `CoreChangeSet`; update the existing load
assertions to check both fields after `p2.load()` so `load()` wiring regressions
for `synced_height` are caught alongside `last_processed_height`.

packages/rs-platform-wallet-storage/tests/sqlite_dashpay_overlay_contract.rs (1)

93-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert that the overlay stays out of the rehydrated identity.

This currently proves only that load() still returns the wallet's core state. If a regression starts merging dashpay_profiles into the loaded identity payload, this test still passes. Please also assert that the seeded identity is present after load() and that its DashPay profile remains absent for the overlay-only write case.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/rs-platform-wallet-storage/tests/sqlite_dashpay_overlay_contract.rs`
around lines 93 - 108, The current test around persister.load() only verifies
wallet.core_state, so it can miss regressions where dashpay_profiles gets merged
into the rehydrated identity. Update the sqlite_dashpay_overlay_contract test to
also inspect the loaded identity payload for the seeded wallet after load() and
assert that the identity is still present while its DashPay profile remains
absent in this overlay-only write scenario. Use the existing persister.load(),
wallets.get(&w), and any identity fields already available in the loaded state
to make the check explicit.

packages/rs-platform-wallet-storage/tests/sqlite_delete_real_apply_failure.rs (1)

67-72: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Also assert that the failed pre-flush left nothing durable.

Restoring the buffer is only half of the contract here. If apply_changeset_to_tx ever leaks the wallets insert before the core_sync_state failure, this test still passes and leaves duplicate-on-retry state behind.

Suggested assertion block

     assert!(
         persister.buffer_has_changeset_for_test(&w),
         "buffered changeset must be restored after a real pre-flush apply failure"
     );
+
+    let conn = persister.lock_conn_for_test();
+    let wallets_rows: i64 = conn
+        .query_row(
+            "SELECT COUNT(*) FROM wallets WHERE wallet_id = ?1",
+            rusqlite::params![w.as_slice()],
+            |row| row.get(0),
+        )
+        .unwrap();
+    let core_rows: i64 = conn
+        .query_row(
+            "SELECT COUNT(*) FROM core_sync_state WHERE wallet_id = ?1",
+            rusqlite::params![w.as_slice()],
+            |row| row.get(0),
+        )
+        .unwrap();
+    assert_eq!(wallets_rows, 0, "failed pre-flush must not durably create the wallet row");
+    assert_eq!(core_rows, 0, "failed pre-flush must not durably create child rows");

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/rs-platform-wallet-storage/tests/sqlite_delete_real_apply_failure.rs`
around lines 67 - 72, The test currently only verifies the buffered changeset is
restored, but it should also verify that a failed pre-flush did not persist any
durable state. In sqlite_delete_real_apply_failure.rs, extend the existing
scenario around the failed delete so it checks the database/transaction state
after the apply failure and confirms no `wallets` insert or other durable side
effects remain from `apply_changeset_to_tx`. Keep the existing
`persister.buffer_has_changeset_for_test(&w)` assertion, and add a second
assertion in the same test that validates the storage is clean after the failure
so retry does not see duplicate-on-retry state.

packages/rs-platform-wallet-storage/src/sqlite/persister.rs (1)

813-814: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Fix the query-budget documentation.

load() currently performs multiple reader calls inside the for wallet_id in wallet_ids loop, so the query count grows with wallet count. Reword this to avoid promising constant query budget.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/rs-platform-wallet-storage/src/sqlite/persister.rs` around lines 813
- 814, Update the query-budget comment in the load path so it no longer claims
constant cost with wallet count; the current load() flow iterates over
wallet_ids and performs multiple reader calls per wallet, so reword the
documentation to describe that it has per-wallet read/query work rather than a
fixed query budget. Keep the note near the wallet_ids loop/load() implementation
and make sure the wording matches the actual behavior of the reader calls.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/rs-platform-wallet-ffi/src/persistence.rs`:
- Around line 3389-3390: The temporary restore stub in the persistence restore
flow should not panic via todo!(); replace it with a recoverable typed error so
callers receive a PersistenceError instead of crashing. Update the restore-path
branch that currently ignores identity_manager and unused_asset_locks to return
an appropriate PersistenceError variant (or equivalent error conversion) from
the same function/method, keeping the signature consistent and preserving the
existing error handling path.

In `@packages/rs-platform-wallet-storage/README.md`:
- Around line 165-168: The README wording around the manager-side rehydration
flow is too strong for this PR because the manager/FFI load path is still
stubbed. Update the description near the watch-only rebuild note to clearly mark
the manager-side `load_from_persistor`/`Wallet::new_watch_only` application as
pending or follow-up work, and keep the current text scoped to the storage-side
behavior only.

In `@packages/rs-platform-wallet-storage/src/kv.rs`:
- Around line 62-65: The key-length validation in validate_key currently assumes
Rust chars().count() matches SQLite length() for all strings, but embedded NULs
break that equivalence. Update the key precheck to explicitly reject keys
containing \0 before comparing length, or adjust the validation/comment so it no
longer claims the same key set; keep the logic aligned with the SQL CHECK
constraint in kv.rs.

In `@packages/rs-platform-wallet-storage/src/secrets/error.rs`:
- Around line 3-5: The file-level non-leakage docs in error.rs are too broad for
the current Io behavior: they claim variants never carry a stringified source,
but Io::fmt/rendering still exposes the underlying source text. Update the docs
to carve out the Io exception, or change Io’s display implementation/tests so it
no longer includes the source string, keeping the wording aligned with the
actual Error and Io rendering behavior.
- Around line 88-91: The UnsupportedEnvelopeVersion error currently truncates
the envelope version to u8, so update the error variant in error.rs to store the
full u32 version value instead. Then adjust the envelope parsing call site that
constructs UnsupportedEnvelopeVersion to pass the original Envelope.version
without narrowing, keeping the reported version accurate in the error message.

In `@packages/rs-platform-wallet-storage/src/secrets/file/format.rs`:
- Around line 21-22: The docs for the nested BTreeMap format currently imply
duplicate (wallet_id, label) pairs are prevented entirely, but the read path
still accepts duplicate JSON keys and serde collapses them. Update the
documentation near the format description to state that uniqueness is only
guaranteed by serialization, or change the deserialization logic in the file
format/parser code to explicitly reject duplicate keys, and make the behavior
match the tests and the intended API.

In `@packages/rs-platform-wallet-storage/src/secrets/file/mod.rs`:
- Around line 628-654: The post-persist Unix handling in the vault write path is
swallowing parent-directory fsync failures and returning success, which makes
`put`/`delete`/`rekey` report a durable commit when only the rename succeeded.
Update the flow around the `persist()`/`sync_all()` block to surface a distinct
“committed but not durable” result or otherwise keep the in-memory commit behind
the durability boundary, and make sure the caller can tell when
`fs::File::open(parent)` or `sync_all()` fails instead of only logging via
`tracing::warn!`.

In `@packages/rs-platform-wallet-storage/src/secrets/store.rs`:
- Around line 255-266: The reprotect method in SecretStore currently does a
non-atomic read-then-write using get_secret followed by set_secret, which can
overwrite concurrent updates with stale plaintext. Update reprotect to use an
atomic backend-specific reprotect/CAS path, or add a version check so the write
only succeeds if the entry has not changed since get_secret; reference
SecretStore::reprotect, get_secret, and set_secret when wiring the fix.

In `@packages/rs-platform-wallet-storage/src/secrets/wire/envelope.rs`:
- Around line 136-141: The scheme-0 plaintext path in the envelope handling
still leaves temporary Vec<u8> buffers unwiped, including the
Unprotected(plaintext.to_vec()) branch and the ExpectedProtectedButUnsealed arm.
Update the envelope logic in the encode/decode flow around the Envelope and
Payload handling to use zeroizing storage for these plaintext temporaries or
explicitly wipe them before drop, while keeping SecretBytes::new only for the
final encoded blob.

In `@packages/rs-platform-wallet-storage/src/sqlite/persister.rs`:
- Around line 179-199: `persist`/`open` currently treats `has_schema_history()`
as the only brand-new-vs-existing check, so a pre-existing non-wallet SQLite
file with no `refinery_schema_history` can still be migrated. Add an explicit
guard in the `had_schema_history` decision path to reject existing SQLite files
that lack wallet schema history, using the same `conn`/`has_schema_history` flow
and returning a typed wallet storage error before any backup, integrity check,
or `migrations::run()` work begins.

In `@packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs`:
- Around line 143-154: The sync-state write path in core_state should treat
last_applied_chain_lock monotonically, not as a blind overwrite. Update the
CoreChangeSet-to-DB flow around upsert_sync_state so the stored chain-lock is
max-merged with the existing row (using the same chain-lock height comparison
logic as the height watermarks) before persisting. Apply this behavior wherever
last_applied_chain_lock is written in the affected core_state update functions
so the persisted chain-lock cannot regress.
- Around line 40-41: The `decode_from_slice` handling in
`last_applied_chain_lock` is too permissive because it accepts a valid prefix
and ignores any appended data. Update this decoding path in `core_state.rs` to
mirror the other blob decoders: after calling `bincode::decode_from_slice` for
`ChainLock`, verify the returned consumed length matches `bytes.len()` and treat
any mismatch as corruption by returning `None` instead of loading the state.

In `@packages/rs-platform-wallet-storage/src/sqlite/schema/identity_keys.rs`:
- Around line 151-169: Mirror the writer-side validation in load_state by
checking that each decoded public_key_blob matches the row’s typed columns
before inserting into cs.upserts. After decode_entry(&payload), verify the
entry’s identity_id, key_id, wallet_id, and public_key_hash against the values
from the identity_keys query, and return a WalletStorageError if any mismatch is
found. Keep the checks local to load_state and use the existing decode_entry,
Identifier::from, and KeyID::try_from flow so inconsistent rows are rejected
instead of loaded silently.

In `@packages/rs-platform-wallet-storage/tests/sqlite_accounts_reader.rs`:
- Around line 46-82: The sqlite_accounts_reader test is too weak because both
AccountRegistrationEntry fixtures use the same xpub and the assertions only
check set membership, so row reordering or xpub/row mixups can still pass.
Update the test to use distinct xpub fixtures for each entry and assert the
loaded manifest in the expected order, using the accounts::load_state result and
the existing AccountType variants to verify each row maps to the correct xpub.

In `@packages/rs-platform-wallet/src/changeset/client_wallet_start_state.rs`:
- Line 33: The doc comment on the wallet start state field still references the
old wallet_metadata table. Update the comment in client_wallet_start_state.rs to
point to the renamed wallets table instead, keeping the wording aligned with the
field’s source of truth and using the existing comment near the network field to
locate it.

In `@packages/rs-platform-wallet/src/manager/load.rs`:
- Around line 8-14: The public rehydration entry point
PlatformWalletManager::load_from_persistor currently panics via todo!, which
turns a caller error into a runtime abort. Replace the todo! with a recoverable
Result path by returning an explicit PlatformWalletError for the unsupported
stub state, or otherwise gate/remove this API until keyless rehydration in
PlatformWalletManager is implemented. Ensure callers receive an error instead of
a panic.

---

Outside diff comments:
In `@packages/rs-platform-wallet-storage/SCHEMA.md`:
- Around line 507-513: The soft-cascade description in SCHEMA.md overstates what
a wallet delete cleans up for identity-scoped metadata. Update the note near the
wallet/identity trigger flow to say that `wallets` deletion only reaches
`meta_identity` and `meta_token` through existing `identities` rows and that
orphan metadata written before an `identities` row exists is not covered; align
the wording with the existing orphan-metadata section and reference the
`wallets` trigger and the `identities` FK cascade path.

In `@packages/rs-platform-wallet-storage/src/sqlite/backup.rs`:
- Around line 243-263: The restore flow in `backup.rs` removes `-wal`/`-shm`
siblings before `tmp.persist(dest_db_path)`, which can leave the original DB
intact but its WAL-mode state lost if persist fails. Change the `restore` logic
to use a rollback-safe replacement strategy: do not unlink siblings until the
destination swap is guaranteed, or replace the whole SQLite set atomically via a
SQLite-native restore path. Keep the fix localized around the sibling cleanup
and `tmp.persist` sequence so the operation remains all-or-nothing.
- Around line 361-374: In backup pruning logic in the `retain_backups` flow,
`keep_last_n` is still being treated like a ceiling because the deletion
condition requires `pass_count` even when `max_age` is also set. Update the
condition around `within_floor`, `pass_count`, and `pass_age` so that the newest
N backups are always kept as a floor and any backup within the age limit is also
retained, using the existing `policy.keep_last_n` and `policy.max_age` checks in
this block.

In `@packages/rs-platform-wallet-storage/src/sqlite/persister.rs`:
- Around line 299-326: restore_from_inner currently restores the database
without checking whether the destination path is already owned by a live
SqlitePersister, which can leave an in-memory handle out of sync with the
replaced file. Add an upfront registry lookup in restore_from_inner for
dest_db_path and return WalletStorageError::AlreadyOpen when the path is already
registered, before any backup or restore work begins. Keep the change localized
around restore_from_inner and the open-path registry used by SqlitePersister so
existing live handles are protected from restore-time replacement.

In `@packages/rs-platform-wallet-storage/src/sqlite/schema/accounts.rs`:
- Around line 27-36: `decode_platform_payment_row` currently decodes the blob
and returns the typed `account_index` without checking that the
`AccountRegistrationEntry` is a `PlatformPayment` for that same index. Update
this helper to validate the decoded `AccountRegistrationEntry` matches the
expected `PlatformPayment` variant and index, and return
`AccountRegistrationEntryMismatch` if it does not. Keep the existing
`safe_cast::i64_to_u32` conversion, but make
`all_platform_payment_registrations()` fail closed by rejecting any corrupted or
mismatched row instead of rehydrating it.

In `@packages/rs-platform-wallet-storage/src/sqlite/schema/identities.rs`:
- Around line 143-150: The load path in load_state is trusting the blob too much
and currently ignores the selected identity_id, so mismatched typed columns can
be silently rehydrated under the blob value. Update the row handling in
load_state to validate that the typed identity_id matches entry_blob.id before
decoding into IdentityEntry, and also verify the blob wallet_id matches the
wallet_id scope passed into managed_identity_from_entry. If either check fails,
reject the row instead of continuing.

In `@packages/rs-platform-wallet-storage/tests/sqlite_migrations.rs`:
- Around line 165-180: The smoke test still treats identity_keys as
identity-scoped, but the schema now scopes it by wallet_id. Update the test
logic in sqlite_migrations.rs so identity_keys uses the wallet_id COUNT query
path instead of the via_identity branch, while keeping the other tables that
still depend on identities routed through identity_id. Use the existing
via_identity handling in the loop over cases to locate and adjust the count_sql
selection.

---

Nitpick comments:
In `@packages/rs-platform-wallet-storage/src/sqlite/persister.rs`:
- Around line 813-814: Update the query-budget comment in the load path so it no
longer claims constant cost with wallet count; the current load() flow iterates
over wallet_ids and performs multiple reader calls per wallet, so reword the
documentation to describe that it has per-wallet read/query work rather than a
fixed query budget. Keep the note near the wallet_ids loop/load() implementation
and make sure the wording matches the actual behavior of the reader calls.

In
`@packages/rs-platform-wallet-storage/tests/sqlite_dashpay_overlay_contract.rs`:
- Around line 93-108: The current test around persister.load() only verifies
wallet.core_state, so it can miss regressions where dashpay_profiles gets merged
into the rehydrated identity. Update the sqlite_dashpay_overlay_contract test to
also inspect the loaded identity payload for the seeded wallet after load() and
assert that the identity is still present while its DashPay profile remains
absent in this overlay-only write scenario. Use the existing persister.load(),
wallets.get(&w), and any identity fields already available in the loaded state
to make the check explicit.

In
`@packages/rs-platform-wallet-storage/tests/sqlite_delete_real_apply_failure.rs`:
- Around line 67-72: The test currently only verifies the buffered changeset is
restored, but it should also verify that a failed pre-flush did not persist any
durable state. In sqlite_delete_real_apply_failure.rs, extend the existing
scenario around the failed delete so it checks the database/transaction state
after the apply failure and confirms no `wallets` insert or other durable side
effects remain from `apply_changeset_to_tx`. Keep the existing
`persister.buffer_has_changeset_for_test(&w)` assertion, and add a second
assertion in the same test that validates the storage is clean after the failure
so retry does not see duplicate-on-retry state.

In `@packages/rs-platform-wallet-storage/tests/sqlite_load_wiring.rs`:
- Around line 91-127: The round-trip test in `sqlite_load_wiring.rs` only
verifies `last_processed_height` from `state.wallets.get(&w).core_state` even
though `synced_height` is also written into `CoreChangeSet`; update the existing
load assertions to check both fields after `p2.load()` so `load()` wiring
regressions for `synced_height` are caught alongside `last_processed_height`.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: edc85543-e83f-4a54-88ef-17800859c720

📥 Commits

Reviewing files that changed from the base of the PR and between 83f7d4f and 2f2a74a.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (79)

• packages/rs-platform-wallet-ffi/src/persistence.rs
• packages/rs-platform-wallet-storage/.cargo/audit.toml
• packages/rs-platform-wallet-storage/Cargo.toml
• packages/rs-platform-wallet-storage/README.md
• packages/rs-platform-wallet-storage/SCHEMA.md
• packages/rs-platform-wallet-storage/SECRETS.md
• packages/rs-platform-wallet-storage/migrations/V001__initial.rs
• packages/rs-platform-wallet-storage/src/bin/platform-wallet-storage.rs
• packages/rs-platform-wallet-storage/src/kv.rs
• packages/rs-platform-wallet-storage/src/lib.rs
• packages/rs-platform-wallet-storage/src/secrets/error.rs
• packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs
• packages/rs-platform-wallet-storage/src/secrets/file/format.rs
• packages/rs-platform-wallet-storage/src/secrets/file/mod.rs
• packages/rs-platform-wallet-storage/src/secrets/keyring.rs
• packages/rs-platform-wallet-storage/src/secrets/mod.rs
• packages/rs-platform-wallet-storage/src/secrets/secret.rs
• packages/rs-platform-wallet-storage/src/secrets/store.rs
• packages/rs-platform-wallet-storage/src/secrets/wire/aad.rs
• packages/rs-platform-wallet-storage/src/secrets/wire/config.rs
• packages/rs-platform-wallet-storage/src/secrets/wire/envelope.rs
• packages/rs-platform-wallet-storage/src/secrets/wire/kdf.rs
• packages/rs-platform-wallet-storage/src/secrets/wire/mod.rs
• packages/rs-platform-wallet-storage/src/sqlite/backup.rs
• packages/rs-platform-wallet-storage/src/sqlite/config.rs
• packages/rs-platform-wallet-storage/src/sqlite/conn.rs
• packages/rs-platform-wallet-storage/src/sqlite/error.rs
• packages/rs-platform-wallet-storage/src/sqlite/kv.rs
• packages/rs-platform-wallet-storage/src/sqlite/migrations.rs
• packages/rs-platform-wallet-storage/src/sqlite/persister.rs
• packages/rs-platform-wallet-storage/src/sqlite/schema/accounts.rs
• packages/rs-platform-wallet-storage/src/sqlite/schema/asset_locks.rs
• packages/rs-platform-wallet-storage/src/sqlite/schema/blob.rs
• packages/rs-platform-wallet-storage/src/sqlite/schema/contacts.rs
• packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs
• packages/rs-platform-wallet-storage/src/sqlite/schema/dashpay.rs
• packages/rs-platform-wallet-storage/src/sqlite/schema/identities.rs
• packages/rs-platform-wallet-storage/src/sqlite/schema/identity_keys.rs
• packages/rs-platform-wallet-storage/src/sqlite/schema/mod.rs
• packages/rs-platform-wallet-storage/src/sqlite/schema/platform_addrs.rs
• packages/rs-platform-wallet-storage/src/sqlite/schema/token_balances.rs
• packages/rs-platform-wallet-storage/src/sqlite/schema/wallets.rs
• packages/rs-platform-wallet-storage/src/sqlite/util/safe_cast.rs
• packages/rs-platform-wallet-storage/tests/common/mod.rs
• packages/rs-platform-wallet-storage/tests/secrets_api.rs
• packages/rs-platform-wallet-storage/tests/secrets_default_on_compiles.rs
• packages/rs-platform-wallet-storage/tests/secrets_scan.rs
• packages/rs-platform-wallet-storage/tests/sqlite_account_zero_attribution.rs
• packages/rs-platform-wallet-storage/tests/sqlite_accounts_reader.rs
• packages/rs-platform-wallet-storage/tests/sqlite_asset_locks_filter.rs
• packages/rs-platform-wallet-storage/tests/sqlite_auto_backup.rs
• packages/rs-platform-wallet-storage/tests/sqlite_check_constraints.rs
• packages/rs-platform-wallet-storage/tests/sqlite_commit_writes_lock_poison_shortcircuit.rs
• packages/rs-platform-wallet-storage/tests/sqlite_compile_time.rs
• packages/rs-platform-wallet-storage/tests/sqlite_contacts_keys_rehydration.rs
• packages/rs-platform-wallet-storage/tests/sqlite_core_state_reader.rs
• packages/rs-platform-wallet-storage/tests/sqlite_dashpay_overlay_contract.rs
• packages/rs-platform-wallet-storage/tests/sqlite_delete_buffer_reconcile.rs
• packages/rs-platform-wallet-storage/tests/sqlite_delete_cross_process_exclusion.rs
• packages/rs-platform-wallet-storage/tests/sqlite_delete_partial_commit_window.rs
• packages/rs-platform-wallet-storage/tests/sqlite_delete_real_apply_failure.rs
• packages/rs-platform-wallet-storage/tests/sqlite_delete_wallet.rs
• packages/rs-platform-wallet-storage/tests/sqlite_error_classification.rs
• packages/rs-platform-wallet-storage/tests/sqlite_fk_changeset_ordering.rs
• packages/rs-platform-wallet-storage/tests/sqlite_foreign_keys.rs
• packages/rs-platform-wallet-storage/tests/sqlite_identity_keys_reader.rs
• packages/rs-platform-wallet-storage/tests/sqlite_load_reconstruction.rs
• packages/rs-platform-wallet-storage/tests/sqlite_load_wiring.rs
• packages/rs-platform-wallet-storage/tests/sqlite_migrations.rs
• packages/rs-platform-wallet-storage/tests/sqlite_money_column_overflow_on_read.rs
• packages/rs-platform-wallet-storage/tests/sqlite_object_metadata.rs
• packages/rs-platform-wallet-storage/tests/sqlite_open_integrity_check.rs
• packages/rs-platform-wallet-storage/tests/sqlite_persist_roundtrip.rs
• packages/rs-platform-wallet-storage/tests/sqlite_qa_identity_tombstone.rs
• packages/rs-platform-wallet-storage/tests/sqlite_second_open_guard.rs
• packages/rs-platform-wallet-storage/tests/sqlite_structural_hardening.rs
• packages/rs-platform-wallet-storage/tests/sqlite_wallet_db_identity.rs
• packages/rs-platform-wallet/src/changeset/client_wallet_start_state.rs
• packages/rs-platform-wallet/src/manager/load.rs

[thepastaclaw]

Code Review

Incremental delta (29bbb14→bc9680e5, a v4.1-dev merge plus the KDF-consolidation/opt-level commits) is clean and test-only — verified. Reconciling scope against the true PR/target merge-base confirmed one carried-forward finding (V003 account_type unconstrained) is genuinely PR-authored and still valid, convergently flagged by both reviewers with verified mechanics (raw string flows unvalidated into OwningAccount; an unmatched value silently falls back to account 0). The other carried-forward finding (load.rs error flattening) is confirmed byte-identical to the actual v4.1-dev target-branch tip this PR is based on — it's inherited base code this PR never touches, not a PR-introduced issue, so it's dropped as out of scope rather than carried forward. Two new in-scope issues surfaced from reconciling the latest merge against this PR's own code: versions.rs silently discards the newly-merged provider_key_account_registrations field with no persistence and no runtime signal (disclosed via comment, tracked in #4113, but absent from the PR's own 'Deferred' list); and backup.rs's restore path has a real, narrow gap beyond its own documented lock-release trade-off, where a fresh peer writing into the just-renamed destination in the microsecond window before sibling cleanup can have its WAL unlinked. No blocking issues — action is COMMENT.

Source: reviewers — Sol reviewer codex (gpt-5.6-sol, general, high, ok) and Sonnet reviewer sonnet5 (claude-sonnet-5, general, high, ok); verifier — sonnet5 (claude-sonnet-5, primary, ok); orchestrator — openai/gpt-5.6-sol (high, orchestration-only; not reviewer/verifier). Experiment sonnet-primary-opus-quarter-sample-20260710, cohort sonnet_primary, bucket 1; Opus not sampled.

🟡 3 suggestion(s)

3 additional finding(s) omitted (not in diff).

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet-storage/migrations/V003__unified.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/migrations/V003__unified.rs:45: Constrain core_address_pool.account_type to the known label domain
  account_type is TEXT NOT NULL with no CHECK, unlike its sibling columns pool_type IN (0,1,2,3) (line 50) and used IN (0,1) (line 53). Verified in sqlite/schema/core_pool.rs: readers (lines 158-159, 249-250) pull this column straight into OwningAccount with zero validation. Verified in sqlite/util/wallet.rs: route_to_funds_account (line 282) matches an unspent UTXO's or used-address's owner against the wallet's known account_keys by equality; any value that doesn't match — including a corrupted/unrecognized label — falls through to .unwrap_or_else, silently attributing the UTXO or used-address to account 0 (only a tracing::warn! is emitted, no hard error). This defeats the address-reuse guard this PR's rehydration path exists to protect, and is inconsistent with the fail-hard-on-corruption contract the rest of load() enforces elsewhere. This is a genuinely new (V003) PR-authored table, not inherited code, so it's fully in scope.

In `packages/rs-platform-wallet-storage/src/sqlite/schema/versions.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/versions.rs:96-112: provider_key_account_registrations is destructured but never persisted anywhere in the storage backend
  The v4.1-dev merge folded in PR #4072's provider_key_account_registrations field (BLS operator-key / EdDSA platform-node-key account registrations) to PlatformWalletChangeSet — confirmed via git log -S: the field-adding commit c6b073f805 postdates the prior review's head (29bbb14b) and only entered this branch through the latest first-parent merge. touched_domains' exhaustive destructure (the crate's R8 'forgotten-domain' compile guard) reconciles it with `let _ = provider_key_account_registrations;`. A repo-wide grep of rs-platform-wallet-storage confirms this is the field's ONLY reference in the crate — no writer persists it, no reader restores it, and unlike the account_type fallback above, not even a tracing::warn! fires when it's discarded on every save. The code's own comment is honest about the deferral and cites #4113, and there's no regression since this is a brand-new crate, but it cuts against this PR's stated purpose ('this PR is the storage half' of durable wallet persistence) for a key type the comment itself calls default-on and live. The PR description's own 'Deferred (TODO-marked, no regression)' section lists only manifest authentication (#3992) and orphaned-wallet-row recovery — this deferral isn't there, consistent with it only appearing once v4.1-dev was merged in after the description was written.

In `packages/rs-platform-wallet-storage/src/sqlite/backup.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/backup.rs:264-295: Sibling WAL/SHM cleanup can unlink a fresh peer's post-restore commit
  restore_from's own doc comment (lines 147-155) already discloses and accepts one race: the EXCLUSIVE lock is dropped before the rename so a peer that was already waiting on the OLD inode can write into it right as it's superseded, losing that write ('nothing escalates', by design — correct rename semantics were judged to outweigh full lock coverage). Verified this is a real, deliberate, documented trade-off, not an oversight. There's a second, undocumented gap in the same window: step 9 (lines 284-295) unlinks `<dest>-wal`/`<dest>-shm` by path, not by inode or timestamp, immediately after the atomic rename (step 8, line 276) lands the restored bytes. A fresh peer process that opens `dest_db_path` for the first time in the gap between the rename and this cleanup — not a waiter on the old inode, but a new connection to the just-restored file — would be writing into the correct, current database, and a commit in that window creates a `-wal` file that step 9's unconditional unlink then deletes, silently discarding a legitimate post-restore write rather than a stale pre-restore one. The existing cross-process tests (sqlite_restore_cross_process_exclusion.rs) only cover a peer that already holds EXCLUSIVE and post-restore lock re-acquisition; neither exercises this unlocked rename-to-cleanup window. This is real and in-scope (backup.rs is almost entirely PR-authored per its diff history), but it's an extension of a risk category the authors already accepted for an adjacent scenario, doesn't corrupt data, and needs a sub-millisecond adversarial timing window against a second process — that combination puts it below blocking severity. A fix would need siblings to be identified by something other than bare path (e.g. only unlink ones that predate the rename, or hold a lighter interlock through step 9) rather than a one-line patch, hence no inline suggestion.

[thepastaclaw]

Preliminary review — Codex only

The asset-lock wire-format fix is sound, but the load-failure cleanup introduces a blocking durability regression: the Swift app reuses a manager after restore failure even though its sole wallet-event persistence adapter has been permanently stopped. The two carried-forward storage suggestions remain valid; the provider-key registration gap is intentionally deferred to #4113.

Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.

Review provenance

• Codex reviewers: gpt-5.6-sol — general (failed), gpt-5.6-sol — general (failed), gpt-5.6-sol — general (failed), gpt-5.6-sol — general (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 1 blocking | 🟡 2 suggestion(s)

3 additional finding(s) omitted (not in diff).

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet/src/manager/load.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/manager/load.rs:42: A recoverable load error permanently disables event-driven core persistence
  load_from_persistor(&self) calls shutdown() after both initial persister errors and later hydration errors (line 206). shutdown() permanently cancels the sole wallet-event adapter and consumes its join handle, with no restart path. The FFI leaves the manager handle valid, while WalletManagerStore.activate catches restore failures as non-fatal, caches that same manager, and explicitly permits subsequent wallet creation or import. Later wallet events therefore no longer reach persister.store, so UTXOs, transaction records, address-use state, and sync heights can be lost across restart. Direct persistence such as wallet registration still works, making the missing event-driven updates especially silent. Keep the adapter operational across recoverable load failures, make it restartable, or invalidate the manager so callers must reconstruct it.

In `packages/rs-platform-wallet-storage/migrations/V003__unified.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/migrations/V003__unified.rs:45: Constrain core_address_pool.account_type to the known label domain
  account_type accepts arbitrary text even though this database is treated as untrusted and the equivalent account_registrations column is constrained to ACCOUNT_TYPE_LABELS. Pool readers copy the raw label into OwningAccount; an unknown label cannot match any reconstructed funds account, so route_to_funds_account warns and silently assigns the associated UTXO or used address to account 0. Add the same label-domain constraint used by account_registrations so semantic corruption fails during insertion or migration rather than restoring funds and address-reuse state under the wrong account.

In `packages/rs-platform-wallet-storage/src/sqlite/backup.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/backup.rs:264-295: Sibling WAL/SHM cleanup can unlink a fresh peer post-restore commit
  The destination lock is released before the restored database is renamed into place, and the destination -wal and -shm paths are removed only afterward. If the restore thread is descheduled after the rename, a peer can open the newly restored database, create current WAL/SHM files, and commit before cleanup resumes. On Unix, cleanup then unlinks those live sidecars; another connection can create different sidecars, and a crash before checkpoint can lose the peer committed transaction. Existing tests cover peers attached before restore and lock acquisition after restore, but not this rename-to-cleanup interval.

[thepastaclaw]

Preliminary review — Codex only

At the exact head, the asset-lock rehydration fix introduces a blocking lifecycle bug: a failed load leaves the retained manager unable to persist later core-wallet events. Two prior storage findings remain valid; provider-key persistence is deferred to #4113, and the FFI retry-classification claim is not actionable on the current fatal-only callback backend.

Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.

Review provenance

• Codex reviewers: gpt-5.6-sol — general (completed), gpt-5.6-sol — ffi-engineer (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 1 blocking | 🟡 2 suggestion(s)

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet/src/manager/load.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/manager/load.rs:42: Keep the manager operational when a retryable load fails
  Every persister load error calls `self.shutdown()`; the hydration rollback path repeats this at line 206. `shutdown()` cancels the one-shot wallet-event adapter and consumes its join handle, while only `PlatformWalletManager::new` can spawn it. The FFI merely borrows the manager, and Swift catches load failures as non-fatal and caches that same manager. It can then operate without persisting core `WalletEvent` changes such as UTXOs, records, used addresses, and sync watermarks. Keep the adapter running, make it restartable, or invalidate the FFI handle and require reconstruction.

In `packages/rs-platform-wallet-storage/migrations/V003__unified.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/migrations/V003__unified.rs:45: [prior-account-type-domain] Constrain core_address_pool.account_type to known labels
  `account_type` remains unconstrained `TEXT NOT NULL`. The core-pool readers copy it directly into `OwningAccount`, and `wallet.rs:282-296` routes an owner absent from the reconstructed manifest to account 0 with only a warning. An unknown label therefore bypasses the fail-hard corruption contract and can misattribute UTXOs and used addresses. Add a schema constraint or reject unknown labels while reading.

In `packages/rs-platform-wallet-storage/src/sqlite/backup.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/backup.rs:284-295: [prior-restore-wal-cleanup-race] Do not unlink a fresh peer's post-restore WAL
  Restore releases its EXCLUSIVE transaction at lines 264-267, renames the restored database at line 276, then removes the destination's `-wal` and `-shm` paths. A peer can open the new database during that unlocked interval and create a current WAL; cleanup cannot distinguish it from a stale sibling and may unlink a legitimate post-restore commit. Keep cleanup interlocked or remove only siblings known to predate the swap.

[thepastaclaw]

🔴 Blocking: Keep the manager operational when a retryable load fails

Every persister load error calls self.shutdown(); the hydration rollback path repeats this at line 206. shutdown() cancels the one-shot wallet-event adapter and consumes its join handle, while only PlatformWalletManager::new can spawn it. The FFI merely borrows the manager, and Swift catches load failures as non-fatal and caches that same manager. It can then operate without persisting core WalletEvent changes such as UTXOs, records, used addresses, and sync watermarks. Keep the adapter running, make it restartable, or invalidate the FFI handle and require reconstruction.

_{source: ['codex']}

[thepastaclaw]

Resolved in 884a6cd — Keep the manager operational when a retryable load fails no longer present.

_{Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.}

[thepastaclaw]

🟡 Suggestion: [prior-account-type-domain] Constrain core_address_pool.account_type to known labels

account_type remains unconstrained TEXT NOT NULL. The core-pool readers copy it directly into OwningAccount, and wallet.rs:282-296 routes an owner absent from the reconstructed manifest to account 0 with only a warning. An unknown label therefore bypasses the fail-hard corruption contract and can misattribute UTXOs and used addresses. Add a schema constraint or reject unknown labels while reading.

_{source: ['codex']}

[thepastaclaw]

Correction: this finding remains valid at commit 7956bb83; the latest verifier kept it as a suggestion. The previous automated reconciliation reply matched a title variant incorrectly. The thread was not resolved and remains part of the preliminary review.

_{Canonical finding: [carried-forward: prior-account-type-domain] Constrain core_address_pool.account_type to known labels.}

[thepastaclaw]

🟡 Suggestion: [prior-restore-wal-cleanup-race] Do not unlink a fresh peer's post-restore WAL

Restore releases its EXCLUSIVE transaction at lines 264-267, renames the restored database at line 276, then removes the destination's -wal and -shm paths. A peer can open the new database during that unlocked interval and create a current WAL; cleanup cannot distinguish it from a stale sibling and may unlink a legitimate post-restore commit. Keep cleanup interlocked or remove only siblings known to predate the swap.

_{source: ['codex']}

[thepastaclaw]

Correction: this finding remains valid at commit 7956bb83; the latest verifier kept it as a suggestion. The previous automated reconciliation reply matched a title variant incorrectly. The thread was not resolved and remains part of the preliminary review.

_{Canonical finding: [carried-forward: prior-restore-wal-cleanup-race] Do not unlink a fresh peer's post-restore WAL.}

[lklimek]

Fix: AssetLockProof blob rehydration failure (#4133)

Commits ad0164c3f4..d18020f526 on this branch fix a bug where AssetLockProof blobs (and other internally-tagged serde enums) could be written via bincode::serde::encode_to_vec but never read back — bincode's serde bridge can't service deserialize_any, which internally-tagged enums (#[serde(tag = "...")]) require. In dash-evo-tool, this manifested as "chain sync never starts" on every relaunch, permanently, the moment any asset lock existed in the wallet DB.

Primary fix: AssetLockEntryWire (sqlite/schema/asset_locks.rs) — mirrors the existing IdentityKeyWire pattern (identity_keys.rs) by pre-encoding the hazardous proof field via dpp's own native bincode Encode/Decode derive, bypassing the serde bridge for just that field while the rest of the struct stays on the generic serde path.

Also included:

• The wallet-event adapter now releases its persister Arc on a failed load (Drop backstop + explicit shutdown() on every load-error path), so a same-process retry hits the real decode error instead of a misleading AlreadyOpen.
• PlatformWalletError::PersisterLoad typed variant, logged with Debug (not Display), so the real error is actually visible going forward.
• Definition-site doc comments on AssetLockProof, IdentityPublicKey, and DataContractConfig pinning the native-vs-serde-bridge hazard where the types are defined, not just at the storage call site — see docs/asset-lock-proof-serde-misuse-guard-design.md for the full writeup, including why an is_human_readable()-based blocking guard turned out to be infeasible (it would break the ValueConvertible path, which relies on the same false-negative flag).
• Full round-trip coverage for both AssetLockProof variants (Instant/Chain), including non-default/non-empty nested fields — the original bug's blind spot (InstantLock::default()'s empty inputs never exercised the real encoding path).

No migration: pre-release, so there's no production data to preserve. Any already-written incompatible row simply won't decode until cleared, same as before this fix, but now with an accurate typed error instead of a silent generic failure.

Fixes #4133.

Companion PR (doc-only, general convention so this class of bug doesn't recur elsewhere): see docs/dpp-serialization-convention branch.

[thepastaclaw]

Preliminary review — Codex only

Carried-forward prior findings: three remain valid—one blocking manager-lifecycle defect and two storage-integrity suggestions concerning account-type validation and post-restore WAL cleanup. New latest-delta findings: none; the migration reconciliation, regenerated fixture, and test updates introduce no additional actionable issue, and target-branch-only changes were excluded.

Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.

Review provenance

• Codex reviewers: gpt-5.6-sol/high — general (failed_before_session), gpt-5.6-sol — general (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 1 blocking

3 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

[thepastaclaw]

Preliminary review — Codex only

At exact head 7956bb8, current-source validation keeps one blocking lifecycle defect and two storage suggestions, all carried forward from the prior review; no new latest-delta finding was established. The manager remains reusable after its persistence machinery is shut down, while the account-domain and post-restore WAL issues remain present; provider-key persistence remains explicitly deferred to #4113.

Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.

Review provenance

• Codex reviewers: gpt-5.6-sol — general (completed), gpt-5.6-sol — ffi-engineer (failed), gpt-5.6-sol — ffi-engineer (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 1 blocking

2 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet/src/manager/load.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/manager/load.rs:42: [carried-forward: prior-manager-load-shutdown] Keep the retained manager operational after load failure
  Both the initial persister error path here and hydration rollback at line 206 call `self.shutdown().await`. That permanently cancels the wallet-event adapter and consumes its join handle in `manager/mod.rs:381-385`; only `PlatformWalletManager::new` spawns the adapter. The FFI load entry point merely borrows the manager and returns the error, while `WalletManagerStore.activate` catches that error, caches the same manager at lines 206-207, and returns the cached instance on later same-SDK activation at lines 162-175. Wallet operations can therefore continue through a live handle while core `WalletEvent` changes no longer reach the persister, silently losing UTXO, transaction, used-address, and sync-watermark updates. Keep or restart the callback machinery on recoverable use, or invalidate the FFI handle so callers must reconstruct the manager.

[thepastaclaw]

🔴 Blocking: [carried-forward: prior-manager-load-shutdown] Keep the retained manager operational after load failure

Both the initial persister error path here and hydration rollback at line 206 call self.shutdown().await. That permanently cancels the wallet-event adapter and consumes its join handle in manager/mod.rs:381-385; only PlatformWalletManager::new spawns the adapter. The FFI load entry point merely borrows the manager and returns the error, while WalletManagerStore.activate catches that error, caches the same manager at lines 206-207, and returns the cached instance on later same-SDK activation at lines 162-175. Wallet operations can therefore continue through a live handle while core WalletEvent changes no longer reach the persister, silently losing UTXO, transaction, used-address, and sync-watermark updates. Keep or restart the callback machinery on recoverable use, or invalidate the FFI handle so callers must reconstruct the manager.

_{source: ['codex']}