diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index c00db0b65..b3509cd4a 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -1498,7 +1498,7 @@ mod tests { #[test] fn empty_thresh() { let descriptor = Descriptor::<bitcoin::PublicKey>::from_str("thresh"); - assert_eq!(descriptor.unwrap_err().to_string(), "unexpected «no arguments given»") + assert_eq!(descriptor.unwrap_err().to_string(), "expected threshold, found terminal"); } #[test] diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index d9bc3ebab..2ed64abf6 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -800,16 +800,20 @@ where None => return Some(Err(Error::UnexpectedStackEnd)), } } - Terminal::Thresh(ref _k, ref subs) if node_state.n_evaluated == 0 => { + Terminal::Thresh(ref thresh) if node_state.n_evaluated == 0 => { self.push_evaluation_state(node_state.node, 1, 0); - self.push_evaluation_state(&subs[0], 0, 0); + self.push_evaluation_state(&thresh.data()[0], 0, 0); } - Terminal::Thresh(k, ref subs) if node_state.n_evaluated == subs.len() => { + Terminal::Thresh(ref thresh) if node_state.n_evaluated == thresh.n() => { match self.stack.pop() { - Some(stack::Element::Dissatisfied) if node_state.n_satisfied == k => { + Some(stack::Element::Dissatisfied) + if node_state.n_satisfied == thresh.k() => + { self.stack.push(stack::Element::Satisfied) } - Some(stack::Element::Satisfied) if node_state.n_satisfied == k - 1 => { + Some(stack::Element::Satisfied) + if node_state.n_satisfied == thresh.k() - 1 => + { self.stack.push(stack::Element::Satisfied) } Some(stack::Element::Satisfied) | Some(stack::Element::Dissatisfied) => { @@ -821,7 +825,7 @@ where None => return Some(Err(Error::UnexpectedStackEnd)), } } - Terminal::Thresh(ref _k, ref subs) if node_state.n_evaluated != 0 => { + Terminal::Thresh(ref thresh) if node_state.n_evaluated != 0 => { match self.stack.pop() { Some(stack::Element::Dissatisfied) => { self.push_evaluation_state( @@ -829,7 +833,11 @@ where node_state.n_evaluated + 1, node_state.n_satisfied, ); - self.push_evaluation_state(&subs[node_state.n_evaluated], 0, 0); + self.push_evaluation_state( + &thresh.data()[node_state.n_evaluated], + 0, + 0, + ); } Some(stack::Element::Satisfied) => { self.push_evaluation_state( @@ -837,7 +845,11 @@ where node_state.n_evaluated + 1, node_state.n_satisfied + 1, ); - self.push_evaluation_state(&subs[node_state.n_evaluated], 0, 0); + self.push_evaluation_state( + &thresh.data()[node_state.n_evaluated], + 0, + 0, + ); } Some(stack::Element::Push(_v)) => { return Some(Err(Error::UnexpectedStackElementPush)) diff --git a/src/iter/mod.rs b/src/iter/mod.rs index 02e59b6e2..e3fb12a9d 100644 --- a/src/iter/mod.rs +++ b/src/iter/mod.rs @@ -37,7 +37,7 @@ impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> TreeLike for &'a Miniscript<Pk, | OrC(ref left, ref right) | OrI(ref left, ref right) => Tree::Binary(left, right), AndOr(ref a, ref b, ref c) => Tree::Nary(Arc::from([a.as_ref(), b, c])), - Thresh(_, ref subs) => Tree::Nary(subs.iter().map(Arc::as_ref).collect()), + Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::as_ref).collect()), } } } @@ -64,7 +64,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> TreeLike for Arc<Miniscript<Pk, Ctx> AndOr(ref a, ref b, ref c) => { Tree::Nary(Arc::from([Arc::clone(a), Arc::clone(b), Arc::clone(c)])) } - Thresh(_, ref subs) => Tree::Nary(subs.iter().map(Arc::clone).collect()), + Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::clone).collect()), } } } diff --git a/src/miniscript/astelem.rs b/src/miniscript/astelem.rs index ba2ec6fbd..58c91befb 100644 --- a/src/miniscript/astelem.rs +++ b/src/miniscript/astelem.rs @@ -19,8 +19,8 @@ use crate::miniscript::{types, ScriptContext}; use crate::prelude::*; use crate::util::MsKeyBuilder; use crate::{ - errstr, expression, AbsLockTime, Error, FromStrKey, Miniscript, MiniscriptKey, RelLockTime, - Terminal, ToPublicKey, + expression, AbsLockTime, Error, FromStrKey, Miniscript, MiniscriptKey, RelLockTime, Terminal, + ToPublicKey, }; impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> { @@ -81,7 +81,13 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> { { fmt_2(f, "or_i(", l, r, is_debug) } - Terminal::Thresh(k, ref subs) => fmt_n(f, "thresh(", k, subs, is_debug), + Terminal::Thresh(ref thresh) => { + if is_debug { + fmt::Debug::fmt(&thresh.debug("thresh", true), f) + } else { + fmt::Display::fmt(&thresh.display("thresh", true), f) + } + } Terminal::Multi(ref thresh) => { if is_debug { fmt::Debug::fmt(&thresh.debug("multi", true), f) @@ -168,21 +174,6 @@ fn fmt_2<D: fmt::Debug + fmt::Display>( conditional_fmt(f, b, is_debug)?; f.write_str(")") } -fn fmt_n<D: fmt::Debug + fmt::Display>( - f: &mut fmt::Formatter, - name: &str, - first: usize, - list: &[D], - is_debug: bool, -) -> fmt::Result { - f.write_str(name)?; - write!(f, "{}", first)?; - for el in list { - f.write_str(",")?; - conditional_fmt(f, el, is_debug)?; - } - f.write_str(")") -} fn conditional_fmt<D: fmt::Debug + fmt::Display>( f: &mut fmt::Formatter, data: &D, @@ -307,25 +298,11 @@ impl<Pk: FromStrKey, Ctx: ScriptContext> crate::expression::FromTree for Termina ("or_d", 2) => expression::binary(top, Terminal::OrD), ("or_c", 2) => expression::binary(top, Terminal::OrC), ("or_i", 2) => expression::binary(top, Terminal::OrI), - ("thresh", n) => { - if n == 0 { - return Err(errstr("no arguments given")); - } - let k = expression::terminal(&top.args[0], expression::parse_num)? as usize; - if k > n - 1 { - return Err(errstr("higher threshold than there are subexpressions")); - } - if n == 1 { - return Err(errstr("empty thresholds not allowed in descriptors")); - } - - let subs: Result<Vec<Arc<Miniscript<Pk, Ctx>>>, _> = top.args[1..] - .iter() - .map(expression::FromTree::from_tree) - .collect(); - - Ok(Terminal::Thresh(k, subs?)) - } + ("thresh", _) => top + .to_null_threshold() + .map_err(Error::ParseThreshold)? + .translate_by_index(|i| Miniscript::from_tree(&top.args[1 + i]).map(Arc::new)) + .map(Terminal::Thresh), ("multi", _) => top .to_null_threshold() .map_err(Error::ParseThreshold)? @@ -475,13 +452,13 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> { .push_opcode(opcodes::all::OP_ELSE) .push_astelem(right) .push_opcode(opcodes::all::OP_ENDIF), - Terminal::Thresh(k, ref subs) => { - builder = builder.push_astelem(&subs[0]); - for sub in &subs[1..] { + Terminal::Thresh(ref thresh) => { + builder = builder.push_astelem(&thresh.data()[0]); + for sub in &thresh.data()[1..] { builder = builder.push_astelem(sub).push_opcode(opcodes::all::OP_ADD); } builder - .push_int(k as i64) + .push_int(thresh.k() as i64) .push_opcode(opcodes::all::OP_EQUAL) } Terminal::Multi(ref thresh) => { diff --git a/src/miniscript/decode.rs b/src/miniscript/decode.rs index 6f6448321..6cbb92931 100644 --- a/src/miniscript/decode.rs +++ b/src/miniscript/decode.rs @@ -181,7 +181,7 @@ pub enum Terminal<Pk: MiniscriptKey, Ctx: ScriptContext> { OrI(Arc<Miniscript<Pk, Ctx>>, Arc<Miniscript<Pk, Ctx>>), // Thresholds /// `[E] ([W] ADD)* k EQUAL` - Thresh(usize, Vec<Arc<Miniscript<Pk, Ctx>>>), + Thresh(Threshold<Arc<Miniscript<Pk, Ctx>>, 0>), /// `k (<key>)* n CHECKMULTISIG` Multi(Threshold<Pk, MAX_PUBKEYS_PER_MULTISIG>), /// `<key> CHECKSIG (<key> CHECKSIGADD)*(n-1) k NUMEQUAL` @@ -549,7 +549,7 @@ pub fn parse<Ctx: ScriptContext>( for _ in 0..n { subs.push(Arc::new(term.pop().unwrap())); } - term.reduce0(Terminal::Thresh(k, subs))?; + term.reduce0(Terminal::Thresh(Threshold::new(k, subs).map_err(Error::Threshold)?))?; } Some(NonTerm::EndIf) => { match_token!( diff --git a/src/miniscript/iter.rs b/src/miniscript/iter.rs index 11cbe067e..a9373abf5 100644 --- a/src/miniscript/iter.rs +++ b/src/miniscript/iter.rs @@ -50,7 +50,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> { Terminal::AndOr(ref node1, ref node2, ref node3) => vec![node1, node2, node3], - Terminal::Thresh(_, ref node_vec) => node_vec.iter().map(Arc::deref).collect(), + Terminal::Thresh(ref thresh) => thresh.iter().map(Arc::deref).collect(), _ => vec![], } @@ -82,7 +82,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> { | (1, Terminal::AndOr(_, node, _)) | (2, Terminal::AndOr(_, _, node)) => Some(node), - (n, Terminal::Thresh(_, node_vec)) => node_vec.get(n).map(|x| &**x), + (n, Terminal::Thresh(thresh)) => thresh.data().get(n).map(|x| &**x), _ => None, } diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index 3fa79be48..725e88586 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -148,11 +148,10 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> { Terminal::After(n) => script_num_size(n.to_consensus_u32() as usize) + 1, Terminal::Older(n) => script_num_size(n.to_consensus_u32() as usize) + 1, Terminal::Verify(ref sub) => usize::from(!sub.ext.has_free_verify), - Terminal::Thresh(k, ref subs) => { - assert!(!subs.is_empty(), "threshold must be nonempty"); - script_num_size(k) // k + Terminal::Thresh(ref thresh) => { + script_num_size(thresh.k()) // k + 1 // EQUAL - + subs.len() // ADD + + thresh.n() // ADD - 1 // no ADD on first element } Terminal::Multi(ref thresh) => { @@ -492,9 +491,9 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> { Terminal::OrD(..) => Terminal::OrD(child_n(0), child_n(1)), Terminal::OrC(..) => Terminal::OrC(child_n(0), child_n(1)), Terminal::OrI(..) => Terminal::OrI(child_n(0), child_n(1)), - Terminal::Thresh(k, ref subs) => { - Terminal::Thresh(k, (0..subs.len()).map(child_n).collect()) - } + Terminal::Thresh(ref thresh) => Terminal::Thresh( + thresh.map_from_post_order_iter(&data.child_indices, &translated), + ), Terminal::Multi(ref thresh) => Terminal::Multi(thresh.translate_ref(|k| t.pk(k))?), Terminal::MultiA(ref thresh) => { Terminal::MultiA(thresh.translate_ref(|k| t.pk(k))?) diff --git a/src/miniscript/satisfy.rs b/src/miniscript/satisfy.rs index ea320a9f6..fcd5cf3aa 100644 --- a/src/miniscript/satisfy.rs +++ b/src/miniscript/satisfy.rs @@ -6,7 +6,7 @@ //! scriptpubkeys. //! -use core::{cmp, fmt, i64, mem}; +use core::{cmp, fmt, mem}; use bitcoin::hashes::hash160; use bitcoin::key::XOnlyPublicKey; @@ -19,7 +19,8 @@ use crate::plan::AssetProvider; use crate::prelude::*; use crate::util::witness_size; use crate::{ - AbsLockTime, Miniscript, MiniscriptKey, RelLockTime, ScriptContext, Terminal, ToPublicKey, + AbsLockTime, Miniscript, MiniscriptKey, RelLockTime, ScriptContext, Terminal, Threshold, + ToPublicKey, }; /// Type alias for 32 byte Preimage. @@ -930,8 +931,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> { // produce a non-malleable satisafaction for thesh frag fn thresh<Ctx, Sat, F>( - k: usize, - subs: &[Arc<Miniscript<Pk, Ctx>>], + thresh: &Threshold<Arc<Miniscript<Pk, Ctx>>, 0>, stfr: &Sat, root_has_sig: bool, leaf_hash: &TapLeafHash, @@ -945,7 +945,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> { Satisfaction<Placeholder<Pk>>, ) -> Satisfaction<Placeholder<Pk>>, { - let mut sats = subs + let mut sats = thresh .iter() .map(|s| { Self::satisfy_helper( @@ -959,7 +959,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> { }) .collect::<Vec<_>>(); // Start with the to-return stack set to all dissatisfactions - let mut ret_stack = subs + let mut ret_stack = thresh .iter() .map(|s| { Self::dissatisfy_helper( @@ -976,7 +976,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> { // Sort everything by (sat cost - dissat cost), except that // satisfactions without signatures beat satisfactions with // signatures - let mut sat_indices = (0..subs.len()).collect::<Vec<_>>(); + let mut sat_indices = (0..thresh.n()).collect::<Vec<_>>(); sat_indices.sort_by_key(|&i| { let stack_weight = match (&sats[i].stack, &ret_stack[i].stack) { (&Witness::Unavailable, _) | (&Witness::Impossible, _) => i64::MAX, @@ -995,7 +995,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> { (is_impossible, sats[i].has_sig, stack_weight) }); - for i in 0..k { + for i in 0..thresh.k() { mem::swap(&mut ret_stack[sat_indices[i]], &mut sats[sat_indices[i]]); } @@ -1004,8 +1004,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> { // then the threshold branch is impossible to satisfy // For example, the fragment thresh(2, hash, 0, 0, 0) // is has an impossible witness - assert!(k > 0); - if sats[sat_indices[k - 1]].stack == Witness::Impossible { + if sats[sat_indices[thresh.k() - 1]].stack == Witness::Impossible { Satisfaction { stack: Witness::Impossible, // If the witness is impossible, we don't care about the @@ -1024,9 +1023,8 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> { // For example, the fragment thresh(2, hash, hash, 0, 0) // is uniquely satisfyiable because there is no satisfaction // for the 0 fragment - else if k < sat_indices.len() - && !sats[sat_indices[k]].has_sig - && sats[sat_indices[k]].stack != Witness::Impossible + else if !sats[sat_indices[thresh.k()]].has_sig + && sats[sat_indices[thresh.k()]].stack != Witness::Impossible { // All arguments should be `d`, so dissatisfactions have no // signatures; and in this branch we assume too many weak @@ -1062,8 +1060,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> { // produce a possily malleable satisafaction for thesh frag fn thresh_mall<Ctx, Sat, F>( - k: usize, - subs: &[Arc<Miniscript<Pk, Ctx>>], + thresh: &Threshold<Arc<Miniscript<Pk, Ctx>>, 0>, stfr: &Sat, root_has_sig: bool, leaf_hash: &TapLeafHash, @@ -1077,7 +1074,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> { Satisfaction<Placeholder<Pk>>, ) -> Satisfaction<Placeholder<Pk>>, { - let mut sats = subs + let mut sats = thresh .iter() .map(|s| { Self::satisfy_helper( @@ -1091,7 +1088,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> { }) .collect::<Vec<_>>(); // Start with the to-return stack set to all dissatisfactions - let mut ret_stack = subs + let mut ret_stack = thresh .iter() .map(|s| { Self::dissatisfy_helper( @@ -1108,7 +1105,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> { // Sort everything by (sat cost - dissat cost), except that // satisfactions without signatures beat satisfactions with // signatures - let mut sat_indices = (0..subs.len()).collect::<Vec<_>>(); + let mut sat_indices = (0..thresh.n()).collect::<Vec<_>>(); sat_indices.sort_by_key(|&i| { // For malleable satifactions, directly choose smallest weights match (&sats[i].stack, &ret_stack[i].stack) { @@ -1122,7 +1119,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> { }); // swap the satisfactions - for i in 0..k { + for i in 0..thresh.k() { mem::swap(&mut ret_stack[sat_indices[i]], &mut sats[sat_indices[i]]); } @@ -1236,8 +1233,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> { Satisfaction<Placeholder<Pk>>, ) -> Satisfaction<Placeholder<Pk>>, G: FnMut( - usize, - &[Arc<Miniscript<Pk, Ctx>>], + &Threshold<Arc<Miniscript<Pk, Ctx>>, 0>, &Sat, bool, &TapLeafHash, @@ -1495,8 +1491,8 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> { }, ) } - Terminal::Thresh(k, ref subs) => { - thresh_fn(k, subs, stfr, root_has_sig, leaf_hash, min_fn) + Terminal::Thresh(ref thresh) => { + thresh_fn(thresh, stfr, root_has_sig, leaf_hash, min_fn) } Terminal::Multi(ref thresh) => { // Collect all available signatures @@ -1606,8 +1602,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> { Satisfaction<Placeholder<Pk>>, ) -> Satisfaction<Placeholder<Pk>>, G: FnMut( - usize, - &[Arc<Miniscript<Pk, Ctx>>], + &Threshold<Arc<Miniscript<Pk, Ctx>>, 0>, &Sat, bool, &TapLeafHash, @@ -1755,8 +1750,8 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> { // Dissatisfactions don't need to non-malleable. Use minimum_mall always Satisfaction::minimum_mall(dissat_1, dissat_2) } - Terminal::Thresh(_, ref subs) => Satisfaction { - stack: subs.iter().fold(Witness::empty(), |acc, sub| { + Terminal::Thresh(ref thresh) => Satisfaction { + stack: thresh.iter().fold(Witness::empty(), |acc, sub| { let nsat = Self::dissatisfy_helper( &sub.node, stfr, diff --git a/src/miniscript/types/extra_props.rs b/src/miniscript/types/extra_props.rs index 983dcc570..7b173e134 100644 --- a/src/miniscript/types/extra_props.rs +++ b/src/miniscript/types/extra_props.rs @@ -6,7 +6,7 @@ use core::cmp; use core::iter::once; -use super::{Error, ErrorKind, ScriptContext}; +use super::{Error, ScriptContext}; use crate::miniscript::context::SigType; use crate::prelude::*; use crate::{script_num_size, AbsLockTime, MiniscriptKey, RelLockTime, Terminal}; @@ -953,21 +953,8 @@ impl ExtData { let ctype = c.ext; Self::and_or(atype, btype, ctype) } - Terminal::Thresh(k, ref subs) => { - if k == 0 { - return Err(Error { - fragment_string: fragment.to_string(), - error: ErrorKind::ZeroThreshold, - }); - } - if k > subs.len() { - return Err(Error { - fragment_string: fragment.to_string(), - error: ErrorKind::OverThreshold(k, subs.len()), - }); - } - - Self::threshold(k, subs.len(), |n| subs[n].ext) + Terminal::Thresh(ref thresh) => { + Self::threshold(thresh.k(), thresh.n(), |n| thresh.data()[n].ext) } }; ret.sanity_checks(); diff --git a/src/miniscript/types/mod.rs b/src/miniscript/types/mod.rs index 7c2a9ccf0..84fb85e4f 100644 --- a/src/miniscript/types/mod.rs +++ b/src/miniscript/types/mod.rs @@ -25,11 +25,6 @@ use crate::{MiniscriptKey, Terminal}; pub enum ErrorKind { /// Passed a `z` argument to a `d` wrapper when `z` was expected NonZeroDupIf, - /// Multisignature or threshold policy had a `k` value of 0 - ZeroThreshold, - /// Multisignature or threshold policy has a `k` value in - /// excess of the number of subfragments - OverThreshold(usize, usize), /// Many fragments (all disjunctions except `or_i` as well as /// `andor` require their left child be dissatisfiable. LeftNotDissatisfiable, @@ -80,17 +75,6 @@ impl fmt::Display for Error { "fragment «{}» represents needs to be `z`, needs to consume zero elements from the stack", self.fragment_string, ), - ErrorKind::ZeroThreshold => write!( - f, - "fragment «{}» has a threshold value of 0", - self.fragment_string, - ), - ErrorKind::OverThreshold(k, n) => write!( - f, - "fragment «{}» is a {}-of-{} threshold, which does not - make sense", - self.fragment_string, k, n, - ), ErrorKind::LeftNotDissatisfiable => write!( f, "fragment «{}» requires its left child be dissatisfiable", @@ -487,22 +471,8 @@ impl Type { let ctype = c.ty; wrap_err(Self::and_or(atype, btype, ctype)) } - Terminal::Thresh(k, ref subs) => { - if k == 0 { - return Err(Error { - fragment_string: fragment.to_string(), - error: ErrorKind::ZeroThreshold, - }); - } - if k > subs.len() { - return Err(Error { - fragment_string: fragment.to_string(), - error: ErrorKind::OverThreshold(k, subs.len()), - }); - } - - let res = Self::threshold(k, subs.iter().map(|ms| &ms.ty)); - + Terminal::Thresh(ref thresh) => { + let res = Self::threshold(thresh.k(), thresh.iter().map(|ms| &ms.ty)); res.map_err(|kind| Error { fragment_string: fragment.to_string(), error: kind }) } }; diff --git a/src/policy/compiler.rs b/src/policy/compiler.rs index 5d1dbd39a..8f9b1e964 100644 --- a/src/policy/compiler.rs +++ b/src/policy/compiler.rs @@ -343,10 +343,7 @@ impl CompilerExtData { } } - fn and_or(a: Self, b: Self, c: Self) -> Result<Self, types::ErrorKind> { - if a.dissat_cost.is_none() { - return Err(ErrorKind::LeftNotDissatisfiable); - } + fn and_or(a: Self, b: Self, c: Self) -> Self { let aprob = a.branch_prob.expect("andor, a prob must be set"); let bprob = b.branch_prob.expect("andor, b prob must be set"); let cprob = c.branch_prob.expect("andor, c prob must be set"); @@ -355,51 +352,48 @@ impl CompilerExtData { .dissat_cost .expect("BUG: and_or first arg(a) must be dissatisfiable"); debug_assert_eq!(aprob, bprob); //A and B must have same branch prob. - Ok(CompilerExtData { + CompilerExtData { branch_prob: None, sat_cost: aprob * (a.sat_cost + b.sat_cost) + cprob * (adis + c.sat_cost), dissat_cost: c.dissat_cost.map(|cdis| adis + cdis), - }) + } } - fn threshold<S>(k: usize, n: usize, mut sub_ck: S) -> Result<Self, types::ErrorKind> + fn threshold<S>(k: usize, n: usize, mut sub_ck: S) -> Self where - S: FnMut(usize) -> Result<Self, types::ErrorKind>, + S: FnMut(usize) -> Self, { let k_over_n = k as f64 / n as f64; let mut sat_cost = 0.0; let mut dissat_cost = 0.0; for i in 0..n { - let sub = sub_ck(i)?; + let sub = sub_ck(i); sat_cost += sub.sat_cost; dissat_cost += sub.dissat_cost.unwrap(); } - Ok(CompilerExtData { + CompilerExtData { branch_prob: None, sat_cost: sat_cost * k_over_n + dissat_cost * (1.0 - k_over_n), dissat_cost: Some(dissat_cost), - }) + } } } impl CompilerExtData { /// Compute the type of a fragment, given a function to look up /// the types of its children. - fn type_check_with_child<Pk, Ctx, C>( - fragment: &Terminal<Pk, Ctx>, - child: C, - ) -> Result<Self, types::Error> + fn type_check_with_child<Pk, Ctx, C>(fragment: &Terminal<Pk, Ctx>, child: C) -> Self where C: Fn(usize) -> Self, Pk: MiniscriptKey, Ctx: ScriptContext, { - let get_child = |_sub, n| Ok(child(n)); + let get_child = |_sub, n| child(n); Self::type_check_common(fragment, get_child) } /// Compute the type of a fragment. - fn type_check<Pk, Ctx>(fragment: &Terminal<Pk, Ctx>) -> Result<Self, types::Error> + fn type_check<Pk, Ctx>(fragment: &Terminal<Pk, Ctx>) -> Self where Pk: MiniscriptKey, Ctx: ScriptContext, @@ -411,99 +405,70 @@ impl CompilerExtData { /// Compute the type of a fragment, given a function to look up /// the types of its children, if available and relevant for the /// given fragment - fn type_check_common<'a, Pk, Ctx, C>( - fragment: &'a Terminal<Pk, Ctx>, - get_child: C, - ) -> Result<Self, types::Error> + fn type_check_common<'a, Pk, Ctx, C>(fragment: &'a Terminal<Pk, Ctx>, get_child: C) -> Self where - C: Fn(&'a Terminal<Pk, Ctx>, usize) -> Result<Self, types::Error>, + C: Fn(&'a Terminal<Pk, Ctx>, usize) -> Self, Pk: MiniscriptKey, Ctx: ScriptContext, { match *fragment { - Terminal::True => Ok(Self::TRUE), - Terminal::False => Ok(Self::FALSE), - Terminal::PkK(..) => Ok(Self::pk_k::<Ctx>()), - Terminal::PkH(..) | Terminal::RawPkH(..) => Ok(Self::pk_h::<Ctx>()), - Terminal::Multi(ref thresh) => Ok(Self::multi(thresh.k(), thresh.n())), - Terminal::MultiA(ref thresh) => Ok(Self::multi_a(thresh.k(), thresh.n())), - Terminal::After(_) => Ok(Self::time()), - Terminal::Older(_) => Ok(Self::time()), - Terminal::Sha256(..) => Ok(Self::hash()), - Terminal::Hash256(..) => Ok(Self::hash()), - Terminal::Ripemd160(..) => Ok(Self::hash()), - Terminal::Hash160(..) => Ok(Self::hash()), - Terminal::Alt(ref sub) => Ok(Self::cast_alt(get_child(&sub.node, 0)?)), - Terminal::Swap(ref sub) => Ok(Self::cast_swap(get_child(&sub.node, 0)?)), - Terminal::Check(ref sub) => Ok(Self::cast_check(get_child(&sub.node, 0)?)), - Terminal::DupIf(ref sub) => Ok(Self::cast_dupif(get_child(&sub.node, 0)?)), - Terminal::Verify(ref sub) => Ok(Self::cast_verify(get_child(&sub.node, 0)?)), - Terminal::NonZero(ref sub) => Ok(Self::cast_nonzero(get_child(&sub.node, 0)?)), - Terminal::ZeroNotEqual(ref sub) => { - Ok(Self::cast_zeronotequal(get_child(&sub.node, 0)?)) - } + Terminal::True => Self::TRUE, + Terminal::False => Self::FALSE, + Terminal::PkK(..) => Self::pk_k::<Ctx>(), + Terminal::PkH(..) | Terminal::RawPkH(..) => Self::pk_h::<Ctx>(), + Terminal::Multi(ref thresh) => Self::multi(thresh.k(), thresh.n()), + Terminal::MultiA(ref thresh) => Self::multi_a(thresh.k(), thresh.n()), + Terminal::After(_) => Self::time(), + Terminal::Older(_) => Self::time(), + Terminal::Sha256(..) => Self::hash(), + Terminal::Hash256(..) => Self::hash(), + Terminal::Ripemd160(..) => Self::hash(), + Terminal::Hash160(..) => Self::hash(), + Terminal::Alt(ref sub) => Self::cast_alt(get_child(&sub.node, 0)), + Terminal::Swap(ref sub) => Self::cast_swap(get_child(&sub.node, 0)), + Terminal::Check(ref sub) => Self::cast_check(get_child(&sub.node, 0)), + Terminal::DupIf(ref sub) => Self::cast_dupif(get_child(&sub.node, 0)), + Terminal::Verify(ref sub) => Self::cast_verify(get_child(&sub.node, 0)), + Terminal::NonZero(ref sub) => Self::cast_nonzero(get_child(&sub.node, 0)), + Terminal::ZeroNotEqual(ref sub) => Self::cast_zeronotequal(get_child(&sub.node, 0)), Terminal::AndB(ref l, ref r) => { - let ltype = get_child(&l.node, 0)?; - let rtype = get_child(&r.node, 1)?; - Ok(Self::and_b(ltype, rtype)) + let ltype = get_child(&l.node, 0); + let rtype = get_child(&r.node, 1); + Self::and_b(ltype, rtype) } Terminal::AndV(ref l, ref r) => { - let ltype = get_child(&l.node, 0)?; - let rtype = get_child(&r.node, 1)?; - Ok(Self::and_v(ltype, rtype)) + let ltype = get_child(&l.node, 0); + let rtype = get_child(&r.node, 1); + Self::and_v(ltype, rtype) } Terminal::OrB(ref l, ref r) => { - let ltype = get_child(&l.node, 0)?; - let rtype = get_child(&r.node, 1)?; - Ok(Self::or_b(ltype, rtype)) + let ltype = get_child(&l.node, 0); + let rtype = get_child(&r.node, 1); + Self::or_b(ltype, rtype) } Terminal::OrD(ref l, ref r) => { - let ltype = get_child(&l.node, 0)?; - let rtype = get_child(&r.node, 1)?; - Ok(Self::or_d(ltype, rtype)) + let ltype = get_child(&l.node, 0); + let rtype = get_child(&r.node, 1); + Self::or_d(ltype, rtype) } Terminal::OrC(ref l, ref r) => { - let ltype = get_child(&l.node, 0)?; - let rtype = get_child(&r.node, 1)?; - Ok(Self::or_c(ltype, rtype)) + let ltype = get_child(&l.node, 0); + let rtype = get_child(&r.node, 1); + Self::or_c(ltype, rtype) } Terminal::OrI(ref l, ref r) => { - let ltype = get_child(&l.node, 0)?; - let rtype = get_child(&r.node, 1)?; - Ok(Self::or_i(ltype, rtype)) + let ltype = get_child(&l.node, 0); + let rtype = get_child(&r.node, 1); + Self::or_i(ltype, rtype) } Terminal::AndOr(ref a, ref b, ref c) => { - let atype = get_child(&a.node, 0)?; - let btype = get_child(&b.node, 1)?; - let ctype = get_child(&c.node, 2)?; - Self::and_or(atype, btype, ctype).map_err(|kind| types::Error { - fragment_string: fragment.to_string(), - error: kind, - }) + let atype = get_child(&a.node, 0); + let btype = get_child(&b.node, 1); + let ctype = get_child(&c.node, 2); + Self::and_or(atype, btype, ctype) } - Terminal::Thresh(k, ref subs) => { - if k == 0 { - return Err(types::Error { - fragment_string: fragment.to_string(), - error: types::ErrorKind::ZeroThreshold, - }); - } - if k > subs.len() { - return Err(types::Error { - fragment_string: fragment.to_string(), - error: types::ErrorKind::OverThreshold(k, subs.len()), - }); - } - - let mut last_err_frag = None; - Self::threshold(k, subs.len(), |n| match get_child(&subs[n].node, n) { - Ok(x) => Ok(x), - Err(e) => { - last_err_frag = Some(e.fragment_string); - Err(e.error) - } - }) - .map_err(|kind| types::Error { fragment_string: fragment.to_string(), error: kind }) + Terminal::Thresh(ref thresh) => { + Self::threshold(thresh.k(), thresh.n(), |n| get_child(&thresh.data()[n].node, n)) } } } @@ -537,7 +502,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> AstElemExt<Pk, Ctx> { impl<Pk: MiniscriptKey, Ctx: ScriptContext> AstElemExt<Pk, Ctx> { fn terminal(ast: Terminal<Pk, Ctx>) -> AstElemExt<Pk, Ctx> { AstElemExt { - comp_ext_data: CompilerExtData::type_check(&ast).unwrap(), + comp_ext_data: CompilerExtData::type_check(&ast), ms: Arc::new(Miniscript::from_ast(ast).expect("Terminal creation must always succeed")), } } @@ -556,7 +521,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> AstElemExt<Pk, Ctx> { //type_check without cache. For Compiler extra data, we supply a cache. let ty = types::Type::type_check(&ast)?; let ext = types::ExtData::type_check(&ast)?; - let comp_ext_data = CompilerExtData::type_check_with_child(&ast, lookup_ext)?; + let comp_ext_data = CompilerExtData::type_check_with_child(&ast, lookup_ext); Ok(AstElemExt { ms: Arc::new(Miniscript::from_components_unchecked(ast, ty, ext)), comp_ext_data, @@ -579,7 +544,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> AstElemExt<Pk, Ctx> { //type_check without cache. For Compiler extra data, we supply a cache. let ty = types::Type::type_check(&ast)?; let ext = types::ExtData::type_check(&ast)?; - let comp_ext_data = CompilerExtData::type_check_with_child(&ast, lookup_ext)?; + let comp_ext_data = CompilerExtData::type_check_with_child(&ast, lookup_ext); Ok(AstElemExt { ms: Arc::new(Miniscript::from_components_unchecked(ast, ty, ext)), comp_ext_data, @@ -995,7 +960,6 @@ where let n = thresh.n(); let k_over_n = k as f64 / n as f64; - let mut sub_ast = Vec::with_capacity(n); let mut sub_ext_data = Vec::with_capacity(n); let mut best_es = Vec::with_capacity(n); @@ -1018,21 +982,33 @@ where min_value.1 = diff; } } - sub_ext_data.push(best_es[min_value.0].0); - sub_ast.push(Arc::clone(&best_es[min_value.0].1.ms)); - for (i, _ast) in thresh.iter().enumerate() { - if i != min_value.0 { - sub_ext_data.push(best_ws[i].0); - sub_ast.push(Arc::clone(&best_ws[i].1.ms)); - } - } - let ast = Terminal::Thresh(k, sub_ast); + // Construct the threshold, swapping the index of the best (i.e. most + // advantageous to be a E vs a W) entry into the first slot so that + // it can be an E. + let mut idx = 0; + let ast = Terminal::Thresh(thresh.map_ref(|_| { + let ret = if idx == 0 { + // swap 0 with min_value... + sub_ext_data.push(best_es[min_value.0].0); + Arc::clone(&best_es[min_value.0].1.ms) + } else if idx == min_value.0 { + // swap min_value with 0... + sub_ext_data.push(best_ws[0].0); + Arc::clone(&best_ws[0].1.ms) + } else { + // ...and leave everything else unchanged + sub_ext_data.push(best_ws[idx].0); + Arc::clone(&best_ws[idx].1.ms) + }; + idx += 1; + ret + })); + if let Ok(ms) = Miniscript::from_ast(ast) { let ast_ext = AstElemExt { ms: Arc::new(ms), - comp_ext_data: CompilerExtData::threshold(k, n, |i| Ok(sub_ext_data[i])) - .expect("threshold subs, which we just compiled, typeck"), + comp_ext_data: CompilerExtData::threshold(k, n, |i| sub_ext_data[i]), }; insert_wrap!(ast_ext); } diff --git a/src/policy/mod.rs b/src/policy/mod.rs index db337b3b5..5c2f83448 100644 --- a/src/policy/mod.rs +++ b/src/policy/mod.rs @@ -157,13 +157,9 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Liftable<Pk> for Terminal<Pk, Ctx> { Arc::new(left.node.lift()?), Arc::new(right.node.lift()?), )), - Terminal::Thresh(k, ref subs) => { - let semantic_subs: Result<Vec<Semantic<Pk>>, Error> = - subs.iter().map(|s| s.node.lift()).collect(); - let semantic_subs = semantic_subs?.into_iter().map(Arc::new).collect(); - // unwrap to be removed in a later commit - Semantic::Thresh(Threshold::new(k, semantic_subs).unwrap()) - } + Terminal::Thresh(ref thresh) => thresh + .translate_ref(|sub| sub.lift().map(Arc::new)) + .map(Semantic::Thresh)?, Terminal::Multi(ref thresh) => Semantic::Thresh( thresh .map_ref(|key| Arc::new(Semantic::Key(key.clone())))