rpc: Add dumpcoinstats #19792

Requires #19521 (this PR only adds the last commit).

The rpc dumpcoinstats dumps the content of the coinstats index into a CSV file for auditing purposes.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

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

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #27607 (init: verify blocks data existence only once for all the indexers by furszy)
• #20892 (tests: Run both descriptor and legacy tests within a single test invocation by achow101)

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.

Taking this out of draft status since I am interested in conceptual feedback on something like this but of course, #19521 still has a long way to go before this can be seriously considered.

Tested ACK 21c42c22c9b50f60a4b4a0ee84c45f47df98752f

Concept ACK, I wish we had gotten streaming to work though so that the file can be downloaded (see my experiment in #7759). RPCs that creates a server-local file, although there are already some others, are not that great from a security and usability point of view.

Rebased on top of the latest changes in #19521

Concept ACK, could have a functional test.

Rebased now that #19521, will also push a functional test shortly.

Added functional test.

Rebased

Error: RPC command "dumpcoinstats" not found in RPC_COMMANDS_SAFE_FOR_FUZZING or RPC_COMMANDS_NOT_SAFE_FOR_FUZZING. Please update test/fuzz/rpc.cpp.

Thanks @MarcoFalke , fixed!

Approach ACK b85813a

Rebased and fixed silent merge conflict.

Concept ACK. Tested and reviewed 87fd7b8, but see inline

Also, not to scope creep, but can you add a few goodies from CBlockIndex to the CSV?

• the number of transactions per block (nTx)
• timestamp (and maybe MTP, but that can be derived in a spreadsheet)
• nChainWork (or per block)
• nVersion

Also, not to scope creep, but can you add a few goodies from CBlockIndex to the CSV?

• the number of transactions per block (nTx)
• timestamp (and maybe MTP, but that can be derived in a spreadsheet)
• nChainWork (or per block)
• nVersion

Easy enough, if it enables more use cases we can easily add more. I have added all of these.

f73b68c262df256e08a32af4a2e8142b3f68dee6 almost there

Addressed @Sjors ' comments, thanks for reviewing! :)

Windows CI seems unhappy:

PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\Users\ContainerAdministrator\AppData\Local\Temp\test_runner_₿_🏃_20220207_231436\feature_coinstatsindex_17\node1\test.csv'

Addressed comments by @Sjors and @MarcoFalke , thanks for reviewing!

tACK 3772c09e16544c7876da13f33e6cb11401c9e4bd

Remaining feedback can wait for a followup, but I'm also happy to review it again.

Maybe document the fields in the help, since some are not obvious. For each item, note if it has a cumulative version.

A slightly more logical ordering of columns would be:

• height
• block_hash
• timestamp
• mediantime
• version
• txs
• maybe insert: txins to count the number of outputs spent. I think you can use coin_count between two blocks and txouts to calculate it.
• txouts (document for cumulative: this is the total number of outputs ever created, not the total in existence at this height)
• maybe insert: the number txouts in existence at this height (the same for cumulative). It may be useful to track its ratio to bogosize over time.
• prevout_spent_amount (maybe rename to amount_spent)
• amount (or amount_created)
• coinbase_amount
• new_outputs_ex_coinbase_amount
• subsidy
• unclaimed_rewards
• unspendable_scripts (docs should clarify this is just OP_RETURN)
• unspendable_amount (docs should clarify this includes both immature and OP_RETURN)
• genesis_block
• bip30
• chainwork
• muhash
• bogosize: Database-independent approximate metric of UTXO set size. It counts every UTXO entry as 50 + the length of its scriptPubKey.

This puts the most useful fields first.

There may be some metrics that are useful for both cumulative and non-cumulative. In particular it's both useful to know how many outputs were created and destroyed in any given block (and their value, and the delta UTXO set size), and how many outputs existed at any given block (and much value they represented and disk space they used).

Addressed @Sjors feedback and also squashed some commits because it was just easier to do the reorganization this way.

Maybe document the fields in the help, since some are not obvious. For each item, note if it has a cumulative version.

I was a bit unsure where to put this and how to format it but I tried my best.

A slightly more logical ordering of columns would be:

I applied that order

• maybe insert: txins to count the number of outputs spent. I think you can use coin_count between two blocks and txouts to calculate it.

I think it's not possible without updating coinstatsindex. The coin_count isn't included there, I am not sure why but probably just bad timing since it was merged while I was already working on the index. Leaving this as a follow-up since I first have to think about updating the index. I also thought about calculating it with txouts but I think it would be (txouts - prev_txouts) - coinbase_txouts - unspendable_txouts and from those I am missing the unspendable counts as well.

• txouts (document for cumulative: this is the total number of outputs ever created, not the total in existence at this height)

I actually didn't have this as non-cummulative yet, but done as well.

• maybe insert: the number txouts in existence at this height (the same for cumulative). It may be useful to track its ratio to bogosize over time.

I think you mean UTXOs here which would basically be coin_count? Or should it also include unspendable outputs?

• prevout_spent_amount (maybe rename to amount_spent)
• amount (or amount_created)

renamed both

• unspendable_amount (docs should clarify this includes both immature and OP_RETURN)

No, immature is not included here. This is the just the total of the other unspendable values (genesis, bip30, scripts and unclaimed rewards)

87f0cc381c77e7d321d9e01a11a4725878c81810 almost there

Leaving this as a follow-up since I first have to think about updating the index

Yeah, let's not touch the index itself here.

I think you mean UTXOs here which would basically be coin_count? Or should it also include unspendable outputs?

Excluding unspendable sounds good to me. Other than the coinbase, they're not really part of the UTXO set in practice, e.g. they don't use RAM and their bogo_size is zero too.

Seeing this in a spreadsheet slightly changed my thoughts on the optimal order. After txouts let's do:

• coinbase_amount
• subsidy (which represents the maximum allowed subsidy)
• amount_generated (in theory this can be less than zero, but afaik there has never been a block that included transactions but forgot to claim both the subsidy and fees)
• unclaimed_rewards
• amount_spent
• new_outputs_ex_coinbase_amount

(then continue as it is now, from unspendable_scripts)

Seeing this in a spreadsheet slightly changed my thoughts on the optimal order. After txouts let's do:

• coinbase_amount
• subsidy (which represents the maximum allowed subsidy)
• amount_generated (in theory this can be less than zero, but afaik there has never been a block that included transactions but forgot to claim both the subsidy and fees)
• unclaimed_rewards
• amount_spent
• new_outputs_ex_coinbase_amount

(then continue as it is now, from unspendable_scripts)

Done

tACK 418331f35424bdc95dedba6023bf0d8f7819fabe

TIL block 367,853 has the highest number of transactions ever: 12,239

Added a wish list for more stats: https://github.com/bitcoin/bitcoin/issues/24581

Way to kill the mood @DrahtBot ! Rebased...

re-utACK 843159e5df3dc0d1ca0341b22aaa9fe86160994a

Rebased

Half of CI seems unhappy...

@Sjors Thanks! Fixed the silent merge conflict.

re-tACK 19b12fa07653cb11a6fd97566af0036241eaa8c9

tACK https://github.com/bitcoin/bitcoin/pull/19792/commits/19b12fa07653cb11a6fd97566af0036241eaa8c9

Rebased and changed indentation per @MarcoFalke 's request.

reACK https://github.com/bitcoin/bitcoin/pull/19792/commits/9d45f7c455318089fb73dc275efbf6e176d971e2

Rebased. The first commit has become obsolete, so it was removed.

re-tACK 3b3e3d966ad44b37bab3659b0d840f8384da05be

Not sure about the cs_main locking assumptions

If the index has not fully been synced yet, the error thrown would be:

Failed to get coins stats at height X. You may have to remove an incomplete dump file before running this command again.

It's not very clear that the index has not finished syncing. Also, the csv file would be created before this error is thrown, so the file would have to be removed manually.

I would suggest checking if the index is synced before dumping and create the file only if the index is ready+enabled, with something like this:

<details> <summary>Diff</summary>

diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 74877d7da..7bb6e2927 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -1039,16 +1039,21 @@ static RPCHelpMan dumpcoinstats()
                 throw JSONRPCError(RPC_INVALID_PARAMETER, filepath.u8string() + " already exists. If you are sure this is what you want, move it out of the way first");
             }
 
+            if (!g_coin_stats_index) {
+                throw JSONRPCError(RPC_MISC_ERROR, "Coinstats index is not enabled");
+            }
+
+            const auto summary{g_coin_stats_index->GetSummary()};
+            if (!summary.synced) {
+                throw JSONRPCError(RPC_MISC_ERROR, strprintf("coinstatsindex is not synced. Current best block height: %d", summary.best_block_height));
+            }
+
             std::ofstream file;
             file.open(filepath);
             if (!file.is_open()) {
                 throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open dump file");
             }
 
-            if (!g_coin_stats_index) {
-                throw JSONRPCError(RPC_MISC_ERROR, "Coinstats index is not enabled");
-            }
-
             const bool cumulative{request.params[1].isNull() ? false : request.params[1].get_bool()};
 
             file << strprintf("height,"

</details>

Addressed comments from @aureleoules , thanks for the review!

Further note: I can't change the status back it seems, but please treat this as a draft for now. This PR will need to be updated and rebased soon when I have resolved the overflow issue noted in #26362.

EDIT: Rebased as well since there was a silent merge conflict.

Thanks @fanquake , lightning speed :)

<!--cf906140f33d8803c4a75a2196329ecb-->

🐙 This pull request conflicts with the target branch and needs rebase.

<!--13523179cfe9479db18ec6c5d236f789-->

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

• Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
• Is it no longer relevant? ➡️ Please close.
• Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

There doesn't seem to be much interest in this feature currently. If someone needs it, feel free to ping me here and I will reorg/rebase for you.