diff options
author | Andrew Chow <github@achow101.com> | 2023-10-04 15:36:48 -0400 |
---|---|---|
committer | Andrew Chow <github@achow101.com> | 2023-10-04 15:36:57 -0400 |
commit | ab163b0fb5bc9c4ec12f03b57ace1753354e05cf (patch) | |
tree | 820ef19c785bf1b9fbfd67dc36c6fd375bdbcc76 /test | |
parent | 30b3477507c7ac8d022a83d68afd560fe7ffc801 (diff) | |
parent | d27b9a2248476439ddab7700327f074005a810d5 (diff) | |
download | bitcoin-ab163b0fb5bc9c4ec12f03b57ace1753354e05cf.tar.xz |
Merge bitcoin/bitcoin#27823: init: return error when block index is non-contiguous, fix feature_init.py file perturbation
d27b9a2248476439ddab7700327f074005a810d5 test: fix feature_init.py file perturbation (Martin Zumsande)
ad66ca1e475d2546dbbda206465307613108a15d 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 d27b9a2248476439ddab7700327f074005a810d5
furszy:
Code ACK d27b9a224
fjahr:
Code review ACK d27b9a2248476439ddab7700327f074005a810d5
Tree-SHA512: 2e54da6030c5813c86bd58f816401e090bb43c5b834764a5e3c0e55dbfe09e423f88042cab823db3742088204b274d4ad2abf58a3832a4b18328b11a30bf7094
Diffstat (limited to 'test')
-rwxr-xr-x | test/functional/feature_init.py | 24 |
1 files changed, 19 insertions, 5 deletions
diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py index 64ca312b84..94f5116f9b 100755 --- a/test/functional/feature_init.py +++ b/test/functional/feature_init.py @@ -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() |