refactor: Replace GetAllNodes() + RemoveAsBanned() with single RemoveNodesIf(predicate) method

- Single lock acquisition instead of O(n) locks
- No intermediate vector allocation
- No redundant map lookups

Author: UdjinM6

src/llmq/net_signing.cpp

@@ -257,12 +257,7 @@ void NetSigning::RemoveBannedNodeStates()
 {
     assert(m_shares_manager != nullptr);
     // Called regularly to cleanup local node states for banned nodes
-    std::vector<NodeId> nodes = m_shares_manager->GetAllNodes();
-    for (NodeId node_id : nodes) {
-        if (m_peer_manager->PeerIsBanned(node_id)) {
-            m_shares_manager->RemoveAsBanned(node_id);
-        }
-    }
+    m_shares_manager->RemoveNodesIf([this](NodeId node_id) { return m_peer_manager->PeerIsBanned(node_id); });
 }
 
 void NetSigning::BanNode(NodeId nodeId)

src/llmq/signing_shares.cpp

@@ -1336,32 +1336,22 @@ void CSigSharesManager::RemoveSigSharesForSession(const uint256& signHash)
     timeSeenForSessions.erase(signHash);
 }
 
-std::vector<NodeId> CSigSharesManager::GetAllNodes() const
-{
-    std::vector<NodeId> nodes;
-
-    LOCK(cs);
-    nodes.reserve(nodeStates.size());
-    for (const auto& [node_id, _] : nodeStates) {
-        nodes.push_back(node_id);
-    }
-    return nodes;
-}
-
-void CSigSharesManager::RemoveAsBanned(NodeId node_id)
+void CSigSharesManager::RemoveNodesIf(std::function<bool(NodeId)> predicate)
 {
     LOCK(cs);
-
-    auto it = nodeStates.find(node_id);
-    if (it != nodeStates.end()) {
-        // re-request sigshares from other nodes
-        // TODO: remove NO_THREAD_SAFETY_ANALYSIS
-        // using here template ForEach makes impossible to use lock annotation
-        it->second.requestedSigShares.ForEach([this](const SigShareKey& k, int64_t) NO_THREAD_SAFETY_ANALYSIS {
-            AssertLockHeld(cs);
-            sigSharesRequested.Erase(k);
-        });
-        nodeStates.erase(it);
+    for (auto it = nodeStates.begin(); it != nodeStates.end();) {
+        if (predicate(it->first)) {
+            // re-request sigshares from other nodes
+            // TODO: remove NO_THREAD_SAFETY_ANALYSIS
+            // using here template ForEach makes impossible to use lock annotation
+            it->second.requestedSigShares.ForEach([this](const SigShareKey& k, int64_t) NO_THREAD_SAFETY_ANALYSIS {
+                AssertLockHeld(cs);
+                sigSharesRequested.Erase(k);
+            });
+            it = nodeStates.erase(it);
+        } else {
+            ++it;
+        }
     }
 }
 

src/llmq/signing_shares.h

@@ -18,6 +18,7 @@
 #include <util/time.h>
 
 #include <atomic>
+#include <functional>
 #include <limits>
 #include <memory>
 #include <optional>
@@ -481,8 +482,7 @@ class CSigSharesManager : public llmq::CRecoveredSigsListener
     void RemoveSigSharesForSession(const uint256& signHash) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
 public:
-    std::vector<NodeId> GetAllNodes() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
-    void RemoveAsBanned(NodeId node_id) EXCLUSIVE_LOCKS_REQUIRED(!cs);
+    void RemoveNodesIf(std::function<bool(NodeId)> predicate) EXCLUSIVE_LOCKS_REQUIRED(!cs);
     void MarkAsBanned(NodeId nodeId) EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
 private: