The rpc blockchain tests don't work with Bitcoin Core 23.0

[afilini]

Describe the bug

Running the integration tests against Bitcoin Core 23.0 I see all of them failing with:

---- blockchain::rpc::bdk_blockchain_tests::test_tx_chain stdout ----
thread 'blockchain::rpc::bdk_blockchain_tests::test_tx_chain' panicked at 'called `Result::unwrap()` on an `Err` value: Rpc(JsonRpc(Rpc(RpcError { code: -4, message: "This type of wallet does not support this command", data: None })))', src/blockchain/rpc.rs:443:1

To Reproduce

I recently updated bitcoind to the latest version which supports Bitcoin Core 23.0 in my taproot branch, I assume this could be reproduced by manually connecting to a 23.0 node.

Expected behavior

Tests should not fail

Build environment

Linux x86_64, but I'm assuming it doesn't matter too much

[rajarshimaitra]

Adding this to my top todos.. I suspect this is something to do with legacy and descriptor wallet type in new core. They might have disabled few RPCs for descriptor type wallets. Would try find the bottom of it..

[afilini]

I'm assigning this to you then :)

[rajarshimaitra]

This is happening because bitcoin v0.23.0 creates a descriptor wallet by default.. And in our rpc setup code we try to initialize the wallet by importmulti which isn't supported by descriptor wallets.

The possible solutions are

• Change bitcoind crate to always initiate a legacy wallet explicitly if version is 23.0 or above.
• We change our rpc sync logic to handle public descriptors instead of importmulti. But this will not work for other bitcoind versions. Older version creates legacy wallet by default.

Looking for suggestion on best way forward..

cc @RCasatta

[RCasatta]

Change bitcoind crate to always initiate a legacy wallet explicitly if version is 23.0 or above.

I am keen to change bitcoind crate with new features, but this looks too specific

Another option is that the rpc setup code create another non-descriptor wallet and then using create_wallet create another bitcoincore_rpc::Client pointing to the non-descriptor wallet

[rajarshimaitra]

Is it possible to change rpc setup code to always use a descriptor wallet?? I feel core has been already moving towards descriptors from legacy. And the importmulti was a work around to put our descriptor into core watch only addresses.. But now with descriptors by default in core we should be able to use descriptors directly.

But I can imagine handling different wallet type depending on version of core could also become problematic.. So more leaning towards always ask for a descriptor wallet explicitly from core.. Descriptor support in core goes back till v0.17.0..

Is there any potential problem in doing that??

[RCasatta]

Importmulti was used in rpc implementation because at that time (not sure now) not every descriptor supported by bdk was supported by core

[rajarshimaitra]

It seems like bitcoincore-rpc doesn't allow creating descriptor wallets yet. I have opened an issue for that rust-bitcoin/rust-bitcoincore-rpc#223

Once we have that, and importdescriptors in bitcoincore-rpc, I think the rest can be handled..

[afilini]

Maybe you can already do it by performing a "raw" call to the RPC? Even if it's not exposed by their API

Importmulti was used in rpc implementation because at that time (not sure now) not every descriptor supported by bdk was supported by core

This is still true today, although I think the Miniscript pr is ready and will hopefully be merged soon.

[notmandatory]

For reference I found these core miniscript PRs:

• Initial support: Miniscript integration bitcoin/bitcoin#24147
• Watch only: Miniscript support in Output Descriptors bitcoin/bitcoin#24148 <-- we only need up to here?
• Signing: Signing support for Miniscript Descriptors bitcoin/bitcoin#24149
• Don't allow duplicate keys + cleanups: Miniscript integration follow-ups bitcoin/bitcoin#24860

[rajarshimaitra]

Maybe you can already do it by performing a "raw" call to the RPC? Even if it's not exposed by their API

Yeah right, I think that can work..

Thanks @notmandatory for the refs..
But it feels better to wait for core to get these miniscript support before transitioning to descriptor wallet fully..

I am still figuring out a way to do legacy in core v0.23. It seems like v0.23 core rpc has a some new modifications and bitcoincore-rpc hasn't included them yet.

For example it seems like the create_wallet() function does sets the descriptor=false here
https://github.com/rust-bitcoin/rust-bitcoincore-rpc/blob/657eebd0dd24c19b7d145b49144f480f5dc09ec3/client/src/client.rs#L272-L291

but it still creates a descriptor wallet. The descriptor flag should be in the handle_default() part, but total args passed in the function are 9, where as v0.23 bitcoin-cli asks for 8 only. So somewhere something is going wrong in parsing??

$ bitcoin-cli help createwallet
createwallet "wallet_name" ( disable_private_keys blank "passphrase" avoid_reuse descriptors load_on_startup external_signer )

Creates and loads a new wallet.

Arguments:
1. wallet_name             (string, required) The name for the new wallet. If this is a path, the wallet will be created at the path location.
2. disable_private_keys    (boolean, optional, default=false) Disable the possibility of private keys (only watchonlys are possible in this mode).
3. blank                   (boolean, optional, default=false) Create a blank wallet. A blank wallet has no keys or HD seed. One can be set using sethdseed.
4. passphrase              (string, optional) Encrypt the wallet with this passphrase.
5. avoid_reuse             (boolean, optional, default=false) Keep track of coin reuse, and treat dirty and clean coins differently with privacy considerations in mind.
6. descriptors             (boolean, optional, default=true) Create a native descriptor wallet. The wallet will use descriptors internally to handle address creation
7. load_on_startup         (boolean, optional) Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged.
8. external_signer         (boolean, optional, default=false) Use an external signer such as a hardware wallet. Requires -signer to be configured. Wallet creation will fail if keys cannot be fetched. Requires disable_private_keys and descriptors set to true.

[Sjors]

Descriptor wallets are different in various ways from legacy wallets, so it might be easiest to first set descriptors to false so everything works again, and then look into using descriptor wallets.

It'll be a while before creating legacy wallets is no longer possible: bitcoin/bitcoin#20160

[rajarshimaitra]

Thanks @Sjors for the timeline ref.. Yes in that case it does make sense to fall back onto legacy and figure out the descriptor part slowly..

I am still digging deeper why v0.23 core is creating a descriptor wallet for the same argument sets that creates a legacy in v0.22 and below..

[sipa]

@rajarshimaitra The default was just changed, see https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-23.0.md;

Descriptor wallets are now the default wallet type. Newly created wallets will use descriptors unless descriptors=false is set during createwallet, or the Descriptor wallet checkbox is unchecked in the GUI.

Up until 22.x, new wallets were legacy type by default. Since 23.0, new wallets are descriptor type by default.

[notmandatory]

Re-opened since even after #628 the test-rpc and test-electrum tests are still not working with bitcoind_0_23_0.