[Claudius-Maginificent]

TL;DR

When the blockchain replaces a few recent blocks, the wallet already corrects itself in memory. This PR makes the saved wallet follow, so the balance and history are still right after the app restarts.

User story

As a wallet user, I want my balance and transaction history to match the real chain after a short chain reorganization, so that I do not see coins that no longer exist, or miss coins that came back, the next time I open the app.

Scenario

Actual behavior

A payment arrives and is confirmed in a block. Shortly after, the network replaces that block with a different one. While the app stays open the wallet shows the right picture. After a restart it shows the old one: the payment from the replaced block is still listed as confirmed, coins it spent are still missing, and the wallet does not look again at the blocks that replaced the old ones.

Expected behavior

After a restart the wallet shows the same balance and history as a wallet that was never closed: transactions of replaced blocks are gone or back to unconfirmed, the coins they spent are available again, and scanning resumes from the point where the chain changed.

Detailed discussion

Issue being fixed or feature implemented

Fixes #5380. Stacked on #5150 (fix/pr-5126), which pins the rust-dashcore revision that emits WalletEvent::ChainTruncated; merge that first, then retarget this PR.

key-wallet rewinds its in-memory state on a fork (truncate_above) and reports what it did. #5307 mapped that event to an empty persistence round, because changeset merging and every backend only move heights upwards and none can delete the records of a dropped block. The stored wallet therefore stayed on the replaced branch.

What was done?

• Changeset. CoreChangeSet gains truncations: Vec<ChainTruncation> (height, removed txids, kept unconfirmed_txids, restored_outpoints). A backend applies them before the rest of the round. Merge reduces earlier content by an incoming truncation (records, coins and heights above the fork), so one merged round stores the same wallet as one round per step.
• Capability. PersistenceCapabilities::CORE_CHAIN_TRUNCATION (bit 15; bit 13 belongs to asset-lock operation IDs and bit 14 is reserved for CORE_HISTORY_RESTORE in fix(platform-wallet)!: replay recorded transaction history on load for every persister #5220). A persister that does not attest it is not handed the truncation; the rest of the round is stored and one error is logged. A truncation a persister rejected is offered again with the next round; a backend that keeps a failed round owns its truncations until they are applied.
• SQLite. apply_truncation runs inside the round's write transaction: it deletes the records of dropped blocks with their coins, returns InstantSend-locked ones to unconfirmed, releases the coins the removed records spent and lowers the stored heights to the fork. No schema change. When a fatal flush makes the store drop its buffer, it keeps that buffer's truncations and applies them ahead of the next write, each once.
• FFI. A new trailing, size-negotiated slot on_persist_wallet_changeset_truncations_fn in PersistenceCallbacksExtension delivers ChainTruncationFFI values before the other callbacks of the round. A transaction a same-round truncation keeps is projected to the host as InstantSend-locked, so the round's ordinary callbacks do not overwrite that state.
• JNI / Kotlin. NativePersistenceBridge.onWalletChangesetChainTruncated; the Room handler applies the truncation in the round's transaction.
• Swift. The SwiftData handler applies the truncation in the round's changeset. A removed row is set aside and deleted at the end of the round unless the same round stores the transaction again, because txid and outpoint are unique attributes and a delete followed by an insert of the same key in one save was not confirmed to be safe.
• Shared transaction rows (Swift, Kotlin). Those stores keep one transaction row for all wallets while rounds commit per wallet, so a truncation takes only the truncating wallet's coins, spend links, pending claims and account involvement; the shared row is deleted only when nothing references it.
• DashPay payments. A sent payment whose block was dropped returns from confirmed to pending. If the store rejected the round that carried that change, it is offered again with the wallet's next round.
• Spend fences. A coin a truncation hands back is fenced again until the undone spend is seen again, so it is not selected for a second spend while the first may still confirm. The fence is set where the wallet is truncated, under the wallet-manager write lock, because fork recovery truncates between two runs of the SPV client, when no event handler is listening.
• The InstantSend lock of a detected transaction is stored when the transaction is detected, so a fork after a restart keeps a locked transaction as unconfirmed instead of removing it.

Asset locks need no change: a truncation never goes below the best ChainLock, and their statuses depend on finality only.

Known limitations

• A persister without the capability keeps the replaced branch until it rescans. A rejected truncation is retried from memory only, so a restart or a panic of the committing thread loses it. A truncation the store cannot apply is retried ahead of every later round of that wallet, so that wallet's writes fail until it applies; it is never skipped. The retry also follows a failure whose outcome the backend could not vouch for; over a partly applied round it removes only that round's content above the fork, which the next start rescans. No in-tree backend can leave a round partly applied, and this convergence is reasoned from the code, not covered by a test.
• A payment returned to pending and not yet accepted by the store is held in memory only, so a restart before the store accepts it leaves the stored payment confirmed. It is offered to the store again after a rejected round, but given up when the round that carries it is refused a second time with a failure that is not transient; the stored payment then stays confirmed. On the Swift store every failure is reported as permanent, so such a row gets one retry.
• When a payment reversal a round reports itself is the cause of a refusal that is not transient, it is carried once and makes the next round fail as well before it is given up.
• A point read of a stored transaction (get_core_tx_record) returns the record as written and does not consult the stored InstantSend locks. A transaction locked after it was stored, or kept by a truncation, therefore reads as a mempool transaction there, while the loaded wallet holds it as InstantSend-locked; sent-payment history built from that read can show it as pending until it is mined. Coins and balances are not affected. This predates the PR for transactions locked after they were stored.
• If the identity that owns such a payment is removed and added again before the retry, the row is written into the new identity's payment history, where it shows as a pending sent payment.
• On the Swift store the pending-input lookup used when a truncation hands a coin back does not report a failed read. If that read fails while a swept-input tombstone exists for the coin, the tombstone survives and can later mark the restored coin spent in the store; the wallet itself still holds the coin. The same lookup serves the sweep path and predates the PR.
• The Swift host's changeset-end callback reports every failed round as a failed rollback, so all of them are classified as fatal and a Swift host never receives a transient verdict for a round in which a callback failed. This predates the PR; separating the cases needs new FFI return codes.
• A transaction row is shared by all wallets of a Swift or Kotlin store, so between two wallets' rounds its block fields can show the branch the other wallet last wrote; coins and balances are per wallet and are not affected.
• Under FlushMode::Manual (not the default; no in-tree host sets it) a fatal flush drops the buffered round. The truncation is kept and applied with the next write, but a sent payment the fork returned to pending is not, so its stored status can stay confirmed.
• The sent-payment reconcile pass confirms from the stored transaction record. If it reads a pre-fork record just before the truncation is stored and writes its confirmation just after, the payment the fork returned to pending is stored and shown as confirmed again; unless the transaction is mined again, nothing corrects it. After a rejected truncation round the same pass can instead leave the payment confirmed in memory and pending in the store until the next start.
• On the Kotlin store a coin spent by an InstantSend-locked transaction that is not in a block is not marked spent, and unconfirmed sends are not replayed at load. When the mining of such a transaction and the truncation of its block reach the store in one round, the coin reloads as unspent after a restart until the transaction is mined again and scanned. The Swift store replays unconfirmed sends at load and is not affected.
• BalanceUpdateHandler takes the post-truncation balance from the SPV event monitor (rs-platform-wallet/src/wallet/core/balance_handler.rs:76-78), which does not receive ChainTruncated during fork recovery, so the lock-free balance read by the UI is not refreshed until the next wallet event. This predates the PR.
• A send that selects a coin handed back by a truncation fails with InputMidBroadcast until the undone spend is seen again, at the latest until the wallet restarts.
• FFI hosts (SwiftData, Room): a fork that happens after a restart is not undone, because the FFI restore path does not replay ordinary transaction records, so key-wallet has nothing above the fork to rewind. The stored state is corrected by the next rescan. fix(platform-wallet)!: replay recorded transaction history on load for every persister #5220 adds the replay of recorded history on the FFI load path for SwiftData; the Kotlin host does not supply history yet. Hosts also store no InstantSend lock bytes (see the known limitations of fix(platform-wallet)!: replay recorded transaction history on load for every persister #5220); a kept transaction is stored in the InstantSend context without them.
• When the detection of an unconfirmed payment and its removal by a conflicting confirmed spend reach the store in one round, SQLite can keep the coin as unspent although the running wallet has it spent. This predates the PR and does not involve a fork; it is marked TODO(sweep-of-unstored-loser).
• After a conflict was settled and the winner's block is then dropped, key-wallet does not hand the coin back (upstream behaviour, related to dash-spv: build a reorg/disconnect pipeline so key-wallet can retract non-chainlocked observed-spent entries rust-dashcore#854); the store follows the wallet.

Out of scope

• Sent-payment status has three writers — the adapter's verdicts, the reversals it carries after a rejected round, and the reconcile pass — and each decides on its own. They need one shared rule for freshness (a writer does not act on evidence older than the last truncation that is not yet stored) and for identity lifecycle (a row belongs to the identity it was decided for, and is dropped when that identity is removed, also if it is added again).
• The point read of a stored transaction (get_core_tx_record) should resolve the stored InstantSend locks the way the full load does, so every reader sees a locked transaction as locked, whether it was locked after it was stored or kept by a truncation.

How Has This Been Tested?

• 304 targeted all-feature tests passed across wallet, storage, wallet FFI and unified JNI, including the 48-case merged-versus-stepwise persistence property test. Clippy passed for the same four packages with --all-targets --all-features --locked -- --no-deps -D warnings; formatting and whitespace checks passed.
• The truncation checkpoint regression failed before its fix and passes afterward. The checkpoint is bounded by the observed live height and fork height, remains subject to the normal epoch checks, and cannot override a later contact-history rewind.
• tests/sqlite_chain_truncation.rs drives a real WalletManager through the adapter into SqlitePersister, reopens the store and compares the loaded wallet with the one that never restarted, with one round per step and with one merged round: a spent coin restored when its spender is not mined again, a transaction stored once when mined again, heights lowered to the fork, an InstantSend-locked transaction kept unconfirmed (also when the fork follows a restart), and a store left untouched when a truncation cannot be applied.
• A property test stores generated sequences of blocks, receipts, spends, syncs and forks both ways and requires the same stored wallet. It does not generate conflicting spends.
• The new tests were shown to fail without the change; disabling the merge reduction fails every merged-round variant.
• Tests also cover a kept transaction projected to FFI hosts as InstantSend-locked, a truncation kept across a dropped buffer and applied once, a payment returned to pending offered again after a rejected round and given up once the store refuses it for good, a restored coin fenced by a truncation of a real manager with no event delivered, and, per host, two wallets sharing a transaction (removed, kept, and the shared row's lifetime).
• Not run locally: the Swift and Kotlin code and their tests (ChainTruncationPersistTests.swift, PlatformWalletPersistenceHandlerTest.kt). No Swift or Android toolchain was available; they run in CI only.

Breaking Changes

None for consensus, persisted data or the C layout. Rust source only: CoreChangeSet gains the public field truncations, PersistenceCallbacksExtension gains on_persist_wallet_changeset_truncations_fn, and PersistenceExtensionCallbacks gains wallet_changeset_truncations. Code that builds any of them with a full struct literal must set the new field; ..Default::default() keeps the previous behaviour for all three (PersistenceCallbacksExtension::default() leaves the new slot None). For C and Swift/Kotlin hosts the slot is trailing and size-negotiated, and the capability bit is additive.

Checklist

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
• I have made corresponding changes to the documentation if needed
• If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request

Lock state of transactions a truncation keeps

A truncation keeps only InstantSend-locked transactions. key-wallet demotes their records to Mempool and holds the lock only as a txid, so no lock payload exists to restore an InstantSend context. CoreChangeSet::rewind_to mirrors that and never fabricates a lock, because a fabricated lock would be persisted and later read back as a real one.

CoreChangeSet::is_unconfirmed_instant_send_locked decides whether such a record is unconfirmed and InstantSend-locked. The FFI changeset projection (WalletChangeSetFFI::from_changeset) only marshals that answer into the row context and the outputs' is_instantlocked flag. The SQLite store uses the same representation: a Mempool record blob plus its core_instant_locks row, which rehydration upgrades to InstantSend.

Hosts still apply the truncation's txids, unconfirmed_txids and restored_outpoints themselves. Emitting those as ordinary upserts needs upstream key-wallet (rust-dashcore) to keep the InstantSend lock payload of a record whose block a truncation drops; that is a follow-up.

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

🤖 Generated with Claude Code

https://claude.ai/code/session_01PSi8cX229kW2fmLxKW6ir4

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

• Source mode: Auto
• Repository: dashpay/platform
• Branch: fix/wallet-persist-chain-truncation
• File: .coderabbit.yaml
• Commit: c800f8a
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: ab8f98e3-c431-4efe-b90c-78b1c797a7d3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[thepastaclaw]

⛔ Final review complete — 3 blocking finding(s) (commit c800f8a) · triage: critical

[thepastaclaw]

Final validation — Phase 1 + Phase 2

Static verification at head 3a1aa90 confirms three in-scope persistence defects and one targeted test-coverage gap. These affect client-wallet persistence rather than consensus and are classified as suggestions under the supplied severity policy. No local builds or tests were run; the supplied CI snapshot reports successful Rust workspace, Swift, and Kotlin checks, with the dedicated Rust wallet job skipped.

🟡 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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This is a large, intricate change to persistent wallet state and coin restoration/spend fencing across Rust, SQLite, JNI, Kotlin, Swift, and FFI, directly affecting funds movement and coin selection in functions such as apply_truncation and spend-observer handling.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-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 verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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/rs-platform-wallet/src/changeset/changeset.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/changeset/changeset.rs:731-735: Preserve InstantSend state when projecting a merged truncation to FFI
  A merged block-observation/truncation round demotes retained transaction records and account slices to Mempool here, while separately preserving lock evidence and marking new_utxos InstantSend-locked. WalletChangeSetFFI::from_changeset does not project those fields: record_new_utxos_ffi derives its lock flag exclusively from the account record's context, and tx_record_to_ffi likewise emits context 0 for the demoted transaction. FFIPersister delivers the truncation callback first, then this ordinary changeset. Swift's upsertTransaction/upsertUtxo and Kotlin's transaction/TXO upserts therefore overwrite the truncation handler's context-1 and locked-output updates with context 0 and is_instantlocked=false. If the transaction was not previously stored, the truncation callback is a no-op and the ordinary callbacks create it unlocked. Separate rounds end with the truncation's locked state, so persistence now depends on batching and loses the retained transaction's finality classification. Preserve retained-lock evidence in the shared FFI projection without overriding a later block observation, and add a merged mining/truncation regression through FFIPersister. This does not require storing lock bytes and is distinct from the documented FFI restore limitation.
- [SUGGESTION] packages/rs-platform-wallet/src/changeset/changeset.rs:327-335: Cover the merged sweep-plus-truncation safety guarantee at the store layer
  This newly documented exception guarantees that a merged sweep/truncation can withhold a coin but cannot hand it out twice when the winner and funding transaction were both dropped, only the funding transaction is re-mined, and no surviving spender exists. should_release_a_restored_coin_from_a_sweep_recorded_before_the_truncation checks the rewritten release list, not the persisted result. The SQLite truncation suite's conflict scenario runs only per-step and explicitly defers merged coverage. Add a focused store-level test applying this merged shape through SQLite apply, then reload and assert the documented withheld end state. This pins the safety direction where sweep ordering and surviving-claim vetoes actually decide it, without requiring exact merged/per-step equality for the acknowledged exception.

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:1307-1309: Prevent another wallet's truncation from deleting already re-mined outputs
  These DAO deletions are global by txid, but core_bridge folds events into a BTreeMap<WalletId, WalletBatch> and commits each wallet's entire round separately. For a transaction T paying distinct outputs to wallets A and B, buffer its truncation and replacement-branch re-mining in both wallets' rounds. A's round deletes T and recreates A's valid output. B's round then deletes T and every output again, including A's newly recreated output, before recreating only B's account slices. Both rounds can succeed and advance their watermarks while A's coin is missing. Swift has the same ownership mismatch: vacateTransaction vacates every shared row.outputs entry and clears all involvedAccounts, while deleteVacatedRows deletes outputs not re-emitted by the current wallet. A txid-only removal is not idempotent across another wallet's intervening replacement writes. Make invalidation distinguish old-branch state from already-applied replacement state, or coordinate shared invalidations before replacement writes; wallet-scoped cleanup must also avoid cascading deletion of other-wallet rows. Add a two-wallet shared-transaction truncation/re-mining backlog regression for both hosts.

In `packages/rs-platform-wallet/src/changeset/core_bridge.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/changeset/core_bridge.rs:893-898: Preserve retry ownership when a backend later drops its buffered truncation
  This branch avoids duplicating a backend-held truncation, but also stops tracking it before durability is established. SqlitePersister restores the complete buffered changeset after SQLITE_BUSY, and its default store_transient_is_reissuable=false makes kept_by_backend true here. A later non-truncation store merges into that buffer. If the resulting flush fails fatally—for example, an unrelated incoming transaction update encounters a corrupt stored record during strict decoding—handle_flush_error drops the entire merged buffer, including the earlier truncation. On that call offered_truncations contains only the incoming round's truncations and is empty, so the adapter saves nothing for retry. Later successful writes leave the old branch's removed records in storage; freezing the existing watermark does not recreate the lost one-shot truncation or lower the old height to the fork. Retain explicit responsibility for backend-held truncations until their successful application is known, or have the backend preserve/return them when discarding a failed buffer. Do not blindly reissue while the backend still holds a copy. Add a SQLite regression covering transient retention, subsequent fatal buffer disposal, and successful recovery.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

The truncation persistence implementation addresses the four prior findings and correctly propagates truncations through the merge, storage, FFI, Kotlin, and Swift layers. Two in-scope retry-state issues remain: a rejected truncation round can lose its DashPay payment reversal, and unknown persistence outcomes are treated as permission to replay only the truncation. Static review only; CI still has Kotlin 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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This is a large, intricate change to persistent wallet storage and cross-language persistence boundaries that directly changes restoration and availability of coins after chain reorganizations, making funds movement and stored wallet state critical surfaces; specifically the SQLite truncation logic in packages/rs-platform-wallet-storage/src/sqlite/persister.rs and the corresponding Kotlin/Swift/FF
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-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; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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/rs-platform-wallet/src/changeset/core_bridge.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/changeset/core_bridge.rs:893-898: Preserve payment reversals when retrying a rejected truncation
  When a ChainTruncated event changes a stored Confirmed sent payment to Pending, sent_payment_verdicts mutates the in-memory entry and places the reversal only in the current WalletBatch's payment overlay. If store() rejects the round, this code retains only the CoreChangeSet truncations in unapplied_truncations and drops the overlay. A later round therefore retries the truncation without the payment reversal: the removed transaction and spent coin are corrected, but the persisted payment can remain Confirmed after restart. Retain the payment overlay together with the owed truncation, or otherwise rederive and persist the reversal when the truncation is retried.
- [SUGGESTION] packages/rs-platform-wallet/src/changeset/core_bridge.rs:893-898: Do not replay truncations after an unknown persistence outcome
  The branch treats every non-transient store error as permission to replay the truncation. That is not equivalent to knowing that the failed round was unapplied: FFIPersister reports a failed rollback from its changeset-end callback as Fatal because the round's disposition is unknown. If a failed round contained a truncation followed by replacement-branch records and some writes were applied before rollback failed, replaying only the truncation in a later round can remove those replacement records without recreating them. The persistence contract should return a disposition that distinguishes known-unapplied, backend-retained, and unknown outcomes; the adapter should replay only a known-unapplied truncation and require recovery for an unknown outcome.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Verified the complete changeset and reviewer claims against head 3d6aeca. Four in-scope correctness suggestions remain concerning spend-fence timing, buffered payment recovery, reconciliation ordering, and Android retained-spend restoration; five prior findings are fixed and the persistence-disposition distinction remains intentionally deferred. This was a static review: the supplied CI snapshot shows Kotlin checks passing and Rust workspace tests pending; Swift schema validation passed but does not establish behavioral-test success.

🟡 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: security-auditor); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 13: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This is a large, intricate change to wallet persistence and truncation handling that directly changes stored coin and transaction state during chain reorganizations in rs-platform-wallet-storage/src/sqlite/persister.rs and cross-language persistence handlers, affecting funds movement and recovery of spendable coins.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-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; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-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/rs-platform-wallet/src/wallet/core/spend_observer.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/core/spend_observer.rs:125-129: Re-arm restored-input fences before releasing the wallet mutation lock
  The restored coin becomes selectable before this handler necessarily installs its fence. PlatformWalletInfo::truncate_above delegates directly to core_wallet.truncate_above, while the pinned dash-spv dependency delivers WalletEvent callbacks through a separately spawned broadcast monitor, not synchronously inside the wallet mutation. After the wallet-manager write guard is released, a concurrent transaction build can select the restored input and pass in_broadcast_conflict before the monitor reaches ChainTruncated. A lagged monitor can also exit without delivering the event. Install the fence at the WalletInfoInterface::truncate_above mutation boundary, before exposing restored inputs, and coordinate spend observations so queued pre-fork events cannot retire the newly installed fence. Add a real-manager rewind regression that withholds event delivery and immediately attempts to build using the restored coin; the current test invokes the event handler directly and misses this interval.

In `packages/rs-platform-wallet-storage/src/sqlite/persister.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/persister.rs:1194-1201: Retain payment reversals with backend-owned truncations
  Fatal flush recovery retains core.truncations but discards the corresponding Confirmed-to-Pending payment overlay. In FlushMode::Manual, store returns Ok after buffering both, so the adapter no longer retains the reversal. If a subsequent flush or commit_writes fails fatally, this replacement PendingWrites contains only the truncations. After the error is repaired, recovery can delete the old-branch transaction and restore its inputs while leaving the authoritative identity blob's payment Confirmed. The fork event does not repeat, and reconcile_sent_payments only processes Pending entries, so reloading does not repair that verdict. Preserve the final reversal overlay with the retained truncations, respecting newer verdicts and identity lifecycle boundaries. Add a buffered acceptance → fatal flush → successful recovery → reload regression that verifies the payment status as well as core state.

In `packages/rs-platform-wallet/src/changeset/core_bridge.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/changeset/core_bridge.rs:744-748: Coordinate retried payment reversals with confirmation reconciliation
  The newer-wins merge only sees verdicts produced by this adapter, although reconcile_sent_payments writes the same payment independently. After a truncation changes the live payment to Pending and its store round fails, reconciliation can read SQLite's still-committed pre-fork confirmed transaction and successfully write the live payment back to Confirmed. The reconciliation path has no outstanding-truncation or sync-fault guard. If a later adapter round re-mines that transaction, sent_payment_verdicts emits no overlay because Confirmed + Final is explicitly a no-op. This merge then replays the retained Pending row over the newer reconciliation write, leaving live state Confirmed and persisted state Pending. Later reconciliation and finality events skip the already-Confirmed live entry. Coordinate reconciliation with outstanding truncations/reversals or apply a shared freshness check across payment writers. Exercise the actual reconciliation and verdict paths in a regression; supplying a newer Confirmed overlay directly does not cover this interleaving.

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:1324-1325: Persist retained transaction input debits in merged truncation rounds
  The kept-transaction updates preserve spent inputs only when an earlier round already marked them spent. Start with an unspent funding coin F persisted, then merge the mining of an InstantSend-locked transaction T spending F with the truncation of T's block before T reaches storage. CoreChangeSet::rewind_to demotes T to Mempool, and the FFI projection correctly sends it as context 1. These truncation updates initially affect no rows. The subsequent transaction and spent-outpoint callbacks both call linkSpender, which sets isSpent only for context >= CONTEXT_IN_BLOCK, so F remains unspent while T's locked change is stored. buildUtxoRestoreData also excludes linked inputs only for context >= CONTEXT_IN_BLOCK, and JNI supplies no unconfirmed outgoing replay records. Reload therefore restores both F and T's change, unlike separate mining/truncation rounds. Carry the round's retained-txid evidence into input debit processing after the records exist, or transport equivalent debit evidence. Add a merged retained-spend regression beginning with only F persisted and assert that reload excludes F.

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.

• Distinguish rollback success from unknown round disposition in the persistence ABI — Swift's endChangeset returns false after a successful rollback, and its C shim converts that to the same nonzero result FFIPersister interprets as rollback failure. The in-tree backends apply rounds atomically, so this does not establish the claimed partial-write defect here, but the ABI cannot express the distinction needed by an independently implemented host with a genuinely unknown outcome. The PR explicitly defers that coordinated contract change.
  • Follow-up: Track a separate persistence-disposition contract change covering Rust error semantics, FFI return codes, mobile callbacks, and recovery fault-injection tests.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Verified the complete PR diff at 307a662 and revalidated all 15 prior findings. Eight findings are fixed; three non-consensus correctness suggestions remain in the Kotlin retained-spend path, Swift truncation acknowledgment, and payment reconciliation ordering. Validation was static only: the supplied CI snapshot reports Kotlin build/tests successful, Rust workspace tests in progress, and the Swift frozen-schema check queued.

🟡 3 suggestion(s)

3 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — This is a large, intricate cross-language wallet-persistence change affecting stored chain state and coin availability, but it does not modify consensus, cryptography, funds movement, coin selection, or a storage migration.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-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)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer

🤖 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:1319-1323: Persist retained transaction input debits in merged truncation rounds
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865317)
  These updates preserve a retained spender's input debit only when an earlier round already stored it. Start with funding coin F stored unspent, then merge the mining of InstantSend-locked spender T with the truncation that retains T. The truncation executes before T exists in Room, and the subsequent FFI record arrives at context 1. Both transaction-input processing and the spent-outpoint callback call linkSpender, whose isSpent expression requires block context for an unstamped input. buildUtxoRestoreData likewise excludes linked inputs only for block context, and JNI supplies null/0 for unconfirmed outgoing replay records. Restart therefore restores F alongside T's locked change, whereas separate mining/truncation rounds leave F spent. The existing ordinary-unconfirmed restoration limitation explains the mechanism, but the new merged truncation path loses an observed mined debit and violates the retained-input contract. Preserve retained-spender evidence through subsequent input callbacks, or apply its debits after the rows materialize within the atomic round. Add a merged-round reload regression starting with only F persisted; general transaction-history restoration is not required for this repair.

In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- [SUGGESTION] packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift:1674-1676: Propagate pending-input lookup failures through the truncation callback
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4252358886)
  This required tombstone lookup cannot propagate failure to the truncation callback's catch. pendingInputRows uses try? for both fetch paths, substitutes an empty result, and registers the failed indexed lookup as fetched. If a detached swept-input tombstone exists and the read fails, restoreTruncatedCoin skips its deletion while the round can still save and acknowledge success to Rust. Rust then relinquishes the truncation, but a later successful drainPendingInputs can consume the surviving tombstone and mark the restored coin spent again. The helper's sweep behavior predates this PR, and the error withholds rather than invents a spendable coin; nevertheless, its new use here can acknowledge an unapplied restoration effect. Make this authoritative lookup throwing, retaining the saved/staged-row merge and caching only successful fetches. A checked path for truncation can be introduced without changing every existing sweep caller. Add failed-fetch coverage proving that the callback rejects the round and its mutations roll back.

In `packages/rs-platform-wallet/src/changeset/core_bridge.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/changeset/core_bridge.rs:745-750: Coordinate retried payment reversals with confirmation reconciliation
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865308)
  The newer-wins overlay merge orders adapter verdicts, not the independent reconciliation writer. sent_payment_verdicts changes the live entry from Confirmed to Pending and releases the manager lock before persistence. reconcile_sent_payments can then read the still-committed pre-fork record at payments.rs:758, allow the adapter to successfully commit the truncation and Pending overlay, and finally call confirm_sent_payment_by_txid using that old evidence. Its write-side check only requires the entry to remain Pending, which the reversal itself ensures. Reconciliation consequently persists Confirmed after the transaction was removed; both memory and disk remain wrong after restart because subsequent passes skip Confirmed entries. This ordering needs no failed store round. Although the wider shared-writer redesign is explicitly deferred, the newly introduced reversible confirmation edge needs an evidence-freshness check coordinated with the truncation commit. Add a regression pausing reconciliation between its record read and confirmation while a successful truncation round commits.

Out-of-scope follow-up suggestions (4)

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.

• Invalidate retained payment overlays across identity lifetimes — The bounded retry policy prevents an absent identity's reversal from indefinitely rejecting core persistence, but the retained overlay still has no identity-incarnation marker. Removing and re-adding the same identity before retry can therefore insert the old payment into its newly blank history. This remaining lifecycle case is explicitly excluded from the selected payment-writer work and is distinct from the fixed persistence wedge.
  • Follow-up: Track identity incarnation or lifecycle invalidation in a separate payment-owner change, with removal/re-addition coverage for retained overlays.
• Restore recorded history for mobile forks after restart — The mobile FFI restore surface does not replay ordinary confirmed transaction history, so key-wallet can lack the records needed to produce a meaningful truncation after restart. This restoration limitation predates the new transport and is explicitly separated into Swift history-restoration work in #5220 and an equivalent Kotlin/JNI follow-up.
  • Follow-up: Complete the separately scoped recorded-history restoration paths and test a restart followed by a fork on each host.
• Preserve retained-spend input debits when mining and truncation share a round — Deduplicated into prior finding 78ad35e85c2e. The mechanism is verified, but its canonical severity is suggestion under the project's policy for correctness defects outside consensus; the fresh lane's blocking classification is not retained.
  • Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.
• Resolve retained InstantSend finality for every storage reader — Outside the selected scope: get_tx_record returns the raw blob, while restore_recorded_transactions resolves separately stored locks. The same divergence occurs for ordinary post-write lock arrival without a fork. The author explicitly separates the shared reader repair; it is not an additional truncation-specific request.
  • Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

The truncation persistence design is largely correct and the previously reported retry, fencing, merge, and wallet-isolation issues are fixed. Four in-scope suggestions remain: the changelog omits the public Rust API break, Kotlin can restore a retained spend's input as unspent after a merged round, payment reconciliation can overwrite a successful truncation reversal with stale evidence, and stale InstantSend locks can survive a transaction being mined again. No consensus or security blocker was identified.

🟡 4 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🟡 Suggestion: Clear stale InstantSend locks when a transaction is mined again after truncation
packages/rs-platform-wallet/src/changeset/changeset.rs:871-875

A detected InstantSend transaction is stored in instant_locks_for_non_final_records, while a later BlockProcessed changeset emits the transaction as InBlock without removing that lock. rewind_to intentionally retains the lock when a truncation keeps the transaction in unconfirmed_txids, and Merge then extends the retained map with the later block changeset. The sequence detect → mine → truncate while retaining the transaction → mine again can therefore leave the old lock in the merged changeset. SQLite updates the transaction to its mined context but does not delete the separate lock row; on restart, rehydration replays the stale lock and can mark confirmed outputs InstantSend-locked. Treat an InBlock or InChainLockedBlock observation as a tombstone for an earlier non-final lock for the same txid, and add a persistence/restart regression for this sequence.

_{source: gpt-6.1-sol (phase2-reviewer: rust-quality)}

3 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This is a large, intricate change to wallet persistence that directly alters stored transaction and coin state during chain reorganizations, including funds-related restoration and deletion across Rust, SQLite, Kotlin, Swift, and FFI boundaries.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-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; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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/rs-platform-wallet/src/changeset/changeset.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/changeset/changeset.rs:871-875: Clear stale InstantSend locks when a transaction is mined again after truncation
  A detected InstantSend transaction is stored in `instant_locks_for_non_final_records`, while a later `BlockProcessed` changeset emits the transaction as `InBlock` without removing that lock. `rewind_to` intentionally retains the lock when a truncation keeps the transaction in `unconfirmed_txids`, and `Merge` then extends the retained map with the later block changeset. The sequence detect → mine → truncate while retaining the transaction → mine again can therefore leave the old lock in the merged changeset. SQLite updates the transaction to its mined context but does not delete the separate lock row; on restart, rehydration replays the stale lock and can mark confirmed outputs InstantSend-locked. Treat an `InBlock` or `InChainLockedBlock` observation as a tombstone for an earlier non-final lock for the same txid, and add a persistence/restart regression for this sequence.

In `CHANGELOG.md`:
- [SUGGESTION] CHANGELOG.md:42-48: Mark the public Rust struct additions as breaking changes
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4258098852)
  The PR title and description now disclose the breaking API change, but the durable changelog entry still presents chain-truncation persistence only as a normal platform-wallet change. `CoreChangeSet::truncations`, `PersistenceCallbacksExtension::on_persist_wallet_changeset_truncations_fn`, and `PersistenceExtensionCallbacks::wallet_changeset_truncations` are public Rust fields that break downstream exhaustive struct literals. Add a breaking-change note and the available `..Default::default()` migration guidance to the changelog entry.

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:1203-1214: Persist retained transaction input debits in merged truncation rounds
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865317)
  When an unspent funding coin F is already persisted and the detection, mining, and truncation of an InstantSend-locked spender T arrive in one round, the truncation callback runs before T's transaction and spent-outpoint callbacks. The later callbacks link F to T with InstantSend context, but `linkSpender` sets `isSpent` only for context at or above `CONTEXT_IN_BLOCK`. Kotlin's restore builder applies the same block-only threshold, and the JNI load path supplies no unconfirmed outgoing transactions to replay. After restart, both F and T's change can therefore be restored, whereas separate persistence rounds leave F spent. Carry retained-spend evidence through the round and mark the corresponding inputs consumed, or complete the native unconfirmed-spend replay path; add a merged-round reload regression beginning with only F persisted.

In `packages/rs-platform-wallet/src/changeset/core_bridge.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/changeset/core_bridge.rs:1561-1566: Coordinate retried payment reversals with confirmation reconciliation
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865308)
  The new `Confirmed` to `Pending` transition is applied to the in-memory payment before the persistence round commits. `reconcile_sent_payments` can concurrently read the still-persisted pre-fork transaction, then call `confirm_sent_payment_by_txid` after the truncation and its `Pending` overlay have committed. That mutation checks only that the entry is still a `Pending` sent payment, so it can write `Confirmed` again without a persistence failure or a newer chain observation. The payment then remains confirmed in memory and storage, and later reconciliation skips it. Apply confirmation evidence only when it is newer than the wallet's outstanding truncation generation, and cover the stale-read, truncation-commit, confirmation-write ordering.

[Claudius-Maginificent]

Re the suggestion that is not shown inline, "Clear stale InstantSend locks when a transaction is mined again after truncation" (rs-platform-wallet/src/changeset/changeset.rs:871-875): no change, the premise does not hold.

The row does stay, but it is not stale and the loaded wallet does not differ. An InstantSend lock is permanent for its transaction: key-wallet latches the coin's flag when the transaction is processed again in a block (key-wallet/src/managed_account/managed_core_funds_account.rs:474-486), so the wallet that never restarted also holds that coin as confirmed and InstantSend-locked. On load the stored lock marks the coin the same way (rs-platform-wallet-storage/src/sqlite/rehydrate.rs:419-421) and leaves a mined record's context alone (:490-494).

Checked with a test that runs detect → mine, with and without a fork that keeps the transaction and a second block, both merged into one round and one round per step: in all four cases the restored coin, balance and record height equal the in-memory ones, and one lock row is stored. The row outliving the confirmation is not specific to a merged round or to a fork; the SQLite backend has stored locks this way since #3968.

The row is also needed. A truncation keeps a transaction only if the wallet knows its lock (key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs:596-610), and after a restart the stored row is the only source of that. Deleting it when the transaction is mined would make a relaunched wallet drop, on a later fork, a transaction the running wallet keeps; should_keep_an_instant_locked_transaction_when_the_fork_follows_a_restart covers that case.

The other three suggestions of this round are carried forward and answered in their threads.

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

The head's SQLite capability withdrawal is fail-closed, but cumulative verification at cfa14d0 confirms six unresolved reorg-persistence defects; eight prior findings are fixed and two are withdrawn. The Kotlin CI failure is also attributable to the cumulative PR: its changed capability-mask assertion expects bit 14 although chain truncation uses bit 15. Verification was static, with current CI checks and the failed job log inspected; no local builds or tests were run.

🔴 7 blocking

6 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This is a large, intricate change to wallet persistence and coin-state restoration across Rust, SQLite, FFI, Kotlin, and Swift, directly affecting funds availability and transaction history.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (lane failed), 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; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt`:
- [BLOCKING] packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt:223: Use bit 15 in the Kotlin capability-mask expectation
  The cumulative PR changes this assertion from 0x0bbf to 0x4bbf, but the handler now adds CORE_CHAIN_TRUNCATION at bit 15, producing 0x8bbf. The current Kotlin SDK build + tests job fails this exact assertion, with 547 tests completed and one failure. Although neither Kotlin file changes in the final five-file increment, both the capability addition and this incorrect expected value are introduced by the PR relative to dae60456ec08feb6baa52f4155670988961b271b, so this is not unrelated validation debt. Update the expectation to the intended bit-15 mask and obtain a passing Kotlin CI run; do not move the capability to bit 14, which is reserved for history restoration.

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:1355-1356: Persist retained transaction input debits in merged truncation rounds
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865317)
  Starting with only funding coin F persisted, merge the mining of InstantSend-locked spender T and truncation of its block before T reaches storage. These truncation updates initially affect no T rows, and the subsequent ordinary callbacks store T at InstantSend context 1. Both input channels use linkSpender, which marks F spent only at block context or when already stamped; buildUtxoRestoreData likewise excludes linked inputs only at block context. JNI supplies null/0 for unconfirmed outgoing replay records at persistence.rs:2957-2958. Restart therefore restores F alongside T's change, exposing an already-consumed input as selectable; separate mining and truncation rounds do not have this result. Swift's outgoing replay avoids this particular failure, and #5220 explicitly excludes Android history restoration. Supply the existing Rust-owned outgoing replay contract through JNI, or transport authoritative debit evidence from Rust, and cover merged-round reload without introducing another Kotlin spendability rule.

In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- [BLOCKING] packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift:1770-1772: Propagate pending-input lookup failures through the truncation callback
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4252358886)
  restoreTruncatedCoin throws, but pendingInputRows suppresses both fetch failures with try? at lines 2724 and 2733; inside a round it also registers the failed lookup as a fetched empty set. If a detached swept-input tombstone exists, the callback can leave it behind while acknowledging the truncation successfully. A later drainPendingInputs applies that surviving tombstone and marks the restored coin spent again, after Rust has relinquished the correction. The helper predating this PR and also serving sweeps supports repairing the shared helper, not acknowledging an incomplete new truncation operation. Make required pending-input lookups throwing, retain their saved/staged-row merge semantics, and propagate errors through the affected callbacks so the round rolls back instead of caching a failed read. Add a detached-tombstone regression with a failed lookup.

In `packages/rs-platform-wallet/src/changeset/core_bridge.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/changeset/core_bridge.rs:1194-1199: Coordinate retried payment reversals with confirmation reconciliation
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865308)
  The adapter's newer-wins merge does not coordinate with reconciliation's independent writer. sent_payment_verdicts changes the live entry to Pending before committing; reconcile_sent_payments can then read the still-stored pre-fork confirmed record at wallet/identity/network/payments.rs:1043-1047. If the adapter commits the truncation and Pending overlay before confirm_sent_payment_by_txid runs, that function checks only Pending/Sent and persists Confirmed from the stale evidence. Memory and storage now permanently report confirmation for a dropped transaction, and restart does not repair it because reconciliation skips Confirmed entries. This requires no persistence failure. The opposite writer order also permits the retry overlay to disagree with live state. Establish a shared evidence-freshness rule in the Rust payment owner covering adapter verdicts, retained reversals and reconciliation, and test the actual read/truncation/confirmation interleaving rather than only overlay merging.
- [BLOCKING] packages/rs-platform-wallet/src/changeset/core_bridge.rs:1195-1198: Cancel retained payment reversals when their owning identity is removed
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4252358882)
  The permanent-refusal fallback bounds retries for an absent identity, but the adapter-private reversal cache has no identity-incarnation ownership. IdentityManager::remove_identity explicitly defines removal as terminal in memory and storage, with re-addition starting blank. Remove and re-add that identity before this merge, and the old Pending payment overlay is accepted into the replacement identity's durable history even though its live payment collection is blank. No permanent refusal occurs, so the new fallback cannot prevent this stale successful write. Invalidate retained reversals when their owning identity lifecycle ends, or carry owner-issued lifecycle evidence validated by the shared Rust identity/payment owner. Cover removal followed by re-addition before retry, independently of the still-owed core truncation.

In `packages/rs-platform-wallet-storage/src/sqlite/persister.rs`:
- [BLOCKING] packages/rs-platform-wallet-storage/src/sqlite/persister.rs:1193-1202: Retain payment reversals with backend-owned truncations
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865297)
  Fatal recovery preserves only core.truncations and drops their accepted Confirmed-to-Pending payment updates. With the public FlushMode::Manual, store has already returned Ok after buffering both, so the adapter no longer retains the reversal. A fatal flush followed by successful recovery can consequently correct the core transaction and coin state while leaving the authoritative identity payment Confirmed; the fork event does not repeat, and reconciliation skips that status after reload. Manual not being selected by an in-tree host limits exposure but does not make the newly introduced selective recovery coherent. Retaining every generic Pending row would indeed be unsafe because some belong to caller-rolled-back sends. Preserve explicit reorg-reversal provenance alongside backend-owned truncations, with newer-verdict and lifecycle handling, or reject this unsupported buffered combination. Add an acceptance, fatal-flush, recovery and reload regression that checks payment status as well as core state.

In `packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs`:
- [BLOCKING] packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs:493-500: Resolve retained InstantSend finality for every storage reader
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4252965583)
  This rewrites a retained transaction's record_blob to Mempool while preserving its InstantSend lock. Full loading resolves that lock in restore_recorded_transactions, but SqlitePersister::get_core_tx_record delegates to get_tx_record and returns the decoded blob without lock resolution. Payment-history reconstruction can therefore persist a Pending entry for a still-final payment, and reconciliation cannot confirm that entry until another mined record arrives, although full wallet restoration recognizes the transaction as locked. Existing Confirmed entries are not directly downgraded by this reader. An equivalent late-lock inconsistency predating the PR does not eliminate the new deterministic route from previously mined records into this state. Share validated lock/context resolution between point reads and full restoration, preserving the transaction-body, txid and typed-height checks, and add a post-truncation point-read regression.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

The chain-truncation persistence design is broadly wired correctly, and most previously reported issues have been fixed. However, the Kotlin test suite still fails because its capability-mask expectation omits the newly advertised bit 15, and several persistence consistency issues remain in truncation, payment reconciliation, and retry handling.

🔴 1 blocking | 🟡 7 suggestion(s)

8 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — This is a large, intricate cross-language wallet-persistence change affecting chain truncation and coin state, but it does not itself modify consensus, funds movement or coin selection, cryptography, or a storage migration.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer

🤖 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/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt`:
- [BLOCKING] packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt:223: Use bit 15 in the Kotlin capability-mask expectation
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4341246219)
  `PlatformWalletPersistenceHandler.persistenceCapabilitiesBits()` now includes `CAPABILITY_CORE_CHAIN_TRUNCATION`, which is `0x8000` (bit 15), while bit 14 remains reserved. The assertion still expects `0x4bbfL`, so the Kotlin SDK test fails on this head; the actual advertised mask is `0x8bbfL`. Update the expectation, preferably by composing it from the shared constants rather than maintaining another literal.

In `packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs:483-500: Resolve retained InstantSend finality for every storage reader
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4252965583)
  When a truncation keeps a transaction, `apply_truncation` rewrites its stored record to `Mempool` but intentionally retains the corresponding row in `core_instant_locks`. Full wallet rehydration upgrades that record using the lock table, but `get_core_tx_record` delegates to the raw record reader and does not resolve the lock. Sent-payment reconstruction and reconciliation can therefore observe the same retained transaction as pending while a fully loaded wallet observes it as InstantSend-locked. Move the lock/context resolution into the shared point-read path so all readers use the same authoritative state.

In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- [SUGGESTION] packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift:1763-1772: Propagate pending-input lookup failures through the truncation callback
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4252358886)
  `restoreTruncatedCoin` is part of the authoritative truncation callback and its successful return allows Rust to relinquish the one-shot truncation. It nevertheless calls `pendingInputRows`, which converts a SwiftData fetch failure into an empty result and caches that result as a successful lookup. If a swept-input tombstone exists but cannot be read, the callback can report success without deleting it; a later tombstone drain can then mark the restored coin spent again, while the truncation is no longer retried. Use a throwing lookup for this truncation path and leave failed reads uncached so the surrounding transaction fails and the truncation remains retryable.

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:1206-1245: Persist retained transaction input debits in merged truncation rounds
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865317)
  In a merged round where an InstantSend transaction is mined and its block is then truncated, the retained transaction is projected with InstantSend context before `linkSpender` processes its input. `linkSpender` marks the funding TXO spent only when `spenderContext >= CONTEXT_IN_BLOCK`, so the funding coin can remain unspent even though the retained transaction's locked output is persisted. Kotlin reload does not replay unconfirmed outgoing transactions, allowing both the funding coin and the retained transaction's output to reappear after restart. Carry retained-transaction input-debit evidence through the round or mark those inputs spent independently of block context.

In `packages/rs-platform-wallet/src/changeset/core_bridge.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/changeset/core_bridge.rs:1194-1208: Coordinate retried payment reversals with confirmation reconciliation
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865308)
  The adapter merges carried payment reversals only with overlays produced by the adapter itself. `reconcile_sent_payments` can independently read the still-committed pre-truncation transaction and write the payment back to `Confirmed` after the truncation has set it to `Pending`; the reverse ordering can also leave memory and storage disagreeing. In particular, reconciliation can confirm a transaction that is no longer in a block, after which later reconciliation skips the already-confirmed entry. All payment writers need a shared freshness rule that treats an outstanding truncation or reversal as newer than a pre-truncation storage read.
- [SUGGESTION] packages/rs-platform-wallet/src/changeset/core_bridge.rs:1194-1199: Cancel retained payment reversals when their owning identity is removed
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4252358882)
  `unstored_reversals` is carried by wallet and payment identity but has no identity-generation or lifecycle check. If a truncation round is rejected, the identity is removed, and the same identity is re-added before retry, the retained reversal can be written into the new identity's payment history. The permanent-refusal path prevents an indefinite wedge, but it does not invalidate a reversal whose original owner no longer exists. Retain lifecycle ownership with the reversal or discard it when identity removal succeeds.
- [SUGGESTION] packages/rs-platform-wallet/src/changeset/core_bridge.rs:1357-1371: Do not replay truncations after an unknown persistence outcome
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251135886)
  Every non-transient store failure that is not explicitly retained by the backend places the truncation in `unapplied_truncations`, including a failed FFI round whose rollback disposition is unknown. If a custom host partially applies replacement-branch writes before reporting failure, a later truncation-only replay can remove those replacement rows without recreating them. The persistence contract should distinguish known-unapplied, backend-retained, and unknown outcomes; replay should be limited to a disposition that guarantees the original round was not durably applied, while an unknown outcome requires a defined recovery path.

In `packages/rs-platform-wallet-storage/src/sqlite/persister.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/persister.rs:1194-1203: Retain payment reversals with backend-owned truncations
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865297)
  With `FlushMode::Manual`, `store()` can acknowledge a round after buffering both the truncation and its Confirmed-to-Pending payment overlay. If a later flush fatally drops that buffer, the recovery code retains only `CoreChangeSet.truncations`; the next successful write can therefore remove the old-branch transaction and restore its inputs while leaving the authoritative payment `Confirmed`. The default mode does not exercise this path, but the public persister configuration permits it. Recovery must retain the associated payment overlay with the backend-owned truncation, with the same newer-wins and identity-lifecycle rules used by the adapter.

[Claudius-Maginificent]

Pushed 7286da481a. The decision whether a record kept by a truncation is unconfirmed and InstantSend-locked now lives in CoreChangeSet::is_unconfirmed_instant_send_locked. The FFI only marshals the answer; its output is byte-identical. See the new "Lock state of transactions a truncation keeps" section in the description.

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

The truncation persistence implementation has several remaining consistency gaps across SQLite, Swift, Kotlin, and payment reconciliation. The Kotlin capability assertion is definitely incorrect and will fail when its queued CI job runs; the other findings can leave persisted wallet state or payment history inconsistent after truncation, retry, restart, or identity lifecycle changes.

🔴 1 blocking | 🟡 7 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🟡 Suggestion: Resolve retained InstantSend finality for every storage reader
packages/rs-platform-wallet-storage/src/sqlite/persister.rs:1662-1676

apply_truncation rewrites a retained transaction's stored record to Mempool while leaving its valid core_instant_locks row intact. Full wallet rehydration resolves that sidecar, but get_core_tx_record delegates directly to the raw record reader and returns the Mempool context. Sent-payment reconstruction and reconciliation can therefore see the transaction as pending while a fully loaded wallet sees it as InstantSend-locked and final. Share the lock/context resolver used by full replay with this point-read path while retaining the existing strict txid and height checks.

_{source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality); gpt-6.1-sol (phase2-reviewer: general, architecture-layering, ffi-engineer)}

7 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This is a large, intricate change to wallet persistence that directly rewrites stored coin and transaction state during chain reorganizations, affecting funds and balance correctness, especially in packages/rs-platform-wallet-storage/src/sqlite/persister.rs and packages/rs-platform-wallet/src/changeset/changeset.rs.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), 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; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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/rs-platform-wallet-storage/src/sqlite/persister.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/persister.rs:1662-1676: Resolve retained InstantSend finality for every storage reader
  apply_truncation rewrites a retained transaction's stored record to Mempool while leaving its valid core_instant_locks row intact. Full wallet rehydration resolves that sidecar, but get_core_tx_record delegates directly to the raw record reader and returns the Mempool context. Sent-payment reconstruction and reconciliation can therefore see the transaction as pending while a fully loaded wallet sees it as InstantSend-locked and final. Share the lock/context resolver used by full replay with this point-read path while retaining the existing strict txid and height checks.
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/persister.rs:1194-1204: Retain payment reversals with backend-owned truncations
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865297)
  In FlushMode::Manual, store accepts the round into the SQLite buffer, so the adapter releases the Confirmed-to-Pending payment overlay. If a later fatal flush drops that buffer, the recovery code extracts and restores only the core truncations. The old transaction and spent coins can then be removed while the authoritative payment row remains Confirmed, with no later fork event required to rederive the reversal. Preserve the payment overlay with backend-owned truncations or return it to the adapter when the buffer is discarded.

In `packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt`:
- [BLOCKING] packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt:223: Use bit 15 in the Kotlin capability-mask expectation
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4341246219)
  The handler advertises CORE_CHAIN_TRUNCATION at bit 15 (0x8000), so the complete capability mask is 0x8bbf. This assertion still expects 0x4bbf, which sets reserved bit 14 instead, and therefore fails against the production handler and does not protect the intended capability assignment.

In `CHANGELOG.md`:
- [SUGGESTION] CHANGELOG.md:65-78: Mark the public Rust struct additions as breaking changes
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4258098852)
  The PR title and description now identify the breaking change, but the durable changelog entry only describes chain-truncation behavior. It does not tell downstream Rust users that adding public fields to CoreChangeSet, PersistenceCallbacksExtension, and PersistenceExtensionCallbacks breaks exhaustive struct literals or identify the available migration using defaults where applicable. Add that source-level API migration note to the changelog.

In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- [SUGGESTION] packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift:1763-1773: Propagate pending-input lookup failures through the truncation callback
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4252358886)
  restoreTruncatedCoin is throwing, but its swept-tombstone lookup calls pendingInputRows, which converts a SwiftData fetch failure into an empty result and caches it as successfully fetched. If a tombstone exists and that read fails, the callback can acknowledge the truncation without deleting the tombstone; Rust then retires the one-shot truncation and a later sweep can mark the restored coin spent again. Use a throwing lookup for this subtractive callback and do not cache failed reads.

In `packages/rs-platform-wallet/src/changeset/core_bridge.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/changeset/core_bridge.rs:1377-1384: Cancel retained payment reversals when their owning identity is removed
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4252358882)
  The retained reversal cache is keyed by wallet and payment, but it has no identity-generation marker. If a reversal survives a rejected truncation, the identity is removed, and the same identity is added again before the retry succeeds, the old reversal can be written into the replacement identity's payment history. Track identity lifecycle or generation with retained reversals and discard rows belonging to a removed generation before retrying them.
- [SUGGESTION] packages/rs-platform-wallet/src/changeset/core_bridge.rs:1194-1199: Coordinate retried payment reversals with confirmation reconciliation
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865308)
  The adapter changes a confirmed payment back to Pending before storing the truncation, while reconcile_sent_payments independently reads the still-committed pre-fork transaction. It can read that stale record, wait for the truncation to be stored, and then confirm the live entry after the adapter has released its lock. The result is a payment that is again Confirmed in memory and storage even though its transaction left the chain, and later reconciliation skips it because it only scans Pending entries. Payment reconciliation needs a freshness barrier tied to outstanding truncations or reversals.

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:1242-1245: Persist retained transaction input debits in merged truncation rounds
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865317)
  In a merged mining-plus-truncation round, the retained transaction is delivered with InstantSend context 1. linkSpender sets isSpent only for an in-block context (2 or greater), and the Kotlin restore path does not replay unconfirmed outgoing transactions. Starting with only the funding coin persisted, the kept transaction's output can therefore be stored while its input remains reloadable after restart. Carry explicit retained-spend evidence through the persistence projection or mark those inputs spent when the retained transaction is stored.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

The truncation pipeline is generally wired correctly, and the corrected Kotlin capability expectation now matches bit 15. However, the current head still has three blocking wallet-state defects: FFI restoration cannot reconstruct ordinary transaction history for post-restart truncations, Kotlin can reload a retained InstantSend spend's funding coin as spendable, and shared mobile transaction rows can be globally demoted by another wallet's truncation. Several additional persistence and payment-consistency issues remain as suggestions. All completed CI checks passed; the compatible runner selection check was still queued at the captured time.

🔴 3 blocking | 🟡 7 suggestion(s)

2 finding(s) not shown inline (the lines are not part of this PR's diff)

🔴 Blocking: Make truncation persistence work after FFI wallet restoration
packages/rs-platform-wallet-ffi/src/persistence.rs:6043-6071

The FFI restore path rebuilds UTXOs and only selectively reconstructs special transaction records such as unresolved asset-lock and provider transactions; it does not restore ordinary persisted transaction records into the key-wallet transaction maps. After a Swift or Kotlin wallet restarts, a later chain truncation therefore has no ordinary transaction history for key-wallet to rewind, so the emitted truncation contains no corresponding removed transaction IDs or restored outpoints and the persistence callback becomes a no-op. The stored wallet remains on the replaced branch until a later rescan, contradicting this PR's primary restart-after-reorg behavior. Restore the ordinary core transaction history before sync, or add an equivalent canonical reconciliation owned by the Rust wallet layer.

_{source: gpt-6.1-sol (phase2-reviewer: architecture-layering)}

🟡 Suggestion: Resolve retained InstantSend finality for every storage reader
packages/rs-platform-wallet-storage/src/sqlite/persister.rs:1662-1676

get_core_tx_record still delegates to the raw schema::core_state::get_tx_record reader and does not consult core_instant_locks. A truncation intentionally rewrites a kept transaction's blob to Mempool while preserving its InstantSend lock row; full wallet rehydration upgrades that combination back to InstantSend, but point readers return Pending. Sent-payment reconstruction and reconciliation can therefore disagree with the fully loaded wallet. Share the lock-aware context resolution between point reads and full rehydration.

_{source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality); gpt-6.1-sol (phase2-reviewer: general, architecture-layering, rust-quality, security-auditor)}

7 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — This is a large, intricate cross-language wallet-persistence change affecting chain truncation, UTXO state, and multiple storage backends, but it does not itself implement funds movement, coin selection, cryptography, consensus, or a storage migration.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer

🤖 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/rs-platform-wallet-ffi/src/persistence.rs`:
- [BLOCKING] packages/rs-platform-wallet-ffi/src/persistence.rs:6043-6071: Make truncation persistence work after FFI wallet restoration
  The FFI restore path rebuilds UTXOs and only selectively reconstructs special transaction records such as unresolved asset-lock and provider transactions; it does not restore ordinary persisted transaction records into the key-wallet transaction maps. After a Swift or Kotlin wallet restarts, a later chain truncation therefore has no ordinary transaction history for key-wallet to rewind, so the emitted truncation contains no corresponding removed transaction IDs or restored outpoints and the persistence callback becomes a no-op. The stored wallet remains on the replaced branch until a later rescan, contradicting this PR's primary restart-after-reorg behavior. Restore the ordinary core transaction history before sync, or add an equivalent canonical reconciliation owned by the Rust wallet layer.

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:1352-1356: Do not globally demote a shared transaction row from one wallet's truncation
  `TransactionEntity` is shared across wallets, but `unconfirmByTxids` updates the row globally without a wallet predicate. If wallet B has already persisted the transaction on its replacement branch and wallet A later processes a truncation retaining the same transaction, A's callback clears the shared block fields and changes the context to InstantSend for B as well, while B's replacement-branch TXOs remain confirmed. Swift has the same shared-row mutation through `unconfirmTransaction`. This leaves another wallet's persisted transaction history inconsistent with its own coins and causes the primary post-reorg history guarantee to depend on callback ordering. Demote the shared row only when no other wallet has a confirmed representation, or store confirmation context per wallet.
- [BLOCKING] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt:1236-1248: Persist retained transaction input debits in merged truncation rounds
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865317)
  In a merged round that mines an InstantSend-locked transaction and then truncates its block, the retained transaction is delivered with context 1. `linkSpender` marks an input spent only for an in-block spender, so the funding TXO remains unspent even though the retained transaction's locked outputs are persisted. `buildUtxoRestoreData` applies the same in-block-only suppression and the Kotlin JNI restore path does not replay unconfirmed outgoing transactions. Starting with only the funding coin persisted, reopening the Room store can therefore restore both the funding coin and the retained transaction's change, allowing the same outpoint to appear spendable twice. Carry retained-spender evidence through the truncation round or replay the retained unconfirmed spend during Kotlin restoration.

In `packages/rs-platform-wallet-storage/src/sqlite/persister.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/persister.rs:1662-1676: Resolve retained InstantSend finality for every storage reader
  `get_core_tx_record` still delegates to the raw `schema::core_state::get_tx_record` reader and does not consult `core_instant_locks`. A truncation intentionally rewrites a kept transaction's blob to Mempool while preserving its InstantSend lock row; full wallet rehydration upgrades that combination back to InstantSend, but point readers return Pending. Sent-payment reconstruction and reconciliation can therefore disagree with the fully loaded wallet. Share the lock-aware context resolution between point reads and full rehydration.
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/persister.rs:1164-1204: Retain payment reversals with backend-owned truncations
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865297)
  In `FlushMode::Manual`, `store()` can buffer both a truncation and its Confirmed-to-Pending payment overlay, so the adapter relinquishes the reversal. If a later fatal flush drops that buffer, `handle_flush_error` retains only the truncations. Recovery can then remove the old-branch transaction and restore its inputs while the durable payment row remains Confirmed; the fork event need not recur and reconciliation skips already-confirmed entries. Backend-owned truncation retry state must retain the corresponding payment overlay with correct newer-verdict and identity-lifecycle handling.

In `CHANGELOG.md`:
- [SUGGESTION] CHANGELOG.md:65-78: Mark the public Rust struct additions as breaking changes
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4258098852)
  The title and PR description now identify the breaking change, but the durable changelog entry still does not mention that `CoreChangeSet`, `PersistenceCallbacksExtension`, and `PersistenceExtensionCallbacks` gained public fields. Downstream callers using exhaustive Rust struct literals must initialize those fields or migrate to `..Default::default()` where available. Record that source-compatibility migration in the changelog rather than relying only on PR metadata.

In `packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs:483-503: Resolve retained InstantSend finality for every storage reader
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4252965583)
  `apply_truncation` deliberately stores a retained transaction as a Mempool record while leaving its valid `core_instant_locks` row intact. That split representation is safe only when every reader performs the lock-aware upgrade; the schema point-read path does not, so it returns a different finality context from full replay. Move the shared resolver to the storage boundary while retaining the existing strict txid and block-integrity checks.

In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- [SUGGESTION] packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift:1763-1773: Propagate pending-input lookup failures through the truncation callback
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4252358886)
  `restoreTruncatedCoin` is part of the authoritative truncation callback, but `pendingInputRows` converts SwiftData fetch failures into an empty result and registers that result as successfully fetched. If a swept-input tombstone exists and the lookup fails, the callback can still return success, Rust relinquishes the one-shot truncation, and a later pending-input drain can mark the restored coin spent again. Use a throwing lookup for this subtractive path and do not cache failed reads; the enclosing changeset must fail and remain retryable.

In `packages/rs-platform-wallet/src/changeset/core_bridge.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/changeset/core_bridge.rs:1194-1199: Coordinate retried payment reversals with confirmation reconciliation
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865308)
  The truncation reversal overlay and `reconcile_sent_payments` remain independent writers. Reconciliation can read the still-committed pre-fork transaction after the live payment has been changed to Pending, then write Confirmed after the truncation round commits. Because a later Confirmed/final observation can produce no reversal, both in-memory and persisted payment state can remain Confirmed for a transaction no longer in a block. Payment writers need a shared freshness barrier tied to outstanding truncations and their retained reversals.
- [SUGGESTION] packages/rs-platform-wallet/src/changeset/core_bridge.rs:1353-1371: Do not replay truncations after an unknown persistence outcome
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251135886)
  The adapter reoffers every truncation after a non-transient persistence failure unless the backend explicitly reports that it retained the round. The FFI changeset-end contract can report failure after either a successful rollback or an unknown rollback outcome. If a host partially applied replacement-branch callbacks before rollback failed, replaying only the truncation can delete those replacement rows without recreating them. The persistence contract needs distinct known-unapplied, backend-retained, and unknown dispositions, with replay restricted to the known-unapplied case.

[thepastaclaw]

🔴 Blocking: Do not globally demote a shared transaction row from one wallet's truncation

TransactionEntity is shared across wallets, but unconfirmByTxids updates the row globally without a wallet predicate. If wallet B has already persisted the transaction on its replacement branch and wallet A later processes a truncation retaining the same transaction, A's callback clears the shared block fields and changes the context to InstantSend for B as well, while B's replacement-branch TXOs remain confirmed. Swift has the same shared-row mutation through unconfirmTransaction. This leaves another wallet's persisted transaction history inconsistent with its own coins and causes the primary post-reorg history guarantee to depend on callback ordering. Demote the shared row only when no other wallet has a confirmed representation, or store confirmation context per wallet.

_{source: gpt-6.1-sol (phase2-reviewer: ffi-engineer)}

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

At c800f8a, the truncation ordering, restored-input fencing, callback projection, and backend retry fixes are present. Three blocking gaps remain: incomplete post-restore truncation support across mobile hosts, Kotlin reload exposing inputs consumed by retained transactions, and reconciliation durably overwriting payment reversals with stale confirmations. This was a static verification; current GitHub checks pass, including Kotlin build/tests, but the available CI evidence does not establish Rust or Swift test execution.

🔴 3 blocking | 🟡 5 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🔴 Blocking: Make truncation persistence work after FFI wallet restoration
packages/rs-platform-wallet-ffi/src/persistence.rs:6046-6052

FFI restoration inserts projected UTXOs but leaves ordinary confirmed transaction history and spend-undo state absent; only selected special records and unconfirmed outgoing sends are restored. In the pinned key-wallet source, truncate_above selects abandoned transactions from the transaction maps and restores inputs from the corresponding spend state, rather than selecting restored coins by height. A fork after loading a mobile wallet can therefore lower its cursor while emitting no removal for an abandoned receipt or no restoration for an ordinary confirmed spend. The new callback acknowledges insufficient evidence and cannot make that snapshot follow the fork. Shared Rust history replay is the correct owner, not an independent reconstruction inside FFI or the hosts. The linked #5220 tracks that replay and the Swift producer, but explicitly excludes Android history transport, so it does not cover the whole supported-host prerequisite. Complete or dependency-gate the restore path, or require recovery/rescan for affected snapshots, and add load-then-fork coverage. A reasonable deferral of this pre-existing prerequisite also needs tracking that explicitly covers the Android producer.

_{source: gpt-6.1-sol (phase2-reviewer: general, rust-quality)}

7 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The intricate cross-backend change directly alters spendable-coin eligibility through restored-outpoint handling in packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs::apply_truncation and wallet generation fencing, meeting the coin-selection critical-surface bar.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), 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; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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/rs-platform-wallet-ffi/src/persistence.rs`:
- [BLOCKING] packages/rs-platform-wallet-ffi/src/persistence.rs:6046-6052: Make truncation persistence work after FFI wallet restoration
  FFI restoration inserts projected UTXOs but leaves ordinary confirmed transaction history and spend-undo state absent; only selected special records and unconfirmed outgoing sends are restored. In the pinned key-wallet source, truncate_above selects abandoned transactions from the transaction maps and restores inputs from the corresponding spend state, rather than selecting restored coins by height. A fork after loading a mobile wallet can therefore lower its cursor while emitting no removal for an abandoned receipt or no restoration for an ordinary confirmed spend. The new callback acknowledges insufficient evidence and cannot make that snapshot follow the fork. Shared Rust history replay is the correct owner, not an independent reconstruction inside FFI or the hosts. The linked #5220 tracks that replay and the Swift producer, but explicitly excludes Android history transport, so it does not cover the whole supported-host prerequisite. Complete or dependency-gate the restore path, or require recovery/rescan for affected snapshots, and add load-then-fork coverage. A reasonable deferral of this pre-existing prerequisite also needs tracking that explicitly covers the Android producer.

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:1355-1356: Persist retained transaction input debits in merged truncation rounds
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865317)
  Start with funding coin F persisted as unspent, then merge mining its InstantSend-locked spender T with truncation of T's block before T reaches storage. The truncation finds no T row; the following ordinary callbacks insert T at context 1. Both input channels use linkSpender, which marks F spent only for an in-block context or an existing spent/tombstone flag, so F remains unspent alongside T's locked change. buildUtxoRestoreData also excludes linked inputs only for in-block spenders, and JNI supplies null/0 for unconfirmed outgoing replay records. Restart therefore restores F even though the retained transaction still consumes it. Separate mining/truncation rounds preserve the debit, making this a new batching-dependent route into the existing unmined-send weakness. Swift avoids this particular reload defect by supplying outgoing records for Rust replay. Forward the required records/finality evidence into Rust-owned replay, or transport an authoritative debit projection, and add a merged-round reload regression starting with only F persisted; do not introduce a separate Kotlin spending policy.
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt:1355-1356: Do not globally demote a shared transaction row from one wallet's truncation
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4411730848)
  The wallet-scoped output changes protect other wallets' coins, but unconfirmByTxids still unconditionally clears the globally shared transaction row's block fields. If wallet A commits T re-mined on the replacement branch and wallet B subsequently commits only its earlier truncation retaining T, B replaces A's newer block observation with context 1 and height 0. onGetCoreTxRecord ignores walletId and reads this shared row, so the stale overwrite is observable and survives restart until another T observation repairs it. Swift's unconfirmTransaction performs the equivalent shared-row mutation before filtering outputs by wallet. Both two-wallet test fixtures re-mine T in B's same round, masking the state after B's truncation-only commit. Preserve observation ownership or carry ordering/provenance from the Rust changeset owner; host arrival order cannot establish branch freshness. Add a test that stops after A's replacement write followed by B's truncation-only commit and checks the transaction reader. This is narrower than the repaired cross-wallet coin-loss defect and does not justify blocking severity.

In `packages/rs-platform-wallet/src/changeset/core_bridge.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/changeset/core_bridge.rs:1194-1199: Coordinate retried payment reversals with confirmation reconciliation
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865308)
  The overlay merge orders adapter-produced verdicts, but not the independent reconciliation writer. sent_payment_verdicts changes the live payment to Pending before persistence. reconcile_sent_payments can then read the still-stored pre-fork confirmed record at payments.rs:1059-1074, let the adapter successfully commit the truncation and Pending overlay, and subsequently call confirm_sent_payment_by_txid. That function rechecks only Pending/Sent under the manager lock, not whether its transaction evidence survived the truncation, and persists Confirmed. Both memory and disk can consequently remain Confirmed for a dropped transaction; restart does not repair it because reconciliation skips Confirmed entries. This ordering requires no failed store round, so the retry fixes do not close it. Introduce a shared Rust-owned freshness boundary covering the evidence read and verdict commit for reconciliation, adapter verdicts, and retained reversals, and test the actual read-before-truncation/write-after-truncation interleaving. The documented limitation is not a tracked deferral.
- [SUGGESTION] packages/rs-platform-wallet/src/changeset/core_bridge.rs:1195-1199: Cancel retained payment reversals when their owning identity is removed
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4252358882)
  The permanent-refusal policy bounds the absent-owner write wedge, but unstored_reversals still has no identity-incarnation authority. After a rejected reversal, removing and re-adding the same owner before the next adapter round allows the cached whole PaymentEntry to be written into the newly initialized history. Storage accepts the owner, so the refusal branch never runs. This contradicts remove_identity's terminal-removal contract and can restore a deleted payment or overwrite a reconstructed row with stale fields. PendingWrites preserves removal/re-addition boundaries only for writes it has already received; it cannot invalidate a row retained privately by this adapter and submitted afterward. Cancel retained verdicts on terminal removal or bind them to a shared Rust-owned lifecycle token, independently of the still-owed core truncation, and test successful removal followed by re-addition before retry.

In `packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs:491-500: Resolve retained InstantSend finality for every storage reader
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4252965583)
  A kept transaction is rewritten to a Mempool record while its valid core_instant_locks row remains. Full restoration validates and applies the lock in restore_recorded_transactions, but SqlitePersister::get_core_tx_record delegates directly to the raw get_tx_record reader. History reconstruction and pending-payment reconciliation therefore see weaker finality than a fully restored wallet for the same stored transaction. The ordinary late-arriving-lock case does predate this PR, but this new truncation rewrite introduces another producer of that inconsistent representation. Keeping the real lock is correct; neither deleting it nor fabricating lock bytes in the changeset is a repair. Share validated lock/context resolution between point reads and replay, preserving the existing txid, input, and stored-height checks, and cover both late-lock and post-truncation reads. If the broader reader fix is deferred, link a focused tracking issue or follow-up PR rather than relying on the limitation text alone.

In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- [SUGGESTION] packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift:1770-1772: Propagate pending-input lookup failures through the truncation callback
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4252358886)
  restoreTruncatedCoin is throwing, but pendingInputRows suppresses errors on both fetch paths and, during a round, registers the failed fetch as an empty successfully fetched result. With a detached swept-input tombstone present, truncation can skip its deletion, save successfully, and release Rust's one-shot truncation ownership. A later drainPendingInputs can consume the surviving tombstone and mark the restored coin spent again. This withholds a real coin rather than inventing funds, but it still acknowledges a restoration that was not fully applied. The helper's existing sweep callers support fixing the common storage-I/O contract for both paths; they do not make the new authoritative acknowledgement safe. Make the lookup throwing, preserve the saved/staged merge, cache only successful reads, and propagate failure through the enclosing callback. Add a detached-tombstone regression with an injected fetch failure.

In `packages/rs-platform-wallet-storage/src/sqlite/persister.rs`:
- [SUGGESTION] packages/rs-platform-wallet-storage/src/sqlite/persister.rs:1194-1206: Retain payment reversals with backend-owned truncations
  (existing thread: https://github.com/dashpay/platform/pull/5384#discussion_r4251865297)
  Under FlushMode::Manual, store accepts a truncation and its Confirmed-to-Pending payment overlay into the buffer, so the adapter relinquishes both. A later fatal flush rebuilds PendingWrites here with only core.truncations. Recovery can then remove the abandoned transaction and restore its inputs while leaving the authoritative identity payment Confirmed indefinitely; reconciliation processes only Pending entries. Immediate being the default and the absence of an in-tree Manual host reduce exposure, but the new recovery path still preserves only half of the accepted fork correction. Retaining every Pending overlay would be wrong because the same field also carries newly recorded sends with different rollback ownership. Represent truncation-derived reversal provenance explicitly and retain only still-current dependent verdicts across buffer disposal, respecting newer verdicts and identity removal. Cover buffered acceptance, fatal flush, recovery, and payment-status reload.

[Claudius-Maginificent]

Merge-order note: #5762 lands in v5.1-dev before this stack.

When this branch next merges its base:

• FFI extension slots. feat(platform-wallet): persist the Orchard recipient of a shielded asset-lock funding #5762 appends two recipient slots to PersistenceCallbacksExtension. Append on_persist_wallet_changeset_truncations_fn after them, then update the struct_size thresholds, the layout-pinning tests and the Swift handler wiring.
• Capability bits. CORE_CHAIN_TRUNCATION = 1 << 15 is free after feat(platform-wallet): persist the Orchard recipient of a shielded asset-lock funding #5762 takes 1 << 14. fix(platform-wallet)!: replay recorded transaction history on load for every persister #5220 has to move to 1 << 16. Keep HIGHEST_DECLARED_BIT and the hex assertions consistent with the final set.
• Asset-lock rows. Conflicts in sqlite/schema/asset_locks.rs and persister.rs: keep feat(platform-wallet): persist the Orchard recipient of a shielded asset-lock funding #5762's shielded_recipient column in the upsert (COALESCE plus conflicting-value rejection) and in the readers. Any struct literal copying a stored row must carry the recipient over.