Merge #76: Upgrade bitcoin and miniscript

f720747 Remove base64 dependency (Tobin C. Harding)
1ce33fa Upgrade bitcoin and miniscript (Tobin C. Harding)

Pull request description:

  This is my first PR to this project.

  - Patch 1 does the upgrade to bitcoin v0.30.0 and miniscript 0.10.0
  - Patch 2 does a little clean up, removing `base64` dependency in favour of `bitcoin::base64`

  Note please, this makes the following API changes:

  - `Network` is now `non_exhaustive` so we implement `TryFrom` instead of `From`. Also add panic in `IntoPy`, we already unwrap so it seems ok, please note.
  - `get_client`: Now takes an `HWIChain` instead of a generic, this is less general, and less usable. Done because `From<Network>` can no longer be implemented on `HWIChain` because of the newly added `non_exhaustive` on `Network` (added a test const `TESTNET` for `HWIChain(Network::Testnet)`)
  - `Address` now has a tag and can only be deserialized unchecked. We just call `assume_checked` in the unit tests.

  Is the error handling for new `TryFrom` acceptable to the project?

ACKs for top commit:
  danielabrozzoni:
    ACK f720747

Tree-SHA512: 672f0e193f17151e53387acfbc7bbf7be4f08b381e69b7896064f3781764fc98d42d27e7e568b126f7140aa01c9770a8c0638edbc287d03f848ad595775e19c1

Author: danielabrozzoni

Cargo.toml

@@ -11,13 +11,12 @@ readme = "README.md"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-bitcoin = { version = "0.29.1", features = ["serde", "base64"] }
+bitcoin = { version = "0.30.0", features = ["serde", "base64"] }
 serde = { version = "^1.0", features = ["derive"] }
 serde_json = { version = "^1.0" }
 pyo3 = { version = "0.15.1", features = ["auto-initialize"] }
-base64 = "0.13.0"
 
-miniscript = { version = "9.0", features = ["serde"], optional = true }
+miniscript = { version = "10.0", features = ["serde"], optional = true }
 
 [dev-dependencies]
 serial_test = "0.6.0"

README.md

@@ -55,7 +55,7 @@ pip install -r requirements.txt
 
 ```rust
 use bitcoin::Network;
-use bitcoin::util::bip32::DerivationPath;
+use bitcoin::bip32::DerivationPath;
 use hwi::error::Error;
 use hwi::HWIClient;
 use std::str::FromStr;
@@ -66,7 +66,7 @@ fn main() -> Result<(), Error> {
         panic!("No devices found!");
     }
     let first_device = devices.remove(0)?;
-    let client = HWIClient::get_client(&first_device, true, Network::Testnet)?;
+    let client = HWIClient::get_client(&first_device, true, Network::Bitcoin.into())?;
     let derivation_path = DerivationPath::from_str("m/44'/1'/0'/0/0").unwrap();
     let s = client.sign_message("I love BDK wallet", &derivation_path)?;
     println!("{:?}", s.signature);

src/interface.rs

@@ -2,11 +2,8 @@ use std::convert::TryInto;
 use std::ops::Deref;
 use std::process::Command;
 
-use bitcoin::consensus::encode::serialize;
-use bitcoin::util::bip32::DerivationPath;
-use bitcoin::util::psbt::PartiallySignedTransaction;
-
-use base64;
+use bitcoin::bip32::DerivationPath;
+use bitcoin::psbt::PartiallySignedTransaction;
 
 use serde::de::DeserializeOwned;
 use serde_json::value::Value;
@@ -101,7 +98,7 @@ impl HWIClient {
     /// let devices = HWIClient::enumerate()?;
     /// for device in devices {
     ///     let device = device?;
-    ///     let client = HWIClient::get_client(&device, false, bitcoin::Network::Testnet)?;
+    ///     let client = HWIClient::get_client(&device, false, bitcoin::Network::Testnet.into())?;
     ///     let xpub = client.get_master_xpub(HWIAddressType::Tap, 0)?;
     ///     println!(
     ///         "I can see a {} here, and its xpub is {}",
@@ -112,18 +109,19 @@ impl HWIClient {
     /// # Ok(())
     /// # }
     /// ```
-    pub fn get_client<C>(device: &HWIDevice, expert: bool, chain: C) -> Result<HWIClient, Error>
-    where
-        C: Into<HWIChain>,
-    {
+    pub fn get_client(
+        device: &HWIDevice,
+        expert: bool,
+        chain: HWIChain,
+    ) -> Result<HWIClient, Error> {
         let libs = HWILib::initialize()?;
         Python::with_gil(|py| {
             let client_args = (
                 device.device_type.to_string(),
                 &device.path,
                 "",
                 expert,
-                chain.into(),
+                chain,
             );
             let client = libs
                 .commands
@@ -211,13 +209,12 @@ impl HWIClient {
         &self,
         psbt: &PartiallySignedTransaction,
     ) -> Result<HWIPartiallySignedTransaction, Error> {
-        let psbt = base64::encode(serialize(psbt));
         Python::with_gil(|py| {
             let output = self
                 .hwilib
                 .commands
                 .getattr(py, "signtx")?
-                .call1(py, (&self.hw_client, psbt))?;
+                .call1(py, (&self.hw_client, psbt.to_string()))?;
             let output = self.hwilib.json_dumps.call1(py, (output,))?;
             deserialize_obj!(&output.to_string())
         })

src/lib.rs

@@ -2,7 +2,7 @@
 //!
 //! # Example - display address with path
 //! ```no_run
-//! use bitcoin::util::bip32::{ChildNumber, DerivationPath};
+//! use bitcoin::bip32::{ChildNumber, DerivationPath};
 //! use hwi::error::Error;
 //! use hwi::interface::HWIClient;
 //! use hwi::types;
@@ -16,12 +16,12 @@
 //!     }
 //!     let device = devices.remove(0)?;
 //!     // Create a client for a device
-//!     let client = HWIClient::get_client(&device, true, bitcoin::Network::Testnet)?;
+//!     let client = HWIClient::get_client(&device, true, bitcoin::Network::Testnet.into())?;
 //!     // Display the address from path
 //!     let derivation_path = DerivationPath::from_str("m/44'/1'/0'/0/0").unwrap();
 //!     let hwi_address =
 //!         client.display_address_with_path(&derivation_path, types::HWIAddressType::Tap)?;
-//!     println!("{}", hwi_address.address);
+//!     println!("{}", hwi_address.address.assume_checked());
 //!     Ok(())
 //! }
 //! ```
@@ -40,13 +40,14 @@ pub mod types;
 
 #[cfg(test)]
 mod tests {
-    use crate::types::{self, HWIDeviceType};
+    use crate::types::{self, HWIDeviceType, TESTNET};
     use crate::HWIClient;
     use std::collections::BTreeMap;
     use std::str::FromStr;
 
+    use bitcoin::bip32::{DerivationPath, KeySource};
+    use bitcoin::locktime::absolute;
     use bitcoin::psbt::{Input, Output};
-    use bitcoin::util::bip32::{DerivationPath, KeySource};
     use bitcoin::{secp256k1, Transaction};
     use bitcoin::{Network, TxIn, TxOut};
 
@@ -81,7 +82,7 @@ mod tests {
             .expect("No devices found. Either plug in a hardware wallet, or start a simulator.")
             .as_ref()
             .expect("Error when opening the first device");
-        HWIClient::get_client(&device, true, Network::Testnet).unwrap()
+        HWIClient::get_client(&device, true, TESTNET).unwrap()
     }
 
     #[test]
@@ -197,7 +198,7 @@ mod tests {
     fn test_sign_tx() {
         let devices = HWIClient::enumerate().unwrap();
         let device = devices.first().unwrap().as_ref().unwrap();
-        let client = HWIClient::get_client(&device, true, Network::Testnet).unwrap();
+        let client = HWIClient::get_client(&device, true, TESTNET).unwrap();
         let derivation_path = DerivationPath::from_str("m/44'/1'/0'/0/0").unwrap();
 
         let address = client
@@ -209,13 +210,15 @@ mod tests {
         // Here device fingerprint is same as master xpub fingerprint
         hd_keypaths.insert(pk.public_key, (device.fingerprint, derivation_path));
 
+        let script_pubkey = address.address.assume_checked().script_pubkey();
+
         let previous_tx = Transaction {
             version: 1,
-            lock_time: bitcoin::PackedLockTime(0),
+            lock_time: absolute::LockTime::from_consensus(0),
             input: vec![TxIn::default()],
             output: vec![TxOut {
                 value: 100,
-                script_pubkey: address.address.script_pubkey(),
+                script_pubkey: script_pubkey.clone(),
             }],
         };
 
@@ -229,11 +232,11 @@ mod tests {
         let psbt = bitcoin::psbt::PartiallySignedTransaction {
             unsigned_tx: Transaction {
                 version: 1,
-                lock_time: bitcoin::PackedLockTime(0),
+                lock_time: absolute::LockTime::from_consensus(0),
                 input: vec![previous_txin],
                 output: vec![TxOut {
                     value: 50,
-                    script_pubkey: address.address.script_pubkey(),
+                    script_pubkey: script_pubkey,
                 }],
             },
             xpub: Default::default(),
@@ -348,7 +351,7 @@ mod tests {
                 // These devices don't support togglepassphrase
                 continue;
             }
-            let client = HWIClient::get_client(&device, true, Network::Testnet).unwrap();
+            let client = HWIClient::get_client(&device, true, TESTNET).unwrap();
             client.toggle_passphrase().unwrap();
             break;
         }
@@ -404,7 +407,7 @@ mod tests {
         for device in devices {
             let device = device.unwrap();
             if supported.contains(&device.device_type) {
-                let client = HWIClient::get_client(&device, true, Network::Testnet).unwrap();
+                let client = HWIClient::get_client(&device, true, TESTNET).unwrap();
                 client.backup_device(Some("My Label"), None).unwrap();
             }
         }
@@ -426,7 +429,7 @@ mod tests {
                 // These devices don't support wipe
                 continue;
             }
-            let client = HWIClient::get_client(&device, true, Network::Testnet).unwrap();
+            let client = HWIClient::get_client(&device, true, TESTNET).unwrap();
             client.wipe_device().unwrap();
         }
     }

src/types.rs

@@ -1,8 +1,11 @@
 use std::convert::TryFrom;
 use std::ops::Deref;
+use std::str::FromStr;
 
-use bitcoin::util::bip32::{ExtendedPubKey, Fingerprint};
-use bitcoin::util::{address::Address, psbt::PartiallySignedTransaction};
+use bitcoin::address::{Address, NetworkUnchecked};
+use bitcoin::base64;
+use bitcoin::bip32::{ExtendedPubKey, Fingerprint};
+use bitcoin::psbt::PartiallySignedTransaction;
 use bitcoin::Network;
 
 use pyo3::types::PyModule;
@@ -49,7 +52,7 @@ impl Deref for HWISignature {
 
 #[derive(Clone, Eq, PartialEq, Debug, Deserialize)]
 pub struct HWIAddress {
-    pub address: Address,
+    pub address: Address<NetworkUnchecked>,
 }
 
 #[derive(Clone, Eq, PartialEq, Debug, Deserialize)]
@@ -61,10 +64,8 @@ pub struct HWIPartiallySignedTransaction {
 fn deserialize_psbt<'de, D: Deserializer<'de>>(
     d: D,
 ) -> Result<PartiallySignedTransaction, D::Error> {
-    let b64_string = String::deserialize(d)?;
-    let bytes = base64::decode(b64_string).map_err(serde::de::Error::custom)?;
-    bitcoin::consensus::deserialize::<PartiallySignedTransaction>(&bytes[..])
-        .map_err(serde::de::Error::custom)
+    let s = String::deserialize(d)?;
+    PartiallySignedTransaction::from_str(&s).map_err(serde::de::Error::custom)
 }
 
 impl Deref for HWIPartiallySignedTransaction {
@@ -139,6 +140,10 @@ impl IntoPy<PyObject> for HWIChain {
             Testnet => chain.get_item("TEST").unwrap().into(),
             Regtest => chain.get_item("REGTEST").unwrap().into(),
             Signet => chain.get_item("SIGNET").unwrap().into(),
+            // This handles non_exhaustive on Network which is only there to future proof
+            // rust-bitcoin, will need to check this when upgrading rust-bitcoin.
+            // Sane as of rust-bitcoin v0.30.0
+            _ => panic!("unknown network"),
         }
     }
 }
@@ -149,6 +154,9 @@ impl From<Network> for HWIChain {
     }
 }
 
+#[cfg(test)]
+pub const TESTNET: HWIChain = HWIChain(Network::Testnet);
+
 // Used internally to deserialize the result of `hwi enumerate`. This might
 // contain an `error`, when it does, it might not contain all the fields `HWIDevice`
 // is supposed to have - for this reason, they're all Option.