From d7801d1405849e1b1eb062ef4e6f1334e253d8c1 Mon Sep 17 00:00:00 2001 From: russeree Date: Tue, 1 Sep 2026 00:16:08 -0700 Subject: [PATCH] fix(miniscript): count exactly k satisfactions in thresh max-size bounds 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). --- src/miniscript/mod.rs | 17 +++++++++++++++++ src/miniscript/types/extra_props.rs | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index 7b5feebc6..70353c820 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -2112,6 +2112,23 @@ mod tests { } } + #[test] + fn test_thresh_sat_data_exact_k_satisfactions() { + let ms = + Miniscript::::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::::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 diff --git a/src/miniscript/types/extra_props.rs b/src/miniscript/types/extra_props.rs index 348fbe43b..0d2ca8866 100644 --- a/src/miniscript/types/extra_props.rs +++ b/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)))