#35524 validation: Avoid rewriting the genesis block during index recovery

ReviewabilityReviewsAgreementSize
#35524 winterrdog validation: Avoid rewriting the genesis block during index recoveryNeeds rebase0Crickets40
  • Prevent reindexing from silently destroying block data on systems without native posix_fallocate
  • Avoid rewriting the genesis block and zeroing subsequent blocks during block index recovery
  • Ready to review
  • No review verdicts recorded
  • No substantive review comments either way yet
  • Suggestion to widen offset type for safety was adopted (thomasbuilds)
  • Author tested an alternative approach and found it did not fix the bug alone (sedited)
  • 40 lines added or modified outside tests
  • 0 lines added or modified in tests
  • 19 lines removed in total
  • 2 files, 2 commits
Utilities (logging, arguments, libraries)#1 of 59
P1 · bug fix
  • P1 after comparing with the other PRs here (assessed alone as P2): Raised to P1 and moved up: silent destruction of existing block data during reindex on non-posix_fallocate platforms is the most consequential correctness problem on this list.
  • P2 because preallocating within existing file boundaries could overwrite or truncate existing data
  • Fixes zero-fill fallback and Windows file allocation to preserve existing file contents
Ranking notes
Validation#3 of 53
P1 · bug fix
  • P1 after comparing with the other PRs here (assessed alone as P2): Raised to P1: silent destruction of block data during reindex on supported platforms (*BSD/OmniOS) is data loss, on par with the other P1 crash/reindex bugs here
  • P2 because recovering a missing index destroyed valid blocks and left nodes stuck at height 0
  • Protects chain data integrity during index recovery on platforms like *BSD and OmniOS
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Avoids rewriting the genesis block to disk during block index recovery if a valid genesis block is already present at block position {0, 0}. Additionally, updates `AllocateFileRange` on Windows and in the zero-fill fallback so existing file contents are not overwritten or truncated when preallocating space within the current file boundaries.

Problem: On platforms without native `posix_fallocate` (such as *BSD and illumos distributions like OmniOS), running `-reindex` or recovering from a missing block index overwrites valid block data in `blk00000.dat`. When `ChainstateManager::LoadGenesisBlock()` unconditionally rewrites the genesis block, `AllocateFileRange` zeroes out subsequent block data, causing `-reindex` to quietly finish with the chain height stuck at 0.

Evidence of importance:

  • Linked issue #33128 ('*BSD, OmniOS: -reindex is broken') reports that running `-reindex` with deleted block index files silently destroyed subsequent blocks, leaving the node at height 0.
  • Commit message: 'The log reports reindexing finished, but the blocks after genesis are gone and the chain height comes back as 0. Now, we check whether a valid genesis block is already present before writing one, and reuse it when possible.'

Risk of a mistake: medium (validation). Mistakes in genesis block detection or disk allocation handling could prevent node startup, corrupt block files, or misinitialize the block index.

Changed since the previous assessment: Added changes to `ChainstateManager::LoadGenesisBlock()` in `src/validation.cpp` to inspect `blk00000.dat` for an existing genesis block at offset {0, 0} and reuse it instead of unconditionally invoking `WriteBlock()`.

Reviewability

Ready · Needs rebase

Conflicts with master; review the logic now, the rebase is mechanical.

Waiting on (from the thread read):

  • reviewer: sedited to respond to author's finding that reading genesis alone does not resolve the bug 2026-09-30

Agreement

Crickets

no substantive comment either way

Thread notes: Updated sedited's suggestion c:5742883004 to answered after winterrdog tested the approach and reported that it does not fix the issue without AllocateFileRange changes.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
thomasbuilds (contributor)suggestion, safetywiden offset to off_t before adding length to match other platformsfixed · fix pushed · author applied the widening castansweredyes2026-06-14: 'nit: the Windows, macOS, and posix computations above all widen `offset` before adding `length`'
Status from: 2026-06-16: winterrdog: 'i think it's a good point for safety... applied the change'
sedited (member)suggestion, approachcheck whether genesis can be read from disk in LoadGenesisBlock instead of patching low-level file handling codeanswered · author tested the approach and found it does not fix the bug without the AllocateFileRange guardansweredyes2026-09-19: 'I wonder if instead of trying to fix this salvaging scenario by doing a bunch of patches in our low-level file handling code, the easier fix might be to check if we can read the genesis block from dis'
Status from: 2026-09-30: winterrdog: 'tested both fixes independently and it turns out the `ReadBlock()` check in `ChainstateManager::LoadGenesisBlock()` alone does not fix the reported bug'

Participants:

  • thomasbuilds (contributor, neutral, 2 statements 2026-06-13 to 2026-06-14): raised nits on indentation and widening offset before addition
  • sedited (member, neutral, 1 statements 2026-09-19 to 2026-09-19): suggested reading genesis from disk in LoadGenesisBlock rather than patching low-level file code

Files

0 lines under test/bench/ci.

  • src/util/fs_helpers.cpp +25/-15
  • src/validation.cpp +15/-4

Card

The short text the ranking pass compares across PRs.

Prevents silent block data destruction during `-reindex` and index recovery on platforms without native `posix_fallocate` (*BSD, OmniOS). `ChainstateManager::LoadGenesisBlock()` now checks for an existing genesis block in block file 0 before writing a new one, avoiding calls to `AllocateFileRange` that previously zeroed out subsequent blocks. In addition, the Windows and fallback implementations of `AllocateFileRange` are fixed to only extend files rather than clobbering existing bytes. This resolves issue #33128.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-01T01:22, confidence high, input hash f8382c48d76bf744
thread read through 2026-09-30 (12 statements), head fd2d21053f
labels: Utils/log/libs, Needs rebase · draft

Categories

Utilities (logging, arguments, libraries)

P1 · bug fix (assessed alone as P2; Raised to P1 and moved up: silent destruction of existing block data during reindex on non-posix_fallocate platforms is the most consequential correctness problem on this list.)

Fixes AllocateFileRange so preallocating within existing file boundaries does not overwrite or truncate existing data on Windows and platforms using the zero-fill fallback. As described, the zero-fill fallback was zeroing out existing data when preallocating within current boundaries, violating basic file utility safety.

Membership: Has the Utils/log/libs label and changes src/util/fs_helpers.cpp (+25/-15).

Factors: security/stability 2, bug 2, performance 0, user value 1, leverage 0

Validation

P1 · bug fix (assessed alone as P2; Raised to P1: silent destruction of block data during reindex on supported platforms (*BSD/OmniOS) is data loss, on par with the other P1 crash/reindex bugs here)

Fixes silent block data destruction during -reindex on platforms without native posix_fallocate (like *BSD and OmniOS). Issue #33128 reported that recovering from a missing block index caused subsequent block data in blk00000.dat to be zeroed out, leaving nodes stuck at height 0; this PR preserves chain integrity by checking for an existing genesis block before writing.

Membership: Modifies ChainstateManager::LoadGenesisBlock in src/validation.cpp to manage block index recovery and prevent blk00000.dat corruption.

Factors: security/stability 2, bug 2, performance 0, user value 1, leverage 0