index: Fix coinstats overflow and introduce index versioning #30469

1. 
   fjahr commented at 10:46 am on July 17, 2024: contributor

   Closes #26362

   This continues the work that was started with #26426. It fixes the overflow issue by switching most of the values tracked in the index from historical totals to values per block. The only effect of this is that it requires iterating over all the entries to get to these historical values, however nothing changes for the capabilities of the RPCs we have today because the switched values were always report per block and the totals were used in concepts like #19792.

   The change requires a rebuild of the index. Because of this, the PR introduces the concept of versioning to indexes and I am looking for feedback on the approach here. If an outdated version is detected the node won’t start until the user has removed the index manually or start without the option. This should prevent unwanted data loss for the user.

   Ideally the approach chosen here should be compatible with any future cases where we require an index to upgrade. The most recently such a scenario was discussed in #28241. I am currently convinced that a different approach would be a better fit there, however the discussion is still in the early stages.

   This PR also does quite a bit of renaming, reorgs some lines and adds some white space. All of this made it easier for me to work with the code.

2. index: Fix coinstatsindex overflow issue 726ec61f2d
3. index: Introduce versioning concept to indexes d6da36b22e
4. 
   DrahtBot commented at 10:46 am on July 17, 2024: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Concept ACK	mzumsande

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #28792 (Embed default ASMap as binary dump header file by fjahr)

   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.

5. DrahtBot added the label UTXO Db and Indexes on Jul 17, 2024
6. DrahtBot added the label CI failed on Jul 17, 2024
7. fjahr force-pushed on Jul 23, 2024
8. test: Add test for outdated coinstats index version 73dd2c3aca
9. fjahr force-pushed on Jul 23, 2024
10. 
    mzumsande commented at 6:07 pm on July 23, 2024: contributor

    Concept ACK

    Haven’t tested anything yet, but did you check what happens in case of a downgrade? (an index generated with this PR is loaded with master)

11. DrahtBot removed the label CI failed on Jul 23, 2024
12. 
    mzumsande commented at 5:09 pm on July 24, 2024: contributor

    Since rebuilding the coinstatsindex takes a long time and it’s not corrupted (yet) on mainnet I thought that auto-migrating to the new format instead of making the user reindex might be more user-friendly: I tried that out in https://github.com/mzumsande/bitcoin/commits/202407_coinstats_v1_automigrate/ (just an untested POC - also doesn’t process any blocks saved by hash instead of height yet, but that should be possible to add.) What do you think of that option?

Contributors
fjahr DrahtBot mzumsande

Labels
UTXO Db and Indexes