Skip to content

Commit ff8f077

Browse files
apoelstraapoelstra
committed
miniscript: use new Threshold type for thresh
1 parent 95f8dac commit ff8f077

File tree

12 files changed

+87
-134
lines changed

12 files changed

+87
-134
lines changed

‎src/descriptor/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1498,7 +1498,7 @@ mod tests {
14981498
#[test]
14991499
fn empty_thresh() {
15001500
let descriptor = Descriptor::<bitcoin::PublicKey>::from_str("thresh");
1501-
assert_eq!(descriptor.unwrap_err().to_string(), "unexpected «no arguments given»")
1501+
assert_eq!(descriptor.unwrap_err().to_string(), "expected threshold, found terminal");
15021502
}
15031503

15041504
#[test]

‎src/interpreter/mod.rs

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -800,16 +800,20 @@ where
800800
None => return Some(Err(Error::UnexpectedStackEnd)),
801801
}
802802
}
803-
Terminal::Thresh(ref _k, ref subs) if node_state.n_evaluated == 0 => {
803+
Terminal::Thresh(ref thresh) if node_state.n_evaluated == 0 => {
804804
self.push_evaluation_state(node_state.node, 1, 0);
805-
self.push_evaluation_state(&subs[0], 0, 0);
805+
self.push_evaluation_state(&thresh.data()[0], 0, 0);
806806
}
807-
Terminal::Thresh(k, ref subs) if node_state.n_evaluated == subs.len() => {
807+
Terminal::Thresh(ref thresh) if node_state.n_evaluated == thresh.n() => {
808808
match self.stack.pop() {
809-
Some(stack::Element::Dissatisfied) if node_state.n_satisfied == k => {
809+
Some(stack::Element::Dissatisfied)
810+
if node_state.n_satisfied == thresh.k() =>
811+
{
810812
self.stack.push(stack::Element::Satisfied)
811813
}
812-
Some(stack::Element::Satisfied) if node_state.n_satisfied == k - 1 => {
814+
Some(stack::Element::Satisfied)
815+
if node_state.n_satisfied == thresh.k() - 1 =>
816+
{
813817
self.stack.push(stack::Element::Satisfied)
814818
}
815819
Some(stack::Element::Satisfied) | Some(stack::Element::Dissatisfied) => {
@@ -821,23 +825,31 @@ where
821825
None => return Some(Err(Error::UnexpectedStackEnd)),
822826
}
823827
}
824-
Terminal::Thresh(ref _k, ref subs) if node_state.n_evaluated != 0 => {
828+
Terminal::Thresh(ref thresh) if node_state.n_evaluated != 0 => {
825829
match self.stack.pop() {
826830
Some(stack::Element::Dissatisfied) => {
827831
self.push_evaluation_state(
828832
node_state.node,
829833
node_state.n_evaluated + 1,
830834
node_state.n_satisfied,
831835
);
832-
self.push_evaluation_state(&subs[node_state.n_evaluated], 0, 0);
836+
self.push_evaluation_state(
837+
&thresh.data()[node_state.n_evaluated],
838+
0,
839+
0,
840+
);
833841
}
834842
Some(stack::Element::Satisfied) => {
835843
self.push_evaluation_state(
836844
node_state.node,
837845
node_state.n_evaluated + 1,
838846
node_state.n_satisfied + 1,
839847
);
840-
self.push_evaluation_state(&subs[node_state.n_evaluated], 0, 0);
848+
self.push_evaluation_state(
849+
&thresh.data()[node_state.n_evaluated],
850+
0,
851+
0,
852+
);
841853
}
842854
Some(stack::Element::Push(_v)) => {
843855
return Some(Err(Error::UnexpectedStackElementPush))

‎src/iter/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> TreeLike for &'a Miniscript<Pk,
3737
| OrC(ref left, ref right)
3838
| OrI(ref left, ref right) => Tree::Binary(left, right),
3939
AndOr(ref a, ref b, ref c) => Tree::Nary(Arc::from([a.as_ref(), b, c])),
40-
Thresh(_, ref subs) => Tree::Nary(subs.iter().map(Arc::as_ref).collect()),
40+
Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::as_ref).collect()),
4141
}
4242
}
4343
}
@@ -64,7 +64,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> TreeLike for Arc<Miniscript<Pk, Ctx>
6464
AndOr(ref a, ref b, ref c) => {
6565
Tree::Nary(Arc::from([Arc::clone(a), Arc::clone(b), Arc::clone(c)]))
6666
}
67-
Thresh(_, ref subs) => Tree::Nary(subs.iter().map(Arc::clone).collect()),
67+
Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::clone).collect()),
6868
}
6969
}
7070
}

‎src/miniscript/astelem.rs

Lines changed: 18 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ use crate::miniscript::{types, ScriptContext};
1919
use crate::prelude::*;
2020
use crate::util::MsKeyBuilder;
2121
use crate::{
22-
errstr, expression, AbsLockTime, Error, FromStrKey, Miniscript, MiniscriptKey, RelLockTime,
23-
Terminal, ToPublicKey,
22+
expression, AbsLockTime, Error, FromStrKey, Miniscript, MiniscriptKey, RelLockTime, Terminal,
23+
ToPublicKey,
2424
};
2525

2626
impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
@@ -81,7 +81,13 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
8181
{
8282
fmt_2(f, "or_i(", l, r, is_debug)
8383
}
84-
Terminal::Thresh(k, ref subs) => fmt_n(f, "thresh(", k, subs, is_debug),
84+
Terminal::Thresh(ref thresh) => {
85+
if is_debug {
86+
fmt::Debug::fmt(&thresh.debug("thresh", true), f)
87+
} else {
88+
fmt::Display::fmt(&thresh.display("thresh", true), f)
89+
}
90+
}
8591
Terminal::Multi(ref thresh) => {
8692
if is_debug {
8793
fmt::Debug::fmt(&thresh.debug("multi", true), f)
@@ -168,21 +174,6 @@ fn fmt_2<D: fmt::Debug + fmt::Display>(
168174
conditional_fmt(f, b, is_debug)?;
169175
f.write_str(")")
170176
}
171-
fn fmt_n<D: fmt::Debug + fmt::Display>(
172-
f: &mut fmt::Formatter,
173-
name: &str,
174-
first: usize,
175-
list: &[D],
176-
is_debug: bool,
177-
) -> fmt::Result {
178-
f.write_str(name)?;
179-
write!(f, "{}", first)?;
180-
for el in list {
181-
f.write_str(",")?;
182-
conditional_fmt(f, el, is_debug)?;
183-
}
184-
f.write_str(")")
185-
}
186177
fn conditional_fmt<D: fmt::Debug + fmt::Display>(
187178
f: &mut fmt::Formatter,
188179
data: &D,
@@ -307,25 +298,11 @@ impl<Pk: FromStrKey, Ctx: ScriptContext> crate::expression::FromTree for Termina
307298
("or_d", 2) => expression::binary(top, Terminal::OrD),
308299
("or_c", 2) => expression::binary(top, Terminal::OrC),
309300
("or_i", 2) => expression::binary(top, Terminal::OrI),
310-
("thresh", n) => {
311-
if n == 0 {
312-
return Err(errstr("no arguments given"));
313-
}
314-
let k = expression::terminal(&top.args[0], expression::parse_num)? as usize;
315-
if k > n - 1 {
316-
return Err(errstr("higher threshold than there are subexpressions"));
317-
}
318-
if n == 1 {
319-
return Err(errstr("empty thresholds not allowed in descriptors"));
320-
}
321-
322-
let subs: Result<Vec<Arc<Miniscript<Pk, Ctx>>>, _> = top.args[1..]
323-
.iter()
324-
.map(expression::FromTree::from_tree)
325-
.collect();
326-
327-
Ok(Terminal::Thresh(k, subs?))
328-
}
301+
("thresh", _) => top
302+
.to_null_threshold()
303+
.map_err(Error::ParseThreshold)?
304+
.translate_by_index(|i| Miniscript::from_tree(&top.args[1 + i]).map(Arc::new))
305+
.map(Terminal::Thresh),
329306
("multi", _) => top
330307
.to_null_threshold()
331308
.map_err(Error::ParseThreshold)?
@@ -475,13 +452,13 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
475452
.push_opcode(opcodes::all::OP_ELSE)
476453
.push_astelem(right)
477454
.push_opcode(opcodes::all::OP_ENDIF),
478-
Terminal::Thresh(k, ref subs) => {
479-
builder = builder.push_astelem(&subs[0]);
480-
for sub in &subs[1..] {
455+
Terminal::Thresh(ref thresh) => {
456+
builder = builder.push_astelem(&thresh.data()[0]);
457+
for sub in &thresh.data()[1..] {
481458
builder = builder.push_astelem(sub).push_opcode(opcodes::all::OP_ADD);
482459
}
483460
builder
484-
.push_int(k as i64)
461+
.push_int(thresh.k() as i64)
485462
.push_opcode(opcodes::all::OP_EQUAL)
486463
}
487464
Terminal::Multi(ref thresh) => {

‎src/miniscript/decode.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ pub enum Terminal<Pk: MiniscriptKey, Ctx: ScriptContext> {
181181
OrI(Arc<Miniscript<Pk, Ctx>>, Arc<Miniscript<Pk, Ctx>>),
182182
// Thresholds
183183
/// `[E] ([W] ADD)* k EQUAL`
184-
Thresh(usize, Vec<Arc<Miniscript<Pk, Ctx>>>),
184+
Thresh(Threshold<Arc<Miniscript<Pk, Ctx>>, 0>),
185185
/// `k (<key>)* n CHECKMULTISIG`
186186
Multi(Threshold<Pk, MAX_PUBKEYS_PER_MULTISIG>),
187187
/// `<key> CHECKSIG (<key> CHECKSIGADD)*(n-1) k NUMEQUAL`
@@ -549,7 +549,7 @@ pub fn parse<Ctx: ScriptContext>(
549549
for _ in 0..n {
550550
subs.push(Arc::new(term.pop().unwrap()));
551551
}
552-
term.reduce0(Terminal::Thresh(k, subs))?;
552+
term.reduce0(Terminal::Thresh(Threshold::new(k, subs).map_err(Error::Threshold)?))?;
553553
}
554554
Some(NonTerm::EndIf) => {
555555
match_token!(

‎src/miniscript/iter.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
5050

5151
Terminal::AndOr(ref node1, ref node2, ref node3) => vec![node1, node2, node3],
5252

53-
Terminal::Thresh(_, ref node_vec) => node_vec.iter().map(Arc::deref).collect(),
53+
Terminal::Thresh(ref thresh) => thresh.iter().map(Arc::deref).collect(),
5454

5555
_ => vec![],
5656
}
@@ -82,7 +82,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
8282
| (1, Terminal::AndOr(_, node, _))
8383
| (2, Terminal::AndOr(_, _, node)) => Some(node),
8484

85-
(n, Terminal::Thresh(_, node_vec)) => node_vec.get(n).map(|x| &**x),
85+
(n, Terminal::Thresh(thresh)) => thresh.data().get(n).map(|x| &**x),
8686

8787
_ => None,
8888
}

‎src/miniscript/mod.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,10 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
148148
Terminal::After(n) => script_num_size(n.to_consensus_u32() as usize) + 1,
149149
Terminal::Older(n) => script_num_size(n.to_consensus_u32() as usize) + 1,
150150
Terminal::Verify(ref sub) => usize::from(!sub.ext.has_free_verify),
151-
Terminal::Thresh(k, ref subs) => {
152-
assert!(!subs.is_empty(), "threshold must be nonempty");
153-
script_num_size(k) // k
151+
Terminal::Thresh(ref thresh) => {
152+
script_num_size(thresh.k()) // k
154153
+ 1 // EQUAL
155-
+ subs.len() // ADD
154+
+ thresh.n() // ADD
156155
- 1 // no ADD on first element
157156
}
158157
Terminal::Multi(ref thresh) => {
@@ -492,9 +491,9 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
492491
Terminal::OrD(..) => Terminal::OrD(child_n(0), child_n(1)),
493492
Terminal::OrC(..) => Terminal::OrC(child_n(0), child_n(1)),
494493
Terminal::OrI(..) => Terminal::OrI(child_n(0), child_n(1)),
495-
Terminal::Thresh(k, ref subs) => {
496-
Terminal::Thresh(k, (0..subs.len()).map(child_n).collect())
497-
}
494+
Terminal::Thresh(ref thresh) => Terminal::Thresh(
495+
thresh.map_from_post_order_iter(&data.child_indices, &translated),
496+
),
498497
Terminal::Multi(ref thresh) => Terminal::Multi(thresh.translate_ref(|k| t.pk(k))?),
499498
Terminal::MultiA(ref thresh) => {
500499
Terminal::MultiA(thresh.translate_ref(|k| t.pk(k))?)

‎src/miniscript/satisfy.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1495,8 +1495,8 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
14951495
},
14961496
)
14971497
}
1498-
Terminal::Thresh(k, ref subs) => {
1499-
thresh_fn(k, subs, stfr, root_has_sig, leaf_hash, min_fn)
1498+
Terminal::Thresh(ref thresh) => {
1499+
thresh_fn(thresh.k(), thresh.data(), stfr, root_has_sig, leaf_hash, min_fn)
15001500
}
15011501
Terminal::Multi(ref thresh) => {
15021502
// Collect all available signatures
@@ -1755,8 +1755,8 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
17551755
// Dissatisfactions don't need to non-malleable. Use minimum_mall always
17561756
Satisfaction::minimum_mall(dissat_1, dissat_2)
17571757
}
1758-
Terminal::Thresh(_, ref subs) => Satisfaction {
1759-
stack: subs.iter().fold(Witness::empty(), |acc, sub| {
1758+
Terminal::Thresh(ref thresh) => Satisfaction {
1759+
stack: thresh.iter().fold(Witness::empty(), |acc, sub| {
17601760
let nsat = Self::dissatisfy_helper(
17611761
&sub.node,
17621762
stfr,

‎src/miniscript/types/extra_props.rs

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
use core::cmp;
77
use core::iter::once;
88

9-
use super::{Error, ErrorKind, ScriptContext};
9+
use super::{Error, ScriptContext};
1010
use crate::miniscript::context::SigType;
1111
use crate::prelude::*;
1212
use crate::{script_num_size, AbsLockTime, MiniscriptKey, RelLockTime, Terminal};
@@ -953,21 +953,8 @@ impl ExtData {
953953
let ctype = c.ext;
954954
Self::and_or(atype, btype, ctype)
955955
}
956-
Terminal::Thresh(k, ref subs) => {
957-
if k == 0 {
958-
return Err(Error {
959-
fragment_string: fragment.to_string(),
960-
error: ErrorKind::ZeroThreshold,
961-
});
962-
}
963-
if k > subs.len() {
964-
return Err(Error {
965-
fragment_string: fragment.to_string(),
966-
error: ErrorKind::OverThreshold(k, subs.len()),
967-
});
968-
}
969-
970-
Self::threshold(k, subs.len(), |n| subs[n].ext)
956+
Terminal::Thresh(ref thresh) => {
957+
Self::threshold(thresh.k(), thresh.n(), |n| thresh.data()[n].ext)
971958
}
972959
};
973960
ret.sanity_checks();

‎src/miniscript/types/mod.rs

Lines changed: 2 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,6 @@ use crate::{MiniscriptKey, Terminal};
2525
pub enum ErrorKind {
2626
/// Passed a `z` argument to a `d` wrapper when `z` was expected
2727
NonZeroDupIf,
28-
/// Multisignature or threshold policy had a `k` value of 0
29-
ZeroThreshold,
30-
/// Multisignature or threshold policy has a `k` value in
31-
/// excess of the number of subfragments
32-
OverThreshold(usize, usize),
3328
/// Many fragments (all disjunctions except `or_i` as well as
3429
/// `andor` require their left child be dissatisfiable.
3530
LeftNotDissatisfiable,
@@ -80,17 +75,6 @@ impl fmt::Display for Error {
8075
"fragment «{}» represents needs to be `z`, needs to consume zero elements from the stack",
8176
self.fragment_string,
8277
),
83-
ErrorKind::ZeroThreshold => write!(
84-
f,
85-
"fragment «{}» has a threshold value of 0",
86-
self.fragment_string,
87-
),
88-
ErrorKind::OverThreshold(k, n) => write!(
89-
f,
90-
"fragment «{}» is a {}-of-{} threshold, which does not
91-
make sense",
92-
self.fragment_string, k, n,
93-
),
9478
ErrorKind::LeftNotDissatisfiable => write!(
9579
f,
9680
"fragment «{}» requires its left child be dissatisfiable",
@@ -487,22 +471,8 @@ impl Type {
487471
let ctype = c.ty;
488472
wrap_err(Self::and_or(atype, btype, ctype))
489473
}
490-
Terminal::Thresh(k, ref subs) => {
491-
if k == 0 {
492-
return Err(Error {
493-
fragment_string: fragment.to_string(),
494-
error: ErrorKind::ZeroThreshold,
495-
});
496-
}
497-
if k > subs.len() {
498-
return Err(Error {
499-
fragment_string: fragment.to_string(),
500-
error: ErrorKind::OverThreshold(k, subs.len()),
501-
});
502-
}
503-
504-
let res = Self::threshold(k, subs.iter().map(|ms| &ms.ty));
505-
474+
Terminal::Thresh(ref thresh) => {
475+
let res = Self::threshold(thresh.k(), thresh.iter().map(|ms| &ms.ty));
506476
res.map_err(|kind| Error { fragment_string: fragment.to_string(), error: kind })
507477
}
508478
};

0 commit comments

Comments
 (0)