Skip to content

Commit 60ad2c9

Browse files
apoelstraapoelstra
committed
Merge #676: Use threshold in Terminal and Satisfaction
6925ae6 clippy: remove now-unneeded i64 import (Andrew Poelstra) 0fb47b7 satisfaction: use new Threshold type (Andrew Poelstra) ff8f077 miniscript: use new Threshold type for thresh (Andrew Poelstra) 95f8dac compiler: eliminate unused error paths (Andrew Poelstra) Pull request description: This completes the "threshold" conversion and completely drops the various ad-hoc error paths related to threshold values (most of which were just random strings). The next set of PRs will be related to cleaning up errors in general a bit, to strongly-type things and add span information. But I am struggling a bit with how to box the associated error types for the MiniscriptKey types so it may be a while. (Very hard to do this in a std/nostd compatible way and the final result will probably be inconvenient/annoying for users.) ACKs for top commit: tcharding: ACK 6925ae6 sanket1729: ACK 6925ae6 Tree-SHA512: 04344be051cb5b16f691eafdb64818ffd44c9adc15f71ad6c664e9f337dc4ed52be66a540bf5c99eeb4b384a95d93e2518f290b84cf6abf9cb092e789b57a3fa
2 parents 9eb4375 + 6925ae6 commit 60ad2c9

File tree

12 files changed

+163
-251
lines changed

12 files changed

+163
-251
lines changed

Diff for: ‎src/descriptor/mod.rs

+1-1
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]

Diff for: ‎src/interpreter/mod.rs

+20-8
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))

Diff for: ‎src/iter/mod.rs

+2-2
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
}

Diff for: ‎src/miniscript/astelem.rs

+18-41
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) => {

Diff for: ‎src/miniscript/decode.rs

+2-2
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!(

Diff for: ‎src/miniscript/iter.rs

+2-2
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
}

Diff for: ‎src/miniscript/mod.rs

+6-7
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))?)

0 commit comments

Comments
 (0)