refactor(hwi,wallet): enhance hardware wallet integration and testing

crates/hwi/Cargo.toml

@@ -11,3 +11,6 @@ readme = "README.md"
 [dependencies]
 bdk_wallet = { path = "../wallet", version = "1.0.0-alpha.13" }
 hwi = { version = "0.9.0", features = [ "miniscript"] }
+
+[dev-dependencies]
+bdk_wallet = { path = "../wallet", version = "1.0.0-alpha.13", features = ["test-util"] }

crates/hwi/src/signer.rs

@@ -1,13 +1,11 @@
 use bdk_wallet::bitcoin::bip32::Fingerprint;
 use bdk_wallet::bitcoin::secp256k1::{All, Secp256k1};
 use bdk_wallet::bitcoin::Psbt;
-
+use bdk_wallet::signer::{SignerCommon, SignerError, SignerId, TransactionSigner};
 use hwi::error::Error;
 use hwi::types::{HWIChain, HWIDevice};
 use hwi::HWIClient;
 
-use bdk_wallet::signer::{SignerCommon, SignerError, SignerId, TransactionSigner};
-
 #[derive(Debug)]
 /// Custom signer for Hardware Wallets
 ///
@@ -41,54 +39,87 @@ impl TransactionSigner for HWISigner {
         _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");
+        let signed_psbt = self
+            .client
+            .sign_tx(psbt)
+            .map_err(|e| {
+                SignerError::External(format!("While signing with hardware wallet: {}", e))
+            })?
+            .psbt;
+
+        psbt.combine(signed_psbt).map_err(|e| {
+            SignerError::External(format!(
+                "Failed to combine HW signed PSBT with passed PSBT: {}",
+                e
+            ))
+        })?;
+
         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_wallet::tests::get_funded_wallet;
-//         use bdk_wallet::signer::SignerOrdering;
-//         use bdk_wallet::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::<String>(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_wallet::KeychainKind::External,
-//             SignerOrdering(200),
-//             Arc::new(custom_signer),
-//         );
-//
-//         let addr = wallet.get_address(bdk_wallet::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);
-//     }
-// }
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use bdk_wallet::bitcoin::Network;
+    use bdk_wallet::signer::SignerOrdering;
+    // use bdk_wallet::wallet::common::get_funded_wallet;
+
+    use bdk_wallet::wallet::test_util::get_funded_wallet;
+    // use bdk_wallet::wallet::AddressIndex;
+    use bdk_wallet::{wallet, KeychainKind};
+    use std::sync::Arc;
+
+    #[test]
+    fn test_hardware_signer() {
+        let mut devices = match HWIClient::enumerate() {
+            Ok(devices) => devices,
+            Err(e) => panic!("Failed to enumerate devices: {}", e),
+        };
+
+        if devices.is_empty() {
+            panic!("No devices found!");
+        }
+
+        let device = match devices.remove(0) {
+            Ok(device) => device,
+            Err(e) => panic!("Failed to remove device: {}", e),
+        };
+
+        let client = match HWIClient::get_client(&device, true, Network::Regtest.into()) {
+            Ok(client) => client,
+            Err(e) => panic!("Failed to get client: {}", e),
+        };
+
+        let descriptors = match client.get_descriptors::<String>(None) {
+            Ok(descriptors) => descriptors,
+            Err(e) => panic!("Failed to get descriptors: {}", e),
+        };
+
+        let custom_signer = match HWISigner::from_device(&device, Network::Regtest.into()) {
+            Ok(signer) => signer,
+            Err(e) => panic!("Failed to create HWISigner: {}", e),
+        };
+
+        let (mut wallet, _) = get_funded_wallet(&descriptors.internal[0]);
+
+        wallet.add_signer(
+            KeychainKind::External,
+            SignerOrdering(200),
+            Arc::new(custom_signer),
+        );
+
+        let addr = wallet
+            // ,(AddressIndex::LastUnused)
+            .peek_address(KeychainKind::External, 0);
+
+        let mut builder = wallet.build_tx();
+        builder.drain_to(addr.script_pubkey()).drain_wallet();
+        let mut psbt = builder.finish().expect("Failed to build transaction");
+
+        let finalized = wallet
+            .sign(&mut psbt, Default::default())
+            .expect("Failed to sign transaction");
+        assert!(finalized);
+    }
+}

crates/wallet/Cargo.toml

@@ -29,6 +29,7 @@ std = ["bitcoin/std", "bitcoin/rand-std", "miniscript/std", "bdk_chain/std"]
 compiler = ["miniscript/compiler"]
 all-keys = ["keys-bip39"]
 keys-bip39 = ["bip39"]
+test-util = []
 
 [dev-dependencies]
 lazy_static = "1.4"

crates/wallet/src/wallet/mod.rs

@@ -55,6 +55,8 @@ use bdk_chain::tx_graph::CalculateFeeError;
 pub mod coin_selection;
 pub mod export;
 pub mod signer;
+#[cfg(any(test, feature = "test-util"))]
+pub mod test_util;
 pub mod tx_builder;
 pub(crate) mod utils;
 

crates/wallet/tests/common.rs → crates/wallet/src/wallet/test_util.rs

@@ -1,6 +1,22 @@
+// Bitcoin Dev Kit
+// Written in 2020 by Alekos Filini <[email protected]>
+//
+// Copyright (c) 2020-2021 Bitcoin Dev Kit Developers
+//
+// This file is licensed under the Apache License, Version 2.0 <LICENSE-APACHE
+// or http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your option.
+// You may not use this file except in accordance with one or both of these
+// licenses.
+
+//! Test util
+//!
+//! Test utilities for the wallet module. Only enabled while testing or when the
+//! `test-utils` feature is enabled.
+
 #![allow(unused)]
 use bdk_chain::{BlockId, ConfirmationBlockTime, ConfirmationTime, TxGraph};
-use bdk_wallet::{
+use crate::{
     wallet::{Update, Wallet},
     KeychainKind, LocalOutput,
 };

crates/wallet/tests/psbt.rs

@@ -1,8 +1,8 @@
 use bdk_wallet::bitcoin::{Amount, FeeRate, Psbt, TxIn};
 use bdk_wallet::{psbt, KeychainKind, SignOptions};
 use core::str::FromStr;
-mod common;
-use common::*;
+
+use bdk_wallet::wallet::test_util::*;
 
 // from bip 174
 const PSBT_STR: &str = "cHNidP8BAKACAAAAAqsJSaCMWvfEm4IS9Bfi8Vqz9cM9zxU4IagTn4d6W3vkAAAAAAD+////qwlJoIxa98SbghL0F+LxWrP1wz3PFTghqBOfh3pbe+QBAAAAAP7///8CYDvqCwAAAAAZdqkUdopAu9dAy+gdmI5x3ipNXHE5ax2IrI4kAAAAAAAAGXapFG9GILVT+glechue4O/p+gOcykWXiKwAAAAAAAEHakcwRAIgR1lmF5fAGwNrJZKJSGhiGDR9iYZLcZ4ff89X0eURZYcCIFMJ6r9Wqk2Ikf/REf3xM286KdqGbX+EhtdVRs7tr5MZASEDXNxh/HupccC1AaZGoqg7ECy0OIEhfKaC3Ibi1z+ogpIAAQEgAOH1BQAAAAAXqRQ1RebjO4MsRwUPJNPuuTycA5SLx4cBBBYAFIXRNTfy4mVAWjTbr6nj3aAfuCMIAAAA";

crates/wallet/tests/wallet.rs

@@ -29,8 +29,7 @@ use bitcoin::{
 use rand::rngs::StdRng;
 use rand::SeedableRng;
 
-mod common;
-use common::*;
+use bdk_wallet::wallet::test_util::*;
 
 fn receive_output(wallet: &mut Wallet, value: u64, height: ConfirmationTime) -> OutPoint {
     let addr = wallet.next_unused_address(KeychainKind::External).address;