Merge #104: feat: add HWISigner, moved from bdk_hwi

7747b3f refactor(signer)!: add `signer` behind a non-default feature (Leonardo Lima)
b25168a feat: add `HWISigner`, moved from `bdk_hwi` (Leonardo Lima)

Pull request description:

  partially addresses bitcoindevkit/bdk#1516

  ## Description

  - adds a new `signer.rs` that contains the previous implementation of `HWISigner`, which implements `bdk_wallet::signer::{SignerCommon, TransactionSigner}` traits.
  - expose the new `signer::HWISigner` as public.
  - updates the crate documentation.
  - TODO: re-add test that relies on `bdk_wallet::tests::common::get_funded_wallet` helper methods.

  ## Notes for Reviewers

  I'm unsure if the documentation covers everything needed, please let me know if I'm missing something.

ACKs for top commit:
  notmandatory:
    ACK 7747b3f

Tree-SHA512: 8be7d681454eba31b5495dcf6cb9b399e2d992760f92f4ffc1d553908872b1272bb2e792ca608b679caf95b66126846bbb4dff37eb6f067a8e28d14454554098

.github/workflows/main.yml

@@ -42,9 +42,10 @@ jobs:
       matrix:
         rust:
           - version: stable # STABLE
-            features: miniscript
           - version: 1.63.0 # MSRV
-            features: miniscript
+        features:
+          - miniscript
+          - signer
         emulator:
           - name: trezor
           - name: ledger
@@ -77,7 +78,7 @@ jobs:
       - name: Update toolchain
         run: rustup update
       - name: Test
-        run: cargo test --features ${{ matrix.rust.features }}
+        run: cargo test --features ${{ matrix.features }}
       - name: Wipe
         run: cargo test test_wipe_device -- --ignored
   test-readme-examples:

Cargo.toml

@@ -12,14 +12,17 @@ readme = "README.md"
 
 [dependencies]
 bitcoin = { version = "0.32", features = ["serde", "base64"] }
+pyo3 = { version = "0.21.2", features = ["auto-initialize"] }
 serde = { version = "^1.0", features = ["derive"] }
 serde_json = { version = "^1.0" }
-pyo3 = { version = "0.21.2", features = ["auto-initialize"] }
 
+bdk_wallet = { version = "1.0.0-beta.1", optional = true }
 miniscript = { version = "12.0", features = ["serde"], optional = true }
 
 [dev-dependencies]
 serial_test = "0.6.0"
 
 [features]
 doctest = []
+signer = ["dep:bdk_wallet"]
+miniscript = ["dep:miniscript"]

src/lib.rs

@@ -1,6 +1,9 @@
-//! Rust wrapper for the [Bitcoin Hardware Wallet Interface](https://github.com/bitcoin-core/HWI/).
+//! This crate is contains both:
+//! - [`HWIClient`]: A Rust wrapper for the [Bitcoin Hardware Wallet Interface](https://github.com/bitcoin-core/HWI/).
+//! - [`HWISigner`]: An implementation of a [`TransactionSigner`] to be used with hardware wallets, that relies on [`HWIClient`].
 //!
-//! # Example - display address with path
+//! # HWIClient Example:
+//! ## Display address with path
 //! ```no_run
 //! use bitcoin::bip32::{ChildNumber, DerivationPath};
 //! use hwi::error::Error;
@@ -25,18 +28,62 @@
 //!     Ok(())
 //! }
 //! ```
+//!
+//! # HWISigner Example:
+//! ## Add custom [`HWISigner`] to [`Wallet`]
+//! ```no_run
+//! # #[cfg(feature = "signer")]
+//! # {
+//! use bdk_wallet::bitcoin::Network;
+//! use bdk_wallet::descriptor::Descriptor;
+//! use bdk_wallet::signer::SignerOrdering;
+//! use bdk_wallet::{KeychainKind, SignOptions, Wallet};
+//! use hwi::{HWIClient, HWISigner};
+//! use std::str::FromStr;
+//! use std::sync::Arc;
+//!
+//! fn main() -> Result<(), Box<dyn std::error::Error>> {
+//!     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::create("", "")
+//!         .network(Network::Testnet)
+//!         .create_wallet_no_persist()?;
+//!
+//!     // Adding the hardware signer to the BDK wallet
+//!     wallet.add_signer(
+//!         KeychainKind::External,
+//!         SignerOrdering(200),
+//!         Arc::new(custom_signer),
+//!     );
+//!
+//!     Ok(())
+//! }
+//! # }
+//! ```
+//!
+//! [`TransactionSigner`]: https://docs.rs/bdk_wallet/latest/bdk_wallet/signer/trait.TransactionSigner.html
+//! [`Wallet`]: https://docs.rs/bdk_wallet/1.0.0-beta.1/bdk_wallet/struct.Wallet.html
 
 #[cfg(test)]
 #[macro_use]
 extern crate serial_test;
 extern crate core;
 
 pub use interface::HWIClient;
+#[cfg(feature = "signer")]
+pub use signer::HWISigner;
 
 #[cfg(feature = "doctest")]
 pub mod doctest;
 pub mod error;
 pub mod interface;
+#[cfg(feature = "signer")]
+pub mod signer;
 pub mod types;
 
 #[cfg(test)]

src/signer.rs

@@ -0,0 +1,55 @@
+use bdk_wallet::bitcoin::bip32::Fingerprint;
+use bdk_wallet::bitcoin::secp256k1::{All, Secp256k1};
+use bdk_wallet::bitcoin::Psbt;
+
+use crate::error::Error;
+use crate::types::{HWIChain, HWIDevice};
+use crate::HWIClient;
+
+use bdk_wallet::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 an instance from the specified device and chain
+    pub fn from_device(device: &HWIDevice, chain: HWIChain) -> Result<HWISigner, Error> {
+        let client = HWIClient::get_client(device, false, chain)?;
+        Ok(HWISigner {
+            fingerprint: device.fingerprint,
+            client,
+        })
+    }
+}
+
+impl SignerCommon for HWISigner {
+    fn id(&self, _secp: &Secp256k1<All>) -> SignerId {
+        SignerId::Fingerprint(self.fingerprint)
+    }
+}
+
+impl TransactionSigner for HWISigner {
+    fn sign_transaction(
+        &self,
+        psbt: &mut Psbt,
+        _sign_options: &bdk_wallet::SignOptions,
+        _secp: &Secp256k1<All>,
+    ) -> 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(())
+    }
+}