fix(platform-wallet-storage): durably apply swept transactions in the SQLite store (#4559)

Author: romchornyi

packages/rs-platform-wallet-storage/SCHEMA.md

@@ -90,10 +90,11 @@ erDiagram
         BLOB outpoint PK "bincode-encoded OutPoint"
         INTEGER value "satoshis"
         BLOB script "scriptPubKey bytes"
-        INTEGER height "NULL if unconfirmed"
+        INTEGER height "funding height, 0 if unconfirmed; NULL only for an unmaterialised sweep placeholder"
         INTEGER account_index
         INTEGER spent "0 | 1"
-        BLOB spent_in_txid "NULL until spend; cleared by trigger on tx delete"
+        BLOB spent_in_txid "set by apply_sweep for an unresolved held input; else NULL"
+        INTEGER winner_mined_height "V007: sweep winner's mined height; NULL when unstamped or materialised"
     }
 
     CORE_INSTANT_LOCKS {
@@ -115,6 +116,7 @@ erDiagram
         BLOB wallet_id PK "one row per wallet"
         INTEGER last_processed_height "NULL until first block processed"
         INTEGER synced_height "NULL until first sync"
+        INTEGER chainlock_height "V007: monotonic-max applied chainlock height; NULL until one is applied"
     }
 ```
 
@@ -381,14 +383,44 @@ is `1` once block context is present.
 
 ### `core_utxos`
 
-One row per UTXO, spent or unspent. `spent_in_txid` is set to NULL
-by a trigger when its referenced `core_transactions` row is deleted
-(instead of a native `ON DELETE SET NULL`, which would also null the
-NOT NULL `wallet_id` column).
+One row per UTXO, spent or unspent. `spent_in_txid` is written only by
+`apply_sweep`, naming the winner that took an input a swept loser claimed
+but this store had no released record for. It is set to NULL by a trigger
+when its referenced `core_transactions` row is deleted (instead of a native
+`ON DELETE SET NULL`, which would also null the NOT NULL `wallet_id`
+column) — and by a later sweep that releases the same outpoint.
+
+What gates the funding UTXO's own later upsert (`execute_upsert_utxo`) is
+the row's shape, not that link: a never-materialised held row (`height`
+NULL, `spent = 1` — the placeholder `apply_sweep` writes for an input whose
+funding this store had not seen) stays spent when the funding arrives, with
+or without a `spent_in_txid` (the trigger can null it underneath a live
+hold). A materialised row follows the wallet: it knows the coin, any
+network-final spender of a coin it knows is wallet-relevant, so its view of
+`spent` is authoritative and a re-delivery clears both `spent` and the
+link. A delivery through `spent_utxos` onto a placeholder materialises it
+the same way instead of marking it in place.
+
+`winner_mined_height` (V007) stamps that claim with the mined height of the
+winner named in `spent_in_txid`, and decides the placeholder's lifetime
+rather than its existence. A block-context sweep stamps the winner's own
+height and `collect_finalized_tombstones` evicts the row once
+`min(chainlock_height, synced_height)` reaches it — upstream's
+`prune_finalized_observed_spends` boundary verbatim. An InstantSend-locked
+winner that is not yet mined leaves it NULL: the lock alone settles the
+input, but it carries no height to key a lifetime on, so the row resolves
+only through proof (the funding upsert materialising it, a later
+block-context sweep re-stamping it, or a release). The funding upsert
+clears the stamp, because a materialised row is the wallet's own coin held
+spent and is permanently outside the collector's reach.
 
 - PK: `(wallet_id, outpoint)`.
 - FK: `wallet_id → wallet_metadata(wallet_id) ON DELETE CASCADE`.
 - Index: `idx_core_utxos_spent(wallet_id, spent)`.
+- Index: `idx_core_utxos_unmaterialized(wallet_id, winner_mined_height)
+  WHERE height IS NULL` (V007) — covers exactly the unmaterialised rows, so
+  the collector's per-round scan touches tombstones rather than the
+  wallet's full spent history.
 
 ### `core_instant_locks`
 
@@ -413,6 +445,13 @@ One row per wallet, holding monotonically-advancing SPV sync watermarks.
 `last_processed_height` and `synced_height` are NULL until the first
 block is processed.
 
+`chainlock_height` (V007) mirrors `CoreChangeSet::last_applied_chain_lock`
+as a monotonic max — the height alone, which this store previously dropped.
+It is one half of the finality boundary
+`collect_finalized_tombstones` collects sweep tombstones against, so a
+tombstone is never collected before a chainlock has been persisted,
+matching upstream's "no-op until a chainlock has been applied".
+
 - PK: `wallet_id` (single-row-per-wallet).
 - FK: `wallet_id → wallet_metadata(wallet_id) ON DELETE CASCADE`.
 

packages/rs-platform-wallet-storage/migrations/V007__utxo_sweep_winner_height.rs

@@ -0,0 +1,53 @@
+//! Anchor sweep tombstones at their winner's mined height and pin the
+//! chainlock finality boundary.
+//!
+//! `core_utxos.winner_mined_height` is the mined height of the
+//! transaction that beat an unmaterialised sweep tombstone's outpoint —
+//! the placeholder row `apply_sweep` writes for a held input whose
+//! funding output has never classified (`height IS NULL AND spent = 1`;
+//! no other writer leaves `height` NULL). The height is carried on the
+//! sweep event itself (`TransactionsSwept::winner_mined_height`), and
+//! `apply_sweep` writes the placeholder for EVERY non-released held
+//! input in EVERY sweep context — only the stamp differs. A
+//! block-context sweep (winner actually mined) stamps the winner's
+//! height, and the collector in `core_state::apply` evicts the row
+//! exactly when `min(chainlock_height, synced_height)` reaches that
+//! height — `prune_finalized_observed_spends`' condition verbatim, with
+//! no observation-age margin. An InstantSend-locked, unmined winner
+//! writes the same row with the stamp NULL — under DIP-10 its lock
+//! alone settles the input, but it carries no height to key a lifetime
+//! on — and the collector never takes an unstamped row: it resolves
+//! only through proof, when the funding upsert materialises it, a later
+//! block-context sweep re-stamps it into the collectible set, or a
+//! release deletes it. See `CORE_SWEEP_REMOVAL` and the `apply_sweep`
+//! doc in `core_state.rs` for why an unstamped hold must survive (it is
+//! the only durable carrier of upstream's in-memory `spent_outpoints`
+//! hold across a restart) and what bounds the foreign-input residue.
+//!
+//! `core_sync_state.chainlock_height` is the monotonic-max height of
+//! the last applied chainlock, mirrored from
+//! `CoreChangeSet::last_applied_chain_lock` (previously dropped by
+//! this store). It is one half of the collector's finality boundary;
+//! rows are never collected before a chainlock has been persisted,
+//! matching upstream's "no-op until a chainlock has been applied".
+//!
+//! The partial index covers exactly the unmaterialised rows — the
+//! collector's scan set is the stamped subset of these — so the
+//! per-round sweep touches tombstones only, not the wallet's full
+//! spent history.
+//!
+//! Version numbers, like capability bits, are append-only: an already-
+//! merged assignment keeps its slot, so an unmerged migration takes the
+//! next free number whenever the mainline lands one ahead of it. Under
+//! the pre-release policy V001's test documents, an unshipped migration
+//! may still be edited in place; a dev database that applied an earlier
+//! shape fails refinery's divergence check and must be recreated.
+
+pub fn migration() -> String {
+    "ALTER TABLE core_utxos ADD COLUMN winner_mined_height INTEGER;
+     ALTER TABLE core_sync_state ADD COLUMN chainlock_height INTEGER;
+     CREATE INDEX idx_core_utxos_unmaterialized
+         ON core_utxos(wallet_id, winner_mined_height)
+         WHERE height IS NULL;"
+        .to_string()
+}

packages/rs-platform-wallet-storage/src/sqlite/persister.rs

@@ -829,6 +829,11 @@ impl PlatformWalletPersistence for SqlitePersister {
         // Do NOT attest WALLET_RESTORE (and therefore not provider restore):
         // `load()` still reports `ClientStartState::wallets` in
         // `LOAD_UNIMPLEMENTED`. Shielded state lives in a separate store.
+        // `CORE_SWEEP_REMOVAL`: the full contract — loser removal, the
+        // outpoint-keyed placeholder for a held input whose funding has not
+        // classified, releases by outpoint, and the finality-boundary
+        // collector — is implemented and documented in `core_state::apply`,
+        // `apply_sweep` and `collect_finalized_tombstones`.
         PersistenceCapabilities::ATOMIC_CHANGESETS
             .union(PersistenceCapabilities::INVITATIONS)
             .union(PersistenceCapabilities::ASSET_LOCK_FUNDING_INDICES)
@@ -837,6 +842,8 @@ impl PlatformWalletPersistence for SqlitePersister {
             .union(PersistenceCapabilities::DPNS_NAME_STATES)
             .union(PersistenceCapabilities::TRACKED_ASSET_LOCKS)
             .union(PersistenceCapabilities::TRACKED_MASTERNODES)
+            .union(PersistenceCapabilities::CORE_SWEEP_REMOVAL)
+            .union(PersistenceCapabilities::DASHPAY_PAYMENTS)
     }
 
     fn persist_tracked_masternodes(

packages/rs-platform-wallet-storage/src/sqlite/schema/asset_locks.rs

@@ -73,10 +73,21 @@ pub fn apply(
     if !cs.removed.is_empty() {
         // Same terminal rule as the upsert guard: a stored `consumed`
         // row is never deleted by a stale tombstone. Consumed rows are
-        // deliberately retained for historical lookup, and the only
-        // removal emitter (`untrack_asset_lock`) fires exclusively for
-        // Built rows whose broadcast was rejected — so a removal
-        // reaching a consumed row is by construction a stale write.
+        // deliberately retained for historical lookup, and neither
+        // removal producer can legitimately name one — a Built row
+        // rejected at broadcast (`untrack_asset_lock`) never got that
+        // far, and the sweep-driven removal that arrives with the
+        // producer only tombstones entries still tracked, which a
+        // consumed lock no longer is — so a removal reaching a consumed
+        // row is by construction a stale write.
+        //
+        // Ordering note, scoped honestly: this applies upserts before
+        // removals. On this branch that is layout rather than a
+        // guarantee — the fold-level cancellation that would make an
+        // upsert and a tombstone for one outpoint impossible lands with
+        // the producer, and even there it does not cover every status.
+        // The `status != 'consumed'` predicate is what this statement
+        // actually relies on, and it holds regardless of the fold.
         let mut stmt = tx.prepare_cached(
             "DELETE FROM asset_locks \
              WHERE wallet_id = ?1 AND outpoint = ?2 AND status != 'consumed'",