Skip to content

Commit f62322d

Browse files
apoelstraapoelstra
committed
miniscript: use new Threshold type for multi/multi_a
1 parent 1ced03b commit f62322d

File tree

13 files changed

+147
-209
lines changed

13 files changed

+147
-209
lines changed

‎src/descriptor/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1492,7 +1492,7 @@ mod tests {
14921492
#[test]
14931493
fn roundtrip_tests() {
14941494
let descriptor = Descriptor::<bitcoin::PublicKey>::from_str("multi");
1495-
assert_eq!(descriptor.unwrap_err().to_string(), "unexpected «no arguments given»")
1495+
assert_eq!(descriptor.unwrap_err().to_string(), "expected threshold, found terminal",);
14961496
}
14971497

14981498
#[test]

‎src/descriptor/sortedmulti.rs‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
3636
// Check the limits before creating a new SortedMultiVec
3737
// For example, under p2sh context the scriptlen can only be
3838
// upto 520 bytes.
39-
let term: Terminal<Pk, Ctx> = Terminal::Multi(self.inner.k(), self.inner.data().to_owned());
39+
let term: Terminal<Pk, Ctx> = Terminal::Multi(self.inner.clone());
4040
let ms = Miniscript::from_ast(term)?;
4141
// This would check all the consensus rules for p2sh/p2wsh and
4242
// even tapscript in future
@@ -113,11 +113,8 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
113113
/// utility function to sanity a sorted multi vec
114114
pub fn sanity_check(&self) -> Result<(), Error> {
115115
let ms: Miniscript<Pk, Ctx> =
116-
Miniscript::from_ast(Terminal::Multi(self.k(), self.pks().to_owned()))
117-
.expect("Must typecheck");
118-
// '?' for doing From conversion
119-
ms.sanity_check()?;
120-
Ok(())
116+
Miniscript::from_ast(Terminal::Multi(self.inner.clone())).expect("Must typecheck");
117+
ms.sanity_check().map_err(From::from)
121118
}
122119
}
123120

@@ -127,16 +124,16 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
127124
where
128125
Pk: ToPublicKey,
129126
{
130-
let mut pks = self.pks().to_owned();
127+
let mut thresh = self.inner.clone();
131128
// Sort pubkeys lexicographically according to BIP 67
132-
pks.sort_by(|a, b| {
129+
thresh.data_mut().sort_by(|a, b| {
133130
a.to_public_key()
134131
.inner
135132
.serialize()
136133
.partial_cmp(&b.to_public_key().inner.serialize())
137134
.unwrap()
138135
});
139-
Terminal::Multi(self.k(), pks)
136+
Terminal::Multi(thresh)
140137
}
141138

142139
/// Encode as a Bitcoin script

‎src/interpreter/mod.rs‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -845,9 +845,9 @@ where
845845
None => return Some(Err(Error::UnexpectedStackEnd)),
846846
}
847847
}
848-
Terminal::MultiA(k, ref subs) => {
849-
if node_state.n_evaluated == subs.len() {
850-
if node_state.n_satisfied == k {
848+
Terminal::MultiA(ref thresh) => {
849+
if node_state.n_evaluated == thresh.n() {
850+
if node_state.n_satisfied == thresh.k() {
851851
self.stack.push(stack::Element::Satisfied);
852852
} else {
853853
self.stack.push(stack::Element::Dissatisfied);
@@ -856,10 +856,10 @@ where
856856
// evaluate each key with as a pk
857857
// note that evaluate_pk will error on non-empty incorrect sigs
858858
// push 1 on satisfied sigs and push 0 on empty sigs
859-
match self
860-
.stack
861-
.evaluate_pk(&mut self.verify_sig, subs[node_state.n_evaluated])
862-
{
859+
match self.stack.evaluate_pk(
860+
&mut self.verify_sig,
861+
thresh.data()[node_state.n_evaluated],
862+
) {
863863
Some(Ok(x)) => {
864864
self.push_evaluation_state(
865865
node_state.node,
@@ -886,34 +886,34 @@ where
886886
}
887887
}
888888
}
889-
Terminal::Multi(ref k, ref subs) if node_state.n_evaluated == 0 => {
889+
Terminal::Multi(ref thresh) if node_state.n_evaluated == 0 => {
890890
let len = self.stack.len();
891-
if len < k + 1 {
891+
if len < thresh.k() + 1 {
892892
return Some(Err(Error::InsufficientSignaturesMultiSig));
893893
} else {
894894
//Non-sat case. If the first sig is empty, others k elements must
895895
//be empty.
896896
match self.stack.last() {
897897
Some(&stack::Element::Dissatisfied) => {
898898
//Remove the extra zero from multi-sig check
899-
let sigs = self.stack.split_off(len - (k + 1));
899+
let sigs = self.stack.split_off(len - (thresh.k() + 1));
900900
let nonsat = sigs
901901
.iter()
902902
.map(|sig| *sig == stack::Element::Dissatisfied)
903903
.filter(|empty| *empty)
904904
.count();
905-
if nonsat == *k + 1 {
905+
if nonsat == thresh.k() + 1 {
906906
self.stack.push(stack::Element::Dissatisfied);
907907
} else {
908908
return Some(Err(Error::MissingExtraZeroMultiSig));
909909
}
910910
}
911911
None => return Some(Err(Error::UnexpectedStackEnd)),
912912
_ => {
913-
match self
914-
.stack
915-
.evaluate_multi(&mut self.verify_sig, &subs[subs.len() - 1])
916-
{
913+
match self.stack.evaluate_multi(
914+
&mut self.verify_sig,
915+
&thresh.data()[thresh.n() - 1],
916+
) {
917917
Some(Ok(x)) => {
918918
self.push_evaluation_state(
919919
node_state.node,
@@ -933,20 +933,20 @@ where
933933
}
934934
}
935935
}
936-
Terminal::Multi(k, ref subs) => {
937-
if node_state.n_satisfied == k {
936+
Terminal::Multi(ref thresh) => {
937+
if node_state.n_satisfied == thresh.k() {
938938
//multi-sig bug: Pop extra 0
939939
if let Some(stack::Element::Dissatisfied) = self.stack.pop() {
940940
self.stack.push(stack::Element::Satisfied);
941941
} else {
942942
return Some(Err(Error::MissingExtraZeroMultiSig));
943943
}
944-
} else if node_state.n_evaluated == subs.len() {
944+
} else if node_state.n_evaluated == thresh.n() {
945945
return Some(Err(Error::MultiSigEvaluationError));
946946
} else {
947947
match self.stack.evaluate_multi(
948948
&mut self.verify_sig,
949-
&subs[subs.len() - node_state.n_evaluated - 1],
949+
&thresh.data()[thresh.n() - node_state.n_evaluated - 1],
950950
) {
951951
Some(Ok(x)) => {
952952
self.push_evaluation_state(

‎src/miniscript/astelem.rs‎

Lines changed: 32 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,20 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
8282
fmt_2(f, "or_i(", l, r, is_debug)
8383
}
8484
Terminal::Thresh(k, ref subs) => fmt_n(f, "thresh(", k, subs, is_debug),
85-
Terminal::Multi(k, ref keys) => fmt_n(f, "multi(", k, keys, is_debug),
86-
Terminal::MultiA(k, ref keys) => fmt_n(f, "multi_a(", k, keys, is_debug),
85+
Terminal::Multi(ref thresh) => {
86+
if is_debug {
87+
fmt::Debug::fmt(&thresh.debug("multi", true), f)
88+
} else {
89+
fmt::Display::fmt(&thresh.display("multi", true), f)
90+
}
91+
}
92+
Terminal::MultiA(ref thresh) => {
93+
if is_debug {
94+
fmt::Debug::fmt(&thresh.debug("multi_a", true), f)
95+
} else {
96+
fmt::Display::fmt(&thresh.display("multi_a", true), f)
97+
}
98+
}
8799
// wrappers
88100
_ => {
89101
if let Some((ch, sub)) = self.wrap_char() {
@@ -314,27 +326,16 @@ impl<Pk: FromStrKey, Ctx: ScriptContext> crate::expression::FromTree for Termina
314326

315327
Ok(Terminal::Thresh(k, subs?))
316328
}
317-
("multi", n) | ("multi_a", n) => {
318-
if n == 0 {
319-
return Err(errstr("no arguments given"));
320-
}
321-
let k = expression::terminal(&top.args[0], expression::parse_num)? as usize;
322-
if k > n - 1 {
323-
return Err(errstr("higher threshold than there were keys in multi"));
324-
}
325-
326-
let pks: Result<Vec<Pk>, _> = top.args[1..]
327-
.iter()
328-
.map(|sub| expression::terminal(sub, Pk::from_str))
329-
.collect();
330-
331-
if frag_name == "multi" {
332-
pks.map(|pks| Terminal::Multi(k, pks))
333-
} else {
334-
// must be multi_a
335-
pks.map(|pks| Terminal::MultiA(k, pks))
336-
}
337-
}
329+
("multi", _) => top
330+
.to_null_threshold()
331+
.map_err(Error::ParseThreshold)?
332+
.translate_by_index(|i| expression::terminal(&top.args[1 + i], Pk::from_str))
333+
.map(Terminal::Multi),
334+
("multi_a", _) => top
335+
.to_null_threshold()
336+
.map_err(Error::ParseThreshold)?
337+
.translate_by_index(|i| expression::terminal(&top.args[1 + i], Pk::from_str))
338+
.map(Terminal::MultiA),
338339
_ => Err(Error::Unexpected(format!(
339340
"{}({} args) while parsing Miniscript",
340341
top.name,
@@ -483,27 +484,27 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
483484
.push_int(k as i64)
484485
.push_opcode(opcodes::all::OP_EQUAL)
485486
}
486-
Terminal::Multi(k, ref keys) => {
487+
Terminal::Multi(ref thresh) => {
487488
debug_assert!(Ctx::sig_type() == SigType::Ecdsa);
488-
builder = builder.push_int(k as i64);
489-
for pk in keys {
489+
builder = builder.push_int(thresh.k() as i64);
490+
for pk in thresh.data() {
490491
builder = builder.push_key(&pk.to_public_key());
491492
}
492493
builder
493-
.push_int(keys.len() as i64)
494+
.push_int(thresh.n() as i64)
494495
.push_opcode(opcodes::all::OP_CHECKMULTISIG)
495496
}
496-
Terminal::MultiA(k, ref keys) => {
497+
Terminal::MultiA(ref thresh) => {
497498
debug_assert!(Ctx::sig_type() == SigType::Schnorr);
498499
// keys must be atleast len 1 here, guaranteed by typing rules
499-
builder = builder.push_ms_key::<_, Ctx>(&keys[0]);
500+
builder = builder.push_ms_key::<_, Ctx>(&thresh.data()[0]);
500501
builder = builder.push_opcode(opcodes::all::OP_CHECKSIG);
501-
for pk in keys.iter().skip(1) {
502+
for pk in thresh.iter().skip(1) {
502503
builder = builder.push_ms_key::<_, Ctx>(pk);
503504
builder = builder.push_opcode(opcodes::all::OP_CHECKSIGADD);
504505
}
505506
builder
506-
.push_int(k as i64)
507+
.push_int(thresh.k() as i64)
507508
.push_opcode(opcodes::all::OP_NUMEQUAL)
508509
}
509510
}

‎src/miniscript/context.rs‎

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,8 @@ use bitcoin::Weight;
1010

1111
use super::decode::ParseableKey;
1212
use crate::miniscript::limits::{
13-
MAX_OPS_PER_SCRIPT, MAX_PUBKEYS_PER_MULTISIG, MAX_SCRIPTSIG_SIZE, MAX_SCRIPT_ELEMENT_SIZE,
14-
MAX_SCRIPT_SIZE, MAX_STACK_SIZE, MAX_STANDARD_P2WSH_SCRIPT_SIZE,
15-
MAX_STANDARD_P2WSH_STACK_ITEMS,
13+
MAX_OPS_PER_SCRIPT, MAX_SCRIPTSIG_SIZE, MAX_SCRIPT_ELEMENT_SIZE, MAX_SCRIPT_SIZE,
14+
MAX_STACK_SIZE, MAX_STANDARD_P2WSH_SCRIPT_SIZE, MAX_STANDARD_P2WSH_STACK_ITEMS,
1615
};
1716
use crate::miniscript::types;
1817
use crate::prelude::*;
@@ -61,8 +60,6 @@ pub enum ScriptContextError {
6160
TaprootMultiDisabled,
6261
/// Stack size exceeded in script execution
6362
StackSizeLimitExceeded { actual: usize, limit: usize },
64-
/// More than 20 keys in a Multi fragment
65-
CheckMultiSigLimitExceeded,
6663
/// MultiA is only allowed in post tapscript
6764
MultiANotAllowed,
6865
}
@@ -87,7 +84,6 @@ impl error::Error for ScriptContextError {
8784
| ImpossibleSatisfaction
8885
| TaprootMultiDisabled
8986
| StackSizeLimitExceeded { .. }
90-
| CheckMultiSigLimitExceeded
9187
| MultiANotAllowed => None,
9288
}
9389
}
@@ -149,9 +145,6 @@ impl fmt::Display for ScriptContextError {
149145
actual, limit
150146
)
151147
}
152-
ScriptContextError::CheckMultiSigLimitExceeded => {
153-
write!(f, "CHECkMULTISIG ('multi()' descriptor) only supports up to 20 pubkeys")
154-
}
155148
ScriptContextError::MultiANotAllowed => {
156149
write!(f, "Multi a(CHECKSIGADD) only allowed post tapscript")
157150
}
@@ -405,11 +398,8 @@ impl ScriptContext for Legacy {
405398

406399
match ms.node {
407400
Terminal::PkK(ref pk) => Self::check_pk(pk),
408-
Terminal::Multi(_k, ref pks) => {
409-
if pks.len() > MAX_PUBKEYS_PER_MULTISIG {
410-
return Err(ScriptContextError::CheckMultiSigLimitExceeded);
411-
}
412-
for pk in pks.iter() {
401+
Terminal::Multi(ref thresh) => {
402+
for pk in thresh.iter() {
413403
Self::check_pk(pk)?;
414404
}
415405
Ok(())
@@ -506,11 +496,8 @@ impl ScriptContext for Segwitv0 {
506496

507497
match ms.node {
508498
Terminal::PkK(ref pk) => Self::check_pk(pk),
509-
Terminal::Multi(_k, ref pks) => {
510-
if pks.len() > MAX_PUBKEYS_PER_MULTISIG {
511-
return Err(ScriptContextError::CheckMultiSigLimitExceeded);
512-
}
513-
for pk in pks.iter() {
499+
Terminal::Multi(ref thresh) => {
500+
for pk in thresh.iter() {
514501
Self::check_pk(pk)?;
515502
}
516503
Ok(())
@@ -620,8 +607,8 @@ impl ScriptContext for Tap {
620607

621608
match ms.node {
622609
Terminal::PkK(ref pk) => Self::check_pk(pk),
623-
Terminal::MultiA(_, ref keys) => {
624-
for pk in keys.iter() {
610+
Terminal::MultiA(ref thresh) => {
611+
for pk in thresh.iter() {
625612
Self::check_pk(pk)?;
626613
}
627614
Ok(())
@@ -716,11 +703,8 @@ impl ScriptContext for BareCtx {
716703
}
717704
match ms.node {
718705
Terminal::PkK(ref key) => Self::check_pk(key),
719-
Terminal::Multi(_k, ref pks) => {
720-
if pks.len() > MAX_PUBKEYS_PER_MULTISIG {
721-
return Err(ScriptContextError::CheckMultiSigLimitExceeded);
722-
}
723-
for pk in pks.iter() {
706+
Terminal::Multi(ref thresh) => {
707+
for pk in thresh.iter() {
724708
Self::check_pk(pk)?;
725709
}
726710
Ok(())
@@ -749,7 +733,7 @@ impl ScriptContext for BareCtx {
749733
Terminal::PkK(_pk) | Terminal::PkH(_pk) => Ok(()),
750734
_ => Err(Error::NonStandardBareScript),
751735
},
752-
Terminal::Multi(_k, subs) if subs.len() <= 3 => Ok(()),
736+
Terminal::Multi(ref thresh) if thresh.n() <= 3 => Ok(()),
753737
_ => Err(Error::NonStandardBareScript),
754738
}
755739
}

‎src/miniscript/decode.rs‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ use crate::miniscript::ScriptContext;
2121
use crate::prelude::*;
2222
#[cfg(doc)]
2323
use crate::Descriptor;
24-
use crate::{hash256, AbsLockTime, Error, Miniscript, MiniscriptKey, RelLockTime, ToPublicKey};
24+
use crate::{
25+
hash256, AbsLockTime, Error, Miniscript, MiniscriptKey, RelLockTime, Threshold, ToPublicKey,
26+
};
2527

2628
/// Trait for parsing keys from byte slices
2729
pub trait ParseableKey: Sized + ToPublicKey + private::Sealed {
@@ -181,9 +183,9 @@ pub enum Terminal<Pk: MiniscriptKey, Ctx: ScriptContext> {
181183
/// `[E] ([W] ADD)* k EQUAL`
182184
Thresh(usize, Vec<Arc<Miniscript<Pk, Ctx>>>),
183185
/// `k (<key>)* n CHECKMULTISIG`
184-
Multi(usize, Vec<Pk>),
186+
Multi(Threshold<Pk, MAX_PUBKEYS_PER_MULTISIG>),
185187
/// `<key> CHECKSIG (<key> CHECKSIGADD)*(n-1) k NUMEQUAL`
186-
MultiA(usize, Vec<Pk>),
188+
MultiA(Threshold<Pk, MAX_PUBKEYS_IN_CHECKSIGADD>),
187189
}
188190

189191
macro_rules! match_token {
@@ -428,6 +430,7 @@ pub fn parse<Ctx: ScriptContext>(
428430
},
429431
// CHECKMULTISIG based multisig
430432
Tk::CheckMultiSig, Tk::Num(n) => {
433+
// Check size before allocating keys
431434
if n as usize > MAX_PUBKEYS_PER_MULTISIG {
432435
return Err(Error::CmsTooManyKeys(n));
433436
}
@@ -446,7 +449,8 @@ pub fn parse<Ctx: ScriptContext>(
446449
Tk::Num(k) => k,
447450
);
448451
keys.reverse();
449-
term.reduce0(Terminal::Multi(k as usize, keys))?;
452+
let thresh = Threshold::new(k as usize, keys).map_err(Error::Threshold)?;
453+
term.reduce0(Terminal::Multi(thresh))?;
450454
},
451455
// MultiA
452456
Tk::NumEqual, Tk::Num(k) => {
@@ -469,7 +473,8 @@ pub fn parse<Ctx: ScriptContext>(
469473
.map_err(|e| Error::PubKeyCtxError(e, Ctx::name_str()))?),
470474
);
471475
keys.reverse();
472-
term.reduce0(Terminal::MultiA(k as usize, keys))?;
476+
let thresh = Threshold::new(k as usize, keys).map_err(Error::Threshold)?;
477+
term.reduce0(Terminal::MultiA(thresh))?;
473478
},
474479
);
475480
}

0 commit comments

Comments
 (0)