Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1523,6 +1523,23 @@ class PlatformWalletPersistenceHandler(
updatedAt = now(),
),
)
// Spend-visibility reconcile: 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 entirely —
// the spender's transaction row then never leaves mempool
// context and onWalletChangesetTransaction's in-block flip never
// runs, leaving the funding TXOs isSpent=0 (spendingTxid set)
// FOREVER. The lock's own STATUS is a signal that provably
// does arrive (the proof wait drives it): once it reaches
// InstantSendLocked (2) the network has locked the inputs, so
// flip the linked TXOs here. Monotonic, and keyed strictly to
// TXOs already linked to THIS lock's funding txid.
if ((status.toInt() and 0xFF) >= ASSET_LOCK_STATUS_INSTANT_SEND_LOCKED) {
val fundingTxid = outPoint.copyOfRange(0, 32)
for (txo in db.txoDao().getUnspentBySpendingTxid(fundingTxid)) {
db.txoDao().upsert(txo.copy(isSpent = true, lastUpdated = now()))
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
0
}
Expand Down Expand Up @@ -2267,6 +2284,23 @@ class PlatformWalletPersistenceHandler(
if (spendingTxid != null) {
val spending = database.transactionDao().getByTxid(spendingTxid)
if (spending != null && spending.context >= CONTEXT_IN_BLOCK) continue
// Asset-lock spender: the 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 its row sits
// at mempool context FOREVER — the guard above never fires,
// and every relaunch resurrects the consumed output into the
// engine's balance. The tracked lock's own status is the
// finality signal that provably arrives; from
// InstantSendLocked on this output is gone. Skip it, and
// heal the flag so isSpent-based readers stop counting it.
val lockKey = encodeOutPointHex(spendingTxid + ByteArray(4))
val lock = database.assetLockDao().getByOutPointHex(lockKey)
if (lock != null && lock.statusRaw >= ASSET_LOCK_STATUS_INSTANT_SEND_LOCKED) {
if (!txo.isSpent) {
database.txoDao().upsert(txo.copy(isSpent = true, lastUpdated = now()))
}
continue
Comment on lines +2298 to +2302

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 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.

Suggested change
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']

}
Comment on lines +2298 to +2303

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.

🩺 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.

Suggested change
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.

}
val account = txo.accountId?.let { database.accountDao().getById(it) }
?: accountByAddress.getOrPut(txo.address) {
Expand Down Expand Up @@ -3090,6 +3124,15 @@ class PlatformWalletPersistenceHandler(
/** `TransactionContext::InBlock` — spends only count once in-block. */
private const val CONTEXT_IN_BLOCK = 2

/**
* Rust `AssetLockStatus` wire bytes (asset_lock_persistence.rs):
* Built 0, Broadcast 1, InstantSendLocked 2, ChainLocked 3,
* Consumed 4. At InstantSendLocked the network has locked the
* funding inputs — the spend-visibility reconcile treats the
* linked TXOs as spent from there.
*/
private const val ASSET_LOCK_STATUS_INSTANT_SEND_LOCKED = 2

/** `Network.testnet` rawValue — the Swift fallback network. */
private const val NETWORK_TESTNET = 1

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import org.dashfoundation.dashsdk.errors.DashSdkError
import org.dashfoundation.dashsdk.ffi.NativePersistenceBridge
import org.dashfoundation.dashsdk.wallet.PlatformWalletPersistenceCapabilities
import org.dashfoundation.dashsdk.persistence.entities.CoreAddressEntity
import org.dashfoundation.dashsdk.persistence.entities.TransactionEntity
import org.dashfoundation.dashsdk.persistence.entities.TxoEntity
import org.dashfoundation.dashsdk.persistence.entities.IdentityEntity
import org.dashfoundation.dashsdk.persistence.entities.PlatformAddressEntity
import org.dashfoundation.dashsdk.persistence.entities.WalletEntity
Expand Down Expand Up @@ -2762,4 +2764,79 @@ class PlatformWalletPersistenceHandlerTest {
// Unchanged pre-invitation behavior: no account row conjured.
assertTrue(db.accountDao().observeByWallet(walletId).first().isEmpty())
}

// ── 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)
}
Comment on lines +2768 to +2841

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 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']

}
88 changes: 87 additions & 1 deletion packages/rs-platform-wallet/src/manager/accessors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ impl<P: PlatformWalletPersistence + 'static> PlatformWalletManager<P> {
let Some(info) = wm.get_wallet_info(wallet_id) else {
return Vec::new();
};
let last_processed_height = info.core_wallet.metadata.last_processed_height;
info.core_wallet
.accounts
.all_accounts()
Expand All @@ -393,7 +394,18 @@ impl<P: PlatformWalletPersistence + 'static> PlatformWalletManager<P> {
// Balance lives on the funds-bearing variant only;
// keys-only accounts (identity, asset-lock, provider)
// never carry UTXOs.
let balance = account.as_funds().map(|a| a.balance).unwrap_or_default();
//
// Computed FRESH from the account's UTXO set — NOT the cached
// `a.balance` field. The cache refreshes only when transaction
// processing runs `update_balance()`, and a self-authored
// asset-lock spend can leave it stale long after the UTXO set
// (which coin selection reads) has moved on. Deriving from the
// same source selection uses makes disagreement impossible;
// the fold is bounded by the account's UTXO count.
let balance = account
.as_funds()
.map(|a| computed_core_balance(a, last_processed_height))
.unwrap_or_default();
// Walk every pool on the account, sum
// `used` + total entries. Cheap — pools are bounded by
// the gap limit.
Expand Down Expand Up @@ -1237,3 +1249,77 @@ mod spv_rescan_tests {
.expect("blocking accessor task");
}
}

/// Read-only [`WalletCoreBalance`] over an account's live UTXO set, with the
/// exact bucket rules of `ManagedCoreFundsAccount::update_balance` (which
/// requires `&mut self` and mutates the cache, so it cannot serve a
/// read-path): locked, else immature, else confirmed when in a block /
/// InstantSend-locked / trusted change, else unconfirmed.
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)
}
Comment on lines +1258 to +1279

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 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']


#[cfg(test)]
mod computed_balance_tests {
use super::*;
use key_wallet::account::StandardAccountType;

/// The accessor's per-account figure must come from the LIVE UTXO set,
/// not the cached balance field: a self-authored asset-lock spend can
/// leave the cache stale long after selection has moved on. Dirty the
/// UTXO set
/// without running update_balance and assert the fold reports the fresh
/// truth while the cache still holds the stale figure.
#[tokio::test]
async fn computed_balance_ignores_the_stale_cache() {
let (wallet_manager, wallet_id, _balance, _signer) =
crate::test_support::funded_wallet_manager(StandardAccountType::BIP44Account).await;

let mut wm = wallet_manager.write().await;
let info = wm.get_wallet_info_mut(&wallet_id).expect("wallet");
let height = info.core_wallet.metadata.last_processed_height;
let account = info
.core_wallet
.accounts
.standard_bip44_accounts
.get_mut(&0)
.expect("bip44 account 0");

// Freshen the cache, then remove every UTXO WITHOUT refreshing it —
// the shape an unprocessed self-spend leaves behind.
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"
);
Comment on lines +1309 to +1323

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 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.

Suggested change
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']

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,33 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable {
)
backgroundContext.insert(record)
}

// Spend-visibility reconcile (mirror of the Kotlin handler's
// onPersistAssetLockUpsert): 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 then never leaves mempool context
// and resolveInputOutpoint's in-block flip never runs, leaving
// the funding TXOs isSpent=false forever. The lock's
// own STATUS keeps arriving via this callback; from
// InstantSendLocked (2) the network has locked the inputs, so
// flip the TXOs already linked to this lock's funding tx.
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) {
Comment on lines +294 to +303

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔴 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']

for txo in stale {
txo.isSpent = true
txo.lastUpdated = Date()
}
}
}
}

for outPointHex in removed {
Expand Down
Loading