Add scanblocks RPC call (attempt 2)

Revives #20664. All feedback from the previous PR has either been responded to inline or incorporated here.

---

Major changes from Jonas' PR:

• consolidated arguments for scantxoutset/scanblocks
• substantial cleanup of the functional test

Here's the range-diff (git range-diff master jonasschnelli/2020/12/filterblocks_rpc jamesob/2021-11-scanblocks): https://gist.github.com/jamesob/aa4a975344209f0316444b8de2ec1d18

Original PR description

The scanblocks RPC call allows one to get relevant blockhashes from a set of descriptors by scanning all blockfilters in a given range.

Example:

scanblocks start '["addr(<bitcoin_address>)"]' 661000 (returns relevant blockhashes for <bitcoin_address> from blockrange 661000->tip)

Why is this useful?

Fast wallet rescans: get the relevant blocks and only rescan those via rescanblockchain getblockheader(<hash>)[height] getblockheader(<hash>)[height]). A future PR may add an option to allow to provide an array of blockhashes to rescanblockchain.

prune wallet rescans: (needs additional changes): together with a call to fetch blocks from the p2p network if they have been pruned, it would allow to rescan wallets back to the genesis block in pruned mode (relevant #15946).

SPV mode (needs additional changes): it would be possible to build the blockfilterindex from the p2p network (rather then deriving them from the blocks) and thus allow some sort of hybrid-SPV mode with moderate bandwidth consumption (related #9483)

Previously involved:

• @Sjors: Concept ACK, tested
• @jonatack: Concept ACK, tested
• @luke-jr: Rebased, tested
• @fjahr: Concept ACK
• @felipsoarez: Concept ACK
• @chrisguida: Tested ACK
• @prayank23: Concept ACK, tested
• @kiminuo: Reviewed
• @achow101: Reviewed
• @theStack: Concept ACK
• @darosior: Concept ACK

Thanks for reviving 20664. There are many interesting PRs by jonasschnelli which never got merged for different reasons.

Will test it this weekend.

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #26283 (p2p: Fill reconciliation sets and request reconciliation (Erlay) by naumenkogs)
• #25366 (util, rpc: Add parameter to deriveaddresses to display address information by w0xlt)
• #24162 (rpc: add require_checksum flag to deriveaddresses by kallewoof)
• #23443 (p2p: Erlay support signaling by naumenkogs)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

I tried the syntax based on example mentioned in PR description and it gives me an error:

$ bitcoin-cli scanblocks start '["addr(mzrj4QmPhk98vc2yQw42uCsgwfBjVzPPLM)"]' 1000000
error code: -1
error message:
Index is not enabled for filtertype basic

I checked the code for RPC help but don't understand what exactly should be enabled and how are filters used in this RPC

I tried the syntax based on example mentioned in PR description and it gives me an error:

$ bitcoin-cli scanblocks start '["addr(mzrj4QmPhk98vc2yQw42uCsgwfBjVzPPLM)"]' 1000000
error code: -1
error message:
Index is not enabled for filtertype basic

I checked the code for RPC help but don't understand what exactly should be enabled and how are filters used in this RPC

You probably did not run bitcoind with -blockfilterindex=1?

I guess that issue could be caught earlier and use a better error message :)

You probably did not run bitcoind with -blockfilterindex=1?

Thanks for the help @fjahr. Tried running bitcoind with -blockfilterindex=1:

2021-11-22T01:44:23Z basic block filter index is enabled at height 2104934
2021-11-22T01:44:23Z basic block filter index thread exit

$ bitcoin-cli scanblocks start '["addr(mzrj4QmPhk98vc2yQw42uCsgwfBjVzPPLM)"]' 1000000

Got response after few minutes with lot of block hashes. I was expecting a quick response based on concept of this PR and time it took for -blockfilterindex=1 so not sure what exactly did we achieve but will check others things later. Will sleep now as its morning here already.

Concept ACK

Is there plans to add functionality similiar to rescanblockchain so it does it for your wallets, without having to enter in the descriptors manually?

On a quick glance over the second commit, I saw that the help examples are not working, see comment below. Will review more in-depth within the next days.

@theStack fixed, thanks!

@prayank23 Without blockfilters it would take many times longer, and without this PR the functionality to scan blocks doesn't exist at all.

@sipa is that considering the time it takes for -blockfilterindex=1 ? PR does not achieve any fast scans IMO. Maybe description should be changed or PR author can take some time to respond with comments when reviewers waste their nights to review the PR

@prayank23 You only need to build the filters once, and you need them too for -peerblockfilters (if you want to support BIP157 protocol).

Proposed changes (feel free to squash): https://github.com/jamesob/bitcoin/compare/2021-11-scanblocks...luke-jr:rpc_scanblocks

Needs rebase:

rpc/blockchain.cpp:2259:32: error: no viable conversion from 'std::atomic<int>' to 'const UniValue'

I just added .load().

rpc/blockchain.cpp:2301:56: error: no member named 'get_int' in 'UniValue'
                block = active_chain[request.params[2].get_int()];

This needs .getInt<int>() now.

I tested with Luke's patches on top. It found all relevant blocks for the set of wpkh() descriptors I tried it on. Despite having to figure out the rebase stuff, and writing the below "script", it was still faster than a regular wallet rescan :-)

In order to recover a wallet you can do this:

First get the list of descriptors:

src/bitcoin-cli -rpcwallet=Test listdescriptors | jq '.descriptors[].desc'   

Then call scanblocks, use getblockheader to get the corresponding block heights and call rescanblockchain with the same start and end height.

src/bitcoin-cli -rpcclienttimeout=0 scanblocks start '["desc1", "desc2", etc]' | jq -r '[.relevant_blocks][][]'  | xargs -I {} sh -c "src/bitcoin-cli getblockheader {} | jq -r '.height'" | xargs -I {} sh -c  "src/bitcoin-cli -rpcwallet=Test rescanblockchain {} {}"

(the xargs incantation might be macOS specific, but it's hideous anyway…)

Maybe someone can turn this into a less ugly Python contrib script. It would of course be easier if scanblockchain accepted an array of block hashes.

I think I've addressed all of @luke-jr, @Sjors' feedback - let me know if I've missed something.

tACK d19cf65daff4792951964e12ae3feb4be43574f8

@theStack thanks; that change accidentally made its way into the wrong commit. Fixed (no change in the resulting code at branch tip).

re-utACK b770100b8fa2043dd8eaf8fff61eef9b09e1da37

Not sure if you will want to cherry-pick the commit here or should I push it in a follow-up PR but the scanblocks flow performance can be improved greatly

Review-wise I'd prefer to keep that for a followup. It's plenty fast already.

Trivial rebase pushed.

Given the number of (tested) ACKs here it would be nice to get this merged sooner rather than later; it's been open almost a year.

On my list to reACK soon(tm)

Unless there are any correctness problems I'm going to leave this PR as-is; 3 solid ACKs, so would be nice to get this merged.

Does this also need release notes? edit: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/scanblocks-RPC-release-notes