From 105d70e9743f651d141c6e0d901d60d583afd4d9 Mon Sep 17 00:00:00 2001 From: Daniela Brozzoni Date: Mon, 9 Oct 2023 16:42:23 +0200 Subject: [PATCH] ref(hwi): Move hwi out of bdk Fixes #872 --- Cargo.toml | 1 + crates/bdk/Cargo.toml | 3 -- crates/bdk/src/lib.rs | 2 - crates/bdk/src/wallet/mod.rs | 4 -- crates/bdk/src/wallet/signer.rs | 18 +++---- crates/bdk/tests/wallet.rs | 35 ------------ crates/hwi/Cargo.toml | 13 +++++ crates/hwi/src/lib.rs | 42 +++++++++++++++ crates/hwi/src/signer.rs | 94 +++++++++++++++++++++++++++++++++ 9 files changed, 156 insertions(+), 56 deletions(-) create mode 100644 crates/hwi/Cargo.toml create mode 100644 crates/hwi/src/lib.rs create mode 100644 crates/hwi/src/signer.rs diff --git a/Cargo.toml b/Cargo.toml index 0e1efc902..e625d581f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,6 +7,7 @@ members = [ "crates/electrum", "crates/esplora", "crates/bitcoind_rpc", + "crates/hwi", "example-crates/example_cli", "example-crates/example_electrum", "example-crates/example_esplora", diff --git a/crates/bdk/Cargo.toml b/crates/bdk/Cargo.toml index cc53cc830..cb260fc45 100644 --- a/crates/bdk/Cargo.toml +++ b/crates/bdk/Cargo.toml @@ -21,7 +21,6 @@ serde_json = { version = "^1.0" } bdk_chain = { path = "../chain", version = "0.7.0", features = ["miniscript", "serde"], default-features = false } # Optional dependencies -hwi = { version = "0.7.0", optional = true, features = [ "miniscript"] } bip39 = { version = "2.0", optional = true } [target.'cfg(target_arch = "wasm32")'.dependencies] @@ -34,8 +33,6 @@ std = ["bitcoin/std", "miniscript/std", "bdk_chain/std"] compiler = ["miniscript/compiler"] all-keys = ["keys-bip39"] keys-bip39 = ["bip39"] -hardware-signer = ["hwi"] -test-hardware-signer = ["hardware-signer"] # This feature is used to run `cargo check` in our CI targeting wasm. It's not recommended # for libraries to explicitly include the "getrandom/js" feature, so we only do it when diff --git a/crates/bdk/src/lib.rs b/crates/bdk/src/lib.rs index 3b07b6e9f..f7c6f3549 100644 --- a/crates/bdk/src/lib.rs +++ b/crates/bdk/src/lib.rs @@ -17,8 +17,6 @@ extern crate std; pub extern crate alloc; pub extern crate bitcoin; -#[cfg(feature = "hardware-signer")] -pub extern crate hwi; pub extern crate miniscript; extern crate serde; extern crate serde_json; diff --git a/crates/bdk/src/wallet/mod.rs b/crates/bdk/src/wallet/mod.rs index 0380e02f8..ce152cb5e 100644 --- a/crates/bdk/src/wallet/mod.rs +++ b/crates/bdk/src/wallet/mod.rs @@ -50,10 +50,6 @@ pub mod tx_builder; pub(crate) mod utils; pub mod error; -#[cfg(feature = "hardware-signer")] -#[cfg_attr(docsrs, doc(cfg(feature = "hardware-signer")))] -pub mod hardwaresigner; - pub use utils::IsDust; #[allow(deprecated)] diff --git a/crates/bdk/src/wallet/signer.rs b/crates/bdk/src/wallet/signer.rs index 18e2062fb..2fe5804ce 100644 --- a/crates/bdk/src/wallet/signer.rs +++ b/crates/bdk/src/wallet/signer.rs @@ -80,6 +80,7 @@ //! ``` use crate::collections::BTreeMap; +use alloc::string::String; use alloc::sync::Arc; use alloc::vec::Vec; use core::cmp::Ordering; @@ -162,16 +163,10 @@ pub enum SignerError { SighashError(sighash::Error), /// Miniscript PSBT error MiniscriptPsbt(MiniscriptPsbtError), - /// Error while signing using hardware wallets - #[cfg(feature = "hardware-signer")] - HWIError(hwi::error::Error), -} - -#[cfg(feature = "hardware-signer")] -impl From for SignerError { - fn from(e: hwi::error::Error) -> Self { - SignerError::HWIError(e) - } + /// To be used only by external libraries implementing [`InputSigner`] or + /// [`TransactionSigner`], so that they can return their own custom errors, without having to + /// modify [`SignerError`] in BDK. + External(String), } impl From for SignerError { @@ -196,8 +191,7 @@ impl fmt::Display for SignerError { Self::InvalidSighash => write!(f, "Invalid SIGHASH for the signing context in use"), Self::SighashError(err) => write!(f, "Error while computing the hash to sign: {}", err), Self::MiniscriptPsbt(err) => write!(f, "Miniscript PSBT error: {}", err), - #[cfg(feature = "hardware-signer")] - Self::HWIError(err) => write!(f, "Error while signing using hardware wallets: {}", err), + Self::External(err) => write!(f, "{}", err), } } } diff --git a/crates/bdk/tests/wallet.rs b/crates/bdk/tests/wallet.rs index 12c442fe0..271b87163 100644 --- a/crates/bdk/tests/wallet.rs +++ b/crates/bdk/tests/wallet.rs @@ -3591,41 +3591,6 @@ fn test_fee_rate_sign_grinding_low_r() { assert_fee_rate!(psbt, fee.unwrap_or(0), fee_rate); } -// #[cfg(feature = "test-hardware-signer")] -// #[test] -// fn test_hardware_signer() { -// use std::sync::Arc; -// -// use bdk::signer::SignerOrdering; -// use bdk::wallet::hardwaresigner::HWISigner; -// use hwi::types::HWIChain; -// use hwi::HWIClient; -// -// let mut devices = HWIClient::enumerate().unwrap(); -// if devices.is_empty() { -// panic!("No devices found!"); -// } -// let device = devices.remove(0).unwrap(); -// let client = HWIClient::get_client(&device, true, HWIChain::Regtest).unwrap(); -// let descriptors = client.get_descriptors::(None).unwrap(); -// let custom_signer = HWISigner::from_device(&device, HWIChain::Regtest).unwrap(); -// -// let (mut wallet, _) = get_funded_wallet(&descriptors.internal[0]); -// wallet.add_signer( -// KeychainKind::External, -// SignerOrdering(200), -// Arc::new(custom_signer), -// ); -// -// let addr = wallet.get_address(LastUnused); -// let mut builder = wallet.build_tx(); -// builder.drain_to(addr.script_pubkey()).drain_wallet(); -// let (mut psbt, _) = builder.finish().unwrap(); -// -// let finalized = wallet.sign(&mut psbt, Default::default()).unwrap(); -// assert!(finalized); -// } - #[test] fn test_taproot_load_descriptor_duplicated_keys() { // Added after issue https://github.com/bitcoindevkit/bdk/issues/760 diff --git a/crates/hwi/Cargo.toml b/crates/hwi/Cargo.toml new file mode 100644 index 000000000..660b9deff --- /dev/null +++ b/crates/hwi/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "bdk_hwi" +version = "0.1.0" +edition = "2021" +homepage = "https://bitcoindevkit.org" +repository = "https://github.com/bitcoindevkit/bdk" +description = "Utilities to use bdk with hardware wallets" +license = "MIT OR Apache-2.0" +readme = "README.md" + +[dependencies] +bdk = { path = "../bdk" } +hwi = { version = "0.7.0", features = [ "miniscript"] } diff --git a/crates/hwi/src/lib.rs b/crates/hwi/src/lib.rs new file mode 100644 index 000000000..079066878 --- /dev/null +++ b/crates/hwi/src/lib.rs @@ -0,0 +1,42 @@ +//! HWI Signer +//! +//! This crate contains HWISigner, an implementation of a [`TransactionSigner`] to be +//! used with hardware wallets. +//! ```no_run +//! # use bdk::bitcoin::Network; +//! # use bdk::signer::SignerOrdering; +//! # use bdk_hwi::HWISigner; +//! # use bdk::wallet::AddressIndex::New; +//! # use bdk::{FeeRate, KeychainKind, SignOptions, Wallet}; +//! # use hwi::HWIClient; +//! # use std::sync::Arc; +//! # +//! # fn main() -> Result<(), Box> { +//! let mut devices = HWIClient::enumerate()?; +//! if devices.is_empty() { +//! panic!("No devices found!"); +//! } +//! let first_device = devices.remove(0)?; +//! let custom_signer = HWISigner::from_device(&first_device, Network::Testnet.into())?; +//! +//! # let mut wallet = Wallet::new_no_persist( +//! # "", +//! # None, +//! # Network::Testnet, +//! # )?; +//! # +//! // Adding the hardware signer to the BDK wallet +//! wallet.add_signer( +//! KeychainKind::External, +//! SignerOrdering(200), +//! Arc::new(custom_signer), +//! ); +//! +//! # Ok(()) +//! # } +//! ``` +//! +//! [`TransactionSigner`]: bdk::wallet::signer::TransactionSigner + +mod signer; +pub use signer::*; diff --git a/crates/hwi/src/signer.rs b/crates/hwi/src/signer.rs new file mode 100644 index 000000000..b16b60c54 --- /dev/null +++ b/crates/hwi/src/signer.rs @@ -0,0 +1,94 @@ +use bdk::bitcoin::bip32::Fingerprint; +use bdk::bitcoin::psbt::PartiallySignedTransaction; +use bdk::bitcoin::secp256k1::{All, Secp256k1}; + +use hwi::error::Error; +use hwi::types::{HWIChain, HWIDevice}; +use hwi::HWIClient; + +use bdk::signer::{SignerCommon, SignerError, SignerId, TransactionSigner}; + +#[derive(Debug)] +/// Custom signer for Hardware Wallets +/// +/// This ignores `sign_options` and leaves the decisions up to the hardware wallet. +pub struct HWISigner { + fingerprint: Fingerprint, + client: HWIClient, +} + +impl HWISigner { + /// Create a instance from the specified device and chain + pub fn from_device(device: &HWIDevice, chain: HWIChain) -> Result { + let client = HWIClient::get_client(device, false, chain)?; + Ok(HWISigner { + fingerprint: device.fingerprint, + client, + }) + } +} + +impl SignerCommon for HWISigner { + fn id(&self, _secp: &Secp256k1) -> SignerId { + SignerId::Fingerprint(self.fingerprint) + } +} + +impl TransactionSigner for HWISigner { + fn sign_transaction( + &self, + psbt: &mut PartiallySignedTransaction, + _sign_options: &bdk::SignOptions, + _secp: &Secp256k1, + ) -> Result<(), SignerError> { + psbt.combine( + self.client + .sign_tx(psbt) + .map_err(|e| { + SignerError::External(format!("While signing with hardware wallet: {}", e)) + })? + .psbt, + ) + .expect("Failed to combine HW signed psbt with passed PSBT"); + Ok(()) + } +} + +// TODO: re-enable this once we have the `get_funded_wallet` test util +// #[cfg(test)] +// mod tests { +// #[test] +// fn test_hardware_signer() { +// use std::sync::Arc; +// +// use bdk::tests::get_funded_wallet; +// use bdk::signer::SignerOrdering; +// use bdk::bitcoin::Network; +// use crate::HWISigner; +// use hwi::HWIClient; +// +// let mut devices = HWIClient::enumerate().unwrap(); +// if devices.is_empty() { +// panic!("No devices found!"); +// } +// let device = devices.remove(0).unwrap(); +// let client = HWIClient::get_client(&device, true, Network::Regtest.into()).unwrap(); +// let descriptors = client.get_descriptors::(None).unwrap(); +// let custom_signer = HWISigner::from_device(&device, Network::Regtest.into()).unwrap(); +// +// let (mut wallet, _) = get_funded_wallet(&descriptors.internal[0]); +// wallet.add_signer( +// bdk::KeychainKind::External, +// SignerOrdering(200), +// Arc::new(custom_signer), +// ); +// +// let addr = wallet.get_address(bdk::wallet::AddressIndex::LastUnused); +// let mut builder = wallet.build_tx(); +// builder.drain_to(addr.script_pubkey()).drain_wallet(); +// let (mut psbt, _) = builder.finish().unwrap(); +// +// let finalized = wallet.sign(&mut psbt, Default::default()).unwrap(); +// assert!(finalized); +// } +// }