Merge pull request bitcoindevkit#200 from sanket1729/satisfy_fixes

Satisfy fixes
nickfarrow · Nov 30, 2020 · 2ff9f11 · 2ff9f11
2 parents 2b940e1 + bb85c70
commit 2ff9f11
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 27 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "miniscript"
-version = "4.0.0"
+version = "4.0.1"
 authors = ["Andrew Poelstra <apoelstra@wpsoftware.net>, Sanket Kanjalkar <sanket1729@gmail.com>"]
 repository = "https://github.com/apoelstra/miniscript"
 description = "Miniscript: a subset of Bitcoin Script designed for analysis"

diff --git a/src/miniscript/satisfy.rs b/src/miniscript/satisfy.rs
@@ -127,21 +127,6 @@ pub struct Older(pub u32);
 
 impl<ToPkCtx: Copy, Pk: MiniscriptKey + ToPublicKey<ToPkCtx>> Satisfier<ToPkCtx, Pk> for Older {
     fn check_older(&self, n: u32) -> bool {
-        // if n > self.0; we will be returning false anyways
-        if n < HEIGHT_TIME_THRESHOLD && self.0 >= HEIGHT_TIME_THRESHOLD {
-            false
-        } else {
-            n <= self.0
-        }
-    }
-}
-
-/// Newtype around `u32` which implements `Satisfier` using `n` as an
-/// absolute locktime
-pub struct After(pub u32);
-
-impl<ToPkCtx: Copy, Pk: MiniscriptKey + ToPublicKey<ToPkCtx>> Satisfier<ToPkCtx, Pk> for After {
-    fn check_after(&self, n: u32) -> bool {
         if self.0 & SEQUENCE_LOCKTIME_DISABLE_FLAG != 0 {
             return true;
         }
@@ -161,6 +146,21 @@ impl<ToPkCtx: Copy, Pk: MiniscriptKey + ToPublicKey<ToPkCtx>> Satisfier<ToPkCtx,
     }
 }
 
+/// Newtype around `u32` which implements `Satisfier` using `n` as an
+/// absolute locktime
+pub struct After(pub u32);
+
+impl<ToPkCtx: Copy, Pk: MiniscriptKey + ToPublicKey<ToPkCtx>> Satisfier<ToPkCtx, Pk> for After {
+    fn check_after(&self, n: u32) -> bool {
+        // if n > self.0; we will be returning false anyways
+        if n < HEIGHT_TIME_THRESHOLD && self.0 >= HEIGHT_TIME_THRESHOLD {
+            false
+        } else {
+            n <= self.0
+        }
+    }
+}
+
 impl<ToPkCtx: Copy, Pk: MiniscriptKey + ToPublicKey<ToPkCtx>> Satisfier<ToPkCtx, Pk>
     for HashMap<Pk, BitcoinSig>
 {
@@ -627,9 +627,9 @@ impl Satisfaction {
         sat_indices.sort_by_key(|&i| {
             let stack_weight = match (&sats[i].stack, &ret_stack[i].stack) {
                 (&Witness::Unavailable, _) | (&Witness::Impossible, _) => i64::MAX,
-                (_, &Witness::Unavailable) | (_, &Witness::Impossible) => {
-                    unreachable!("Threshold fragments must be 'd'")
-                }
+                // This can only be the case when we have PkH without the corresponding
+                // Pubkey.
+                (_, &Witness::Unavailable) | (_, &Witness::Impossible) => i64::MIN,
                 (&Witness::Stack(ref s), &Witness::Stack(ref d)) => {
                     witness_size(s) as i64 - witness_size(d) as i64
                 }
@@ -669,7 +669,9 @@ impl Satisfaction {
         // For example, the fragment thresh(2, hash, hash, 0, 0)
         // is uniquely satisfyiable because there is no satisfaction
         // for the 0 fragment
-        else if !sats[sat_indices[k]].has_sig && sats[sat_indices[k]].stack != Witness::Impossible
+        else if k < sat_indices.len()
+            && !sats[sat_indices[k]].has_sig
+            && sats[sat_indices[k]].stack != Witness::Impossible
         {
             // All arguments should be `d`, so dissatisfactions have no
             // signatures; and in this branch we assume too many weak
@@ -744,9 +746,8 @@ impl Satisfaction {
         sat_indices.sort_by_key(|&i| {
             let stack_weight = match (&sats[i].stack, &ret_stack[i].stack) {
                 (&Witness::Unavailable, _) | (&Witness::Impossible, _) => i64::MAX,
-                (_, &Witness::Unavailable) | (_, &Witness::Impossible) => {
-                    unreachable!("Threshold fragments must be 'd'")
-                }
+                // This is only possible when one of the branches has PkH
+                (_, &Witness::Unavailable) | (_, &Witness::Impossible) => i64::MIN,
                 (&Witness::Stack(ref s), &Witness::Stack(ref d)) => {
                     witness_size(s) as i64 - witness_size(d) as i64
                 }

diff --git a/src/psbt/finalizer.rs b/src/psbt/finalizer.rs
@@ -63,6 +63,9 @@ fn get_amt(psbt: &Psbt, index: usize) -> Result<u64, InputError> {
 // Also sanity checks that the witness script and
 // redeem script are consistent with the script pubkey.
 // Does *not* check signatures
+// We parse the insane version while satisfying because
+// we want to move the script is probably already created
+// and we want to satisfy it in any way possible.
 fn get_descriptor(psbt: &Psbt, index: usize) -> Result<Descriptor<PublicKey>, InputError> {
     // Figure out Scriptpubkey
     let script_pubkey = get_scriptpubkey(psbt, index)?;
@@ -120,7 +123,7 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result<Descriptor<PublicKey>, In
                     p2wsh_expected: script_pubkey.clone(),
                 });
             }
-            let ms = Miniscript::<bitcoin::PublicKey, Segwitv0>::parse(witness_script)?;
+            let ms = Miniscript::<bitcoin::PublicKey, Segwitv0>::parse_insane(witness_script)?;
             Ok(Descriptor::Wsh(ms))
         } else {
             Err(InputError::MissingWitnessScript)
@@ -144,7 +147,9 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result<Descriptor<PublicKey>, In
                                 p2wsh_expected: redeem_script.clone(),
                             });
                         }
-                        let ms = Miniscript::<bitcoin::PublicKey, Segwitv0>::parse(witness_script)?;
+                        let ms = Miniscript::<bitcoin::PublicKey, Segwitv0>::parse_insane(
+                            witness_script,
+                        )?;
                         Ok(Descriptor::ShWsh(ms))
                     } else {
                         Err(InputError::MissingWitnessScript)
@@ -170,7 +175,8 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result<Descriptor<PublicKey>, In
                         return Err(InputError::NonEmptyWitnessScript);
                     }
                     if let Some(ref redeem_script) = inp.redeem_script {
-                        let ms = Miniscript::<bitcoin::PublicKey, Legacy>::parse(redeem_script)?;
+                        let ms =
+                            Miniscript::<bitcoin::PublicKey, Legacy>::parse_insane(redeem_script)?;
                         Ok(Descriptor::Sh(ms))
                     } else {
                         Err(InputError::MissingWitnessScript)
@@ -186,7 +192,7 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result<Descriptor<PublicKey>, In
         if inp.redeem_script.is_some() {
             return Err(InputError::NonEmptyRedeemScript);
         }
-        let ms = Miniscript::<bitcoin::PublicKey, Bare>::parse(script_pubkey)?;
+        let ms = Miniscript::<bitcoin::PublicKey, Bare>::parse_insane(script_pubkey)?;
         Ok(Descriptor::Bare(ms))
     }
 }