From fae6ab6aed3b9fdc9201bb19a307dfc3d9b89891 Mon Sep 17 00:00:00 2001 From: James O'Beirne <james.obeirne@gmail.com> Date: Wed, 24 Jul 2019 11:45:04 -0400 Subject: [PATCH 1/3] refactor: pcoinsTip -> CChainState::CoinsTip() This aliasing makes subsequent commits easier to review; eventually CoinsTip() will return the CCoinsViewCache managed by CChainState. --- src/interfaces/node.cpp | 2 +- src/net_processing.cpp | 9 +++--- src/node/coin.cpp | 3 +- src/node/transaction.cpp | 2 +- src/rest.cpp | 4 +-- src/rpc/blockchain.cpp | 11 ++++--- src/rpc/rawtransaction.cpp | 6 ++-- src/test/miner_tests.cpp | 6 ++-- src/test/txvalidationcache_tests.cpp | 22 ++++++------- src/txmempool.h | 2 +- src/validation.cpp | 47 +++++++++++++++------------- src/validation.h | 12 +++++-- test/functional/feature_dbcrash.py | 2 +- 13 files changed, 70 insertions(+), 58 deletions(-) diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index bcd226edd..fc4981750 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -232,7 +232,7 @@ public: bool getUnspentOutput(const COutPoint& output, Coin& coin) override { LOCK(::cs_main); - return ::pcoinsTip->GetCoin(output, coin); + return ::ChainstateActive().CoinsTip().GetCoin(output, coin); } std::string getWalletDir() override { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 5efb4adee..7f2c5720d 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1291,11 +1291,12 @@ bool static AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main) LOCK(g_cs_orphans); if (mapOrphanTransactions.count(inv.hash)) return true; } + const CCoinsViewCache& coins_cache = ::ChainstateActive().CoinsTip(); return recentRejects->contains(inv.hash) || mempool.exists(inv.hash) || - pcoinsTip->HaveCoinInCache(COutPoint(inv.hash, 0)) || // Best effort: only try output 0 and 1 - pcoinsTip->HaveCoinInCache(COutPoint(inv.hash, 1)); + coins_cache.HaveCoinInCache(COutPoint(inv.hash, 0)) || // Best effort: only try output 0 and 1 + coins_cache.HaveCoinInCache(COutPoint(inv.hash, 1)); } case MSG_BLOCK: case MSG_WITNESS_BLOCK: @@ -1844,7 +1845,7 @@ void static ProcessOrphanTx(CConnman* connman, std::set<uint256>& orphan_work_se EraseOrphanTx(orphanHash); done = true; } - mempool.check(pcoinsTip.get()); + mempool.check(&::ChainstateActive().CoinsTip()); } } @@ -2497,7 +2498,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (!AlreadyHave(inv) && AcceptToMemoryPool(mempool, state, ptx, &fMissingInputs, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) { - mempool.check(pcoinsTip.get()); + mempool.check(&::ChainstateActive().CoinsTip()); RelayTransaction(tx.GetHash(), *connman); for (unsigned int i = 0; i < tx.vout.size(); i++) { auto it_by_prev = mapOrphanTransactionsByPrev.find(COutPoint(inv.hash, i)); diff --git a/src/node/coin.cpp b/src/node/coin.cpp index bb98e63f3..ad8d1d3af 100644 --- a/src/node/coin.cpp +++ b/src/node/coin.cpp @@ -10,8 +10,7 @@ void FindCoins(std::map<COutPoint, Coin>& coins) { LOCK2(cs_main, ::mempool.cs); - assert(pcoinsTip); - CCoinsViewCache& chain_view = *::pcoinsTip; + CCoinsViewCache& chain_view = ::ChainstateActive().CoinsTip(); CCoinsViewMemPool mempool_view(&chain_view, ::mempool); for (auto& coin : coins) { if (!mempool_view.GetCoin(coin.first, coin.second)) { diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 8e5649635..037576b82 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -25,7 +25,7 @@ TransactionError BroadcastTransaction(const CTransactionRef tx, std::string& err LOCK(cs_main); // If the transaction is already confirmed in the chain, don't do anything // and return early. - CCoinsViewCache &view = *pcoinsTip; + CCoinsViewCache &view = ::ChainstateActive().CoinsTip(); for (size_t o = 0; o < tx->vout.size(); o++) { const Coin& existingCoin = view.AccessCoin(COutPoint(hashTx, o)); // IsSpent doesnt mean the coin is spent, it means the output doesnt' exist. diff --git a/src/rest.cpp b/src/rest.cpp index eba7aae50..2c4d47554 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -503,12 +503,12 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart) if (fCheckMemPool) { // use db+mempool as cache backend in case user likes to query mempool LOCK2(cs_main, mempool.cs); - CCoinsViewCache& viewChain = *pcoinsTip; + CCoinsViewCache& viewChain = ::ChainstateActive().CoinsTip(); CCoinsViewMemPool viewMempool(&viewChain, mempool); process_utxos(viewMempool, mempool); } else { LOCK(cs_main); // no need to lock mempool! - process_utxos(*pcoinsTip, CTxMemPool()); + process_utxos(::ChainstateActive().CoinsTip(), CTxMemPool()); } for (size_t i = 0; i < hits.size(); ++i) { diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index b7dcd59c6..c35c478f3 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1126,19 +1126,21 @@ UniValue gettxout(const JSONRPCRequest& request) fMempool = request.params[2].get_bool(); Coin coin; + CCoinsViewCache* coins_view = &::ChainstateActive().CoinsTip(); + if (fMempool) { LOCK(mempool.cs); - CCoinsViewMemPool view(pcoinsTip.get(), mempool); + CCoinsViewMemPool view(coins_view, mempool); if (!view.GetCoin(out, coin) || mempool.isSpent(out)) { return NullUniValue; } } else { - if (!pcoinsTip->GetCoin(out, coin)) { + if (!coins_view->GetCoin(out, coin)) { return NullUniValue; } } - const CBlockIndex* pindex = LookupBlockIndex(pcoinsTip->GetBestBlock()); + const CBlockIndex* pindex = LookupBlockIndex(coins_view->GetBestBlock()); ret.pushKV("bestblock", pindex->GetBlockHash().GetHex()); if (coin.nHeight == MEMPOOL_HEIGHT) { ret.pushKV("confirmations", 0); @@ -1180,7 +1182,8 @@ static UniValue verifychain(const JSONRPCRequest& request) if (!request.params[1].isNull()) nCheckDepth = request.params[1].get_int(); - return CVerifyDB().VerifyDB(Params(), pcoinsTip.get(), nCheckLevel, nCheckDepth); + return CVerifyDB().VerifyDB( + Params(), &::ChainstateActive().CoinsTip(), nCheckLevel, nCheckDepth); } /** Implementation of IsSuperMajority with better feedback */ diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 966c159f0..ffbad4571 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -259,7 +259,7 @@ static UniValue gettxoutproof(const JSONRPCRequest& request) // Loop through txids and try to find which block they're in. Exit loop once a block is found. for (const auto& tx : setTxids) { - const Coin& coin = AccessByTxid(*pcoinsTip, tx); + const Coin& coin = AccessByTxid(::ChainstateActive().CoinsTip(), tx); if (!coin.IsSpent()) { pblockindex = ::ChainActive()[coin.nHeight]; break; @@ -636,7 +636,7 @@ static UniValue combinerawtransaction(const JSONRPCRequest& request) { LOCK(cs_main); LOCK(mempool.cs); - CCoinsViewCache &viewChain = *pcoinsTip; + CCoinsViewCache &viewChain = ::ChainstateActive().CoinsTip(); CCoinsViewMemPool viewMempool(&viewChain, mempool); view.SetBackend(viewMempool); // temporarily switch cache backend to db+mempool view @@ -1505,7 +1505,7 @@ UniValue utxoupdatepsbt(const JSONRPCRequest& request) CCoinsViewCache view(&viewDummy); { LOCK2(cs_main, mempool.cs); - CCoinsViewCache &viewChain = *pcoinsTip; + CCoinsViewCache &viewChain = ::ChainstateActive().CoinsTip(); CCoinsViewMemPool viewMempool(&viewChain, mempool); view.SetBackend(viewMempool); // temporarily switch cache backend to db+mempool view diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 05d7f7698..c9661b730 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -372,7 +372,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) CBlockIndex* prev = ::ChainActive().Tip(); CBlockIndex* next = new CBlockIndex(); next->phashBlock = new uint256(InsecureRand256()); - pcoinsTip->SetBestBlock(next->GetBlockHash()); + ::ChainstateActive().CoinsTip().SetBestBlock(next->GetBlockHash()); next->pprev = prev; next->nHeight = prev->nHeight + 1; next->BuildSkip(); @@ -384,7 +384,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) CBlockIndex* prev = ::ChainActive().Tip(); CBlockIndex* next = new CBlockIndex(); next->phashBlock = new uint256(InsecureRand256()); - pcoinsTip->SetBestBlock(next->GetBlockHash()); + ::ChainstateActive().CoinsTip().SetBestBlock(next->GetBlockHash()); next->pprev = prev; next->nHeight = prev->nHeight + 1; next->BuildSkip(); @@ -414,7 +414,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) while (::ChainActive().Tip()->nHeight > nHeight) { CBlockIndex* del = ::ChainActive().Tip(); ::ChainActive().SetTip(del->pprev); - pcoinsTip->SetBestBlock(del->pprev->GetBlockHash()); + ::ChainstateActive().CoinsTip().SetBestBlock(del->pprev->GetBlockHash()); delete del->phashBlock; delete del; } diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index f99a3748c..e69ebcc2c 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -97,7 +97,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, TestChain100Setup) BOOST_CHECK_EQUAL(mempool.size(), 0U); } -// Run CheckInputs (using pcoinsTip) on the given transaction, for all script +// Run CheckInputs (using CoinsTip()) on the given transaction, for all script // flags. Test that CheckInputs passes for all flags that don't overlap with // the failing_flags argument, but otherwise fails. // CHECKLOCKTIMEVERIFY and CHECKSEQUENCEVERIFY (and future NOP codes that may @@ -125,7 +125,7 @@ static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t fail // WITNESS requires P2SH test_flags |= SCRIPT_VERIFY_P2SH; } - bool ret = CheckInputs(tx, state, pcoinsTip.get(), true, test_flags, true, add_to_cache, txdata, nullptr); + bool ret = CheckInputs(tx, state, &::ChainstateActive().CoinsTip(), true, test_flags, true, add_to_cache, txdata, nullptr); // CheckInputs should succeed iff test_flags doesn't intersect with // failing_flags bool expected_return_value = !(test_flags & failing_flags); @@ -135,13 +135,13 @@ static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t fail if (ret && add_to_cache) { // Check that we get a cache hit if the tx was valid std::vector<CScriptCheck> scriptchecks; - BOOST_CHECK(CheckInputs(tx, state, pcoinsTip.get(), true, test_flags, true, add_to_cache, txdata, &scriptchecks)); + BOOST_CHECK(CheckInputs(tx, state, &::ChainstateActive().CoinsTip(), true, test_flags, true, add_to_cache, txdata, &scriptchecks)); BOOST_CHECK(scriptchecks.empty()); } else { // Check that we get script executions to check, if the transaction // was invalid, or we didn't add to cache. std::vector<CScriptCheck> scriptchecks; - BOOST_CHECK(CheckInputs(tx, state, pcoinsTip.get(), true, test_flags, true, add_to_cache, txdata, &scriptchecks)); + BOOST_CHECK(CheckInputs(tx, state, &::ChainstateActive().CoinsTip(), true, test_flags, true, add_to_cache, txdata, &scriptchecks)); BOOST_CHECK_EQUAL(scriptchecks.size(), tx.vin.size()); } } @@ -204,13 +204,13 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) CValidationState state; PrecomputedTransactionData ptd_spend_tx(spend_tx); - BOOST_CHECK(!CheckInputs(CTransaction(spend_tx), state, pcoinsTip.get(), true, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, nullptr)); + BOOST_CHECK(!CheckInputs(CTransaction(spend_tx), state, &::ChainstateActive().CoinsTip(), true, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, nullptr)); // If we call again asking for scriptchecks (as happens in // ConnectBlock), we should add a script check object for this -- we're // not caching invalidity (if that changes, delete this test case). std::vector<CScriptCheck> scriptchecks; - BOOST_CHECK(CheckInputs(CTransaction(spend_tx), state, pcoinsTip.get(), true, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, &scriptchecks)); + BOOST_CHECK(CheckInputs(CTransaction(spend_tx), state, &::ChainstateActive().CoinsTip(), true, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, &scriptchecks)); BOOST_CHECK_EQUAL(scriptchecks.size(), 1U); // Test that CheckInputs returns true iff DERSIG-enforcing flags are @@ -227,7 +227,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) block = CreateAndProcessBlock({spend_tx}, p2pk_scriptPubKey); LOCK(cs_main); BOOST_CHECK(::ChainActive().Tip()->GetBlockHash() == block.GetHash()); - BOOST_CHECK(pcoinsTip->GetBestBlock() == block.GetHash()); + BOOST_CHECK(::ChainstateActive().CoinsTip().GetBestBlock() == block.GetHash()); // Test P2SH: construct a transaction that is valid without P2SH, and // then test validity with P2SH. @@ -272,7 +272,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) invalid_with_cltv_tx.vin[0].scriptSig = CScript() << vchSig << 100; CValidationState state; PrecomputedTransactionData txdata(invalid_with_cltv_tx); - BOOST_CHECK(CheckInputs(CTransaction(invalid_with_cltv_tx), state, pcoinsTip.get(), true, SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, true, true, txdata, nullptr)); + BOOST_CHECK(CheckInputs(CTransaction(invalid_with_cltv_tx), state, ::ChainstateActive().CoinsTip(), true, SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, true, true, txdata, nullptr)); } // TEST CHECKSEQUENCEVERIFY @@ -300,7 +300,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) invalid_with_csv_tx.vin[0].scriptSig = CScript() << vchSig << 100; CValidationState state; PrecomputedTransactionData txdata(invalid_with_csv_tx); - BOOST_CHECK(CheckInputs(CTransaction(invalid_with_csv_tx), state, pcoinsTip.get(), true, SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, true, true, txdata, nullptr)); + BOOST_CHECK(CheckInputs(CTransaction(invalid_with_csv_tx), state, &::ChainstateActive().CoinsTip(), true, SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, true, true, txdata, nullptr)); } // TODO: add tests for remaining script flags @@ -362,12 +362,12 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) CValidationState state; PrecomputedTransactionData txdata(tx); // This transaction is now invalid under segwit, because of the second input. - BOOST_CHECK(!CheckInputs(CTransaction(tx), state, pcoinsTip.get(), true, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, nullptr)); + BOOST_CHECK(!CheckInputs(CTransaction(tx), state, &::ChainstateActive().CoinsTip(), true, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, nullptr)); std::vector<CScriptCheck> scriptchecks; // Make sure this transaction was not cached (ie because the first // input was valid) - BOOST_CHECK(CheckInputs(CTransaction(tx), state, pcoinsTip.get(), true, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, &scriptchecks)); + BOOST_CHECK(CheckInputs(CTransaction(tx), state, &::ChainstateActive().CoinsTip(), true, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, &scriptchecks)); // Should get 2 script checks back -- caching is on a whole-transaction basis. BOOST_CHECK_EQUAL(scriptchecks.size(), 2U); } diff --git a/src/txmempool.h b/src/txmempool.h index 7169e80da..6e5ba445d 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -497,7 +497,7 @@ public: * * 1. Locking both `cs_main` and `mempool.cs` will give a view of mempool * that is consistent with current chain tip (`::ChainActive()` and - * `pcoinsTip`) and is fully populated. Fully populated means that if the + * `CoinsTip()`) and is fully populated. Fully populated means that if the * current active chain is missing transactions that were present in a * previously active chain, all the missing transactions will have been * re-added to the mempool and should be present if they meet size and diff --git a/src/validation.cpp b/src/validation.cpp index b4677df62..f9d38a309 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -260,8 +260,8 @@ bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flag lockPair.second = lp->time; } else { - // pcoinsTip contains the UTXO set for ::ChainActive().Tip() - CCoinsViewMemPool viewMemPool(pcoinsTip.get(), pool); + // CoinsTip() contains the UTXO set for ::ChainActive().Tip() + CCoinsViewMemPool viewMemPool(&::ChainstateActive().CoinsTip(), pool); std::vector<int> prevheights; prevheights.resize(tx.vin.size()); for (size_t txinIndex = 0; txinIndex < tx.vin.size(); txinIndex++) { @@ -320,7 +320,7 @@ static void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age) std::vector<COutPoint> vNoSpendsRemaining; pool.TrimToSize(limit, &vNoSpendsRemaining); for (const COutPoint& removed : vNoSpendsRemaining) - pcoinsTip->Uncache(removed); + ::ChainstateActive().CoinsTip().Uncache(removed); } static bool IsCurrentForFeeEstimation() EXCLUSIVE_LOCKS_REQUIRED(cs_main) @@ -382,7 +382,7 @@ static void UpdateMempoolForReorg(DisconnectedBlockTransactions& disconnectpool, mempool.UpdateTransactionsFromBlock(vHashUpdate); // We also need to remove any now-immature transactions - mempool.removeForReorg(pcoinsTip.get(), ::ChainActive().Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS); + mempool.removeForReorg(&::ChainstateActive().CoinsTip(), ::ChainActive().Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS); // Re-limit mempool size, in case we added any transactions LimitMempoolSize(mempool, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); } @@ -414,7 +414,7 @@ static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, CValidationSt assert(txFrom->vout.size() > txin.prevout.n); assert(txFrom->vout[txin.prevout.n] == coin.out); } else { - const Coin& coinFromDisk = pcoinsTip->AccessCoin(txin.prevout); + const Coin& coinFromDisk = ::ChainstateActive().CoinsTip().AccessCoin(txin.prevout); assert(!coinFromDisk.IsSpent()); assert(coinFromDisk.out == coin.out); } @@ -514,12 +514,13 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool CCoinsViewCache view(&dummy); LockPoints lp; - CCoinsViewMemPool viewMemPool(pcoinsTip.get(), pool); + CCoinsViewCache& coins_cache = ::ChainstateActive().CoinsTip(); + CCoinsViewMemPool viewMemPool(&coins_cache, pool); view.SetBackend(viewMemPool); // do all inputs exist? for (const CTxIn& txin : tx.vin) { - if (!pcoinsTip->HaveCoinInCache(txin.prevout)) { + if (!coins_cache.HaveCoinInCache(txin.prevout)) { coins_to_uncache.push_back(txin.prevout); } @@ -530,7 +531,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // Are inputs missing because we already have the tx? for (size_t out = 0; out < tx.vout.size(); out++) { // Optimistically just do efficient check of cache for outputs - if (pcoinsTip->HaveCoinInCache(COutPoint(hash, out))) { + if (coins_cache.HaveCoinInCache(COutPoint(hash, out))) { return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, REJECT_DUPLICATE, "txn-already-known"); } } @@ -860,7 +861,7 @@ static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPo // (`CCoinsViewCache::cacheCoins`). for (const COutPoint& hashTx : coins_to_uncache) - pcoinsTip->Uncache(hashTx); + ::ChainstateActive().CoinsTip().Uncache(hashTx); } // After we've (potentially) uncached entries, ensure our coins cache is still within its size limits CValidationState stateDummy; @@ -2014,7 +2015,7 @@ bool CChainState::FlushStateToDisk( nLastFlush = nNow; } int64_t nMempoolSizeMax = gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000; - int64_t cacheSize = pcoinsTip->DynamicMemoryUsage(); + int64_t cacheSize = CoinsTip().DynamicMemoryUsage(); int64_t nTotalSpace = nCoinCacheUsage + std::max<int64_t>(nMempoolSizeMax - nMempoolUsage, 0); // The cache is large and we're within 10% and 10 MiB of the limit, but we have time now (not in the middle of a block processing). bool fCacheLarge = mode == FlushStateMode::PERIODIC && cacheSize > std::max((9 * nTotalSpace) / 10, nTotalSpace - MAX_BLOCK_COINSDB_USAGE * 1024 * 1024); @@ -2058,17 +2059,17 @@ bool CChainState::FlushStateToDisk( nLastWrite = nNow; } // Flush best chain related state. This can only be done if the blocks / block index write was also done. - if (fDoFullFlush && !pcoinsTip->GetBestBlock().IsNull()) { + if (fDoFullFlush && !CoinsTip().GetBestBlock().IsNull()) { // Typical Coin structures on disk are around 48 bytes in size. // Pushing a new one to the database can cause it to be written // twice (once in the log, and once in the tables). This is already // an overestimation, as most will delete an existing entry or // overwrite one. Still, use a conservative safety factor of 2. - if (!CheckDiskSpace(GetDataDir(), 48 * 2 * 2 * pcoinsTip->GetCacheSize())) { + if (!CheckDiskSpace(GetDataDir(), 48 * 2 * 2 * CoinsTip().GetCacheSize())) { return AbortNode(state, "Disk space is too low!", _("Error: Disk space is too low!").translated, CClientUIInterface::MSG_NOPREFIX); } // Flush the chainstate (which may refer to block index entries). - if (!pcoinsTip->Flush()) + if (!CoinsTip().Flush()) return AbortNode(state, "Failed to write to coin database"); nLastFlush = nNow; full_flush_completed = true; @@ -2162,7 +2163,7 @@ void static UpdateTip(const CBlockIndex *pindexNew, const CChainParams& chainPar 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), pcoinsTip->DynamicMemoryUsage() * (1.0 / (1<<20)), pcoinsTip->GetCacheSize()); + GuessVerificationProgress(chainParams.TxData(), pindexNew), ::ChainstateActive().CoinsTip().DynamicMemoryUsage() * (1.0 / (1<<20)), ::ChainstateActive().CoinsTip().GetCacheSize()); if (!warningMessages.empty()) LogPrintf(" warning='%s'", warningMessages); /* Continued */ LogPrintf("\n"); @@ -2191,7 +2192,7 @@ bool CChainState::DisconnectTip(CValidationState& state, const CChainParams& cha // Apply the block atomically to the chain state. int64_t nStart = GetTimeMicros(); { - CCoinsViewCache view(pcoinsTip.get()); + CCoinsViewCache view(&CoinsTip()); assert(view.GetBestBlock() == pindexDelete->GetBlockHash()); if (DisconnectBlock(block, pindexDelete, view) != DISCONNECT_OK) return error("DisconnectTip(): DisconnectBlock %s failed", pindexDelete->GetBlockHash().ToString()); @@ -2319,7 +2320,7 @@ bool CChainState::ConnectTip(CValidationState& state, const CChainParams& chainp int64_t nTime3; LogPrint(BCLog::BENCH, " - Load block from disk: %.2fms [%.2fs]\n", (nTime2 - nTime1) * MILLI, nTimeReadFromDisk * MICRO); { - CCoinsViewCache view(pcoinsTip.get()); + CCoinsViewCache view(&CoinsTip()); bool rv = ConnectBlock(blockConnecting, state, pindexNew, view, chainparams); GetMainSignals().BlockChecked(blockConnecting, state); if (!rv) { @@ -2506,7 +2507,7 @@ bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainPar // any disconnected transactions back to the mempool. UpdateMempoolForReorg(disconnectpool, true); } - mempool.check(pcoinsTip.get()); + mempool.check(&CoinsTip()); // Callbacks/notifications for a new best chain. if (fInvalidFound) @@ -3508,7 +3509,7 @@ bool TestBlockValidity(CValidationState& state, const CChainParams& chainparams, { AssertLockHeld(cs_main); assert(pindexPrev && pindexPrev == ::ChainActive().Tip()); - CCoinsViewCache viewNew(pcoinsTip.get()); + CCoinsViewCache viewNew(&::ChainstateActive().CoinsTip()); uint256 block_hash(block.GetHash()); CBlockIndex indexDummy(block); indexDummy.pprev = pindexPrev; @@ -3861,12 +3862,14 @@ bool static LoadBlockIndexDB(const CChainParams& chainparams) EXCLUSIVE_LOCKS_RE bool LoadChainTip(const CChainParams& chainparams) { AssertLockHeld(cs_main); - assert(!pcoinsTip->GetBestBlock().IsNull()); // Never called when the coins view is empty + const CCoinsViewCache& coins_cache = ::ChainstateActive().CoinsTip(); + assert(!coins_cache.GetBestBlock().IsNull()); // Never called when the coins view is empty - if (::ChainActive().Tip() && ::ChainActive().Tip()->GetBlockHash() == pcoinsTip->GetBestBlock()) return true; + if (::ChainActive().Tip() && + ::ChainActive().Tip()->GetBlockHash() == coins_cache.GetBestBlock()) return true; // Load pointer to end of best chain - CBlockIndex* pindex = LookupBlockIndex(pcoinsTip->GetBestBlock()); + CBlockIndex* pindex = LookupBlockIndex(coins_cache.GetBestBlock()); if (!pindex) { return false; } @@ -3943,7 +3946,7 @@ bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview, } } // check level 3: check for inconsistencies during memory-only disconnect of tip blocks - if (nCheckLevel >= 3 && (coins.DynamicMemoryUsage() + pcoinsTip->DynamicMemoryUsage()) <= nCoinCacheUsage) { + if (nCheckLevel >= 3 && (coins.DynamicMemoryUsage() + ::ChainstateActive().CoinsTip().DynamicMemoryUsage()) <= nCoinCacheUsage) { assert(coins.GetBestBlock() == pindex->GetBlockHash()); DisconnectResult res = ::ChainstateActive().DisconnectBlock(block, pindex, coins); if (res == DISCONNECT_FAILED) { diff --git a/src/validation.h b/src/validation.h index d747fdbf2..6661369bf 100644 --- a/src/validation.h +++ b/src/validation.h @@ -505,6 +505,9 @@ public: CBlockIndex** ppindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); }; +/** Global variable that points to the active CCoinsView (protected by cs_main) */ +extern std::unique_ptr<CCoinsViewCache> pcoinsTip; + /** * CChainState stores and provides an API to update our local knowledge of the * current best chain. @@ -566,6 +569,12 @@ public: */ std::set<CBlockIndex*, CBlockIndexWorkComparator> setBlockIndexCandidates; + //! @returns A reference to the in-memory cache of the UTXO set. + CCoinsViewCache& CoinsTip() + { + return *::pcoinsTip; + } + /** * Update the on-disk chain state. * The caches and indexes are flushed depending on the mode we're called with @@ -662,9 +671,6 @@ BlockMap& BlockIndex(); /** Global variable that points to the coins database (protected by cs_main) */ extern std::unique_ptr<CCoinsViewDB> pcoinsdbview; -/** Global variable that points to the active CCoinsView (protected by cs_main) */ -extern std::unique_ptr<CCoinsViewCache> pcoinsTip; - /** Global variable that points to the active block tree (protected by cs_main) */ extern std::unique_ptr<CBlockTreeDB> pblocktree; diff --git a/test/functional/feature_dbcrash.py b/test/functional/feature_dbcrash.py index ff014de0e..b86f6af4c 100755 --- a/test/functional/feature_dbcrash.py +++ b/test/functional/feature_dbcrash.py @@ -58,7 +58,7 @@ class ChainstateWriteCrashTest(BitcoinTestFramework): self.base_args = ["-limitdescendantsize=0", "-maxmempool=0", "-rpcservertimeout=900", "-dbbatchsize=200000"] # Set different crash ratios and cache sizes. Note that not all of - # -dbcache goes to pcoinsTip. + # -dbcache goes to the in-memory coins cache. self.node0_args = ["-dbcrashratio=8", "-dbcache=4"] + self.base_args self.node1_args = ["-dbcrashratio=16", "-dbcache=8"] + self.base_args self.node2_args = ["-dbcrashratio=24", "-dbcache=16"] + self.base_args From 569353068568444a25b301bbd6513bb510157dc9 Mon Sep 17 00:00:00 2001 From: James O'Beirne <james.obeirne@gmail.com> Date: Wed, 24 Jul 2019 13:23:48 -0400 Subject: [PATCH 2/3] refactor: have CCoins* data managed under CChainState This change encapsulates UTXO set data within CChainState instances, removing global data `pcoinsTip` and `pcoinsviewdb`. This is necessary if we want to maintain multiple chainstates with their own rendering of the UTXO set. We introduce a class CoinsViews which consolidates the construction of a CCoins* hierarchy. Construction of its various pieces (db, coinscatcher, in-memory cache) is split up so that we avoid flushing bad state to disk if startup is interrupted. We also introduce `CChainState::CanFlushToDisk()` which tells us when it is safe to flush the chainstate based on this partial construction. This commit could be broken into smaller pieces, but it would require more ephemeral diffs to, e.g., temporarily change CCoinsViewDB's constructor invocations. Other changes: - A parameter has been added to the CCoinsViewDB constructor that allows the name of the corresponding leveldb directory to be specified. Thanks to Russell Yanofsky and Marco Falke for helpful feedback. --- src/init.cpp | 46 ++++++++++--------- src/rpc/blockchain.cpp | 4 +- src/test/setup_common.cpp | 11 +++-- src/txdb.cpp | 2 +- src/txdb.h | 5 ++- src/validation.cpp | 51 ++++++++++++++++++--- src/validation.h | 94 +++++++++++++++++++++++++++++++++++---- 7 files changed, 170 insertions(+), 43 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index bb3ff8d88..50ae0c625 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -15,7 +15,6 @@ #include <blockfilter.h> #include <chain.h> #include <chainparams.h> -#include <coins.h> #include <compat/sanity.h> #include <consensus/validation.h> #include <fs.h> @@ -149,7 +148,6 @@ NODISCARD static bool CreatePidFile() // shutdown thing. // -static std::unique_ptr<CCoinsViewErrorCatcher> pcoinscatcher; static std::unique_ptr<ECCVerifyHandle> globalVerifyHandle; static boost::thread_group threadGroup; @@ -234,8 +232,11 @@ void Shutdown(InitInterfaces& interfaces) } // FlushStateToDisk generates a ChainStateFlushed callback, which we should avoid missing - if (pcoinsTip != nullptr) { - ::ChainstateActive().ForceFlushStateToDisk(); + // + // g_chainstate is referenced here directly (instead of ::ChainstateActive()) because it + // may not have been initialized yet. + if (g_chainstate && g_chainstate->CanFlushToDisk()) { + g_chainstate->ForceFlushStateToDisk(); } // After there are no more peers/RPC left to give us new data which may generate @@ -250,12 +251,10 @@ void Shutdown(InitInterfaces& interfaces) { LOCK(cs_main); - if (pcoinsTip != nullptr) { - ::ChainstateActive().ForceFlushStateToDisk(); + if (g_chainstate && g_chainstate->CanFlushToDisk()) { + g_chainstate->ForceFlushStateToDisk(); + g_chainstate->ResetCoinsViews(); } - pcoinsTip.reset(); - pcoinscatcher.reset(); - pcoinsdbview.reset(); pblocktree.reset(); } for (const auto& client : interfaces.chain_clients) { @@ -1466,10 +1465,10 @@ bool AppInitMain(InitInterfaces& interfaces) bool is_coinsview_empty; try { LOCK(cs_main); + // This statement makes ::ChainstateActive() usable. + g_chainstate = MakeUnique<CChainState>(); UnloadBlockIndex(); - pcoinsTip.reset(); - pcoinsdbview.reset(); - pcoinscatcher.reset(); + // new CBlockTreeDB tries to delete the existing file, which // fails if it's still open from the previous loop. Close it first: pblocktree.reset(); @@ -1520,9 +1519,12 @@ bool AppInitMain(InitInterfaces& interfaces) // At this point we're either in reindex or we've loaded a useful // block tree into BlockIndex()! - pcoinsdbview.reset(new CCoinsViewDB(nCoinDBCache, false, fReset || fReindexChainState)); - pcoinscatcher.reset(new CCoinsViewErrorCatcher(pcoinsdbview.get())); - pcoinscatcher->AddReadErrCallback([]() { + ::ChainstateActive().InitCoinsDB( + /* cache_size_bytes */ nCoinDBCache, + /* in_memory */ false, + /* should_wipe */ fReset || fReindexChainState); + + ::ChainstateActive().CoinsErrorCatcher().AddReadErrCallback([]() { uiInterface.ThreadSafeMessageBox( _("Error reading from database, shutting down.").translated, "", CClientUIInterface::MSG_ERROR); @@ -1530,23 +1532,25 @@ bool AppInitMain(InitInterfaces& interfaces) // If necessary, upgrade from older database format. // This is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate - if (!pcoinsdbview->Upgrade()) { + if (!::ChainstateActive().CoinsDB().Upgrade()) { strLoadError = _("Error upgrading chainstate database").translated; break; } // ReplayBlocks is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate - if (!ReplayBlocks(chainparams, pcoinsdbview.get())) { + if (!ReplayBlocks(chainparams, &::ChainstateActive().CoinsDB())) { strLoadError = _("Unable to replay blocks. You will need to rebuild the database using -reindex-chainstate.").translated; break; } // The on-disk coinsdb is now in a good state, create the cache - pcoinsTip.reset(new CCoinsViewCache(pcoinscatcher.get())); + ::ChainstateActive().InitCoinsCache(); + assert(::ChainstateActive().CanFlushToDisk()); - is_coinsview_empty = fReset || fReindexChainState || pcoinsTip->GetBestBlock().IsNull(); + is_coinsview_empty = fReset || fReindexChainState || + ::ChainstateActive().CoinsTip().GetBestBlock().IsNull(); if (!is_coinsview_empty) { - // LoadChainTip sets ::ChainActive() based on pcoinsTip's best block + // LoadChainTip sets ::ChainActive() based on CoinsTip()'s best block if (!LoadChainTip(chainparams)) { strLoadError = _("Error initializing block database").translated; break; @@ -1588,7 +1592,7 @@ bool AppInitMain(InitInterfaces& interfaces) break; } - if (!CVerifyDB().VerifyDB(chainparams, pcoinsdbview.get(), gArgs.GetArg("-checklevel", DEFAULT_CHECKLEVEL), + if (!CVerifyDB().VerifyDB(chainparams, &::ChainstateActive().CoinsDB(), gArgs.GetArg("-checklevel", DEFAULT_CHECKLEVEL), gArgs.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS))) { strLoadError = _("Corrupted block database detected").translated; break; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index c35c478f3..8061939a2 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1062,7 +1062,7 @@ static UniValue gettxoutsetinfo(const JSONRPCRequest& request) CCoinsStats stats; ::ChainstateActive().ForceFlushStateToDisk(); - if (GetUTXOStats(pcoinsdbview.get(), stats)) { + if (GetUTXOStats(&::ChainstateActive().CoinsDB(), stats)) { ret.pushKV("height", (int64_t)stats.nHeight); ret.pushKV("bestblock", stats.hashBlock.GetHex()); ret.pushKV("transactions", (int64_t)stats.nTransactions); @@ -2206,7 +2206,7 @@ UniValue scantxoutset(const JSONRPCRequest& request) { LOCK(cs_main); ::ChainstateActive().ForceFlushStateToDisk(); - pcursor = std::unique_ptr<CCoinsViewCursor>(pcoinsdbview->Cursor()); + pcursor = std::unique_ptr<CCoinsViewCursor>(::ChainstateActive().CoinsDB().Cursor()); assert(pcursor); } bool res = FindScriptPubKey(g_scan_progress, g_should_abort_scan, count, pcursor.get(), needles, coins); diff --git a/src/test/setup_common.cpp b/src/test/setup_common.cpp index de877fd16..0ff081e47 100644 --- a/src/test/setup_common.cpp +++ b/src/test/setup_common.cpp @@ -85,8 +85,12 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha mempool.setSanityCheck(1.0); pblocktree.reset(new CBlockTreeDB(1 << 20, true)); - pcoinsdbview.reset(new CCoinsViewDB(1 << 23, true)); - pcoinsTip.reset(new CCoinsViewCache(pcoinsdbview.get())); + g_chainstate = MakeUnique<CChainState>(); + ::ChainstateActive().InitCoinsDB( + /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); + assert(!::ChainstateActive().CanFlushToDisk()); + ::ChainstateActive().InitCoinsCache(); + assert(::ChainstateActive().CanFlushToDisk()); if (!LoadGenesisBlock(chainparams)) { throw std::runtime_error("LoadGenesisBlock failed."); } @@ -113,8 +117,7 @@ TestingSetup::~TestingSetup() g_connman.reset(); g_banman.reset(); UnloadBlockIndex(); - pcoinsTip.reset(); - pcoinsdbview.reset(); + g_chainstate.reset(); pblocktree.reset(); } diff --git a/src/txdb.cpp b/src/txdb.cpp index df9851396..18be07e6d 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -52,7 +52,7 @@ struct CoinEntry { } -CCoinsViewDB::CCoinsViewDB(size_t nCacheSize, bool fMemory, bool fWipe) : db(GetDataDir() / "chainstate", nCacheSize, fMemory, fWipe, true) +CCoinsViewDB::CCoinsViewDB(fs::path ldb_path, size_t nCacheSize, bool fMemory, bool fWipe) : db(ldb_path, nCacheSize, fMemory, fWipe, true) { } diff --git a/src/txdb.h b/src/txdb.h index c4ece1150..140ce2c7f 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -48,7 +48,10 @@ class CCoinsViewDB final : public CCoinsView protected: CDBWrapper db; public: - explicit CCoinsViewDB(size_t nCacheSize, bool fMemory = false, bool fWipe = false); + /** + * @param[in] ldb_path Location in the filesystem where leveldb data will be stored. + */ + explicit CCoinsViewDB(fs::path ldb_path, size_t nCacheSize, bool fMemory, bool fWipe); bool GetCoin(const COutPoint &outpoint, Coin &coin) const override; bool HaveCoin(const COutPoint &outpoint) const override; diff --git a/src/validation.cpp b/src/validation.cpp index f9d38a309..0f24321aa 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -82,11 +82,17 @@ namespace { BlockManager g_blockman; } // anon namespace -static CChainState g_chainstate(g_blockman); +std::unique_ptr<CChainState> g_chainstate; -CChainState& ChainstateActive() { return g_chainstate; } +CChainState& ChainstateActive() { + assert(g_chainstate); + return *g_chainstate; +} -CChain& ChainActive() { return g_chainstate.m_chain; } +CChain& ChainActive() { + assert(g_chainstate); + return g_chainstate->m_chain; +} /** * Mutex to guard access to validation specific variables, such as reading @@ -173,8 +179,6 @@ CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& loc return chain.Genesis(); } -std::unique_ptr<CCoinsViewDB> pcoinsdbview; -std::unique_ptr<CCoinsViewCache> pcoinsTip; std::unique_ptr<CBlockTreeDB> pblocktree; // See definition for documentation @@ -525,7 +529,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool } // Note: this call may add txin.prevout to the coins cache - // (pcoinsTip.cacheCoins) by way of FetchCoin(). It should be removed + // (CoinsTip().cacheCoins) by way of FetchCoin(). It should be removed // later (via coins_to_uncache) if this tx turns out to be invalid. if (!view.HaveCoin(txin.prevout)) { // Are inputs missing because we already have the tx? @@ -1041,6 +1045,40 @@ CAmount GetBlockSubsidy(int nHeight, const Consensus::Params& consensusParams) return nSubsidy; } +CoinsViews::CoinsViews( + std::string ldb_name, + size_t cache_size_bytes, + bool in_memory, + bool should_wipe) : m_dbview( + GetDataDir() / ldb_name, cache_size_bytes, in_memory, should_wipe), + m_catcherview(&m_dbview) {} + +void CoinsViews::InitCache() +{ + m_cacheview = MakeUnique<CCoinsViewCache>(&m_catcherview); +} + +// NOTE: for now m_blockman is set to a global, but this will be changed +// in a future commit. +CChainState::CChainState() : m_blockman(g_blockman) {} + + +void CChainState::InitCoinsDB( + size_t cache_size_bytes, + bool in_memory, + bool should_wipe, + std::string leveldb_name) +{ + m_coins_views = MakeUnique<CoinsViews>( + leveldb_name, cache_size_bytes, in_memory, should_wipe); +} + +void CChainState::InitCoinsCache() +{ + assert(m_coins_views != nullptr); + m_coins_views->InitCache(); +} + // Note that though this is marked const, we may end up modifying `m_cached_finished_ibd`, which // is a performance-related implementation detail. This function must be marked // `const` so that `CValidationInterface` clients (which are given a `const CChainState*`) @@ -1982,6 +2020,7 @@ bool CChainState::FlushStateToDisk( { int64_t nMempoolUsage = mempool.DynamicMemoryUsage(); LOCK(cs_main); + assert(this->CanFlushToDisk()); static int64_t nLastWrite = 0; static int64_t nLastFlush = 0; std::set<int> setFilesToPrune; diff --git a/src/validation.h b/src/validation.h index 6661369bf..2a268d8ca 100644 --- a/src/validation.h +++ b/src/validation.h @@ -19,6 +19,7 @@ #include <script/script_error.h> #include <sync.h> #include <txmempool.h> // For CTxMemPool::cs +#include <txdb.h> #include <versionbits.h> #include <algorithm> @@ -37,7 +38,6 @@ class CBlockIndex; class CBlockTreeDB; class CBlockUndo; class CChainParams; -class CCoinsViewDB; class CInv; class CConnman; class CScriptCheck; @@ -505,8 +505,40 @@ public: CBlockIndex** ppindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); }; -/** Global variable that points to the active CCoinsView (protected by cs_main) */ -extern std::unique_ptr<CCoinsViewCache> pcoinsTip; +/** + * A convenience class for constructing the CCoinsView* hierarchy used + * to facilitate access to the UTXO set. + * + * This class consists of an arrangement of layered CCoinsView objects, + * preferring to store and retrieve coins in memory via `m_cacheview` but + * ultimately falling back on cache misses to the canonical store of UTXOs on + * disk, `m_dbview`. + */ +class CoinsViews { + +public: + //! The lowest level of the CoinsViews cache hierarchy sits in a leveldb database on disk. + //! All unspent coins reside in this store. + CCoinsViewDB m_dbview; + + //! This view wraps access to the leveldb instance and handles read errors gracefully. + CCoinsViewErrorCatcher m_catcherview; + + //! This is the top layer of the cache hierarchy - it keeps as many coins in memory as + //! can fit per the dbcache setting. + std::unique_ptr<CCoinsViewCache> m_cacheview; + + //! This constructor initializes CCoinsViewDB and CCoinsViewErrorCatcher instances, but it + //! *does not* create a CCoinsViewCache instance by default. This is done separately because the + //! presence of the cache has implications on whether or not we're allowed to flush the cache's + //! state to disk, which should not be done until the health of the database is verified. + //! + //! All arguments forwarded onto CCoinsViewDB. + CoinsViews(std::string ldb_name, size_t cache_size_bytes, bool in_memory, bool should_wipe); + + //! Initialize the CCoinsViewCache member. + void InitCache(); +}; /** * CChainState stores and provides an API to update our local knowledge of the @@ -556,12 +588,39 @@ private: //! easily as opposed to referencing a global. BlockManager& m_blockman; + //! Manages the UTXO set, which is a reflection of the contents of `m_chain`. + std::unique_ptr<CoinsViews> m_coins_views; + public: - CChainState(BlockManager& blockman) : m_blockman(blockman) { } + CChainState(BlockManager& blockman) : m_blockman(blockman) {} + CChainState(); + + /** + * Initialize the CoinsViews UTXO set database management data structures. The in-memory + * cache is initialized separately. + * + * All parameters forwarded to CoinsViews. + */ + void InitCoinsDB( + size_t cache_size_bytes, + bool in_memory, + bool should_wipe, + std::string leveldb_name = "chainstate"); + + //! Initialize the in-memory coins cache (to be done after the health of the on-disk database + //! is verified). + void InitCoinsCache(); + + //! @returns whether or not the CoinsViews object has been fully initialized and we can + //! safely flush this object to disk. + bool CanFlushToDisk() { + return m_coins_views && m_coins_views->m_cacheview; + } //! The current chain of blockheaders we consult and build on. //! @see CChain, CBlockIndex. CChain m_chain; + /** * The set of all CBlockIndex entries with BLOCK_VALID_TRANSACTIONS (for itself and all ancestors) and * as good as our current tip or better. Entries may be failed, though, and pruning nodes may be @@ -572,9 +631,26 @@ public: //! @returns A reference to the in-memory cache of the UTXO set. CCoinsViewCache& CoinsTip() { - return *::pcoinsTip; + assert(m_coins_views->m_cacheview); + return *m_coins_views->m_cacheview.get(); } + //! @returns A reference to the on-disk UTXO set database. + CCoinsViewDB& CoinsDB() + { + return m_coins_views->m_dbview; + } + + //! @returns A reference to a wrapped view of the in-memory UTXO set that + //! handles disk read errors gracefully. + CCoinsViewErrorCatcher& CoinsErrorCatcher() + { + return m_coins_views->m_catcherview; + } + + //! Destructs all objects related to accessing the UTXO set. + void ResetCoinsViews() { m_coins_views.reset(); } + /** * Update the on-disk chain state. * The caches and indexes are flushed depending on the mode we're called with @@ -606,7 +682,7 @@ public: bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pindex, CCoinsViewCache& view, const CChainParams& chainparams, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - // Block disconnection on our pcoinsTip: + // Apply the effects of a block disconnection on the UTXO set. bool DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); // Manual block validity manipulation: @@ -668,8 +744,10 @@ CChain& ChainActive(); /** @returns the global block index map. */ BlockMap& BlockIndex(); -/** Global variable that points to the coins database (protected by cs_main) */ -extern std::unique_ptr<CCoinsViewDB> pcoinsdbview; +// Most often ::ChainstateActive() should be used instead of this, but some code +// may not be able to assume that this has been initialized yet and so must use it +// directly, e.g. init.cpp. +extern std::unique_ptr<CChainState> g_chainstate; /** Global variable that points to the active block tree (protected by cs_main) */ extern std::unique_ptr<CBlockTreeDB> pblocktree; From 582d2cd74754d6b9a2394616a9c82a89d2d71976 Mon Sep 17 00:00:00 2001 From: James O'Beirne <james.obeirne@gmail.com> Date: Wed, 24 Jul 2019 13:39:42 -0400 Subject: [PATCH 3/3] Cover UTXO set access with lock annotations i.e. any CoinsViews members. Adds a lock acquisition to `gettxoutsetinfo` RPC to comply with added annotations. Co-authored-by: Russell Yanofsky <russ@yanofsky.org> --- src/init.cpp | 7 +++++-- src/rpc/blockchain.cpp | 4 +++- src/validation.cpp | 7 +++++-- src/validation.h | 18 +++++++++--------- 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 50ae0c625..97be4637a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -235,8 +235,11 @@ void Shutdown(InitInterfaces& interfaces) // // g_chainstate is referenced here directly (instead of ::ChainstateActive()) because it // may not have been initialized yet. - if (g_chainstate && g_chainstate->CanFlushToDisk()) { - g_chainstate->ForceFlushStateToDisk(); + { + LOCK(cs_main); + if (g_chainstate && g_chainstate->CanFlushToDisk()) { + g_chainstate->ForceFlushStateToDisk(); + } } // After there are no more peers/RPC left to give us new data which may generate diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 8061939a2..e3d5394a2 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1062,7 +1062,9 @@ static UniValue gettxoutsetinfo(const JSONRPCRequest& request) CCoinsStats stats; ::ChainstateActive().ForceFlushStateToDisk(); - if (GetUTXOStats(&::ChainstateActive().CoinsDB(), stats)) { + + CCoinsView* coins_view = WITH_LOCK(cs_main, return &ChainstateActive().CoinsDB()); + if (GetUTXOStats(coins_view, stats)) { ret.pushKV("height", (int64_t)stats.nHeight); ret.pushKV("bestblock", stats.hashBlock.GetHex()); ret.pushKV("transactions", (int64_t)stats.nTransactions); diff --git a/src/validation.cpp b/src/validation.cpp index 0f24321aa..8f44e2565 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -314,7 +314,8 @@ bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flag // Returns the script flags which should be checked for a given block static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consensus::Params& chainparams); -static void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age) EXCLUSIVE_LOCKS_REQUIRED(pool.cs) +static void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age) + EXCLUSIVE_LOCKS_REQUIRED(pool.cs, ::cs_main) { int expired = pool.Expire(GetTime() - age); if (expired != 0) { @@ -2160,7 +2161,9 @@ static void AppendWarning(std::string& res, const std::string& warn) } /** Check warning conditions and do some notifications on new chain tip set. */ -void static UpdateTip(const CBlockIndex *pindexNew, const CChainParams& chainParams) { +void static UpdateTip(const CBlockIndex* pindexNew, const CChainParams& chainParams) + EXCLUSIVE_LOCKS_REQUIRED(::cs_main) +{ // New best block mempool.AddTransactionsUpdated(1); diff --git a/src/validation.h b/src/validation.h index 2a268d8ca..65f86e307 100644 --- a/src/validation.h +++ b/src/validation.h @@ -519,14 +519,14 @@ class CoinsViews { public: //! The lowest level of the CoinsViews cache hierarchy sits in a leveldb database on disk. //! All unspent coins reside in this store. - CCoinsViewDB m_dbview; + CCoinsViewDB m_dbview GUARDED_BY(cs_main); //! This view wraps access to the leveldb instance and handles read errors gracefully. - CCoinsViewErrorCatcher m_catcherview; + CCoinsViewErrorCatcher m_catcherview GUARDED_BY(cs_main); //! This is the top layer of the cache hierarchy - it keeps as many coins in memory as //! can fit per the dbcache setting. - std::unique_ptr<CCoinsViewCache> m_cacheview; + std::unique_ptr<CCoinsViewCache> m_cacheview GUARDED_BY(cs_main); //! This constructor initializes CCoinsViewDB and CCoinsViewErrorCatcher instances, but it //! *does not* create a CCoinsViewCache instance by default. This is done separately because the @@ -537,7 +537,7 @@ public: CoinsViews(std::string ldb_name, size_t cache_size_bytes, bool in_memory, bool should_wipe); //! Initialize the CCoinsViewCache member. - void InitCache(); + void InitCache() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); }; /** @@ -609,11 +609,11 @@ public: //! Initialize the in-memory coins cache (to be done after the health of the on-disk database //! is verified). - void InitCoinsCache(); + void InitCoinsCache() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); //! @returns whether or not the CoinsViews object has been fully initialized and we can //! safely flush this object to disk. - bool CanFlushToDisk() { + bool CanFlushToDisk() EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return m_coins_views && m_coins_views->m_cacheview; } @@ -629,21 +629,21 @@ public: std::set<CBlockIndex*, CBlockIndexWorkComparator> setBlockIndexCandidates; //! @returns A reference to the in-memory cache of the UTXO set. - CCoinsViewCache& CoinsTip() + CCoinsViewCache& CoinsTip() EXCLUSIVE_LOCKS_REQUIRED(cs_main) { assert(m_coins_views->m_cacheview); return *m_coins_views->m_cacheview.get(); } //! @returns A reference to the on-disk UTXO set database. - CCoinsViewDB& CoinsDB() + CCoinsViewDB& CoinsDB() EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return m_coins_views->m_dbview; } //! @returns A reference to a wrapped view of the in-memory UTXO set that //! handles disk read errors gracefully. - CCoinsViewErrorCatcher& CoinsErrorCatcher() + CCoinsViewErrorCatcher& CoinsErrorCatcher() EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return m_coins_views->m_catcherview; }