From: Vihiga Tyonum Date: Wed, 14 Oct 2026 23:35:05 +0000 (+0100) Subject: ref(hwi): move descriptor utils into descriptor utils X-Git-Url: http://internal-gitweb-vhost/-script/src/example_cli/database/struct.Mempool.html?a=commitdiff_plain;h=c2466e91eb7605a71b68c334224a303deb3e9a2f;p=bdk-cli ref(hwi): move descriptor utils into descriptor utils - Move the single-sig script-type logic and multipath detection out of the HWI handler and into utils/descriptors. - Add SingleSigScriptType (ValueEnum) with purpose/as_str/descriptor and from_derivation_path. - Add is_multipath_descriptor, a wrapper over the existing descriptor_path_count primitive. - Drop the contains_private_key helper; the final DescriptorPublicKey parse already rejects private keys. - Add BitBox simulator endpoint as a const. --- diff --git a/src/handlers/hwi.rs b/src/handlers/hwi.rs index 1db0070..d04210c 100644 --- a/src/handlers/hwi.rs +++ b/src/handlers/hwi.rs @@ -19,6 +19,7 @@ use std::str::FromStr; use crate::error::BDKCliError as Error; use crate::handlers::{AppContext, AsyncAppCommand, Init, OfflineOperations}; +use crate::utils::descriptors::{SingleSigScriptType, is_multipath_descriptor, strip_checksum}; use crate::utils::hwi::{HwiWallet, enumerate_hwi_devices, first_hwi_device}; use bdk_wallet::bitcoin::{ @@ -31,6 +32,10 @@ use bdk_wallet::miniscript::{Descriptor, DescriptorPublicKey, descriptor::ShInne /// Device-only HWI utilities that do not require a wallet. #[derive(Debug, Parser, Clone, PartialEq, Eq)] pub struct HwiCommand { + /// Target a specific connected by master fingerprint. + #[arg(long, global = true)] + pub fingerprint: Option, + #[command(subcommand)] pub subcommand: HwiSubCommand, } @@ -43,17 +48,13 @@ pub enum HwiSubCommand { /// /// Needed to build a wallet descriptor for the device. Xpub { - /// Derivation path, e.g. `m/84'/1'/0'`. - // #[arg(default_value = "m/84'/1'/0'")] + /// Derivation path. Defaults to the standard path for `--type` and the + /// current network, e.g. `m/84'/1'/0'`. path: Option, - /// Output descriptor script type. - #[arg(long = "type", value_parser = ["wpkh", "tr", "sh-wpkh", "pkh"])] - script_type: Option, - - /// Target a specific connected device by master fingerprint (hex). - #[arg(long)] - fingerprint: Option, + /// Output descriptor script type. Inferred from the path when omitted. + #[arg(long = "type", value_enum)] + script_type: Option, }, /// List all connected hardware wallet devices. Devices, @@ -66,24 +67,24 @@ impl AsyncAppCommand> for HwiCommand { let network = ctx.network; // Device-only commands operate without a wallet policy. let hwi_wallet = HwiWallet::default(); + let target_fp = self.fingerprint.as_deref(); match &self.subcommand { - HwiSubCommand::Xpub { - path, - script_type, - fingerprint, - } => { + HwiSubCommand::Xpub { path, script_type } => { + let script_type = *script_type; + // Default to the standard path for the requested (or wpkh) type. let path = match path { Some(path) => path.clone(), None => { let coin = if network == Network::Bitcoin { 0 } else { 1 }; - format!("m/84'/{coin}'/0'") + let purpose = script_type.map_or(84, SingleSigScriptType::purpose); + format!("m/{purpose}'/{coin}'/0'") } }; let derivation = DerivationPath::from_str(&path) .map_err(|e| Error::Generic(format!("Invalid derivation path: {e}")))?; - let device = first_hwi_device(network, &hwi_wallet, fingerprint.as_deref()).await?; + let device = first_hwi_device(network, &hwi_wallet, target_fp).await?; let xpub = device.get_extended_pubkey(&derivation).await?; let master_fingerprint = device.get_master_fingerprint().await?.to_string(); let origin = path.trim_start_matches('m').trim_start_matches('/'); @@ -96,16 +97,12 @@ impl AsyncAppCommand> for HwiCommand { "key_expression": key_expression, }); - match script_type - .clone() - .or_else(|| infer_script_type(&path).map(str::to_string)) + match script_type.or_else(|| SingleSigScriptType::from_derivation_path(&derivation)) { Some(stype) => { - out["script_type"] = json!(stype); - out["external_descriptor"] = - json!(build_descriptor(&stype, &key_expression, 0)?); - out["internal_descriptor"] = - json!(build_descriptor(&stype, &key_expression, 1)?); + out["script_type"] = json!(stype.as_str()); + out["external_descriptor"] = json!(stype.descriptor(&key_expression, 0)?); + out["internal_descriptor"] = json!(stype.descriptor(&key_expression, 1)?); } None => { out["note"] = json!( @@ -120,10 +117,15 @@ impl AsyncAppCommand> for HwiCommand { let devices = enumerate_hwi_devices(network, &hwi_wallet).await?; let mut listed = Vec::with_capacity(devices.len()); for device in &devices { - let fingerprint = device.get_master_fingerprint().await?.to_string(); + let (fingerprint, error) = match device.get_master_fingerprint().await { + Ok(fp) => (Some(fp.to_string()), None), + Err(e) => (None, Some(e.to_string())), + }; + listed.push(json!({ "fingerprint": fingerprint, "model": device.device_kind().to_string(), + "error": error, })); } Ok(json!({ "count": listed.len(), "devices": listed })) @@ -137,10 +139,10 @@ impl AsyncAppCommand> for HwiCommand { pub struct WalletHwiCommand { /// Registration HMAC returned by `register`, required to reuse a registered /// policy on a Ledger for `address`/`sign`. - #[arg(env = "HWI_HMAC", long)] + #[arg(env = "HWI_HMAC", long, global = true)] pub hmac: Option, /// Target a specific connected device by master fingerprint (hex). - #[arg(long)] + #[arg(long, global = true)] pub fingerprint: Option, #[command(subcommand)] pub subcommand: WalletHwiSubCommand, @@ -208,7 +210,7 @@ impl WalletHwiCommand { let ext = ext_desc.to_string(); let int = int_desc.to_string(); let int = if int == ext { None } else { Some(int) }; - let policy = build_device_policy(&ext, int.as_deref())?; + let policy = build_device_policy(&ext, int.as_deref(), network)?; let hwi_wallet = HwiWallet { name: Some(wallet_name), @@ -218,7 +220,7 @@ impl WalletHwiCommand { match &self.subcommand { WalletHwiSubCommand::Register => { - tracing::debug!("Registering wallet '{wallet_name}' with policy '{policy}'"); + log::debug!("Registering wallet '{wallet_name}' with policy '{policy}'"); let device = first_hwi_device(network, &hwi_wallet, target_fp).await?; match device.register_wallet(wallet_name, &policy).await { Ok(hmac) => { @@ -305,11 +307,6 @@ impl WalletHwiCommand { } } -/// Strip a trailing descriptor checksum. -fn strip_checksum(descriptor: &str) -> &str { - descriptor.split('#').next().unwrap_or(descriptor) -} - /// How a wallet's descriptor maps onto a hardware device's capabilities. #[derive(Clone, Copy, PartialEq, Eq)] enum WalletKind { @@ -360,64 +357,23 @@ fn taproot_address_path( Some(base.extend([index])) } -/// Infer the single-sig descriptor script type from a derivation path's. -fn infer_script_type(path: &str) -> Option<&'static str> { - let purpose = path - .split('/') - .filter(|s| !s.is_empty() && *s != "m") - .find_map(|s| s.trim_end_matches(['\'', 'h']).parse::().ok())?; - match purpose { - 44 => Some("pkh"), - 49 => Some("sh-wpkh"), - 84 => Some("wpkh"), - 86 => Some("tr"), - _ => None, - } -} - -/// Wrap a key expression into a checksummed single-sig descriptor for the given -/// keychain branch (0 = external, 1 = internal). -fn build_descriptor(script_type: &str, key_expr: &str, branch: u32) -> Result { - let raw = match script_type { - "wpkh" => format!("wpkh({key_expr}/{branch}/*)"), - "tr" => format!("tr({key_expr}/{branch}/*)"), - "pkh" => format!("pkh({key_expr}/{branch}/*)"), - "sh-wpkh" => format!("sh(wpkh({key_expr}/{branch}/*))"), - other => return Err(Error::Generic(format!("Unsupported script type: {other}"))), - }; - let descriptor = Descriptor::::from_str(&raw) - .map_err(|e| Error::Generic(format!("Failed to build descriptor: {e}")))?; - Ok(descriptor.to_string()) -} - /// Build the wallet policy /// -/// - Rejects private-key. -/// - Passes an already-multipath descriptor (`/**` or `/<..>/*`) through -/// unchanged. -/// - Combines a standard `/0/*` external and `/1/*` internal pair into a single +/// - Passes an already-multipath descriptor through unchanged. +/// - Combines a standard external and internal pair into a single /// `/<0;1>/*` multipath descriptor. -fn build_device_policy(ext: &str, int: Option<&str>) -> Result { +/// +/// The result is parsed as a [`DescriptorPublicKey`] descriptor, which rejects +/// any private key. +fn build_device_policy(ext: &str, int: Option<&str>, network: Network) -> Result { let ext = strip_checksum(ext).trim(); - if contains_private_key(ext) { - return Err(Error::Generic( - "Hardware wallet policy must use public keys (xpub), not private keys".to_string(), - )); - } - let raw = if ext.contains("/**") || ext.contains('<') { - // Already a multipath descriptor. + let raw = if is_multipath_descriptor(ext, network)? { ext.to_string() } else { match int { Some(int) => { let int = strip_checksum(int).trim(); - if contains_private_key(int) { - return Err(Error::Generic( - "Hardware wallet policy must use public keys (xpub), not private keys" - .to_string(), - )); - } // `/0/*` and `/1/*` only ever appear as a key's wildcard branch. if ext.replace("/0/*", "/1/*") != int { return Err(Error::Generic( @@ -429,7 +385,7 @@ fn build_device_policy(ext: &str, int: Option<&str>) -> Result { ext.replace("/0/*", "/<0;1>/*") } None => { - tracing::warn!( + log::warn!( "Wallet has no separate internal (change) descriptor; the device policy will \ only cover receive addresses." ); @@ -438,20 +394,20 @@ fn build_device_policy(ext: &str, int: Option<&str>) -> Result { } }; - // Canonicalize so the device receives one well-formed, checksummed form. - let descriptor = Descriptor::::from_str(&raw) - .map_err(|e| Error::Generic(format!("Invalid wallet policy descriptor: {e}")))?; + // Parsing as a public descriptor also rejects any private key. + let descriptor = Descriptor::::from_str(&raw).map_err(|e| { + Error::Generic(format!( + "Invalid wallet policy (must be a public descriptor): {e}" + )) + })?; Ok(descriptor.to_string()) } -fn contains_private_key(descriptor: &str) -> bool { - descriptor.contains("xprv") || descriptor.contains("tprv") -} - #[cfg(test)] mod tests { use super::*; use crate::commands::CliOpts; + use bdk_wallet::bitcoin::{Network, bip32::Xpriv}; use clap::CommandFactory; const EXT: &str = "wpkh([f5acc2fd/84'/1'/0']tpubDCbK3Ysvk8HjcF6mPyrgMu3KgLiaaP19RjKpNezd8GrbAbNg6v5BtWLaCt8FNm6QkLseopKLf5MNYQFtochDTKHdfgG6iqJ8cqnLNAwtXuP/0/*)"; @@ -462,23 +418,9 @@ mod tests { CliOpts::command().debug_assert(); } - #[test] - fn infers_script_type_from_purpose() { - assert_eq!(infer_script_type("m/44'/1'/0'"), Some("pkh")); - assert_eq!(infer_script_type("m/49'/1'/0'"), Some("sh-wpkh")); - assert_eq!(infer_script_type("m/84'/1'/0'"), Some("wpkh")); - assert_eq!(infer_script_type("m/86h/0h/0h"), Some("tr")); - assert_eq!(infer_script_type("m/48'/1'/0'/2'"), None); - } - - #[test] - fn build_descriptor_rejects_unknown_type() { - assert!(build_descriptor("foo", "key", 0).is_err()); - } - #[test] fn policy_combines_and_canonicalizes() { - let policy = build_device_policy(EXT, Some(INT)).unwrap(); + let policy = build_device_policy(EXT, Some(INT), Network::Testnet).unwrap(); // Combined into a single multipath descriptor... assert!(policy.contains("<0;1>")); assert!(!policy.contains("/0/*")); @@ -489,13 +431,14 @@ mod tests { #[test] fn policy_rejects_private_keys() { - let xprv = "wpkh([f5acc2fd/84'/1'/0']tprv8ZgxMBicQKsPd.../0/*)"; - assert!(build_device_policy(xprv, None).is_err()); + let tprv = Xpriv::new_master(Network::Testnet, &[0u8; 32]).unwrap(); + let desc = format!("wpkh({tprv}/0/*)"); + assert!(build_device_policy(&desc, None, Network::Testnet).is_err()); } #[test] fn policy_rejects_mismatched_pair() { let other = "wpkh([f5acc2fd/84'/1'/0']tpubDDtb2WPYwEWw2WWDV7reLV348iJHw2HmhzvPysKKrJw3hYmvrd4jasyoioVPdKGQqjyaBMEvTn1HvHWDSVqQ6amyyxRZ5YjpPBBGjJ8yu8S/1/*)"; - assert!(build_device_policy(EXT, Some(other)).is_err()); + assert!(build_device_policy(EXT, Some(other), Network::Testnet).is_err()); } } diff --git a/src/handlers/offline.rs b/src/handlers/offline.rs index 6aebddd..15cdd26 100644 --- a/src/handlers/offline.rs +++ b/src/handlers/offline.rs @@ -35,13 +35,6 @@ use { bdk_message_signer::{MessageProof, MessageSigner}, }; -/// When built with hardware wallet support, `create_tx` produces a -/// PSBT that offline signers and hardware wallets can consume. -#[cfg(feature = "hwi")] -const DEFAULT_OFFLINE_SIGNER: bool = true; -#[cfg(not(feature = "hwi"))] -const DEFAULT_OFFLINE_SIGNER: bool = false; - impl OfflineWalletSubCommand { pub fn execute(&self, ctx: &mut AppContext>) -> Result<(), Error> { match self { @@ -212,7 +205,7 @@ pub struct CreateTxCommand { pub enable_rbf: bool, /// Make a PSBT that can be signed by offline signers and hardware wallets. Forces the addition of `PSBT_GLOBAL_XPUB` and more details to let the signer identify the change output. - #[arg(long = "offline_signer", default_value_t = DEFAULT_OFFLINE_SIGNER)] + #[arg(long = "offline_signer")] pub offline_signer: bool, /// Selects which utxos *must* be spent. diff --git a/src/utils/descriptors.rs b/src/utils/descriptors.rs index 3fa0490..edba4fc 100644 --- a/src/utils/descriptors.rs +++ b/src/utils/descriptors.rs @@ -1,6 +1,10 @@ use bdk_wallet::bitcoin::Network; +#[cfg(feature = "hwi")] +use bdk_wallet::bitcoin::bip32::ChildNumber; use bdk_wallet::descriptor::IntoWalletDescriptor; use bdk_wallet::keys::GeneratableKey; +#[cfg(feature = "hwi")] +use clap::ValueEnum; use std::{str::FromStr, sync::Arc}; use bdk_wallet::keys::DescriptorPublicKey; @@ -240,6 +244,88 @@ pub fn validate_descriptor_pair( Ok(ext_is_multipath) } +/// Strip a trailing descriptor checksum. +#[cfg(feature = "hwi")] +pub(crate) fn strip_checksum(descriptor: &str) -> &str { + descriptor.split('#').next().unwrap_or(descriptor) +} + +/// Returns `true` if `descriptor` is a BIP-389 multipath descriptor (more than +/// one derivation path). +/// +/// Errors if the descriptor is unparseable or its keys don't match `network`. +#[cfg(feature = "hwi")] +pub fn is_multipath_descriptor(descriptor: &str, network: Network) -> Result { + Ok(descriptor_path_count(descriptor, network)? > 1) +} + +/// Single-sig descriptor script types. +#[cfg(feature = "hwi")] +#[derive(Clone, Copy, Debug, PartialEq, Eq, ValueEnum)] +pub enum SingleSigScriptType { + Wpkh, + Tr, + ShWpkh, + Pkh, +} + +#[cfg(feature = "hwi")] +impl SingleSigScriptType { + /// The BIP-43 purpose index for this type. + pub fn purpose(self) -> u32 { + match self { + Self::Pkh => 44, + Self::ShWpkh => 49, + Self::Wpkh => 84, + Self::Tr => 86, + } + } + + /// Map a BIP-43 purpose index back to a type. + fn from_purpose(purpose: u32) -> Option { + match purpose { + 44 => Some(Self::Pkh), + 49 => Some(Self::ShWpkh), + 84 => Some(Self::Wpkh), + 86 => Some(Self::Tr), + _ => None, + } + } + + /// Infer the type from a derivation path's purpose (BIP-43). The purpose + /// level must be hardened, so an unhardened path returns `None`. + pub fn from_derivation_path(path: &DerivationPath) -> Option { + let &ChildNumber::Hardened { index: purpose } = path.into_iter().next()? else { + return None; + }; + Self::from_purpose(purpose) + } + + /// The `--type` / descriptor name for this type. + pub fn as_str(self) -> &'static str { + match self { + Self::Wpkh => "wpkh", + Self::Tr => "tr", + Self::ShWpkh => "sh-wpkh", + Self::Pkh => "pkh", + } + } + + /// Build a checksummed single-sig descriptor from a key expression for the + /// given keychain branch (0 = external, 1 = internal). + pub fn descriptor(self, key_expr: &str, branch: u32) -> Result { + let raw = match self { + Self::Wpkh => format!("wpkh({key_expr}/{branch}/*)"), + Self::Tr => format!("tr({key_expr}/{branch}/*)"), + Self::ShWpkh => format!("sh(wpkh({key_expr}/{branch}/*))"), + Self::Pkh => format!("pkh({key_expr}/{branch}/*)"), + }; + let descriptor = Descriptor::::from_str(&raw) + .map_err(|e| Error::Generic(format!("Failed to build descriptor: {e}")))?; + Ok(descriptor.to_string()) + } +} + #[cfg(test)] mod multipath_tests { use super::*; @@ -259,4 +345,24 @@ mod multipath_tests { fn accepts_single_path_with_internal_descriptor() { assert!(!validate_descriptor_pair(SINGLE, Some(SINGLE), Network::Testnet).unwrap()); } + #[cfg(feature = "hwi")] + #[test] + fn detects_multipath_descriptor() { + assert!(is_multipath_descriptor(MULTIPATH, Network::Testnet).unwrap()); + assert!(!is_multipath_descriptor(SINGLE, Network::Testnet).unwrap()); + } + + #[cfg(feature = "hwi")] + #[test] + fn infers_script_type_from_purpose() { + let p = |s: &str| DerivationPath::from_str(s).unwrap(); + type T = SingleSigScriptType; + assert_eq!(T::from_derivation_path(&p("m/44'/1'/0'")), Some(T::Pkh)); + assert_eq!(T::from_derivation_path(&p("m/49'/1'/0'")), Some(T::ShWpkh)); + assert_eq!(T::from_derivation_path(&p("m/84'/1'/0'")), Some(T::Wpkh)); + assert_eq!(T::from_derivation_path(&p("m/86h/0h/0h")), Some(T::Tr)); + assert_eq!(T::from_derivation_path(&p("m/48'/1'/0'/2'")), None); + // BIP-43: an unhardened purpose is not a standard path. + assert_eq!(T::from_derivation_path(&p("m/84/1/0")), None); + } } diff --git a/src/utils/hwi.rs b/src/utils/hwi.rs index 296eabd..4aafe75 100644 --- a/src/utils/hwi.rs +++ b/src/utils/hwi.rs @@ -18,10 +18,13 @@ use async_hwi::{ specter::{Specter, SpecterSimulator}, }; use bdk_wallet::bitcoin::{Network, hex::FromHex}; -use tracing::warn; +use log::warn; use crate::error::BDKCliError as Error; +/// TCP endpoint for BitBox02 simulator. +const BITBOX_SIMULATOR_ENDPOINT: &str = "127.0.0.1:15423"; + /// Optional wallet context required by some devices (Ledger/Coldcard/BitBox) /// to register and operate on a specific multisig/miniscript policy. #[derive(Default, Clone)] @@ -53,7 +56,12 @@ pub async fn enumerate_hwi_devices( match Jade::enumerate().await { Ok(jades) => { for device in jades { - let device = device.with_network(network); + let mut device = device.with_network(network); + // The wallet name is required for `display_address` to find the + // registered descriptor on the device. + if let Some(name) = wallet.name { + device = device.with_wallet(name.to_string()); + } match device.get_info().await { Ok(info) => { if info.jade_state == jade::api::JadeState::Locked @@ -132,8 +140,11 @@ pub async fn enumerate_hwi_devices( } // BitBox02 simulator - if let Ok(bitbox) = - BitBoxApi::::from_simulator(None, Box::new(NoiseConfigNoCache {})).await + if tokio::net::TcpStream::connect(BITBOX_SIMULATOR_ENDPOINT) + .await + .is_ok() + && let Ok(bitbox) = + BitBoxApi::::from_simulator(None, Box::new(NoiseConfigNoCache {})).await { match bitbox.unlock_and_pair().await { Ok(pairing) => match pairing.wait_confirm().await {