validation: Avoid rewriting genesis block when already on disk #35524

fixes #33128

this picks up #33164

on a normal start (without -reindex), CompleteChainstateInitialization() calls LoadGenesisBlock(), which writes the genesis block to blk00000.dat unless the block index already contains it. if blocks/index was deleted, then the index is empty, so genesis is unconditionally written again even though the file is still intact on disk and holds all the later blocks

that write eventually reaches AllocateFileRange(), which allocates space from the start of the file. with posix_fallocate(), this only reserves space. on platforms without it (e.g. OpenBSD, NetBSD), the fallback writes zeroes over the requested range, replacing the blocks that were already there

only after that, does the node notice that the chainstate no longer matches the block index and fail with "Error initializing block database", asking for -reindex. by then, the blocks are already gone, so the reindex finds the genesis only, reports success, and getblockcount returns 0

this PR makes LoadGenesisBlock() read the genesis block back from disk first and only write it when it is not already there. if genesis is already on disk, update the blockfile info with its position and size so that the next block is written after it instead of overwriting it

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--006a51241073e994b41acfe9ec718e94-->

External sites

• Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/35524
• Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/35524.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/35524#author-ralph

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

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

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

No conflicts as of last run.

<!--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

[!WARNING] EDIT: this is incorrect, do not take it very seriously because i later found out what i did wrong when implementing the suggestion. look at this comment for the fixed flaw

@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> <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

changes:

• updated the PR description & title
• rebased and fixed conflicts from merged changes
• my earlier approach of the ReadBlock() check (suggested by @sedited) read at {0, 0}, but block positions point past the 8-byte block header, so it never matched and genesis was always rewritten. now, it reads at {0, STORAGE_HEADER_BYTES}
• dropped the AllocateFileRange() changes i had proposed earlier because they are not exactly what is needed to resolve this issue albeit were an alternate fix, but at a much lower level

<!--85328a0da195eb286784d51f73fa0af9-->

🚧 At least one of the CI tasks failed. _{Task macOS native, fuzz: https://github.com/bitcoin/bitcoin/actions/runs/42369614221/job/128394828567} _{LLM reason (✨ experimental): The fuzz-test script failed because the connect_block target hit a ReadBlock check failure and exited with code 250.}

<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:

• updated the commit message and PR description to reflect the changes.
• when genesis is already on disk, its blockfile info is now updated via UpdateBlockInfo(), so the next block is written after it instead of overwriting it. skipping this caused the connect_block fuzz target to fail because the next block overwrote genesis, causing ReadBlock() to fail the hash check.