From 4668ded6d6ea4299d998abbb57543f37519812e2 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 8 Sep 2020 14:36:31 -0400 Subject: [PATCH 1/7] validation: Move ~CMainCleanup logic to ~BlockManager ~CMainCleanup: 1. Is vestigial 2. References the g_chainman global (we should minimize g_chainman refs) 3. Only acts on g_chainman.m_blockman 4. Does the same thing as BlockManager::Unload --- src/validation.cpp | 14 -------------- src/validation.h | 4 ++++ 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index bdbae6651..5beff88a0 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5226,20 +5226,6 @@ double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex *pin return std::min(pindex->nChainTx / fTxTotal, 1.0); } -class CMainCleanup -{ -public: - CMainCleanup() {} - ~CMainCleanup() { - // block headers - BlockMap::iterator it1 = g_chainman.BlockIndex().begin(); - for (; it1 != g_chainman.BlockIndex().end(); it1++) - delete (*it1).second; - g_chainman.BlockIndex().clear(); - } -}; -static CMainCleanup instance_of_cmaincleanup; - Optional ChainstateManager::SnapshotBlockhash() const { if (m_active_chainstate != nullptr) { // If a snapshot chainstate exists, it will always be our active. diff --git a/src/validation.h b/src/validation.h index 53c2dd65e..12d1d4c91 100644 --- a/src/validation.h +++ b/src/validation.h @@ -416,6 +416,10 @@ public: BlockValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + + ~BlockManager() { + Unload(); + } }; /** From 74f73c783d46b012f375d819e2cd09c792820cd5 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 25 Aug 2020 15:23:57 -0400 Subject: [PATCH 2/7] validation: Pass in chainman to UnloadBlockIndex --- src/init.cpp | 2 +- src/qt/test/apptests.cpp | 7 +++++-- src/test/util/setup_common.cpp | 2 +- src/validation.cpp | 4 ++-- src/validation.h | 2 +- 5 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 7dceef7ff..90bf3c646 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1559,7 +1559,7 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA chainman.m_total_coinstip_cache = nCoinCacheUsage; chainman.m_total_coinsdb_cache = nCoinDBCache; - UnloadBlockIndex(node.mempool.get()); + UnloadBlockIndex(node.mempool.get(), chainman); // new CBlockTreeDB tries to delete the existing file, which // fails if it's still open from the previous loop. Close it first: diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp index 0b5c34154..8dffd2f59 100644 --- a/src/qt/test/apptests.cpp +++ b/src/qt/test/apptests.cpp @@ -84,8 +84,11 @@ void AppTests::appTests() // Reset global state to avoid interfering with later tests. LogInstance().DisconnectTestLogger(); AbortShutdown(); - UnloadBlockIndex(/* mempool */ nullptr); - WITH_LOCK(::cs_main, g_chainman.Reset()); + { + LOCK(cs_main); + UnloadBlockIndex(/* mempool */ nullptr, g_chainman); + g_chainman.Reset(); + } } //! Entry point for BitcoinGUI tests. diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 08aff0744..2d3137e1e 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -187,7 +187,7 @@ TestingSetup::~TestingSetup() m_node.connman.reset(); m_node.banman.reset(); m_node.args = nullptr; - UnloadBlockIndex(m_node.mempool.get()); + UnloadBlockIndex(m_node.mempool.get(), *m_node.chainman); m_node.mempool.reset(); m_node.scheduler.reset(); m_node.chainman->Reset(); diff --git a/src/validation.cpp b/src/validation.cpp index 5beff88a0..7478fb551 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4605,10 +4605,10 @@ void CChainState::UnloadBlockIndex() { // May NOT be used after any connections are up as much // of the peer-processing logic assumes a consistent // block index state -void UnloadBlockIndex(CTxMemPool* mempool) +void UnloadBlockIndex(CTxMemPool* mempool, ChainstateManager& chainman) { LOCK(cs_main); - g_chainman.Unload(); + chainman.Unload(); pindexBestInvalid = nullptr; pindexBestHeader = nullptr; if (mempool) mempool->clear(); diff --git a/src/validation.h b/src/validation.h index 12d1d4c91..2d9d2cb91 100644 --- a/src/validation.h +++ b/src/validation.h @@ -159,7 +159,7 @@ void LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFi /** Ensures we have a genesis block in the block tree, possibly writing one to disk. */ bool LoadGenesisBlock(const CChainParams& chainparams); /** Unload database information */ -void UnloadBlockIndex(CTxMemPool* mempool); +void UnloadBlockIndex(CTxMemPool* mempool, ChainstateManager& chainman); /** Run an instance of the script checking thread */ void ThreadScriptCheck(int worker_num); /** From f8d4975ab3fcd3553843cf0862251289c88c106b Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 25 Aug 2020 19:05:50 -0400 Subject: [PATCH 3/7] validation: Move PruneOneBlockFile to BlockManager [META] This is a pure refactor commit. Move PruneBlockFile to BlockManager because: 1. PruneOneBlockFile only acts on BlockManager 2. Eliminates the need for callers (FindFilesToPrune{,Manual}) to have a reference to the larger ChainstateManager, just a reference to BlockManager is enough. See following commits. --- src/validation.cpp | 12 ++++++------ src/validation.h | 6 +++--- src/wallet/test/wallet_tests.cpp | 6 +++--- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 7478fb551..76f8bcba7 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3912,12 +3912,12 @@ uint64_t CalculateCurrentUsage() return retval; } -void ChainstateManager::PruneOneBlockFile(const int fileNumber) +void BlockManager::PruneOneBlockFile(const int fileNumber) { AssertLockHeld(cs_main); LOCK(cs_LastBlockFile); - for (const auto& entry : m_blockman.m_block_index) { + for (const auto& entry : m_block_index) { CBlockIndex* pindex = entry.second; if (pindex->nFile == fileNumber) { pindex->nStatus &= ~BLOCK_HAVE_DATA; @@ -3931,12 +3931,12 @@ void ChainstateManager::PruneOneBlockFile(const int fileNumber) // to be downloaded again in order to consider its chain, at which // point it would be considered as a candidate for // m_blocks_unlinked or setBlockIndexCandidates. - auto range = m_blockman.m_blocks_unlinked.equal_range(pindex->pprev); + auto range = m_blocks_unlinked.equal_range(pindex->pprev); while (range.first != range.second) { std::multimap::iterator _it = range.first; range.first++; if (_it->second == pindex) { - m_blockman.m_blocks_unlinked.erase(_it); + m_blocks_unlinked.erase(_it); } } } @@ -3972,7 +3972,7 @@ static void FindFilesToPruneManual(ChainstateManager& chainman, std::set& s for (int fileNumber = 0; fileNumber < nLastBlockFile; fileNumber++) { if (vinfoBlockFile[fileNumber].nSize == 0 || vinfoBlockFile[fileNumber].nHeightLast > nLastBlockWeCanPrune) continue; - chainman.PruneOneBlockFile(fileNumber); + chainman.m_blockman.PruneOneBlockFile(fileNumber); setFilesToPrune.insert(fileNumber); count++; } @@ -4047,7 +4047,7 @@ static void FindFilesToPrune(ChainstateManager& chainman, std::set& setFile if (vinfoBlockFile[fileNumber].nHeightLast > nLastBlockWeCanPrune) continue; - chainman.PruneOneBlockFile(fileNumber); + chainman.m_blockman.PruneOneBlockFile(fileNumber); // Queue up the files for removal setFilesToPrune.insert(fileNumber); nCurrentUsage -= nBytesToPrune; diff --git a/src/validation.h b/src/validation.h index 2d9d2cb91..40611fa61 100644 --- a/src/validation.h +++ b/src/validation.h @@ -407,6 +407,9 @@ public: /** Create a new block index entry for a given block hash */ CBlockIndex* InsertBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + //! Mark one block file as pruned (modify associated database entries) + void PruneOneBlockFile(const int fileNumber) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + /** * If a block header hasn't already been seen, call CheckBlockHeader on it, ensure * that it doesn't descend from an invalid block, and then add it to m_block_index. @@ -903,9 +906,6 @@ public: */ bool ProcessNewBlockHeaders(const std::vector& block, BlockValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex = nullptr) LOCKS_EXCLUDED(cs_main); - //! Mark one block file as pruned (modify associated database entries) - void PruneOneBlockFile(const int fileNumber) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - //! Load the block tree and coins database from disk, initializing state if we're running with -reindex bool LoadBlockIndex(const CChainParams& chainparams) EXCLUSIVE_LOCKS_REQUIRED(cs_main); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 6b98482f9..4393bb770 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -126,7 +126,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Prune the older block file. { LOCK(cs_main); - Assert(m_node.chainman)->PruneOneBlockFile(oldTip->GetBlockPos().nFile); + Assert(m_node.chainman)->m_blockman.PruneOneBlockFile(oldTip->GetBlockPos().nFile); } UnlinkPrunedFiles({oldTip->GetBlockPos().nFile}); @@ -152,7 +152,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Prune the remaining block file. { LOCK(cs_main); - Assert(m_node.chainman)->PruneOneBlockFile(newTip->GetBlockPos().nFile); + Assert(m_node.chainman)->m_blockman.PruneOneBlockFile(newTip->GetBlockPos().nFile); } UnlinkPrunedFiles({newTip->GetBlockPos().nFile}); @@ -189,7 +189,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) // Prune the older block file. { LOCK(cs_main); - Assert(m_node.chainman)->PruneOneBlockFile(oldTip->GetBlockPos().nFile); + Assert(m_node.chainman)->m_blockman.PruneOneBlockFile(oldTip->GetBlockPos().nFile); } UnlinkPrunedFiles({oldTip->GetBlockPos().nFile}); From 3f5b5f3f6db0e5716911b3fba1460ce327e8a845 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 25 Aug 2020 19:38:23 -0400 Subject: [PATCH 4/7] validation: Move FindFilesToPrune{,Manual} to BlockManager [META] No behaviour change is intended in this commit. [META] This commit should be followed up by removing the comments and assertions meant only to show that the change is correct. Also stop FindFilesToPrune{,Manual} from unnecessary reaching for ::ChainActive() by passing in the necessary information. --- src/validation.cpp | 66 +++++++++++++++++++++++++++++++++++----------- src/validation.h | 10 ++++++- 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 76f8bcba7..389faa99a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -197,9 +197,6 @@ CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& loc std::unique_ptr pblocktree; -// See definition for documentation -static void FindFilesToPruneManual(ChainstateManager& chainman, std::set& setFilesToPrune, int nManualPruneHeight); -static void FindFilesToPrune(ChainstateManager& chainman, std::set& setFilesToPrune, uint64_t nPruneAfterHeight); bool CheckInputScripts(const CTransaction& tx, TxValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector *pvChecks = nullptr); static FILE* OpenUndoFile(const FlatFilePos &pos, bool fReadOnly = false); static FlatFileSeq BlockFileSeq(); @@ -2284,14 +2281,53 @@ bool CChainState::FlushStateToDisk( CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(&m_mempool); LOCK(cs_LastBlockFile); if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) { + // Previously, we called the global function ::ChainActive() in + // FindFilesToPrune{,Manual} to get the tip height and to determine + // whether or not a tip even exists. Now, we are simply passing in + // m_chain.Height() (which returns -1 if the tip doesn't exist). To + // make sure we're not changing behaviour, let's check that + // ::ChainActive() is the same object as m_chain (not just + // identical). + // + // This comment and the following assert will be removed in a + // subsequent commit, as they're just meant to demonstrate + // correctness (you can run tests against it and see that nothing + // exit unexpectedly). + assert(std::addressof(::ChainActive()) == std::addressof(m_chain)); if (nManualPruneHeight > 0) { LOG_TIME_MILLIS_WITH_CATEGORY("find files to prune (manual)", BCLog::BENCH); - FindFilesToPruneManual(g_chainman, setFilesToPrune, nManualPruneHeight); + m_blockman.FindFilesToPruneManual(setFilesToPrune, nManualPruneHeight, m_chain.Height()); } else { LOG_TIME_MILLIS_WITH_CATEGORY("find files to prune", BCLog::BENCH); - FindFilesToPrune(g_chainman, setFilesToPrune, chainparams.PruneAfterHeight()); + // Previously, we called the global function + // ::ChainstateActive() in FindFilesToPrune{,Manual} to get the + // IBD status. Now, we are simply passing in + // IsInitialBlockDownload(). To make sure we're not changing + // behaviour, let's check that ::ChainstateActive() is the same + // object as *this (not just identical). + // + // This comment and the following assert will be removed in a + // subsequent commit, as they're just meant to demonstrate + // correctness (you can run tests against it and see that + // nothing exit unexpectedly). + assert(std::addressof(::ChainstateActive()) == std::addressof(*this)); + + // Previously, we called PruneOneBlockFile on g_chainman's + // m_blockman in FindFilesToPrune{,Manual}. Now, we are instead + // calling PruneOneBlockFile on _our_ m_blockman. To make sure + // we're not changing behaviour, let's check that + // g_chainman.m_blockman is the same object as _our_ m_blockman + // (not just identical). + // + // This comment and the following assert will be removed in a + // subsequent commit, as they're just meant to demonstrate + // correctness (you can run tests against it and see that + // nothing exit unexpectedly). + assert(std::addressof(g_chainman.m_blockman) == std::addressof(m_blockman)); + + m_blockman.FindFilesToPrune(setFilesToPrune, chainparams.PruneAfterHeight(), m_chain.Height(), IsInitialBlockDownload()); fCheckForPruning = false; } if (!setFilesToPrune.empty()) { @@ -3958,21 +3994,21 @@ void UnlinkPrunedFiles(const std::set& setFilesToPrune) } /* Calculate the block/rev files to delete based on height specified by user with RPC command pruneblockchain */ -static void FindFilesToPruneManual(ChainstateManager& chainman, std::set& setFilesToPrune, int nManualPruneHeight) +void BlockManager::FindFilesToPruneManual(std::set& setFilesToPrune, int nManualPruneHeight, int chain_tip_height) { assert(fPruneMode && nManualPruneHeight > 0); LOCK2(cs_main, cs_LastBlockFile); - if (::ChainActive().Tip() == nullptr) + if (chain_tip_height < 0) return; // last block to prune is the lesser of (user-specified height, MIN_BLOCKS_TO_KEEP from the tip) - unsigned int nLastBlockWeCanPrune = std::min((unsigned)nManualPruneHeight, ::ChainActive().Tip()->nHeight - MIN_BLOCKS_TO_KEEP); + unsigned int nLastBlockWeCanPrune = std::min((unsigned)nManualPruneHeight, chain_tip_height - MIN_BLOCKS_TO_KEEP); int count=0; for (int fileNumber = 0; fileNumber < nLastBlockFile; fileNumber++) { if (vinfoBlockFile[fileNumber].nSize == 0 || vinfoBlockFile[fileNumber].nHeightLast > nLastBlockWeCanPrune) continue; - chainman.m_blockman.PruneOneBlockFile(fileNumber); + PruneOneBlockFile(fileNumber); setFilesToPrune.insert(fileNumber); count++; } @@ -4005,17 +4041,17 @@ void PruneBlockFilesManual(int nManualPruneHeight) * * @param[out] setFilesToPrune The set of file indices that can be unlinked will be returned */ -static void FindFilesToPrune(ChainstateManager& chainman, std::set& setFilesToPrune, uint64_t nPruneAfterHeight) +void BlockManager::FindFilesToPrune(std::set& setFilesToPrune, uint64_t nPruneAfterHeight, int chain_tip_height, bool is_ibd) { LOCK2(cs_main, cs_LastBlockFile); - if (::ChainActive().Tip() == nullptr || nPruneTarget == 0) { + if (chain_tip_height < 0 || nPruneTarget == 0) { return; } - if ((uint64_t)::ChainActive().Tip()->nHeight <= nPruneAfterHeight) { + if ((uint64_t)chain_tip_height <= nPruneAfterHeight) { return; } - unsigned int nLastBlockWeCanPrune = ::ChainActive().Tip()->nHeight - MIN_BLOCKS_TO_KEEP; + unsigned int nLastBlockWeCanPrune = chain_tip_height - 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 @@ -4029,7 +4065,7 @@ static void FindFilesToPrune(ChainstateManager& chainman, std::set& setFile // To avoid excessive prune events negating the benefit of high dbcache // values, we should not prune too rapidly. // So when pruning in IBD, increase the buffer a bit to avoid a re-prune too soon. - if (::ChainstateActive().IsInitialBlockDownload()) { + if (is_ibd) { // Since this is only relevant during IBD, we use a fixed 10% nBuffer += nPruneTarget / 10; } @@ -4047,7 +4083,7 @@ static void FindFilesToPrune(ChainstateManager& chainman, std::set& setFile if (vinfoBlockFile[fileNumber].nHeightLast > nLastBlockWeCanPrune) continue; - chainman.m_blockman.PruneOneBlockFile(fileNumber); + PruneOneBlockFile(fileNumber); // Queue up the files for removal setFilesToPrune.insert(fileNumber); nCurrentUsage -= nBytesToPrune; diff --git a/src/validation.h b/src/validation.h index 40611fa61..94bf16968 100644 --- a/src/validation.h +++ b/src/validation.h @@ -356,7 +356,15 @@ struct CBlockIndexWorkComparator * This data is used mostly in `CChainState` - information about, e.g., * candidate tips is not maintained here. */ -class BlockManager { +class BlockManager +{ + friend CChainState; + +private: + // See definition for documentation + void FindFilesToPruneManual(std::set& setFilesToPrune, int nManualPruneHeight, int chain_tip_height); + void FindFilesToPrune(std::set& setFilesToPrune, uint64_t nPruneAfterHeight, int chain_tip_height, bool is_ibd); + public: BlockMap m_block_index GUARDED_BY(cs_main); From 485899a93c6f5fff62090907efb0ac938992e1fb Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 15 Sep 2020 14:09:24 -0400 Subject: [PATCH 5/7] style: Make FindFilesToPrune{,Manual} match style guide [META] This is a pure style commit. --- src/validation.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 389faa99a..85bece454 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3999,15 +3999,17 @@ void BlockManager::FindFilesToPruneManual(std::set& setFilesToPrune, int nM assert(fPruneMode && nManualPruneHeight > 0); LOCK2(cs_main, cs_LastBlockFile); - if (chain_tip_height < 0) + if (chain_tip_height < 0) { return; + } // last block to prune is the lesser of (user-specified height, MIN_BLOCKS_TO_KEEP from the tip) unsigned int nLastBlockWeCanPrune = std::min((unsigned)nManualPruneHeight, chain_tip_height - MIN_BLOCKS_TO_KEEP); - int count=0; + int count = 0; for (int fileNumber = 0; fileNumber < nLastBlockFile; fileNumber++) { - if (vinfoBlockFile[fileNumber].nSize == 0 || vinfoBlockFile[fileNumber].nHeightLast > nLastBlockWeCanPrune) + if (vinfoBlockFile[fileNumber].nSize == 0 || vinfoBlockFile[fileNumber].nHeightLast > nLastBlockWeCanPrune) { continue; + } PruneOneBlockFile(fileNumber); setFilesToPrune.insert(fileNumber); count++; @@ -4058,7 +4060,7 @@ void BlockManager::FindFilesToPrune(std::set& setFilesToPrune, uint64_t nPr // before the next pruning. uint64_t nBuffer = BLOCKFILE_CHUNK_SIZE + UNDOFILE_CHUNK_SIZE; uint64_t nBytesToPrune; - int count=0; + int count = 0; if (nCurrentUsage + nBuffer >= nPruneTarget) { // On a prune event, the chainstate DB is flushed. @@ -4073,15 +4075,18 @@ void BlockManager::FindFilesToPrune(std::set& setFilesToPrune, uint64_t nPr for (int fileNumber = 0; fileNumber < nLastBlockFile; fileNumber++) { nBytesToPrune = vinfoBlockFile[fileNumber].nSize + vinfoBlockFile[fileNumber].nUndoSize; - if (vinfoBlockFile[fileNumber].nSize == 0) + if (vinfoBlockFile[fileNumber].nSize == 0) { continue; + } - if (nCurrentUsage + nBuffer < nPruneTarget) // are we below our target? + if (nCurrentUsage + nBuffer < nPruneTarget) { // are we below our target? break; + } // don't prune files that could have a block within MIN_BLOCKS_TO_KEEP of the main chain's tip but keep scanning - if (vinfoBlockFile[fileNumber].nHeightLast > nLastBlockWeCanPrune) + if (vinfoBlockFile[fileNumber].nHeightLast > nLastBlockWeCanPrune) { continue; + } PruneOneBlockFile(fileNumber); // Queue up the files for removal From 3756853b15902d63f4b5a3129e8b5d82e84e125b Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Mon, 14 Sep 2020 10:19:12 -0400 Subject: [PATCH 6/7] docs: Move FindFilesToPrune{,Manual} doxygen comment [META] This is a pure comment commit. They belong in the member declarations in the header file. --- src/validation.cpp | 16 ---------------- src/validation.h | 18 +++++++++++++++++- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 85bece454..97b185e9c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3993,7 +3993,6 @@ void UnlinkPrunedFiles(const std::set& setFilesToPrune) } } -/* Calculate the block/rev files to delete based on height specified by user with RPC command pruneblockchain */ void BlockManager::FindFilesToPruneManual(std::set& setFilesToPrune, int nManualPruneHeight, int chain_tip_height) { assert(fPruneMode && nManualPruneHeight > 0); @@ -4028,21 +4027,6 @@ void PruneBlockFilesManual(int nManualPruneHeight) } } -/** - * Prune block and undo files (blk???.dat and undo???.dat) so that the disk space used is less than a user-defined target. - * The user sets the target (in MB) on the command line or in config file. This will be run on startup and whenever new - * space is allocated in a block or undo file, staying below the target. Changing back to unpruned requires a reindex - * (which in this case means the blockchain must be re-downloaded.) - * - * Pruning functions are called from FlushStateToDisk when the global fCheckForPruning flag has been set. - * Block and undo files are deleted in lock-step (when blk00003.dat is deleted, so is rev00003.dat.) - * Pruning cannot take place until the longest chain is at least a certain length (100000 on mainnet, 1000 on testnet, 1000 on regtest). - * Pruning will never delete a block within a defined distance (currently 288) from the active chain's tip. - * The block index is updated by unsetting HAVE_DATA and HAVE_UNDO for any blocks that were stored in the deleted files. - * A db flag records the fact that at least some block files have been pruned. - * - * @param[out] setFilesToPrune The set of file indices that can be unlinked will be returned - */ void BlockManager::FindFilesToPrune(std::set& setFilesToPrune, uint64_t nPruneAfterHeight, int chain_tip_height, bool is_ibd) { LOCK2(cs_main, cs_LastBlockFile); diff --git a/src/validation.h b/src/validation.h index 94bf16968..9114b24c9 100644 --- a/src/validation.h +++ b/src/validation.h @@ -361,8 +361,24 @@ class BlockManager friend CChainState; private: - // See definition for documentation + /* Calculate the block/rev files to delete based on height specified by user with RPC command pruneblockchain */ void FindFilesToPruneManual(std::set& setFilesToPrune, int nManualPruneHeight, int chain_tip_height); + + /** + * Prune block and undo files (blk???.dat and undo???.dat) so that the disk space used is less than a user-defined target. + * The user sets the target (in MB) on the command line or in config file. This will be run on startup and whenever new + * space is allocated in a block or undo file, staying below the target. Changing back to unpruned requires a reindex + * (which in this case means the blockchain must be re-downloaded.) + * + * Pruning functions are called from FlushStateToDisk when the global fCheckForPruning flag has been set. + * Block and undo files are deleted in lock-step (when blk00003.dat is deleted, so is rev00003.dat.) + * Pruning cannot take place until the longest chain is at least a certain length (100000 on mainnet, 1000 on testnet, 1000 on regtest). + * Pruning will never delete a block within a defined distance (currently 288) from the active chain's tip. + * The block index is updated by unsetting HAVE_DATA and HAVE_UNDO for any blocks that were stored in the deleted files. + * A db flag records the fact that at least some block files have been pruned. + * + * @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); public: From 72a1d5c6f3834e206719ee5121df7727aed5b786 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 15 Sep 2020 14:16:04 -0400 Subject: [PATCH 7/7] validation: Remove review-only comments + assertions [META] This is a followup to "validation: Move FindFilesToPrune{,Manual} to BlockManager" removing comments and assertions meant only to show that the change is correct. --- src/validation.cpp | 39 --------------------------------------- 1 file changed, 39 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 97b185e9c..47aed6d0d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2281,19 +2281,6 @@ bool CChainState::FlushStateToDisk( CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(&m_mempool); LOCK(cs_LastBlockFile); if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) { - // Previously, we called the global function ::ChainActive() in - // FindFilesToPrune{,Manual} to get the tip height and to determine - // whether or not a tip even exists. Now, we are simply passing in - // m_chain.Height() (which returns -1 if the tip doesn't exist). To - // make sure we're not changing behaviour, let's check that - // ::ChainActive() is the same object as m_chain (not just - // identical). - // - // This comment and the following assert will be removed in a - // subsequent commit, as they're just meant to demonstrate - // correctness (you can run tests against it and see that nothing - // exit unexpectedly). - assert(std::addressof(::ChainActive()) == std::addressof(m_chain)); if (nManualPruneHeight > 0) { LOG_TIME_MILLIS_WITH_CATEGORY("find files to prune (manual)", BCLog::BENCH); @@ -2301,32 +2288,6 @@ bool CChainState::FlushStateToDisk( } else { LOG_TIME_MILLIS_WITH_CATEGORY("find files to prune", BCLog::BENCH); - // Previously, we called the global function - // ::ChainstateActive() in FindFilesToPrune{,Manual} to get the - // IBD status. Now, we are simply passing in - // IsInitialBlockDownload(). To make sure we're not changing - // behaviour, let's check that ::ChainstateActive() is the same - // object as *this (not just identical). - // - // This comment and the following assert will be removed in a - // subsequent commit, as they're just meant to demonstrate - // correctness (you can run tests against it and see that - // nothing exit unexpectedly). - assert(std::addressof(::ChainstateActive()) == std::addressof(*this)); - - // Previously, we called PruneOneBlockFile on g_chainman's - // m_blockman in FindFilesToPrune{,Manual}. Now, we are instead - // calling PruneOneBlockFile on _our_ m_blockman. To make sure - // we're not changing behaviour, let's check that - // g_chainman.m_blockman is the same object as _our_ m_blockman - // (not just identical). - // - // This comment and the following assert will be removed in a - // subsequent commit, as they're just meant to demonstrate - // correctness (you can run tests against it and see that - // nothing exit unexpectedly). - assert(std::addressof(g_chainman.m_blockman) == std::addressof(m_blockman)); - m_blockman.FindFilesToPrune(setFilesToPrune, chainparams.PruneAfterHeight(), m_chain.Height(), IsInitialBlockDownload()); fCheckForPruning = false; }