MOVEONLY: check for disjoint conflicts and ancestors to policy/rbf

This checks that a transaction isn't trying to replace something it
supposedly depends on.

Author: glozow

src/policy/rbf.cpp

@@ -112,3 +112,20 @@ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx,
     }
     return std::nullopt;
 }
+
+std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& setAncestors,
+                                                   const std::set<uint256>& setConflicts,
+                                                   const uint256& txid)
+{
+    for (CTxMemPool::txiter ancestorIt : setAncestors)
+    {
+        const uint256 &hashAncestor = ancestorIt->GetTx().GetHash();
+        if (setConflicts.count(hashAncestor))
+        {
+            return strprintf("%s spends conflicting transaction %s",
+                             txid.ToString(),
+                             hashAncestor.ToString());
+        }
+    }
+    return std::nullopt;
+}

src/policy/rbf.h

@@ -56,4 +56,17 @@ std::optional<std::string> GetEntriesForConflicts(const CTransaction& tx, CTxMem
 std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx, const CTxMemPool& m_pool,
                                                const CTxMemPool::setEntries& setIterConflicting)
                                                EXCLUSIVE_LOCKS_REQUIRED(m_pool.cs);
+
+/** Check the intersection between two sets of transactions (a set of mempool entries and a set of
+ * txids) to make sure they are disjoint.
+ * @param[in]   setAncestors    Set of mempool entries corresponding to ancestors of the
+ *                              replacement transactions.
+ * @param[in]   setConflicts    Set of txids corresponding to the mempool conflicts
+ *                              (candidates to be replaced).
+ * @param[in]   txid            Transaction ID, included in the error message if violation occurs.
+ * @returns error message if the sets intersect, std::nullopt if they are disjoint.
+ */
+std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& setAncestors,
+                                                   const std::set<uint256>& setConflicts,
+                                                   const uint256& txid);
 #endif // BITCOIN_POLICY_RBF_H

src/validation.cpp

@@ -770,16 +770,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
     // that we have the set of all ancestors we can detect this
     // pathological case by making sure setConflicts and setAncestors don't
     // intersect.
-    for (CTxMemPool::txiter ancestorIt : setAncestors)
-    {
-        const uint256 &hashAncestor = ancestorIt->GetTx().GetHash();
-        if (setConflicts.count(hashAncestor))
-        {
-            return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-spends-conflicting-tx",
-                    strprintf("%s spends conflicting transaction %s",
-                        hash.ToString(),
-                        hashAncestor.ToString()));
-        }
+    if (const auto err_string{EntriesAndTxidsDisjoint(setAncestors, setConflicts, hash)}) {
+        return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-spends-conflicting-tx", *err_string);
     }