refactor: move inexpensive checks earlier in ProcessInstantSendLock()

It's faster to check the InstantSend database than to search for a
transaction and the block that included it, so allow that faster bail
out. Additionally, we'll do some additional things:

* Assign the success of `GetTransaction` to `found_transaction` to mark
  the two different processing paths taken when we know or don't know
  the transaction associated with the ISLock
* Invert an if-else block to improve reading
* Push down the networking requests to the tail end of the function as
  they are independent of the changes made by `ProcessInstantSendLock()`
  * The `peerman` passed to `RemoveMempoolConflictsForLock()` will be
    sorted by the next commit

Review with `git log -p -n1 --color-moved=dimmed_zebra`.

Author: kwvg

src/instantsend/instantsend.cpp

@@ -353,18 +353,32 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, PeerManager& peerm
 {
     LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txid=%s, islock=%s: processing islock, peer=%d\n",
              __func__, islock->txid.ToString(), hash.ToString(), from);
+
     if (m_signer) {
         m_signer->ClearLockFromQueue(islock);
     }
     if (db.KnownInstantSendLock(hash)) {
         return;
     }
 
+    if (const auto sameTxIsLock = db.GetInstantSendLockByTxid(islock->txid)) {
+        // can happen, nothing to do
+        return;
+    }
+    for (const auto& in : islock->inputs) {
+        const auto sameOutpointIsLock = db.GetInstantSendLockByInput(in);
+        if (sameOutpointIsLock != nullptr) {
+            LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: conflicting outpoint in islock. input=%s, other islock=%s, peer=%d\n", __func__,
+                      islock->txid.ToString(), hash.ToString(), in.ToStringShort(), ::SerializeHash(*sameOutpointIsLock).ToString(), from);
+        }
+    }
+
     uint256 hashBlock{};
     const auto tx = GetTransaction(nullptr, &mempool, islock->txid, Params().GetConsensus(), hashBlock);
     const CBlockIndex* pindexMined{nullptr};
+    const bool found_transaction{tx != nullptr};
     // we ignore failure here as we must be able to propagate the lock even if we don't have the TX locally
-    if (tx && !hashBlock.IsNull()) {
+    if (found_transaction && !hashBlock.IsNull()) {
         pindexMined = WITH_LOCK(::cs_main, return m_chainstate.m_blockman.LookupBlockIndex(hashBlock));
 
         // Let's see if the TX that was locked by this islock is already mined in a ChainLocked block. If yes,
@@ -376,55 +390,40 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, PeerManager& peerm
         }
     }
 
-    const auto sameTxIsLock = db.GetInstantSendLockByTxid(islock->txid);
-    if (sameTxIsLock != nullptr) {
-        // can happen, nothing to do
-        return;
-    }
-    for (const auto& in : islock->inputs) {
-        const auto sameOutpointIsLock = db.GetInstantSendLockByInput(in);
-        if (sameOutpointIsLock != nullptr) {
-            LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: conflicting outpoint in islock. input=%s, other islock=%s, peer=%d\n", __func__,
-                      islock->txid.ToString(), hash.ToString(), in.ToStringShort(), ::SerializeHash(*sameOutpointIsLock).ToString(), from);
-        }
-    }
-
-    if (tx == nullptr) {
-        // put it in a separate pending map and try again later
-        LOCK(cs_pendingLocks);
-        pendingNoTxInstantSendLocks.try_emplace(hash, std::make_pair(from, islock));
-    } else {
+    if (found_transaction) {
         db.WriteNewInstantSendLock(hash, islock);
         if (pindexMined) {
             db.WriteInstantSendLockMined(hash, pindexMined->nHeight);
         }
+    } else {
+        // put it in a separate pending map and try again later
+        LOCK(cs_pendingLocks);
+        pendingNoTxInstantSendLocks.try_emplace(hash, std::make_pair(from, islock));
     }
 
     // This will also add children TXs to pendingRetryTxs
     RemoveNonLockedTx(islock->txid, true);
     // We don't need the recovered sigs for the inputs anymore. This prevents unnecessary propagation of these sigs.
     // We only need the ISLOCK from now on to detect conflicts
     TruncateRecoveredSigsForInputs(*islock);
-
-    CInv inv(MSG_ISDLOCK, hash);
-    if (tx != nullptr) {
-        peerman.RelayInvFiltered(inv, *tx, ISDLOCK_PROTO_VERSION);
-    } else {
-        // we don't have the TX yet, so we only filter based on txid. Later when that TX arrives, we will re-announce
-        // with the TX taken into account.
-        peerman.RelayInvFiltered(inv, islock->txid, ISDLOCK_PROTO_VERSION);
-    }
-
     ResolveBlockConflicts(hash, *islock);
 
-    if (tx != nullptr) {
+    if (found_transaction) {
         RemoveMempoolConflictsForLock(peerman, hash, *islock);
         LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- notify about lock %s for tx %s\n", __func__,
                  hash.ToString(), tx->GetHash().ToString());
         GetMainSignals().NotifyTransactionLock(tx, islock);
         // bump mempool counter to make sure newly locked txes are picked up by getblocktemplate
         mempool.AddTransactionsUpdated(1);
+    }
+
+    CInv inv(MSG_ISDLOCK, hash);
+    if (found_transaction) {
+        peerman.RelayInvFiltered(inv, *tx, ISDLOCK_PROTO_VERSION);
     } else {
+        // we don't have the TX yet, so we only filter based on txid. Later when that TX arrives, we will re-announce
+        // with the TX taken into account.
+        peerman.RelayInvFiltered(inv, islock->txid, ISDLOCK_PROTO_VERSION);
         peerman.AskPeersForTransaction(islock->txid, /*is_masternode=*/m_signer != nullptr);
     }
 }