Skip to content

feat(platform-wallet,swift-sdk): query per-account balances via FFI#3572

Merged
QuantumExplorer merged 8 commits intov3.1-devfrom
per-account-balances
Apr 30, 2026
Merged

feat(platform-wallet,swift-sdk): query per-account balances via FFI#3572
QuantumExplorer merged 8 commits intov3.1-devfrom
per-account-balances

Conversation

@QuantumExplorer
Copy link
Copy Markdown
Member

@QuantumExplorer QuantumExplorer commented Apr 30, 2026

Issue being fixed or feature implemented

Replace the expensive TXO-query-based balance computation in SwiftExampleApp views with a direct FFI query that reads Rust's in-memory ManagedCoreAccount.balance values maintained during SPV processing. Depends on rust-dashcore PR #706 which added per-account balance tracking to the event bus.

What was done?

Rust side:

  • PlatformWalletManager::account_balances_blocking() — reads per-account balances directly from the in-memory WalletManager via blocking_read (no disk I/O)
  • platform_wallet_manager_get_account_balances FFI function — returns a heap-allocated array of AccountBalanceEntryFFI structs (one per account), each carrying the typed AccountTypeTagFFI discriminants plus confirmed/unconfirmed/immature/locked balance fields
  • platform_wallet_manager_free_account_balances — frees the returned array
  • AccountBalanceEntryFFI #[repr(C)] struct reusing the existing AccountChangeSetTags tag projection
  • Fixed API breaks from the new rust-dashcore rev: from_wallet now requires birth_height, account_type renamed to managed_account_type

Swift side:

  • PlatformWalletManager.AccountBalance struct + accountBalances(for walletId:) wrapper that calls the FFI, marshals results, and frees the array
  • AccountRowView now takes coreConfirmedBalance/coreUnconfirmedBalance directly instead of filtering TXO arrays
  • BalanceCardView sums FFI-queried balances instead of walking walletTxos
  • AccountDetailView.balanceCard() queries FFI for the specific account instead of reading never-written PersistentAccount.balanceConfirmed/balanceUnconfirmed
  • Removed per-account TXO indexing infrastructure from AccountListView (the txosByAddress dictionary, the txos(for:index:) method, and the walletTxos parameter)

How Has This Been Tested?

  • cargo check -p platform-wallet -p platform-wallet-ffi passes clean
  • cargo fmt --all applied
  • Verified the C header struct layout matches the Rust #[repr(C)] layout exactly

Breaking Changes

None — the FFI additions are purely additive. The Swift view parameter changes are internal to the example app.

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

Summary by CodeRabbit

  • New Features

    • Per-account balance query added (confirmed, unconfirmed, immature, locked) and surfaced in the SDK.
    • Example app now shows real-time core balances across AccountDetail, AccountList, WalletDetail.
    • Wallet explorer expanded into a diagnostics view with sync states, identity/token syncs, and detailed per-wallet/account balances.
  • Chores

    • Workspace dependencies updated to newer revisions.

…emory state via FFI

Replace the expensive TXO-query-based balance computation in
SwiftExampleApp views with a direct FFI query that reads Rust's
in-memory ManagedCoreAccount.balance values, updated during SPV
processing. Bumps rust-dashcore deps to pick up the event-bus
balance tracking from dashcore PR #706.

Rust side:
- Add account_balances_blocking() on PlatformWalletManager
- Add platform_wallet_manager_get_account_balances FFI function
- Add AccountBalanceEntryFFI repr(C) struct
- Fix API breaks from new rust-dashcore rev (from_wallet birth_height
  param, managed_account_type rename)

Swift side:
- Add AccountBalance struct and accountBalances(for:) on
  PlatformWalletManager
- AccountRowView/BalanceCardView/AccountDetailView now read balances
  from FFI instead of filtering TXO rows

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added this to the v3.1.0 milestone Apr 30, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 30, 2026

Warning

Rate limit exceeded

@QuantumExplorer has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 40 minutes and 51 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a4f96948-ae34-4b07-82a0-4d5d1df16223

📥 Commits

Reviewing files that changed from the base of the PR and between 50efe04 and b138497.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • Cargo.toml
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/WalletMemoryExplorerView.swift
📝 Walkthrough

Walkthrough

Bumps several workspace Git dependency revisions; adds FFI types and functions to return per-account balances; exposes account-tag/index types; adds manager accessor for blocking per-account balances; updates WalletInfo constructors (birth_height) and Swift SDK/UI to use FFI-sourced balances.

Changes

Cohort / File(s) Summary
Workspace Dependencies
Cargo.toml
Bumps workspace dependency revisions (dashcore, dash-spv, dash-spv-ffi, key-wallet, key-wallet-ffi, key-wallet-manager, dashcore-rpc) from commit ea33cbc8fe9ea3.
FFI Type Definitions
packages/rs-platform-wallet-ffi/src/core_wallet_types.rs
Adds AccountBalanceEntryFFI (C-ABI), makes AccountChangeSetTags public and adds index: u32, and exposes account_type_to_tags (now populating index).
FFI Functions
packages/rs-platform-wallet-ffi/src/wallet.rs
Adds platform_wallet_manager_get_account_balances to return a heap-allocated array of AccountBalanceEntryFFI + count and platform_wallet_manager_free_account_balances to free it; includes null-checks and handle validation.
FFI Callsite Updates
packages/rs-platform-wallet-ffi/src/platform_wallet_info.rs, packages/rs-platform-wallet-ffi/src/persistence.rs
Updates PlatformWalletInfo::from_wallet / ManagedWalletInfo constructor calls to pass an explicit birth_height (0).
Rust Manager Accessors
packages/rs-platform-wallet/src/manager/accessors.rs
Adds pub fn account_balances_blocking(&self, wallet_id: &WalletId) -> Vec<(AccountType, WalletCoreBalance)> returning per-account balance snapshots from in-memory state.
Wallet Lifecycle & Account Fields
packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs, packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs
Adjusts wallet registration to new constructor signature and switches account-type reads from account.account_type to account.managed_account_type where applicable.
Trait API Changes
packages/rs-platform-wallet/src/wallet/platform_wallet_traits.rs
WalletInfoInterface::from_wallet and from_wallet_with_name now accept birth_height: CoreBlockHeight; set_birth_height removed from PlatformWalletInfo impl.
Swift FFI Wrapper
packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift
Adds AccountBalance struct and accountBalances(for:) calling the new FFI, mapping entries, and freeing the returned buffer with defer.
Swift UI Refactor
packages/swift-sdk/SwiftExampleApp/.../AccountDetailView.swift, AccountListView.swift, WalletDetailView.swift, WalletMemoryExplorerView.swift
Replaces TXO-derived balance calculations with FFI-sourced per-account balances; updates view initializers/props and row rendering to use core confirmed/unconfirmed/immature/locked balances; expands diagnostics to surface manager state.

Sequence Diagram

sequenceDiagram
    actor UI as Swift UI
    participant SwiftFFI as Swift FFI wrapper
    participant FFI as Rust FFI exports
    participant Manager as PlatformWalletManager
    participant State as In-Memory State

    UI->>SwiftFFI: accountBalances(for: walletId)
    SwiftFFI->>FFI: platform_wallet_manager_get_account_balances(handle, walletId)
    FFI->>Manager: validate handle & walletId
    Manager->>State: blocking read -> collect accounts & balances
    State-->>Manager: Vec<(AccountType, WalletCoreBalance)>
    Manager-->>FFI: assemble boxed AccountBalanceEntryFFI array + count
    FFI-->>SwiftFFI: pointer + count (or null/0 on error)
    SwiftFFI-->>UI: map entries -> AccountBalance[]
    UI->>SwiftFFI: platform_wallet_manager_free_account_balances(ptr, count)
    SwiftFFI->>FFI: free request
    FFI->>Manager: drop boxed slice -> memory reclaimed
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested Reviewers

  • shumkov

"🐇 I hopped through Rust and Swift today,
Balances fetched the tidy new way,
No TXO sifting, neat and bright,
FFI brings balances into sight! ✨"

🚥 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(platform-wallet,swift-sdk): query per-account balances via FFI' clearly and specifically describes the main change: adding per-account balance querying functionality via FFI across the platform-wallet and swift-sdk packages.
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 per-account-balances

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
Review rate limit: 0/1 reviews remaining, refill in 40 minutes and 51 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@thepastaclaw
Copy link
Copy Markdown
Collaborator

thepastaclaw commented Apr 30, 2026

Review Gate

Commit: b1384977

  • Debounce: 14m ago (need 30m)

  • CI checks: checks still running (8 pending)

  • CodeRabbit review: comment found

  • Off-peak hours: peak window (5am-11am PT) — currently 09:59 AM PT Thursday

  • Run review now (check to override)

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift (1)

603-615: 💤 Low value

Consider caching FFI balance results to avoid duplicate calls.

Both confirmedBalance and unconfirmedBalance independently call walletManager.accountBalances(for:), resulting in two FFI round-trips per view render. While the call is documented as "no disk I/O", acquiring the blocking read lock twice is unnecessary overhead.

A single cached property could serve both:

private var accountBalances: [PlatformWalletManager.AccountBalance] {
    walletManager.accountBalances(for: wallet.walletId)
}

private var confirmedBalance: UInt64 {
    accountBalances.reduce(0) { $0 + $1.confirmed }
}

private var unconfirmedBalance: UInt64 {
    accountBalances.reduce(0) { $0 + $1.unconfirmed }
}

However, since this is an example app and the FFI call is fast, this is a minor optimization.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift`
around lines 603 - 615, The view currently performs
walletManager.accountBalances(for: wallet.walletId) twice via the computed
properties confirmedBalance and unconfirmedBalance; cache the result by
introducing a single computed property (e.g. accountBalances) that returns
walletManager.accountBalances(for: wallet.walletId) and then change
confirmedBalance and unconfirmedBalance to reduce over accountBalances instead
of calling walletManager.accountBalances(...) directly so the FFI/read-lock is
only acquired once per render.
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/AccountListView.swift (1)

65-72: ⚡ Quick win

Pre-index balances to avoid repeated linear scans during row rendering.

balances.first { ... } runs per row, making this section quadratic as account count grows. Build a lookup map once and do O(1) row lookups.

Proposed refactor
-                let balances = walletManager.accountBalances(for: wallet.walletId)
+                let balances = walletManager.accountBalances(for: wallet.walletId)
+                let balanceByKey = Dictionary(
+                    uniqueKeysWithValues: balances.map {
+                        ((UInt32($0.typeTag), $0.standardTag, $0.index), $0)
+                    }
+                )
                 List(orderedAccounts) { account in
                     NavigationLink(destination: AccountDetailView(wallet: wallet, account: account)) {
-                        let match = balances.first { b in
-                            UInt32(b.typeTag) == account.accountType &&
-                            b.standardTag == account.standardTag &&
-                            b.index == account.accountIndex
-                        }
+                        let match = balanceByKey[(account.accountType, account.standardTag, account.accountIndex)]
                         AccountRowView(
                             account: account,
                             coreConfirmedBalance: match?.confirmed ?? 0,
                             coreUnconfirmedBalance: match?.unconfirmed ?? 0
                         )
                     }
                 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/AccountListView.swift`
around lines 65 - 72, The code currently calls balances.first { ... } inside the
List row (in AccountListView where NavigationLink is built), causing O(n^2)
work; instead, precompute a lookup map once before rendering by iterating
walletManager.accountBalances(for: wallet.walletId) and creating a dictionary
keyed by the tuple of (typeTag, standardTag, index) (or a concatenated string
key) to the balance; then inside the List/NavigationLink replace balances.first
{ ... } with a direct dictionary lookup using account.accountType,
account.standardTag and account.accountIndex so each row does O(1) work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Cargo.toml`:
- Around line 52-58: This upgrade pins multiple workspace crates (dashcore,
dash-spv, dash-spv-ffi, key-wallet, key-wallet-ffi, key-wallet-manager,
dashcore-rpc) to a new git rev that introduces API breaks (notably the
from_wallet birth_height param change and managed_account_type rename); verify
and fix all dependent workspace consumers (rs-dpp, rs-dapi,
rs-platform-encryption and any FFI consumers) by compiling each crate, updating
call sites to the new function/parameter names and types (e.g., adjust uses of
from_wallet to supply/handle the revised birth_height parameter and rename
managed_account_type occurrences), run cargo workspace tests, and add a short
note to the PR documenting which consumers were built and any code modifications
made to accommodate the API changes.

In `@packages/rs-platform-wallet-ffi/src/wallet.rs`:
- Around line 220-223: The current code boxes an empty slice and writes a
pointer even when there are no entries; change the logic after computing let
count = entries.len() to check if count == 0 and, in that branch, set
*out_entries to a null pointer (e.g. std::ptr::null()) and *out_count = 0
(instead of boxing) to match the FFI contract; otherwise proceed to box the
slice and set *out_entries = Box::into_raw(boxed) as *const _ and *out_count =
count.

In
`@packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift`:
- Around line 416-420: The guard in the FFI response handling (checking
PLATFORM_WALLET_FFI_RESULT_SUCCESS, outEntries and outCount) currently returns
[] on failure; instead, before returning, set or publish the failure via
lastError (or call the manager's error propagation method) with the FFI
result/error details so callers can distinguish a real empty result from a query
failure, then return; update the handling in the function that processes
outEntries (e.g., in PlatformWalletManager's FFI query method) to populate
lastError when result != PLATFORM_WALLET_FFI_RESULT_SUCCESS or outEntries is
nil/outCount == 0.

---

Nitpick comments:
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/AccountListView.swift`:
- Around line 65-72: The code currently calls balances.first { ... } inside the
List row (in AccountListView where NavigationLink is built), causing O(n^2)
work; instead, precompute a lookup map once before rendering by iterating
walletManager.accountBalances(for: wallet.walletId) and creating a dictionary
keyed by the tuple of (typeTag, standardTag, index) (or a concatenated string
key) to the balance; then inside the List/NavigationLink replace balances.first
{ ... } with a direct dictionary lookup using account.accountType,
account.standardTag and account.accountIndex so each row does O(1) work.

In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift`:
- Around line 603-615: The view currently performs
walletManager.accountBalances(for: wallet.walletId) twice via the computed
properties confirmedBalance and unconfirmedBalance; cache the result by
introducing a single computed property (e.g. accountBalances) that returns
walletManager.accountBalances(for: wallet.walletId) and then change
confirmedBalance and unconfirmedBalance to reduce over accountBalances instead
of calling walletManager.accountBalances(...) directly so the FFI/read-lock is
only acquired once per render.
🪄 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: b7779ee7-7862-4faa-b093-9b6d17c4ba84

📥 Commits

Reviewing files that changed from the base of the PR and between 964ab60 and eac5b20.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (13)
  • Cargo.toml
  • packages/rs-platform-wallet-ffi/src/core_wallet_types.rs
  • packages/rs-platform-wallet-ffi/src/persistence.rs
  • packages/rs-platform-wallet-ffi/src/platform_wallet_info.rs
  • packages/rs-platform-wallet-ffi/src/wallet.rs
  • packages/rs-platform-wallet/src/manager/accessors.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_wallet_traits.rs
  • packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/AccountDetailView.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/AccountListView.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift

Comment thread Cargo.toml
Comment thread packages/rs-platform-wallet-ffi/src/wallet.rs
@thepastaclaw
Copy link
Copy Markdown
Collaborator

Review Gate

Commit: eac5b201

  • Debounce: 109m ago (need 30m)

  • CI checks: builds passed, 0/0 tests passed

  • CodeRabbit review: comment found

  • Off-peak hours: peak window (5am-11am PT) — currently 07:00 AM PT Thursday

  • Run review now (check to override)

…ager state

Add sections for every piece of in-memory state accessible from
PlatformWalletManager:
- SPV sync progress (headers, filters, masternodes)
- Platform address sync state (running, syncing, last timestamp)
- Identity token sync state (per-identity token balances, nonces)
- Per-wallet balance breakdown (confirmed/unconfirmed/immature/locked)
- Per-account core balances via the new FFI query
- Per-identity token sync snapshot on the identity detail view

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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/SwiftExampleApp/SwiftExampleApp/Views/WalletMemoryExplorerView.swift`:
- Around line 307-308: The view currently uses guard summary == nil in
loadOnce() which blocks subsequent updates to walletBalance, accountBalances,
identityIds, and watchedIds after the first successful load; change the logic so
loadOnce() (or a new loadIfNeeded/refresh method) does not bail out solely on
summary being non-nil and instead either subscribes to a walletManager refresh
trigger or re-fetches balances and id lists on every onAppear; update calls in
WalletMemoryExplorerView (and the same pattern around lines 483-514) to
explicitly refresh walletBalance/accountBalances/identityIds/watchedIds when the
view appears or when walletManager emits a sync update, keeping summary
assignment but not using it as the single gate for all reloads.
- Around line 98-104: The explorer's sync state is being snapshot on .onAppear
using `@State` vars (addressSyncRunning, addressSyncing, addressSyncLastUnix,
identitySyncRunning, identitySyncing, identityTokenRows, loadError) which causes
stale UI; update WalletMemoryExplorerView to derive these values directly from
PlatformWalletManager (or subscribe to its publisher) instead of setting them
once in onAppear — replace the `@State` snapshots with computed properties or use
.task(id: walletManager) / .onReceive(walletManager.publisher) to update state
reactively so the "Platform Address Sync" and "Identity Token Sync" sections
reflect live manager updates.
- Around line 227-233: The walletRow view is synchronously calling
wallet.inMemorySummary() and wallet.balance() during SwiftUI rendering which
causes repeated Rust/FFI reads and UI jank; move these expensive calls out of
the body by creating a cached snapshot published by the walletManager (or
compute and store InMemoryWalletSummary and balance when wallets update) and
have walletRow subscribe to that snapshot, or precompute per-wallet summaries
via a method on WalletManager (e.g., getWalletSnapshot(walletId:)) so walletRow
only reads lightweight published properties instead of calling inMemorySummary()
and balance() directly.
- Around line 255-259: The current try? calls (for
walletManager.isPlatformAddressSyncRunning(), isPlatformAddressSyncing(),
lastPlatformAddressSyncUnixSeconds(), isIdentityTokenSyncRunning(),
isIdentityTokenSyncing()) swallow errors and present healthy defaults; change
them to use do/try/catch and surface failures into the View's loadError the same
way allIdentityTokenSyncRows() does — call each method with try inside a do
block, assign the value on success, and in catch append or set the thrown error
to loadError so the UI shows real error states instead of defaulting to false/0.
🪄 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: 9327d6b3-0d9d-41d9-8adc-8aa3e54bab47

📥 Commits

Reviewing files that changed from the base of the PR and between eac5b20 and 97e3257.

📒 Files selected for processing (1)
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/WalletMemoryExplorerView.swift

Comment on lines +98 to +104
@State private var addressSyncRunning = false
@State private var addressSyncing = false
@State private var addressSyncLastUnix: UInt64 = 0
@State private var identitySyncRunning = false
@State private var identitySyncing = false
@State private var identityTokenRows: [IdentityTokenSyncRow] = []
@State private var loadError: String?
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make the explorer sections reactive instead of snapshotting them once.

These @State fields are populated only from .onAppear, so the "Platform Address Sync" and "Identity Token Sync" sections stop updating while the manager keeps syncing in the background. That leaves stale diagnostics on screen even though walletManager is still changing. Please derive these values directly from PlatformWalletManager or refresh them from a publisher / task(id:) tied to manager updates.

Also applies to: 123-124

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/WalletMemoryExplorerView.swift`
around lines 98 - 104, The explorer's sync state is being snapshot on .onAppear
using `@State` vars (addressSyncRunning, addressSyncing, addressSyncLastUnix,
identitySyncRunning, identitySyncing, identityTokenRows, loadError) which causes
stale UI; update WalletMemoryExplorerView to derive these values directly from
PlatformWalletManager (or subscribe to its publisher) instead of setting them
once in onAppear — replace the `@State` snapshots with computed properties or use
.task(id: walletManager) / .onReceive(walletManager.publisher) to update state
reactively so the "Platform Address Sync" and "Identity Token Sync" sections
reflect live manager updates.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional for a diagnostics view — SPV progress is already reactive via @Published, and the address/identity sync booleans are point-in-time FFI reads that don't have publishers. Adding a polling timer or publisher bridge for a debug screen would add complexity without meaningful benefit. The user can swipe-to-dismiss and re-open to refresh.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@QuantumExplorer, understood — that's a reasonable design for a diagnostics screen. Since the address/identity sync booleans are point-in-time FFI reads with no publisher backing, and SPV progress is already covered reactively via @Published, snapshotting on .onAppear is the appropriate approach here without over-engineering a polling bridge for a debug surface. I'll note this for future reviews.


✏️ Learnings added
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 3572
File: packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/WalletMemoryExplorerView.swift:98-104
Timestamp: 2026-04-30T16:28:31.615Z
Learning: In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/WalletMemoryExplorerView.swift`, the `State` snapshot pattern (addressSyncRunning, addressSyncing, addressSyncLastUnix, identitySyncRunning, identitySyncing, identityTokenRows) populated once in `.onAppear` is intentional for this diagnostics/debug view. SPV progress is already reactive via `Published` on `PlatformWalletManager`; the address/identity sync booleans are point-in-time FFI reads with no publisher backing. Adding a polling timer or publisher bridge for this debug screen is considered unnecessary complexity — the user can swipe-to-dismiss and re-open to refresh. Do not flag this as a stale-UI bug.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2026-04-24T14:52:52.435Z
Learning: All wallet operations now flow through `PlatformWalletManager`; all Core wallet data is surfaced to views via SwiftData Query on the Persistent* models

Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2026-04-24T14:52:52.435Z
Learning: `PlatformWalletManager` drives wallet creation, SPV sync, and BLAST balance sync via the Rust-side `platform-wallet` crate, holds N wallets concurrently keyed by walletId, publishes `spvProgress`, `wallets`, and `lastError`, and persists data via SwiftData using `PlatformWalletPersistenceHandler`

Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-23T11:23:55.445Z
Learning: In SwiftExampleApp, use `UnifiedAppState` to coordinate Core and Platform features, `WalletService` for SPV wallet operations, and `PlatformService` for identity and document operations

Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 3377
File: packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/UnifiedAppState.swift:156-171
Timestamp: 2026-03-17T16:45:59.645Z
Learning: In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/UnifiedAppState.swift`, `performPlatformBalanceSync()` intentionally processes only `wallets.first` (single-wallet design). Multi-wallet BLAST sync is deferred until the SwiftExampleApp UI supports wallet selection. Do not flag this as a bug.

Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2026-04-24T14:52:52.435Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*.swift : Use SwiftData for local persistence with key models: `PersistentIdentity`, `PersistentDocument`, `PersistentContract`, `PersistentToken`, `PersistentTokenBalance`, `PersistentPublicKey`, `PersistentWallet`, `PersistentAccount`, `PersistentTransaction`, and `PersistentUtxo`

Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2026-04-24T14:52:52.435Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*.swift : Use `Query` with SwiftUI views for reactive data binding to SwiftData models, breaking complex views into smaller components to avoid compiler timeouts

Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 3377
File: packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swift:543-550
Timestamp: 2026-03-18T12:23:15.555Z
Learning: In `packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swift`, the `getPlatformAddresses(walletId:accountIndex:keyClass:)` method intentionally returns an empty array `[]` for invalid/missing wallet IDs rather than throwing an error. This "soft-lookup" pattern is consistent with `getManagedAccountCollection(walletId:)`, which also returns `nil` silently for bad IDs. Do not flag the missing 32-byte length guard in this method as a bug.

Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/CLAUDE.md:0-0
Timestamp: 2026-04-25T12:18:49.530Z
Learning: Applies to packages/swift-sdk/**/Sources/SwiftDashSDK/**/*.swift : Core SPV sync and UTXO tracking must route through platform-wallet, not be re-implemented in Swift

Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2026-04-24T14:52:52.435Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*.swift : Use `NavigationLink` for drill-down navigation and implement proper loading and error states in UI views

Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 3564
File: packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs:300-317
Timestamp: 2026-04-29T20:49:13.577Z
Learning: In `packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs`, the `platform_wallet.identity().sync().await` call inside `register_wallet` is intentionally blocking (not detached to a background task). This is a UX design choice: for recovery flows, identity data must be hydrated before the wallet detail view renders to avoid showing an empty identity list that fills in seconds later. The sync is bounded by Platform query timeouts (~10s worst case) and failures are non-fatal (caught + logged with `tracing::warn`). Do not flag this as a blocking-call issue.

Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 3377
File: packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/CoreWalletManager.swift:76-83
Timestamp: 2026-03-18T12:23:18.483Z
Learning: In `packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/CoreWalletManager.swift`, the DIP-17 Platform Payment account (created via `sdkWalletManager.ensurePlatformPaymentAccount(walletId:)`) is intentionally ephemeral — it is NOT persisted in the serialized wallet bytes captured by `addWalletAndSerialize`. Instead, it is recreated on every app launch during wallet restore. This means it is expected and correct that `ensurePlatformPaymentAccount` runs after `serializedBytes` is captured, and no re-serialization is needed after the platform account is added.

Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 3564
File: packages/rs-platform-wallet/src/changeset/core_bridge.rs:105-109
Timestamp: 2026-04-29T20:49:13.090Z
Learning: In `packages/rs-platform-wallet/src/changeset/core_bridge.rs`, the `RecvError::Lagged` arm in `spawn_wallet_event_adapter` intentionally only logs a warning and continues. A full wallet reconciliation/resync cannot be triggered here because the upstream `key_wallet_manager`/`dashcore` crate does not yet expose a rescan entry point. This is known technical debt to revisit when rust-dashcore adds a rescan API.

Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/CLAUDE.md:0-0
Timestamp: 2026-04-25T12:18:49.530Z
Learning: Applies to packages/swift-sdk/**/Sources/SwiftDashSDK/**/*.swift : Identity discovery (gap-limit scan) operations must route through platform-wallet, not be re-implemented in Swift

Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/CLAUDE.md:0-0
Timestamp: 2026-04-25T12:18:49.530Z
Learning: Applies to packages/swift-sdk/**/Sources/SwiftDashSDK/**/*.swift : High-level operations spanning identities, platform balances, core sync, tokens, DashPay, or identity key derivation must route through platform-wallet and must not be re-implemented in the Swift SDK

Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/CLAUDE.md:0-0
Timestamp: 2026-04-25T12:18:49.530Z
Learning: Applies to packages/swift-sdk/**/Sources/SwiftDashSDK/**/*.swift : Token balance sync operations must route through platform-wallet, not be re-implemented in Swift

Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/CLAUDE.md:0-0
Timestamp: 2026-04-25T12:18:49.530Z
Learning: Applies to packages/swift-sdk/**/Sources/SwiftDashSDK/**/*.swift : Platform balance sync operations must route through platform-wallet, not be re-implemented in Swift

Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/CLAUDE.md:0-0
Timestamp: 2026-04-25T12:18:49.530Z
Learning: Applies to packages/swift-sdk/**/Sources/SwiftDashSDK/**/*.swift : Identity key derivation and any action using a derived key must route through platform-wallet, not be re-implemented in Swift

Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/src/mimic/test_quorum.rs:159-164
Timestamp: 2024-11-20T16:16:01.830Z
Learning: QuantumExplorer prefers not to receive auto-generated messages asking to post on social media.

Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 3220
File: packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/transform_into_action/v0/mod.rs:70-87
Timestamp: 2026-03-10T16:51:54.576Z
Learning: In `packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/transform_into_action/v0/mod.rs` (and the parallel unshield path), the pool balance check at the balance sufficiency guard only validates `current_total_balance >= unshielding_amount`, but does NOT account for the fee component. This is a known issue (related to AUDIT_FINDINGS.md H2: fee_amount hardcoded to 0, and M1: wrong fee semantics for Unshield/ShieldedWithdrawal). The fix — computing `total_debit = unshielding_amount + fee` and comparing against `current_total_balance` — is deferred to a follow-up PR that restructures the shielded pool fee model. Do not flag this as an unreviewed bug in future reviews of this file; it is tracked.

Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 3377
File: packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Models/DashAddress.swift:26-32
Timestamp: 2026-03-18T06:10:47.250Z
Learning: In packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Models/DashAddress.swift, the platform address HRPs used in Swift (`dashevo` for mainnet, `tdashevo` for testnet) are transitional/demo values. The canonical, authoritative DIP-0018 bech32m encoding for platform addresses goes through the Rust FFI function `dash_sdk_encode_platform_address`, which internally calls `PlatformAddress::to_bech32m_string` from the DPP crate. The Swift parsing path in `DashAddress.swift` is expected to be replaced in a follow-up.

QuantumExplorer and others added 4 commits April 30, 2026 23:29
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Only show accounts with non-zero balance in the disclosure list.
Special-purpose accounts (identity registration, provider keys, etc.)
never hold funds; showing them all at 0 DASH was just noise.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Key-derivation-only account types (IdentityRegistration, ProviderKeys,
etc.) can never hold funds. Filter them in account_balances_blocking()
instead of hiding zero-balance accounts in the Swift UI — accounts with
0 balance (e.g. an empty Standard account) should still be shown.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Return null+0 for empty FFI results instead of boxing empty slice
- Propagate FFI errors via lastError instead of silently returning []
- Surface try? failures in memory explorer diagnostics view
- Allow wallet detail view to refresh on each appearance

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/SwiftExampleApp/SwiftExampleApp/Views/WalletMemoryExplorerView.swift`:
- Around line 253-288: The loadManagerState() function collects errors into a
local errors array but never clears loadError on success, causing stale error
messages; update loadManagerState() to reset loadError at the start (e.g., set
loadError = nil) and/or set loadError = nil when errors.isEmpty after all
walletManager calls so a successful reload clears previous errors (refer to
loadManagerState(), loadError, and the errors array).
- Around line 503-532: In loadOnce(), clear previous error state at the start by
setting summaryError = nil and loadError = nil (and any other per-refresh error
fields) before attempting new loads; keep the existing local errors array for
this invocation and continue appending failures, then assign loadError =
errors.joined(...) only if errors is not empty as currently done.
🪄 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: b135ce6a-45f5-42e5-97c4-4f86c06a78b2

📥 Commits

Reviewing files that changed from the base of the PR and between 97e3257 and 50efe04.

📒 Files selected for processing (4)
  • packages/rs-platform-wallet-ffi/src/wallet.rs
  • packages/rs-platform-wallet/src/manager/accessors.rs
  • packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/WalletMemoryExplorerView.swift
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift

QuantumExplorer and others added 2 commits May 1, 2026 00:44
@github-actions
Copy link
Copy Markdown
Contributor

✅ DashSDKFFI.xcframework built for this PR.

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

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

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.

@QuantumExplorer QuantumExplorer merged commit 9d6f078 into v3.1-dev Apr 30, 2026
34 checks passed
@QuantumExplorer QuantumExplorer deleted the per-account-balances branch April 30, 2026 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants