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 /src/node | |
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 'src/node')
-rw-r--r-- | src/node/blockstorage.cpp | 5 |
1 files changed, 5 insertions, 0 deletions
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 5e61ed3100..706b62ea9b 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -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); |