From e1a1372baec86da784af697e025615b8e0bccf4e Mon Sep 17 00:00:00 2001 From: Alekos Filini Date: Tue, 7 Jun 2022 13:14:52 +0200 Subject: [PATCH] rpc: use `importdescriptors` with Core >= 0.21 Only use the old `importmulti` with Core versions that don't support descriptor-based (sqlite) wallets. Add an extra feature to test against Core 0.20 called `test-rpc-legacy` --- .github/workflows/cont_integration.yml | 3 + CHANGELOG.md | 1 + Cargo.toml | 9 +- README.md | 2 +- src/blockchain/rpc.rs | 117 +++++++++++++++++++------ src/testutils/blockchain_tests.rs | 16 +++- 6 files changed, 113 insertions(+), 35 deletions(-) diff --git a/.github/workflows/cont_integration.yml b/.github/workflows/cont_integration.yml index 4a7c2ea95..b9f7aece9 100644 --- a/.github/workflows/cont_integration.yml +++ b/.github/workflows/cont_integration.yml @@ -95,6 +95,9 @@ jobs: - name: rpc testprefix: blockchain::rpc::test features: test-rpc + - name: rpc-legacy + testprefix: blockchain::rpc::test + features: test-rpc-legacy - name: esplora testprefix: esplora features: test-esplora,use-esplora-reqwest,verify diff --git a/CHANGELOG.md b/CHANGELOG.md index 18a5ae06c..73188e107 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Creation of Taproot PSBTs (BIP-371) - Signing Taproot PSBTs (key spend and script spend) - Support for `tr()` descriptors in the `descriptor!()` macro +- Add support for Bitcoin Core 23.0 when using the `rpc` blockchain ## [v0.18.0] - [v0.17.0] diff --git a/Cargo.toml b/Cargo.toml index ddbc46124..d67b2b8d4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -88,16 +88,17 @@ reqwest-default-tls = ["reqwest/default-tls"] # Debug/Test features test-blockchains = ["bitcoincore-rpc", "electrum-client"] -test-electrum = ["electrum", "electrsd/electrs_0_8_10", "test-blockchains"] -test-rpc = ["rpc", "electrsd/electrs_0_8_10", "test-blockchains"] -test-esplora = ["electrsd/legacy", "electrsd/esplora_a33e97e1", "test-blockchains"] +test-electrum = ["electrum", "electrsd/electrs_0_8_10", "electrsd/bitcoind_22_0", "test-blockchains"] +test-rpc = ["rpc", "electrsd/electrs_0_8_10", "electrsd/bitcoind_22_0", "test-blockchains"] +test-rpc-legacy = ["rpc", "electrsd/electrs_0_8_10", "electrsd/bitcoind_0_20_0", "test-blockchains"] +test-esplora = ["electrsd/legacy", "electrsd/esplora_a33e97e1", "electrsd/bitcoind_22_0", "test-blockchains"] test-md-docs = ["electrum"] [dev-dependencies] lazy_static = "1.4" env_logger = "0.7" clap = "2.33" -electrsd = { version= "0.19.1", features = ["bitcoind_22_0"] } +electrsd = "0.19.1" [[example]] name = "address_validator" diff --git a/README.md b/README.md index b5007de7b..54832afcd 100644 --- a/README.md +++ b/README.md @@ -166,7 +166,7 @@ Integration testing require testing features, for example: cargo test --features test-electrum ``` -The other options are `test-esplora` or `test-rpc`. +The other options are `test-esplora`, `test-rpc` or `test-rpc-legacy` which runs against an older version of Bitcoin Core. Note that `electrs` and `bitcoind` binaries are automatically downloaded (on mac and linux), to specify you already have installed binaries you must use `--no-default-features` and provide `BITCOIND_EXE` and `ELECTRS_EXE` as environment variables. ## License diff --git a/src/blockchain/rpc.rs b/src/blockchain/rpc.rs index 7eb059204..d1366368d 100644 --- a/src/blockchain/rpc.rs +++ b/src/blockchain/rpc.rs @@ -32,15 +32,17 @@ //! ``` use crate::bitcoin::consensus::deserialize; +use crate::bitcoin::hashes::hex::ToHex; use crate::bitcoin::{Address, Network, OutPoint, Transaction, TxOut, Txid}; use crate::blockchain::*; use crate::database::{BatchDatabase, DatabaseUtils}; +use crate::descriptor::get_checksum; use crate::{BlockTime, Error, FeeRate, KeychainKind, LocalUtxo, TransactionDetails}; use bitcoincore_rpc::json::{ GetAddressInfoResultLabel, ImportMultiOptions, ImportMultiRequest, ImportMultiRequestScriptPubkey, ImportMultiRescanSince, }; -use bitcoincore_rpc::jsonrpc::serde_json::Value; +use bitcoincore_rpc::jsonrpc::serde_json::{json, Value}; use bitcoincore_rpc::Auth as RpcAuth; use bitcoincore_rpc::{Client, RpcApi}; use log::debug; @@ -54,6 +56,8 @@ use std::str::FromStr; pub struct RpcBlockchain { /// Rpc client to the node, includes the wallet name client: Client, + /// Whether the wallet is a "descriptor" or "legacy" wallet in Core + is_descriptors: bool, /// Blockchain capabilities, cached here at startup capabilities: HashSet, /// Skip this many blocks of the blockchain at the first rescan, if None the rescan is done from the genesis block @@ -177,22 +181,53 @@ impl WalletSync for RpcBlockchain { "importing {} script_pubkeys (some maybe already imported)", scripts_pubkeys.len() ); - let requests: Vec<_> = scripts_pubkeys - .iter() - .map(|s| ImportMultiRequest { - timestamp: ImportMultiRescanSince::Timestamp(0), - script_pubkey: Some(ImportMultiRequestScriptPubkey::Script(s)), - watchonly: Some(true), - ..Default::default() - }) - .collect(); - let options = ImportMultiOptions { - rescan: Some(false), - }; - // Note we use import_multi because as of bitcoin core 0.21.0 many descriptors are not supported - // https://bitcoindevkit.org/descriptors/#compatibility-matrix - //TODO maybe convenient using import_descriptor for compatible descriptor and import_multi as fallback - self.client.import_multi(&requests, Some(&options))?; + + if self.is_descriptors { + // Core still doesn't support complex descriptors like BDK, but when the wallet type is + // "descriptors" we should import individual addresses using `importdescriptors` rather + // than `importmulti`, using the `raw()` descriptor which allows us to specify an + // arbitrary script + let requests = Value::Array( + scripts_pubkeys + .iter() + .map(|s| { + let desc = format!("raw({})", s.to_hex()); + json!({ + "timestamp": "now", + "desc": format!("{}#{}", desc, get_checksum(&desc).unwrap()), + }) + }) + .collect(), + ); + + let res: Vec = self.client.call("importdescriptors", &[requests])?; + res.into_iter() + .map(|v| match v["success"].as_bool() { + Some(true) => Ok(()), + Some(false) => Err(Error::Generic( + v["error"]["message"] + .as_str() + .unwrap_or("Unknown error") + .to_string(), + )), + _ => Err(Error::Generic("Unexpected response from Core".to_string())), + }) + .collect::, _>>()?; + } else { + let requests: Vec<_> = scripts_pubkeys + .iter() + .map(|s| ImportMultiRequest { + timestamp: ImportMultiRescanSince::Timestamp(0), + script_pubkey: Some(ImportMultiRequestScriptPubkey::Script(s)), + watchonly: Some(true), + ..Default::default() + }) + .collect(); + let options = ImportMultiOptions { + rescan: Some(false), + }; + self.client.import_multi(&requests, Some(&options))?; + } loop { let current_height = self.get_height()?; @@ -369,20 +404,36 @@ impl ConfigurableBlockchain for RpcBlockchain { debug!("connecting to {} auth:{:?}", wallet_url, config.auth); let client = Client::new(wallet_url.as_str(), config.auth.clone().into())?; + let rpc_version = client.version()?; + let loaded_wallets = client.list_wallets()?; if loaded_wallets.contains(&wallet_name) { debug!("wallet already loaded {:?}", wallet_name); + } else if list_wallet_dir(&client)?.contains(&wallet_name) { + client.load_wallet(&wallet_name)?; + debug!("wallet loaded {:?}", wallet_name); } else { - let existing_wallets = list_wallet_dir(&client)?; - if existing_wallets.contains(&wallet_name) { - client.load_wallet(&wallet_name)?; - debug!("wallet loaded {:?}", wallet_name); - } else { + // pre-0.21 use legacy wallets + if rpc_version < 210_000 { client.create_wallet(&wallet_name, Some(true), None, None, None)?; - debug!("wallet created {:?}", wallet_name); + } else { + // TODO: move back to api call when https://github.com/rust-bitcoin/rust-bitcoincore-rpc/issues/225 is closed + let args = [ + Value::String(wallet_name.clone()), + Value::Bool(true), + Value::Bool(false), + Value::Null, + Value::Bool(false), + Value::Bool(true), + ]; + let _: Value = client.call("createwallet", &args)?; } + + debug!("wallet created {:?}", wallet_name); } + let is_descriptors = is_wallet_descriptor(&client)?; + let blockchain_info = client.get_blockchain_info()?; let network = match blockchain_info.chain.as_str() { "main" => Network::Bitcoin, @@ -399,7 +450,6 @@ impl ConfigurableBlockchain for RpcBlockchain { } let mut capabilities: HashSet<_> = vec![Capability::FullHistory].into_iter().collect(); - let rpc_version = client.version()?; if rpc_version >= 210_000 { let info: HashMap = client.call("getindexinfo", &[]).unwrap(); if info.contains_key("txindex") { @@ -416,6 +466,7 @@ impl ConfigurableBlockchain for RpcBlockchain { Ok(RpcBlockchain { client, capabilities, + is_descriptors, _storage_address: storage_address, skip_blocks: config.skip_blocks, }) @@ -438,6 +489,20 @@ fn list_wallet_dir(client: &Client) -> Result, Error> { Ok(result.wallets.into_iter().map(|n| n.name).collect()) } +/// Returns whether a wallet is legacy or descriptors by calling `getwalletinfo`. +/// +/// This API is mapped by bitcoincore_rpc, but it doesn't have the fields we need (either +/// "descriptors" or "format") so we have to call the RPC manually +fn is_wallet_descriptor(client: &Client) -> Result { + #[derive(Deserialize)] + struct CallResult { + descriptors: Option, + } + + let result: CallResult = client.call("getwalletinfo", &[])?; + Ok(result.descriptors.unwrap_or(false)) +} + /// Factory of [`RpcBlockchain`] instances, implements [`BlockchainFactory`] /// /// Internally caches the node url and authentication params and allows getting many different [`RpcBlockchain`] @@ -500,7 +565,7 @@ impl BlockchainFactory for RpcBlockchainFactory { } #[cfg(test)] -#[cfg(feature = "test-rpc")] +#[cfg(any(feature = "test-rpc", feature = "test-rpc-legacy"))] mod test { use super::*; use crate::testutils::blockchain_tests::TestClient; @@ -514,7 +579,7 @@ mod test { url: test_client.bitcoind.rpc_url(), auth: Auth::Cookie { file: test_client.bitcoind.params.cookie_file.clone() }, network: Network::Regtest, - wallet_name: format!("client-wallet-test-{:?}", std::time::SystemTime::now() ), + wallet_name: format!("client-wallet-test-{}", std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos() ), skip_blocks: None, }; RpcBlockchain::from_config(&config).unwrap() diff --git a/src/testutils/blockchain_tests.rs b/src/testutils/blockchain_tests.rs index 297f26772..a78e4a893 100644 --- a/src/testutils/blockchain_tests.rs +++ b/src/testutils/blockchain_tests.rs @@ -387,6 +387,7 @@ macro_rules! bdk_blockchain_tests { Wallet::new(&descriptors.0.to_string(), descriptors.1.as_ref(), Network::Regtest, MemoryDatabase::new()).unwrap() } + #[allow(dead_code)] enum WalletType { WpkhSingleSig, TaprootKeySpend, @@ -421,7 +422,7 @@ macro_rules! bdk_blockchain_tests { let wallet = get_wallet_from_descriptors(&descriptors); // rpc need to call import_multi before receiving any tx, otherwise will not see tx in the mempool - #[cfg(feature = "test-rpc")] + #[cfg(any(feature = "test-rpc", feature = "test-rpc-legacy"))] wallet.sync(&blockchain, SyncOptions::default()).unwrap(); (wallet, blockchain, descriptors, test_client) @@ -446,7 +447,7 @@ macro_rules! bdk_blockchain_tests { // the RPC blockchain needs to call `sync()` during initialization to import the // addresses (see `init_single_sig()`), so we skip this assertion - #[cfg(not(feature = "test-rpc"))] + #[cfg(not(any(feature = "test-rpc", feature = "test-rpc-legacy")))] assert!(wallet.database().deref().get_sync_time().unwrap().is_none(), "initial sync_time not none"); wallet.sync(&blockchain, SyncOptions::default()).unwrap(); @@ -933,7 +934,7 @@ macro_rules! bdk_blockchain_tests { #[test] fn test_sync_bump_fee_add_input_no_change() { let (wallet, blockchain, descriptors, mut test_client) = init_single_sig(); - let node_addr = test_client.get_node_address(None); + let node_addr = test_client.get_node_address(None); test_client.receive(testutils! { @tx ( (@external descriptors, 0) => 50_000, (@external descriptors, 1) => 25_000 ) (@confirmations 1) @@ -1021,6 +1022,7 @@ macro_rules! bdk_blockchain_tests { } #[test] + #[cfg(not(feature = "test-rpc-legacy"))] fn test_send_to_bech32m_addr() { use std::str::FromStr; use serde; @@ -1207,7 +1209,7 @@ macro_rules! bdk_blockchain_tests { let blockchain = get_blockchain(&test_client); let wallet = get_wallet_from_descriptors(&descriptors); - #[cfg(feature = "test-rpc")] + #[cfg(any(feature = "test-rpc", feature = "test-rpc-legacy"))] wallet.sync(&blockchain, SyncOptions::default()).unwrap(); let _ = test_client.receive(testutils! { @@ -1231,6 +1233,7 @@ macro_rules! bdk_blockchain_tests { } #[test] + #[cfg(not(feature = "test-rpc-legacy"))] fn test_taproot_key_spend() { let (wallet, blockchain, descriptors, mut test_client) = init_wallet(WalletType::TaprootKeySpend); @@ -1251,6 +1254,7 @@ macro_rules! bdk_blockchain_tests { } #[test] + #[cfg(not(feature = "test-rpc-legacy"))] fn test_taproot_script_spend() { let (wallet, blockchain, descriptors, mut test_client) = init_wallet(WalletType::TaprootScriptSpend); @@ -1279,20 +1283,24 @@ macro_rules! bdk_blockchain_tests { } #[test] + #[cfg(not(feature = "test-rpc-legacy"))] fn test_sign_taproot_core_keyspend_psbt() { test_sign_taproot_core_psbt(WalletType::TaprootKeySpend); } #[test] + #[cfg(not(feature = "test-rpc-legacy"))] fn test_sign_taproot_core_scriptspend2_psbt() { test_sign_taproot_core_psbt(WalletType::TaprootScriptSpend2); } #[test] + #[cfg(not(feature = "test-rpc-legacy"))] fn test_sign_taproot_core_scriptspend3_psbt() { test_sign_taproot_core_psbt(WalletType::TaprootScriptSpend3); } + #[cfg(not(feature = "test-rpc-legacy"))] fn test_sign_taproot_core_psbt(wallet_type: WalletType) { use std::str::FromStr; use serde_json;