[Claudius-Maginificent]

TL;DR

Transaction history in the Swift SDK's local store showed wrong amounts and directions for transactions that spent the wallet's own coins. This PR makes the store recompute them from the coins it already tracks.

User story

As a wallet user on iOS, I want each entry in my transaction history to show what that transaction really took from or gave to my wallet, so that the history adds up to my balance.

Scenario

Actual behavior

A wallet sends a payment and gets change back. The history lists it as money received, for the amount of the change. An asset lock that used the whole balance is listed as a transfer of zero. The balance itself is right, but the history shows large receipts that never happened. When two wallets on the same device take part in one transaction, both show the same amount.

Expected behavior

The payment is listed as sent, for the amount that left the wallet. The asset lock shows what it locked. When two local wallets share a transaction, each shows its own side of it. When the store cannot work an amount out, it says the amount is unavailable instead of showing a wrong number.

Detailed discussion

Issue being fixed or feature implemented

Fixes #5126 for the SwiftData store, together with #5150, which repairs the Rust accounting and the SQLite store.

Stacked on #5150 (fix/pr-5126). These changes were developed on that branch and were moved here unchanged so that #5150 is about platform-wallet-storage only. Nothing in this diff depends on the Rust changes of #5150 and no FFI surface changes; the stacking keeps the tested tree the same. Merge #5150 first, then retarget this PR.

What was done?

• Reconcile at load, at the end of each persistence round, and when a TXO heal completes a spend. PlatformWalletPersistenceHandler recomputes amount and direction of stored transactions from their linked TXO rows. A failure of the round-end pass is logged under its own event and does not fail the round. Funded asset-lock amounts are protected from empty updates.
• Contact-only outputs (DashPay contact addresses) are left out of the sum. A received amount is still reported next to a contact's output when the wallet is the only participant and the stored amount agrees with the one computed from its own outputs.
• Wallet-scoped answers. PersistentTransaction.netAmount(for:) and direction(for:) answer for one wallet when several local wallets take part in a transaction; preserveAccounting(removingWallet:) keeps the survivor's values when a participant is deleted.
• Sender-scoped stored amount. A linked output counts for the stored amount only when its wallet is one of the spending wallets, like an address-matched one. A transfer between two local wallets is stored as the sender's full outgoing amount instead of the difference. The wallet-scoped answers for those two wallets are unchanged; with a third local wallet that takes part only through its keys, its direction for that transfer becomes outgoing.
• Schema V5 adds netAmountUnavailable, a lightweight migration from the exact frozen V4 in the merged base, so an amount that cannot be determined survives a restart and a wallet deletion.
• Example app. The transaction list and detail views show wallet-relative amounts and direction (TransactionDirectionStyle).

Known limitations

• Displaced spender. A coin row links to one spending transaction. When a competing transaction spends the same coin, the link moves to it. The earlier transaction's stored amount still counts the coin, because it is recomputed from the transaction's own inputs, as Rust records it. Wallet-scoped answers are derived from the links: for a transaction shared by two local wallets, or one that pays a DashPay contact, they show only the change as a positive amount (direction incoming for a shared transaction) until one of the two becomes final. Balances are not affected.
• Self-transfer with an observing wallet. direction(for:) reports a self-transfer as outgoing when a second wallet only recorded the transaction, and preserveAccounting keeps that after the second wallet is deleted.
• A payment from outside to two local wallets shows "Amount unavailable" in both. Account-detail screens keep the unscoped stored values. A row written before contact outputs were excluded, whose stored amount came from the contact's account, also stays "Amount unavailable" while it has a pending input.
• All of these come from deriving per-wallet amounts from TXO links on the Swift side. platform-wallet: make Rust the single source of Core transaction accounting (drop Swift re-derivation) #5226 replaces this with amounts computed in Rust and stored per wallet.
• The failure paths of the round-end pass and of the heal-time pass have no fault-injection test yet (TODO(test-round-accounting-failure), TODO(test-heal-accounting-failure)).

How Has This Been Tested?

• Frozen-schema generation (148 files) and historical-fixture checks (36 files) passed. Python helper tests passed after updating the generated-file count: 98 tests passed in the initial 99-test run, and the corrected 28-test schema suite passed. SwiftData runtime and Swift tests require macOS CI and were not executed on this Linux host.

• The Swift SDK build-and-test job (warnings as errors) and the frozen schema check pass in CI on 70a1b30c3a. Nothing was built or run locally: no Swift toolchain was available.

• TransactionAccountingTests.swift (new) covers the reconcile, contact outputs, wallet-scoped amounts and direction, foreign inputs, pending inputs, a contact output next to a received amount, a transfer between two local wallets and participant deletion; CoreTxoReconcileTests covers the accounting repaired by a heal; DashReleasedSchemaTests, DashModelMigrationTests and DashLegacySchemaMigrationTests cover V5, the migration from frozen V4 and the earlier supported migration routes.

Breaking Changes

None for the FFI or for consensus. SwiftData gains schema V5 with a lightweight migration from frozen V4. The accepted V1 route still skips historical V2 and migrates through frozen V3 and V4.

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
• If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request

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

🤖 Generated with Claude Code

https://claude.ai/code/session_01PSi8cX229kW2fmLxKW6ir4

[coderabbitai]

Important

Review skipped

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

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

⚙️ Run configuration

• Configuration used: Repository: dashpay/platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: a1349aea-88c7-41a5-a73d-1844e8f356e0

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

Use the checkbox below for a quick retry:

• 🔍 Trigger review

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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[thepastaclaw]

⛔ Final review complete — 1 blocking finding(s) (commit 94f9833) · triage: critical

[thepastaclaw]

Final validation — Phase 1 + Phase 2

The PR addresses the primary SwiftData accounting scenario and the supplied Swift build, tests, and schema checks pass. Three in-scope correctness gaps remain: contact-only outputs can incorrectly make a known amount unavailable, linked outputs are not scoped to the spending wallet when persisting reconciliation results, and TXO healing records dirty transactions without applying their accounting repairs.

🟡 3 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 5: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The diff substantially changes SwiftData persistence and schema migration behavior in PersistentTransaction, DashModelContainer, and PlatformWalletPersistenceHandler, a critical storage-migration surface affecting persisted transaction accounting.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 14% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — ffi-engineer (completed, effort xhigh); agent phase2-reviewer
• Model comparison: every Phase-2 reviewer also ran on gpt-6-astra; the verifier weighed both sets without knowing which model wrote which

🤖 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/Persistence/Models/PersistentTransaction.swift`:
- [SUGGESTION] packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTransaction.swift:251-261: Ignore contact-only outputs when deciding whether a wallet amount is unavailable
  `isWalletOwnedTxo` intentionally excludes DashPay contact/watch-only outputs, but `hasUnownedTxos` treats those outputs as evidence that the wallet's amount cannot be determined. For an incoming transaction with a foreign input, one normal output to this wallet, and one contact output, the normal output still determines the wallet's received amount. However, the contact output makes this check bypass the sole-participant fast path, and the pending foreign input then causes `netAmount(for:)` to return `nil`, displaying “Amount unavailable” instead of the known received amount.

In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- [SUGGESTION] packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift:7007-7013: Scope linked outputs to the spending wallets during reconciliation
  `ownedVouts` currently includes every locally owned linked output, regardless of its wallet. The address fallback below correctly limits unlinked outputs to `spendingWallets`, so linked and unlinked outputs have inconsistent scope. If wallet A spends 100 and wallet B receives 90 in the same transaction, wallet B's linked output is included in the scalar persisted for the transaction, producing `-10` rather than wallet A's `-100` and potentially classifying the transaction as internal. Filter linked outputs by their resolved wallet in the same way as the address fallback.
- [SUGGESTION] packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift:2932-2935: Apply accounting repairs after out-of-round TXO healing
  `drainPendingInputs` adds the parent and spender transactions to `accountingDirty`, but that dictionary is consumed only by `endChangeset`. `reconcileHealMissingTxos` runs outside a changeset and commits through `reconcileSave`, which saves the newly materialized TXO without calling `reconcileTransactionAccounting`. When a healed prevout resolves a previously incomplete spend, the TXO graph is repaired but the transaction's stored `netAmount` and `direction` remain stale until a later load-time reconciliation. Use a common finalization path for `endChangeset` and out-of-round reconciliation saves, or drain and reconcile these dirty transactions before `reconcileSave` commits.

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

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

• Centralize accounting semantics in Rust — The Swift model and persistence handler duplicate the wallet-accounting case table already represented by the Rust platform-wallet implementation. This is a concrete long-term drift risk, but the PR explicitly scopes itself to the SwiftData workaround, makes no FFI changes, and identifies #5226 as the follow-up for authoritative per-wallet accounting.
  • Follow-up: Implement the planned #5226 Rust/FFI accounting path and have SwiftData persist the authoritative per-wallet result.

[thepastaclaw]

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

The PR fixes the three previously reported accounting issues, including heal-time reconciliation, wallet-scoped linked outputs, and contact-only pending inputs. One new in-scope SwiftData bug remains: reconciliation counts a wallet-owned prevout for a transaction even after that prevout's spending relationship has moved to a competing transaction, which can overwrite the losing transaction's amount and direction.

🟡 1 suggestion(s)

Review provenance

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

• Triage: critical by gpt-6.1-sol (effort low) — This is a large, intricate change to persisted SwiftData storage, including a V4 schema migration and wallet-scoped transaction accounting that directly affects displayed financial history.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 14% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer

🤖 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`:
- [SUGGESTION] packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift:7004-7007: Do not count a prevout unless it is still linked to this spender
  This lookup verifies only that the decoded prevout exists and belongs to a wallet. When a competing transaction displaces the earlier spender, `adoptSpendObservation` moves `row.spendingTransaction` to the competitor while the losing transaction remains persisted and is marked dirty for reconciliation. This loop then still counts the shared prevout as an input of the loser, potentially rewriting its amount and direction as though it still spent the coin. Require the resolved TXO's `spendingTransaction` to match `transaction` before treating the prevout as a complete input; otherwise leave the loser's stored accounting unchanged.

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

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

• Keep wallet accounting in the Rust accounting owner — The SwiftData model and persistence handler independently infer wallet amounts and direction from TXO relationships, while the canonical accounting logic already exists in Rust. The PR explicitly documents the remaining divergence cases and tracks replacing this Swift-side derivation with Rust-derived per-wallet accounting in #5226, so that broader ownership change is outside this PR.
  • Follow-up: Implement the planned #5226 FFI and persistence boundary for Rust-derived per-wallet accounting.
• Account detail views still use unscoped transaction accounting — The account-detail surfaces still render the shared stored amount and direction without passing the owning wallet identifier. This is a concrete display limitation, but those views are unchanged by this PR and the limitation is explicitly disclosed in the PR description.
  • Follow-up: Update account-detail transaction views to pass the owning wallet identifier in a separate UI-focused change.

[thepastaclaw]

Re-review after discussion — commit 70a1b30

Standing review was COMMENTED; this re-review is APPROVE: no blocking findings remain.

Approved: the discussion withdrew or deferred every finding the full review of 70a1b30c (run 4931) raised, and the approval rests on that review.

Same commit as the standing review; the inline threads above carry the per-finding outcome. This follow-up exists only to correct the verdict.

Review provenance

Source: conversation lane gpt-6.1-sol (agent: conversation); no reviewer or verifier lanes ran for this follow-up

• Verdict moved because the inline discussion withdrew or deferred findings of the standing review of this commit; the code was not re-reviewed

[thepastaclaw]

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

The SQLite capability fix is correctly propagated, and independent source inspection confirms three prior findings are fixed and the prevout-link finding should remain withdrawn. However, the complete PR introduces transaction-accounting policy in Swift, violating the documented Rust ownership boundary; the reviewers' incremental-only scope does not justify deferring this newly introduced logic. Current listed CI checks pass, but none establishes Rust-wallet or Swift SDK build/test success at this head; validation here was static only.

🔴 1 blocking

Review provenance

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

• Triage: critical by gpt-6.1-sol (effort low) — The change combines intricate wallet-relative accounting with a persisted-storage migration in DashModelContainer.swift and a frozen V4 schema, meeting the size/complexity and storage-migration criteria.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (lane failed), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — platform-versioning (completed, effort xhigh); agent phase2-reviewer
• Model comparison: every Phase-2 reviewer also ran on gpt-6-astra; the verifier weighed both sets without knowing which model wrote which

🤖 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/Persistence/Models/PersistentTransaction.swift`:
- [BLOCKING] packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTransaction.swift:367-376: Keep authoritative transaction accounting in Rust
  This PR adds a second implementation of wallet accounting in Swift: this helper determines net amount and direction, while PlatformWalletPersistenceHandler.reconcileTransactionAccounting reconstructs ownership from decoded transactions and TXO rows and overwrites persisted accounting. Rust already owns these rules in rs-platform-wallet/src/changeset/wallet_accounting.rs. Both book/src/contributing/coding-conventions.md ("FFI and mobile SDKs hold no business logic") and packages/swift-sdk/CLAUDE.md require Swift to persist Rust-emitted state rather than independently decide wallet accounting. Comparing the actual PR base dae60456ec08feb6baa52f4155670988961b271b with this head confirms these Swift implementations are additions of this PR, not unrelated pre-existing code. Their being unchanged in the latest five-file merge therefore does not make the ownership issue out of scope. Issue dashpay/platform#5226 tracks the correct destination, but a follow-up cannot defer new wrong-layer logic introduced here. Implement the ownership/accounting fold and repair in rs-platform-wallet, emit authoritative wallet-scoped amounts, directions and unavailable state through persistence callbacks, and leave Swift to store and display those results.

[thepastaclaw]

🔴 Blocking: Keep authoritative transaction accounting in Rust

This PR adds a second implementation of wallet accounting in Swift: this helper determines net amount and direction, while PlatformWalletPersistenceHandler.reconcileTransactionAccounting reconstructs ownership from decoded transactions and TXO rows and overwrites persisted accounting. Rust already owns these rules in rs-platform-wallet/src/changeset/wallet_accounting.rs. Both book/src/contributing/coding-conventions.md ("FFI and mobile SDKs hold no business logic") and packages/swift-sdk/CLAUDE.md require Swift to persist Rust-emitted state rather than independently decide wallet accounting. Comparing the actual PR base dae6045 with this head confirms these Swift implementations are additions of this PR, not unrelated pre-existing code. Their being unchanged in the latest five-file merge therefore does not make the ownership issue out of scope. Issue #5226 tracks the correct destination, but a follow-up cannot defer new wrong-layer logic introduced here. Implement the ownership/accounting fold and repair in rs-platform-wallet, emit authoritative wallet-scoped amounts, directions and unavailable state through persistence callbacks, and leave Swift to store and display those results.

_{source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, platform-versioning); gpt-6-astra (phase2-reviewer: architecture-layering, platform-versioning)}