chore: apply review suggestions

kwvg · kwvg · commit c976c7c3552c · 2026-01-02T23:44:07.000+05:30
diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp
@@ -26,7 +26,7 @@ uint256 MakeQuorumKey(const CQuorum& q)
     return hw.GetHash();
 }
 
-void DataCleanupHelper(CDBWrapper& db, std::set<uint256> skip_list, bool compact)
+void DataCleanupHelper(CDBWrapper& db, const std::set<uint256>& skip_list, bool compact)
 {
     const auto prefixes = {DB_QUORUM_QUORUM_VVEC, DB_QUORUM_SK_SHARE};
 
@@ -124,7 +124,7 @@ bool CQuorum::SetVerificationVector(const std::vector<CBLSPublicKey>& quorumVecI
 
 bool CQuorum::SetSecretKeyShare(const CBLSSecretKey& secretKeyShare, const uint256& protx_hash)
 {
-    if (!secretKeyShare.IsValid() || (secretKeyShare.GetPublicKey() != GetPubKeyShare(GetMemberIndex(protx_hash)))) {
+    if (protx_hash.IsNull() || !secretKeyShare.IsValid() || (secretKeyShare.GetPublicKey() != GetPubKeyShare(GetMemberIndex(protx_hash)))) {
         return false;
     }
     LOCK(cs_vvec_shShare);
diff --git a/src/llmq/quorums.h b/src/llmq/quorums.h
@@ -17,6 +17,7 @@
 #include <uint256.h>
 #include <util/time.h>
 
+#include <set>
 #include <string>
 #include <vector>
 
@@ -34,7 +35,7 @@ extern const std::string DB_QUORUM_SK_SHARE;
 extern const std::string DB_QUORUM_QUORUM_VVEC;
 
 uint256 MakeQuorumKey(const CQuorum& q);
-void DataCleanupHelper(CDBWrapper& db, std::set<uint256> skip_list, bool compact = false);
+void DataCleanupHelper(CDBWrapper& db, const std::set<uint256>& skip_list, bool compact = false);
 
 /**
  * Object used as a key to store CQuorumDataRequest
diff --git a/src/llmq/quorumsman.cpp b/src/llmq/quorumsman.cpp
@@ -481,16 +481,22 @@ MessageProcessingResult CQuorumManager::ProcessMessage(CNode& pfrom, CConnman& c
         }
 
         // Check if request wants ENCRYPTED_CONTRIBUTIONS data
-        MessageProcessingResult ret{};
+        CQuorumDataRequest::Errors ret_err{CQuorumDataRequest::Errors::NONE};
+        MessageProcessingResult qdata_ret{}, ret{};
         if (m_handler) {
             ret = m_handler->ProcessContribQGETDATA(request_limit_exceeded, ssResponseData, *pQuorum, request, pQuorumBaseBlockIndex);
             if (auto request_err = request.GetError(); request_err != CQuorumDataRequest::Errors::NONE &&
                                                        request_err != CQuorumDataRequest::Errors::UNDEFINED) {
-                auto qdata_ret = sendQDATA(request_err, request_limit_exceeded);
-                return ret.empty() ? qdata_ret : ret;
+                ret_err = request_err;
             }
         }
-        return ret.empty() ? sendQDATA(CQuorumDataRequest::Errors::NONE, request_limit_exceeded, ssResponseData) : ret;
+        // sendQDATA also pushes a message independent of the returned value
+        if (ret_err != CQuorumDataRequest::Errors::NONE) {
+            qdata_ret = sendQDATA(ret_err, request_limit_exceeded);
+        } else {
+            qdata_ret = sendQDATA(CQuorumDataRequest::Errors::NONE, request_limit_exceeded, ssResponseData);
+        }
+        return ret.empty() ? qdata_ret : ret;
     }
 
     if (msg_type == NetMsgType::QDATA) {
@@ -518,7 +524,7 @@ MessageProcessingResult CQuorumManager::ProcessMessage(CNode& pfrom, CConnman& c
         }
 
         if (request.GetError() != CQuorumDataRequest::Errors::NONE) {
-            LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- %s: %s, from peer=%d\n", __func__, msg_type, strprintf("Error %d (%s)", request.GetError(), request.GetErrorString()), pfrom.GetId());
+            LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- %s: Error %d (%s), from peer=%d\n", __func__, msg_type, request.GetError(), request.GetErrorString(), pfrom.GetId());
             return {};
         }
 

Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ uint256 MakeQuorumKey(const CQuorum& q)`
`26`	`26`	`return hw.GetHash();`
`27`	`27`	`}`
`28`	`28`
`29`		`-void DataCleanupHelper(CDBWrapper& db, std::set<uint256> skip_list, bool compact)`
	`29`	`+void DataCleanupHelper(CDBWrapper& db, const std::set<uint256>& skip_list, bool compact)`
`30`	`30`	`{`
`31`	`31`	`const auto prefixes = {DB_QUORUM_QUORUM_VVEC, DB_QUORUM_SK_SHARE};`
`32`	`32`
`@@ -124,7 +124,7 @@ bool CQuorum::SetVerificationVector(const std::vector<CBLSPublicKey>& quorumVecI`
`124`	`124`
`125`	`125`	`bool CQuorum::SetSecretKeyShare(const CBLSSecretKey& secretKeyShare, const uint256& protx_hash)`
`126`	`126`	`{`
`127`		`- if (!secretKeyShare.IsValid() \|\| (secretKeyShare.GetPublicKey() != GetPubKeyShare(GetMemberIndex(protx_hash)))) {`
	`127`	`+ if (protx_hash.IsNull() \|\| !secretKeyShare.IsValid() \|\| (secretKeyShare.GetPublicKey() != GetPubKeyShare(GetMemberIndex(protx_hash)))) {`
`128`	`128`	`return false;`
`129`	`129`	`}`
`130`	`130`	`LOCK(cs_vvec_shShare);`
Original file line number	Diff line number	Diff line change
`@@ -481,16 +481,22 @@ MessageProcessingResult CQuorumManager::ProcessMessage(CNode& pfrom, CConnman& c`
`481`	`481`	`}`
`482`	`482`
`483`	`483`	`// Check if request wants ENCRYPTED_CONTRIBUTIONS data`
`484`		`- MessageProcessingResult ret{};`
	`484`	`+ CQuorumDataRequest::Errors ret_err{CQuorumDataRequest::Errors::NONE};`
	`485`	`+ MessageProcessingResult qdata_ret{}, ret{};`
`485`	`486`	`if (m_handler) {`
`486`	`487`	`ret = m_handler->ProcessContribQGETDATA(request_limit_exceeded, ssResponseData, *pQuorum, request, pQuorumBaseBlockIndex);`
`487`	`488`	`if (auto request_err = request.GetError(); request_err != CQuorumDataRequest::Errors::NONE &&`
`488`	`489`	`request_err != CQuorumDataRequest::Errors::UNDEFINED) {`
`489`		`- auto qdata_ret = sendQDATA(request_err, request_limit_exceeded);`
`490`		`- return ret.empty() ? qdata_ret : ret;`
	`490`	`+ ret_err = request_err;`
`491`	`491`	`}`
`492`	`492`	`}`
`493`		`- return ret.empty() ? sendQDATA(CQuorumDataRequest::Errors::NONE, request_limit_exceeded, ssResponseData) : ret;`
	`493`	`+ // sendQDATA also pushes a message independent of the returned value`
	`494`	`+ if (ret_err != CQuorumDataRequest::Errors::NONE) {`
	`495`	`+ qdata_ret = sendQDATA(ret_err, request_limit_exceeded);`
	`496`	`+ } else {`
	`497`	`+ qdata_ret = sendQDATA(CQuorumDataRequest::Errors::NONE, request_limit_exceeded, ssResponseData);`
	`498`	`+ }`
	`499`	`+ return ret.empty() ? qdata_ret : ret;`
`494`	`500`	`}`
`495`	`501`
`496`	`502`	`if (msg_type == NetMsgType::QDATA) {`
`@@ -518,7 +524,7 @@ MessageProcessingResult CQuorumManager::ProcessMessage(CNode& pfrom, CConnman& c`
`518`	`524`	`}`
`519`	`525`
`520`	`526`	`if (request.GetError() != CQuorumDataRequest::Errors::NONE) {`
`521`		`- LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- %s: %s, from peer=%d\n", __func__, msg_type, strprintf("Error %d (%s)", request.GetError(), request.GetErrorString()), pfrom.GetId());`
	`527`	`+ LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- %s: Error %d (%s), from peer=%d\n", __func__, msg_type, request.GetError(), request.GetErrorString(), pfrom.GetId());`
`522`	`528`	`return {};`
`523`	`529`	`}`
`524`	`530`