fix(platform-wallet): reconcile send_payment's reservation, and carry suppressed spends across the FFI

packages/rs-platform-wallet-ffi/src/core_wallet_types.rs

@@ -254,6 +254,23 @@ impl WalletChangeSetFFI {
     /// (the records are authoritative), and re-deriving keeps the
     /// per-account routing self-contained.
     ///
+    /// That redundancy stopped holding for spends once
+    /// dashpay/platform#4363 began *suppressing* a contact's watch-only
+    /// record from `records` while still emitting its spend — the
+    /// stale-TXO heal. With nothing left in `records` to re-derive from,
+    /// those spends reached no mobile host at all; only the SQLite backend,
+    /// which reads `spent_utxos` directly, ever ran the heal.
+    /// `CoreChangeSet::unrecorded_spends` carries exactly that residue —
+    /// outpoint, spending txid and owning account — and is folded into the
+    /// per-account `utxos_spent` arrays below.
+    ///
+    /// That closes the gap *at this boundary only*. The entries now cross
+    /// the FFI, but neither host acts on them yet: both gate the `isSpent`
+    /// flip on resolving `spending_txid` to a persisted transaction row, and
+    /// for a suppressed record no such row is ever written. Do not read the
+    /// fold below as "the heal runs on mobile" — see
+    /// `CoreChangeSet::unrecorded_spends` for what still has to land.
+    ///
     /// `chain` carries `synced_height` from the changeset's
     /// `synced_height` field; `block_hash` is omitted because
     /// `WalletEvent::SyncHeightAdvanced` doesn't carry it (the upstream
@@ -351,6 +368,17 @@ impl WalletChangeSetFFI {
             }
         }
 
+        // Same for an account carrying only suppressed spend-clears. This is
+        // the common shape of the #4363 heal: a contact spends a stale row
+        // and their watch-only record — the batch's only record for that
+        // account — is dropped, so the bucket would not exist at all and the
+        // spend would have nowhere to be emitted.
+        for spend in &cs.unrecorded_spends {
+            if !by_account.iter().any(|(at, _)| at == &spend.account_type) {
+                by_account.push((spend.account_type, Vec::new()));
+            }
+        }
+
         let mut ffi_accounts = Vec::with_capacity(by_account.len());
         for (account_type, recs) in by_account {
             let type_name = CString::new(format!("{:?}", account_type))
@@ -367,6 +395,16 @@ impl WalletChangeSetFFI {
                 utxos_added.extend(record_new_utxos_ffi(rec));
                 utxos_spent.extend(record_spent_outpoints_ffi(rec));
             }
+            // Fold in the spends whose record never made it into `records`.
+            // Disjoint from the loop above by construction — `unrecorded_spends`
+            // is populated only for records that are suppressed — so no entry
+            // is emitted twice.
+            utxos_spent.extend(
+                cs.unrecorded_spends
+                    .iter()
+                    .filter(|spend| spend.account_type == account_type)
+                    .map(unrecorded_spend_ffi),
+            );
 
             // Transactions for this account.
             let transactions: Vec<TransactionRecordFFI> =
@@ -882,6 +920,36 @@ fn record_spent_outpoints_ffi(
         .collect()
 }
 
+/// Project an [`UnrecordedSpend`] — a spend whose record was suppressed from
+/// the changeset's `records` — into the same wire shape
+/// [`record_spent_outpoints_ffi`] produces.
+///
+/// The spending txid is carried through rather than zeroed: both host
+/// handlers resolve it to decide whether the spend is final, and a zero txid
+/// makes the emit a no-op on each of them.
+///
+/// A non-zero txid is not sufficient either, and today it is not: the hosts
+/// look the txid up in their own transaction table, and a suppressed record
+/// never puts it there, so these entries currently land and do nothing. The
+/// txid is carried because it is the one fact that cannot be recovered
+/// downstream once the record is gone — see
+/// `CoreChangeSet::unrecorded_spends`.
+///
+/// [`UnrecordedSpend`]: platform_wallet::changeset::UnrecordedSpend
+fn unrecorded_spend_ffi(spend: &platform_wallet::changeset::UnrecordedSpend) -> SpentOutPointFFI {
+    let mut txid = [0u8; 32];
+    txid.copy_from_slice(spend.outpoint.txid.as_ref());
+    let mut spending_txid = [0u8; 32];
+    spending_txid.copy_from_slice(spend.spending_txid.as_ref());
+    SpentOutPointFFI {
+        outpoint: OutPointFFI {
+            txid,
+            vout: spend.outpoint.vout,
+        },
+        spending_txid,
+    }
+}
+
 /// Map upstream `TransactionType` to a stable `u8` discriminant for
 /// the FFI wire shape. Order mirrors the enum declaration in
 /// `key_wallet::transaction_checking::transaction_router::mod.rs`,
@@ -1832,6 +1900,110 @@ mod tests {
         unsafe { free_wallet_changeset_ffi(&ffi) };
     }
 
+    /// A contact spending a stale pre-#4363 TXO must cross the FFI as a
+    /// spend-clear, carrying the spending txid.
+    ///
+    /// This is the changeset shape the #4363 heal produces: the contact's
+    /// watch-only record is suppressed from `records` (it is not a
+    /// transaction of ours), so the batch has **no record at all** for that
+    /// account — and `from_changeset` built its buckets, and derived every
+    /// spend, from `records` alone. The clear reached the SQLite backend
+    /// (which reads `cs.spent_utxos` directly) and no mobile host.
+    ///
+    /// Pre-fix this emitted zero accounts and zero spends.
+    ///
+    /// Scope: this pins the *FFI boundary*, not the heal. Both host handlers
+    /// still require a persisted row for `spending_txid` before they touch
+    /// `isSpent`, and a suppressed record produces none — so what this test
+    /// asserts is that the entry arrives with everything a host would need,
+    /// not that any host acts on it today.
+    #[test]
+    fn contact_spend_of_a_stale_txo_crosses_the_ffi() {
+        use dashcore::hashes::Hash;
+        use platform_wallet::changeset::UnrecordedSpend;
+
+        let account = AccountType::DashpayExternalAccount {
+            index: 0,
+            user_identity_id: [1u8; 32],
+            friend_identity_id: [2u8; 32],
+        };
+        let stale_txid = dashcore::Txid::from_slice(&[0x11; 32]).expect("txid");
+        let spending_txid = dashcore::Txid::from_slice(&[0x22; 32]).expect("txid");
+
+        let mut cs = CoreChangeSet::default();
+        cs.unrecorded_spends.push(UnrecordedSpend {
+            outpoint: dashcore::OutPoint {
+                txid: stale_txid,
+                vout: 1,
+            },
+            spending_txid,
+            account_type: account,
+        });
+
+        let ffi = WalletChangeSetFFI::from_changeset(&cs);
+        assert_eq!(
+            ffi.accounts_count, 1,
+            "the suppressed record's account must still get a bucket to carry \
+             the spend-clear"
+        );
+        let bucket = unsafe { &*ffi.accounts };
+        assert_eq!(
+            bucket.transactions_count, 0,
+            "the contact's spend is still not a transaction row of ours"
+        );
+        assert_eq!(
+            bucket.utxos_spent_count, 1,
+            "the stale TXO's spend-clear must cross the FFI"
+        );
+        let spent = unsafe { &*bucket.utxos_spent };
+        assert_eq!(spent.outpoint.txid, stale_txid.to_byte_array());
+        assert_eq!(spent.outpoint.vout, 1);
+        assert_eq!(
+            spent.spending_txid,
+            spending_txid.to_byte_array(),
+            "the spending txid must survive — both host handlers resolve it to \
+             decide the spend is final, and a zero txid makes the emit a no-op"
+        );
+        unsafe { free_wallet_changeset_ffi(&ffi) };
+    }
+
+    /// The projection must not double-count: an account that has both a
+    /// surviving record and (from another record) a suppressed spend emits
+    /// each spend once. `unrecorded_spends` is populated only for suppressed
+    /// records, so the two sources are disjoint by construction — this pins
+    /// that they stay so.
+    #[test]
+    fn unrecorded_spends_do_not_duplicate_record_derived_spends() {
+        use dashcore::hashes::Hash;
+        use platform_wallet::changeset::UnrecordedSpend;
+
+        let account = AccountType::DashpayExternalAccount {
+            index: 0,
+            user_identity_id: [1u8; 32],
+            friend_identity_id: [2u8; 32],
+        };
+        let mut cs = CoreChangeSet::default();
+        for vout in 0..3u32 {
+            cs.unrecorded_spends.push(UnrecordedSpend {
+                outpoint: dashcore::OutPoint {
+                    txid: dashcore::Txid::from_slice(&[0x33; 32]).expect("txid"),
+                    vout,
+                },
+                spending_txid: dashcore::Txid::from_slice(&[0x44; 32]).expect("txid"),
+                account_type: account,
+            });
+        }
+
+        let ffi = WalletChangeSetFFI::from_changeset(&cs);
+        assert_eq!(ffi.accounts_count, 1, "all three share one account bucket");
+        let bucket = unsafe { &*ffi.accounts };
+        assert_eq!(
+            bucket.utxos_spent_count, 3,
+            "each suppressed spend emits exactly once"
+        );
+        unsafe { free_wallet_changeset_ffi(&ffi) };
+    }
+
     /// The `has_*` flags stay false (values -1) for accounts with
     /// records but no watermark update this batch, so the Swift
     /// persister never regresses a stored value on a no-usage round.

packages/rs-platform-wallet/src/changeset/changeset.rs

@@ -58,6 +58,26 @@ use crate::wallet::identity::{
 // Core wallet changeset — projection of upstream `WalletEvent` data
 // ---------------------------------------------------------------------------
 
+/// One outpoint whose spend must still be persisted even though the record
+/// that spent it never reaches the persister's `records` list.
+///
+/// See [`CoreChangeSet::unrecorded_spends`] for why this exists, why the
+/// spending txid cannot be recovered downstream, and which backends act on
+/// it today.
+#[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+pub struct UnrecordedSpend {
+    /// The outpoint being spent — the stale row to clear.
+    pub outpoint: OutPoint,
+    /// Txid of the transaction that spends it. Both host persisters resolve
+    /// this to decide whether the spend is final, so it must survive.
+    pub spending_txid: Txid,
+    /// The account whose (suppressed) record spent the outpoint. Used only
+    /// to route the entry into a per-account bucket on the FFI surface;
+    /// both host handlers resolve the TXO by outpoint, wallet-wide.
+    pub account_type: AccountType,
+}
+
 /// Platform-owned projection of the core-wallet deltas that upstream's
 /// `WalletEvent` bus delivers.
 ///
@@ -104,6 +124,47 @@ pub struct CoreChangeSet {
     /// `OutputRole::Change` per the upstream `TransactionRecord`).
     pub new_utxos: Vec<Utxo>,
 
+    /// Spend-clears whose spending record was deliberately **suppressed**
+    /// from [`Self::records`] — a contact's watch-only chain (see
+    /// `core_bridge`'s `is_contact_watch_only`, dashpay/platform#4363).
+    ///
+    /// [`Self::spent_utxos`] already carries these outpoints, and the
+    /// backend that consumes it directly (SQLite) needs nothing from this
+    /// field — the #4363 heal is live there. The FFI persister is the gap:
+    /// it derives its per-account spend lists from `records`, so a spend
+    /// whose only record was suppressed had nothing left to be derived
+    /// from and produced no account bucket and no spend entry at all.
+    ///
+    /// Reconstructing it from `spent_utxos` alone is not possible: a
+    /// [`Utxo`] carries no spending txid, and both host handlers key the
+    /// `isSpent` flip on resolving the spending transaction. So this
+    /// carries the two things record-suppression destroys — *which*
+    /// transaction did the spending, and *which* account's record it was —
+    /// leaving `spent_utxos` and its existing consumers untouched.
+    ///
+    /// # Status: crosses the FFI, inert on Android/iOS today
+    ///
+    /// This field closes the *Rust-side* gap only. Both mobile handlers
+    /// resolve `spending_txid` to a persisted transaction row before they
+    /// will touch `isSpent` (`getByTxid(spendingTxid)` on Android, the
+    /// `PersistentTransaction` fetch on iOS), and for a suppressed record
+    /// that row is precisely what never gets written. So a pure
+    /// contact → third-party spend still does not flip `isSpent` on either
+    /// host: the entry arrives, the lookup misses, and the handler leaves
+    /// the row alone. The mixed case — the contact spends the stale coin in
+    /// a transaction that also carries a surviving record of ours — is
+    /// already healed without this field, because that record's FFI emit
+    /// carries `input_outpoints` for *every* input of the transaction and
+    /// the hosts reconcile the spend from there.
+    ///
+    /// The plumbing stays because it is the half that cannot be done
+    /// downstream, and it becomes load-bearing the moment either of the two
+    /// planned pieces lands: a host-visible "this spend is final, no
+    /// transaction row is coming" contract on `SpentOutPointFFI`, or the
+    /// store-reconciliation pass that heals stale rows out of band. Until
+    /// then, do not describe the mobile heal as working.
+    pub unrecorded_spends: Vec<UnrecordedSpend>,
+
     /// InstantSend locks observed for records that are NOT yet in a
     /// chain-locked block (i.e. records still in `Mempool`,
     /// `InstantSend`, or `InBlock` context — anything `InChainLockedBlock`
@@ -240,6 +301,7 @@ impl Merge for CoreChangeSet {
         self.records.extend(other.records);
         self.spent_utxos.extend(other.spent_utxos);
         self.new_utxos.extend(other.new_utxos);
+        self.unrecorded_spends.extend(other.unrecorded_spends);
 
         // IS-lock map: last-write-wins per txid. A second IS-lock for
         // the same txid (e.g. a follow-up event re-confirming the lock)
@@ -338,6 +400,7 @@ impl Merge for CoreChangeSet {
         self.records.is_empty()
             && self.spent_utxos.is_empty()
             && self.new_utxos.is_empty()
+            && self.unrecorded_spends.is_empty()
             && self.instant_locks_for_non_final_records.is_empty()
             && self.last_processed_height.is_none()
             && self.synced_height.is_none()