fix(platform-wallet-storage): persist provider key accounts and platform-node public keys (closes #4113)

[Claudius-Maginificent]

Why this PR exists

• Problem: rs-platform-wallet-storage's SQLite persister exhaustively destructures PlatformWalletChangeSet but only ever does let _ = provider_key_account_registrations; — BLS operator-key and EdDSA platform-node-key account registrations were silently dropped on every store(). The FFI persistence backend already handled these; SQLite never did. Closes rs-platform-wallet-storage: persist provider_key_account_registrations (BLS/EdDSA provider-key accounts) #4113.
• Architecture shift mid-review: feat(platform-wallet): persist typed BLS/EdDSA provider keys as core address rows #4127 (merged during this PR's life, by a different author) retired this PR's original mechanism for persisting individual pre-derived platform-node Ed25519 keys (a dedicated batch: V004 child table, ProviderKeyRegistrationBlob, ProviderNodeKeyConflict) and replaced it with the generic account_address_pools pipeline already used for every other account type — but wired that replacement up for the FFI backend only. It never touched rs-platform-wallet-storage, leaving a fresh, undiscovered gap: the SQLite backend's generic pool writer persisted script_pubkey/used per address-pool entry but silently dropped public_key.
• What breaks without it: Ed25519/SLIP-10 platform-node keys are hardened-only — there is no way to re-derive them from a watch-only account xpub. On a SQLite-backed store, register a wallet with provider accounts, restart, and the ProviderPlatformKeys pool comes back empty either way (old mechanism removed and never had SQLite coverage anyway, or new mechanism has the same gap) — provider_masternode_txs_blocking silently returns zero platform-node-owned masternodes.
• Blocking relationship: stacked on feat(platform-wallet-storage): embeddable SQLite persistence backend with seedless rehydration #3968 (feat/platform-wallet-storage-rehydration), which introduced the SQLite backend this PR extends.

What was done

Provider key account registrations (original scope, unaffected by #4127)

• SQLite writer apply_provider_registrations / reader load_provider_state persist BLS/EdDSA provider account xpubs via ProviderKeyAccountEntry, wired into touched_domains and the store/load round trip.
• build_wallet rebuilds BLS/EdDSA watch-only provider accounts from persisted rows via the shared rebuild_provider_key_account helper (used by both SQLite and FFI restore paths).
• Fail-closed validation: curve/type pairing mismatches and conflicting xpubs for the same account are rejected before any SQL runs.

Platform-node public key persistence (new — completes #4127's generic pipeline for SQLite)

• Removed the pre-feat(platform-wallet): persist typed BLS/EdDSA provider keys as core address rows #4127 dedicated node-key-batch mechanism entirely (V004__provider_key_accounts.rs, ProviderKeyRegistrationBlob, ProviderNodeKeyConflict, associated tests) — redundant with the generic pipeline feat(platform-wallet): persist typed BLS/EdDSA provider keys as core address rows #4127 introduced.
• New migration V005__pool_public_key.rs — additive public_key BLOB NULL / key_type INTEGER NULL columns on core_address_pool.
• apply_pools() (core_pool.rs) now persists AddressInfo::public_key alongside every pool row; load_typed_pool_entries() reads it back with fail-closed width/discriminant checks.
• restore_provider_platform_node_pool() (util/wallet.rs) rehydrates the ProviderPlatformKeys account's AbsentHardened pool on load(), reconstructing each AddressInfo via the same shared insert_platform_node_pool_entry helper (rs-platform-wallet) that populate_platform_node_pool uses at registration time — a single implementation backs both paths rather than two hand-duplicated copies. Wired into persister.rs::load().

Typed pool-key data integrity

• apply_pools() rejects a write that would silently change already-stored public_key/key_type at an existing pool index (WalletStorageError::TypedPoolKeyConflict), mirroring the existing ProviderKeyAccountConflict fail-closed pattern at the account level. Identical re-writes (idempotent resync) and fresh indices are unaffected — only a genuine conflicting overwrite is rejected, since these hardened-derivation keys have no recovery path if clobbered.
• insert_platform_node_pool_entry's missing-AbsentHardened-pool case is a hard error in both call sites (previously one path silently no-op'd, the other errored — now unified).

Testing

• New repro test platform_node_key_public_keys_survive_sqlite_store_and_load — confirmed RED against the pre-fix code (left: 0, right: 3), GREEN after the V005 fix.
• conflicting_typed_pool_key_is_rejected_and_original_survives_load, identical_typed_pool_key_is_idempotent, typed_pool_key_at_fresh_index_succeeds — cover the conflict guard's reject/idempotent/fresh-index paths through the real store()/load() API.
• populate_platform_node_pool_rejects_missing_hardened_pool — covers the unified missing-pool hard error.
• cargo test -p platform-wallet -p platform-wallet-storage -p platform-wallet-ffi --all-features --all-targets: all green, 0 failures.
• cargo clippy -p platform-wallet -p platform-wallet-storage -p platform-wallet-ffi --all-targets --all-features -- -D warnings: clean.
• cargo fmt: applied.
• Schema-pinning fingerprints (sqlite_schema_pinning.rs) re-baselined for the V005 migration.
• Independently reviewed by three specialist passes (security, project-consistency, adversarial QA) before this push; the conflict guard was empirically reproduced against the real store()/load() API before being classified and fixed.

Breaking Changes

None. V005 is purely additive (nullable columns, existing rows unaffected). Retiring the old dedicated-batch migration (V004__provider_key_accounts.rs, distinct from the current tree's V004, which is invitations) is safe: it was introduced and removed within this same unmerged PR, so it was never released.

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have added "!" to the title and described breaking changes in the corresponding section if my code contains any — n/a, no breaking changes
• I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

_{🤖 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: b5449f08-9de8-4243-b70d-fa287368b5f8

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/4113-provider-key-account-persistence

---

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]

⛔ Blockers found — Sonnet deferred (commit 3c26fc1)
Canonical validated blockers: 3

[thepastaclaw]

Preliminary review — Codex only

Provider account xpubs round-trip and the targeted 24-test persistence suite passes, but the persisted hardened node-key batch is discarded from the public SQLite load result. The writer also fails to reject xpub conflicts against an existing provider account, leaving two blocking persistence defects in the new behavior.

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 (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 2 blocking

🤖 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/util/wallet.rs`:
- [BLOCKING] packages/rs-platform-wallet-storage/src/sqlite/util/wallet.rs:50-57: Expose persisted platform-node keys after SQLite load
  `load_provider_state` reconstructs each provider entry with its `derived_platform_node_keys`, but this loop consumes only `account_type` and `extended_public_key`. The manifest is then dropped, `ClientWalletStartState` has no field for the derived batch, and the schema reader is private in production. The restored EdDSA account contains only its watch-only account xpub, which cannot derive hardened SLIP-10 children without the seed, so `SqlitePersister::load()` makes the newly persisted node-key rows unreachable. Swift has a separate host-side persistence path for these keys, but SQLite exposes no equivalent side channel. This defeats the PR's stated seedless recovery goal, and the end-to-end test misses it because it asserts only the restored account xpubs.

In `packages/rs-platform-wallet-storage/src/sqlite/schema/accounts.rs`:
- [BLOCKING] packages/rs-platform-wallet-storage/src/sqlite/schema/accounts.rs:249-255: Reject xpub conflicts against the persisted provider account
  This conflict check compares xpub payloads only within the current `store()` batch. On a later store, `UPSERT_ACCOUNT_SQL` replaces the existing account's `account_xpub_bytes` without comparing it to the persisted value. The cross-flush node-key guard does not close this gap: BLS entries never carry child rows, while an EdDSA entry with an empty or disjoint batch avoids overlapping-index checks and leaves append-only children derived from the old xpub attached to the new parent. Load then returns a silently replaced BLS account or an internally inconsistent EdDSA snapshot. Compare the incoming payload with the existing parent row before performing either write, using the same fail-closed policy already applied to in-batch xpub conflicts and persisted node-key conflicts.

[thepastaclaw]

Preliminary review — Codex only

At exact head b7f3af9, carried-forward prior-1 remains a blocking SQLite rehydration defect: persisted platform-node public keys do not reach the restored managed pool. Prior-2 is fixed by the persisted parent-payload conflict check at accounts.rs:302-308. No new finding was identified in the latest reviewed-head delta.

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 (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 1 blocking

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

[thepastaclaw]

Preliminary review — Codex only

Carried-Forward Prior Findings: none; both prior blockers are fixed. New Findings In Latest Delta: one blocking writer-validation defect and one corruption-validation suggestion remain. Static inspection and git diff --check passed; the targeted test could not run offline because the pinned rust-dashcore revision is unavailable locally.

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), gpt-5.6-sol/high — security-auditor (failed), gpt-5.6-sol/high — rust-quality (failed), gpt-5.6-sol/high — ffi-engineer (failed), gpt-5.6-sol — general (completed), gpt-5.6-sol — security-auditor (completed), gpt-5.6-sol — rust-quality (failed), gpt-5.6-sol — ffi-engineer (completed), gpt-5.6-sol — rust-quality (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 1 blocking | 🟡 1 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-storage/src/sqlite/schema/core_pool.rs`:
- [BLOCKING] packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs:87-95: Reject malformed typed-key widths before committing them
  PublicKeyType variants contain unconstrained Vec<u8> values, but this writer persists them without enforcing the 33/32/48-byte widths that load_typed_pool_entries requires. A malformed ProviderPlatformKeys EdDSA entry therefore makes store() succeed and causes the next load() to fail, durably poisoning the wallet database. Validate the variant-specific width before executing the upsert.
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs:131-150: Do not filter malformed public-key rows out before validation
  V005 permits public_key and key_type to be populated independently, but both queries select only key_type IS NOT NULL. A row containing key bytes with a NULL discriminator is silently omitted, contradicting the loader's fail-hard corruption policy and restoring an incomplete platform-node pool. Select rows where either typed column is present and reject mismatched nullability, or enforce paired nullability in the schema.

[thepastaclaw]

🔴 Blocking: Reject malformed typed-key widths before committing them

PublicKeyType variants contain unconstrained Vec values, but this writer persists them without enforcing the 33/32/48-byte widths that load_typed_pool_entries requires. A malformed ProviderPlatformKeys EdDSA entry therefore makes store() succeed and causes the next load() to fail, durably poisoning the wallet database. Validate the variant-specific width before executing the upsert.

_{source: ['codex']}

[thepastaclaw]

🟡 Suggestion: Do not filter malformed public-key rows out before validation

V005 permits public_key and key_type to be populated independently, but both queries select only key_type IS NOT NULL. A row containing key bytes with a NULL discriminator is silently omitted, contradicting the loader's fail-hard corruption policy and restoring an incomplete platform-node pool. Select rows where either typed column is present and reject mismatched nullability, or enforce paired nullability in the schema.

_{source: ['codex']}

[thepastaclaw]

Preliminary review — Codex only

At exact head 3c26fc1, the two earlier blockers concerning SQLite key restoration and persisted provider-xpub conflicts are fixed. The two carried-forward core-pool findings remain valid, and the latest delta adds valid blockers for untyped key overwrites and incomplete used-index restoration. The targeted test could not run offline because the pinned rust-dashcore revision is unavailable locally.

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 (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 3 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-storage/src/sqlite/schema/core_pool.rs`:
- [BLOCKING] packages/rs-platform-wallet-storage/src/sqlite/schema/core_pool.rs:107-143: Reject untyped overwrites of persisted typed keys
  New in the latest delta. The conflict query runs only when the incoming row has a typed key. If a later snapshot supplies `public_key: None` for an index that already contains a typed key, the guard is skipped and the upsert replaces both `public_key` and `key_type` with NULL. This irreversibly discards hardened Ed25519 platform-node material that cannot be regenerated from the account xpub, and the next load silently omits that row. Reject removal of an existing typed key while continuing to allow a typed key to upgrade a legacy NULL/NULL row.

In `packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs:301-325: Restore used platform-node entries into pool bookkeeping
  New in the latest delta. SQLite passes the persisted `used` flag into this shared insertion helper, but the helper sets only `AddressInfo.used` and `highest_generated`; it leaves `used_at`, `pool.used_indices`, and `pool.highest_used` unset. The restored pool consequently reports the individual row as used while exposing `last_used_index = -1`, and key-wallet's `mark_used` and `mark_index_used` cannot repair the indexes because they update bookkeeping only during an unused-to-used transition. Mirror the FFI restore path by setting `used_at` and advancing `used_indices` and `highest_used` whenever `used` is true.

[thepastaclaw]

🔴 Blocking: Reject untyped overwrites of persisted typed keys

New in the latest delta. The conflict query runs only when the incoming row has a typed key. If a later snapshot supplies public_key: None for an index that already contains a typed key, the guard is skipped and the upsert replaces both public_key and key_type with NULL. This irreversibly discards hardened Ed25519 platform-node material that cannot be regenerated from the account xpub, and the next load silently omits that row. Reject removal of an existing typed key while continuing to allow a typed key to upgrade a legacy NULL/NULL row.

_{source: ['codex']}

[thepastaclaw]

🔴 Blocking: Restore used platform-node entries into pool bookkeeping

New in the latest delta. SQLite passes the persisted used flag into this shared insertion helper, but the helper sets only AddressInfo.used and highest_generated; it leaves used_at, pool.used_indices, and pool.highest_used unset. The restored pool consequently reports the individual row as used while exposing last_used_index = -1, and key-wallet's mark_used and mark_index_used cannot repair the indexes because they update bookkeeping only during an unused-to-used transition. Mirror the FFI restore path by setting used_at and advancing used_indices and highest_used whenever used is true.

_{source: ['codex']}