From 79a5b197b0b3fa01c69d671e1d54fe824f7c20d7 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 4 Apr 2023 19:07:00 +0300 Subject: [PATCH] refactor/fix: replace expired requests with a new one in RequestQuorumData (#5286) ## Issue being fixed or feature implemented should fix "qdata: Already received" discouraging issue the root of the issue is that we remove expired requests on UpdatedBlockTip which is too late sometimes. ## What was done? replacing expired requests with a new one in RequestQuorumData kind of does the same (drops the expired request) but without waiting for UpdatedBlockTip ## How Has This Been Tested? ## Breaking Changes ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation **For repository code-owners and collaborators only** - [ ] I have assigned this pull request to a milestone --- src/llmq/quorums.cpp | 45 +++++++++++++++----------------------------- src/llmq/quorums.h | 14 ++++++++++---- 2 files changed, 25 insertions(+), 34 deletions(-) diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index 522e60d0a1..b3a8998258 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -468,21 +468,22 @@ 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; - 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 CQuorumDataRequestKey key(pfrom->GetVerifiedProRegTxHash(), true, pQuorumBaseBlockIndex->GetBlockHash(), llmqType); + 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; } @@ -656,11 +657,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 +730,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 +906,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 +932,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 6e24d369d1..c254393322 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();