]> Untitled Git - bdk/commitdiff
[signer] Adjust signing behavior with `SignOptions`
authorAlekos Filini <alekos.filini@gmail.com>
Mon, 19 Apr 2021 12:16:39 +0000 (14:16 +0200)
committerAlekos Filini <alekos.filini@gmail.com>
Thu, 6 May 2021 06:58:38 +0000 (08:58 +0200)
CHANGELOG.md
README.md
src/lib.rs
src/wallet/mod.rs
src/wallet/signer.rs
testutils-macros/src/lib.rs

index fecd885ec1988d14470dc600266a8234547f8739..a0a8c964e94dd0700f44bd9bc4fbbc32bab8de43 100644 (file)
@@ -16,6 +16,7 @@ Timelocks are considered (optionally) in building the `satisfaction` field
 ### Wallet
 
 - 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`
 
 ## [v0.6.0] - [v0.5.1]
 
index fac0608e9ba22e49719c7993c7ff82357c63d4c0..2d8c3c21272fc8c2d5232155c659440d741eba2e 100644 (file)
--- a/README.md
+++ b/README.md
@@ -130,7 +130,7 @@ fn main() -> Result<(), bdk::Error> {
 ### Sign a transaction
 
 ```rust,no_run
-use bdk::{Wallet, database::MemoryDatabase};
+use bdk::{Wallet, SignOptions, database::MemoryDatabase};
 
 use bitcoin::consensus::deserialize;
 
@@ -145,7 +145,7 @@ fn main() -> Result<(), bdk::Error> {
     let psbt = "...";
     let mut psbt = deserialize(&base64::decode(psbt).unwrap())?;
 
-    let finalized = wallet.sign(&mut psbt, None)?;
+    let finalized = wallet.sign(&mut psbt, SignOptions::default())?;
 
     Ok(())
 }
index 917d6a8f6f0094b06a792d6d535f8b1d17b9a8fa..f5323b1aece5157eee77a86324ab6c4cf029f42f 100644 (file)
@@ -256,6 +256,7 @@ pub use error::Error;
 pub use types::*;
 pub use wallet::address_validator;
 pub use wallet::signer;
+pub use wallet::signer::SignOptions;
 pub use wallet::tx_builder::TxBuilder;
 pub use wallet::Wallet;
 
index 6ae397a6de2a5fe7233ff62241e35fe67d8cdf72..ef945a13c3767bff3400055a90d66b143ca1fbac 100644 (file)
@@ -46,7 +46,7 @@ pub use utils::IsDust;
 
 use address_validator::AddressValidator;
 use coin_selection::DefaultCoinSelectionAlgorithm;
-use signer::{Signer, SignerOrdering, SignersContainer};
+use signer::{SignOptions, Signer, SignerOrdering, SignersContainer};
 use tx_builder::{BumpFee, CreateTx, FeePolicy, TxBuilder, TxParams};
 use utils::{check_nlocktime, check_nsequence_rbf, After, Older, SecpCtx, DUST_LIMIT_SATOSHI};
 
@@ -706,7 +706,7 @@ where
     ///         .enable_rbf();
     ///     builder.finish()?
     /// };
-    /// let _ = wallet.sign(&mut psbt, None)?;
+    /// let _ = wallet.sign(&mut psbt, SignOptions::default())?;
     /// let tx = psbt.extract_tx();
     /// // broadcast tx but it's taking too long to confirm so we want to bump the fee
     /// let (mut psbt, _) =  {
@@ -716,7 +716,7 @@ where
     ///     builder.finish()?
     /// };
     ///
-    /// let _ = wallet.sign(&mut psbt, None)?;
+    /// let _ = wallet.sign(&mut psbt, SignOptions::default())?;
     /// let fee_bumped_tx = psbt.extract_tx();
     /// // broadcast fee_bumped_tx to replace original
     /// # Ok::<(), bdk::Error>(())
@@ -833,6 +833,11 @@ where
     /// Sign a transaction with all the wallet's signers, in the order specified by every signer's
     /// [`SignerOrdering`]
     ///
+    /// The [`SignOptions`] can be used to tweak the behavior of the software signers, and the way
+    /// the transaction is finalized at the end. Note that it can't be guaranteed that *every*
+    /// signers will follow the options, but the "software signers" (WIF keys and `xprv`) defined
+    /// in this library will.
+    ///
     /// ## Example
     ///
     /// ```
@@ -848,13 +853,23 @@ where
     ///     builder.add_recipient(to_address.script_pubkey(), 50_000);
     ///     builder.finish()?
     /// };
-    /// let  finalized = wallet.sign(&mut psbt, None)?;
+    /// let  finalized = wallet.sign(&mut psbt, SignOptions::default())?;
     /// assert!(finalized, "we should have signed all the inputs");
     /// # Ok::<(), bdk::Error>(())
-    pub fn sign(&self, psbt: &mut PSBT, assume_height: Option<u32>) -> Result<bool, Error> {
+    pub fn sign(&self, psbt: &mut PSBT, sign_options: SignOptions) -> Result<bool, Error> {
         // this helps us doing our job later
         self.add_input_hd_keypaths(psbt)?;
 
+        // If we aren't allowed to use `witness_utxo`, ensure that every input has the
+        // `non_witness_utxo`
+        if !sign_options.trust_witness_utxo {
+            for input in &psbt.inputs {
+                if input.non_witness_utxo.is_none() {
+                    return Err(Error::Signer(signer::SignerError::MissingNonWitnessUtxo));
+                }
+            }
+        }
+
         for signer in self
             .signers
             .signers()
@@ -871,7 +886,7 @@ where
         }
 
         // attempt to finalize
-        self.finalize_psbt(psbt, assume_height)
+        self.finalize_psbt(psbt, sign_options)
     }
 
     /// Return the spending policies for the wallet's descriptor
@@ -907,11 +922,9 @@ where
     }
 
     /// Try to finalize a PSBT
-    pub fn finalize_psbt(
-        &self,
-        psbt: &mut PSBT,
-        assume_height: Option<u32>,
-    ) -> Result<bool, Error> {
+    ///
+    /// The [`SignOptions`] can be used to tweak the behavior of the finalizer.
+    pub fn finalize_psbt(&self, psbt: &mut PSBT, sign_options: SignOptions) -> Result<bool, Error> {
         let tx = &psbt.global.unsigned_tx;
         let mut finished = true;
 
@@ -927,7 +940,7 @@ where
                 .borrow()
                 .get_tx(&input.previous_output.txid, false)?
                 .map(|tx| tx.height.unwrap_or(std::u32::MAX));
-            let current_height = assume_height.or(self.current_height);
+            let current_height = sign_options.assume_height.or(self.current_height);
 
             debug!(
                 "Input #{} - {}, using `create_height` = {:?}, `current_height` = {:?}",
@@ -2440,14 +2453,14 @@ mod test {
             "foreign_utxo should be in there"
         );
 
-        let finished = wallet1.sign(&mut psbt, None).unwrap();
+        let finished = wallet1.sign(&mut psbt, Default::default()).unwrap();
 
         assert!(
             !finished,
             "only one of the inputs should have been signed so far"
         );
 
-        let finished = wallet2.sign(&mut psbt, None).unwrap();
+        let finished = wallet2.sign(&mut psbt, Default::default()).unwrap();
         assert!(finished, "all the inputs should have been signed now");
     }
 
@@ -3468,7 +3481,7 @@ mod test {
             .drain_wallet();
         let (mut psbt, _) = builder.finish().unwrap();
 
-        let finalized = wallet.sign(&mut psbt, None).unwrap();
+        let finalized = wallet.sign(&mut psbt, Default::default()).unwrap();
         assert_eq!(finalized, true);
 
         let extracted = psbt.extract_tx();
@@ -3485,7 +3498,7 @@ mod test {
             .drain_wallet();
         let (mut psbt, _) = builder.finish().unwrap();
 
-        let finalized = wallet.sign(&mut psbt, None).unwrap();
+        let finalized = wallet.sign(&mut psbt, Default::default()).unwrap();
         assert_eq!(finalized, true);
 
         let extracted = psbt.extract_tx();
@@ -3502,7 +3515,7 @@ mod test {
             .drain_wallet();
         let (mut psbt, _) = builder.finish().unwrap();
 
-        let finalized = wallet.sign(&mut psbt, None).unwrap();
+        let finalized = wallet.sign(&mut psbt, Default::default()).unwrap();
         assert_eq!(finalized, true);
 
         let extracted = psbt.extract_tx();
@@ -3519,7 +3532,7 @@ mod test {
             .drain_wallet();
         let (mut psbt, _) = builder.finish().unwrap();
 
-        let finalized = wallet.sign(&mut psbt, None).unwrap();
+        let finalized = wallet.sign(&mut psbt, Default::default()).unwrap();
         assert_eq!(finalized, true);
 
         let extracted = psbt.extract_tx();
@@ -3537,7 +3550,7 @@ mod test {
             .drain_wallet();
         let (mut psbt, _) = builder.finish().unwrap();
 
-        let finalized = wallet.sign(&mut psbt, None).unwrap();
+        let finalized = wallet.sign(&mut psbt, Default::default()).unwrap();
         assert_eq!(finalized, true);
 
         let extracted = psbt.extract_tx();
@@ -3557,7 +3570,7 @@ mod test {
         psbt.inputs[0].bip32_derivation.clear();
         assert_eq!(psbt.inputs[0].bip32_derivation.len(), 0);
 
-        let finalized = wallet.sign(&mut psbt, None).unwrap();
+        let finalized = wallet.sign(&mut psbt, Default::default()).unwrap();
         assert_eq!(finalized, true);
 
         let extracted = psbt.extract_tx();
@@ -3606,7 +3619,7 @@ mod test {
 
         psbt.inputs.push(dud_input);
         psbt.global.unsigned_tx.input.push(bitcoin::TxIn::default());
-        let is_final = wallet.sign(&mut psbt, None).unwrap();
+        let is_final = wallet.sign(&mut psbt, Default::default()).unwrap();
         assert!(
             !is_final,
             "shouldn't be final since we can't sign one of the inputs"
index 45ab6bf4b29122b9183e3699c103e6898f041350..3198a61f854e0472526c7000b54d94b192abdd8f 100644 (file)
@@ -427,6 +427,43 @@ impl SignersContainer {
     }
 }
 
+/// Options for a software signer
+///
+/// Adjust the behavior of our software signers and the way a transaction is finalized
+#[derive(Debug, Clone)]
+pub struct SignOptions {
+    /// Whether the signer should trust the `witness_utxo`, if the `non_witness_utxo` hasn't been
+    /// provided
+    ///
+    /// Defaults to `false` to mitigate the "SegWit bug" which chould trick the wallet into
+    /// paying a fee larger than expected.
+    ///
+    /// Some wallets, especially if relatively old, might not provide the `non_witness_utxo` for
+    /// SegWit transactions in the PSBT they generate: in those cases setting this to `true`
+    /// should correctly produce a signature, at the expense of an increased trust in the creator
+    /// of the PSBT.
+    ///
+    /// For more details see: <https://blog.trezor.io/details-of-firmware-updates-for-trezor-one-version-1-9-1-and-trezor-model-t-version-2-3-1-1eba8f60f2dd>
+    pub trust_witness_utxo: bool,
+
+    /// Whether the wallet should assume a specific height has been reached when trying to finalize
+    /// a transaction
+    ///
+    /// The wallet will only "use" a timelock to satisfy the spending policy of an input if the
+    /// timelock height has already been reached. This option allows overriding the "current height" to let the
+    /// wallet use timelocks in the future to spend a coin.
+    pub assume_height: Option<u32>,
+}
+
+impl Default for SignOptions {
+    fn default() -> Self {
+        SignOptions {
+            trust_witness_utxo: false,
+            assume_height: None,
+        }
+    }
+}
+
 pub(crate) trait ComputeSighash {
     fn sighash(
         psbt: &psbt::PartiallySignedTransaction,
@@ -492,20 +529,37 @@ impl ComputeSighash for Segwitv0 {
         }
 
         let psbt_input = &psbt.inputs[input_index];
+        let tx_input = &psbt.global.unsigned_tx.input[input_index];
 
         let sighash = psbt_input.sighash_type.unwrap_or(SigHashType::All);
 
-        let witness_utxo = psbt_input
-            .witness_utxo
-            .as_ref()
-            .ok_or(SignerError::MissingWitnessUtxo)?;
-        let value = witness_utxo.value;
+        // Always try first with the non-witness utxo
+        let utxo = if let Some(prev_tx) = &psbt_input.non_witness_utxo {
+            // Check the provided prev-tx
+            if prev_tx.txid() != tx_input.previous_output.txid {
+                return Err(SignerError::InvalidNonWitnessUtxo);
+            }
+
+            // The output should be present, if it's missing the `non_witness_utxo` is invalid
+            prev_tx
+                .output
+                .get(tx_input.previous_output.vout as usize)
+                .ok_or(SignerError::InvalidNonWitnessUtxo)?
+        } else if let Some(witness_utxo) = &psbt_input.witness_utxo {
+            // Fallback to the witness_utxo. If we aren't allowed to use it, signing should fail
+            // before we get to this point
+            witness_utxo
+        } else {
+            // Nothing has been provided
+            return Err(SignerError::MissingNonWitnessUtxo);
+        };
+        let value = utxo.value;
 
         let script = match psbt_input.witness_script {
             Some(ref witness_script) => witness_script.clone(),
             None => {
-                if witness_utxo.script_pubkey.is_v0_p2wpkh() {
-                    p2wpkh_script_code(&witness_utxo.script_pubkey)
+                if utxo.script_pubkey.is_v0_p2wpkh() {
+                    p2wpkh_script_code(&utxo.script_pubkey)
                 } else if psbt_input
                     .redeem_script
                     .as_ref()
index 2d42db14cc3ed5ea7565be871c6405fc922f5390..db01e1ed7b4ebcf5ed5d57bb7215e98da05d2044 100644 (file)
@@ -298,7 +298,7 @@ pub fn bdk_blockchain_tests(attr: TokenStream, item: TokenStream) -> TokenStream
                     let mut builder = wallet.build_tx();
                     builder.add_recipient(node_addr.script_pubkey(), 25_000);
                     let (mut psbt, details) = builder.finish().unwrap();
-                    let finalized = wallet.sign(&mut psbt, None).unwrap();
+                    let finalized = wallet.sign(&mut psbt, Default::default()).unwrap();
                     assert!(finalized, "Cannot finalize transaction");
                     let tx = psbt.extract_tx();
                     println!("{}", bitcoin::consensus::encode::serialize_hex(&tx));
@@ -327,7 +327,7 @@ pub fn bdk_blockchain_tests(attr: TokenStream, item: TokenStream) -> TokenStream
                     let mut builder = wallet.build_tx();
                     builder.add_recipient(node_addr.script_pubkey(), 25_000);
                     let (mut psbt, details) = builder.finish().unwrap();
-                    let finalized = wallet.sign(&mut psbt, None).unwrap();
+                    let finalized = wallet.sign(&mut psbt, Default::default()).unwrap();
                     assert!(finalized, "Cannot finalize transaction");
                     let sent_txid = wallet.broadcast(psbt.extract_tx()).unwrap();
 
@@ -368,7 +368,7 @@ pub fn bdk_blockchain_tests(attr: TokenStream, item: TokenStream) -> TokenStream
                         let mut builder = wallet.build_tx();
                         builder.add_recipient(node_addr.script_pubkey(), 5_000);
                         let (mut psbt, details) = builder.finish().unwrap();
-                        let finalized = wallet.sign(&mut psbt, None).unwrap();
+                        let finalized = wallet.sign(&mut psbt, Default::default()).unwrap();
                         assert!(finalized, "Cannot finalize transaction");
                         wallet.broadcast(psbt.extract_tx()).unwrap();
 
@@ -402,7 +402,7 @@ pub fn bdk_blockchain_tests(attr: TokenStream, item: TokenStream) -> TokenStream
                     let mut builder = wallet.build_tx();
                     builder.add_recipient(node_addr.script_pubkey().clone(), 5_000).enable_rbf();
                     let (mut psbt, details) = builder.finish().unwrap();
-                    let finalized = wallet.sign(&mut psbt, None).unwrap();
+                    let finalized = wallet.sign(&mut psbt, Default::default()).unwrap();
                     assert!(finalized, "Cannot finalize transaction");
                     wallet.broadcast(psbt.extract_tx()).unwrap();
                     wallet.sync(noop_progress(), None).unwrap();
@@ -412,7 +412,7 @@ pub fn bdk_blockchain_tests(attr: TokenStream, item: TokenStream) -> TokenStream
                     let mut builder = wallet.build_fee_bump(details.txid).unwrap();
                     builder.fee_rate(FeeRate::from_sat_per_vb(2.1));
                     let (mut new_psbt, new_details) = builder.finish().unwrap();
-                    let finalized = wallet.sign(&mut new_psbt, None).unwrap();
+                    let finalized = wallet.sign(&mut new_psbt, Default::default()).unwrap();
                     assert!(finalized, "Cannot finalize transaction");
                     wallet.broadcast(new_psbt.extract_tx()).unwrap();
                     wallet.sync(noop_progress(), None).unwrap();
@@ -438,7 +438,7 @@ pub fn bdk_blockchain_tests(attr: TokenStream, item: TokenStream) -> TokenStream
                     let mut builder = wallet.build_tx();
                     builder.add_recipient(node_addr.script_pubkey().clone(), 49_000).enable_rbf();
                     let (mut psbt, details) = builder.finish().unwrap();
-                    let finalized = wallet.sign(&mut psbt, None).unwrap();
+                    let finalized = wallet.sign(&mut psbt, Default::default()).unwrap();
                     assert!(finalized, "Cannot finalize transaction");
                     wallet.broadcast(psbt.extract_tx()).unwrap();
                     wallet.sync(noop_progress(), None).unwrap();
@@ -448,7 +448,7 @@ pub fn bdk_blockchain_tests(attr: TokenStream, item: TokenStream) -> TokenStream
                     let mut builder = wallet.build_fee_bump(details.txid).unwrap();
                     builder.fee_rate(FeeRate::from_sat_per_vb(5.0));
                     let (mut new_psbt, new_details) = builder.finish().unwrap();
-                    let finalized = wallet.sign(&mut new_psbt, None).unwrap();
+                    let finalized = wallet.sign(&mut new_psbt, Default::default()).unwrap();
                     assert!(finalized, "Cannot finalize transaction");
                     wallet.broadcast(new_psbt.extract_tx()).unwrap();
                     wallet.sync(noop_progress(), None).unwrap();
@@ -474,7 +474,7 @@ pub fn bdk_blockchain_tests(attr: TokenStream, item: TokenStream) -> TokenStream
                     let mut builder = wallet.build_tx();
                     builder.add_recipient(node_addr.script_pubkey().clone(), 49_000).enable_rbf();
                     let (mut psbt, details) = builder.finish().unwrap();
-                    let finalized = wallet.sign(&mut psbt, None).unwrap();
+                    let finalized = wallet.sign(&mut psbt, Default::default()).unwrap();
                     assert!(finalized, "Cannot finalize transaction");
                     wallet.broadcast(psbt.extract_tx()).unwrap();
                     wallet.sync(noop_progress(), None).unwrap();
@@ -484,7 +484,7 @@ pub fn bdk_blockchain_tests(attr: TokenStream, item: TokenStream) -> TokenStream
                     let mut builder = wallet.build_fee_bump(details.txid).unwrap();
                     builder.fee_rate(FeeRate::from_sat_per_vb(10.0));
                     let (mut new_psbt, new_details) = builder.finish().unwrap();
-                    let finalized = wallet.sign(&mut new_psbt, None).unwrap();
+                    let finalized = wallet.sign(&mut new_psbt, Default::default()).unwrap();
                     assert!(finalized, "Cannot finalize transaction");
                     wallet.broadcast(new_psbt.extract_tx()).unwrap();
                     wallet.sync(noop_progress(), None).unwrap();
@@ -508,7 +508,7 @@ pub fn bdk_blockchain_tests(attr: TokenStream, item: TokenStream) -> TokenStream
                     let mut builder = wallet.build_tx();
                     builder.add_recipient(node_addr.script_pubkey().clone(), 49_000).enable_rbf();
                     let (mut psbt, details) = builder.finish().unwrap();
-                    let finalized = wallet.sign(&mut psbt, None).unwrap();
+                    let finalized = wallet.sign(&mut psbt, Default::default()).unwrap();
                     assert!(finalized, "Cannot finalize transaction");
                     wallet.broadcast(psbt.extract_tx()).unwrap();
                     wallet.sync(noop_progress(), None).unwrap();
@@ -520,7 +520,7 @@ pub fn bdk_blockchain_tests(attr: TokenStream, item: TokenStream) -> TokenStream
                     let (mut new_psbt, new_details) = builder.finish().unwrap();
                     println!("{:#?}", new_details);
 
-                    let finalized = wallet.sign(&mut new_psbt, None).unwrap();
+                    let finalized = wallet.sign(&mut new_psbt, Default::default()).unwrap();
                     assert!(finalized, "Cannot finalize transaction");
                     wallet.broadcast(new_psbt.extract_tx()).unwrap();
                     wallet.sync(noop_progress(), None).unwrap();