[lklimek]

Restores stored Core transaction history through the shared wallet load path so that rediscovered funding cannot make already-spent outputs available again.

Issue being fixed or feature implemented

Stacked on #5150 (fix/pr-5126). Addresses the #5150 review thread "Confirmed-history replay is implemented only for SQLite".

After a restart the wallet must know which outputs its confirmed transactions already spent (spent_outpoints / observed_spent_outpoints). Without that, a redelivered funding transaction (rescan, gap-limit rediscovery) credits an already-spent output again. #5150 rebuilt this state only inside the SQLite persister. The FFI persister (iOS / SwiftData) replayed only unconfirmed outgoing sends, so confirmed spends were never replayed on that path.

What was done?

Where the replay runs

• Shared replay. A history replay lives in rs-platform-wallet (manager/history_replay.rs, replay_recorded_history). load_from_persistor runs it, before the balance is mirrored into the wallet, for every persister that hands its stored records back through ClientWalletStartState::recorded_history (RecordedHistory { transactions, instant_locks }, types in changeset/recorded_history.rs). Today that is the FFI persister (iOS/SwiftData). It runs inside the async load and awaits the checker.
• SQLite keeps replaying its stored history itself, inside the load() transaction, exactly as on fix(platform-wallet): correct wallet-level transaction history accounting #5150. It returns an empty recorded_history, so nothing is replayed twice, and declares CORE_HISTORY_RESTORE. sqlite/rehydrate.rs is identical to the base.
• FFI.
  • New #[repr(C)] RecordedTransactionRestoreFFI, appended to WalletRestoreEntryFFI as recorded_transactions / recorded_transactions_count.
  • Decoding fails closed per row: bytes that don't decode completely, don't hash to the row's txid, or carry an unknown context are dropped, and the drops are counted in a warning. Both the recorded-history and the unconfirmed-outgoing decoders reject trailing bytes.
  • The entry also carries the stored net_amount / direction. They are unused here, reserved for the follow-up that makes Rust the single source of transaction accounting.
• Swift. loadWalletList fetches the stored history once and hands each restorable wallet its own records. The fetch prefetches the relationships used to determine wallet ownership. A fetch error rejects the snapshot rather than claiming the wallet has no history.
• Capability. New CORE_HISTORY_RESTORE (bit 14; bit 13 belongs to asset-lock operation IDs). SQLite and Swift declare it; the FFI attests it only when the wallet-list load callbacks are wired. It gates nothing: load_from_persistor logs a warning when a persister restores wallets without it.

What the replay does

• Credits stay with the store. The persisted UTXO set is authoritative for credits: an output the replay credits but the store did not hold as unspent is dropped again, so the replay only ever removes. A send listed in both the unconfirmed outgoing list and the history is accounted once and still rebroadcast.
• Raw records. Raw-restored asset-lock and provider records are detached for the replay, so their presence cannot suppress the checker's spend reconstruction. Labels, fees and the records needed for proof lookup survive, unless a final conflicting spend removes the record.
• Conflict handling, matching fix(platform-wallet): correct wallet-level transaction history accounting #5150.
  • Owned inputs recorded on a stored transaction (StoredTransaction::owned_inputs) are staged before the replay, so a spend whose funding survives only as a height row rebuilds its spent mark.
  • Conflicts are planned on the whole stored history before anything is replayed. Stored contexts are settled first: a block at or below the restored chain lock is chain-locked, and a mempool record with a matching lock is InstantSend-locked. Losing transactions and their unconfirmed descendants, including ones paying another account, are kept out of the replay and the projection. One sweep of the settled survivors runs after the raw records are restored. The result does not depend on record order.
• Planning cost. Planning indexes the unconfirmed stored records once, by spent outpoint and by parent transaction. A settled record is swept only while another unconfirmed record still spends one of its inputs, in a scratch account holding only those competitors and their unconfirmed descendants. The upstream sweep still decides every conflict. A transaction recorded in several accounts counts as one spender.
• Coins parked in a fallback account. A coin the projection parks in a fallback account moves to the account that derives it as soon as the replay credits it there, so a later record spends it once.
• Persisted coin state. A coin the replay keeps also keeps its stored spend lock, InstantSend status, trusted mark and confirmation. A coin stored as unconfirmed is not promoted, and a block the replay confirmed is not overwritten by an older stored height.
• Claims for irreversible spends. A replayed spend whose funding is not in the history marks no coin spent: the coin is not in the projection and, without recorded owned inputs, cannot be staged. For such a spend in a chain-locked block or under an InstantSend lock, the replay restores a spend claim on each input (irreversible_spend_claims), so a funding delivered later is not credited again. A spend in a plain block or in the mempool is not claimed, because nothing retires a claim when a fork or an eviction drops its claimant.
• Retired sends. replay_recorded_history returns HistoryReplay { replayed, retired, defeated }. retired holds every stored transaction the replay removed or left out; defeated holds only those a stored conflict winner beat, with their descendants. An unconfirmed send the replay retired is dropped from the list re-dispatched at load (HistoryReplay::resendable).

Amounts the checker cannot take

• Before a stored record reaches the wallet checker, unsafe_replay_amounts checks its operands: restored coins, outputs including asset-lock credit outputs, and recorded owned inputs, each alone and summed. A failing record is left out of the replay and the load continues. No valid transaction carries such amounts, so a confirmed one is store corruption and is logged as an error. The record still takes part in conflict settlement: a stored final winner removes it during planning, and one delivered after the load removes it and releases the input the winner does not spend.
• Each rejected record comes with its reason (UnsafeAmounts). Outputs means the recorded transaction bodies prove the overflow on their own, so the transaction can never confirm. Stored means the overflow depends on a value stored beside the transaction (a restored coin or a recorded owned input), so the transaction itself may be valid.
• Restored coins and recorded owned-input outpoints stay reserved even when their funding is absent from the projection. Excluded unconfirmed records remain in accounts owning their recorded inputs, so a confirmed conflict can sweep the claimant and release its other input. Reservations name one claimant (reservation_claimant). Among Outputs records the smallest txid stands for all. A single Stored record is named, so the coin stays guarded while that record stands. Two Stored records leave no claimant whose removal proves the coin free.

Tracked asset locks

• Conflict evidence. Before the replay consumes the history, the load reads from it which pending asset locks had an input spent by another confirmed transaction (recorded_input_conflicts), so that knowledge survives chain-lock finalization. The pending locks' inputs are indexed by outpoint once.
• Inputs of a final lock. After the replay, the load restores a spend claim, naming the funding transaction, for every input of a tracked lock at InstantSendLocked, ChainLocked, Consumed or RecoveredFromChain (final_lock_input_claims). A host may drop those inputs from the projection on the lock's status alone while the funding's own record guards nothing.
• The rule for a lock's funding. What a tracked asset lock proves about its funding outranks the funding's stored context, and the replay's verdict outranks the lock's status.
  • A lock with an Instant proof hands its InstantSend lock to the history (add_tracked_instant_locks), so a funding stored as unconfirmed replays as InstantSend-locked: it sweeps an unconfirmed competitor and stands beside one in a plain block. The lock is taken only when it, the proof's transaction and the tracked funding name one transaction the history holds.
  • A chain-final lock (ChainLocked, Consumed, RecoveredFromChain) proves a height and no block. Its funding's unconfirmed record is kept out of the replay (withhold_chain_final_fundings) unless a lock in the history settles it: its coins stay as stored and its inputs stay claimed. A history lock settles it only if the replay accepts that lock for the record (lock_matches_record); a lock filed under the funding that names another transaction does not. A record of that funding already attached to the restored wallet sits the replay out as well: the load detaches it before the replay and puts it back afterwards (detach_unconfirmed_records, reattach_records). A host attaches one when a sibling credit output of the same funding transaction is still at Broadcast.
  • A funding the replay still defeats gets no claim, whatever its lock's status.

Known limitations:

• A spend in a block above the restored chain lock that pays nothing to the wallet, and whose funding is not in the history, is guarded only by the observed spend. Once its block becomes final within the session that guard is pruned, and a later delivery of the funding credits the coin again until the next load. Closing this needs the host to supply owned inputs through the FFI (TODO(ffi-recorded-input-details)) or provisional evidence promoted at finality; tracked in platform-wallet: a spent coin can return as spendable within a session on FFI hosts (recorded history carries no owned inputs) #5412.
• Host contract for StoredTransaction::owned_inputs: a store that drops the coin of an unconfirmed spend from the load projection must record that spend's owned inputs. The replay guards an unconfirmed spend through a restored coin or its recorded owned-input details.
• A coin shared by two Stored records stays claimed until the next load, even once both records are swept.
• A coin shared by two Outputs records is held for the one with the smaller txid. Sweeping that record alone drops the guard while the other still names the coin; neither record can ever confirm, so no funds are at risk.
• A tracked lock whose funding the replay defeated stays tracked. Live settlement removes such a lock and persists the removal; the load does not write to the store.
• Chain-final asset-lock funding withheld from replay gets no new wallet record from that replay.
• A lock's status is trusted as stored. An Instant proof is matched by identity only; its signature is not verified at load.
• A chain proof carries a height and no block, so a chain-final funding cannot be replayed in its block. That needs the block from the host.
• Unresolved finality blocker: InstantSend rows replay as mempool on the FFI path because the host stores no standalone InstantLock bytes; tracked asset-lock Instant proofs are an exception. This loses finality during conflict settlement. Preserving trusted stored finality through replay and live sweeps requires key-wallet support and an existing-row recovery policy; this PR does not yet close that gap.
• Unresolved resource blocker: Swift and Rust still materialize the full recorded history at startup. A safe budget needs early preflight, bounded allocation and actionable restore-error handling; supporting large histories requires resumable replay with durable spend guards.
• The shared replay settles conflicts in memory only. An FFI store keeps the losing rows, and they are planned away again on each load (belongs with platform-wallet: make Rust the single source of Core transaction accounting (drop Swift re-derivation) #5226).
• The replay takes each stored row's context as stored, so a row the store still holds from a reorged-out block takes part in settlement.
• A locked coin reserved for an unsafe record comes back unlocked when it is released and re-credited within the same run; the store keeps the lock and the next load restores it.
• A restored coin whose own value exceeds i64::MAX stays in the projection and the balance; only records spending it are left out.
• The reservation for a left-out record lives in memory and is rebuilt on each load, because the FFI store keeps the row.
• Conflict planning examines a competitor holding an InstantSend lock, and its descendants, once per plain-block winner it survives.
• TODO(bound-load-history-replay): every stored record is replayed on each load. TODO(expire-unconfirmed-spend-reservations): stored mempool records, incoming ones included on the FFI path, are replayed with no expiry.
• The SQLite load path does not yet keep asset-lock conflict evidence across finalization: platform-wallet-storage: SQLite load forgets a buried spend of a pending asset lock's input #5379.

Out of scope:

• Amount validation on the live path. A peer sending a transaction with unsafe amounts after load still reaches the unchecked arithmetic in key-wallet / dash-spv; only the load replay and the wallet's own re-dispatch are covered here.
• Unifying the two replay engines. SQLite replays in its own load transaction and has its own amount check (validate_replay_amounts); the shared replay serves record-returning persisters.
• History restore on Android. The Kotlin host does not supply history yet: its JNI fields are null/0 and it does not attest the capability (TODO(android-core-history-restore)).

How Has This Been Tested?

Validation after merging fix/pr-5126 at fa110a12cda7350ab3b0da36f67a84a50beb881f:

• Targeted all-feature nextest across platform-wallet, platform-wallet-ffi and platform-wallet-storage: 165 passed, including history replay, capabilities, transaction records, snapshot loading and completed-drain funding protection.
• Scoped Clippy for wallet, storage, wallet FFI and unified JNI with --all-targets --all-features --locked -- --no-deps -D warnings: clean. Formatting and diff checks passed.
• should_reserve_absent_inputs_of_unsafe_stored_record_until_swept failed before the fix and covers absent funding, redelivery protection and release after confirmed conflict.
• Full workspace and native iOS suites were not run for this revision.

What the main tests cover:

• Conflicts and order. should_sweep_conflicting_spend_for_either_sibling_replay_order (block, chain-lock and persisted-chain-lock winners), should_drop_attributable_descendant_of_swept_conflict_for_any_replay_order, should_remove_cross_account_descendant_of_conflicting_spend, and should_plan_the_same_removals_as_whole_history_sweeps, which compares the planner with whole-history sweeps on 500 generated histories.
• Guards and claims. should_rebuild_spent_mark_for_height_only_funding_from_owned_inputs, should_guard_unreplayed_funding_against_irreversible_spends_only, should_credit_funding_again_after_a_fork_drops_the_restored_spend, should_keep_unsafe_amounts_from_the_checker_and_reserve_the_coins_they_spend, should_hold_shared_coin_while_a_record_that_may_confirm_spends_it, should_release_coin_shared_by_spends_of_impossible_recorded_amounts.
• Tracked asset locks. should_settle_stored_conflicts_by_what_a_tracked_lock_proves asserts the rule as a table: lock evidence (Instant proof, chain-final status) against competitor (unconfirmed, plain block, chain-locked), checking the funding's change, the competitor's change and whether the funding's other input stays guarded; two more rows cover a history lock that names another transaction, and a funding with two credit outputs at mixed statuses whose record the host attached. Around it: should_not_credit_inputs_of_a_final_asset_lock_again, should_not_claim_inputs_of_a_tracked_funding_the_replay_defeated, should_keep_the_claim_of_a_final_lock_whose_funding_was_only_left_out, should_remember_stored_spender_of_pending_asset_lock_input_on_load. should_settle_an_attached_unconfirmed_record_the_history_does_not_hold pins what the replay does with an attached unconfirmed record, which is why the load detaches it; should_detach_unconfirmed_records_of_withheld_fundings_and_put_them_back covers the detaching.

Also covered: normal and overlapping FFI restore, malformed and trailing transaction bytes, proof-record and metadata retention, funding redelivery, and re-dispatch of retired sends. Each regression test was seen failing before its fix.

Not run:

• Native Swift build and tests; this environment has neither swift nor xcodebuild. The Swift changes were read, not run.

Breaking Changes

• WalletRestoreEntryFFI gains two trailing fields (recorded_transactions, recorded_transactions_count), plus the new element struct RecordedTransactionRestoreFFI. The entry has no size field, so the host and the Rust library must be built together (same contract as unconfirmed_outgoing_tx_records).
• ClientWalletStartState gains the public field recorded_history. Struct-literal constructors must set it; Default::default() gives the previous behaviour.
• CORE_HISTORY_RESTORE (bit 14; bit 13 belongs to asset-lock operation IDs) is additive.
• New public module manager::history_replay (additive): replay_recorded_history, HistoryReplay, unsafe_replay_amounts, UnsafeAmounts and ReplayOperands.

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

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

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

[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/pr-5126
• File: .coderabbit.yaml
• Commit: 50eb8b3
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 4250aee8-a279-4d63-b432-ec094e43da66

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 — 9 blocking finding(s) (commit ecc4921) · triage: critical

[thepastaclaw]

Final validation — Phase 1 + Phase 2

History replay is correctly centralized, and the normal FFI path test demonstrates that recorded confirmed spends guard redelivered funding outputs. However, the FFI also raw-inserts unresolved asset-lock records before shared replay, while the new held-txid skip suppresses replay of the same confirmed funding and spender rows; their spend guards therefore remain absent and a redelivered funding transaction can resurrect an already-spent output. This is a blocking correctness issue.

🔴 1 blocking | 🟡 3 suggestion(s)

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: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: ffi-engineer); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: security-auditor); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 10: 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 cross-language persistence and wallet-replay change that directly alters funds movement and spent-output accounting in platform-wallet load paths, including FFI deserialization and storage restoration.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — architecture-layering (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — ffi-engineer (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — security-auditor (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 13% left, 5h 100% left), 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 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/manager/history_replay.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/manager/history_replay.rs:93-95: Do not skip history merely because a raw-restored transaction record exists
  The `held` set only proves that an account contains a transaction-map entry; it does not prove that the checker restored the transaction's spend effects. The FFI load path raw-inserts unresolved asset-lock records, including confirmed spenders of asset-lock inputs, through `transactions_mut().insert(...)` before `recorded_history` is replayed. Swift supplies those same rows in both the unresolved-asset-lock array and the recorded-history array. Because their txids are already held, this branch skips them, and the raw synthetic records do not populate `spent_outpoints` or `observed_spent_outpoints`. A later redelivery of the funding transaction can therefore credit an output already consumed by the confirmed spender. The existing FFI regression does not include the raw unresolved-record array, so it does not exercise this overlap. Stage retention-only records after shared replay, avoid raw-inserting records that will be checker-replayed, or otherwise rebuild the spend guards independently; add a regression with a raw-restored confirmed spender and funding redelivery.
- [SUGGESTION] packages/rs-platform-wallet/src/manager/history_replay.rs:63-72: Build the held transaction set once instead of scanning all accounts per record
  The `held` filter reconstructs `all_accounts()` and scans every account for every stored transaction. Since load replays the full persisted history on each startup, this adds repeated account-collection work proportional to history size. Build an owned txid set from all account transaction maps once before filtering; this also avoids retaining references into `wallet_info` across the subsequent mutable replay.

In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- [SUGGESTION] packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift:7140-7149: Prefetch transaction relationships during history restore
  The new history fetch retrieves every `PersistentTransaction`, then evaluates `walletOwnsTransaction` for each restorable wallet. That predicate touches `involvedAccounts`, `outputs`, `inputs`, and `pendingInputs`, but this descriptor does not prefetch any of them. On a cold launch, relationship faults can therefore be triggered repeatedly while filtering the history for each wallet. Prefetch these relationships, or bucket ownership in one pass, to keep history restoration from scaling poorly as wallets and transaction history grow.

In `packages/rs-platform-wallet-ffi/src/persistence.rs`:
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/persistence.rs:5245-5257: Reject trailing bytes in recorded transaction rows
  `consensus_decode(&mut &bytes[..])` accepts a valid transaction prefix without requiring the input cursor to be exhausted. Consequently, a row containing a valid transaction followed by arbitrary bytes passes the txid check and is replayed, despite the decoder's fail-closed contract and the surrounding use of full `deserialize` for other restore paths. Require complete cursor consumption before accepting the row; apply the same exact-consumption rule to the unconfirmed decoder for consistency.

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.

• Define expiry for persisted unconfirmed spend reservations — The shared replay now also replays host-supplied FFI mempool rows, so a never-mined or invalid observed spender can reserve an outpoint across every restart until a conflicting confirmed or InstantSend transaction is observed. This is an availability concern, but the same policy already exists on the SQLite path and is explicitly tracked by the moved expiry TODO rather than being introduced as an implementation mistake in this change.
  • Follow-up: Track a separate policy change defining how persisted unconfirmed reservations are revalidated or expired, including safe release of dependent transactions.

[thepastaclaw]

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

The shared history replay fixes the confirmed-spend reconstruction gap and the four prior findings are resolved. Two replay-order defects remain: load-time chain-lock finalization can erase asset-lock conflict evidence before it is seeded, and final conflict reconciliation is skipped when the checker regenerates a conflicting raw record. Both can leave incorrect asset-lock wait behavior or wallet balance/spend state after restore.

🔴 2 blocking

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 cross-language change that directly alters wallet transaction-history replay, spent-output reconstruction, and spend reservations in persistence and loading paths, affecting funds movement and coin-selection state across SQLite, FFI, Swift, and Rust.
• 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 13% 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/manager/history_replay.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/manager/history_replay.rs:168-170: Preserve asset-lock conflict evidence before load-time finalization
  The replay finalizes the restored chain lock after rebuilding history, but `load_from_persistor` seeds `observed_input_conflicts` only afterward. With the default `keep-finalized-transactions = OFF`, `apply_chain_lock` evicts a confirmed spender record once its block is buried, leaving only its finalized txid. The subsequent seeder reads `transaction_history()` and the cache, so a restored confirmed spender that was needed to identify an `AssetLockInputContested` conflict disappears before it can be recorded. This regresses the bounded proof-wait behavior for a Built/Broadcast asset lock whose input was spent by a restored transaction. Capture conflict evidence before applying the restored chain lock, or seed the cache from the replayed records before finalization while retaining the finalization needed to rebuild spend guards.
- [BLOCKING] packages/rs-platform-wallet/src/manager/history_replay.rs:162-166: Reconcile final conflicts after raw-history restoration
  `sweep_conflicts` is invoked only when `restored_fallback` becomes true. If a final winner is replayed before a conflicting mempool loser, all raw records have already been detached, so the winner's initial sweep cannot see the loser. The checker can then regenerate the loser because its wallet-owned change output is attributable, causing metadata restoration to take the existing-record branch at lines 152–155 and leaving `restored_fallback` false. For an InstantSend winner, the checker also does not populate `observed_spent_outpoints`, so the loser can recreate change and reserve additional inputs. Because the persisted projection filter preserves that change when it was present in the restored UTXO set, both conflicting state and an inflated balance can survive load. Reconcile all final transactions after the complete replay and metadata-restoration pass, independently of whether fallback records were reinserted; add a regression with an attributable loser and an InstantSend winner replayed first.

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

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.

• Revalidate indefinitely retained unconfirmed spend reservations — Stored incoming or outgoing mempool records are replayed across restarts without expiry or chain revalidation. A never-mined transaction can therefore keep a wallet outpoint reserved indefinitely. This behavior predates the shared replay and is explicitly tracked by TODO(expire-unconfirmed-spend-reservations), so it should not be fixed in this PR.
  • Follow-up: Track a separate change for safe admission and reservation revalidation that does not release inputs belonging to genuinely broadcast transactions merely because peers become silent.
• Bound startup history replay without discarding spend guards — A peer can accumulate many wallet-relevant unconfirmed records, increasing retained history and startup replay work on every launch. The SQLite path already replayed complete history, and this PR moves that behavior into the shared implementation; truncating history here would reopen spent-output resurrection. This is explicitly tracked by TODO(bound-load-history-replay) and is outside this fix.
  • Follow-up: Design a separate bounded-history mechanism backed by durable spend-guard or finality checkpoints, with adversarial history-size coverage.

[thepastaclaw]

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

Static verification confirms two prior blockers remain: block-confirmed conflicts can retain persisted losing change, and load-time finalization can erase asset-lock conflict evidence before recovery caches it. The new descendant cleanup also fails to remove already-reconstructed descendants and introduces avoidable quadratic startup work; four prior findings are fixed. No builds or tests were run; the supplied CI snapshot confirms Kotlin validation but contains no Rust or native Swift validation results.

🔴 3 blocking | 🟡 1 suggestion(s)

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

Review provenance

Source: reviewer 1: 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: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: 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) — The intricate cross-persister replay in packages/rs-platform-wallet/src/manager/history_replay.rs::replay_recorded_history changes spend reservations, conflict resolution, and spendable UTXO reconstruction, directly affecting funds availability and coin selection after wallet reload.
• 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 39% left, 5h 11% 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

🤖 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/manager/history_replay.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/manager/history_replay.rs:157-163: Remove reconstructed descendants when their parent was already swept
  drop_swept_descendants only extends the swept set, and this continue only skips restoration of the held original. Neither operation removes a live descendant already reconstructed by the checker. The permitted replay order loser → InstantSend winner → mempool child exposes this: the winner removes the parent while the child is detached, then a child with a wallet-owned output is reconstructed and its persisted output survives retention. The post-replay InstantSend sweep cannot discover that child because the conflicting parent record is already absent; the pinned conflict walker returns when it finds no direct loser. This helper identifies the child through held history, but skipping its original leaves the live record and selectable output intact. Remove reconstructed records and UTXOs for the swept descendant closure, with appropriate spend-mark cleanup, or prevent those descendants from being applied. The existing orphan regression uses an unattributable child and does not exercise this case.
- [SUGGESTION] packages/rs-platform-wallet/src/manager/history_replay.rs:202-226: Index held descendants instead of repeatedly scanning them
  This closure repeatedly scans every followable held record until no new txid is added. Held records are collected in account/transaction-map order rather than dependency order, so a deep descendant chain can require one pass per generation, producing O(depth × held history) startup work. The pinned checker's conflict walker already avoids this scaling problem with a parent-to-children index. Build that index once and traverse newly swept txids with a worklist, preserving the existing confirmed/locked exclusions while making traversal linear in records and input edges.
- [BLOCKING] packages/rs-platform-wallet/src/manager/history_replay.rs:175-179: Reconcile final conflicts after raw-history restoration
  (existing thread: https://github.com/dashpay/platform/pull/5220#discussion_r4153965344)
  The final reconciliation runs only when an unattributable held record was reinserted. A block-confirmed winner replays before its conflicting mempool loser while the held loser is detached, so the initial sweep cannot remove it. If the loser pays a wallet-owned address, the checker subsequently reconstructs its record. The pinned checker's observed-spend guard returns before modifying UTXOs, which prevents new credits but leaves the loser's already-restored change untouched. That change passes the projection retention filter, and restoring the held record takes the metadata-only branch, leaving restored_fallback false. The unconditional sweep at lines 122–123 covers only InstantSend winners, so neither the reconstructed loser nor its selectable change is removed. Reconcile block-confirmed winners after complete replay and raw restoration independently of fallback insertion, and cover an attributable loser whose change is present in the restored UTXO projection.
- [BLOCKING] packages/rs-platform-wallet/src/manager/history_replay.rs:181-184: Preserve asset-lock conflict evidence before load-time finalization
  (existing thread: https://github.com/dashpay/platform/pull/5220#discussion_r4153965337)
  Replay applies the restored chain lock before load_from_persistor constructs PlatformWalletInfo and calls seed_observed_input_conflicts at load.rs:283. With keep-finalized-transactions disabled, as it is by default, the pinned key-wallet implementation promotes and evicts covered InBlock spender records, retaining only their txids. Swift explicitly supplies confirmed spenders of unresolved asset-lock inputs through its unresolved-record buffer, but restoring those records before this finalization does not preserve their input-to-spender association afterward. The seeder reads only surviving transaction_history(), and the new conflict cache starts empty, so a Built/Broadcast asset lock loses its conflict-capped proof wait and contested-input verdict. Already-chainlocked history can also be evicted during checker replay when no held fallback preserves it. Capture the relevant input, spender, and height observations before either eviction point and carry them into PlatformWalletInfo while retaining core finalization. Cover manager load with a competing confirmed spender and a higher persisted chain lock under the default retention configuration.

[thepastaclaw]

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

Static verification at f6f2004 confirms that all eight prior findings are addressed. Two in-scope blockers remain: unchecked monetary operands in shared replay and attacker-amplifiable whole-history conflict sweeps; rebroadcasting transactions already swept as losers is a lower-impact issue. No builds or tests were run in this lane; the supplied exact-head CI snapshot shows successful Kotlin checks but no dedicated Rust-wallet or native Swift validation.

🔴 2 blocking | 🟡 1 suggestion(s)

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

🟡 Suggestion: Do not rebroadcast unconfirmed sends that the history replay swept as losers
packages/rs-platform-wallet/src/manager/load.rs:373-387

Shared replay excludes defeated transactions from accounting, but pending_resends still receives the complete unconfirmed_outgoing_txs list because replay returns only a count. A send present in both lists can therefore be swept as a loser and then handed back to SPV's broadcast/retry machinery. Swift's outgoing selection reduces this overlap but does not eliminate it: a losing send with an additional wallet input can remain selected through that still-unspent input even after a winner takes the shared input. This retries a transaction the restored history has already classified as defeated and, for an attributable transaction, routes it through mempool processing again after its record was removed. Return or otherwise expose the swept txids and exclude them from pending_resends, while retaining rebroadcast for surviving overlapping sends.

_{source: muse-spark-1.3-contributor (phase1-reviewer: general)}

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: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); 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); 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 replay in packages/rs-platform-wallet/src/manager/history_replay.rs changes spent-output reconstruction, conflicting-spend resolution, and balance accounting during wallet load, directly affecting which funds remain available to spend.
• 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 — 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/src/manager/history_replay.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/manager/history_replay.rs:128-139: Validate replay amounts before invoking the wallet checker
  Shared replay stages stored input values and invokes the checker without the operand validation already used by SQLite's restore_recorded_transactions. The FFI decoder checks complete encoding, txid and context, but those checks do not bound transaction amounts. In the exact pinned key-wallet dependency, ManagedCoreFundsAccount::record_transaction calculates `received as i64 - sent as i64`, and wallet_checker performs the equivalent subtraction for wallet totals. A stored transaction spending a restored 1-duff wallet coin and paying `1_u64 << 63` duffs to a wallet address therefore reaches `i64::MIN - 1`: overflow-checked builds panic during load, while unchecked builds produce wrapped accounting. Owned-output accumulation can overflow as well, and the projection filter runs only after this arithmetic. Add a checked preflight before mutating the restored state, covering individual values and aggregate checker operands from existing coins, replayed parent outputs, owned-input details and asset-lock credit outputs. Handle unsafe records through a controlled restore outcome; excluding an unsafe mempool record must preserve its input reservations rather than silently making those coins spendable. Add shared/FFI regressions corresponding to SQLite's operand-safety coverage.
- [BLOCKING] packages/rs-platform-wallet/src/manager/history_replay.rs:273-278: Avoid whole-history sweeps for every contested historical spender
  The planner's contested-input filter does not bound the total sweep work. The scratch account contains the complete stored history, and the pinned drop_conflicted_transactions scans that account's transaction map on every sweep_conflicts call, including calls that remove nothing. A wallet-paying mempool transaction listing inputs already spent by N distinct stored confirmed transactions puts all N confirmed transactions into the precomputed winners list. The first sweep removes the mempool competitor, but the other N−1 winners remain eligible and each scans the retained confirmed history, producing Ω(N²) planning work from an O(N)-sized conflicting record. This is reachable from an unauthenticated SPV peer: mempool handling routes transactions through wallet relevance checks, and a matching wallet output still produces a transaction record even when update_utxos rejects its outputs as doomed by an observed block spend. The FFI store retains rows removed only by shared replay, so the same startup amplification recurs on subsequent loads. Resolve conflicts with an input-to-spenders index and a shared descendant worklist, or invalidate candidates as their remaining competitors disappear while keeping work restricted to affected records. Preserve finality precedence and cross-account descendant removal.

In `packages/rs-platform-wallet/src/manager/load.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/manager/load.rs:373-387: Do not rebroadcast unconfirmed sends that the history replay swept as losers
  Shared replay excludes defeated transactions from accounting, but pending_resends still receives the complete unconfirmed_outgoing_txs list because replay returns only a count. A send present in both lists can therefore be swept as a loser and then handed back to SPV's broadcast/retry machinery. Swift's outgoing selection reduces this overlap but does not eliminate it: a losing send with an additional wallet input can remain selected through that still-unspent input even after a winner takes the shared input. This retries a transaction the restored history has already classified as defeated and, for an attributable transaction, routes it through mempool processing again after its record was removed. Return or otherwise expose the swept txids and exclude them from pending_resends, while retaining rebroadcast for surviving overlapping sends.

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

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.

• Preserve pre-finalization asset-lock conflict evidence on SQLite load — SQLite finalizes history inside its own load transaction and returns an empty recorded_history, so the manager's new pre-replay evidence capture cannot preserve SQLite's equivalent conflict evidence. This is an unchanged, explicitly acknowledged limitation tracked by #5379, not an unresolved instance of the shared-replay finding.
  • Follow-up: Keep #5379 as the separate follow-up for capturing evidence before SQLite finalization and carrying it through the startup snapshot.
• Authenticate or explicitly manage peer-originated spend reservations — The underlying unauthenticated mempool reservation policy predates this PR and is explicitly acknowledged by TODO(expire-unconfirmed-spend-reservations). A broader authentication, expiry or user-recovery policy is outside this change; the newly introduced startup-work amplification is retained separately as an in-scope finding.
  • Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.

[thepastaclaw]

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

The shared replay addresses eight of the eleven prior findings, including raw-record detachment, descendant exclusion, finality reconciliation, and complete-buffer decoding. Two blockers remain: cross-account duplication can bypass the amount preflight and overflow the checker, and independent competitors still cause quadratic conflict planning; swept-send rebroadcast and duplicate competitor counting also remain actionable. Validation was static only; the supplied head-matched CI snapshot had Rust workspace tests and several builds pending, wallet-specific tests skipped, and no native Swift build evidence.

🔴 2 blocking | 🟡 2 suggestion(s)

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

🟡 Suggestion: Do not rebroadcast unconfirmed sends that the history replay swept as losers
packages/rs-platform-wallet/src/manager/load.rs:381-386

History replay excludes defeated transactions and their descendants, but returns only a count, while this queue receives the original unconfirmed_outgoing_txs unchanged. The history_txids filter prevents duplicate accounting but does not distinguish surviving sends from swept ones. This is reachable through Swift's TXO-based selection: an unconfirmed loser spending A+B can remain selected through the unspent B row when a stored confirmed winner spends only A. Replay correctly removes the loser, yet resend_one checks only wallet-generation liveness before broadcasting it. The known loser is consequently handed back to SPV's rebroadcast timer on subsequent loads. Re-sending identical bytes does not authorize a new spend, but retrying transactions already defeated by restored finality is unnecessary and inconsistent with replay's settlement decision. Return swept txids from replay and filter the resend list, preserving eligible overlapping survivors; cover both the defeated parent and its descendants.

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

2 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: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); 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: 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) — The intricate replay_recorded_history implementation in packages/rs-platform-wallet/src/manager/history_replay.rs directly changes spend reservations, conflicting-transaction handling, and spendable UTXO reconstruction, affecting funds availability and coin selection.
• 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 — 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/src/manager/load.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/manager/load.rs:381-386: Do not rebroadcast unconfirmed sends that the history replay swept as losers
  History replay excludes defeated transactions and their descendants, but returns only a count, while this queue receives the original `unconfirmed_outgoing_txs` unchanged. The `history_txids` filter prevents duplicate accounting but does not distinguish surviving sends from swept ones. This is reachable through Swift's TXO-based selection: an unconfirmed loser spending A+B can remain selected through the unspent B row when a stored confirmed winner spends only A. Replay correctly removes the loser, yet `resend_one` checks only wallet-generation liveness before broadcasting it. The known loser is consequently handed back to SPV's rebroadcast timer on subsequent loads. Re-sending identical bytes does not authorize a new spend, but retrying transactions already defeated by restored finality is unnecessary and inconsistent with replay's settlement decision. Return swept txids from replay and filter the resend list, preserving eligible overlapping survivors; cover both the defeated parent and its descendants.

In `packages/rs-platform-wallet/src/manager/history_replay.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/manager/history_replay.rs:467-475: Deduplicate transaction identities when counting live competitors
  `unconfirmed_spends` counts account records rather than distinct spending transactions. The checker records a pooled or multi-account transaction in each affected account, but `is_contested` subtracts only one occurrence for the candidate itself. An InstantSend transaction represented in two accounts is therefore considered contested even when no other transaction spends its inputs. After replay, N independent multi-account InstantSend transactions trigger N additional whole-history sweeps that remove nothing, introducing quadratic work on legitimate restored history. Count each txid's inputs once before applying `is_contested`, and add a regression with a surviving InstantSend transaction represented in multiple accounts and no competing spender.
- [BLOCKING] packages/rs-platform-wallet/src/manager/history_replay.rs:325-333: Validate replay amounts before invoking the wallet checker
  (existing thread: https://github.com/dashpay/platform/pull/5220#discussion_r4246956206)
  The preflight bounds unique outpoints, but the checker aggregates account matches, and replay can temporarily duplicate an outpoint across accounts. Restore a coin worth `1_u64 << 62` in account A whose output script belongs to account B, and supply its funding transaction plus a child spending it to a 1-duff wallet output. Every individual value and preflight sum fits `i64`. Funding replay creates the coin in B without removing A's fallback; the child therefore matches the input in both accounts. In the pinned key-wallet revision, `ManagedAccountCollection::check_transaction` adds both matches into `total_sent`, yielding `1_u64 << 63`, and the wallet checker evaluates `1 - i64::MIN`. This panics with overflow checks or wraps without them. The `misplaced` cleanup occurs only after all checker calls. Normalize fallback duplicates before they can be spent during replay, or make the preflight bound actual cross-account checker operands. Add a two-account misplaced-projection regression.
- [BLOCKING] packages/rs-platform-wallet/src/manager/history_replay.rs:426-432: Avoid whole-history sweeps for every contested historical spender
  (existing thread: https://github.com/dashpay/platform/pull/5220#discussion_r4246956215)
  Removing confirmed records from the scratch map and updating competitor counts fixes the single competitor naming many historical inputs, but independent competitors still cause quadratic planning. With N confirmed winners, N separate unconfirmed competitors, and N unrelated unconfirmed bystanders, each winner remains contested until its own sweep. The pinned `drop_conflicted_transactions` scans the retained transaction map for direct losers and rebuilds its descendant index on every successful sweep, so the bystanders alone contribute N full scans. The second half of `should_plan_sweeps_only_while_a_competitor_remains` constructs this shape and permits 200 sweeps without bounding record visits. This is attacker-reachable: SPV's peer transaction handler forwards transactions to wallet relevance checking without validating their signatures or input spendability, and a matching wallet output produces a persisted record even when observed-spend guards prevent crediting it. The FFI store retains the losing rows, so the startup cost repeats across launches. Reuse an input-to-spenders index and a shared descendant worklist, or restrict each sweep to affected records while preserving finality precedence and cross-account descendants. Test total record visits for independent competitors and bystanders, not only sweep count.

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

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.

• Consolidate backend-neutral replay while preserving transactional storage repair — Outside this PR's scope: SQLite's in-transaction replay is unchanged from the base, and the PR explicitly retains that implementation and defers consolidation. The confirmed-conflict evidence limitation is separately tracked in #5379; a broader persistence redesign is not an additional finding against this change.
  • Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.
• Complete history restoration inputs on the remaining mobile paths — Outside this PR's scope: the PR explicitly documents Kotlin's null/zero history fields and the FFI decoder's empty owned-input details as deferred limitations. These are known follow-up areas, not newly undisclosed defects warranting expansion of this review.
  • Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.

[lklimek]

Cross-repo note from the review of dashpay/rust-dashcore#1336 (which adds key_wallet::transaction_checking::spend_authorization::verify_input_authorization), checked against this PR's head c2780e91e1:

1. Restored spend claims are not authorized on the generic load path. restore_spent_outpoints is called in rs-platform-wallet/src/manager/history_replay.rs:379 and rs-platform-wallet/src/manager/load.rs:288 without an owner-authorization check. The only caller of verify_input_authorization is the SQLite persister's repair step (rs-platform-wallet-storage/src/sqlite/schema/core_spend_authorization.rs:113). Any other persister therefore restores pending-spend claims unchecked, while this PR replays history "for every persister". Could the check run where the claims are built, in history_replay / load, rather than inside one storage backend?
2. Duplicate primitive. rs-platform-wallet/src/wallet/core/spend_authorization.rs is a copy of the key-wallet function. Once the rust-dashcore pin includes fix(dpp): identity constructor #1336, it can be replaced with key_wallet::transaction_checking::spend_authorization, including its SpendAuthorizationError.

Upstream follow-up idea, not blocking here: key-wallet could take authorized claims, e.g. a restore variant that receives the spending transaction and the funding TxOut, so the check cannot be skipped.

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

[thepastaclaw]

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

At ecc4921, shared replay, authorization checks, verified finality and atomic storage repair address most prior findings, but nine blocking issues remain in restore integrity, claim lifecycle, accounting persistence, legacy recovery, resource bounds and layer ownership. Current-head CI passes Kotlin build/tests but fails Storage Explorer coverage because the new InstantLock model is missing from three views; the review check remains pending. Verification was static, including inspection of the pinned and default upstream sources and the failed CI log; no builds or tests were run.

🔴 9 blocking

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

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (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); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The large, intricate diff changes spend-state reconstruction in rs-platform-wallet/src/manager/history_replay.rs and persistent-state migrations in rs-platform-wallet-storage/src/sqlite/migrations/v021.rs, directly affecting whether spent funds become available again and how stored wallet data is restored.
• 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/replay.rs`:
- [BLOCKING] packages/rs-platform-wallet-ffi/src/persistence/replay.rs:347-351: Retire restored block-spend claims when their block is reorged out
  The adapter discards StoredReplayClaim.mined_height and restores every hold through the lifetime-free restore_spent_outpoints API. Swift's loadReplayClaims exports ordinary pending-input/block-spend holds, and SQLite's load_one_wallet performs the same conversion for spent rows. In the pinned key-wallet, truncate_above removes reversible observed spends but does not retire spent_claims. Consequently, restoring a plain-block spend at height 101, truncating to 100 and redelivering its now-unspent funding can leave the output rejected by the restored claim. Narrowing irreversible_spend_claims fixed the direct replay helper, but this subsequent adapter restoration bypasses that rule. Interpret claim lifetime in the shared wallet owner and restore reversible evidence through a lifecycle-aware mechanism; cover the full backend load followed by truncation and funding redelivery.
- [BLOCKING] packages/rs-platform-wallet-ffi/src/persistence/replay.rs:327-352: Move replay repair-and-reread policy out of the FFI adapter
  replay_in_scope does more than transport callbacks and apply owner-computed effects: it chooses whether to repair and rerun replay, interprets which remaining domain outcomes invalidate restoration, filters resendable transactions and reconstructs wallet claims. replay_wallet_history_blocking does not own this complete acceptance/restoration contract, so SQLite independently reproduces it in load_one_wallet. This violates the book's 'FFI and mobile SDKs hold no business logic' rule and the explicit wrapper rule against assembling replay/state-reconstruction policy. Move the complete repair/reread/validation/claim-restoration operation into rs-platform-wallet behind a storage-adapter interface. Keep transaction-scope plumbing, C conversions, buffer ownership and application of typed storage effects in the FFI adapter.

In `packages/rs-platform-wallet/src/manager/load.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/manager/load.rs:837-841: Do not let accounting backfill overwrite a newer live accounting row
  Backfill selects unaccounted transactions and computes rows under a manager read lock, but releases that lock before this unconditional persistence round. The wallets are already published and SPV can update them. An address-pool extension or late attribution can therefore persist a more complete accounting row after computation but before the backfill write. FFIPersister::store_history_accounting uses the ordinary live accounting callback, and Swift's upsertTransactionAccounting overwrites an existing row without rechecking that it remains unaccounted. The stale backfill can replace the newer amount/direction and commit the completion marker, preserving the incorrect display across loads. Give backfill insert-if-still-unaccounted or version-checked semantics, distinct from live upserts, or fence computation through commit against live accounting updates. Add coverage for a live update committed before the stale backfill round.

In `packages/rs-platform-wallet/src/manager/history_replay.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/manager/history_replay.rs:466-467: Provide recovery for legacy InstantSend observations without lock bytes
  The required-evidence gate prevents the old silent finality downgrade, but an ordinary pre-upgrade InstantSend row without a sidecar or tracked proof has no recovery through the advertised synchronization/retry flow. The V4-to-V5 lightweight migration creates an empty PersistentCoreInstantLock table; replay_lock_candidates enumerates actual lock blobs rather than required txids, and verify_history_locks only verifies those candidates. The pinned DashSpvClient::verify_instant_lock explicitly performs no network request. Unless other stored evidence independently resolves the row, repeated loads therefore return MissingFinalityEvidence, and the wallet is never registered for normal catch-up to update its context. Distinguish missing historical proof material from a verifier that is merely not ready, and provide an explicit spend-safe recovery mode or proof-retrieval path in the Rust load owner. Cover migration and retry of a status-only legacy row without injecting proof directly.
- [BLOCKING] packages/rs-platform-wallet/src/manager/history_replay.rs:1100-1105: Allow valid stored-amount records to settle conflicts
  (existing thread: https://github.com/dashpay/platform/pull/5220#discussion_r4389526541)
  This filter excludes UnsafeAmounts::Stored records from winning conflicts even though that classification explicitly permits a valid transaction with inconsistent auxiliary amounts. A verified InstantSend spend A with an overflowing owned-input annotation can therefore fail to remove an authorized mempool competitor B and B's persisted change. A has no block_info(), so UnsafeMinedAmounts does not reject it; partitioning puts it in set_aside, and the closing sweep iterates only replay. The earlier mark_instant_send_utxos call cannot settle this case because matching records have already been detached, and upstream needs A's record to find its inputs. Resolve verified finality through topology-only settlement independently of arithmetic eligibility, or reject this snapshot explicitly before publication. Extend the unsafe-finality regression to assert removal of the mempool competitor and its outputs, not merely survival of A.
- [BLOCKING] packages/rs-platform-wallet/src/manager/history_replay.rs:907-913: Keep overlapping unsafe-record reservations releasable
  (existing thread: https://github.com/dashpay/platform/pull/5220#discussion_r4249695152)
  Two surviving Stored-only claimants still produce None, which the pinned key-wallet's release_spent_claims never releases. For A spending X/Y and B spending X/Z, a confirmed winner spending Y/Z can remove both records while the genuinely unspent X remains unavailable for the rest of the session. The current regression explicitly asserts this hold even after both records are swept. The Outputs/Stored distinction correctly prevents premature release, and rejecting the entire wallet is not required, but it does not fix the newly added reservation path's loss of availability. Add the owner-side multi-claimant lifecycle in key-wallet, or an equivalent shared-owner mechanism that retains X while any claimant survives and releases it after the last claimant disappears. The linked upstream limitation explains the missing capability; it does not make this introduced defect a safe deferral.

In `packages/swift-sdk/Sources/SwiftDashSDK/Persistence/DashModelContainer.swift`:
- [BLOCKING] packages/swift-sdk/Sources/SwiftDashSDK/Persistence/DashModelContainer.swift:98-99: Add the InstantLock model to the required Storage Explorer views
  PersistentCoreInstantLock is registered here but is absent from StorageExplorerView.swift, StorageModelListViews.swift and StorageRecordDetailViews.swift. The exact-head 'Verify explorer covers all SwiftData models' job fails for these three omissions: its log reports each missing reference and ends with 'FAILED: 3 model type(s) missing from Storage Explorer views' and exit code 1. The source and check script confirm the failure is caused by this schema addition. Add the model's explorer navigation/count, list and detail coverage, following the Transaction Accounting registration pattern.

In `packages/rs-platform-wallet-ffi/src/persistence.rs`:
- [BLOCKING] packages/rs-platform-wallet-ffi/src/persistence.rs:6095-6104: Do not turn unreadable confirmed history into missing spend evidence
  (existing thread: https://github.com/dashpay/platform/pull/5220#discussion_r4389526583)
  Malformed or identity-mismatched transaction bodies are counted and discarded while restoration succeeds. Swift's buildRecordedTransactionBuffer likewise skips rows missing bodies or confirmed block context. Independent claim loading does not make this safe: the supported no-replay-callback FFI route has no equivalent-claim requirement, and even a restored input hold cannot replace the missing confirmed transaction's conflict topology. If that row was the winner over a signed mempool competitor, replay can retain the competitor's persisted change because it never sees the winner. Return a typed corrupt/incomplete-snapshot result before publication, or carry explicit incomplete-history information into the shared wallet owner's recovery policy; do not represent unreadable evidence as absent history.

In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- [BLOCKING] packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift:7464-7475: Bound the attacker-amplifiable full-history replay during wallet load
  (existing thread: https://github.com/dashpay/platform/pull/5220#discussion_r4265416171)
  Startup still materializes the complete transaction table and prefetched relationships, filters them for each wallet, and copies selected bodies across FFI without an aggregate count/byte budget. Candidate discovery, initial snapshot loading and authoritative replay repeat history reads, while Rust clones the supplied history and a repair can require another replay. Valid payments to monitored addresses can grow this workload without the user's participation, eventually exhausting memory or monopolizing startup on every launch. The indexed planner, atomic transaction scope and per-claim buffer limit do not bound this initial allocation. Your objection to fetchLimit is correct: truncating history would discard spend evidence. Establish an early resource preflight with bounded allocations on both sides of FFI and an actionable recovery outcome, or bounded resumable replay backed by durable spend guards/checkpoints.

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

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.

• Complete owner-side checked accounting for live and late-funding paths — Both the pinned and default key-wallet sources still contain unchecked signed accounting outside this PR's guarded replay. This pre-existing live-path exposure remains important, but the requested ownership follow-up is already tracked in dashpay/rust-dashcore#1213.
  • Follow-up: Implement the checked, atomic accounting and topology-preserving restoration API in dashpay/rust-dashcore#1213, then replace downstream defensive sanitization with that owner API.
• Close the deferred owned-input finalization window — The recorded-history ABI still omits owned-input details. A plain-block spend with missing funding can lose provisional protection when its observed evidence is pruned at finality. This window predates the PR, is narrowed by replay and has an explicit tracked deferral in #5412.
  • Follow-up: Resolve #5412 in a follow-up PR by transporting owned-input details or promoting provisional evidence at finality while retiring it on reorg.

[thepastaclaw]

🔴 Blocking: Retire restored block-spend claims when their block is reorged out

The adapter discards StoredReplayClaim.mined_height and restores every hold through the lifetime-free restore_spent_outpoints API. Swift's loadReplayClaims exports ordinary pending-input/block-spend holds, and SQLite's load_one_wallet performs the same conversion for spent rows. In the pinned key-wallet, truncate_above removes reversible observed spends but does not retire spent_claims. Consequently, restoring a plain-block spend at height 101, truncating to 100 and redelivering its now-unspent funding can leave the output rejected by the restored claim. Narrowing irreversible_spend_claims fixed the direct replay helper, but this subsequent adapter restoration bypasses that rule. Interpret claim lifetime in the shared wallet owner and restore reversible evidence through a lifecycle-aware mechanism; cover the full backend load followed by truncation and funding redelivery.

_{source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, ffi-engineer, platform-versioning, rust-quality, security-auditor)}

[thepastaclaw]

🔴 Blocking: Do not let accounting backfill overwrite a newer live accounting row

Backfill selects unaccounted transactions and computes rows under a manager read lock, but releases that lock before this unconditional persistence round. The wallets are already published and SPV can update them. An address-pool extension or late attribution can therefore persist a more complete accounting row after computation but before the backfill write. FFIPersister::store_history_accounting uses the ordinary live accounting callback, and Swift's upsertTransactionAccounting overwrites an existing row without rechecking that it remains unaccounted. The stale backfill can replace the newer amount/direction and commit the completion marker, preserving the incorrect display across loads. Give backfill insert-if-still-unaccounted or version-checked semantics, distinct from live upserts, or fence computation through commit against live accounting updates. Add coverage for a live update committed before the stale backfill round.

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

[thepastaclaw]

🔴 Blocking: Provide recovery for legacy InstantSend observations without lock bytes

The required-evidence gate prevents the old silent finality downgrade, but an ordinary pre-upgrade InstantSend row without a sidecar or tracked proof has no recovery through the advertised synchronization/retry flow. The V4-to-V5 lightweight migration creates an empty PersistentCoreInstantLock table; replay_lock_candidates enumerates actual lock blobs rather than required txids, and verify_history_locks only verifies those candidates. The pinned DashSpvClient::verify_instant_lock explicitly performs no network request. Unless other stored evidence independently resolves the row, repeated loads therefore return MissingFinalityEvidence, and the wallet is never registered for normal catch-up to update its context. Distinguish missing historical proof material from a verifier that is merely not ready, and provide an explicit spend-safe recovery mode or proof-retrieval path in the Rust load owner. Cover migration and retry of a status-only legacy row without injecting proof directly.

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

[thepastaclaw]

🔴 Blocking: Move replay repair-and-reread policy out of the FFI adapter

replay_in_scope does more than transport callbacks and apply owner-computed effects: it chooses whether to repair and rerun replay, interprets which remaining domain outcomes invalidate restoration, filters resendable transactions and reconstructs wallet claims. replay_wallet_history_blocking does not own this complete acceptance/restoration contract, so SQLite independently reproduces it in load_one_wallet. This violates the book's 'FFI and mobile SDKs hold no business logic' rule and the explicit wrapper rule against assembling replay/state-reconstruction policy. Move the complete repair/reread/validation/claim-restoration operation into rs-platform-wallet behind a storage-adapter interface. Keep transaction-scope plumbing, C conversions, buffer ownership and application of typed storage effects in the FFI adapter.

_{source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, ffi-engineer, platform-versioning, rust-quality, security-auditor)}

[thepastaclaw]

🔴 Blocking: Add the InstantLock model to the required Storage Explorer views

PersistentCoreInstantLock is registered here but is absent from StorageExplorerView.swift, StorageModelListViews.swift and StorageRecordDetailViews.swift. The exact-head 'Verify explorer covers all SwiftData models' job fails for these three omissions: its log reports each missing reference and ends with 'FAILED: 3 model type(s) missing from Storage Explorer views' and exit code 1. The source and check script confirm the failure is caused by this schema addition. Add the model's explorer navigation/count, list and detail coverage, following the Transaction Accounting registration pattern.

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