Allow package RBF as long as replacing transactions are a cluster of …

…size 2
instagibbs · Nov 14, 2023 · f6f39e5 · f6f39e5
1 parent 1b3ac98
commit f6f39e5
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 14 deletions.
diff --git a/src/validation.cpp b/src/validation.cpp
@@ -1077,12 +1077,9 @@ bool MemPoolAccept::PackageReplacementChecks(std::vector<Workspace>& workspaces,
     m_rbf = std::any_of(workspaces.cbegin(), workspaces.cend(), [](const auto& ws){return !ws.m_conflicts.empty();});
     if (!m_rbf) return true;
 
-    // Unless the transaction is V3, its own fees must meet the requirements for replacing its conflicts.
-    for (const auto& ws : workspaces) {
-        // If this transaction has a conflict, it must be V3.
-        if (!ws.m_iters_conflicting.empty() && ws.m_ptx->nVersion != 3) {
-            return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: V3 required");
-        }
+    // We only allow clusters of size 2 to initiate package RBFs
+    if (!collective_ancestors.empty() || workspaces.size() != 2) {
+        return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: replacing cluster not size two");
     }
 
     CTxMemPool::setEntries direct_conflict_iters;
@@ -1153,7 +1150,7 @@ bool MemPoolAccept::PackageMempoolChecks(const ATMPArgs& args,
     assert(std::all_of(txns.cbegin(), txns.cend(), [this](const auto& tx)
                        { return !m_pool.exists(GenTxid::Txid(tx->GetHash()));}));
 
-    // Populate with the union of all transactions' ancestors.
+    // Populate with the union of all transactions' mempool ancestors.
     CTxMemPool::setEntries collective_ancestors;
     for (const auto& ws : workspaces) {
         for (const auto& it : ws.m_ancestors) collective_ancestors.insert(it);

diff --git a/test/functional/mempool_package_rbf.py b/test/functional/mempool_package_rbf.py
@@ -39,7 +39,7 @@ def assert_mempool_contents(self, expected=None, unexpected=None):
         for tx in unexpected:
             assert tx.rehash() not in mempool
 
-    def create_simple_package(self, parent_coin, parent_fee=0, child_fee=DEFAULT_FEE, heavy_child=False, version=3):
+    def create_simple_package(self, parent_coin, parent_fee=0, child_fee=DEFAULT_FEE, heavy_child=False, version=2):
         """Create a 1 parent 1 child package using the coin passed in as the parent's input. The
         parent has 1 output, used to fund 1 child transaction.
         All transactions signal BIP125 replaceability, but nSequence changes based on self.ctr. This
@@ -99,7 +99,7 @@ def test_package_rbf_basic(self):
         # Reuse the same coins so that the transactions conflict with one another.
         parent_coin = self.coins.pop()
         package_hex1, package_txns1 = self.create_simple_package(parent_coin, DEFAULT_FEE, DEFAULT_FEE)
-        package_hex2, package_txns2 = self.create_simple_package(parent_coin, 0, DEFAULT_FEE * 5)
+        package_hex2, package_txns2 = self.create_simple_package(parent_coin, 0, DEFAULT_FEE * 5, version=3)
         node.submitpackage(package_hex1)
         self.assert_mempool_contents(expected=package_txns1, unexpected=package_txns2)
 
@@ -153,11 +153,11 @@ def test_package_rbf_additional_fees(self):
         self.assert_mempool_contents(expected=package_txns1)
         # Package 2 has a higher feerate but lower absolute fee
         package_fees1 = DEFAULT_FEE * 2
-        package_hex2, package_txns2 = self.create_simple_package(coin, parent_fee=0, child_fee=package_fees1 - Decimal("0.000000001"))
+        package_hex2, package_txns2 = self.create_simple_package(coin, parent_fee=0, child_fee=package_fees1 - Decimal("0.000000001"), version=3)
         assert_raises_rpc_error(-25, "package RBF failed: insufficient fee", node.submitpackage, package_hex2)
         self.assert_mempool_contents(expected=package_txns1, unexpected=package_txns2)
         # Package 3 has a higher feerate and absolute fee
-        package_hex3, package_txns3 = self.create_simple_package(coin, parent_fee=0, child_fee=package_fees1 * 3)
+        package_hex3, package_txns3 = self.create_simple_package(coin, parent_fee=0, child_fee=package_fees1 * 3, version=3)
         node.submitpackage(package_hex3)
         self.assert_mempool_contents(expected=package_txns3, unexpected=package_txns1 + package_txns2)
         self.generate(node, 1)
@@ -168,7 +168,7 @@ def test_package_rbf_additional_fees(self):
         package_fees1 = 2 * DEFAULT_FEE
         node.submitpackage(package_hex1)
         self.assert_mempool_contents(expected=package_txns1, unexpected=[])
-        package_hex2, package_txns2 = self.create_simple_package(coin, child_fee=package_fees1 + Decimal("0.000000001"))
+        package_hex2, package_txns2 = self.create_simple_package(coin, child_fee=package_fees1 + Decimal("0.000000001"), version=3)
         assert_raises_rpc_error(-25, "package RBF failed: insufficient fee", node.submitpackage, package_hex2)
         self.assert_mempool_contents(expected=package_txns1, unexpected=package_txns2)
         self.generate(node, 1)
@@ -196,8 +196,8 @@ def test_package_rbf_conflicting_conflicts(self):
         self.log.info("Check that different package transactions cannot share the same conflicts")
         coin = self.coins.pop()
         package_hex1, package_txns1 = self.create_simple_package(coin, DEFAULT_FEE, DEFAULT_FEE)
-        package_hex2, package_txns2 = self.create_simple_package(coin, Decimal("0.00009"), DEFAULT_FEE * 2)
-        package_hex3, package_txns3 = self.create_simple_package(coin, 0, DEFAULT_FEE * 5)
+        package_hex2, package_txns2 = self.create_simple_package(coin, Decimal("0.00009"), DEFAULT_FEE * 2, version=3)
+        package_hex3, package_txns3 = self.create_simple_package(coin, 0, DEFAULT_FEE * 5, version=3)
         node.submitpackage(package_hex1)
         self.assert_mempool_contents(expected=package_txns1)
         # The first two transactions have the same conflicts