Page MenuHomePhabricator

refactor: Make mapBlocksUnknownParent local, and rename it
ClosedPublic

Authored by PiRK on Dec 18 2023, 09:42.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC9225846943a3: refactor: Make mapBlocksUnknownParent local, and rename it
Summary

This PR has two motivations:

  • Improve code hygiene by eliminating a global variable, mapBlocksUnknownParent
  • Fix fuzz test OOM when running too long (see #19594 comment)

A minor added advantage is to release mapBlocksUnknownParent memory when the reindexing phase is done. The current situation is somewhat similar to a memory leak because this map exists unused for the remaining lifetime of the process. It's true that this map should be empty of data elements after use, but its internal metadata (indexing structures, etc.) can have non-trivial size because there can be many thousands of simultaneous elements in this map.

This PR helps our efforts to reduce the use of global variables. This variable isn't just global, it's hidden inside a function (it looks like a local variable but has the static attribute).

This global variable exists because the -reindex processing code calls LoadExternalBlockFile() multiple times (once for each block file), but that function must preserve some state between calls (the mapBlocksUnknownParent map). This PR fixes this by allocating this map as a local variable in the caller's scope and passing it in on each call. When reindexing completes, the map goes out of scope and is deallocated.

I tested this manually by reindexing on mainnet and signet. Also, the existing feature_reindex.py functional test passes.

Also use BlockHash instead of uint256.

This is a backport of core#25571

Co-authored-by: Larry Ruane <larryruane@gmail.com>

Test Plan

ninja all check-all bitcoin-fuzzers