rpc: add getdescriptoractivity

[jamesob]

The RPC command scanblocks provides a useful way to get a set of blockhashes that have activity relevant to a set of descriptors (relevant_blocks). However actually extracting the activity from those blocks is left as an exercise to the end user.

This process involves not only generating the (potentially ranged) set of scripts for the descriptor set on the client side (maybe via deriveaddresses), but then the user must retrieve each block's contents one-by-one using getblock <hash>, which is transmitted over a network link. And that's all before they perform the actual search over block content. There's even more work required to incorporate unconfirmed transactions.

This PR introduces an RPC getdescriptoractivity that dovetails with scanblocks output, handling the process described above. Users specify the blockhashes (perhaps from relevant_blocks) and a set of descriptors; they are then given all spend/receive activity in that set of blocks.

This is a very useful tool when implementing lightweight wallets that want neither to require a third-party indexer like electrs, nor the overhead of creating and managing watch-only wallets in Core. This allows Core to be more easily used in a "stateless" manner by wallets, with potentially many nodes interchangeably acting as backends.

Example usage

% ./src/bitcoin-cli scanblocks start \
    '["addr(bc1p0cp0vyag6snlta2l7c4am3rue7eef9f72l7uhx52m4v27vfydx9s8tfs7t)"]' \
    857263
{
  "from_height": 857263,
  "to_height": 858263,
  "relevant_blocks": [
    "00000000000000000002bc5cc78f5b0913a5230a8f4b0d5060bc9a60900a5a88",
    "00000000000000000001c5291ed6a40c06d3db5c8fb738567654b24a14b24ecb"
  ],
  "completed": true
}


% ./src/bitcoin-cli getdescriptoractivity \
    '["00000000000000000002bc5cc78f5b0913a5230a8f4b0d5060bc9a60900a5a88", "00000000000000000001c5291ed6a40c06d3db5c8fb738567654b24a14b24ecb"]' \
    '["addr(bc1p0cp0vyag6snlta2l7c4am3rue7eef9f72l7uhx52m4v27vfydx9s8tfs7t)"]'
{
  "activity": [
    {
      "type": "receive",
      "address": "bc1p0cp0vyag6snlta2l7c4am3rue7eef9f72l7uhx52m4v27vfydx9s8tfs7t",
      "desc": "rawtr(7e02f613a8d427f5f55ff62bddc47ccfb394953e57fdcb9a8add58af3124698b)#yewcd80j",
      "amount": 0.00002900,
      "blockhash": "00000000000000000002bc5cc78f5b0913a5230a8f4b0d5060bc9a60900a5a88",
      "height": 857907,
      "txid": "c9d34f202c1f66d80cae76f305350f5fdde910b97cf6ae6bf79f5bcf2a337d06",
      "vout": 254
    },
    {
      "type": "spend",
      "address": "bc1p0cp0vyag6snlta2l7c4am3rue7eef9f72l7uhx52m4v27vfydx9s8tfs7t",
      "desc": "rawtr(7e02f613a8d427f5f55ff62bddc47ccfb394953e57fdcb9a8add58af3124698b)#yewcd80j",
      "amount": 0.00002900,
      "blockhash": "00000000000000000001c5291ed6a40c06d3db5c8fb738567654b24a14b24ecb",
      "height": 857907,
      "spend_txid": "7f61d1b248d4ee46376f9c6df272f63fbb0c17039381fb23ca5d90473b823c36",
      "spend_vin": 0,
      "prevout_txid": "c9d34f202c1f66d80cae76f305350f5fdde910b97cf6ae6bf79f5bcf2a337d06",
      "prevout_vout": 254
    }
  ]
}

[DrahtBot]

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/30708.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	tdb3, instagibbs, achow101, rkrux
Concept ACK	danielabrozzoni, pablomartin4btc

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

[DrahtBot]

🚧 At least one of the CI tasks failed. _{Debug: https://github.com/bitcoin/bitcoin/runs/29204025345}

Hints

Make sure to run all tests locally, according to the documentation.

The failure may happen due to a number of reasons, for example:

• Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

• A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

• An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

[jamesob]

Thought about a potential alternative that updates/enhances scanblocks instead of adding a new RPC (with the advantage of not needing to provide block hashes). Adding getdescriptoractivity seems like a better solution, since it avoids breaking RPC compatibility. Carrying block hashes from scanblocks to getdescriptoractivity seems like a reasonably price to pay for compatibility.

I thought about this initially and I think it's probably worth doing at some point via an additional with_activity parameter or something. The code here could be factored out and reused. But ultimately I think it's good to have a separate RPC command.

One possibility (that is probably more valuable) is to add an incremental relevant_blocks key to scanblocks status output, so that the client can begin calling getdescriptoractivity progressively as results roll in but before the entire scan is finished.

[tdb3]

One possibility (that is probably more valuable) is to add an incremental relevant_blocks key to scanblocks status output, so that the client can begin calling getdescriptoractivity progressively as results roll in but before the entire scan is finished.

Great idea. Created an initial draft PR #30713

[jamesob]

I think either a with_activity parameter in scanblocks or a new getdescriptoractivity RPC command would be a good solution. I prefer the former, as the user would only have to wait for one (potentially slow) RPC call instead of two.

This change in conjunction with https://github.com/bitcoin/bitcoin/pull/30713 will actually allow the most rapid report of descriptor activity, since this call over just a few blockhashes is actually very fast. Results can be scanned progressively as they appear; I don't think we could get the same responsiveness using a scanblocks [with_activity] approach even though that would be simpler for the caller.

[jamesob]

One downside of this approach compared to scanblocks [with_activity] is the re-parsing and re-derivation of the descriptor on each getdescriptoractivity call. E.g. for a ranged descriptor with default values, each command call would involve deriving 1,000 keys and generating their corresponding matching scripts, which are then added to the filter elements set to perform the membership test.

How slow is this actually, though? Some kind of bounded cache would probably fix this pretty easily if it wound up taking non-negligible time relative to the other operations for these calls, which I would expect to swamp a non-IO computation like descriptor derivation.

[tdb3]

A quick data point. On mainnet, getting activity for 39C7fxSzEACPjM78Z7xdPxhf7mKxJwvfMJ over 200 block hashes was pretty fast. About 2.5s (on a Zen 2 CPU and a pedestrian DRAM-less SSD).

time src/bitcoin-cli getdescriptoractivity '["00000000000000000000c58c91c455930e9ebd8d463529e1f0f833a16e132a51","000000000000000000014ffca17b9ec566e98e28b655cc835a0f933564b526ba",...]' '["addr(39C7fxSzEACPjM78Z7xdPxhf7mKxJwvfMJ)"]'
...
real    0m2.513s
user    0m0.003s
sys     0m0.006s 

[jamesob]

I think this is ready for "actual" review, in case anyone was wondering.

[jamesob]

Rebased.

[jamesob]

Can someone rerun the windows job? Now that I'm not a member of the org, I can't kick CI Jobs.

[fanquake]

Can someone rerun the windows job?

Kicked it now that the fix is in.

[instagibbs]

light review Ack 878b6c85466366c4ae5f454ec49b5a5f561e0ed2

Reviewed tests and interface mostly

[jamesob]

Ready for a re-review when you're ready, @tdb3 @rkrux.

[DrahtBot]

🚧 At least one of the CI tasks failed. _{Debug: https://github.com/bitcoin/bitcoin/runs/33383517836}

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

• Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

• A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

• An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

[jamesob]

Tests passing, ready for re-review. I think we're close on this one.

[jamesob]

@achow101 ready for merge?

[jamesob]

Pushed a small fix addressing blockhash nullability.

[instagibbs]

reACK 37a5c5d83664c31d83fc649d3c8c858bd5f10f21

blockhash is now an optional return

[achow101]

ACK 37a5c5d83664c31d83fc649d3c8c858bd5f10f21