blockstorage: fail instead of storing a null XOR key over a lost one #35938

kwsantiago commented at 3:37 AM on August 9, 2026: contributor

blocks/xor.dat holds the 8-byte key that block and undo data is obfuscated with. If the file goes missing, InitBlocksdirXorKey silently stores an all-zero key, every read of the existing data then fails, and the node reports:

Corrupted block database detected.
Please restart with -reindex or -reindex-chainstate to recover.

The data is not corrupt, only the key was lost. Restoring the key at that point brings the chain back untouched.

Following the suggested recovery instead makes things worse. -reindex reports no error at all: it completes, leaves the node at height 0 having silently discarded the chain, and rewrites the start of the block file with a null-key genesis. Restoring the key afterwards then still loads 0 blocks on the first reindex, and only recovers on a second one. A user who sees an empty chain twice concludes the data is gone and resyncs from scratch.

A missing key file is legitimate for a fresh blocksdir, and for one written before v28 where the data is unobfuscated and the null key is correct. Both are distinguishable from a lost key by whether the block files still start with the network magic, so check that and fail with an actionable error instead. The null key is not written in that case, so the original key can still be restored.

One file matching the magic proves the data is unobfuscated. Concluding the opposite takes every file disagreeing, so a single damaged file cannot misfire the check.

This guards the moment the key goes missing. It cannot help a node that already stored a null key on an earlier version, since xor.dat then exists and is read as-is.

Reproduce on master

Build a chain and keep a copy of the key:

bitcoind -regtest -daemonwait
bitcoin-cli -regtest createwallet w
bitcoin-cli -regtest generatetoaddress 20 $(bitcoin-cli -regtest -rpcwallet=w getnewaddress)
bitcoin-cli -regtest stop
cp <datadir>/regtest/blocks/xor.dat /tmp/xor.bak

The data is intact and the key alone recovers it:

rm <datadir>/regtest/blocks/xor.dat
bitcoind -regtest                       # "Corrupted block database detected."
cp /tmp/xor.bak <datadir>/regtest/blocks/xor.dat
bitcoind -regtest -daemonwait
bitcoin-cli -regtest getblockcount      # 20
bitcoin-cli -regtest verifychain 4 20   # true

Following the error message instead:

rm <datadir>/regtest/blocks/xor.dat
bitcoind -regtest -reindex -daemonwait
bitcoin-cli -regtest getblockcount      # 0, and no error was reported

With this change the node refuses to start on both paths, leaves the block file untouched, and does not write a null key.

Testing

Three cases added to feature_blocksxor.py: a lost key is detected and no null key is written over it; an unobfuscated blocksdir with no key still starts with XOR enabled (the pre-v28 upgrade path); and a single unreadable block file is not mistaken for a lost key. The first fails on master with the output above. The pre-existing steps still pass, and the full functional suite passes.

DrahtBot added the label Block storage on Aug 9, 2026

DrahtBot commented at 3:37 AM on August 9, 2026: 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/35938.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept NACK	l0rinc

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#30342 (kernel, logging: Pass Logger instances to kernel objects 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.

DrahtBot added the label CI failed on Aug 9, 2026

DrahtBot commented at 5:00 AM on August 9, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task tidy: https://github.com/bitcoin/bitcoin/actions/runs/31292797905/job/93192788106</sub> <sub>LLM reason (✨ experimental): CI failed because clang-tidy (modernize-use-starts-ends-with) reported an error in node/blockstorage.cpp for using substr instead of starts_with.</sub>

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

blockstorage: fail instead of storing a null XOR key over a lost one

When blocks/xor.dat is missing, a null key is stored and any pre-existing
obfuscated block data becomes unreadable. This surfaces later as
"Corrupted block database detected", which leads users to delete the block
files and resync, even though the data is intact and only the 8-byte key
was lost.

A missing key file is legitimate for a fresh blocksdir, and for one written
before the XOR key was introduced where the data is stored unobfuscated, so
distinguish those from a lost key by checking whether an existing block file
still starts with the network magic, and only fail when it does not.

72ced9ee0d

kwsantiago force-pushed on Aug 9, 2026

DrahtBot removed the label CI failed on Aug 9, 2026

l0rinc commented at 3:52 AM on August 10, 2026: contributor

If the file goes missing

The data is not corrupt, only the key was lost

What would be the reason for this loss? We don't usually handle it gracefully when someone was fiddling with the local data.

kwsantiago commented at 5:28 PM on August 10, 2026: contributor

Whatever the cause (a partial copy of a blocks directory, or a storage fault taking out the one small file sitting next to the large stable ones), the problem is that we then report "Corrupted block database detected" on intact data and the -reindex we recommend completes without error at height 0, silently discarding the chain.

l0rinc commented at 7:29 PM on August 10, 2026: contributor

Thanks, I hope we can get there eventually (gracefully handling careless local tinkering), but we have much bigger problems to fix, I think this isn't a priority: NACK.

in doc/files.md:53 in 72ced9ee0d

  49 | @@ -52,13 +50,12 @@ Subdirectory       | File(s)               | Description
  50 |  `blocks/index/`    | LevelDB database      | Block index; `-blocksdir` option does not affect this path
  51 |  `blocks/`          | `blkNNNNN.dat`<sup>[\[2\]](#note2)</sup> | Actual Bitcoin blocks (dumped in network format, 128 MiB per file)
  52 |  `blocks/`          | `revNNNNN.dat`<sup>[\[2\]](#note2)</sup> | Block undo data (custom format)
  53 | -`blocks/`          | `xor.dat`             | Rolling XOR pattern for block and undo data files
  54 | +`blocks/`          | `xor.dat`             | Rolling XOR pattern for block and undo data files; required to read them, so losing it makes any existing block and undo data unreadable

sedited commented at 8:42 PM on August 10, 2026:

Adding this comment doesn't make sense to me. I would remove it again.

in src/node/blockstorage.cpp:1174 in 72ced9ee0d

1213 | +        obfuscated = true;
1214 |      }
1215 | -
1216 | -    return pos;
1217 | +    return obfuscated;
1218 |  }

sedited commented at 8:46 PM on August 10, 2026:

This seems a bit involved to me. From what I can tell from your description the actual problem is that the current corruption detection logic will prompt a user to reindex if the block files themselves cannot be read properly. I think it would be preferable to improve that instead, i.e. either crashing or shutting down in a way that does not trigger the reindex prompt.