From a5c477b53050a49d96333b1f221bff51039bd49c Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 7 Aug 2025 15:22:34 +0000 Subject: [PATCH 1/6] clippy: fix all nightly lints The lifetime stuff in particular has been blocking https://github.com/rust-bitcoin/rust-miniscript/pull/805 every week for several months now. --- src/descriptor/key.rs | 4 +--- src/descriptor/mod.rs | 2 +- src/descriptor/tr/mod.rs | 2 +- src/descriptor/tr/spend_info.rs | 2 +- src/descriptor/tr/taptree.rs | 2 +- src/miniscript/iter.rs | 4 ++-- src/miniscript/mod.rs | 2 +- src/policy/concrete.rs | 2 +- src/primitives/threshold.rs | 2 +- 9 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/descriptor/key.rs b/src/descriptor/key.rs index 2162010d7..241f931ee 100644 --- a/src/descriptor/key.rs +++ b/src/descriptor/key.rs @@ -1203,9 +1203,7 @@ impl DescriptorXKey { }; if &compare_fingerprint == fingerprint - && compare_path - .into_iter() - .eq(path_excluding_wildcard.into_iter()) + && compare_path.into_iter().eq(&path_excluding_wildcard) { Some(path_excluding_wildcard) } else { diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 44b93ce59..9bc6f2c31 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -294,7 +294,7 @@ impl Descriptor { /// /// If the descriptor is not a Taproot descriptor, **or** if the descriptor is a /// Taproot descriptor containing only a keyspend, returns an empty iterator. - pub fn tap_tree_iter(&self) -> tr::TapTreeIter { + pub fn tap_tree_iter(&self) -> tr::TapTreeIter<'_, Pk> { if let Descriptor::Tr(ref tr) = self { if let Some(tree) = tr.tap_tree() { return tree.leaves(); diff --git a/src/descriptor/tr/mod.rs b/src/descriptor/tr/mod.rs index f0f4ccfc4..203e3cee2 100644 --- a/src/descriptor/tr/mod.rs +++ b/src/descriptor/tr/mod.rs @@ -108,7 +108,7 @@ impl Tr { /// /// The yielded elements include the Miniscript for each leave as well as its depth /// in the tree, which is the data required by PSBT (BIP 371). - pub fn leaves(&self) -> TapTreeIter { + pub fn leaves(&self) -> TapTreeIter<'_, Pk> { match self.tree { Some(ref t) => t.leaves(), None => TapTreeIter::empty(), diff --git a/src/descriptor/tr/spend_info.rs b/src/descriptor/tr/spend_info.rs index 3bace2d18..b0d23121d 100644 --- a/src/descriptor/tr/spend_info.rs +++ b/src/descriptor/tr/spend_info.rs @@ -181,7 +181,7 @@ impl TrSpendInfo { /// This yields the same leaves in the same order as [`super::Tr::leaves`] on the original /// [`super::Tr`]. However, in addition to yielding the leaves and their depths, it also /// yields their scripts, leafhashes, and control blocks. - pub fn leaves(&self) -> TrSpendInfoIter { + pub fn leaves(&self) -> TrSpendInfoIter<'_, Pk> { TrSpendInfoIter { spend_info: self, index: 0, diff --git a/src/descriptor/tr/taptree.rs b/src/descriptor/tr/taptree.rs index 35f23a95e..27e860390 100644 --- a/src/descriptor/tr/taptree.rs +++ b/src/descriptor/tr/taptree.rs @@ -53,7 +53,7 @@ impl TapTree { /// /// The yielded elements include the Miniscript for each leave as well as its depth /// in the tree, which is the data required by PSBT (BIP 371). - pub fn leaves(&self) -> TapTreeIter { TapTreeIter::from_tree(self) } + pub fn leaves(&self) -> TapTreeIter<'_, Pk> { TapTreeIter::from_tree(self) } /// Converts keys from one type of public key to another. pub fn translate_pk( diff --git a/src/miniscript/iter.rs b/src/miniscript/iter.rs index f82e329aa..0ac48e651 100644 --- a/src/miniscript/iter.rs +++ b/src/miniscript/iter.rs @@ -18,12 +18,12 @@ impl Miniscript { /// Creates a new [Iter] iterator that will iterate over all [Miniscript] items within /// AST by traversing its branches. For the specific algorithm please see /// [Iter::next] function. - pub fn iter(&self) -> Iter { Iter::new(self) } + pub fn iter(&self) -> Iter<'_, Pk, Ctx> { Iter::new(self) } /// Creates a new [PkIter] iterator that will iterate over all plain public keys (and not /// key hash values) present in [Miniscript] items within AST by traversing all its branches. /// For the specific algorithm please see [PkIter::next] function. - pub fn iter_pk(&self) -> PkIter { PkIter::new(self) } + pub fn iter_pk(&self) -> PkIter<'_, Pk, Ctx> { PkIter::new(self) } /// Enumerates all child nodes of the current AST node (`self`) and returns a `Vec` referencing /// them. diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index 269f36a4d..286f1e269 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -604,7 +604,7 @@ impl Miniscript { /// The type information and extra properties are implied by the AST. impl PartialOrd for Miniscript { fn partial_cmp(&self, other: &Miniscript) -> Option { - Some(self.node.cmp(&other.node)) + Some(self.cmp(other)) } } diff --git a/src/policy/concrete.rs b/src/policy/concrete.rs index 01ac117a2..547b4e86d 100644 --- a/src/policy/concrete.rs +++ b/src/policy/concrete.rs @@ -192,7 +192,7 @@ impl Policy { /// Since this splitting might lead to exponential blow-up, we constrain the number of /// leaf-nodes to [`MAX_COMPILATION_LEAVES`]. #[cfg(feature = "compiler")] - fn tapleaf_probability_iter(&self) -> TapleafProbabilityIter { + fn tapleaf_probability_iter(&self) -> TapleafProbabilityIter<'_, Pk> { TapleafProbabilityIter { stack: vec![(1.0, self)] } } diff --git a/src/primitives/threshold.rs b/src/primitives/threshold.rs index ee0c1bda5..f1021e9a4 100644 --- a/src/primitives/threshold.rs +++ b/src/primitives/threshold.rs @@ -217,7 +217,7 @@ impl Threshold { pub fn into_data(self) -> Vec { self.inner } /// Passthrough to an iterator on the underlying vector. - pub fn iter(&self) -> core::slice::Iter { self.inner.iter() } + pub fn iter(&self) -> core::slice::Iter<'_, T> { self.inner.iter() } } impl Threshold { From 040c29df21f39143dc994b0565e667227b16177e Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 11 May 2025 13:24:08 +0000 Subject: [PATCH 2/6] miniscript: remove MalleablePkH script context rule This rule is weird and wrong in a number of ways. It was introduced in PR #97 c93bdefeea1c972d4161c5f1b1a1b49ba25c49e1 which was the massive PR that introduced the Ctx parameter. This particular rule wasn't commented on, but we can infer some things: * From the docs on the error and my vague memory, the problem was that we couldn't estimate satisfaction for a pubkeyhash fragment because we didn't know if the key was 33 or 65 bytes ahead of time. * Therefore, the code bans this in a legacy context, but not in bare contexts (in bare contexts there is a comment saying that bare fragments "can't contain miniscript because of standardness rules" so maybe we thought about this ... but that comment has never been true, so we thought wrong..). We should've banned the fragment in bare contexts as well. * Also, while the error is called MalleablePkH, none of this has anything to do with malleability! Later, in #431, we introduced the RawPkH fragment and put a pubkey into the PkH fragment. At that point, for PkH we always knew the pubkey and could just directly compute its size using the `is_uncompressed` accessor. The RawPkH fragment was a second-class citizen which didn't even have a string serialization. At this point we should have dropped the MalleablePkH rule. Still later, in #461, we introduced `ExtParams` and the ability to parse "insane" scripts in a more flexible way, including enabling the experimental `expr_raw_raw_pkh` fragment. We then gained an error `Analyzable::ContainsRawPkh` which conceptually overlaps with `ScriptContextError::MalleablePkH` but which isn't tied to any context, just whether the user has enabled this second-class feature. --- src/miniscript/context.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/miniscript/context.rs b/src/miniscript/context.rs index 326f283cc..b08d8ed3c 100644 --- a/src/miniscript/context.rs +++ b/src/miniscript/context.rs @@ -21,10 +21,6 @@ use crate::{hash256, Error, ForEachKey, Miniscript, MiniscriptKey, Terminal}; /// Error for Script Context #[derive(Clone, PartialEq, Eq, Debug)] pub enum ScriptContextError { - /// Script Context does not permit PkH for non-malleability - /// It is not possible to estimate the pubkey size at the creation - /// time because of uncompressed pubkeys - MalleablePkH, /// Script Context does not permit OrI for non-malleability /// Legacy fragments allow non-minimal IF which results in malleability MalleableOrI, @@ -74,8 +70,7 @@ impl error::Error for ScriptContextError { use self::ScriptContextError::*; match self { - MalleablePkH - | MalleableOrI + MalleableOrI | MalleableDupIf | CompressedOnly(_) | XOnlyKeysNotAllowed(_, _) @@ -97,7 +92,6 @@ impl error::Error for ScriptContextError { impl fmt::Display for ScriptContextError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { - ScriptContextError::MalleablePkH => write!(f, "PkH is malleable under Legacy rules"), ScriptContextError::MalleableOrI => write!(f, "OrI is malleable under Legacy rules"), ScriptContextError::MalleableDupIf => { write!(f, "DupIf is malleable under Legacy rules") @@ -380,8 +374,6 @@ impl ScriptContext for Legacy { frag: &Terminal, ) -> Result<(), ScriptContextError> { match *frag { - Terminal::PkH(ref _pkh) => Err(ScriptContextError::MalleablePkH), - Terminal::RawPkH(ref _pk) => Err(ScriptContextError::MalleablePkH), Terminal::OrI(ref _a, ref _b) => Err(ScriptContextError::MalleableOrI), Terminal::DupIf(ref _ms) => Err(ScriptContextError::MalleableDupIf), _ => Ok(()), From 887464000d89f2b5a352afbbf984f57b17bc76f1 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 20 May 2025 00:25:03 +0000 Subject: [PATCH 3/6] miniscript: fix ExtData to correctly account for uncompressed keys In our existing code we assume that all "ECDSA keys" are compressed, i.e. 34 bytes long. This is not a valid assumption. We originally did this because under some circumstances we legitimately did not know the key and didn't want to penalize users by assuming uncompressed keys when this was unlikely the case. However, now the only way to have a pk or pkh where we don't know the key is if the user is using a raw pkh. And if they do this in Taproot or Segwit we still know that there are no uncompressed keys (though with the current Ctx trait we can't distinguish Segwit from pre-Segwit so the new logic assumes uncompressed keys even for Segwit.) This logic will be cleaned up and improved with ValidationParams; but currently it is wrong, and it is causing me trouble with later commits. So fix the situation here as best we can. The next commit will make a similar change to the multi() combinator (multi_a is fine because the keys and sigs are fixed size); the commit after that will introduce a unit test to SortedMulti which exercises this logic. --- src/miniscript/mod.rs | 6 ++-- src/miniscript/types/extra_props.rs | 48 ++++++++++++++++------------- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index 286f1e269..5bc54cc2c 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -183,9 +183,9 @@ mod private { /// The `pk_k` combinator. pub fn pk_k(pk: Pk) -> Self { Self { + ext: types::extra_props::ExtData::pk_k::<_, Ctx>(&pk), node: Terminal::PkK(pk), ty: types::Type::pk_k(), - ext: types::extra_props::ExtData::pk_k::(), phantom: PhantomData, } } @@ -193,9 +193,9 @@ mod private { /// The `pk_h` combinator. pub fn pk_h(pk: Pk) -> Self { Self { + ext: types::extra_props::ExtData::pk_h::<_, Ctx>(Some(&pk)), node: Terminal::PkH(pk), ty: types::Type::pk_h(), - ext: types::extra_props::ExtData::pk_h::(), phantom: PhantomData, } } @@ -205,7 +205,7 @@ mod private { Self { node: Terminal::RawPkH(hash), ty: types::Type::pk_h(), - ext: types::extra_props::ExtData::pk_h::(), + ext: types::extra_props::ExtData::pk_h::(None), phantom: PhantomData, } } diff --git a/src/miniscript/types/extra_props.rs b/src/miniscript/types/extra_props.rs index 3db46b89f..36bfc0f3a 100644 --- a/src/miniscript/types/extra_props.rs +++ b/src/miniscript/types/extra_props.rs @@ -7,7 +7,6 @@ use core::cmp; use core::iter::once; use super::ScriptContext; -use crate::miniscript::context::SigType; use crate::prelude::*; use crate::{script_num_size, AbsLockTime, MiniscriptKey, RelLockTime, Terminal}; @@ -191,20 +190,22 @@ impl ExtData { } /// Extra properties for the `pk_k` fragment. - pub fn pk_k() -> Self { + /// + /// The key must be provided to determine its size. + pub fn pk_k(pk: &Pk) -> Self { + let (key_bytes, max_sig_bytes) = match Ctx::sig_type() { + crate::SigType::Ecdsa if pk.is_uncompressed() => (65, 73), + crate::SigType::Ecdsa => (34, 73), + crate::SigType::Schnorr => (33, 66), + }; + ExtData { - pk_cost: match Ctx::sig_type() { - SigType::Ecdsa => 34, - SigType::Schnorr => 33, - }, + pk_cost: key_bytes, has_free_verify: false, ops: OpLimits::new(0, Some(0), Some(0)), stack_elem_count_sat: Some(1), stack_elem_count_dissat: Some(1), - max_sat_size: match Ctx::sig_type() { - SigType::Ecdsa => Some((73, 73)), - SigType::Schnorr => Some((66, 66)), - }, + max_sat_size: Some((max_sig_bytes, max_sig_bytes)), max_dissat_size: Some((1, 1)), timelock_info: TimelockInfo::default(), exec_stack_elem_count_sat: Some(1), // pushes the pk @@ -214,21 +215,25 @@ impl ExtData { } /// Extra properties for the `pk_h` fragment. - pub fn pk_h() -> Self { + /// + /// If the key is known, it should be provided to gain a size estimate from + /// it. If not, the worst-case for the context will be assumed. + pub fn pk_h(pk: Option<&Pk>) -> Self { + // With a raw pkh we don't know the preimage size so we have to assume the worst. + // FIXME with ValidationParams we will be able to determine if Ctx is Segwitv0 and exclude uncompressed keys. + let (key_bytes, max_sig_bytes) = match (Ctx::sig_type(), pk) { + (crate::SigType::Ecdsa, Some(pk)) if pk.is_uncompressed() => (65, 73), + (crate::SigType::Ecdsa, _) => (34, 73), + (crate::SigType::Schnorr, _) => (33, 66), + }; ExtData { pk_cost: 24, has_free_verify: false, ops: OpLimits::new(3, Some(0), Some(0)), stack_elem_count_sat: Some(2), stack_elem_count_dissat: Some(2), - max_sat_size: match Ctx::sig_type() { - SigType::Ecdsa => Some((34 + 73, 34 + 73)), - SigType::Schnorr => Some((66 + 33, 33 + 66)), - }, - max_dissat_size: match Ctx::sig_type() { - SigType::Ecdsa => Some((35, 35)), - SigType::Schnorr => Some((34, 34)), - }, + max_sat_size: Some((key_bytes + max_sig_bytes, key_bytes + max_sig_bytes)), + max_dissat_size: Some((key_bytes + 1, key_bytes + 1)), timelock_info: TimelockInfo::default(), exec_stack_elem_count_sat: Some(2), // dup and hash push exec_stack_elem_count_dissat: Some(2), @@ -932,8 +937,9 @@ impl ExtData { let ret = match *fragment { Terminal::True => Self::TRUE, Terminal::False => Self::FALSE, - Terminal::PkK(..) => Self::pk_k::(), - Terminal::PkH(..) | Terminal::RawPkH(..) => Self::pk_h::(), + Terminal::PkK(ref k) => Self::pk_k::<_, Ctx>(k), + Terminal::PkH(ref k) => Self::pk_h::<_, Ctx>(Some(k)), + Terminal::RawPkH(..) => Self::pk_h::(None), Terminal::Multi(ref thresh) => Self::multi(thresh.k(), thresh.n()), Terminal::MultiA(ref thresh) => Self::multi_a(thresh.k(), thresh.n()), Terminal::After(t) => Self::after(t), From 3250e40a5cb4461d74f1aad5f342458309506f43 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 20 May 2025 00:44:40 +0000 Subject: [PATCH 4/6] miniscript: take pubkey size into account in multi() type extdata As in the previous commit, we were previously assuming that all pubkeys were compressed. This isn't so. We have the keys, we can query them for whether they are uncompressed or not. --- src/miniscript/mod.rs | 2 +- src/miniscript/types/extra_props.rs | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index 5bc54cc2c..b941682a6 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -275,7 +275,7 @@ mod private { pub fn multi(thresh: crate::Threshold) -> Self { Self { ty: types::Type::multi(), - ext: types::extra_props::ExtData::multi(thresh.k(), thresh.n()), + ext: types::extra_props::ExtData::multi(&thresh), node: Terminal::Multi(thresh), phantom: PhantomData, } diff --git a/src/miniscript/types/extra_props.rs b/src/miniscript/types/extra_props.rs index 36bfc0f3a..696e46150 100644 --- a/src/miniscript/types/extra_props.rs +++ b/src/miniscript/types/extra_props.rs @@ -7,6 +7,7 @@ use core::cmp; use core::iter::once; use super::ScriptContext; +use crate::miniscript::limits::MAX_PUBKEYS_PER_MULTISIG; use crate::prelude::*; use crate::{script_num_size, AbsLockTime, MiniscriptKey, RelLockTime, Terminal}; @@ -242,7 +243,10 @@ impl ExtData { } /// Extra properties for the `multi` fragment. - pub fn multi(k: usize, n: usize) -> Self { + pub fn multi( + thresh: &crate::Threshold, + ) -> Self { + let (n, k) = (thresh.n(), thresh.k()); let num_cost = match (k > 16, n > 16) { (true, true) => 4, (false, true) => 3, @@ -250,7 +254,12 @@ impl ExtData { (false, false) => 2, }; ExtData { - pk_cost: num_cost + 34 * n + 1, + pk_cost: num_cost + + thresh + .iter() + .map(|k| if k.is_uncompressed() { 65 } else { 34 }) + .sum::() + + 1, has_free_verify: true, // Multi is the only case because of which we need to count additional // executed opcodes. @@ -940,7 +949,7 @@ impl ExtData { Terminal::PkK(ref k) => Self::pk_k::<_, Ctx>(k), Terminal::PkH(ref k) => Self::pk_h::<_, Ctx>(Some(k)), Terminal::RawPkH(..) => Self::pk_h::(None), - Terminal::Multi(ref thresh) => Self::multi(thresh.k(), thresh.n()), + Terminal::Multi(ref thresh) => Self::multi(thresh), Terminal::MultiA(ref thresh) => Self::multi_a(thresh.k(), thresh.n()), Terminal::After(t) => Self::after(t), Terminal::Older(t) => Self::older(t), From 276cf9492043f354f62002e8038319c1c1dca39f Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 20 May 2025 00:17:03 +0000 Subject: [PATCH 5/6] sortedmulti: simplify constructor and improve unit test Update the constructor to avoid calling Miniscript::from_ast, which can return many kinds of errors, none of which are reachable. Instead call Miniscript::multi and then do a validation check once this is constructed. There is a comment in the constructor explaining why the validation is needed: it may be that an otherwise-valid checkmultisig script could exceed the 520-byte limit for p2sh outputs. HOWEVER, we have no test for this behavior, and when trying to test it, I found several issues. One, that our accounting for uncompressed keys is wrong, I fixed in the previous commits. The others are: 1. In the existing unit test we try to create a sortedmulti with 21 keys, violating the constructor for Threshold. This is a fine test... 2. ...except that we set k == 0 which makes the constructor fail no matter what n is, so we're not actually testing "too many keys". 3. Furthermore, we have no test at all that tries to exceed the P2SH limits, and when I added one I was able to exceed these limits because of the type system bugs fixed in the last two commits. --- src/descriptor/sortedmulti.rs | 37 +++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/descriptor/sortedmulti.rs b/src/descriptor/sortedmulti.rs index ff3f62227..694a23fde 100644 --- a/src/descriptor/sortedmulti.rs +++ b/src/descriptor/sortedmulti.rs @@ -33,13 +33,10 @@ pub struct SortedMultiVec { impl SortedMultiVec { fn constructor_check(mut self) -> Result { + let ms = Miniscript::::multi(self.inner); // Check the limits before creating a new SortedMultiVec // For example, under p2sh context the scriptlen can only be // upto 520 bytes. - let term: Terminal = 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)?; if let Terminal::Multi(inner) = ms.node { self.inner = inner; @@ -229,31 +226,45 @@ impl fmt::Display for SortedMultiVec, Error> = SortedMultiVec::new(1, pks); + let error = res.expect_err("constructor should err"); - let mut pks = Vec::new(); - for _ in 0..over { - pks.push(pk); + match error { + Error::Threshold(_) => {} // ok + other => panic!("unexpected error: {:?}", other), } + } - let res: Result, Error> = SortedMultiVec::new(0, pks); + #[test] + fn too_many_pubkeys_for_p2sh() { + // Arbitrary 65-byte public key (66 with length prefix). + let pk = PublicKey::from_str( + "0400232a2acfc9b43fa89f1b4f608fde335d330d7114f70ea42bfb4a41db368a3e3be6934a4097dd25728438ef73debb1f2ffdb07fec0f18049df13bdc5285dc5b", + ) + .unwrap(); + + // This is legal for CHECKMULTISIG, but the 8 keys consume the whole 520 bytes + // allowed by P2SH, meaning that the full script goes over the limit. + let pks = vec![pk; 8]; + let res: Result, Error> = SortedMultiVec::new(2, pks); let error = res.expect_err("constructor should err"); match error { - Error::Threshold(_) => {} // ok + Error::ContextError(ScriptContextError::MaxRedeemScriptSizeExceeded { .. }) => {} // ok other => panic!("unexpected error: {:?}", other), } } From b5db340809c62d11dce26958bab51b57ae44408c Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 20 May 2025 01:55:43 +0000 Subject: [PATCH 6/6] sortedmulti: take Thresh as input to constructors We currently take a k value and a vector as input to the constructor to the SortedMultiVec type, and propagate this out to all the sortedmulti constructors. This means that we have to construct the threshold inside the method and then return errors outward. It's more flexible, though potentially a bit more annoying, to make the user construct the Threshold. Then they can deal with the error. This means that the descriptor constructors *only* return a validation error, which will let us tighten the Result type in a later commit. --- src/descriptor/mod.rs | 21 ++++++++++++++------- src/descriptor/segwitv0.rs | 9 +++++---- src/descriptor/sh.rs | 13 ++++++++----- src/descriptor/sortedmulti.rs | 27 ++++----------------------- 4 files changed, 31 insertions(+), 39 deletions(-) diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 9bc6f2c31..16e5b5ab6 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -23,12 +23,13 @@ use sync::Arc; use crate::expression::FromTree as _; use crate::miniscript::decode::Terminal; +use crate::miniscript::limits::MAX_PUBKEYS_PER_MULTISIG; use crate::miniscript::{satisfy, Legacy, Miniscript, Segwitv0}; use crate::plan::{AssetProvider, Plan}; use crate::prelude::*; use crate::{ expression, hash256, BareCtx, Error, ForEachKey, FromStrKey, MiniscriptKey, ParseError, - Satisfier, ToPublicKey, TranslateErr, Translator, + Satisfier, Threshold, ToPublicKey, TranslateErr, Translator, }; mod bare; @@ -223,21 +224,27 @@ impl Descriptor { /// Create a new sh sortedmulti descriptor with threshold `k` /// and Vec of `pks`. /// Errors when miniscript exceeds resource limits under p2sh context - pub fn new_sh_sortedmulti(k: usize, pks: Vec) -> Result { - Ok(Descriptor::Sh(Sh::new_sortedmulti(k, pks)?)) + pub fn new_sh_sortedmulti( + thresh: Threshold, + ) -> Result { + Ok(Descriptor::Sh(Sh::new_sortedmulti(thresh)?)) } /// Create a new sh wrapped wsh sortedmulti descriptor from threshold /// `k` and Vec of `pks` /// Errors when miniscript exceeds resource limits under segwit context - pub fn new_sh_wsh_sortedmulti(k: usize, pks: Vec) -> Result { - Ok(Descriptor::Sh(Sh::new_wsh_sortedmulti(k, pks)?)) + pub fn new_sh_wsh_sortedmulti( + thresh: Threshold, + ) -> Result { + Ok(Descriptor::Sh(Sh::new_wsh_sortedmulti(thresh)?)) } /// Create a new wsh sorted multi descriptor /// Errors when miniscript exceeds resource limits under p2sh context - pub fn new_wsh_sortedmulti(k: usize, pks: Vec) -> Result { - Ok(Descriptor::Wsh(Wsh::new_sortedmulti(k, pks)?)) + pub fn new_wsh_sortedmulti( + thresh: Threshold, + ) -> Result { + Ok(Descriptor::Wsh(Wsh::new_sortedmulti(thresh)?)) } /// Create new tr descriptor diff --git a/src/descriptor/segwitv0.rs b/src/descriptor/segwitv0.rs index 8ad173b76..a78867fba 100644 --- a/src/descriptor/segwitv0.rs +++ b/src/descriptor/segwitv0.rs @@ -14,14 +14,15 @@ use super::SortedMultiVec; use crate::descriptor::{write_descriptor, DefiniteDescriptorKey}; use crate::expression::{self, FromTree}; use crate::miniscript::context::{ScriptContext, ScriptContextError}; +use crate::miniscript::limits::MAX_PUBKEYS_PER_MULTISIG; use crate::miniscript::satisfy::{Placeholder, Satisfaction, Witness}; use crate::plan::AssetProvider; use crate::policy::{semantic, Liftable}; use crate::prelude::*; use crate::util::varint_len; use crate::{ - Error, ForEachKey, FromStrKey, Miniscript, MiniscriptKey, Satisfier, Segwitv0, ToPublicKey, - TranslateErr, Translator, + Error, ForEachKey, FromStrKey, Miniscript, MiniscriptKey, Satisfier, Segwitv0, Threshold, + ToPublicKey, TranslateErr, Translator, }; /// A Segwitv0 wsh descriptor #[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash)] @@ -45,10 +46,10 @@ impl Wsh { } /// Create a new sortedmulti wsh descriptor - pub fn new_sortedmulti(k: usize, pks: Vec) -> Result { + pub fn new_sortedmulti(thresh: Threshold) -> Result { // The context checks will be carried out inside new function for // sortedMultiVec - Ok(Self { inner: WshInner::SortedMulti(SortedMultiVec::new(k, pks)?) }) + Ok(Self { inner: WshInner::SortedMulti(SortedMultiVec::new(thresh)?) }) } /// Get the descriptor without the checksum diff --git a/src/descriptor/sh.rs b/src/descriptor/sh.rs index 8b0195e8f..4acb5fa73 100644 --- a/src/descriptor/sh.rs +++ b/src/descriptor/sh.rs @@ -17,6 +17,7 @@ use super::{SortedMultiVec, Wpkh, Wsh}; use crate::descriptor::{write_descriptor, DefiniteDescriptorKey}; use crate::expression::{self, FromTree}; use crate::miniscript::context::ScriptContext; +use crate::miniscript::limits::MAX_PUBKEYS_PER_MULTISIG; use crate::miniscript::satisfy::{Placeholder, Satisfaction}; use crate::plan::AssetProvider; use crate::policy::{semantic, Liftable}; @@ -24,7 +25,7 @@ use crate::prelude::*; use crate::util::{varint_len, witness_to_scriptsig}; use crate::{ push_opcode_size, Error, ForEachKey, FromStrKey, Legacy, Miniscript, MiniscriptKey, Satisfier, - Segwitv0, ToPublicKey, TranslateErr, Translator, + Segwitv0, Threshold, ToPublicKey, TranslateErr, Translator, }; /// A Legacy p2sh Descriptor @@ -125,10 +126,10 @@ impl Sh { /// Create a new p2sh sortedmulti descriptor with threshold `k` /// and Vec of `pks`. - pub fn new_sortedmulti(k: usize, pks: Vec) -> Result { + pub fn new_sortedmulti(thresh: Threshold) -> Result { // The context checks will be carried out inside new function for // sortedMultiVec - Ok(Self { inner: ShInner::SortedMulti(SortedMultiVec::new(k, pks)?) }) + Ok(Self { inner: ShInner::SortedMulti(SortedMultiVec::new(thresh)?) }) } /// Create a new p2sh wrapped wsh descriptor with the raw miniscript @@ -152,10 +153,12 @@ impl Sh { /// Create a new p2sh wrapped wsh sortedmulti descriptor from threshold /// `k` and Vec of `pks` - pub fn new_wsh_sortedmulti(k: usize, pks: Vec) -> Result { + pub fn new_wsh_sortedmulti( + thresh: Threshold, + ) -> Result { // The context checks will be carried out inside new function for // sortedMultiVec - Ok(Self { inner: ShInner::Wsh(Wsh::new_sortedmulti(k, pks)?) }) + Ok(Self { inner: ShInner::Wsh(Wsh::new_sortedmulti(thresh)?) }) } /// Create a new p2sh wrapped wpkh from `Pk` diff --git a/src/descriptor/sortedmulti.rs b/src/descriptor/sortedmulti.rs index 694a23fde..5ba952ff0 100644 --- a/src/descriptor/sortedmulti.rs +++ b/src/descriptor/sortedmulti.rs @@ -49,9 +49,8 @@ impl SortedMultiVec { /// Create a new instance of `SortedMultiVec` given a list of keys and the threshold /// /// Internally checks all the applicable size limits and pubkey types limitations according to the current `Ctx`. - pub fn new(k: usize, pks: Vec) -> Result { - let ret = - Self { inner: Threshold::new(k, pks).map_err(Error::Threshold)?, phantom: PhantomData }; + pub fn new(thresh: Threshold) -> Result { + let ret = Self { inner: thresh, phantom: PhantomData }; ret.constructor_check() } @@ -231,24 +230,6 @@ mod tests { use super::*; use crate::miniscript::context::{Legacy, ScriptContextError}; - #[test] - fn too_many_pubkeys() { - // Arbitrary 33-byte public key (34 with length prefix). - let pk = PublicKey::from_str( - "02e6642fd69bd211f93f7f1f36ca51a26a5290eb2dd1b0d8279a87bb0d480c8443", - ) - .unwrap(); - - let pks = vec![pk; 1 + MAX_PUBKEYS_PER_MULTISIG]; - let res: Result, Error> = SortedMultiVec::new(1, pks); - let error = res.expect_err("constructor should err"); - - match error { - Error::Threshold(_) => {} // ok - other => panic!("unexpected error: {:?}", other), - } - } - #[test] fn too_many_pubkeys_for_p2sh() { // Arbitrary 65-byte public key (66 with length prefix). @@ -259,8 +240,8 @@ mod tests { // This is legal for CHECKMULTISIG, but the 8 keys consume the whole 520 bytes // allowed by P2SH, meaning that the full script goes over the limit. - let pks = vec![pk; 8]; - let res: Result, Error> = SortedMultiVec::new(2, pks); + let thresh = Threshold::new(2, vec![pk; 8]).expect("the thresh is ok.."); + let res: Result, Error> = SortedMultiVec::new(thresh); let error = res.expect_err("constructor should err"); match error {