Skip to content

Commit 9b0d761

Browse files
darosiordarosior
committed
desc: check for mismatch in the number of derivation paths between multipath keys
This is invalid by BIP389 (as combinations would blowup when getting single descriptors otherwise). We add a num_der_paths method to MiniscriptKey in order to be able to check for this at parsing time without specializing from_str for Descriptor<DescriptorPublicKey>.
1 parent fa6c199 commit 9b0d761

File tree

4 files changed

+101
-8
lines changed

4 files changed

+101
-8
lines changed

‎src/descriptor/key.rs

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -976,6 +976,14 @@ impl MiniscriptKey for DescriptorPublicKey {
976976
_ => false,
977977
}
978978
}
979+
980+
fn num_der_paths(&self) -> usize {
981+
match self {
982+
DescriptorPublicKey::Single(_) => 0,
983+
DescriptorPublicKey::XPub(_) => 1,
984+
DescriptorPublicKey::MultiXPub(xpub) => xpub.derivation_paths.len(),
985+
}
986+
}
979987
}
980988

981989
impl DefiniteDescriptorKey {
@@ -1069,6 +1077,10 @@ impl MiniscriptKey for DefiniteDescriptorKey {
10691077
fn is_x_only_key(&self) -> bool {
10701078
self.0.is_x_only_key()
10711079
}
1080+
1081+
fn num_der_paths(&self) -> usize {
1082+
self.0.num_der_paths()
1083+
}
10721084
}
10731085

10741086
impl ToPublicKey for DefiniteDescriptorKey {
@@ -1109,7 +1121,7 @@ mod test {
11091121

11101122
use super::{
11111123
DescriptorKeyParseError, DescriptorMultiXKey, DescriptorPublicKey, DescriptorSecretKey,
1112-
Wildcard,
1124+
MiniscriptKey, Wildcard,
11131125
};
11141126
use crate::prelude::*;
11151127

@@ -1296,8 +1308,12 @@ mod test {
12961308
);
12971309
}
12981310

1299-
fn get_multipath_xpub(key_str: &str) -> DescriptorMultiXKey<bip32::ExtendedPubKey> {
1311+
fn get_multipath_xpub(
1312+
key_str: &str,
1313+
num_paths: usize,
1314+
) -> DescriptorMultiXKey<bip32::ExtendedPubKey> {
13001315
let desc_key = DescriptorPublicKey::from_str(key_str).unwrap();
1316+
assert_eq!(desc_key.num_der_paths(), num_paths);
13011317
match desc_key {
13021318
DescriptorPublicKey::MultiXPub(xpub) => xpub,
13031319
_ => unreachable!(),
@@ -1317,7 +1333,7 @@ mod test {
13171333
let secp = secp256k1::Secp256k1::signing_only();
13181334

13191335
// We can have a key in a descriptor that has multiple paths
1320-
let xpub = get_multipath_xpub("tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/2/<0;1;42;9854>");
1336+
let xpub = get_multipath_xpub("tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/2/<0;1;42;9854>", 4);
13211337
assert_eq!(
13221338
xpub.derivation_paths,
13231339
vec![
@@ -1328,7 +1344,7 @@ mod test {
13281344
],
13291345
);
13301346
// Even if it's in the middle of the derivation path.
1331-
let xpub = get_multipath_xpub("tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/2/<0;1;9854>/0/5/10");
1347+
let xpub = get_multipath_xpub("tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/2/<0;1;9854>/0/5/10", 3);
13321348
assert_eq!(
13331349
xpub.derivation_paths,
13341350
vec![
@@ -1338,7 +1354,7 @@ mod test {
13381354
],
13391355
);
13401356
// Even if it is a wildcard extended key.
1341-
let xpub = get_multipath_xpub("tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/2/<0;1;9854>/3456/9876/*");
1357+
let xpub = get_multipath_xpub("tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/2/<0;1;9854>/3456/9876/*", 3);
13421358
assert_eq!(xpub.wildcard, Wildcard::Unhardened);
13431359
assert_eq!(
13441360
xpub.derivation_paths,
@@ -1349,7 +1365,7 @@ mod test {
13491365
],
13501366
);
13511367
// Also even if it has an origin.
1352-
let xpub = get_multipath_xpub("[abcdef00/0'/1']tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/<0;1>/*");
1368+
let xpub = get_multipath_xpub("[abcdef00/0'/1']tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/<0;1>/*", 2);
13531369
assert_eq!(xpub.wildcard, Wildcard::Unhardened);
13541370
assert_eq!(
13551371
xpub.derivation_paths,
@@ -1360,7 +1376,7 @@ mod test {
13601376
);
13611377
// Also if it has hardened steps in the derivation path. In fact, it can also have hardened
13621378
// indexes even at the step with multiple indexes!
1363-
let xpub = get_multipath_xpub("[abcdef00/0'/1']tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/9478'/<0';1h>/8h/*'");
1379+
let xpub = get_multipath_xpub("[abcdef00/0'/1']tpubDBrgjcxBxnXyL575sHdkpKohWu5qHKoQ7TJXKNrYznh5fVEGBv89hA8ENW7A8MFVpFUSvgLqc4Nj1WZcpePX6rrxviVtPowvMuGF5rdT2Vi/9478'/<0';1h>/8h/*'", 2);
13641380
assert_eq!(xpub.wildcard, Wildcard::Hardened);
13651381
assert_eq!(
13661382
xpub.derivation_paths,

‎src/descriptor/mod.rs

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -791,6 +791,42 @@ impl Descriptor<DescriptorPublicKey> {
791791
}
792792
}
793793

794+
impl<Pk: MiniscriptKey> Descriptor<Pk> {
795+
/// Whether this descriptor is a multipath descriptor that contains any 2 multipath keys
796+
/// with a different number of derivation paths.
797+
/// Such a descriptor is invalid according to BIP389.
798+
pub fn multipath_length_mismatch(&self) -> bool {
799+
// (Ab)use `for_each_key` to record the number of derivation paths a multipath key has.
800+
#[derive(PartialEq)]
801+
enum MultipathLenChecker {
802+
SinglePath,
803+
MultipathLen(usize),
804+
LenMismatch,
805+
}
806+
807+
let mut checker = MultipathLenChecker::SinglePath;
808+
self.for_each_key(|key| {
809+
match key.num_der_paths() {
810+
0 | 1 => {}
811+
n => match checker {
812+
MultipathLenChecker::SinglePath => {
813+
checker = MultipathLenChecker::MultipathLen(n);
814+
}
815+
MultipathLenChecker::MultipathLen(len) => {
816+
if len != n {
817+
checker = MultipathLenChecker::LenMismatch;
818+
}
819+
}
820+
MultipathLenChecker::LenMismatch => {}
821+
},
822+
}
823+
true
824+
});
825+
826+
checker == MultipathLenChecker::LenMismatch
827+
}
828+
}
829+
794830
impl Descriptor<DefiniteDescriptorKey> {
795831
/// Convert all the public keys in the descriptor to [`bitcoin::PublicKey`] by deriving them or
796832
/// otherwise converting them. All [`bitcoin::XOnlyPublicKey`]s are converted to by adding a
@@ -861,13 +897,19 @@ impl_from_str!(
861897
// tr tree parsing has special code
862898
// Tr::from_str will check the checksum
863899
// match "tr(" to handle more extensibly
864-
if s.starts_with("tr(") {
900+
let desc = if s.starts_with("tr(") {
865901
Ok(Descriptor::Tr(Tr::from_str(s)?))
866902
} else {
867903
let desc_str = verify_checksum(s)?;
868904
let top = expression::Tree::from_str(desc_str)?;
869905
expression::FromTree::from_tree(&top)
906+
}?;
907+
908+
if desc.multipath_length_mismatch() {
909+
return Err(Error::MultipathDescLenMismatch);
870910
}
911+
912+
Ok(desc)
871913
}
872914
);
873915

@@ -1911,6 +1953,7 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
19111953
// We can parse a multipath descriptors, and make it into separate single-path descriptors.
19121954
let desc = Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/<7';8h;20>/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/<0;1;987>/*)))").unwrap();
19131955
assert!(desc.is_multipath());
1956+
assert!(!desc.multipath_length_mismatch());
19141957
assert_eq!(desc.into_single_descriptors().unwrap(), vec![
19151958
Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/7'/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/0/*)))").unwrap(),
19161959
Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/8h/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/1/*)))").unwrap(),
@@ -1920,6 +1963,7 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
19201963
// Even if only one of the keys is multipath.
19211964
let desc = Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/<0;1>/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/*)))").unwrap();
19221965
assert!(desc.is_multipath());
1966+
assert!(!desc.multipath_length_mismatch());
19231967
assert_eq!(desc.into_single_descriptors().unwrap(), vec![
19241968
Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/0/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/*)))").unwrap(),
19251969
Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/1/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/*)))").unwrap(),
@@ -1928,9 +1972,14 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
19281972
// We can detect regular single-path descriptors.
19291973
let notmulti_desc = Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/*)))").unwrap();
19301974
assert!(!notmulti_desc.is_multipath());
1975+
assert!(!notmulti_desc.multipath_length_mismatch());
19311976
assert_eq!(
19321977
notmulti_desc.clone().into_single_descriptors().unwrap(),
19331978
vec![notmulti_desc]
19341979
);
1980+
1981+
// We refuse to parse multipath descriptors with a mismatch in the number of derivation paths between keys.
1982+
Descriptor::<DescriptorPublicKey>::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/<0;1>/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/<0;1;2;3;4>/*)))").unwrap_err();
1983+
Descriptor::<DescriptorPublicKey>::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/<0;1;2;3>/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/<0;1;2>/*)))").unwrap_err();
19351984
}
19361985
}

‎src/interpreter/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,10 @@ impl MiniscriptKey for BitcoinKey {
138138
type Hash256 = hash256::Hash;
139139
type Ripemd160 = ripemd160::Hash;
140140
type Hash160 = hash160::Hash;
141+
142+
fn num_der_paths(&self) -> usize {
143+
0
144+
}
141145
}
142146

143147
impl<'txin> Interpreter<'txin> {

‎src/lib.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,10 @@ pub trait MiniscriptKey: Clone + Eq + Ord + fmt::Debug + fmt::Display + hash::Ha
162162
false
163163
}
164164

165+
/// Returns the number of different derivation paths in this key. Only >1 for keys
166+
/// in BIP389 multipath descriptors.
167+
fn num_der_paths(&self) -> usize;
168+
165169
/// The associated [`sha256::Hash`] for this [`MiniscriptKey`],
166170
/// used in the hash256 fragment.
167171
type Sha256: Clone + Eq + Ord + fmt::Display + fmt::Debug + hash::Hash;
@@ -183,6 +187,10 @@ impl MiniscriptKey for bitcoin::secp256k1::PublicKey {
183187
type Hash256 = hash256::Hash;
184188
type Ripemd160 = ripemd160::Hash;
185189
type Hash160 = hash160::Hash;
190+
191+
fn num_der_paths(&self) -> usize {
192+
0
193+
}
186194
}
187195

188196
impl MiniscriptKey for bitcoin::PublicKey {
@@ -191,6 +199,10 @@ impl MiniscriptKey for bitcoin::PublicKey {
191199
!self.compressed
192200
}
193201

202+
fn num_der_paths(&self) -> usize {
203+
0
204+
}
205+
194206
type Sha256 = sha256::Hash;
195207
type Hash256 = hash256::Hash;
196208
type Ripemd160 = ripemd160::Hash;
@@ -206,13 +218,21 @@ impl MiniscriptKey for bitcoin::secp256k1::XOnlyPublicKey {
206218
fn is_x_only_key(&self) -> bool {
207219
true
208220
}
221+
222+
fn num_der_paths(&self) -> usize {
223+
0
224+
}
209225
}
210226

211227
impl MiniscriptKey for String {
212228
type Sha256 = String; // specify hashes as string
213229
type Hash256 = String;
214230
type Ripemd160 = String;
215231
type Hash160 = String;
232+
233+
fn num_der_paths(&self) -> usize {
234+
0
235+
}
216236
}
217237

218238
/// Trait describing public key types which can be converted to bitcoin pubkeys
@@ -345,6 +365,10 @@ impl MiniscriptKey for DummyKey {
345365
type Hash256 = DummyHash256Hash;
346366
type Ripemd160 = DummyRipemd160Hash;
347367
type Hash160 = DummyHash160Hash;
368+
369+
fn num_der_paths(&self) -> usize {
370+
0
371+
}
348372
}
349373

350374
impl hash::Hash for DummyKey {

0 commit comments

Comments
 (0)