mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-22 23:07:59 +01:00
Code Issues Projects Releases Wiki Activity

Merge bitcoin/bitcoin#27823: init: return error when block index is non-contiguous, fix feature_init.py file perturbation

Browse source 
d27b9a2248 test: fix feature_init.py file perturbation (Martin Zumsande)
ad66ca1e47 init: abort loading of blockindex in case of missing height. (Martin Zumsande)

Pull request description:

  When the block index database is non-contiguous due to file corruption (i.e. it contains indexes of height `x-1` and `x+1`, but not `x`), bitcoind can currently crash with an assert in `BuildSkip()` / `GetAncestor()` during `BlockManager::LoadBlockIndex()`:
  ```
  bitcoind: chain.cpp:112: const CBlockIndex* CBlockIndex::GetAncestor(int) const: Assertion `pindexWalk->pprev' failed.
  ```
  This PR changes it such that we instead return an `InitError` to the user.

  I stumbled upon this because I noticed that the file perturbation in `feature_init.py`  wasn't working as intended, which is fixed in the second commit:
  * Opening the file twice in one `with` statement would lead to `tf_read` being empty, so the test wouldn't perturb anything but replace the file with a new one. Fixed by first opening for read, then for write.
  * We need to restore the previous state after perturbations, so that only the current perturbation is active and not a mix of the current and previous ones.
  * I also added `checkblocks=200` to the startup parameters so that corruption in earlier blocks of `blk00000.dat` is detected during init verification and not ignored.

  After fixing `feature_init.py` like that I'd run into the `assert` mentioned above (so running the testfix from the second commit without the first one is a way to reproduce it).

ACKs for top commit:
  achow101:
    ACK d27b9a2248
  furszy:
    Code ACK d27b9a224
  fjahr:
    Code review ACK d27b9a2248

Tree-SHA512: 2e54da6030c5813c86bd58f816401e090bb43c5b834764a5e3c0e55dbfe09e423f88042cab823db3742088204b274d4ad2abf58a3832a4b18328b11a30bf7094
This commit is contained in:
Andrew Chow 2023-10-04 15:36:48 -04:00
commit ab163b0fb5
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
2 changed files with 24 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
src/node/blockstorage.cpp
View file

@ -410,8 +410,13 @@ bool BlockManager::LoadBlockIndex(const std::optional<uint256>& snapshot_blockha
std::sort(vSortedByHeight.begin(), vSortedByHeight.end(),
CBlockIndexHeightOnlyComparator());
CBlockIndex* previous_index{nullptr};
for (CBlockIndex* pindex : vSortedByHeight) {
if (m_interrupt) return false;
if (previous_index && pindex->nHeight > previous_index->nHeight + 1) {
return error("%s: block index is non-contiguous, index of height %d missing", __func__, previous_index->nHeight + 1);
}
previous_index = pindex;
pindex->nChainWork = (pindex->pprev ? pindex->pprev->nChainWork : 0) + GetBlockProof(*pindex);
pindex->nTimeMax = (pindex->pprev ? std::max(pindex->pprev->nTimeMax, pindex->nTime) : pindex->nTime);

24
test/functional/feature_init.py
View file

@ -5,6 +5,7 @@
"""Stress tests related to node initialization."""
import os
from pathlib import Path
import shutil
from test_framework.test_framework import BitcoinTestFramework, SkipTest
from test_framework.test_node import ErrorMatch
@ -47,7 +48,7 @@ class InitStressTest(BitcoinTestFramework):
def start_expecting_error(err_fragment):
node.assert_start_raises_init_error(
extra_args=['-txindex=1', '-blockfilterindex=1', '-coinstatsindex=1'],
extra_args=['-txindex=1', '-blockfilterindex=1', '-coinstatsindex=1', '-checkblocks=200', '-checklevel=4'],
expected_msg=err_fragment,
match=ErrorMatch.PARTIAL_REGEX,
)
@ -101,9 +102,9 @@ class InitStressTest(BitcoinTestFramework):
}
files_to_perturb = {
'blocks/index/*.ldb': 'Error opening block database.',
'blocks/index/*.ldb': 'Error loading block database.',
'chainstate/*.ldb': 'Error opening block database.',
'blocks/blk*.dat': 'Error opening block database.',
'blocks/blk*.dat': 'Corrupted block database detected.',
}
for file_patt, err_fragment in files_to_delete.items():
@ -124,18 +125,31 @@ class InitStressTest(BitcoinTestFramework):
check_clean_start()
self.stop_node(0)
self.log.info("Test startup errors after perturbing certain essential files")
for file_patt, err_fragment in files_to_perturb.items():
shutil.copytree(node.chain_path / "blocks", node.chain_path / "blocks_bak")
shutil.copytree(node.chain_path / "chainstate", node.chain_path / "chainstate_bak")
target_files = list(node.chain_path.glob(file_patt))
for target_file in target_files:
self.log.info(f"Perturbing file to ensure failure {target_file}")
with open(target_file, "rb") as tf_read, open(target_file, "wb") as tf_write:
with open(target_file, "rb") as tf_read:
contents = tf_read.read()
tweaked_contents = bytearray(contents)
tweaked_contents[50:250] = b'1' * 200
# Since the genesis block is not checked by -checkblocks, the
# perturbation window must be chosen such that a higher block
# in blk*.dat is affected.
tweaked_contents[150:350] = b'1' * 200
with open(target_file, "wb") as tf_write:
tf_write.write(bytes(tweaked_contents))
start_expecting_error(err_fragment)
shutil.rmtree(node.chain_path / "blocks")
shutil.rmtree(node.chain_path / "chainstate")
shutil.move(node.chain_path / "blocks_bak", node.chain_path / "blocks")
shutil.move(node.chain_path / "chainstate_bak", node.chain_path / "chainstate")
if __name__ == '__main__':
InitStressTest().main()