Merge bitcoin/bitcoin#28339: validation: improve performance of CheckBlockIndex

5bc2077e8f592442b089affdf0b5795fbc053bb8 validation: allow to specify frequency for -checkblockindex (Martin Zumsande)
d5a631b9597e5029a5048d9b8ad84ea4536bbac0 validation: improve performance of CheckBlockIndex (Martin Zumsande)
32c80413fdb063199f3bee719c4651bd63f05fce bench: add benchmark for checkblockindex (Martin Zumsande)

Pull request description:

  `CheckBlockIndex() ` are consistency checks that are currently enabled by default on regtest.

  The function is rather slow, which is annoying if you
  * attempt to run it on other networks, especially if not fully synced
  * want to generate a long chain on regtest and see block generation slow down because you forgot to disable `-checkblockindex` or don't know it existed.

  One reason why it's slow is that in order to be able to traverse the block tree depth-first from genesis, it inserts pointers to all block indices into a `std::multimap` - for which inserts and lookups become slow once there are hundred thousands of entries.
  However, typically the block index is mostly chain-like with just a few forks so a multimap isn't really needed for the most part. This PR suggests to store the block indices of the chain ending in the best header in a vector instead, and store only the rest of the indices in a multimap. This does not change the actual consistency checks that are being performed for each index, just the way the block index tree is stored and traversed.

  This adds a bit of complication to make sure each block is visited (note that there are asserts that check it), making sure that the two containers are traversed correctly, but it speeds up the function considerably:

  On master, a single invocation of `CheckBlockIndex` takes ~1.4s on mainnet for me (4.9s on testnet which has >2.4 million blocks).
  With this branch, the runtime goes down to ~0.27s (0.85s on testnet).This is a speedup by a factor ~5.

ACKs for top commit:
  achow101:
    ACK 5bc2077e8f592442b089affdf0b5795fbc053bb8
  furszy:
    ACK 5bc2077e8f592442b089affdf0b5795fbc053bb8
  ryanofsky:
    Code review ACK 5bc2077e8f592442b089affdf0b5795fbc053bb8. Just added suggested assert and simplification since last review

Tree-SHA512: 6b9c3e3e5069d6152b45a09040f962380d114851ff0f9ff1771cf8cad7bb4fa0ba25cd787ceaa3dfa5241fb249748e2ee6987af0ccb24b786a5301b2836f8487

1 files changed, 4 insertions, 1 deletions

diff --git a/src/node/chainstatemanager_args.cpp b/src/node/chainstatemanager_args.cpp
index 1cc126cb05..bc4a815a3e 100644
--- a/src/node/chainstatemanager_args.cpp
+++ b/src/node/chainstatemanager_args.cpp
@@ -24,7 +24,10 @@
 namespace node {
 util::Result<void> ApplyArgsManOptions(const ArgsManager& args, ChainstateManager::Options& opts)
 {
-    if (auto value{args.GetBoolArg("-checkblockindex")}) opts.check_block_index = *value;
+    if (auto value{args.GetIntArg("-checkblockindex")}) {
+        // Interpret bare -checkblockindex argument as 1 instead of 0.
+        opts.check_block_index = args.GetArg("-checkblockindex")->empty() ? 1 : *value;
+    }
 
     if (auto value{args.GetBoolArg("-checkpoints")}) opts.checkpoints_enabled = *value;