Merge #1040: fix(miniscript): count exactly k satisfactions in thresh max-size bounds

d7801d1 fix(miniscript): count exactly k satisfactions in thresh max-size bounds (russeree)

Pull request description:

  A `thresh(k, ...)` witness satisfies exactly k children: the script checks
  the running sum against k with `OP_EQUAL`, so any witness with a different
  number of satisfied children is invalid. The maximum-satisfaction
  computation in `ExtData::threshold` nevertheless selected k+1 children as
  satisfied (`i <= k` instead of `i < k`), e.g. computing the maximum witness
  size of `thresh(1,pk(A),s:pk(B))` as 146 bytes (two signatures) instead of
  74 (one signature plus one empty dissatisfaction).
  
  This inflates `Miniscript::max_satisfaction_weight()` and everything
  derived from it, making fee/weight estimates overestimate by up to one
  largest-child satisfaction per thresh node.
  
  Found by differential testing against Bitcoin Core's miniscript
  implementation (`Node::GetWitnessSize`/`GetStackSize` agree with the fixed
  values, and with the reference implementation's satisfaction table,
  which permits exactly k satisfactions).


ACKs for top commit:
  apoelstra:
    ACK d7801d1; successfully ran local tests


Tree-SHA512: 6b4a96f1d6fe27794d38c53b605630571a3333a9645ef7faf31014c67b2a693e4c72a02d321437938d121eab9bda86c4ea646ab2c5daa1cfd03dbac798b39f55

Author: apoelstra

src/miniscript/mod.rs

@@ -2112,6 +2112,23 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_thresh_sat_data_exact_k_satisfactions() {
+        let ms =
+            Miniscript::<String, Segwitv0>::from_str_insane("thresh(1,pk(A),s:pk(B))").unwrap();
+        let sat = ms.ext.sat_data.expect("thresh(1,...) is satisfiable");
+        assert_eq!(sat.max_witness_stack_size, 74);
+        assert_eq!(sat.max_witness_stack_count, 2);
+        assert_eq!(sat.max_script_sig_size, 74);
+
+        let ms = Miniscript::<String, Segwitv0>::from_str_insane("thresh(2,pk(A),s:pk(B),a:pk(C))")
+            .unwrap();
+        let sat = ms.ext.sat_data.expect("thresh(2,...) is satisfiable");
+        assert_eq!(sat.max_witness_stack_size, 147);
+        assert_eq!(sat.max_witness_stack_count, 3);
+        assert_eq!(sat.max_script_sig_size, 147);
+    }
+
     #[test]
     fn test_context_global_consensus() {
         // Test from string tests

src/miniscript/types/extra_props.rs

@@ -901,7 +901,7 @@ impl ExtData {
                     .rev()
                     .enumerate()
                     .try_fold(0, |acc, (i, &(sat, dissat))| {
-                        if i <= k {
+                        if i < k {
                             sat.map(|x| cmp(acc, proj(x)))
                         } else {
                             dissat.map(|y| cmp(acc, proj(y)))