From d47cad554f55a9bb18f151835017e6114ac05400 Mon Sep 17 00:00:00 2001 From: sanket1729 Date: Mon, 30 Nov 2020 08:47:56 -0600 Subject: [PATCH 1/4] Psbt finalizer should work on insane scripts If one is at the finalizing state of psbt workflow, we have already finalized what script we want to deal with and thus we should accept insane scripts(malleable/repeated pk) etc --- src/psbt/finalizer.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/psbt/finalizer.rs b/src/psbt/finalizer.rs index 13d96371ce..622ac66157 100644 --- a/src/psbt/finalizer.rs +++ b/src/psbt/finalizer.rs @@ -63,6 +63,9 @@ fn get_amt(psbt: &Psbt, index: usize) -> Result { // 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, InputError> { // Figure out Scriptpubkey let script_pubkey = get_scriptpubkey(psbt, index)?; @@ -120,7 +123,7 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result, In p2wsh_expected: script_pubkey.clone(), }); } - let ms = Miniscript::::parse(witness_script)?; + let ms = Miniscript::::parse_insane(witness_script)?; Ok(Descriptor::Wsh(ms)) } else { Err(InputError::MissingWitnessScript) @@ -144,7 +147,9 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result, In p2wsh_expected: redeem_script.clone(), }); } - let ms = Miniscript::::parse(witness_script)?; + let ms = Miniscript::::parse_insane( + witness_script, + )?; Ok(Descriptor::ShWsh(ms)) } else { Err(InputError::MissingWitnessScript) @@ -170,7 +175,8 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result, In return Err(InputError::NonEmptyWitnessScript); } if let Some(ref redeem_script) = inp.redeem_script { - let ms = Miniscript::::parse(redeem_script)?; + let ms = + Miniscript::::parse_insane(redeem_script)?; Ok(Descriptor::Sh(ms)) } else { Err(InputError::MissingWitnessScript) @@ -186,7 +192,7 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result, In if inp.redeem_script.is_some() { return Err(InputError::NonEmptyRedeemScript); } - let ms = Miniscript::::parse(script_pubkey)?; + let ms = Miniscript::::parse_insane(script_pubkey)?; Ok(Descriptor::Bare(ms)) } } From 116b912aea0b0c1dafb0039791778a845cb68542 Mon Sep 17 00:00:00 2001 From: sanket1729 Date: Mon, 30 Nov 2020 08:50:14 -0600 Subject: [PATCH 2/4] Another bug for after/older --- src/miniscript/satisfy.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/miniscript/satisfy.rs b/src/miniscript/satisfy.rs index bb018bfdd9..aea55c6c1d 100644 --- a/src/miniscript/satisfy.rs +++ b/src/miniscript/satisfy.rs @@ -127,21 +127,6 @@ pub struct Older(pub u32); impl> Satisfier 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> Satisfier for After { - fn check_after(&self, n: u32) -> bool { if self.0 & SEQUENCE_LOCKTIME_DISABLE_FLAG != 0 { return true; } @@ -161,6 +146,21 @@ impl> Satisfier> Satisfier 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> Satisfier for HashMap { From d9fa2cbce048472601e86711542413d0c18f56ca Mon Sep 17 00:00:00 2001 From: sanket1729 Date: Mon, 30 Nov 2020 08:50:33 -0600 Subject: [PATCH 3/4] Remove inocrrect panic from satisfy It is true that all thresh fragments must be `d`, but in certain edge cases it is possible that we don't have the information to dissatify it. For example, the preimage to the PkH fragment. Although users should always use descriptors and DescriptorPublicKey type when dealing with this and this would not be an issue in practise we should still remove the panic --- src/miniscript/satisfy.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/miniscript/satisfy.rs b/src/miniscript/satisfy.rs index aea55c6c1d..1e9790d794 100644 --- a/src/miniscript/satisfy.rs +++ b/src/miniscript/satisfy.rs @@ -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 } From bb85c709f490e91fdf579eff60720c62dedfbf96 Mon Sep 17 00:00:00 2001 From: sanket1729 Date: Mon, 30 Nov 2020 09:00:36 -0600 Subject: [PATCH 4/4] Bump cargo patch version --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index c157976b6e..b5f0a12034 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "miniscript" -version = "4.0.0" +version = "4.0.1" authors = ["Andrew Poelstra , Sanket Kanjalkar "] repository = "https://github.com/apoelstra/miniscript" description = "Miniscript: a subset of Bitcoin Script designed for analysis"