From 666859b47f14e6a11d428ec48eab5046ee3ba15c Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 18 Jul 2022 22:26:51 +0300 Subject: [PATCH] feat(llmq): Ensure connections between IS quorums (#4917) * fix(llmq): Ensure connections between quorums Every masternode will now "watch" a single node from _every other_ quorum in addition to intra-quorum connections. This should make propagation of recsigs produced by one quorum to other quorums much more reliable. * fix: Do this only for masternodes which participate in IS quorums * refactor: rename `CQuorumManager::EnsureQuorumConnections` to better match the actual behaviour (and avoid confusion with `CLLMQUtils::EnsureQuorumConnections`) * refactor: move IS quorums watch logic into `CQuorumManager::CheckQuorumConnections` avoid calling slow `ScanQuorums` (no caching atm) inside the loop * tests: check that inter-quorum connections are added * use `ranges::any_of` --- src/llmq/quorums.cpp | 31 +++++++++++++++++++-- src/llmq/quorums.h | 2 +- src/llmq/utils.cpp | 11 ++++++++ test/functional/feature_llmq_connections.py | 14 ++++++++++ 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index 3f7e717298..0f6271f6a7 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -272,7 +272,7 @@ void CQuorumManager::UpdatedBlockTip(const CBlockIndex* pindexNew, bool fInitial } for (auto& params : Params().GetConsensus().llmqs) { - EnsureQuorumConnections(params, pindexNew); + CheckQuorumConnections(params, pindexNew); } { @@ -291,7 +291,7 @@ void CQuorumManager::UpdatedBlockTip(const CBlockIndex* pindexNew, bool fInitial TriggerQuorumDataRecoveryThreads(pindexNew); } -void CQuorumManager::EnsureQuorumConnections(const Consensus::LLMQParams& llmqParams, const CBlockIndex* pindexNew) const +void CQuorumManager::CheckQuorumConnections(const Consensus::LLMQParams& llmqParams, const CBlockIndex* pindexNew) const { auto lastQuorums = ScanQuorums(llmqParams.type, pindexNew, (size_t)llmqParams.keepOldConnections); @@ -318,11 +318,36 @@ void CQuorumManager::EnsureQuorumConnections(const Consensus::LLMQParams& llmqPa LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- llmqType[%d] h[%d] keeping mn quorum connections for quorum: [%d:%s]\n", __func__, int(llmqParams.type), pindexNew->nHeight, curDkgHeight, curDkgBlock.ToString()); } + const auto myProTxHash = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash); + bool isISType = llmqParams.type == Params().GetConsensus().llmqTypeInstantSend || + llmqParams.type == Params().GetConsensus().llmqTypeDIP0024InstantSend; + + bool watchOtherISQuorums = isISType && !myProTxHash.IsNull() && + ranges::any_of(lastQuorums, [&myProTxHash](const auto& old_quorum){ + return old_quorum->IsMember(myProTxHash); + }); + for (const auto& quorum : lastQuorums) { - if (CLLMQUtils::EnsureQuorumConnections(llmqParams, quorum->m_quorum_base_block_index, connman, WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash))) { + if (CLLMQUtils::EnsureQuorumConnections(llmqParams, quorum->m_quorum_base_block_index, connman, myProTxHash)) { if (connmanQuorumsToDelete.erase(quorum->qc->quorumHash) > 0) { LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- llmqType[%d] h[%d] keeping mn quorum connections for quorum: [%d:%s]\n", __func__, int(llmqParams.type), pindexNew->nHeight, quorum->m_quorum_base_block_index->nHeight, quorum->m_quorum_base_block_index->GetBlockHash().ToString()); } + } else if (watchOtherISQuorums && !quorum->IsMember(myProTxHash)) { + std::set connections; + const auto& cindexes = CLLMQUtils::CalcDeterministicWatchConnections(llmqParams.type, quorum->m_quorum_base_block_index, quorum->members.size(), 1); + for (auto idx : cindexes) { + connections.emplace(quorum->members[idx]->proTxHash); + } + if (!connections.empty()) { + if (!connman.HasMasternodeQuorumNodes(llmqParams.type, quorum->m_quorum_base_block_index->GetBlockHash())) { + LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- llmqType[%d] h[%d] adding mn inter-quorum connections for quorum: [%d:%s]\n", __func__, int(llmqParams.type), pindexNew->nHeight, quorum->m_quorum_base_block_index->nHeight, quorum->m_quorum_base_block_index->GetBlockHash().ToString()); + connman.SetMasternodeQuorumNodes(llmqParams.type, quorum->m_quorum_base_block_index->GetBlockHash(), connections); + connman.SetMasternodeQuorumRelayMembers(llmqParams.type, quorum->m_quorum_base_block_index->GetBlockHash(), connections); + } + if (connmanQuorumsToDelete.erase(quorum->qc->quorumHash) > 0) { + LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- llmqType[%d] h[%d] keeping mn inter-quorum connections for quorum: [%d:%s]\n", __func__, int(llmqParams.type), pindexNew->nHeight, quorum->m_quorum_base_block_index->nHeight, quorum->m_quorum_base_block_index->GetBlockHash().ToString()); + } + } } } for (const auto& quorumHash : connmanQuorumsToDelete) { diff --git a/src/llmq/quorums.h b/src/llmq/quorums.h index 0f40517f7f..5fd69fef25 100644 --- a/src/llmq/quorums.h +++ b/src/llmq/quorums.h @@ -227,7 +227,7 @@ public: private: // all private methods here are cs_main-free - void EnsureQuorumConnections(const Consensus::LLMQParams& llmqParams, const CBlockIndex *pindexNew) const; + void CheckQuorumConnections(const Consensus::LLMQParams& llmqParams, const CBlockIndex *pindexNew) const; CQuorumPtr BuildQuorumFromCommitment(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex) const EXCLUSIVE_LOCKS_REQUIRED(quorumsCacheCs); bool BuildQuorumContributions(const CFinalCommitmentPtr& fqc, const std::shared_ptr& quorum) const; diff --git a/src/llmq/utils.cpp b/src/llmq/utils.cpp index a35745f4d4..0598bac8cb 100644 --- a/src/llmq/utils.cpp +++ b/src/llmq/utils.cpp @@ -723,13 +723,24 @@ std::set CLLMQUtils::CalcDeterministicWatchConnections(Consensus::LLMQTy bool CLLMQUtils::EnsureQuorumConnections(const Consensus::LLMQParams& llmqParams, const CBlockIndex* pQuorumBaseBlockIndex, CConnman& connman, const uint256& myProTxHash) { + if (!fMasternodeMode && !CLLMQUtils::IsWatchQuorumsEnabled()) { + return false; + } + auto members = GetAllQuorumMembers(llmqParams.type, pQuorumBaseBlockIndex); + if (members.empty()) { + return false; + } + bool isMember = ranges::find_if(members, [&](const auto& dmn) { return dmn->proTxHash == myProTxHash; }) != members.end(); if (!isMember && !CLLMQUtils::IsWatchQuorumsEnabled()) { return false; } + LogPrint(BCLog::NET_NETCONN, "CLLMQUtils::%s -- isMember=%d for quorum %s:\n", + __func__, isMember, pQuorumBaseBlockIndex->GetBlockHash().ToString()); + std::set connections; std::set relayMembers; if (isMember) { diff --git a/test/functional/feature_llmq_connections.py b/test/functional/feature_llmq_connections.py index 8ce9a61fe0..712851d49e 100755 --- a/test/functional/feature_llmq_connections.py +++ b/test/functional/feature_llmq_connections.py @@ -85,6 +85,20 @@ class LLMQConnections(DashTestFramework): break assert removed # no way we removed none + self.log.info("check that inter-quorum masternode conections are added") + added = False + for mn in self.mninfo: + if len(mn.node.quorum("memberof", mn.proTxHash)) > 0: + try: + with mn.node.assert_debug_log(['adding mn inter-quorum connections']): + self.mine_quorum() + added = True + except: + pass # it's ok to not add connections sometimes + if added: + break + assert added # no way we added none + def check_reconnects(self, expected_connection_count): self.log.info("disable and re-enable networking on all masternodes") for mn in self.mninfo: