From b8c560479e2ad1ae7cc7948f7eee73f973dcbf5a Mon Sep 17 00:00:00 2001
From: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
Date: Tue, 30 Nov 2021 00:03:08 -0500
Subject: [PATCH] refactor: misc refactoring prefer std algorithm, range for
 loops; fix broken loop (#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
---
 src/llmq/dkgsessionhandler.cpp | 23 ++++++-----------------
 src/llmq/signing_shares.cpp    | 17 +++++++----------
 src/llmq/signing_shares.h      |  2 +-
 3 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/src/llmq/dkgsessionhandler.cpp b/src/llmq/dkgsessionhandler.cpp
index 254e9ef620..82b7ea48a6 100644
--- a/src/llmq/dkgsessionhandler.cpp
+++ b/src/llmq/dkgsessionhandler.cpp
@@ -379,30 +379,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
index 1621aec29c..97ab5f7471 100644
--- a/src/llmq/signing_shares.cpp
+++ b/src/llmq/signing_shares.cpp
@@ -88,9 +88,7 @@ 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
@@ -98,8 +96,8 @@ 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();
 }
@@ -437,8 +435,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?
@@ -1250,10 +1248,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
index 6e0b954a5a..725397025e 100644
--- a/src/llmq/signing_shares.h
+++ b/src/llmq/signing_shares.h
@@ -421,7 +421,7 @@ private:
     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);