Skip to content

Commit c7c5ca6

Browse files
committed
typeck: fix mistake in previous correction to thresh exec_stack_count
I am putting this in its own commit because it's kinda hard to reason about and I wanted the change to be separate, even though it changes the same code that previous commits changed.
1 parent 1e6aead commit c7c5ca6

File tree

2 files changed

+14
-5
lines changed

2 files changed

+14
-5
lines changed

src/miniscript/mod.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1906,15 +1906,18 @@ mod tests {
19061906
let ms_str = "thresh(2,pk(A),s:pk(B),s:pk(C))";
19071907
let ms = Miniscript::<String, Segwitv0>::from_str_insane(ms_str).unwrap();
19081908

1909-
// Each pk has exec_stack_count of 1
1910-
// With the fix, threshold should take max(1,1,1) = 1, not sum 1+1+1 = 3
1909+
// Each pk has exec_stack_count of 1, plus an extra stack element for the thresh accumulator.
1910+
// With the fix, threshold should take max(1,1,1) + 1 = 2, not sum 1+1+1 = 3
19111911
if let Some(sat_data) = ms.ext.sat_data {
1912-
assert_eq!(sat_data.max_exec_stack_count, 1);
1912+
assert_eq!(sat_data.max_exec_stack_count, 2);
19131913
} else {
19141914
panic!("Expected sat_data to be Some");
19151915
}
19161916

1917-
// Test with a more complex threshold to make the difference more obvious
1917+
// Test with a more complex threshold, where the first child has a strictly higher
1918+
// exec_stack_count. This time, we take the maximum *without* adding +1 for the
1919+
// accumulator, since on the first child of `thresh` there is no accumulator yet
1920+
// (its initial value is the output value for the first child).
19181921
let complex_ms_str = "thresh(1,and_b(pk(A),s:pk(B)),s:pk(C))";
19191922
let complex_ms = Miniscript::<String, Segwitv0>::from_str_insane(complex_ms_str).unwrap();
19201923

src/miniscript/types/extra_props.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -864,7 +864,13 @@ impl ExtData {
864864
(
865865
&mut max_exec_stack_count,
866866
&(|data: SatData| data.max_exec_stack_count) as &dyn Fn(_) -> usize,
867-
&(|acc: usize, x: usize| cmp::max(acc, x)) as &dyn Fn(_, _) -> usize,
867+
// For each fragment except the first, we have the accumulated count on the
868+
// stack, which sits there during the whole child execution before
869+
// being ADDed to the result at the end.
870+
//
871+
// We use "acc > 0" as a hacky way to check "is this the first child
872+
// or not".
873+
&(|acc: usize, x: usize| cmp::max(acc, x + usize::from(acc > 0))) as &dyn Fn(_, _) -> usize,
868874
),
869875
(
870876
&mut max_exec_op_count,

0 commit comments

Comments
 (0)