diff --git a/src/bench/duplicate_inputs.cpp b/src/bench/duplicate_inputs.cpp index 6ce1552cee35..fd90908c4418 100644 --- a/src/bench/duplicate_inputs.cpp +++ b/src/bench/duplicate_inputs.cpp @@ -25,7 +25,7 @@ static void DuplicateInputs(benchmark::Bench& bench) CMutableTransaction coinbaseTx{}; CMutableTransaction naughtyTx{}; - CBlockIndex* pindexPrev = testing_setup->m_node.chainman->ActiveChain().Tip(); + CBlockIndex* pindexPrev = WITH_LOCK(testing_setup->m_node.chainman->GetMutex(), return testing_setup->m_node.chainman->ActiveChain().Tip()); assert(pindexPrev != nullptr); block.nBits = GetNextWorkRequired(pindexPrev, &block, chainparams.GetConsensus()); block.nNonce = 0; diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 271a6ed7ed5c..0c6ddf58d831 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -153,12 +153,14 @@ int main(int argc, char* argv[]) // Main program logic starts here std::cout << "Hello! I'm going to print out some information about your datadir." << std::endl - << "\t" << "Path: " << gArgs.GetDataDirNet() << std::endl + << "\t" << "Path: " << gArgs.GetDataDirNet() << std::endl; + { + LOCK(chainman.GetMutex()); + std::cout << "\t" << "Reindexing: " << std::boolalpha << node::fReindex.load() << std::noboolalpha << std::endl << "\t" << "Snapshot Active: " << std::boolalpha << chainman.IsSnapshotActive() << std::noboolalpha << std::endl << "\t" << "Active Height: " << chainman.ActiveHeight() << std::endl << "\t" << "Active IBD: " << std::boolalpha << chainman.ActiveChainstate().IsInitialBlockDownload() << std::noboolalpha << std::endl; - { CBlockIndex* tip = chainman.ActiveTip(); if (tip) { std::cout << "\t" << tip->ToString() << std::endl; diff --git a/src/chainlock/clsig.cpp b/src/chainlock/clsig.cpp index 980be33a3750..6ccdcaae443c 100644 --- a/src/chainlock/clsig.cpp +++ b/src/chainlock/clsig.cpp @@ -27,4 +27,13 @@ llmq::VerifyRecSigStatus VerifyChainLock(const Consensus::Params& params, const return llmq::VerifyRecoveredSig(llmqType, chain, qman, clsig.getHeight(), request_id, clsig.getBlockHash(), clsig.getSig()); } + +llmq::VerifyRecSigStatus VerifyChainLock(const Consensus::Params& params, const llmq::CQuorumManager& qman, + const chainlock::ChainLockSig& clsig, const CBlockIndex* pindexStart) +{ + const auto llmqType = params.llmqTypeChainLocks; + const uint256 request_id = GenSigRequestId(clsig.getHeight()); + + return llmq::VerifyRecoveredSig(llmqType, qman, pindexStart, request_id, clsig.getBlockHash(), clsig.getSig()); +} } // namespace chainlock diff --git a/src/chainlock/clsig.h b/src/chainlock/clsig.h index 7ff6c7fee67a..5c46c2732a46 100644 --- a/src/chainlock/clsig.h +++ b/src/chainlock/clsig.h @@ -8,6 +8,7 @@ #include class CChain; +class CBlockIndex; class uint256; namespace Consensus { @@ -27,6 +28,8 @@ uint256 GenSigRequestId(const int32_t nHeight); llmq::VerifyRecSigStatus VerifyChainLock(const Consensus::Params& params, const CChain& chain, const llmq::CQuorumManager& qman, const ChainLockSig& clsig); +llmq::VerifyRecSigStatus VerifyChainLock(const Consensus::Params& params, const llmq::CQuorumManager& qman, + const ChainLockSig& clsig, const CBlockIndex* pindexStart); } // namespace chainlock #endif // BITCOIN_CHAINLOCK_CLSIG_H diff --git a/src/chainlock/handler.cpp b/src/chainlock/handler.cpp index b62a92b3d39c..3c2153cb3558 100644 --- a/src/chainlock/handler.cpp +++ b/src/chainlock/handler.cpp @@ -102,7 +102,9 @@ MessageProcessingResult ChainlockHandler::ProcessNewChainLock(const NodeId from, } } - if (const auto ret = chainlock::VerifyChainLock(Params().GetConsensus(), m_chainman.ActiveChain(), qman, clsig); + const CBlockIndex* pindex_start = WITH_LOCK(::cs_main, + return llmq::SelectQuorumForSigningStartBlock(m_chainman.ActiveChain(), clsig.getHeight())); + if (const auto ret = chainlock::VerifyChainLock(Params().GetConsensus(), qman, clsig, pindex_start); ret != llmq::VerifyRecSigStatus::Valid) { LogPrint(BCLog::CHAINLOCKS, "ChainlockHandler::%s -- invalid CLSIG (%s), status=%d peer=%d\n", __func__, clsig.ToString(), std23::to_underlying(ret), from); diff --git a/src/governance/signing.cpp b/src/governance/signing.cpp index 3a3968bec224..e18d42ec9876 100644 --- a/src/governance/signing.cpp +++ b/src/governance/signing.cpp @@ -60,7 +60,7 @@ std::optional GovernanceSigner::CreateSuperblockCandidate(int CSuperblock::GetNearestSuperblocksHeights(nHeight, nLastSuperblock, nNextSuperblock); auto SBEpochTime = static_cast(GetTime().count() + (nNextSuperblock - nHeight) * 2.62 * 60); - auto governanceBudget = CSuperblock::GetPaymentsLimit(m_chainman.ActiveChain(), nNextSuperblock); + auto governanceBudget = WITH_LOCK(::cs_main, return CSuperblock::GetPaymentsLimit(m_chainman.ActiveChain(), nNextSuperblock)); CAmount budgetAllocated{}; for (const auto& proposal : approvedProposals) { diff --git a/src/init.cpp b/src/init.cpp index 3884ed3fd7f4..32df5abaef2c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -913,7 +913,7 @@ static void PeriodicStats(NodeContext& node) LogPrintf("%s: GetUTXOStats failed\n", __func__); } - CBlockIndex *tip = chainman.ActiveChain().Tip(); + CBlockIndex *tip = WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); double nNetworkHashPS = [&]() { // Short version of GetNetworkHashPS(120, -1); CBlockIndex *pindex = tip; @@ -2298,7 +2298,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) RegisterValidationInterface(node.cj_walletman.get()); } - bool fLoadCacheFiles = !(fReindex || fReindexChainState) && (chainman.ActiveChain().Tip() != nullptr); + bool fLoadCacheFiles = !(fReindex || fReindexChainState) && WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip() != nullptr); if (!node.netfulfilledman->LoadCache(fLoadCacheFiles)) { auto file_path = fs::PathToString(gArgs.GetDataDirNet() / "netfulfilled.dat"); @@ -2470,7 +2470,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // Either install a handler to notify us when genesis activates, or set fHaveGenesis directly. // No locking, as this happens before any background thread is started. boost::signals2::connection block_notify_genesis_wait_connection; - if (chainman.ActiveChain().Tip() == nullptr) { + if (WITH_LOCK(chainman.GetMutex(), return chainman.ActiveChain().Tip() == nullptr)) { block_notify_genesis_wait_connection = uiInterface.NotifyBlockTip_connect(std::bind(BlockNotifyGenesisWait, std::placeholders::_2)); } else { fHaveGenesis = true; @@ -2528,7 +2528,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // Seed InstantSend tip-height cache; NetInstantSend receives future // updates via CValidationInterface but misses InitializeCurrentBlockTip. // TODO: move cache updates from NetInstantSend to g_ds_notification due to specific of Tip's processing - node.llmq_ctx->isman->CacheTipHeight(chainman.ActiveChain().Tip()); + node.llmq_ctx->isman->CacheTipHeight(WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip())); { // Get all UTXOs for each MN collateral in one go so that we can fill coin cache early @@ -2592,7 +2592,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } if (node.active_ctx) { - node.active_ctx->nodeman->Init(chainman.ActiveTip()); + node.active_ctx->nodeman->Init(WITH_LOCK(::cs_main, return chainman.ActiveTip())); // Now that nodeman->Init has set proTxHash, fan out the // startup tip to all CValidationInterface subscribers. // The earlier call only kicked CDSNotificationInterface @@ -2804,12 +2804,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // At this point, the RPC is "started", but still in warmup, which means it // cannot yet be called. Before we make it callable, we need to make sure // that the RPC's view of the best block is valid and consistent with - // ChainstateManager's ActiveTip. + // ChainstateManager's active tip. // // If we do not do this, RPC's view of the best block will be height=0 and // hash=0x0. This will lead to erroroneous responses for things like // waitforblockheight. - RPCNotifyBlockChange(chainman.ActiveTip()); + RPCNotifyBlockChange(WITH_LOCK(chainman.GetMutex(), return chainman.ActiveTip())); SetRPCWarmupFinished(); uiInterface.InitMessage(_("Done loading").translated); diff --git a/src/llmq/ehf_signals.cpp b/src/llmq/ehf_signals.cpp index c9f89b6bc131..be878235393c 100644 --- a/src/llmq/ehf_signals.cpp +++ b/src/llmq/ehf_signals.cpp @@ -67,7 +67,8 @@ void CEHFSignalsHandler::trySignEHFSignal(int bit, const CBlockIndex* const pind return; } - const auto quorum = llmq::SelectQuorumForSigning(llmq_params_opt.value(), m_chainman.ActiveChain(), qman, requestId); + const CChain& active_chain = *WITH_LOCK(::cs_main, return &m_chainman.ActiveChain()); + const auto quorum = llmq::SelectQuorumForSigning(llmq_params_opt.value(), active_chain, qman, requestId); if (!quorum) { LogPrintf("CEHFSignalsHandler::trySignEHFSignal no quorum for id=%s\n", requestId.ToString()); return; diff --git a/src/llmq/quorumsman.cpp b/src/llmq/quorumsman.cpp index 0e2c17969a51..7b64829e71cc 100644 --- a/src/llmq/quorumsman.cpp +++ b/src/llmq/quorumsman.cpp @@ -518,23 +518,26 @@ void CQuorumManager::MigrateOldQuorumDB(CEvoDB& evoDb) const LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- done\n", __func__); } -CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, const CChain& active_chain, const CQuorumManager& qman, - const uint256& selectionHash, int signHeight, int signOffset) +CBlockIndex* SelectQuorumForSigningStartBlock(const CChain& active_chain, int signHeight, int signOffset) { - size_t poolSize = llmq_params.signingActiveQuorumCount; + AssertLockHeld(::cs_main); + if (signHeight == -1) { + signHeight = active_chain.Height(); + } + const int startBlockHeight = signHeight - signOffset; + if (startBlockHeight > active_chain.Height() || startBlockHeight < 0) { + return nullptr; + } + return active_chain[startBlockHeight]; +} - CBlockIndex* pindexStart; - { - LOCK(::cs_main); - if (signHeight == -1) { - signHeight = active_chain.Height(); - } - int startBlockHeight = signHeight - signOffset; - if (startBlockHeight > active_chain.Height() || startBlockHeight < 0) { - return {}; - } - pindexStart = active_chain[startBlockHeight]; +CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, const CQuorumManager& qman, + const uint256& selectionHash, const CBlockIndex* pindexStart) +{ + if (pindexStart == nullptr) { + return nullptr; } + size_t poolSize = llmq_params.signingActiveQuorumCount; // don't remove connections for the currently in-progress DKG round if (IsQuorumRotationEnabled(llmq_params, pindexStart)) { @@ -581,13 +584,20 @@ CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, con } } -VerifyRecSigStatus VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain& active_chain, const CQuorumManager& qman, - int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig, - const int signOffset) +CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, const CChain& active_chain, const CQuorumManager& qman, + const uint256& selectionHash, int signHeight, int signOffset) +{ + const CBlockIndex* pindexStart = WITH_LOCK(::cs_main, return SelectQuorumForSigningStartBlock(active_chain, signHeight, signOffset)); + return SelectQuorumForSigning(llmq_params, qman, selectionHash, pindexStart); +} + +VerifyRecSigStatus VerifyRecoveredSig(Consensus::LLMQType llmqType, const CQuorumManager& qman, + const CBlockIndex* pindexStart, const uint256& id, const uint256& msgHash, + const CBLSSignature& sig) { const auto& llmq_params_opt = Params().GetLLMQ(llmqType); assert(llmq_params_opt.has_value()); - auto quorum = SelectQuorumForSigning(llmq_params_opt.value(), active_chain, qman, id, signedAtHeight, signOffset); + auto quorum = SelectQuorumForSigning(llmq_params_opt.value(), qman, id, pindexStart); if (!quorum) { return VerifyRecSigStatus::NoQuorum; } @@ -597,4 +607,12 @@ VerifyRecSigStatus VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain return ret ? VerifyRecSigStatus::Valid : VerifyRecSigStatus::Invalid; } +VerifyRecSigStatus VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain& active_chain, const CQuorumManager& qman, + int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig, + const int signOffset) +{ + const CBlockIndex* pindexStart = WITH_LOCK(::cs_main, return SelectQuorumForSigningStartBlock(active_chain, signedAtHeight, signOffset)); + return VerifyRecoveredSig(llmqType, qman, pindexStart, id, msgHash, sig); +} + } // namespace llmq diff --git a/src/llmq/quorumsman.h b/src/llmq/quorumsman.h index c814af452454..1a35b1cf0ab0 100644 --- a/src/llmq/quorumsman.h +++ b/src/llmq/quorumsman.h @@ -31,6 +31,7 @@ class CDeterministicMNManager; class CDBWrapper; class CEvoDB; class ChainstateManager; +extern RecursiveMutex cs_main; // NOLINT(readability-redundant-declaration) namespace util { struct DbWrapperParams; } // namespace util @@ -175,9 +176,19 @@ class CQuorumManager final // which are not 100% at the chain tip. static constexpr int SIGN_HEIGHT_OFFSET{8}; +CBlockIndex* SelectQuorumForSigningStartBlock(const CChain& active_chain, int signHeight = -1 /*chain tip*/, + int signOffset = SIGN_HEIGHT_OFFSET) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + +CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, const CQuorumManager& qman, + const uint256& selectionHash, const CBlockIndex* pindexStart); + CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, const CChain& active_chain, const CQuorumManager& qman, const uint256& selectionHash, int signHeight = -1 /*chain tip*/, int signOffset = SIGN_HEIGHT_OFFSET); +VerifyRecSigStatus VerifyRecoveredSig(Consensus::LLMQType llmqType, const CQuorumManager& qman, + const CBlockIndex* pindexStart, const uint256& id, const uint256& msgHash, + const CBLSSignature& sig); + // Verifies a recovered sig that was signed while the chain tip was at signedAtTip VerifyRecSigStatus VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain& active_chain, const CQuorumManager& qman, int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig, diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 4849e2940ee9..f5b23b0e1a76 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -744,7 +744,8 @@ bool CSigSharesManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigning // the quorum list and no recovered signature has been created in the mean time const auto& llmq_params_opt = Params().GetLLMQ(llmqType); assert(llmq_params_opt.has_value()); - return SelectQuorumForSigning(llmq_params_opt.value(), m_chainman.ActiveChain(), qman, id); + CChain& active_chain = *WITH_LOCK(::cs_main, return &m_chainman.ActiveChain()); + return SelectQuorumForSigning(llmq_params_opt.value(), active_chain, qman, id); } else { return qman.GetQuorum(llmqType, quorumHash); } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 2aabdc199172..8f7eddc64b69 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3322,7 +3322,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, const std::string msg_type = uses_compressed ? NetMsgType::GETHEADERS2 : NetMsgType::GETHEADERS; if (nCount == GetHeadersLimit(pfrom, uses_compressed)) { // Headers message had its maximum size; the peer may have more headers. - if (MaybeSendGetHeaders(pfrom, msg_type, m_chainman.ActiveChain().GetLocator(pindexLast), peer)) { + if (MaybeSendGetHeaders(pfrom, msg_type, WITH_LOCK(m_chainman.GetMutex(), return m_chainman.ActiveChain().GetLocator(pindexLast)), peer)) { LogPrint(BCLog::NET, "more %s (%d) to end to peer=%d (startheight:%d)\n", msg_type, pindexLast->nHeight, pfrom.GetId(), peer.m_starting_height); } @@ -4707,7 +4707,7 @@ void PeerManagerImpl::ProcessMessage( pindex = m_chainman.ActiveChain().Next(pindex); } - const auto send_headers = [this /* for m_connman */, &hashStop, &pindex, &nodestate, &pfrom, &msgMaker](auto msg_type_internal, auto& v_headers, auto callback) { + const auto send_headers = [this /* for m_connman */, &hashStop, &pindex, &nodestate, &pfrom, &msgMaker](auto msg_type_internal, auto& v_headers, auto callback) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { int nLimit = GetHeadersLimit(pfrom, msg_type_internal == NetMsgType::HEADERS2); for (; pindex; pindex = m_chainman.ActiveChain().Next(pindex)) { v_headers.emplace_back(callback(pindex)); diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index ec7888668773..49fd4fb537c8 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -400,6 +400,23 @@ bool BlockManager::LoadBlockIndexDB() return true; } +void BlockManager::ScanAndUnlinkAlreadyPrunedFiles() +{ + AssertLockHeld(::cs_main); + if (!m_have_pruned) { + return; + } + + std::set block_files_to_prune; + for (int file_number = 0; file_number < m_last_blockfile; file_number++) { + if (m_blockfile_info[file_number].nSize == 0) { + block_files_to_prune.insert(file_number); + } + } + + UnlinkPrunedFiles(block_files_to_prune); +} + const CBlockIndex* BlockManager::GetLastCheckpoint(const CCheckpointData& data) { const MapCheckpoints& checkpoints = data.mapCheckpoints; @@ -575,11 +592,14 @@ uint64_t BlockManager::CalculateCurrentUsage() void UnlinkPrunedFiles(const std::set& setFilesToPrune) { + std::error_code ec; for (std::set::iterator it = setFilesToPrune.begin(); it != setFilesToPrune.end(); ++it) { FlatFilePos pos(*it, 0); - fs::remove(BlockFileSeq().FileName(pos)); - fs::remove(UndoFileSeq().FileName(pos)); - LogPrint(BCLog::BLOCKSTORE, "Prune: %s deleted blk/rev (%05u)\n", __func__, *it); + const bool removed_blockfile{fs::remove(BlockFileSeq().FileName(pos), ec)}; + const bool removed_undofile{fs::remove(UndoFileSeq().FileName(pos), ec)}; + if (removed_blockfile || removed_undofile) { + LogPrint(BCLog::BLOCKSTORE, "Prune: %s deleted blk/rev (%05u)\n", __func__, *it); + } } } diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index c05a7fc70f03..cb5473aaabb8 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -169,6 +169,13 @@ class BlockManager bool WriteBlockIndexDB() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); bool LoadBlockIndexDB() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + /** + * Remove any pruned block & undo files that are still on disk. + * This could happen on some systems if the file was still being read while unlinked, + * or if we crash before unlinking. + */ + void ScanAndUnlinkAlreadyPrunedFiles() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + CBlockIndex* AddToBlockIndex(const CBlockHeader& block, const uint256& hash, CBlockIndex*& best_header, enum BlockStatus nStatus = BLOCK_VALID_TREE) EXCLUSIVE_LOCKS_REQUIRED(cs_main); diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 08587e77a973..138f8959e759 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -324,7 +324,7 @@ class GOVImpl : public GOV std::optional getProposalFundedHeight(const uint256& proposal_hash) override { if (context().chain_helper != nullptr && context().chainman != nullptr) { - const int32_t nTipHeight = context().chainman->ActiveHeight(); + const int32_t nTipHeight = WITH_LOCK(::cs_main, return context().chainman->ActiveHeight()); for (const auto& trigger : context().chain_helper->superblocks->GetActiveTriggers()) { if (!trigger || trigger->GetBlockHeight() > nTipHeight) continue; for (const auto& hash : trigger->GetProposalHashes()) { @@ -343,8 +343,12 @@ class GOVImpl : public GOV const auto tip_mn_list{context().dmnman->GetListAtChainTip()}; if (const auto proposals{context().govman->GetApprovedProposals(tip_mn_list)}; !proposals.empty()) { int32_t last_sb{0}, next_sb{0}; - CSuperblock::GetNearestSuperblocksHeights(context().chainman->ActiveHeight(), last_sb, next_sb); - const CAmount budget{CSuperblock::GetPaymentsLimit(context().chainman->ActiveChain(), next_sb)}; + CAmount budget{0}; + { + LOCK(::cs_main); + CSuperblock::GetNearestSuperblocksHeights(context().chainman->ActiveHeight(), last_sb, next_sb); + budget = CSuperblock::GetPaymentsLimit(context().chainman->ActiveChain(), next_sb); + } for (const auto& proposal : proposals) { UniValue json = proposal->GetJSONObject(); CAmount payment_amount{0}; diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 327d7eacc07b..f051563a8532 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -138,14 +138,14 @@ void TestGUI(interfaces::Node& node) if (!wallet->AddWalletDescriptor(w_desc, provider, "", false)) assert(false); CTxDestination dest = PKHash(test.coinbaseKey.GetPubKey()); wallet->SetAddressBook(dest, "", "receive"); - wallet->SetLastBlockProcessed(105, node.context()->chainman->ActiveChain().Tip()->GetBlockHash()); + wallet->SetLastBlockProcessed(105, WITH_LOCK(node.context()->chainman->GetMutex(), return node.context()->chainman->ActiveChain().Tip()->GetBlockHash())); } { WalletRescanReserver reserver(*wallet); reserver.reserve(); CWallet::ScanResult result = wallet->ScanForWalletTransactions(Params().GetConsensus().hashGenesisBlock, /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/true, /*save_progress=*/false); QCOMPARE(result.status, CWallet::ScanResult::SUCCESS); - QCOMPARE(result.last_scanned_block, node.context()->chainman->ActiveChain().Tip()->GetBlockHash()); + QCOMPARE(result.last_scanned_block, WITH_LOCK(node.context()->chainman->GetMutex(), return node.context()->chainman->ActiveChain().Tip()->GetBlockHash())); QVERIFY(result.last_failed_block.IsNull()); } wallet->SetBroadcastTransactions(true); diff --git a/src/rest.cpp b/src/rest.cpp index 87f5671cf000..ef0768b5e407 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -821,14 +821,18 @@ static bool rest_getutxos(const CoreContext& context, HTTPRequest* req, const st ChainstateManager* maybe_chainman = GetChainman(context, req); if (!maybe_chainman) return false; ChainstateManager& chainman = *maybe_chainman; + decltype(chainman.ActiveHeight()) active_height; + uint256 active_hash; { - auto process_utxos = [&vOutPoints, &outs, &hits](const CCoinsView& view, const CTxMemPool* mempool) { + auto process_utxos = [&vOutPoints, &outs, &hits, &active_height, &active_hash, &chainman](const CCoinsView& view, const CTxMemPool* mempool ) EXCLUSIVE_LOCKS_REQUIRED(chainman.GetMutex()) { for (const COutPoint& vOutPoint : vOutPoints) { Coin coin; bool hit = (!mempool || !mempool->isSpent(vOutPoint)) && view.GetCoin(vOutPoint, coin); hits.push_back(hit); if (hit) outs.emplace_back(std::move(coin)); } + active_height = chainman.ActiveHeight(); + active_hash = chainman.ActiveTip()->GetBlockHash(); }; if (fCheckMemPool) { @@ -856,7 +860,7 @@ static bool rest_getutxos(const CoreContext& context, HTTPRequest* req, const st // serialize data // use exact same output as mentioned in Bip64 CDataStream ssGetUTXOResponse(SER_NETWORK, PROTOCOL_VERSION); - ssGetUTXOResponse << chainman.ActiveChain().Height() << chainman.ActiveChain().Tip()->GetBlockHash() << bitmap << outs; + ssGetUTXOResponse << active_height << active_hash << bitmap << outs; std::string ssGetUTXOResponseString = ssGetUTXOResponse.str(); req->WriteHeader("Content-Type", "application/octet-stream"); @@ -866,7 +870,7 @@ static bool rest_getutxos(const CoreContext& context, HTTPRequest* req, const st case RESTResponseFormat::HEX: { CDataStream ssGetUTXOResponse(SER_NETWORK, PROTOCOL_VERSION); - ssGetUTXOResponse << chainman.ActiveChain().Height() << chainman.ActiveChain().Tip()->GetBlockHash() << bitmap << outs; + ssGetUTXOResponse << active_height << active_hash << bitmap << outs; std::string strHex = HexStr(ssGetUTXOResponse) + "\n"; req->WriteHeader("Content-Type", "text/plain"); @@ -879,8 +883,8 @@ static bool rest_getutxos(const CoreContext& context, HTTPRequest* req, const st // pack in some essentials // use more or less the same output as mentioned in Bip64 - objGetUTXOResponse.pushKV("chainHeight", chainman.ActiveChain().Height()); - objGetUTXOResponse.pushKV("chaintipHash", chainman.ActiveChain().Tip()->GetBlockHash().GetHex()); + objGetUTXOResponse.pushKV("chainHeight", active_height); + objGetUTXOResponse.pushKV("chaintipHash", active_hash.GetHex()); objGetUTXOResponse.pushKV("bitmap", bitmapStringRepresentation); UniValue utxos(UniValue::VARR); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 700daad3a328..64f4ca393385 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -520,7 +520,7 @@ static RPCHelpMan getblockfrompeer() // Fetching blocks before the node has syncing past their height can prevent block files from // being pruned, so we avoid it if the node is in prune mode. - if (index->nHeight > chainman.ActiveChain().Tip()->nHeight && node::fPruneMode) { + if (index->nHeight > WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()->nHeight) && node::fPruneMode) { throw JSONRPCError(RPC_MISC_ERROR, "In prune mode, only blocks that the node has already synced previously can be fetched from a peer"); } @@ -1910,13 +1910,12 @@ static RPCHelpMan getchaintxstats() { ChainstateManager& chainman = EnsureAnyChainman(request.context); - CChain& active_chain = chainman.ActiveChain(); const CBlockIndex* pindex; int blockcount = 30 * 24 * 60 * 60 / chainman.GetParams().GetConsensus().nPowTargetSpacing; // By default: 1 month if (request.params[1].isNull()) { LOCK(cs_main); - pindex = active_chain.Tip(); + pindex = chainman.ActiveChain().Tip(); } else { uint256 hash(ParseHashV(request.params[1], "blockhash")); LOCK(cs_main); @@ -1924,7 +1923,7 @@ static RPCHelpMan getchaintxstats() if (!pindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); } - if (!active_chain.Contains(pindex)) { + if (!chainman.ActiveChain().Contains(pindex)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Block is not in main chain"); } } diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index 401f78af8042..23a600fe1370 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -985,7 +985,7 @@ static RPCHelpMan getgovernanceinfo() obj.pushKV("lastsuperblock", nLastSuperblock); obj.pushKV("nextsuperblock", nNextSuperblock); obj.pushKV("fundingthreshold", CHECK_NONFATAL(node.dmnman)->GetListAtChainTip().GetCounts().m_valid_weighted / 10); - obj.pushKV("governancebudget", ValueFromAmount(CSuperblock::GetPaymentsLimit(chainman.ActiveChain(), nNextSuperblock))); + obj.pushKV("governancebudget", ValueFromAmount(WITH_LOCK(::cs_main, return CSuperblock::GetPaymentsLimit(chainman.ActiveChain(), nNextSuperblock)))); return obj; }, @@ -1014,7 +1014,7 @@ static RPCHelpMan getsuperblockbudget() } const ChainstateManager& chainman = EnsureAnyChainman(request.context); - return ValueFromAmount(CSuperblock::GetPaymentsLimit(chainman.ActiveChain(), nBlockHeight)); + return ValueFromAmount(WITH_LOCK(::cs_main, return CSuperblock::GetPaymentsLimit(chainman.ActiveChain(), nBlockHeight))); }, }; } diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index 931c775ed9c0..7483db54afcc 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -534,7 +534,8 @@ static UniValue quorum_sign_helper(const JSONRPCRequest& request, Consensus::LLM } else { const auto pQuorum = [&]() { if (quorumHash.IsNull()) { - return llmq::SelectQuorumForSigning(llmq_params_opt.value(), chainman.ActiveChain(), *llmq_ctx.qman, id); + const CChain& active_chain = *WITH_LOCK(::cs_main, return &chainman.ActiveChain()); + return llmq::SelectQuorumForSigning(llmq_params_opt.value(), active_chain, *llmq_ctx.qman, id); } else { return llmq_ctx.qman->GetQuorum(llmqType, quorumHash); } @@ -686,7 +687,8 @@ static RPCHelpMan quorum_verify() if (!request.params[5].isNull()) { signHeight = request.params[5].getInt(); } - return VerifyRecoveredSigLatestQuorums(*llmq_params_opt, chainman.ActiveChain(), *llmq_ctx.qman, signHeight, id, msgHash, sig); + const CChain& active_chain = *WITH_LOCK(::cs_main, return &chainman.ActiveChain()); + return VerifyRecoveredSigLatestQuorums(*llmq_params_opt, active_chain, *llmq_ctx.qman, signHeight, id, msgHash, sig); } uint256 quorumHash(ParseHashV(request.params[4], "quorumHash")); @@ -830,7 +832,8 @@ static RPCHelpMan quorum_selectquorum() UniValue ret(UniValue::VOBJ); - const auto quorum = llmq::SelectQuorumForSigning(llmq_params_opt.value(), chainman.ActiveChain(), *llmq_ctx.qman, id); + const CChain& active_chain = *WITH_LOCK(::cs_main, return &chainman.ActiveChain()); + const auto quorum = llmq::SelectQuorumForSigning(llmq_params_opt.value(), active_chain, *llmq_ctx.qman, id); if (!quorum) { throw JSONRPCError(RPC_MISC_ERROR, "no quorums active"); } @@ -1116,7 +1119,8 @@ static RPCHelpMan verifychainlock() } const LLMQContext& llmq_ctx = EnsureLLMQContext(node); - return chainlock::VerifyChainLock(Params().GetConsensus(), chainman.ActiveChain(), *CHECK_NONFATAL(llmq_ctx.qman), + const CChain& active_chain = *WITH_LOCK(::cs_main, return &chainman.ActiveChain()); + return chainlock::VerifyChainLock(Params().GetConsensus(), active_chain, *CHECK_NONFATAL(llmq_ctx.qman), chainlock::ChainLockSig{nBlockHeight, nBlockHash, sig}) == llmq::VerifyRecSigStatus::Valid; }, @@ -1192,7 +1196,8 @@ static RPCHelpMan verifyislock() auto llmqType = Params().GetConsensus().llmqTypeDIP0024InstantSend; const auto llmq_params_opt = Params().GetLLMQ(llmqType); CHECK_NONFATAL(llmq_params_opt.has_value()); - return VerifyRecoveredSigLatestQuorums(*llmq_params_opt, chainman.ActiveChain(), *CHECK_NONFATAL(llmq_ctx.qman), + const CChain& active_chain = *WITH_LOCK(::cs_main, return &chainman.ActiveChain()); + return VerifyRecoveredSigLatestQuorums(*llmq_params_opt, active_chain, *CHECK_NONFATAL(llmq_ctx.qman), signHeight, id, txid, sig); }, }; @@ -1231,8 +1236,9 @@ static RPCHelpMan submitchainlock() const ChainstateManager& chainman = EnsureChainman(node); const auto clsig{chainlock::ChainLockSig(nBlockHeight, nBlockHash, sig)}; + const CChain& active_chain = *WITH_LOCK(::cs_main, return &chainman.ActiveChain()); const llmq::VerifyRecSigStatus ret{ - chainlock::VerifyChainLock(Params().GetConsensus(), chainman.ActiveChain(), *llmq_ctx.qman, clsig)}; + chainlock::VerifyChainLock(Params().GetConsensus(), active_chain, *llmq_ctx.qman, clsig)}; if (ret == llmq::VerifyRecSigStatus::NoQuorum) { LOCK(cs_main); const CBlockIndex* pIndex{chainman.ActiveChain().Tip()}; diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 3efd753d2ac0..9c2cbaad0e5b 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -762,10 +762,16 @@ static RPCHelpMan getassetunlockstatuses() if (!request.params[1].isNull()) { nSpecificCoreHeight = request.params[1].getInt(); - if (nSpecificCoreHeight.value() < 0 || nSpecificCoreHeight.value() > chainman.ActiveChain().Height()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Block height out of range"); + const CBlockIndex* pBlockIndex{nullptr}; + { + LOCK(::cs_main); + if (nSpecificCoreHeight.value() < 0 || nSpecificCoreHeight.value() > chainman.ActiveChain().Height()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Block height out of range"); + } + pBlockIndex = chainman.ActiveChain()[nSpecificCoreHeight.value()]; } - poolCL = std::make_optional(chain_helper.GetCreditPool(chainman.ActiveChain()[nSpecificCoreHeight.value()])); + CHECK_NONFATAL(pBlockIndex); + poolCL = std::make_optional(chain_helper.GetCreditPool(pBlockIndex)); } else { const auto pBlockIndexBestCL = [&]() -> const CBlockIndex* { diff --git a/src/test/block_reward_reallocation_tests.cpp b/src/test/block_reward_reallocation_tests.cpp index fc4a655bdb81..128e7651592c 100644 --- a/src/test/block_reward_reallocation_tests.cpp +++ b/src/test/block_reward_reallocation_tests.cpp @@ -159,7 +159,7 @@ BOOST_FIXTURE_TEST_CASE(block_reward_reallocation, TestChainBRRBeforeActivationS CKey ownerKey; CBLSSecretKey operatorKey; auto utxos = BuildSimpleUtxoMap(m_coinbase_txns); - auto tx = CreateProRegTx(m_node.chainman->ActiveChain(), *m_node.mempool, utxos, 1, GenerateRandomAddress(), coinbaseKey, ownerKey, operatorKey); + auto tx = WITH_LOCK(::cs_main, return CreateProRegTx(m_node.chainman->ActiveChain(), *m_node.mempool, utxos, 1, GenerateRandomAddress(), coinbaseKey, ownerKey, operatorKey)); CreateAndProcessBlock({tx}, coinbasePubKey); @@ -191,7 +191,7 @@ BOOST_FIXTURE_TEST_CASE(block_reward_reallocation, TestChainBRRBeforeActivationS LOCK(cs_main); dmnman.UpdatedBlockTip(m_node.chainman->ActiveChain().Tip()); } - BOOST_CHECK(m_node.chainman->ActiveChain().Height() < Params().GetConsensus().BRRHeight); + BOOST_CHECK(WITH_LOCK(::cs_main, return m_node.chainman->ActiveChain().Height()) < Params().GetConsensus().BRRHeight); CreateAndProcessBlock({}, coinbasePubKey); { @@ -252,7 +252,7 @@ BOOST_FIXTURE_TEST_CASE(block_reward_reallocation, TestChainBRRBeforeActivationS BOOST_CHECK_EQUAL(pblocktemplate->voutMasternodePayments[0].nValue, masternode_payment); } } - BOOST_CHECK(DeploymentActiveAfter(m_node.chainman->ActiveChain().Tip(), consensus_params, Consensus::DEPLOYMENT_V20)); + BOOST_CHECK(WITH_LOCK(::cs_main, return DeploymentActiveAfter(m_node.chainman->ActiveChain().Tip(), consensus_params, Consensus::DEPLOYMENT_V20))); // Allocation of block subsidy is 60% MN, 20% miners and 20% treasury { // Reward split should reach ~75/25 after reallocation is done @@ -273,7 +273,7 @@ BOOST_FIXTURE_TEST_CASE(block_reward_reallocation, TestChainBRRBeforeActivationS BOOST_CHECK_EQUAL(pblocktemplate->voutMasternodePayments[0].nValue, masternode_payment); BOOST_CHECK_EQUAL(pblocktemplate->voutMasternodePayments[0].nValue, 106300596); // 0.75 } - BOOST_CHECK(!DeploymentActiveAfter(m_node.chainman->ActiveChain().Tip(), consensus_params, Consensus::DEPLOYMENT_MN_RR)); + BOOST_CHECK(WITH_LOCK(::cs_main, return !DeploymentActiveAfter(m_node.chainman->ActiveChain().Tip(), consensus_params, Consensus::DEPLOYMENT_MN_RR))); // Reward split should stay ~75/25 after reallocation is done, // check 10 next superblocks @@ -299,7 +299,7 @@ BOOST_FIXTURE_TEST_CASE(block_reward_reallocation, TestChainBRRBeforeActivationS BOOST_CHECK_EQUAL(pblocktemplate->voutMasternodePayments[payment_index].nValue, masternode_payment); } - BOOST_CHECK(DeploymentActiveAfter(m_node.chainman->ActiveChain().Tip(), consensus_params, Consensus::DEPLOYMENT_MN_RR)); + BOOST_CHECK(WITH_LOCK(::cs_main, return DeploymentActiveAfter(m_node.chainman->ActiveChain().Tip(), consensus_params, Consensus::DEPLOYMENT_MN_RR))); { // At this moment Masternode reward should be reallocated to platform // Allocation of block subsidy is 60% MN, 20% miners and 20% treasury LOCK(cs_main); diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index 6b29b76e2184..da5fee7c1876 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -12,6 +13,8 @@ using node::BlockManager; using node::BLOCK_SERIALIZATION_HEADER_SIZE; +using node::MAX_BLOCKFILE_SIZE; +using node::OpenBlockFile; // use BasicTestingSetup here for the data directory configuration, setup, and cleanup BOOST_FIXTURE_TEST_SUITE(blockmanager_tests, BasicTestingSetup) @@ -42,4 +45,45 @@ BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) BOOST_CHECK_EQUAL(actual.nPos, BLOCK_SERIALIZATION_HEADER_SIZE + ::GetSerializeSize(params->GenesisBlock(), CLIENT_VERSION) + BLOCK_SERIALIZATION_HEADER_SIZE); } +BOOST_FIXTURE_TEST_CASE(blockmanager_scan_unlink_already_pruned_files, TestChain100Setup) +{ + // Cap last block file size, and mine new block in a new block file. + auto& chainman{*Assert(m_node.chainman)}; + auto& blockman{chainman.m_blockman}; + const CBlockIndex* old_tip{WITH_LOCK(chainman.GetMutex(), return chainman.ActiveChain().Tip())}; + WITH_LOCK(chainman.GetMutex(), blockman.GetBlockFileInfo(old_tip->GetBlockPos().nFile)->nSize = MAX_BLOCKFILE_SIZE); + CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); + + // Prune the older block file, but don't unlink it + int file_number; + { + LOCK(chainman.GetMutex()); + file_number = old_tip->GetBlockPos().nFile; + blockman.PruneOneBlockFile(file_number); + } + + const FlatFilePos pos(file_number, 0); + + // Check that the file is not unlinked after ScanAndUnlinkAlreadyPrunedFiles + // if m_have_pruned is not yet set + WITH_LOCK(chainman.GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles()); + BOOST_CHECK(!CAutoFile(OpenBlockFile(pos, true), SER_DISK, CLIENT_VERSION).IsNull()); + + // Check that the file is unlinked after ScanAndUnlinkAlreadyPrunedFiles + // once m_have_pruned is set + blockman.m_have_pruned = true; + WITH_LOCK(chainman.GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles()); + BOOST_CHECK(CAutoFile(OpenBlockFile(pos, true), SER_DISK, CLIENT_VERSION).IsNull()); + + // Check that calling with already pruned files doesn't cause an error + WITH_LOCK(chainman.GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles()); + + // Check that the new tip file has not been removed + const CBlockIndex* new_tip{WITH_LOCK(chainman.GetMutex(), return chainman.ActiveChain().Tip())}; + BOOST_CHECK_NE(old_tip, new_tip); + const int new_file_number{WITH_LOCK(chainman.GetMutex(), return new_tip->GetBlockPos().nFile)}; + const FlatFilePos new_pos(new_file_number, 0); + BOOST_CHECK(!CAutoFile(OpenBlockFile(new_pos, true), SER_DISK, CLIENT_VERSION).IsNull()); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/bls_tests.cpp b/src/test/bls_tests.cpp index ffc6c05082b1..4ebfeb931586 100644 --- a/src/test/bls_tests.cpp +++ b/src/test/bls_tests.cpp @@ -659,8 +659,8 @@ BOOST_AUTO_TEST_CASE(v19_boundary_validation_failure_restores_bls_scheme) auto& chainman = *Assert(setup.m_node.chainman.get()); const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); - BOOST_REQUIRE(!DeploymentActiveAt(*chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); - BOOST_REQUIRE(DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); + BOOST_REQUIRE(WITH_LOCK(::cs_main, return !DeploymentActiveAt(*chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19))); + BOOST_REQUIRE(WITH_LOCK(::cs_main, return DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19))); struct ScopedBLSLegacySchemeRestore { explicit ScopedBLSLegacySchemeRestore(bool saved_scheme) : m_saved_scheme(saved_scheme) {} ~ScopedBLSLegacySchemeRestore() { bls::bls_legacy_scheme.store(m_saved_scheme); } @@ -686,9 +686,9 @@ BOOST_AUTO_TEST_CASE(v19_boundary_validation_failure_restores_bls_scheme) BOOST_CHECK(bls::bls_legacy_scheme.load()); CBlock connect_block = setup.CreateBlock({bad_tx}, coinbase_pk, chainman.ActiveChainstate()); - const int height_before_invalid_block{chainman.ActiveChain().Height()}; + const int height_before_invalid_block{WITH_LOCK(::cs_main, return chainman.ActiveChain().Height())}; (void)chainman.ProcessNewBlock(std::make_shared(connect_block), /*force_processing=*/true, /*new_block=*/nullptr); - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), height_before_invalid_block); + BOOST_CHECK_EQUAL(WITH_LOCK(::cs_main, return chainman.ActiveChain().Height()), height_before_invalid_block); BOOST_CHECK(bls::bls_legacy_scheme.load()); } diff --git a/src/test/evo_cbtx_tests.cpp b/src/test/evo_cbtx_tests.cpp index 363c3c01a27a..f7f33c823bfd 100644 --- a/src/test/evo_cbtx_tests.cpp +++ b/src/test/evo_cbtx_tests.cpp @@ -34,7 +34,7 @@ BOOST_AUTO_TEST_SUITE(evo_cbtx_tests) BOOST_FIXTURE_TEST_CASE(check_cbtx_best_chainlock_rejects_excessive_height_diff, RegTestingSetup) { const auto& consensus_params = Params().GetConsensus(); - const auto& chain = m_node.chainman->ActiveChain(); + const auto& chain = *WITH_LOCK(::cs_main, return &m_node.chainman->ActiveChain()); auto& qman = *Assert(m_node.llmq_ctx)->qman; auto& chainlocks = *Assert(m_node.chainlocks); diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index fa821d8a5674..4241faa7fa8f 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -258,33 +258,37 @@ void FuncDIP3Activation(TestChainSetup& setup) { auto& chainman = *Assert(setup.m_node.chainman.get()); auto& dmnman = *Assert(setup.m_node.dmnman); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + auto tip_height = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Height()); }; + auto tip_hash = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()->GetBlockHash()); }; + auto sync_dmn_tip = [&] { dmnman.UpdatedBlockTip(tip_index()); }; auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); CKey ownerKey; CBLSSecretKey operatorKey; CTxDestination payoutDest = DecodeDestination("yRq1Ky1AfFmf597rnotj7QRxsDUKePVWNF"); - auto tx = CreateProRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, 1, GetScriptForDestination(payoutDest), setup.coinbaseKey, ownerKey, operatorKey); + auto tx = WITH_LOCK(::cs_main, return CreateProRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, 1, GetScriptForDestination(payoutDest), setup.coinbaseKey, ownerKey, operatorKey)); std::vector txns = {tx}; const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); - int nHeight = chainman.ActiveChain().Height(); + int nHeight = tip_height(); // We start one block before DIP3 activation, so mining a block with a DIP3 transaction should fail auto block = std::make_shared(setup.CreateBlock(txns, coinbase_pk, chainman.ActiveChainstate())); chainman.ProcessNewBlock(block, true, nullptr); - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight); - BOOST_REQUIRE(block->GetHash() != chainman.ActiveChain().Tip()->GetBlockHash()); + BOOST_CHECK_EQUAL(tip_height(), nHeight); + BOOST_REQUIRE(block->GetHash() != tip_hash()); BOOST_REQUIRE(!dmnman.GetListAtChainTip().HasMN(tx.GetHash())); // This block should activate DIP3 setup.CreateAndProcessBlock({}, coinbase_pk); - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight + 1); + BOOST_CHECK_EQUAL(tip_height(), nHeight + 1); // Mining a block with a DIP3 transaction should succeed now block = std::make_shared(setup.CreateBlock(txns, coinbase_pk, chainman.ActiveChainstate())); BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight + 2); - BOOST_CHECK_EQUAL(block->GetHash(), chainman.ActiveChain().Tip()->GetBlockHash()); + sync_dmn_tip(); + BOOST_CHECK_EQUAL(tip_height(), nHeight + 2); + BOOST_CHECK_EQUAL(block->GetHash(), tip_hash()); BOOST_REQUIRE(dmnman.GetListAtChainTip().HasMN(tx.GetHash())); }; @@ -292,8 +296,11 @@ void FuncV19Activation(TestChainSetup& setup) { auto& chainman = *Assert(setup.m_node.chainman.get()); auto& dmnman = *Assert(setup.m_node.dmnman); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + auto tip_height = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Height()); }; + auto sync_dmn_tip = [&] { dmnman.UpdatedBlockTip(tip_index()); }; - BOOST_REQUIRE(!DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); + BOOST_REQUIRE(!DeploymentActiveAfter(tip_index(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); // create auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); @@ -302,36 +309,36 @@ void FuncV19Activation(TestChainSetup& setup) CKey collateral_key; collateral_key.MakeNewKey(false); auto collateralScript = GetScriptForDestination(PKHash(collateral_key.GetPubKey())); - auto tx_reg = CreateProRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, 1, collateralScript, setup.coinbaseKey, owner_key, operator_key); + auto tx_reg = WITH_LOCK(::cs_main, return CreateProRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, 1, collateralScript, setup.coinbaseKey, owner_key, operator_key)); auto tx_reg_hash = tx_reg.GetHash(); const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); - int nHeight = chainman.ActiveChain().Height(); + int nHeight = tip_height(); auto block = std::make_shared(setup.CreateBlock({tx_reg}, coinbase_pk, chainman.ActiveChainstate())); BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); - BOOST_REQUIRE(!DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); + BOOST_REQUIRE(!DeploymentActiveAfter(tip_index(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); ++nHeight; - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + BOOST_CHECK_EQUAL(tip_height(), nHeight); + sync_dmn_tip(); dmnman.DoMaintenance(); auto tip_list = dmnman.GetListAtChainTip(); BOOST_REQUIRE(tip_list.HasMN(tx_reg_hash)); - auto pindex_create = chainman.ActiveChain().Tip(); + auto pindex_create = tip_index(); auto base_list = dmnman.GetListForBlock(pindex_create); std::vector diffs; // update CBLSSecretKey operator_key_new; operator_key_new.MakeNewKey(); - auto tx_upreg = CreateProUpRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, tx_reg_hash, owner_key, operator_key_new.GetPublicKey(), owner_key.GetPubKey().GetID(), collateralScript, setup.coinbaseKey); + auto tx_upreg = WITH_LOCK(::cs_main, return CreateProUpRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, tx_reg_hash, owner_key, operator_key_new.GetPublicKey(), owner_key.GetPubKey().GetID(), collateralScript, setup.coinbaseKey)); block = std::make_shared(setup.CreateBlock({tx_upreg}, coinbase_pk, chainman.ActiveChainstate())); BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); - BOOST_REQUIRE(!DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); + BOOST_REQUIRE(!DeploymentActiveAfter(tip_index(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); ++nHeight; - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + BOOST_CHECK_EQUAL(tip_height(), nHeight); + sync_dmn_tip(); dmnman.DoMaintenance(); tip_list = dmnman.GetListAtChainTip(); BOOST_REQUIRE(tip_list.HasMN(tx_reg_hash)); @@ -348,10 +355,10 @@ void FuncV19Activation(TestChainSetup& setup) BOOST_REQUIRE(SignSignature(signing_provider, CTransaction(tx_reg), tx_spend, 0, SIGHASH_ALL)); block = std::make_shared(setup.CreateBlock({tx_spend}, coinbase_pk, chainman.ActiveChainstate())); BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); - BOOST_REQUIRE(!DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); + BOOST_REQUIRE(!DeploymentActiveAfter(tip_index(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); ++nHeight; - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + BOOST_CHECK_EQUAL(tip_height(), nHeight); + sync_dmn_tip(); dmnman.DoMaintenance(); diffs.push_back(tip_list.BuildDiff(dmnman.GetListAtChainTip())); tip_list = dmnman.GetListAtChainTip(); @@ -360,10 +367,10 @@ void FuncV19Activation(TestChainSetup& setup) // mine another block so that it's not the last one before V19 setup.CreateAndProcessBlock({}, coinbase_pk); - BOOST_REQUIRE(!DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); + BOOST_REQUIRE(!DeploymentActiveAfter(tip_index(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); ++nHeight; - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + BOOST_CHECK_EQUAL(tip_height(), nHeight); + sync_dmn_tip(); dmnman.DoMaintenance(); diffs.push_back(tip_list.BuildDiff(dmnman.GetListAtChainTip())); tip_list = dmnman.GetListAtChainTip(); @@ -372,10 +379,10 @@ void FuncV19Activation(TestChainSetup& setup) // this block should activate V19 setup.CreateAndProcessBlock({}, coinbase_pk); - BOOST_REQUIRE(DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); + BOOST_REQUIRE(DeploymentActiveAfter(tip_index(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); ++nHeight; - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + BOOST_CHECK_EQUAL(tip_height(), nHeight); + sync_dmn_tip(); dmnman.DoMaintenance(); diffs.push_back(tip_list.BuildDiff(dmnman.GetListAtChainTip())); tip_list = dmnman.GetListAtChainTip(); @@ -385,7 +392,7 @@ void FuncV19Activation(TestChainSetup& setup) // check mn list/diff CDeterministicMNListDiff dummy_diff = base_list.BuildDiff(tip_list); CDeterministicMNList dummy_list{base_list}; - dummy_list.ApplyDiff(chainman.ActiveChain().Tip(), dummy_diff); + dummy_list.ApplyDiff(tip_index(), dummy_diff); // Lists should match BOOST_REQUIRE(dummy_list == tip_list); @@ -394,9 +401,9 @@ void FuncV19Activation(TestChainSetup& setup) { setup.CreateAndProcessBlock({}, coinbase_pk); BOOST_REQUIRE( - DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight + 1 + i); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + DeploymentActiveAfter(tip_index(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); + BOOST_CHECK_EQUAL(tip_height(), nHeight + 1 + i); + sync_dmn_tip(); dmnman.DoMaintenance(); diffs.push_back(tip_list.BuildDiff(dmnman.GetListAtChainTip())); tip_list = dmnman.GetListAtChainTip(); @@ -405,23 +412,23 @@ void FuncV19Activation(TestChainSetup& setup) } // check mn list/diff - const CBlockIndex* v19_index = chainman.ActiveChain().Tip()->GetAncestor(Params().GetConsensus().V19Height); + const CBlockIndex* v19_index = WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()->GetAncestor(Params().GetConsensus().V19Height)); auto v19_list = dmnman.GetListForBlock(v19_index); dummy_diff = v19_list.BuildDiff(tip_list); dummy_list = v19_list; - dummy_list.ApplyDiff(chainman.ActiveChain().Tip(), dummy_diff); + dummy_list.ApplyDiff(tip_index(), dummy_diff); BOOST_REQUIRE(dummy_list == tip_list); // NOTE: this fails on v19/v19.1 with errors like: // "RemoveMN: Can't delete a masternode ... with a pubKeyOperator=..." dummy_diff = base_list.BuildDiff(tip_list); dummy_list = base_list; - dummy_list.ApplyDiff(chainman.ActiveChain().Tip(), dummy_diff); + dummy_list.ApplyDiff(tip_index(), dummy_diff); BOOST_REQUIRE(dummy_list == tip_list); dummy_list = base_list; for (const auto& diff : diffs) { - dummy_list.ApplyDiff(chainman.ActiveChain().Tip(), diff); + dummy_list.ApplyDiff(tip_index(), diff); } BOOST_REQUIRE(dummy_list == tip_list); }; @@ -430,41 +437,43 @@ void FuncProUpRegTxVersionHandlingBeforeV24(TestChainSetup& setup) { auto& chainman = *Assert(setup.m_node.chainman.get()); auto& dmnman = *Assert(setup.m_node.dmnman); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + auto sync_dmn_tip = [&] { dmnman.UpdatedBlockTip(tip_index()); }; const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); - BOOST_REQUIRE(!DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); + BOOST_REQUIRE(!DeploymentActiveAfter(tip_index(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); BOOST_REQUIRE(bls::bls_legacy_scheme.load()); auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); CKey owner_key; CBLSSecretKey operator_key; - auto tx_reg = CreateProRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, 1, GenerateRandomAddress(), - setup.coinbaseKey, owner_key, operator_key); + auto tx_reg = WITH_LOCK(::cs_main, return CreateProRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, 1, GenerateRandomAddress(), + setup.coinbaseKey, owner_key, operator_key)); const auto proTxHash = tx_reg.GetHash(); setup.CreateAndProcessBlock({tx_reg}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); auto dmn = dmnman.GetListAtChainTip().GetMN(proTxHash); BOOST_REQUIRE(dmn); BOOST_CHECK_EQUAL(dmn->pdmnState->nVersion, ProTxVersion::LegacyBLS); - while (!DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)) { + while (!DeploymentActiveAfter(tip_index(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)) { setup.CreateAndProcessBlock({}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); } - BOOST_REQUIRE(!DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman, Consensus::DEPLOYMENT_V24)); + BOOST_REQUIRE(!DeploymentActiveAfter(tip_index(), chainman, Consensus::DEPLOYMENT_V24)); BOOST_REQUIRE(!bls::bls_legacy_scheme.load()); const auto payoutScript = GenerateRandomAddress(); - auto tx_upreg = CreateProUpRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, proTxHash, owner_key, + auto tx_upreg = WITH_LOCK(::cs_main, return CreateProUpRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, proTxHash, owner_key, operator_key.GetPublicKey(), owner_key.GetPubKey().GetID(), payoutScript, - setup.coinbaseKey); + setup.coinbaseKey)); const auto opt_upreg = GetTxPayload(tx_upreg); BOOST_REQUIRE(opt_upreg); BOOST_CHECK_EQUAL(opt_upreg->nVersion, ProTxVersion::BasicBLS); setup.CreateAndProcessBlock({tx_upreg}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); dmn = dmnman.GetListAtChainTip().GetMN(proTxHash); BOOST_REQUIRE(dmn); @@ -475,11 +484,11 @@ void FuncProUpRegTxVersionHandlingBeforeV24(TestChainSetup& setup) CBLSSecretKey operator_key_new; operator_key_new.MakeNewKey(); const auto payoutScript2 = GenerateRandomAddress(); - auto tx_upreg2 = CreateProUpRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, proTxHash, owner_key, + auto tx_upreg2 = WITH_LOCK(::cs_main, return CreateProUpRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, proTxHash, owner_key, operator_key_new.GetPublicKey(), owner_key.GetPubKey().GetID(), payoutScript2, - setup.coinbaseKey); + setup.coinbaseKey)); setup.CreateAndProcessBlock({tx_upreg2}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); dmn = dmnman.GetListAtChainTip().GetMN(proTxHash); BOOST_REQUIRE(dmn); @@ -501,15 +510,15 @@ void FuncProUpRegTxVersionHandlingBeforeV24(TestChainSetup& setup) CMutableTransaction tx_upreg3; tx_upreg3.nVersion = 3; tx_upreg3.nType = TRANSACTION_PROVIDER_UPDATE_REGISTRAR; - FundTransaction(chainman.ActiveChain(), tx_upreg3, utxos, GetScriptForDestination(PKHash(setup.coinbaseKey.GetPubKey())), - 1 * COIN, setup.coinbaseKey); + WITH_LOCK(::cs_main, FundTransaction(chainman.ActiveChain(), tx_upreg3, utxos, GetScriptForDestination(PKHash(setup.coinbaseKey.GetPubKey())), + 1 * COIN, setup.coinbaseKey)); proTxLegacy.inputsHash = CalcTxInputsHash(CTransaction(tx_upreg3)); CHashSigner::SignHash(::SerializeHash(proTxLegacy), owner_key, proTxLegacy.vchSig); SetTxPayload(tx_upreg3, proTxLegacy); SignTransaction(*(setup.m_node.mempool), tx_upreg3, setup.coinbaseKey); setup.CreateAndProcessBlock({tx_upreg3}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); dmn = dmnman.GetListAtChainTip().GetMN(proTxHash); BOOST_REQUIRE(dmn); @@ -527,30 +536,32 @@ void FuncProUpRegTxV4OnLegacyRejected(TestChainSetup& setup) { auto& chainman = *Assert(setup.m_node.chainman.get()); auto& dmnman = *Assert(setup.m_node.dmnman); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + auto sync_dmn_tip = [&] { dmnman.UpdatedBlockTip(tip_index()); }; const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); - BOOST_REQUIRE(!DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); - BOOST_REQUIRE(!DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman, Consensus::DEPLOYMENT_V24)); + BOOST_REQUIRE(!DeploymentActiveAfter(tip_index(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); + BOOST_REQUIRE(!DeploymentActiveAfter(tip_index(), chainman, Consensus::DEPLOYMENT_V24)); auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); CKey owner_key; CBLSSecretKey operator_key; - auto tx_reg = CreateProRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, 1, GenerateRandomAddress(), - setup.coinbaseKey, owner_key, operator_key); + auto tx_reg = WITH_LOCK(::cs_main, return CreateProRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, 1, GenerateRandomAddress(), + setup.coinbaseKey, owner_key, operator_key)); const auto proTxHash = tx_reg.GetHash(); setup.CreateAndProcessBlock({tx_reg}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); auto dmn = dmnman.GetListAtChainTip().GetMN(proTxHash); BOOST_REQUIRE(dmn); BOOST_CHECK_EQUAL(dmn->pdmnState->nVersion, ProTxVersion::LegacyBLS); - for (int i = 0; i < 2000 && !DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman, Consensus::DEPLOYMENT_V24); ++i) { + for (int i = 0; i < 2000 && !DeploymentActiveAfter(tip_index(), chainman, Consensus::DEPLOYMENT_V24); ++i) { setup.CreateAndProcessBlock({}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); } - BOOST_REQUIRE(DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); - BOOST_REQUIRE(DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman, Consensus::DEPLOYMENT_V24)); + BOOST_REQUIRE(DeploymentActiveAfter(tip_index(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); + BOOST_REQUIRE(DeploymentActiveAfter(tip_index(), chainman, Consensus::DEPLOYMENT_V24)); BOOST_REQUIRE(!bls::bls_legacy_scheme.load()); BOOST_REQUIRE_EQUAL(dmnman.GetListAtChainTip().GetMN(proTxHash)->pdmnState->nVersion, ProTxVersion::LegacyBLS); @@ -564,8 +575,8 @@ void FuncProUpRegTxV4OnLegacyRejected(TestChainSetup& setup) CMutableTransaction tx; tx.nVersion = 3; tx.nType = TRANSACTION_PROVIDER_UPDATE_REGISTRAR; - FundTransaction(chainman.ActiveChain(), tx, utxos, GetScriptForDestination(PKHash(setup.coinbaseKey.GetPubKey())), - 1 * COIN, setup.coinbaseKey); + WITH_LOCK(::cs_main, FundTransaction(chainman.ActiveChain(), tx, utxos, GetScriptForDestination(PKHash(setup.coinbaseKey.GetPubKey())), + 1 * COIN, setup.coinbaseKey)); proTx.inputsHash = CalcTxInputsHash(CTransaction(tx)); CHashSigner::SignHash(::SerializeHash(proTx), owner_key, proTx.vchSig); SetTxPayload(tx, proTx); @@ -584,14 +595,16 @@ void FuncProUpRegTxV2CannotBypassV4PayoutCollateralReuse(TestChainSetup& setup) { auto& chainman = *Assert(setup.m_node.chainman.get()); auto& dmnman = *Assert(setup.m_node.dmnman); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + auto sync_dmn_tip = [&] { dmnman.UpdatedBlockTip(tip_index()); }; const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); - for (int i = 0; i < 2000 && !DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman, Consensus::DEPLOYMENT_V24); ++i) { + for (int i = 0; i < 2000 && !DeploymentActiveAfter(tip_index(), chainman, Consensus::DEPLOYMENT_V24); ++i) { setup.CreateAndProcessBlock({}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); } - BOOST_REQUIRE(DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); - BOOST_REQUIRE(DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman, Consensus::DEPLOYMENT_V24)); + BOOST_REQUIRE(DeploymentActiveAfter(tip_index(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); + BOOST_REQUIRE(DeploymentActiveAfter(tip_index(), chainman, Consensus::DEPLOYMENT_V24)); BOOST_REQUIRE(!bls::bls_legacy_scheme.load()); auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); @@ -606,11 +619,11 @@ void FuncProUpRegTxV2CannotBypassV4PayoutCollateralReuse(TestChainSetup& setup) const auto script_payout = GenerateRandomAddress(); CMutableTransaction tx_collateral; - FundTransaction(chainman.ActiveChain(), tx_collateral, utxos, script_collateral, dmn_types::Regular.collat_amount, - setup.coinbaseKey); + WITH_LOCK(::cs_main, FundTransaction(chainman.ActiveChain(), tx_collateral, utxos, script_collateral, dmn_types::Regular.collat_amount, + setup.coinbaseKey)); SignTransaction(*(setup.m_node.mempool), tx_collateral, setup.coinbaseKey); setup.CreateAndProcessBlock({tx_collateral}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); CProRegTx pro_reg; pro_reg.nVersion = ProTxVersion::MultiPayout; @@ -630,15 +643,15 @@ void FuncProUpRegTxV2CannotBypassV4PayoutCollateralReuse(TestChainSetup& setup) CMutableTransaction tx_reg; tx_reg.nVersion = 3; tx_reg.nType = TRANSACTION_PROVIDER_REGISTER; - FundTransaction(chainman.ActiveChain(), tx_reg, utxos, GetScriptForDestination(PKHash(setup.coinbaseKey.GetPubKey())), - 1 * COIN, setup.coinbaseKey); + WITH_LOCK(::cs_main, FundTransaction(chainman.ActiveChain(), tx_reg, utxos, GetScriptForDestination(PKHash(setup.coinbaseKey.GetPubKey())), + 1 * COIN, setup.coinbaseKey)); pro_reg.inputsHash = CalcTxInputsHash(CTransaction(tx_reg)); CMessageSigner::SignMessage(pro_reg.MakeSignString(), pro_reg.vchSig, collateral_key); SetTxPayload(tx_reg, pro_reg); SignTransaction(*(setup.m_node.mempool), tx_reg, setup.coinbaseKey); const auto proTxHash = tx_reg.GetHash(); setup.CreateAndProcessBlock({tx_reg}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); auto dmn = dmnman.GetListAtChainTip().GetMN(proTxHash); BOOST_REQUIRE(dmn); @@ -656,8 +669,8 @@ void FuncProUpRegTxV2CannotBypassV4PayoutCollateralReuse(TestChainSetup& setup) CMutableTransaction tx_upreg; tx_upreg.nVersion = 3; tx_upreg.nType = TRANSACTION_PROVIDER_UPDATE_REGISTRAR; - FundTransaction(chainman.ActiveChain(), tx_upreg, utxos, GetScriptForDestination(PKHash(setup.coinbaseKey.GetPubKey())), - 1 * COIN, setup.coinbaseKey); + WITH_LOCK(::cs_main, FundTransaction(chainman.ActiveChain(), tx_upreg, utxos, GetScriptForDestination(PKHash(setup.coinbaseKey.GetPubKey())), + 1 * COIN, setup.coinbaseKey)); pro_upreg.inputsHash = CalcTxInputsHash(CTransaction(tx_upreg)); CHashSigner::SignHash(::SerializeHash(pro_upreg), owner_key, pro_upreg.vchSig); SetTxPayload(tx_upreg, pro_upreg); @@ -690,13 +703,16 @@ void FuncMNPaymentMultiplicityV24Boundary(TestChainSetup& setup) { auto& chainman = *Assert(setup.m_node.chainman.get()); auto& dmnman = *Assert(setup.m_node.dmnman); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + auto tip_hash = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()->GetBlockHash()); }; + auto sync_dmn_tip = [&] { dmnman.UpdatedBlockTip(tip_index()); }; const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); const auto& consensus = chainman.GetConsensus(); // Start in the pre-v24 window: v19 active (basic BLS scheme) so we can register a v2 MN, but // v24 -- and thus strict multiplicity matching -- not yet active. - BOOST_REQUIRE(DeploymentActiveAfter(chainman.ActiveChain().Tip(), consensus, Consensus::DEPLOYMENT_V19)); - BOOST_REQUIRE(!DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman, Consensus::DEPLOYMENT_V24)); + BOOST_REQUIRE(DeploymentActiveAfter(tip_index(), consensus, Consensus::DEPLOYMENT_V19)); + BOOST_REQUIRE(!DeploymentActiveAfter(tip_index(), chainman, Consensus::DEPLOYMENT_V24)); BOOST_REQUIRE(!bls::bls_legacy_scheme.load()); auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); @@ -716,11 +732,11 @@ void FuncMNPaymentMultiplicityV24Boundary(TestChainSetup& setup) // Fund the collateral. CMutableTransaction tx_collateral; - FundTransaction(chainman.ActiveChain(), tx_collateral, utxos, script_collateral, dmn_types::Regular.collat_amount, - setup.coinbaseKey); + WITH_LOCK(::cs_main, FundTransaction(chainman.ActiveChain(), tx_collateral, utxos, script_collateral, dmn_types::Regular.collat_amount, + setup.coinbaseKey)); SignTransaction(*(setup.m_node.mempool), tx_collateral, setup.coinbaseKey); setup.CreateAndProcessBlock({tx_collateral}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); // Register a plain v2 (BasicBLS) MN -- deliberately NOT multi-payout, and the max version // eligible while v24 is pending -- with a single legacy owner payout -> shared_script and a @@ -746,15 +762,15 @@ void FuncMNPaymentMultiplicityV24Boundary(TestChainSetup& setup) CMutableTransaction tx_reg; tx_reg.nVersion = 3; tx_reg.nType = TRANSACTION_PROVIDER_REGISTER; - FundTransaction(chainman.ActiveChain(), tx_reg, utxos, GetScriptForDestination(PKHash(setup.coinbaseKey.GetPubKey())), - 1 * COIN, setup.coinbaseKey); + WITH_LOCK(::cs_main, FundTransaction(chainman.ActiveChain(), tx_reg, utxos, GetScriptForDestination(PKHash(setup.coinbaseKey.GetPubKey())), + 1 * COIN, setup.coinbaseKey)); pro_reg.inputsHash = CalcTxInputsHash(CTransaction(tx_reg)); CMessageSigner::SignMessage(pro_reg.MakeSignString(), pro_reg.vchSig, collateral_key); SetTxPayload(tx_reg, pro_reg); SignTransaction(*(setup.m_node.mempool), tx_reg, setup.coinbaseKey); const auto proTxHash = tx_reg.GetHash(); setup.CreateAndProcessBlock({tx_reg}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); auto dmn = dmnman.GetListAtChainTip().GetMN(proTxHash); BOOST_REQUIRE(dmn); @@ -773,14 +789,14 @@ void FuncMNPaymentMultiplicityV24Boundary(TestChainSetup& setup) CMutableTransaction tx_ups; tx_ups.nVersion = 3; tx_ups.nType = TRANSACTION_PROVIDER_UPDATE_SERVICE; - FundTransaction(chainman.ActiveChain(), tx_ups, utxos, GetScriptForDestination(PKHash(setup.coinbaseKey.GetPubKey())), - 1 * COIN, setup.coinbaseKey); + WITH_LOCK(::cs_main, FundTransaction(chainman.ActiveChain(), tx_ups, utxos, GetScriptForDestination(PKHash(setup.coinbaseKey.GetPubKey())), + 1 * COIN, setup.coinbaseKey)); pro_ups.inputsHash = CalcTxInputsHash(CTransaction(tx_ups)); pro_ups.sig = operator_key.Sign(::SerializeHash(pro_ups), bls::bls_legacy_scheme); SetTxPayload(tx_ups, pro_ups); SignTransaction(*(setup.m_node.mempool), tx_ups, setup.coinbaseKey); setup.CreateAndProcessBlock({tx_ups}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); BOOST_REQUIRE(dmnman.GetListAtChainTip().GetMN(proTxHash)->pdmnState->scriptOperatorPayout == shared_script); // Two identical coinbase outputs paying shared_script (the owner/operator collision). @@ -805,7 +821,7 @@ void FuncMNPaymentMultiplicityV24Boundary(TestChainSetup& setup) CBlock good = setup.CreateBlock({}, coinbase_pk, chainman.ActiveChainstate()); if (auto dup = find_duplicate(good)) return {good, *dup}; BOOST_REQUIRE(chainman.ProcessNewBlock(std::make_shared(good), /*force_processing=*/true, nullptr)); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); } BOOST_REQUIRE_MESSAGE(false, "expected owner/operator collision to yield a duplicate coinbase output"); return {}; // unreachable, BOOST_REQUIRE above aborts the test @@ -847,33 +863,33 @@ void FuncMNPaymentMultiplicityV24Boundary(TestChainSetup& setup) const auto [good, dup] = mine_until_duplicate(); // The collision must be reached while v24 is still pending, otherwise this phase would be // testing post-v24 behaviour by accident. - BOOST_REQUIRE(!DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman, Consensus::DEPLOYMENT_V24)); + BOOST_REQUIRE(!DeploymentActiveAfter(tip_index(), chainman, Consensus::DEPLOYMENT_V24)); const CBlock merged = build_merge_cheat(good, dup); BOOST_REQUIRE(chainman.ProcessNewBlock(std::make_shared(merged), /*force_processing=*/true, nullptr)); - BOOST_CHECK_EQUAL(chainman.ActiveChain().Tip()->GetBlockHash(), merged.GetHash()); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + BOOST_CHECK_EQUAL(tip_hash(), merged.GetHash()); + sync_dmn_tip(); } // ---- Mine across v24 activation (the same v2 MN is kept; its collision is version-independent). - for (int i = 0; i < 2000 && !DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman, Consensus::DEPLOYMENT_V24); ++i) { + for (int i = 0; i < 2000 && !DeploymentActiveAfter(tip_index(), chainman, Consensus::DEPLOYMENT_V24); ++i) { setup.CreateAndProcessBlock({}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); } - BOOST_REQUIRE(DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman, Consensus::DEPLOYMENT_V24)); + BOOST_REQUIRE(DeploymentActiveAfter(tip_index(), chainman, Consensus::DEPLOYMENT_V24)); // ---- Post-v24: strict multiplicity matching REJECTS the same merge (two expected outputs, one // distinct actual). Rejection is observed as the tip not advancing. { const auto [good, dup] = mine_until_duplicate(); const CBlock merged = build_merge_cheat(good, dup); - const uint256 tip_before = chainman.ActiveChain().Tip()->GetBlockHash(); + const uint256 tip_before = tip_hash(); chainman.ProcessNewBlock(std::make_shared(merged), /*force_processing=*/true, nullptr); - BOOST_CHECK(chainman.ActiveChain().Tip()->GetBlockHash() == tip_before); - BOOST_CHECK(chainman.ActiveChain().Tip()->GetBlockHash() != merged.GetHash()); + BOOST_CHECK(tip_hash() == tip_before); + BOOST_CHECK(tip_hash() != merged.GetHash()); // The faithful block (both identical outputs present) connects. BOOST_REQUIRE(chainman.ProcessNewBlock(std::make_shared(good), /*force_processing=*/true, nullptr)); - BOOST_CHECK_EQUAL(chainman.ActiveChain().Tip()->GetBlockHash(), good.GetHash()); + BOOST_CHECK_EQUAL(tip_hash(), good.GetHash()); } } @@ -922,14 +938,16 @@ void FuncProRegTxRejectsInvalidDeserializedExtNetInfo(TestChainSetup& setup) { auto& chainman = *Assert(setup.m_node.chainman.get()); auto& dmnman = *Assert(setup.m_node.dmnman); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + auto sync_dmn_tip = [&] { dmnman.UpdatedBlockTip(tip_index()); }; const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); - for (int i = 0; i < 2000 && !DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman, Consensus::DEPLOYMENT_V24); ++i) { + for (int i = 0; i < 2000 && !DeploymentActiveAfter(tip_index(), chainman, Consensus::DEPLOYMENT_V24); ++i) { setup.CreateAndProcessBlock({}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); } - BOOST_REQUIRE(DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); - BOOST_REQUIRE(DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman, Consensus::DEPLOYMENT_V24)); + BOOST_REQUIRE(DeploymentActiveAfter(tip_index(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); + BOOST_REQUIRE(DeploymentActiveAfter(tip_index(), chainman, Consensus::DEPLOYMENT_V24)); BOOST_REQUIRE(!bls::bls_legacy_scheme.load()); auto check_reject_reason = [&](std::shared_ptr net_info, const std::string& reject_reason) { @@ -968,11 +986,14 @@ void FuncDIP3Protx(TestChainSetup& setup) { auto& chainman = *Assert(setup.m_node.chainman.get()); auto& dmnman = *Assert(setup.m_node.dmnman); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + auto tip_height = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Height()); }; + auto sync_dmn_tip = [&] { dmnman.UpdatedBlockTip(tip_index()); }; auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); - int nHeight = chainman.ActiveChain().Height(); + int nHeight = tip_height(); int port = 1; std::vector dmnHashes; @@ -983,7 +1004,7 @@ void FuncDIP3Protx(TestChainSetup& setup) for (size_t i = 0; i < 6; i++) { CKey ownerKey; CBLSSecretKey operatorKey; - auto tx = CreateProRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, port++, GenerateRandomAddress(), setup.coinbaseKey, ownerKey, operatorKey); + auto tx = WITH_LOCK(::cs_main, return CreateProRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, port++, GenerateRandomAddress(), setup.coinbaseKey, ownerKey, operatorKey)); dmnHashes.emplace_back(tx.GetHash()); ownerKeys.emplace(tx.GetHash(), ownerKey); operatorKeys.emplace(tx.GetHash(), operatorKey); @@ -1007,27 +1028,27 @@ void FuncDIP3Protx(TestChainSetup& setup) BOOST_REQUIRE(!CheckTransactionSignature(*(setup.m_node.mempool), tx2)); setup.CreateAndProcessBlock({tx}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight + 1); + BOOST_CHECK_EQUAL(tip_height(), nHeight + 1); BOOST_REQUIRE(dmnman.GetListAtChainTip().HasMN(tx.GetHash())); nHeight++; } int DIP0003EnforcementHeightBackup = Params().GetConsensus().DIP0003EnforcementHeight; - const_cast(Params().GetConsensus()).DIP0003EnforcementHeight = chainman.ActiveChain().Height() + 1; + const_cast(Params().GetConsensus()).DIP0003EnforcementHeight = tip_height() + 1; setup.CreateAndProcessBlock({}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); nHeight++; // check MN reward payments for (size_t i = 0; i < 20; i++) { - auto dmnExpectedPayee = dmnman.GetListAtChainTip().GetMNPayee(chainman.ActiveChain().Tip()); + auto dmnExpectedPayee = dmnman.GetListAtChainTip().GetMNPayee(tip_index()); BOOST_ASSERT(dmnExpectedPayee); CBlock block = setup.CreateAndProcessBlock({}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); BOOST_REQUIRE(!block.vtx.empty()); auto dmnPayout = FindPayoutDmn(dmnman, block); @@ -1043,15 +1064,15 @@ void FuncDIP3Protx(TestChainSetup& setup) for (size_t j = 0; j < 3; j++) { CKey ownerKey; CBLSSecretKey operatorKey; - auto tx = CreateProRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, port++, GenerateRandomAddress(), setup.coinbaseKey, ownerKey, operatorKey); + auto tx = WITH_LOCK(::cs_main, return CreateProRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, port++, GenerateRandomAddress(), setup.coinbaseKey, ownerKey, operatorKey)); dmnHashes.emplace_back(tx.GetHash()); ownerKeys.emplace(tx.GetHash(), ownerKey); operatorKeys.emplace(tx.GetHash(), operatorKey); txns.emplace_back(tx); } setup.CreateAndProcessBlock(txns, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight + 1); + sync_dmn_tip(); + BOOST_CHECK_EQUAL(tip_height(), nHeight + 1); for (size_t j = 0; j < 3; j++) { BOOST_REQUIRE(dmnman.GetListAtChainTip().HasMN(txns[j].GetHash())); @@ -1061,20 +1082,20 @@ void FuncDIP3Protx(TestChainSetup& setup) } // test ProUpServTx - auto tx = CreateProUpServTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, dmnHashes[0], operatorKeys[dmnHashes[0]], 1000, CScript(), setup.coinbaseKey); + auto tx = WITH_LOCK(::cs_main, return CreateProUpServTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, dmnHashes[0], operatorKeys[dmnHashes[0]], 1000, CScript(), setup.coinbaseKey)); setup.CreateAndProcessBlock({tx}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight + 1); + sync_dmn_tip(); + BOOST_CHECK_EQUAL(tip_height(), nHeight + 1); nHeight++; auto dmn = dmnman.GetListAtChainTip().GetMN(dmnHashes[0]); BOOST_REQUIRE(dmn != nullptr && dmn->pdmnState->netInfo->GetPrimary().GetPort() == 1000); // test ProUpRevTx - tx = CreateProUpRevTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, dmnHashes[0], operatorKeys[dmnHashes[0]], setup.coinbaseKey); + tx = WITH_LOCK(::cs_main, return CreateProUpRevTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, dmnHashes[0], operatorKeys[dmnHashes[0]], setup.coinbaseKey)); setup.CreateAndProcessBlock({tx}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight + 1); + sync_dmn_tip(); + BOOST_CHECK_EQUAL(tip_height(), nHeight + 1); nHeight++; dmn = dmnman.GetListAtChainTip().GetMN(dmnHashes[0]); @@ -1082,11 +1103,11 @@ void FuncDIP3Protx(TestChainSetup& setup) // test that the revoked MN does not get paid anymore for (size_t i = 0; i < 20; i++) { - auto dmnExpectedPayee = dmnman.GetListAtChainTip().GetMNPayee(chainman.ActiveChain().Tip()); + auto dmnExpectedPayee = dmnman.GetListAtChainTip().GetMNPayee(tip_index()); BOOST_REQUIRE(dmnExpectedPayee && dmnExpectedPayee->proTxHash != dmnHashes[0]); CBlock block = setup.CreateAndProcessBlock({}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); BOOST_REQUIRE(!block.vtx.empty()); auto dmnPayout = FindPayoutDmn(dmnman, block); @@ -1100,7 +1121,7 @@ void FuncDIP3Protx(TestChainSetup& setup) CBLSSecretKey newOperatorKey; newOperatorKey.MakeNewKey(); dmn = dmnman.GetListAtChainTip().GetMN(dmnHashes[0]); - tx = CreateProUpRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, dmnHashes[0], ownerKeys[dmnHashes[0]], newOperatorKey.GetPublicKey(), ownerKeys[dmnHashes[0]].GetPubKey().GetID(), dmn->pdmnState->scriptPayout, setup.coinbaseKey); + tx = WITH_LOCK(::cs_main, return CreateProUpRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, dmnHashes[0], ownerKeys[dmnHashes[0]], newOperatorKey.GetPublicKey(), ownerKeys[dmnHashes[0]].GetPubKey().GetID(), dmn->pdmnState->scriptPayout, setup.coinbaseKey)); // check malleability protection again, but this time by also relying on the signature inside the ProUpRegTx auto tx2 = MalleateProTxPayout(tx); TxValidationState dummy_state; @@ -1115,14 +1136,14 @@ void FuncDIP3Protx(TestChainSetup& setup) BOOST_REQUIRE(!CheckTransactionSignature(*(setup.m_node.mempool), tx2)); // now process the block setup.CreateAndProcessBlock({tx}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight + 1); + sync_dmn_tip(); + BOOST_CHECK_EQUAL(tip_height(), nHeight + 1); nHeight++; - tx = CreateProUpServTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, dmnHashes[0], newOperatorKey, 100, CScript(), setup.coinbaseKey); + tx = WITH_LOCK(::cs_main, return CreateProUpServTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, dmnHashes[0], newOperatorKey, 100, CScript(), setup.coinbaseKey)); setup.CreateAndProcessBlock({tx}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight + 1); + sync_dmn_tip(); + BOOST_CHECK_EQUAL(tip_height(), nHeight + 1); nHeight++; dmn = dmnman.GetListAtChainTip().GetMN(dmnHashes[0]); @@ -1132,14 +1153,14 @@ void FuncDIP3Protx(TestChainSetup& setup) // test that the revived MN gets payments again bool foundRevived = false; for (size_t i = 0; i < 20; i++) { - auto dmnExpectedPayee = dmnman.GetListAtChainTip().GetMNPayee(chainman.ActiveChain().Tip()); + auto dmnExpectedPayee = dmnman.GetListAtChainTip().GetMNPayee(tip_index()); BOOST_ASSERT(dmnExpectedPayee); if (dmnExpectedPayee->proTxHash == dmnHashes[0]) { foundRevived = true; } CBlock block = setup.CreateAndProcessBlock({}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + sync_dmn_tip(); BOOST_REQUIRE(!block.vtx.empty()); auto dmnPayout = FindPayoutDmn(dmnman, block); @@ -1156,9 +1177,12 @@ void FuncDIP3Protx(TestChainSetup& setup) void FuncTestMempoolReorg(TestChainSetup& setup) { auto& chainman = *Assert(setup.m_node.chainman.get()); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + auto tip_height = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Height()); }; + auto tip_hash = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()->GetBlockHash()); }; const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); - int nHeight = chainman.ActiveChain().Height(); + int nHeight = tip_height(); auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); CKey ownerKey; @@ -1176,14 +1200,14 @@ void FuncTestMempoolReorg(TestChainSetup& setup) // Create a MN with an external collateral CMutableTransaction tx_collateral; - FundTransaction(chainman.ActiveChain(), tx_collateral, utxos, scriptCollateral, dmn_types::Regular.collat_amount, setup.coinbaseKey); + WITH_LOCK(::cs_main, FundTransaction(chainman.ActiveChain(), tx_collateral, utxos, scriptCollateral, dmn_types::Regular.collat_amount, setup.coinbaseKey)); SignTransaction(*(setup.m_node.mempool), tx_collateral, setup.coinbaseKey); auto block = std::make_shared(setup.CreateBlock({tx_collateral}, coinbase_pk, chainman.ActiveChainstate())); BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); - setup.m_node.dmnman->UpdatedBlockTip(chainman.ActiveChain().Tip()); - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight + 1); - BOOST_CHECK_EQUAL(block->GetHash(), chainman.ActiveChain().Tip()->GetBlockHash()); + setup.m_node.dmnman->UpdatedBlockTip(tip_index()); + BOOST_CHECK_EQUAL(tip_height(), nHeight + 1); + BOOST_CHECK_EQUAL(block->GetHash(), tip_hash()); CProRegTx payload; payload.nVersion = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false); @@ -1204,7 +1228,7 @@ void FuncTestMempoolReorg(TestChainSetup& setup) CMutableTransaction tx_reg; tx_reg.nVersion = 3; tx_reg.nType = TRANSACTION_PROVIDER_REGISTER; - FundTransaction(chainman.ActiveChain(), tx_reg, utxos, scriptPayout, dmn_types::Regular.collat_amount, setup.coinbaseKey); + WITH_LOCK(::cs_main, FundTransaction(chainman.ActiveChain(), tx_reg, utxos, scriptPayout, dmn_types::Regular.collat_amount, setup.coinbaseKey)); payload.inputsHash = CalcTxInputsHash(CTransaction(tx_reg)); CMessageSigner::SignMessage(payload.MakeSignString(), payload.vchSig, collateralKey); SetTxPayload(tx_reg, payload); @@ -1246,7 +1270,7 @@ void FuncTestMempoolDualProregtx(TestChainSetup& setup) // Create a MN CKey ownerKey1; CBLSSecretKey operatorKey1; - auto tx_reg1 = CreateProRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, 1, GenerateRandomAddress(), setup.coinbaseKey, ownerKey1, operatorKey1); + auto tx_reg1 = WITH_LOCK(::cs_main, return CreateProRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, 1, GenerateRandomAddress(), setup.coinbaseKey, ownerKey1, operatorKey1)); // Create a MN with an external collateral that references tx_reg1 CKey ownerKey; @@ -1280,7 +1304,7 @@ void FuncTestMempoolDualProregtx(TestChainSetup& setup) CMutableTransaction tx_reg2; tx_reg2.nVersion = 3; tx_reg2.nType = TRANSACTION_PROVIDER_REGISTER; - FundTransaction(chainman.ActiveChain(), tx_reg2, utxos, scriptPayout, dmn_types::Regular.collat_amount, setup.coinbaseKey); + WITH_LOCK(::cs_main, FundTransaction(chainman.ActiveChain(), tx_reg2, utxos, scriptPayout, dmn_types::Regular.collat_amount, setup.coinbaseKey)); payload.inputsHash = CalcTxInputsHash(CTransaction(tx_reg2)); CMessageSigner::SignMessage(payload.MakeSignString(), payload.vchSig, collateralKey); SetTxPayload(tx_reg2, payload); @@ -1302,9 +1326,13 @@ void FuncVerifyDB(TestChainSetup& setup) { auto& chainman = *Assert(setup.m_node.chainman.get()); auto& dmnman = *Assert(setup.m_node.dmnman); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + auto tip_height = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Height()); }; + auto tip_hash = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()->GetBlockHash()); }; + auto sync_dmn_tip = [&] { dmnman.UpdatedBlockTip(tip_index()); }; const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); - int nHeight = chainman.ActiveChain().Height(); + int nHeight = tip_height(); auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); CKey ownerKey; @@ -1322,14 +1350,14 @@ void FuncVerifyDB(TestChainSetup& setup) // Create a MN with an external collateral CMutableTransaction tx_collateral; - FundTransaction(chainman.ActiveChain(), tx_collateral, utxos, scriptCollateral, dmn_types::Regular.collat_amount, setup.coinbaseKey); + WITH_LOCK(::cs_main, FundTransaction(chainman.ActiveChain(), tx_collateral, utxos, scriptCollateral, dmn_types::Regular.collat_amount, setup.coinbaseKey)); SignTransaction(*(setup.m_node.mempool), tx_collateral, setup.coinbaseKey); auto block = std::make_shared(setup.CreateBlock({tx_collateral}, coinbase_pk, chainman.ActiveChainstate())); BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight + 1); - BOOST_CHECK_EQUAL(block->GetHash(), chainman.ActiveChain().Tip()->GetBlockHash()); + sync_dmn_tip(); + BOOST_CHECK_EQUAL(tip_height(), nHeight + 1); + BOOST_CHECK_EQUAL(block->GetHash(), tip_hash()); CProRegTx payload; payload.nVersion = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false); @@ -1350,7 +1378,7 @@ void FuncVerifyDB(TestChainSetup& setup) CMutableTransaction tx_reg; tx_reg.nVersion = 3; tx_reg.nType = TRANSACTION_PROVIDER_REGISTER; - FundTransaction(chainman.ActiveChain(), tx_reg, utxos, scriptPayout, dmn_types::Regular.collat_amount, setup.coinbaseKey); + WITH_LOCK(::cs_main, FundTransaction(chainman.ActiveChain(), tx_reg, utxos, scriptPayout, dmn_types::Regular.collat_amount, setup.coinbaseKey)); payload.inputsHash = CalcTxInputsHash(CTransaction(tx_reg)); CMessageSigner::SignMessage(payload.MakeSignString(), payload.vchSig, collateralKey); SetTxPayload(tx_reg, payload); @@ -1360,21 +1388,21 @@ void FuncVerifyDB(TestChainSetup& setup) block = std::make_shared(setup.CreateBlock({tx_reg}, coinbase_pk, chainman.ActiveChainstate())); BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight + 2); - BOOST_CHECK_EQUAL(block->GetHash(), chainman.ActiveChain().Tip()->GetBlockHash()); + sync_dmn_tip(); + BOOST_CHECK_EQUAL(tip_height(), nHeight + 2); + BOOST_CHECK_EQUAL(block->GetHash(), tip_hash()); BOOST_REQUIRE(dmnman.GetListAtChainTip().HasMN(tx_reg_hash)); // Now spend the collateral while updating the same MN SimpleUTXOMap collateral_utxos; collateral_utxos.emplace(payload.collateralOutpoint, std::make_pair(1, 1000)); - auto proUpRevTx = CreateProUpRevTx(chainman.ActiveChain(), *(setup.m_node.mempool), collateral_utxos, tx_reg_hash, operatorKey, collateralKey); + auto proUpRevTx = WITH_LOCK(::cs_main, return CreateProUpRevTx(chainman.ActiveChain(), *(setup.m_node.mempool), collateral_utxos, tx_reg_hash, operatorKey, collateralKey)); block = std::make_shared(setup.CreateBlock({proUpRevTx}, coinbase_pk, chainman.ActiveChainstate())); BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); - BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight + 3); - BOOST_CHECK_EQUAL(block->GetHash(), chainman.ActiveChain().Tip()->GetBlockHash()); + sync_dmn_tip(); + BOOST_CHECK_EQUAL(tip_height(), nHeight + 3); + BOOST_CHECK_EQUAL(block->GetHash(), tip_hash()); BOOST_REQUIRE(!dmnman.GetListAtChainTip().HasMN(tx_reg_hash)); // Verify db consistency @@ -1505,9 +1533,8 @@ struct TestChainV19Setup : public TestChainV19BeforeActivationSetup { for (int i = 0; i < 5; ++i) { CreateAndProcessBlock({}, coinbase_pk); } - bool v19_just_activated{ - DeploymentActiveAfter(m_node.chainman->ActiveChain().Tip(), m_node.chainman->GetConsensus(), Consensus::DEPLOYMENT_V19) && - !DeploymentActiveAt(*m_node.chainman->ActiveChain().Tip(), m_node.chainman->GetConsensus(), Consensus::DEPLOYMENT_V19)}; + bool v19_just_activated{WITH_LOCK(::cs_main, return DeploymentActiveAfter(m_node.chainman->ActiveChain().Tip(), m_node.chainman->GetConsensus(), Consensus::DEPLOYMENT_V19) && + !DeploymentActiveAt(*m_node.chainman->ActiveChain().Tip(), m_node.chainman->GetConsensus(), Consensus::DEPLOYMENT_V19))}; assert(v19_just_activated); } }; @@ -1516,8 +1543,8 @@ struct TestChainV19Setup : public TestChainV19BeforeActivationSetup { TestChainV19BeforeActivationSetup::TestChainV19BeforeActivationSetup() : TestChainSetup(494, CBaseChainParams::REGTEST, {"-testactivationheight=v19@500", "-testactivationheight=v20@500", "-testactivationheight=mn_rr@500"}) { - bool v19_active{DeploymentActiveAfter(m_node.chainman->ActiveChain().Tip(), m_node.chainman->GetConsensus(), - Consensus::DEPLOYMENT_V19)}; + bool v19_active{WITH_LOCK(::cs_main, return DeploymentActiveAfter(m_node.chainman->ActiveChain().Tip(), m_node.chainman->GetConsensus(), + Consensus::DEPLOYMENT_V19))}; assert(!v19_active); } @@ -1528,10 +1555,10 @@ struct TestChainV24SignalBeforeV19Setup : public TestChainSetup { "-testactivationheight=mn_rr@500", "-vbparams=v24:0:9999999999:0:500:400:300:5:0"}) { - assert(!DeploymentActiveAfter(m_node.chainman->ActiveChain().Tip(), m_node.chainman->GetConsensus(), - Consensus::DEPLOYMENT_V19)); - assert(!DeploymentActiveAfter(m_node.chainman->ActiveChain().Tip(), *m_node.chainman, - Consensus::DEPLOYMENT_V24)); + assert(WITH_LOCK(::cs_main, return !DeploymentActiveAfter(m_node.chainman->ActiveChain().Tip(), m_node.chainman->GetConsensus(), + Consensus::DEPLOYMENT_V19))); + assert(WITH_LOCK(::cs_main, return !DeploymentActiveAfter(m_node.chainman->ActiveChain().Tip(), *m_node.chainman, + Consensus::DEPLOYMENT_V24))); } }; @@ -1550,12 +1577,12 @@ struct TestChainV24PendingSetup : public TestChainSetup { auto& chainman = *Assert(m_node.chainman); auto& dmnman = *Assert(m_node.dmnman); // Mine just enough to activate v19/v20/mn_rr (height 500) while keeping v24 pending. - for (int i = 0; i < 20 && !DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19); ++i) { + for (int i = 0; i < 20 && WITH_LOCK(::cs_main, return !DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); ++i) { CreateAndProcessBlock({}, coinbase_pk); - dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); + dmnman.UpdatedBlockTip(WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip())); } - assert(DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); - assert(!DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman, Consensus::DEPLOYMENT_V24)); + assert(WITH_LOCK(::cs_main, return DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19))); + assert(WITH_LOCK(::cs_main, return !DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman, Consensus::DEPLOYMENT_V24))); assert(!bls::bls_legacy_scheme.load()); } }; diff --git a/src/test/evo_mnhf_tests.cpp b/src/test/evo_mnhf_tests.cpp index 22cb33919240..f05d9d648922 100644 --- a/src/test/evo_mnhf_tests.cpp +++ b/src/test/evo_mnhf_tests.cpp @@ -66,7 +66,7 @@ BOOST_AUTO_TEST_CASE(verify_mnhf_specialtx_tests) auto& chainman = Assert(m_node.chainman); auto& qman = *Assert(m_node.llmq_ctx)->qman; - const CBlockIndex* pindex = chainman->ActiveChain().Tip(); + const CBlockIndex* pindex = WITH_LOCK(::cs_main, return chainman->ActiveChain().Tip()); uint256 hash = GetRandHash(); TxValidationState state; diff --git a/src/test/evo_trivialvalidation.cpp b/src/test/evo_trivialvalidation.cpp index 1e47c3f9dfa2..ce3fdaff815c 100644 --- a/src/test/evo_trivialvalidation.cpp +++ b/src/test/evo_trivialvalidation.cpp @@ -64,8 +64,8 @@ void trivialvalidation_runner(const ChainstateManager& chainman, const std::stri BOOST_CHECK(test[2].get_str() == "basic" || test[2].get_str() == "legacy"); // Determine which pindexPrev to supply based on whether we want to validate legacy or basic // TODO: Introduce trivial validation test vectors for extended addresses - const CBlockIndex* pindexPrev{(test[2].get_str() == "basic") ? chainman.ActiveChain()[100] - : chainman.ActiveChain()[98]}; + const CBlockIndex* pindexPrev{WITH_LOCK(::cs_main, return (test[2].get_str() == "basic") ? chainman.ActiveChain()[100] + : chainman.ActiveChain()[98])}; assert(pindexPrev); // Raw transaction CDataStream stream(ParseHex(test[3].get_str()), SER_NETWORK, PROTOCOL_VERSION); diff --git a/src/test/evo_utils_tests.cpp b/src/test/evo_utils_tests.cpp index b955023e32e8..3c892dda8128 100644 --- a/src/test/evo_utils_tests.cpp +++ b/src/test/evo_utils_tests.cpp @@ -18,7 +18,7 @@ BOOST_AUTO_TEST_SUITE(evo_utils_tests) void Test(NodeContext& node) { using namespace llmq; - auto tip = node.chainman->ActiveTip(); + auto tip = WITH_LOCK(::cs_main, return node.chainman->ActiveTip()); const auto& consensus_params = Params().GetConsensus(); BOOST_CHECK_EQUAL(node.chainman->IsQuorumTypeEnabled(consensus_params.llmqTypeDIP0024InstantSend, tip, /*optDIP0024IsActive=*/false, /*optHaveDIP0024Quorums=*/false), diff --git a/src/test/interfaces_tests.cpp b/src/test/interfaces_tests.cpp index 49b7d2003b40..11c10a4f6432 100644 --- a/src/test/interfaces_tests.cpp +++ b/src/test/interfaces_tests.cpp @@ -17,6 +17,7 @@ BOOST_FIXTURE_TEST_SUITE(interfaces_tests, TestChain100Setup) BOOST_AUTO_TEST_CASE(findBlock) { + LOCK(Assert(m_node.chainman)->GetMutex()); auto& chain = m_node.chain; const CChain& active = Assert(m_node.chainman)->ActiveChain(); @@ -61,6 +62,7 @@ BOOST_AUTO_TEST_CASE(findBlock) BOOST_AUTO_TEST_CASE(findFirstBlockWithTimeAndHeight) { + LOCK(Assert(m_node.chainman)->GetMutex()); auto& chain = m_node.chain; const CChain& active = Assert(m_node.chainman)->ActiveChain(); uint256 hash; @@ -73,6 +75,7 @@ BOOST_AUTO_TEST_CASE(findFirstBlockWithTimeAndHeight) BOOST_AUTO_TEST_CASE(findAncestorByHeight) { + LOCK(Assert(m_node.chainman)->GetMutex()); auto& chain = m_node.chain; const CChain& active = Assert(m_node.chainman)->ActiveChain(); uint256 hash; @@ -83,6 +86,7 @@ BOOST_AUTO_TEST_CASE(findAncestorByHeight) BOOST_AUTO_TEST_CASE(findAncestorByHash) { + LOCK(Assert(m_node.chainman)->GetMutex()); auto& chain = m_node.chain; const CChain& active = Assert(m_node.chainman)->ActiveChain(); int height = -1; @@ -94,7 +98,7 @@ BOOST_AUTO_TEST_CASE(findAncestorByHash) BOOST_AUTO_TEST_CASE(findCommonAncestor) { auto& chain = m_node.chain; - const CChain& active = Assert(m_node.chainman)->ActiveChain(); + const CChain& active = *WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return &Assert(m_node.chainman)->ActiveChain()); auto* orig_tip = active.Tip(); for (int i = 0; i < 10; ++i) { BlockValidationState state; diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 7cbddbcd5d5b..8452c69ee69b 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -617,7 +617,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) std::vector txFirst; auto createAndProcessEmptyBlock = [&]() { - int i = m_node.chainman->ActiveChain().Height() % blockinfo_size; + int i = WITH_LOCK(::cs_main, return m_node.chainman->ActiveChain().Height()) % blockinfo_size; CBlock *pblock = &pemptyblocktemplate->block; // pointer for convenience { LOCK(cs_main); diff --git a/src/test/util/mining.cpp b/src/test/util/mining.cpp index 832941de1bcc..e5a0e5484e97 100644 --- a/src/test/util/mining.cpp +++ b/src/test/util/mining.cpp @@ -86,7 +86,7 @@ std::shared_ptr PrepareBlock(const NodeContext& node, const CScript& coi .CreateNewBlock(coinbase_scriptPubKey) ->block); - block->nTime = Assert(node.chainman)->ActiveChain().Tip()->GetMedianTimePast() + 1; + block->nTime = WITH_LOCK(::cs_main, return Assert(node.chainman)->ActiveChain().Tip()->GetMedianTimePast()) + 1; block->hashMerkleRoot = BlockMerkleRoot(*block); return block; diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index e08c88b44e02..4c1979feac27 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -267,7 +267,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) // Run the test multiple times for (int test_runs = 3; test_runs > 0; --test_runs) { - BOOST_CHECK_EQUAL(last_mined->GetHash(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); + BOOST_CHECK_EQUAL(last_mined->GetHash(), WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip()->GetBlockHash())); // Later on split from here const uint256 split_hash{last_mined->hashPrevBlock}; @@ -356,7 +356,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) ProcessBlock(b); } // Check that the reorg was eventually successful - BOOST_CHECK_EQUAL(last_mined->GetHash(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); + BOOST_CHECK_EQUAL(last_mined->GetHash(), WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip()->GetBlockHash())); // We can join the other thread, which returns when the reorg was successful rpc_thread.join(); diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 829c79588064..049bff77d60a 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -59,12 +59,12 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) auto all = manager.GetAll(); BOOST_CHECK_EQUAL_COLLECTIONS(all.begin(), all.end(), chainstates.begin(), chainstates.end()); - auto& active_chain = manager.ActiveChain(); + auto& active_chain = WITH_LOCK(manager.GetMutex(), return manager.ActiveChain()); BOOST_CHECK_EQUAL(&active_chain, &c1.m_chain); - BOOST_CHECK_EQUAL(manager.ActiveHeight(), -1); + BOOST_CHECK_EQUAL(WITH_LOCK(manager.GetMutex(), return manager.ActiveHeight()), -1); - auto active_tip = manager.ActiveTip(); + auto active_tip = WITH_LOCK(manager.GetMutex(), return manager.ActiveTip()); auto exp_tip = c1.m_chain.Tip(); BOOST_CHECK_EQUAL(active_tip, exp_tip); @@ -103,12 +103,12 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) auto all2 = manager.GetAll(); BOOST_CHECK_EQUAL_COLLECTIONS(all2.begin(), all2.end(), chainstates.begin(), chainstates.end()); - auto& active_chain2 = manager.ActiveChain(); + auto& active_chain2 = WITH_LOCK(manager.GetMutex(), return manager.ActiveChain()); BOOST_CHECK_EQUAL(&active_chain2, &c2.m_chain); - BOOST_CHECK_EQUAL(manager.ActiveHeight(), 0); + BOOST_CHECK_EQUAL(WITH_LOCK(manager.GetMutex(), return manager.ActiveHeight()), 0); - auto active_tip2 = manager.ActiveTip(); + auto active_tip2 = WITH_LOCK(manager.GetMutex(), return manager.ActiveTip()); auto exp_tip2 = c2.m_chain.Tip(); BOOST_CHECK_EQUAL(active_tip2, exp_tip2); @@ -260,7 +260,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup) BOOST_CHECK(WITH_LOCK(::cs_main, return !chainman.ActiveChain().Genesis()->IsAssumedValid())); const AssumeutxoData& au_data = *ExpectedAssumeutxo(snapshot_height, ::Params()); - const CBlockIndex* tip = chainman.ActiveTip(); + const CBlockIndex* tip = WITH_LOCK(chainman.GetMutex(), return chainman.ActiveTip()); BOOST_CHECK_EQUAL(tip->nChainTx, au_data.nChainTx); @@ -359,7 +359,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup) const int assumed_valid_start_idx = last_assumed_valid_idx - expected_assumed_valid; CBlockIndex* validated_tip{nullptr}; - CBlockIndex* assumed_tip{chainman.ActiveChain().Tip()}; + CBlockIndex* assumed_tip{WITH_LOCK(chainman.GetMutex(), return chainman.ActiveChain().Tip())}; auto reload_all_block_indexes = [&]() { for (CChainState* cs : chainman.GetAll()) { diff --git a/src/validation.cpp b/src/validation.cpp index 012783385685..443090ec911b 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4712,6 +4712,8 @@ bool ChainstateManager::LoadBlockIndex() bool ret{m_blockman.LoadBlockIndexDB()}; if (!ret) return false; + m_blockman.ScanAndUnlinkAlreadyPrunedFiles(); + std::vector vSortedByHeight{m_blockman.GetAllBlockIndices()}; std::sort(vSortedByHeight.begin(), vSortedByHeight.end(), CBlockIndexHeightOnlyComparator()); diff --git a/src/validation.h b/src/validation.h index 307865f5f558..a74af25f9623 100644 --- a/src/validation.h +++ b/src/validation.h @@ -936,6 +936,19 @@ class ChainstateManager const CChainParams& GetParams() const { return m_chainparams; } const Consensus::Params& GetConsensus() const { return m_chainparams.GetConsensus(); } + /** + * Alias for ::cs_main. + * Should be used in new code to make it easier to make ::cs_main a member + * of this class. + * Generally, methods of this class should be annotated to require this + * mutex. This will make calling code more verbose, but also help to: + * - Clarify that the method will acquire a mutex that heavily affects + * overall performance. + * - Force call sites to think how long they need to acquire the mutex to + * get consistent results. + */ + RecursiveMutex& GetMutex() const LOCK_RETURNED(::cs_main) { return ::cs_main; } + std::thread m_load_block; //! A single BlockManager instance is shared across each constructed //! chainstate to avoid duplicating block metadata. @@ -1007,9 +1020,9 @@ class ChainstateManager //! The most-work chain. CChainState& ActiveChainstate() const; - CChain& ActiveChain() const { return ActiveChainstate().m_chain; } - int ActiveHeight() const { return ActiveChain().Height(); } - CBlockIndex* ActiveTip() const { return ActiveChain().Tip(); } + CChain& ActiveChain() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) { return ActiveChainstate().m_chain; } + int ActiveHeight() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) { return ActiveChain().Height(); } + CBlockIndex* ActiveTip() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) { return ActiveChain().Tip(); } node::BlockMap& BlockIndex() EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { diff --git a/src/wallet/test/availablecoins_tests.cpp b/src/wallet/test/availablecoins_tests.cpp index 65dc29f70d8a..0afb81d062e0 100644 --- a/src/wallet/test/availablecoins_tests.cpp +++ b/src/wallet/test/availablecoins_tests.cpp @@ -18,7 +18,7 @@ class AvailableCoinsTestingSetup : public TestChain100Setup AvailableCoinsTestingSetup() { CreateAndProcessBlock({}, {}); - wallet = CreateSyncedWallet(*m_node.chain, *m_node.coinjoin_loader, m_node.chainman->ActiveChain(), m_args, coinbaseKey); + wallet = CreateSyncedWallet(*m_node.chain, *m_node.coinjoin_loader, *m_node.chainman, m_args, coinbaseKey); } ~AvailableCoinsTestingSetup() @@ -42,11 +42,12 @@ class AvailableCoinsTestingSetup : public TestChain100Setup } CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); + const CBlockIndex* tip{WITH_LOCK(m_node.chainman->GetMutex(), return m_node.chainman->ActiveChain().Tip())}; LOCK(wallet->cs_wallet); - wallet->SetLastBlockProcessed(wallet->GetLastBlockHeight() + 1, m_node.chainman->ActiveChain().Tip()->GetBlockHash()); + wallet->SetLastBlockProcessed(tip->nHeight, tip->GetBlockHash()); auto it = wallet->mapWallet.find(tx->GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); - it->second.m_state = TxStateConfirmed{m_node.chainman->ActiveChain().Tip()->GetBlockHash(), m_node.chainman->ActiveChain().Height(), /*index=*/1}; + it->second.m_state = TxStateConfirmed{tip->GetBlockHash(), tip->nHeight, /*index=*/1}; return it->second; } @@ -57,11 +58,13 @@ BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, AvailableCoinsTestingSetup) { CoinsResult available_coins; util::Result dest{util::Error{}}; - LOCK(wallet->cs_wallet); // Verify our wallet has one usable coinbase UTXO before starting // This UTXO is a P2PK, so it should show up in the Other bucket - available_coins = AvailableCoins(*wallet); + { + LOCK(wallet->cs_wallet); + available_coins = AvailableCoins(*wallet); + } BOOST_CHECK_EQUAL(available_coins.size(), 1U); BOOST_CHECK_EQUAL(available_coins.other.size(), 1U); @@ -73,10 +76,16 @@ BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, AvailableCoinsTestingSetup) // 2. One UTXO from the change, due to payment address matching logic // Legacy (P2PKH) - dest = wallet->GetNewDestination(""); + { + LOCK(wallet->cs_wallet); + dest = wallet->GetNewDestination(""); + } BOOST_ASSERT(dest); AddTx(CRecipient{{GetScriptForDestination(*dest)}, 4 * COIN, /*fSubtractFeeFromAmount=*/true}); - available_coins = AvailableCoins(*wallet); + { + LOCK(wallet->cs_wallet); + available_coins = AvailableCoins(*wallet); + } BOOST_CHECK_EQUAL(available_coins.legacy.size(), 2U); } diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp index db84ff428d0d..876b65bb2669 100644 --- a/src/wallet/test/spend_tests.cpp +++ b/src/wallet/test/spend_tests.cpp @@ -17,7 +17,7 @@ BOOST_FIXTURE_TEST_SUITE(spend_tests, WalletTestingSetup) BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup) { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - auto wallet = CreateSyncedWallet(*m_node.chain, *m_node.coinjoin_loader, m_node.chainman->ActiveChain(), m_args, coinbaseKey); + auto wallet = CreateSyncedWallet(*m_node.chain, *m_node.coinjoin_loader, *Assert(m_node.chainman), m_args, coinbaseKey); // Check that a subtract-from-recipient transaction slightly less than the // coinbase input amount does not create a change output (because it would diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp index 3c9f6b58be74..915304828a13 100644 --- a/src/wallet/test/util.cpp +++ b/src/wallet/test/util.cpp @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include #include @@ -16,12 +18,22 @@ #include namespace wallet { -std::unique_ptr CreateSyncedWallet(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, CChain& cchain, ArgsManager& args, const CKey& key) +std::unique_ptr CreateSyncedWallet(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, ChainstateManager& chainman, ArgsManager& args, const CKey& key) { + struct ChainInfo { + int height; + uint256 tip_hash; + uint256 genesis_hash; + }; + const ChainInfo chain_info{WITH_LOCK(chainman.GetMutex(), return (ChainInfo{ + chainman.ActiveChain().Height(), + chainman.ActiveChain().Tip()->GetBlockHash(), + chainman.ActiveChain().Genesis()->GetBlockHash()}))}; + auto wallet = std::make_unique(&chain, &coinjoin_loader, "", args, CreateMockWalletDatabase()); { LOCK(wallet->cs_wallet); - wallet->SetLastBlockProcessed(cchain.Height(), cchain.Tip()->GetBlockHash()); + wallet->SetLastBlockProcessed(chain_info.height, chain_info.tip_hash); } wallet->LoadWallet(); { @@ -38,10 +50,10 @@ std::unique_ptr CreateSyncedWallet(interfaces::Chain& chain, interfaces } WalletRescanReserver reserver(*wallet); reserver.reserve(); - CWallet::ScanResult result = wallet->ScanForWalletTransactions(cchain.Genesis()->GetBlockHash(), /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false); + CWallet::ScanResult result = wallet->ScanForWalletTransactions(chain_info.genesis_hash, /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS); - BOOST_CHECK_EQUAL(result.last_scanned_block, cchain.Tip()->GetBlockHash()); - BOOST_CHECK_EQUAL(*result.last_scanned_height, cchain.Height()); + BOOST_CHECK_EQUAL(result.last_scanned_block, chain_info.tip_hash); + BOOST_CHECK_EQUAL(*result.last_scanned_height, chain_info.height); BOOST_CHECK(result.last_failed_block.IsNull()); return wallet; } diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h index b6293072374d..39df9a4eb9b8 100644 --- a/src/wallet/test/util.h +++ b/src/wallet/test/util.h @@ -8,7 +8,7 @@ #include class ArgsManager; -class CChain; +class ChainstateManager; class CKey; namespace interfaces { class Chain; @@ -20,7 +20,7 @@ class Loader; namespace wallet { class CWallet; -std::unique_ptr CreateSyncedWallet(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, CChain& cchain, ArgsManager& args, const CKey& key); +std::unique_ptr CreateSyncedWallet(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, ChainstateManager& chainman, ArgsManager& args, const CKey& key); } // namespace wallet #endif // BITCOIN_WALLET_TEST_UTIL_H diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index d4e9380b172f..2ed122d5c9c5 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -105,10 +105,10 @@ static void AddKey(CWallet& wallet, const CKey& key) BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) { // Cap last block file size, and mine new block in a new block file. - CBlockIndex* oldTip = m_node.chainman->ActiveChain().Tip(); + CBlockIndex* oldTip = WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip()); WITH_LOCK(::cs_main, m_node.chainman->m_blockman.GetBlockFileInfo(oldTip->GetBlockPos().nFile)->nSize = MAX_BLOCKFILE_SIZE); CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - CBlockIndex* newTip = m_node.chainman->ActiveChain().Tip(); + CBlockIndex* newTip = WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip()); // Verify ScanForWalletTransactions fails to read an unknown start block. { @@ -117,7 +117,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) { LOCK(wallet.cs_wallet); wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); - wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); + wallet.SetLastBlockProcessed(newTip->nHeight, newTip->GetBlockHash()); } AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(wallet); @@ -137,7 +137,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) { LOCK(wallet.cs_wallet); wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); - wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); + wallet.SetLastBlockProcessed(newTip->nHeight, newTip->GetBlockHash()); } AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(wallet); @@ -181,7 +181,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) { LOCK(wallet.cs_wallet); wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); - wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); + wallet.SetLastBlockProcessed(newTip->nHeight, newTip->GetBlockHash()); } AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(wallet); @@ -208,7 +208,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) { LOCK(wallet.cs_wallet); wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); - wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); + wallet.SetLastBlockProcessed(newTip->nHeight, newTip->GetBlockHash()); } AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(wallet); @@ -225,10 +225,10 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) { // Cap last block file size, and mine new block in a new block file. - CBlockIndex* oldTip = m_node.chainman->ActiveChain().Tip(); + CBlockIndex* oldTip = WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip()); WITH_LOCK(::cs_main, m_node.chainman->m_blockman.GetBlockFileInfo(oldTip->GetBlockPos().nFile)->nSize = MAX_BLOCKFILE_SIZE); CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - CBlockIndex* newTip = m_node.chainman->ActiveChain().Tip(); + CBlockIndex* newTip = WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip()); // Prune the older block file. int file_number; @@ -291,7 +291,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) { // Create two blocks with same timestamp to verify that importwallet rescan // will pick up both blocks, not just the first. - const int64_t BLOCK_TIME = m_node.chainman->ActiveChain().Tip()->GetBlockTimeMax() + 5; + const int64_t BLOCK_TIME = WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip()->GetBlockTimeMax() + 5); SetMockTime(BLOCK_TIME); m_coinbase_txns.emplace_back(CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); m_coinbase_txns.emplace_back(CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); @@ -306,6 +306,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // Import key into wallet and call dumpwallet to create backup file. { + const CBlockIndex* tip{WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip())}; WalletContext context; context.args = &m_args; const std::shared_ptr wallet = std::make_shared(m_node.chain.get(), m_node.coinjoin_loader.get(), "", m_args, CreateDummyWalletDatabase()); @@ -316,7 +317,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) spk_man->AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()); AddWallet(context, wallet); - wallet->SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); + wallet->SetLastBlockProcessed(tip->nHeight, tip->GetBlockHash()); } JSONRPCRequest request; request.context = context; @@ -330,6 +331,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // Call importwallet RPC and verify all blocks with timestamps >= BLOCK_TIME // were scanned, and no prior blocks were scanned. { + const CBlockIndex* tip{WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip())}; const std::shared_ptr wallet = std::make_shared(m_node.chain.get(), m_node.coinjoin_loader.get(), "", m_args, CreateDummyWalletDatabase()); LOCK(wallet->cs_wallet); wallet->SetupLegacyScriptPubKeyMan(); @@ -341,7 +343,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) request.params.setArray(); request.params.push_back(backup_file); AddWallet(context, wallet); - wallet->SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); + wallet->SetLastBlockProcessed(tip->nHeight, tip->GetBlockHash()); wallet::importwallet().HandleRequest(request); RemoveWallet(context, wallet, /*load_on_start=*/std::nullopt); @@ -364,13 +366,13 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) { CWallet wallet(m_node.chain.get(), m_node.coinjoin_loader.get(), "", m_args, CreateDummyWalletDatabase()); - CWalletTx wtx{m_coinbase_txns.back(), TxStateConfirmed{m_node.chainman->ActiveChain().Tip()->GetBlockHash(), m_node.chainman->ActiveChain().Height(), /*index=*/0}}; - + const CBlockIndex* tip{WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip())}; LOCK(wallet.cs_wallet); + CWalletTx wtx{m_coinbase_txns.back(), TxStateConfirmed{tip->GetBlockHash(), tip->nHeight, /*index=*/0}}; wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); wallet.SetupDescriptorScriptPubKeyMans("", ""); - wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); + wallet.SetLastBlockProcessed(tip->nHeight, tip->GetBlockHash()); // Call GetImmatureCredit() once before adding the key to the wallet to // cache the current immature credit amount, which is 0. @@ -578,7 +580,7 @@ class ListCoinsTestingSetup : public TestChain100Setup ListCoinsTestingSetup() { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - wallet = CreateSyncedWallet(*m_node.chain, *m_node.coinjoin_loader, m_node.chainman->ActiveChain(), m_args, coinbaseKey); + wallet = CreateSyncedWallet(*m_node.chain, *m_node.coinjoin_loader, *Assert(m_node.chainman), m_args, coinbaseKey); } ~ListCoinsTestingSetup() @@ -603,11 +605,12 @@ class ListCoinsTestingSetup : public TestChain100Setup } CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); + const CBlockIndex* tip{WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip())}; LOCK(wallet->cs_wallet); - wallet->SetLastBlockProcessed(wallet->GetLastBlockHeight() + 1, m_node.chainman->ActiveChain().Tip()->GetBlockHash()); + wallet->SetLastBlockProcessed(tip->nHeight, tip->GetBlockHash()); auto it = wallet->mapWallet.find(tx->GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); - it->second.m_state = TxStateConfirmed{m_node.chainman->ActiveChain().Tip()->GetBlockHash(), m_node.chainman->ActiveChain().Height(), /*index=*/1}; + it->second.m_state = TxStateConfirmed{tip->GetBlockHash(), tip->nHeight, /*index=*/1}; return it->second; } @@ -1040,11 +1043,18 @@ class CreateTransactionTestSetup : public TestChain100Setup AddLegacyKey(*wallet, coinbaseKey); WalletRescanReserver reserver(*wallet); reserver.reserve(); + struct ChainInfo { + const CBlockIndex* tip; + uint256 genesis_hash; + }; + const auto chain_info{WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return (ChainInfo{ + m_node.chainman->ActiveChain().Tip(), + m_node.chainman->ActiveChain().Genesis()->GetBlockHash()}))}; { LOCK(wallet->cs_wallet); - wallet->SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); + wallet->SetLastBlockProcessed(chain_info.tip->nHeight, chain_info.tip->GetBlockHash()); } - CWallet::ScanResult result = wallet->ScanForWalletTransactions(m_node.chainman->ActiveChain().Genesis()->GetBlockHash(), /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false); + CWallet::ScanResult result = wallet->ScanForWalletTransactions(chain_info.genesis_hash, /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS); } @@ -1153,11 +1163,12 @@ class CreateTransactionTestSetup : public TestChain100Setup blocktx = CMutableTransaction(*tx); } CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); + const CBlockIndex* tip{WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip())}; LOCK(wallet->cs_wallet); auto it = wallet->mapWallet.find(tx->GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); - wallet->SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); - it->second.m_state = TxStateConfirmed{m_node.chainman->ActiveChain().Tip()->GetBlockHash(), m_node.chainman->ActiveChain().Height(), /*index=*/1}; + wallet->SetLastBlockProcessed(tip->nHeight, tip->GetBlockHash()); + it->second.m_state = TxStateConfirmed{tip->GetBlockHash(), tip->nHeight, /*index=*/1}; std::vector vecOutpoints; size_t n; @@ -1484,15 +1495,22 @@ BOOST_FIXTURE_TEST_CASE(select_coins_grouped_by_addresses, ListCoinsTestingSetup wallet->CommitTransaction(txr1, {}, {}); BOOST_CHECK_EQUAL(GetAvailableBalance(*wallet), 0); CreateAndProcessBlock({CMutableTransaction(*txr2)}, GetScriptForRawPubKey({})); + struct ChainInfo { + const CBlockIndex* tip; + uint256 genesis_hash; + }; + const auto chain_info{WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return (ChainInfo{ + m_node.chainman->ActiveChain().Tip(), + m_node.chainman->ActiveChain().Genesis()->GetBlockHash()}))}; { LOCK(wallet->cs_wallet); - wallet->SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); + wallet->SetLastBlockProcessed(chain_info.tip->nHeight, chain_info.tip->GetBlockHash()); } // Reveal the mined tx, it should conflict with the one we have in the wallet already. WalletRescanReserver reserver(*wallet); reserver.reserve(); - auto result = wallet->ScanForWalletTransactions(m_node.chainman->ActiveChain().Genesis()->GetBlockHash(), /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false); + auto result = wallet->ScanForWalletTransactions(chain_info.genesis_hash, /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS); { LOCK(wallet->cs_wallet); diff --git a/test/functional/feature_remove_pruned_files_on_startup.py b/test/functional/feature_remove_pruned_files_on_startup.py new file mode 100755 index 000000000000..569fa996f4bf --- /dev/null +++ b/test/functional/feature_remove_pruned_files_on_startup.py @@ -0,0 +1,56 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test removing undeleted pruned blk files on startup.""" + +import os +from test_framework.governance import EXPECTED_STDERR_NO_GOV_PRUNE +from test_framework.test_framework import BitcoinTestFramework + +class FeatureRemovePrunedFilesOnStartupTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + self.extra_args = [["-fastprune", "-prune=1"]] + + def mine_batches(self, blocks): + n = blocks // 250 + for _ in range(n): + self.generate(self.nodes[0], 250) + self.generate(self.nodes[0], blocks % 250) + self.sync_blocks() + + def run_test(self): + blk0 = os.path.join(self.nodes[0].datadir, self.nodes[0].chain, 'blocks', 'blk00000.dat') + rev0 = os.path.join(self.nodes[0].datadir, self.nodes[0].chain, 'blocks', 'rev00000.dat') + blk1 = os.path.join(self.nodes[0].datadir, self.nodes[0].chain, 'blocks', 'blk00001.dat') + rev1 = os.path.join(self.nodes[0].datadir, self.nodes[0].chain, 'blocks', 'rev00001.dat') + self.mine_batches(800) + fo1 = os.open(blk0, os.O_RDONLY) + fo2 = os.open(rev1, os.O_RDONLY) + fd1 = os.fdopen(fo1) + fd2 = os.fdopen(fo2) + self.nodes[0].pruneblockchain(600) + + # Windows systems will not remove files with an open fd + if os.name != 'nt': + assert not os.path.exists(blk0) + assert not os.path.exists(rev0) + assert not os.path.exists(blk1) + assert not os.path.exists(rev1) + else: + assert os.path.exists(blk0) + assert not os.path.exists(rev0) + assert not os.path.exists(blk1) + assert os.path.exists(rev1) + + # Check that the files are removed on restart once the fds are closed + fd1.close() + fd2.close() + self.restart_node(0, expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE) + assert not os.path.exists(blk0) + assert not os.path.exists(rev1) + self.stop_node(0, expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE) + +if __name__ == '__main__': + FeatureRemovePrunedFilesOnStartupTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index e303013a6b75..966827a6f35b 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -405,6 +405,7 @@ 'p2p_permissions.py', 'feature_blocksdir.py', 'wallet_startup.py', + 'feature_remove_pruned_files_on_startup.py', 'p2p_i2p_ports.py', 'p2p_i2p_sessions.py', 'feature_config_args.py',