From a857d3411974355c5fa4a338c063c46b46c60d32 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 23ac3bf33dced..3a4e33dad95f0 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -243,7 +243,7 @@ void CQuorumManager::UpdatedBlockTip(const CBlockIndex* pindexNew, bool fInitial } for (auto& params : Params().GetConsensus().llmqs) { - EnsureQuorumConnections(params, pindexNew); + CheckQuorumConnections(params, pindexNew); } { @@ -262,7 +262,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); @@ -289,11 +289,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, WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash))) { + if (CLLMQUtils::EnsureQuorumConnections(llmqParams, quorum->m_quorum_base_block_index, 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 (!g_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()); + g_connman->SetMasternodeQuorumNodes(llmqParams.type, quorum->m_quorum_base_block_index->GetBlockHash(), connections); + g_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 5bdb9e3f0d86d..815de674ce394 100644 --- a/src/llmq/quorums.h +++ b/src/llmq/quorums.h @@ -224,7 +224,7 @@ class CQuorumManager 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 9481b8d7d3a5c..f4aef9474effe 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, const uint256& myProTxHash) { + if (!fMasternodeMode && !CLLMQUtils::IsWatchQuorumsEnabled()) { + return false; + } + auto members = GetAllQuorumMembers(llmqParams.type, pQuorumBaseBlockIndex); + if (members.empty()) { + return false; + } + bool isMember = std::find_if(members.begin(), members.end(), [&](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 debc06dad65f3..9a90cd65ca427 100755 --- a/test/functional/feature_llmq_connections.py +++ b/test/functional/feature_llmq_connections.py @@ -69,6 +69,20 @@ def run_test(self): self.check_reconnects(4) + 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: