diff options
author | Jonas Schnelli <dev@jonasschnelli.ch> | 2021-02-18 09:40:33 +0100 |
---|---|---|
committer | Jonas Schnelli <dev@jonasschnelli.ch> | 2021-02-18 09:40:42 +0100 |
commit | 9017d55e7c325eba486075bc3403dc8c9b252b75 (patch) | |
tree | dddd7540534e19dc5945a9a8f97dc79cdb3822d6 | |
parent | cd66d8b1d8b5fcb96b80cdf167d0d1152b99aee0 (diff) | |
parent | 84716b134e9afca2fba7731de4af1f22fa1b1518 (diff) | |
download | bitcoin-9017d55e7c325eba486075bc3403dc8c9b252b75.tar.xz |
Merge #15946: Allow maintaining the blockfilterindex when using prune
84716b134e9afca2fba7731de4af1f22fa1b1518 Add "index/blockfilterindex -> validation -> index/blockfilterindex" to expected circular dependencies (Jonas Schnelli)
ab3a0a2fb915d8b8384c30a8b38b4b5cc35236fd Add functional test for blockfilterindex in prune-mode (Jonas Schnelli)
c286a22f7b63a8bd336d5d7606c339053ee0054b Add debug startup parameter -fastprune for more effective pruning tests (Jonas Schnelli)
5e112269c311a559bfded814d3c3c438349a1986 Avoid pruning below the blockfilterindex sync height (Jonas Schnelli)
00d57ff76854938ead800767fb673a8af46eac8e Avoid accessing nullpointer in BaseIndex::GetSummary() (Jonas Schnelli)
6abe9f5b11cd4a5ecb6caca8443fe2950a417842 Allow blockfilter in conjunction with prune (Jonas Schnelli)
Pull request description:
Maintaining the blockfilterindexes in prune mode is possible and may lead to efficient p2p based rescans of wallets (restore backups, import/sweep keys) beyond the prune height (rescans not part of that PR).
This PR allows running the blockfilterindex(es) in conjunction with pruning.
* Bitcoind/Qt will shutdown during startup when missing block data has been detected ([re]enable `-blockfilterindex` when we already have pruned)
* manual block pruning is disabled during blockfilterindex sync
* auto-pruning is delayed during blockfilterindex sync
ToDos:
* [x] Functional tests
ACKs for top commit:
fjahr:
Code review ACK 84716b1
ryanofsky:
Code review ACK 84716b134e9afca2fba7731de4af1f22fa1b1518. Only changes since last review were suggested new FindFilesToPrune argument and test.
benthecarman:
tACK 84716b134e9afca2fba7731de4af1f22fa1b1518
Tree-SHA512: 91d832c6c562c463f7ec7655c08956385413a99a896640b9737bda0183607fac530435d03d87c3c0e70c61ccdfe73fe8f3639bc7d26d33ca7e60925ebb97d77a
-rw-r--r-- | src/chainparams.cpp | 2 | ||||
-rw-r--r-- | src/index/base.cpp | 43 | ||||
-rw-r--r-- | src/init.cpp | 4 | ||||
-rw-r--r-- | src/validation.cpp | 21 | ||||
-rw-r--r-- | src/validation.h | 2 | ||||
-rwxr-xr-x | test/functional/feature_blockfilterindex_prune.py | 49 | ||||
-rwxr-xr-x | test/functional/test_runner.py | 1 | ||||
-rwxr-xr-x | test/lint/lint-circular-dependencies.sh | 1 |
8 files changed, 111 insertions, 12 deletions
diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 4796045730..16efffa6f0 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -412,7 +412,7 @@ public: pchMessageStart[2] = 0xb5; pchMessageStart[3] = 0xda; nDefaultPort = 18444; - nPruneAfterHeight = 1000; + nPruneAfterHeight = gArgs.GetBoolArg("-fastprune", false) ? 100 : 1000; m_assumed_blockchain_size = 0; m_assumed_chain_state_size = 0; diff --git a/src/index/base.cpp b/src/index/base.cpp index 3d3dda95b1..25644c3b41 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -65,6 +65,43 @@ bool BaseIndex::Init() m_best_block_index = g_chainman.m_blockman.FindForkInGlobalIndex(::ChainActive(), locator); } m_synced = m_best_block_index.load() == ::ChainActive().Tip(); + if (!m_synced) { + bool prune_violation = false; + if (!m_best_block_index) { + // index is not built yet + // make sure we have all block data back to the genesis + const CBlockIndex* block = ::ChainActive().Tip(); + while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) { + block = block->pprev; + } + prune_violation = block != ::ChainActive().Genesis(); + } + // in case the index has a best block set and is not fully synced + // check if we have the required blocks to continue building the index + else { + const CBlockIndex* block_to_test = m_best_block_index.load(); + if (!ChainActive().Contains(block_to_test)) { + // if the bestblock is not part of the mainchain, find the fork + // and make sure we have all data down to the fork + block_to_test = ::ChainActive().FindFork(block_to_test); + } + const CBlockIndex* block = ::ChainActive().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)) { + if (block_to_test == block) { + prune_violation = false; + break; + } + block = block->pprev; + } + } + if (prune_violation) { + // throw error and graceful shutdown if we can't build the index + FatalError("%s: %s best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)", __func__, GetName()); + return false; + } + } return true; } @@ -177,6 +214,10 @@ bool BaseIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_ti assert(current_tip->GetAncestor(new_tip->nHeight) == new_tip); // In the case of a reorg, ensure persisted block locator is not stale. + // Pruning has a minimum of 288 blocks-to-keep and getting the index + // out of sync may be possible but a users fault. + // In case we reorg beyond the pruned depth, ReadBlockFromDisk would + // throw and lead to a graceful shutdown m_best_block_index = new_tip; if (!Commit()) { // If commit fails, revert the best block index to avoid corruption. @@ -325,6 +366,6 @@ IndexSummary BaseIndex::GetSummary() const IndexSummary summary{}; summary.name = GetName(); summary.synced = m_synced; - summary.best_block_height = m_best_block_index.load()->nHeight; + summary.best_block_height = m_best_block_index ? m_best_block_index.load()->nHeight : 0; return summary; } diff --git a/src/init.cpp b/src/init.cpp index 3beb421bf9..dd04c1938c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -386,6 +386,7 @@ void SetupServerArgs(NodeContext& node) #endif argsman.AddArg("-assumevalid=<hex>", strprintf("If this block is in the chain assume that it and its ancestors are valid and potentially skip their script verification (0 to verify all, default: %s, testnet: %s, signet: %s)", defaultChainParams->GetConsensus().defaultAssumeValid.GetHex(), testnetChainParams->GetConsensus().defaultAssumeValid.GetHex(), signetChainParams->GetConsensus().defaultAssumeValid.GetHex()), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-blocksdir=<dir>", "Specify directory to hold blocks subdirectory for *.dat files (default: <datadir>)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-fastprune", "Use smaller block files and lower minimum prune height for testing purposes", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); #if HAVE_SYSTEM argsman.AddArg("-blocknotify=<cmd>", "Execute command when the best block changes (%s in cmd is replaced by block hash)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); #endif @@ -1028,9 +1029,6 @@ bool AppInitParameterInteraction(const ArgsManager& args) if (args.GetArg("-prune", 0)) { if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) return InitError(_("Prune mode is incompatible with -txindex.")); - if (!g_enabled_filter_types.empty()) { - return InitError(_("Prune mode is incompatible with -blockfilterindex.")); - } } // -bind and -whitebind can't be set when not listening diff --git a/src/validation.cpp b/src/validation.cpp index 31609ea3e5..f7723764f9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -17,6 +17,7 @@ #include <cuckoocache.h> #include <flatfile.h> #include <hash.h> +#include <index/blockfilterindex.h> #include <index/txindex.h> #include <logging.h> #include <logging/timer.h> @@ -2240,17 +2241,25 @@ bool CChainState::FlushStateToDisk( { bool fFlushForPrune = false; bool fDoFullFlush = false; + CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(&m_mempool); LOCK(cs_LastBlockFile); if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) { + // make sure we don't prune above the blockfilterindexes bestblocks + // pruning is height-based + int last_prune = m_chain.Height(); // last height we can prune + ForEachBlockFilterIndex([&](BlockFilterIndex& index) { + last_prune = std::max(1, std::min(last_prune, index.GetSummary().best_block_height)); + }); + if (nManualPruneHeight > 0) { LOG_TIME_MILLIS_WITH_CATEGORY("find files to prune (manual)", BCLog::BENCH); - m_blockman.FindFilesToPruneManual(setFilesToPrune, nManualPruneHeight, m_chain.Height()); + m_blockman.FindFilesToPruneManual(setFilesToPrune, std::min(last_prune, nManualPruneHeight), m_chain.Height()); } else { LOG_TIME_MILLIS_WITH_CATEGORY("find files to prune", BCLog::BENCH); - m_blockman.FindFilesToPrune(setFilesToPrune, chainparams.PruneAfterHeight(), m_chain.Height(), IsInitialBlockDownload()); + m_blockman.FindFilesToPrune(setFilesToPrune, chainparams.PruneAfterHeight(), m_chain.Height(), last_prune, IsInitialBlockDownload()); fCheckForPruning = false; } if (!setFilesToPrune.empty()) { @@ -3194,7 +3203,7 @@ static bool FindBlockPos(FlatFilePos &pos, unsigned int nAddSize, unsigned int n bool finalize_undo = false; if (!fKnown) { - while (vinfoBlockFile[nFile].nSize + nAddSize >= MAX_BLOCKFILE_SIZE) { + while (vinfoBlockFile[nFile].nSize + nAddSize >= (gArgs.GetBoolArg("-fastprune", false) ? 0x10000 /* 64kb */ : MAX_BLOCKFILE_SIZE)) { // when the undo file is keeping up with the block file, we want to flush it explicitly // when it is lagging behind (more blocks arrive than are being connected), we let the // undo block write case handle it @@ -3925,7 +3934,7 @@ void PruneBlockFilesManual(int nManualPruneHeight) } } -void BlockManager::FindFilesToPrune(std::set<int>& setFilesToPrune, uint64_t nPruneAfterHeight, int chain_tip_height, bool is_ibd) +void BlockManager::FindFilesToPrune(std::set<int>& setFilesToPrune, uint64_t nPruneAfterHeight, int chain_tip_height, int prune_height, bool is_ibd) { LOCK2(cs_main, cs_LastBlockFile); if (chain_tip_height < 0 || nPruneTarget == 0) { @@ -3935,7 +3944,7 @@ void BlockManager::FindFilesToPrune(std::set<int>& setFilesToPrune, uint64_t nPr return; } - unsigned int nLastBlockWeCanPrune = chain_tip_height - MIN_BLOCKS_TO_KEEP; + unsigned int nLastBlockWeCanPrune = std::min(prune_height, chain_tip_height - static_cast<int>(MIN_BLOCKS_TO_KEEP)); uint64_t nCurrentUsage = CalculateCurrentUsage(); // We don't check to prune until after we've allocated new space for files // So we should leave a buffer under our target to account for another allocation @@ -3986,7 +3995,7 @@ void BlockManager::FindFilesToPrune(std::set<int>& setFilesToPrune, uint64_t nPr static FlatFileSeq BlockFileSeq() { - return FlatFileSeq(GetBlocksDir(), "blk", BLOCKFILE_CHUNK_SIZE); + return FlatFileSeq(GetBlocksDir(), "blk", gArgs.GetBoolArg("-fastprune", false) ? 0x4000 /* 16kb */ : BLOCKFILE_CHUNK_SIZE); } static FlatFileSeq UndoFileSeq() diff --git a/src/validation.h b/src/validation.h index d6058e7dac..1a71dd0d49 100644 --- a/src/validation.h +++ b/src/validation.h @@ -399,7 +399,7 @@ private: * * @param[out] setFilesToPrune The set of file indices that can be unlinked will be returned */ - void FindFilesToPrune(std::set<int>& setFilesToPrune, uint64_t nPruneAfterHeight, int chain_tip_height, bool is_ibd); + void FindFilesToPrune(std::set<int>& setFilesToPrune, uint64_t nPruneAfterHeight, int chain_tip_height, int prune_height, bool is_ibd); public: BlockMap m_block_index GUARDED_BY(cs_main); diff --git a/test/functional/feature_blockfilterindex_prune.py b/test/functional/feature_blockfilterindex_prune.py new file mode 100755 index 0000000000..a380aae098 --- /dev/null +++ b/test/functional/feature_blockfilterindex_prune.py @@ -0,0 +1,49 @@ +#!/usr/bin/env python3 +# Copyright (c) 2020 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test blockfilterindex in conjunction with prune.""" +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_raises_rpc_error, + assert_greater_than, +) + +class FeatureBlockfilterindexPruneTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 2 + self.extra_args = [["-fastprune", "-prune=1"], ["-fastprune", "-prune=1", "-blockfilterindex=1"]] + + def run_test(self): + # test basic pruning compatibility & filter access of pruned blocks + self.log.info("check if we can access a blockfilter when pruning is enabled but no blocks are actually pruned") + assert(len(self.nodes[1].getblockfilter(self.nodes[1].getbestblockhash())['filter']) > 0) + self.nodes[1].generate(500) + self.sync_all() + self.log.info("prune some blocks") + pruneheight = self.nodes[1].pruneblockchain(400) + assert(pruneheight != 0) + self.log.info("check if we can access the tips blockfilter when we have pruned some blocks") + assert(len(self.nodes[1].getblockfilter(self.nodes[1].getbestblockhash())['filter']) > 0) + self.log.info("check if we can access the blockfilter of a pruned block") + assert(len(self.nodes[1].getblockfilter(self.nodes[1].getblockhash(2))['filter']) > 0) + self.log.info("start node without blockfilterindex") + self.stop_node(1) + self.start_node(1, extra_args=self.extra_args[0]) + 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[1].getblockfilter, self.nodes[1].getblockhash(2)) + self.nodes[1].generate(1000) + self.log.info("prune below the blockfilterindexes best block while blockfilters are disabled") + pruneheight_new = self.nodes[1].pruneblockchain(1000) + assert_greater_than(pruneheight_new, pruneheight) + self.stop_node(1) + self.log.info("make sure we get an init error when starting the node again with block filters") + self.nodes[1].assert_start_raises_init_error(extra_args=self.extra_args[1]) + self.nodes[1].assert_debug_log(["basic block filter index best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)"]) + self.log.info("make sure the node starts again with the -reindex arg") + reindex_args = self.extra_args[1] + reindex_args.append("-reindex") + self.start_node(1, extra_args=reindex_args) + +if __name__ == '__main__': + FeatureBlockfilterindexPruneTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 354cacf95a..d742ef4eee 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -289,6 +289,7 @@ BASE_SCRIPTS = [ 'feature_help.py', 'feature_shutdown.py', 'p2p_ibd_txrelay.py', + 'feature_blockfilterindex_prune.py' # Don't append tests at the end to avoid merge conflicts # Put them in a random line within the section that fits their approximate run-time ] diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh index 5312dbbfdb..0b15f99448 100755 --- a/test/lint/lint-circular-dependencies.sh +++ b/test/lint/lint-circular-dependencies.sh @@ -11,6 +11,7 @@ export LC_ALL=C EXPECTED_CIRCULAR_DEPENDENCIES=( "chainparamsbase -> util/system -> chainparamsbase" "index/txindex -> validation -> index/txindex" + "index/blockfilterindex -> validation -> index/blockfilterindex" "policy/fees -> txmempool -> policy/fees" "qt/addresstablemodel -> qt/walletmodel -> qt/addresstablemodel" "qt/bitcoingui -> qt/walletframe -> qt/bitcoingui" |