Repository navigation
fix(kotlin-sdk): port #4638 to Android — credit verdicts at save time and the post-sync TXO reconcile - #5259
Open
HashEngineering wants to merge 20 commits into
Open
HashEngineering wants to merge 20 commits into
HashEngineering wants to merge 20 commits into
Conversation
…lassifier #4638 landed the wallet-wide paged UTXO inventory and the outpoint classifier in platform-wallet, and stopped at the C boundary — the Swift host reaches them, Kotlin does not. These two exports carry them across JNI and no further. Each is a serialization shim over exactly ONE platform-wallet call. The account ordering, the cursor semantics, the page bound, the address rendering and every classification rule stay in `platform-wallet`, so both hosts walk the identical inventory and get identical verdicts. Nothing here decides anything: serde shapes the JSON against the same field names the Kotlin side decodes, so a renamed key fails that decode loudly instead of healing a defaulted row. Paged rather than swept whole because inventory size is chain-controlled: anyone who knows a watched address can keep sending dust to it, and a periodic full-inventory read would let them decide how much a phone allocates on every pass. Classification queries carry the account tuple the STORE files each coin under and the script it recorded. The engine checks that claim against its own pools rather than trusting it, which is what lets it answer `not-owned` — so a bare outpoint cannot ask the question. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Room `txos` mirror is write-behind with no feedback loop: a changeset
that fails to deliver an owned output leaves a permanent hole. Because
the engine is REBUILT from this mirror on restart, that hole graduates to
a fund loss on the next launch — observed in the field as the job-flower
106.43 -> 86.33 restart drop, where a rescan nondeterministically drops
the change outputs of sends funded from CoinJoin-account outputs.
Two bounded passes, sharing only the watch-only contact accounts they
both exclude:
* the insert pass walks the ENGINE's paged inventory and inserts the
rows the store lacks, one Room transaction per page. Insert-only and
idempotent, so a sweep is many small commits rather than one large
one. Only outputs at least `minConfirmations` deep are healed;
immature holes age into the next sweep. The engine's own isConfirmed
/ isInstantLocked / isCoinbase travel with each row and are stamped
as given — a coinbase output filed as an ordinary one would misstate
maturity in the mirror the engine reloads from.
* the classification pass is inverted: it pages the STORE and asks the
engine about one page at a time, so neither side builds a set over a
whole inventory. Every verdict is log-only. `known-uncredited` is the
one class that is positive evidence (the owning account knows the
funding txid, owns the script, does not hold the coin, and a funds
account holds a MINED record spending it), and even that is only
counted: flipping spent state is the one direction where a
reconciler bug spends a coin the user still owns. Acting on it is a
separate, reviewable change.
Watch-only DIP-15 contact coins are excluded on both sides — they are the
contact's money. The engine's inventory omits those accounts outright;
the store-side check resolves ownership through both the explicit account
link and the `core_addresses` projection production writes actually use,
because an accountId-only check silently classifies every contact row.
Triggered inside the SDK on the SPV SYNCED transition and every 30
minutes while synced, rather than left to each host: the hole it repairs
becomes a fund loss on the next engine reload if any host forgets to call
it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…eries
kotlinx's default Json omits a property equal to its default value, and
OutpointQuery defaulted standardTag/index/registrationIndex/keyClass to 0.
For the common BIP44 account-0 store row every one of those is 0, so the
query went out as {"typeTag":0,"txid":…,"vout":…,"scriptHex":…} and
rs-unified-sdk-jni's OutpointQuery (whose flattened UtxoAccountTuple
requires those fields) rejected every batch:
malformed outpoint queries: missing field `standardTag` at line 1 column 162
The reverse (store→engine) half of reconcileTxoStore therefore never
classified a single row on device. Drop the defaults so the fields are
always encoded; only the identity halves stay optional, matching the
native tuple. The test FakeEngine now rejects a batch missing any key the
native deserializer requires, which is why the existing reconcile tests
did not catch this.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The heal pass counted and logged healed coins whose transaction record might carry a short netAmount. It was log-only and is not part of #4638, which this branch ports; it can follow on its own. The heal still never touches netAmount, and the tests keep checking that. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Register the persistence-extension slot on_persist_wallet_changeset_utxo_verdicts_fn that #4638 added and the JNI layer left unset. The new trampoline packs the round's verdicts into flat arrays (36-byte outpoint keys, verdict codes, observed- spent heights) and calls NativePersistenceBridge.onWalletChangesetUtxoVerdicts, whose default body ignores them, so a bridge that does not override it writes exactly what it wrote before. The descriptor is in the bridge verification table and pinned by a test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ation Kotlin port of #4638 Part 1. The handler stages the round's credit verdicts (they arrive before the changeset callback) and upsertUtxoRow applies them as Swift's upsertUtxo does: - observed_spent / doomed: the row is written spent, with no spender link; - any verdict vetoes the redelivery recovery clear (flag and sweep stamp); - uncredited changes nothing else; - verdicts live in the round buffer, so a rolled-back round leaves none. One log line per round that applied a verdict, counts only. Tests port BornSpentTxoPersistTests, including a reopened on-disk store restoring zero coins for a verdict-spent row. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… TXOs Groundwork for the #4638 store reconcile, which asks the engine about a page of store rows outside the callback lock and applies the verdicts under it: - committedRoundGeneration, bumped when a changeset round's Room transaction commits (and on a standalone callback write), so a verdict read before a round that may have re-credited the coin is refused; - TxoDao.pageUnspentByWallet, the outpoint-cursor walk over a wallet's isSpent = 0 rows that the classify pass pages through. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings PlatformWalletPersistenceHandler.reconcileTxos in line with Swift's runCoreTxoReconcile from #4638 Part 2; the Kotlin pass was carried over from #4439 and differed from it. Heal pass (engine -> store), Swift's gates in Swift's order: malformed row (txid not 32 bytes, no script, no address) is invalid; a coin the engine does not call confirmed, or under 100 confirmations, is immature; a coin already in the store is present; a coin whose seven-field account tuple names no store account is refused (no longer inserted unowned). Inserted rows go through upsertUtxoRow with isConfirmed = true, and a row the pending-input drain wrote spent is counted as healedSpent. The contact (DashpayExternalAccount) skip stays. Classify pass (store -> engine): only the wallet's isSpent = 0 rows are asked about, by outpoint cursor. A KnownUncredited verdict marks the row spent (no spender link, lastUpdated bumped) and drops the pending-input rows on it; Unspent, Unknown and NotOwned are counted only. Each page is read with committedRoundGeneration, classified outside the callback lock, and applied under it only if no round committed in between; otherwise the page is re-read, at most 5 times in a row. Every store step re-checks a caller cancellation flag under the lock. A failed engine read, the first page included, now yields an incomplete report instead of null; a failed store step rolls back and is counted. The report mirrors Swift's CoreTxoReconcileReport, and the logs carry counts and SHA-256 reference digests only: one summary line per run ("txos reconcile: mirror consistent (...)" when nothing changed) and one item line per healed / healed-spent / flipped row. Tests updated to the new semantics and ported from CoreTxoReconcileTests, CoreTxoReconcileShutdownTests and CoreTxoReconcilePrivacyTests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PlatformWalletManager.reconcileTxoStore(walletId) now follows Swift's reconcileCoreTxoStore(for:) from #4638 and returns a TxoReconcileOutcome: Skipped(reason) or Reconciled(report). Gates, in order: manager closed; wallet not loaded; a run for it already in flight; SPV not in steady state (SYNCED, or WAIT_FOR_EVENTS with the filter phase at a non-zero target); no scan tip (filter height, else header height); syncFaultDetected(); the wallet's durable watermark (the Room wallet row's syncedHeight) more than 6 blocks behind the tip. The automatic trigger runs on the rising edge into steady state and then every 30 minutes per wallet, stamped at schedule time, never while that wallet's run is in flight. closeSuspending() and removeWallet() bump a reconcile epoch, and the run stops at its next page or store step with completed = false; the run also holds a teardown-gate borrow so the native manager is never destroyed under an engine read. Skip, transport-failure and run-failure lines carry the reason or the exception type and a wallet reference digest only; the cursor and native error messages are no longer logged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reconcile skipped itself at the SYNCED edge of a restore (WALLET_BEHIND_TIP) and, stamped at schedule time, did not try again for 30 minutes. The gate read the wallet's persisted Room syncedHeight, which trails the engine by up to one persistence round — exactly the window the SYNCED transition lands in. Swift gates on the engine's own value (platform_wallet_core_wallet_state). Bind it — one JNI export over that one FFI call — and gate on it, through a pure TxoReconcileGates.walletCaughtUp with its own test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
|
⛔ Final review complete — 1 blocking finding(s) (commit da997b5) · triage: critical |
Merged
3 of 7 tasks
HashEngineering
marked this pull request as ready for review
October 8, 2026 03:05
Contributor
|
Bots are done — your move: post |
github-actions
Bot
added
waiting-self-review
Collaborator
Author
|
/self-review |
github-actions
Bot
removed
the
waiting-self-review
thepastaclaw
reviewed
Oct 8, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
Verified the supplied findings against head 420996c and the stated Android reconciliation scope. Four non-blocking suggestions remain: restore/reconcile ownership parity, diagnostic-log privacy, strict inventory decoding, and transport-contract coverage. This was a static review; the supplied CI snapshot reports Kotlin build/tests passing, with Rust workspace tests and several builds still pending.
🟡 4 suggestion(s)
Review provenance
Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); reviewer 11: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 12: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — The intricate reconciliation and save-time verdict logic in PlatformWalletPersistenceHandler.kt, including upsertUtxoRow and classifyStoreRows, changes which coins are persisted as spendable and restored into the engine, directly affecting coin-selection eligibility. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 15% left, 5h 100% left),glm-5.3-flash(not used above high effort; tier asks max) - Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— security-auditor (completed, effort xhigh); agentphase2-reviewer - Model comparison: every Phase-2 reviewer also ran on
gpt-6-astra; the verifier weighed both sets without knowing which model wrote which
🤖 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`:
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt:2043-2047: Resolve the same TXO owners during reconciliation and restore
The new resolver returns null when coreAddressId is absent, but buildUtxoRestoreData resolves the account through txo.address in that case. This difference is reachable: upsertUtxoRow leaves coreAddressId null when the address row does not yet exist, and the address-pool callback subsequently inserts that address without backfilling the TXO relationship. An unspent legacy row can therefore be eligible for restoration into Rust while every reconciliation run skips it as unresolved, even when the engine has positive evidence that it was spent. Share the restore path's address fallback with reconciliation and add a regression test where the address arrives after the TXO and a KnownUncredited verdict removes the row from the restore set.
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt:2130-2134: Avoid recoverable outpoint references in reconciliation logs
Each mutation logs an outpoint reference alongside a stable wallet reference. logReference computes an unkeyed SHA-256 digest truncated to six bytes, so a recipient of diagnostic logs can enumerate public blockchain outpoints, hash their wire-order txid plus little-endian vout, and match the reference. The wallet reference then groups those matched outputs into one wallet's history; truncation and the absence of literal identifiers do not prevent this lookup. Remove the outpoint-derived reference and retain aggregate diagnostics, or use a randomly keyed process-local reference whose key is never logged. Extend the privacy test beyond excluding raw identifiers and long hexadecimal strings. This is a non-blocking diagnostic-data exposure under the supplied severity policy, rather than a consensus or authentication violation.
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt:1358-1362: Require inventory account fields to enforce fail-loud decoding
EngineUtxoRow and engineJson explicitly promise that missing routing fields fail decoding rather than healing a row under a defaulted owner. However, standardTag, index, registrationIndex, and keyClass default to zero. A page missing index can therefore decode and resolve to account zero instead of failing. The current Rust producer emits all four fields, so this is a contract-hardening issue rather than a demonstrated failure with the current producer. Remove these defaults, retain the intentionally optional identity halves, and add a missing-routing-field decode test.
In `packages/rs-unified-sdk-jni/src/wallet_manager.rs`:
- [SUGGESTION] packages/rs-unified-sdk-jni/src/wallet_manager.rs:3354-3376: Test the actual Rust/Kotlin reconciliation JSON contract
The new Kotlin persistence tests use FakeEngine, which supplies handwritten inventory JSON, represents cursors as ordinal strings, and validates classification fields against a separately maintained RUST_REQUIRED_QUERY_KEYS list. No Rust tests exercise the new serde models, identity conversion, or account/outpoint cursor reconstruction. These tests consequently cannot detect disagreement between the actual Rust and Kotlin codecs, despite the documented zero-valued-field omission previously rejecting classification batches. Add shared golden payloads decoded by Rust's Vec<OutpointQuery> and Kotlin's EngineUtxoPage serializer, plus cursor round-trip tests through the actual conversion code. Cover zero-valued standard accounts, non-default account fields and DashPay identity halves, inventory flags, and malformed identity/txid rejection.
Out-of-scope follow-up suggestions (1)
These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.
- Track the same public-outpoint disclosure in existing Swift diagnostics — Existing Swift reconciliation logs also emit outpoint references, and SDKLogger.reference uses unkeyed SHA-256 truncated to six bytes. The same public-outpoint lookup applies, but those implementations are unchanged in this PR and do not expand its merge requirements.
- Follow-up: Track a separate privacy issue to remove public-outpoint-derived references or replace them with secret-keyed diagnostic references.
Contributor
|
Ready for review — files with no dedicated owner: QuantumExplorer or shumkov. |
github-actions
Bot
added
the
ready-for-human
Contributor
|
Waiting for bot review — coderabbitai not yet · thepastaclaw ✓. Wait for the missing reviews, or a writer can post |
github-actions
Bot
added
the
waiting-bots
The classify pass resolved a row's owner through the explicit accountId link or coreAddressId only, while buildUtxoRestoreData falls back to the row's own address. upsertUtxoRow leaves coreAddressId null when the address row does not exist yet, and the address-pool callback that inserts it later does not backfill the TXO, so such a row was restored into the engine at every launch while every reconcile run counted it unresolved, even when the engine proved it spent. Both paths now share one resolver: the explicit link, then the address projection keyed by coreAddressId or, failing that, the row's own non-empty address. Adds a regression test where the address row lands after the TXO and a known-uncredited verdict flips the row out of the restore set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
logReference was a plain SHA-256 prefix of the outpoint (and of the wallet id), so anyone holding diagnostic logs could hash public outpoints, match the per-item lines, and use the wallet reference on the same line to group the matches into one wallet's history. References are now the first six bytes of an HMAC-SHA256 under a 32-byte SecureRandom key drawn once per process on first use and never logged. They keep their 12-hex-character shape and still correlate within one run, but match no digest computable outside the process and do not correlate across runs. The privacy test now also rejects the unkeyed digests of the logged outpoints and wallet id, and a new test pins stability within the process and non-equality with SHA-256. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… fixture The Kotlin reconcile tests drive a FakeEngine with handwritten inventory JSON and a separately maintained list of the keys the Rust query deserializer requires, and nothing in Rust exercised the serde models, the identity conversion or the cursor reconstruction, so the two codecs could disagree without any test noticing. One checked-in fixture, kotlin-sdk/sdk/src/test/resources/golden/ txo_reconcile_transport_v1.json, is now read by both sides. The Rust tests (include_str! of that file) serialize the real UtxoPage for four accessor entries and require the fixture page exactly: a zero-valued BIP44 account, non-default account fields with every flag set and an amount past 32 bits, a registration index with no address form, and a DashPay account with both identity halves. They decode the fixture's cursor back into the resume point, round-trip a cursor carrying every account field, reject cursors this layer did not emit, decode the fixture's classification queries through the real OutpointQuery conversion back to the same coins, fail a batch missing any required field, and reject a malformed identity id, txid or script by name. The Kotlin tests decode the fixture page with the production decoder and build the fixture queries with the production encoder. To make that testable without a JVM the row, page, cursor and query conversions move out of the two JNI exports into small functions on the serde types; the exports behave as before. On the Kotlin side the page decode and the query construction get the same treatment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
EngineUtxoRow promised that a missing routing field fails the decode instead of healing a row under a defaulted owner, but standardTag, index, registrationIndex and keyClass defaulted to zero, so a page missing index decoded and resolved to account 0. The current Rust producer emits all four, so this hardens the contract rather than fixing an observed failure. The four fields are now required; the DashPay identity halves stay optional. Test inventory JSON that omitted registrationIndex and keyClass now carries them, and a new test proves a row missing any one account field fails to decode. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Waiting for bot review — coderabbitai not yet · thepastaclaw ✓. Wait for the missing reviews, or a writer can post |
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
At 0c674bc, the transport and persistence changes preserve the intended layering, but two in-scope, non-consensus correctness issues remain: classification can apply during an open persistence round, and stopping SPV does not reset the reconciliation scheduling edge. Four prior findings are fixed; the pagination index remains explicitly deferred. Validation was static only, and the supplied exact-head CI snapshot still shows build prerequisites queued and policy checks pending.
🟡 2 suggestion(s)
Review provenance
Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 9: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 11: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); reviewer 13: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 14: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 15: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 16: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 17: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — The large, intricate changes to PlatformWalletPersistenceHandler.upsertUtxoRow and applyEngineClasses directly alter which persisted coins are treated as spent or restored as spendable, meeting the critical coin-selection surface bar. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 14% left, 5h 100% left),glm-5.3-flash(not used above high effort; tier asks max) - Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
- Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer - Model comparison: every Phase-2 reviewer also ran on
gpt-6-astra; the verifier weighed both sets without knowing which model wrote which
🤖 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`:
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt:1939-1944: Defer classification applies while a persistence round is open
The generation check covers committed rounds, but not an open round: guarded releases callbackExclusion after each callback, and committedRoundGeneration advances only after onChangesetEnd commits. After classification returns KnownUncredited, a round can begin and stage a re-credited utxos_added before this apply acquires the mutex. The generation still matches, so the stale verdict marks the row spent. For a previously unspent row with a mempool spender link, the subsequent credited redelivery preserves that newly set flag through upsertUtxoRow's linked/alreadySpent handling; linkSpender also preserves existing spent flags. The engine can therefore hold the coin while the store excludes it from restoration, and subsequent reconciliation does not repair it because the heal pass skips existing rows and the classify pass skips spent rows. Refuse the apply while the wallet has an open round, releasing callbackExclusion while waiting or stopping the run incomplete, and reclassify after the round closes. Add a regression that stages the credited redelivery between classification and apply but ends the round afterward, including a mempool-linked row; the existing stale-round test commits before apply and misses this interleaving. This is a client persistence correctness issue, classified as a suggestion under the supplied non-consensus severity policy.
In `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt`:
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt:2298-2300: Reset the reconcile steady-state edge when SPV stops
Only the running branch feeds progress to TxoReconcileCoordinator. The stopped branch publishes SpvSyncProgressData.EMPTY without passing it to the coordinator, and stopSpv does not reset the coordinator either, so wasSteady remains true. If SPV restarts and the first accepted progress read is already steady, wallets scheduled within the previous 30 minutes miss the promised new steady-state reconciliation and wait for the remaining cadence. Feed the stopped/reset state to the coordinator or explicitly reset its edge. Add coverage for steady → stopped → steady without an intermediate syncing poll.
Out-of-scope follow-up suggestions (1)
These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.
- Use keyed references in Swift reconciliation logs — SDKLogger.reference still uses an unkeyed six-byte SHA-256 prefix, and Swift TXO reconciliation logs pair outpoint references with wallet references. A diagnostic-log recipient can hash candidate public outpoints and group matching outputs by wallet reference. These Swift paths are unchanged by this PR, which fixes the equivalent Android exposure.
- Follow-up: Track a separate Swift privacy fix using process-local randomly keyed references or aggregate-only diagnostics, with tests rejecting publicly computable outpoint references.
Contributor
|
@coderabbitai review No review for |
Contributor
|
Bots are done — your move: post |
github-actions
Bot
added
waiting-self-review
Collaborator
Author
|
/self-reviewed |
Contributor
|
Ready for review — files with no dedicated owner: QuantumExplorer or shumkov. |
github-actions
Bot
added
ready-for-human
…open The classify pass asks the engine about a page outside the callback lock and applies the verdicts under it only if committedRoundGeneration has not moved. That count advances only when a round commits, so a round that began in the gap and staged a credited re-delivery of one of the page's coins went unnoticed: the stale verdict marked the row spent before the round committed. For a row with a mempool spender link the re-delivery then keeps the flag, so the store dropped the coin from the restore while the engine still held it, and no later run looked at the spent row again. The apply now also refuses while the wallet has an open round, through the existing stale path: nothing is written, the page is classified again, and after the existing retry limit the run stops incomplete so the rows are classified again once the round has closed. A new test opens a round during classification that re-delivers a plain row and a mempool-linked row, keeps it open past the apply and closes it after the run. Both rows stay unspent and restorable, and a later run classifies them again; without the guard the linked row is left spent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Only the running branch of the SPV progress poll fed progress to TxoReconcileCoordinator. The stopped branch published the empty progress without telling the coordinator and stopSpv did not reset it either, so wasSteady stayed true across a stop. If SPV restarted and the first progress read was already steady, no rising edge was seen and every wallet waited out the 30-minute cadence instead of getting the steady-state reconcile. The coordinator now has spvStopped(), which clears the edge. The poll calls it on every stopped read and stopSpv calls it after the native stop, which also covers a restart inside one poll interval that the poll never sees as stopped. A new coordinator test goes steady, stopped, steady with no syncing read in between and expects every wallet to be due on the second steady read, and only on that one. With the reset removed it fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…xo-reconcile-4638-port # Conflicts: # Cargo.lock
Contributor
|
Waiting for bot review — coderabbitai not yet · thepastaclaw requested changes — dismiss the review or push a fix, 1 thread unresolved — resolve it. Wait for the missing reviews, or a writer can post |
github-actions
Bot
added
waiting-bots
thepastaclaw
suggested changes
Oct 17, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
At head 92fe869, six prior findings are fixed; the new Kotlin-owned reconciliation policy remains a blocking ownership violation, and the pagination index remains an untracked, non-blocking suggestion. Validation was static only; no builds or tests were run locally. Current CI shows Kotlin SDK build/tests passing and Rust workspace tests still pending.
🔴 1 blocking | 🟡 1 suggestion(s)
1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
Review provenance
Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 9: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 10: gemini-3.8-flash-high (agent: phase1-reviewer, role: rust-quality); reviewer 11: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — The intricate changes to PlatformWalletPersistenceHandler.kt, especially upsertUtxoRow and the post-sync reconcile, directly change which wallet outputs are restored as spendable, affecting coin selection through credit verdicts and concurrent spent-state updates. - Phase 1 reviewers:
gemini-3.8-flash-high— general (completed, effort high); agentphase1-reviewer,gemini-3.8-flash-high— rust-quality (completed, effort high); agentphase1-reviewer - Phase 1 model:
gemini-3.8-flash-high— antigravity quota: weekly 71% left, 5h 100% left - Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— security-auditor (completed, effort xhigh); agentphase2-reviewer - Model comparison: every Phase-2 reviewer also ran on
gpt-6-astra; the verifier weighed both sets without knowing which model wrote which
🤖 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:1799-1803: Move TXO reconciliation policy into the Rust wallet
This branch decides whether an engine-owned coin qualifies for reconstruction in durable wallet state. Together with the steady-state interpretation, six-block margin and cadence in TxoReconcileGates/TxoReconcileCoordinator, and the stale-page retry policy in classifyStoreRows, it introduces wallet-repair policy in Kotlin rather than merely persisting Rust-approved mutations. The Rust wallet_utxos_page and classify_outpoints accessors provide inventory and evidence, but do not own this reconciliation operation; Kotlin also repeats their watch-only contact exclusion. The book's “Where code goes” and “FFI and mobile SDKs hold no business logic” sections explicitly assign mobile orchestration to rs-platform-wallet, and packages/kotlin-sdk/CLAUDE.md prohibits policy-loop orchestration. Porting the existing Swift implementation creates another host-owned implementation rather than sharing these decisions. Move repair eligibility, scan gates, cadence and retry policy into rs-platform-wallet and expose them through thin FFI/JNI bindings. Keep Room pagination, relationship resolution, transactions and callback-exclusion/generation fences in the Kotlin persistence adapter.
In `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/dao/TxoDao.kt`:
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/dao/TxoDao.kt:48-52: Add an index matching the unspent pagination query
(existing thread: https://github.com/dashpay/platform/pull/5259#discussion_r4236385885)
TxoEntity and the current exported Room schema have a walletId index and the outpoint primary-key index, but no composite index supporting this query's wallet/spent predicates and cursor ordering. SQLite can filter and sort wallet-index results or walk the outpoint index while rejecting unrelated and spent rows; LIMIT bounds returned rows, not the work required to find them. readUnspentPage executes inside reconcileStoreStep under callbackExclusion, so expensive pages also delay persistence callbacks during the new automatic reconciliation walk. Add a non-unique (walletId, isSpent, outpoint) index with its Room migration and exported schema update. Keeping this PR schema-neutral explains the proposed separate delivery, but the supplied deferral has no linked tracking issue or PR; provide that reference before treating this suggestion as intentionally deferred.
Comment on lines
+1799
to
+1803
| if (!row.isConfirmed || row.height <= 0 || tipHeight < row.height || | ||
| tipHeight - row.height + 1 < minConfirmations | ||
| ) { | ||
| counts.skippedImmature++ | ||
| return |
Collaborator
There was a problem hiding this comment.
🔴 Blocking: Move TXO reconciliation policy into the Rust wallet
This branch decides whether an engine-owned coin qualifies for reconstruction in durable wallet state. Together with the steady-state interpretation, six-block margin and cadence in TxoReconcileGates/TxoReconcileCoordinator, and the stale-page retry policy in classifyStoreRows, it introduces wallet-repair policy in Kotlin rather than merely persisting Rust-approved mutations. The Rust wallet_utxos_page and classify_outpoints accessors provide inventory and evidence, but do not own this reconciliation operation; Kotlin also repeats their watch-only contact exclusion. The book's “Where code goes” and “FFI and mobile SDKs hold no business logic” sections explicitly assign mobile orchestration to rs-platform-wallet, and packages/kotlin-sdk/CLAUDE.md prohibits policy-loop orchestration. Porting the existing Swift implementation creates another host-owned implementation rather than sharing these decisions. Move repair eligibility, scan gates, cadence and retry policy into rs-platform-wallet and expose them through thin FFI/JNI bindings. Keep Room pagination, relationship resolution, transactions and callback-exclusion/generation fences in the Kotlin persistence adapter.
source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, ffi-engineer, rust-quality, security-auditor); gpt-6-astra (phase2-reviewer: general, architecture-layering, ffi-engineer, rust-quality, security-auditor)
…xo-reconcile-4638-port # Conflicts: # packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/NativePersistenceBridge.kt # packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt # packages/rs-unified-sdk-jni/src/persistence.rs
Contributor
|
Waiting for bot review — coderabbitai not yet · thepastaclaw requested changes — dismiss the review or push a fix, 1 thread unresolved — resolve it. Wait for the missing reviews, or a writer can post |
thepastaclaw
suggested changes
Oct 18, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
Verified the findings against da997b5: six prior issues are fixed, while the reconciliation-policy ownership blocker and pagination-index suggestion remain. The Rust APIs provide authoritative inventory and classification, but Kotlin still owns the reconciliation workflow contrary to the repository's explicit mobile-layer rules. This was a static verification; current CI reports successful Rust workspace and Kotlin SDK build/tests, with PR Hygiene pending.
🔴 1 blocking | 🟡 1 suggestion(s)
2 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
Review provenance
Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 8: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 9: gemini-3.8-flash-high (agent: phase1-reviewer, role: rust-quality); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 11: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — The intricate changes in PlatformWalletPersistenceHandler.kt, including upsertUtxoRow and post-sync reconciliation, alter persisted spent status and therefore which coins are restored to the engine as spendable, directly affecting coin selection. - Phase 1 reviewers:
gemini-3.8-flash-high— general (completed, effort high); agentphase1-reviewer,gemini-3.8-flash-high— rust-quality (completed, effort high); agentphase1-reviewer - Phase 1 model:
gemini-3.8-flash-high— antigravity quota: weekly 71% left, 5h 100% left - Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— security-auditor (completed, effort xhigh); agentphase2-reviewer - Model comparison: every Phase-2 reviewer also ran on
gpt-6-astra; the verifier weighed both sets without knowing which model wrote which
🤖 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:1832-1836: Move TXO reconciliation policy into the Rust wallet
(existing thread: https://github.com/dashpay/platform/pull/5259#discussion_r4304446179)
This gate decides whether an engine-owned coin is sufficiently confirmed to enter durable, restorable wallet state. The surrounding implementation also selects contact-account exclusions, reclassification retries, steady-state eligibility, the six-block watermark margin, and scheduling cadence. These are wallet-repair decisions, not merely Room persistence operations. The book's "Where code goes" table assigns mobile orchestration to rs-platform-wallet, its "FFI and mobile SDKs hold no business logic" section restricts Kotlin to persisting Rust-emitted state and bridging existing Rust operations, and packages/kotlin-sdk/CLAUDE.md explicitly prohibits policy-loop orchestration. The Rust wallet_utxos_page and classify_outpoints accessors own inventory and spend evidence, but not this reconciliation operation; copying Swift therefore creates another host-owned policy implementation rather than sharing one. Move reconciliation eligibility and orchestration into rs-platform-wallet and expose the operation through thin bindings. Keep Room pagination, relationship resolution, transactional application, and callback synchronization in the Kotlin adapter.
In `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/dao/TxoDao.kt`:
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/dao/TxoDao.kt:48-52: Add an index matching the unspent pagination query
(existing thread: https://github.com/dashpay/platform/pull/5259#discussion_r4236385885)
TxoEntity and exported Room schema v15 have single-column indexes but no index matching this query's walletId/isSpent predicates and outpoint cursor ordering. SQLite can repeatedly filter and sort wallet-index results, or traverse the outpoint index while discarding unrelated and spent rows; LIMIT bounds returned rows, not rows examined. readUnspentPage executes under callbackExclusion, while synchronous native persistence callbacks wait for that same mutex through guarded/runBlocking, so expensive pages can delay persistence rounds on large histories. Add a non-unique (walletId, isSpent, outpoint) index with the Room migration and exported schema update. Keeping this PR schema-neutral explains the proposed separate delivery, but the supplied discussion contains no linked follow-up; a tracking issue or PR is needed before recording this suggestion as intentionally deferred.
Contributor
|
Your move: thepastaclaw requested changes on this head; dismiss the review or push a fix; thepastaclaw left review threads unresolved; resolve them. |
github-actions
Bot
added
waiting-self-review
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
#4638 fixed, for iOS, TXO rows the store writes as unspent for coins the engine never credited
(the rust-dashcore#992 shape: a CoinJoin collateral burn processed before the coin's funding transaction). A
restore then hands those rows back to the engine as spendable at the next launch. #4638 added two things: credit
verdicts delivered at save time (Part 1, prevention) and a post-sync store↔engine reconcile (Part 2). Its Rust
side is already in
v5.1-dev. Android uses neither: the JNI layer never registers the verdict slot, and thereconcile only exists in #4439, which cannot build on
v5.1-devbecause its address-pool hole repair needsrust-dashcore#979's
ensure_contiguous_to.This PR is #4638 for Kotlin: the same Rust, the same rules, nothing that needs #979.
What was done?
wallet_utxos_pageandclassify_outpoints, both added by fix(swift-sdk): stop born-spent TXO rows at the persistence seam and reconcile the store after a full scan #4638.rules):
its target) and every 30 minutes per wallet.
behind the scan tip; a run for the wallet already in flight. Closing the manager or removing the wallet stops
a run between pages.
confirmed by the engine, at least 100 confirmations deep, and owned by an account the store can resolve from
the seven-field tuple. An unresolvable owner is counted, never filed unowned.
only on
KnownUncredited(a mined spender on record), and drops pending-input claims on it. The page is readwith a count of committed persistence rounds; if a round committed before the apply, nothing is written and
the page is classified again.
walletManagerCoreWalletSyncedHeight: a single call toplatform_wallet_core_wallet_state, so the reconcile gates on the engine's watermark as Swift does. Thepersisted copy in Room trails it by a round, which is where the SYNCED transition lands.
rs-unified-sdk-jniregisterson_persist_wallet_changeset_utxo_verdicts_fnand passes the round'sverdicts to
NativePersistenceBridge.onWalletChangesetUtxoVerdictsas flat arrays, before the changesetcallback.
PlatformWalletPersistenceHandlerkeeps them in the round's buffer.upsertUtxoRowwrites anobserved_spent/doomedoutput spent at creation, with no spender link. Any verdict vetoes theredelivery recovery clear (both the flag and the sweep stamp).
uncreditedchanges nothing else. Arolled-back round leaves no verdict behind.
log. Both can follow separately.
Kotlin-specific choices, same behaviour:
open round.
How Has This Been Tested?
On this branch (
v5.1-dev, rust-dashcoree4208c90, no #979):cargo test -p platform-wallet --libcargo test -p platform-wallet-ffi --libcargo test -p rs-unified-sdk-jnicargo clippy … -D warningscargo fmt --check--rerun-tasks)The Kotlin tests port
BornSpentTxoPersistTests(observed_spent / doomed / uncredited, the redelivery veto,round scope, rollback, and a restart over a reopened on-disk store that restores zero coins for a verdict-spent
row) and the relevant cases of
CoreTxoReconcileTests,CoreTxoReconcileShutdownTestsandCoreTxoReconcilePrivacyTests:separately;
and long hex or Base58 runs.
On an Android emulator (Pixel 9 AVD), with the same commits on the integration line, restored from seed and
then relaunched:
rows written spent at creation. The reconcile ran 2 s after SYNCED on the first launch and 1 s after on the
relaunch: 1,081 engine coins, 1,081 store rows, all unspent, zero mutations. 0 collateral burns left unspent.
rows written spent at creation. The reconcile ran 1 s after SYNCED on the first launch and on the relaunch: 779
engine coins, 779 store rows, all unspent, zero mutations.
unchanged and Part 1 fired the same way. (Transaction history is a different matter; that belongs to fix: same block core chain lock height #979.)
Breaking Changes
None. One additive bridge method with a no-op default, and one additional extension slot wired.
Checklist
🤖 Generated with Claude Code
PR Hygiene ·
da997b5/self-reviewedCargo.lock,packages/rs-unified-sdk-jni/Cargo.toml,packages/rs-unified-sdk-jni/src/persistence.rsand 1 more) — QuantumExplorer or shumkovkotlin-sdk— you own itWhen every merge requirement is met, the
PR Hygienecheck passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.