feat(wallet-storage)!: restore complete Core wallet snapshots from SQLite

[lklimek]

TL;DR

Reopening a SQLite-backed wallet restores its Core address reservations and used-key positions, so previously handed-out funding keys are not offered again.

User story

As a wallet user, I can restart the app with my saved Core balances, pending spends and address reservations intact, and receive an error when required saved data cannot be restored.

Scenario

Use funding addresses or reserve a new address, close the wallet, and reopen the same database. The wallet restores the used and reserved addresses before allocating another key. While a wallet is loading, a write from another connection is refused, so that wallet's state comes from one snapshot.

Detailed discussion

Dependencies and overlap

Stacked on #5207 (fix/sqlite-asset-lock-reconciliation), which is stacked on #5150. The history replay, pending-spend reservations, accounting repair and migrations of #5150 are reused unchanged; this PR is limited to SQLite address-pool restoration, the capability declaration, typed load errors and focused tests.

What was done?

• Restore persisted address-pool entries across managed Core account roles, including special funding keys and provider key pools. Preserve used state, reservation timestamps, derivation positions and reverse lookups.
• Preserve existing per-address accounting and reject conflicting typed keys, addresses or derivation indices.
• Restore the pools before history replay and before the stored coins mark their addresses used, so a funded address restored beyond the freshly derived window is not handed out again.
• Check every restored row against its derivation: each pool is derived from the account's public key, and a row whose key type, address, key or index contradicts the derived entry fails the wallet with a typed error. The derivation is budgeted by the pool's stored row count plus 10,000 uncovered indices, so a pool of any real depth restores in full and a damaged index cannot force unbounded work. A row the budget cannot reach is not restored and is counted as pool_row_beyond_derivation_budget; a stored reservation that no restored pool takes back is counted as unrestored_pool_reservation. Both are reported under Strict and Recovery and do not fail the wallet, except for one-time funding key accounts (next item). Hardened pools (platform-node keys) cannot be re-derived from public material and stay trusted as stored.
• Fail the wallet when a used or reserved row of a one-time funding key account (identity registration, top-up, unbound top-up, invitation, asset-lock address and shielded top-up) is not taken back by a restored pool: Strict fails the load, Recovery drops and names that wallet. Handing such a key out again would reuse a one-time funding key.
• Refuse a row filed under a pool its account does not have, instead of silently dropping the reservation it carries.
• Refill a restored pool's gap window. A pool whose stored rows already mark addresses used gets the window past them derived during load, as before pools were restored first. Without it the coin on such an address stayed in the balance, but the addresses past the saved rows were not derived, so they were unobserved and a funded one could later be handed out as fresh. Not verified: whether such a coin could be spent while its address was missing from the pool, and live SPV sync on that state.
• Count load incidents once per wallet. When startup replay repair rebuilds a wallet, the per-site degradation counts describe the wallet that is returned. Counters that predate this PR, such as unresolved_utxo_address, also stop doubling during startup repair.
• Log where a failure sits. A refused pool row logs its wallet, account type, pool type and index; under Strict the load logs the wallet id and error kind of any wallet it could not rebuild. Addresses and keys are not logged.
• Known limitation: for every other account, a reservation stored on a row that matches no restored account is reported as unrestored_pool_reservation, and a row beyond the derivation budget as pool_row_beyond_derivation_budget; neither is restored, so that address can be handed out again until a coin on it is seen.
• Known limitation: a pool row that was deleted outright cannot be detected: the row is the only record that a key was handed out.
• Known limitation: a wallet that rebuilds no account restores no pool and is not checked.
• Preserve typed storage errors through wallet reconstruction; under Recovery a dropped wallet is attributed to that error's kind instead of backend.
• Loading relies on the per-wallet BEGIN IMMEDIATE transaction introduced by fix(platform-wallet)!: restore Core spending state and repair persisted accounting #5150: a writer on another connection is refused while a wallet loads, so its records, spend claims, coins, pools and sync checkpoints come from one snapshot. The guarantee is per wallet, not per load: platform address state, the wallet id list, shielded viewing keys and the unimplemented-row count are read outside those transactions.
• Declare the existing WALLET_RESTORE capability after completing its Core-snapshot restore path. Capability meanings and bit values remain unchanged. PROVIDER_TRANSACTIONS is not newly declared.
• Keep the existing watch-only restore model; no secret import, new schema migration or automatic transaction resend is introduced.

WALLET_RESTORE is the existing persisted Core wallet snapshot contract. Separate token caches, DashPay profile overlays, deferred contact queues and invitation UI records are not made complete by this flag; their readback remains separate. DET's #1036 pin remains on the narrower #5207. Declaring it lets SQLite hosts pass preflights that require it, such as funding a shielded pool from an asset lock.

Out of scope

• A healthy wallet that has burned one-time funding keys reports orphaned_utxo_owner and unresolved_utxo_address on every load: the used-address projection from fix(platform-wallet)!: restore Core spending state and repair persisted accounting #5150 routes addresses of key-only accounts to the first funds account. The keys themselves are restored correctly; only the report is noisy.
• validate_account_type still admits standard for pool rows although no writer produces it; it belongs to the reader introduced with feat(platform-wallet-storage): embeddable SQLite persistence backend with seedless rehydration #3968.
• The wallet-wide audit of one-time funding key rows reads the two identity columns without the length() gate the other readers use. A mis-sized row is still refused; the cost is one transient copy bounded by the connection's 32 MiB value limit.
• ONE_TIME_FUNDING_LABELS repeats the account filter of the asset-lock writer in platform-wallet; a test fails when the two differ. A typed predicate exported from platform-wallet would replace both.

How Has This Been Tested?

• The full platform-wallet-storage suite passes with all features and the locked dependency graph: 1,106 tests, 0 failed. Clippy with --all-targets --all-features --locked -- --no-deps -D warnings is clean for platform-wallet-storage and platform-wallet; formatting check passes.
• sqlite_wallet_restore.rs covers reopen and real manager hydration, special funding-key non-reuse, reservation timestamps, exact pending-spend and per-address accounting, malformed-key rejection before partial manager load, a typed platform-node pool failure under Strict and Recovery, a writer refused as busy while a wallet's load transaction is open, and ProReg/ProUpReg key-account history and positions. It also covers a funded address beyond the derived window, a foreign script stored beyond that window, a row filed under a missing pool, two DashPay contact pools sharing an index, and each pool-conflict guard under Strict and Recovery. It also covers the pools of a standard account registered under the legacy label, and a used one-time funding key row that no restored pool takes back. Unit tests pin that restoring pools twice leaves per-address accounting unchanged and that a row the derivation budget cannot reach is skipped and counted, with nothing derived past the budget.
• The busy-writer, typed-failure and pool-guard tests were each shown to fail when the behaviour they guard is removed.
• Not run: platform-wallet tests (its only change is two doc comments), the full workspace, live-network or user-funds testing. No Swift file differs from the base.

Breaking Changes

Rust source only: platform_wallet_storage::LoadSite gains PoolRowBeyondDerivationBudget and UnrestoredPoolReservation and becomes #[non_exhaustive]; code matching it exhaustively needs a wildcard arm. No persisted encoding, FFI layout or capability bit changes.

Checklist

• Self-review performed
• Existing PRs checked and overlapping work reused
• Reopen and real manager-hydration regression coverage
• Validation limitations documented

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

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

• Configuration used: Repository: dashpay/platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: c8715fbc-399f-4ed6-b539-4f1e32b4b6f4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[thepastaclaw]

✅ Final review complete — no blockers (commit ce03450)

[thepastaclaw]

Final validation — Phase 1 + Phase 2

Static verification at the exact head confirms three pool-restoration correctness gaps, plus targeted coverage and startup-cost improvements. These client-side issues do not violate consensus invariants and are classified as suggestions under the supplied severity policy. No builds or tests were run; the supplied CI snapshot shows successful completed checks but no dedicated Rust wallet-storage test result.

🟡 6 suggestion(s) | 💬 1 nitpick(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The intricate restore_pools implementation in packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs changes key handling by reconstructing typed key pools, derivation indices, used-key positions and funding-address reservations.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 14% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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/src/sqlite/persister.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/persister.rs:1881-1882: Apply the UTXO-derived reuse guard after restoring persisted pools
  The overlay runs after `apply_persisted_core_state` has already applied the used-address set to the reconstructed pools. A valid Standard external-pool snapshot with indices 0–39 Reserved and index 40 Available, followed by a recordless UTXO on index 40, exposes the ordering problem: initial derivation covers 0–29, and discovery stops at index 30 because no used address anchors a deeper horizon. The overlay then inserts index 40 as Available because there is no existing entry whose Used state it can preserve. Recordless UTXOs are supported by the writer, and history replay returns immediately when there are no records, so the next allocation can return the funded address. Restore pools before applying the reuse guard, or reapply the known-used address set after the overlay. Add a reopen regression with reservations beyond the initial gap and a recordless UTXO on a saved available address.

In `packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs:605-615: Validate derivation for entries outside the already-generated pool
  Address/key consistency is checked only when the reconstructed pool already contains the persisted index. Special funding-key pools initially derive just their gap window and are not extended by the funds-account projection, so deeper persisted entries bypass this check. A corrupted row carrying another child's valid P2PKH script and correctly sized ECDSA key is accepted and assigned the recorded index's derivation path. The pinned key-wallet allocation methods return cached Available entries without deriving them again, while signing derives from the recorded path; the wallet can therefore return an address its corresponding signing key does not control. Pass the registered account key material into restoration and independently derive and compare every publicly derivable persisted index before insertion, including indices absent from the skeleton. Preserve explicit handling for hardened entries that cannot be regenerated from public material, and test valid-width beyond-window mismatches under Strict and Recovery.
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs:503-505: Reject persisted rows whose pool discriminator cannot be restored
  Selecting rows separately for each reconstructed pool means rows with an incompatible discriminator are never validated. For example, changing a reserved `identity_invitation` row from Absent (2) to External (0) passes the database CHECK constraint, but this account exposes only an Absent pool. The overlay skips the row, and the earlier used-address reader also skips it because `used = 0`. Strict load succeeds with the skeleton entry Available, silently losing the reservation and allowing the same funding key to be allocated again. Validate that saved rows for registered accounts map to an actual managed pool and return a typed error for incompatible discriminators. Add a regression proving that an impossible pool discriminator cannot discard a reservation during a successful Strict load.
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs:660-663: Compute the highest used index once per restored pool
  The pinned dependency stores `used_indices` in a HashSet, so `iter().max()` scans the accumulated set on every restored row, including unused rows. A pool with N used entries therefore incurs quadratic work during restoration while the connection mutex and per-wallet IMMEDIATE transaction remain held. No used index is removed here, so maintain the maximum incrementally or compute it once after the row loop.
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs:605-625: Exercise the new pool-conflict guards under Strict and Recovery
  The new account/key-type mismatch, derived address-or-key conflict and duplicate-address-across-indices errors have no direct regression coverage. The malformed-key test exercises a width check, and the provider wrong-curve test fails earlier in `restore_provider_platform_node_pool`, so neither verifies these overlay guards or their policy handling. Add fixtures that reach each guard: assert Strict preserves the typed storage error, and Recovery isolates the affected wallet with the correct degradation kind while retaining a healthy sibling wallet.
- [NITPICK] packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs:500: Reuse the identical restore statement across pools
  Every account/pool iteration prepares the same SELECT text with different bound parameters. Using `prepare_cached`, as the repeated-shape writer and ownership query already do, avoids repeated SQL preparation during wallet loading. This is a small startup optimization, separate from the per-row quadratic scan.

In `packages/rs-platform-wallet-storage/tests/sqlite_wallet_restore.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/tests/sqlite_wallet_restore.rs:160-179: Add DashPay contact-pool restore coverage
  The reopen test creates no DashPay accounts, so the new restore query's user/friend identity bindings are exercised only with zero sentinels. Existing contact tests cover writer separation and registration migration, not restoration of used and reserved pool entries for multiple contacts. Two contacts can share the same account label, numeric index and key class, making the identity pair essential to selecting the correct saved pool. Add a reopen test with two contacts at the same numeric index and distinct non-zero identity pairs, then assert their used states, reservation timestamps and addresses restore independently without cross-contact overwrite.

Out-of-scope follow-up suggestions (1)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

• Validate restored HD entries through the key-wallet owner — The confirmed beyond-window validation defect is merged into 'Validate derivation for entries outside the already-generated pool'. Requiring an upstream restoration API and dependency-pin update is not established as necessary; the essential requirement is independent validation using the account key material.
  • Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

All seven prior findings are fixed at the reviewed head. Three non-blocking findings remain: unmatched pool coordinates can silently discard reservations, the restoration ceiling rejects valid snapshots the writer accepts, and beyond-window BLS restoration lacks regression coverage. This was a static review without builds or test execution; the supplied CI snapshot does not independently confirm the reported wallet-storage test and clippy results.

🟡 3 suggestion(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The intricate restoration logic in core_pool.rs::restore_pools and derive_through changes key handling by validating persisted public keys against derived entries and restoring reservation and used-key positions that determine which funding addresses can be handed out again.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 14% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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/src/sqlite/schema/core_pool.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs:564-568: Reject pool rows with impossible account discriminators
  The pool-type preflight and both restoration queries filter by the reconstructed account's complete coordinates before examining saved rows. A reserved IdentityInvitation row with a nonzero key_class satisfies the SQLite schema but cannot match any IdentityInvitation account: accounts::account_key_class always returns zero for that variant. With no transaction history or UTXO on the address, the used-address reader also excludes the row because used is false. Strict loading consequently succeeds while leaving the reconstructed address Available, silently losing its reservation and permitting allocation again. Validate saved pool coordinates against the reconstructed account set before applying these filters, returning a typed error for impossible or unmatched coordinates. Add Strict and Recovery coverage that verifies rejection and isolation of the affected wallet.
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs:525-529: Preserve restoration compatibility for valid indices above the discovery ceiling
  The documented 10,000-index bound protects restoration from unbounded sequential derivation, but it also introduces a store/load incompatibility. Neither the core_address_pool schema nor apply_pools restricts valid child indices to this ceiling, so a correctly derived Standard pool snapshot containing index 10,001 can still be stored successfully. On reopen, this new check fails Strict loading and excludes the entire wallet under Recovery. The base loader accepted such databases and deferred unresolved deep addresses to synchronization rather than rejecting the wallet. Preserve bounded validation without treating supported historical indices as corruption, or explicitly reconcile the write contract and compatibility policy with the new restriction. Add a reopen regression using correctly derived entries just above the ceiling; the existing i32::MAX corruption test does not cover this case.

In `packages/rs-platform-wallet-storage/tests/sqlite_wallet_restore.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/tests/sqlite_wallet_restore.rs:132-135: Exercise BLS pool restoration beyond the initial derivation window
  This fixture obtains key material only through account_of_type, so ProviderOperatorKeys takes the fallback branch instead of generating beyond-window entries. It changes only indices 0 and 1 in the initial operator pool, allowing derive_through to return before exercising the new KeySource::BLSPublic expansion path. The provider-history test also uses operator index 0, and the post-reopen allocation assertion here is skipped when source is None. Obtain the operator source through bls_account_of_type and KeySource::BLSPublic in both source-selection sites, then persist and reopen an operator reservation beyond its initial window. This lets the existing state, address, path, key, and non-reuse assertions detect regressions in BLS pool expansion.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Verified the complete diff at 921dbb8: nine prior findings are fixed, while reservation loss for type-impossible account coordinates remains valid. Five in-scope suggestions remain, including a newly confirmed address-reuse regression; none violates the consensus-critical invariants that the supplied severity policy classifies as blocking. Scoped wallet CI passed at this exact head; this verification was static, with no local builds or tests run.

🟡 5 suggestion(s)

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

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The intricate restore_pools and derive_towards changes in packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs directly alter key derivation, address validation, and used-key and reservation restoration that prevent funding-key reuse.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 14% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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/src/sqlite/persister.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/persister.rs:1874-1875: Run the initial gap refill even when restored entries are already Used
  The restore-before-projection ordering fixes saved beyond-window addresses, but it also changes the existing mark/refill loop's starting state. Consider a Standard external pool saved through index 29 with index 20 Used, plus a recordless UTXO at index 45 that has no saved pool row. After this overlay, discovery excludes index 20 because it is already present and stops at index 30 without resolving index 45. In rehydrate.rs:1298-1306, mark_used(20) returns false because the entry is already Used, and mark_used(45) returns false because it is absent, so the loop exits without refilling. The exact pinned key-wallet implementation confirms both return conditions. The base loader instead marked index 20, refilled through index 50, then marked index 45 on the next iteration. The new path can eventually derive and reserve funded index 45 as Available, and recordless history cannot repair it. Preserve the new ordering, but execute an initial bounded refill using the restored highest_used even when no state flips, then continue marking newly derived entries to a fixed point. Add a reopen regression combining saved Used entries with a recordless UTXO outside the saved pool; the existing direct wedge-zone test starts without the overlay.

In `packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs:859-867: Count unrestored pool rows once across replay repair
  These counts are added immediately to the shared LoadCtx, but load_one_wallet calls load_wallet_snapshot twice when startup history replay produces repair sweeps (persister.rs:1693-1698). Both reconstructions run restore_pools with the same context, and apply_replay_sweeps does not change core_address_pool. One unmatched reservation is consequently counted twice under UnrestoredPoolReservation; an unreachable pool row is likewise counted twice under PoolRowBeyondDerivationBudget. This contradicts LoadDegradation's documented per-row/per-entry semantics under both policies. Accumulate these diagnostics once per load or deduplicate them by persisted row coordinates across repair passes. Add coverage combining an unmatched reservation with history that triggers startup repair.
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs:775-777: Retain row coordinates when reporting typed pool failures
  The new conflict errors preserve their type but contain only a static reason. restore_pools propagates them directly, and the Strict wallet-isolation boundary returns the original error without logging its wallet or row coordinates. A failed load therefore identifies neither the affected wallet nor the account, pool and index that need repair, although all are available at this guard. Retain the typed source and classification while adding structured coordinates to a diagnostic before propagation or to a contextual typed error. Apply the same treatment to the account/key-type and duplicate-address guards.
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs:847-851: Reject pool rows with impossible account discriminators
  (existing thread: https://github.com/dashpay/platform/pull/5210#discussion_r4248454211)
  The compatibility policy correctly preserves representable legacy or orphaned accounts, but it also accepts coordinates that those cases cannot produce. accounts::account_key_class always returns zero for IdentityInvitation, apply_pools uses that mapping, and V008's legacy pool conversion delegates to the canonical writer. An invitation reservation with key_class = 1 is therefore distinguishable from a legacy standard label or an absent contact account. The new should_report_a_reservation_that_no_restored_pool_takes_back regression confirms that both policies load the wallet with this previously reserved address Available; reporting the loss does not prevent allocation again. Add account-type coordinate validation before account-filtered restoration queries hide malformed rows. Reject only type-impossible sentinel combinations with a typed error, preserving report-only handling for representable unmatched coordinates. Strict should reject the malformed wallet and Recovery should isolate it.

In `packages/rs-platform-wallet-storage/src/sqlite/load_ctx.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/load_ctx.rs:77-81: Document the Rust source break from extending LoadSite
  LoadSite is publicly re-exported, and the v5.0.0-beta.2 tag contains the previous exhaustive enum without #[non_exhaustive]. Adding these variants breaks downstream exhaustive matches even though persisted encodings and FFI layouts are unchanged. The PR's blanket Breaking Changes: None statement and the current Unreleased changelog omit this Rust API change. Document the two new cases and the required downstream match updates in the compatibility discussion and release notes. This belongs to the normal crate/workspace release boundary, not PlatformVersion or PERSISTENCE_CAPABILITIES_VERSION; appending the variants or adding #[non_exhaustive] now would still break existing exhaustive matches.

Out-of-scope follow-up suggestions (2)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

• Isolate exported invitation private keys from their persisted parent xpub — The existing invitation flow exports a non-hardened funding child's private key, while account registration persists its parent extended public key and chain code. An invitation recipient who also obtains a readable database or backup can recover the invitation account private key using the BIP32 non-hardened derivation relation, then derive sibling voucher keys. The export, derivation and parent-xpub persistence mechanisms predate this diff; the SQLite restoration changes do not introduce or worsen them.
  • Follow-up: Track a separate security follow-up for hardened or independently generated voucher keys, including compatibility handling for existing invitations and a regression demonstrating sibling-key isolation.
• Centralize pool-entry integrity below persistence adapters — Out of scope: the mobile adapter's unchecked insertion path predates this diff, which changes only SQLite restoration. Moving restoration into an upstream key-wallet API and updating both adapters is a broader ownership redesign, not a prerequisite for the guarded SQLite path to work.
  • Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Static review of the exact head confirms that Core pool restoration now validates derived entries, restores provider pools, preserves usage state, and runs before UTXO reconciliation. Two in-scope issues remain: omitted reserved rows can be reused for one-time funding pools, and legacy standard-labelled pool rows are not selected during restoration.

🔴 1 blocking | 🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The substantial new restore_pools and derive_towards logic in packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs directly changes key derivation validation and restoration of used and reserved funding addresses, meeting both the intricacy and key-handling criteria.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 14% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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/src/sqlite/schema/core_pool.rs`:
- [BLOCKING] packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs:899-922: Do not successfully restore wallets when one-time funding pool entries are omitted
  `restore_pools` treats every reservation not taken back by a reconstructed pool as a non-fatal degradation. Rows beyond the derivation budget are skipped at lines 799–803, while rows whose account coordinates match no reconstructed account are never queried; both cases are included in `stored_reservations` but not `restored_reservations`, leaving the corresponding address absent or Available. For an `IdentityInvitation` or other exported one-time funding pool, the previously exported bearer key can therefore be selected again after reopen. Because `WALLET_RESTORE` is advertised and the PR's purpose is to prevent funding-key reuse, Strict must fail closed for these pools or Recovery must quarantine the affected indices instead of returning a successfully restorable wallet.
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs:637-658: Restore persisted pools stored under the legacy standard account label
  The restoration queries bind only `accounts::account_type_db_label(&account_type)` (for example, `standard_bip44` or `standard_bip32`). V008 deliberately admits the historical `standard` label as compatible with either Standard variant, but a row stored under that label is never selected by these queries. Its reservation is later reported as `UnrestoredPoolReservation` and the reconstructed address can become available again. Resolve the legacy alias using unambiguous account-manifest information, or withhold `WALLET_RESTORE` for wallets containing rows that cannot be safely assigned; otherwise the advertised restore capability is not compatible with valid historical SQLite stores.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Reviewed the complete diff at ce03450 and found no remaining blocking defect; two non-blocking suggestions remain around coordinate-blob validation and shared funding-role classification. All 16 prior findings were independently revalidated: 14 are fixed, one is withdrawn, and one is intentionally deferred outside the corrected one-time funding-key case. Validation was static only; the supplied CI snapshot still has Rust workspace tests and Kotlin SDK checks queued, with the dedicated Rust wallet test job skipped.

🟡 2 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The substantial restoration logic in packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs directly changes key handling by validating derived keys and restoring used positions and reservations that prevent one-time funding key reuse.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 14% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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/src/sqlite/schema/core_pool.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs:973-979: Check identity-blob widths before copying unmatched funding rows
  The new wallet-wide audit reads both identity columns directly into Vec<u8>, bypassing the pre-allocation checks used by load_used_addresses_with_ctx and owning_account_for_script. A reserved, unused funding row with malformed identity coordinates is excluded by the used-address reader and cannot match the per-account restoration queries, so this audit is its first validation boundary. V009 imposes no width constraint on these columns: the audit can therefore copy multi-megabyte blobs before rejecting coordinates that must contain exactly 32 bytes. The connection's 32 MiB ceiling bounds the exposure, so this is hardening rather than an unbounded or unauthenticated remote vulnerability. Select length(user_identity_id) and length(friend_identity_id), validate both with blob::check_fixed_width(..., 32, ...), and only then retrieve the vectors. Add reserved-row coverage for malformed and oversized identities, checking the typed Strict failure and Recovery isolation with a healthy sibling wallet.
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs:556-572: Share the one-time funding-role definition with the asset-lock writer
  ONE_TIME_FUNDING_LABELS independently reproduces the funding-account classification in wallet/asset_lock/build.rs::persist_asset_lock_account_pools. The sets currently agree, but their connection is enforced by a test that reads and parses the private writer's source syntax. This makes maintenance of the fail-closed restoration rule depend on another crate's implementation text rather than a typed wallet-domain contract. Export a shared AccountType predicate or equivalent classification API from platform-wallet's asset_lock module and consume it in both paths. Keep database labels, coordinate validation and Strict/Recovery handling in SQLite; translating the shared roles through account_type_db_label removes the parallel semantic lists without moving backend policy into the wallet layer.

Out-of-scope follow-up suggestions (1)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

• Track one-time key issuance independently of individual pool rows — The fixture should_hand_out_again_a_one_time_funding_key_whose_row_was_deleted asserts the documented residual: deleting an issued invitation key's row leaves the loader unable to distinguish that key from an unused one. This is a concrete funding-key integrity concern, but detecting missing issuance history requires additional durable metadata and a recovery policy beyond this PR's restoration of present rows.
  • Follow-up: Track a separate issuance-integrity task covering missing pool rows and explicitly distinguishing local issuance-watermark protection from stale-backup rollback protection.