Skip to content

refactor(swift-sdk): redesign Persistent* tx schema and fix per-wallet tx push stall#3561

Merged
QuantumExplorer merged 4 commits intov3.1-devfrom
claude/focused-mcclintock-e8c54d
Apr 28, 2026
Merged

refactor(swift-sdk): redesign Persistent* tx schema and fix per-wallet tx push stall#3561
QuantumExplorer merged 4 commits intov3.1-devfrom
claude/focused-mcclintock-e8c54d

Conversation

@QuantumExplorer
Copy link
Copy Markdown
Member

@QuantumExplorer QuantumExplorer commented Apr 28, 2026

Issue being fixed or feature implemented

Two intertwined problems in the Core/SwiftData side of the iOS SDK:

  1. Schema modeled transactions as single-owner. PersistentTransaction carried walletId and account, and PersistentAccount cascaded outputs: [PersistentUtxo]. A single on-chain tx can produce outputs into multiple accounts (or even multiple wallets) — the schema couldn't represent that without ambiguity, and several display paths showed wrong totals.
  2. "View All Transactions" stalled the click, badly enough that even an empty wallet on a freshly installed app would freeze the simulator on tap. Root cause turned out to be SwiftUI's closure-based NavigationLink { Destination } re-running the destination's init (and its @Query registration) on every parent body invocation — sync triggers fire that constantly, so the destination's setup work was paid hundreds of times before the user could tap.

What was done?

Schema redesign (all under DashSchemaV1; dev stores get rebuilt)

  • PersistentTransaction.txid: hex String → raw 32-byte Data. Drops walletId and account (a tx can span several accounts/wallets). Renames utxosoutputs; adds inputs: [PersistentTxo] paired with PersistentTxo.spendingTransaction (.nullify, since inputs reference TXOs owned by other transactions).
  • PersistentUtxo renamed to PersistentTxo — it represents any output, spent or unspent. outpoint switched to raw 36-byte Data. Adds walletId denorm (so per-wallet predicates avoid the optional-relationship-chain crash), coreAddress: PersistentCoreAddress? (canonical address linkage), and spendingTransaction: PersistentTransaction?.
  • PersistentCoreAddress gains txos: [PersistentTxo] reverse relationship (.nullify) so detail views can navigate to the outputs at an address. The persistence handler links the TXO to its address row on upsert.
  • PersistentAccount drops the cascade-owned outputs collection — per-account TXOs derive through coreAddresses.flatMap(\.txos). wallet tightened from optional to non-optional. isWatchOnly removed (FFI-side state, never persisted).
  • PersistentWallet drops isWatchOnly for the same reason.
  • PlatformWalletPersistenceHandler updated for every site touched by the above (the stub-tx path no longer stamps account, the TXO upsert now stamps walletId + coreAddress).

Storage Explorer

Updated to match the new graph:

  • TXO detail surfaces the canonical coreAddress?.account path with the one-way account field as fallback, plus Created By / Spent By rows.
  • Core address detail shows txos.count and the parent account.
  • Account detail derives distinct-tx count via coreAddresses.flatMap(\.txos) — transactions are no longer account-scoped, so the count must be computed in Swift. Same shift in TransactionListView and WalletDetailView.

Per-wallet transaction push fix

  • TransactionListView now queries PersistentTransaction directly with a relationship-traversal predicate (tx.outputs.contains { $0.walletId == walletId } || tx.inputs.contains { ... }), sorted in SQLite, no Swift dedupe and no fault chain. Takes walletId: Data rather than PersistentWallet.
  • Both pushes in the wallets stack (wallets list → wallet detail, wallet detail → transactions) converted to value-based navigation: NavigationLink(value:) + two .navigationDestination(for:) modifiers on WalletsContentView (the stack root). Mixing paradigms (closure outer + value inner) produced animate-then-pop, so both layers go value-based. Now the destination is built only on actual push.

How Has This Been Tested?

  • xcodebuild -project SwiftExampleApp/SwiftExampleApp.xcodeproj -scheme SwiftExampleApp -sdk iphonesimulator -destination 'platform=iOS Simulator,name=iPhone 16,arch=arm64,OS=18.6' clean build** BUILD SUCCEEDED **.
  • Manual smoke on the iOS 26.3 simulator: tapping "View All Transactions" is now instant on an empty wallet and on a wallet with 1,468 transactions. Both pushes (wallets → detail and detail → transactions) animate cleanly without the previous animate-then-pop on the inner push.
  • Storage Explorer detail rows render correctly for TXOs / addresses / accounts under the new graph.

Breaking Changes

None at the consensus / protocol level. The Swift SDK's persistence schema changes are destructive within DashSchemaV1 (existing dev stores fail to open and get rebuilt on next sync). The migration policy comment in DashModelContainer.swift enumerates each delta. Public Swift types renamed: PersistentUtxoPersistentTxo. Per project policy, SDK/FFI public-API churn doesn't qualify for feat!/fix!.

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

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Introduced a TXO persistence model and per-address TXO relationships for richer output tracking.
  • UI Changes

    • Replaced UTXO labels with TXO across the app.
    • Transaction IDs now display and copy as hex.
    • Wallet rows no longer show watch-only badges.
    • Navigation updated to value-based routes for transaction lists.
  • Breaking Changes

    • Schema is destructive; development stores will be recreated on next launch.
  • Behavioral

    • Account–wallet relationship is now required; counts are recomputed from TXO/address data.

QuantumExplorer and others added 2 commits April 28, 2026 20:53
…View

Restrict the @query to the columns TransactionRowView actually
reads and key List rows on PersistentIdentifier instead of txid.
Prevents the navigation push from stalling on wallets with ~1.5k
transactions while SwiftData faulted in transactionData blobs for
every match on the main thread.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t tx push stall

Restructures the Core SwiftData persistence around a single shared
transaction record (multi-account, multi-wallet) and replaces the
broken per-wallet TransactionListView push.

Schema changes (all under DashSchemaV1, dev stores get rebuilt):

- PersistentTransaction.txid: String hex -> raw 32-byte Data;
  drops walletId and account fields (a tx can land in several
  accounts/wallets); renames `utxos` -> `outputs`; adds inputs
  relationship paired with PersistentTxo.spendingTransaction.
- PersistentUtxo renamed to PersistentTxo (it represents any output,
  spent or unspent). outpoint switched to raw 36-byte Data, gains
  walletId denorm, coreAddress link, and spendingTransaction back-
  reference.
- PersistentCoreAddress gains txos: [PersistentTxo] reverse
  relationship (.nullify) so detail views can navigate to the
  outputs at an address; persistence handler now links the TXO to
  its address row on upsert.
- PersistentAccount drops the cascade-owned outputs collection
  (per-account TXOs derive through coreAddresses.flatMap(\.txos));
  wallet tightened from optional to non-optional; isWatchOnly
  removed (FFI-side state, never persisted).
- PersistentWallet drops isWatchOnly for the same reason.
- PlatformWalletPersistenceHandler updated for every site touched
  by the above.

Storage Explorer updates to match the new graph: TXO detail shows
the canonical coreAddress?.account path with the one-way account
field as fallback, plus Created By / Spent By rows. Core address
detail surfaces txos.count. Account detail derives distinct-tx
count via coreAddresses.flatMap(\.txos) (transactions are no longer
account-scoped, so the count must be computed in Swift).

Per-wallet "View All Transactions" push was stalling on iOS 26
even on an empty wallet. Root cause was closure-based
NavigationLink { Destination } re-running the destination's init
(and its @query registration) on every parent body invocation,
which fired hundreds of times during sync. Switched both pushes in
the wallets stack to value-based navigation: NavigationLink(value:)
plus two .navigationDestination(for:) modifiers on
WalletsContentView (the stack root). Mixing paradigms (closure
outer + value inner) was producing animate-then-pop, so both
layers go value-based now.

TransactionListView now queries PersistentTransaction directly
with a relationship-traversal predicate (tx.outputs.contains
{ \$0.walletId == walletId } || tx.inputs.contains { ... }),
sorted in SQLite, no Swift dedupe and no fault chain.

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 28, 2026
@thepastaclaw
Copy link
Copy Markdown
Collaborator

thepastaclaw commented Apr 28, 2026

🕓 Ready for review — 6 ahead in queue (commit 1f71165)
Queue position: 7/7

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 28, 2026

Warning

Rate limit exceeded

@QuantumExplorer has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 52 minutes and 12 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: ffc7f243-6f93-4323-a859-8c0144988521

📥 Commits

Reviewing files that changed from the base of the PR and between 72afd0e and 1f71165.

📒 Files selected for processing (1)
  • packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTxo.swift
📝 Walkthrough

Walkthrough

This PR replaces the UTXO persistence model with a new TXO model, converts txid storage from hex String to raw Data, makes account.wallet non-optional, removes persisted watch-only flags, and reworks relationships so transaction/TXO membership is derived from address/TXO graphs.

Changes

Cohort / File(s) Summary
Model container & schema
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/DashModelContainer.swift
Swapped registered model from PersistentUtxoPersistentTxo; documented destructive V1 schema mutations.
Removed model
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentUtxo.swift
Deleted the PersistentUtxo model and its fields/behaviour.
New TXO model
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTxo.swift
Added PersistentTxo (@Model) with outpoint (txid+vout), metadata, flags, denormalized walletId, relationships to creating/spending transactions, optional account/coreAddress, utilities (makeOutpoint, hex/formatters).
Transaction model
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTransaction.swift
Changed txid from StringData, added txidHex; removed wallet/account denormalization; replaced utxos with outputs and inputs relationships; adjusted indexes.
Account & CoreAddress
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentAccount.swift, packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentCoreAddress.swift
Made PersistentAccount.wallet non-optional; removed isWatchOnly, transactions, utxos from account; added txos relationship to PersistentCoreAddress.
Wallet model
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentWallet.swift
Removed persisted isWatchOnly property and related init parameter.
Persistence handler
packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift
Refactored to use non-optional account.wallet, write PersistentTransaction.txid as Data, remove wallet-scoped transaction denormalization, migrate UTXO persistence to PersistentTxo (outpoint construction, walletId backfill, coreAddress attachment), and update spent/lock marking to target TXOs.
Transaction list/detail & routing
packages/swift-sdk/SwiftExampleApp/.../TransactionListView.swift, .../TransactionDetailView.swift, .../WalletDetailView.swift, .../WalletsContentView.swift
Transaction list now accepts walletId: Data, fetches via TXO inputs/outputs; displays txidHex; added TransactionsRoute and value-based navigation via navigationDestination.
UI view updates (counts/labels)
packages/swift-sdk/SwiftExampleApp/.../AccountDetailView.swift, AccountListView.swift, CoreContentView.swift, ReceiveAddressView.swift, CreateIdentityView.swift, TopUpIdentityView.swift, StorageExplorerView.swift, StorageModelListViews.swift, StorageRecordDetailViews.swift, TopUpIdentityView.swift
Replaced model-derived transaction/UTXO counts with address-pool derived TXO/transaction unions; removed watch-only UI; renamed UTXO UI to TXO; tightened predicates to use non-optional account.wallet.walletId; updated displays to use txidHex/outpointHex.

Sequence Diagram(s)

sequenceDiagram
  participant Handler as PlatformWalletPersistenceHandler
  participant TX as PersistentTransaction
  participant TXO as PersistentTxo
  participant Addr as PersistentCoreAddress
  participant DB as SwiftData/SQLite

  Handler->>TX: ensure/create transaction (txid: Data)
  Handler->>TXO: makeOutpoint(txid,vout) → create/update TXO row
  TXO->>TX: set creatingTransaction / spendingTransaction references
  Handler->>Addr: lookup coreAddress by address string
  Addr-->>TXO: attach inverse relationship (txos)
  TXO->>DB: persist TXO (walletId denormalized, timestamps, flags)
  TX->>DB: persist Transaction (txid Data, outputs/inputs relations)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

  • shumkov

Poem

🐰 Hops through rows both new and old,

Outpoints stitched from bytes and gold,
Watchful eyes have hopped away,
TXOs now leap in bright array,
A rabbit cheers — migrations play! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the two main objectives: a schema redesign for Persistent* transaction models and a fix for per-wallet transaction navigation stall.
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 claude/focused-mcclintock-e8c54d

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

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

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

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/Persistence/DashModelContainer.swift (1)

53-60: ⚠️ Potential issue | 🟠 Major

Handle incompatible schema changes gracefully instead of relying on empty migration plan.

The DashSchemaV1 changes include destructive modifications (String→Data retype for txid and outpoint) that SwiftData's lightweight migration cannot handle. While the comment suggests pre-refactor dev databases will be "rebuilt," the code only passes an empty DashMigrationPlan to ModelContainer with no error recovery. If container creation fails due to schema incompatibility, SwiftExampleAppApp calls fatalError() instead of deleting the old store and retrying.

Either add migration stages to handle the destructive changes, or catch the incompatibility error, delete the backing store files, and retry container creation.

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

In `@packages/swift-sdk/Sources/SwiftDashSDK/Persistence/DashModelContainer.swift`
around lines 53 - 60, DashSchemaV1 introduces destructive type changes that
SwiftData cannot lightweight-migrate, so update the ModelContainer creation in
DashModelContainer (where
ModelContainer(for:schema:migrationPlan:configurations:)) to either (A) add
explicit stages to DashMigrationPlan.stages that transform txid/outpoint types
to Data, or (B) wrap the ModelContainer init in a do/catch that on
migration-incompatibility errors deletes the existing persistent store files
(resolve the store URL(s) used by the container via the same logic that
constructs modelConfiguration) and retries ModelContainer creation instead of
propagating/fatalError; ensure the catch targets SwiftData migration/schema
incompatibility errors, performs safe file removal with FileManager, and retries
once before surfacing an error to SwiftExampleAppApp.
packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift (1)

347-395: ⚠️ Potential issue | 🟠 Major

Keep wallet-relative fields off the shared transaction row.

After keying PersistentTransaction only by txid, this method still overwrites context, direction, netAmount, and label on that shared row. If the same tx is emitted for multiple accounts or wallets, the last account processed decides what every wallet sees. These values need a wallet-scoped home, or they need to be derived from wallet-scoped TXOs at read time.

🤖 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 347 - 395, The upsertTransaction(account: TransactionRecordFFI)
function is writing wallet-specific values into the shared PersistentTransaction
row (fields like context, direction, netAmount, label, fee), which causes
cross-wallet pollution; stop mutating those wallet-relative fields on
PersistentTransaction and instead persist them to a wallet-scoped entity (e.g.,
per-account membership or TXO record) or derive them from TXOs at read-time.
Specifically, in upsertTransaction replace assignments to
PersistentTransaction.context, .direction, .netAmount, .label, and .fee (and any
other wallet-specific fields) with logic that upserts/updates a wallet-scoped
record (use the consumed account parameter and your existing per-account/TXO
models) or remove those writes so PersistentTransaction remains txid-global;
leave only truly global fields (txid, blockHeight, blockHash, blockTimestamp,
transactionType, transactionData, lastUpdated) on PersistentTransaction.
🤖 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/Persistence/Models/PersistentAccount.swift`:
- Around line 65-68: PersistentAccount.wallet was made non-optional but callers
still perform nil checks (e.g., allAccounts.filter { $0.wallet == nil } and
rendering an “Unlinked” section in AccountStorageListView); update the call
sites to stop checking for nil (remove or rewrite allAccounts.filter { $0.wallet
== nil } usages) and adjust AccountStorageListView to no longer render an
“Unlinked” section or to compute its content from a different criterion, or
alternatively revert wallet to Optional if the semantic requirement remains;
locate references to PersistentAccount.wallet and the AccountStorageListView /
allAccounts.filter usages and make the corresponding cleanup so the codebase
compiles without nil comparisons.

In
`@packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTransaction.swift`:
- Around line 26-31: PersistentTransaction stores txid as little-endian raw Data
but the txidHex accessor currently hex-encodes txid as-is (so UI shows the
reversed/canonical ID incorrectly); update the txidHex logic in the
PersistentTransaction type to reverse the bytes of txid before converting to a
hex string (and similarly fix any other hex-accessors/formatters that reference
txid, e.g., the other txidHex implementation around the 103-107 region) so the
exposed hex is the big-endian/canonical transaction id; ensure you only reverse
for presentation (keep stored txid unchanged).

In
`@packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- Around line 482-496: markUtxoSpent currently only sets txo.isSpent = true,
leaving PersistentTxo.spendingTransaction nil which breaks the inputs/outputs
graph for spend-only transactions; update markUtxoSpent(_ op: OutPointFFI) to
look up or create the spending PersistentTransaction (using the outpoint/op info
or a referenced spending txid if available from FFI), set
txo.spendingTransaction to that PersistentTransaction, and update
txo.lastUpdated before saving to backgroundContext so spend-only transactions
are included in per-wallet queries; reference markUtxoSpent,
PersistentTxo.spendingTransaction, OutPointFFI and the backgroundContext.fetch
descriptor when making the change.

---

Outside diff comments:
In
`@packages/swift-sdk/Sources/SwiftDashSDK/Persistence/DashModelContainer.swift`:
- Around line 53-60: DashSchemaV1 introduces destructive type changes that
SwiftData cannot lightweight-migrate, so update the ModelContainer creation in
DashModelContainer (where
ModelContainer(for:schema:migrationPlan:configurations:)) to either (A) add
explicit stages to DashMigrationPlan.stages that transform txid/outpoint types
to Data, or (B) wrap the ModelContainer init in a do/catch that on
migration-incompatibility errors deletes the existing persistent store files
(resolve the store URL(s) used by the container via the same logic that
constructs modelConfiguration) and retries ModelContainer creation instead of
propagating/fatalError; ensure the catch targets SwiftData migration/schema
incompatibility errors, performs safe file removal with FileManager, and retries
once before surfacing an error to SwiftExampleAppApp.

In
`@packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- Around line 347-395: The upsertTransaction(account: TransactionRecordFFI)
function is writing wallet-specific values into the shared PersistentTransaction
row (fields like context, direction, netAmount, label, fee), which causes
cross-wallet pollution; stop mutating those wallet-relative fields on
PersistentTransaction and instead persist them to a wallet-scoped entity (e.g.,
per-account membership or TXO record) or derive them from TXOs at read-time.
Specifically, in upsertTransaction replace assignments to
PersistentTransaction.context, .direction, .netAmount, .label, and .fee (and any
other wallet-specific fields) with logic that upserts/updates a wallet-scoped
record (use the consumed account parameter and your existing per-account/TXO
models) or remove those writes so PersistentTransaction remains txid-global;
leave only truly global fields (txid, blockHeight, blockHash, blockTimestamp,
transactionType, transactionData, lastUpdated) on PersistentTransaction.
🪄 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: 993e7042-3fca-4a71-9f18-66d9fa3f234b

📥 Commits

Reviewing files that changed from the base of the PR and between 9bd37f2 and 5d549c7.

📒 Files selected for processing (21)
  • packages/swift-sdk/Sources/SwiftDashSDK/Persistence/DashModelContainer.swift
  • packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentAccount.swift
  • packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentCoreAddress.swift
  • packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTransaction.swift
  • packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTxo.swift
  • packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentUtxo.swift
  • packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentWallet.swift
  • packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.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/CoreContentView.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/ReceiveAddressView.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionDetailView.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionListView.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletsContentView.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageExplorerView.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageModelListViews.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageRecordDetailViews.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TopUpIdentityView.swift
💤 Files with no reviewable changes (3)
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swift
  • packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentUtxo.swift
  • packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentWallet.swift

Comment on lines +65 to +68
/// Parent wallet. Every account currently belongs to a wallet. If
/// standalone non-wallet accounts are introduced later, this
/// becomes optional again.
public var wallet: PersistentWallet
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.

⚠️ Potential issue | 🔴 Critical

This non-optional relationship still has nil-based callers.

After this change, PersistentAccount.wallet can no longer be compared against nil, but the provided example app code still does allAccounts.filter { $0.wallet == nil } and renders an “Unlinked” section in AccountStorageListView. That leaves this PR in a compile-broken state unless those callers are removed or rewritten.

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

In
`@packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentAccount.swift`
around lines 65 - 68, PersistentAccount.wallet was made non-optional but callers
still perform nil checks (e.g., allAccounts.filter { $0.wallet == nil } and
rendering an “Unlinked” section in AccountStorageListView); update the call
sites to stop checking for nil (remove or rewrite allAccounts.filter { $0.wallet
== nil } usages) and adjust AccountStorageListView to no longer render an
“Unlinked” section or to compute its content from a different criterion, or
alternatively revert wallet to Optional if the semantic requirement remains;
locate references to PersistentAccount.wallet and the AccountStorageListView /
allAccounts.filter usages and make the corresponding cleanup so the codebase
compiles without nil comparisons.

Comment on lines 482 to +496
private func markUtxoSpent(_ op: OutPointFFI) {
let outpoint = "\(hashHex(op.txid)):\(op.vout)"
let descriptor = FetchDescriptor<PersistentUtxo>(
let outpoint = PersistentTxo.makeOutpoint(txid: hashData(op.txid), vout: op.vout)
let descriptor = FetchDescriptor<PersistentTxo>(
predicate: #Predicate { $0.outpoint == outpoint }
)
if let utxo = try? backgroundContext.fetch(descriptor).first {
utxo.isSpent = true
utxo.lastUpdated = Date()
if let txo = try? backgroundContext.fetch(descriptor).first {
txo.isSpent = true
// The FFI's spent-utxo notification only carries the
// outpoint, not the spending tx — so we cannot populate
// `txo.spendingTransaction` here. `isSpent = true` with
// `spendingTransaction == nil` is the steady-state we
// reach for now; future work: have the FFI emit the
// spending txid alongside each spent outpoint and link
// them up here.
txo.lastUpdated = Date()
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.

⚠️ Potential issue | 🟠 Major

This leaves the new inputs graph unusable for spend-only transactions.

The per-wallet transaction query now depends on outputs/inputs, but spent TXOs are only marked with isSpent = true here. Without setting txo.spendingTransaction, any transaction that only spends this wallet's outputs and creates no wallet-owned outputs will never match the wallet query, so it drops out of "View All Transactions".

🤖 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 482 - 496, markUtxoSpent currently only sets txo.isSpent = true,
leaving PersistentTxo.spendingTransaction nil which breaks the inputs/outputs
graph for spend-only transactions; update markUtxoSpent(_ op: OutPointFFI) to
look up or create the spending PersistentTransaction (using the outpoint/op info
or a referenced spending txid if available from FFI), set
txo.spendingTransaction to that PersistentTransaction, and update
txo.lastUpdated before saving to backgroundContext so spend-only transactions
are included in per-wallet queries; reference markUtxoSpent,
PersistentTxo.spendingTransaction, OutPointFFI and the backgroundContext.fetch
descriptor when making the change.

…ytes for canonical display

Two follow-ups from the schema redesign:

1. StorageModelListViews.swift had a leftover orphanAccounts
   filter (allAccounts.filter { \$0.wallet == nil }) and an
   "Unlinked" section, both predicated on PersistentAccount.wallet
   being optional. With wallet now non-optional the filter is dead
   code and the warnings-as-errors CI build was rejecting the
   nil comparison.

2. PersistentTransaction.txidHex (and PersistentTxo.txidHex
   which now delegates to it) was hex-encoding the raw 32 bytes
   in storage/wire order. Bitcoin/Dash convention reverses those
   bytes for display — dashcore::Txid: Display does the same
   flip — so the previous output was the reverse of what users
   see in block explorers. Fix reverses bytes only on the read
   side; storage stays unflipped so existing predicate fetches
   keep working without re-encoding.

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

Fixes pushed in 72afd0e, plus a note on the third comment:

1. PersistentAccount.wallet nil comparison (✓ fixed)StorageModelListViews.swift had a leftover allAccounts.filter { \$0.wallet == nil } driving an "Unlinked" section. Removed both. That was the warnings-as-errors CI failure.

2. txidHex byte order (✓ fixed) — Good catch. The txid: Data is stored in wire/internal order (matches what dashcore::Txid::as_ref() hands the FFI). The canonical display convention is the reverse, which is what dashcore::Txid: Display does. PersistentTransaction.txidHex now reverses on read, and PersistentTxo.txidHex delegates to it so the flip lives in one place. Storage stays unflipped, so existing predicate fetches comparing raw Data keep working without re-encoding. Worth noting: the prior String-based txid (hashHex(...)) had the same un-reversed behavior, so this is also a pre-existing display bug fix, not just a regression-prevention.

3. markUtxoSpent not setting spendingTransaction (acknowledged, deferred) — Real correctness gap, but the FFI's AccountChangeSetFFI.utxos_spent is [OutPointFFI] only; there's no spending-tx context emitted alongside. To populate spendingTransaction we'd need either an FFI extension to emit (spent_outpoint, spending_txid) tuples, or Swift-side parsing of TransactionRecordFFI.tx_data to walk the inputs. Both are Rust-side or non-trivial Swift changes outside this PR's scope. Filed as known limitation. Until then, transactions that only spend wallet outputs (and create no wallet-owned outputs) will still show their input TXOs flipped to isSpent = true but won't appear in the per-wallet TransactionListView. Tracking as a follow-up.

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: 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/Persistence/Models/PersistentTxo.swift`:
- Around line 136-159: The txid, txidHex, and outpointHex accessors currently
return empty values when transaction is nil even though the 36-byte outpoint
contains the TXID; update those accessors (txid, txidHex, outpointHex) to derive
values from the existing outpoint when transaction == nil by parsing the 36-byte
outpoint: extract the first 32 bytes as the raw txid Data (and the trailing
bytes for vout if needed), use the same byte-order/hex formatting used by
PersistentTransaction.txidHex to produce txidHex, and build outpointHex from
that derived txidHex and vout; keep falling back to transaction when present
(i.e., transaction?.txid / transaction?.txidHex) and only parse outpoint when
transaction is nil.
🪄 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: 0c610d25-0a24-4eeb-a364-24dbed9bdfbe

📥 Commits

Reviewing files that changed from the base of the PR and between 5d549c7 and 72afd0e.

📒 Files selected for processing (3)
  • packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTransaction.swift
  • packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTxo.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/StorageModelListViews.swift

…transaction is unattached

The 36-byte outpoint already carries the txid in its first 32 bytes,
so falling back to that when the `transaction` inverse is briefly
nil during insert keeps storage-explorer rows from collapsing to
empty `:vout` strings. `txidHex` now mirrors
`PersistentTransaction.txidHex`'s byte-reversal directly (rather
than forwarding) so the canonical block-explorer flip is applied
in both the attached and unattached cases. `outpointHex` flows
through `txidHex`, picking up the same fix.

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

Fixed in 1f71165. Good catch — outpoint does carry the txid in its first 32 bytes, so the fallback uses that when transaction is briefly nil instead of collapsing to empty. txidHex now mirrors PersistentTransaction.txidHex's byte-reversal directly rather than forwarding, so the canonical block-explorer flip applies in both the attached and unattached cases. outpointHex flows through it.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 28, 2026

✅ 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: "ecf8009bf0cf6f558d2a2bd8ed31f19663bb964a39243ebbe30ede878fd717aa"
)

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
Copy link
Copy Markdown
Member Author

Self Reviewed

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