From f6f39e513b7db9b53f25cd66f145a5ac499b9e98 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 14 Nov 2023 14:38:23 -0500 Subject: [PATCH] Allow package RBF as long as replacing transactions are a cluster of size 2 --- src/validation.cpp | 11 ++++------- test/functional/mempool_package_rbf.py | 14 +++++++------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 6f8487d01d0531..e78e8971d7cfac 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1077,12 +1077,9 @@ bool MemPoolAccept::PackageReplacementChecks(std::vector& 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 index 219aa584e0ff1b..948886d95d466d 100755 --- 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