highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-01-19 06:43:45 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#23365: index: Fix backwards search for bestblock

Browse Source 
9600ea0145 test: Add edge case of pruning up to index height (Martin Zumsande)
698c524698 index: Fix backwards search for bestblock (Martin Zumsande)

Pull request description:

  This PR attempts to fix an intermittent Init issue encountered during the stress testing of #23289, which relates to the pruning-compatible filter reconstruction logic introduced in #15946.

  The problem would occur when the node starts with `-txindex=1` but `ThreadSync` is interrupted after it sets `m_best_block_index` to Genesis, and before it gets do any further work.
  In that case, during the next restart of the node, an Init error would be thrown because  `BaseIndex::Init()` tries to backtrack from the tip to the last block which has been successfully indexed (here: Genesis), but the backtracking logic didn't work properly in this case:
  The loop
  `while (block_to_test && block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA))` checks if a predecessor exists **before** performing the check `block_to_test == block` and then possbily setting `prune_violation = false`
  If `block_to_test` and `block` are the Genesis block this check will not be reached because `block->pprev` does not exist.

  To reproduce this bug on regtest:
  1) start a node with a fresh datadir using `-txindex=1` (or any other index)
  2) stop and restart without any index
  3) mine a block
  3) stop and restart again with the index enabled
  ->InitError `Error: txindex best block of the index goes beyond pruned data. (...)`

  Fix this by requiring that we have the data for the block of the current iteration `block` (instead of requiring it for the predecessor `block->pprev`)
  That way, the check for `block_to_test == block` is also reached when `block_to_test` is the Genesis block.
  No longer requiring the data of `block->pprev` also means that we can now prune up to `m_best_block_index` height without requiring a reindex (one block more than before). I added this edge case to `feature_blockfilterindex_prune.py`, the new version should fail on master.

ACKs for top commit:
  ryanofsky:
    Partial code review ACK 9600ea0145 for the code change, not the test changes. (Test changes are indirect and little over my head.) It seems obvious that previous code `prune_violation = true, while (block->pprev)` would incorrectly detect a prune violation at the genesis block, and the fix here make sense and looks correct.

Tree-SHA512: c717f372cee8fd49718b1b18bfe237aa6ba3ff4468588c10e1272d7a2ef3981d10af4e57de51dec295e2ca72d441bc6c2812f7990011a94d7f818775e3ff1a38
This commit is contained in:
MarcoFalke
2021-12-13 13:37:55 +01:00
parent eb63b8fab9 9600ea0145
commit bf66e258a8
2 changed files with 19 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
src/index/base.cpp
View File

@@ -91,11 +91,12 @@ bool BaseIndex::Init()
const CBlockIndex* block = active_chain.Tip();
prune_violation = true;
// check backwards from the tip if we have all block data until we reach the indexes bestblock
while (block_to_test && block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) {
while (block_to_test && block && (block->nStatus & BLOCK_HAVE_DATA)) {
if (block_to_test == block) {
prune_violation = false;
break;
}
assert(block->pprev);
block = block->pprev;
}
}

23
test/functional/feature_blockfilterindex_prune.py
View File

@@ -24,9 +24,7 @@ class FeatureBlockfilterindexPruneTest(BitcoinTestFramework):
self.log.info("check if we can access a blockfilter when pruning is enabled but no blocks are actually pruned")
self.sync_index(height=200)
assert_greater_than(len(self.nodes[0].getblockfilter(self.nodes[0].getbestblockhash())['filter']), 0)
# Mine two batches of blocks to avoid hitting NODE_NETWORK_LIMITED_MIN_BLOCKS disconnection
self.generate(self.nodes[0], 250)
self.generate(self.nodes[0], 250)
self.generate(self.nodes[0], 500)
self.sync_index(height=700)
self.log.info("prune some blocks")
@@ -39,16 +37,29 @@ class FeatureBlockfilterindexPruneTest(BitcoinTestFramework):
self.log.info("check if we can access the blockfilter of a pruned block")
assert_greater_than(len(self.nodes[0].getblockfilter(self.nodes[0].getblockhash(2))['filter']), 0)
# mine and sync index up to a height that will later be the pruneheight
self.generate(self.nodes[0], 298)
self.sync_index(height=998)
self.log.info("start node without blockfilterindex")
self.restart_node(0, extra_args=["-fastprune", "-prune=1"])
self.log.info("make sure accessing the blockfilters throws an error")
assert_raises_rpc_error(-1, "Index is not enabled for filtertype basic", self.nodes[0].getblockfilter, self.nodes[0].getblockhash(2))
self.generate(self.nodes[0], 1000)
self.generate(self.nodes[0], 502)
self.log.info("prune exactly up to the blockfilterindexes best block while blockfilters are disabled")
pruneheight_2 = self.nodes[0].pruneblockchain(1000)
assert_equal(pruneheight_2, 998)
self.restart_node(0, extra_args=["-fastprune", "-prune=1", "-blockfilterindex=1"])
self.log.info("make sure that we can continue with the partially synced index after having pruned up to the index height")
self.sync_index(height=1500)
self.log.info("prune below the blockfilterindexes best block while blockfilters are disabled")
pruneheight_new = self.nodes[0].pruneblockchain(1000)
assert_greater_than(pruneheight_new, pruneheight)
self.restart_node(0, extra_args=["-fastprune", "-prune=1"])
self.generate(self.nodes[0], 1000)
pruneheight_3 = self.nodes[0].pruneblockchain(2000)
assert_greater_than(pruneheight_3, pruneheight_2)
self.stop_node(0)
self.log.info("make sure we get an init error when starting the node again with block filters")