From 052f6c817a285ee004edff753d1c9c1a570576f3 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 3 Apr 2023 18:35:22 +0300 Subject: [PATCH 1/2] refactor: make it a bit easier to use CQuorumDataRequestKey --- src/llmq/quorums.cpp | 30 +++++------------------------- src/llmq/quorums.h | 14 ++++++++++---- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index 522e60d0a1d9..b4b2926f80d9 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -468,11 +468,7 @@ bool CQuorumManager::RequestQuorumData(CNode* pfrom, Consensus::LLMQType llmqTyp } LOCK(cs_data_requests); - CQuorumDataRequestKey key; - key.proRegTx = pfrom->GetVerifiedProRegTxHash(); - key.flag = true; - key.quorumHash = pQuorumBaseBlockIndex->GetBlockHash(); - key.llmqType = llmqType; + const CQuorumDataRequestKey key(pfrom->GetVerifiedProRegTxHash(), true, pQuorumBaseBlockIndex->GetBlockHash(), llmqType); auto it = mapQuorumDataRequests.emplace(key, CQuorumDataRequest(llmqType, pQuorumBaseBlockIndex->GetBlockHash(), nDataMask, proTxHash)); if (!it.second && !it.first->second.IsExpired(/*add_bias=*/true)) { LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- Already requested\n", __func__); @@ -656,11 +652,7 @@ void CQuorumManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, C bool request_limit_exceeded(false); { LOCK2(cs_main, cs_data_requests); - CQuorumDataRequestKey key; - key.proRegTx = pfrom.GetVerifiedProRegTxHash(); - key.flag = false; - key.quorumHash = request.GetQuorumHash(); - key.llmqType = request.GetLLMQType(); + const CQuorumDataRequestKey key(pfrom.GetVerifiedProRegTxHash(), false, request.GetQuorumHash(), request.GetLLMQType()); auto it = mapQuorumDataRequests.find(key); if (it == mapQuorumDataRequests.end()) { it = mapQuorumDataRequests.emplace(key, request).first; @@ -733,11 +725,7 @@ void CQuorumManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, C { LOCK2(cs_main, cs_data_requests); - CQuorumDataRequestKey key; - key.proRegTx = pfrom.GetVerifiedProRegTxHash(); - key.flag = true; - key.quorumHash = request.GetQuorumHash(); - key.llmqType = request.GetLLMQType(); + const CQuorumDataRequestKey key(pfrom.GetVerifiedProRegTxHash(), true, request.GetQuorumHash(), request.GetLLMQType()); auto it = mapQuorumDataRequests.find(key); if (it == mapQuorumDataRequests.end()) { errorHandler("Not requested"); @@ -913,11 +901,7 @@ void CQuorumManager::StartQuorumDataRecoveryThread(const CQuorumCPtr pQuorum, co pCurrentMemberHash = &vecMemberHashes[(nMyStartOffset + nTries++) % vecMemberHashes.size()]; { LOCK(cs_data_requests); - CQuorumDataRequestKey key; - key.proRegTx = *pCurrentMemberHash; - key.flag = true; - key.quorumHash = pQuorum->qc->quorumHash; - key.llmqType = pQuorum->qc->llmqType; + const CQuorumDataRequestKey key(*pCurrentMemberHash, true, pQuorum->qc->quorumHash, pQuorum->qc->llmqType); auto it = mapQuorumDataRequests.find(key); if (it != mapQuorumDataRequests.end() && !it->second.IsExpired(/*add_bias=*/true)) { printLog("Already asked"); @@ -943,11 +927,7 @@ void CQuorumManager::StartQuorumDataRecoveryThread(const CQuorumCPtr pQuorum, co printLog("Requested"); } else { LOCK(cs_data_requests); - CQuorumDataRequestKey key; - key.proRegTx = *pCurrentMemberHash; - key.flag = true; - key.quorumHash = pQuorum->qc->quorumHash; - key.llmqType = pQuorum->qc->llmqType; + const CQuorumDataRequestKey key(*pCurrentMemberHash, true, pQuorum->qc->quorumHash, pQuorum->qc->llmqType); auto it = mapQuorumDataRequests.find(key); if (it == mapQuorumDataRequests.end()) { printLog("Failed"); diff --git a/src/llmq/quorums.h b/src/llmq/quorums.h index 6e24d369d1fd..c254393322a9 100644 --- a/src/llmq/quorums.h +++ b/src/llmq/quorums.h @@ -39,14 +39,20 @@ static constexpr bool DEFAULT_WATCH_QUORUMS{false}; struct CQuorumDataRequestKey { uint256 proRegTx; - //TODO: Investigate purpose of this flag and rename accordingly - bool flag; + bool m_we_requested; uint256 quorumHash; Consensus::LLMQType llmqType; + CQuorumDataRequestKey(const uint256& proRegTxIn, const bool _m_we_requested, const uint256& quorumHashIn, const Consensus::LLMQType llmqTypeIn) : + proRegTx(proRegTxIn), + m_we_requested(_m_we_requested), + quorumHash(quorumHashIn), + llmqType(llmqTypeIn) + {} + bool operator ==(const CQuorumDataRequestKey& obj) const { - return (proRegTx == obj.proRegTx && flag == obj.flag && quorumHash == obj.quorumHash && llmqType == obj.llmqType); + return (proRegTx == obj.proRegTx && m_we_requested == obj.m_we_requested && quorumHash == obj.quorumHash && llmqType == obj.llmqType); } }; @@ -275,7 +281,7 @@ struct SaltedHasherImpl { CSipHasher c(k0, k1); c.Write((unsigned char*)&(v.proRegTx), sizeof(v.proRegTx)); - c.Write((unsigned char*)&(v.flag), sizeof(v.flag)); + c.Write((unsigned char*)&(v.m_we_requested), sizeof(v.m_we_requested)); c.Write((unsigned char*)&(v.quorumHash), sizeof(v.quorumHash)); c.Write((unsigned char*)&(v.llmqType), sizeof(v.llmqType)); return c.Finalize(); From f4e910717907b442a5cae1cf83651a93b664de63 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 3 Apr 2023 18:38:38 +0300 Subject: [PATCH 2/2] fix: replace expired requests with a new one in RequestQuorumData --- src/llmq/quorums.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index b4b2926f80d9..b3a8998258b7 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -469,16 +469,21 @@ bool CQuorumManager::RequestQuorumData(CNode* pfrom, Consensus::LLMQType llmqTyp LOCK(cs_data_requests); const CQuorumDataRequestKey key(pfrom->GetVerifiedProRegTxHash(), true, pQuorumBaseBlockIndex->GetBlockHash(), llmqType); - auto it = mapQuorumDataRequests.emplace(key, CQuorumDataRequest(llmqType, pQuorumBaseBlockIndex->GetBlockHash(), nDataMask, proTxHash)); - if (!it.second && !it.first->second.IsExpired(/*add_bias=*/true)) { - LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- Already requested\n", __func__); - return false; + const CQuorumDataRequest request(llmqType, pQuorumBaseBlockIndex->GetBlockHash(), nDataMask, proTxHash); + auto [old_pair, exists] = mapQuorumDataRequests.emplace(key, request); + if (!exists) { + if (old_pair->second.IsExpired(/*add_bias=*/true)) { + old_pair->second = request; + } else { + LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- Already requested\n", __func__); + return false; + } } LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- sending QGETDATA quorumHash[%s] llmqType[%d] proRegTx[%s]\n", __func__, key.quorumHash.ToString(), ToUnderlying(key.llmqType), key.proRegTx.ToString()); CNetMsgMaker msgMaker(pfrom->GetSendVersion()); - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::QGETDATA, it.first->second)); + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::QGETDATA, request)); return true; }