More max_satisfaction_weight fixes

This commit 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.

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_satisfaction_weight().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
+    // 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_satisfaction_weight().unwrap();
-    assert_eq!(max_sat_wt, 274);
+    assert_eq!(max_sat_wt, 269);
 
     // Compute the bitcoin address and check if it matches
     let network = Network::Bitcoin;

src/descriptor/bare.rs

@@ -66,18 +66,16 @@ impl<Pk: MiniscriptKey> Bare<Pk> {
         Ok(())
     }
 
-    /// Computes an upper bound on the weight of a satisfying witness to the
-    /// transaction.
+    /// Computes an upper bound on the difference in weight between a
+    /// non-satisfied `TxIn` (with empty `scriptSig` and `witness` fields) and a
+    /// satisfied `TxIn`.
     ///
-    /// Assumes all ec-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)).
+    /// Assumes a ec-signature is 73 bytes (inclusive of sigHash and op_push/varint).
     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))
+        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))
     }
 }
 
@@ -211,14 +209,17 @@ impl<Pk: MiniscriptKey> Pkh<Pk> {
         self.pk
     }
 
-    /// Computes an upper bound on the weight of a satisfying witness to the
-    /// transaction.
+    /// Computes an upper bound on the difference in weight between a
+    /// non-satisfied `TxIn` (with empty `scriptSig` and `witness` fields) and a
+    /// satisfied `TxIn`.
     ///
-    /// Assumes all ec-signatures are 73 bytes, including push opcode and
-    /// sighash suffix. Includes the weight of the VarInts encoding the
-    /// scriptSig and witness stack length.
+    /// Assumes a ec-signature is 73 bytes (inclusive of sigHash and op_push/varint).
     pub fn max_satisfaction_weight(&self) -> usize {
-        4 * (1 + 73 + BareCtx::pk_len(&self.pk))
+        // 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)
     }
 }
 

src/descriptor/mod.rs

@@ -317,12 +317,22 @@ impl<Pk: MiniscriptKey> Descriptor<Pk> {
         }
     }
 
-    /// Computes an upper bound on the weight of a satisfying witness to the
-    /// transaction.
+    /// Computes an upper bound on the difference in weight between a
+    /// non-satisfied `TxIn` and a satisfied `TxIn`.
     ///
-    /// Assumes all ec-signatures are 73 bytes, including push opcode and
-    /// sighash suffix. Includes the weight of the VarInts encoding the
-    /// scriptSig and witness stack length.
+    /// A non-satisfied `TxIn` contains the `scriptSigLen` varint recording the
+    /// value 0 (which is 4WU) and also the `witnessStackLen` for transactions
+    /// that have at least one witness spend, which will also record the value 0
+    /// (which is 1WU).
+    ///
+    /// Hence, "satisfaction weight" contains the additional varint weight of
+    /// `scriptSigLen` and `witnessStackLen` (which occurs when the varint value
+    /// increases over the 1byte threshold), and also the weights of `scriptSig`
+    /// and the rest of the `witnessField` (which is the stack items and the
+    /// stack-item-len varints of each item).
+    ///
+    /// This assumes a ec-signatures is always 73 bytes, and the sighash prefix
+    /// is always included (+1 byte).
     ///
     /// # Errors
     /// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)).

src/descriptor/segwitv0.rs

@@ -84,17 +84,16 @@ impl<Pk: MiniscriptKey> Wsh<Pk> {
         Ok(())
     }
 
-    /// Computes an upper bound on the weight of a satisfying witness to the
-    /// transaction.
+    /// Computes an upper bound on the difference in weight between a
+    /// non-satisfied `TxIn` (with empty `scriptSig` and `witness` fields) and a
+    /// satisfied `TxIn`.
     ///
-    /// Assumes all ec-signatures are 73 bytes, including push opcode and
-    /// sighash suffix. Includes the weight of the VarInts encoding the
-    /// scriptSig and witness stack length.
+    /// Assumes a ec-signature is 73 bytes (inclusive of sigHash and op_push/varint).
     ///
     /// # Errors
     /// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)).
     pub fn max_satisfaction_weight(&self) -> Result<usize, Error> {
-        let (script_size, max_sat_elems, max_sat_size) = match self.inner {
+        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(),
@@ -106,11 +105,11 @@ impl<Pk: MiniscriptKey> Wsh<Pk> {
                 ms.max_satisfaction_size()?,
             ),
         };
-        Ok(4 +  // scriptSig length byte
-            varint_len(script_size) +
-            script_size +
-            varint_len(max_sat_elems) +
-            max_sat_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)
     }
 }
 
@@ -322,14 +321,17 @@ impl<Pk: MiniscriptKey> Wpkh<Pk> {
         }
     }
 
-    /// Computes an upper bound on the weight of a satisfying witness to the
-    /// transaction.
+    /// Computes an upper bound on the difference in weight between a
+    /// non-satisfied `TxIn` (with empty `scriptSig` and `witness` fields) and a
+    /// satisfied `TxIn`.
     ///
-    /// Assumes all ec-signatures are 73 bytes, including push opcode and
-    /// sighash suffix. Includes the weight of the VarInts encoding the
-    /// scriptSig and witness stack length.
+    /// Assumes a ec-signature is 73 bytes (inclusive of sigHash and op_push/varint).
     pub fn max_satisfaction_weight(&self) -> usize {
-        4 + 1 + 73 + Segwitv0::pk_len(&self.pk)
+        // 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
     }
 }
 

src/descriptor/sh.rs

@@ -205,32 +205,45 @@ impl<Pk: MiniscriptKey> Sh<Pk> {
         }
     }
 
-    /// Computes an upper bound on the weight of a satisfying witness to the
-    /// transaction.
+    /// Computes an upper bound on the difference in weight between a
+    /// non-satisfied `TxIn` (with empty `scriptSig` and `witness` fields) and a
+    /// satisfied `TxIn`.
     ///
-    /// Assumes all ec-signatures are 73 bytes, including push opcode and
-    /// sighash suffix. Includes the weight of the VarInts encoding the
-    /// scriptSig and witness stack length.
+    /// Assumes a ec-signature is 73 bytes and the sigHash suffix is always
+    /// included (+1 byte).
     ///
     /// # Errors
     /// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)).
     pub fn max_satisfaction_weight(&self) -> Result<usize, Error> {
         Ok(match self.inner {
             // add weighted script sig, len byte stays the same
-            ShInner::Wsh(ref wsh) => 4 * 35 + wsh.max_satisfaction_weight()?,
+            ShInner::Wsh(ref wsh) => {
+                // scriptSig: OP_0 OP_32 <32-byte-hash>
+                let scriptsig_size = 1 + 1 + 32;
+                // scriptSigLen varint difference between non-satisfied (0) and satisfied
+                let scriptsig_varint_diff = varint_len(scriptsig_size) - varint_len(0);
+                4 * (scriptsig_varint_diff + scriptsig_size) + wsh.max_satisfaction_weight()?
+            }
             ShInner::SortedMulti(ref smv) => {
                 let ss = smv.script_size();
                 let ps = push_opcode_size(ss);
-                let scriptsig_len = ps + ss + smv.max_satisfaction_size();
-                4 * (varint_len(scriptsig_len) + scriptsig_len)
+                let scriptsig_size = ps + ss + smv.max_satisfaction_size();
+                let scirptsig_varint_diff = varint_len(scriptsig_size) - varint_len(0);
+                4 * (scirptsig_varint_diff + scriptsig_size)
             }
             // add weighted script sig, len byte stays the same
-            ShInner::Wpkh(ref wpkh) => 4 * 23 + wpkh.max_satisfaction_weight(),
+            ShInner::Wpkh(ref wpkh) => {
+                // scriptSig: OP_0 OP_20 <20-byte-hash>
+                let scriptsig_size = 1 + 1 + 20;
+                let scriptsig_varint_diff = varint_len(scriptsig_size) - varint_len(0);
+                4 * (scriptsig_varint_diff + scriptsig_size) + wpkh.max_satisfaction_weight()
+            }
             ShInner::Ms(ref ms) => {
                 let ss = ms.script_size();
                 let ps = push_opcode_size(ss);
-                let scriptsig_len = ps + ss + ms.max_satisfaction_size()?;
-                4 * (varint_len(scriptsig_len) + scriptsig_len)
+                let scriptsig_size = ps + ss + ms.max_satisfaction_size()?;
+                let scriptsig_varint_diff = varint_len(scriptsig_size) - varint_len(0);
+                4 * (scriptsig_varint_diff + scriptsig_size)
             }
         })
     }

src/descriptor/tr.rs

@@ -254,20 +254,26 @@ impl<Pk: MiniscriptKey> Tr<Pk> {
         Ok(())
     }
 
-    /// Computes an upper bound on the weight of a satisfying witness to the
-    /// transaction.
+    /// Computes an upper bound on the difference in weight between a
+    /// non-satisfied `TxIn` (with empty `scriptSig` and `witness` fields) and a
+    /// satisfied `TxIn`.
     ///
-    /// Assumes all ec-signatures are 73 bytes, including push opcode and
-    /// sighash suffix. Includes the weight of the VarInts encoding the
-    /// scriptSig and witness stack length.
+    /// Assumes a ec-signature is 73 bytes and the sigHash suffix is always
+    /// included (+1 byte).
     ///
     /// # Errors
     /// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)).
     pub fn max_satisfaction_weight(&self) -> Result<usize, Error> {
         let tree = match self.taptree() {
-            // key spend path:
-            // scriptSigLen(4) + stackLen(1) + stack[Sig]Len(1) + stack[Sig](65)
-            None => return Ok(4 + 1 + 1 + 65),
+            None => {
+                // key spend path
+                // item: varint(sig+sigHash) + <sig(74)+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,
         };
@@ -278,11 +284,12 @@ impl<Pk: MiniscriptKey> Tr<Pk> {
                 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(
-                    // scriptSig len byte
-                    4 +
-                    // witness field stack len (+2 for control block & script)
-                    varint_len(max_sat_elems + 2) +
+                    stack_varint_diff +
                     // size of elements to satisfy script
                     max_sat_size +
                     // second to last element: script

src/lib.rs

@@ -74,7 +74,14 @@
 //! assert!(desc.sanity_check().is_ok());
 //!
 //! // Estimate the satisfaction cost
-//! assert_eq!(desc.max_satisfaction_weight().unwrap(), 293);
+//! // scriptSig:
+//! // * OP_0 OP_32 <32-byte-hash>
+//! // = (1 + 1 + 32) * 4 = 136 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 + 72+1 = 74 WU (TODO: Figure out why sig is 72bytes here)
+//! // Expected satisfaction weight: 136 + 74 + 74 = 284
+//! assert_eq!(desc.max_satisfaction_weight().unwrap(), 284);
 //! ```
 //!
 

src/miniscript/context.rs

@@ -222,6 +222,7 @@ where
 
     /// Depending on script context, the size of a satifaction witness may slightly differ.
     fn max_satisfaction_size<Pk: MiniscriptKey>(ms: &Miniscript<Pk, Self>) -> Option<usize>;
+
     /// Depending on script Context, some of the Terminals might not
     /// be valid under the current consensus rules.
     /// Or some of the script resource limits may have been exceeded.