refactor: replace pair with structured PendingISLockFromPeer in CInstantSendManager

Updated the CInstantSendManager to use a new struct, PendingISLockFromPeer, for better clarity and type safety. This change replaces the use of std::pair for storing node ID and InstantSendLockPtr, enhancing code readability and maintainability across multiple functions handling instant send locks.

Author: PastaPastaPasta

src/instantsend/instantsend.cpp

@@ -168,7 +168,7 @@ MessageProcessingResult CInstantSendManager::ProcessMessage(NodeId from, std::st
     }
 
     LOCK(cs_pendingLocks);
-    pendingInstantSendLocks.emplace(hash, std::make_pair(from, islock));
+    pendingInstantSendLocks.emplace(hash, instantsend::PendingISLockFromPeer{from, islock});
     return ret;
 }
 
@@ -239,7 +239,7 @@ instantsend::PendingState CInstantSendManager::ProcessPendingInstantSendLocks()
 
 Uint256HashSet CInstantSendManager::ProcessPendingInstantSendLocks(
     const Consensus::LLMQParams& llmq_params, int signOffset, bool ban,
-    const Uint256HashMap<std::pair<NodeId, instantsend::InstantSendLockPtr>>& pend,
+    const Uint256HashMap<instantsend::PendingISLockFromPeer>& pend,
     std::vector<std::pair<NodeId, MessageProcessingResult>>& peer_activity)
 {
     CBLSBatchVerifier<NodeId, uint256> batchVerifier(false, true, 8);
@@ -249,8 +249,8 @@ Uint256HashSet CInstantSendManager::ProcessPendingInstantSendLocks(
     size_t alreadyVerified = 0;
     for (const auto& p : pend) {
         const auto& hash = p.first;
-        auto nodeId = p.second.first;
-        const auto& islock = p.second.second;
+        auto nodeId = p.second.node_id;
+        const auto& islock = p.second.islock;
 
         if (batchVerifier.badSources.count(nodeId)) {
             continue;
@@ -321,8 +321,8 @@ Uint256HashSet CInstantSendManager::ProcessPendingInstantSendLocks(
     }
     for (const auto& p : pend) {
         const auto& hash = p.first;
-        auto nodeId = p.second.first;
-        const auto& islock = p.second.second;
+        auto nodeId = p.second.node_id;
+        const auto& islock = p.second.islock;
 
         if (batchVerifier.badMessages.count(hash)) {
             LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txid=%s, islock=%s: invalid sig in islock, peer=%d\n",
@@ -399,7 +399,7 @@ MessageProcessingResult CInstantSendManager::ProcessInstantSendLock(NodeId from,
     } else {
         // put it in a separate pending map and try again later
         LOCK(cs_pendingLocks);
-        pendingNoTxInstantSendLocks.try_emplace(hash, std::make_pair(from, islock));
+        pendingNoTxInstantSendLocks.try_emplace(hash, instantsend::PendingISLockFromPeer{from, islock});
     }
 
     // This will also add children TXs to pendingRetryTxs
@@ -442,11 +442,11 @@ void CInstantSendManager::TransactionAddedToMempool(const CTransactionRef& tx)
         LOCK(cs_pendingLocks);
         auto it = pendingNoTxInstantSendLocks.begin();
         while (it != pendingNoTxInstantSendLocks.end()) {
-            if (it->second.second->txid == tx->GetHash()) {
+            if (it->second.islock->txid == tx->GetHash()) {
                 // we received an islock earlier
                 LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txid=%s, islock=%s\n", __func__,
                          tx->GetHash().ToString(), it->first.ToString());
-                islock = it->second.second;
+                islock = it->second.islock;
                 pendingInstantSendLocks.try_emplace(it->first, it->second);
                 pendingNoTxInstantSendLocks.erase(it);
                 break;
@@ -539,7 +539,7 @@ void CInstantSendManager::AddNonLockedTx(const CTransactionRef& tx, const CBlock
         LOCK(cs_pendingLocks);
         auto it = pendingNoTxInstantSendLocks.begin();
         while (it != pendingNoTxInstantSendLocks.end()) {
-            if (it->second.second->txid == tx->GetHash()) {
+            if (it->second.islock->txid == tx->GetHash()) {
                 // we received an islock earlier, let's put it back into pending and verify/lock
                 LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txid=%s, islock=%s\n", __func__,
                          tx->GetHash().ToString(), it->first.ToString());
@@ -626,7 +626,7 @@ void CInstantSendManager::TryEmplacePendingLock(const uint256& hash, const NodeI
     if (db.KnownInstantSendLock(hash)) return;
     LOCK(cs_pendingLocks);
     if (!pendingInstantSendLocks.count(hash)) {
-        pendingInstantSendLocks.emplace(hash, std::make_pair(id, islock));
+        pendingInstantSendLocks.emplace(hash, instantsend::PendingISLockFromPeer{id, islock});
     }
 }
 
@@ -848,11 +848,11 @@ bool CInstantSendManager::GetInstantSendLockByHash(const uint256& hash, instants
         LOCK(cs_pendingLocks);
         auto it = pendingInstantSendLocks.find(hash);
         if (it != pendingInstantSendLocks.end()) {
-            islock = it->second.second;
+            islock = it->second.islock;
         } else {
             auto itNoTx = pendingNoTxInstantSendLocks.find(hash);
             if (itNoTx != pendingNoTxInstantSendLocks.end()) {
-                islock = itNoTx->second.second;
+                islock = itNoTx->second.islock;
             } else {
                 return false;
             }
@@ -889,7 +889,7 @@ bool CInstantSendManager::IsWaitingForTx(const uint256& txHash) const
     LOCK(cs_pendingLocks);
     auto it = pendingNoTxInstantSendLocks.begin();
     while (it != pendingNoTxInstantSendLocks.end()) {
-        if (it->second.second->txid == txHash) {
+        if (it->second.islock->txid == txHash) {
             LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txid=%s, islock=%s\n", __func__, txHash.ToString(),
                      it->first.ToString());
             return true;

src/instantsend/instantsend.h

@@ -38,6 +38,11 @@ struct DbWrapperParams;
 namespace instantsend {
 class InstantSendSigner;
 
+struct PendingISLockFromPeer {
+    NodeId node_id;
+    InstantSendLockPtr islock;
+};
+
 struct PendingState {
     bool m_pending_work{false};
     std::vector<std::pair<NodeId, MessageProcessingResult>> m_peer_activity{};
@@ -69,9 +74,9 @@ class CInstantSendManager final : public instantsend::InstantSendSignerParent
 
     mutable Mutex cs_pendingLocks;
     // Incoming and not verified yet
-    Uint256HashMap<std::pair<NodeId, instantsend::InstantSendLockPtr>> pendingInstantSendLocks GUARDED_BY(cs_pendingLocks);
+    Uint256HashMap<instantsend::PendingISLockFromPeer> pendingInstantSendLocks GUARDED_BY(cs_pendingLocks);
     // Tried to verify but there is no tx yet
-    Uint256HashMap<std::pair<NodeId, instantsend::InstantSendLockPtr>> pendingNoTxInstantSendLocks GUARDED_BY(cs_pendingLocks);
+    Uint256HashMap<instantsend::PendingISLockFromPeer> pendingNoTxInstantSendLocks GUARDED_BY(cs_pendingLocks);
 
     // TXs which are neither IS locked nor ChainLocked. We use this to determine for which TXs we need to retry IS
     // locking of child TXs
@@ -117,7 +122,7 @@ class CInstantSendManager final : public instantsend::InstantSendSignerParent
         EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
 
     Uint256HashSet ProcessPendingInstantSendLocks(const Consensus::LLMQParams& llmq_params, int signOffset, bool ban,
-                                                  const Uint256HashMap<std::pair<NodeId, instantsend::InstantSendLockPtr>>& pend,
+                                                  const Uint256HashMap<instantsend::PendingISLockFromPeer>& pend,
                                                   std::vector<std::pair<NodeId, MessageProcessingResult>>& peer_activity)
         EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
     MessageProcessingResult ProcessInstantSendLock(NodeId from, const uint256& hash,