Skip to content

Commit ddc42c7

Browse files
apoelstraapoelstra
committed
Merge #915: refactor: remove SortedMultiVec and use Terminal::SortedMulti
01d25c1 refactor: remove SortedMultiVec and use Terminal::SortedMulti (Trevor Arjeski) Pull request description: - Utilize the Miniscript parsing to handle sortedmulti as a Terminal. - Deleted sortedmulti.rs (SortedMultiVec) - Refactor Wsh to only wrap a Miniscript now that SortedMultiVec isn't used. - Refactor ShInner to remove SortedMulti variant and only use the Ms variant for sortedmulti scripts --- Now that sortedmulti_a is supported as a Terminal, I think it makes sense to move sortedmulti over in the same way by following what multi does and applying the sorting functions that were introduced in eba1aff. Like sortedmulti_a, sortedmulti is sorted upon encoding and cannot be decoded into from Script. ACKs for top commit: apoelstra: ACK 01d25c1; successfully ran local tests Tree-SHA512: 17578f2e08c803c0c3930ee74cdbd0cfe22156c03edfdda4a9a44eefe752b1b3f8b7de9ddb2d7e63b2e090fe45c874e6c293297c05d0fe3cebc59675671e0b1d
2 parents 4109aaf + 01d25c1 commit ddc42c7

24 files changed

Lines changed: 209 additions & 484 deletions

File tree

‎bitcoind-tests/tests/test_cpp.rs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,7 @@ pub fn test_from_cpp_ms(cl: &Client, testdata: &TestData) {
147147
for i in 0..psbts.len() {
148148
let wsh_derived = desc_vec[i].derived_descriptor(&secp);
149149
let ms = if let Descriptor::Wsh(wsh) = &wsh_derived {
150-
match wsh.as_inner() {
151-
miniscript::descriptor::WshInner::Ms(ms) => ms,
152-
_ => unreachable!(),
153-
}
150+
wsh.as_inner()
154151
} else {
155152
unreachable!("Only Wsh descriptors are supported");
156153
};

‎bitcoind-tests/tests/test_desc.rs‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -225,29 +225,15 @@ pub fn test_desc_satisfy(
225225
Descriptor::Pkh(pk) => find_sk_single_key(*pk.as_inner(), testdata),
226226
Descriptor::Wpkh(pk) => find_sk_single_key(*pk.as_inner(), testdata),
227227
Descriptor::Sh(sh) => match sh.as_inner() {
228-
miniscript::descriptor::ShInner::Wsh(wsh) => match wsh.as_inner() {
229-
miniscript::descriptor::WshInner::SortedMulti(ref smv) => {
230-
let ms = Miniscript::from_ast(smv.sorted_node()).unwrap();
231-
find_sks_ms(&ms, testdata)
232-
}
233-
miniscript::descriptor::WshInner::Ms(ref ms) => find_sks_ms(ms, testdata),
234-
},
228+
miniscript::descriptor::ShInner::Wsh(wsh) => {
229+
find_sks_ms(wsh.as_inner(), testdata)
230+
}
235231
miniscript::descriptor::ShInner::Wpkh(pk) => {
236232
find_sk_single_key(*pk.as_inner(), testdata)
237233
}
238-
miniscript::descriptor::ShInner::SortedMulti(smv) => {
239-
let ms = Miniscript::from_ast(smv.sorted_node()).unwrap();
240-
find_sks_ms(&ms, testdata)
241-
}
242234
miniscript::descriptor::ShInner::Ms(ms) => find_sks_ms(ms, testdata),
243235
},
244-
Descriptor::Wsh(wsh) => match wsh.as_inner() {
245-
miniscript::descriptor::WshInner::SortedMulti(ref smv) => {
246-
let ms = Miniscript::from_ast(smv.sorted_node()).unwrap();
247-
find_sks_ms(&ms, testdata)
248-
}
249-
miniscript::descriptor::WshInner::Ms(ref ms) => find_sks_ms(ms, testdata),
250-
},
236+
Descriptor::Wsh(wsh) => find_sks_ms(wsh.as_inner(), testdata),
251237
Descriptor::Tr(_tr) => unreachable!("Tr checked earlier"),
252238
};
253239
let msg = psbt

‎src/descriptor/iter.rs‎

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ pub struct PkIter<'desc, Pk: MiniscriptKey> {
1414
ms_iter_legacy: Option<miniscript::iter::PkIter<'desc, Pk, Legacy>>,
1515
ms_iter_segwit: Option<miniscript::iter::PkIter<'desc, Pk, Segwitv0>>,
1616
ms_iter_taproot: Option<miniscript::iter::PkIter<'desc, Pk, Tap>>,
17-
sorted_multi: Option<core::slice::Iter<'desc, Pk>>,
1817
}
1918

2019
impl<'desc, Pk: MiniscriptKey> PkIter<'desc, Pk> {
@@ -26,7 +25,6 @@ impl<'desc, Pk: MiniscriptKey> PkIter<'desc, Pk> {
2625
ms_iter_legacy: None,
2726
ms_iter_segwit: None,
2827
ms_iter_taproot: None,
29-
sorted_multi: None,
3028
}
3129
}
3230

@@ -38,7 +36,6 @@ impl<'desc, Pk: MiniscriptKey> PkIter<'desc, Pk> {
3836
ms_iter_legacy: None,
3937
ms_iter_segwit: None,
4038
ms_iter_taproot: None,
41-
sorted_multi: None,
4239
}
4340
}
4441

@@ -50,7 +47,6 @@ impl<'desc, Pk: MiniscriptKey> PkIter<'desc, Pk> {
5047
ms_iter_legacy: Some(ms.iter_pk()),
5148
ms_iter_segwit: None,
5249
ms_iter_taproot: None,
53-
sorted_multi: None,
5450
}
5551
}
5652

@@ -62,19 +58,6 @@ impl<'desc, Pk: MiniscriptKey> PkIter<'desc, Pk> {
6258
ms_iter_legacy: None,
6359
ms_iter_segwit: Some(ms.iter_pk()),
6460
ms_iter_taproot: None,
65-
sorted_multi: None,
66-
}
67-
}
68-
69-
pub(super) fn from_sortedmulti(sm: &'desc [Pk]) -> Self {
70-
Self {
71-
single_key: None,
72-
taptree_iter: None,
73-
ms_iter_bare: None,
74-
ms_iter_legacy: None,
75-
ms_iter_segwit: None,
76-
ms_iter_taproot: None,
77-
sorted_multi: Some(sm.iter()),
7861
}
7962
}
8063

@@ -86,7 +69,6 @@ impl<'desc, Pk: MiniscriptKey> PkIter<'desc, Pk> {
8669
ms_iter_legacy: None,
8770
ms_iter_segwit: None,
8871
ms_iter_taproot: None,
89-
sorted_multi: None,
9072
}
9173
}
9274
}
@@ -118,9 +100,7 @@ impl<'desc, Pk: MiniscriptKey> Iterator for PkIter<'desc, Pk> {
118100
// Finally run through the train of other iterators.
119101
self.ms_iter_bare.as_mut().and_then(Iterator::next).or_else(
120102
|| self.ms_iter_legacy.as_mut().and_then(Iterator::next).or_else(
121-
|| self.ms_iter_segwit.as_mut().and_then(Iterator::next).or_else(
122-
|| self.sorted_multi.as_mut().and_then(Iterator::next).cloned()
123-
)
103+
|| self.ms_iter_segwit.as_mut().and_then(Iterator::next)
124104
)
125105
)
126106
}

‎src/descriptor/mod.rs‎

Lines changed: 48 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,13 @@ mod bare;
3636
mod iter;
3737
mod segwitv0;
3838
mod sh;
39-
mod sortedmulti;
4039
mod tr;
4140

4241
// Descriptor Exports
4342
pub use self::bare::{Bare, Pkh};
4443
pub use self::iter::PkIter;
45-
pub use self::segwitv0::{Wpkh, Wsh, WshInner};
44+
pub use self::segwitv0::{Wpkh, Wsh};
4645
pub use self::sh::{Sh, ShInner};
47-
pub use self::sortedmulti::SortedMultiVec;
4846
pub use self::tr::{
4947
TapTree, TapTreeDepthError, TapTreeIter, TapTreeIterItem, Tr, TrSpendInfo, TrSpendInfoIter,
5048
TrSpendInfoIterItem,
@@ -256,18 +254,11 @@ impl<Pk: MiniscriptKey> Descriptor<Pk> {
256254
Descriptor::Pkh(ref pk) => PkIter::from_key(pk.as_inner().clone()),
257255
Descriptor::Wpkh(ref pk) => PkIter::from_key(pk.as_inner().clone()),
258256
Descriptor::Sh(ref sh) => match *sh.as_inner() {
259-
ShInner::Wsh(ref wsh) => match wsh.as_inner() {
260-
WshInner::SortedMulti(ref sorted) => PkIter::from_sortedmulti(sorted.pks()),
261-
WshInner::Ms(ref ms) => PkIter::from_miniscript_segwit(ms),
262-
},
257+
ShInner::Wsh(ref wsh) => PkIter::from_miniscript_segwit(wsh.as_inner()),
263258
ShInner::Wpkh(ref pk) => PkIter::from_key(pk.as_inner().clone()),
264-
ShInner::SortedMulti(ref sorted) => PkIter::from_sortedmulti(sorted.pks()),
265259
ShInner::Ms(ref ms) => PkIter::from_miniscript_legacy(ms),
266260
},
267-
Descriptor::Wsh(ref wsh) => match wsh.as_inner() {
268-
WshInner::SortedMulti(ref sorted) => PkIter::from_sortedmulti(sorted.pks()),
269-
WshInner::Ms(ref ms) => PkIter::from_miniscript_segwit(ms),
270-
},
261+
Descriptor::Wsh(ref wsh) => PkIter::from_miniscript_segwit(wsh.as_inner()),
271262
Descriptor::Tr(ref tr) => PkIter::from_tr(tr),
272263
}
273264
}
@@ -313,18 +304,29 @@ impl<Pk: MiniscriptKey> Descriptor<Pk> {
313304
Descriptor::Pkh(ref _pkh) => DescriptorType::Pkh,
314305
Descriptor::Wpkh(ref _wpkh) => DescriptorType::Wpkh,
315306
Descriptor::Sh(ref sh) => match sh.as_inner() {
316-
ShInner::Wsh(ref wsh) => match wsh.as_inner() {
317-
WshInner::SortedMulti(ref _smv) => DescriptorType::ShWshSortedMulti,
318-
WshInner::Ms(ref _ms) => DescriptorType::ShWsh,
319-
},
307+
ShInner::Wsh(ref wsh) => {
308+
if let Terminal::SortedMulti(..) = wsh.as_inner().node {
309+
DescriptorType::ShWshSortedMulti
310+
} else {
311+
DescriptorType::ShWsh
312+
}
313+
}
320314
ShInner::Wpkh(ref _wpkh) => DescriptorType::ShWpkh,
321-
ShInner::SortedMulti(ref _smv) => DescriptorType::ShSortedMulti,
322-
ShInner::Ms(ref _ms) => DescriptorType::Sh,
323-
},
324-
Descriptor::Wsh(ref wsh) => match wsh.as_inner() {
325-
WshInner::SortedMulti(ref _smv) => DescriptorType::WshSortedMulti,
326-
WshInner::Ms(ref _ms) => DescriptorType::Wsh,
315+
ShInner::Ms(ref ms) => {
316+
if let Terminal::SortedMulti(..) = ms.node {
317+
DescriptorType::ShSortedMulti
318+
} else {
319+
DescriptorType::Sh
320+
}
321+
}
327322
},
323+
Descriptor::Wsh(ref wsh) => {
324+
if let Terminal::SortedMulti(..) = wsh.as_inner().node {
325+
DescriptorType::WshSortedMulti
326+
} else {
327+
DescriptorType::Wsh
328+
}
329+
}
328330
Descriptor::Tr(ref _tr) => DescriptorType::Tr,
329331
}
330332
}
@@ -1208,6 +1210,7 @@ mod tests {
12081210

12091211
use super::{checksum, *};
12101212
use crate::hex_script;
1213+
use crate::miniscript::context::ScriptContextError;
12111214
#[cfg(feature = "compiler")]
12121215
use crate::policy;
12131216

@@ -1257,7 +1260,7 @@ mod tests {
12571260
StdDescriptor::from_str("sh(sortedmulti)")
12581261
.unwrap_err()
12591262
.to_string(),
1260-
"sortedmulti must have at least 1 children, but found 0"
1263+
"expected threshold, found terminal"
12611264
); //issue 202
12621265
assert_eq!(
12631266
StdDescriptor::from_str(&format!("sh(sortedmulti(2,{}))", &TEST_PK[3..69]))
@@ -2884,4 +2887,26 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
28842887
let definite: Result<Descriptor<DefiniteDescriptorKey>, _> = desc.try_into();
28852888
assert!(matches!(definite, Err(NonDefiniteKeyError::Wildcard)));
28862889
}
2890+
2891+
#[test]
2892+
fn too_many_pubkeys_for_p2sh() {
2893+
// Arbitrary 65-byte public key (66 with length prefix).
2894+
let pk = PublicKey::from_str(
2895+
"0400232a2acfc9b43fa89f1b4f608fde335d330d7114f70ea42bfb4a41db368a3e3be6934a4097dd25728438ef73debb1f2ffdb07fec0f18049df13bdc5285dc5b",
2896+
)
2897+
.unwrap();
2898+
2899+
// This is legal for CHECKMULTISIG, but the 8 keys consume the whole 520 bytes
2900+
// allowed by P2SH, meaning that the full script goes over the limit.
2901+
let thresh = Threshold::new(2, vec![pk; 8]).expect("the thresh is ok..");
2902+
let script = Miniscript::<_, Legacy>::sortedmulti(thresh).encode();
2903+
let res = Miniscript::<_, Legacy>::decode(&script);
2904+
2905+
let error = res.expect_err("decoding should err");
2906+
2907+
match error {
2908+
Error::ContextError(ScriptContextError::MaxRedeemScriptSizeExceeded { .. }) => {} // ok
2909+
other => panic!("unexpected error: {:?}", other),
2910+
}
2911+
}
28872912
}

0 commit comments

Comments
 (0)