feat(key-wallet-manager): carry the sweep winner's mined height on TransactionsSwept

dash-spv-ffi/src/callbacks.rs

@@ -797,6 +797,14 @@ impl FFIOutPoint {
 /// be wallet-relevant at all (it can spend our coin while paying only
 /// external addresses), so it may never appear in any other callback. Null
 /// with a zero count when the removal released nothing.
+///
+/// The Rust `WalletEvent::TransactionsSwept` additionally carries
+/// `winner_mined_height` (the winner's block height, `None` for an
+/// InstantSend-locked winner not yet mined). It is deliberately not
+/// forwarded here: this signature has no safely-degrading insertion point
+/// for hand-declared C consumers (see the released-outpoints addition),
+/// so growing it is a flagged breaking change deferred until a C consumer
+/// needs the field.
 /// All pointer parameters are borrowed and only valid for the duration of the
 /// callback. `balance` is the wallet's balance *after* the removal;
 /// `account_balances` follows the same contract as on
@@ -1091,6 +1099,16 @@ impl FFIWalletEventCallbacks {
                 wallet_id,
                 txids,
                 superseded_by,
+                // Deliberately not forwarded across the C ABI. #962
+                // established that `OnTransactionsSweptCallback` has no
+                // safely-degrading insertion point: a C consumer declaring
+                // the function pointer by hand keeps compiling against a
+                // grown parameter list and reads shifted arguments. Rust
+                // consumers of `WalletEvent` get the field directly; the C
+                // surface stays byte-for-byte identical until a consumer
+                // needs it there, at which point the addition must be a
+                // flagged breaking change like #962's.
+                winner_mined_height: _,
                 released_outpoints,
                 balance,
                 account_balances,
@@ -1398,6 +1416,7 @@ mod tests {
             wallet_id: [7u8; 32],
             txids: vec![Txid::from_byte_array([1u8; 32])],
             superseded_by: Txid::from_byte_array([2u8; 32]),
+            winner_mined_height: Some(1_000),
             released_outpoints: Vec::new(),
             balance: WalletCoreBalance::default(),
             account_balances: BTreeMap::new(),
@@ -1445,6 +1464,7 @@ mod tests {
             wallet_id: [7u8; 32],
             txids: vec![Txid::from_byte_array([1u8; 32])],
             superseded_by: Txid::from_byte_array([2u8; 32]),
+            winner_mined_height: None,
             released_outpoints: vec![
                 dashcore::OutPoint {
                     txid: parent,

key-wallet-manager/src/event_tests.rs

@@ -625,17 +625,133 @@ async fn test_block_winner_emits_swept_event_naming_the_released_outpoints() {
                 wallet_id: wid,
                 txids,
                 superseded_by,
+                winner_mined_height,
                 released_outpoints,
                 ..
-            } => Some((wid, txids, superseded_by, released_outpoints)),
+            } => Some((wid, txids, superseded_by, winner_mined_height, released_outpoints)),
             _ => None,
         })
         .unwrap_or_else(|| panic!("a sweep must be emitted, got {:?}", events));
 
     assert_eq!(swept.0, &wallet_id);
     assert_eq!(swept.1, &vec![loser.txid()], "the beaten transaction is named");
     assert_eq!(swept.2, &winner.txid(), "attributed to the transaction that beat it");
-    assert_eq!(swept.3, &vec![coin_b], "only the coin the winner did not take is released");
+    assert_eq!(
+        swept.3,
+        &Some(101),
+        "a block-context sweep must carry the winner's mined height — the height of \
+         the block whose processing triggered the sweep, not None and not any other \
+         height the manager has seen"
+    );
+    assert_eq!(swept.4, &vec![coin_b], "only the coin the winner did not take is released");
+}
+
+/// The mempool emission site's counterpart to the block test above, pinning
+/// the other half of `winner_mined_height`'s contract: a sweep triggered by
+/// an InstantSend-locked winner that has not been mined carries `None`. This
+/// is the distinction the field exists to make — a consumer retiring durable
+/// records of the swept spends can anchor a block-context sweep to the
+/// winner's height, while an IS-locked winner has no mining deadline, so
+/// conflating the two (a height where there is none, or vice versa) would
+/// let those records be retired while the conflict is still unmined.
+#[tokio::test]
+async fn test_is_locked_mempool_winner_sweeps_with_no_mined_height() {
+    let (mut manager, wallet_id, addr) = setup_manager_with_wallet();
+
+    // Same shape as the block test: one funding transaction pays us twice so
+    // the loser spends a coin the winner does not.
+    let funding = Transaction {
+        version: 2,
+        lock_time: 0,
+        input: vec![TxIn {
+            previous_output: OutPoint {
+                txid: Txid::from_byte_array([0x6a; 32]),
+                vout: 0,
+            },
+            script_sig: ScriptBuf::new(),
+            sequence: u32::MAX,
+            witness: Witness::default(),
+        }],
+        output: vec![
+            TxOut {
+                value: 500_000,
+                script_pubkey: addr.script_pubkey(),
+            },
+            TxOut {
+                value: 400_000,
+                script_pubkey: addr.script_pubkey(),
+            },
+        ],
+        special_transaction_payload: None,
+    };
+    let funding_block = make_block(vec![funding.clone()], 0x6a, 2000);
+    let wallets = BTreeSet::from([wallet_id]);
+    manager
+        .process_block_for_wallets(&funding_block, funding_block.block_hash(), 100, &wallets)
+        .await;
+
+    let coin_a = OutPoint {
+        txid: funding.txid(),
+        vout: 0,
+    };
+    let coin_b = OutPoint {
+        txid: funding.txid(),
+        vout: 1,
+    };
+    let spend = |inputs: Vec<OutPoint>, value: u64| Transaction {
+        version: 2,
+        lock_time: 0,
+        input: inputs
+            .into_iter()
+            .map(|previous_output| TxIn {
+                previous_output,
+                script_sig: ScriptBuf::new(),
+                sequence: u32::MAX,
+                witness: Witness::default(),
+            })
+            .collect(),
+        output: vec![TxOut {
+            value,
+            script_pubkey: addr.script_pubkey(),
+        }],
+        special_transaction_payload: None,
+    };
+
+    let loser = spend(vec![coin_a, coin_b], 800_000);
+    manager.process_mempool_transaction(&loser, None).await;
+
+    let mut rx = manager.subscribe_events();
+
+    // The winner arrives off-chain with an InstantSend lock — final enough
+    // to sweep the loser, but not mined anywhere.
+    let winner = spend(vec![coin_a], 400_000);
+    manager.process_mempool_transaction(&winner, Some(dummy_instant_lock(winner.txid()))).await;
+
+    let events = drain_events(&mut rx);
+    let swept = events
+        .iter()
+        .find_map(|event| match event {
+            WalletEvent::TransactionsSwept {
+                wallet_id: wid,
+                txids,
+                superseded_by,
+                winner_mined_height,
+                released_outpoints,
+                ..
+            } => Some((wid, txids, superseded_by, winner_mined_height, released_outpoints)),
+            _ => None,
+        })
+        .unwrap_or_else(|| panic!("a sweep must be emitted, got {:?}", events));
+
+    assert_eq!(swept.0, &wallet_id);
+    assert_eq!(swept.1, &vec![loser.txid()], "the beaten transaction is named");
+    assert_eq!(swept.2, &winner.txid(), "attributed to the transaction that beat it");
+    assert_eq!(
+        swept.3, &None,
+        "an IS-locked winner is not mined: fabricating a height here would give the \
+         consumer a finality horizon the winner does not have"
+    );
+    assert_eq!(swept.4, &vec![coin_b], "only the coin the winner did not take is released");
 }
 
 #[tokio::test]

key-wallet-manager/src/events.rs

@@ -238,6 +238,25 @@ pub enum WalletEvent {
         txids: Vec<Txid>,
         /// The transaction whose arrival settled the inputs, for provenance.
         superseded_by: Txid,
+        /// Mined height of `superseded_by` when this sweep was triggered by
+        /// its arrival in a block; `None` when it was triggered by an
+        /// InstantSend-locked transaction still waiting to be mined (those
+        /// are the only two triggers — an unlocked mempool arrival never
+        /// sweeps, see `WalletTransactionChecker::check_core_transaction`).
+        ///
+        /// This is the winner's finality context, and only the emission site
+        /// has it: `superseded_by` need not be wallet-relevant (see
+        /// `released_outpoints` below), so a consumer cannot look the height
+        /// up in its own records — the winner may never appear anywhere else
+        /// in this wallet's event stream. A consumer durably mirroring the
+        /// removed spends (e.g. observed-spent rows kept so a restored
+        /// wallet does not re-credit the swept coins) needs it to retire
+        /// those rows soundly: a block-context sweep anchors the winner at a
+        /// height that chainlocks, giving the rows a finality horizon, while
+        /// an IS-locked winner has no mining deadline — aging its rows out
+        /// by wall clock would delete a genuine hold while the conflict is
+        /// still unmined.
+        winner_mined_height: Option<CoreBlockHeight>,
         /// Outpoints the sweep released: inputs the removed transactions
         /// claimed to spend that no surviving record spends too (a loser
         /// spending A+B against a winner spending only A leaves A marked and
@@ -446,15 +465,17 @@ impl fmt::Display for WalletEvent {
             WalletEvent::TransactionsSwept {
                 txids,
                 superseded_by,
+                winner_mined_height,
                 released_outpoints,
                 balance,
                 account_balances,
                 ..
             } => write!(
                 f,
-                "TransactionsSwept(count={}, superseded_by={}, released={}, balance={}, account_balances={})",
+                "TransactionsSwept(count={}, superseded_by={}, winner_mined_height={:?}, released={}, balance={}, account_balances={})",
                 txids.len(),
                 superseded_by,
+                winner_mined_height,
                 released_outpoints.len(),
                 balance,
                 format_account_balances(account_balances),
@@ -506,6 +527,44 @@ impl fmt::Display for WalletEvent {
     }
 }
 
+#[cfg(test)]
+mod display_tests {
+    use super::*;
+    use std::collections::BTreeMap;
+
+    fn swept(winner_mined_height: Option<CoreBlockHeight>) -> WalletEvent {
+        WalletEvent::TransactionsSwept {
+            wallet_id: WalletId::from([7u8; 32]),
+            txids: vec![Txid::from_raw_hash(dashcore::hashes::Hash::from_byte_array([1u8; 32]))],
+            superseded_by: Txid::from_raw_hash(dashcore::hashes::Hash::from_byte_array([2u8; 32])),
+            winner_mined_height,
+            released_outpoints: Vec::new(),
+            balance: WalletCoreBalance::default(),
+            account_balances: BTreeMap::new(),
+        }
+    }
+
+    /// The winner's finality context is the first thing a reader of these
+    /// logs needs when a swept coin misbehaves, so `Display` must
+    /// distinguish the two triggers rather than printing one shape for
+    /// both. Both legs are asserted together: a formatter that dropped the
+    /// field would satisfy neither.
+    #[test]
+    fn transactions_swept_display_reports_the_winners_finality_context() {
+        let mined = format!("{}", swept(Some(1_000)));
+        assert!(
+            mined.contains("winner_mined_height=Some(1000)"),
+            "a block-triggered sweep must report the winner's height: {mined}"
+        );
+
+        let unmined = format!("{}", swept(None));
+        assert!(
+            unmined.contains("winner_mined_height=None"),
+            "an InstantSend-triggered sweep must report that no height exists yet: {unmined}"
+        );
+    }
+}
+
 #[cfg(test)]
 mod project_derived_addresses_tests {
     use super::*;

key-wallet-manager/src/process_block.rs

@@ -101,6 +101,9 @@ impl<T: WalletInfoInterface + Send + Sync + 'static> WalletInterface for WalletM
                     wallet_id,
                     txids,
                     superseded_by: tx.txid(),
+                    // The winner is a transaction of this very block, so its
+                    // mined height is the block's height.
+                    winner_mined_height: Some(height),
                     released_outpoints,
                     balance: info.balance(),
                     account_balances: BTreeMap::new(),
@@ -231,6 +234,10 @@ impl<T: WalletInfoInterface + Send + Sync + 'static> WalletInterface for WalletM
                 wallet_id,
                 txids,
                 superseded_by: tx.txid(),
+                // Off-chain arrival: only an InstantSend-locked winner sweeps
+                // from this path (an unlocked mempool arrival never does),
+                // and an IS-locked winner is by definition not mined yet.
+                winner_mined_height: None,
                 released_outpoints,
                 balance: info.balance(),
                 account_balances: per_wallet_account_diff