Cluster mempool #33629

0    argsman.AddArg("-limitclustersize=<n>", strprintf("Do not accept transactions whose size with all in-mempool connected transactions exceeds <n> virtual kilobytes (default: %u)", DEFAULT_CLUSTER_SIZE_LIMIT_KVB), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);

 0diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
 1index 492693c9cef..1b62c74f0b3 100644
 2--- a/src/rpc/mempool.cpp
 3+++ b/src/rpc/mempool.cpp
 4@@ -812,6 +812,8 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool)
 5     ret.pushKV("fullrbf", true);
 6     ret.pushKV("permitbaremultisig", pool.m_opts.permit_bare_multisig);
 7     ret.pushKV("maxdatacarriersize", pool.m_opts.max_datacarrier_bytes.value_or(0));
 8+    ret.pushKV("limitclustercount", pool.m_opts.limits.cluster_count);
 9+    ret.pushKV("limitclustersize", pool.m_opts.limits.cluster_size_vbytes);
10     return ret;
11 }
12 
13@@ -836,6 +838,8 @@ static RPCHelpMan getmempoolinfo()
14                 {RPCResult::Type::BOOL, "fullrbf", "True if the mempool accepts RBF without replaceability signaling inspection (DEPRECATED)"},
15                 {RPCResult::Type::BOOL, "permitbaremultisig", "True if the mempool accepts transactions with bare multisig outputs"},
16                 {RPCResult::Type::NUM, "maxdatacarriersize", "Maximum number of bytes that can be used by OP_RETURN outputs in the mempool"},
17+                {RPCResult::Type::NUM, "limitclustercount", "Maximum number of transactions that can be in a cluster (configured by -limitclustercount)"},
18+                {RPCResult::Type::NUM, "limitclustersize", "Maximum size of a cluster in virtual bytes (configured by -limitclustersize)"},
19             }},
20         RPCExamples{
21             HelpExampleCli("getmempoolinfo", "")

0    // Check if the transactions would exceed the cluster size limit.

  0diff --git a/test/functional/mempool_cluster.py b/test/functional/mempool_cluster.py
  1index 3da8b477a2f..c75eb22bf9b 100755
  2--- a/test/functional/mempool_cluster.py
  3+++ b/test/functional/mempool_cluster.py
  4@@ -4,59 +4,322 @@
  5 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
  6 """Test cluster mempool accessors and limits"""
  7 
  8+from decimal import Decimal
  9+
 10+from test_framework.mempool_util import (
 11+    DEFAULT_CLUSTER_LIMIT,
 12+    DEFAULT_CLUSTER_SIZE_LIMIT_KVB,
 13+)
 14+from test_framework.messages import (
 15+    COIN,
 16+)
 17 from test_framework.test_framework import BitcoinTestFramework
 18 from test_framework.wallet import (
 19     MiniWallet,
 20 )
 21 from test_framework.util import (
 22+    assert_equal,
 23+    assert_greater_than,
 24+    assert_greater_than_or_equal,
 25     assert_raises_rpc_error,
 26 )
 27 
 28-MAX_CLUSTER_COUNT = 64
 29+def cleanup(func):
 30+    def wrapper(self, *args, **kwargs):
 31+        try:
 32+            func(self, *args, **kwargs)
 33+        finally:
 34+            # Mine blocks to clear the mempool and replenish the wallet's confirmed UTXOs.
 35+            while (len(self.nodes[0].getrawmempool()) > 0):
 36+                self.generate(self.nodes[0], 1)
 37+            self.wallet.rescan_utxos(include_mempool=True)
 38+    return wrapper
 39 
 40 class MempoolClusterTest(BitcoinTestFramework):
 41     def set_test_params(self):
 42         self.num_nodes = 1
 43 
 44-    def run_test(self):
 45-        node = self.nodes[0]
 46-        self.wallet = MiniWallet(node)
 47-
 48-        node = self.nodes[0]
 49-        parent_tx = self.wallet.send_self_transfer(from_node=node)
 50+    def add_chain_cluster(self, node, cluster_count, target_vsize=None):
 51+        """Create a cluster of transactions, with the count specified.
 52+        The topology is a chain: the i'th transaction depends on the (i-1)'th transaction.
 53+        Optionally provide a target_vsize for each transaction.
 54+        """
 55+        parent_tx = self.wallet.send_self_transfer(from_node=node, confirmed_only=True, target_vsize=target_vsize)
 56         utxo_to_spend = parent_tx["new_utxo"]
 57-        ancestors = [parent_tx["txid"]]
 58-        while len(node.getrawmempool()) < MAX_CLUSTER_COUNT:
 59-            next_tx = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=utxo_to_spend)
 60+        all_txids = [parent_tx["txid"]]
 61+        all_results = [parent_tx]
 62+
 63+        while len(all_results) < cluster_count:
 64+            next_tx = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=utxo_to_spend, target_vsize=target_vsize)
 65+            assert next_tx["txid"] in node.getrawmempool()
 66+
 67             # Confirm that each transaction is in the same cluster as the first.
 68-            assert node.getmempoolcluster(next_tx['txid']) == node.getmempoolcluster(parent_tx['txid'])
 69+            assert_equal(node.getmempoolcluster(next_tx['txid']), node.getmempoolcluster(parent_tx['txid']))
 70 
 71             # Confirm that the ancestors are what we expect
 72             mempool_ancestors = node.getmempoolancestors(next_tx['txid'])
 73-            assert sorted(mempool_ancestors) == sorted(ancestors)
 74+            assert_equal(sorted(mempool_ancestors), sorted(all_txids))
 75 
 76             # Confirm that each successive transaction is added as a descendant.
 77-            assert all([ next_tx["txid"] in node.getmempooldescendants(x) for x in ancestors ])
 78+            assert all([ next_tx["txid"] in node.getmempooldescendants(x) for x in all_txids ])
 79 
 80             # Update for next iteration
 81-            ancestors.append(next_tx["txid"])
 82+            all_results.append(next_tx)
 83+            all_txids.append(next_tx["txid"])
 84             utxo_to_spend = next_tx["new_utxo"]
 85 
 86-        assert node.getmempoolcluster(parent_tx['txid'])['txcount'] == MAX_CLUSTER_COUNT
 87+        assert node.getmempoolcluster(parent_tx['txid'])['txcount'] == cluster_count
 88+        return all_results
 89+
 90+    def check_feerate_diagram(self, node):
 91+        """Sanity check the feerate diagram."""
 92         feeratediagram = node.getmempoolfeeratediagram()
 93         last_val = [0, 0]
 94         for x in feeratediagram:
 95+            # The vsize is always positive, except for the first iteration
 96             assert x['vsize'] > 0 or x['fee'] == 0
 97-            assert last_val[0]*x['fee'] >= last_val[1]*x['vsize']
 98+            # Monotonically decreasing fee per vsize
 99+            assert_greater_than_or_equal(last_val[0] * x['fee'], last_val[1] * x['vsize'])
100             last_val = [x['vsize'], x['fee']]
101 
102+    def test_limit_enforcement(self, cluster_submitted, target_vsize_per_tx=None):
103+        """
104+        the cluster may change as a result of these transactions, so cluster_submitted is mutated accordingly
105+        """
106+        # Cluster has already been submitted and has at least 3 transactions, otherwise this test won't work.
107+        assert_greater_than_or_equal(len(cluster_submitted), 3)
108+        node = self.nodes[0]
109+        last_result = cluster_submitted[-1]
110+        # We assume that this is the maximum cluster count
111+        cluster_limit = node.getmempoolcluster(cluster_submitted[-1]["txid"])['txcount']
112+
113         # Test that adding one more transaction to the cluster will fail.
114-        bad_tx = self.wallet.create_self_transfer(utxo_to_spend=utxo_to_spend)
115+        bad_tx = self.wallet.create_self_transfer(utxo_to_spend=last_result["new_utxo"], target_vsize=target_vsize_per_tx)
116         assert_raises_rpc_error(-26, "too-large-cluster", node.sendrawtransaction, bad_tx["hex"])
117 
118-        # TODO: verify that the size limits are also enforced.
119-        # TODO: add tests that exercise rbf, package submission, and package
120-        # rbf and verify that cluster limits are enforced.
121+        # It should also work during replacement
122+        utxo_to_double_spend = self.wallet.get_utxo(confirmed_only=True)
123+        fee = Decimal("0.000001")
124+        tx_to_replace = self.wallet.create_self_transfer(utxo_to_spend=utxo_to_double_spend, fee=fee)
125+        node.sendrawtransaction(tx_to_replace["hex"])
126+
127+        # Multiply fee by 5, which should easily cover the cost to replace. Otherwise, use the target vsize at 10sat/vB
128+        fee_to_use = target_vsize_per_tx * 10 if target_vsize_per_tx is not None else int(fee * COIN * 5)
129+        bad_tx_also_replacement = self.wallet.create_self_transfer_multi(
130+            utxos_to_spend=[last_result["new_utxo"], utxo_to_double_spend],
131+            target_vsize=target_vsize_per_tx,
132+            fee_per_output=fee_to_use,
133+        )
134+        assert_raises_rpc_error(-26, "replacement-failed", node.sendrawtransaction, bad_tx_also_replacement["hex"])
135+
136+        # Replace the last transaction. We are extending the cluster by one, but also removing one: 64 + 1 - 1 = 64
137+        # In the case of vsize, it should similarly cancel out.
138+        second_to_last_utxo = cluster_submitted[-2]["new_utxo"]
139+        fee_to_beat = cluster_submitted[-1]["fee"]
140+        vsize_to_use = cluster_submitted[-1]["tx"].get_vsize() if target_vsize_per_tx is not None else None
141+        good_tx_replacement = self.wallet.create_self_transfer(utxo_to_spend=second_to_last_utxo, fee=fee_to_beat * 5, target_vsize=vsize_to_use)
142+        node.sendrawtransaction(good_tx_replacement["hex"], maxfeerate=0)
143+
144+        cluster_submitted[-1] = good_tx_replacement
145+
146+    def test_limit_enforcement_package(self, cluster_submitted):
147+        node = self.nodes[0]
148+        # Create a package from the second to last transaction. This shouldn't work because the effect is 64 + 2 - 1 = 65
149+        last_utxo = cluster_submitted[-2]["new_utxo"]
150+        fee_to_beat = cluster_submitted[-1]["fee"]
151+        # We do not use package RBF here because it has additional restrictions on mempool ancestors.
152+        parent_tx_bad = self.wallet.create_self_transfer(utxo_to_spend=last_utxo, fee=fee_to_beat * 5)
153+        child_tx_bad = self.wallet.create_self_transfer(utxo_to_spend=parent_tx_bad["new_utxo"])
154+        package_result = node.submitpackage([parent_tx_bad["hex"], child_tx_bad["hex"]], maxfeerate=0)
155+        # The parent should be submitted, but the child rejected.
156+        result_parent_only = node.submitpackage([parent_tx_bad["hex"], child_tx_bad["hex"]])
157+
158+        assert parent_tx_bad["txid"] in node.getrawmempool()
159+        assert child_tx_bad["txid"] not in node.getrawmempool()
160+        assert_equal(result_parent_only["package_msg"], "transaction failed")
161+        assert_equal(result_parent_only["tx-results"][child_tx_bad["wtxid"]]["error"], "too-large-cluster")
162+
163+        # Now, create a package from the second to last transaction. This should work because the effect is 64 + 2 - 2 = 64
164+        third_to_last_utxo = cluster_submitted[-3]["new_utxo"]
165+        parent_tx_good = self.wallet.create_self_transfer(utxo_to_spend=third_to_last_utxo)
166+        child_tx_good = self.wallet.create_self_transfer(utxo_to_spend=parent_tx_good["new_utxo"], fee=fee_to_beat * 5)
167+        result_both_good = node.submitpackage([parent_tx_good["hex"], child_tx_good["hex"]], maxfeerate=0)
168+        assert_equal(result_both_good["package_msg"], "success")
169+        assert parent_tx_good["txid"] in node.getrawmempool()
170+        assert child_tx_good["txid"] in node.getrawmempool()
171+
172+ [@cleanup](/bitcoin-bitcoin/contributor/cleanup/)
173+    def test_cluster_count_limit(self, max_cluster_count):
174+        node = self.nodes[0]
175+        cluster_submitted = self.add_chain_cluster(node, max_cluster_count)
176+        self.check_feerate_diagram(node)
177+        for result in cluster_submitted:
178+            assert_equal(node.getmempoolcluster(result["txid"])['txcount'], max_cluster_count)
179+
180+        self.log.info("Test that cluster count limit is enforced")
181+        self.test_limit_enforcement(cluster_submitted)
182+        self.log.info("Test that the resulting cluster count is correctly calculated in a package")
183+        self.test_limit_enforcement_package(cluster_submitted)
184+
185+ [@cleanup](/bitcoin-bitcoin/contributor/cleanup/)
186+    def test_cluster_size_limit(self, max_cluster_size_vbytes):
187+        node = self.nodes[0]
188+        # This number should be smaller than the cluster count limit.
189+        num_txns = 10
190+        # Leave some buffer so it is possible to add a reasonably-sized transaction.
191+        target_vsize_per_tx = int((max_cluster_size_vbytes - 500) / num_txns)
192+        cluster_submitted = self.add_chain_cluster(node, num_txns, target_vsize_per_tx)
193+
194+        vsize_remaining = max_cluster_size_vbytes - node.getmempoolcluster(cluster_submitted[0]["txid"])['vsize']
195+        self.log.info("Test that cluster size limit is enforced")
196+        self.test_limit_enforcement(cluster_submitted, target_vsize_per_tx=vsize_remaining + 4)
197+
198+        # Try another cluster and add a small transaction: it should succeed
199+        last_result = cluster_submitted[-1]
200+        small_tx = self.wallet.create_self_transfer(utxo_to_spend=last_result["new_utxo"], target_vsize=vsize_remaining)
201+        node.sendrawtransaction(small_tx["hex"])
202+
203+ [@cleanup](/bitcoin-bitcoin/contributor/cleanup/)
204+    def test_cluster_merging(self, max_cluster_count):
205+        node = self.nodes[0]
206+
207+        self.log.info(f"Test merging 2 clusters with transaction counts totaling {max_cluster_count}")
208+        for num_txns_cluster1 in [1, 5, 10]:
209+            # Create a chain of transactions
210+            cluster1 = self.add_chain_cluster(node, num_txns_cluster1)
211+            for result in cluster1:
212+                node.sendrawtransaction(result["hex"])
213+            utxo_from_cluster1 = cluster1[-1]["new_utxo"]
214+
215+            # Make the next cluster, which contains the remaining transactions
216+            assert_greater_than(max_cluster_count, num_txns_cluster1)
217+            num_txns_cluster2 = max_cluster_count - num_txns_cluster1
218+            cluster2 = self.add_chain_cluster(node, num_txns_cluster2)
219+            for result in cluster2:
220+                node.sendrawtransaction(result["hex"])
221+            utxo_from_cluster2 = cluster2[-1]["new_utxo"]
222+
223+            # Now create a transaction that spends from both clusters, which would merge them.
224+            tx_merger = self.wallet.create_self_transfer_multi(utxos_to_spend=[utxo_from_cluster1, utxo_from_cluster2])
225+            assert_raises_rpc_error(-26, "too-large-cluster", node.sendrawtransaction, tx_merger["hex"])
226+
227+            # Spending from the clusters independently should work
228+            tx_spending_cluster1 = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=utxo_from_cluster1)
229+            tx_spending_cluster2 = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=utxo_from_cluster2)
230+            assert tx_spending_cluster1["txid"] in node.getrawmempool()
231+            assert tx_spending_cluster2["txid"] in node.getrawmempool()
232+
233+        self.log.info(f"Test merging {max_cluster_count} clusters with 1 transaction spending from all of them")
234+        utxos_to_merge = []
235+        for _ in range(max_cluster_count):
236+            # Use a confirmed utxo to ensure distinct clusters
237+            confirmed_utxo = self.wallet.get_utxo(confirmed_only=True)
238+            singleton = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=confirmed_utxo)
239+            assert singleton["txid"] in node.getrawmempool()
240+            utxos_to_merge.append(singleton["new_utxo"])
241+
242+        assert_equal(len(utxos_to_merge), max_cluster_count)
243+        tx_merger = self.wallet.create_self_transfer_multi(utxos_to_spend=utxos_to_merge)
244+        assert_raises_rpc_error(-26, "too-large-cluster", node.sendrawtransaction, tx_merger["hex"])
245+
246+        # Spending from 1 fewer cluster should work
247+        tx_merger_all_but_one = self.wallet.create_self_transfer_multi(utxos_to_spend=utxos_to_merge[:-1])
248+        node.sendrawtransaction(tx_merger_all_but_one["hex"])
249+        assert tx_merger_all_but_one["txid"] in node.getrawmempool()
250+
251+ [@cleanup](/bitcoin-bitcoin/contributor/cleanup/)
252+    def test_cluster_merging_size(self, max_cluster_size_vbytes):
253+        node = self.nodes[0]
254+
255+        self.log.info(f"Test merging clusters with sizes totaling {max_cluster_size_vbytes} vB")
256+        num_txns = 10
257+        # Leave some buffer so it is possible to add a reasonably-sized transaction.
258+        target_vsize_per_tx = int((max_cluster_size_vbytes - 500) / num_txns)
259+        utxos_to_merge = []
260+        vsize_remaining = max_cluster_size_vbytes
261+        for _ in range(num_txns):
262+            confirmed_utxo = self.wallet.get_utxo(confirmed_only=True)
263+            singleton = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=confirmed_utxo)
264+            assert singleton["txid"] in node.getrawmempool()
265+            utxos_to_merge.append(singleton["new_utxo"])
266+            vsize_remaining -= singleton["tx"].get_vsize()
267+
268+        assert_greater_than_or_equal(vsize_remaining, 500)
269+
270+        # Create a transaction spending from all clusters that exceeds the cluster size limit.
271+        tx_merger_too_big = self.wallet.create_self_transfer_multi(utxos_to_spend=utxos_to_merge, target_vsize=vsize_remaining + 4, fee_per_output=10000)
272+        assert_raises_rpc_error(-26, "too-large-cluster", node.sendrawtransaction, tx_merger_too_big["hex"])
273+
274+        # A transaction that is slightly smaller should work.
275+        tx_merger_small = self.wallet.create_self_transfer_multi(utxos_to_spend=utxos_to_merge[:-1], target_vsize=vsize_remaining - 4, fee_per_output=10000)
276+        node.sendrawtransaction(tx_merger_small["hex"])
277+        assert tx_merger_small["txid"] in node.getrawmempool()
278+
279+ [@cleanup](/bitcoin-bitcoin/contributor/cleanup/)
280+    def test_cluster_limit_rbf(self, max_cluster_count):
281+        node = self.nodes[0]
282+
283+        # Use min feerate for the to-be-replaced transactions. There are many, so replacement cost can be expensive.
284+        min_feerate = node.getmempoolinfo()["mempoolminfee"]
285+
286+        self.log.info("Test that cluster size calculation takes RBF into account")
287+        utxos_created_by_parents = []
288+        fees_rbf_sats = 0
289+        for _ in range(max_cluster_count - 1):
290+            parent_tx = self.wallet.send_self_transfer(from_node=node, confirmed_only=True)
291+            utxo_to_replace = parent_tx["new_utxo"]
292+            child_tx = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=utxo_to_replace, fee_rate=min_feerate)
293+
294+            fees_rbf_sats += int(child_tx["fee"] * COIN)
295+            utxos_created_by_parents.append(utxo_to_replace)
296+
297+        # This transaction would create a cluster of size max_cluster_count
298+        # Importantly, the node should account for the fact that half of the transactions will be replaced.
299+        tx_merger_replacer = self.wallet.create_self_transfer_multi(utxos_to_spend=utxos_created_by_parents, fee_per_output=fees_rbf_sats * 2)
300+        node.sendrawtransaction(tx_merger_replacer["hex"])
301+        assert tx_merger_replacer["txid"] in node.getrawmempool()
302+        assert_equal(node.getmempoolcluster(tx_merger_replacer["txid"])['txcount'], max_cluster_count)
303+
304+        self.log.info("Test that cluster size calculation takes package RBF into account")
305+        utxos_to_replace = []
306+        fee_rbf_decimal = 0
307+        for _ in range(max_cluster_count):
308+            confirmed_utxo = self.wallet.get_utxo(confirmed_only=True)
309+            tx_to_replace = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=confirmed_utxo, fee_rate=min_feerate)
310+            fee_rbf_decimal += tx_to_replace["fee"]
311+            utxos_to_replace.append(confirmed_utxo)
312+
313+        tx_replacer = self.wallet.create_self_transfer_multi(utxos_to_spend=utxos_to_replace)
314+        tx_replacer_sponsor = self.wallet.create_self_transfer(utxo_to_spend=tx_replacer["new_utxos"][0], fee=fee_rbf_decimal * 2)
315+        node.submitpackage([tx_replacer["hex"], tx_replacer_sponsor["hex"]], maxfeerate=0)
316+        assert tx_replacer["txid"] in node.getrawmempool()
317+        assert tx_replacer_sponsor["txid"] in node.getrawmempool()
318+        assert_equal(node.getmempoolcluster(tx_replacer["txid"])['txcount'], 2)
319+
320+
321+    def run_test(self):
322+        node = self.nodes[0]
323+        self.wallet = MiniWallet(node)
324+        self.generate(self.wallet, 400)
325+
326+        self.test_cluster_limit_rbf(DEFAULT_CLUSTER_LIMIT)
327+
328+        for cluster_size_limit_kvb in [10, 20, 33, 100, DEFAULT_CLUSTER_SIZE_LIMIT_KVB]:
329+            self.log.info(f"-> Resetting node with -limitclustersize={cluster_size_limit_kvb}")
330+            self.restart_node(0, extra_args=[f"-limitclustersize={cluster_size_limit_kvb}"])
331+
332+            cluster_size_limit = cluster_size_limit_kvb * 1000
333+            self.test_cluster_size_limit(cluster_size_limit)
334+            self.test_cluster_merging_size(cluster_size_limit)
335+
336+        for cluster_count_limit in [4, 10, 16, 32, DEFAULT_CLUSTER_LIMIT]:
337+            self.log.info(f"-> Resetting node with -limitclustercount={cluster_count_limit}")
338+            self.restart_node(0, extra_args=[f"-limitclustercount={cluster_count_limit}"])
339+
340+            self.test_cluster_count_limit(cluster_count_limit)
341+            if cluster_count_limit > 10:
342+                self.test_cluster_merging(cluster_count_limit)
343+
344 
345 if __name__ == '__main__':
346     MempoolClusterTest(__file__).main()
347diff --git a/test/functional/test_framework/mempool_util.py b/test/functional/test_framework/mempool_util.py
348index 3c4609c0b44..89e2558307e 100644
349--- a/test/functional/test_framework/mempool_util.py
350+++ b/test/functional/test_framework/mempool_util.py
351@@ -33,6 +33,8 @@ from .wallet import (
352 DEFAULT_MIN_RELAY_TX_FEE = 100
353 # Default for -incrementalrelayfee in sat/kvB
354 DEFAULT_INCREMENTAL_RELAY_FEE = 100
355+DEFAULT_CLUSTER_LIMIT = 64
356+DEFAULT_CLUSTER_SIZE_LIMIT_KVB = 101
357 
358 TRUC_MAX_VSIZE = 10000
359 TRUC_CHILD_MAX_VSIZE = 1000

 0diff --git a/src/node/miner.cpp b/src/node/miner.cpp
 1index 52103bda82..1bfe771444 100644
 2--- a/src/node/miner.cpp
 3+++ b/src/node/miner.cpp
 4@@ -206,8 +206,8 @@ bool BlockAssembler::TestPackage(FeePerWeight package_feerate, int64_t packageSi
 5 // Perform transaction-level checks before adding to block:
 6 // - transaction finality (locktime)
 7-bool BlockAssembler::TestPackageTransactions(const std::vector<const CTxMemPoolEntry *>& txs) const
 8+bool BlockAssembler::TestPackageTransactions(const std::vector<CTxMemPoolEntryRef>& txs) const
 9 {
10-    for (auto tx : txs) {
11-        if (!IsFinalTx(tx->GetTx(), nHeight, m_lock_time_cutoff)) {
12+    for (const auto tx : txs) {
13+        if (!IsFinalTx(tx.get().GetTx(), nHeight, m_lock_time_cutoff)) {
14             return false;
15         }
16@@ -243,13 +243,11 @@ void BlockAssembler::addChunks()
17 
18     std::vector<CTxMemPoolEntry::CTxMemPoolEntryRef> selected_transactions;
19+    selected_transactions.reserve(MAX_CLUSTER_COUNT_LIMIT);
20     FeePerWeight chunk_feerate;
21 
22+    // This fills selected_transactions
23     chunk_feerate = m_mempool->GetBlockBuilderChunk(selected_transactions);
24     FeePerVSize chunk_feerate_vsize = ToFeePerVSize(chunk_feerate);
25 
26-    std::vector<const CTxMemPoolEntry*> chunk_txs;
27-    // We'll add at most one chunk per iteration below, and chunk count is bounded by
28-    // the cluster size limit.
29-    chunk_txs.reserve(MAX_CLUSTER_COUNT_LIMIT);
30     while (selected_transactions.size() > 0) {
31         // Check to see if min fee rate is still respected.
32@@ -260,12 +258,10 @@ void BlockAssembler::addChunks()
33 
34         int64_t package_sig_ops = 0;
35-        chunk_txs.clear();
36         for (const auto& tx : selected_transactions) {
37-            chunk_txs.emplace_back(&tx.get());
38             package_sig_ops += tx.get().GetSigOpCost();
39         }
40 
41         // Check to see if this chunk will fit.
42-        if (!TestPackage(chunk_feerate, package_sig_ops) || !TestPackageTransactions(chunk_txs)) {
43+        if (!TestPackage(chunk_feerate, package_sig_ops) || !TestPackageTransactions(selected_transactions)) {
44             m_mempool->SkipBuilderChunk();
45             // This chunk won't fit, so we let it be removed from the heap and
46@@ -283,6 +279,6 @@ void BlockAssembler::addChunks()
47             // This chunk will fit, so add it to the block.
48             nConsecutiveFailed = 0;
49-            for (const auto& tx : chunk_txs) {
50-                AddToBlock(*tx);
51+            for (const auto& tx : selected_transactions) {
52+                AddToBlock(tx);
53             }
54             pblocktemplate->m_package_feerates.emplace_back(chunk_feerate_vsize);
55diff --git a/src/node/miner.h b/src/node/miner.h
56index 58fedcc1af..a42e2e2cb4 100644
57--- a/src/node/miner.h
58+++ b/src/node/miner.h
59@@ -117,5 +117,5 @@ private:
60       * These checks should always succeed, and they're here
61       * only as an extra check in case of suboptimal node configuration */
62-    bool TestPackageTransactions(const std::vector<const CTxMemPoolEntry *>& txs) const;
63+    bool TestPackageTransactions(const std::vector<CTxMemPoolEntryRef>& txs) const;
64 };

0        // If we can't calculate a feerate, it's because the cluster size limits were hit, and we may want to try package RBF.

0        Assume(!m_txgraph->IsOversized(TxGraph::Level::MAIN));
1        return m_txgraph->CountDistinctClusters(entries, TxGraph::Level::MAIN);

0    // helper functions for addChunks()

 0diff --git a/src/node/miner.cpp b/src/node/miner.cpp
 1index 1bfe7714440..e80d55c1f31 100644
 2--- a/src/node/miner.cpp
 3+++ b/src/node/miner.cpp
 4@@ -192,12 +192,12 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock()
 5     return std::move(pblocktemplate);
 6 }
 7 
 8-bool BlockAssembler::TestPackage(FeePerWeight package_feerate, int64_t packageSigOpsCost) const
 9+bool BlockAssembler::TestChunk(FeePerWeight chunk_feerate, int64_t chunk_sigops_cost) const
10 {
11-    if (nBlockWeight + package_feerate.size >= m_options.nBlockMaxWeight) {
12+    if (nBlockWeight + chunk_feerate.size >= m_options.nBlockMaxWeight) {
13         return false;
14     }
15-    if (nBlockSigOpsCost + packageSigOpsCost >= MAX_BLOCK_SIGOPS_COST) {
16+    if (nBlockSigOpsCost + chunk_sigops_cost >= MAX_BLOCK_SIGOPS_COST) {
17         return false;
18     }
19     return true;
20@@ -205,7 +205,7 @@ bool BlockAssembler::TestPackage(FeePerWeight package_feerate, int64_t packageSi
21 
22 // Perform transaction-level checks before adding to block:
23 // - transaction finality (locktime)
24-bool BlockAssembler::TestPackageTransactions(const std::vector<CTxMemPoolEntryRef>& txs) const
25+bool BlockAssembler::TestChunkTransactions(const std::vector<CTxMemPoolEntryRef>& txs) const
26 {
27     for (const auto tx : txs) {
28         if (!IsFinalTx(tx.get().GetTx(), nHeight, m_lock_time_cutoff)) {
29@@ -256,13 +256,13 @@ void BlockAssembler::addChunks()
30             return;
31         }
32 
33-        int64_t package_sig_ops = 0;
34+        int64_t chunk_sig_ops = 0;
35         for (const auto& tx : selected_transactions) {
36-            package_sig_ops += tx.get().GetSigOpCost();
37+            chunk_sig_ops += tx.get().GetSigOpCost();
38         }
39 
40         // Check to see if this chunk will fit.
41-        if (!TestPackage(chunk_feerate, package_sig_ops) || !TestPackageTransactions(selected_transactions)) {
42+        if (!TestChunk(chunk_feerate, chunk_sig_ops) || !TestChunkTransactions(selected_transactions)) {
43             m_mempool->SkipBuilderChunk();
44             // This chunk won't fit, so we let it be removed from the heap and
45             // we'll try the next best.
46@@ -281,7 +281,7 @@ void BlockAssembler::addChunks()
47             for (const auto& tx : selected_transactions) {
48                 AddToBlock(tx);
49             }
50-            pblocktemplate->m_package_feerates.emplace_back(chunk_feerate_vsize);
51+            pblocktemplate->m_chunk_feerates.emplace_back(chunk_feerate_vsize);
52         }
53 
54         selected_transactions.clear();
55diff --git a/src/node/miner.h b/src/node/miner.h
56index a42e2e2cb48..5fee028d9cd 100644
57--- a/src/node/miner.h
58+++ b/src/node/miner.h
59@@ -47,9 +47,9 @@ struct CBlockTemplate
60     // Sigops per transaction, not including coinbase transaction (unlike CBlock::vtx).
61     std::vector<int64_t> vTxSigOpsCost;
62     std::vector<unsigned char> vchCoinbaseCommitment;
63-    /* A vector of package fee rates, ordered by the sequence in which
64-     * packages are selected for inclusion in the block template.*/
65-    std::vector<FeePerVSize> m_package_feerates;
66+    /* A vector of chunk fee rates, ordered by the sequence in which
67+     * chunks are selected for inclusion in the block template.*/
68+    std::vector<FeePerVSize> m_chunk_feerates;
69 };
70 
71 /** Generate a new block, without valid proof-of-work */
72@@ -109,14 +109,14 @@ private:
73     */
74     void addChunks() EXCLUSIVE_LOCKS_REQUIRED(m_mempool->cs);
75 
76-    // helper functions for addPackageTxs()
77-    /** Test if a new package would "fit" in the block */
78-    bool TestPackage(FeePerWeight package_feerate, int64_t packageSigOpsCost) const;
79-    /** Perform checks on each transaction in a package:
80+    // helper functions for addChunkTxs()
81+    /** Test if a new chunk would "fit" in the block */
82+    bool TestChunk(FeePerWeight chunk_feerate, int64_t chunk_sigops_cost) const;
83+    /** Perform checks on each transaction in a chunk:
84       * locktime, premature-witness, serialized size (if necessary)
85       * These checks should always succeed, and they're here
86       * only as an extra check in case of suboptimal node configuration */
87-    bool TestPackageTransactions(const std::vector<CTxMemPoolEntryRef>& txs) const;
88+    bool TestChunkTransactions(const std::vector<CTxMemPoolEntryRef>& txs) const;
89 };
90 
91 /**

0        """Replacements that evict too many clusters are rejected"""

0            assert_greater_than_or_equal(last_val[1] * x['vsize'], last_val[0] * x['fee'])

  0diff --git a/test/functional/mempool_cluster.py b/test/functional/mempool_cluster.py
  1index ee8005a1cd2..c86e6a1b5c0 100755
  2--- a/test/functional/mempool_cluster.py
  3+++ b/test/functional/mempool_cluster.py
  4@@ -7,6 +7,8 @@
  5 from decimal import Decimal
  6 
  7 from test_framework.mempool_util import (
  8+    assert_equal_feerate_diagram,
  9+    check_feerate_diagram_monotonically_decreasing,
 10     DEFAULT_CLUSTER_LIMIT,
 11     DEFAULT_CLUSTER_SIZE_LIMIT_KVB,
 12 )
 13@@ -71,17 +73,6 @@ class MempoolClusterTest(BitcoinTestFramework):
 14         assert node.getmempoolcluster(parent_tx['txid'])['txcount'] == cluster_count
 15         return all_results
 16 
 17-    def check_feerate_diagram(self, node):
 18-        """Sanity check the feerate diagram."""
 19-        feeratediagram = node.getmempoolfeeratediagram()
 20-        last_val = [0, 0]
 21-        for x in feeratediagram:
 22-            # The vsize is always positive, except for the first iteration
 23-            assert x['vsize'] > 0 or x['fee'] == 0
 24-            # Monotonically decreasing fee per vsize
 25-            assert_greater_than_or_equal(last_val[0] * x['fee'], last_val[1] * x['vsize'])
 26-            last_val = [x['vsize'], x['fee']]
 27-
 28     def test_limit_enforcement(self, cluster_submitted, target_vsize_per_tx=None):
 29         """
 30         the cluster may change as a result of these transactions, so cluster_submitted is mutated accordingly
 31@@ -149,7 +140,7 @@ class MempoolClusterTest(BitcoinTestFramework):
 32     def test_cluster_count_limit(self, max_cluster_count):
 33         node = self.nodes[0]
 34         cluster_submitted = self.add_chain_cluster(node, max_cluster_count)
 35-        self.check_feerate_diagram(node)
 36+        check_feerate_diagram_monotonically_decreasing(node.getmempoolfeeratediagram())
 37         for result in cluster_submitted:
 38             assert_equal(node.getmempoolcluster(result["txid"])['txcount'], max_cluster_count)
 39 
 40@@ -292,6 +283,151 @@ class MempoolClusterTest(BitcoinTestFramework):
 41         assert tx_replacer_sponsor["txid"] in node.getrawmempool()
 42         assert_equal(node.getmempoolcluster(tx_replacer["txid"])['txcount'], 2)
 43 
 44+ [@cleanup](/bitcoin-bitcoin/contributor/cleanup/)
 45+    def test_feerate_diagram(self):
 46+        node = self.nodes[0]
 47+        self.log.info("Test that the feerate diagram shows chunks correctly")
 48+
 49+        # 1 sat/vB as Decimal BTC/kvB
 50+        feerate_1000sat_kvb = Decimal(1000) / COIN
 51+
 52+        def sats_to_btc(sats):
 53+            """Convert int sats to Decimal BTC with 8 decimal places"""
 54+            return Decimal(sats) / Decimal(1e8)
 55+
 56+        # txA (0sat / 500vB) <- txB (1000sat / 500vB)
 57+        # Use v3 to allow 0 fee
 58+        txA = self.wallet.create_self_transfer(confirmed_only=True, fee=0, fee_rate=0, version=3, target_vsize=500)
 59+        txB = self.wallet.create_self_transfer(utxo_to_spend=txA["new_utxo"], fee=sats_to_btc(1000), version=3, target_vsize=500)
 60+        result_ab = node.submitpackage([txA["hex"], txB["hex"]])
 61+        assert_equal(result_ab["package_msg"], "success")
 62+
 63+        # one cluster, one chunk with feerate 1sat/vB
 64+        assert_equal(result_ab["tx-results"][txA["wtxid"]]["fees"]["effective-feerate"], feerate_1000sat_kvb)
 65+        assert_equal(result_ab["tx-results"][txB["wtxid"]]["fees"]["effective-feerate"], feerate_1000sat_kvb)
 66+        assert_equal(node.getmempoolcluster(txA["txid"])['txcount'], 2)
 67+        expected_feerate_diagram_ab = [
 68+            [0, 0],
 69+            [1000, 1000], # [txA, txB] 1sat/vB
 70+        ]
 71+        assert_equal_feerate_diagram(expected_feerate_diagram_ab, node.getmempoolfeeratediagram())
 72+
 73+        # txC (0sat / 1000vB) <- txD (2000sat / 1000vB)
 74+        # Use v3 to allow 0 fee
 75+        txC = self.wallet.create_self_transfer(confirmed_only=True, fee=0, fee_rate=0, version=3, target_vsize=1000)
 76+        txD = self.wallet.create_self_transfer(utxo_to_spend=txC["new_utxo"], fee=sats_to_btc(2000), version=3, target_vsize=1000)
 77+        result_cd = node.submitpackage([txC["hex"], txD["hex"]])
 78+
 79+        # one cluster, one chunks with feerate 1sat/vB
 80+        assert_equal(result_cd["package_msg"], "success")
 81+        assert_equal(result_cd["tx-results"][txC["wtxid"]]["fees"]["effective-feerate"], feerate_1000sat_kvb)
 82+        assert_equal(result_cd["tx-results"][txD["wtxid"]]["fees"]["effective-feerate"], feerate_1000sat_kvb)
 83+        assert_equal(node.getmempoolcluster(txC["txid"])['txcount'], 2)
 84+        # Same chunk feerate as [txA, txB], but [txC, txD] has larger vsize.
 85+        expected_feerate_diagram_cd = [
 86+            [0, 0],
 87+            [1000, 1000], # [txA, txB] 1sat/vB
 88+            [3000, 3000], # [txC, txD] 1sat/vB
 89+        ]
 90+        assert_equal_feerate_diagram(expected_feerate_diagram_cd, node.getmempoolfeeratediagram())
 91+
 92+        self.log.info("Test that the feerate diagram uses modified fees")
 93+        # txE (800sat / 400vB)
 94+        # 799sat of fees will come from prioritisetransaction
 95+        txE = self.wallet.create_self_transfer(confirmed_only=True, fee=sats_to_btc(1), target_vsize=400)
 96+        node.prioritisetransaction(txid=txE["txid"], fee_delta=799)
 97+        node.sendrawtransaction(txE["hex"])
 98+        assert_equal(node.getmempoolcluster(txE["txid"])['txcount'], 1)
 99+        expected_feerate_diagram_e = [
100+            [0, 0],
101+            [800, 400], # [txE] 2sat/vB
102+            [1800, 1400], # [txA, txB] 1sat/vB
103+            [3800, 3400], # [txC, txD] 1sat/vB
104+        ]
105+        assert_equal_feerate_diagram(expected_feerate_diagram_e, node.getmempoolfeeratediagram())
106+
107+        # txF (750sat / 500vB) <- txG (6250sat / 500vB)
108+        txF = self.wallet.create_self_transfer(confirmed_only=True, fee=sats_to_btc(750), target_vsize=500)
109+        txG = self.wallet.create_self_transfer(utxo_to_spend=txF["new_utxo"], fee=sats_to_btc(6250), target_vsize=500)
110+
111+        # Submit them individually to see txF's chunk feerate change.
112+        node.sendrawtransaction(txF["hex"])
113+        assert_equal(node.getmempoolcluster(txF["txid"])['txcount'], 1)
114+        # txF has a feerate of 1.5sat/vB, so it's in the middle
115+        expected_feerate_diagram_f = [
116+            [0, 0],
117+            [800, 400], # [txE] 2sat/vB
118+            [1550, 900], # [txF] 1.5sat/vB
119+            [2550, 1900], # [txA, txB] 1sat/vB
120+            [4550, 3900], # [txC, txD] 1sat/vB
121+        ]
122+        assert_equal_feerate_diagram(expected_feerate_diagram_f, node.getmempoolfeeratediagram())
123+
124+        # txG bumps txF's chunk feerate to 7sat/vB
125+        node.sendrawtransaction(txG["hex"])
126+        assert_equal(node.getmempoolcluster(txF["txid"])['txcount'], 2)
127+        expected_feerate_diagram_g = [
128+            [0, 0],
129+            [7000, 1000], # [txF, txG] 7sat/vB
130+            [7800, 1400], # [txE] 2sat/vB
131+            [8800, 2400], # [txA, txB] 1sat/vB
132+            [10800, 4400], # [txC, txD] 1sat/vB
133+        ]
134+        assert_equal_feerate_diagram(expected_feerate_diagram_g, node.getmempoolfeeratediagram())
135+
136+        self.log.info("Test that prioritisetransaction on a mempool entry affects the feerate diagram")
137+        # Prioritise txD to make its chunk feerate a little more than 8sat/vB
138+        node.prioritisetransaction(txid=txD["txid"], fee_delta=14012)
139+        expected_feerate_diagram_d_prio = [
140+            [0, 0],
141+            [16012, 2000], # [txC, txD] 8.006sat/vB
142+            [23012, 3000], # [txF, txG] 7sat/vB
143+            [23812, 3400], # [txE] 2sat/vB
144+            [24812, 4400], # [txA, txB] 1sat/vB
145+        ]
146+        assert_equal_feerate_diagram(expected_feerate_diagram_d_prio, node.getmempoolfeeratediagram())
147+
148+        # De-prioritise txG to split up the chunk, putting txF behind txE and txG at the very end.
149+        node.prioritisetransaction(txid=txG["txid"], fee_delta=-6195)
150+        expected_feerate_diagram_g_deprio = [
151+            [0, 0],
152+            [16012, 2000], # [txC, txD] 8.006sat/vB
153+            [16812, 2400], # [txE] 2sat/vB
154+            [17562, 2900], # [txF] 1.5sat/vB
155+            [18562, 3900], # [txA, txB] 1sat/vB
156+            [18617, 4400], # [txG] 0.11sat/vB
157+        ]
158+        assert_equal_feerate_diagram(expected_feerate_diagram_g_deprio, node.getmempoolfeeratediagram())
159+
160+        # txH (30sat / 300vB) Spend txE and txG to merge their clusters, but keeping the chunking the same.
161+        txH = self.wallet.create_self_transfer_multi(utxos_to_spend=[txE["new_utxo"], txG["new_utxo"]], fee_per_output=30, target_vsize=300)
162+        node.sendrawtransaction(txH["hex"])
163+        # The cluster is now EFGH
164+        assert_equal(node.getmempoolcluster(txE["txid"])['txcount'], 4)
165+        expected_feerate_diagram_h = [
166+            [0, 0],
167+            [16012, 2000], # [txC, txD] 8.006sat/vB
168+            [16812, 2400], # [txE] 2sat/vB
169+            [17562, 2900], # [txF] 1.5sat/vB
170+            [18562, 3900], # [txA, txB] 1sat/vB
171+            [18617, 4400], # [txG] 0.11sat/vB
172+            [18647, 4700], # [txH] 0.1sat/vB
173+        ]
174+        assert_equal_feerate_diagram(expected_feerate_diagram_h, node.getmempoolfeeratediagram())
175+
176+        # txI (2150sat / 200vB) bumps txF, txG, txH to 1.99sat/vB, combining them into a single chunk
177+        txI = self.wallet.create_self_transfer(utxo_to_spend=txH["new_utxos"][0], fee=sats_to_btc(2150), target_vsize=200)
178+        node.sendrawtransaction(txI["hex"])
179+        # The cluster is now EFGHI
180+        assert_equal(node.getmempoolcluster(txI["txid"])['txcount'], 5)
181+        expected_feerate_diagram_i = [
182+            [0, 0],
183+            [16012, 2000], # [txC, txD] 8.006sat/vB
184+            [16812, 2400], # [txE] 2sat/vB
185+            [19797, 3900], # [txF, txG, txH, txI] 1.99sat/vB
186+            [20797, 4900], # [txA, txB] 1sat/vB
187+        ]
188+        assert_equal_feerate_diagram(expected_feerate_diagram_i, node.getmempoolfeeratediagram())
189 
190     def run_test(self):
191         node = self.nodes[0]
192@@ -299,6 +435,7 @@ class MempoolClusterTest(BitcoinTestFramework):
193         self.generate(self.wallet, 400)
194 
195         self.test_cluster_limit_rbf(DEFAULT_CLUSTER_LIMIT)
196+        self.test_feerate_diagram()
197 
198         for cluster_size_limit_kvb in [10, 20, 33, 100, DEFAULT_CLUSTER_SIZE_LIMIT_KVB]:
199             self.log.info(f"-> Resetting node with -limitclustersize={cluster_size_limit_kvb}")
200diff --git a/test/functional/test_framework/mempool_util.py b/test/functional/test_framework/mempool_util.py
201index 89e2558307e..2bce87bda2f 100644
202--- a/test/functional/test_framework/mempool_util.py
203+++ b/test/functional/test_framework/mempool_util.py
204@@ -10,6 +10,7 @@ from .blocktools import (
205 )
206 from .messages import (
207     COutPoint,
208+    COIN,
209     CTransaction,
210     CTxIn,
211     CTxInWitness,
212@@ -22,6 +23,7 @@ from .script import (
213 from .util import (
214     assert_equal,
215     assert_greater_than,
216+    assert_greater_than_or_equal,
217     create_lots_of_big_transactions,
218     gen_return_txouts,
219 )
220@@ -131,3 +133,28 @@ def create_large_orphan():
221     tx.wit.vtxinwit[0].scriptWitness.stack = [CScript(b'X' * 390000)]
222     tx.vout = [CTxOut(100, CScript([OP_RETURN, b'a' * 20]))]
223     return tx
224+
225+def check_feerate_diagram_monotonically_decreasing(feerate_diagram):
226+    """Sanity check the feerate diagram."""
227+    last_val = [0, 0]
228+    for x in feerate_diagram:
229+        # The vsize is always positive, except for the first iteration
230+        assert x['vsize'] > 0 or x['fee'] == 0
231+        # Monotonically decreasing fee per vsize
232+        assert_greater_than_or_equal(last_val[0] * x['vsize'], last_val[1] * x['fee'])
233+        last_val = [x['vsize'], x['fee']]
234+
235+def assert_equal_feerate_diagram(expected, actual):
236+    """Check that expected and actual are equal, handling Decimal values and giving helpful error messages.
237+    expected: list of [fee, vsize] pairs where fee is an integer number of satoshis
238+    actual: list of { "fee": Decimal, "vsize": int } from the getmempoolfeeratediagram RPC
239+    Also sanity checks that the actual feerates are monotonically decreasing.
240+    """
241+    assert_equal(len(expected), len(actual))
242+    for i in range(len(expected)):
243+        # We convert the Decimal to an integer number to avoid Decimal comparisons.
244+        # For example, Decimal('0') == Decimal('0E-8') and Decimal('0.0001') == Decimal('0.00010000')
245+        assert_equal(expected[i][0], int(actual[i]["fee"] * COIN))
246+        assert_equal(expected[i][1], actual[i]["vsize"])
247+
248+    check_feerate_diagram_monotonically_decreasing(actual)

0* Feerate diagram policy enabled in conjunction with switch to cluster mempool as of **v31.0**.

  0diff --git a/doc/policy/packages.md b/doc/policy/packages.md
  1index 7522a984435..2e8f5cf8e0d 100644
  2--- a/doc/policy/packages.md
  3+++ b/doc/policy/packages.md
  4@@ -38,17 +38,10 @@ The following rules are enforced for all packages:
  5 
  6    - Packages are 1-parent-1-child, with no in-mempool ancestors of the package.
  7 
  8-   - All conflicting clusters (connected components of mempool transactions) must be clusters of up to size 2.
  9+   - The child's individual feerate must meet the mempool minimum feerate.
 10 
 11-   - No more than MAX_REPLACEMENT_CANDIDATES transactions can be replaced, analogous to
 12-     regular [replacement rule](./mempool-replacements.md) 5).
 13-
 14-   - Replacements must pay more total fees at the incremental relay fee (analogous to
 15-     regular [replacement rules](./mempool-replacements.md) 3 and 4).
 16-
 17-   - Parent feerate must be lower than package feerate.
 18-
 19-   - Must improve [feerate diagram](https://delvingbitcoin.org/t/mempool-incentive-compatibility/553). (#29242)
 20+   - All other [replacement rules](./mempool-replacements.md) are met: no more than 100 distinct clusters, total package
 21+   fees pay for the package size at the incremental relay feerate, and the feerate diagram improves.
 22 
 23    - *Rationale*: Basic support for package RBF can be used by wallets
 24      by making chains of no longer than two, then directly conflicting
 25@@ -56,18 +49,6 @@ The following rules are enforced for all packages:
 26      result in more robust fee bumping. More general package RBF may be
 27      enabled in the future.
 28 
 29-* When packages are evaluated against ancestor/descendant limits, the union of all transactions'
 30-  descendants and ancestors is considered. (#21800)
 31-
 32-   - *Rationale*: This is essentially a "worst case" heuristic intended for packages that are
 33-     heavily connected, i.e. some transaction in the package is the ancestor or descendant of all
 34-     the other transactions.
 35-
 36-* [CPFP Carve Out](./mempool-limits.md#CPFP-Carve-Out) is disabled in packaged contexts. (#21800)
 37-
 38-   - *Rationale*: This carve out cannot be accurately applied when there are multiple transactions'
 39-     ancestors and descendants being considered at the same time.
 40-
 41 The following rules are only enforced for packages to be submitted to the mempool (not
 42 enforced for test accepts):
 43 
 44diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp
 45index 1f167586fe9..fbdaa49f70e 100644
 46--- a/src/test/txpackage_tests.cpp
 47+++ b/src/test/txpackage_tests.cpp
 48@@ -1078,6 +1078,7 @@ BOOST_AUTO_TEST_CASE(package_cpfp_tests)
 49 BOOST_AUTO_TEST_CASE(package_rbf_tests)
 50 {
 51     mineBlocks(5);
 52+    MockMempoolMinFee(CFeeRate(1000));
 53     LOCK(::cs_main);
 54     size_t expected_pool_size = m_node.mempool->size();
 55     CKey child_key{GenerateRandomKey()};
 56@@ -1159,16 +1160,19 @@ BOOST_AUTO_TEST_CASE(package_rbf_tests)
 57             tx_parent_3, /*input_vout=*/0, /*input_height=*/101,
 58             child_key, child_spk, coinbase_value - 199 - 1300, /*submit=*/false));
 59 
 60+        CTransactionRef tx_parent_4 = MakeTransactionRef(CreateValidMempoolTransaction(
 61+            m_coinbase_txns[1], /*input_vout=*/0, /*input_height=*/0,
 62+            coinbaseKey, parent_spk, coinbase_value - 1763, /*submit=*/false));
 63+        CTransactionRef tx_child_4 = MakeTransactionRef(CreateValidMempoolTransaction(
 64+            tx_parent_4, /*input_vout=*/0, /*input_height=*/101,
 65+            child_key, child_spk, coinbase_value - 1763 - 60, /*submit=*/false));
 66+
 67         // In all packages, the parents conflict with each other
 68         BOOST_CHECK(tx_parent_1->GetHash() != tx_parent_2->GetHash() && tx_parent_2->GetHash() != tx_parent_3->GetHash());
 69 
 70         // 1 parent paying 200sat, 1 child paying 200sat.
 71         Package package1{tx_parent_1, tx_child_1};
 72-        // 1 parent paying 800sat, 1 child paying 200sat.
 73-        Package package2{tx_parent_2, tx_child_2};
 74-        // 1 parent paying 199sat, 1 child paying 1300sat.
 75-        Package package3{tx_parent_3, tx_child_3};
 76-
 77+        {
 78         const auto submit1 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package1, false, std::nullopt);
 79         if (auto err_1{CheckPackageMempoolAcceptResult(package1, submit1, /*expect_valid=*/true, m_node.mempool.get())}) {
 80             BOOST_ERROR(err_1.value());
 81@@ -1177,11 +1181,20 @@ BOOST_AUTO_TEST_CASE(package_rbf_tests)
 82         auto it_child_1 = submit1.m_tx_results.find(tx_child_1->GetWitnessHash());
 83         BOOST_CHECK_EQUAL(it_parent_1->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
 84         BOOST_CHECK_EQUAL(it_child_1->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
 85+        // Both validated individually.
 86+        BOOST_CHECK_EQUAL(it_parent_1->second.m_wtxids_fee_calculations.value().size(), 1);
 87+        BOOST_CHECK_EQUAL(it_parent_1->second.m_wtxids_fee_calculations.value().front(), tx_parent_1->GetWitnessHash());
 88+        BOOST_CHECK_EQUAL(it_child_1->second.m_wtxids_fee_calculations.value().size(), 1);
 89+        BOOST_CHECK_EQUAL(it_child_1->second.m_wtxids_fee_calculations.value().front(), tx_child_1->GetWitnessHash());
 90+        }
 91         expected_pool_size += 2;
 92         BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
 93 
 94         // This replacement is actually not package rbf; the parent carries enough fees
 95         // to replace the entire package on its own.
 96+        // 1 parent paying 800sat, 1 child paying 200sat.
 97+        Package package2{tx_parent_2, tx_child_2};
 98+        {
 99         const auto submit2 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package2, false, std::nullopt);
100         if (auto err_2{CheckPackageMempoolAcceptResult(package2, submit2, /*expect_valid=*/true, m_node.mempool.get())}) {
101             BOOST_ERROR(err_2.value());
102@@ -1190,8 +1203,17 @@ BOOST_AUTO_TEST_CASE(package_rbf_tests)
103         auto it_child_2 = submit2.m_tx_results.find(tx_child_2->GetWitnessHash());
104         BOOST_CHECK_EQUAL(it_parent_2->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
105         BOOST_CHECK_EQUAL(it_child_2->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
106+        // Both validated individually.
107+        BOOST_CHECK_EQUAL(it_parent_2->second.m_wtxids_fee_calculations.value().size(), 1);
108+        BOOST_CHECK_EQUAL(it_parent_2->second.m_wtxids_fee_calculations.value().front(), tx_parent_2->GetWitnessHash());
109+        BOOST_CHECK_EQUAL(it_child_2->second.m_wtxids_fee_calculations.value().size(), 1);
110+        BOOST_CHECK_EQUAL(it_child_2->second.m_wtxids_fee_calculations.value().front(), tx_child_2->GetWitnessHash());
111+        }
112         BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
113 
114+        // 1 parent paying 199sat, 1 child paying 1300sat.
115+        Package package3{tx_parent_3, tx_child_3};
116+        {
117         // Package RBF, in which the replacement transaction's child sponsors the fees to meet RBF feerate rules
118         const auto submit3 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package3, false, std::nullopt);
119         if (auto err_3{CheckPackageMempoolAcceptResult(package3, submit3, /*expect_valid=*/true, m_node.mempool.get())}) {
120@@ -1212,27 +1234,91 @@ BOOST_AUTO_TEST_CASE(package_rbf_tests)
121         BOOST_CHECK(it_child_3->second.m_wtxids_fee_calculations.value() == expected_package3_wtxids);
122         BOOST_CHECK_EQUAL(it_parent_3->second.m_effective_feerate.value().GetFee(package3_total_vsize), 199 + 1300);
123         BOOST_CHECK_EQUAL(it_child_3->second.m_effective_feerate.value().GetFee(package3_total_vsize), 199 + 1300);
124-
125+        }
126         BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
127 
128         // Finally, check that we can prioritise tx_child_1 to get package1 into the mempool.
129         // It should not be possible to resubmit package1 and get it in without prioritisation.
130+        // 1 parent paying 200sat, 1 child paying 200sat.
131+        Package package4{package1};
132+        {
133         const auto submit4 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package1, false, std::nullopt);
134-        if (auto err_4{CheckPackageMempoolAcceptResult(package1, submit4, /*expect_valid=*/false, m_node.mempool.get())}) {
135+        if (auto err_4{CheckPackageMempoolAcceptResult(package4, submit4, /*expect_valid=*/false, m_node.mempool.get())}) {
136             BOOST_ERROR(err_4.value());
137         }
138+        }
139+
140+        // 1 parent paying 200sat, 1 child paying 1563sat.
141+        Package package5{package1};
142         m_node.mempool->PrioritiseTransaction(tx_child_1->GetHash(), 1363);
143-        const auto submit5 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package1, false, std::nullopt);
144-        if (auto err_5{CheckPackageMempoolAcceptResult(package1, submit5, /*expect_valid=*/true, m_node.mempool.get())}) {
145+        {
146+        const auto submit5 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package5, false, std::nullopt);
147+        if (auto err_5{CheckPackageMempoolAcceptResult(package5, submit5, /*expect_valid=*/true, m_node.mempool.get())}) {
148             BOOST_ERROR(err_5.value());
149         }
150-        it_parent_1 = submit5.m_tx_results.find(tx_parent_1->GetWitnessHash());
151-        it_child_1 = submit5.m_tx_results.find(tx_child_1->GetWitnessHash());
152-        BOOST_CHECK_EQUAL(it_parent_1->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
153-        BOOST_CHECK_EQUAL(it_child_1->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
154-        LOCK(m_node.mempool->cs);
155-        BOOST_CHECK(m_node.mempool->GetIter(tx_parent_1->GetHash()).has_value());
156-        BOOST_CHECK(m_node.mempool->GetIter(tx_child_1->GetHash()).has_value());
157+        auto it_parent_5 = submit5.m_tx_results.find(tx_parent_1->GetWitnessHash());
158+        auto it_child_5 = submit5.m_tx_results.find(tx_child_1->GetWitnessHash());
159+        BOOST_CHECK_EQUAL(it_parent_5->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
160+        BOOST_CHECK_EQUAL(it_child_5->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
161+        // Validated together.
162+        const auto expected_wtxids_5 = std::vector<Wtxid>({tx_parent_1->GetWitnessHash(), tx_child_1->GetWitnessHash()});
163+        BOOST_CHECK(it_parent_5->second.m_wtxids_fee_calculations.value() == expected_wtxids_5);
164+        BOOST_CHECK(it_child_5->second.m_wtxids_fee_calculations.value() == expected_wtxids_5);
165+        const CFeeRate feerate_parent_child_5(1363 + 200 + 200, GetVirtualTransactionSize(*tx_parent_1) + GetVirtualTransactionSize(*tx_child_1));
166+        BOOST_CHECK(it_parent_5->second.m_effective_feerate.value() == feerate_parent_child_5);
167+        BOOST_CHECK(it_child_5->second.m_effective_feerate.value() == feerate_parent_child_5);
168+        }
169+        BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
170+
171+        // Check that the grouping for package RBF does not allow a parent to pay for a child below mempool minimum feerate.
172+        // This parent cannot pay for its own replacement, and can get assistance from the child, but the child should
173+        // not be eligible for submission at all.
174+        // 1 parent paying 1763sat, 1 child paying 60sat.
175+        Package package6{tx_parent_4, tx_child_4};
176+        {
177+        const auto submit6 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package6, false, std::nullopt);
178+        if (auto err_6{CheckPackageMempoolAcceptResult(package6, submit6, /*expect_valid=*/false, m_node.mempool.get())}) {
179+            BOOST_ERROR(err_6.value());
180+        }
181+        auto it_parent_6 = submit6.m_tx_results.find(tx_parent_4->GetWitnessHash());
182+        auto it_child_6 = submit6.m_tx_results.find(tx_child_4->GetWitnessHash());
183+        BOOST_CHECK_EQUAL(it_parent_6->second.m_result_type, MempoolAcceptResult::ResultType::INVALID);
184+        BOOST_CHECK_EQUAL(it_child_6->second.m_result_type, MempoolAcceptResult::ResultType::INVALID);
185+        // Parent does not meet rule 4
186+        BOOST_CHECK_EQUAL(it_parent_6->second.m_state.GetResult(), TxValidationResult::TX_RECONSIDERABLE);
187+        BOOST_CHECK_EQUAL(it_parent_6->second.m_state.GetRejectReason(), "insufficient fee");
188+        // Child needs parent's output
189+        BOOST_CHECK_EQUAL(it_child_6->second.m_state.GetResult(), TxValidationResult::TX_MISSING_INPUTS);
190+        BOOST_CHECK_EQUAL(it_child_6->second.m_state.GetRejectReason(), "bad-txns-inputs-missingorspent");
191+        }
192+        BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
193+
194+        // It is ok if the child pays fewer fees but subsidizes the parent's replacement cost, as long as the child meets the mempool minimum feerate.
195+        // 1 parent paying 1763sat, 1 child paying 160sat.
196+        // Even though the parent has a higher feerate, this isn't "parent pays for child" because the child meets the mempool minimum feerate.
197+        Package package7{package6};
198+        m_node.mempool->PrioritiseTransaction(tx_child_4->GetHash(), 100);
199+        {
200+        const auto submit7 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package7, false, std::nullopt);
201+        if (auto err_7{CheckPackageMempoolAcceptResult(package7, submit7, /*expect_valid=*/true, m_node.mempool.get())}) {
202+            BOOST_ERROR(err_7.value());
203+        }
204+        auto it_parent_7 = submit7.m_tx_results.find(tx_parent_4->GetWitnessHash());
205+        auto it_child_7 = submit7.m_tx_results.find(tx_child_4->GetWitnessHash());
206+        BOOST_CHECK_EQUAL(it_parent_7->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
207+        BOOST_CHECK_EQUAL(it_child_7->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
208+        // The two transactions were validated together.
209+        BOOST_CHECK_EQUAL(it_parent_7->second.m_wtxids_fee_calculations.value().size(), 2);
210+        BOOST_CHECK_EQUAL(it_parent_7->second.m_wtxids_fee_calculations.value().front(), tx_parent_4->GetWitnessHash());
211+        BOOST_CHECK_EQUAL(it_parent_7->second.m_wtxids_fee_calculations.value().back(), tx_child_4->GetWitnessHash());
212+        BOOST_CHECK_EQUAL(it_child_7->second.m_wtxids_fee_calculations.value().size(), 2);
213+        BOOST_CHECK_EQUAL(it_child_7->second.m_wtxids_fee_calculations.value().front(), tx_parent_4->GetWitnessHash());
214+        BOOST_CHECK_EQUAL(it_child_7->second.m_wtxids_fee_calculations.value().back(), tx_child_4->GetWitnessHash());
215+        const CFeeRate feerate_parent_child_7(1763 + 160, GetVirtualTransactionSize(*tx_parent_4) + GetVirtualTransactionSize(*tx_child_4));
216+        BOOST_CHECK(it_parent_7->second.m_effective_feerate.value() == feerate_parent_child_7);
217+        BOOST_CHECK(it_child_7->second.m_effective_feerate.value() == feerate_parent_child_7);
218+        }
219+        BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
220     }
221 }
222 BOOST_AUTO_TEST_SUITE_END()
223diff --git a/src/validation.cpp b/src/validation.cpp
224index 1102f2bb051..508892faea2 100644
225--- a/src/validation.cpp
226+++ b/src/validation.cpp
227@@ -1048,8 +1048,23 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txn
228         return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: package must be 1-parent-1-child");
229     }
230 
231-    // If the package has in-mempool parents, we won't consider a package RBF
232-    // since it would result in a cluster larger than 2.
233+    // Use the child as the transaction for attributing errors because it is presumably sponsoring this package RBF.
234+    auto& child_ws = workspaces[1];
235+    const Txid& child_hash = child_ws.m_ptx->GetHash();
236+
237+    // Check if this child is eligible to be a sponsor. We allow a child with a lower feerate than that of its
238+    // parents (and thus only helping pay for the replacement). However, the child itself must meet the mempool minimum
239+    // feerate, otherwise it would sponsor this replacement, potentially using the parents' fees to pay for its entry,
240+    // but be immediately evicted next time we trim the mempool.
241+    if (!CheckFeeRate(child_ws.m_vsize, child_ws.m_modified_fees, child_ws.m_state)) {
242+        // Add the CheckFeeRate error as the debug string.
243+        return package_state.Invalid(PackageValidationResult::PCKG_POLICY,
244+                                     "package RBF failed: child cannot sponsor because it does not meet mempool minimum feerate",
245+                                     child_ws.m_state.ToString());
246+    }
247+
248+
249+    // If the package has in-mempool parents, we won't consider a package RBF for now.
250     // N.B. To relax this constraint we will need to revisit how CCoinsViewMemPool::PackageAddTransaction
251     // is being used inside AcceptMultipleTransactions to track available inputs while processing a package.
252     // Specifically we would need to check that the ancestors of the new
253@@ -1062,6 +1077,7 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txn
254         }
255     }
256 
257+    // The remaining checks are similar to those for a single transaction's replacement(s).
258     // Aggregate all conflicts into one set.
259     CTxMemPool::setEntries direct_conflict_iters;
260     for (Workspace& ws : workspaces) {
261@@ -1069,9 +1085,6 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txn
262         direct_conflict_iters.merge(ws.m_iters_conflicting);
263     }
264 
265-    const auto& parent_ws = workspaces[0];
266-    const auto& child_ws = workspaces[1];
267-
268     // Don't consider replacements that would cause us to remove a large number of mempool entries.
269     // This limit is not increased in a package RBF. Use the aggregate number of transactions.
270     CTxMemPool::setEntries all_conflicts;
271@@ -1087,8 +1100,6 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txn
272         m_subpackage.m_conflicting_size += it->GetTxSize();
273     }
274 
275-    // Use the child as the transaction for attributing errors to.
276-    const Txid& child_hash = child_ws.m_ptx->GetHash();
277     if (const auto err_string{PaysForRBF(/*original_fees=*/m_subpackage.m_conflicting_fees,
278                                          /*replacement_fees=*/m_subpackage.m_total_modified_fees,
279                                          /*replacement_vsize=*/m_subpackage.m_total_vsize,
280@@ -1097,16 +1108,6 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txn
281                                      "package RBF failed: insufficient anti-DoS fees", *err_string);
282     }
283 
284-    // Ensure this two transaction package is a "chunk" on its own; we don't want the child
285-    // to be only paying anti-DoS fees
286-    const CFeeRate parent_feerate(parent_ws.m_modified_fees, parent_ws.m_vsize);
287-    const CFeeRate package_feerate(m_subpackage.m_total_modified_fees, m_subpackage.m_total_vsize);
288-    if (package_feerate <= parent_feerate) {
289-        return package_state.Invalid(PackageValidationResult::PCKG_POLICY,
290-                                     "package RBF failed: package feerate is less than or equal to parent feerate",
291-                                     strprintf("package feerate %s <= parent feerate is %s", package_feerate.ToString(), parent_feerate.ToString()));
292-    }
293-
294     // Check if it's economically rational to mine this package rather than the ones it replaces.
295     // This takes the place of ReplacementChecks()'s PaysMoreThanConflicts() in the package RBF setting.
296     if (const auto err_tup{ImprovesFeerateDiagram(*m_subpackage.m_changeset)}) {
297diff --git a/test/functional/mempool_package_rbf.py b/test/functional/mempool_package_rbf.py
298index 759e3cb07d3..c1983cab57b 100755
299--- a/test/functional/mempool_package_rbf.py
300+++ b/test/functional/mempool_package_rbf.py
301@@ -178,14 +178,10 @@ class PackageRBFTest(BitcoinTestFramework):
302         package_hex4, package_txns4 = self.create_simple_package(coin, parent_fee=DEFAULT_FEE, child_fee=DEFAULT_CHILD_FEE)
303         node.submitpackage(package_hex4)
304         self.assert_mempool_contents(expected=package_txns4)
305-        package_hex5, _package_txns5 = self.create_simple_package(coin, parent_fee=DEFAULT_CHILD_FEE, child_fee=DEFAULT_CHILD_FEE)
306-        pkg_results5 = node.submitpackage(package_hex5)
307-        assert 'package RBF failed: package feerate is less than or equal to parent feerate' in pkg_results5["package_msg"]
308-        self.assert_mempool_contents(expected=package_txns4)
309+        package_hex5, package_txns5 = self.create_simple_package(coin, parent_fee=DEFAULT_CHILD_FEE + DEFAULT_FEE, child_fee=DEFAULT_CHILD_FEE)
310+        node.submitpackage(package_hex5)
311+        self.assert_mempool_contents(expected=package_txns5)
312 
313-        package_hex5_1, package_txns5_1 = self.create_simple_package(coin, parent_fee=DEFAULT_CHILD_FEE, child_fee=DEFAULT_CHILD_FEE + Decimal("0.00000001"))
314-        node.submitpackage(package_hex5_1)
315-        self.assert_mempool_contents(expected=package_txns5_1)
316         self.generate(node, 1)
317 
318     def test_package_rbf_max_conflicts(self):