From 8d7179633552f58ca0d23305196dcb4249b6dce7 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Tue, 27 Jul 2021 11:49:34 +0100
Subject: [PATCH] [validation] quit RBF logic earlier and separate loops

No behavior change.
While we're looking through the descendants and calculating how many
transactions we might replace, quit early, as soon as we hit 100.
Since we're failing faster, we can also separate the loops - yes, we
loop through more times, but this helps us detangle the different BIP125
rules later.
---
 src/validation.cpp | 55 ++++++++++++++++++++++++----------------------
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index 0dade35ab25..fdcf1289242 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -782,9 +782,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
         }
     }
 
-    // Check if it's economically rational to mine this transaction rather
-    // than the ones it replaces.
-    uint64_t nConflictingCount = 0;
 
     // If we don't hold the lock allConflicting might be incomplete; the
     // subsequent RemoveStaged() and addUnchecked() calls don't guarantee
@@ -793,7 +790,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
     if (fReplacementTransaction)
     {
         CFeeRate newFeeRate(nModifiedFees, nSize);
-        std::set<uint256> setConflictsParents;
         for (const auto& mi : setIterConflicting) {
             // Don't allow the replacement to reduce the feerate of the
             // mempool.
@@ -818,33 +814,40 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
                             newFeeRate.ToString(),
                             oldFeeRate.ToString()));
             }
+        }
 
+        uint64_t nConflictingCount = 0;
+        for (const auto& mi : setIterConflicting) {
+            nConflictingCount += mi->GetCountWithDescendants();
+            // This potentially overestimates the number of actual descendants
+            // but we just want to be conservative to avoid doing too much
+            // work.
+            if (nConflictingCount > MAX_BIP125_REPLACEMENT_CANDIDATES) {
+                return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too many potential replacements",
+                        strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n",
+                            hash.ToString(),
+                            nConflictingCount,
+                            MAX_BIP125_REPLACEMENT_CANDIDATES));
+            }
+        }
+        // If not too many to replace, then calculate the set of
+        // transactions that would have to be evicted
+        for (CTxMemPool::txiter it : setIterConflicting) {
+            m_pool.CalculateDescendants(it, allConflicting);
+        }
+        // Check if it's economically rational to mine this transaction rather
+        // than the ones it replaces.
+        for (CTxMemPool::txiter it : allConflicting) {
+            nConflictingFees += it->GetModifiedFee();
+            nConflictingSize += it->GetTxSize();
+        }
+
+        std::set<uint256> setConflictsParents;
+        for (const auto& mi : setIterConflicting) {
             for (const CTxIn &txin : mi->GetTx().vin)
             {
                 setConflictsParents.insert(txin.prevout.hash);
             }
-
-            nConflictingCount += mi->GetCountWithDescendants();
-        }
-        // This potentially overestimates the number of actual descendants
-        // but we just want to be conservative to avoid doing too much
-        // work.
-        if (nConflictingCount <= MAX_BIP125_REPLACEMENT_CANDIDATES) {
-            // If not too many to replace, then calculate the set of
-            // transactions that would have to be evicted
-            for (CTxMemPool::txiter it : setIterConflicting) {
-                m_pool.CalculateDescendants(it, allConflicting);
-            }
-            for (CTxMemPool::txiter it : allConflicting) {
-                nConflictingFees += it->GetModifiedFee();
-                nConflictingSize += it->GetTxSize();
-            }
-        } else {
-            return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too many potential replacements",
-                    strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n",
-                        hash.ToString(),
-                        nConflictingCount,
-                        MAX_BIP125_REPLACEMENT_CANDIDATES));
         }
 
         for (unsigned int j = 0; j < tx.vin.size(); j++)