Skip to content

Commit 4b96844

Browse files
fix(kotlin-sdk): review round — the reconcile mutates nothing it cannot prove
thepastaclaw review round on dashpay#4439, all four blockers: - The spent-flip is demoted to LOG-ONLY (wouldFlipSpent): the engine's spent set records every input of every recorded transaction including MEMPOOL spends, with no context — persisting the flip would settle an unconfirmed spend, contradicting this handler's own in-block gating. Re-arm as a mutation only when the engine exports spends with context. - upsertUtxoRow reports whether it wrote: a globally-swept-parent refusal is now visible to the reconcile (skippedSwept), which no longer counts phantom heals nor flags netAmounts for rows that were never inserted. - The netAmount repair is demoted to LOG-ONLY (netAmountSuspects): a corrective record callback can land while its TXO delivery races this sweep, and blind addition double-credits. The event pipeline owns net correctness; the reconcile reports the suspicion. - The restore-time pool repair announces every pool it cannot repair (DashPay contact pools have no public key source by design and re-derive through DashPay sync; hardened pools cannot be publicly derived). Plus the review suggestions: contact-row exclusion now resolves ownership through coreAddressId -> core_addresses.accountId (production rows leave txos.accountId null, so the accountId-only check was ineffective), the neither-inventory log names the finalized-drop ambiguity, and the JNI spent-outpoint export uses the canonical OutPointFFI conversion. The reconcile is now fully observe-and-heal-forward: its only mutation is inserting provably-owned engine UTXOs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 9fdc9a3 commit 4b96844

4 files changed

Lines changed: 194 additions & 69 deletions

File tree

‎packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt‎

Lines changed: 88 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,7 +1075,7 @@ class PlatformWalletPersistenceHandler(
10751075
isConfirmed: Boolean,
10761076
isInstantLocked: Boolean,
10771077
isLocked: Boolean,
1078-
) {
1078+
): Boolean {
10791079
val outpoint = makeOutpoint(txid, vout)
10801080
val parentTx = db.transactionDao().getByTxid(txid)
10811081
// A globally-swept parent is a transaction Rust has already
@@ -1099,7 +1099,7 @@ class PlatformWalletPersistenceHandler(
10991099
// defensive here is correct: there is no record in flight to
11001100
// attribute a resurrected output to. See
11011101
// TransactionEntity.isGloballySwept.
1102-
if (parentTx?.isGloballySwept == true) return
1102+
if (parentTx?.isGloballySwept == true) return false
11031103
// Ensure a parent transaction row exists (stub if missing, so
11041104
// the TXO FK holds; the real tx upsert overwrites it later).
11051105
if (parentTx == null) {
@@ -1209,6 +1209,7 @@ class PlatformWalletPersistenceHandler(
12091209
}
12101210
for (p in pending) db.documentDao().deletePendingInput(p)
12111211
}
1212+
return true
12121213
}
12131214

12141215
/**
@@ -1222,15 +1223,26 @@ class PlatformWalletPersistenceHandler(
12221223
val engineUtxos: Int,
12231224
val inserted: Int,
12241225
val insertedDuffs: Long,
1225-
val netAmountRepairs: Int,
1226+
/** Healed TXOs whose pre-existing record's netAmount MAY be short by
1227+
* the healed amount. LOG-ONLY: the record can already carry the
1228+
* corrected net (a corrective callback racing this sweep), and
1229+
* blind addition double-credits. The event pipeline owns net
1230+
* correctness. */
1231+
val netAmountSuspects: Int,
12261232
val skippedImmature: Int,
12271233
val skippedNoAddress: Int,
12281234
val accountErrors: Int,
1229-
/** Store rows marked unspent whose outpoint the engine knows was
1230-
* spent — the lost-spend-update class (dashpay/platform#4425);
1231-
* flipped to spent in place. */
1232-
val flippedSpent: Int = 0,
1233-
val flippedSpentDuffs: Long = 0,
1235+
/** Store rows marked unspent whose outpoint the engine records as
1236+
* spent — the lost-spend-update class (dashpay/platform#4425).
1237+
* LOG-ONLY: the engine's spent set includes mempool spends with no
1238+
* context, so flipping would persist an unconfirmed spend as
1239+
* settled. */
1240+
val wouldFlipSpent: Int = 0,
1241+
val wouldFlipSpentDuffs: Long = 0,
1242+
/** Engine UTXOs whose insert the shared discipline refused (their
1243+
* parent transaction is globally swept) — excluded from
1244+
* [inserted], never netAmount-affecting. */
1245+
val skippedSwept: Int = 0,
12341246
/** Store rows marked unspent that the engine has in NEITHER
12351247
* inventory — swept/abandoned residue (pre-rust-dashcore#971
12361248
* stores). LOG-ONLY: counted and named in the log, never removed
@@ -1299,11 +1311,12 @@ class PlatformWalletPersistenceHandler(
12991311
val accountErrors = root["errors"]?.jsonArray?.size ?: 0
13001312
var inserted = 0
13011313
var insertedDuffs = 0L
1302-
var netAmountRepairs = 0
1314+
var netAmountSuspects = 0
13031315
var skippedImmature = 0
13041316
var skippedNoAddress = 0
1305-
var flippedSpent = 0
1306-
var flippedSpentDuffs = 0L
1317+
var wouldFlipSpent = 0
1318+
var wouldFlipSpentDuffs = 0L
1319+
var skippedSwept = 0
13071320
var wouldRemove = 0
13081321
var wouldRemoveDuffs = 0L
13091322
var skippedForeign = 0
@@ -1351,22 +1364,39 @@ class PlatformWalletPersistenceHandler(
13511364
val scriptPubKey =
13521365
row["scriptHex"]?.jsonPrimitive?.content.orEmpty().hexToByteArray()
13531366
val isLocked = row["isLocked"]?.jsonPrimitive?.boolean ?: false
1354-
// netAmount repair decision BEFORE the insert stubs a row.
1355-
val priorTx = database.transactionDao().getByTxid(txid)
1356-
upsertUtxoRow(
1367+
val wrote = upsertUtxoRow(
13571368
database, walletId, txid, vout, amount, address, scriptPubKey,
13581369
height,
13591370
isCoinbase = false,
13601371
isConfirmed = true,
13611372
isInstantLocked = false,
13621373
isLocked = isLocked,
13631374
)
1375+
if (!wrote) {
1376+
// The shared insert discipline refused (globally-swept
1377+
// parent). Counting it as healed — or repairing a
1378+
// netAmount for it — would falsify the report.
1379+
skippedSwept++
1380+
continue
1381+
}
13641382
inserted++
13651383
insertedDuffs += amount
1384+
// netAmount is NOT mutated here. The record's net may
1385+
// already be correct (a corrective record callback can
1386+
// land while its TXO delivery races this sweep), and
1387+
// adding the healed amount to an already-corrected net
1388+
// double-credits. The event pipeline owns net
1389+
// correctness; this pass only reports the suspicion.
1390+
val priorTx = database.transactionDao().getByTxid(txid)
13661391
if (priorTx != null && priorTx.transactionData.isNotEmpty()) {
1367-
if (database.transactionDao().addToNetAmount(txid, amount) > 0) {
1368-
netAmountRepairs++
1369-
}
1392+
netAmountSuspects++
1393+
Log.w(
1394+
TAG,
1395+
"txos reconcile: healed TXO ${'$'}{txid.toHex()}:${'$'}vout " +
1396+
"(${'$'}amount duffs) has a pre-existing record whose " +
1397+
"netAmount may be short by that amount — LOG-ONLY, " +
1398+
"storedNet=${'$'}{priorTx.netAmount}",
1399+
)
13701400
}
13711401
}
13721402

@@ -1380,6 +1410,16 @@ class PlatformWalletPersistenceHandler(
13801410
.filter { it.accountType == ACCOUNT_TYPE_TAG_DASHPAY_EXTERNAL }
13811411
.map { it.id }
13821412
.toSet()
1413+
// Production changeset writes leave txos.accountId null and
1414+
// route ownership through coreAddressId -> core_addresses
1415+
// .accountId, so the exclusion must resolve BOTH paths — an
1416+
// accountId-only check silently classifies every contact row.
1417+
suspend fun rowIsForeign(row: org.dashfoundation.dashsdk.persistence.entities.TxoEntity): Boolean {
1418+
if (row.accountId != null) return row.accountId in foreignAccountIds
1419+
val addr = row.coreAddressId ?: return false
1420+
val owner = database.coreAddressDao().getByAddress(addr)?.accountId
1421+
return owner != null && owner in foreignAccountIds
1422+
}
13831423
@Suppress("NAME_SHADOWING")
13841424
val storeRows = database.txoDao().observeByWallet(walletId).first()
13851425
// Case 3 (log-only): rows marked spent for coins the engine
@@ -1388,7 +1428,7 @@ class PlatformWalletPersistenceHandler(
13881428
// un-marked, only reported.
13891429
for (row in storeRows) {
13901430
if (!row.isSpent) continue
1391-
if (row.accountId != null && row.accountId in foreignAccountIds) continue
1431+
if (rowIsForeign(row)) continue
13921432
val key = "${row.txid?.toHex() ?: continue}:${row.vout}"
13931433
if (key in engineUnspentKeys) {
13941434
stuckSpent++
@@ -1403,26 +1443,31 @@ class PlatformWalletPersistenceHandler(
14031443
}
14041444
val storeUnspent = storeRows.filter { !it.isSpent }
14051445
for (row in storeUnspent) {
1406-
if (row.accountId != null && row.accountId in foreignAccountIds) {
1446+
if (rowIsForeign(row)) {
14071447
skippedForeign++
14081448
continue
14091449
}
14101450
val key = "${row.txid?.toHex() ?: continue}:${row.vout}"
14111451
when {
14121452
key in engineUnspentKeys -> {}
14131453
key in engineSpentKeys -> {
1414-
// Lost spend update (#4425): the engine knows this
1415-
// coin was spent; the row missed the flip. Flip in
1416-
// place — spendingTxid stays as-is (usually null;
1417-
// the spender's row, if it ever arrives, relinks
1418-
// via the deferred-input drain).
1419-
database.txoDao().upsert(row.copy(isSpent = true))
1420-
flippedSpent++
1421-
flippedSpentDuffs += row.amount
1454+
// Lost spend update (#4425) — PROBABLY. The
1455+
// engine's spent set records every input of every
1456+
// recorded transaction, INCLUDING mempool spends,
1457+
// and carries no context; flipping the store on
1458+
// it would persist an unconfirmed spend as
1459+
// settled, contradicting this handler's own
1460+
// in-block gating (see onWalletChangesetUtxoSpent).
1461+
// LOG-ONLY until the engine exports spends with
1462+
// their confirmation context.
1463+
wouldFlipSpent++
1464+
wouldFlipSpentDuffs += row.amount
14221465
Log.w(
14231466
TAG,
1424-
"txos reconcile: flipped lost-spend row to spent " +
1425-
"outpoint=$key amount=${row.amount}",
1467+
"txos reconcile: store row unspent but the engine " +
1468+
"records a spend (context unknown, possibly " +
1469+
"mempool) outpoint=$key amount=${row.amount} — " +
1470+
"LOG-ONLY, not flipped",
14261471
)
14271472
}
14281473
else -> {
@@ -1435,9 +1480,11 @@ class PlatformWalletPersistenceHandler(
14351480
wouldRemoveDuffs += row.amount
14361481
Log.w(
14371482
TAG,
1438-
"txos reconcile: store row unknown to engine " +
1439-
"(swept/abandoned residue?) outpoint=$key " +
1440-
"amount=${row.amount} — LOG-ONLY, not removed",
1483+
"txos reconcile: store row in neither engine " +
1484+
"inventory outpoint=$key amount=${row.amount} — " +
1485+
"ambiguous (swept/abandoned residue, or a " +
1486+
"finalized spend whose engine record was " +
1487+
"dropped) — LOG-ONLY, not removed",
14411488
)
14421489
}
14431490
}
@@ -1448,26 +1495,28 @@ class PlatformWalletPersistenceHandler(
14481495
engineUtxos = utxos.size,
14491496
inserted = inserted,
14501497
insertedDuffs = insertedDuffs,
1451-
netAmountRepairs = netAmountRepairs,
1498+
netAmountSuspects = netAmountSuspects,
14521499
skippedImmature = skippedImmature,
14531500
skippedNoAddress = skippedNoAddress,
14541501
accountErrors = accountErrors,
1455-
flippedSpent = flippedSpent,
1456-
flippedSpentDuffs = flippedSpentDuffs,
1502+
wouldFlipSpent = wouldFlipSpent,
1503+
wouldFlipSpentDuffs = wouldFlipSpentDuffs,
1504+
skippedSwept = skippedSwept,
14571505
wouldRemove = wouldRemove,
14581506
wouldRemoveDuffs = wouldRemoveDuffs,
14591507
skippedForeign = skippedForeign,
14601508
stuckSpent = stuckSpent,
14611509
stuckSpentDuffs = stuckSpentDuffs,
14621510
)
1463-
if (inserted > 0 || accountErrors > 0 || flippedSpent > 0 || wouldRemove > 0 ||
1464-
stuckSpent > 0
1511+
if (inserted > 0 || accountErrors > 0 || wouldFlipSpent > 0 || wouldRemove > 0 ||
1512+
stuckSpent > 0 || skippedSwept > 0
14651513
) {
14661514
Log.w(
14671515
TAG,
14681516
"txos reconcile: healed $inserted missing TXO(s) ($insertedDuffs duffs), " +
1469-
"$netAmountRepairs netAmount repair(s), " +
1470-
"flippedSpent=$flippedSpent ($flippedSpentDuffs duffs), " +
1517+
"$netAmountSuspects netAmount suspect(s) (log-only), " +
1518+
"wouldFlipSpent=$wouldFlipSpent ($wouldFlipSpentDuffs duffs, log-only), " +
1519+
"skippedSwept=$skippedSwept, " +
14711520
"wouldRemove=$wouldRemove ($wouldRemoveDuffs duffs, log-only), " +
14721521
"stuckSpent=$stuckSpent ($stuckSpentDuffs duffs, log-only), " +
14731522
"engine=${report.engineUtxos} " +

0 commit comments

Comments
 (0)