Fix and actually test the sat stack elements calculation

[darosior]

There are unit tests to exerce the calculation of the maximum number of elements of a satisfaction: https://github.com/rust-bitcoin/rust-miniscript/blob/a2d6fff55a6b4a81a92fe740ee4dea86746875cc/src/miniscript/mod.rs#L586-L631

However they are not executed... https://github.com/rust-bitcoin/rust-miniscript/blob/a2d6fff55a6b4a81a92fe740ee4dea86746875cc/src/miniscript/mod.rs#L570

Actually executing them makes the test fail. It looks like most vectors were taken from the C++ implem and therefore the Rust implem of the calculation is not on par with the C++ one.

diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs
index c20ed89..2b4d0da 100644
--- a/src/miniscript/mod.rs
+++ b/src/miniscript/mod.rs
@@ -567,7 +567,7 @@ mod tests {
         non_mal: bool,
         need_sig: bool,
         ops: usize,
-        _stack: usize,
+        stack: usize,
     ) {
         let ms: Result<Segwitv0Script, _> = Miniscript::from_str_insane(ms);
         match (ms, valid) {
@@ -576,6 +576,7 @@ mod tests {
                 assert_eq!(ms.ty.mall.non_malleable, non_mal);
                 assert_eq!(ms.ty.mall.safe, need_sig);
                 assert_eq!(ms.ext.ops_count_sat.unwrap(), ops);
+                assert_eq!(ms.ext.stack_elem_count_sat.unwrap(), stack);
             }
             (Err(_), false) => return,
             _ => unreachable!(),

A quick look at the first vector failing with the above patch suggests that this is the calculation that needs to be fixed: the vector looks right. https://github.com/rust-bitcoin/rust-miniscript/blob/a2d6fff55a6b4a81a92fe740ee4dea86746875cc/src/miniscript/mod.rs#L610

[darosior]

Looks like it was introduced in https://github.com/rust-bitcoin/rust-miniscript/pull/49.

[sanket1729]

As a part of the miniscript review for c++, I have been doing a line-by-line comparison with rust-miniscript. I still have opcode and stack size limits to double. Hopefully, will report back today/tomorrow

[sanket1729]

Note that the logic of calculating stack size is slightly different in both implementations, but they should result in the same output.