feat(key-wallet-manager)!: name the outpoints a sweep releases

dash-spv-ffi/src/bin/ffi_cli.rs

@@ -231,6 +231,8 @@ extern "C" fn on_transactions_swept(
     txids: *const [u8; 32],
     txids_count: usize,
     superseded_by: *const [u8; 32],
+    released_outpoints: *const dash_spv_ffi::FFIOutPoint,
+    released_outpoints_count: usize,
     balance: *const FFIBalance,
     _account_balances: *const dash_spv_ffi::FFIAccountBalance,
     _account_balances_count: u32,
@@ -243,12 +245,22 @@ extern "C" fn on_transactions_swept(
     }
     let list = unsafe { std::slice::from_raw_parts(txids, txids_count) };
     let winner = unsafe { &*superseded_by };
+    let released = if released_outpoints.is_null() {
+        &[][..]
+    } else {
+        unsafe { std::slice::from_raw_parts(released_outpoints, released_outpoints_count) }
+    };
     let b = read_balance(balance);
     println!(
-        "[Wallet] TXs swept: wallet={}..., removed=[{}], superseded_by={}, balance[confirmed={}, unconfirmed={}]",
+        "[Wallet] TXs swept: wallet={}..., removed=[{}], superseded_by={}, released=[{}], balance[confirmed={}, unconfirmed={}]",
         wallet_short,
         list.iter().map(hex::encode).collect::<Vec<_>>().join(","),
         hex::encode(winner),
+        released
+            .iter()
+            .map(|o| format!("{}:{}", hex::encode(o.txid), o.vout))
+            .collect::<Vec<_>>()
+            .join(","),
         b.confirmed,
         b.unconfirmed,
     );

dash-spv-ffi/src/callbacks.rs

@@ -752,6 +752,28 @@ pub type OnTransactionDetectedCallback = Option<
     ),
 >;
 
+/// C representation of a Core [`OutPoint`](dashcore::OutPoint): the parent
+/// txid and output index of a coin.
+#[repr(C)]
+pub struct FFIOutPoint {
+    /// Parent transaction id.
+    pub txid: [u8; 32],
+    /// Output index within the parent transaction.
+    pub vout: u32,
+}
+
+impl FFIOutPoint {
+    fn from_slice(outpoints: &[dashcore::OutPoint]) -> Vec<Self> {
+        outpoints
+            .iter()
+            .map(|outpoint| FFIOutPoint {
+                txid: *outpoint.txid.as_byte_array(),
+                vout: outpoint.vout,
+            })
+            .collect()
+    }
+}
+
 /// Callback for `WalletEvent::TransactionsSwept`.
 ///
 /// Fires when the wallet removes transactions that a later, final transaction
@@ -766,6 +788,15 @@ pub type OnTransactionDetectedCallback = Option<
 ///
 /// `txids` points to `txids_count` consecutive 32-byte txids.
 /// `superseded_by` is the transaction whose arrival settled the inputs.
+/// `released_outpoints` points to `released_outpoints_count` outpoints freed
+/// by the removal: inputs the removed transactions claimed to spend that no
+/// surviving record spends too. Mark these coins spendable again. This is
+/// not the same set as `txids`' inputs — a loser spending A+B against a
+/// winner spending only A leaves A marked and frees only B — and it cannot
+/// be recomputed from `txids` on the consumer side: `superseded_by` need not
+/// 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.
 /// 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
@@ -776,6 +807,8 @@ pub type OnTransactionsSweptCallback = Option<
         txids: *const [u8; 32],
         txids_count: usize,
         superseded_by: *const [u8; 32],
+        released_outpoints: *const FFIOutPoint,
+        released_outpoints_count: usize,
         balance: *const FFIBalance,
         account_balances: *const FFIAccountBalance,
         account_balances_count: u32,
@@ -1058,6 +1091,7 @@ impl FFIWalletEventCallbacks {
                 wallet_id,
                 txids,
                 superseded_by,
+                released_outpoints,
                 balance,
                 account_balances,
             } => {
@@ -1067,6 +1101,12 @@ impl FFIWalletEventCallbacks {
                     let raw_txids: Vec<[u8; 32]> =
                         txids.iter().map(|t| t.to_byte_array()).collect();
                     let raw_superseded_by = superseded_by.to_byte_array();
+                    let ffi_released_outpoints = FFIOutPoint::from_slice(released_outpoints);
+                    let released_outpoints_ptr = if ffi_released_outpoints.is_empty() {
+                        ptr::null()
+                    } else {
+                        ffi_released_outpoints.as_ptr()
+                    };
                     let ffi_balance = FFIBalance::from(*balance);
                     let ffi_account_balances = FFIAccountBalance::from_map(account_balances);
                     let account_balances_ptr = if ffi_account_balances.is_empty() {
@@ -1080,13 +1120,16 @@ impl FFIWalletEventCallbacks {
                         raw_txids.as_ptr(),
                         raw_txids.len(),
                         &raw_superseded_by as *const [u8; 32],
+                        released_outpoints_ptr,
+                        ffi_released_outpoints.len(),
                         &ffi_balance as *const FFIBalance,
                         account_balances_ptr,
                         ffi_account_balances.len() as u32,
                         self.user_data,
                     );
 
                     drop(ffi_account_balances);
+                    drop(ffi_released_outpoints);
                 } else {
                     // Deliberately loud: every other wallet callback is
                     // additive, so a consumer that leaves this one unset keeps
@@ -1316,6 +1359,115 @@ mod tests {
     use std::collections::{BTreeMap, BTreeSet};
     use std::sync::atomic::{AtomicU32, Ordering};
 
+    /// A sweep with nothing released must hand the callback `null` and `0`,
+    /// not a dangling pointer into an empty `Vec`.
+    ///
+    /// This is the layer where getting it wrong does not fail an assertion:
+    /// a consumer reading `count` first sees zero and stops, but one that
+    /// dereferences the pointer defensively would read whatever the empty
+    /// allocation points at. The ordinary resend — the winner takes every
+    /// input the loser named — releases nothing, so this is the common case,
+    /// not the edge one.
+    #[test]
+    fn test_transactions_swept_dispatch_passes_null_when_nothing_released() {
+        static RELEASED_PTR_WAS_NULL: AtomicU32 = AtomicU32::new(u32::MAX);
+        static RELEASED_COUNT: AtomicU32 = AtomicU32::new(u32::MAX);
+
+        extern "C" fn cb(
+            _wallet_id: *const c_char,
+            _txids: *const [u8; 32],
+            _txids_count: usize,
+            _superseded_by: *const [u8; 32],
+            released: *const FFIOutPoint,
+            released_count: usize,
+            _balance: *const FFIBalance,
+            _account_balances: *const FFIAccountBalance,
+            _account_balances_count: u32,
+            _user: *mut c_void,
+        ) {
+            RELEASED_PTR_WAS_NULL.store(u32::from(released.is_null()), Ordering::SeqCst);
+            RELEASED_COUNT.store(released_count as u32, Ordering::SeqCst);
+        }
+
+        let callbacks = FFIWalletEventCallbacks {
+            on_transactions_swept: Some(cb),
+            ..FFIWalletEventCallbacks::default()
+        };
+
+        callbacks.dispatch(&WalletEvent::TransactionsSwept {
+            wallet_id: [7u8; 32],
+            txids: vec![Txid::from_byte_array([1u8; 32])],
+            superseded_by: Txid::from_byte_array([2u8; 32]),
+            released_outpoints: Vec::new(),
+            balance: WalletCoreBalance::default(),
+            account_balances: BTreeMap::new(),
+        });
+
+        assert_eq!(RELEASED_PTR_WAS_NULL.load(Ordering::SeqCst), 1, "expected a null pointer");
+        assert_eq!(RELEASED_COUNT.load(Ordering::SeqCst), 0);
+    }
+
+    /// The released outpoints must arrive intact and in order, and the
+    /// parameters after them must not be shifted by their insertion — the
+    /// balance a consumer reads has to be the balance, not an outpoint.
+    #[test]
+    fn test_transactions_swept_dispatch_marshals_released_outpoints() {
+        static FIRST_TXID: std::sync::Mutex<[u8; 32]> = std::sync::Mutex::new([0u8; 32]);
+        static VOUTS: std::sync::Mutex<Vec<u32>> = std::sync::Mutex::new(Vec::new());
+        static CONFIRMED: AtomicU32 = AtomicU32::new(u32::MAX);
+
+        extern "C" fn cb(
+            _wallet_id: *const c_char,
+            _txids: *const [u8; 32],
+            _txids_count: usize,
+            _superseded_by: *const [u8; 32],
+            released: *const FFIOutPoint,
+            released_count: usize,
+            balance: *const FFIBalance,
+            _account_balances: *const FFIAccountBalance,
+            _account_balances_count: u32,
+            _user: *mut c_void,
+        ) {
+            assert!(!released.is_null());
+            let entries = unsafe { std::slice::from_raw_parts(released, released_count) };
+            *FIRST_TXID.lock().expect("txid") = entries[0].txid;
+            *VOUTS.lock().expect("vouts") = entries.iter().map(|e| e.vout).collect();
+            CONFIRMED.store(unsafe { (*balance).confirmed } as u32, Ordering::SeqCst);
+        }
+
+        let callbacks = FFIWalletEventCallbacks {
+            on_transactions_swept: Some(cb),
+            ..FFIWalletEventCallbacks::default()
+        };
+
+        let parent = Txid::from_byte_array([9u8; 32]);
+        callbacks.dispatch(&WalletEvent::TransactionsSwept {
+            wallet_id: [7u8; 32],
+            txids: vec![Txid::from_byte_array([1u8; 32])],
+            superseded_by: Txid::from_byte_array([2u8; 32]),
+            released_outpoints: vec![
+                dashcore::OutPoint {
+                    txid: parent,
+                    vout: 3,
+                },
+                dashcore::OutPoint {
+                    txid: parent,
+                    vout: 7,
+                },
+            ],
+            balance: WalletCoreBalance::new(123_456, 0, 0, 0),
+            account_balances: BTreeMap::new(),
+        });
+
+        assert_eq!(*FIRST_TXID.lock().expect("txid"), *parent.as_byte_array());
+        assert_eq!(*VOUTS.lock().expect("vouts"), vec![3, 7]);
+        assert_eq!(
+            CONFIRMED.load(Ordering::SeqCst),
+            123_456,
+            "the balance parameter must not be shifted by the released-outpoint insertion"
+        );
+    }
+
     /// `BlocksNeeded` dispatch must pass exactly one entry per
     /// `FilterMatchKey` to the FFI callback (i.e. iterate keys, not
     /// inflated by the per-block wallet attribution).

key-wallet-manager/src/event_tests.rs

@@ -533,6 +533,111 @@ async fn test_block_with_index_less_account_tx_carries_account_type() {
     }
 }
 
+/// The sweep event's own coverage at the manager level: a block whose
+/// transaction beats a recorded mempool spend must emit `TransactionsSwept`
+/// naming the removed transaction and the coins its removal freed.
+///
+/// The released set is the half a consumer cannot recompute, so it is worth
+/// pinning where it is actually assembled — this exercises the per-wallet
+/// aggregation in `check_transactions` and the block path's emission
+/// together, neither of which the account-level sweep tests reach.
+#[tokio::test]
+async fn test_block_winner_emits_swept_event_naming_the_released_outpoints() {
+    let (mut manager, wallet_id, addr) = setup_manager_with_wallet();
+
+    // One funding transaction pays us twice, so the loser can spend 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([0x5a; 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()], 0x5a, 1000);
+    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;
+
+    // Subscribe only now: the funding block and the loser's arrival are
+    // setup, and the sweep is what this test is about.
+    let mut rx = manager.subscribe_events();
+
+    let winner = spend(vec![coin_a], 400_000);
+    let winner_block = make_block(vec![winner.clone()], 0x5b, 1100);
+    manager
+        .process_block_for_wallets(&winner_block, winner_block.block_hash(), 101, &wallets)
+        .await;
+
+    let events = drain_events(&mut rx);
+    let swept = events
+        .iter()
+        .find_map(|event| match event {
+            WalletEvent::TransactionsSwept {
+                wallet_id: wid,
+                txids,
+                superseded_by,
+                released_outpoints,
+                ..
+            } => Some((wid, txids, superseded_by, 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");
+}
+
 #[tokio::test]
 async fn test_empty_block_for_idle_wallet_emits_nothing() {
     let (mut manager, wallet_id, _addr) = setup_manager_with_wallet();