diff --git a/src/evo/evonotificationinterface.cpp b/src/evo/evonotificationinterface.cpp index 25056f970192a..cecb97b68d70c 100644 --- a/src/evo/evonotificationinterface.cpp +++ b/src/evo/evonotificationinterface.cpp @@ -27,7 +27,7 @@ void EvoNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, con // background thread updates llmq::chainLocksHandler->UpdatedBlockTip(pindexNew, pindexFork); llmq::quorumDKGSessionManager->UpdatedBlockTip(pindexNew, fInitialDownload); - llmq::quorumManager->UpdatedBlockTip(pindexNew, pindexFork, fInitialDownload); + llmq::quorumManager->UpdatedBlockTip(pindexNew, fInitialDownload); } void EvoNotificationInterface::NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& oldMNList, const CDeterministicMNListDiff& diff) diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index 2105a6005f078..4c60db513a799 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -43,7 +43,9 @@ CQuorum::~CQuorum() { // most likely the thread is already done stopCachePopulatorThread = true; - if (cachePopulatorThread.joinable()) { + // watch out to not join the thread when we're called from inside the thread, which might happen on shutdown. This + // is because on shutdown the thread is the last owner of the shared CQuorum instance and thus the destroyer of it. + if (cachePopulatorThread.joinable() && cachePopulatorThread.get_id() != std::this_thread::get_id()) { cachePopulatorThread.join(); } } @@ -158,18 +160,16 @@ CQuorumManager::CQuorumManager(CEvoDB& _evoDb, CBLSWorker& _blsWorker, CDKGSessi { } -void CQuorumManager::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) +void CQuorumManager::UpdatedBlockTip(const CBlockIndex* pindexNew, bool fInitialDownload) { if (fInitialDownload || !activeMasternodeManager || !deterministicMNManager->IsDIP3Enforced(pindexNew->nHeight)) { return; } - LOCK(cs_main); - for (auto& p : Params().GetConsensus().llmqs) { const auto& params = Params().GetConsensus().llmqs.at(p.first); - auto lastQuorums = ScanQuorums(p.first, (size_t)params.keepOldConnections); + auto lastQuorums = ScanQuorums(p.first, pindexNew, (size_t)params.keepOldConnections); llmq::EnsureLatestQuorumConnections(p.first, pindexNew, activeMasternodeManager->GetProTx(), lastQuorums); } @@ -178,14 +178,9 @@ void CQuorumManager::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockI bool CQuorumManager::BuildQuorumFromCommitment(const CFinalCommitment& qc, const CBlockIndex* pindexQuorum, const uint256& minedBlockHash, std::shared_ptr& quorum) const { - AssertLockHeld(cs_main); + assert(pindexQuorum); + assert(qc.quorumHash == pindexQuorum->GetBlockHash()); - if (!mapBlockIndex.count(qc.quorumHash)) { - LogPrintf("CQuorumManager::%s -- block %s not found", __func__, qc.quorumHash.ToString()); - return false; - } - auto& params = Params().GetConsensus().llmqs.at((Consensus::LLMQType)qc.llmqType); - auto quorumIndex = mapBlockIndex[qc.quorumHash]; auto members = deterministicMNManager->GetAllQuorumMembers((Consensus::LLMQType)qc.llmqType, pindexQuorum); quorum->Init(minedBlockHash, pindexQuorum, members, qc.validMembers, qc.quorumPublicKey); @@ -197,7 +192,7 @@ bool CQuorumManager::BuildQuorumFromCommitment(const CFinalCommitment& qc, const quorum->WriteContributions(evoDb); hasValidVvec = true; } else { - LogPrintf("CQuorumManager::%s -- quorum.ReadContributions and BuildQuorumContributions for block %s failed", __func__, qc.quorumHash.ToString()); + LogPrintf("CQuorumManager::%s -- quorum.ReadContributions and BuildQuorumContributions for block %s failed\n", __func__, qc.quorumHash.ToString()); } } @@ -254,33 +249,22 @@ bool CQuorumManager::HasQuorum(Consensus::LLMQType llmqType, const uint256& quor std::vector CQuorumManager::ScanQuorums(Consensus::LLMQType llmqType, size_t maxCount) { - LOCK(cs_main); - return ScanQuorums(llmqType, chainActive.Tip()->GetBlockHash(), maxCount); + const CBlockIndex* pindex = WITH_LOCK(cs_main, return chainActive.Tip()); + return ScanQuorums(llmqType, pindex, maxCount); } -std::vector CQuorumManager::ScanQuorums(Consensus::LLMQType llmqType, const uint256& startBlock, size_t maxCount) +std::vector CQuorumManager::ScanQuorums(Consensus::LLMQType llmqType, const CBlockIndex* pindexStart, size_t maxCount) { + // TODO: speed up even more by using a cache + auto& params = Params().GetConsensus().llmqs.at(llmqType); + auto quorumIndexes = quorumBlockProcessor->GetMinedCommitmentsUntilBlock(params.type, pindexStart, maxCount); std::vector result; - - LOCK(cs_main); - if (!mapBlockIndex.count(startBlock)) { - return result; - } - - result.reserve(maxCount); - - CBlockIndex* pindex = mapBlockIndex[startBlock]; - - while (pindex != NULL && result.size() < maxCount && deterministicMNManager->IsDIP3Enforced(pindex->nHeight)) { - if (HasQuorum(llmqType, pindex->GetBlockHash())) { - auto quorum = GetQuorum(llmqType, pindex->GetBlockHash()); - if (quorum) { - result.emplace_back(quorum); - } - } - - // TODO speedup (skip blocks where no quorums could have been mined) - pindex = pindex->pprev; + result.reserve(quorumIndexes.size()); + for (auto& quorumIndex : quorumIndexes) { + assert(quorumIndex); + auto quorum = GetQuorum(params.type, quorumIndex); + assert(quorum != nullptr); + result.emplace_back(quorum); } return result; @@ -291,13 +275,12 @@ CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, const uint25 CBlockIndex* pindexQuorum; { LOCK(cs_main); - auto quorumIt = mapBlockIndex.find(quorumHash); + pindexQuorum = LookupBlockIndex(quorumHash); - if (quorumIt == mapBlockIndex.end()) { + if (!pindexQuorum) { LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- block %s not found", __func__, quorumHash.ToString()); return nullptr; } - pindexQuorum = quorumIt->second; } return GetQuorum(llmqType, pindexQuorum); } @@ -338,13 +321,4 @@ CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, const CBlock return quorum; } -CQuorumCPtr CQuorumManager::GetNewestQuorum(Consensus::LLMQType llmqType) -{ - auto quorums = ScanQuorums(llmqType, 1); - if (quorums.empty()) { - return nullptr; - } - return quorums.front(); -} - } // namespace llmq diff --git a/src/llmq/quorums.h b/src/llmq/quorums.h index 1aca3ef8f9319..bb9f747d24524 100644 --- a/src/llmq/quorums.h +++ b/src/llmq/quorums.h @@ -89,24 +89,27 @@ class CQuorumManager public: CQuorumManager(CEvoDB& _evoDb, CBLSWorker& _blsWorker, CDKGSessionManager& _dkgManager); - void UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload); + void UpdatedBlockTip(const CBlockIndex* pindexNew, bool fInitialDownload); public: bool HasQuorum(Consensus::LLMQType llmqType, const uint256& quorumHash); // all these methods will lock cs_main CQuorumCPtr GetQuorum(Consensus::LLMQType llmqType, const uint256& quorumHash); - CQuorumCPtr GetQuorum(Consensus::LLMQType llmqType, const CBlockIndex* pindexQuorum); - CQuorumCPtr GetNewestQuorum(Consensus::LLMQType llmqType); std::vector ScanQuorums(Consensus::LLMQType llmqType, size_t maxCount); - std::vector ScanQuorums(Consensus::LLMQType llmqType, const uint256& startBlock, size_t maxCount); + + // this method will not lock cs_main + std::vector ScanQuorums(Consensus::LLMQType llmqType, const CBlockIndex* pindexStart, size_t maxCount); private: + // all these methods will not lock cs_main void EnsureQuorumConnections(Consensus::LLMQType llmqType, const CBlockIndex* pindexNew); bool BuildQuorumFromCommitment(const CFinalCommitment& qc, const CBlockIndex* pindexQuorum, const uint256& minedBlockHash, std::shared_ptr& quorum) const; bool BuildQuorumContributions(const CFinalCommitment& fqc, std::shared_ptr& quorum) const; + + CQuorumCPtr GetQuorum(Consensus::LLMQType llmqType, const CBlockIndex* pindexQuorum); }; extern std::unique_ptr quorumManager; diff --git a/src/llmq/quorums_connections.cpp b/src/llmq/quorums_connections.cpp index e39b6e68c2b81..f3764d7eeac36 100644 --- a/src/llmq/quorums_connections.cpp +++ b/src/llmq/quorums_connections.cpp @@ -109,8 +109,6 @@ std::set CalcDeterministicWatchConnections(Consensus::LLMQType llmqType, // ensure connection to a given list of quorums void EnsureLatestQuorumConnections(Consensus::LLMQType llmqType, const CBlockIndex* pindexNew, const uint256& myProTxHash, std::vector& lastQuorums) { - AssertLockHeld(cs_main); - const auto& params = Params().GetConsensus().llmqs.at(llmqType); auto connman = g_connman->GetTierTwoConnMan(); @@ -118,7 +116,7 @@ void EnsureLatestQuorumConnections(Consensus::LLMQType llmqType, const CBlockInd // don't remove connections for the currently in-progress DKG round int curDkgHeight = pindexNew->nHeight - (pindexNew->nHeight % params.dkgInterval); - auto curDkgBlock = chainActive[curDkgHeight]->GetBlockHash(); + auto curDkgBlock = pindexNew->GetAncestor(curDkgHeight)->GetBlockHash(); connmanQuorumsToDelete.erase(curDkgBlock); for (auto& quorum : lastQuorums) { diff --git a/src/llmq/quorums_signing.cpp b/src/llmq/quorums_signing.cpp index 6a8a97b21957a..1cc125d5e6816 100644 --- a/src/llmq/quorums_signing.cpp +++ b/src/llmq/quorums_signing.cpp @@ -550,16 +550,16 @@ CQuorumCPtr CSigningManager::SelectQuorumForSigning(Consensus::LLMQType llmqType auto& llmqParams = Params().GetConsensus().llmqs.at(llmqType); size_t poolSize = (size_t)llmqParams.signingActiveQuorumCount; - uint256 startBlock; + CBlockIndex* pindexStart; { LOCK(cs_main); if (signHeight > chainActive.Height()) { return nullptr; } - startBlock = chainActive[signHeight - SIGN_HEIGHT_OFFSET]->GetBlockHash(); + pindexStart = chainActive[signHeight - SIGN_HEIGHT_OFFSET]; } - auto quorums = quorumManager->ScanQuorums(llmqType, startBlock, poolSize); + auto quorums = quorumManager->ScanQuorums(llmqType, pindexStart, poolSize); if (quorums.empty()) { return nullptr; } diff --git a/src/llmq/quorums_utils.cpp b/src/llmq/quorums_utils.cpp index 7203b190e9bbe..5259a6eb10390 100644 --- a/src/llmq/quorums_utils.cpp +++ b/src/llmq/quorums_utils.cpp @@ -50,7 +50,6 @@ std::string ToHexStr(const std::vector& vBits) bool IsQuorumActive(Consensus::LLMQType llmqType, const uint256& quorumHash) { - AssertLockHeld(cs_main); auto& params = Params().GetConsensus().llmqs.at(llmqType); diff --git a/src/rpc/rpcquorums.cpp b/src/rpc/rpcquorums.cpp index 6062f719414ab..e0cd374f03744 100644 --- a/src/rpc/rpcquorums.cpp +++ b/src/rpc/rpcquorums.cpp @@ -138,7 +138,7 @@ UniValue listquorums(const JSONRPCRequest& request) for (auto& p : Params().GetConsensus().llmqs) { UniValue v(UniValue::VARR); - auto quorums = llmq::quorumManager->ScanQuorums(p.first, chainActive.Tip()->GetBlockHash(), count); + auto quorums = llmq::quorumManager->ScanQuorums(p.first, chainActive.Tip(), count); for (auto& q : quorums) { v.push_back(q->pindexQuorum->GetBlockHash().ToString()); } diff --git a/test/functional/tiertwo_chainlocks.py b/test/functional/tiertwo_chainlocks.py index 5831f18afc476..1fab530eda18d 100755 --- a/test/functional/tiertwo_chainlocks.py +++ b/test/functional/tiertwo_chainlocks.py @@ -35,6 +35,11 @@ def run_test(self): (qfc, badmembers) = self.mine_quorum() assert_equal(171, miner.getblockcount()) + # in order to avoid sync issues signing sessions quorum selection looks for quorums mined at most at chaintip - 8 blocks. + # let's round it and mine 10 blocks + miner.generate(10) + self.sync_all() + # mine single block, wait for chainlock self.nodes[0].generate(1) self.wait_for_chainlock_tip_all_nodes() diff --git a/test/functional/tiertwo_signing_session.py b/test/functional/tiertwo_signing_session.py index d166d06d11af5..adb2279fe0ba7 100755 --- a/test/functional/tiertwo_signing_session.py +++ b/test/functional/tiertwo_signing_session.py @@ -30,6 +30,12 @@ def run_test(self): self.log.info("----------------------------------") (qfc, badmembers) = self.mine_quorum() assert_equal(171, miner.getblockcount()) + + # in order to avoid sync issues signing sessions quorum selection looks for quorums mined at most at chaintip - 8 blocks. + # let's round it and mine 10 blocks + miner.generate(10) + self.sync_all() + # Quorum members members = self.get_quorum_members(qfc['quorumHash'])