Merge #41: Migrate to corepc for bitcoind integration tests

f8da0a4 Break out contributing docs (Nick Johnson)
8f88b0b Migrate to corepc for bitcoind integration tests (Nick Johnson)

Pull request description:

  Added a new CI job to run the integration tests, based it on rust-miniscript.

  Closes #29

ACKs for top commit:
  tcharding:
    ACK f8da0a4

Tree-SHA512: 7c61c372edd76ece8007cee3cce2543321ca12ab1cb64c3bcbe766084042527da0970c3a9d8c4e18f4220cdfcb6b404196f2b6572c9b8f515a244177ecc770f9

Author: tcharding

.github/workflows/rust.yml

@@ -151,3 +151,33 @@ jobs:
         run: rustup component add rustfmt
       - name: "Check formatting"
         run: cargo fmt --all -- --check
+
+  Integration:                  # 1 job for each bitcoind version we support.
+    name: Integration tests against bitcoind
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        feature:
+          [
+            "29_0",
+            "28_2",
+            "27_2",
+            "26_2",
+            "25_2",
+            "24_2",
+            "23_2",
+            "22_1",
+            "0_21_2",
+            "0_20_2",
+            "0_19_1",
+            "0_18_1",
+            "0_17_2",
+          ]
+    steps:
+      - name: "Checkout repo"
+        uses: actions/checkout@v4
+      - name: "Select toolchain"
+        uses: dtolnay/rust-toolchain@stable
+      - name: "Run integration tests"
+        run: cd bitcoind-tests && cargo test --features=${{ matrix.feature }}

CONTRIBUTING.md

@@ -0,0 +1,25 @@
+# Contributing
+
+We generally follow the contribution guidelines of [rust-bitcoin](https://github.com/rust-bitcoin/rust-bitcoin/blob/master/CONTRIBUTING.md).
+
+## Development Workflow
+
+We use [`just`](https://just.systems/man/en/) for running development workflow commands. Run `just` from your shell to see the list of available commands.
+
+### Git Hooks
+
+To catch errors before running CI, we provide git hooks. To use them:
+
+```bash
+git config --local core.hooksPath githooks/
+```
+
+Alternatively, add symlinks in your `.git/hooks` directory to any of the githooks we provide.
+
+## Integration Tests with Bitcoin Core
+
+The `bitcoind-tests/` package contains integration tests that run against real Bitcoin Core instances. A separate package is used so that bitcoind version flags don't pollute the rust-psbt crate. The package is not a member of the workspace so that it doesn't effect dependency version resolution.
+
+### NixOS Users
+
+The auto-downloaded Bitcoin Core binaries don't work on NixOS due to dynamic linking requirements. If you're on NixOS you could manually configure the `BITCOIND_EXE` environment variable to use a Nix-provided `bitcoind` of the correct version.

README.md

@@ -3,46 +3,16 @@
 Implementation of the Partially Signed Bitcoin Transaction Format as defined in [BIP-174] and
 PSBT version 2 as defined in [BIP-370].
 
-## Contributing
-
-For now we more or less just follow the contribution guidelines of 
-[rust-bitcoin](https://github.com/rust-bitcoin/rust-bitcoin/CONTRIBUTING.md).
-
-### Minimum Supported Rust Version (MSRV)
+## Minimum Supported Rust Version (MSRV)
 
 This library should always compile with any combination of features on **Rust 1.74.0**.
 
 To build with the MSRV you will likely need to pin a bunch of dependencies, see `./contrib/test.sh`
 for the current list.
 
-### Just
-
-We support [`just`](https://just.systems/man/en/) for running dev workflow commands. Run `just` from
-your shell to see list available sub-commands.
-
-### Building the docs
-
-We build docs with the nightly toolchain, you may wish to use the following shell alias to check
-your documentation changes build correctly.
-
-```
-alias build-docs='RUSTDOCFLAGS="--cfg docsrs" cargo +nightly rustdoc --features="$FEATURES" -- -D rustdoc::broken-intra-doc-links'
-```
-
-### Githooks
-
-To assist devs in catching errors _before_ running CI we provide some githooks. If you do not
-already have locally configured githooks you can use the ones in this repository by running, in the
-root directory of the repository:
-```
-git config --local core.hooksPath githooks/
-```
-
-Alternatively add symlinks in your `.git/hooks` directory to any of the githooks we provide.
-
-### rustfmt
+## Contributing
 
-We format with `cargo +nightly fmt`, see `./rusntfmt.toml` for the current configuration.
+Contributions are welcome! Please see [CONTRIBUTING.md](CONTRIBUTING.md) for guidelines.
 
 ## License
 

bitcoind-tests/Cargo.toml

@@ -4,12 +4,32 @@ version = "0.1.0"
 authors = ["Tobin C. Harding <me@tobin.cc>"]
 license = "CC0-1.0"
 edition = "2021"
+publish = false
 
 [dependencies]
 anyhow = "1"
-bitcoin = { version = "0.31.0", features = ["rand-std"] }
-bitcoind = { version = "0.34.1", features = ["25_1"] }
-secp256k1 = { version = "0.28.2", features = ["global-context", "rand-std"] }
+bitcoin = { version = "0.32.6", features = ["rand-std"] }
+bitcoind = { package = "corepc-node", version = "0.10.1", features = ["download"] }
+secp256k1 = { version = "0.29.0", features = ["global-context", "rand-std"] }
 psbt-v2 = { path = "..", features = ["std", "miniscript"] }
 
 [dev-dependencies]
+
+[features]
+# Bitcoin Core version features for integration testing.
+# corepc-node provides one feature per major version (latest patch release).
+# Bitcoin Core does not support PSBT v2 (BIP-370) yet.
+"29_0" = ["bitcoind/29_0"]
+"28_2" = ["bitcoind/28_2"]
+"27_2" = ["bitcoind/27_2"]
+"26_2" = ["bitcoind/26_2"]
+"25_2" = ["bitcoind/25_2"]
+"24_2" = ["bitcoind/24_2"]
+"23_2" = ["bitcoind/23_2"]
+"22_1" = ["bitcoind/22_1"]
+"0_21_2" = ["bitcoind/0_21_2"]
+"0_20_2" = ["bitcoind/0_20_2"]
+"0_19_1" = ["bitcoind/0_19_1"]
+"0_18_1" = ["bitcoind/0_18_1"]
+# Starting from 0.17.0 because that's when PSBT v0 (BIP-174) was first introduced.
+"0_17_2" = ["bitcoind/0_17_2"]

bitcoind-tests/src/client.rs

@@ -6,18 +6,15 @@
 
 // We depend upon and import directly from bitcoin because this module is not concerned with PSBT
 // i.e., it is lower down the stack than the psbt_v2 crate.
-use bitcoin::{consensus, Address, Amount, Network, Transaction, Txid};
-use bitcoind::bitcoincore_rpc::bitcoincore_rpc_json::{AddressType, GetBlockchainInfoResult};
-use bitcoind::bitcoincore_rpc::RpcApi;
-use bitcoind::BitcoinD;
+use bitcoin::{Address, Amount, Transaction, Txid};
+use bitcoind::{AddressType, Node, vtype::GetBlockchainInfo};
 
-const NETWORK: Network = Network::Regtest;
 const FIFTY_BTC: Amount = Amount::from_int_btc(50);
 
 /// A custom bitcoind client.
 pub struct Client {
     /// Handle for the regtest `bitcoind` instance.
-    bitcoind: BitcoinD,
+    bitcoind: Node,
     /// This is public so we don't have to handle the complexity of know if send/receives are
     /// to/from the Core controlled wallet or somewhere else. User is required to manage this.
     pub balance: BalanceTracker,
@@ -26,7 +23,8 @@ pub struct Client {
 impl Client {
     /// Creates a new [`Client`].
     pub fn new() -> anyhow::Result<Self> {
-        let bitcoind = BitcoinD::from_downloaded()?;
+        let exe_path = bitcoind::exe_path()?;
+        let bitcoind = Node::new(exe_path)?;
         let balance = BalanceTracker::zero();
 
         let client = Client { bitcoind, balance };
@@ -60,29 +58,28 @@ impl Client {
     }
 
     /// Calls through to bitcoincore_rpc client.
-    pub fn get_blockchain_info(&self) -> anyhow::Result<GetBlockchainInfoResult> {
+    pub fn get_blockchain_info(&self) -> anyhow::Result<GetBlockchainInfo> {
         let client = &self.bitcoind.client;
         Ok(client.get_blockchain_info()?)
     }
 
     /// Gets an address controlled by the currently loaded Bitcoin Core wallet (via `bitcoind`).
     pub fn core_wallet_controlled_address(&self) -> anyhow::Result<Address> {
         let client = &self.bitcoind.client;
-        let label = None;
-        let address_type = Some(AddressType::Bech32m);
-        let address = client.get_new_address(label, address_type)?.require_network(NETWORK)?;
+        // Use Bech32 (segwit v0) for compatibility with all Bitcoin Core versions.
+        // Bech32m (taproot) is only available from Bitcoin Core 22.0+.
+        let address = client.new_address_with_type(AddressType::Bech32)?;
         Ok(address)
     }
 
     pub fn balance(&self) -> anyhow::Result<Amount> {
         let client = &self.bitcoind.client;
-        let minconf = None; // What is this?
-        let include_watchonly = None;
-        Ok(client.get_balance(minconf, include_watchonly)?)
+        let balance = client.get_balance()?.balance()?;
+        Ok(balance)
     }
 
     /// Mines `n` blocks to a new address controlled by the currently loaded Bitcoin Core wallet.
-    fn mine_blocks(&self, n: u64) -> anyhow::Result<()> {
+    fn mine_blocks(&self, n: usize) -> anyhow::Result<()> {
         let client = &self.bitcoind.client;
         // Generate to an address controlled by the bitcoind wallet and wait for funds to mature.
         let address = self.core_wallet_controlled_address()?;
@@ -91,45 +88,25 @@ impl Client {
         Ok(())
     }
 
-    /// Send `amount` to `address` setting all other `bitcoincore_prc::send_to_address` args to `None`.
+    /// Send `amount` to `address`.
     ///
     /// Caller required to update balance (ie, call self.balance.send()).
     pub fn send(&self, amount: Amount, address: &Address) -> anyhow::Result<Txid> {
         let client = &self.bitcoind.client;
-
-        let comment = None;
-        let comment_to = None;
-        let subtract_fee = None;
-        let replacable = None;
-        let confirmation_target = None;
-        let estimate_mode = None;
-
-        let txid = client.send_to_address(
-            address,
-            amount,
-            comment,
-            comment_to,
-            subtract_fee,
-            replacable,
-            confirmation_target,
-            estimate_mode,
-        )?;
-
+        let txid = client.send_to_address(address, amount)?.txid()?;
         Ok(txid)
     }
 
     pub fn get_transaction(&self, txid: &Txid) -> anyhow::Result<Transaction> {
         let client = &self.bitcoind.client;
-        let include_watchonly = None;
-        let res = client.get_transaction(txid, include_watchonly)?;
-        let tx: Transaction = consensus::encode::deserialize(&res.hex)?;
+        let res = client.get_transaction(*txid)?;
+        let tx = res.into_model()?.tx;
         Ok(tx)
     }
 
     pub fn send_raw_transaction(&self, tx: &Transaction) -> anyhow::Result<Txid> {
         let client = &self.bitcoind.client;
-        let hex = consensus::encode::serialize_hex(&tx);
-        let txid = client.send_raw_transaction(hex)?;
+        let txid = client.send_raw_transaction(tx)?.txid()?;
         Ok(txid)
     }
 }

bitcoind-tests/tests/basic.rs

@@ -4,7 +4,7 @@ use core::str::FromStr;
 
 // Only depend on `psbt` (and `bitcoind_tests`) because we are explicitly testing the `psbt_v2` crate.
 use bitcoind_tests::client::Client;
-use psbt::bitcoin::{Address, Amount, Network, OutPoint, PublicKey, Script, Transaction, TxOut};
+use psbt::bitcoin::{Address, Amount, CompressedPublicKey, Network, OutPoint, Script, Transaction, TxOut};
 use psbt::v2::{Constructor, InputBuilder, Modifiable, OutputBuilder};
 // The `psbt_v2` crate, as we expect downstream to use it
 // E.g., in manifest file `use psbt = { package = "psbt_v2" ... }`
@@ -74,7 +74,7 @@ impl TransactionExt for Transaction {
         let mut utxos = vec![];
         for (index, utxo) in self.output.iter().enumerate() {
             if &utxo.script_pubkey == script_pubkey {
-                let out_point = OutPoint { txid: self.txid(), vout: index as u32 };
+                let out_point = OutPoint { txid: self.compute_txid(), vout: index as u32 };
 
                 utxos.push((out_point, utxo));
             }
@@ -85,24 +85,24 @@ impl TransactionExt for Transaction {
 
 /// A super basic entity with a single public key.
 pub struct Alice {
-    /// The single public key.
-    public_key: PublicKey,
+    /// The single compressed public key.
+    public_key: CompressedPublicKey,
 }
 
 impl Alice {
     /// Creates a new Alice.
     pub fn new() -> Self {
         // An arbitrary public key, assume the secret key is held by another entity.
-        let public_key = PublicKey::from_str(
+        let public_key = CompressedPublicKey::from_str(
             "032e58afe51f9ed8ad3cc7897f634d881fdbe49a81564629ded8156bebd2ffd1af",
         )
         .unwrap();
 
         Alice { public_key }
     }
 
-    /// Returns a bech32m address from a key Alice controls.
+    /// Returns a bech32 address from a key Alice controls.
     pub fn address(&self) -> Address {
-        Address::p2wpkh(&self.public_key, NETWORK).expect("uncompressed key")
+        Address::p2wpkh(&self.public_key, NETWORK)
     }
 }

justfile

@@ -16,10 +16,14 @@ check:
 build:
   cargo build --all --all-targets --all-features
 
-# Cargo test everything.
+# Test everything.
 test:
   cargo test --all-targets --all-features
-  cd bitcoind-tests; cargo test
+  just test-bitcoind
+
+# Test bitcoind integration with a bitcoind version.
+test-bitcoind version="29_0":
+  cd {{justfile_directory()}}/bitcoind-tests && cargo test --features={{version}}
 
 # Lint everything.
 lint: