Merge #476: Another attempt at max_satisfaction_weight fixes

7296f8e Introduce `max_weight_to_satisfy` (志宇)

Pull request description:

  Replaces #474, refer to #474 (comment)

  This PR has two intentions:

  1. Redefine `max_satisfaction_weight` to be the difference in `TxIn` weight between "satisfied" and "unsatisfied" states. In an "unsatisfied" state, we still need to include the `scriptSigLen` varint, as well as the `witnessStackLen` (for txs with at least one segwit spend).
  2. Attempt further fixes to improve accuracy of `max_satisfaction_weight`.

  Comments, tests and examples have been updated to reflect the above intentions.

  ### Notes for reviewers

  The new definition of `max_satisfaction_weight` can be seen in this comment:
  https://github.com/rust-bitcoin/rust-miniscript/blob/08cff39fa862ff957c7ff96d17a0011dd6446f87/src/descriptor/mod.rs#L320-L339

ACKs for top commit:
  sanket1729:
    ACK 7296f8e

Tree-SHA512: ecae8ff742198289b598aefde83ad66a4b7c7cb67da0625b4d84271df510331408c3da6fc8796fc234ce095af08a2f34a5beb5a10549d7ce20a9878b3ab6fd47

Author: sanket1729

embedded/src/main.rs

@@ -53,7 +53,7 @@ fn main() -> ! {
     assert!(desc.sanity_check().is_ok());
 
     // Estimate the satisfaction cost
-    assert_eq!(desc.max_satisfaction_weight().unwrap(), 293);
+    assert_eq!(desc.max_weight_to_satisfy().unwrap(), 288);
     // end miniscript test
 
     // exit QEMU

examples/psbt_sign_finalize.rs

@@ -33,7 +33,7 @@ fn main() {
     );
     println!(
         "Weight for witness satisfaction cost {}",
-        bridge_descriptor.max_satisfaction_weight().unwrap()
+        bridge_descriptor.max_weight_to_satisfy().unwrap()
     );
 
     let master_private_key_str = "cQhdvB3McbBJdx78VSSumqoHQiSXs75qwLptqwxSQBNBMDxafvaw";

examples/sign_multisig.rs

@@ -30,9 +30,9 @@ fn main() {
     let descriptor = miniscript::Descriptor::<bitcoin::PublicKey>::from_str(&s).unwrap();
 
     // Check weight for witness satisfaction cost ahead of time.
-    // 4 (scriptSig length of 0) + 1 (witness stack size) + 106 (serialized witnessScript)
-    // + 73*2 (signature length + signatures + sighash bytes) + 1 (dummy byte) = 258
-    assert_eq!(descriptor.max_satisfaction_weight().unwrap(), 258);
+    // 106 (serialized witnessScript)
+    // + 73*2 (signature length + signatures + sighash bytes) + 1 (dummy byte) = 253
+    assert_eq!(descriptor.max_weight_to_satisfy().unwrap(), 253);
 
     // Sometimes it is necessary to have additional information to get the
     // `bitcoin::PublicKey` from the `MiniscriptKey` which can be supplied by

examples/taproot.rs

@@ -100,11 +100,11 @@ fn main() {
 
     // Max Satisfaction Weight for compilation, corresponding to the script-path spend
     // `multi_a(2,PUBKEY_1,PUBKEY_2) at taptree depth 1, having
-    // Max Witness Size = scriptSig len + witnessStack len + varint(control_block_size) +
-    //                    control_block size + varint(script_size) + script_size + max_satisfaction_size
-    //                  = 4 + 1 + 1 + 65 + 1 + 70 + 132 = 274
-    let max_sat_wt = real_desc.max_satisfaction_weight().unwrap();
-    assert_eq!(max_sat_wt, 274);
+    // Max Witness Size = varint(control_block_size) + control_block size +
+    //                    varint(script_size) + script_size + max_satisfaction_size
+    //                  = 1 + 65 + 1 + 70 + 132 = 269
+    let max_sat_wt = real_desc.max_weight_to_satisfy().unwrap();
+    assert_eq!(max_sat_wt, 269);
 
     // Compute the bitcoin address and check if it matches
     let network = Network::Bitcoin;

src/descriptor/bare.rs

@@ -56,6 +56,26 @@ impl<Pk: MiniscriptKey> Bare<Pk> {
         Ok(())
     }
 
+    /// Computes an upper bound on the difference between a non-satisfied
+    /// `TxIn`'s `segwit_weight` and a satisfied `TxIn`'s `segwit_weight`
+    ///
+    /// Since this method uses `segwit_weight` instead of `legacy_weight`,
+    /// if you want to include only legacy inputs in your transaction,
+    /// you should remove 1WU from each input's `max_weight_to_satisfy`
+    /// for a more accurate estimate.
+    ///
+    /// Assumes all ECDSA signatures are 73 bytes, including push opcode and
+    /// sighash suffix.
+    ///
+    /// # Errors
+    /// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)).
+    pub fn max_weight_to_satisfy(&self) -> Result<usize, Error> {
+        let scriptsig_size = self.ms.max_satisfaction_size()?;
+        // scriptSig varint difference between non-satisfied (0) and satisfied
+        let scriptsig_varint_diff = varint_len(scriptsig_size) - varint_len(0);
+        Ok(4 * (scriptsig_varint_diff + scriptsig_size))
+    }
+
     /// Computes an upper bound on the weight of a satisfying witness to the
     /// transaction.
     ///
@@ -65,6 +85,7 @@ impl<Pk: MiniscriptKey> Bare<Pk> {
     ///
     /// # Errors
     /// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)).
+    #[deprecated(note = "use max_weight_to_satisfy instead")]
     pub fn max_satisfaction_weight(&self) -> Result<usize, Error> {
         let scriptsig_len = self.ms.max_satisfaction_size()?;
         Ok(4 * (varint_len(scriptsig_len) + scriptsig_len))
@@ -202,6 +223,27 @@ impl<Pk: MiniscriptKey> Pkh<Pk> {
         self.pk
     }
 
+    /// Computes an upper bound on the difference between a non-satisfied
+    /// `TxIn`'s `segwit_weight` and a satisfied `TxIn`'s `segwit_weight`
+    ///
+    /// Since this method uses `segwit_weight` instead of `legacy_weight`,
+    /// if you want to include only legacy inputs in your transaction,
+    /// you should remove 1WU from each input's `max_weight_to_satisfy`
+    /// for a more accurate estimate.
+    ///
+    /// Assumes all ECDSA signatures are 73 bytes, including push opcode and
+    /// sighash suffix.
+    ///
+    /// # Errors
+    /// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)).
+    pub fn max_weight_to_satisfy(&self) -> usize {
+        // OP_72 + <sig(71)+sigHash(1)> + OP_33 + <pubkey>
+        let scriptsig_size = 73 + BareCtx::pk_len(&self.pk);
+        // scriptSig varint different between non-satisfied (0) and satisfied
+        let scriptsig_varint_diff = varint_len(scriptsig_size) - varint_len(0);
+        4 * (scriptsig_varint_diff + scriptsig_size)
+    }
+
     /// Computes an upper bound on the weight of a satisfying witness to the
     /// transaction.
     ///

src/descriptor/mod.rs

@@ -307,6 +307,56 @@ impl<Pk: MiniscriptKey> Descriptor<Pk> {
         }
     }
 
+    /// Computes an upper bound on the difference between a non-satisfied
+    /// `TxIn`'s `segwit_weight` and a satisfied `TxIn`'s `segwit_weight`
+    ///
+    /// Since this method uses `segwit_weight` instead of `legacy_weight`,
+    /// if you want to include only legacy inputs in your transaction,
+    /// you should remove 1WU from each input's `max_weight_to_satisfy`
+    /// for a more accurate estimate.
+    ///
+    /// In other words, for segwit inputs or legacy inputs included in
+    /// segwit transactions, the following will hold for each input if
+    /// that input was satisfied with the largest possible witness:
+    /// ```ignore
+    /// for i in 0..transaction.input.len() {
+    ///     assert_eq!(
+    ///         descriptor_for_input[i].max_weight_to_satisfy(),
+    ///         transaction.input[i].segwit_weight() - Txin::default().segwit_weight()
+    ///     );
+    /// }
+    /// ```
+    ///
+    /// Instead, for legacy transactions, the following will hold for each input
+    /// if that input was satisfied with the largest possible witness:
+    /// ```ignore
+    /// for i in 0..transaction.input.len() {
+    ///     assert_eq!(
+    ///         descriptor_for_input[i].max_weight_to_satisfy(),
+    ///         transaction.input[i].legacy_weight() - Txin::default().legacy_weight()
+    ///     );
+    /// }
+    /// ```
+    ///
+    /// Assumes all ECDSA signatures are 73 bytes, including push opcode and
+    /// sighash suffix.
+    /// Assumes all Schnorr signatures are 66 bytes, including push opcode and
+    /// sighash suffix.
+    ///
+    /// # Errors
+    /// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)).
+    pub fn max_weight_to_satisfy(&self) -> Result<usize, Error> {
+        let weight = match *self {
+            Descriptor::Bare(ref bare) => bare.max_weight_to_satisfy()?,
+            Descriptor::Pkh(ref pkh) => pkh.max_weight_to_satisfy(),
+            Descriptor::Wpkh(ref wpkh) => wpkh.max_weight_to_satisfy(),
+            Descriptor::Wsh(ref wsh) => wsh.max_weight_to_satisfy()?,
+            Descriptor::Sh(ref sh) => sh.max_weight_to_satisfy()?,
+            Descriptor::Tr(ref tr) => tr.max_weight_to_satisfy()?,
+        };
+        Ok(weight)
+    }
+
     /// Computes an upper bound on the weight of a satisfying witness to the
     /// transaction.
     ///
@@ -316,6 +366,8 @@ impl<Pk: MiniscriptKey> Descriptor<Pk> {
     ///
     /// # Errors
     /// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)).
+    #[deprecated(note = "use max_weight_to_satisfy instead")]
+    #[allow(deprecated)]
     pub fn max_satisfaction_weight(&self) -> Result<usize, Error> {
         let weight = match *self {
             Descriptor::Bare(ref bare) => bare.max_satisfaction_weight()?,

src/descriptor/segwitv0.rs

@@ -72,6 +72,34 @@ impl<Pk: MiniscriptKey> Wsh<Pk> {
         Ok(())
     }
 
+    /// Computes an upper bound on the difference between a non-satisfied
+    /// `TxIn`'s `segwit_weight` and a satisfied `TxIn`'s `segwit_weight`
+    ///
+    /// Assumes all ECDSA signatures are 73 bytes, including push opcode and
+    /// sighash suffix.
+    ///
+    /// # Errors
+    /// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)).
+    pub fn max_weight_to_satisfy(&self) -> Result<usize, Error> {
+        let (redeem_script_size, max_sat_elems, max_sat_size) = match self.inner {
+            WshInner::SortedMulti(ref smv) => (
+                smv.script_size(),
+                smv.max_satisfaction_witness_elements(),
+                smv.max_satisfaction_size(),
+            ),
+            WshInner::Ms(ref ms) => (
+                ms.script_size(),
+                ms.max_satisfaction_witness_elements()?,
+                ms.max_satisfaction_size()?,
+            ),
+        };
+        // stack size varint difference between non-satisfied (0) and satisfied
+        // `max_sat_elems` is inclusive of the "witness script" (redeem script)
+        let stack_varint_diff = varint_len(max_sat_elems) - varint_len(0);
+
+        Ok(stack_varint_diff + varint_len(redeem_script_size) + redeem_script_size + max_sat_size)
+    }
+
     /// Computes an upper bound on the weight of a satisfying witness to the
     /// transaction.
     ///
@@ -81,6 +109,7 @@ impl<Pk: MiniscriptKey> Wsh<Pk> {
     ///
     /// # Errors
     /// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)).
+    #[deprecated(note = "use max_weight_to_satisfy instead")]
     pub fn max_satisfaction_weight(&self) -> Result<usize, Error> {
         let (script_size, max_sat_elems, max_sat_size) = match self.inner {
             WshInner::SortedMulti(ref smv) => (
@@ -315,6 +344,19 @@ impl<Pk: MiniscriptKey> Wpkh<Pk> {
         }
     }
 
+    /// Computes an upper bound on the difference between a non-satisfied
+    /// `TxIn`'s `segwit_weight` and a satisfied `TxIn`'s `segwit_weight`
+    ///
+    /// Assumes all ec-signatures are 73 bytes, including push opcode and
+    /// sighash suffix.
+    pub fn max_weight_to_satisfy(&self) -> usize {
+        // stack items: <varint(sig+sigHash)> <sig(71)+sigHash(1)> <varint(pubkey)> <pubkey>
+        let stack_items_size = 73 + Segwitv0::pk_len(&self.pk);
+        // stackLen varint difference between non-satisfied (0) and satisfied
+        let stack_varint_diff = varint_len(2) - varint_len(0);
+        stack_varint_diff + stack_items_size
+    }
+
     /// Computes an upper bound on the weight of a satisfying witness to the
     /// transaction.
     ///

src/descriptor/sh.rs

@@ -196,15 +196,66 @@ impl<Pk: MiniscriptKey> Sh<Pk> {
         }
     }
 
+    /// Computes an upper bound on the difference between a non-satisfied
+    /// `TxIn`'s `segwit_weight` and a satisfied `TxIn`'s `segwit_weight`
+    ///
+    /// Since this method uses `segwit_weight` instead of `legacy_weight`,
+    /// if you want to include only legacy inputs in your transaction,
+    /// you should remove 1WU from each input's `max_weight_to_satisfy`
+    /// for a more accurate estimate.
+    ///
+    /// Assumes all ec-signatures are 73 bytes, including push opcode and
+    /// sighash suffix.
+    ///
+    /// # Errors
+    /// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)).
+    pub fn max_weight_to_satisfy(&self) -> Result<usize, Error> {
+        let (scriptsig_size, witness_size) = match self.inner {
+            // add weighted script sig, len byte stays the same
+            ShInner::Wsh(ref wsh) => {
+                // scriptSig: OP_34 <OP_0 OP_32 <32-byte-hash>>
+                let scriptsig_size = 1 + 1 + 1 + 32;
+                let witness_size = wsh.max_weight_to_satisfy()?;
+                (scriptsig_size, witness_size)
+            }
+            ShInner::SortedMulti(ref smv) => {
+                let ss = smv.script_size();
+                let ps = push_opcode_size(ss);
+                let scriptsig_size = ps + ss + smv.max_satisfaction_size();
+                (scriptsig_size, 0)
+            }
+            // add weighted script sig, len byte stays the same
+            ShInner::Wpkh(ref wpkh) => {
+                // scriptSig: OP_22 <OP_0 OP_20 <20-byte-hash>>
+                let scriptsig_size = 1 + 1 + 1 + 20;
+                let witness_size = wpkh.max_weight_to_satisfy();
+                (scriptsig_size, witness_size)
+            }
+            ShInner::Ms(ref ms) => {
+                let ss = ms.script_size();
+                let ps = push_opcode_size(ss);
+                let scriptsig_size = ps + ss + ms.max_satisfaction_size()?;
+                (scriptsig_size, 0)
+            }
+        };
+
+        // scriptSigLen varint difference between non-satisfied (0) and satisfied
+        let scriptsig_varint_diff = varint_len(scriptsig_size) - varint_len(0);
+
+        Ok(4 * (scriptsig_varint_diff + scriptsig_size) + witness_size)
+    }
+
     /// Computes an upper bound on the weight of a satisfying witness to the
     /// transaction.
     ///
-    /// Assumes all ec-signatures are 73 bytes, including push opcode and
+    /// Assumes all ECDSA signatures are 73 bytes, including push opcode and
     /// sighash suffix. Includes the weight of the VarInts encoding the
     /// scriptSig and witness stack length.
     ///
     /// # Errors
     /// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)).
+    #[deprecated(note = "use max_weight_to_satisfy instead")]
+    #[allow(deprecated)]
     pub fn max_satisfaction_weight(&self) -> Result<usize, Error> {
         Ok(match self.inner {
             // add weighted script sig, len byte stays the same

src/descriptor/tr.rs

@@ -247,6 +247,55 @@ impl<Pk: MiniscriptKey> Tr<Pk> {
         Ok(())
     }
 
+    /// Computes an upper bound on the difference between a non-satisfied
+    /// `TxIn`'s `segwit_weight` and a satisfied `TxIn`'s `segwit_weight`
+    ///
+    /// Assumes all Schnorr signatures are 66 bytes, including push opcode and
+    /// sighash suffix.
+    ///
+    /// # Errors
+    /// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)).
+    pub fn max_weight_to_satisfy(&self) -> Result<usize, Error> {
+        let tree = match self.taptree() {
+            None => {
+                // key spend path
+                // item: varint(sig+sigHash) + <sig(64)+sigHash(1)>
+                let item_sig_size = 1 + 65;
+                // 1 stack item
+                let stack_varint_diff = varint_len(1) - varint_len(0);
+
+                return Ok(stack_varint_diff + item_sig_size);
+            }
+            // script path spend..
+            Some(tree) => tree,
+        };
+
+        tree.iter()
+            .filter_map(|(depth, ms)| {
+                let script_size = ms.script_size();
+                let max_sat_elems = ms.max_satisfaction_witness_elements().ok()?;
+                let max_sat_size = ms.max_satisfaction_size().ok()?;
+                let control_block_size = control_block_len(depth);
+
+                // stack varint difference (+1 for ctrl block, witness script already included)
+                let stack_varint_diff = varint_len(max_sat_elems + 1) - varint_len(0);
+
+                Some(
+                    stack_varint_diff +
+                    // size of elements to satisfy script
+                    max_sat_size +
+                    // second to last element: script
+                    varint_len(script_size) +
+                    script_size +
+                    // last element: control block
+                    varint_len(control_block_size) +
+                    control_block_size,
+                )
+            })
+            .max()
+            .ok_or(Error::ImpossibleSatisfaction)
+    }
+
     /// Computes an upper bound on the weight of a satisfying witness to the
     /// transaction.
     ///
@@ -256,6 +305,7 @@ impl<Pk: MiniscriptKey> Tr<Pk> {
     ///
     /// # Errors
     /// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)).
+    #[deprecated(note = "use max_weight_to_satisfy instead")]
     pub fn max_satisfaction_weight(&self) -> Result<usize, Error> {
         let tree = match self.taptree() {
             // key spend path:

src/lib.rs

@@ -62,7 +62,14 @@
 //! assert!(desc.sanity_check().is_ok());
 //!
 //! // Estimate the satisfaction cost.
-//! assert_eq!(desc.max_satisfaction_weight().unwrap(), 293);
+//! // scriptSig: OP_PUSH34 <OP_0 OP_32 <32-byte-hash>>
+//! // = (1 + 1 + 1 + 32) * 4 = 140 WU
+//! // redeemScript: varint <OP_33 <pk1> OP_CHECKSIG OP_IFDUP OP_NOTIF OP_33 <pk2> OP_CHECKSIG OP_ENDIF>
+//! // = 1 + (1 + 33 + 1 + 1 + 1 + 1 + 33 + 1 + 1) = 74 WU
+//! // stackItem[Sig]: varint <sig+sighash>
+//! // = 1 + 73 = 74 WU
+//! // Expected satisfaction weight: 140 + 74 + 74 = 288
+//! assert_eq!(desc.max_weight_to_satisfy().unwrap(), 288);
 //! ```
 //!