fix(platform-wallet): make asset-lock spends visible to every balance reader - #4336
Conversation
… reader An SDK-built asset lock left both persistence mirrors and the per-account balance aggregates believing its funding TXOs were still spendable, long after the lock chain-locked and its top-up was credited — observed live as an Android wallet displaying 0.21 DASH with 0.1 spendable, and a MAX button offering the phantom total. Two independent gaps, one cause: an asset-lock tx burns its value into the special-tx PAYLOAD and often has no wallet-owned standard output, so SPV block matching can miss it and the spender's transaction record never leaves mempool context. Gap 1 — the persistence mirrors never flipped isSpent. Both handlers flip a TXO's spent flag only when the SPENDING tx reaches in-block context; for a block-matching-missed asset lock that context advance never arrives, so the TXO kept isSpent=false (spendingTxid set) forever. The lock's own STATUS is a signal that provably does arrive — the proof wait drives Built → Broadcast → InstantSendLocked/ChainLocked → Consumed upserts. From InstantSendLocked on, the network has locked the inputs, so the asset-lock upsert now flips the TXOs already linked to the lock's funding txid. Kotlin (onPersistAssetLockUpsert) and Swift (persistAssetLocks) get the same reconcile; a terminal Consumed upsert heals rows an earlier missed IS/CL left stale. Gap 2 — account_balances_blocking served the cached per-account balance field, which refreshes only when transaction processing runs update_balance(); the miss above leaves it stale indefinitely, while the coin selection reads the live UTXO set (and was always right). The accessor now computes the balance read-only from account.utxos with the exact bucket rules of ManagedCoreFundsAccount::update_balance, so the per-account snapshot derives from the same source selection uses and cannot disagree with it. Deeper engine follow-up (out of scope here): promote a self-authored transaction's record context by txid when its block is processed, so records do not depend on script matching — that would make the reconciles above redundant rather than load-bearing. Tests: Rust — a dirtied UTXO set with a deliberately stale cache, the accessor must report the live truth (571+1 pass). Kotlin/Robolectric — Broadcast must NOT flip, InstantSendLocked flips, terminal Consumed heals a stale row. clippy clean; kotlin-sdk unit suite green. Swift mirror is compile-unverified here (no iOS toolchain run) — flagged for CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ed outputs Completes the spend-visibility fix for the RESTART path. The restore guard skipped a spent-linked TXO only when its spending tx had reached in-block context — the very context a block-matching-missed asset lock never reaches — so every relaunch handed the consumed outputs back to Rust as spendable and the engine's lock-free balance re-inflated (observed live: a 0.1 wallet re-showing 0.22 after each restart, and persisting the phantom as the next launch's holding figure). The guard now also consults the tracked lock's own status — the finality signal that provably arrives — via the spending txid's outpoint key (credit outpoints are always vout 0): from InstantSendLocked on, the output is skipped AND its isSpent flag is healed in place, so already-poisoned wallets converge on their next launch without waiting for a status re-upsert that terminal Consumed rows will never send. kotlin-sdk unit suite green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe Kotlin and Swift persistence handlers reconcile asset-lock funding TXOs after finalized statuses. Wallet restoration excludes finalized spends and heals stale flags. The Rust wallet computes funds-account balances from live UTXOs and processed height. Tests cover reconciliation, failure isolation, and balance classification. ChangesAsset-lock reconciliation
Live wallet balance calculation
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟡 Moderate · up to The PR improves wallet balance and asset-lock spend visibility, but callback ordering can still temporarily restore or retain a consumed output as spendable in persisted wallet state. This is a bounded correctness risk that should be fixed or explicitly accepted before merge. Sequence Diagram(s)sequenceDiagram
participant WalletPersistence
participant AssetLockStore
participant TxoStore
participant WalletRestore
WalletPersistence->>AssetLockStore: read asset-lock status
AssetLockStore-->>WalletPersistence: return funding transaction status
WalletPersistence->>TxoStore: mark linked TXOs spent
WalletRestore->>AssetLockStore: check finalized asset-lock funding transactions
AssetLockStore-->>WalletRestore: return finalized transaction IDs
WalletRestore->>TxoStore: exclude and heal finalized TXOs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
⛔ Blockers found — Opus deferred (commit 4deaf3b) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift (1)
294-294: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the InstantSendLocked threshold into a named constant.
entry.statusRaw >= 2uses a raw literal for theInstantSendLockedwire value. The Kotlin mirror of this same reconcile names itASSET_LOCK_STATUS_INSTANT_SEND_LOCKEDwith a comment citing the Rust source. Mirror that here so both SDKs document the same protocol value in one place and a future Rust enum change is easier to grep for.♻️ Proposed named constant
+ /// `AssetLockStatus` wire value for `InstantSendLocked` (mirrors the + /// Kotlin handler's `ASSET_LOCK_STATUS_INSTANT_SEND_LOCKED`). + private static let assetLockStatusInstantSendLocked = 2 + func persistAssetLocks(- if entry.statusRaw >= 2, + if entry.statusRaw >= Self.assetLockStatusInstantSendLocked,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift` at line 294, Replace the raw threshold in the reconciliation condition around entry.statusRaw with a named constant representing InstantSendLocked, such as ASSET_LOCK_STATUS_INSTANT_SEND_LOCKED. Define the constant in the appropriate shared scope, document that its value mirrors the Rust wire enum, and update the comparison to use it.packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt (1)
2767-2841: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd restore-time coverage for the asset-lock heal branch.
Seed a stale
TxoEntityand anAssetLockEntitywithstatusRaw >= ASSET_LOCK_STATUS_INSTANT_SEND_LOCKED, then callonLoadWalletList(). Assert that the TXO is excluded fromutxosand persisted withisSpent = true.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt` around lines 2767 - 2841, Add a restore-time test alongside asset-lock persistence coverage that seeds a stale unspent TxoEntity linked by spendingTxid to an AssetLockEntity whose statusRaw is at least ASSET_LOCK_STATUS_INSTANT_SEND_LOCKED, then invokes onLoadWalletList(). Assert the loaded wallet excludes that TXO from utxos and the database row is persisted with isSpent = true.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt`:
- Around line 2298-2303: Guard the opportunistic txoDao().upsert healing write
in buildUtxoRestoreData with the same Throwable-catching, logging, and
continuation pattern used by scrubAliases. Keep marking the txo as spent when
the write succeeds, but ensure a failed single-row heal does not propagate
through onLoadWalletList or cause guardedLoad to discard the entire wallet list.
- Around line 1526-1542: The asset-lock spend reconciliation must be scoped to
the current wallet. In
packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt:1526-1542,
update the TXO query in stage(walletId) to filter by walletId, spendingTxid, and
isSpent = 0. In
packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift:283-309,
update staleDescriptor to include the walletId predicate alongside the existing
transaction filter.
---
Nitpick comments:
In
`@packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt`:
- Around line 2767-2841: Add a restore-time test alongside asset-lock
persistence coverage that seeds a stale unspent TxoEntity linked by spendingTxid
to an AssetLockEntity whose statusRaw is at least
ASSET_LOCK_STATUS_INSTANT_SEND_LOCKED, then invokes onLoadWalletList(). Assert
the loaded wallet excludes that TXO from utxos and the database row is persisted
with isSpent = true.
In
`@packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- Line 294: Replace the raw threshold in the reconciliation condition around
entry.statusRaw with a named constant representing InstantSendLocked, such as
ASSET_LOCK_STATUS_INSTANT_SEND_LOCKED. Define the constant in the appropriate
shared scope, document that its value mirrors the Rust wire enum, and update the
comparison to use it.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 79d79ff2-a571-4ace-9bd5-b602bf5e7216
📒 Files selected for processing (4)
packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.ktpackages/rs-platform-wallet/src/manager/accessors.rspackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
The Rust live-balance calculation and callback-time asset-lock reconciliation are sound, but the Swift restore path still rehydrates stale funding TXOs for locks finalized before this fix, leaving upgraded iOS wallets with persistent phantom balances. The remaining suggestions cover failure isolation and regression coverage for the Kotlin restore repair, shared ownership of the Rust balance rules, and a stronger non-empty-balance assertion.
Source: reviewers gpt-5.6-sol (general) and gpt-5.6-sol (rust-quality); final verifier gpt-5.6-sol. Orchestration-only: openclaw-agent/cliproxy/gpt-5.6-sol (not reviewer evidence).
Validated blockers were found in the Codex precheck. Opus is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— rust-quality (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 1 blocking | 🟡 4 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- [BLOCKING] packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift:294-303: Previously consumed Swift asset locks never trigger this reconciliation
The new reconciliation only runs when `persistAssetLocks` receives another upsert. A wallet upgraded with an existing lock at `InstantSendLocked`, `ChainLocked`, or especially terminal `Consumed` status can already have linked TXOs persisted with `isSpent == false`. The Swift load path at lines 4482–4484 still fetches every such row solely by `isSpent == false` and marshals it back to Rust without consulting the persisted asset-lock status. Consumed rows are intentionally retained for history and never advance again, so no future callback is guaranteed to repair them; the phantom UTXO can therefore return on every launch. Add a load-time finalized-lock exclusion and healing step equivalent to Kotlin's `buildUtxoRestoreData` guard.
In `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt`:
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt:2298-2302: Guard the healing write against exceptions inside the load path
This healing upsert runs inside `onLoadWalletList`, whose entire body is wrapped by `guardedLoad(emptyArray())`. If the single-row write throws, the exception escapes the wallet loop and `guardedLoad` returns an empty array for every wallet, even though excluding this finalized TXO from the current restore does not depend on the repair being durable. Treat the write as opportunistic: log its failure and continue skipping the consumed row so one failed repair cannot discard the complete restore result.
In `packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt`:
- [SUGGESTION] packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt:2768-2841: Add coverage for the finalized asset-lock restore guard
The added tests exercise only callback-time flips for statuses 2 and 4. They never invoke `onLoadWalletList()` with the legacy state this new restore branch is meant to repair: an unspent TXO linked to a mempool-context spending transaction and an already-finalized asset-lock row. Add a restore test that seeds this state, verifies the TXO is absent from the returned `utxos`, and verifies its Room row is healed to `isSpent = true`; this also pins the synthetic vout-0 key and txid byte orientation.
In `packages/rs-platform-wallet/src/manager/accessors.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/manager/accessors.rs:1258-1279: Balance classification now has two independent implementations
`computed_core_balance` duplicates the pinned key-wallet `ManagedCoreFundsAccount::update_balance` bucket rules line for line. A future key-wallet change to maturity, locking, trust, or confirmed/unconfirmed classification can compile cleanly while this accessor retains the old behavior, recreating disagreement between balance readers. Add an immutable balance-calculation method in key-wallet and have both its mutating cache update and this accessor call that shared implementation.
- [SUGGESTION] packages/rs-platform-wallet/src/manager/accessors.rs:1309-1323: Regression test passes if the calculator always returns zero
The test checks `computed_core_balance` only after clearing every UTXO and expecting zero. An implementation that always returned `WalletCoreBalance::default()` would therefore pass. Before clearing the account, compare the computed balance with the freshly updated, non-empty cache so the test establishes that the live fold classifies funded UTXOs as well as observing their removal.
QuantumExplorer
added
the
temp hold
QuantumExplorer
left a comment
QuantumExplorer
left a comment
There was a problem hiding this comment.
Something seems weird here.
|
I'll work on this later |
@QuantumExplorer Within the mobile apps, many functions have a MAX button to send the entire wallet balance and I was going that here. The same issue was found when doing a drain on a Maya Sell Swap (outputs are OP_RETURN and an address outside the wallet only). In both cases, there was no address in the outputs of the transaction that are owned by the wallet, and for some reason such transactions are not accounted for when determining the balance or marking inputs as spent. Two PR's were submitted to do the drain on Maya swaps (OP_RETURN)
When testing these two, which allow for draining the wallet balance with a TX with an OP_RETURN output, I found the same problem that this PR was attempting to fix -- that drain TX was not counted in the balance or it wasn't counting the inputs as spent. In theory, a regular send that drains the entire balance would result in the same problem. The balance would remain the same as it was before the TX was sent. |
The rust-dashcore fix behind this, and whether this PR is still needed after itWhat actually needs to change in rust-dashcoreThe engine follow-up this PR deferred turns out to be simpler — and stronger — than "promote a self-authored tx's record context by txid when its block is processed." Context promotion only helps if the block gets downloaded at all; a spender in an otherwise-irrelevant block never triggers block processing to promote from. The real gap is one step earlier, in compact-filter matching:
The fix: have filter matching watch the wallet's UTXO scripts alongside the monitored scripts, mirroring the bloom path's outpoint watching:
With that, any block spending a known UTXO matches regardless of the transaction's outputs — asset locks and Maya drains alike get their in-block context, and the ordinary This is implemented with regression coverage (a drain-shaped block matched via UTXO scripts and missed without them; a confirmed drain settling the balance to zero), pending a rust-dashcore PR. Is this PR still needed after the engine fix? Yes.
In short: the engine fix makes these reconciles what this PR's description hoped they'd become — redundant rather than load-bearing — for new transactions. They remain the convergence path for existing data. Merge as-is; engine fix follows as the root-cause companion. |
|
CoinJoin mixing fee transactions that only have an OP_RETURN output may also not be counted in the balance calculation. |
…end-visibility # Conflicts: # packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt
…tlin heals Addresses the review on dashpay#4336. Swift load-path heal (blocking finding). The callback-time reconcile in `persistAssetLocks` only fires when a lock upserts again, and `Consumed` is terminal — it never upserts again. A wallet whose lock finalized before that reconcile existed therefore has no future callback to repair its funding TXOs, and `loadWalletList`'s `isSpent == false` fetch handed them back to Rust as spendable on every launch. `loadWalletList` now collects the funding txids of every lock at `InstantSendLocked` or beyond, drops the TXOs those locks consumed from the restore set, and saves the repaired flags opportunistically (log + rollback on failure, the `healIdentityIsLocalFlags` pattern). Exclusion happens before the save, so it does not depend on the repair becoming durable. Non-clobbering, non-fatal heal writes. Both Kotlin heals were read-then-upsert round trips that wrote the whole row back from an in-memory copy. They are now column-scoped `UPDATE … SET isSpent = 1 … WHERE isSpent = 0` DAO queries: they cannot regress an already-spent row and cannot write back stale copies of columns they do not own. Vout-independent Kotlin finality lookup. The load guard probed the synthetic `<spending txid>:0` outpoint, but DIP-0027 lets one funding transaction carry several credit outputs and `wallet/asset_lock/sync/reconstruction.rs` persists each tracked lock under its own credit-output index — a valid lock can live at `:1` with no `:0` row at all, and the guard missed it. Replaced with `AssetLockDao.maxStatusForTxid`, the same guarded 64-hex `<txid>:` prefix match `fundingTypeForTxid` already uses, reduced with `MAX`: finality belongs to the transaction, so any of its locks reaching `InstantSendLocked` means the inputs are gone. One explicit failure policy for that lookup. It is the only new read in `buildUtxoRestoreData`, it runs under `guardedLoad(emptyArray())`, and the Android load surface is array-only — an escaping failure returned a SUCCESSFUL EMPTY restore, which Rust reads as a fresh coinless device for every wallet. `spendByFinalizedAssetLock` now answers `true` / `false` / `null`, where `null` means "could not read": the caller drops that one candidate and never heals it (nothing was proven), and every unrelated wallet, account and TXO still restores. Under-reporting one output for a launch is recoverable and non-durable; handing a consumed output back as spendable is what the guard exists to stop. Swift error propagation. Two load-bearing `try?`s failed OPEN, each in the direction that defeats its own guard: - the callback-time reconcile treated a stale-TXO fetch failure as "no rows to heal" and the shim returned 0, so a terminal `Consumed` status could commit while its funding TXOs stayed `isSpent == false` with no future upsert to repair them. `persistAssetLocks` is now result-bearing and `persistAssetLocksCallback` returns nonzero, which fails the Rust round and rolls the changeset back. - `finalizedAssetLockFundingTxids` mapped a thrown fetch to an empty set, and the caller reads an empty set as the positive claim "no lock has finalized" — restoring exactly the phantom inputs the guard withholds. It now throws, and `loadWalletList` returns `(nil, 0, true)` like the adjacent unspent fetch. Rust regression drives the production accessor. The previous version asserted on `computed_core_balance` directly, so it never called `account_balances_blocking` and stayed green against the pre-fix cached read. `account_balances_blocking_ignores_the_stale_cache` now builds a `PlatformWalletManager` over the funded fixture and asserts the returned `AccountBalanceRow` buckets in three steps: reproduce a freshly updated non-empty balance, track a live confirmed→locked reclassification the cache has not seen, then track the drain to zero. The direct-fold assertions remain as `computed_core_balance_matches_update_balance_bucket_for_bucket`, a bucket-policy companion (the fold duplicates `ManagedCoreFundsAccount::update_balance` and nothing in the type system keeps the two in step). Regression coverage, red before / green after. - `account_balances_blocking_ignores_the_stale_cache` (Rust): FAIL with line 432 reverted to `.map(|a| a.balance)` — `(10000000,0,0,0)` vs the expected `(3000000,0,0,7000000)` — and FAIL against a `WalletCoreBalance::default()` helper stub; PASS as written. - `loadSkipsATxoConsumedByAFinalizedAssetLockPersistedAtANonZeroVout` (Kotlin): FAIL with the vout-0 `getByOutPointHex` probe restored; PASS with the txid-prefix lookup. - `aFailingFinalizedLockLookupDropsOnlyItsOwnCandidate` (Kotlin): a Room open-helper factory faults the single `SELECT MAX(statusRaw) FROM asset_locks` statement — not the table, which two other restore builders also read — and asserts an unrelated TXO on the same wallet and an unrelated wallet's TXO both still restore, and that the unanswered candidate is dropped unhealed. FAIL with the failure allowed to escape (the whole restore comes back empty); PASS as written. - `AssetLockSpendVisibilityTests` (Swift, new): the SwiftData coverage both heals were missing — load excludes and heals a TXO consumed by a finalized lock, does so for a lock persisted at a non-zero vout, and the callback reconcile flips the linked TXO and reports success. Each FAILs with its guard disabled; all PASS as written. - `testPersistAssetLocksFailsTheRoundWhenTheStaleTxoFetchThrows` (Swift): faults only the `PersistentTxo` read, then asserts `persistAssetLocks` reports failure, the round closed as failed, and no `PersistentAssetLock` row committed. FAIL with the catch reverted to log-and-continue — the round reports success, commits, and lands the terminal status over funding TXOs still `isSpent == false`; PASS as written. - `testLoadReportsFailureWhenTheFinalizedLockFetchThrows` (Swift): faults only the `PersistentAssetLock` read, asserts the seam observed `PersistentWallet, PersistentTxo, PersistentAssetLock` (the earlier reads were served, so the failure is the lock read alone), then `errored == true` with nil entries and count 0. FAIL with the fetch reverted to `try? … ?? []` — the load returns a successful one-entry restore carrying the phantom UTXO; PASS as written. Also names the Swift `InstantSendLocked` threshold instead of a bare `2`, mirroring Kotlin, and corrects both status docs to list `RecoveredFromChain` (5) — added by dashpay#4347 after this branch forked. Fault-injection seam for the Swift fail-closed reads. Both Swift guards only take their failure branch when a `fetch` throws, which a live store never does on demand, and store-level corruption is no substitute: it trips `loadWalletList`'s earlier wallet/TXO fetches too, so a load test built on it would pass for the wrong reason. The reads whose failure must reject the round therefore go through `ModelFetching`, a handler-owned one-method protocol whose production implementation is `ModelContext.fetch` verbatim. A test injects a fetcher that faults one model type and serves every other read live, so each regression pins its own fetch — the load test asserts the wallet and unspent-TXO reads were served before the lock read threw. Verified: `cargo nextest run -p platform-wallet -p platform-wallet-storage -p platform-wallet-ffi --all-features --locked -E 'not test(~shield)'` 1580 passed; `cargo test -p platform-wallet --lib` 890 passed; `cargo clippy -p platform-wallet -p platform-wallet-storage -p platform-wallet-ffi -p rs-unified-sdk-jni --all-features --all-targets` clean for these crates; `cargo fmt --all --check` clean; `:sdk:testDebugUnitTest :sdk:testReleaseUnitTest :sdk:compileDebugAndroidTestKotlin` green; `xcodebuild test -scheme SwiftDashSDK -only-testing:SwiftDashSDKTests` on an iOS 26 simulator, 391 tests, only the two pre-existing `KeychainSignerAdditionalSigningKeys` keychain-environment failures (identical with this branch's Swift changes reverted). `rs-drive`'s unused `DocumentPropertyType` import (a7ccac2) makes a workspace-wide `-D warnings` clippy fail on unmodified code — pre-existing, not from here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@QuantumExplorer — fair reaction. Here is the part that is established, and the part What the client is actually seeing. An asset-lock transaction is a Core special tx Every downstream "spent" signal is keyed on in-block context, so all of them stay
The consumed output therefore stays "unspent" in the mirror and the restore path hands Why the reconciles key on the lock's own status. The one signal that provably The Rust change is a separate, ordinary bug — no SPV involved. Where I'd push back on the stated root cause. The comment above says block matching
So one of these has to be true, and I could not settle it from the platform side: One case I can confirm from the pinned code: for a CoinJoin account the scan On scope. The Rust accessor fix stands on its own either way. The status-keyed Happy to split the client heals into their own PR if you'd rather nail the engine |
|
Follow-up to the analysis above — we ran both experiments and the question is settled. (i) is false: the filters are fine. Dash Core's BASIC filter inserts the scriptPubKey of every spent coin ( (ii) is true, with a subtler mechanism than either of us guessed: the transaction is starved of relevance by its own mempool sighting. Reproduced against the exact platform-pinned engine (
Minimal engine fix (scoped in the diagnostic report): when fresh relevance is false and the context is We have a ready-made failing promotion test (asserts the expected-success promotion and fails on the current engine) that becomes the red regression for that fix — happy to hand it over or open the rust-dashcore PR, whichever @HashEngineering prefers for the promised companion. The CoinJoin scan-query pruning (#948) remains a real second path for CoinJoin-funded locks and is unaffected by this fix. This PR's heals remain necessary under this outcome for the reasons above: already-poisoned rows, the deployment window, and earlier finality. |
|
Engine companion is up: dashpay/rust-dashcore#998 — implements the confirmed-context fallback for the root cause settled above (the relevance gate dropping a wallet's own spend-only tx after the mempool sighting consumed its UTXO), with the diagnostic red→green regression. It reaches platform at the next rust-dashcore pin bump; this PR's heals remain necessary independently (already-poisoned rows, deployment window, earlier finality). |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
4 participants
Issue being fixed or feature implemented
An SDK-built asset lock leaves every balance reader believing its funding TXOs are still spendable, long after the lock chain-locked and its top-up was credited. Observed live on an Android testnet wallet: the home header and MAX displayed 0.21 DASH on a 0.1 wallet, the phantom figure was persisted as the next launch's holding balance, and each relaunch re-inflated the engine's lock-free balance back to the phantom (0.22 after a later deposit).
Root cause, one sentence: an asset-lock tx burns its value into the special-tx payload and often has no wallet-owned standard output, so SPV block matching can miss it — the spender's transaction record never leaves mempool context, and every downstream "spent" signal keyed on in-block context never fires.
Three readers were affected:
isSpentflips only when the spending tx reaches in-block context — which never arrives — so rows sit atisSpent = falsewithspendingTxidset, forever.account_balances_blocking): served the cached per-accountWalletCoreBalance, which refreshes only when transaction processing runsupdate_balance()— stale indefinitely, while coin selection (reading the live UTXO set) was always right.What was done?
The tracked lock's own status is a signal that provably arrives (the proof wait drives Built → Broadcast → InstantSendLocked/ChainLocked → Consumed upserts), so the fixes key on it:
isSpenton the TXOs already linked to the lock's funding txid once status reachesInstantSendLocked(the network has locked the inputs). A terminalConsumedupsert heals rows an earlier missed flip left stale.account_balances_blocking: computes each account's balance read-only fromaccount.utxoswith the exact bucket rules ofManagedCoreFundsAccount::update_balance, instead of serving the cache — the snapshot now derives from the same source selection uses and cannot disagree with it.InstantSendLockedon, the output is skipped and itsisSpenthealed in place, so already-poisoned wallets converge on their next launch — terminalConsumedrows never re-upsert, so waiting for a status write would never heal them.Deeper engine follow-up (deliberately out of scope): promote a self-authored transaction's record context by txid when its block is processed, so records don't depend on script matching — that would make these reconciles redundant rather than load-bearing.
How Has This Been Tested?
cargo test -p platform-wallet --lib: 572 passed).Broadcastmust not flip (a pre-broadcast abort can still release the inputs),InstantSendLockedflips, and a terminalConsumedheals a stale row (:sdk:testReleaseUnitTestgreen).Breaking Changes
None. All three changes tighten existing readers; no API or schema changes.
Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit