diff --git a/src/index/base.cpp b/src/index/base.cpp index 866fbc9697..5390411432 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -94,11 +94,14 @@ 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; } + // block->pprev must exist at this point, since block_to_test is part of the chain + // and thus must be encountered when going backwards from the tip + assert(block->pprev); block = block->pprev; } } diff --git a/src/init.cpp b/src/init.cpp index 09aea533c5..c76b767d16 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -554,8 +554,8 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-version", "Print version and exit", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-addressindex", strprintf("Maintain a full address index, used to query for the balance, txids and unspent outputs for addresses (default: %u)", DEFAULT_ADDRESSINDEX), ArgsManager::ALLOW_ANY, OptionsCategory::INDEXING); - argsman.AddArg("-reindex", "Rebuild chain state and block index from the blk*.dat files on disk", ArgsManager::ALLOW_ANY, OptionsCategory::INDEXING); - argsman.AddArg("-reindex-chainstate", "Rebuild chain state from the currently indexed blocks. When in pruning mode or if blocks on disk might be corrupted, use full -reindex instead.", ArgsManager::ALLOW_ANY, OptionsCategory::INDEXING); + argsman.AddArg("-reindex", "Rebuild chain state and block index from the blk*.dat files on disk. This will also rebuild active optional indexes.", ArgsManager::ALLOW_ANY, OptionsCategory::INDEXING); + argsman.AddArg("-reindex-chainstate", "Rebuild chain state from the currently indexed blocks. When in pruning mode or if blocks on disk might be corrupted, use full -reindex instead. Deactivate all optional indexes before running this.", ArgsManager::ALLOW_ANY, OptionsCategory::INDEXING); argsman.AddArg("-spentindex", strprintf("Maintain a full spent index, used to query the spending txid and input index for an outpoint (default: %u)", DEFAULT_SPENTINDEX), ArgsManager::ALLOW_ANY, OptionsCategory::INDEXING); argsman.AddArg("-timestampindex", strprintf("Maintain a timestamp index for block hashes, used to query blocks hashes by a range of timestamps (default: %u)", DEFAULT_TIMESTAMPINDEX), ArgsManager::ALLOW_ANY, OptionsCategory::INDEXING); argsman.AddArg("-txindex", strprintf("Maintain a full transaction index, used by the getrawtransaction rpc call (default: %u)", DEFAULT_TXINDEX), ArgsManager::ALLOW_ANY, OptionsCategory::INDEXING); @@ -1343,6 +1343,19 @@ bool AppInitParameterInteraction(const ArgsManager& args) return InitError(_("No proxy server specified. Use -proxy= or -proxy=.")); } + if (args.GetBoolArg("-reindex-chainstate", false)) { + // indexes that must be deactivated to prevent index corruption, see #24630 + if (args.GetBoolArg("-coinstatsindex", DEFAULT_COINSTATSINDEX)) { + return InitError(_("-reindex-chainstate option is not compatible with -coinstatsindex. Please temporarily disable coinstatsindex while using -reindex-chainstate, or replace -reindex-chainstate with -reindex to fully rebuild all indexes.")); + } + if (g_enabled_filter_types.count(BlockFilterType::BASIC_FILTER)) { + return InitError(_("-reindex-chainstate option is not compatible with -blockfilterindex. Please temporarily disable blockfilterindex while using -reindex-chainstate, or replace -reindex-chainstate with -reindex to fully rebuild all indexes.")); + } + if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) { + return InitError(_("-reindex-chainstate option is not compatible with -txindex. Please temporarily disable txindex while using -reindex-chainstate, or replace -reindex-chainstate with -reindex to fully rebuild all indexes.")); + } + } + try { const bool fRecoveryEnabled{llmq::QuorumDataRecoveryEnabled()}; const bool fQuorumVvecRequestsEnabled{llmq::GetEnabledQuorumVvecSyncEntries().size() > 0}; diff --git a/test/functional/feature_blockfilterindex_prune.py b/test/functional/feature_blockfilterindex_prune.py index f418f2c75d..361a9cce01 100755 --- a/test/functional/feature_blockfilterindex_prune.py +++ b/test/functional/feature_blockfilterindex_prune.py @@ -15,7 +15,7 @@ from test_framework.governance import EXPECTED_STDERR_NO_GOV_PRUNE class FeatureBlockfilterindexPruneTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - self.extra_args = [["-fastprune", "-prune=1", "-blockfilterindex=1"]] + self.extra_args = [["-fastprune", "-prune=1", "-blockfilterindex=1", "-testactivationheight=v20@2000"]] def sync_index(self, height): expected = {'basic block filter index': {'synced': True, 'best_block_height': height}} @@ -25,13 +25,13 @@ 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") pruneheight = self.nodes[0].pruneblockchain(400) + # the prune heights used here and below are magic numbers that are determined by the + # thresholds at which block files wrap, so they depend on disk serialization and default block file size. assert_equal(pruneheight, 366) self.log.info("check if we can access the tips blockfilter when we have pruned some blocks") @@ -40,16 +40,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", '-testactivationheight=v20@2000'], expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE) + self.restart_node(0, extra_args=["-fastprune", "-prune=1", "-testactivationheight=v20@2000"], expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE) 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, 932) + self.restart_node(0, extra_args=["-fastprune", "-prune=1", "-blockfilterindex=1", "-testactivationheight=v20@2000"], expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE) + 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", "-testactivationheight=v20@2000"], expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE) + self.generate(self.nodes[0], 1000) + pruneheight_3 = self.nodes[0].pruneblockchain(2000) + assert_greater_than(pruneheight_3, pruneheight_2) self.stop_node(0, expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE) self.log.info("make sure we get an init error when starting the node again with block filters") diff --git a/test/functional/feature_coinstatsindex.py b/test/functional/feature_coinstatsindex.py index 91d66fc9c5..b038ddcc65 100755 --- a/test/functional/feature_coinstatsindex.py +++ b/test/functional/feature_coinstatsindex.py @@ -245,6 +245,20 @@ class CoinStatsIndexTest(BitcoinTestFramework): res10 = index_node.gettxoutsetinfo('muhash') assert(res8['txouts'] < res10['txouts']) + self.log.info("Test that the index works with -reindex") + + self.restart_node(1, extra_args=["-coinstatsindex", "-reindex"]) + res11 = index_node.gettxoutsetinfo('muhash') + assert_equal(res11, res10) + + self.log.info("Test that -reindex-chainstate is disallowed with coinstatsindex") + + self.nodes[1].assert_start_raises_init_error( + expected_msg='Error: -reindex-chainstate option is not compatible with -coinstatsindex. ' + 'Please temporarily disable coinstatsindex while using -reindex-chainstate, or replace -reindex-chainstate with -reindex to fully rebuild all indexes.', + extra_args=['-coinstatsindex', '-reindex-chainstate'], + ) + def _test_use_index_option(self): self.log.info("Test use_index option for nodes running the index") diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py new file mode 100755 index 0000000000..13c7326519 --- /dev/null +++ b/test/functional/feature_init.py @@ -0,0 +1,129 @@ +#!/usr/bin/env python3 +# Copyright (c) 2021 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Stress tests related to node initialization.""" +import os +from pathlib import Path + +from test_framework.test_framework import BitcoinTestFramework, SkipTest +from test_framework.test_node import ErrorMatch +from test_framework.util import assert_equal + + +class InitStressTest(BitcoinTestFramework): + """ + Ensure that initialization can be interrupted at a number of points and not impair + subsequent starts. + """ + + def set_test_params(self): + self.setup_clean_chain = False + self.num_nodes = 1 + + def run_test(self): + """ + - test terminating initialization after seeing a certain log line. + - test removing certain essential files to test startup error paths. + """ + # TODO: skip Windows for now since it isn't clear how to SIGTERM. + # + # Windows doesn't support `process.terminate()`. + # and other approaches (like below) don't work: + # + # os.kill(node.process.pid, signal.CTRL_C_EVENT) + if os.name == 'nt': + raise SkipTest("can't SIGTERM on Windows") + + self.stop_node(0) + node = self.nodes[0] + + def sigterm_node(): + node.process.terminate() + node.process.wait() + + def check_clean_start(): + """Ensure that node restarts successfully after various interrupts.""" + node.start() + node.wait_for_rpc_connection() + assert_equal(200, node.getblockcount()) + + lines_to_terminate_after = [ + b'Validating signatures for all blocks', + b'scheduler thread start', + b'Starting HTTP server', + b'Loading P2P addresses', + b'Loading banlist', + b'Loading block index', + b'Switching active chainstate', + b'Checking all blk files are present', + b'Loaded best chain:', + b'init message: Verifying blocks', + b'init message: Starting network threads', + b'net thread start', + b'addcon thread start', + b'loadblk thread start', + b'txindex thread start', + b'block filter index thread start', + b'coinstatsindex thread start', + b'msghand thread start', + b'net thread start', + b'addcon thread start', + ] + if self.is_wallet_compiled(): + lines_to_terminate_after.append(b'Verifying wallet') + + for terminate_line in lines_to_terminate_after: + self.log.info(f"Starting node and will exit after line {terminate_line}") + with node.wait_for_debug_log([terminate_line]): + node.start(extra_args=['-txindex=1', '-blockfilterindex=1', '-coinstatsindex=1']) + self.log.debug("Terminating node after terminate line was found") + sigterm_node() + + check_clean_start() + self.stop_node(0) + + self.log.info("Test startup errors after removing certain essential files") + + files_to_disturb = { + 'blocks/index/*.ldb': 'Error opening block database.', + 'chainstate/*.ldb': 'Error opening block database.', + 'blocks/blk*.dat': 'Error loading block database.', + } + + for file_patt, err_fragment in files_to_disturb.items(): + target_files = list(node.chain_path.glob(file_patt)) + + for target_file in target_files: + self.log.info(f"Tweaking file to ensure failure {target_file}") + bak_path = str(target_file) + ".bak" + target_file.rename(bak_path) + + # TODO: at some point, we should test perturbing the files instead of removing + # them, e.g. + # + # contents = target_file.read_bytes() + # tweaked_contents = bytearray(contents) + # tweaked_contents[50:250] = b'1' * 200 + # target_file.write_bytes(bytes(tweaked_contents)) + # + # At the moment I can't get this to work (bitcoind loads successfully?) so + # investigate doing this later. + + node.assert_start_raises_init_error( + extra_args=['-txindex=1', '-blockfilterindex=1', '-coinstatsindex=1'], + expected_msg=err_fragment, + match=ErrorMatch.PARTIAL_REGEX, + ) + + for target_file in target_files: + bak_path = str(target_file) + ".bak" + self.log.debug(f"Restoring file from {bak_path} and restarting") + Path(bak_path).rename(target_file) + + check_clean_start() + self.stop_node(0) + + +if __name__ == '__main__': + InitStressTest().main() diff --git a/test/functional/feature_reindex.py b/test/functional/feature_reindex.py index 844af1b168..2d65f83dda 100755 --- a/test/functional/feature_reindex.py +++ b/test/functional/feature_reindex.py @@ -22,7 +22,7 @@ class ReindexTest(BitcoinTestFramework): self.generatetoaddress(self.nodes[0], 3, self.nodes[0].get_deterministic_priv_key().address) blockcount = self.nodes[0].getblockcount() self.stop_nodes() - extra_args = [["-reindex-chainstate" if justchainstate else "-reindex"]] + extra_args = [["-reindex-chainstate" if justchainstate else "-reindex", "-txindex=0"]] self.start_nodes(extra_args) assert_equal(self.nodes[0].getblockcount(), blockcount) # start_node is blocking on reindex self.log.info("Success") diff --git a/test/functional/p2p_blockfilters.py b/test/functional/p2p_blockfilters.py index 2f52a226fe..242dd26ce2 100755 --- a/test/functional/p2p_blockfilters.py +++ b/test/functional/p2p_blockfilters.py @@ -251,6 +251,17 @@ class CompactFiltersTest(BitcoinTestFramework): msg = "Error: Cannot set -peerblockfilters without -blockfilterindex." self.nodes[0].assert_start_raises_init_error(expected_msg=msg) + self.log.info("Test unknown value to -blockfilterindex raises an error") + self.nodes[0].extra_args = ["-blockfilterindex=abc"] + msg = "Error: Unknown -blockfilterindex value abc." + self.nodes[0].assert_start_raises_init_error(expected_msg=msg) + + self.log.info("Test -blockfilterindex with -reindex-chainstate raises an error") + self.nodes[0].assert_start_raises_init_error( + expected_msg='Error: -reindex-chainstate option is not compatible with -blockfilterindex. ' + 'Please temporarily disable blockfilterindex while using -reindex-chainstate, or replace -reindex-chainstate with -reindex to fully rebuild all indexes.', + extra_args=['-blockfilterindex', '-reindex-chainstate'], + ) def compute_last_header(prev_header, hashes): """Compute the last filter header from a starting header and a sequence of filter hashes.""" diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 0ae866c377..e69781d346 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -20,6 +20,7 @@ import urllib.parse import shlex import sys import collections +from pathlib import Path from .authproxy import JSONRPCException from .descriptors import descsum_create @@ -400,22 +401,31 @@ class TestNode(): def wait_until_stopped(self, timeout=BITCOIND_PROC_WAIT_TIMEOUT): wait_until_helper(self.is_node_stopped, timeout=timeout, timeout_factor=self.timeout_factor) + @property + def chain_path(self) -> Path: + return Path(self.datadir) / get_chain_folder(self.datadir, self.chain) + + @property + def debug_log_path(self) -> Path: + return self.chain_path / 'debug.log' + + def debug_log_bytes(self) -> int: + with open(self.debug_log_path, encoding='utf-8') as dl: + dl.seek(0, 2) + return dl.tell() + @contextlib.contextmanager def assert_debug_log(self, expected_msgs, unexpected_msgs=None, timeout=2): if unexpected_msgs is None: unexpected_msgs = [] time_end = time.time() + timeout * self.timeout_factor - chain = get_chain_folder(self.datadir, self.chain) - debug_log = os.path.join(self.datadir, chain, 'debug.log') - with open(debug_log, encoding='utf-8') as dl: - dl.seek(0, 2) - prev_size = dl.tell() + prev_size = self.debug_log_bytes() yield while True: found = True - with open(debug_log, encoding='utf-8') as dl: + with open(self.debug_log_path, encoding='utf-8') as dl: dl.seek(prev_size) log = dl.read() print_log = " - " + "\n - ".join(log.splitlines()) @@ -432,6 +442,42 @@ class TestNode(): time.sleep(0.05) self._raise_assertion_error('Expected messages "{}" does not partially match log:\n\n{}\n\n'.format(str(expected_msgs), print_log)) + @contextlib.contextmanager + def wait_for_debug_log(self, expected_msgs, timeout=60): + """ + Block until we see a particular debug log message fragment or until we exceed the timeout. + Return: + the number of log lines we encountered when matching + """ + time_end = time.time() + timeout * self.timeout_factor + prev_size = self.debug_log_bytes() + + yield + + while True: + found = True + with open(self.debug_log_path, "rb") as dl: + dl.seek(prev_size) + log = dl.read() + + for expected_msg in expected_msgs: + if expected_msg not in log: + found = False + + if found: + return + + if time.time() >= time_end: + print_log = " - " + "\n - ".join(log.decode("utf8", errors="replace").splitlines()) + break + + # No sleep here because we want to detect the message fragment as fast as + # possible. + + self._raise_assertion_error( + 'Expected messages "{}" does not partially match log:\n\n{}\n\n'.format( + str(expected_msgs), print_log)) + @contextlib.contextmanager def profile_with_perf(self, profile_name: str): """ diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 1ad6cc54eb..5e0cffdf2d 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -327,6 +327,7 @@ BASE_SCRIPTS = [ 'rpc_wipewallettxes.py', 'feature_dip0020_activation.py', 'feature_uacomment.py', + 'feature_init.py', 'wallet_coinbase_category.py --legacy-wallet', 'wallet_coinbase_category.py --descriptors', 'feature_filelock.py',