spend: deterministic, canonical input construction

liana/src/spend.rs

@@ -277,6 +277,15 @@ fn select_coins_for_spend(
     max_sat_weight: u64,
     must_have_change: bool,
 ) -> Result<CoinSelectionRes, InsufficientFunds> {
+    // Callers pass `candidate_coins` from a HashMap, so sort by outpoint to keep
+    // selection deterministic.
+    let sorted_candidates = {
+        let mut v = candidate_coins.to_vec();
+        v.sort_unstable_by_key(|c| c.outpoint);
+        v
+    };
+    let candidate_coins: &[CandidateCoin] = &sorted_candidates;
+
     let out_value_nochange = base_tx.output.iter().map(|o| o.value.to_sat()).sum();
     let out_weight_nochange = {
         let mut total: u64 = 0;
@@ -482,6 +491,33 @@ fn derived_desc(
     desc.derive(coin.deriv_index, secp)
 }
 
+/// Build the canonical `(TxIn, PsbtIn)` for spending a coin, so every caller
+/// constructs inputs the same way.
+pub fn coin_to_psbt_input(
+    outpoint: bitcoin::OutPoint,
+    amount: bitcoin::Amount,
+    sequence: bitcoin::Sequence,
+    coin_desc: &descriptors::DerivedSinglePathLianaDesc,
+    is_taproot: bool,
+    prev_tx: impl FnOnce() -> Option<bitcoin::Transaction>,
+) -> (bitcoin::TxIn, PsbtIn) {
+    let txin = bitcoin::TxIn {
+        previous_output: outpoint,
+        sequence,
+        ..Default::default()
+    };
+    let mut psbt_in = PsbtIn::default();
+    coin_desc.update_psbt_in(&mut psbt_in);
+    psbt_in.witness_utxo = Some(bitcoin::TxOut {
+        value: amount,
+        script_pubkey: coin_desc.script_pubkey(),
+    });
+    if !is_taproot {
+        psbt_in.non_witness_utxo = prev_tx();
+    }
+    (txin, psbt_in)
+}
+
 /// Get value to use for transaction nLockTime in order to
 /// discourage fee sniping.
 ///
@@ -766,27 +802,20 @@ pub fn create_spend(
     // Iterate through selected coins and add necessary information to the PSBT inputs.
     let mut psbt_ins = Vec::with_capacity(selected.len());
     for cand in &selected {
+        // TODO: once we move to Taproot, anti-fee-sniping using nSequence
         let sequence = cand
             .sequence
             .unwrap_or(bitcoin::Sequence::ENABLE_RBF_NO_LOCKTIME);
-        tx.input.push(bitcoin::TxIn {
-            previous_output: cand.outpoint,
-            sequence,
-            // TODO: once we move to Taproot, anti-fee-sniping using nSequence
-            ..bitcoin::TxIn::default()
-        });
-
-        // Populate the PSBT input with the information needed by signers.
-        let mut psbt_in = PsbtIn::default();
         let coin_desc = derived_desc(secp, main_descriptor, cand);
-        coin_desc.update_psbt_in(&mut psbt_in);
-        psbt_in.witness_utxo = Some(bitcoin::TxOut {
-            value: cand.amount,
-            script_pubkey: coin_desc.script_pubkey(),
-        });
-        if !main_descriptor.is_taproot() {
-            psbt_in.non_witness_utxo = tx_getter.get_tx(&cand.outpoint.txid);
-        }
+        let (txin, psbt_in) = coin_to_psbt_input(
+            cand.outpoint,
+            cand.amount,
+            sequence,
+            &coin_desc,
+            main_descriptor.is_taproot(),
+            || tx_getter.get_tx(&cand.outpoint.txid),
+        );
+        tx.input.push(txin);
         psbt_ins.push(psbt_in);
     }
 
@@ -927,4 +956,61 @@ mod tests {
             LockTime::from_height(1).unwrap() // subtract 90
         );
     }
+
+    #[test]
+    fn test_coin_selection_is_deterministic() {
+        use bitcoin::hashes::Hash;
+        use bitcoin::{
+            absolute::LockTime, transaction::Version, Amount, OutPoint, ScriptBuf, Transaction,
+            TxOut, Txid,
+        };
+
+        // Equal-value coins, so selection has ties, with distinct outpoints.
+        let candidates: Vec<CandidateCoin> = (0..6u32)
+            .map(|vout| CandidateCoin {
+                outpoint: OutPoint {
+                    txid: Txid::from_byte_array([0u8; 32]),
+                    vout,
+                },
+                amount: Amount::from_sat(100_000),
+                deriv_index: bip32::ChildNumber::from_normal_idx(0).unwrap(),
+                is_change: false,
+                must_select: false,
+                sequence: None,
+                ancestor_info: None,
+            })
+            .collect();
+
+        let base_tx = Transaction {
+            version: Version::TWO,
+            lock_time: LockTime::ZERO,
+            input: vec![],
+            output: vec![TxOut {
+                value: Amount::from_sat(250_000),
+                script_pubkey: ScriptBuf::new(),
+            }],
+        };
+        let change_txo = TxOut {
+            value: Amount::MAX,
+            script_pubkey: ScriptBuf::new(),
+        };
+        let select = |cands: &[CandidateCoin]| {
+            select_coins_for_spend(
+                cands,
+                base_tx.clone(),
+                change_txo.clone(),
+                2.0,
+                None,
+                100,
+                false,
+            )
+            .expect("enough funds")
+            .selected
+        };
+
+        // The same coins in a different input order must yield the same selection.
+        let mut reversed = candidates.clone();
+        reversed.reverse();
+        assert_eq!(select(&candidates), select(&reversed));
+    }
 }