Merge #6898: refactor: change GetMasternodeQuorumNodes return type from unordered_set to vector

f0b71d5 refactor: change GetMasternodeQuorumNodes return type from unordered_set to vector (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Updated the GetMasternodeQuorumNodes method to return a vector of NodeId instead of an unordered_set. This change simplifies the logic for filtering and transforming nodes, enhancing readability and maintainability. Additionally, adjusted related code in ProcessSigShare to accommodate the new return type.

  It's better to use a vector over a set, as all users are just iterating through this. They don't actually benefit from the O(1) lookup, so let's get the better benefits of cache locality.

  ## What was done?

  ## How Has This Been Tested?

  ## Breaking Changes

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] 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
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK f0b71d5
  kwvg:
    utACK f0b71d5
  knst:
    utACK f0b71d5

Tree-SHA512: e7817d947b6e3b1681301062d5585dd5df44ddd30cce8f7c4fdcb730a16b8fe51c34f9774a3fc713f11d7c3c4d43332c4c386b093db332308c79f0086a578d4a

Author: PastaPastaPasta

src/llmq/signing_shares.cpp

@@ -717,9 +717,11 @@ void CSigSharesManager::ProcessSigShare(PeerManager& peerman, const CSigShare& s
     auto llmqType = quorum->params.type;
     bool canTryRecovery = false;
 
+    const bool isAllMembersConnectedEnabled = IsAllMembersConnectedEnabled(llmqType, m_sporkman);
+
     // prepare node set for direct-push in case this is our sig share
-    std::unordered_set<NodeId> quorumNodes;
-    if (!IsAllMembersConnectedEnabled(llmqType, m_sporkman) && sigShare.getQuorumMember() == quorum->GetMemberIndex(m_mn_activeman->GetProTxHash())) {
+    std::vector<NodeId> quorumNodes;
+    if (!isAllMembersConnectedEnabled && sigShare.getQuorumMember() == quorum->GetMemberIndex(m_mn_activeman->GetProTxHash())) {
         quorumNodes = connman.GetMasternodeQuorumNodes(sigShare.getLlmqType(), sigShare.getQuorumHash());
     }
 
@@ -733,7 +735,7 @@ void CSigSharesManager::ProcessSigShare(PeerManager& peerman, const CSigShare& s
         if (!sigShares.Add(sigShare.GetKey(), sigShare)) {
             return;
         }
-        if (!IsAllMembersConnectedEnabled(llmqType, m_sporkman)) {
+        if (!isAllMembersConnectedEnabled) {
             sigSharesQueuedToAnnounce.Add(sigShare.GetKey(), true);
         }
 

src/net.cpp

@@ -4348,7 +4348,7 @@ Uint256HashSet CConnman::GetMasternodeQuorums(Consensus::LLMQType llmqType) cons
     return result;
 }
 
-std::unordered_set<NodeId> CConnman::GetMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash) const
+std::vector<NodeId> CConnman::GetMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash) const
 {
     LOCK2(m_nodes_mutex, cs_vPendingMasternodes);
     auto it = masternodeQuorumNodes.find(std::make_pair(llmqType, quorumHash));
@@ -4357,16 +4357,18 @@ std::unordered_set<NodeId> CConnman::GetMasternodeQuorumNodes(Consensus::LLMQTyp
     }
     const auto& proRegTxHashes = it->second;
 
-    std::unordered_set<NodeId> nodes;
-    for (const auto pnode : m_nodes) {
-        if (pnode->fDisconnect) {
-            continue;
-        }
-        auto verifiedProRegTxHash = pnode->GetVerifiedProRegTxHash();
-        if (!pnode->qwatch && (verifiedProRegTxHash.IsNull() || !proRegTxHashes.count(verifiedProRegTxHash))) {
-            continue;
-        }
-        nodes.emplace(pnode->GetId());
+    std::vector<NodeId> nodes;
+
+    auto IsMasternodeQuorumNode = [&](const CNode* n) {
+        if (n->fDisconnect) return false;
+        const auto h = n->GetVerifiedProRegTxHash();
+        return n->qwatch || (!h.IsNull() && proRegTxHashes.contains(h));
+    };
+
+    for (NodeId id : m_nodes
+        | std::views::filter(IsMasternodeQuorumNode)
+        | std::views::transform([](const CNode* n){ return n->GetId(); })) {
+        nodes.push_back(id);
     }
     return nodes;
 }

src/net.h

@@ -1485,7 +1485,7 @@ friend class CNode;
     bool HasMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash) const;
     Uint256HashSet GetMasternodeQuorums(Consensus::LLMQType llmqType) const;
     // also returns QWATCH nodes
-    std::unordered_set<NodeId> GetMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash) const;
+    std::vector<NodeId> GetMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash) const;
     void RemoveMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash);
     bool IsMasternodeQuorumNode(const CNode* pnode, const CDeterministicMNList& tip_mn_list) const;
     bool IsMasternodeQuorumRelayMember(const uint256& protxHash);