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

full analysis

https://github.com/bitcoin/bitcoin/pull/35938 · kwsantiago · +63/-0 in 2 files, 1 commits · labels: Block storage

Goal

  • Prevent node from overwriting a missing block obfuscation key with a null key
  • Stop prompting users to run a destructive reindex that silently wipes valid block storage
  • Fail startup with an explicit error directing users to restore the key file or resync

Detects whether existing block files are XOR-obfuscated when `blocks/xor.dat` is missing by inspecting block magic bytes. Instead of silently storing an all-zero XOR key and prompting users to perform a destructive `-reindex`, the node fails startup with an explicit error directing the user to restore the key or resync.

Problem: When `blocks/xor.dat` is missing, bitcoind assumes the blocks directory is either fresh or pre-v28 and stores a null XOR key. This makes existing obfuscated block data unreadable, falsely reports database corruption, and encourages a `-reindex` that silently resets the chain to height 0 and writes over genesis data.

Category: Validation (#19 of 48)

P3 · bug fix

  • P3 because prevents destructive reindexing that resets valid block data when one key file is lost
  • Limited impact because it requires out-of-band file deletion or external storage faults to trigger

Addresses a genuine UX and data preservation failure where a missing 8-byte XOR key causes the node to suggest a reindex that silently discards the block index. While requiring out-of-band file deletion or storage faults to trigger, preventing destructive actions on intact block storage is worthwhile.

Membership: Touches block file storage logic in src/node/blockstorage.cpp.

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

Reviewability: Ready

  • Ready to review

CI is passing, there are no merge conflicts, and no blocking author action is pending.

Author status: active; responded to review feedback and force-pushed updates

Open concerns:

  • l0rinc Concept NACKed, arguing that handling data loss from local tampering is not a priority compared to bigger issues.
  • sedited suggested the logic is too involved and preferred improving corruption detection so it does not trigger a reindex prompt on unreadable block files.

Resolved concerns:

  • sedited questioned a doc/files.md change in an earlier push, which the author subsequently dropped.

Agreement: Disputed

  • Concept NACK arguing recovery from local tampering is low priority (l0rinc)
  • Prefers improving corruption detection over special-casing unreadable block files (sedited)

Disputed: l0rinc Concept NACKs as low priority; sedited suggested fixing the corruption prompt instead

l0rinc posted a Concept NACK arguing that graceful recovery from local tinkering is low priority. sedited questioned the approach and suggested adjusting the corruption prompt instead, to which the author explained why suppressing reindex prompts globally would harm valid use cases.

  • l0rinc: 'We don't usually handle it gracefully when someone was fiddling with the local data... I think this isn't a priority: NACK.'
  • sedited: 'From what I can tell from your description the actual problem is that the current corruption detection logic will prompt a user to reindex... I think it would be preferable to improve that instead'
  • kwsantiago: 'That same error also covers problems where reindexing is the right fix, so suppressing the prompt across the board would hurt users that it currently helps'

Review verdicts (DrahtBot): 0 -1

Files

22 lines under test/bench/ci.

  • src/node/blockstorage.cpp +41/-0
  • test/functional/feature_blocksxor.py +22/-0

Card

This PR checks whether existing block files are obfuscated when `blocks/xor.dat` is missing, failing startup with an explicit error instead of writing an all-zero key. Currently, bitcoind falsely reports database corruption and suggests running `-reindex`, which silently truncates the chain to height 0 and leaves intact block files unreadable. The change prevents irreversible reindexing when block data is still recoverable from backup keys. The PR is ready for review but currently disputed due to a Concept NACK from l0rinc regarding whether handling local data tampering is worth prioritizing.

Data

dossier JSON · extract JSON · model openrouter/google/gemini-3.8-flash, generated 2026-09-17T16:22, confidence high, input hash b42ddbdbe9b7e534