From: Vihiga Tyonum Date: Tue, 4 Aug 2026 22:25:15 +0000 (+0100) Subject: feat(hwi): Move register, address & sign to wallet X-Git-Url: http://internal-gitweb-vhost/-script/src/example_cli/database/struct.KeychainChangeSet.html?a=commitdiff_plain;h=e59a64d9036b5f77e98f2d0dd8b4a96b2421856d;p=bdk-cli feat(hwi): Move register, address & sign to wallet - move the register, address and sign commands to subwallet commands --- diff --git a/src/commands.rs b/src/commands.rs index e4f9dce..a9abb73 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -61,7 +61,7 @@ use crate::handlers::dns::{CreateDnsTxCommand, ResolveDnsRecipientCommand}; use crate::utils::parse_proxy_auth; #[cfg(feature = "hwi")] -use crate::handlers::hwi::HwiCommand; +use crate::handlers::hwi::{HwiCommand, WalletHwiCommand}; /// The BDK Command Line Wallet App /// @@ -223,6 +223,12 @@ pub enum CliSubCommand { pub enum WalletSubCommand { /// Save wallet configuration to `config.toml`. Config(SaveConfigCommand), + /// Hardware wallet (HWI) operations for this wallet. + /// + /// Register the wallet's policy on a device, display a receive + /// address, or sign a PSBT. + #[cfg(feature = "hwi")] + Hwi(WalletHwiCommand), #[cfg(any( feature = "electrum", feature = "esplora", diff --git a/src/handlers/hwi.rs b/src/handlers/hwi.rs index ea6b256..8553172 100644 --- a/src/handlers/hwi.rs +++ b/src/handlers/hwi.rs @@ -1,48 +1,50 @@ //! Hardware wallet (HWI) command handlers. //! -//! Provides the `hwi` top-level command with subcommands to list connected -//! devices, register a wallet policy, display a receive address on the device -//! and sign a PSBT. Device discovery lives in [`crate::utils::hwi`]. +//! HWI operations are split by their data dependency: +//! +//! - Device-only utilities live under the top-level `hwi` command +//! ([`HwiCommand`]): `hwi xpub` reads a device extended public key and +//! `hwi devices` lists connected devices. Neither needs a wallet — `hwi xpub` +//! is the bootstrap step used to build a descriptor in the first place. +//! - Wallet-scoped operations live under `wallet hwi …` +//! ([`WalletHwiCommand`]): `register`, `address` and `sign`. These take the +//! wallet policy from the loaded wallet's descriptors, so no descriptor needs +//! to be passed on the command line. +//! +//! `async-hwi` expects a *wallet policy*: a single, public (xpub) descriptor +//! whose keys carry origin info and use multipath (`/<0;1>/*`) notation so the +//! policy covers both the receive and change chains. BDK exposes the external +//! and internal chains as two single-path public descriptors, so +//! [`build_device_policy`] combines them into the multipath form the device +//! needs. -use crate::commands::WalletOpts; -use crate::error::BDKCliError as Error; -use crate::handlers::{AppContext, AsyncAppCommand, Init}; -use crate::persister::new_wallet; -use crate::utils::hwi::{HwiWallet, enumerate_hwi_devices, first_hwi_device}; -use crate::utils::load_wallet_config; use async_hwi::AddressScript; +use bdk_wallet::KeychainKind; use bdk_wallet::bitcoin::{Psbt, bip32::DerivationPath, hex::DisplayHex}; -use bdk_wallet::{KeychainKind, Wallet}; -use clap::{Args, Parser, Subcommand}; +use clap::{Parser, Subcommand}; use serde_json::json; -use std::path::Path; use std::str::FromStr; -/// Options shared by all HWI subcommands. -#[derive(Clone, Debug, PartialEq, Eq, Args)] -pub struct HwiOpts { - /// Wallet name registered on the device (required by Ledger and Coldcard). - #[arg(env = "WALLET_NAME", short = 'w', long)] - pub wallet: Option, - - /// External descriptor / wallet policy to register or operate on. - #[arg(env = "EXT_DESCRIPTOR", short = 'e', long)] - pub ext_descriptor: Option, +use crate::error::BDKCliError as Error; +use crate::handlers::{AppContext, AsyncAppCommand, Init, OfflineOperations}; +use crate::utils::hwi::{HwiWallet, enumerate_hwi_devices, first_hwi_device}; - /// Registration HMAC returned by `hwi register` (required to reuse a - /// registered policy on a Ledger). - #[arg(env = "HWI_HMAC", long)] - pub hmac: Option, +/// Device-only HWI utilities that do not require a wallet. +#[derive(Debug, Parser, Clone, PartialEq, Eq)] +pub struct HwiCommand { + #[command(subcommand)] + pub subcommand: HwiSubCommand, } -/// HWI subcommands. +/// Subcommands for the top-level `hwi` command. #[derive(Debug, Subcommand, Clone, PartialEq, Eq)] #[command(rename_all = "snake")] pub enum HwiSubCommand { /// Read an extended public key from the device at a derivation path. /// /// Use this first to obtain the key material needed to build a wallet - /// descriptor for the device. + /// descriptor for the device. The output includes a ready-to-use key + /// expression (`[fingerprint/path]xpub`) to drop into a descriptor. Xpub { /// Derivation path, e.g. `m/84'/1'/0'`. #[arg(default_value = "m/84'/1'/0'")] @@ -50,93 +52,6 @@ pub enum HwiSubCommand { }, /// List all connected hardware wallet devices. Devices, - /// Register a wallet policy on the device, returning its HMAC. - Register, - /// Display a receive address on the device for verification. - Address { - /// Address index to derive. - #[arg(long, default_value_t = 0)] - index: u32, - /// Derive from the change (internal) keychain instead of external. - #[arg(long)] - change: bool, - }, - /// Sign a PSBT with the device. - /// - /// The device produces partial signatures; the returned PSBT still needs to - /// be finalized before it can be broadcasted. - Sign { - /// The base64-encoded PSBT to sign. - psbt: String, - }, -} - -/// The `hwi` top-level command. -#[derive(Debug, Parser, Clone, PartialEq, Eq)] -pub struct HwiCommand { - #[command(flatten)] - pub opts: HwiOpts, - #[command(subcommand)] - pub subcommand: HwiSubCommand, -} - -impl HwiCommand { - /// Load the saved wallet config when a descriptor is needed. Errors if `--wallet` - /// is set but its config cannot be read. - fn load_wallet_opts(&self, datadir: &Path) -> Result, Error> { - if self.opts.ext_descriptor.is_some() { - return Ok(None); - } - match &self.opts.wallet { - Some(name) => load_wallet_config(datadir, name).map(|(opts, _network)| Some(opts)), - None => Ok(None), - } - } - - /// Build the device wallet context, preferring an explicit `--ext-descriptor` - /// and falling back to the loaded config's external descriptor. - fn hwi_wallet<'a>(&'a self, opts: Option<&'a WalletOpts>) -> HwiWallet<'a> { - let policy = self - .opts - .ext_descriptor - .as_deref() - .or_else(|| opts.map(|o| o.ext_descriptor.as_str())); - HwiWallet { - name: self.opts.wallet.as_deref(), - policy, - hmac: self.opts.hmac.as_deref(), - } - } - - /// Derive the address at `index` for the given keychain, from the saved - /// config when available or from an explicit external descriptor otherwise. - fn derive_address( - &self, - network: bdk_wallet::bitcoin::Network, - opts: Option<&WalletOpts>, - index: u32, - change: bool, - ) -> Result, Error> { - let keychain = if change { - KeychainKind::Internal - } else { - KeychainKind::External - }; - - let wallet = if let Some(opts) = opts { - Some(new_wallet(network, opts)?) - } else if let Some(desc) = self.opts.ext_descriptor.as_deref() { - Some( - Wallet::create_single(desc.to_string()) - .network(network) - .create_wallet_no_persist()?, - ) - } else { - None - }; - - Ok(wallet.map(|w| w.peek_address(keychain, index).address.to_string())) - } } impl AsyncAppCommand> for HwiCommand { @@ -144,25 +59,28 @@ impl AsyncAppCommand> for HwiCommand { async fn execute(&self, ctx: &mut AppContext) -> Result { let network = ctx.network; + // Device-only commands operate without a wallet policy. + let hwi_wallet = HwiWallet::default(); match &self.subcommand { HwiSubCommand::Xpub { path } => { let derivation = DerivationPath::from_str(path) .map_err(|e| Error::Generic(format!("Invalid derivation path: {e}")))?; - let wallet = self.hwi_wallet(None); - let device = first_hwi_device(network, &wallet).await?; + let device = first_hwi_device(network, &hwi_wallet).await?; let xpub = device.get_extended_pubkey(&derivation).await?; let fingerprint = device.get_master_fingerprint().await?.to_string(); + let origin = path.trim_start_matches('m').trim_start_matches('/'); + let key_expression = format!("[{fingerprint}/{origin}]{xpub}"); Ok(json!({ "path": path, "xpub": xpub.to_string(), "master_fingerprint": fingerprint, + "key_expression": key_expression, })) } HwiSubCommand::Devices => { - let wallet = self.hwi_wallet(None); - let devices = enumerate_hwi_devices(network, &wallet).await?; + 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(); @@ -173,52 +91,141 @@ impl AsyncAppCommand> for HwiCommand { } Ok(json!({ "count": listed.len(), "devices": listed })) } + } + } +} + +/// Wallet-scoped HWI operations (`wallet hwi …`). +#[derive(Debug, Parser, Clone, PartialEq, Eq)] +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)] + pub hmac: Option, + #[command(subcommand)] + pub subcommand: WalletHwiSubCommand, +} - HwiSubCommand::Register => { - let opts = self.load_wallet_opts(&ctx.datadir)?; - let wallet = self.hwi_wallet(opts.as_ref()); - let name = wallet.name.ok_or_else(|| { - Error::Generic("Wallet name is required to register".to_string()) - })?; - let policy = wallet.policy.ok_or_else(|| { - Error::Generic( - "No descriptor found: pass --ext-descriptor or save a wallet config for --wallet" - .to_string(), - ) - })?; - tracing::debug!("Registering wallet '{name}' with policy '{policy}'"); - // let device = first_hwi_device(network, &wallet).await?; - let hmac = device_register(network, &wallet, name, policy).await?; - - // let hmac_hex = hmac.map(|h| h.to_lower_hex_string()); - Ok(json!({ "success": true, "hmac": hmac })) +/// Subcommands for `wallet hwi`. +#[derive(Debug, Subcommand, Clone, PartialEq, Eq)] +#[command(rename_all = "snake")] +pub enum WalletHwiSubCommand { + /// Register the wallet's policy on the device, returning its HMAC. + Register, + /// Display a receive address on the device. + Address { + /// Address index to derive. + #[arg(long, default_value_t = 0)] + index: u32, + /// Derive from the change (internal) keychain instead of external. + #[arg(long)] + change: bool, + }, + /// Sign a PSBT with the device. + /// + /// The device produces partial signatures; the returned PSBT still needs to + /// be finalized before it can be broadcasted. + Sign { + /// The base64-encoded PSBT to sign. + psbt: String, + }, +} + +impl WalletHwiCommand { + /// Run a wallet-scoped HWI operation. The wallet policy is derived from the + /// loaded wallet's public descriptors and `wallet_name` is used as the + /// device registration name. + pub async fn run( + &self, + ctx: &mut AppContext>, + wallet_name: &str, + ) -> Result { + let network = ctx.network; + + // Derive the multipath policy from the wallet's public descriptors. + let ext = ctx + .state + .wallet + .public_descriptor(KeychainKind::External) + .to_string(); + let int = ctx + .state + .wallet + .public_descriptor(KeychainKind::Internal) + .to_string(); + // A single-descriptor wallet returns the same descriptor for both + // keychains. + let int = if int == ext { None } else { Some(int) }; + let policy = build_device_policy(&ext, int.as_deref())?; + + let hwi_wallet = HwiWallet { + name: Some(wallet_name), + policy: Some(&policy), + hmac: self.hmac.as_deref(), + }; + + match &self.subcommand { + WalletHwiSubCommand::Register => { + tracing::debug!("Registering wallet '{wallet_name}' with policy '{policy}'"); + let device = first_hwi_device(network, &hwi_wallet).await?; + match device.register_wallet(wallet_name, &policy).await { + Ok(hmac) => Ok(json!({ + "success": true, + "hmac": hmac.map(|h| h.to_lower_hex_string()), + })), + // Devices like Specter don't use BIP-388 policy registration. + Err(async_hwi::Error::UnimplementedMethod) => Ok(json!({ + "success": true, + "hmac": null, + "message": "This device does not require policy registration", + })), + Err(e) => Err(Error::HwiError(e)), + } } - HwiSubCommand::Address { index, change } => { - let opts = self.load_wallet_opts(&ctx.datadir)?; - let wallet = self.hwi_wallet(opts.as_ref()); - let device = first_hwi_device(network, &wallet).await?; + WalletHwiSubCommand::Address { index, change } => { + let device = first_hwi_device(network, &hwi_wallet).await?; let script = AddressScript::Miniscript { index: *index, change: *change, }; - device.display_address(&script).await?; - let address = self.derive_address(network, opts.as_ref(), *index, *change)?; + // Not every device can show an address on screen (e.g. Specter); + // fall back to the locally derived address instead of failing. + let displayed = match device.display_address(&script).await { + Ok(()) => true, + Err(async_hwi::Error::UnimplementedMethod) => false, + Err(e) => return Err(Error::HwiError(e)), + }; + let keychain = if *change { + KeychainKind::Internal + } else { + KeychainKind::External + }; + let address = ctx + .state + .wallet + .peek_address(keychain, *index) + .address + .to_string(); + let message = if displayed { + "Verify this address matches the one shown on your device" + } else { + "This device cannot display an address on screen; derived locally" + }; Ok(json!({ "success": true, "index": index, "change": change, "address": address, - "message": "Verify this address matches the one shown on your device", + "displayed_on_device": displayed, + "message": message, })) } - HwiSubCommand::Sign { psbt } => { + WalletHwiSubCommand::Sign { psbt } => { let mut psbt = Psbt::from_str(psbt) .map_err(|e| Error::Generic(format!("Failed to parse PSBT: {e}")))?; - let opts = self.load_wallet_opts(&ctx.datadir)?; - let wallet = self.hwi_wallet(opts.as_ref()); - let device = first_hwi_device(network, &wallet).await?; + let device = first_hwi_device(network, &hwi_wallet).await?; device.sign_tx(&mut psbt).await?; Ok(json!({ "psbt": psbt.to_string() })) } @@ -226,14 +233,61 @@ impl AsyncAppCommand> for HwiCommand { } } -/// Register a policy on the first connected device and return its HMAC (hex). -async fn device_register( - network: bdk_wallet::bitcoin::Network, - wallet: &HwiWallet<'_>, - name: &str, - policy: &str, -) -> Result, Error> { - let device = first_hwi_device(network, wallet).await?; - let hmac = device.register_wallet(name, policy).await?; - Ok(hmac.map(|h| h.to_lower_hex_string())) +/// Strip a trailing descriptor checksum. +fn strip_checksum(descriptor: &str) -> &str { + descriptor.split('#').next().unwrap_or(descriptor) +} + +/// Build the multipath wallet policy async-hwi expects from a wallet's external +/// and (optional) internal single-path public descriptors. +/// +/// - Rejects private-key material — a device must only ever see public keys. +/// - Passes an already-multipath descriptor (`/**` or `/<..>/*`) through +/// unchanged. +/// - Combines a standard `/0/*` external and `/1/*` internal pair into a single +/// `/<0;1>/*` multipath descriptor. +/// - With no internal descriptor, returns the external descriptor and warns +/// that only the receive chain is covered. +fn build_device_policy(ext: &str, int: Option<&str>) -> Result { + let ext = strip_checksum(ext).trim().to_string(); + + if ext.contains("xprv") || ext.contains("tprv") { + return Err(Error::Generic( + "Hardware wallet policy must use public keys (xpub), not private keys".to_string(), + )); + } + + // Already a multipath descriptor: use as-is. + if ext.contains("/**") || ext.contains('<') { + return Ok(ext); + } + + match int { + Some(int) => { + let int = strip_checksum(int).trim(); + if int.contains("xprv") || int.contains("tprv") { + 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 (key + // origins inside `[..]` are hardened and carry no `*`). + if ext.replace("/0/*", "/1/*") != int { + return Err(Error::Generic( + "Cannot derive a multipath policy: the wallet's external and internal \ + descriptors are not a standard `/0/*` and `/1/*` pair." + .to_string(), + )); + } + Ok(ext.replace("/0/*", "/<0;1>/*")) + } + None => { + tracing::warn!( + "Wallet has no separate internal (change) descriptor; the device policy will \ + only cover receive addresses." + ); + Ok(ext) + } + } } diff --git a/src/handlers/repl.rs b/src/handlers/repl.rs index b15b653..b93916e 100644 --- a/src/handlers/repl.rs +++ b/src/handlers/repl.rs @@ -86,6 +86,12 @@ pub(crate) async fn respond( .map_err(|e| e.to_string())?; Some(()) } + #[cfg(feature = "hwi")] + WalletSubCommand::Hwi(_) => { + writeln!(std::io::stdout(), "hwi is not available in REPL mode") + .map_err(|e| e.to_string())?; + Some(()) + } }, ReplSubCommand::Descriptor(cmd) => { diff --git a/src/main.rs b/src/main.rs index b0b528d..ebba66b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -115,6 +115,21 @@ async fn run(cli_opts: CliOpts) -> Result<(), Error> { config_cmd.execute(&mut ctx)?.write_out(std::io::stdout())?; } + #[cfg(feature = "hwi")] + WalletSubCommand::Hwi(cmd) => { + let runtime = WalletRuntime::load(&home_dir, &wallet_name)?; + let mut wallet = runtime.build_wallet(false)?; + + let mut ctx = AppContext::new_offline_wallet( + runtime.network, + runtime.home_dir.clone(), + &mut wallet, + ); + + cmd.run(&mut ctx, &wallet_name) + .await? + .write_out(std::io::stdout())?; + } }, CliSubCommand::Key { subcommand } => {