MOVEONLY: getting mempool conflicts to policy/rbf

Author: glozow

src/policy/rbf.cpp

@@ -3,6 +3,10 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <policy/rbf.h>
+
+#include <policy/settings.h>
+#include <tinyformat.h>
+#include <util/moneystr.h>
 #include <util/rbf.h>
 
 RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool)
@@ -42,3 +46,34 @@ RBFTransactionState IsRBFOptInEmptyMempool(const CTransaction& tx)
     // If we don't have a local mempool we can only check the transaction itself.
     return SignalsOptInRBF(tx) ? RBFTransactionState::REPLACEABLE_BIP125 : RBFTransactionState::UNKNOWN;
 }
+
+bool GetEntriesForConflicts(const CTransaction& tx,
+                            CTxMemPool& m_pool,
+                            const CTxMemPool::setEntries& setIterConflicting,
+                            CTxMemPool::setEntries& allConflicting,
+                            std::string& err_string)
+{
+    AssertLockHeld(m_pool.cs);
+    const uint256 hash = tx.GetHash();
+    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) {
+            err_string = strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n",
+                        hash.ToString(),
+                        nConflictingCount,
+                        MAX_BIP125_REPLACEMENT_CANDIDATES);
+            return false;
+        }
+    }
+    // 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);
+    }
+    return true;
+}
+

src/policy/rbf.h

@@ -35,4 +35,19 @@ enum class RBFTransactionState {
 RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(pool.cs);
 RBFTransactionState IsRBFOptInEmptyMempool(const CTransaction& tx);
 
+/** Get all descendants of setIterConflicting. Also enforce BIP125 Rule #5, "The number of original
+ * transactions to be replaced and their descendant transactions which will be evicted from the
+ * mempool must not exceed a total of 100 transactions." Quit as early as possible. There cannot be
+ * more than MAX_BIP125_REPLACEMENT_CANDIDATES potential entries.
+ * @param[in]   setIterConflicting  The set of iterators to mempool entries.
+ * @param[out]  err_string          Used to return errors, if any.
+ * @param[out]  allConflicting      Populated with all the mempool entries that would be replaced,
+ *                                  which includes descendants of setIterConflicting. Not cleared at
+ *                                  the start; any existing mempool entries will remain in the set.
+ * @returns false if Rule 5 is broken.
+ */
+bool GetEntriesForConflicts(const CTransaction& tx, CTxMemPool& m_pool,
+                            const CTxMemPool::setEntries& setIterConflicting,
+                            CTxMemPool::setEntries& allConflicting,
+                            std::string& err_string) EXCLUSIVE_LOCKS_REQUIRED(m_pool.cs);
 #endif // BITCOIN_POLICY_RBF_H

src/validation.cpp

@@ -789,6 +789,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
     fReplacementTransaction = setConflicts.size();
     if (fReplacementTransaction)
     {
+        std::string err_string;
         CFeeRate newFeeRate(nModifiedFees, nSize);
         for (const auto& mi : setIterConflicting) {
             // Don't allow the replacement to reduce the feerate of the
@@ -816,25 +817,11 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
             }
         }
 
-        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);
+        // Calculate all conflicting entries and enforce Rule #5.
+        if (!GetEntriesForConflicts(tx, m_pool, setIterConflicting, allConflicting, err_string)) {
+            return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too many potential replacements", err_string);
         }
+
         // Check if it's economically rational to mine this transaction rather
         // than the ones it replaces.
         for (CTxMemPool::txiter it : allConflicting) {