From 3f033f01a6b0f7772ae1b21044903b8f4249ad08 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Tue, 27 Jul 2021 15:55:25 +0100
Subject: [PATCH] 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.
---
 src/policy/rbf.cpp | 17 +++++++++++++++++
 src/policy/rbf.h   | 13 +++++++++++++
 src/validation.cpp | 12 ++----------
 3 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp
index f6b3bc783a..95b74123c9 100644
--- a/src/policy/rbf.cpp
+++ b/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;
+}
diff --git a/src/policy/rbf.h b/src/policy/rbf.h
index 0f9a3d9856..6c4e218959 100644
--- a/src/policy/rbf.h
+++ b/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
diff --git a/src/validation.cpp b/src/validation.cpp
index 20970bceb8..710fe2d873 100644
--- a/src/validation.cpp
+++ b/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);
     }