Flip argument order of Witness::combine for and_v and and_b

Given an `and` instruction, miniscript produces a witness stack that
is wrongly ordered:

The left branch of an `and` instruction is evaluated first, which means
whatever witness stack is produced by this branch needs to come _after_
the witness stack of the right branch.

Unfortunately, Witness::combine simply concatenates the two vectors,
resulting in the witness stack for the left branch to end up _before_ the
one for the right branch.

This is what we currently have:

Witness in memory:

|----A-----|----B-----|--and(A,B)--|

Witness stack evaluation:

Round 0:
        Stack:          Instructions:

                        check(A) check(B)
        |----B-----|
        |----A-----|

Round 1:
        Stack:          Instructions:

                        |----B-----| check(A) check(B)
        |----A-----|

Round 2:
        Failure due to wrong signature.

With this patch, this changes to what we actually want:

Witness in memory:

|----B-----|----A-----|--and(A,B)--|

Witness stack evaluation:

Round 0:
        Stack:          Instructions:

                        check(A) check(B)
        |----A-----|
        |----B-----|

Round 1:
        Stack:          Instructions:

                        |----A-----| check(A) check(B)
        |----B-----|

Round 2:
        Stack:          Instructions:

                        check(B)
        |----B-----|

Round 3:
        Stack:          Instructions:

                        |----B-----| check(B)

Round 4:
        Script successfully executed!

Co-authored-by: Lucas Soriano del Pino <l.soriano.del.pino@gmail.com>

Author: thomaseizinger

src/descriptor/mod.rs

@@ -555,6 +555,7 @@ mod tests {
     use bitcoin::hashes::{hash160, sha256};
     use bitcoin::{self, secp256k1, PublicKey};
     use miniscript::satisfy::BitcoinSig;
+    use std::collections::HashMap;
     use std::str::FromStr;
     use {Descriptor, DummyKey, Miniscript, Satisfier};
 
@@ -947,4 +948,59 @@ mod tests {
             "unexpected «no arguments given»"
         )
     }
+
+    #[test]
+    fn witness_stack_for_andv_is_arranged_in_correct_order() {
+        // arrange
+        let a = bitcoin::PublicKey::from_str(
+            "02937402303919b3a2ee5edd5009f4236f069bf75667b8e6ecf8e5464e20116a0e",
+        )
+        .unwrap();
+        let sig_a = secp256k1::Signature::from_str("3045022100a7acc3719e9559a59d60d7b2837f9842df30e7edcd754e63227e6168cec72c5d022066c2feba4671c3d99ea75d9976b4da6c86968dbf3bab47b1061e7a1966b1778c").unwrap();
+
+        let b = bitcoin::PublicKey::from_str(
+            "02eb64639a17f7334bb5a1a3aad857d6fec65faef439db3de72f85c88bc2906ad3",
+        )
+        .unwrap();
+        let sig_b = secp256k1::Signature::from_str("3044022075b7b65a7e6cd386132c5883c9db15f9a849a0f32bc680e9986398879a57c276022056d94d12255a4424f51c700ac75122cb354895c9f2f88f0cbb47ba05c9c589ba").unwrap();
+
+        let descriptor = Descriptor::<bitcoin::PublicKey>::from_str(&format!(
+            "wsh(and_v(v:pk({A}),pk({B})))",
+            A = a,
+            B = b
+        ))
+        .unwrap();
+
+        let mut txin = bitcoin::TxIn {
+            previous_output: bitcoin::OutPoint::default(),
+            script_sig: bitcoin::Script::new(),
+            sequence: 0,
+            witness: vec![],
+        };
+        let satisfier = {
+            let mut satisfier = HashMap::with_capacity(2);
+
+            satisfier.insert(a, (sig_a.clone(), ::bitcoin::SigHashType::All));
+            satisfier.insert(b, (sig_b.clone(), ::bitcoin::SigHashType::All));
+
+            satisfier
+        };
+
+        // act
+        descriptor.satisfy(&mut txin, &satisfier).unwrap();
+
+        // assert
+        let witness0 = &txin.witness[0];
+        let witness1 = &txin.witness[1];
+
+        let sig0 = secp256k1::Signature::from_der(&witness0[..witness0.len() - 1]).unwrap();
+        let sig1 = secp256k1::Signature::from_der(&witness1[..witness1.len() - 1]).unwrap();
+
+        // why are we asserting this way?
+        // The witness stack is evaluated from top to bottom. Given an `and` instruction, the left arm of the and is going to evaluate first,
+        // meaning the next witness element (on a three element stack, that is the middle one) needs to be the signature for the left side of the `and`.
+        // The left side of the `and` performs a CHECKSIG against public key `a` so `sig1` needs to be `sig_a` and `sig0` needs to be `sig_b`.
+        assert_eq!(sig1, sig_a);
+        assert_eq!(sig0, sig_b);
+    }
 }

src/miniscript/satisfy.rs

@@ -580,7 +580,7 @@ impl Satisfaction {
                 let l_sat = Self::satisfy(&l.node, stfr);
                 let r_sat = Self::satisfy(&r.node, stfr);
                 Satisfaction {
-                    stack: Witness::combine(l_sat.stack, r_sat.stack),
+                    stack: Witness::combine(r_sat.stack, l_sat.stack),
                     has_sig: l_sat.has_sig || r_sat.has_sig,
                 }
             }
@@ -592,11 +592,11 @@ impl Satisfaction {
 
                 Self::minimum(
                     Satisfaction {
-                        stack: Witness::combine(a_sat.stack, b_sat.stack),
+                        stack: Witness::combine(b_sat.stack, a_sat.stack),
                         has_sig: a_sat.has_sig || b_sat.has_sig,
                     },
                     Satisfaction {
-                        stack: Witness::combine(a_nsat.stack, c_sat.stack),
+                        stack: Witness::combine(c_sat.stack, a_nsat.stack),
                         has_sig: a_nsat.has_sig || c_sat.has_sig,
                     },
                 )