diff --git a/crates/wallet/src/descriptor/error.rs b/crates/wallet/src/descriptor/error.rs index e018b5352..ec716b6cc 100644 --- a/crates/wallet/src/descriptor/error.rs +++ b/crates/wallet/src/descriptor/error.rs @@ -68,7 +68,7 @@ impl fmt::Display for Error { ), Self::MultiPath => write!( f, - "The descriptor contains multipath keys, which are not supported yet" + "The descriptor contains multipath keys when none were expected" ), Self::Key(err) => write!(f, "Key error: {}", err), Self::Policy(err) => write!(f, "Policy error: {}", err), diff --git a/crates/wallet/src/descriptor/mod.rs b/crates/wallet/src/descriptor/mod.rs index f5ef59578..b8c080168 100644 --- a/crates/wallet/src/descriptor/mod.rs +++ b/crates/wallet/src/descriptor/mod.rs @@ -914,4 +914,48 @@ mod test { assert_eq!(psbt_input.redeem_script, Some(script.to_p2wsh())); assert_eq!(psbt_input.witness_script, Some(script)); } + + #[test] + fn test_into_wallet_descriptor_multi() -> anyhow::Result<()> { + // See + let secp = Secp256k1::new(); + + // multipath tpub + let descriptor_str = "wpkh([9a6a2580/84'/0'/0']tpubDDnGNapGEY6AZAdQbfRJgMg9fvz8pUBrLwvyvUqEgcUfgzM6zc2eVK4vY9x9L5FJWdX8WumXuLEDV5zDZnTfbn87vLe9XceCFwTu9so9Kks/<0;1>/*)"; + let (descriptor, _key_map) = descriptor_str + .into_wallet_descriptor(&secp, Network::Testnet) + .expect("should parse multipath tpub"); + + assert!(descriptor.is_multipath()); + + // invalid network for descriptor + let descriptor_str = "wpkh([9a6a2580/84'/0'/0']xpub6DEzNop46vmxR49zYWFnMwmEfawSNmAMf6dLH5YKDY463twtvw1XD7ihwJRLPRGZJz799VPFzXHpZu6WdhT29WnaeuChS6aZHZPFmqczR5K/<0;1>/*)"; + let res = descriptor_str.into_wallet_descriptor(&secp, Network::Testnet); + + assert!(matches!( + res, + Err(DescriptorError::Key(KeyError::InvalidNetwork)) + )); + + // multipath xpub + let descriptor_str = "wpkh([9a6a2580/84'/0'/0']xpub6DEzNop46vmxR49zYWFnMwmEfawSNmAMf6dLH5YKDY463twtvw1XD7ihwJRLPRGZJz799VPFzXHpZu6WdhT29WnaeuChS6aZHZPFmqczR5K/<0;1>/*)"; + let (descriptor, _key_map) = descriptor_str + .into_wallet_descriptor(&secp, Network::Bitcoin) + .expect("should parse multipath xpub"); + + assert!(descriptor.is_multipath()); + + // Miniscript can't make an extended private key with multiple paths into a public key. + // ref: + let descriptor_str = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/<1;0>/*)"; + assert!(matches!( + Descriptor::parse_descriptor(&secp, descriptor_str), + Err(miniscript::Error::Unexpected(..)), + )); + let _ = descriptor_str + .into_wallet_descriptor(&secp, Network::Testnet) + .unwrap_err(); + + Ok(()) + } } diff --git a/crates/wallet/src/keys/mod.rs b/crates/wallet/src/keys/mod.rs index eac374964..4c4f732e4 100644 --- a/crates/wallet/src/keys/mod.rs +++ b/crates/wallet/src/keys/mod.rs @@ -27,7 +27,7 @@ use bitcoin::secp256k1::{self, Secp256k1, Signing}; use bitcoin::bip32; use bitcoin::{key::XOnlyPublicKey, Network, PrivateKey, PublicKey}; -use miniscript::descriptor::{Descriptor, DescriptorXKey, Wildcard}; +use miniscript::descriptor::{Descriptor, DescriptorMultiXKey, DescriptorXKey, Wildcard}; pub use miniscript::descriptor::{ DescriptorPublicKey, DescriptorSecretKey, KeyMap, SinglePriv, SinglePub, SinglePubKey, SortedMultiVec, @@ -878,10 +878,20 @@ impl IntoDescriptorKey for DescriptorPublicKey { fn into_descriptor_key(self) -> Result, KeyError> { let networks = match self { DescriptorPublicKey::Single(_) => any_network(), - DescriptorPublicKey::XPub(DescriptorXKey { xkey, .. }) if xkey.network.is_mainnet() => { - mainnet_network() + DescriptorPublicKey::XPub(DescriptorXKey { xkey, .. }) => { + if xkey.network.is_mainnet() { + mainnet_network() + } else { + test_networks() + } + } + DescriptorPublicKey::MultiXPub(DescriptorMultiXKey { xkey, .. }) => { + if xkey.network.is_mainnet() { + mainnet_network() + } else { + test_networks() + } } - _ => test_networks(), }; Ok(DescriptorKey::from_public(self, networks)) diff --git a/crates/wallet/src/test_utils.rs b/crates/wallet/src/test_utils.rs index 7e1778fab..1d805be63 100644 --- a/crates/wallet/src/test_utils.rs +++ b/crates/wallet/src/test_utils.rs @@ -145,6 +145,11 @@ pub fn get_test_wpkh_and_change_desc() -> (&'static str, &'static str) { "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/1/*)") } +/// `wpkh` multipath tpub +pub fn get_test_wpkh_multipath() -> &'static str { + "wpkh([e273fe42/84'/1'/0']tpubDCmr3Luq75npLaYmRqqW1rLfSbfpnBXwLwAmUbR333fp95wjCHar3zoc9zSWovZFwrWr53mm3NTVqt6d1Pt6G26uf4etQjc3Pr5Hxe9QEQ2/<0;1>/*)" +} + /// `wsh` descriptor with policy `and(pk(A),older(6))` pub fn get_test_single_sig_csv() -> &'static str { "wsh(and_v(v:pk(cVpPVruEDdmutPzisEsYvtST1usBR3ntr8pXSyt6D2YYqXRyPcFW),older(6)))" diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index e4dc6d056..a2d2c4cfd 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -44,7 +44,7 @@ use bitcoin::{ Sequence, Transaction, TxOut, Txid, Weight, Witness, }; use miniscript::{ - descriptor::KeyMap, + descriptor::{Descriptor, KeyMap}, psbt::{PsbtExt, PsbtInputExt, PsbtInputSatisfier}, }; use rand_core::RngCore; @@ -281,24 +281,56 @@ impl std::error::Error for ApplyBlockError {} pub type WalletTx<'a> = CanonicalTx<'a, Arc, ConfirmationBlockTime>; impl Wallet { - /// Build a new single descriptor [`Wallet`]. + /// Build a [`Wallet`] from a single descriptor. /// /// If you have previously created a wallet, use [`load`](Self::load) instead. /// + /// The structure of the resulting [`Wallet`] depends on whether the provided descriptor + /// contains multiple derivation paths. If the parsed descriptor is one of [`Single`] or + /// [`XPub`], the returned wallet will have only 1 keychain, the [`External`] kind. See + /// details and usage considerations below. + /// + /// If the descriptor is a [`MultiXPub`] we automatically attempt to parse it further + /// into two single descriptors and return a wallet with both external and internal keychains. + /// The index values of the multipath specifier must not be the same, and any indexes of a + /// multipath longer than 2 elements are discarded. A wallet created this way is + /// functionally equivalent to splitting a multipath descriptor into single descriptors + /// and passing them to [`Wallet::create`]. Note also that the [`ChangeSet`] produced + /// by such a wallet will have the [`change_descriptor`] set. + /// + /// # Example + /// + /// ```rust + /// // Create a wallet from a multipath descriptor string + /// # use bdk_wallet::Wallet; + /// # use bdk_wallet::KeychainKind::*; + /// # use bitcoin::Network; + /// # use bdk_wallet::rusqlite::Connection; + /// let mut db = Connection::open_in_memory()?; + /// let descriptor_str = "wpkh(xpub6DEzNop46vmxR49zYWFnMwmEfawSNmAMf6dLH5YKDY463twtvw1XD7ihwJRLPRGZJz799VPFzXHpZu6WdhT29WnaeuChS6aZHZPFmqczR5K/<0;1>/*)"; + /// let wallet = Wallet::create_single(descriptor_str).create_wallet(&mut db)?; + /// assert_eq!( + /// wallet.public_descriptor(External).to_string(), + /// "wpkh(xpub6DEzNop46vmxR49zYWFnMwmEfawSNmAMf6dLH5YKDY463twtvw1XD7ihwJRLPRGZJz799VPFzXHpZu6WdhT29WnaeuChS6aZHZPFmqczR5K/0/*)#tp6xtjkj", + /// ); + /// assert_eq!( + /// wallet.public_descriptor(Internal).to_string(), + /// "wpkh(xpub6DEzNop46vmxR49zYWFnMwmEfawSNmAMf6dLH5YKDY463twtvw1XD7ihwJRLPRGZJz799VPFzXHpZu6WdhT29WnaeuChS6aZHZPFmqczR5K/1/*)#64l8k8x2", + /// ); + /// # Ok::<_, anyhow::Error>(()) + /// ``` + /// /// # Note /// - /// Only use this method when creating a wallet designed to be used with a single - /// descriptor and keychain. Otherwise the recommended way to construct a new wallet is - /// by using [`Wallet::create`]. It's worth noting that not all features are available - /// with single descriptor wallets, for example setting a [`change_policy`] on [`TxBuilder`] - /// and related methods such as [`do_not_spend_change`]. This is because all payments are - /// received on the external keychain (including change), and without a change keychain - /// BDK lacks enough information to distinguish between change and outside payments. + /// Users should be aware of the limitations that come with using single-keychain wallets. + /// For example the [`ChangeSpendPolicy`] is meaningless in this context because all payments + /// are received on the external keychain including change, and without a designated change + /// keychain BDK lacks the information to distinguish between change and incoming payments. /// - /// Additionally because this wallet has no internal (change) keychain, all methods that - /// require a [`KeychainKind`] as input, e.g. [`reveal_next_address`] should only be called - /// using the [`External`] variant. In most cases passing [`Internal`] is treated as the - /// equivalent of [`External`] but this behavior must not be relied on. + /// Additionally, for a wallet with no internal (change) keychain, any methods requiring a + /// [`KeychainKind`] as input, e.g. [`reveal_next_address`] should only be called with the + /// [`External`] variant. In some cases passing [`Internal`] is treated the same as + /// [`External`] but this behavior must not be relied on. /// /// # Example /// @@ -308,7 +340,7 @@ impl Wallet { /// # const EXTERNAL_DESC: &str = "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/0/*)"; /// # let temp_dir = tempfile::tempdir().expect("must create tempdir"); /// # let file_path = temp_dir.path().join("store.db"); - /// // Create a wallet that is persisted to SQLite database. + /// // Create a single keychain wallet persisted to a SQLite database. /// use bdk_wallet::rusqlite::Connection; /// let mut conn = Connection::open(file_path)?; /// let wallet = Wallet::create_single(EXTERNAL_DESC) @@ -316,11 +348,14 @@ impl Wallet { /// .create_wallet(&mut conn)?; /// # Ok::<_, anyhow::Error>(()) /// ``` - /// [`change_policy`]: TxBuilder::change_policy - /// [`do_not_spend_change`]: TxBuilder::do_not_spend_change + /// [`change_descriptor`]: ChangeSet::change_descriptor + /// [`ChangeSpendPolicy`]: crate::ChangeSpendPolicy /// [`External`]: KeychainKind::External /// [`Internal`]: KeychainKind::Internal /// [`reveal_next_address`]: Self::reveal_next_address + /// [`Single`]: miniscript::DescriptorPublicKey::Single + /// [`MultiXPub`]: miniscript::DescriptorPublicKey::MultiXPub + /// [`XPub`]: miniscript::DescriptorPublicKey::XPub pub fn create_single(descriptor: D) -> CreateParams where D: IntoWalletDescriptor + Send + Clone + 'static, @@ -332,6 +367,10 @@ impl Wallet { /// /// If you have previously created a wallet, use [`load`](Self::load) instead. /// + /// This should be used when the parsed `descriptor` and `change_descriptor` are either + /// a [`Single`] key or [`XPub`]. If creating a wallet from a [multipath] descriptor, + /// use [`create_single`] instead. + /// /// # Synopsis /// /// ```rust @@ -356,6 +395,11 @@ impl Wallet { /// # Ok(()) /// # } /// ``` + /// [`Single`]: miniscript::DescriptorPublicKey::Single + /// [`XPub`]: miniscript::DescriptorPublicKey::XPub + /// [`DescriptorPublicKey`]: miniscript::DescriptorPublicKey + /// [multipath]: miniscript::Descriptor::is_multipath + /// [`create_single`]: Wallet::create_single pub fn create(descriptor: D, change_descriptor: D) -> CreateParams where D: IntoWalletDescriptor + Send + Clone + 'static, @@ -363,10 +407,34 @@ impl Wallet { CreateParams::new(descriptor, change_descriptor) } - /// Create a new [`Wallet`] with given `params`. + /// Create a new [`Wallet`] with the given `params`. Refer to [`Wallet::create`] for more. + /// + /// Note on [multipath] descriptors: + /// + /// If the parsed descriptor is a [`MultiXPub`] we use the fully parsed single descriptors to + /// set the value of the change descriptor, meaning that if creating a wallet from a multipath + /// descriptor, only the first descriptor of the `params` is needed and the change descriptor + /// can be omitted. + /// + /// Extra options on the `params`, such as [`keymap`], should be provided assuming that + /// the descriptors have already been split into their single counterparts. See + /// [`create_single`](Self::create_single) for more details. + /// + /// # Errors /// - /// Refer to [`Wallet::create`] for more. - pub fn create_with_params(params: CreateParams) -> Result { + /// This returns a [`DescriptorError`] in a number of situations: + /// + /// - If parsing a [`Descriptor`] fails + /// - If there's a mismatch in the length of two or more multipath specifiers + /// - If a multipath descriptor appears where one was not expected + /// - If the descriptor and change descriptor are the same + /// + /// [multipath]: miniscript::Descriptor::is_multipath + /// [`keymap`]: CreateParams::keymap + /// [`change_descriptor`]: CreateParams::change_descriptor + /// [`MultiXPub`]: miniscript::DescriptorPublicKey::MultiXPub + /// [`Descriptor`]: miniscript::Descriptor + pub fn create_with_params(mut params: CreateParams) -> Result { let secp = SecpCtx::new(); let network = params.network; let genesis_hash = params @@ -374,7 +442,17 @@ impl Wallet { .unwrap_or(genesis_block(network).block_hash()); let (chain, chain_changeset) = LocalChain::from_genesis_hash(genesis_hash); - let (descriptor, mut descriptor_keymap) = (params.descriptor)(&secp, network)?; + let (mut descriptor, mut descriptor_keymap) = (params.descriptor)(&secp, network)?; + + // In case of multipath we parse the descriptor into single descriptors and + // assign each one back to the `descriptor` and `change_descriptor`. + if descriptor.is_multipath() { + let mut desc_iter = descriptor.into_single_descriptors()?.into_iter(); + descriptor = desc_iter.next().expect("must have a descriptor"); + descriptor_keymap = Descriptor::parse_descriptor(&secp, &descriptor.to_string())?.1; + params.change_descriptor = desc_iter.next().map(params::make_descriptor_to_extract); + } + check_wallet_descriptor(&descriptor)?; descriptor_keymap.extend(params.descriptor_keymap); diff --git a/crates/wallet/src/wallet/params.rs b/crates/wallet/src/wallet/params.rs index 7cf3bdd27..eaf80c399 100644 --- a/crates/wallet/src/wallet/params.rs +++ b/crates/wallet/src/wallet/params.rs @@ -22,7 +22,7 @@ type DescriptorToExtract = Box< + 'static, >; -fn make_descriptor_to_extract(descriptor: D) -> DescriptorToExtract +pub(crate) fn make_descriptor_to_extract(descriptor: D) -> DescriptorToExtract where D: IntoWalletDescriptor + Send + 'static, { diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs index 35cbd85d4..56d4ba227 100644 --- a/crates/wallet/tests/wallet.rs +++ b/crates/wallet/tests/wallet.rs @@ -46,6 +46,61 @@ const P2WPKH_FAKE_WITNESS_SIZE: usize = 106; const DB_MAGIC: &[u8] = &[0x21, 0x24, 0x48]; +#[test] +fn test_create_with_params_multipath() { + // `Wallet::create_single` parses a multipath descriptor and returns + // a wallet with 2 keychains + let xpub = "xpub6DEzNop46vmxR49zYWFnMwmEfawSNmAMf6dLH5YKDY463twtvw1XD7ihwJRLPRGZJz799VPFzXHpZu6WdhT29WnaeuChS6aZHZPFmqczR5K"; + + // test two different multipaths: <0;1> and <23;19> + for item in [[0, 1], [23, 19]] { + let i = item[0]; + let j = item[1]; + + let desc_str = format!("wpkh({xpub}/<{i};{j}>/*)"); + let wallet = Wallet::create_single(desc_str) + .create_wallet_no_persist() + .unwrap(); + + assert_eq!(wallet.keychains().count(), 2); + + use KeychainKind::*; + // We should have parsed the descriptor into single descriptors + for (i, keychain) in item.into_iter().zip([External, Internal]) { + let desc = wallet.public_descriptor(keychain); + assert!(!desc.is_multipath(), "we should have split the descriptor"); + assert!(!desc.to_string().contains(['<', '>', ';'])); + // Steps in the multipath go to their respective keychains + let parsed = format!("{i}/*"); + assert!(desc.to_string().contains(&parsed)); + } + } +} + +#[test] +fn test_create_with_params_error_multipath() { + // `create_with_params` rejects a multipath *change* descriptor + let (desc, _) = get_test_wpkh_and_change_desc(); + let multipath = get_test_wpkh_multipath(); + let res = Wallet::create(desc, multipath) + .network(Network::Testnet) + .create_wallet_no_persist(); + assert!(matches!(res, Err(DescriptorError::MultiPath))); + + // rejects multipaths of mismatched length + let pk_0 = "xpub661MyMwAqRbcF3yVrV2KyYetLMYA5mCbv4BhrKwUrFE9LZM6JRR1AEt8Jq4V4C8LwtTke6YEEdCZqgXp85YRk2j74EfJKhe3QybQ9kcUjs4"; + let pk_1 = "xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL"; + let pk_2 = "xpub661MyMwAqRbcGDZQUKLqmWodYLcoBQnQH33yYkkF3jjxeLvY8qr2wWGEWkiKFaaQfJCoi3HeEq3Dc5DptfbCyjD38fNhSqtKc1UHaP4ba3t"; + let desc_str = format!("tr({pk_0}/*,{{pk({pk_1}/<1;2;3>/*),pk({pk_2}/<1;2>/*)}})"); + let res = Wallet::create_single(desc_str).create_wallet_no_persist(); + assert!(matches!( + res, + Err(DescriptorError::Miniscript( + miniscript::Error::MultipathDescLenMismatch + )) + )); +} + #[test] fn wallet_is_persisted() -> anyhow::Result<()> { fn run(