From 6abe9f5b11cd4a5ecb6caca8443fe2950a417842 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Thu, 2 May 2019 20:41:01 +0200 Subject: [PATCH 1/6] Allow blockfilter in conjunction with prune --- src/index/base.cpp | 41 +++++++++++++++++++++++++++++++++++++++++ src/init.cpp | 3 --- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index 3d3dda95b10..38958a82e91 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. diff --git a/src/init.cpp b/src/init.cpp index a77e2cf1cbf..52e9bd6d66a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1023,9 +1023,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 From 00d57ff76854938ead800767fb673a8af46eac8e Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Thu, 10 Dec 2020 13:36:47 +0100 Subject: [PATCH 2/6] Avoid accessing nullpointer in BaseIndex::GetSummary() --- src/index/base.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index 38958a82e91..25644c3b41e 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -366,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; } From 5e112269c311a559bfded814d3c3c438349a1986 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Thu, 10 Dec 2020 13:21:54 +0100 Subject: [PATCH 3/6] Avoid pruning below the blockfilterindex sync height --- src/validation.cpp | 17 +++++++++++++---- src/validation.h | 2 +- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 8f7d36bfd3f..6f76d8f5fd9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -2249,17 +2250,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()) { @@ -3934,7 +3943,7 @@ void PruneBlockFilesManual(int nManualPruneHeight) } } -void BlockManager::FindFilesToPrune(std::set& setFilesToPrune, uint64_t nPruneAfterHeight, int chain_tip_height, bool is_ibd) +void BlockManager::FindFilesToPrune(std::set& 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) { @@ -3944,7 +3953,7 @@ void BlockManager::FindFilesToPrune(std::set& 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(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 diff --git a/src/validation.h b/src/validation.h index e85c7bbf1ad..30d8c9c17a4 100644 --- a/src/validation.h +++ b/src/validation.h @@ -361,7 +361,7 @@ private: * * @param[out] setFilesToPrune The set of file indices that can be unlinked will be returned */ - void FindFilesToPrune(std::set& setFilesToPrune, uint64_t nPruneAfterHeight, int chain_tip_height, bool is_ibd); + void FindFilesToPrune(std::set& setFilesToPrune, uint64_t nPruneAfterHeight, int chain_tip_height, int prune_height, bool is_ibd); public: BlockMap m_block_index GUARDED_BY(cs_main); From c286a22f7b63a8bd336d5d7606c339053ee0054b Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Thu, 10 Dec 2020 11:39:37 +0100 Subject: [PATCH 4/6] Add debug startup parameter -fastprune for more effective pruning tests --- src/chainparams.cpp | 2 +- src/init.cpp | 1 + src/validation.cpp | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 88cf5ef0a88..f439436e075 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -406,7 +406,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/init.cpp b/src/init.cpp index 52e9bd6d66a..c3af44fa0ca 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -386,6 +386,7 @@ void SetupServerArgs(NodeContext& node) #endif argsman.AddArg("-assumevalid=", 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=", "Specify directory to hold blocks subdirectory for *.dat files (default: )", 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=", "Execute command when the best block changes (%s in cmd is replaced by block hash)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); #endif diff --git a/src/validation.cpp b/src/validation.cpp index 6f76d8f5fd9..13860454dc3 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3212,7 +3212,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 @@ -4004,7 +4004,7 @@ void BlockManager::FindFilesToPrune(std::set& 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() From ab3a0a2fb915d8b8384c30a8b38b4b5cc35236fd Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Thu, 10 Dec 2020 11:43:46 +0100 Subject: [PATCH 5/6] Add functional test for blockfilterindex in prune-mode --- .../feature_blockfilterindex_prune.py | 49 +++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 50 insertions(+) create mode 100755 test/functional/feature_blockfilterindex_prune.py diff --git a/test/functional/feature_blockfilterindex_prune.py b/test/functional/feature_blockfilterindex_prune.py new file mode 100755 index 00000000000..a380aae0981 --- /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 1e7d9e4b0d6..d744ac218ce 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -288,6 +288,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 ] From 84716b134e9afca2fba7731de4af1f22fa1b1518 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Thu, 10 Dec 2020 20:11:48 +0100 Subject: [PATCH 6/6] Add "index/blockfilterindex -> validation -> index/blockfilterindex" to expected circular dependencies --- test/lint/lint-circular-dependencies.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh index c4ad00e954f..509c9231d24 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"