rpc: use importdescriptors with Core >= 0.21

[afilini]

Description

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.

This also makes us compatible with Core 23.0 and is thus a replacement for #613, which actually looking back at it was adding support for 23.0 but probably breaking older wallets by adding the extra argument to createwallet.

I believe #613 should now only focus on getting the tests to work against 23.0, which is still important but not such a high priority as being compatible with the latest version of Core.

Also fixes #598

Checklists

All Submissions:

• I've signed all my commits
• I followed the contribution guidelines
• I ran cargo fmt and cargo clippy before committing

New Features:

• I've added tests for the new feature
• I've added docs for the new feature
• I've updated CHANGELOG.md

[afilini]

I should also point out that the check for whether or not we should use importdescriptors is not done against the Core version, but against the individual wallet type (using getwalletinfo). This means that if you had a legacy wallet and then you upgrade Core, BDK will still use the legacy RPC with your wallet, which is the correct behavior.

The version is only checked when creating new wallets, in that case we use descriptors if we can.

[rajarshimaitra]

Initial Concept ACK..

I was also thinking maybe the easiest route now is to have both, and yes the trade for that is another feature flag, but I think that would have been necessary eventually for the wallet compatibility..

Also interestingly the tests are passing here and the major diff between this and #613 is electrs v0.9.1. And thats where the test breaks were indicating too. So until that gets sorted its better to move ahead with this one..

I will again have a through pass and test them out for different core versions..

In the mean time I think we can close #613 and open a new issue with this potential electrs problem.

[notmandatory]

Suggested change

	- Add support for Bitcoin Core 23.0 when using the `rpc` blockchain
	- Add support for Bitcoin Core 22.0 and pre-22.0 when using the `rpc` blockchain

[afilini]

22.0 and before were already supported previously, since we were always creating legacy wallets and using importmulti.

With this PR we add support for 23 which uses descriptor wallets by default, and change a bit the way we interact with wallets 0.21 and up.. but they were already supported before, so from the user's perspective I guess the only change is that now they can use it with 23 while previously they couldn't

[notmandatory]

But in this PR the CI is testing with electrsd/bitcoind_22_0 for test-electrum, test-rpc and test-esplora , and when I tried changing these to bitcoind_23_0 the tests hung and failed for test-electrum and test-rpc, I have some notes in the (resolved) comment above.

Since we're not adding any new support for rpc with core 23 how about I remove the changelog message instead?

[afilini]

Ah I see what you mean. I would keep it there because we've actually added support for 23, it's just the automated tests that are still failing.

I guess the confusion comes from the fact that my original issue was referring to the tests against 23 failing, but in fact it was not just the tests, it was BDK being completely broken with 23.0. So now this PR fixed BDK so that it can properly interact with newer nodes, but since the tests are still failing somewhere in our mechanism to automate them, I kept the tests running against 22.

I tested it manually by switching the bitcoind feature to use bitcoin 23 and running the rpcwallet example, which internally spawns a core node with electrsd, and it works just fine. Same thing for older versions compared to what we test for, it works fine all the way back to 0.18.1. It's just again the tests hanging with core < 0.20 for some reason.

I'm attaching here the logs for the 23.0 and 0.18.1 run, they are a bit messy because there's the stdout from Core itself, plus logging from bdk and bitcoincore_rpc so that you can see all the calls being made. You'll notice that with the old node we use importmulti, while with the newer one we use importdescriptors which is what I implemented in this PR.

So long story short, BDK works with 23 now, we just have to figure out how to run automated tests reliably against it!

[notmandatory]

Ok that's great! I won't take your changelog message out and @rajarshimaitra and I are going to keep working on how to fix the automated tests.

[notmandatory]

ACK e1a1372