fix(kotlin-sdk): reconcile the TXO store against the engine and repair restored address pools - #4439
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 f0f632e) |
HashEngineering
changed the title
590ee97 to
ac7c9b2
Compare
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
The reconciliation and restore repair target real persistence divergence, but four correctness defects can durably mark unconfirmed coins spent, repeatedly report or credit an insertion that never occurred, corrupt an already-correct transaction amount, and leave DashPay receiving pools sparse. Three additional in-scope issues weaken contact-row classification, make the periodic inventory scan quadratic in account count, and bypass the canonical outpoint conversion.
Source: Codex reviewer lanes codex-general, codex-rust-quality, and codex-ffi-engineer (exact backend model IDs were not supplied in the evidence); final verifier backend grok-4.5; orchestration-only openclaw-agent/cliproxy/gpt-5.6-sol is 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),gpt-5.6-sol— ffi-engineer (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 4 blocking | 🟡 3 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/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt`:
- [BLOCKING] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt:1345-1352: Do not persist unconfirmed inputs as spent
The exact pinned key-wallet revision inserts every input of a recorded transaction into `spent_outpoints`, regardless of whether its context is mempool or in-block. Treating membership in that set as a confirmed spend contradicts this handler's established rule at lines 928-934 and 3073-3080: mempool-linked inputs remain unspent in Room so they can be restored and reclassified after restart. A reconciliation while a payment is unconfirmed therefore makes the coin durably spent; if the transaction is later abandoned and its release update is lost or interrupted, the deliberate never-unmark policy prevents every later reconciliation from recovering it. Export spender context/finality with each outpoint, or only flip rows for spends proven confirmed by another authoritative source.
- [BLOCKING] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt:1288-1301: Only account for TXOs that were actually inserted
`upsertUtxoRow` returns early when the parent transaction is still marked `isGloballySwept`, but it returns `Unit`, so this caller cannot distinguish that refusal from a successful insert. The reconciler then increments `inserted`, adds to `insertedDuffs`, and may update `netAmount` even though the TXO remains absent. A missed reinstatement record can leave exactly this stale tombstone while the engine authoritatively holds the output; every periodic pass then repeats the false heal and can repeatedly add the same amount. Make the helper report whether it materialized the row and perform all counters and amount repair only after a successful insert, or explicitly reconcile the stale swept state first.
- [BLOCKING] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt:1298-1301: Do not infer netAmount from TXO presence
A missing `txos` projection does not prove that the independently persisted transaction record omitted the output from `netAmount`. For example, a corrective transaction callback can already store the engine's recomputed net amount while delivery of the corresponding UTXO projection is omitted, or a TXO can disappear later without changing its parent transaction. In either case this unconditional delta overstates the transaction, and the newly inserted row makes the corruption permanent because later passes become no-ops. The inventory does not include an authoritative expected net amount, so repair must compare against one or recompute the amount from authoritative ownership data rather than infer it from row absence.
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt:1321-1340: Resolve contact ownership through coreAddressId
The exclusion only checks `row.accountId`, but production changeset writes leave that field null and route TXO ownership through `coreAddressId -> core_addresses.accountId`, as documented by `buildUtxoRestoreData` at lines 3065-3069. The regression test manually fills `accountId`, so it does not represent production contact rows. Resolve the effective account through `coreAddressId` when the direct FK is null in both reverse-pass loops; otherwise normal DIP-15 contact outputs are repeatedly reported as engine-unknown and defeat the intended regression-tripwire signal.
In `packages/rs-platform-wallet-ffi/src/persistence.rs`:
- [BLOCKING] packages/rs-platform-wallet-ffi/src/persistence.rs:4085-4113: Repair DashPay pools with their concrete account xpub
Both DashPay variants reach this resolver, but the exact pinned implementation of `Wallet::key_source_for_account_type` explicitly returns `NoKeySource` for `DashpayReceivingFunds` and `DashpayExternalAccount`. The guard at line 4131 therefore always skips their hole repair. This is especially harmful for `DashpayReceivingFunds`: it is wallet-owned and funds-bearing, and its concrete account already carries the xpub needed to reconstruct missing addresses. Resolve the full `AccountType` through `wallet.accounts.account_of_type(account_type)` and use that account's xpub as `KeySource::Public`, retaining the existing helper as the fallback for special account types such as the BLS provider account. Add a restore test with a sparse DashPay receiving pool and a real signing wallet; the current test passes `None` and cannot exercise this path.
In `packages/rs-unified-sdk-jni/src/wallet_manager.rs`:
- [SUGGESTION] packages/rs-unified-sdk-jni/src/wallet_manager.rs:3205-3207: Snapshot all account inventories in one pass
The JNI method enumerates all N accounts and then invokes separate per-account UTXO and spent-outpoint FFI accessors. Each accessor reacquires `wallet_manager.blocking_read()`, allocates `all_accounts()`, and linearly searches that N-element collection, making one full reconciliation O(N²) with 2N+1 lock acquisitions. DashPay creates accounts per contact, so this periodic scan scales with wallet history and runs both at sync completion and every 30 minutes. Add a manager-level snapshot that gathers account identity, UTXOs, and spent outpoints under one read lock, then expose it through one FFI operation.
In `packages/rs-platform-wallet-ffi/src/manager_diagnostics.rs`:
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/manager_diagnostics.rs:655-661: Use the canonical OutPointFFI conversion
`OutPointFFI` already implements `From<&dashcore::OutPoint>` and documents that implementation as the single authority for preserving the persistence join key and txid byte order. The new export manually duplicates the conversion. It is equivalent today but can silently diverge if the canonical representation changes, so route this export through the existing conversion boundary.
| key in engineSpentKeys -> { | ||
| // Lost spend update (#4425): the engine knows this | ||
| // coin was spent; the row missed the flip. Flip in | ||
| // place — spendingTxid stays as-is (usually null; | ||
| // the spender's row, if it ever arrives, relinks | ||
| // via the deferred-input drain). | ||
| database.txoDao().upsert(row.copy(isSpent = true)) | ||
| flippedSpent++ |
There was a problem hiding this comment.
🔴 Blocking: Do not persist unconfirmed inputs as spent
The exact pinned key-wallet revision inserts every input of a recorded transaction into spent_outpoints, regardless of whether its context is mempool or in-block. Treating membership in that set as a confirmed spend contradicts this handler's established rule at lines 928-934 and 3073-3080: mempool-linked inputs remain unspent in Room so they can be restored and reclassified after restart. A reconciliation while a payment is unconfirmed therefore makes the coin durably spent; if the transaction is later abandoned and its release update is lost or interrupted, the deliberate never-unmark policy prevents every later reconciliation from recovering it. Export spender context/finality with each outpoint, or only flip rows for spends proven confirmed by another authoritative source.
source: ['codex']
There was a problem hiding this comment.
Resolved in 2195b18 — Do not persist unconfirmed inputs as spent no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| upsertUtxoRow( | ||
| database, walletId, txid, vout, amount, address, scriptPubKey, | ||
| height, | ||
| isCoinbase = false, | ||
| isConfirmed = true, | ||
| isInstantLocked = false, | ||
| isLocked = isLocked, | ||
| ) | ||
| inserted++ | ||
| insertedDuffs += amount | ||
| if (priorTx != null && priorTx.transactionData.isNotEmpty()) { | ||
| if (database.transactionDao().addToNetAmount(txid, amount) > 0) { | ||
| netAmountRepairs++ | ||
| } |
There was a problem hiding this comment.
🔴 Blocking: Only account for TXOs that were actually inserted
upsertUtxoRow returns early when the parent transaction is still marked isGloballySwept, but it returns Unit, so this caller cannot distinguish that refusal from a successful insert. The reconciler then increments inserted, adds to insertedDuffs, and may update netAmount even though the TXO remains absent. A missed reinstatement record can leave exactly this stale tombstone while the engine authoritatively holds the output; every periodic pass then repeats the false heal and can repeatedly add the same amount. Make the helper report whether it materialized the row and perform all counters and amount repair only after a successful insert, or explicitly reconcile the stale swept state first.
source: ['codex']
There was a problem hiding this comment.
Resolved in 2195b18 — Only account for TXOs that were actually inserted no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| if (priorTx != null && priorTx.transactionData.isNotEmpty()) { | ||
| if (database.transactionDao().addToNetAmount(txid, amount) > 0) { | ||
| netAmountRepairs++ | ||
| } |
There was a problem hiding this comment.
🔴 Blocking: Do not infer netAmount from TXO presence
A missing txos projection does not prove that the independently persisted transaction record omitted the output from netAmount. For example, a corrective transaction callback can already store the engine's recomputed net amount while delivery of the corresponding UTXO projection is omitted, or a TXO can disappear later without changing its parent transaction. In either case this unconditional delta overstates the transaction, and the newly inserted row makes the corruption permanent because later passes become no-ops. The inventory does not include an authoritative expected net amount, so repair must compare against one or recompute the amount from authoritative ownership data rather than infer it from row absence.
source: ['codex']
There was a problem hiding this comment.
Resolved in 2195b18 — Do not infer netAmount from TXO presence no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| let key_source = signing_wallet | ||
| .and_then(|wallet| { | ||
| key_wallet::transaction_checking::transaction_router::AccountTypeToCheck::try_from( | ||
| &*managed_type, | ||
| ) | ||
| .ok() | ||
| .map(|check_type| { | ||
| let account_index = match &account_type { | ||
| AccountType::Standard { | ||
| index, .. | ||
| } | ||
| | AccountType::CoinJoin { | ||
| index, | ||
| } | ||
| | AccountType::DashpayReceivingFunds { | ||
| index, .. | ||
| } | ||
| | AccountType::DashpayExternalAccount { | ||
| index, .. | ||
| } => Some(*index), | ||
| AccountType::IdentityTopUp { | ||
| registration_index, | ||
| } => Some(*registration_index), | ||
| _ => None, | ||
| }; | ||
| wallet.key_source_for_account_type(&check_type, account_index) | ||
| }) | ||
| }) | ||
| .unwrap_or(key_wallet::KeySource::NoKeySource); |
There was a problem hiding this comment.
🔴 Blocking: Repair DashPay pools with their concrete account xpub
Both DashPay variants reach this resolver, but the exact pinned implementation of Wallet::key_source_for_account_type explicitly returns NoKeySource for DashpayReceivingFunds and DashpayExternalAccount. The guard at line 4131 therefore always skips their hole repair. This is especially harmful for DashpayReceivingFunds: it is wallet-owned and funds-bearing, and its concrete account already carries the xpub needed to reconstruct missing addresses. Resolve the full AccountType through wallet.accounts.account_of_type(account_type) and use that account's xpub as KeySource::Public, retaining the existing helper as the fallback for special account types such as the BLS provider account. Add a restore test with a sparse DashPay receiving pool and a real signing wallet; the current test passes None and cannot exercise this path.
source: ['codex']
There was a problem hiding this comment.
Resolved in 2195b18 — Repair DashPay pools with their concrete account xpub no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| for (row in storeRows) { | ||
| if (!row.isSpent) continue | ||
| if (row.accountId != null && row.accountId in foreignAccountIds) continue | ||
| val key = "${row.txid?.toHex() ?: continue}:${row.vout}" | ||
| if (key in engineUnspentKeys) { | ||
| stuckSpent++ | ||
| stuckSpentDuffs += row.amount | ||
| Log.w( | ||
| TAG, | ||
| "txos reconcile: store row spent but engine lists it " + | ||
| "unspent outpoint=$key amount=${row.amount} — LOG-ONLY " + | ||
| "(lost release, or a live spend racing the engine)", | ||
| ) | ||
| } | ||
| } | ||
| val storeUnspent = storeRows.filter { !it.isSpent } | ||
| for (row in storeUnspent) { | ||
| if (row.accountId != null && row.accountId in foreignAccountIds) { | ||
| skippedForeign++ | ||
| continue |
There was a problem hiding this comment.
🟡 Suggestion: Resolve contact ownership through coreAddressId
The exclusion only checks row.accountId, but production changeset writes leave that field null and route TXO ownership through coreAddressId -> core_addresses.accountId, as documented by buildUtxoRestoreData at lines 3065-3069. The regression test manually fills accountId, so it does not represent production contact rows. Resolve the effective account through coreAddressId when the direct FK is null in both reverse-pass loops; otherwise normal DIP-15 contact outputs are repeatedly reported as engine-unknown and defeat the intended regression-tripwire signal.
source: ['codex']
There was a problem hiding this comment.
Resolved in 2195b18 — Resolve contact ownership through coreAddressId no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| if !entries.is_null() && count > 0 { | ||
| let accounts = unsafe { std::slice::from_raw_parts(entries, count) }; | ||
| for acc in accounts { |
There was a problem hiding this comment.
🟡 Suggestion: Snapshot all account inventories in one pass
The JNI method enumerates all N accounts and then invokes separate per-account UTXO and spent-outpoint FFI accessors. Each accessor reacquires wallet_manager.blocking_read(), allocates all_accounts(), and linearly searches that N-element collection, making one full reconciliation O(N²) with 2N+1 lock acquisitions. DashPay creates accounts per contact, so this periodic scan scales with wallet history and runs both at sync completion and every 30 minutes. Add a manager-level snapshot that gathers account identity, UTXOs, and spent outpoints under one read lock, then expose it through one FFI operation.
source: ['codex']
| let entries: Vec<OutPointFFI> = rows | ||
| .into_iter() | ||
| .map(|op| OutPointFFI { | ||
| txid: txid_to_array(&op.txid), | ||
| vout: op.vout, | ||
| }) | ||
| .collect(); |
There was a problem hiding this comment.
🟡 Suggestion: Use the canonical OutPointFFI conversion
OutPointFFI already implements From<&dashcore::OutPoint> and documents that implementation as the single authority for preserving the persistence join key and txid byte order. The new export manually duplicates the conversion. It is equivalent today but can silently diverge if the canonical representation changes, so route this export through the existing conversion boundary.
| let entries: Vec<OutPointFFI> = rows | |
| .into_iter() | |
| .map(|op| OutPointFFI { | |
| txid: txid_to_array(&op.txid), | |
| vout: op.vout, | |
| }) | |
| .collect(); | |
| let entries: Vec<OutPointFFI> = rows.iter().map(OutPointFFI::from).collect(); |
source: ['codex']
There was a problem hiding this comment.
Resolved in 2195b18 — Use the canonical OutPointFFI conversion no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
|
@bfoss765 — this is the store-reconcile series from the job-flower investigation, retargeted off the integration branch per review: it now stacks on #4406 (whose swept-tombstone semantics the shared 🤖 Generated with Claude Code |
|
Heads-up on the base, since this is stacked on Two changes are landing on that branch that touch files you also touch, so it is worth knowing before you build further on them. Already landed ( Coming next, and this one does overlap you: the swept-tombstone lifetime rule is being reworked. The current version stamps a tombstone with the height at which the sweep was observed and collects it after a fixed margin; that is unsound for an InstantSend-locked winner that stays unmined, so it is being replaced with the winner's actual mined height, carried on the event by dashpay/rust-dashcore#975. Concretely, on this branch that will change:
It also needs a repin once #975 merges. Room schema v13 ( No action needed from you — the base moves under you and your PR keeps rebasing — but if you are about to write anything in the tombstone or |
…ot 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>
|
Review round addressed in 2195b18 — all four blockers plus the suggestions:
Suggestions: contact-row exclusion now resolves ownership through 117 handler tests green (all reconcile semantics re-pinned to log-only behavior), FFI 277 green. @romchornyi thanks for the base heads-up — will take the tombstone-lifetime rework and the #975 repin as a merge round here when it lands, and expect the v13 schema renumber. 🤖 Generated with Claude Code |
|
Device validation of this branch exactly as pushed (f0f632e + the rust-dashcore#979 pin): full uninstall → seed restore → rescan → kill+relaunch on the CoinJoin-heavy testnet wallet. Reconcile reported One composition note for reviewers: transaction-history net sums only converge with #4438 landed too. This branch heals coins and balances on its own, but multi-account history rows persist one account's slice until the record fold (#4438) is in — observed directly: the same wallet's history sum converges on a base that includes the fold and doesn't here. The two PRs are independent to merge but complementary in effect. Also observed, expected but worth documenting: contact-payment coins land only after a DashPay backfill pass that needs an unlocked wallet session (deferred contact-crypto builds gate it) — a freshly restored wallet that is never unlocked shows a balance short by its incoming contact payments until first unlock. 🤖 Generated with Claude Code |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
The latest review round correctly demotes ambiguous spend and net-amount changes to logging, counts only successful inserts, resolves contact ownership through core addresses, and uses the canonical outpoint conversion. Three blockers remain: restored DashPay pools still skip repair despite carrying usable account xpubs, healed TXOs can lose their exported account ownership and disappear again on restart, and the periodic JNI inventory fully materializes an unbounded chain-inflatable snapshot in a mobile process. Source: Codex general, security-auditor, and FFI-engineer reviewer lanes (exact backend model IDs were not supplied); final verifier: Claude Agent SDK (exact backend model ID was not supplied); openclaw-agent/cliproxy/gpt-5.6-sol is orchestration-only and 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— security-auditor (completed),gpt-5.6-sol— ffi-engineer (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 3 blocking | 🟡 1 suggestion(s)
2 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 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/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt`:
- [BLOCKING] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt:1299-1306: Preserve exported account ownership on healed TXOs
The native inventory identifies the account that owns each UTXO, but this insertion ignores those tags. `upsertUtxoRow` preserves an existing `accountId` and sets `coreAddressId` only when the corresponding `core_addresses` row already exists. If persistence lost both the TXO and its address row—the two divergence classes this PR is intended to repair—the reconciled row is inserted with neither ownership link. On the next launch, `buildUtxoRestoreData` can resolve ownership only through `txo.accountId` or the address relationship, so it skips the healed output and recreates the visible fund loss. Emit the complete account tuple already present in `AccountBalanceEntryFFI` (`typeTag`, `standardTag`, index, registration index, key class, and DashPay identity IDs), resolve the Room account during reconciliation, and persist its ID on the healed TXO even when the address projection is absent.
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt:1327-1330: Interpolate values in the healed-TXO diagnostic
Each dollar sign is escaped with `${'$'}`, so the diagnostic emits literal placeholders such as `${txid.toHex()}:$vout`, `$amount`, and `${priorTx.netAmount}`. That removes the outpoint and values needed to investigate the nonzero reconciliation heal this warning is intended to identify.
In `packages/rs-unified-sdk-jni/src/wallet_manager.rs`:
- [BLOCKING] packages/rs-unified-sdk-jni/src/wallet_manager.rs:3202-3348: Do not materialize an unbounded chain-controlled inventory
This periodic operation stores one formatted `String` per UTXO and spent outpoint, duplicates them through `join`, builds another complete JSON string with `format!`, copies that value across JNI, and then Kotlin parses it into a full JSON DOM and constructs additional full-inventory hash sets. Inventory cardinality is unbounded and remotely inflatable because anyone who knows a watched address can repeatedly send dust outputs to it. On a memory-constrained mobile process, an attacker-inflated wallet can therefore make every SYNCED transition and 30-minute reconciliation allocate several simultaneous copies of the inventory, causing repeated allocation failure or process termination. Expose a bounded, cursor-based snapshot or stream/iterate records through a native callback or compact binary representation so neither Rust nor Kotlin must hold the complete serialized inventory at once.
- [SUGGESTION] packages/rs-unified-sdk-jni/src/wallet_manager.rs:3205-3207: Snapshot all account inventories in one pass
(existing thread: https://github.com/dashpay/platform/pull/4439#discussion_r3826530759)
The JNI method first enumerates all N accounts and then invokes separate per-account UTXO and spent-outpoint accessors. Each accessor reacquires `wallet_manager.blocking_read()`, rebuilds `all_accounts()`, and linearly searches that N-element collection. One reconciliation therefore performs O(N²) account traversal and 2N+1 lock acquisitions. DashPay adds accounts per contact, and this path runs at every SYNCED transition and every 30 minutes. Add a manager-level inventory operation that gathers each account's identity, UTXOs, and spent outpoints under one read lock before exposing the snapshot through FFI.
In `packages/rs-platform-wallet-ffi/src/persistence.rs`:
- [BLOCKING] packages/rs-platform-wallet-ffi/src/persistence.rs:4085-4113: Repair DashPay pools with their concrete account xpub
(existing thread: https://github.com/dashpay/platform/pull/4439#discussion_r3826530747)
The resolver still routes both DashPay variants through `Wallet::key_source_for_account_type`, which the pinned key-wallet revision explicitly maps to `NoKeySource`. The subsequent `repairable` guard therefore skips every DashPay pool. This is not a cryptographic limitation: `build_wallet_start_state` reconstructs these ECDSA accounts with their persisted `account_xpub`, `AccountCollection::account_of_type` supports both full DashPay account variants, and the normal DashPay registration paths construct the same `Absent` address pools from `KeySource::Public(account.account_xpub)`. A sparse `DashpayReceivingFunds` pool consequently remains unable to recognize payments to omitted indices after cold restore until an unlocked contact-sync happens, so the restore-time repair promised by this PR is incomplete. Resolve the concrete full `AccountType` through `wallet.accounts.account_of_type(account_type)` and use its xpub as the public key source, retaining the existing helper as a fallback for special key accounts. Add a sparse DashPay receiving-pool restore test using a real wallet; the current test passes `None` and cannot exercise derivation.
| val wrote = upsertUtxoRow( | ||
| database, walletId, txid, vout, amount, address, scriptPubKey, | ||
| height, | ||
| isCoinbase = false, | ||
| isConfirmed = true, | ||
| isInstantLocked = false, | ||
| isLocked = isLocked, | ||
| ) |
There was a problem hiding this comment.
🔴 Blocking: Preserve exported account ownership on healed TXOs
The native inventory identifies the account that owns each UTXO, but this insertion ignores those tags. upsertUtxoRow preserves an existing accountId and sets coreAddressId only when the corresponding core_addresses row already exists. If persistence lost both the TXO and its address row—the two divergence classes this PR is intended to repair—the reconciled row is inserted with neither ownership link. On the next launch, buildUtxoRestoreData can resolve ownership only through txo.accountId or the address relationship, so it skips the healed output and recreates the visible fund loss. Emit the complete account tuple already present in AccountBalanceEntryFFI (typeTag, standardTag, index, registration index, key class, and DashPay identity IDs), resolve the Room account during reconciliation, and persist its ID on the healed TXO even when the address projection is absent.
source: ['codex']
| let mut rows: Vec<String> = Vec::new(); | ||
| let mut spent_rows: Vec<String> = Vec::new(); | ||
| let mut errors: Vec<String> = Vec::new(); | ||
| if !entries.is_null() && count > 0 { | ||
| let accounts = unsafe { std::slice::from_raw_parts(entries, count) }; | ||
| for acc in accounts { | ||
| let spec = platform_wallet_ffi::AccountSpecFFI { | ||
| type_tag: acc.type_tag as u8, | ||
| standard_tag: acc.standard_tag as u8, | ||
| index: acc.index, | ||
| registration_index: acc.registration_index, | ||
| key_class: acc.key_class, | ||
| user_identity_id: acc.user_identity_id, | ||
| friend_identity_id: acc.friend_identity_id, | ||
| account_xpub_bytes: ptr::null(), | ||
| account_xpub_bytes_len: 0, | ||
| }; | ||
| let mut utxos: *const platform_wallet_ffi::AccountUtxoEntryFFI = ptr::null(); | ||
| let mut utxo_count: usize = 0; | ||
| let res = unsafe { | ||
| platform_wallet_ffi::platform_wallet_account_utxos( | ||
| manager_handle as Handle, | ||
| wid.as_ptr(), | ||
| &spec, | ||
| &mut utxos, | ||
| &mut utxo_count, | ||
| ) | ||
| }; | ||
| if let Some(msg) = pwffi_error_message(res) { | ||
| errors.push(format!( | ||
| "{{\"typeTag\":{},\"index\":{},\"message\":{}}}", | ||
| acc.type_tag as u8, | ||
| acc.index, | ||
| json_escape(&msg), | ||
| )); | ||
| continue; | ||
| } | ||
| if utxos.is_null() || utxo_count == 0 { | ||
| continue; | ||
| } | ||
| let items = unsafe { std::slice::from_raw_parts(utxos, utxo_count) }; | ||
| for u in items { | ||
| let script: &[u8] = if u.script_pubkey.is_null() || u.script_pubkey_len == 0 { | ||
| &[] | ||
| } else { | ||
| unsafe { | ||
| std::slice::from_raw_parts(u.script_pubkey, u.script_pubkey_len) | ||
| } | ||
| }; | ||
| let script_buf = dashcore::ScriptBuf::from(script.to_vec()); | ||
| let address = dashcore::Address::from_script(&script_buf, net) | ||
| .map(|a| a.to_string()) | ||
| .unwrap_or_default(); | ||
| rows.push(format!( | ||
| "{{\"typeTag\":{},\"standardTag\":{},\"index\":{},\ | ||
| \"txid\":\"{}\",\"vout\":{},\"amount\":{},\ | ||
| \"address\":{},\"scriptHex\":\"{}\",\ | ||
| \"height\":{},\"isLocked\":{}}}", | ||
| acc.type_tag as u8, | ||
| acc.standard_tag as u8, | ||
| acc.index, | ||
| hex_lower(&u.outpoint_txid), | ||
| u.outpoint_vout, | ||
| u.value_duffs, | ||
| json_escape(&address), | ||
| hex_lower(script), | ||
| u.height, | ||
| u.is_locked, | ||
| )); | ||
| } | ||
| unsafe { | ||
| platform_wallet_ffi::platform_wallet_account_utxos_free( | ||
| utxos as *mut platform_wallet_ffi::AccountUtxoEntryFFI, | ||
| utxo_count, | ||
| ) | ||
| }; | ||
| } | ||
| // Second inventory half: the engine's spent outpoints, so the | ||
| // reconcile can classify a store row still marked unspent — | ||
| // present here means the row lost its spend update | ||
| // (dashpay/platform#4425, flip it); present in neither | ||
| // inventory means swept/abandoned residue | ||
| // (pre-rust-dashcore#971 stores, log-only). Soft-fail like the | ||
| // UTXO loop: one bad account must not mask the rest. | ||
| for acc in accounts { | ||
| let spec = platform_wallet_ffi::AccountSpecFFI { | ||
| type_tag: acc.type_tag as u8, | ||
| standard_tag: acc.standard_tag as u8, | ||
| index: acc.index, | ||
| registration_index: acc.registration_index, | ||
| key_class: acc.key_class, | ||
| user_identity_id: acc.user_identity_id, | ||
| friend_identity_id: acc.friend_identity_id, | ||
| account_xpub_bytes: ptr::null(), | ||
| account_xpub_bytes_len: 0, | ||
| }; | ||
| let mut outpoints: *const platform_wallet_ffi::OutPointFFI = ptr::null(); | ||
| let mut spent_count: usize = 0; | ||
| let res = unsafe { | ||
| platform_wallet_ffi::platform_wallet_account_spent_outpoints( | ||
| manager_handle as Handle, | ||
| wid.as_ptr(), | ||
| &spec, | ||
| &mut outpoints, | ||
| &mut spent_count, | ||
| ) | ||
| }; | ||
| if let Some(msg) = pwffi_error_message(res) { | ||
| errors.push(format!( | ||
| "{{\"typeTag\":{},\"index\":{},\"message\":{}}}", | ||
| acc.type_tag as u8, | ||
| acc.index, | ||
| json_escape(&msg), | ||
| )); | ||
| continue; | ||
| } | ||
| if outpoints.is_null() || spent_count == 0 { | ||
| continue; | ||
| } | ||
| let items = unsafe { std::slice::from_raw_parts(outpoints, spent_count) }; | ||
| for op in items { | ||
| spent_rows.push(format!( | ||
| "{{\"txid\":\"{}\",\"vout\":{}}}", | ||
| hex_lower(&op.txid), | ||
| op.vout, | ||
| )); | ||
| } | ||
| unsafe { | ||
| platform_wallet_ffi::platform_wallet_account_spent_outpoints_free( | ||
| outpoints as *mut platform_wallet_ffi::OutPointFFI, | ||
| spent_count, | ||
| ) | ||
| }; | ||
| } | ||
| } | ||
| unsafe { | ||
| platform_wallet_ffi::platform_wallet_manager_free_account_balances( | ||
| entries as *mut platform_wallet_ffi::AccountBalanceEntryFFI, | ||
| count, | ||
| ) | ||
| }; | ||
| let json = format!( | ||
| "{{\"utxos\":[{}],\"spent\":[{}],\"errors\":[{}]}}", | ||
| rows.join(","), | ||
| spent_rows.join(","), | ||
| errors.join(","), | ||
| ); |
There was a problem hiding this comment.
🔴 Blocking: Do not materialize an unbounded chain-controlled inventory
This periodic operation stores one formatted String per UTXO and spent outpoint, duplicates them through join, builds another complete JSON string with format!, copies that value across JNI, and then Kotlin parses it into a full JSON DOM and constructs additional full-inventory hash sets. Inventory cardinality is unbounded and remotely inflatable because anyone who knows a watched address can repeatedly send dust outputs to it. On a memory-constrained mobile process, an attacker-inflated wallet can therefore make every SYNCED transition and 30-minute reconciliation allocate several simultaneous copies of the inventory, causing repeated allocation failure or process termination. Expose a bounded, cursor-based snapshot or stream/iterate records through a native callback or compact binary representation so neither Rust nor Kotlin must hold the complete serialized inventory at once.
source: ['codex']
| "txos reconcile: healed TXO ${'$'}{txid.toHex()}:${'$'}vout " + | ||
| "(${'$'}amount duffs) has a pre-existing record whose " + | ||
| "netAmount may be short by that amount — LOG-ONLY, " + | ||
| "storedNet=${'$'}{priorTx.netAmount}", |
There was a problem hiding this comment.
🟡 Suggestion: Interpolate values in the healed-TXO diagnostic
Each dollar sign is escaped with ${'$'}, so the diagnostic emits literal placeholders such as ${txid.toHex()}:$vout, $amount, and ${priorTx.netAmount}. That removes the outpoint and values needed to investigate the nonzero reconciliation heal this warning is intended to identify.
| "txos reconcile: healed TXO ${'$'}{txid.toHex()}:${'$'}vout " + | |
| "(${'$'}amount duffs) has a pre-existing record whose " + | |
| "netAmount may be short by that amount — LOG-ONLY, " + | |
| "storedNet=${'$'}{priorTx.netAmount}", | |
| "txos reconcile: healed TXO ${txid.toHex()}:$vout " + | |
| "($amount duffs) has a pre-existing record whose " + | |
| "netAmount may be short by that amount — LOG-ONLY, " + | |
| "storedNet=${priorTx.netAmount}", |
source: ['codex']
…engine's UTXO inventory The Room txos mirror is write-behind with no feedback loop: a changeset that fails to deliver an owned output leaves a permanent hole, and the engine is REBUILT from the mirror on restart (buildUtxoRestoreData), so the hole graduates to a fund-loss on the next launch. Observed on job-flower as the 106.43 -> 86.33 restart drop: the rescan nondeterministically drops the change outputs of sends funded from CoinJoin-account outputs (all three known restores dropped the three Aug 5 change outputs; one of three also dropped the Aug 15 one). - walletManagerAllUtxosJson (JNI): the engine's full per-account UTXO inventory as JSON — account_balances sweep to enumerate accounts, platform_wallet_account_utxos per account, address derived from the script; per-account faults reported in-band so one bad account cannot mask the others' repair. - PlatformWalletManager.reconcileTxoStore / handler.reconcileTxos: insert-only diff of that inventory against Room — never flips spend state, never deletes (the mirror may legitimately be ahead on live spends and carries watch-only contact outputs). 100-conf gate because the snapshot cannot carry isCoinbase/isInstantLocked; fresher holes age into the next sweep. - netAmount repair: a record born blind to its own change output persisted netAmount short by exactly that value (verified: 6cef55ab stored -10.00010000 vs true -0.11000227); credit it back when the transaction row pre-exists with real bytes. - onWalletChangesetUtxoAdded body extracted to upsertUtxoRow so the callback and the reconciler share one insert discipline (stub tx FK row + pending-input drain). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…core changeset A gap-limit rescan correction (rust-dashcore fix/key-wallet-rescan-changeset) arrives as a BlockProcessed *updated* record whose output roles flipped from Sent to Received/Change. Deriving new/spent UTXOs from inserted records only delivered the corrected row but left the store's TXO hole in place — the reload fund-loss shape. Ordinary re-confirmations re-emit the same UTXOs, which the persisters absorb idempotently (upsertUtxoRow preserves spend linkage; spend-first outputs are flipped by the deferred-input drain). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tore restore_core_address_pools ingested the persisted address rows as-is: a mirror that dropped rows (observed in the field: BIP44-change indices 875..=890 absent between surviving rows) produced an in-memory pool with holes, and the row-derived highest_generated suppressed the gap-limit re-derivation that would have filled them. Outputs paying the missing addresses were permanently unrecognizable — the reason a blockchain rescan could not recover funds a fresh seed-restore could (the rescan rebuilds pools from the store; a fresh restore derives them from the seed). The loader now resolves each pool's key source from the signing wallet (built from the persisted account xpubs a few lines earlier) and calls AddressPool::ensure_contiguous_to after row ingestion: every missing index up to the persisted watermark is re-derived, existing rows and used flags untouched. Unresolvable key sources and hardened pools skip the repair and restore exactly as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ngine disagrees with Review feedback on the reconcile (Layer 1): the insert-only pass misses other producers of the same store-divergence class. The engine inventory export now carries both halves (unspent UTXOs + spent outpoints, via a new platform_wallet_account_spent_outpoints FFI), and the reconcile adds a reverse pass classifying every store row: - store-unspent, engine-spent: the row lost its spend update (dashpay#4425) — flipped to spent in place. The only mutation in the reverse pass; worst-case error hides a coin the next reconcile re-inserts, and four upstream layers now keep the post-SYNCED engine trustworthy. - store-unspent, engine-unknown: swept/abandoned residue (pre-rust-dashcore#971 stores) — LOG-ONLY, counted and named, never removed. Removal by reconciliation is the one direction where a bug destroys user-visible data. - store-spent, engine-unspent: lost release event, or a live spend racing the engine's map — indistinguishable at reconcile time, and un-marking a coin mid-payment would let the wallet double-spend it. LOG-ONLY. - Watch-only DIP-15 contact rows are excluded up front: the engine's accounts never report them, so their absence is expected, not divergence. Five new handler tests pin the flip, the never-remove, the never-unmark, the young-coin consistency case, and the contact-row exclusion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ot 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>
…unt coins
The foreign exclusion lived only on the reverse pass: store rows were
checked against the watch-only DIP-15 external accounts, but the insert
pass healed every engine-inventory UTXO the store lacked — and the
engine's inventory export includes those accounts' coins (it tracks
them to show payments TO contacts; they are the contact's money).
Device evidence (fresh restore of a 7,300-tx CoinJoin wallet,
2026-08-25): the post-backfill reconcile healed 12 contact-payment
coins (5,692,493 duffs) into the store as ownerless rows while the
reverse pass counted the very same rows as foreign — and the
mirror-reload path hands such store rows back to the engine at the
next launch.
Hoist the foreign-account resolution above the insert pass and skip
any engine UTXO whose address resolves (via core_addresses.accountId,
the same second path rowIsForeign uses) to an external account,
counting it as foreign rather than healed. An unresolvable address is
not provably foreign and proceeds, keeping the pass's provable-only
discipline symmetric.
Also un-escape the netAmount-suspect log template, which printed
literal "${txid.toHex()}:$vout" instead of values.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…th dev (a5d7ea0b) The dashpay#979 engine fixes and dashpay#974's coalesced committed-range sweeps now live on one rev: fix/key-wallet-rescan-changeset merged with dev, with the durable pending-sweep re-keyed to the coalesced model (manager-level swept-awaiting-commit receipt, resume-only seeding). Replaces the two superseded pins this branch carried (6768f983, 9a68e652). The rev also carries rust-dashcore dashpay#981 (Mnemonic::from_phrase is now the auto-detecting parse), so the three parse_mnemonic_any_language wordlist walks (wallet_lifecycle, rs-platform-wallet-ffi derivation + identity_keys_from_mnemonic, rs-sdk-ffi signer_simple) collapse to thin delegates and the language-tagged test call sites drop the argument. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
f0f632e to
7c1abcb
Compare
Review finding: the inventory identifies each UTXO's owning account, but the heal ignored the tags — upsertUtxoRow preserved an existing accountId and set coreAddressId only when the address row already existed. A heal into a store that lost BOTH the TXO and its address row (the two divergence classes this PR repairs) inserted a row with neither ownership link, which buildUtxoRestoreData skips at the next mirror-reload — recreating the visible fund loss the heal repaired. The inventory export now emits the complete account tuple per row (registrationIndex, keyClass, and the DashPay identity ids when set, alongside the existing typeTag/standardTag/index), the reconcile resolves the Room account from it and passes the id through a new upsertUtxoRow parameter (existing rows' accountId still wins; changeset callbacks pass null and keep their address-projection behavior). A heal whose account cannot be resolved still lands but is surfaced as healedUnowned in the report and log line. The account tag also becomes the authoritative insert-pass foreign check: a watch-only external account's coin is skipped by its typeTag whether or not its address row survived persistence, with the address-based lookup kept as a fallback for untagged inventories. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e adaptation The pin bump's adaptation to rust-dashcore#981 (from_phrase now auto-detects the wordlist) missed four test call sites in this file, so `cargo test -p platform-wallet-ffi` did not COMPILE on this branch — the crate's 279 tests never ran, here or in CI. Drops the now-removed Language argument and the import that only served it. No behavior change: the same static BIP-39 English vector parses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review finding: the reconcile pulled the engine's ENTIRE UTXO inventory
across the boundary in one call, formatted it as one JSON string, and
built HashSets over both engine inventories to classify store rows. A
wallet's UTXO count is chain-controlled — anyone who knows a watched
address can keep sending dust to it — so a remote party decided how
much a phone allocated at every SYNCED transition and every 30-minute
cadence tick. The pass meant to protect funds was itself the
unbounded allocation.
Both directions are now paged, and nothing bigger than one page is ever
materialized, copied across JNI, parsed, or held as a set.
Engine side, two new accessors next to the ones they bound:
account_utxos_page_blocking takes (after, limit) and returns the page
plus whether more follow — the account's UTXOs live in a BTreeMap keyed
by outpoint, so a page is a partial select over the keys with no copy
of the rows it skipped, and has_more is one lazy step past the page.
classify_outpoints_blocking answers a batch of outpoints positionally
(0 unknown / 1 unspent / 2 spent, unspent wins ties) by probing each
account's UTXO map and borrowed spent set under ONE read lock, so its
cost is the batch size times the account count, never an inventory
size.
walletManagerAllUtxosJson is replaced by walletManagerUtxosPageJson,
returning {utxos,errors,cursor,hasMore} with the same per-row format
including the account tuple. Accounts are swept in packed-tuple sort
order rather than the get_account_balances ordinal: the sweep resumes
across calls, so it needs an order a concurrently registered or removed
account cannot shift underneath it — an ordinal cursor could skip or
repeat data already paged. Page size is clamped natively (512 default,
4096 cap) rather than trusted from the host. The spent-outpoint export
is no longer part of the inventory; walletManagerClassifyOutpoints
replaces it, taking the flat n*36 blob that IS the Room txos.outpoint
primary key, so the caller concatenates the column and reads verdicts
back positionally.
Handler side, reconcileTxos takes the two transports as lambdas plus a
page size. The insert pass applies one engine page per Room
transaction — many small commits instead of one giant one, which is
safe precisely because the pass is insert-only and idempotent — and
fetches pages outside the exclusion lock, since the lock exists to keep
changeset callbacks out of our writes, not out of the engine. The
reverse pass is inverted: it pages the STORE's own rows (new
TxoDao.pageByWallet, an index walk on the outpoint primary key, so no
row is visited twice or skipped when another is inserted mid-sweep) and
asks the engine about one page at a time. Being read-only it now runs
outside both the lock and any transaction. The engineUnspentKeys /
engineSpentKeys HashSets are deleted.
Every verdict, counter, and the single summary log line are unchanged.
A page or classification batch that fails truncates the sweep instead
of faulting it — whatever already landed is correct, and the rest waits
for the next cadence tick — recorded in the new transportFailures
counter. reconcileTxoStore prefetches page 0 so a dead transport still
means "no report", the contract callers had before.
Tests: 2 Rust accessor tests (pages cover the account exactly once,
terminate, agree with the unpaged accessor, and stay empty for
keys-only accounts; classification is positional at both edges) and 4
Kotlin (page walk, mid-sweep page failure keeping earlier heals,
batched store classification, classification failure). A FakeEngine
serves a whole-inventory blob as pages behind an opaque cursor, and the
existing reconcile tests now run through it at 2 rows per page, so they
exercise the cursor loop rather than a single page.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3 participants
Issue being fixed
The Room store (
txos/transactions/core_addresses) diverged from the engine under rescan, interruption, and delivery loss — and because the engine is rebuilt FROM the store at launch (buildUtxoRestoreData), divergence graduated into visible fund loss on relaunch (testnet field case: 106.43 → 86.33 after restart; root causes fixed engine-side in dashpay/rust-dashcore#979).Stacked on #4406 (this branch's base): the reconcile's shared TXO-insert path absorbs #4406's swept-tombstone semantics, so both writers honor the same rules. Depends on dashpay/rust-dashcore#979: the first commit pins rust-dashcore to that PR's head so CI builds; re-pin to the
devmerge commit once it lands.What was done
reconcileTxoStore— post-sync audit of the store against the engine's full inventory (newwalletManagerAllUtxosJsonJNI export): heals missing TXOs (insert-only, 100-conf gate), repairs the netAmounts those holes falsified, logs every action. Runs on the SPV SYNCED transition and every 30 min. Nonzero heals after fix: same block core chain lock height #979 = regression tripwire.restore_core_address_poolsresolves each pool's key source from the signing wallet and re-derives indices missing from the persisted rows (holes observed in the field made funds rescan-proof invisible; this also closes the rescan-vs-fresh-restore divergence).platform_wallet_account_spent_outpoints). Store rows the engine proves spent are flipped; rows the engine has never seen are LOGGED, never deleted; spent rows the engine disputes are LOGGED, never un-marked (a live spend racing the engine is indistinguishable from lost-release residue, and un-marking could double-spend). Watch-only DIP-15 contact rows are excluded from classification.derive_new_utxosoverupdatedrecords is included for consistency, with a reviewer note thatfrom_changesetre-derives from records and ignorescs.new_utxos— that field may be vestigial and worth a follow-up decision.How this was tested
Merge order: rust-dashcore#979 → re-pin here → #4406 → this.
🤖 Generated with Claude Code