index: move disk read lookups to base class #32694

1. 
   furszy commented at 8:38 pm on June 6, 2025: member

   Combining common refactors from #24230 and #26966, aiming to move both efforts forward while reducing their size and review burden.

   Broadly, #24230 focuses on enabling indexes to run in a separate process, and #26966 aims to parallelize the indexes initial synchronization process. A shared prerequisite for both is ensuring that only the base index class interacts with the node’s chain internals - child index classes should instead operate solely through chain events.

   This PR moves disk read lookups from child index classes to the base index class. It also includes a few documentation improvements and a test-only code cleanup.

2. test: indexes, avoid creating threads when sync runs synchronously

   The indexes test call StartBackgroundSync(), which spawns a thread to run Sync(),
   only for the test thread to wait for it to complete by calling IndexWaitSynced().
   
   So, since the sync is performed synchronously, we can skip the extra thread creation
   entirely and call Sync() directly.

   331a25cb16
3. index: document NextSyncBlock() behavior

   It wasn’t immediately clear that this function can also return
   the first block after the fork point during a reorg.

   c0bd2c5a8a
4. indexes, refactor: Stop requiring CBlockIndex type to call IsBIP30Unspendable

   This commit does not change behavior in any way.

   289ec6f945
5. indexes, refactor: Remove remaining CBlockIndex* uses in index Rewind methods

   Move backwards ReadBlockFromDisk code from CoinStatsIndex::CustomRewind
   to BaseIndex::Rewind
   
   Move undo UndoReadFromDisk code from CoinStatsIndex::ReverseBlock to
   BaseIndex::Rewind
   
   This commit does change behavior slightly. Since the new CustomRemove
   methods only take a single block at a time instead of a range of
   disconnected blocks, when they call CopyHeightIndexToHashIndex they will
   now do an index seek for each removed block instead of only seeking once
   to the height of the earliest removed block. Seeking instead of scanning
   is a little worse for performance if there is a >1 block reorg, but
   probably not noticable unless the reorg is very deep.

   50696f0778
6. index: remove CBlockIndex access from CustomAppend()

   Moved CBlockUndo disk read lookups from child index classes to
   the base index class.
   
   The goal is for child index classes to synchronize only through
   events, without directly accessing the chain database.
   
   This change will enable future parallel synchronization mechanisms,
   reduce database access (when batched), and contribute toward the
   goal of running indexes in a separate process (with no chain
   database access).

   3f390efa31
7. 
   DrahtBot commented at 8:38 pm on June 6, 2025: contributor

   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/32694.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #31308 (ci, iwyu: Treat warnings as errors for specific directories by hebasto)
   • #30469 (index: Fix coinstats overflow and introduce index versioning by fjahr)
   • #29307 (util: explicitly close all AutoFiles that have been written by vasild)
   • #26966 (index: initial sync speedup, parallelize process by furszy)

   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.

8. DrahtBot added the label UTXO Db and Indexes on Jun 6, 2025

Contributors
furszy DrahtBot

Labels
UTXO Db and Indexes