Stop enforcing descendant size/count limits

Cluster size limits should be enough.

src/txmempool.cpp

@@ -177,12 +177,6 @@ util::Result<CTxMemPool::setEntries> CTxMemPool::CalculateAncestorsAndCheckLimit
         ancestors.insert(stageit);
         staged_ancestors.erase(stage);
 
-        if (stageit->GetSizeWithDescendants() + entry_size > limits.descendant_size_vbytes) {
-            return util::Error{Untranslated(strprintf("exceeds descendant size limit for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limits.descendant_size_vbytes))};
-        } else if (stageit->GetCountWithDescendants() + entry_count > static_cast<uint64_t>(limits.descendant_count)) {
-            return util::Error{Untranslated(strprintf("too many descendants for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limits.descendant_count))};
-        }
-
         const CTxMemPoolEntry::Parents& parents = stageit->GetMemPoolParentsConst();
         for (const CTxMemPoolEntry& parent : parents) {
             txiter parent_it = mapTx.iterator_to(parent);
@@ -200,15 +194,6 @@ util::Result<CTxMemPool::setEntries> CTxMemPool::CalculateAncestorsAndCheckLimit
 util::Result<void> CTxMemPool::CheckPackageLimits(const Package& package,
                                                   const int64_t total_vsize) const
 {
-    size_t pack_count = package.size();
-
-    // Package itself is busting mempool limits; should be rejected even if no staged_ancestors exist
-    if (pack_count > static_cast<uint64_t>(m_opts.limits.descendant_count)) {
-        return util::Error{Untranslated(strprintf("package count %u exceeds descendant count limit [limit: %u]", pack_count, m_opts.limits.descendant_count))};
-    } else if (total_vsize > m_opts.limits.descendant_size_vbytes) {
-        return util::Error{Untranslated(strprintf("package size %u exceeds descendant size limit [limit: %u]", total_vsize, m_opts.limits.descendant_size_vbytes))};
-    }
-
     CTxMemPoolEntry::Parents staged_ancestors;
     for (const auto& tx : package) {
         for (const auto& input : tx->vin) {

test/functional/feature_rbf.py

@@ -27,15 +27,6 @@ MAX_CLUSTER_LIMIT = 64
 class ReplaceByFeeTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
-        self.extra_args = [
-            [
-                "-limitancestorcount=64",
-                "-limitdescendantcount=64",
-            ],
-            # second node has default mempool parameters
-            [
-            ],
-        ]
         self.uses_wallet = None
 
     def run_test(self):

test/functional/mempool_package_limits.py

@@ -15,7 +15,7 @@ from test_framework.wallet import MiniWallet
 # 2) run the subtest, which may submit some transaction(s) to the mempool and
 #    create a list of hex transactions
 # 3) testmempoolaccept the package hex and check that it fails with the error
-#    "package-mempool-limits" for each tx
+#    "too-large-cluster" for each tx
 # 4) after mining a block, clearing the pre-submitted transactions from mempool,
 #    check that submitting the created package succeeds
 def check_package_limits(func):
@@ -26,7 +26,7 @@ def check_package_limits(func):
         testres_error_expected = node.testmempoolaccept(rawtxs=package_hex)
         assert_equal(len(testres_error_expected), len(package_hex))
         for txres in testres_error_expected:
-            assert "package-mempool-limits" in txres["package-error"]
+            assert "too-large-cluster" in txres["package-error"]
 
         # Clear mempool and check that the package passes now
         self.generate(node, 1)
@@ -39,6 +39,7 @@ class MempoolPackageLimitsTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
         self.setup_clean_chain = True
+        self.extra_args = [["-limitclustercount=25"]]
 
     def run_test(self):
         self.wallet = MiniWallet(self.nodes[0])

test/functional/mempool_packages.py

@@ -7,19 +7,18 @@
 from decimal import Decimal
 
 from test_framework.messages import (
-    DEFAULT_DESCENDANT_LIMIT,
     DEFAULT_CLUSTER_LIMIT,
 )
 from test_framework.p2p import P2PTxInvStore
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
-    assert_raises_rpc_error,
 )
 from test_framework.wallet import MiniWallet
 
 # custom limits for node1
-CUSTOM_DESCENDANT_LIMIT = 11
+CUSTOM_CLUSTER_LIMIT = 10
+assert CUSTOM_CLUSTER_LIMIT < DEFAULT_CLUSTER_LIMIT
 
 class MempoolPackagesTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -30,7 +29,7 @@ class MempoolPackagesTest(BitcoinTestFramework):
             [
             ],
             [
-                "-limitdescendantcount={}".format(CUSTOM_DESCENDANT_LIMIT),
+                "-limitclustercount={}".format(CUSTOM_CLUSTER_LIMIT),
             ],
         ]
 
@@ -40,8 +39,8 @@ class MempoolPackagesTest(BitcoinTestFramework):
 
         peer_inv_store = self.nodes[0].add_p2p_connection(P2PTxInvStore()) # keep track of invs
 
-        # DEFAULT_DESCENDANT_LIMIT transactions off a confirmed tx should be fine
+        # DEFAULT_CLUSTER_LIMIT transactions off a confirmed tx should be fine for default node
-        chain = self.wallet.create_self_transfer_chain(chain_length=DEFAULT_DESCENDANT_LIMIT)
+        chain = self.wallet.create_self_transfer_chain(chain_length=DEFAULT_CLUSTER_LIMIT)
         witness_chain = [t["wtxid"] for t in chain]
         ancestor_vsize = 0
         ancestor_fees = Decimal(0)
@@ -55,22 +54,18 @@ class MempoolPackagesTest(BitcoinTestFramework):
         # Otherwise, getrawmempool may be inconsistent with getmempoolentry if unbroadcast changes in between
         peer_inv_store.wait_for_broadcast(witness_chain)
 
-        # Check mempool has DEFAULT_DESCENDANT_LIMIT transactions in it, and descendant and ancestor
+        # Check mempool has DEFAULT_CLUSTER_LIMIT transactions in it, and descendant and ancestor
         # count and fees should look correct
         mempool = self.nodes[0].getrawmempool(True)
-        assert_equal(len(mempool), DEFAULT_DESCENDANT_LIMIT)
+        assert_equal(len(mempool), DEFAULT_CLUSTER_LIMIT)
         descendant_count = 1
         descendant_fees = 0
         descendant_vsize = 0
 
         assert_equal(ancestor_vsize, sum([mempool[tx]['vsize'] for tx in mempool]))
-        ancestor_count = DEFAULT_DESCENDANT_LIMIT
+        ancestor_count = DEFAULT_CLUSTER_LIMIT
         assert_equal(ancestor_fees, sum([mempool[tx]['fees']['base'] for tx in mempool]))
 
-        # Adding one more transaction on to the chain should fail.
-        next_hop = self.wallet.create_self_transfer(utxo_to_spend=chain[-1]["new_utxo"])["hex"]
-        assert_raises_rpc_error(-26, "too-long-mempool-chain", lambda: self.nodes[0].sendrawtransaction(next_hop))
-
         descendants = []
         ancestors = [t["txid"] for t in chain]
         chain = [t["txid"] for t in chain]
@@ -186,23 +181,7 @@ class MempoolPackagesTest(BitcoinTestFramework):
                 assert_equal(entry['fees']['modified'], entry['fees']['base'] + Decimal("0.00002"))
             assert_equal(entry['fees']['descendant'], descendant_fees + Decimal("0.00002"))
 
-        # Check that node1's mempool is as expected (-> custom ancestor limit)
-        mempool0 = self.nodes[0].getrawmempool(False)
-        mempool1 = self.nodes[1].getrawmempool(False)
-        assert_equal(len(mempool1), CUSTOM_DESCENDANT_LIMIT)
-        assert set(mempool1).issubset(set(mempool0))
-        for tx in chain[:CUSTOM_DESCENDANT_LIMIT]:
-            assert tx in mempool1
-            entry0 = self.nodes[0].getmempoolentry(tx)
-            entry1 = self.nodes[1].getmempoolentry(tx)
-            assert not entry0['unbroadcast']
-            assert not entry1['unbroadcast']
-            assert_equal(entry1['fees']['base'], entry0['fees']['base'])
-            assert_equal(entry1['vsize'], entry0['vsize'])
-            assert_equal(entry1['depends'], entry0['depends'])
-
         # Now test descendant chain limits
-
         tx_children = []
         # First create one parent tx with 10 children
         tx_with_children = self.wallet.send_self_transfer_multi(from_node=self.nodes[0], num_outputs=10)
@@ -211,7 +190,7 @@ class MempoolPackagesTest(BitcoinTestFramework):
 
         # Sign and send up to MAX_DESCENDANT transactions chained off the parent tx
         chain = [] # save sent txs for the purpose of checking node1's mempool later (see below)
-        for _ in range(DEFAULT_DESCENDANT_LIMIT - 1):
+        for _ in range(DEFAULT_CLUSTER_LIMIT - 1):
             utxo = transaction_package.pop(0)
             new_tx = self.wallet.send_self_transfer_multi(from_node=self.nodes[0], num_outputs=10, utxos_to_spend=[utxo])
             txid = new_tx["txid"]
@@ -221,22 +200,16 @@ class MempoolPackagesTest(BitcoinTestFramework):
             transaction_package.extend(new_tx["new_utxos"])
 
         mempool = self.nodes[0].getrawmempool(True)
-        assert_equal(mempool[parent_transaction]['descendantcount'], DEFAULT_DESCENDANT_LIMIT)
+        assert_equal(mempool[parent_transaction]['descendantcount'], DEFAULT_CLUSTER_LIMIT)
         assert_equal(sorted(mempool[parent_transaction]['spentby']), sorted(tx_children))
 
         for child in tx_children:
             assert_equal(mempool[child]['depends'], [parent_transaction])
 
-        # Sending one more chained transaction will fail
-        next_hop = self.wallet.create_self_transfer(utxo_to_spend=transaction_package.pop(0))["hex"]
-        assert_raises_rpc_error(-26, "too-long-mempool-chain", lambda: self.nodes[0].sendrawtransaction(next_hop))
-
         # Check that node1's mempool is as expected, containing:
-        # - txs from previous ancestor test (-> custom ancestor limit)
         # - parent tx for descendant test
         # - txs chained off parent tx (-> custom descendant limit)
-        self.wait_until(lambda: len(self.nodes[1].getrawmempool()) ==
+        self.wait_until(lambda: len(self.nodes[1].getrawmempool()) == 2*CUSTOM_CLUSTER_LIMIT, timeout=10)
-                                2*CUSTOM_DESCENDANT_LIMIT, timeout=10)
         mempool0 = self.nodes[0].getrawmempool(False)
         mempool1 = self.nodes[1].getrawmempool(False)
         assert set(mempool1).issubset(set(mempool0))
@@ -247,7 +220,7 @@ class MempoolPackagesTest(BitcoinTestFramework):
                 entry1 = self.nodes[1].getmempoolentry(tx)
                 assert not entry0['unbroadcast']
                 assert not entry1['unbroadcast']
-                assert entry1["descendantcount"] <= CUSTOM_DESCENDANT_LIMIT
+                assert entry1["descendantcount"] <= CUSTOM_CLUSTER_LIMIT
                 assert_equal(entry1['fees']['base'], entry0['fees']['base'])
                 assert_equal(entry1['vsize'], entry0['vsize'])
                 assert_equal(entry1['depends'], entry0['depends'])

test/functional/mempool_sigoplimit.py

@@ -169,14 +169,14 @@ class BytesPerSigOpTest(BitcoinTestFramework):
         assert_equal(parent_individual_testres["vsize"], max_multisig_vsize)
 
         # But together, it's exceeding limits in the *package* context. If sigops adjusted vsize wasn't being checked
-        # here, it would get further in validation and give too-long-mempool-chain error instead.
+        # here, it would get further in validation and give too-large-cluster error instead.
         packet_test = self.nodes[0].testmempoolaccept([tx_parent.serialize().hex(), tx_child.serialize().hex()])
-        expected_package_error = f"package-mempool-limits, package size {2*max_multisig_vsize} exceeds descendant size limit [limit: 101000]"
+        expected_package_error = "too-large-cluster"
         assert_equal([x["package-error"] for x in packet_test], [expected_package_error] * 2)
 
-        # When we actually try to submit, the parent makes it into the mempool, but the child would exceed ancestor vsize limits
+        # When we actually try to submit, the parent makes it into the mempool, but the child would exceed cluster vsize limits
         res = self.nodes[0].submitpackage([tx_parent.serialize().hex(), tx_child.serialize().hex()])
-        assert "too-long-mempool-chain" in res["tx-results"][tx_child.wtxid_hex]["error"]
+        assert "too-large-cluster" in res["tx-results"][tx_child.wtxid_hex]["error"]
         assert tx_parent.txid_hex in self.nodes[0].getrawmempool()
 
         # Transactions are tiny in weight

test/functional/mempool_truc.py

@@ -196,15 +196,15 @@ class MempoolTRUC(BitcoinTestFramework):
         node.invalidateblock(block["hash"])
         self.check_mempool([tx_v3_block["txid"], tx_v2_block["txid"], tx_v3_block2["txid"], tx_v2_from_v3["txid"], tx_v3_from_v2["txid"], tx_v3_child_large["txid"], tx_chain_1["txid"], tx_chain_2["txid"], tx_chain_3["txid"], tx_chain_4["txid"]])
 
-    @cleanup(extra_args=["-limitdescendantsize=10"])
+    @cleanup(extra_args=["-limitclustercount=1"])
     def test_nondefault_package_limits(self):
         """
-        Max standard tx size + TRUC rules imply the ancestor/descendant rules (at their default
+        Max standard tx size + TRUC rules imply the cluster rules (at their default
         values), but those checks must not be skipped. Ensure both sets of checks are done by
-        changing the ancestor/descendant limit configurations.
+        changing the cluster limit configurations.
         """
         node = self.nodes[0]
-        self.log.info("Test that a decreased limitdescendantsize also applies to TRUC child")
+        self.log.info("Test that a decreased cluster count limit also applies to TRUC child")
         parent_target_vsize = 9990
         child_target_vsize = 500
         tx_v3_parent_large1 = self.wallet.send_self_transfer(
@@ -218,12 +218,11 @@ class MempoolTRUC(BitcoinTestFramework):
             version=3
         )
 
-        # Parent and child are within v3 limits, but parent's 10kvB descendant limit is exceeded
+        # Parent and child are within v3 limits, but cluster count limit is exceeded.
         assert_greater_than_or_equal(TRUC_MAX_VSIZE, tx_v3_parent_large1["tx"].get_vsize())
         assert_greater_than_or_equal(TRUC_CHILD_MAX_VSIZE, tx_v3_child_large1["tx"].get_vsize())
-        assert_greater_than(tx_v3_parent_large1["tx"].get_vsize() + tx_v3_child_large1["tx"].get_vsize(), 10000)
 
-        assert_raises_rpc_error(-26, f"too-long-mempool-chain, exceeds descendant size limit for tx {tx_v3_parent_large1['txid']}", node.sendrawtransaction, tx_v3_child_large1["hex"])
+        assert_raises_rpc_error(-26, "too-large-cluster", node.sendrawtransaction, tx_v3_child_large1["hex"])
         self.check_mempool([tx_v3_parent_large1["txid"]])
         assert_equal(node.getmempoolentry(tx_v3_parent_large1["txid"])["descendantcount"], 1)
         self.generate(node, 1)

test/functional/mempool_updatefromblock.py

@@ -27,8 +27,7 @@ CUSTOM_DESCENDANT_COUNT = CUSTOM_ANCESTOR_COUNT
 class MempoolUpdateFromBlockTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
-        # Ancestor and descendant limits depend on transaction_graph_test requirements
+        self.extra_args = [['-limitclustersize=1000']]
-        self.extra_args = [['-limitdescendantsize=1000', '-limitancestorsize=1000', f'-limitancestorcount={CUSTOM_ANCESTOR_COUNT}', f'-limitdescendantcount={CUSTOM_DESCENDANT_COUNT}', "-limitclustersize=1000"]]
 
     def create_empty_fork(self, fork_length):
         '''