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

fixes #33128.

this picks up #33164.

Chainstate::LoadGenesisBlock() could rewrite the genesis block even when blk00000.dat already contained valid block data. this becomes a problem when the block index is missing or corrupted while the block files are still intact, such as during an explicit -reindex or an incomplete shutdown.

a call to LoadGenesisBlock() eventually reaches AllocateFileRange(), which falls back to writing zeroes to extend the file on platforms without a real posix_fallocate() syscall. when the index is missing, this can overwrite existing block data beyond genesis. in #33128, deleting blocks/index and running -reindex could therefore destroy the remaining blocks and leave the node with only genesis, without reporting any error.

the primary fix is in LoadGenesisBlock(): check for an existing valid genesis block at {0, 0} before writing it, and reuse it when present.

AllocateFileRange() also gets a small defensive change: its fallback no longer writes over a range that is already within the file. it only extends the file when necessary, protecting other callers that may encounter the same situation.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35524.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

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

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #35646 (RFC: Separate out runtime errors from BlockValidationState using util::Expected by yuvicc)
• #30342 (kernel, logging: Deliver each context's log output to its own logging connection by ryanofsky)
• #29700 (kernel, refactor: return error status on all fatal errors by ryanofsky)

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.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

test plan

<details> <summary>bug reproduction (before the fix) </summary> <i>(assuming you're at this PR's merge base i.e. ed7dd7cf4e1561a97edf72eba29a67b14e28c717)</i>

• on an affected system (e.g. OpenBSD or NetBSD), follow the steps in the issue's description: #33128#issue-3286010421

• on systems that have posix_fallocate(), force the fallback path by applying this patch (profusely based on maflcko's comment):

  diff --git a/src/util/fs_helpers.cpp b/src/util/fs_helpers.cpp
  index be7f1ee5a2..ada3280dbc 100644
  --- a/src/util/fs_helpers.cpp
  +++ b/src/util/fs_helpers.cpp
  @@ -200,7 +200,7 @@ void AllocateFileRange(FILE* file, unsigned int offset, unsigned int length)
       }
       ftruncate(fileno(file), static_cast<off_t>(offset) + length);
   #else
  -#if defined(HAVE_POSIX_FALLOCATE)
  +#if 0
       // Version using posix_fallocate
       off_t nEndPos = (off_t)offset + length;
       if (0 == posix_fallocate(fileno(file), 0, nEndPos)) return;
  

  then run:

  ./build/test/functional/test_runner.py feature_reindex_init.py --failfast
  

  which MUST fail with: AssertionError: not(0 == 200).

</details>

<details> <summary>fix verification (after applying the fix) </summary>

after reindexing, getblockcount should return the same number of blocks that existed before deleting the index/ directory.

</details>

cc: @cedwies @hebasto

I was a bit confused initially by the PR description. If I understand the higher level problem here correctly it is that if a node is restarted with a broken block index and we land in LoadGenesisBlock, it will write the genesis block over the existing one. On platforms without posix_fallocate this will clobber the rest of the block file with zeros, because the pre-allocation code, absent a block index providing it with the relevant block file cursors, thinks it needs to allocate the file first. Can you re-write the description to make it a bit more clear what is going on?

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 disk in LoadGenesisBlock by calling ReadBlock with a 0 flat file position?

Can you re-write the description to make it a bit more clear what is going on?

that's right. consider it done

though.. before i rewrite the PR description, i just wanted to quickly establish consensus with you on the direction i would like to take so that we are on the same page. i am going to expound on this, just below

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 disk in LoadGenesisBlock by calling ReadBlock with a 0 flat file position?

good idea! thanks for the suggestion. much easier to prevent the clobbering much earlier in the AllocateFileRange()'s caller graph

so what i plan on doing is to make the LoadGenesisBlock() fix (your suggestion) the primary change i.e. read genesis back from {0,0} before writing, and skip WriteBlock() entirely if there is already a valid genesis block there. that directly fixes the scenario we are concerned about.

for AllocateFileRange(), i am going to drop the cleverness i was originally playing with around block-spaced single-byte writes and keep the change small. the fallback will simply skip the write entirely if the target range is already within the current file size, otherwise fall back to the old bulk-write behavior unchanged => EDIT: oh, wait! i remember why i added the clever counting (the remaining = end_pos - file_size). i went with that because once we know the current file size, the fallback only needs to write enough bytes to reach end_pos, instead of the full length. the AllocateFileRange's doc comment: https://github.com/bitcoin/bitcoin/blob/5ca3773414df36b52d6ea6c0a0ae0b1a1ad1f435/src/util/fs_helpers.cpp#L196-L199 allows offset to be before the current EOF (most common case in practice), so using length directly can extend the file on disk past the requested end. for instance, with a 100,000-byte file and offset = 90,000, length = 50,000, the target is 140,000 bytes: remaining writes 40,000 bytes and reaches the target exactly, while using length writes 50,000 bytes and extends the file to 150,000 bytes

also, FlatFileSeq::Allocate's doc comment: https://github.com/bitcoin/bitcoin/blob/f839ee1afb0ff02c58422dae6d2678e5e654b605/src/flatfile.h#L64-L73 says allocations are rounded up (ceil division) to the nearest chunk-size multiple, meaning a block file's real on-disk size is routinely ahead of the logical write position by design, with that preallocated space being used up gradually). so offset < file_size is a pretty common situation pragmatically, which is why i had the fallback case compute the exact gap instead of writing the full length on top of whatever headroom that already happens to exist

also poked at the caller graph for AllocateFileRange() (via FlatFileSeq::Allocate) out of curiosity, to see if LoadGenesisBlock() was the only thing exposed to this pattern. nothing conclusive, but flagging in case it is useful:

• WriteFilterToDisk (block filter index) goes through the same allocator, and a full index rebuild resets its write cursor from scratch. in theory, a stale-cursor-vs-intact-disk-data mismatch could recreate the same bug shape as genesis, but i have not fully confirmed whether that is actually reachable in practice or just a narrow edge case
• i also wondered about WriteBlockUndo/rev*.dat via ConnectBlock, but it looks like -reindex has historically deleted rev*.dat files upfront specifically to keep block-file bookkeeping in sync with disk -- so this one is probably not a real issue

not claiming either is a live bug, just noting that the same pattern can turn up elsewhere in the caller graph. that is why i am leaning towards keeping the backstop (at least) rather than scoping the fix to genesis alone

thoughts on the approach ?

for now, i am marking this as draft so that i can work on the new implementation of the fix

<!--85328a0da195eb286784d51f73fa0af9-->

🚧 At least one of the CI tasks failed. _{Task NetBSD Cross: https://github.com/bitcoin/bitcoin/actions/runs/36272354532/job/108488583289} _{LLM reason (✨ experimental): CI failed due to a C++ build error in src/validation.cpp where block was used without being declared (use of undeclared identifier 'block').}

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

• Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

• A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

• An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

changes made:

• rebased on latest master and pushed the new fix implementation
• updated both the PR title and description to better capture what is being fixed in this PR as requested in #35524 (comment)

also, i verified that the fix works correctly on both NetBSD 11.0 and FreeBSD 15.0

@sedited

from: #35524 (comment)

the easier fix might be to check if we can read the genesis block from disk in LoadGenesisBlock by calling ReadBlock with a 0 flat file position?

uhmm! i tried this approach, but it did not fix the issue.

i used the following patch to implement your suggestion, along with forcing the fallback path since i was testing on Linux:

<details open> <summary>diff </summary>

diff --git a/src/util/fs_helpers.cpp b/src/util/fs_helpers.cpp
index 9723c145de..5dfb53c680 100644
--- a/src/util/fs_helpers.cpp
+++ b/src/util/fs_helpers.cpp
@@ -224,7 +224,7 @@ void AllocateFileRange(FILE* file, unsigned int offset, unsigned int length)
     }
     ftruncate(fileno(file), static_cast<off_t>(offset) + length);
 #else
-#if defined(HAVE_POSIX_FALLOCATE)
+#if 0
     // Version using posix_fallocate
     off_t nEndPos = (off_t)offset + length;
     if (0 == posix_fallocate(fileno(file), 0, nEndPos)) return;
diff --git a/src/validation.cpp b/src/validation.cpp
index c85a3af730..919fee915e 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -4957,10 +4957,21 @@ bool ChainstateManager::LoadGenesisBlock()
     }

     try {
-        FlatFilePos blockPos{m_blockman.WriteBlock(genesis_block, 0)};
-        if (blockPos.IsNull()) {
-            LogError("Writing genesis block to disk failed");
-            return false;
+        FlatFilePos blockPos{0, 0};
+        // The block index may be missing or corrupted (e.g. after an
+        // incomplete shutdown or manual recovery) while block file 0
+        // still contains a valid genesis block from a previous run.
+        // Check for that first. Otherwise, WriteBlock() eventually
+        // reaches AllocateFileRange(), whose fallback on platforms without
+        // a real preallocation syscall cannot tell that the file already
+        // contains data beyond this point and may zero it out.
+        CBlock existing_block;
+        if (!m_blockman.ReadBlock(existing_block, blockPos, genesis_block.GetHash())) {
+            blockPos = m_blockman.WriteBlock(genesis_block, 0);
+            if (blockPos.IsNull()) {
+                LogError("Writing genesis block to disk failed");
+                return false;
+            }
         }
         CBlockIndex* pindex{m_blockman.AddToBlockIndex(genesis_block, m_best_header)};
         ReceivedBlockTransactions(genesis_block, pindex, blockPos);

</details>

i then ran ./build/test/functional/test_runner.py feature_reindex_init, which still failed with getblockcount() returning 0 instead of the expected 200:

<details> <summary>output </summary>

Temporary test directory at /tmp/test_runner_₿_🏃_20260930_215123
Remaining jobs: [feature_reindex_init.py]
1/1 - feature_reindex_init.py failed (exit code 1), Duration: 5 s

stdout:
2026-09-30T18:51:23.738462Z TestFramework (INFO): PRNG seed is: 3969550340272191845
2026-09-30T18:51:23.789421Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20260930_215123/feature_reindex_init_0
2026-09-30T18:51:27.014326Z TestFramework (INFO): Removing the block index leads to init error
2026-09-30T18:51:27.784859Z TestFramework (INFO): Allowing the reindex should work fine
2026-09-30T18:51:29.057559Z TestFramework (ERROR): Unexpected exception:
Traceback (most recent call last):
  File "/home/madman/Documents/btc/btc-core/my-btc-fork/test/functional/test_framework/test_framework.py", line 145, in main
    self.run_test()
  File "/home/madman/Documents/btc/btc-core/my-btc-fork/build/test/functional/feature_reindex_init.py", line 29, in run_test
    assert_equal(node.getblockcount(), 200)
  File "/home/madman/Documents/btc/btc-core/my-btc-fork/test/functional/test_framework/util.py", line 94, in assert_equal
    raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
AssertionError: not(0 == 200)
2026-09-30T18:51:29.112141Z TestFramework (INFO): Not stopping nodes as test failed. The dangling processes will be cleaned up later.
2026-09-30T18:51:29.112592Z TestFramework (WARNING): Not cleaning up dir /tmp/test_runner_₿_🏃_20260930_215123/feature_reindex_init_0
2026-09-30T18:51:29.112854Z TestFramework (ERROR): Test failed. Test logging available at /tmp/test_runner_₿_🏃_20260930_215123/feature_reindex_init_0/test_framework.log

...

TEST                    | STATUS    | DURATION

feature_reindex_init.py | ✖ Failed  | 5 s

ALL                     | ✖ Failed  | 5 s (accumulated)
Runtime: 5 s

</details>

tested both fixes independently and it turns out the ReadBlock() check in ChainstateManager::LoadGenesisBlock() alone does not fix the reported bug; the test still fails without the AllocateFileRange guard unlike the one with the AllocateFileRange guard