fix(platform-wallet): make asset-lock spends visible to every balance reader - #4336
fix(platform-wallet): make asset-lock spends visible to every balance reader#4336HashEngineering wants to merge 2 commits into
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>
📝 WalkthroughWalkthroughThe Kotlin and Swift persistence handlers reconcile asset-lock funding TXOs after finalized statuses. The Rust wallet computes funds-account balances from live UTXOs and processed height instead of cached values. Regression tests cover both behaviors. ChangesAsset-lock reconciliation
Live wallet balance calculation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant WalletPersistence
participant AssetLock
participant FundingTXOs
WalletPersistence->>AssetLock: read asset-lock status and funding outpoint
AssetLock-->>WalletPersistence: return finalized status
WalletPersistence->>FundingTXOs: mark linked TXOs spent
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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
| if (lock != null && lock.statusRaw >= ASSET_LOCK_STATUS_INSTANT_SEND_LOCKED) { | ||
| if (!txo.isSpent) { | ||
| database.txoDao().upsert(txo.copy(isSpent = true, lastUpdated = now())) | ||
| } | ||
| continue | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard the healing write against exceptions inside the load path.
database.txoDao().upsert(...) here runs unguarded inside buildUtxoRestoreData, which is called from onLoadWalletList. That whole call is wrapped in guardedLoad(emptyArray()) { ... }: if this single-row healing write throws, the exception propagates out of the loop over wallets, and the catch in guardedLoad returns an empty array for the ENTIRE wallet list — not just the one stale row. A single failed heal would make every wallet look non-restorable.
scrubAliases in this same file follows the safer pattern for opportunistic cleanup: it catches Throwable, logs, and lets the round continue rather than propagating. Apply the same pattern here.
🛡️ Proposed fix to prevent a healing failure from emptying the whole restore
if (lock != null && lock.statusRaw >= ASSET_LOCK_STATUS_INSTANT_SEND_LOCKED) {
if (!txo.isSpent) {
- database.txoDao().upsert(txo.copy(isSpent = true, lastUpdated = now()))
+ try {
+ database.txoDao().upsert(txo.copy(isSpent = true, lastUpdated = now()))
+ } catch (t: Throwable) {
+ Log.w(TAG, "load: failed to heal stale asset-lock-consumed TXO", t)
+ }
}
continue
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (lock != null && lock.statusRaw >= ASSET_LOCK_STATUS_INSTANT_SEND_LOCKED) { | |
| if (!txo.isSpent) { | |
| database.txoDao().upsert(txo.copy(isSpent = true, lastUpdated = now())) | |
| } | |
| continue | |
| } | |
| if (lock != null && lock.statusRaw >= ASSET_LOCK_STATUS_INSTANT_SEND_LOCKED) { | |
| if (!txo.isSpent) { | |
| try { | |
| database.txoDao().upsert(txo.copy(isSpent = true, lastUpdated = now())) | |
| } catch (t: Throwable) { | |
| Log.w(TAG, "load: failed to heal stale asset-lock-consumed TXO", t) | |
| } | |
| } | |
| continue | |
| } |
🤖 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/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt`
around lines 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.
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.
| if entry.statusRaw >= 2, | ||
| let displayTxidHex = entry.outPointHex.split(separator: ":").first, | ||
| let displayTxid = Data(hexString: String(displayTxidHex)) { | ||
| let wireTxid = Data(displayTxid.reversed()) | ||
| let staleDescriptor = FetchDescriptor<PersistentTxo>( | ||
| predicate: #Predicate { | ||
| $0.spendingTransaction?.txid == wireTxid && $0.isSpent == false | ||
| } | ||
| ) | ||
| if let stale = try? backgroundContext.fetch(staleDescriptor) { |
There was a problem hiding this comment.
🔴 Blocking: 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.
source: ['codex']
| if (lock != null && lock.statusRaw >= ASSET_LOCK_STATUS_INSTANT_SEND_LOCKED) { | ||
| if (!txo.isSpent) { | ||
| database.txoDao().upsert(txo.copy(isSpent = true, lastUpdated = now())) | ||
| } | ||
| continue |
There was a problem hiding this comment.
🟡 Suggestion: 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.
| if (lock != null && lock.statusRaw >= ASSET_LOCK_STATUS_INSTANT_SEND_LOCKED) { | |
| if (!txo.isSpent) { | |
| database.txoDao().upsert(txo.copy(isSpent = true, lastUpdated = now())) | |
| } | |
| continue | |
| if (lock != null && lock.statusRaw >= ASSET_LOCK_STATUS_INSTANT_SEND_LOCKED) { | |
| if (!txo.isSpent) { | |
| try { | |
| database.txoDao().upsert(txo.copy(isSpent = true, lastUpdated = now())) | |
| } catch (t: Throwable) { | |
| Log.w(TAG, "load: failed to heal stale asset-lock-consumed TXO", t) | |
| } | |
| } | |
| continue | |
| } |
source: ['coderabbit']
| // ── Asset-lock spend visibility ──────────────────────────────────── | ||
|
|
||
| /** | ||
| * 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 row never advances past mempool context and | ||
| * the in-block flip in onWalletChangesetTransaction never runs — the | ||
| * funding TXO sits at isSpent=false (spendingTxid set) FOREVER, and every | ||
| * isSpent-based balance read overstates the wallet. The lock's own status | ||
| * DOES keep arriving; from InstantSendLocked on, the upsert must flip | ||
| * linked TXOs. | ||
| */ | ||
| @Test | ||
| fun assetLockStatusAdvanceFlipsItsFundingTxos() = runTest { | ||
| handler.onPersistWalletMetadata(walletId, testnet, groupId, 0) | ||
| val lockTxid = ByteArray(32) { 7 } | ||
| db.transactionDao().upsert( | ||
| TransactionEntity(txid = lockTxid, transactionData = ByteArray(4), context = 0), | ||
| ) | ||
| val outpoint = ByteArray(36) { 9 } | ||
| db.txoDao().upsert( | ||
| TxoEntity( | ||
| outpoint = outpoint, | ||
| vout = 0, | ||
| amount = 1_000_000, | ||
| address = "yTest", | ||
| walletId = walletId, | ||
| spendingTxid = lockTxid, | ||
| spendingInputIndex = 0, | ||
| isSpent = false, | ||
| ), | ||
| ) | ||
|
|
||
| // Broadcast (1) must NOT flip — the network holds no lock yet and a | ||
| // pre-broadcast abort could still release the inputs. | ||
| handler.onPersistAssetLockUpsert( | ||
| walletId, lockTxid + ByteArray(4), ByteArray(4), 0, 1, 0, 999_545, 1, null, | ||
| ) | ||
| assertFalse(db.txoDao().getByOutpoint(outpoint)!!.isSpent) | ||
|
|
||
| // InstantSendLocked (2): the network has locked the inputs — flip. | ||
| handler.onPersistAssetLockUpsert( | ||
| walletId, lockTxid + ByteArray(4), ByteArray(4), 0, 1, 0, 999_545, 2, null, | ||
| ) | ||
| assertTrue(db.txoDao().getByOutpoint(outpoint)!!.isSpent) | ||
| } | ||
|
|
||
| /** The Consumed (4) terminal upsert heals rows a missed IS/CL never flipped. */ | ||
| @Test | ||
| fun assetLockConsumedHealsAStaleUnspentRow() = runTest { | ||
| handler.onPersistWalletMetadata(walletId, testnet, groupId, 0) | ||
| val lockTxid = ByteArray(32) { 8 } | ||
| db.transactionDao().upsert( | ||
| TransactionEntity(txid = lockTxid, transactionData = ByteArray(4), context = 0), | ||
| ) | ||
| val outpoint = ByteArray(36) { 10 } | ||
| db.txoDao().upsert( | ||
| TxoEntity( | ||
| outpoint = outpoint, | ||
| vout = 0, | ||
| amount = 9_999_545, | ||
| address = "yTest2", | ||
| walletId = walletId, | ||
| spendingTxid = lockTxid, | ||
| spendingInputIndex = 0, | ||
| isSpent = false, | ||
| ), | ||
| ) | ||
|
|
||
| handler.onPersistAssetLockUpsert( | ||
| walletId, lockTxid + ByteArray(4), ByteArray(4), 0, 1, 0, 9_999_545, 4, null, | ||
| ) | ||
| assertTrue(db.txoDao().getByOutpoint(outpoint)!!.isSpent) | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: 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.
source: ['codex']
| fn computed_core_balance( | ||
| account: &key_wallet::managed_account::ManagedCoreFundsAccount, | ||
| last_processed_height: u32, | ||
| ) -> key_wallet::wallet::balance::WalletCoreBalance { | ||
| let mut confirmed = 0u64; | ||
| let mut unconfirmed = 0u64; | ||
| let mut immature = 0u64; | ||
| let mut locked = 0u64; | ||
| for utxo in account.utxos.values() { | ||
| let value = utxo.txout.value; | ||
| if utxo.is_locked { | ||
| locked += value; | ||
| } else if !utxo.is_mature(last_processed_height) { | ||
| immature += value; | ||
| } else if utxo.is_confirmed || utxo.is_instantlocked || utxo.is_trusted { | ||
| confirmed += value; | ||
| } else { | ||
| unconfirmed += value; | ||
| } | ||
| } | ||
| key_wallet::wallet::balance::WalletCoreBalance::new(confirmed, unconfirmed, immature, locked) | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: 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.
source: ['codex']
| account.update_balance(height); | ||
| let cached_before = account.balance; | ||
| assert!(cached_before.total() > 0, "fixture must be funded"); | ||
| account.utxos.clear(); | ||
|
|
||
| assert_eq!( | ||
| account.balance.total(), | ||
| cached_before.total(), | ||
| "precondition: the cache must still hold the stale figure" | ||
| ); | ||
| assert_eq!( | ||
| computed_core_balance(account, height).total(), | ||
| 0, | ||
| "the accessor's fold must see the live (empty) UTXO set" | ||
| ); |
There was a problem hiding this comment.
🟡 Suggestion: 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.
| account.update_balance(height); | |
| let cached_before = account.balance; | |
| assert!(cached_before.total() > 0, "fixture must be funded"); | |
| account.utxos.clear(); | |
| assert_eq!( | |
| account.balance.total(), | |
| cached_before.total(), | |
| "precondition: the cache must still hold the stale figure" | |
| ); | |
| assert_eq!( | |
| computed_core_balance(account, height).total(), | |
| 0, | |
| "the accessor's fold must see the live (empty) UTXO set" | |
| ); | |
| account.update_balance(height); | |
| let cached_before = account.balance; | |
| assert!(cached_before.total() > 0, "fixture must be funded"); | |
| assert_eq!( | |
| computed_core_balance(account, height), | |
| cached_before, | |
| "the live fold must reproduce a freshly updated non-empty balance" | |
| ); | |
| account.utxos.clear(); | |
| assert_eq!( | |
| account.balance.total(), | |
| cached_before.total(), | |
| "precondition: the cache must still hold the stale figure" | |
| ); | |
| assert_eq!( | |
| computed_core_balance(account, height).total(), | |
| 0, | |
| "the accessor's fold must see the live (empty) UTXO set" | |
| ); |
source: ['codex']
2 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