frost/bdk
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove redundant duplicated keys check

Browse Source 
This check is redundant since it's already performed by miniscript (see
https://docs.rs/miniscript/7.0.0/miniscript/miniscript/analyzable/enum.AnalysisError.html#variant.RepeatedPubkeys)
and it was incorrectly failing on tr descriptors that contain duplicated
keys across different taproot leaves

Fixes #760
This commit is contained in:
Alekos Filini 2022-09-24 13:48:05 +02:00
parent c3faf05be9
commit e2bf9734b1
No known key found for this signature in database
GPG Key ID: 431401E4A4530061
3 changed files with 24 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/descriptor/error.rs
View File

@ -20,8 +20,6 @@ pub enum Error {
InvalidDescriptorChecksum, InvalidDescriptorChecksum,
/// The descriptor contains hardened derivation steps on public extended keys /// The descriptor contains hardened derivation steps on public extended keys
HardenedDerivationXpub, HardenedDerivationXpub,
/// The descriptor contains multiple keys with the same BIP32 fingerprint
DuplicatedKeys,
/// Error thrown while working with [`keys`](crate::keys) /// Error thrown while working with [`keys`](crate::keys)
Key(crate::keys::KeyError), Key(crate::keys::KeyError),

28
src/descriptor/mod.rs
View File

@ -14,7 +14,7 @@
//! This module contains generic utilities to work with descriptors, plus some re-exported types //! This module contains generic utilities to work with descriptors, plus some re-exported types
//! from [`miniscript`]. //! from [`miniscript`].
use std::collections::{BTreeMap, HashSet}; use std::collections::BTreeMap;
use std::ops::Deref; use std::ops::Deref;
use bitcoin::util::bip32::{ChildNumber, DerivationPath, ExtendedPubKey, Fingerprint, KeySource}; use bitcoin::util::bip32::{ChildNumber, DerivationPath, ExtendedPubKey, Fingerprint, KeySource};
@ -222,23 +222,9 @@ pub(crate) fn into_wallet_descriptor_checked<T: IntoWalletDescriptor>(
return Err(DescriptorError::HardenedDerivationXpub); return Err(DescriptorError::HardenedDerivationXpub);
} }
// Ensure that there are no duplicated keys // Run miniscript's sanity check, which will look for duplicated keys and other potential
let mut found_keys = HashSet::new(); // issues
let descriptor_contains_duplicated_keys = descriptor.for_any_key(|k| { descriptor.sanity_check()?;
if let DescriptorPublicKey::XPub(xkey) = k.as_key() {
let fingerprint = xkey.root_fingerprint(secp);
if found_keys.contains(&fingerprint) {
return true;
}
found_keys.insert(fingerprint);
}
false
});
if descriptor_contains_duplicated_keys {
return Err(DescriptorError::DuplicatedKeys);
}
Ok((descriptor, keymap)) Ok((descriptor, keymap))
} }
@ -923,14 +909,10 @@ mod test {
DescriptorError::HardenedDerivationXpub DescriptorError::HardenedDerivationXpub
)); ));
let descriptor = "wsh(multi(2,tpubD6NzVbkrYhZ4XHndKkuB8FifXm8r5FQHwrN6oZuWCz13qb93rtgKvD4PQsqC4HP4yhV3tA2fqr2RbY5mNXfM7RxXUoeABoDtsFUq2zJq6YK/0/*,tpubD6NzVbkrYhZ4XHndKkuB8FifXm8r5FQHwrN6oZuWCz13qb93rtgKvD4PQsqC4HP4yhV3tA2fqr2RbY5mNXfM7RxXUoeABoDtsFUq2zJq6YK/1/*))"; let descriptor = "wsh(multi(2,tpubD6NzVbkrYhZ4XHndKkuB8FifXm8r5FQHwrN6oZuWCz13qb93rtgKvD4PQsqC4HP4yhV3tA2fqr2RbY5mNXfM7RxXUoeABoDtsFUq2zJq6YK/0/*,tpubD6NzVbkrYhZ4XHndKkuB8FifXm8r5FQHwrN6oZuWCz13qb93rtgKvD4PQsqC4HP4yhV3tA2fqr2RbY5mNXfM7RxXUoeABoDtsFUq2zJq6YK/0/*))";
let result = into_wallet_descriptor_checked(descriptor, &secp, Network::Testnet); let result = into_wallet_descriptor_checked(descriptor, &secp, Network::Testnet);
assert!(result.is_err()); assert!(result.is_err());
assert!(matches!(
result.unwrap_err(),
DescriptorError::DuplicatedKeys
));
} }
#[test] #[test]

19
src/wallet/mod.rs
View File

@ -2077,6 +2077,10 @@ pub(crate) mod test {
"tr(cNJmN3fH9DDbDt131fQNkVakkpzawJBSeybCUNmP1BovpmGQ45xG,{pk(tprv8ZgxMBicQKsPdDArR4xSAECuVxeX1jwwSXR4ApKbkYgZiziDc4LdBy2WvJeGDfUSE4UT4hHhbgEwbdq8ajjUHiKDegkwrNU6V55CxcxonVN/*),pk(8aee2b8120a5f157f1223f72b5e62b825831a27a9fdf427db7cc697494d4a642)})" "tr(cNJmN3fH9DDbDt131fQNkVakkpzawJBSeybCUNmP1BovpmGQ45xG,{pk(tprv8ZgxMBicQKsPdDArR4xSAECuVxeX1jwwSXR4ApKbkYgZiziDc4LdBy2WvJeGDfUSE4UT4hHhbgEwbdq8ajjUHiKDegkwrNU6V55CxcxonVN/*),pk(8aee2b8120a5f157f1223f72b5e62b825831a27a9fdf427db7cc697494d4a642)})"
} }
pub(crate) fn get_test_tr_dup_keys() -> &'static str {
"tr(cNJmN3fH9DDbDt131fQNkVakkpzawJBSeybCUNmP1BovpmGQ45xG,{pk(8aee2b8120a5f157f1223f72b5e62b825831a27a9fdf427db7cc697494d4a642),pk(8aee2b8120a5f157f1223f72b5e62b825831a27a9fdf427db7cc697494d4a642)})"
}
macro_rules! assert_fee_rate { macro_rules! assert_fee_rate {
($psbt:expr, $fees:expr, $fee_rate:expr $( ,@dust_change $( $dust_change:expr )* )* $( ,@add_signature $( $add_signature:expr )* )* ) => ({ ($psbt:expr, $fees:expr, $fee_rate:expr $( ,@dust_change $( $dust_change:expr )* )* $( ,@add_signature $( $add_signature:expr )* )* ) => ({
let psbt = $psbt.clone(); let psbt = $psbt.clone();
@ -5554,4 +5558,19 @@ pub(crate) mod test {
let finalized = wallet.sign(&mut psbt, Default::default()).unwrap(); let finalized = wallet.sign(&mut psbt, Default::default()).unwrap();
assert!(finalized); assert!(finalized);
} }
#[test]
fn test_taproot_load_descriptor_duplicated_keys() {
// Added after issue https://github.com/bitcoindevkit/bdk/issues/760
//
// Having the same key in multiple taproot leaves is safe and should be accepted by BDK
let (wallet, _, _) = get_funded_wallet(get_test_tr_dup_keys());
let addr = wallet.get_address(New).unwrap();
assert_eq!(
addr.to_string(),
"bcrt1pvysh4nmh85ysrkpwtrr8q8gdadhgdejpy6f9v424a8v9htjxjhyqw9c5s5"
);
}
} }