Merge bitcoin/bitcoin#25810: scripted-diff: test: rename MAX_{ANCESTORS,DESCENDANTS} to DEFAULT_{ANCESTOR,DESCENDANT}_LIMIT

b4a5ab9 test: refactor: deduplicate `DEFAULT_{ANCESTOR,DESCENDANT}_LIMIT` constants (Sebastian Falbesoner)
0fda1c7 scripted-diff: test: rename `MAX_{ANCESTORS,DESCENDANTS}` to `DEFAULT_{ANCESTOR,DESCENDANT}_LIMIT` (Sebastian Falbesoner)

Pull request description:

  This PR renames the default in-mempool max ancestors/descendants constants `MAX_ANCESTORS`/`MAX_DESCENDANTS` in the functional tests to match the ones in the codebase:
  https://github.com/bitcoin/bitcoin/blob/c012875b9ded0a5183602f002738ca823d559518/src/policy/policy.h#L58-L59
  https://github.com/bitcoin/bitcoin/blob/c012875b9ded0a5183602f002738ca823d559518/src/policy/policy.h#L62-L63
  The custom limit constants `MAX_ANCESTORS_CUSTOM`/`MAX_DESCENDANTS_CUSTOM` are also renamed accordingly to better fit to this naming style. In the second commit, the default constants are deduplicated by moving them into the `messages.py` module. (Not sure if this module is really appropriate, as it doesn't have a connection to messages. If someone has a good suggestion, would be glad to hear it.)

ACKs for top commit:
  w0xlt:
    ACK bitcoin/bitcoin@b4a5ab9
  glozow:
    utACK b4a5ab9
  fanquake:
    ACK b4a5ab9

Tree-SHA512: a15c8256170afce3e383fceddcb562f588a02be97ce4202c84a2ebf22d73ab843f5e5a7d7c98e9ea044d8bcb7a4aeae0081d0e84c53e8fc0edffbcca00460139

Author: MacroFake

test/functional/mempool_package_onemore.py

@@ -7,6 +7,9 @@
    size.
 """
 
+from test_framework.messages import (
+    DEFAULT_ANCESTOR_LIMIT,
+)
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
@@ -15,10 +18,6 @@
 from test_framework.wallet import MiniWallet
 
 
-MAX_ANCESTORS = 25
-MAX_DESCENDANTS = 25
-
-
 class MempoolPackagesTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
@@ -34,19 +33,19 @@ def run_test(self):
         self.wallet = MiniWallet(self.nodes[0])
         self.wallet.rescan_utxos()
 
-        # MAX_ANCESTORS transactions off a confirmed tx should be fine
+        # DEFAULT_ANCESTOR_LIMIT transactions off a confirmed tx should be fine
         chain = []
         utxo = self.wallet.get_utxo()
         for _ in range(4):
             utxo, utxo2 = self.chain_tx([utxo], num_outputs=2)
             chain.append(utxo2)
-        for _ in range(MAX_ANCESTORS - 4):
+        for _ in range(DEFAULT_ANCESTOR_LIMIT - 4):
             utxo, = self.chain_tx([utxo])
             chain.append(utxo)
         second_chain, = self.chain_tx([self.wallet.get_utxo()])
 
-        # Check mempool has MAX_ANCESTORS + 1 transactions in it
-        assert_equal(len(self.nodes[0].getrawmempool()), MAX_ANCESTORS + 1)
+        # Check mempool has DEFAULT_ANCESTOR_LIMIT + 1 transactions in it
+        assert_equal(len(self.nodes[0].getrawmempool()), DEFAULT_ANCESTOR_LIMIT + 1)
 
         # Adding one more transaction on to the chain should fail.
         assert_raises_rpc_error(-26, "too-long-mempool-chain, too many unconfirmed ancestors [limit: 25]", self.chain_tx, [utxo])
@@ -67,7 +66,7 @@ def run_test(self):
         self.nodes[0].sendrawtransaction(replacable_tx.serialize().hex())
 
         # Finally, check that we added two transactions
-        assert_equal(len(self.nodes[0].getrawmempool()), MAX_ANCESTORS + 3)
+        assert_equal(len(self.nodes[0].getrawmempool()), DEFAULT_ANCESTOR_LIMIT + 3)
 
 
 if __name__ == '__main__':

test/functional/mempool_packages.py

@@ -7,7 +7,11 @@
 from decimal import Decimal
 
 from test_framework.blocktools import COINBASE_MATURITY
-from test_framework.messages import COIN
+from test_framework.messages import (
+    COIN,
+    DEFAULT_ANCESTOR_LIMIT,
+    DEFAULT_DESCENDANT_LIMIT,
+)
 from test_framework.p2p import P2PTxInvStore
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
@@ -16,13 +20,12 @@
     chain_transaction,
 )
 
-# default limits
-MAX_ANCESTORS = 25
-MAX_DESCENDANTS = 25
+
 # custom limits for node1
-MAX_ANCESTORS_CUSTOM = 5
-MAX_DESCENDANTS_CUSTOM = 10
-assert MAX_DESCENDANTS_CUSTOM >= MAX_ANCESTORS_CUSTOM
+CUSTOM_ANCESTOR_LIMIT = 5
+CUSTOM_DESCENDANT_LIMIT = 10
+assert CUSTOM_DESCENDANT_LIMIT >= CUSTOM_ANCESTOR_LIMIT
+
 
 class MempoolPackagesTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -34,8 +37,8 @@ def set_test_params(self):
             ],
             [
                 "-maxorphantx=1000",
-                "-limitancestorcount={}".format(MAX_ANCESTORS_CUSTOM),
-                "-limitdescendantcount={}".format(MAX_DESCENDANTS_CUSTOM),
+                "-limitancestorcount={}".format(CUSTOM_ANCESTOR_LIMIT),
+                "-limitdescendantcount={}".format(CUSTOM_DESCENDANT_LIMIT),
             ],
         ]
 
@@ -55,12 +58,12 @@ def run_test(self):
         assert 'ancestorfees' not in utxo[0]
 
         fee = Decimal("0.0001")
-        # MAX_ANCESTORS transactions off a confirmed tx should be fine
+        # DEFAULT_ANCESTOR_LIMIT transactions off a confirmed tx should be fine
         chain = []
         witness_chain = []
         ancestor_vsize = 0
         ancestor_fees = Decimal(0)
-        for i in range(MAX_ANCESTORS):
+        for i in range(DEFAULT_ANCESTOR_LIMIT):
             (txid, sent_value) = chain_transaction(self.nodes[0], [txid], [0], value, fee, 1)
             value = sent_value
             chain.append(txid)
@@ -81,16 +84,16 @@ def run_test(self):
         # Otherwise, getrawmempool may be inconsistent with getmempoolentry if unbroadcast changes in between
         peer_inv_store.wait_for_broadcast(witness_chain)
 
-        # Check mempool has MAX_ANCESTORS transactions in it, and descendant and ancestor
+        # Check mempool has DEFAULT_ANCESTOR_LIMIT transactions in it, and descendant and ancestor
         # count and fees should look correct
         mempool = self.nodes[0].getrawmempool(True)
-        assert_equal(len(mempool), MAX_ANCESTORS)
+        assert_equal(len(mempool), DEFAULT_ANCESTOR_LIMIT)
         descendant_count = 1
         descendant_fees = 0
         descendant_vsize = 0
 
         assert_equal(ancestor_vsize, sum([mempool[tx]['vsize'] for tx in mempool]))
-        ancestor_count = MAX_ANCESTORS
+        ancestor_count = DEFAULT_ANCESTOR_LIMIT
         assert_equal(ancestor_fees, sum([mempool[tx]['fees']['base'] for tx in mempool]))
 
         descendants = []
@@ -213,9 +216,9 @@ def run_test(self):
         # 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), MAX_ANCESTORS_CUSTOM)
+        assert_equal(len(mempool1), CUSTOM_ANCESTOR_LIMIT)
         assert set(mempool1).issubset(set(mempool0))
-        for tx in chain[:MAX_ANCESTORS_CUSTOM]:
+        for tx in chain[:CUSTOM_ANCESTOR_LIMIT]:
             assert tx in mempool1
         # TODO: more detailed check of node1's mempool (fees etc.)
         # check transaction unbroadcast info (should be false if in both mempools)
@@ -240,7 +243,7 @@ def run_test(self):
 
         # 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(MAX_DESCENDANTS - 1):
+        for _ in range(DEFAULT_DESCENDANT_LIMIT - 1):
             utxo = transaction_package.pop(0)
             (txid, sent_value) = chain_transaction(self.nodes[0], [utxo['txid']], [utxo['vout']], utxo['amount'], fee, 10)
             chain.append(txid)
@@ -250,7 +253,7 @@ def run_test(self):
                 transaction_package.append({'txid': txid, 'vout': j, 'amount': sent_value})
 
         mempool = self.nodes[0].getrawmempool(True)
-        assert_equal(mempool[parent_transaction]['descendantcount'], MAX_DESCENDANTS)
+        assert_equal(mempool[parent_transaction]['descendantcount'], DEFAULT_DESCENDANT_LIMIT)
         assert_equal(sorted(mempool[parent_transaction]['spentby']), sorted(tx_children))
 
         for child in tx_children:
@@ -265,14 +268,14 @@ def run_test(self):
         # - parent tx for descendant test
         # - txs chained off parent tx (-> custom descendant limit)
         self.wait_until(lambda: len(self.nodes[1].getrawmempool()) ==
-                                MAX_ANCESTORS_CUSTOM + 1 + MAX_DESCENDANTS_CUSTOM, timeout=10)
+                                CUSTOM_ANCESTOR_LIMIT + 1 + CUSTOM_DESCENDANT_LIMIT, timeout=10)
         mempool0 = self.nodes[0].getrawmempool(False)
         mempool1 = self.nodes[1].getrawmempool(False)
         assert set(mempool1).issubset(set(mempool0))
         assert parent_transaction in mempool1
-        for tx in chain[:MAX_DESCENDANTS_CUSTOM]:
+        for tx in chain[:CUSTOM_DESCENDANT_LIMIT]:
             assert tx in mempool1
-        for tx in chain[MAX_DESCENDANTS_CUSTOM:]:
+        for tx in chain[CUSTOM_DESCENDANT_LIMIT:]:
             assert tx not in mempool1
         # TODO: more detailed check of node1's mempool (fees etc.)
 

test/functional/test_framework/messages.py

@@ -65,6 +65,9 @@
 
 WITNESS_SCALE_FACTOR = 4
 
+DEFAULT_ANCESTOR_LIMIT = 25    # default max number of in-mempool ancestors
+DEFAULT_DESCENDANT_LIMIT = 25  # default max number of in-mempool descendants
+
 
 def sha256(s):
     return hashlib.sha256(s).digest()