[blockchain] Add support for pruned RPC blockchain

[kjain1810]

Description

Current RpcBlockchain grants full access to transaction history, however, it requires a non-pruned node (a node storing the full blockchain, which at the time of writing is over 400GB).
However, even without a wallet's full transaction history it is still possible to compute the wallet balance and be able to spend with only a pruned core full node.
The key to achieve that is using RPC method scantxoutset

This PR implements blockchain::rpc::PrunedRpcBlockchain and blockchain::rpc::PrunedRpcConfig for the same and use scantxout to compute the wallet balance.

Implements #530

Notes to the reviewers

This is currently a draft PR
This PR is a part of my project for Summer of Bitcoin 2022. My project mentors are @afilini and @RCasatta .

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'm not adding any review comments since I can see this is still at the beginning, I'm just gonna say (as you've probably already realized) that ideally we should try to reuse all the common code with the normal rpc blockchain (for example I can see that you've made a copy of the Auth struct) because otherwise we end up with two versions of the same thing to maintain.

I'll let you work on the implementation of setup/sync now, if once you are done we can see that the struct definition is very similar with RpcBlockchain and RpcBlockchainConfig we could consider merging the two structs and use a generic parameter to differentiate between pruned and non-pruned blockchain.

For example, we could end up with RpcBlockchain<Full> and RpcBlockchain<Pruned> (same thing for the config) so that we can share the struct definition. Most of the other traits could also be shared (GetHeight for example should be the same) and the traits that have a different behavior can be implemented differently for the two structs.

But we'll talk about this later on, I think you are taking the right approach right now: it's better to copy/paste initially and just focus on the implementation and then see if we can share some of the code.

[kjain1810]

Yeah, right now, I am copy pasting much of the structures because I suspect that when I start implementing or optimising the wallet setup and sync traits, additional information could be useful.

Apart from that, since both these features are still experimental and the RPC API for pruned nodes can also be subjected to quite a few changes, I believe it would be better if we keep them separated in the future as well.

[afilini]

Yeah that's a good point, let's see how the code looks once we are done and then we'll decide.

[evanlinjin]

I'm wondering whether a separate Blockchain implementation makes sense or whether we should find a way to have RpcBlockchain support both pruned and non-pruned. The latter makes sense to me since it's a single command to make a Core node from full to pruned.

We can check whether Core is pruned or not via getblockchaininfo RPC call. It's an if/else statement from there for variations in sync logic.

[kjain1810]

As the code has evolved, I too have notices that having a separate class for it is an overkill.

The changes essentially boil down to calling a different RPC for the pruned node (scantxout instead of listtransactions) and processing the current utxo set with a slight modification.

I am trying to find the bug in my code right now and plan to integrate it into the RPC class moving forward after discussing it with @afilini

Any help in the debugging process is appreciated since I am a bit lost where I went wrong :/

[evanlinjin]

@kjain1810 I'll have a look this weekend and see how I can help.

Note that RpcBlockchain got a complete rewrite (merged in master) that fixes multiple bugs. The code potentially may be cleaner to work with and base your changes on (but please discuss with @afilini to see what he recommends as the better approach).

[evanlinjin]

I am trying to find the bug in my code right now and plan to integrate it into the RPC class moving forward after discussing it with @afilini

@kjain1810 Which bug are you referring to? Currently tests are failing with:

...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 })))'

Have you looked into which RPC calls are resulting in this error? (hint: it's getindexinfo).

Things that are good to know/figure out:

• What version(s) of bitcoin core we are using for integration tests
• Which RPC call is causing the aforementioned error (and to what version of bitcoin core)
• What commands are supported by which bitcoin core versions

[kjain1810]

@evanlinjin
Weird, I am actually not seeing this error at all. My build keeps failing on the createwallet RPC itself with message

thread 'blockchain::pruned_rpc::test::bdk_blockchain_tests::test_sign_taproot_core_scriptspend2_psbt' 
panicked at 'created wallet: JsonRpc(Rpc(RpcError { code: -1, message: "createwallet \"wallet_name\" ( disable_private_keys blank \"passphrase\" avoid_reuse )
\n\nCreates and loads a new wallet.\n\nArguments:\n1. wallet_name             (string, required) The name for the new wallet. If this is a path, the wallet will be created at the path location.
\n2. disable_private_keys    (boolean, optional, default=false) Disable the possibility of private keys (only watchonlys are possible in this mode).
\n3. blank                   (boolean, optional, default=false) Create a blank wallet. A blank wallet has no keys or HD seed. One can be set using sethdseed.
\n4. passphrase              (string) Encrypt the wallet with this passphrase.\n5. avoid_reuse             (boolean, optional, default=false) Keep track of coin reuse, and treat dirty and clean coins differently with privacy considerations in mind.
\n\nResult:\n{                       (json object)\n  \"name\" : \"str\",       (string) The wallet name if created successfully. If the wallet was created using a full path, the wallet_name will be the full path.\n  \"warning\" : \"str\"     (string) Warning message if wallet was not loaded cleanly.\n}\n\nExamples:\n> bitcoin-cli createwallet \"testwallet\"\n> curl --user myusername --data-binary '{\"jsonrpc\": \"1.0\", \"id\": \"curltest\", \"method\": \"createwallet\", \"params\": [\"testwallet\"]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/\n", data: None }))', src/blockchain/pruned_rpc.rs:522:5

I think I forgot to mention this earlier but I am testing with the command

cargo test --features test-pruned-rpc-legacy

[evanlinjin]

@kjain1810 it is great that you identified this. Do you know what steps you need to take to debug this?

[kjain1810]

@evanlinjin i went through the documentation of this call and how bitcoincore_rpc calls it but couldn't find the error

Lost on what other steps i can take now

[evanlinjin]

@kjain1810 bitcoincore_rpc is a client library that calls the JSON-RPC interface of Bitcoin Core. Have you identified which version of Bitcoin Core it is interacting with when the error occurs? And how the Bitcoin Core instance is run by the integration tests in BDK?

[kjain1810]

@evanlinjin yeah both of them seem to be working on v22

[evanlinjin]

@kjain1810 that's great. What version of bitcoind will we be using with the cargo feature test-pruned-rpc-legacy?

[kjain1810]

The same as for test-rpc-legacy, thats v22 only, right?

[danielabrozzoni]

@kjain1810 not really: https://github.com/bitcoindevkit/bdk/pull/631/files#diff-2e9d962a08321605940b5a657135052fbcef87b5e360662bb527c96d9a615542R95

It's called legacy, as it uses an older version of Bitcoin Core - in this case, v0.20

[evanlinjin]

@kjain1810 Are you still planning to continue with this? If so, I personally think it is better to base changes on the current implementation of rpc blockchain (it has basically been rewritten).

I would close this PR for now if there is no further objections.

[danielabrozzoni]

Closing this for now (we have too many open and abandoned PRs), but feel free to re-open if needed :)