From 42f2756ae75f2a0e52afa8e250d930c67086d170 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Tue, 6 Jul 2021 17:05:25 -0400 Subject: [PATCH] merge bitcoin#22415: Make m_mempool optional in CChainState --- src/init.cpp | 2 +- src/test/util/setup_common.cpp | 2 +- src/test/validation_chainstate_tests.cpp | 2 +- .../validation_chainstatemanager_tests.cpp | 10 +- src/test/validation_flush_tests.cpp | 25 ++- src/validation.cpp | 142 +++++++++--------- src/validation.h | 54 +++++-- 7 files changed, 131 insertions(+), 106 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index baffee12a4..2462f5b18e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2024,7 +2024,7 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc node.evodb = std::make_unique(nEvoDbCache, false, fReset || fReindexChainState); chainman.Reset(); - chainman.InitializeChainstate(llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor, *node.evodb, *Assert(node.mempool)); + chainman.InitializeChainstate(Assert(node.mempool.get()), *node.evodb, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor); chainman.m_total_coinstip_cache = nCoinCacheUsage; chainman.m_total_coinsdb_cache = nCoinDBCache; diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index fd5de41fd6..f3f1405bcb 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -232,7 +232,7 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vectorInitializeChainstate(llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor, *m_node.evodb, *m_node.mempool); + m_node.chainman->InitializeChainstate(m_node.mempool.get(), *m_node.evodb, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor); ::ChainstateActive().InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); assert(!::ChainstateActive().CanFlushToDisk()); diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index e6eb56ef83..684bb81e26 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -40,7 +40,7 @@ BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches) return outp; }; - CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor, *m_node.evodb, mempool)); + CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(&mempool, *m_node.evodb, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor)); c1.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); WITH_LOCK(::cs_main, c1.InitCoinsCache(1 << 23)); diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index d09fc93e1e..6893162bf4 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -41,7 +41,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) // Create a legacy (IBD) chainstate. // - CChainState& c1 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate(llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor, evodb, mempool)); + CChainState& c1 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate(&mempool, evodb, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor)); chainstates.push_back(&c1); c1.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); @@ -71,8 +71,8 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) // const uint256 snapshot_blockhash = GetRandHash(); CChainState& c2 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate( - llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor, - evodb, mempool, snapshot_blockhash) + &mempool, evodb, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor, + snapshot_blockhash) ); chainstates.push_back(&c2); @@ -136,7 +136,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) // Create a legacy (IBD) chainstate. // - CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor, evodb, mempool)); + CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(&mempool, evodb, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor)); chainstates.push_back(&c1); c1.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); @@ -154,7 +154,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) // Create a snapshot-based chainstate. // - CChainState& c2 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor, evodb, mempool, GetRandHash())); + CChainState& c2 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(&mempool, evodb, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor, GetRandHash())); chainstates.push_back(&c2); c2.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp index 482d4f2ff2..a26c7a8d49 100644 --- a/src/test/validation_flush_tests.cpp +++ b/src/test/validation_flush_tests.cpp @@ -24,10 +24,9 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) { CTxMemPool mempool; BlockManager blockman{}; - CChainState chainstate(blockman, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor, *m_node.evodb, mempool); + CChainState chainstate(&mempool, blockman, *m_node.evodb, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor); chainstate.InitCoinsDB(/*cache_size_bytes*/ 1 << 10, /*in_memory*/ true, /*should_wipe*/ false); WITH_LOCK(::cs_main, chainstate.InitCoinsCache(1 << 10)); - CTxMemPool tx_pool{}; constexpr bool is_64_bit = sizeof(void*) == 8; @@ -61,7 +60,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) // Without any coins in the cache, we shouldn't need to flush. BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0), + chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0), CoinsCacheSizeState::OK); // If the initial memory allocations of cacheCoins don't match these common @@ -76,7 +75,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) } BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0), + chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0), CoinsCacheSizeState::CRITICAL); BOOST_TEST_MESSAGE("Exiting cache flush tests early due to unsupported arch"); @@ -97,7 +96,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) print_view_mem_usage(view); BOOST_CHECK_EQUAL(view.AccessCoin(res).DynamicMemoryUsage(), COIN_SIZE); BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0), + chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0), CoinsCacheSizeState::OK); } @@ -105,26 +104,26 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) for (int i{0}; i < 4; ++i) { add_coin(view); print_view_mem_usage(view); - if (chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0) == + if (chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0) == CoinsCacheSizeState::CRITICAL) { break; } } BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0), + chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0), CoinsCacheSizeState::CRITICAL); // Passing non-zero max mempool usage should allow us more headroom. BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10), + chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10), CoinsCacheSizeState::OK); for (int i{0}; i < 3; ++i) { add_coin(view); print_view_mem_usage(view); BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10), + chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10), CoinsCacheSizeState::OK); } @@ -140,7 +139,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) BOOST_CHECK(usage_percentage >= 0.9); BOOST_CHECK(usage_percentage < 1); BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, 1 << 10), + chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, 1 << 10), CoinsCacheSizeState::LARGE); } @@ -148,7 +147,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) for (int i{0}; i < 1000; ++i) { add_coin(view); BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(&tx_pool), + chainstate.GetCoinsCacheSizeState(), CoinsCacheSizeState::OK); } @@ -156,7 +155,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) // preallocated memory that doesn't get reclaimed even after flush. BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, 0), + chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, 0), CoinsCacheSizeState::CRITICAL); view.SetBestBlock(InsecureRand256()); @@ -164,7 +163,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) print_view_mem_usage(view); BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, 0), + chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, 0), CoinsCacheSizeState::CRITICAL); } diff --git a/src/validation.cpp b/src/validation.cpp index 0eb73e12f4..01b7a23822 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -425,24 +425,15 @@ static bool IsCurrentForFeeEstimation(CChainState& active_chainstate) EXCLUSIVE_ return true; } -/* Make mempool consistent after a reorg, by re-adding or recursively erasing - * disconnected block transactions from the mempool, and also removing any - * other transactions from the mempool that are no longer valid given the new - * tip/height. - * - * Note: we assume that disconnectpool only contains transactions that are NOT - * confirmed in the current chain nor already in the mempool (otherwise, - * in-mempool descendants of such transactions would be removed). - * - * Passing fAddToMempool=false will skip trying to add the transactions back, - * and instead just erase from the mempool as needed. - */ - -static void UpdateMempoolForReorg(CChainState& active_chainstate, CTxMemPool& mempool, DisconnectedBlockTransactions& disconnectpool, bool fAddToMempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, mempool.cs) +void CChainState::MaybeUpdateMempoolForReorg( + DisconnectedBlockTransactions& disconnectpool, + bool fAddToMempool) { + if (!m_mempool) return; + AssertLockHeld(cs_main); - AssertLockHeld(mempool.cs); - assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate)); + AssertLockHeld(m_mempool->cs); + assert(std::addressof(::ChainstateActive()) == std::addressof(*this)); std::vector vHashUpdate; // disconnectpool's insertion_order index sorts the entries from // oldest to newest, but the oldest entry will be the last tx from the @@ -455,12 +446,12 @@ static void UpdateMempoolForReorg(CChainState& active_chainstate, CTxMemPool& me // ignore validation errors in resurrected transactions TxValidationState stateDummy; if (!fAddToMempool || (*it)->IsCoinBase() || - !AcceptToMemoryPool(active_chainstate, mempool, stateDummy, *it, + !AcceptToMemoryPool(*this, *m_mempool, stateDummy, *it, true /* bypass_limits */, 0 /* nAbsurdFee */)) { // If the transaction doesn't make it in to the mempool, remove any // transactions that depend on it (which would now be orphans). - mempool.removeRecursive(**it, MemPoolRemovalReason::REORG); - } else if (mempool.exists((*it)->GetHash())) { + m_mempool->removeRecursive(**it, MemPoolRemovalReason::REORG); + } else if (m_mempool->exists((*it)->GetHash())) { vHashUpdate.push_back((*it)->GetHash()); } ++it; @@ -471,12 +462,16 @@ static void UpdateMempoolForReorg(CChainState& active_chainstate, CTxMemPool& me // previously-confirmed transactions back to the mempool. // UpdateTransactionsFromBlock finds descendants of any transactions in // the disconnectpool that were added back and cleans up the mempool state. - mempool.UpdateTransactionsFromBlock(vHashUpdate); + m_mempool->UpdateTransactionsFromBlock(vHashUpdate); // We also need to remove any now-immature transactions - mempool.removeForReorg(active_chainstate, STANDARD_LOCKTIME_VERIFY_FLAGS); + m_mempool->removeForReorg(*this, STANDARD_LOCKTIME_VERIFY_FLAGS); // Re-limit mempool size, in case we added any transactions - LimitMempoolSize(mempool, active_chainstate.CoinsTip(), gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, std::chrono::hours{gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY)}); + LimitMempoolSize( + *m_mempool, + this->CoinsTip(), + gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, + std::chrono::hours{gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY)}); } // Used to avoid mempool polluting consensus critical paths if CCoinsViewMempool @@ -1296,12 +1291,12 @@ void CoinsViews::InitCache() m_cacheview = std::make_unique(&m_catcherview); } -CChainState::CChainState(BlockManager& blockman, +CChainState::CChainState(CTxMemPool* mempool, + BlockManager& blockman, + CEvoDB& evoDb, const std::unique_ptr& clhandler, const std::unique_ptr& isman, const std::unique_ptr& quorum_block_processor, - CEvoDB& evoDb, - CTxMemPool& mempool, std::optional from_snapshot_blockhash) : m_mempool(mempool), m_params(::Params()), @@ -2541,20 +2536,18 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, return true; } -CoinsCacheSizeState CChainState::GetCoinsCacheSizeState(const CTxMemPool* tx_pool) +CoinsCacheSizeState CChainState::GetCoinsCacheSizeState() { return this->GetCoinsCacheSizeState( - tx_pool, m_coinstip_cache_size_bytes, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); } CoinsCacheSizeState CChainState::GetCoinsCacheSizeState( - const CTxMemPool* tx_pool, size_t max_coins_cache_size_bytes, size_t max_mempool_size_bytes) { - const int64_t nMempoolUsage = tx_pool ? tx_pool->DynamicMemoryUsage() : 0; + const int64_t nMempoolUsage = m_mempool ? m_mempool->DynamicMemoryUsage() : 0; int64_t cacheSize = CoinsTip().DynamicMemoryUsage(); int64_t nTotalSpace = max_coins_cache_size_bytes + std::max(max_mempool_size_bytes - nMempoolUsage, 0); @@ -2592,7 +2585,8 @@ bool CChainState::FlushStateToDisk( { bool fFlushForPrune = false; bool fDoFullFlush = false; - CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(&m_mempool); + + CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(); LOCK(cs_LastBlockFile); if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) { if (nManualPruneHeight > 0) { @@ -2743,12 +2737,12 @@ static void AppendWarning(std::string& res, const std::string& warn) res += warn; } -/** Check warning conditions and do some notifications on new chain tip set. */ -static void UpdateTip(CTxMemPool& mempool, const CBlockIndex* pindexNew, const CChainParams& chainParams, CChainState& active_chainstate, const CEvoDB& evoDb) - EXCLUSIVE_LOCKS_REQUIRED(::cs_main) +void CChainState::UpdateTip(const CBlockIndex* pindexNew) { // New best block - mempool.AddTransactionsUpdated(1); + if (m_mempool) { + m_mempool->AddTransactionsUpdated(1); + } { LOCK(g_best_block_mutex); @@ -2757,14 +2751,14 @@ static void UpdateTip(CTxMemPool& mempool, const CBlockIndex* pindexNew, const C } std::string warningMessages; - assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate)); - if (!active_chainstate.IsInitialBlockDownload()) + assert(std::addressof(::ChainstateActive()) == std::addressof(*this)); + if (!this->IsInitialBlockDownload()) { int nUpgraded = 0; const CBlockIndex* pindex = pindexNew; for (int bit = 0; bit < VERSIONBITS_NUM_BITS; bit++) { WarningBitsConditionChecker checker(bit); - ThresholdState state = checker.GetStateFor(pindex, chainParams.GetConsensus(), warningcache[bit]); + ThresholdState state = checker.GetStateFor(pindex, m_params.GetConsensus(), warningcache[bit]); if (state == ThresholdState::ACTIVE || state == ThresholdState::LOCKED_IN) { const std::string strWarning = strprintf(_("Warning: unknown new rules activated (versionbit %i)").translated, bit); if (state == ThresholdState::ACTIVE) { @@ -2777,7 +2771,7 @@ static void UpdateTip(CTxMemPool& mempool, const CBlockIndex* pindexNew, const C // Check the version of the last 100 blocks to see if we need to upgrade: for (int i = 0; i < 100 && pindex != nullptr; i++) { - int32_t nExpectedVersion = ComputeBlockVersion(pindex->pprev, chainParams.GetConsensus()); + int32_t nExpectedVersion = ComputeBlockVersion(pindex->pprev, m_params.GetConsensus()); if (pindex->nVersion > VERSIONBITS_LAST_OLD_BLOCK_VERSION && (pindex->nVersion & ~nExpectedVersion) != 0) ++nUpgraded; pindex = pindex->pprev; @@ -2785,20 +2779,20 @@ static void UpdateTip(CTxMemPool& mempool, const CBlockIndex* pindexNew, const C if (nUpgraded > 0) AppendWarning(warningMessages, strprintf(_("%d of last 100 blocks have unexpected version").translated, nUpgraded)); } - assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate)); + assert(std::addressof(::ChainstateActive()) == std::addressof(*this)); LogPrintf("%s: new best=%s height=%d version=0x%08x log2_work=%.8g tx=%lu date='%s' progress=%f cache=%.1fMiB(%utxo) evodb_cache=%.1fMiB%s\n", __func__, pindexNew->GetBlockHash().ToString(), pindexNew->nHeight, pindexNew->nVersion, log(pindexNew->nChainWork.getdouble())/log(2.0), (unsigned long)pindexNew->nChainTx, FormatISO8601DateTime(pindexNew->GetBlockTime()), - GuessVerificationProgress(chainParams.TxData(), pindexNew), active_chainstate.CoinsTip().DynamicMemoryUsage() * (1.0 / (1<<20)), active_chainstate.CoinsTip().GetCacheSize(), - evoDb.GetMemoryUsage() * (1.0 / (1<<20)), + GuessVerificationProgress(m_params.TxData(), pindexNew), this->CoinsTip().DynamicMemoryUsage() * (1.0 / (1<<20)), this->CoinsTip().GetCacheSize(), + m_evoDb.GetMemoryUsage() * (1.0 / (1<<20)), !warningMessages.empty() ? strprintf(" warning='%s'", warningMessages) : ""); } /** Disconnect m_chain's tip. * After calling, the mempool will be in an inconsistent state, with * transactions from disconnected blocks being added to disconnectpool. You - * should make the mempool consistent again by calling UpdateMempoolForReorg. + * should make the mempool consistent again by calling MaybeUpdateMempoolForReorg. * with cs_main held. * * If disconnectpool is nullptr, then no disconnected transactions are added to @@ -2808,7 +2802,7 @@ static void UpdateTip(CTxMemPool& mempool, const CBlockIndex* pindexNew, const C bool CChainState::DisconnectTip(BlockValidationState& state, DisconnectedBlockTransactions* disconnectpool) { AssertLockHeld(cs_main); - AssertLockHeld(m_mempool.cs); + if (m_mempool) AssertLockHeld(m_mempool->cs); CBlockIndex *pindexDelete = m_chain.Tip(); assert(pindexDelete); @@ -2837,7 +2831,7 @@ bool CChainState::DisconnectTip(BlockValidationState& state, DisconnectedBlockTr return false; } - if (disconnectpool) { + if (disconnectpool && m_mempool) { // Save transactions to re-add to mempool at end of reorg for (auto it = block.vtx.rbegin(); it != block.vtx.rend(); ++it) { disconnectpool->addTransaction(*it); @@ -2845,14 +2839,14 @@ bool CChainState::DisconnectTip(BlockValidationState& state, DisconnectedBlockTr while (disconnectpool->DynamicMemoryUsage() > MAX_DISCONNECTED_TX_POOL_SIZE * 1000) { // Drop the earliest entry, and remove its children from the mempool. auto it = disconnectpool->queuedTx.get().begin(); - m_mempool.removeRecursive(**it, MemPoolRemovalReason::REORG); + m_mempool->removeRecursive(**it, MemPoolRemovalReason::REORG); disconnectpool->removeEntry(it); } } m_chain.SetTip(pindexDelete->pprev); - UpdateTip(m_mempool, pindexDelete->pprev, m_params, *this, m_evoDb); + UpdateTip(pindexDelete->pprev); // Let wallets know transactions went from 1-confirmed to // 0-confirmed or conflicted: GetMainSignals().BlockDisconnected(pblock, pindexDelete); @@ -2915,7 +2909,7 @@ bool CChainState::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew { boost::posix_time::ptime start = boost::posix_time::microsec_clock::local_time(); AssertLockHeld(cs_main); - AssertLockHeld(m_mempool.cs); + if (m_mempool) AssertLockHeld(m_mempool->cs); assert(pindexNew->pprev == m_chain.Tip()); // Read block from disk. @@ -2962,11 +2956,13 @@ bool CChainState::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew int64_t nTime5 = GetTimeMicros(); nTimeChainState += nTime5 - nTime4; LogPrint(BCLog::BENCHMARK, " - Writing chainstate: %.2fms [%.2fs (%.2fms/blk)]\n", (nTime5 - nTime4) * MILLI, nTimeChainState * MICRO, nTimeChainState * MILLI / nBlocksTotal); // Remove conflicting transactions from the mempool.; - m_mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight); - disconnectpool.removeForBlock(blockConnecting.vtx); + if (m_mempool) { + m_mempool->removeForBlock(blockConnecting.vtx, pindexNew->nHeight); + disconnectpool.removeForBlock(blockConnecting.vtx); + } // Update m_chain & related variables. m_chain.SetTip(pindexNew); - UpdateTip(m_mempool, pindexNew, m_params, *this, m_evoDb); + UpdateTip(pindexNew); int64_t nTime6 = GetTimeMicros(); nTimePostConnect += nTime6 - nTime5; nTimeTotal += nTime6 - nTime1; LogPrint(BCLog::BENCHMARK, " - Connect postprocess: %.2fms [%.2fs (%.2fms/blk)]\n", (nTime6 - nTime5) * MILLI, nTimePostConnect * MICRO, nTimePostConnect * MILLI / nBlocksTotal); @@ -3064,7 +3060,7 @@ void CChainState::PruneBlockIndexCandidates() { bool CChainState::ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) { AssertLockHeld(cs_main); - AssertLockHeld(m_mempool.cs); + if (m_mempool) AssertLockHeld(m_mempool->cs); assert(std::addressof(::ChainstateActive()) == std::addressof(*this)); const CBlockIndex* pindexOldTip = m_chain.Tip(); @@ -3077,7 +3073,7 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, CBlockIndex if (!DisconnectTip(state, &disconnectpool)) { // This is likely a fatal error, but keep the mempool consistent, // just in case. Only remove from the mempool in this case. - UpdateMempoolForReorg(*this, m_mempool, disconnectpool, false); + MaybeUpdateMempoolForReorg(disconnectpool, false); // If we're unable to disconnect a block during normal operation, // then that is a failure of our local system -- we should abort @@ -3121,7 +3117,7 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, CBlockIndex // A system error occurred (disk space, database error, ...). // Make the mempool consistent with the current tip, just in case // any observers try to use it before shutdown. - UpdateMempoolForReorg(*this, m_mempool, disconnectpool, false); + MaybeUpdateMempoolForReorg(disconnectpool, false); return false; } } else { @@ -3138,9 +3134,9 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, CBlockIndex if (fBlocksDisconnected) { // If any blocks were disconnected, disconnectpool may be non empty. Add // any disconnected transactions back to the mempool. - UpdateMempoolForReorg(*this, m_mempool, disconnectpool, true); + MaybeUpdateMempoolForReorg(disconnectpool, true); } - m_mempool.check(*this); + if (m_mempool) m_mempool->check(*this); CheckForkWarningConditions(); @@ -3209,7 +3205,8 @@ bool CChainState::ActivateBestChain(BlockValidationState& state, std::shared_ptr { LOCK(cs_main); - LOCK(m_mempool.cs); // Lock transaction pool for at least as long as it takes for connectTrace to be consumed + // Lock transaction pool for at least as long as it takes for connectTrace to be consumed + LOCK(MempoolMutex()); CBlockIndex* starting_tip = m_chain.Tip(); bool blocks_connected = false; do { @@ -3364,7 +3361,9 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, CBlockIndex* pind LimitValidationInterfaceQueue(); LOCK(cs_main); - LOCK(m_mempool.cs); // Lock for as long as disconnectpool is in scope to make sure UpdateMempoolForReorg is called after DisconnectTip without unlocking in between + // Lock for as long as disconnectpool is in scope to make sure MaybeUpdateMempoolForReorg is + // called after DisconnectTip without unlocking in between + LOCK(MempoolMutex()); if (!m_chain.Contains(pindex)) break; pindex_was_in_chain = true; CBlockIndex *invalid_walk_tip = m_chain.Tip(); @@ -3385,7 +3384,7 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, CBlockIndex* pind // and we're not doing a very deep invalidation (in which case // keeping the mempool up to date is probably futile anyway). assert(std::addressof(::ChainstateActive()) == std::addressof(*this)); - UpdateMempoolForReorg(*this, m_mempool, disconnectpool, /* fAddToMempool = */ (++disconnected <= 10) && ret); + MaybeUpdateMempoolForReorg(disconnectpool, /* fAddToMempool = */ (++disconnected <= 10) && ret); if (!ret) return false; assert(invalid_walk_tip->pprev == m_chain.Tip()); @@ -3515,7 +3514,7 @@ bool CChainState::MarkConflictingBlock(BlockValidationState& state, CBlockIndex } { - LOCK(m_mempool.cs); // Lock for as long as disconnectpool is in scope to make sure UpdateMempoolForReorg is called after DisconnectTip without unlocking in between + LOCK(MempoolMutex()); // Lock for as long as disconnectpool is in scope to make sure UpdateMempoolForReorg is called after DisconnectTip without unlocking in between DisconnectedBlockTransactions disconnectpool; while (m_chain.Contains(pindex)) { const CBlockIndex* pindexOldTip = m_chain.Tip(); @@ -3525,7 +3524,7 @@ bool CChainState::MarkConflictingBlock(BlockValidationState& state, CBlockIndex if (!DisconnectTip(state, &disconnectpool)) { // It's probably hopeless to try to make the mempool consistent // here if DisconnectTip failed, but we can try. - UpdateMempoolForReorg(*this, m_mempool, disconnectpool, false); + MaybeUpdateMempoolForReorg(disconnectpool, false); return false; } if (pindexOldTip == pindexBestHeader) { @@ -3547,7 +3546,7 @@ bool CChainState::MarkConflictingBlock(BlockValidationState& state, CBlockIndex // DisconnectTip will add transactions to disconnectpool; try to add these // back to the mempool. - UpdateMempoolForReorg(*this, m_mempool, disconnectpool, true); + MaybeUpdateMempoolForReorg(disconnectpool, true); } // m_mempool.cs // The resulting new best tip may not be in setBlockIndexCandidates anymore, so @@ -4731,11 +4730,12 @@ bool CChainState::LoadBlockIndexDB() void CChainState::LoadMempool(const ArgsManager& args) { + if (!m_mempool) return; if (args.GetArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { assert(std::addressof(::ChainstateActive()) == std::addressof(*this)); - ::LoadMempool(m_mempool, *this); + ::LoadMempool(*m_mempool, *this); } - m_mempool.SetIsLoaded(!ShutdownRequested()); + m_mempool->SetIsLoaded(!ShutdownRequested()); } bool CChainState::LoadChainTip() @@ -5669,11 +5669,11 @@ std::vector ChainstateManager::GetAll() return out; } -CChainState& ChainstateManager::InitializeChainstate(const std::unique_ptr& clhandler, +CChainState& ChainstateManager::InitializeChainstate(CTxMemPool* mempool, + CEvoDB& evoDb, + const std::unique_ptr& clhandler, const std::unique_ptr& isman, const std::unique_ptr& quorum_block_processor, - CEvoDB& evoDb, - CTxMemPool& mempool, const std::optional& snapshot_blockhash) { bool is_snapshot = snapshot_blockhash.has_value(); @@ -5684,7 +5684,7 @@ CChainState& ChainstateManager::InitializeChainstate(const std::unique_ptr( - m_blockman, this->ActiveChainstate().m_clhandler, this->ActiveChainstate().m_isman, - this->ActiveChainstate().m_quorum_block_processor, this->ActiveChainstate().m_evoDb, - this->ActiveChainstate().m_mempool, base_blockhash + /* mempool */ nullptr, m_blockman, this->ActiveChainstate().m_evoDb, + this->ActiveChainstate().m_clhandler, this->ActiveChainstate().m_isman, + this->ActiveChainstate().m_quorum_block_processor, base_blockhash ) ); @@ -5866,7 +5866,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot( } const auto snapshot_cache_state = WITH_LOCK(::cs_main, - return snapshot_chainstate.GetCoinsCacheSizeState(&snapshot_chainstate.m_mempool)); + return snapshot_chainstate.GetCoinsCacheSizeState()); if (snapshot_cache_state >= CoinsCacheSizeState::CRITICAL) { diff --git a/src/validation.h b/src/validation.h index 9cf420d5ab..2406334850 100644 --- a/src/validation.h +++ b/src/validation.h @@ -570,8 +570,9 @@ private: */ mutable std::atomic m_cached_finished_ibd{false}; - //! mempool that is kept in sync with the chain - CTxMemPool& m_mempool; + //! Optional mempool that is kept in sync with the chain. + //! Only the active chainstate has a mempool. + CTxMemPool* m_mempool; const CChainParams& m_params; @@ -589,12 +590,12 @@ public: //! CChainState instances. BlockManager& m_blockman; - explicit CChainState(BlockManager& blockman, + explicit CChainState(CTxMemPool* mempool, + BlockManager& blockman, + CEvoDB& evoDb, const std::unique_ptr& clhandler, const std::unique_ptr& isman, const std::unique_ptr& quorum_block_processor, - CEvoDB& evoDb, - CTxMemPool& mempool, std::optional from_snapshot_blockhash = std::nullopt); /** @@ -723,7 +724,7 @@ public: bool ConnectBlock(const CBlock& block, BlockValidationState& state, CBlockIndex* pindex, CCoinsViewCache& view, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); // Apply the effects of a block disconnection on the UTXO set. - bool DisconnectTip(BlockValidationState& state, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs); + bool DisconnectTip(BlockValidationState& state, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs); // Manual block validity manipulation: /** Mark a block as precious and reorganize. @@ -767,19 +768,17 @@ public: //! Dictates whether we need to flush the cache to disk or not. //! //! @return the state of the size of the coins cache. - CoinsCacheSizeState GetCoinsCacheSizeState(const CTxMemPool* tx_pool) - EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + CoinsCacheSizeState GetCoinsCacheSizeState() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); CoinsCacheSizeState GetCoinsCacheSizeState( - const CTxMemPool* tx_pool, size_t max_coins_cache_size_bytes, size_t max_mempool_size_bytes) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); std::string ToString() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); private: - bool ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs); - bool ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs); + bool ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs); + bool ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs); void InvalidBlockFound(CBlockIndex* pindex, const BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main); CBlockIndex* FindMostWorkChain() EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -799,6 +798,33 @@ private: bool LoadBlockIndexDB() EXCLUSIVE_LOCKS_REQUIRED(cs_main); + //! Indirection necessary to make lock annotations work with an optional mempool. + RecursiveMutex* MempoolMutex() const LOCK_RETURNED(m_mempool->cs) + { + return m_mempool ? &m_mempool->cs : nullptr; + } + + /** + * Make mempool consistent after a reorg, by re-adding or recursively erasing + * disconnected block transactions from the mempool, and also removing any + * other transactions from the mempool that are no longer valid given the new + * tip/height. + * + * Note: we assume that disconnectpool only contains transactions that are NOT + * confirmed in the current chain nor already in the mempool (otherwise, + * in-mempool descendants of such transactions would be removed). + * + * Passing fAddToMempool=false will skip trying to add the transactions back, + * and instead just erase from the mempool as needed. + */ + void MaybeUpdateMempoolForReorg( + DisconnectedBlockTransactions& disconnectpool, + bool fAddToMempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs); + + /** Check warning conditions and do some notifications on new chain tip set. */ + void UpdateTip(const CBlockIndex* pindexNew) + EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + friend ChainstateManager; }; @@ -911,11 +937,11 @@ public: // constructor //! @param[in] snapshot_blockhash If given, signify that this chainstate //! is based on a snapshot. - CChainState& InitializeChainstate(const std::unique_ptr& clhandler, + CChainState& InitializeChainstate(CTxMemPool* mempool, + CEvoDB& evoDb, + const std::unique_ptr& clhandler, const std::unique_ptr& isman, const std::unique_ptr& quorum_block_processor, - CEvoDB& evoDb, - CTxMemPool& mempool, const std::optional& snapshot_blockhash = std::nullopt) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);