Merge bitcoin#27350: test: refactor: dedup mempool_package_limits.py subtests via decorator

glozow · knst · commit f5f95e64e6bf · 2026-01-07T21:03:25.000+07:00
e669833 test: dedup package limit checks via decorator in mempool_package_limits.py (Sebastian Falbesoner) 72f25e2 test: refactor: use Satoshis for fees in mempool_package_limits.py (Sebastian Falbesoner) Pull request description: The subtests in the functional test mempool_package_limits.py all follow the same pattern: 1. first, check that the mempool is currently empty 2. create and submit certain single txs to the mempool, prepare list of hex transactions 3. check that `testmempoolaccept` on the package hex fails with a "package-mempool-limits" error on each tx result 4. after mining a block, check that submitting the package succeeds Note that steps 1,3,4 are identical for each of the subtests and only step 2 varies, so this might be a nice opportunity to deduplicate code by using a newly introduced decorator which executes the necessary before and after the essential part of the subtest. This also makes it easier to add new subtests without having to copy-paste those parts once again. In addition, the first commit switches the fee unit from BTC to Satoshis, which allows to get rid of some imports (`COIN` and `Decimal`) and a comment for the `test_desc_size_limits` subtest is fixed (s/25KvB/21KvB/). ACKs for top commit: ismaelsadeeq: ACK e669833 glozow: utACK e669833 Tree-SHA512: 84a85e739de7387391c13bd46aeb015a74302ea7c6f0ca3d4e2b1b487d38df390dc118eb5b1c11d3e4206bff316a4dab60ef6b25d8feced672345d4e36ffd205
diff --git a/test/functional/mempool_package_limits.py b/test/functional/mempool_package_limits.py
@@ -8,14 +8,36 @@
 
 from test_framework.blocktools import COINBASE_MATURITY
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.messages import (
-    COIN,
-)
 from test_framework.util import (
     assert_equal,
 )
 from test_framework.wallet import MiniWallet
 
+# Decorator to
+# 1) check that mempool is empty at the start of a subtest
+# 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
+# 4) after mining a block, clearing the pre-submitted transactions from mempool,
+#    check that submitting the created package succeeds
+def check_package_limits(func):
+    def func_wrapper(self, *args, **kwargs):
+        node = self.nodes[0]
+        assert_equal(0, node.getmempoolinfo()["size"])
+        package_hex = func(self, *args, **kwargs)
+        testres_error_expected = node.testmempoolaccept(rawtxs=package_hex)
+        assert_equal(len(testres_error_expected), len(package_hex))
+        for txres in testres_error_expected:
+            assert_equal(txres["package-error"], "package-mempool-limits")
+
+        # Clear mempool and check that the package passes now
+        self.generate(node, 1)
+        assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=package_hex)])
+
+    return func_wrapper
+
+
 class MempoolPackageLimitsTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
@@ -39,9 +61,9 @@ def run_test(self):
         self.test_anc_size_limits()
         self.test_desc_size_limits()
 
+    @check_package_limits
     def test_chain_limits_helper(self, mempool_count, package_count):
         node = self.nodes[0]
-        assert_equal(0, node.getmempoolinfo()["size"])
         chain_hex = []
 
         chaintip_utxo = self.wallet.send_self_transfer_chain(from_node=node, chain_length=mempool_count)[-1]["new_utxo"]
@@ -50,13 +72,7 @@ def test_chain_limits_helper(self, mempool_count, package_count):
             tx = self.wallet.create_self_transfer(utxo_to_spend=chaintip_utxo)
             chaintip_utxo = tx["new_utxo"]
             chain_hex.append(tx["hex"])
-        testres_too_long = node.testmempoolaccept(rawtxs=chain_hex)
-        for txres in testres_too_long:
-            assert_equal(txres["package-error"], "package-mempool-limits")
-
-        # Clear mempool and check that the package passes now
-        self.generate(node, 1)
-        assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=chain_hex)])
+        return chain_hex
 
     def test_chain_limits(self):
         """Create chains from mempool and package transactions that are longer than 25,
@@ -75,6 +91,7 @@ def test_chain_limits(self):
         # 13 transactions in the mempool and 13 in the package.
         self.test_chain_limits_helper(13, 13)
 
+    @check_package_limits
     def test_desc_count_limits(self):
         """Create an 'A' shaped package with 24 transactions in the mempool and 2 in the package:
                     M1
@@ -92,7 +109,6 @@ def test_desc_count_limits(self):
         package transactions).
         """
         node = self.nodes[0]
-        assert_equal(0, node.getmempoolinfo()["size"])
         self.log.info("Check that in-mempool and in-package descendants are calculated properly in packages")
         # Top parent in mempool, M1
         m1_utxos = self.wallet.send_self_transfer_multi(from_node=node, num_outputs=2)['new_utxos']
@@ -112,14 +128,9 @@ def test_desc_count_limits(self):
 
         assert_equal(24, node.getmempoolinfo()["size"])
         assert_equal(2, len(package_hex))
-        testres_too_long = node.testmempoolaccept(rawtxs=package_hex)
-        for txres in testres_too_long:
-            assert_equal(txres["package-error"], "package-mempool-limits")
-
-        # Clear mempool and check that the package passes now
-        self.generate(node, 1)
-        assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=package_hex)])
+        return package_hex
 
+    @check_package_limits
     def test_desc_count_limits_2(self):
         """Create a Package with 24 transaction in mempool and 2 transaction in package:
                       M1
@@ -156,15 +167,9 @@ def test_desc_count_limits_2(self):
 
         assert_equal(24, node.getmempoolinfo()["size"])
         assert_equal(2, len(package_hex))
-        testres = node.testmempoolaccept(rawtxs=package_hex)
-        assert_equal(len(testres), len(package_hex))
-        for txres in testres:
-            assert_equal(txres["package-error"], "package-mempool-limits")
-
-        # Clear mempool and check that the package passes now
-        self.generate(node, 1)
-        assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=package_hex)])
+        return package_hex
 
+    @check_package_limits
     def test_anc_count_limits(self):
         """Create a 'V' shaped chain with 24 transactions in the mempool and 3 in the package:
         M1a                    M1b
@@ -182,7 +187,6 @@ def test_anc_count_limits(self):
         and in-package ancestors are all considered together.
         """
         node = self.nodes[0]
-        assert_equal(0, node.getmempoolinfo()["size"])
         package_hex = []
         pc_parent_utxos = []
 
@@ -202,14 +206,9 @@ def test_anc_count_limits(self):
 
         assert_equal(24, node.getmempoolinfo()["size"])
         assert_equal(3, len(package_hex))
-        testres_too_long = node.testmempoolaccept(rawtxs=package_hex)
-        for txres in testres_too_long:
-            assert_equal(txres["package-error"], "package-mempool-limits")
-
-        # Clear mempool and check that the package passes now
-        self.generate(node, 1)
-        assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=package_hex)])
+        return package_hex
 
+    @check_package_limits
     def test_anc_count_limits_2(self):
         """Create a 'Y' shaped chain with 24 transactions in the mempool and 2 in the package:
         M1a                M1b
@@ -227,7 +226,6 @@ def test_anc_count_limits_2(self):
         and in-package ancestors are all considered together.
         """
         node = self.nodes[0]
-        assert_equal(0, node.getmempoolinfo()["size"])
         pc_parent_utxos = []
 
         self.log.info("Check that in-mempool and in-package ancestors are calculated properly in packages")
@@ -244,14 +242,9 @@ def test_anc_count_limits_2(self):
         pd_tx = self.wallet.create_self_transfer(utxo_to_spend=pc_tx["new_utxos"][0])
 
         assert_equal(24, node.getmempoolinfo()["size"])
-        testres_too_long = node.testmempoolaccept(rawtxs=[pc_tx["hex"], pd_tx["hex"]])
-        for txres in testres_too_long:
-            assert_equal(txres["package-error"], "package-mempool-limits")
-
-        # Clear mempool and check that the package passes now
-        self.generate(node, 1)
-        assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=[pc_tx["hex"], pd_tx["hex"]])])
+        return [pc_tx["hex"], pd_tx["hex"]]
 
+    @check_package_limits
     def test_anc_count_limits_bushy(self):
         """Create a tree with 20 transactions in the mempool and 6 in the package:
         M1...M4 M5...M8 M9...M12 M13...M16 M17...M20
@@ -264,7 +257,6 @@ def test_anc_count_limits_bushy(self):
         combined, PC has 25 in-mempool and in-package parents.
         """
         node = self.nodes[0]
-        assert_equal(0, node.getmempoolinfo()["size"])
         package_hex = []
         pc_parent_utxos = []
         for _ in range(5): # Make package transactions P0 ... P4
@@ -281,14 +273,9 @@ def test_anc_count_limits_bushy(self):
 
         assert_equal(20, node.getmempoolinfo()["size"])
         assert_equal(6, len(package_hex))
-        testres = node.testmempoolaccept(rawtxs=package_hex)
-        for txres in testres:
-            assert_equal(txres["package-error"], "package-mempool-limits")
-
-        # Clear mempool and check that the package passes now
-        self.generate(node, 1)
-        assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=package_hex)])
+        return package_hex
 
+    @check_package_limits
     def test_anc_size_limits(self):
         """Test Case with 2 independent transactions in the mempool and a parent + child in the
         package, where the package parent is the child of both mempool transactions (30KvB each):
@@ -302,10 +289,9 @@ def test_anc_size_limits(self):
         """
         node = self.nodes[0]
         parent_utxos = []
-        assert_equal(0, node.getmempoolinfo()["size"])
-        parents_tx = []
-        target_weight = 1000 * 30 # 30KvB
-        high_fee = Decimal("0.003") # 10 sats/vB
+        target_vsize = 30_000
+        high_fee = 10 * target_vsize  # 10 sats/vB
+        target_weight = target_vsize
         fee_rate=Decimal("0.01")
         self.log.info("Check that in-mempool and in-package ancestor size limits are calculated properly in packages")
         # Mempool transactions A and B
@@ -315,22 +301,17 @@ def test_anc_size_limits(self):
             parent_utxos.append(bulked_tx["new_utxo"])
 
         # Package transaction C
-        pc_tx = self.wallet.create_self_transfer_multi(utxos_to_spend=parent_utxos, fee_per_output=int(high_fee * COIN), target_weight=target_weight)
+        pc_tx = self.wallet.create_self_transfer_multi(utxos_to_spend=parent_utxos, fee_per_output=high_fee, target_weight=target_weight)
 
         # Package transaction D
         pd_tx = self.wallet.create_self_transfer(utxo_to_spend=pc_tx["new_utxos"][0], target_weight=target_weight, fee_rate=fee_rate)
 
         assert_equal(2, node.getmempoolinfo()["size"])
-        testres_too_heavy = node.testmempoolaccept(rawtxs=[pc_tx["hex"], pd_tx["hex"]])
-        for txres in testres_too_heavy:
-            assert_equal(txres["package-error"], "package-mempool-limits")
-
-        # Clear mempool and check that the package passes now
-        self.generate(node, 1)
-        assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=[pc_tx["hex"], pd_tx["hex"]])])
+        return [pc_tx["hex"], pd_tx["hex"]]
 
+    @check_package_limits
     def test_desc_size_limits(self):
-        """Create 3 mempool transactions and 2 package transactions (25KvB each):
+        """Create 3 mempool transactions and 2 package transactions (21KvB each):
               Ma
              ^ ^
             Mb  Mc
@@ -340,12 +321,12 @@ def test_desc_size_limits(self):
         and in-package descendants are all considered together.
         """
         node = self.nodes[0]
-        assert_equal(0, node.getmempoolinfo()["size"])
-        target_weight = 21 * 1000
-        high_fee = Decimal("0.0021") # 10 sats/vB
+        target_vsize = 21_000
+        high_fee = 10 * target_vsize  # 10 sats/vB
+        target_weight = target_vsize
         self.log.info("Check that in-mempool and in-package descendant sizes are calculated properly in packages")
         # Top parent in mempool, Ma
-        ma_tx = self.wallet.create_self_transfer_multi(num_outputs=2, fee_per_output=int(high_fee / 2 * COIN), target_weight=target_weight)
+        ma_tx = self.wallet.create_self_transfer_multi(num_outputs=2, fee_per_output=high_fee // 2, target_weight=target_weight)
         self.wallet.sendrawtransaction(from_node=node, tx_hex=ma_tx["hex"])
 
         package_hex = []
@@ -360,13 +341,8 @@ def test_desc_size_limits(self):
 
         assert_equal(3, node.getmempoolinfo()["size"])
         assert_equal(2, len(package_hex))
-        testres_too_heavy = node.testmempoolaccept(rawtxs=package_hex)
-        for txres in testres_too_heavy:
-            assert_equal(txres["package-error"], "package-mempool-limits")
+        return package_hex
 
-        # Clear mempool and check that the package passes now
-        self.generate(node, 1)
-        assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=package_hex)])
 
 if __name__ == "__main__":
     MempoolPackageLimitsTest().main()
