Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Threshold type in concrete policy and in Terminal::multi/multi_a #674

Merged
merged 3 commits into from
Apr 6, 2024
Merged

Use Threshold type in concrete policy and in Terminal::multi/multi_a #674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1ced03b
policy: use Threshold in concrete policy
apoelstra Apr 4, 2024
f62322d
miniscript: use new Threshold type for multi/multi_a
apoelstra Mar 12, 2024
79d3f7e
sortedmulti: eliminate allocation in constructor_check
apoelstra Apr 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/descriptor/mod.rs
View file
Open in desktop
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
34 changes: 17 additions & 17 deletions src/descriptor/sortedmulti.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,21 @@ pub struct SortedMultiVec<Pk: MiniscriptKey, Ctx: ScriptContext> {
}

impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
fn constructor_check(&self) -> Result<(), Error> {
fn constructor_check(mut self) -> Result<Self, Error> {
// 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);
let ms = Miniscript::from_ast(term)?;
// This would check all the consensus rules for p2sh/p2wsh and
// even tapscript in future
Ctx::check_local_validity(&ms).map_err(From::from)
Ctx::check_local_validity(&ms)?;
if let Terminal::Multi(inner) = ms.node {
self.inner = inner;
Ok(self)
} else {
unreachable!()
}
}

/// Create a new instance of `SortedMultiVec` given a list of keys and the threshold
Expand All @@ -49,8 +55,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
pub fn new(k: usize, pks: Vec<Pk>) -> Result<Self, Error> {
let ret =
Self { inner: Threshold::new(k, pks).map_err(Error::Threshold)?, phantom: PhantomData };
ret.constructor_check()?;
Ok(ret)
ret.constructor_check()
}

/// Parse an expression tree into a SortedMultiVec
Expand All @@ -66,8 +71,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
.translate_by_index(|i| expression::terminal(&tree.args[i + 1], Pk::from_str))?,
phantom: PhantomData,
};
ret.constructor_check()?;
Ok(ret)
ret.constructor_check()
}

/// This will panic if fpk returns an uncompressed key when
Expand All @@ -85,8 +89,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
inner: self.inner.translate_ref(|pk| t.pk(pk))?,
phantom: PhantomData,
};
ret.constructor_check().map_err(TranslateErr::OuterError)?;
Ok(ret)
ret.constructor_check().map_err(TranslateErr::OuterError)
}

/// The threshold value for the multisig.
Expand All @@ -113,11 +116,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 +127,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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading
Loading