refactor: misc refactoring prefer std algorithm, range for loops; fix…

… broken loop (dashpay#4593) * fix: replace seemingly buggy loop with std::adjacent_find * Remove redundant variable declaration * use std::fill instead of a loop * remove a few raw for loops
cosanta · Nov 16, 2023 · df14590 · df14590
1 parent 7d943e1
commit df14590
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 28 deletions.
diff --git a/src/llmq/dkgsessionhandler.cpp b/src/llmq/dkgsessionhandler.cpp
@@ -363,30 +363,19 @@ std::set<NodeId> BatchVerifyMessageSigs(CDKGSession& session, const std::vector<
         messageHashes.emplace_back(msgHash);
     }
     if (!revertToSingleVerification) {
-        bool valid = aggSig.VerifyInsecureAggregated(pubKeys, messageHashes);
-        if (valid) {
+        if (aggSig.VerifyInsecureAggregated(pubKeys, messageHashes)) {
             // all good
             return ret;
         }
 
         // are all messages from the same node?
-        NodeId firstNodeId{-1};
-        first = true;
-        bool nodeIdsAllSame = true;
-        for (auto it = messages.begin(); it != messages.end(); ++it) {
-            if (first) {
-                firstNodeId = it->first;
-            } else {
-                first = false;
-                if (it->first != firstNodeId) {
-                    nodeIdsAllSame = false;
-                    break;
-                }
-            }
-        }
+        bool nodeIdsAllSame = std::adjacent_find( messages.begin(), messages.end(), [](const auto& first, const auto& second){
+            return first.first != second.first;
+        }) == messages.end();
+
         // if yes, take a short path and return a set with only him
         if (nodeIdsAllSame) {
-            ret.emplace(firstNodeId);
+            ret.emplace(messages[0].first);
             return ret;
         }
         // different nodes, let's figure out who are the bad ones

diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp
@@ -88,18 +88,16 @@ void CSigSharesInv::Set(uint16_t quorumMember, bool v)
 
 void CSigSharesInv::SetAll(bool v)
 {
-    for (size_t i = 0; i < inv.size(); i++) {
-        inv[i] = v;
-    }
+    std::fill(inv.begin(), inv.end(), v);
 }
 
 std::string CBatchedSigShares::ToInvString() const
 {
     CSigSharesInv inv;
     // we use 400 here no matter what the real size is. We don't really care about that size as we just want to call ToString()
     inv.Init(400);
-    for (size_t i = 0; i < sigShares.size(); i++) {
-        inv.inv[sigShares[i].first] = true;
+    for (const auto& sigShare : sigShares) {
+        inv.inv[sigShare.first] = true;
     }
     return inv.ToString();
 }
@@ -426,8 +424,8 @@ bool CSigSharesManager::ProcessMessageBatchedSigShares(const CNode* pfrom, const
         LOCK(cs);
         auto& nodeState = nodeStates[pfrom->GetId()];
 
-        for (size_t i = 0; i < batchedSigShares.sigShares.size(); i++) {
-            CSigShare sigShare = RebuildSigShare(sessionInfo, batchedSigShares, i);
+        for (const auto& sigSharetmp : batchedSigShares.sigShares) {
+            CSigShare sigShare = RebuildSigShare(sessionInfo, sigSharetmp);
             nodeState.requestedSigShares.Erase(sigShare.GetKey());
 
             // TODO track invalid sig shares received for PoSe?
@@ -1245,10 +1243,9 @@ bool CSigSharesManager::GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId
     return nodeStates[nodeId].GetSessionInfoByRecvId(sessionId, retInfo);
 }
 
-CSigShare CSigSharesManager::RebuildSigShare(const CSigSharesNodeState::SessionInfo& session, const CBatchedSigShares& batchedSigShares, size_t idx)
+CSigShare CSigSharesManager::RebuildSigShare(const CSigSharesNodeState::SessionInfo& session, const std::pair<uint16_t, CBLSLazySignature>& in)
 {
-    assert(idx < batchedSigShares.sigShares.size());
-    const auto& [member, sig] = batchedSigShares.sigShares[idx];
+    const auto& [member, sig] = in;
     CSigShare sigShare;
     sigShare.llmqType = session.llmqType;
     sigShare.quorumHash = session.quorumHash;

diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h
@@ -424,7 +424,7 @@ class CSigSharesManager : public CRecoveredSigsListener
     void TryRecoverSig(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash);
 
     bool GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId, CSigSharesNodeState::SessionInfo& retInfo);
-    static CSigShare RebuildSigShare(const CSigSharesNodeState::SessionInfo& session, const CBatchedSigShares& batchedSigShares, size_t idx);
+    static CSigShare RebuildSigShare(const CSigSharesNodeState::SessionInfo& session, const std::pair<uint16_t, CBLSLazySignature>& in);
 
     void Cleanup();
     void RemoveSigSharesForSession(const uint256& signHash) EXCLUSIVE_LOCKS_REQUIRED(cs);