Skip to content

Commit

Permalink
miniscript: use new Threshold type for multi/multi_a
Browse files Browse the repository at this point in the history
  • Loading branch information
@apoelstra
apoelstra committed Apr 4, 2024
1 parent 7e7ddd2 commit 051af85
Show file tree
Hide file tree
Showing 13 changed files with 147 additions and 209 deletions.
2 changes: 1 addition & 1 deletion src/descriptor/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1492,7 +1492,7 @@ mod tests {
#[test]
fn roundtrip_tests() {
let descriptor = Descriptor::<bitcoin::PublicKey>::from_str("multi");
assert_eq!(descriptor.unwrap_err().to_string(), "unexpected «no arguments given»")
assert_eq!(descriptor.unwrap_err().to_string(), "expected threshold, found terminal",);
}

#[test]
Expand Down
15 changes: 6 additions & 9 deletions src/descriptor/sortedmulti.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
// Check the limits before creating a new SortedMultiVec
// For example, under p2sh context the scriptlen can only be
// upto 520 bytes.
let term: Terminal<Pk, Ctx> = Terminal::Multi(self.inner.k(), self.inner.data().to_owned());
let term: Terminal<Pk, Ctx> = Terminal::Multi(self.inner.clone());
let ms = Miniscript::from_ast(term)?;
// This would check all the consensus rules for p2sh/p2wsh and
// even tapscript in future
Expand Down Expand Up @@ -113,11 +113,8 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
/// utility function to sanity a sorted multi vec
pub fn sanity_check(&self) -> Result<(), Error> {
let ms: Miniscript<Pk, Ctx> =
Miniscript::from_ast(Terminal::Multi(self.k(), self.pks().to_owned()))
.expect("Must typecheck");
// '?' for doing From conversion
ms.sanity_check()?;
Ok(())
Miniscript::from_ast(Terminal::Multi(self.inner.clone())).expect("Must typecheck");
ms.sanity_check().map_err(From::from)
}
}

Expand All @@ -127,16 +124,16 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
where
Pk: ToPublicKey,
{
let mut pks = self.pks().to_owned();
let mut thresh = self.inner.clone();
// Sort pubkeys lexicographically according to BIP 67
pks.sort_by(|a, b| {
thresh.data_mut().sort_by(|a, b| {
a.to_public_key()
.inner
.serialize()
.partial_cmp(&b.to_public_key().inner.serialize())
.unwrap()
});
Terminal::Multi(self.k(), pks)
Terminal::Multi(thresh)
}

/// Encode as a Bitcoin script
Expand Down
38 changes: 19 additions & 19 deletions src/interpreter/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -845,9 +845,9 @@ where
None => return Some(Err(Error::UnexpectedStackEnd)),
}
}
Terminal::MultiA(k, ref subs) => {
if node_state.n_evaluated == subs.len() {
if node_state.n_satisfied == k {
Terminal::MultiA(ref thresh) => {
if node_state.n_evaluated == thresh.n() {
if node_state.n_satisfied == thresh.k() {
self.stack.push(stack::Element::Satisfied);
} else {
self.stack.push(stack::Element::Dissatisfied);
Expand All @@ -856,10 +856,10 @@ where
// evaluate each key with as a pk
// note that evaluate_pk will error on non-empty incorrect sigs
// push 1 on satisfied sigs and push 0 on empty sigs
match self
.stack
.evaluate_pk(&mut self.verify_sig, subs[node_state.n_evaluated])
{
match self.stack.evaluate_pk(
&mut self.verify_sig,
thresh.data()[node_state.n_evaluated],
) {
Some(Ok(x)) => {
self.push_evaluation_state(
node_state.node,
Expand All @@ -886,34 +886,34 @@ where
}
}
}
Terminal::Multi(ref k, ref subs) if node_state.n_evaluated == 0 => {
Terminal::Multi(ref thresh) if node_state.n_evaluated == 0 => {
let len = self.stack.len();
if len < k + 1 {
if len < thresh.k() + 1 {
return Some(Err(Error::InsufficientSignaturesMultiSig));
} else {
//Non-sat case. If the first sig is empty, others k elements must
//be empty.
match self.stack.last() {
Some(&stack::Element::Dissatisfied) => {
//Remove the extra zero from multi-sig check
let sigs = self.stack.split_off(len - (k + 1));
let sigs = self.stack.split_off(len - (thresh.k() + 1));
let nonsat = sigs
.iter()
.map(|sig| *sig == stack::Element::Dissatisfied)
.filter(|empty| *empty)
.count();
if nonsat == *k + 1 {
if nonsat == thresh.k() + 1 {
self.stack.push(stack::Element::Dissatisfied);
} else {
return Some(Err(Error::MissingExtraZeroMultiSig));
}
}
None => return Some(Err(Error::UnexpectedStackEnd)),
_ => {
match self
.stack
.evaluate_multi(&mut self.verify_sig, &subs[subs.len() - 1])
{
match self.stack.evaluate_multi(
&mut self.verify_sig,
&thresh.data()[thresh.n() - 1],
) {
Some(Ok(x)) => {
self.push_evaluation_state(
node_state.node,
Expand All @@ -933,20 +933,20 @@ where
}
}
}
Terminal::Multi(k, ref subs) => {
if node_state.n_satisfied == k {
Terminal::Multi(ref thresh) => {
if node_state.n_satisfied == thresh.k() {
//multi-sig bug: Pop extra 0
if let Some(stack::Element::Dissatisfied) = self.stack.pop() {
self.stack.push(stack::Element::Satisfied);
} else {
return Some(Err(Error::MissingExtraZeroMultiSig));
}
} else if node_state.n_evaluated == subs.len() {
} else if node_state.n_evaluated == thresh.n() {
return Some(Err(Error::MultiSigEvaluationError));
} else {
match self.stack.evaluate_multi(
&mut self.verify_sig,
&subs[subs.len() - node_state.n_evaluated - 1],
&thresh.data()[thresh.n() - node_state.n_evaluated - 1],
) {
Some(Ok(x)) => {
self.push_evaluation_state(
Expand Down
63 changes: 32 additions & 31 deletions src/miniscript/astelem.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,20 @@ 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::Multi(k, ref keys) => fmt_n(f, "multi(", k, keys, is_debug),
Terminal::MultiA(k, ref keys) => fmt_n(f, "multi_a(", k, keys, is_debug),
Terminal::Multi(ref thresh) => {
if is_debug {
fmt::Debug::fmt(&thresh.debug("multi", true), f)
} else {
fmt::Display::fmt(&thresh.display("multi", true), f)
}
}
Terminal::MultiA(ref thresh) => {
if is_debug {
fmt::Debug::fmt(&thresh.debug("multi_a", true), f)
} else {
fmt::Display::fmt(&thresh.display("multi_a", true), f)
}
}
// wrappers
_ => {
if let Some((ch, sub)) = self.wrap_char() {
Expand Down Expand Up @@ -314,27 +326,16 @@ impl<Pk: FromStrKey, Ctx: ScriptContext> crate::expression::FromTree for Termina

Ok(Terminal::Thresh(k, subs?))
}
("multi", n) | ("multi_a", 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 were keys in multi"));
}

let pks: Result<Vec<Pk>, _> = top.args[1..]
.iter()
.map(|sub| expression::terminal(sub, Pk::from_str))
.collect();

if frag_name == "multi" {
pks.map(|pks| Terminal::Multi(k, pks))
} else {
// must be multi_a
pks.map(|pks| Terminal::MultiA(k, pks))
}
}
("multi", _) => top
.to_null_threshold()
.map_err(Error::ParseThreshold)?
.translate_by_index(|i| expression::terminal(&top.args[1 + i], Pk::from_str))
.map(Terminal::Multi),
("multi_a", _) => top
.to_null_threshold()
.map_err(Error::ParseThreshold)?
.translate_by_index(|i| expression::terminal(&top.args[1 + i], Pk::from_str))
.map(Terminal::MultiA),
_ => Err(Error::Unexpected(format!(
"{}({} args) while parsing Miniscript",
top.name,
Expand Down Expand Up @@ -483,27 +484,27 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
.push_int(k as i64)
.push_opcode(opcodes::all::OP_EQUAL)
}
Terminal::Multi(k, ref keys) => {
Terminal::Multi(ref thresh) => {
debug_assert!(Ctx::sig_type() == SigType::Ecdsa);
builder = builder.push_int(k as i64);
for pk in keys {
builder = builder.push_int(thresh.k() as i64);
for pk in thresh.data() {
builder = builder.push_key(&pk.to_public_key());
}
builder
.push_int(keys.len() as i64)
.push_int(thresh.n() as i64)
.push_opcode(opcodes::all::OP_CHECKMULTISIG)
}
Terminal::MultiA(k, ref keys) => {
Terminal::MultiA(ref thresh) => {
debug_assert!(Ctx::sig_type() == SigType::Schnorr);
// keys must be atleast len 1 here, guaranteed by typing rules
builder = builder.push_ms_key::<_, Ctx>(&keys[0]);
builder = builder.push_ms_key::<_, Ctx>(&thresh.data()[0]);
builder = builder.push_opcode(opcodes::all::OP_CHECKSIG);
for pk in keys.iter().skip(1) {
for pk in thresh.iter().skip(1) {
builder = builder.push_ms_key::<_, Ctx>(pk);
builder = builder.push_opcode(opcodes::all::OP_CHECKSIGADD);
}
builder
.push_int(k as i64)
.push_int(thresh.k() as i64)
.push_opcode(opcodes::all::OP_NUMEQUAL)
}
}
Expand Down
38 changes: 11 additions & 27 deletions src/miniscript/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@ use bitcoin::Weight;

use super::decode::ParseableKey;
use crate::miniscript::limits::{
MAX_OPS_PER_SCRIPT, MAX_PUBKEYS_PER_MULTISIG, MAX_SCRIPTSIG_SIZE, MAX_SCRIPT_ELEMENT_SIZE,
MAX_SCRIPT_SIZE, MAX_STACK_SIZE, MAX_STANDARD_P2WSH_SCRIPT_SIZE,
MAX_STANDARD_P2WSH_STACK_ITEMS,
MAX_OPS_PER_SCRIPT, MAX_SCRIPTSIG_SIZE, MAX_SCRIPT_ELEMENT_SIZE, MAX_SCRIPT_SIZE,
MAX_STACK_SIZE, MAX_STANDARD_P2WSH_SCRIPT_SIZE, MAX_STANDARD_P2WSH_STACK_ITEMS,
};
use crate::miniscript::types;
use crate::prelude::*;
Expand Down Expand Up @@ -61,8 +60,6 @@ pub enum ScriptContextError {
TaprootMultiDisabled,
/// Stack size exceeded in script execution
StackSizeLimitExceeded { actual: usize, limit: usize },
/// More than 20 keys in a Multi fragment
CheckMultiSigLimitExceeded,
/// MultiA is only allowed in post tapscript
MultiANotAllowed,
}
Expand All @@ -87,7 +84,6 @@ impl error::Error for ScriptContextError {
| ImpossibleSatisfaction
| TaprootMultiDisabled
| StackSizeLimitExceeded { .. }
| CheckMultiSigLimitExceeded
| MultiANotAllowed => None,
}
}
Expand Down Expand Up @@ -149,9 +145,6 @@ impl fmt::Display for ScriptContextError {
actual, limit
)
}
ScriptContextError::CheckMultiSigLimitExceeded => {
write!(f, "CHECkMULTISIG ('multi()' descriptor) only supports up to 20 pubkeys")
}
ScriptContextError::MultiANotAllowed => {
write!(f, "Multi a(CHECKSIGADD) only allowed post tapscript")
}
Expand Down Expand Up @@ -405,11 +398,8 @@ impl ScriptContext for Legacy {

match ms.node {
Terminal::PkK(ref pk) => Self::check_pk(pk),
Terminal::Multi(_k, ref pks) => {
if pks.len() > MAX_PUBKEYS_PER_MULTISIG {
return Err(ScriptContextError::CheckMultiSigLimitExceeded);
}
for pk in pks.iter() {
Terminal::Multi(ref thresh) => {
for pk in thresh.iter() {
Self::check_pk(pk)?;
}
Ok(())
Expand Down Expand Up @@ -506,11 +496,8 @@ impl ScriptContext for Segwitv0 {

match ms.node {
Terminal::PkK(ref pk) => Self::check_pk(pk),
Terminal::Multi(_k, ref pks) => {
if pks.len() > MAX_PUBKEYS_PER_MULTISIG {
return Err(ScriptContextError::CheckMultiSigLimitExceeded);
}
for pk in pks.iter() {
Terminal::Multi(ref thresh) => {
for pk in thresh.iter() {
Self::check_pk(pk)?;
}
Ok(())
Expand Down Expand Up @@ -620,8 +607,8 @@ impl ScriptContext for Tap {

match ms.node {
Terminal::PkK(ref pk) => Self::check_pk(pk),
Terminal::MultiA(_, ref keys) => {
for pk in keys.iter() {
Terminal::MultiA(ref thresh) => {
for pk in thresh.iter() {
Self::check_pk(pk)?;
}
Ok(())
Expand Down Expand Up @@ -716,11 +703,8 @@ impl ScriptContext for BareCtx {
}
match ms.node {
Terminal::PkK(ref key) => Self::check_pk(key),
Terminal::Multi(_k, ref pks) => {
if pks.len() > MAX_PUBKEYS_PER_MULTISIG {
return Err(ScriptContextError::CheckMultiSigLimitExceeded);
}
for pk in pks.iter() {
Terminal::Multi(ref thresh) => {
for pk in thresh.iter() {
Self::check_pk(pk)?;
}
Ok(())
Expand Down Expand Up @@ -749,7 +733,7 @@ impl ScriptContext for BareCtx {
Terminal::PkK(_pk) | Terminal::PkH(_pk) => Ok(()),
_ => Err(Error::NonStandardBareScript),
},
Terminal::Multi(_k, subs) if subs.len() <= 3 => Ok(()),
Terminal::Multi(ref thresh) if thresh.n() <= 3 => Ok(()),
_ => Err(Error::NonStandardBareScript),
}
}
Expand Down
15 changes: 10 additions & 5 deletions src/miniscript/decode.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ use crate::miniscript::ScriptContext;
use crate::prelude::*;
#[cfg(doc)]
use crate::Descriptor;
use crate::{hash256, AbsLockTime, Error, Miniscript, MiniscriptKey, RelLockTime, ToPublicKey};
use crate::{
hash256, AbsLockTime, Error, Miniscript, MiniscriptKey, RelLockTime, Threshold, ToPublicKey,
};

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

macro_rules! match_token {
Expand Down Expand Up @@ -428,6 +430,7 @@ pub fn parse<Ctx: ScriptContext>(
},
// CHECKMULTISIG based multisig
Tk::CheckMultiSig, Tk::Num(n) => {
// Check size before allocating keys
if n as usize > MAX_PUBKEYS_PER_MULTISIG {
return Err(Error::CmsTooManyKeys(n));
}
Expand All @@ -446,7 +449,8 @@ pub fn parse<Ctx: ScriptContext>(
Tk::Num(k) => k,
);
keys.reverse();
term.reduce0(Terminal::Multi(k as usize, keys))?;
let thresh = Threshold::new(k as usize, keys).map_err(Error::Threshold)?;
term.reduce0(Terminal::Multi(thresh))?;
},
// MultiA
Tk::NumEqual, Tk::Num(k) => {
Expand All @@ -469,7 +473,8 @@ pub fn parse<Ctx: ScriptContext>(
.map_err(|e| Error::PubKeyCtxError(e, Ctx::name_str()))?),
);
keys.reverse();
term.reduce0(Terminal::MultiA(k as usize, keys))?;
let thresh = Threshold::new(k as usize, keys).map_err(Error::Threshold)?;
term.reduce0(Terminal::MultiA(thresh))?;
},
);
}
Expand Down
Loading

0 comments on commit 051af85

Please sign in to comment.