feat(swift-sdk): wallet memory explorer + persistor UTXO/sync load

[QuantumExplorer]

Issue being fixed or feature implemented

Two threads of work that landed together:

1. WalletMemoryExplorer in SwiftExampleApp had to mirror PlatformWalletManager end-to-end so iOS engineers can see exactly what's in memory after load_from_persistor runs (sync watermark, identity buckets, address pools, UTXOs, asset locks…). This is the diagnostic surface that unblocked the next item.

2. load_from_persistor stopped at recomputing the Wallet from per-account xpubs — UTXOs and sync metadata weren't being repopulated, so a restored wallet booted with a 0 DASH balance even though PersistentTxo rows existed in SwiftData. That problem is fixed here.

A handful of upstream rust-dashcore changes also landed in lockstep — ManagedCoreAccount was split into ManagedCoreFundsAccount / ManagedCoreKeysAccount, is_watch_only / custom_name / first_loaded_at / total_transactions were dropped, and keep_txs_in_memory became a non-default Cargo feature. Platform-wallet was realigned against those.

What was done?

Memory explorer (read-only diagnostics)

• 15 new *_blocking accessors on PlatformWalletManager for the manager-level tunables (address-sync / identity-sync config, registered wallet ids), per-wallet state (core / identity / platform-address provider), per-wallet floating state (tracked asset locks, InstantSend lock txids), per-account drill-down (metadata, address pools with encoded address + derived public-key bytes, UTXOs), and the identity manager bucket layout.
• New manager_diagnostics.rs FFI surface with paired _free fns for every heap-owning shape; new PlatformWalletManagerDiagnostics.swift wrappers.
• Explorer split into Core Funds Accounts vs Core Keys Accounts with green/blue kind badges. Balance / UTXOs / "Total UTXOs" rows are suppressed on keys-only accounts (they don't carry funds by construction). Each address-pool row now shows the encoded address plus the derived public-key bytes.
• The standalone "PlatformWalletInfo Metadata" section was removed — every meaningful field either duplicated CoreWalletStateSnapshot or had no active populator. The Swift wrapper and Rust accessor were dropped in lockstep.

Persistor restore — WalletRestoreEntryFFI + build_wallet_start_state + manager::load

• New per-wallet sync metadata fields: birth_height, synced_height, last_processed_height, last_synced. Zero is treated as "unknown" so ManagedWalletInfo::from_wallet's seeded birth_height - 1 default survives for fresh wallets.
• New per-wallet utxos: [UtxoRestoreEntryFFI] array. Each row carries the same account-tag block AccountSpecFFI uses, so account_type_from_spec handles routing into the matching ManagedCoreFundsAccount.utxos. Keys-only / PlatformPayment variants and snapshot-drift rows are silently skipped — re-sync recovers them.
• After insertion the loader calls wallet_info.update_balance() (recomputes per-account confirmed/unconfirmed/immature/locked from UTXOs against metadata.last_processed_height, sums into the wallet rollup). manager::load::load_from_persistor then mirrors the recomputed inner balance into the lock-free Arc<WalletBalance> the UI reads — WalletBalance::set is pub(crate) to platform-wallet, which is why this step has to live there rather than the FFI loader.
• Restored wallets are now WalletType::ExternalSignable instead of WatchOnly. The mnemonic / seed lives in the iOS Keychain; signing requests route back through the host signer surface rather than erroring out.
• Swift loadWalletList populates the new fields from PersistentWallet.{birthHeight, syncedHeight, lastSynced} and walks unspent PersistentTxo rows (isSpent == false) for UTXOs, copying tags off each row's account relationship. The composite outpoint field on PersistentTxo is 36 bytes (32-byte txid + LE u32 vout), so the FFI handoff uses record.txid (which prefers transaction.txid and falls back to outpoint.prefix(32)).

Upstream rust-dashcore alignment

• Pinned to e2e8fcf852130383b5922d3c2d907dda334296ee on v0.42-platform-nightly.
• Field-to-method renames on ManagedAccountRef (managed_account_type(), monitor_revision()); variant-aware reads via as_funds() for balance / utxos; transactions_iter() for the ref-enum tx walk (returns empty when keep_txs_in_memory is off — tx history is event-driven now).
• ManagedCoreAccount → ManagedCoreFundsAccount; accounts.insert(...) → accounts.insert_funds(...) for DashPay funds-bearing inserts.
• Per-account is_watch_only / custom_name snapshots removed (upstream dropped both fields). Watch-only is wallet-level now (read off Wallet.wallet_type).
• WalletInfoInterface no longer declares first_loaded_at / set_first_loaded_at; the platform delegating impls were removed.

SwiftExampleApp cleanup (touched along the way)

• PersistentWallet balance fields (balanceConfirmed/Unconfirmed/Immature/Locked) are no longer rendered — the canonical source is walletManager.accountBalances(for:). The persister callback no longer writes them either.
• Wallet-detail "Total" / breakdown logic and SendTransactionView were switched to the in-memory account-balance source.
• SPV progress label multiplies by 100 (was rendering 1.0% for fully-synced wallets).
• KeyWallet/ManagedAccount.swift: dropped the isWatchOnly getter; the matching C function managed_core_account_get_is_watch_only no longer exists upstream.

How Has This Been Tested?

• cargo check -p platform-wallet-ffi — clean.
• ./build_ios.sh --target sim — clean. Verified end-to-end against a recovered testnet wallet on iPhone 17 Pro Simulator (iOS 26.3):
  • 18 unspent PersistentTxo rows → 18 routed into the BIP44 #0 funds account on load.
  • BIP44 #0 reads 10.31727369 DASH (9.31727595 confirmed + 0.99999774 unconfirmed); wallet rollup matches.
  • Keys-only accounts (IdentityRegistration / AssetLockShieldedTopUp / etc.) render with Keys badge, no Balance/UTXOs sections, address pools fully populated.
  • SPV progress reads 100% when fully synced.

Breaking Changes

None outside of FFI surface evolution that's already kept in sync with the rust-dashcore upstream split. Restored wallets going from WatchOnly to ExternalSignable is the most user-visible behavior change but only affects in-memory state; persistence layout is unchanged.

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
• I have made corresponding changes to the documentation if needed

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Atomic diagnostics: enumerate wallet IDs and snapshot manager/wallet/account/provider/identity states, tracked locks, InstantSend txids, UTXO and paginated transaction listings exposed to Swift UI.
  • Per-account key-usage: keysUsed/keysTotal available in SDK and UI.

• Bug Fixes

  • Wallet restore: improved UTXO recovery with core-chain sync metadata preserved; malformed persisted rows are skipped (non-fatal) with warnings.

• Refactor

  • Balances now derived from live per-account totals; cached wallet-level Core balance fields removed and UI views updated to use snapshots.

[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

Adds atomic diagnostic snapshot APIs and FFI diagnostics module, extends ABI types (including keys_used/keys_total and UTXO restore rows), hardens persistence/restore with validation and rollback, hydrates restored balances, and updates Swift bridge, models, and UI to consume per-account snapshots and diagnostics.

Changes

Diagnostics Snapshot + Persistence + Swift Bridge

Layer / File(s)	Summary
Dependency Update Cargo.toml	Pins multiple dash* and key-wallet* workspace git dependencies to rev = "e2e8fcf852130383b5922d3c2d907dda334296ee".
Data Shape: FFI Types packages/rs-platform-wallet-ffi/src/core_wallet_types.rs, packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs	Adds keys_used/keys_total to AccountBalanceEntryFFI; introduces snapshot FFI structs (platform/identity/core/provider state, tracked asset-lock, account metadata, address-pool rows, UTXO/transaction rows, wallet-identity rows); adds UtxoRestoreEntryFFI and extends WalletRestoreEntryFFI with core sync metadata and UTXO array.
Data Shape: Rust Snapshots packages/rs-platform-wallet/src/manager/accessors.rs	Adds public snapshot structs mirroring FFI shapes; changes account_balances_blocking to return Vec<AccountBalanceRow> including balance and key-usage totals.
Core Implementation: Manager Accessors packages/rs-platform-wallet/src/manager/accessors.rs, .../identity_sync.rs, .../mod.rs	Implements many *_blocking snapshot accessors (wallet id listing, address/identity sync config, core/identity/provider state, tracked locks, instant-send locks, per-account metadata/address pools/utxos/transactions, identity-manager rows); adds try_queue_depth() and makes accessors public.
FFI Bridge Implementation packages/rs-platform-wallet-ffi/src/manager_diagnostics.rs, packages/rs-platform-wallet-ffi/src/lib.rs	Adds manager_diagnostics module and ~20 unsafe extern "C" FFI functions: pointer validation, call blocking snapshots, marshal into FFI structs or flat byte buffers, return counts, and paired *_free deep-free functions; re-exports at crate root.
Persistence: Restore & UTXO Handling packages/rs-platform-wallet-ffi/src/persistence.rs, packages/swift-sdk/.../PlatformWalletPersistenceHandler.swift	Hardened AccountSpec encoding/decoding; reconstructs external-signable wallets, validates and routes UtxoRestoreEntryFFI only into funds-bearing accounts, counts/skips unmappable rows with warnings; Swift prefetches unspent txos, marshals UTXO restore buffers, validates enum bytes, and aborts/rolls back when malformed.
Runtime: Wallet Load Safety & Balance Hydration packages/rs-platform-wallet/src/manager/load.rs	Hydrates restored core balances into WalletBalance Arc before registration; records inserted ids and rolls back previously inserted wallets on load/init failure to avoid half-registered state.
Core Wallet Adjustments packages/rs-platform-wallet/src/...	Removes delegations for first_loaded_at; registers funds-bearing DashPay accounts via ManagedCoreFundsAccount + insert_funds; chain-lock detection now uses get_transaction; provider exposes diagnostic counts and sync height; wallet exposes provider clone for diagnostics.
FFI Mapping: Account Balances packages/rs-platform-wallet-ffi/src/wallet.rs	platform_wallet_manager_get_account_balances maps AccountBalanceRow into AccountBalanceEntryFFI including keys_used/keys_total.
Swift Model & Persistence Changes packages/swift-sdk/.../ManagedAccount.swift, .../PersistentWallet.swift	Removes ManagedAccount.isWatchOnly; removes wallet-level cached balance properties; persistence handler stops applying signed balance deltas and builds validated compact buffers for platform-address balances and UTXO restore rows.
Swift Diagnostics Bridge & Types packages/swift-sdk/.../PlatformWalletManagerDiagnostics.swift	New Swift diagnostics extension exposing snapshot types and methods that validate inputs, call FFI functions, decode flat buffers / FFI arrays into Swift types, and free returned buffers.
Swift Manager & UI Updates packages/swift-sdk/...	AccountBalance adds keysUsed/keysTotal; many UI views (Core list rows, Send, Receive, Memory Explorer, Storage detail) derive Core balances from walletManager.accountBalances(for:), add diagnostics panes, change next-receive selection to “unreceived”, and adapt UTXO/transaction rendering and pagination.
Build / Crate Dependency packages/rs-platform-wallet-ffi/Cargo.toml	Adds tracing = "0.1" dependency for persistence loader warnings.

Sequence Diagram(s)

sequenceDiagram
    participant UI as Swift UI
    participant SwiftMgr as PlatformWalletManager (Swift)
    participant FFI as DashSDKFFI (C)
    participant RustMgr as PlatformWalletManager (Rust)
    participant DB as Persistor/DB

    UI->>SwiftMgr: request diagnostic snapshot (e.g., wallet detail)
    SwiftMgr->>FFI: call platform_wallet_core_wallet_state / other FFI
    FFI->>RustMgr: invoke core_wallet_state_blocking(&wallet_id)
    RustMgr->>RustMgr: build atomic snapshot (accounts, provider, identity)
    RustMgr->>DB: (optional) load persisted UTXO/restore data during startup/restore
    RustMgr-->>FFI: return snapshot struct or flat buffers
    FFI-->>SwiftMgr: marshal C types to Swift, return pointer/count
    SwiftMgr-->>UI: mapped Swift value types (Data, ints, arrays)
    UI->>SwiftMgr: on completion call free helpers (defer) to release buffers

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• feat(platform-wallet,swift-sdk): query per-account balances via FFI #3572 — overlaps on adding account balance FFI surface and manager accessor changes; likely directly related.

Suggested reviewers

• shumkov

Poem

🐇 I hop through bytes at break of day,

Snapshots lined in tidy array,
Rust hums state to Swift's bright land,
Keys and UTXOs held in hand,
Diagnostics bloom — a carrot-sweet play.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat(swift-sdk): wallet memory explorer + persistor UTXO/sync load' accurately summarizes the two main features: WalletMemoryExplorer diagnostics UI and persistor UTXO/sync metadata restoration.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/wallet-memory-explorer-and-utxo-load

---

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 and usage tips.}

[thepastaclaw]

🔍 Review in progress — actively reviewing now (commit a74717e)
Queue position: 1/1

[github-actions]

✅ DashSDKFFI.xcframework built for this PR.

• Workflow run: https://github.com/dashpay/platform/actions/runs/25303651903
• Artifacts: DashSDKFFI.xcframework (folder), DashSDKFFI.xcframework.zip, xc_checksum.txt

SwiftPM (host the zip at a stable URL, then use):

.binaryTarget(
  name: "DashSDKFFI",
  url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
  checksum: "da56575678905734e1b259cbb654289a663ce95149fe84e6181ff71b7fb8375c"
)

Xcode manual integration:

• Download 'DashSDKFFI.xcframework' artifact from the run link above.
• Drag it into your app target (Frameworks, Libraries & Embedded Content) and set Embed & Sign.
• If using the Swift wrapper package, point its binaryTarget to the xcframework location or add the package and place the xcframework at the expected path.

[coderabbitai]

Actionable comments posted: 4

Caution

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

⚠️ Outside diff range comments (1)

packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs (1)

467-502: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Make external contact registration atomic.

wallet.add_account(...) now happens before insert_funds(...); if the second insert fails, the wallet is left half-registered and retries can hit duplicate-account errors. Please ensure both collections are updated together or roll back the first insert on failure.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs` around
lines 467 - 502, The current sequence calls wallet.add_account(...) before
info.core_wallet.accounts.insert_funds(...), which can leave the wallet
half-registered if insert_funds fails; make the operation atomic by performing
the insert_funds first and then calling wallet.add_account, and if
wallet.add_account returns an error, roll back the funds-side insert by removing
the ManagedCoreFundsAccount (use the corresponding removal API on
info.core_wallet.accounts, e.g. remove_funds or equivalent keyed by the
ManagedCoreFundsAccount identity); alternatively, wrap both calls in a scoped
transaction if such a facility exists so both succeed or both are undone. Ensure
you reference and use ManagedCoreFundsAccount::from_account,
info.core_wallet.accounts.insert_funds, info.core_wallet.accounts.remove_funds
(or the correct removal method), and wallet.add_account when implementing the
change.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerDiagnostics.swift`:
- Around line 395-414: In accountTransactions(for:balance:pageOffset:pageLimit:)
ensure pageOffset and pageLimit are validated as non-negative before converting
to UInt; e.g., clamp or guard against negative values (returning [] or setting
to 0) and then pass the safe UInt values into
platform_wallet_account_transactions when calling it (the call that currently
uses UInt(pageOffset) and UInt(pageLimit)). Update the code around the
platform_wallet_account_transactions invocation to use the validated
UInt(pageOffsetSafe) and UInt(pageLimitSafe).

In
`@packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- Around line 2078-2081: The assignment in PlatformWalletPersistenceHandler that
sets entry.last_processed_height = w.syncedHeight incorrectly synthesizes a
processing watermark; change this to not synthesize from w.syncedHeight (e.g.,
set entry.last_processed_height to 0 or the explicit "unknown" sentinel) unless
you have a real persisted last-processed value to use; update the logic around
the entry and Wallet (w) handling so last_processed_height is only written from
a true persisted last-processed watermark and not derived from syncedHeight.

In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swift`:
- Around line 677-703: The various computed properties (coreConfirmed,
coreUnconfirmed, coreImmature, coreLocked, totalCoreBalance and
balanceBreakdown()) repeatedly call walletManager.accountBalances(for:) causing
multiple FFI hits; modify CoreContentView to capture a single snapshot once
(e.g. store walletManager.accountBalances(for: wallet.walletId) in a private
let/var snapshot like coreBalancesSnapshot) and have the computed properties and
balanceBreakdown() read from that snapshot instead of re-invoking
walletManager.accountBalances(for:), ensuring the snapshot is created once per
body evaluation; update the other identical block (the one referenced at lines
~756-775) similarly.

In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/SendTransactionView.swift`:
- Around line 164-171: The computed property coreBalance currently calls
walletManager.accountBalances(for:) repeatedly during rendering (used by summary
rows, availableSources, and balance(for:)), causing blocking FFI calls; hoist
that lookup out of the render path by taking a single snapshot of
walletManager.accountBalances(for: wallet.walletId) once before computing the
view body (e.g., capture into a local constant or `@State/`@StateObject updated
when walletId changes), then pass that snapshot into the helper methods
(availableSources, balance(for:), and any summary row code) so they read from
the cached balances instead of re-invoking walletManager.accountBalances(for:).

---

Outside diff comments:
In `@packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs`:
- Around line 467-502: The current sequence calls wallet.add_account(...) before
info.core_wallet.accounts.insert_funds(...), which can leave the wallet
half-registered if insert_funds fails; make the operation atomic by performing
the insert_funds first and then calling wallet.add_account, and if
wallet.add_account returns an error, roll back the funds-side insert by removing
the ManagedCoreFundsAccount (use the corresponding removal API on
info.core_wallet.accounts, e.g. remove_funds or equivalent keyed by the
ManagedCoreFundsAccount identity); alternatively, wrap both calls in a scoped
transaction if such a facility exists so both succeed or both are undone. Ensure
you reference and use ManagedCoreFundsAccount::from_account,
info.core_wallet.accounts.insert_funds, info.core_wallet.accounts.remove_funds
(or the correct removal method), and wallet.add_account when implementing the
change.

🪄 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: a7064c01-b85e-42f3-9510-6f348c9eaa68

📥 Commits

Reviewing files that changed from the base of the PR and between 7739439 and ed5e30a.

⛔ Files ignored due to path filters (1)

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

📒 Files selected for processing (27)

• Cargo.toml
• packages/rs-platform-wallet-ffi/src/core_wallet_types.rs
• packages/rs-platform-wallet-ffi/src/lib.rs
• packages/rs-platform-wallet-ffi/src/manager_diagnostics.rs
• packages/rs-platform-wallet-ffi/src/persistence.rs
• packages/rs-platform-wallet-ffi/src/wallet.rs
• packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs
• packages/rs-platform-wallet/src/changeset/core_bridge.rs
• packages/rs-platform-wallet/src/manager/accessors.rs
• packages/rs-platform-wallet/src/manager/identity_sync.rs
• packages/rs-platform-wallet/src/manager/load.rs
• packages/rs-platform-wallet/src/manager/mod.rs
• packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs
• packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs
• packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs
• packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs
• packages/rs-platform-wallet/src/wallet/platform_wallet_traits.rs
• packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedAccount.swift
• packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentWallet.swift
• packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift
• packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerDiagnostics.swift
• packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift
• packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swift
• packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/ReceiveAddressView.swift
• packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/SendTransactionView.swift
• packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageRecordDetailViews.swift
• packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/WalletMemoryExplorerView.swift

💤 Files with no reviewable changes (1)

• packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageRecordDetailViews.swift

[thepastaclaw]

Code Review

Three blocking issues on the persistor-restore path: legacy IdentityAuthentication account rows abort the entire wallet restore via ?-propagation (codex-general), malformed cached platform-address rows can hand Rust uninitialized FFI memory because the published count exceeds the number of writes (codex-general + codex-ffi-engineer), and load_from_persistor mutates wallet_manager before validating expected_wallet_id so error paths leave orphan registrations behind. Several worthwhile suggestions about hardcoded last_processed_height = 0, silent UTXO/key-discriminant drops, and the inconsistent Box/Vec allocator pairing in diagnostic free fns. No consensus-critical paths are touched. 5 nitpicks dropped as triv/speculative.

Reviewed commit: 7e802b4

🔴 3 blocking | 🟡 3 suggestion(s) | 💬 2 nitpick(s)

3 additional findings

🔴 blocking: Uninitialized AddressBalanceEntryFFI slots crossing into Rust when persisted hash != 20 bytes

packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift (lines 2010-2065)

The buffer is sized to cachedBalances.count and indexed via for (j, cached) in cachedBalances.enumerated(). The guard hash.count == 20 else { continue } skip keeps j advancing on the input array but never writes buf[j]. entry.platform_address_balances_count is still set to UInt(cachedBalances.count) at line 2065, so Rust's slice::from_raw_parts(entry.platform_address_balances, entry.platform_address_balances_count) (persistence.rs:1359-1364) reads the uninitialized slots as valid AddressBalanceEntryFFI — undefined behaviour the moment a single malformed SwiftData row exists. The fix is to compact: track a written counter, write buf[written], and publish written (or nil + 0 if no rows survived) instead of cachedBalances.count. The same written-counter pattern is already used correctly in buildUtxoRestoreBuffer immediately below.

🔴 blocking: load_from_persistor leaves orphan wallets in `wallet_manager` on mid-load errors

packages/rs-platform-wallet/src/manager/load.rs (lines 85-137)

Each wallet is registered into self.wallet_manager (line 87) before two distinct fail-able checks run: the wallet_id != expected_wallet_id validation at line 95-101, and platform().initialize_from_persisted(...) at line 119-129. Either failure returns Err while the entry is still live in wallet_manager, and may also be missing from self.wallets (which is only inserted at line 136). That breaks the manager's invariant that wallet_manager and self.wallets describe the same set, and a user-driven retry will now collide on duplicate registration / partial state instead of cleanly re-running. Stage the work transactionally — validate expected_wallet_id and run initialize_from_persisted against a temporary structure first, or roll back the insert_wallet on every error path before returning.

🟡 suggestion: Invalid persisted public-key discriminants coerced into valid DPP enums instead of skipped

packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift (lines 2269-2278)

UInt8(pk.keyType) ?? 0 (and the analogous purpose / securityLevel lines) silently rewrite parse failures to 0, which is a valid discriminant for all three enums (ECDSA_SECP256K1 / AUTHENTICATION / MASTER). Rust's build_identity_public_keys at persistence.rs:1525-1533 is intentionally designed to skip rows whose try_from fails — your ?? 0 defeats that safeguard and turns malformed SwiftData rows into 'valid ECDSA master keys' carrying the original byte payload, which is data corruption rather than a no-op skip. The inline comment claims this avoids dropping the row entirely, but a corrupt row crossing the FFI boundary as a misclassified key is a worse outcome than dropping. Use UInt8.max (or any value the try_from impls reject) so Rust's filter handles it.

🤖 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-ffi/src/persistence.rs`:
- [BLOCKING] lines 1168-1181: Legacy IdentityAuthentication account rows abort the entire wallet restore
  `account_type_from_spec` at line 1652-1658 explicitly returns `Err` for `AccountTypeTagFFI::IdentityAuthenticationEcdsa | IdentityAuthenticationBls` because the upstream `AccountType` variants were removed. The new account-loading loop calls `account_type_from_spec(spec)?` at line 1169 and propagates that error, which aborts `build_wallet_start_state` and consequently `FFIPersister::load()` for the whole client — every wallet, not just the offending one. The UTXO loop a few lines below already treats the identical failure mode as recoverable snapshot drift via `let Ok(account_type) = account_type_from_spec(&spec) else { continue; };`. Persisted wallets carrying these (still-ABI-valid) tags will fail to restore on every launch until the row is purged. The account loader needs the same skip-and-continue treatment.
- [SUGGESTION] lines 1239-1323: UTXO restore silently drops rows on four distinct failure modes — under-reports balance with no signal
  The new restore loop `continue`s on tag-not-mappable, bad txid bytes, unrenderable script, and account-not-in-wallet. Because `wallet_info.update_balance()` only sees successfully routed rows, any skipped persisted TXO quietly reduces the restored balance even though the snapshot claimed it existed. The four cases have very different operational meanings (corruption vs legitimate snapshot drift vs legitimate keys-only routing) but currently look identical to operators. Given this PR's stated goal is making cold-start balances accurate from `PersistentTxo` rows, a small schema drift can recreate the original 'restored wallet shows zero balance' symptom silently. At minimum, emit a `tracing::warn!` per dropped row tagged by reason and return a `(routed, dropped)` counter to the caller so the surface above can detect divergence.

In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- [BLOCKING] lines 2010-2065: Uninitialized AddressBalanceEntryFFI slots crossing into Rust when persisted hash != 20 bytes
  The buffer is sized to `cachedBalances.count` and indexed via `for (j, cached) in cachedBalances.enumerated()`. The `guard hash.count == 20 else { continue }` skip keeps `j` advancing on the input array but never writes `buf[j]`. `entry.platform_address_balances_count` is still set to `UInt(cachedBalances.count)` at line 2065, so Rust's `slice::from_raw_parts(entry.platform_address_balances, entry.platform_address_balances_count)` (persistence.rs:1359-1364) reads the uninitialized slots as valid `AddressBalanceEntryFFI` — undefined behaviour the moment a single malformed SwiftData row exists. The fix is to compact: track a `written` counter, write `buf[written]`, and publish `written` (or `nil` + `0` if no rows survived) instead of `cachedBalances.count`. The same `written`-counter pattern is already used correctly in `buildUtxoRestoreBuffer` immediately below.
- [SUGGESTION] lines 2071-2082: `last_processed_height = 0` hardcoded — restored core state regresses against `synced_height`
  Swift always sends `entry.last_processed_height = 0` and the Rust loader treats `0` as 'unknown' (persistence.rs:1201-1203), so `from_wallet`'s `birth_height.saturating_sub(1)` survives. After a launch this leaves `metadata.last_processed_height ≈ birth_height - 1` while `metadata.synced_height` is correctly preserved at the persisted watermark. Two concrete consequences: (1) `ManagedWalletInfo::update_balance()` consults `last_processed_height` to classify coinbase maturity, so restored mined wallets will mis-bucket matured coinbase outputs as immature until SPV advances the watermark; (2) any caller that uses the gap between the two heights for resume / replay decisions reads a wildly stale value. The PR comment justifies this as deferred work pending a real persisted column. As a stop-gap, `entry.last_processed_height = w.syncedHeight` would keep the two aligned for non-pruning wallets, matching the runtime invariant. The proper fix is a dedicated `lastProcessedHeight` column on `PersistentWallet`.
- [SUGGESTION] lines 2269-2278: Invalid persisted public-key discriminants coerced into valid DPP enums instead of skipped
  `UInt8(pk.keyType) ?? 0` (and the analogous `purpose` / `securityLevel` lines) silently rewrite parse failures to `0`, which is a *valid* discriminant for all three enums (ECDSA_SECP256K1 / AUTHENTICATION / MASTER). Rust's `build_identity_public_keys` at persistence.rs:1525-1533 is intentionally designed to skip rows whose `try_from` fails — your `?? 0` defeats that safeguard and turns malformed SwiftData rows into 'valid ECDSA master keys' carrying the original byte payload, which is data corruption rather than a no-op skip. The inline comment claims this avoids dropping the row entirely, but a corrupt row crossing the FFI boundary as a misclassified key is a worse outcome than dropping. Use `UInt8.max` (or any value the `try_from` impls reject) so Rust's filter handles it.

In `packages/rs-platform-wallet/src/manager/load.rs`:
- [BLOCKING] lines 85-137: load_from_persistor leaves orphan wallets in `wallet_manager` on mid-load errors
  Each wallet is registered into `self.wallet_manager` (line 87) before two distinct fail-able checks run: the `wallet_id != expected_wallet_id` validation at line 95-101, and `platform().initialize_from_persisted(...)` at line 119-129. Either failure returns Err while the entry is still live in `wallet_manager`, and may also be missing from `self.wallets` (which is only inserted at line 136). That breaks the manager's invariant that `wallet_manager` and `self.wallets` describe the same set, and a user-driven retry will now collide on duplicate registration / partial state instead of cleanly re-running. Stage the work transactionally — validate `expected_wallet_id` and run `initialize_from_persisted` against a temporary structure first, or roll back the `insert_wallet` on every error path before returning.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- Around line 2162-2171: The fetch predicate for PersistentTxo currently only
selects rows where $0.walletId == walletId, which skips legacy rows with an
empty or nil walletId; update the FetchDescriptor<PersistentTxo> predicate used
when creating descriptor so it also includes legacy rows (e.g. $0.walletId ==
walletId || $0.walletId == "" || $0.walletId == nil) and then continue to filter
spent and account != nil as before (descriptor, backgroundContext.fetch,
routable). Ensure any empty/nil walletId rows are treated/mapped to the current
walletId before further processing so they are included in balance/UTXO restore.

🪄 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: 3394d3ab-03ab-44f7-a346-603702d134e0

📥 Commits

Reviewing files that changed from the base of the PR and between ed5e30a and 46d42fb.

⛔ Files ignored due to path filters (1)

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

📒 Files selected for processing (8)

• packages/rs-platform-wallet-ffi/Cargo.toml
• packages/rs-platform-wallet-ffi/src/manager_diagnostics.rs
• packages/rs-platform-wallet-ffi/src/persistence.rs
• packages/rs-platform-wallet/src/manager/load.rs
• packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerDiagnostics.swift
• packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift
• packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swift
• packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/SendTransactionView.swift

✅ Files skipped from review due to trivial changes (1)

• packages/rs-platform-wallet-ffi/Cargo.toml

🚧 Files skipped from review as they are similar to previous changes (3)

• packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/SendTransactionView.swift
• packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerDiagnostics.swift
• packages/rs-platform-wallet-ffi/src/manager_diagnostics.rs

[thepastaclaw]

Code Review

Round-2 fixes addressed every prior finding cleanly (legacy account-tag skip, AddressBalanceEntryFFI compaction, per-wallet rollback, last_processed_height alignment, key-discriminant fallback, UTXO-drop telemetry, manager_diagnostics dealloc parity). Two blocking issues remain in the restore path: load_from_persistor only rolls back the current iteration so a later-wallet failure leaves earlier wallets registered (poisoning retries), and a stale platform-address balance row aborts the entire restore at persistence.rs:1442. Quality-level follow-ups concern an asymmetric register_wallet rollback gap, a positional 4-tuple on a public accessor, an FFI repr(u8) enum hardening pattern, and minor diagnostic surface nits.

Reviewed commit: 46d42fb

🔴 2 blocking | 🟡 2 suggestion(s) | 💬 3 nitpick(s)

3 additional findings

🔴 blocking: load_from_persistor commits earlier wallets before the whole batch is known-good

packages/rs-platform-wallet/src/manager/load.rs (lines 45-156)

The new rollback only removes the wallet currently being processed (lines 109-110, 142-143). If the snapshot contains multiple wallets and wallet N fails its id check or initialize_from_persisted, wallets 0..N-1 are already inserted into both self.wallet_manager and self.wallets, but the function still returns Err. The caller now holds a half-loaded manager with no signal as to which wallets made it in, and a clean retry of load_from_persistor collides on WalletManager::insert_wallet (returning WalletAlreadyExists) for every previously-loaded wallet — effectively poisoning the manager until the process is rebuilt. For a startup hydration API this needs to be transactional at the manager level: stage all reconstructed (Wallet, PlatformWalletInfo, PlatformWallet) tuples in local Vecs, validate each one, and only commit them to wallet_manager and self.wallets once every iteration has succeeded — or, on any later-iteration failure, unwind every previously-inserted wallet from this call before returning Err.

🔴 blocking: Stale platform-address balance row aborts the entire wallet restore

packages/rs-platform-wallet-ffi/src/persistence.rs (lines 1435-1447)

per_account is built only from wallet.accounts.platform_payment_accounts (lines 1410-1419), but the cached platform_address_balances slice exported from Swift includes every PersistentPlatformAddress for the wallet regardless of whether the corresponding PersistentAccount was restorable from xpubs. If a single snapshot-drift row references a platform-payment account that is missing from the reconstructed wallet (deleted, not yet hydrated, or stale cache), get_mut(...).ok_or_else(...)? at line 1442 aborts build_wallet_start_state and therefore the whole FFIPersister::load() for the wallet. The cache is recoverable by the next platform-address sync, so this should mirror the UTXO restore path above (lines 1364-1371): skip-and-warn on unroutable rows with a per-skip and rollup tracing line, not fail the restore.

🟡 suggestion: register_wallet has the same orphan-registration pattern that load_from_persistor was just fixed for

packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs (lines 173-294)

The PR fix in manager/load.rs rolls back wm.insert_wallet on both id-mismatch and initialize_from_persisted failure. register_wallet here has the structurally identical shape: line 175 inserts into wallet_manager, then load_persisted (line 274) and initialize_from_persisted (lines 281-291) propagate Err via ? with the wallet still registered in wallet_manager and absent from self.wallets. A user retry after a transient persister or platform-address restore failure then hits WalletManager::insert_wallet returning WalletAlreadyExists. Not introduced by this PR, but the fix for the failure mode the prior reviewer identified is incomplete without addressing this site too — apply the same remove_wallet rollback before each ? return after line 175.

🤖 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] lines 45-156: load_from_persistor commits earlier wallets before the whole batch is known-good
  The new rollback only removes the wallet currently being processed (lines 109-110, 142-143). If the snapshot contains multiple wallets and wallet N fails its id check or `initialize_from_persisted`, wallets 0..N-1 are already inserted into both `self.wallet_manager` and `self.wallets`, but the function still returns `Err`. The caller now holds a half-loaded manager with no signal as to which wallets made it in, and a clean retry of `load_from_persistor` collides on `WalletManager::insert_wallet` (returning `WalletAlreadyExists`) for every previously-loaded wallet — effectively poisoning the manager until the process is rebuilt. For a startup hydration API this needs to be transactional at the manager level: stage all reconstructed (Wallet, PlatformWalletInfo, PlatformWallet) tuples in local Vecs, validate each one, and only commit them to `wallet_manager` and `self.wallets` once every iteration has succeeded — or, on any later-iteration failure, unwind every previously-inserted wallet from this call before returning Err.

In `packages/rs-platform-wallet-ffi/src/persistence.rs`:
- [BLOCKING] lines 1435-1447: Stale platform-address balance row aborts the entire wallet restore
  `per_account` is built only from `wallet.accounts.platform_payment_accounts` (lines 1410-1419), but the cached `platform_address_balances` slice exported from Swift includes every `PersistentPlatformAddress` for the wallet regardless of whether the corresponding `PersistentAccount` was restorable from xpubs. If a single snapshot-drift row references a platform-payment account that is missing from the reconstructed wallet (deleted, not yet hydrated, or stale cache), `get_mut(...).ok_or_else(...)?` at line 1442 aborts `build_wallet_start_state` and therefore the whole `FFIPersister::load()` for the wallet. The cache is recoverable by the next platform-address sync, so this should mirror the UTXO restore path above (lines 1364-1371): skip-and-warn on unroutable rows with a per-skip and rollup tracing line, not fail the restore.

In `packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs`:
- [SUGGESTION] lines 173-294: register_wallet has the same orphan-registration pattern that load_from_persistor was just fixed for
  The PR fix in `manager/load.rs` rolls back `wm.insert_wallet` on both id-mismatch and `initialize_from_persisted` failure. `register_wallet` here has the structurally identical shape: line 175 inserts into `wallet_manager`, then `load_persisted` (line 274) and `initialize_from_persisted` (lines 281-291) propagate Err via `?` with the wallet still registered in `wallet_manager` and absent from `self.wallets`. A user retry after a transient persister or platform-address restore failure then hits `WalletManager::insert_wallet` returning `WalletAlreadyExists`. Not introduced by this PR, but the fix for the failure mode the prior reviewer identified is incomplete without addressing this site too — apply the same `remove_wallet` rollback before each `?` return after line 175.

In `packages/rs-platform-wallet/src/manager/accessors.rs`:
- [SUGGESTION] lines 250-294: account_balances_blocking returns a positional 4-tuple — name the fields
  `pub fn account_balances_blocking(&self, wallet_id: &WalletId) -> Vec<(AccountType, WalletCoreBalance, u32, u32)>` is a public accessor that grew tuple slots for `keys_used`/`keys_total`, with the FFI consumer destructuring positionally at `wallet.rs:154`. Tuples of 3+ meaningfully-different fields are a maintenance hazard — adding the next field (e.g. `pool_count`, `last_used_height`) means walking every callsite to fix the destructuring, and the field meanings are invisible at the type level. The rest of this file already uses named-field snapshot structs (`CoreWalletStateSnapshot`, `AccountAddressPoolSnapshot`). Define an `AccountBalanceSnapshot { account_type, balance, keys_used, keys_total }` to match the file's own convention before more callers grow.

[thepastaclaw]

Code Review

Round-4 delta is a single Swift-side migration shim widening the SwiftData TXO fetch to also include legacy empty-walletId rows (routed via the parent account → wallet relationship). The shim is correct but introduces a per-wallet O(legacy_rows × wallets) refetch. All 7 prior findings from the 46d42fb review are still valid at head 4085463 — none of the referenced files were touched, so the two blocking restore-path issues (load_from_persistor multi-wallet rollback gap; stale platform-address row aborting whole restore) and the five lower-severity items remain unresolved.

Reviewed commit: 4085463

🔴 2 blocking | 🟡 2 suggestion(s) | 💬 4 nitpick(s)

3 additional findings

🔴 blocking: load_from_persistor commits earlier wallets before the whole batch is known-good

packages/rs-platform-wallet/src/manager/load.rs (lines 45-156)

Carried forward from prior review — STILL VALID at 4085463. The per-iteration rollback at lines 109-110 and 142-143 only removes the wallet currently being processed. If the snapshot contains multiple wallets and wallet N fails its id check (line 104) or initialize_from_persisted (line 137), wallets 0..N-1 are already committed to both self.wallet_manager (line 96) and self.wallets (line 155) but the function still returns Err. The caller now holds a half-loaded manager with no signal as to which wallets made it in, and a clean retry of load_from_persistor collides on WalletManager::insert_wallet (returning WalletAlreadyExists) for every previously-loaded wallet — effectively poisoning the manager until the process is rebuilt. Because this is invoked from Swift via FFI, the half-registered state is observable across the boundary with no boundary-side reset path. Fix transactionally at the manager level: stage all reconstructed (Wallet, PlatformWalletInfo, PlatformWallet) tuples in local Vecs, validate every one, and only commit them to wallet_manager and self.wallets once every iteration has succeeded — or, on any later-iteration failure, unwind every previously-inserted wallet from this call before returning Err.

🔴 blocking: Stale platform-address balance row aborts the entire wallet restore

packages/rs-platform-wallet-ffi/src/persistence.rs (lines 1435-1447)

Carried forward from prior review — STILL VALID at 4085463. per_account is built only from wallet.accounts.platform_payment_accounts (lines 1410-1419), but the cached platform_address_balances slice exported from Swift includes every PersistentPlatformAddress for the wallet regardless of whether the corresponding PersistentAccount was restorable from xpubs. If a single snapshot-drift row references a platform-payment account that is missing from the reconstructed wallet (deleted, not yet hydrated, or stale cache), per_account.get_mut(...).ok_or_else(...)? at line 1442 aborts build_wallet_start_state and therefore the whole FFIPersister::load() for the wallet. The cache is recoverable on the next platform-address sync, so this should mirror the UTXO restore path immediately above (lines 1373-1389): skip-and-warn on unroutable rows with a per-skip and rollup tracing line (e.g. dropped_unknown_account), not fail the restore.

🟡 suggestion: register_wallet has the same orphan-registration pattern that load_from_persistor was just fixed for

packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs (lines 173-294)

Carried forward from prior review — STILL VALID at 4085463. The PR fix in manager/load.rs rolls back wm.insert_wallet on both id-mismatch and initialize_from_persisted failure. register_wallet here has the structurally identical shape: line 175 inserts into wallet_manager, then load_persisted (line 274) and initialize_from_persisted (lines 281-291) propagate Err via ? with the wallet still registered in wallet_manager and absent from self.wallets. A user retry after a transient persister or platform-address restore failure then hits WalletManager::insert_wallet returning WalletAlreadyExists. Since register_wallet is invoked from Swift via FFI, the orphan registration is observable across the boundary with no recovery surface on the Swift side. Apply the same wm.remove_wallet(&wallet_id) rollback before each ? return after line 175.

🤖 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] lines 45-156: load_from_persistor commits earlier wallets before the whole batch is known-good
  Carried forward from prior review — STILL VALID at 4085463. The per-iteration rollback at lines 109-110 and 142-143 only removes the wallet currently being processed. If the snapshot contains multiple wallets and wallet N fails its id check (line 104) or initialize_from_persisted (line 137), wallets 0..N-1 are already committed to both self.wallet_manager (line 96) and self.wallets (line 155) but the function still returns Err. The caller now holds a half-loaded manager with no signal as to which wallets made it in, and a clean retry of load_from_persistor collides on WalletManager::insert_wallet (returning WalletAlreadyExists) for every previously-loaded wallet — effectively poisoning the manager until the process is rebuilt. Because this is invoked from Swift via FFI, the half-registered state is observable across the boundary with no boundary-side reset path. Fix transactionally at the manager level: stage all reconstructed (Wallet, PlatformWalletInfo, PlatformWallet) tuples in local Vecs, validate every one, and only commit them to wallet_manager and self.wallets once every iteration has succeeded — or, on any later-iteration failure, unwind every previously-inserted wallet from this call before returning Err.

In `packages/rs-platform-wallet-ffi/src/persistence.rs`:
- [BLOCKING] lines 1435-1447: Stale platform-address balance row aborts the entire wallet restore
  Carried forward from prior review — STILL VALID at 4085463. per_account is built only from wallet.accounts.platform_payment_accounts (lines 1410-1419), but the cached platform_address_balances slice exported from Swift includes every PersistentPlatformAddress for the wallet regardless of whether the corresponding PersistentAccount was restorable from xpubs. If a single snapshot-drift row references a platform-payment account that is missing from the reconstructed wallet (deleted, not yet hydrated, or stale cache), per_account.get_mut(...).ok_or_else(...)? at line 1442 aborts build_wallet_start_state and therefore the whole FFIPersister::load() for the wallet. The cache is recoverable on the next platform-address sync, so this should mirror the UTXO restore path immediately above (lines 1373-1389): skip-and-warn on unroutable rows with a per-skip and rollup tracing line (e.g. dropped_unknown_account), not fail the restore.

In `packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs`:
- [SUGGESTION] lines 173-294: register_wallet has the same orphan-registration pattern that load_from_persistor was just fixed for
  Carried forward from prior review — STILL VALID at 4085463. The PR fix in manager/load.rs rolls back wm.insert_wallet on both id-mismatch and initialize_from_persisted failure. register_wallet here has the structurally identical shape: line 175 inserts into wallet_manager, then load_persisted (line 274) and initialize_from_persisted (lines 281-291) propagate Err via ? with the wallet still registered in wallet_manager and absent from self.wallets. A user retry after a transient persister or platform-address restore failure then hits WalletManager::insert_wallet returning WalletAlreadyExists. Since register_wallet is invoked from Swift via FFI, the orphan registration is observable across the boundary with no recovery surface on the Swift side. Apply the same wm.remove_wallet(&wallet_id) rollback before each ? return after line 175.

In `packages/rs-platform-wallet/src/manager/accessors.rs`:
- [SUGGESTION] lines 250-294: account_balances_blocking returns a positional 4-tuple — name the fields
  Carried forward from prior review — STILL VALID at 4085463. pub fn account_balances_blocking(&self, wallet_id: &WalletId) -> Vec<(AccountType, WalletCoreBalance, u32, u32)> is a public accessor that grew tuple slots for keys_used/keys_total, with the FFI consumer destructuring positionally. Tuples of 3+ meaningfully-different fields are a maintenance hazard — adding the next field (e.g. pool_count, last_used_height) means walking every callsite to fix the destructuring, and the field meanings are invisible at the type level. The rest of this file already uses named-field snapshot structs (CoreWalletStateSnapshot, AccountAddressPoolSnapshot). Define an AccountBalanceSnapshot { account_type, balance, keys_used, keys_total } to match the file's own convention before more callers grow.

[thepastaclaw]

Code Review

At 5270cd5a141d459ffc8131c71209a4b2a696f939, the previously reported transactional restore and stale-row issues are fixed, but two restore-boundary correctness problems remain. One is an FFI soundness bug on persisted account tags, and the other is a silent SwiftData failure path that can still restore wallets with zero core-chain funds while reporting success.

Reviewed commit: 5270cd5

🔴 2 blocking

1 additional finding

🔴 blocking: Restore FFI structs materialize unvalidated Swift tag bytes as Rust enums

packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs (lines 89-91)

AccountSpecFFI stores type_tag: AccountTypeTagFFI and standard_tag: StandardAccountTypeTagFFI directly, and UtxoRestoreEntryFFI repeats the same layout at lines 232-234. On the Swift side those fields are written as raw bytes (UInt8(truncatingIfNeeded: ...) / UInt8), and Rust later reads slices of these structs from foreign memory and immediately matches on spec.type_tag and spec.standard_tag in account_type_from_spec (packages/rs-platform-wallet-ffi/src/persistence.rs:1700). In Rust, an out-of-range discriminant in a repr(u8) enum is undefined behavior before the match executes, so a corrupted persisted row, a forward-versioned tag, or any malformed host buffer turns the restore path into UB instead of a recoverable validation error. This FFI layer already uses the safe pattern elsewhere by transporting discriminants as u8 and converting explicitly; these restore structs need the same treatment.

🤖 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-ffi/src/wallet_restore_types.rs`:
- [BLOCKING] lines 89-91: Restore FFI structs materialize unvalidated Swift tag bytes as Rust enums
  `AccountSpecFFI` stores `type_tag: AccountTypeTagFFI` and `standard_tag: StandardAccountTypeTagFFI` directly, and `UtxoRestoreEntryFFI` repeats the same layout at lines 232-234. On the Swift side those fields are written as raw bytes (`UInt8(truncatingIfNeeded: ...)` / `UInt8`), and Rust later reads slices of these structs from foreign memory and immediately matches on `spec.type_tag` and `spec.standard_tag` in `account_type_from_spec` (`packages/rs-platform-wallet-ffi/src/persistence.rs:1700`). In Rust, an out-of-range discriminant in a `repr(u8)` enum is undefined behavior before the `match` executes, so a corrupted persisted row, a forward-versioned tag, or any malformed host buffer turns the restore path into UB instead of a recoverable validation error. This FFI layer already uses the safe pattern elsewhere by transporting discriminants as `u8` and converting explicitly; these restore structs need the same treatment.

In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- [BLOCKING] lines 1978-2004: Unspent-TXO fetch errors are swallowed and restore proceeds as if every wallet had no UTXOs
  `loadWalletList()` wraps the authoritative unspent-TXO preload in `if let unspent = try? backgroundContext.fetch(unspentDescriptor)`, and `loadWalletListCallback()` always returns success (`0`) to Rust. If SwiftData throws here, `unspentBuckets` remains empty and the function still emits `WalletRestoreEntryFFI` records for every wallet. Rust then sees `entry.utxos_count == 0` for those wallets, `build_wallet_start_state()` routes no persisted UTXOs, and `wallet_info.update_balance()` is never called because it only runs when at least one UTXO was restored (`packages/rs-platform-wallet-ffi/src/persistence.rs:1407`). The result is a successful restore with zero core-chain funds and no surfaced error, which reintroduces the same restored-zero-balance failure mode this path is meant to eliminate. The callback contract already supports non-zero failure returns, so this fetch failure must abort the load instead of silently degrading to an empty snapshot.

[coderabbitai]

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (2)

packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift (2)

1975-1980: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Release the preallocated load buffer on early failure.

entriesPtr is allocated before the unspent-TXO fetch. If Line 2003 throws, loadWalletList() returns with errored = true but never releases allocation, so the entries array leaks on each failed restore attempt. Move the allocation below the fallible fetches or explicitly release it before returning.

Also applies to: 2001-2009

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`
around lines 1975 - 1980, The preallocation of entriesPtr via
UnsafeMutablePointer<WalletRestoreEntryFFI>.allocate (assigned into
LoadAllocation.entries) occurs before fallible operations and can leak if an
early throw/return happens; to fix, move the allocation of entriesPtr (and
setting allocation.entries/allocation.entriesCount) to after the unspent-TXO
fetches/safe points in loadWalletList(), or if you must keep it earlier, ensure
you deallocate/free allocation.entries (and reset allocation.entriesCount)
before any early return/throw path so LoadAllocation is always released; look
for the LoadAllocation variable, entriesPtr allocation, WalletRestoreEntryFFI
usage and the errored return paths around loadWalletList() and apply the
deallocation or relocation accordingly.

---

2051-2053: ⚠️ Potential issue | 🟠 Major

Validate persisted account tags before narrowing to UInt8, and release allocations on error paths.

Lines 2052 and 2265 use UInt8(truncatingIfNeeded: acc.accountType) where accountType is UInt32. Truncation silently wraps out-of-range values into the 0–255 range; a corrupt row with accountType > 255 will wrap to a potentially valid tag instead of being rejected by Rust's try_from_u8 validation. Use UInt8(exactly:) and skip or fail when the cast overflows.

Additionally, if the unspent-TXO fetch fails at line 2003–2009, the function returns before storing allocation in loadAllocations, causing the preallocated entriesPtr buffer to leak because LoadAllocation.release() is never called on the orphaned object. Ensure early error paths either release the allocation explicitly or store it in loadAllocations before any fallible operation.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`
around lines 2051 - 2053, The code narrows acc.accountType (UInt32) to
spec.type_tag using truncating cast which can silently wrap invalid values;
change to use UInt8(exactly:) and handle the nil (overflow) case by
skipping/failing the row so invalid accountType >255 is rejected (update the
AccountSpecFFI population where spec.type_tag is set). Also ensure any
preallocated LoadAllocation (entriesPtr) is released on early error paths: if
the unspent-TXO fetch (the block around the fetch that may return early) can
fail, either call LoadAllocation.release() on the orphaned allocation before
returning or store the allocation into loadAllocations immediately before
performing any fallible operation so it’s not leaked (refer to
LoadAllocation.release(), loadAllocations, and the unspent-TXO fetch code).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/rs-platform-wallet-ffi/src/persistence.rs`:
- Around line 1173-1190: The current code swallows every PersistenceError from
account_type_from_spec; change the Err handling to only treat the specific
legacy removed tags as recoverable. Replace the let Ok(...) =
account_type_from_spec(spec) else { ... continue } with a match on
account_type_from_spec(spec) that on Ok(account_type) proceeds, on Err(e)
inspects either the error kind or spec.type_tag (the legacy discriminants like
IdentityAuthenticationEcdsa and IdentityAuthenticationBls) and logs+continues
only for those legacy tags, otherwise propagate the error (return Err(e) or use
?). Apply the same change to the UTXO loop handling at the other block (lines
around the second account_type_from_spec usage) so only legacy-tag cases are
skipped and all other PersistenceError::Backend/validation failures are
surfaced.

---

Outside diff comments:
In
`@packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- Around line 1975-1980: The preallocation of entriesPtr via
UnsafeMutablePointer<WalletRestoreEntryFFI>.allocate (assigned into
LoadAllocation.entries) occurs before fallible operations and can leak if an
early throw/return happens; to fix, move the allocation of entriesPtr (and
setting allocation.entries/allocation.entriesCount) to after the unspent-TXO
fetches/safe points in loadWalletList(), or if you must keep it earlier, ensure
you deallocate/free allocation.entries (and reset allocation.entriesCount)
before any early return/throw path so LoadAllocation is always released; look
for the LoadAllocation variable, entriesPtr allocation, WalletRestoreEntryFFI
usage and the errored return paths around loadWalletList() and apply the
deallocation or relocation accordingly.
- Around line 2051-2053: The code narrows acc.accountType (UInt32) to
spec.type_tag using truncating cast which can silently wrap invalid values;
change to use UInt8(exactly:) and handle the nil (overflow) case by
skipping/failing the row so invalid accountType >255 is rejected (update the
AccountSpecFFI population where spec.type_tag is set). Also ensure any
preallocated LoadAllocation (entriesPtr) is released on early error paths: if
the unspent-TXO fetch (the block around the fetch that may return early) can
fail, either call LoadAllocation.release() on the orphaned allocation before
returning or store the allocation into loadAllocations immediately before
performing any fallible operation so it’s not leaked (refer to
LoadAllocation.release(), loadAllocations, and the unspent-TXO fetch code).

🪄 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: b8cccd7b-0724-4bd2-8038-96efc6f2083b

📥 Commits

Reviewing files that changed from the base of the PR and between 5270cd5 and 91af15f.

📒 Files selected for processing (4)

• packages/rs-platform-wallet-ffi/src/manager_diagnostics.rs
• packages/rs-platform-wallet-ffi/src/persistence.rs
• packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs
• packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift

✅ Files skipped from review due to trivial changes (1)

• packages/rs-platform-wallet-ffi/src/manager_diagnostics.rs

[thepastaclaw]

Code Review

At head 6472d011b6393c863964d3ccd9160a168562d737, the restore-path hardening is mostly in place: the Rust side now validates raw account-tag bytes, SwiftData fetch failures propagate correctly, and the earlier transactional rollback issues are addressed. One restore-boundary defect remains in the Swift marshalling path: malformed persisted account tags are skipped locally and still reported to Rust as a successful restore, which can produce partially restored or empty wallets without surfacing an error.

Reviewed commit: 6472d01

🔴 1 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/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- [BLOCKING] lines 2066-2072: Corrupt persisted account tags are dropped silently and still reported as a successful restore
  `loadWalletList()` filters `restorable` wallets before validating any account tags, then skips rows whose `accountType` does not fit in `UInt8` and continues building the wallet entry. The callback still returns success unless a SwiftData fetch failed, so Rust accepts the entry with `accounts_count == 0` or with a truncated account set. `build_wallet_start_state()` treats that as a valid snapshot and constructs a watch-only wallet from the reduced account list, and `PlatformWalletManager.loadFromPersistor()` still asks SwiftData for the same wallet ids via `restorableWalletIds()`. The result is a restore that appears successful even though corrupted persisted rows were discarded and funds-bearing accounts or their UTXOs may be missing. This code needs to fail the load callback when a persisted account tag is invalid instead of degrading to a partial restore.

[thepastaclaw]

🔴 Blocking: Corrupt persisted account tags are dropped silently and still reported as a successful restore

loadWalletList() filters restorable wallets before validating any account tags, then skips rows whose accountType does not fit in UInt8 and continues building the wallet entry. The callback still returns success unless a SwiftData fetch failed, so Rust accepts the entry with accounts_count == 0 or with a truncated account set. build_wallet_start_state() treats that as a valid snapshot and constructs a watch-only wallet from the reduced account list, and PlatformWalletManager.loadFromPersistor() still asks SwiftData for the same wallet ids via restorableWalletIds(). The result is a restore that appears successful even though corrupted persisted rows were discarded and funds-bearing accounts or their UTXOs may be missing. This code needs to fail the load callback when a persisted account tag is invalid instead of degrading to a partial restore.

_{source: ['codex-general']}

🤖 Fix this 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/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- [BLOCKING] lines 2066-2072: Corrupt persisted account tags are dropped silently and still reported as a successful restore
  `loadWalletList()` filters `restorable` wallets before validating any account tags, then skips rows whose `accountType` does not fit in `UInt8` and continues building the wallet entry. The callback still returns success unless a SwiftData fetch failed, so Rust accepts the entry with `accounts_count == 0` or with a truncated account set. `build_wallet_start_state()` treats that as a valid snapshot and constructs a watch-only wallet from the reduced account list, and `PlatformWalletManager.loadFromPersistor()` still asks SwiftData for the same wallet ids via `restorableWalletIds()`. The result is a restore that appears successful even though corrupted persisted rows were discarded and funds-bearing accounts or their UTXOs may be missing. This code needs to fail the load callback when a persisted account tag is invalid instead of degrading to a partial restore.

[coderabbitai]

🧹 Nitpick comments (1)

packages/rs-platform-wallet-ffi/src/persistence.rs (1)

1188-1204: 💤 Low value

Optionally extract the is_legacy_tag predicate into a private helper.

The identical two-line check:

spec.type_tag == AccountTypeTagFFI::IdentityAuthenticationEcdsa as u8
    || spec.type_tag == AccountTypeTagFFI::IdentityAuthenticationBls as u8

is duplicated verbatim at both the account-loader and the UTXO-loader callsites. If a third AccountType is deprecated and needs the same "skip-and-continue" treatment, both callers must be updated consistently.

♻️ Proposed refactor

Add a module-private helper below account_type_from_spec:

+/// Returns `true` for the ABI-only `IdentityAuthentication{Ecdsa,Bls}` tag bytes
+/// whose upstream `AccountType` variants were removed (TODO(events)).  These are
+/// the only tags that `account_type_from_spec` deliberately returns `Err` for
+/// while still being valid discriminants — callers use this predicate to
+/// distinguish "recoverable drift" from "real corruption / out-of-range byte".
+fn is_legacy_removed_account_tag(type_tag: u8) -> bool {
+    type_tag == AccountTypeTagFFI::IdentityAuthenticationEcdsa as u8
+        || type_tag == AccountTypeTagFFI::IdentityAuthenticationBls as u8
+}

Then in the account loader (line 1191–1194):

-                let is_legacy_tag = spec.type_tag
-                    == AccountTypeTagFFI::IdentityAuthenticationEcdsa as u8
-                    || spec.type_tag == AccountTypeTagFFI::IdentityAuthenticationBls as u8;
-                if is_legacy_tag {
+                if is_legacy_removed_account_tag(spec.type_tag) {

And in the UTXO loader (line 1294–1297):

-                let is_legacy_tag = u.type_tag
-                    == AccountTypeTagFFI::IdentityAuthenticationEcdsa as u8
-                    || u.type_tag == AccountTypeTagFFI::IdentityAuthenticationBls as u8;
-                if is_legacy_tag {
+                if is_legacy_removed_account_tag(u.type_tag) {

Also applies to: 1291-1308

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/rs-platform-wallet-ffi/src/persistence.rs` around lines 1188 - 1204,
Extract the duplicated predicate into a module-private helper (e.g., fn
is_legacy_identity_tag(type_tag: u8) -> bool) placed near or below
account_type_from_spec, implement it to return true when type_tag equals
AccountTypeTagFFI::IdentityAuthenticationEcdsa as u8 or
AccountTypeTagFFI::IdentityAuthenticationBls as u8, then replace the duplicate
inline checks in the account-loader and UTXO-loader error branches to call
is_legacy_identity_tag(spec.type_tag) and keep the existing warn+continue
behavior when it returns true.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/rs-platform-wallet-ffi/src/persistence.rs`:
- Around line 1188-1204: Extract the duplicated predicate into a module-private
helper (e.g., fn is_legacy_identity_tag(type_tag: u8) -> bool) placed near or
below account_type_from_spec, implement it to return true when type_tag equals
AccountTypeTagFFI::IdentityAuthenticationEcdsa as u8 or
AccountTypeTagFFI::IdentityAuthenticationBls as u8, then replace the duplicate
inline checks in the account-loader and UTXO-loader error branches to call
is_legacy_identity_tag(spec.type_tag) and keep the existing warn+continue
behavior when it returns true.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2932d441-e93c-4f4b-822e-51b0a82e7f4a

📥 Commits

Reviewing files that changed from the base of the PR and between 91af15f and 48eec7e.

📒 Files selected for processing (2)

• packages/rs-platform-wallet-ffi/src/persistence.rs
• packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift