[signer] Replace `force_non_witness_utxo` with `only_witness_utxo`

Instead of providing an opt-in option to force the addition of the
`non_witness_utxo`, we will now add them by default and provide the
option to disable them when they aren't considered necessary.

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a0a8c964e94dd0700f44bd9bc4fbbc32bab8de43..718b949b10c3d813e22343b40234b1d76fc85288 100644 (file)
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,6 +17,7 @@ Timelocks are considered (optionally) in building the `satisfaction` field
 
 - Changed `Wallet::{sign, finalize_psbt}` now take a `&mut psbt` rather than consuming it.
 - Require and validate `non_witness_utxo` for SegWit signatures by default, can be adjusted with `SignOptions`
+- Replace the opt-in builder option `force_non_witness_utxo` with the opposite `only_witness_utxo`. From now on we will provide the `non_witness_utxo`, unless explicitly asked not to.
 
 ## [v0.6.0] - [v0.5.1]
 

diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs
index ef945a13c3767bff3400055a90d66b143ca1fbac..98be933837239a289ac4e61dd207f0db8002254c 100644 (file)
--- a/src/wallet/mod.rs
+++ b/src/wallet/mod.rs
@@ -1271,28 +1271,23 @@ where
 
             match utxo {
                 Utxo::Local(utxo) => {
-                    *psbt_input = match self.get_psbt_input(
-                        utxo,
-                        params.sighash,
-                        params.force_non_witness_utxo,
-                    ) {
-                        Ok(psbt_input) => psbt_input,
-                        Err(e) => match e {
-                            Error::UnknownUtxo => Input {
-                                sighash_type: params.sighash,
-                                ..Input::default()
+                    *psbt_input =
+                        match self.get_psbt_input(utxo, params.sighash, params.only_witness_utxo) {
+                            Ok(psbt_input) => psbt_input,
+                            Err(e) => match e {
+                                Error::UnknownUtxo => Input {
+                                    sighash_type: params.sighash,
+                                    ..Input::default()
+                                },
+                                _ => return Err(e),
                             },
-                            _ => return Err(e),
-                        },
-                    }
+                        }
                 }
                 Utxo::Foreign {
                     psbt_input: foreign_psbt_input,
                     outpoint,
                 } => {
-                    if params.force_non_witness_utxo
-                        && foreign_psbt_input.non_witness_utxo.is_none()
-                    {
+                    if !params.only_witness_utxo && foreign_psbt_input.non_witness_utxo.is_none() {
                         return Err(Error::Generic(format!(
                             "Missing non_witness_utxo on foreign utxo {}",
                             outpoint
@@ -1336,7 +1331,7 @@ where
         &self,
         utxo: LocalUtxo,
         sighash_type: Option<SigHashType>,
-        force_non_witness_utxo: bool,
+        only_witness_utxo: bool,
     ) -> Result<Input, Error> {
         // Try to find the prev_script in our db to figure out if this is internal or external,
         // and the derivation index
@@ -1363,7 +1358,7 @@ where
             if desc.is_witness() {
                 psbt_input.witness_utxo = Some(prev_tx.output[prev_output.vout as usize].clone());
             }
-            if !desc.is_witness() || force_non_witness_utxo {
+            if !desc.is_witness() || !only_witness_utxo {
                 psbt_input.non_witness_utxo = Some(prev_tx);
             }
         }
@@ -2250,6 +2245,7 @@ mod test {
         let mut builder = wallet.build_tx();
         builder
             .set_single_recipient(addr.script_pubkey())
+            .only_witness_utxo()
             .drain_wallet();
         let (psbt, _) = builder.finish().unwrap();
 
@@ -2268,20 +2264,18 @@ mod test {
             .drain_wallet();
         let (psbt, _) = builder.finish().unwrap();
 
-        assert!(psbt.inputs[0].non_witness_utxo.is_none());
         assert!(psbt.inputs[0].witness_utxo.is_some());
     }
 
     #[test]
-    fn test_create_tx_both_non_witness_utxo_and_witness_utxo() {
+    fn test_create_tx_both_non_witness_utxo_and_witness_utxo_default() {
         let (wallet, _, _) =
             get_funded_wallet("wsh(pk(cVpPVruEDdmutPzisEsYvtST1usBR3ntr8pXSyt6D2YYqXRyPcFW))");
         let addr = wallet.get_address(New).unwrap();
         let mut builder = wallet.build_tx();
         builder
             .set_single_recipient(addr.script_pubkey())
-            .drain_wallet()
-            .force_non_witness_utxo();
+            .drain_wallet();
         let (psbt, _) = builder.finish().unwrap();
 
         assert!(psbt.inputs[0].non_witness_utxo.is_some());
@@ -2419,6 +2413,7 @@ mod test {
         let (wallet1, _, _) = get_funded_wallet(get_test_wpkh());
         let (wallet2, _, _) =
             get_funded_wallet("wpkh(cVbZ8ovhye9AoAHFsqobCf7LxbXDAECy9Kb8TZdfsDYMZGBUyCnm)");
+
         let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX").unwrap();
         let utxo = wallet2.list_unspent().unwrap().remove(0);
         let foreign_utxo_satisfaction = wallet2
@@ -2434,6 +2429,7 @@ mod test {
         let mut builder = wallet1.build_tx();
         builder
             .add_recipient(addr.script_pubkey(), 60_000)
+            .only_witness_utxo()
             .add_foreign_utxo(utxo.outpoint, psbt_input, foreign_utxo_satisfaction)
             .unwrap();
         let (mut psbt, details) = builder.finish().unwrap();
@@ -2453,14 +2449,30 @@ mod test {
             "foreign_utxo should be in there"
         );
 
-        let finished = wallet1.sign(&mut psbt, Default::default()).unwrap();
+        let finished = wallet1
+            .sign(
+                &mut psbt,
+                SignOptions {
+                    trust_witness_utxo: true,
+                    ..Default::default()
+                },
+            )
+            .unwrap();
 
         assert!(
             !finished,
             "only one of the inputs should have been signed so far"
         );
 
-        let finished = wallet2.sign(&mut psbt, Default::default()).unwrap();
+        let finished = wallet2
+            .sign(
+                &mut psbt,
+                SignOptions {
+                    trust_witness_utxo: true,
+                    ..Default::default()
+                },
+            )
+            .unwrap();
         assert!(finished, "all the inputs should have been signed now");
     }
 
@@ -2538,7 +2550,7 @@ mod test {
     }
 
     #[test]
-    fn test_add_foreign_utxo_force_non_witness_utxo() {
+    fn test_add_foreign_utxo_only_witness_utxo() {
         let (wallet1, _, _) = get_funded_wallet(get_test_wpkh());
         let (wallet2, _, txid2) =
             get_funded_wallet("wpkh(cVbZ8ovhye9AoAHFsqobCf7LxbXDAECy9Kb8TZdfsDYMZGBUyCnm)");
@@ -2551,9 +2563,7 @@ mod test {
             .unwrap();
 
         let mut builder = wallet1.build_tx();
-        builder
-            .add_recipient(addr.script_pubkey(), 60_000)
-            .force_non_witness_utxo();
+        builder.add_recipient(addr.script_pubkey(), 60_000);
 
         {
             let mut builder = builder.clone();
@@ -2570,6 +2580,22 @@ mod test {
             );
         }
 
+        {
+            let mut builder = builder.clone();
+            let psbt_input = psbt::Input {
+                witness_utxo: Some(utxo2.txout.clone()),
+                ..Default::default()
+            };
+            builder
+                .only_witness_utxo()
+                .add_foreign_utxo(utxo2.outpoint, psbt_input, satisfaction_weight)
+                .unwrap();
+            assert!(
+                builder.finish().is_ok(),
+                "psbt_input with just witness_utxo should succeed when `only_witness_utxo` is enabled"
+            );
+        }
+
         {
             let mut builder = builder.clone();
             let tx2 = wallet2
@@ -2589,7 +2615,7 @@ mod test {
                 .unwrap();
             assert!(
                 builder.finish().is_ok(),
-                "psbt_input with non_witness_utxo should succeed with force_non_witness_utxo"
+                "psbt_input with non_witness_utxo should succeed by default"
             );
         }
     }
@@ -3619,7 +3645,15 @@ mod test {
 
         psbt.inputs.push(dud_input);
         psbt.global.unsigned_tx.input.push(bitcoin::TxIn::default());
-        let is_final = wallet.sign(&mut psbt, Default::default()).unwrap();
+        let is_final = wallet
+            .sign(
+                &mut psbt,
+                SignOptions {
+                    trust_witness_utxo: true,
+                    ..Default::default()
+                },
+            )
+            .unwrap();
         assert!(
             !is_final,
             "shouldn't be final since we can't sign one of the inputs"

diff --git a/src/wallet/tx_builder.rs b/src/wallet/tx_builder.rs
index eb413490330488274e44831c9782b45e890849fc..77ef5180e420fd11b7c9d9a3b0880239150ece1c 100644 (file)
--- a/src/wallet/tx_builder.rs
+++ b/src/wallet/tx_builder.rs
@@ -146,7 +146,7 @@ pub(crate) struct TxParams {
     pub(crate) rbf: Option<RbfValue>,
     pub(crate) version: Option<Version>,
     pub(crate) change_policy: ChangeSpendPolicy,
-    pub(crate) force_non_witness_utxo: bool,
+    pub(crate) only_witness_utxo: bool,
     pub(crate) add_global_xpubs: bool,
     pub(crate) include_output_redeem_witness_script: bool,
     pub(crate) bumping_fee: Option<PreviousFee>,
@@ -336,10 +336,10 @@ impl<'a, B, D: BatchDatabase, Cs: CoinSelectionAlgorithm<D>, Ctx: TxBuilderConte
     /// 1. The `psbt_input` does not contain a `witness_utxo` or `non_witness_utxo`.
     /// 2. The data in `non_witness_utxo` does not match what is in `outpoint`.
     ///
-    /// Note if you set [`force_non_witness_utxo`] any `psbt_input` you pass to this method must
+    /// Note unless you set [`only_witness_utxo`] any `psbt_input` you pass to this method must
     /// have `non_witness_utxo` set otherwise you will get an error when [`finish`] is called.
     ///
-    /// [`force_non_witness_utxo`]: Self::force_non_witness_utxo
+    /// [`only_witness_utxo`]: Self::only_witness_utxo
     /// [`finish`]: Self::finish
     /// [`max_satisfaction_weight`]: miniscript::Descriptor::max_satisfaction_weight
     pub fn add_foreign_utxo(
@@ -464,12 +464,13 @@ impl<'a, B, D: BatchDatabase, Cs: CoinSelectionAlgorithm<D>, Ctx: TxBuilderConte
         self
     }
 
-    /// Fill-in the [`psbt::Input::non_witness_utxo`](bitcoin::util::psbt::Input::non_witness_utxo) field even if the wallet only has SegWit
-    /// descriptors.
+    /// Only Fill-in the [`psbt::Input::witness_utxo`](bitcoin::util::psbt::Input::witness_utxo) field when spending from
+    /// SegWit descriptors.
     ///
-    /// This is useful for signers which always require it, like Trezor hardware wallets.
-    pub fn force_non_witness_utxo(&mut self) -> &mut Self {
-        self.params.force_non_witness_utxo = true;
+    /// This reduces the size of the PSBT, but some signers might reject them due to the lack of
+    /// the `non_witness_utxo`.
+    pub fn only_witness_utxo(&mut self) -> &mut Self {
+        self.params.only_witness_utxo = true;
         self
     }