Consolidate usage of mempool constants in the functional tests.
As well as one instance of MAX_FUTURE_BLOCK_TIME.
test: consolidate mempool constants #36611
pull fanquake wants to merge 5 commits into bitcoin:master from fanquake:consolidate_testframework_mempool changing 10 files +38 −35-
fanquake commented at 5:01 PM on October 22, 2026: member
- DrahtBot added the label Tests on Oct 22, 2026
-
DrahtBot commented at 5:01 PM on October 22, 2026: contributor
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
External sites
- Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36611
- Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36611.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36611#author-ralph
<!--021abf342d371248e50ceaed478a90ca-->
Reviews
See the guideline and AI policy for information on the review process.
Type Reviewers ACK stickies-v, mercie-ux, hodlinator If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--174a7506f384e20aa4161008e828411d-->
Conflicts
Reviewers, this pull request conflicts with the following ones:
- #36192 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36192.svg"></sub> (test: cover unsatisfiable mining timestamp by Sjors)
- #35793 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35793.svg"></sub> (Implement BIP 54 (Consensus Cleanup) without mainnet activation by darosior)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
-
mercie-ux commented at 4:39 AM on October 23, 2026: contributor
ACK 4385d415eb5dbcce28051f40687cc844c5f8dfdb
Non-blocking
feature_rbf.pystill definesMAX_REPLACEMENT_LIMIT = 100and uses it in the “too many conflicting clusters” test. Since this is the same limit, could this test also import and useMAX_REPLACEMENT_CANDIDATESfrommempool_util? https://github.com/bitcoin/bitcoin/blob/4385d415eb5dbcce28051f40687cc844c5f8dfdb/test/functional/feature_rbf.py#L25 -
in test/functional/mempool_package_rbf.py:195 in 4385d415eb outdated
190 | @@ -190,8 +191,8 @@ def test_package_rbf_additional_fees(self): 191 | 192 | def test_package_rbf_max_conflicts(self): 193 | node = self.nodes[0] 194 | - self.log.info("Check Package RBF cannot conflict with more than MAX_REPLACEMENT_CANDIDATES clusters") 195 | - num_coins = 101 196 | + self.log.info("Check Package RBF cannot conflict with more than MAX_REPLACEMENT_CANDIDATES clusters") 197 | + num_coins = MAX_REPLACEMENT_CANDIDATES + 1
stickies-v commented at 8:23 AM on October 23, 2026:nit: replace further usage of the literal:
<details> <summary>git diff on 4385d415eb</summary>
diff --git a/test/functional/mempool_package_rbf.py b/test/functional/mempool_package_rbf.py index c8f6a7c11a..6a2a011476 100755 --- a/test/functional/mempool_package_rbf.py +++ b/test/functional/mempool_package_rbf.py @@ -196,7 +196,7 @@ class PackageRBFTest(BitcoinTestFramework): parent_coins = self.coins[:num_coins] del self.coins[:num_coins] - # Original transactions: 101 transactions with 2 descendants each -> 303 total transactions, 101 clusters + # Original transactions: num_coins transactions with 2 descendants each -> num_coins * 3 total transactions, num_coins clusters size_three_clusters = [] for coin in parent_coins: size_three_clusters.append(self.wallet.send_self_transfer_chain(from_node=node, chain_length=3, utxo_to_spend=coin)) @@ -211,13 +211,13 @@ class PackageRBFTest(BitcoinTestFramework): parent_fee_per_conflict = 10000 child_feerate = 10000 * DEFAULT_FEE - # Conflict against all transactions by double-spending each parent, causing 101 cluster conflicts + # Conflict against all transactions by double-spending each parent, causing num_coins cluster conflicts package_parent = self.wallet.create_self_transfer_multi(utxos_to_spend=parent_coins, fee_per_output=parent_fee_per_conflict) package_child = self.wallet.create_self_transfer(fee_rate=child_feerate, utxo_to_spend=package_parent["new_utxos"][0]) pkg_results = node.submitpackage([package_parent["hex"], package_child["hex"]], maxfeerate=0) assert_equal("transaction failed", pkg_results["package_msg"]) - assert_equal(f"too many potential replacements, rejecting replacement {package_parent['txid']}; too many conflicting clusters (101 > 100)", pkg_results["tx-results"][package_parent["wtxid"]]["error"]) + assert_equal(f"too many potential replacements, rejecting replacement {package_parent['txid']}; too many conflicting clusters ({num_coins} > {MAX_REPLACEMENT_CANDIDATES})", pkg_results["tx-results"][package_parent["wtxid"]]["error"]) self.assert_mempool_contents(expected=expected_txns) # Make singleton tx to conflict with in next batch @@ -233,7 +233,7 @@ class PackageRBFTest(BitcoinTestFramework): package_child = self.wallet.create_self_transfer(fee_rate=child_feerate, utxo_to_spend=package_parent["new_utxos"][0]) pkg_results = node.submitpackage([package_parent["hex"], package_child["hex"]], maxfeerate=0) assert_equal("transaction failed", pkg_results["package_msg"]) - assert_equal(f"too many potential replacements, rejecting replacement {package_parent['txid']}; too many conflicting clusters (101 > 100)", pkg_results["tx-results"][package_parent["wtxid"]]["error"]) + assert_equal(f"too many potential replacements, rejecting replacement {package_parent['txid']}; too many conflicting clusters ({num_coins} > {MAX_REPLACEMENT_CANDIDATES})", pkg_results["tx-results"][package_parent["wtxid"]]["error"]) self.assert_mempool_contents(expected=expected_txns) # Finally, conflict with MAX_REPLACEMENT_CANDIDATES clusters</details>
in test/functional/feature_taproot.py:673 in 4385d415eb outdated
672 | @@ -670,7 +673,6 @@ def byte_popper(expr): 673 | MIN_FEE = 50000 674 | 675 | TX_STANDARD_VERSIONS = [1, 2, TX_MAX_STANDARD_VERSION] 676 | -TRUC_MAX_VSIZE = 10000 # test doesn't cover in-mempool spends, so only this limit is hit
stickies-v commented at 8:26 AM on October 23, 2026:nit: docstring could be preserved:
<details> <summary>git diff on 4385d415eb</summary>
diff --git a/test/functional/feature_taproot.py b/test/functional/feature_taproot.py index 7a1a2d9a6b..2cba934648 100755 --- a/test/functional/feature_taproot.py +++ b/test/functional/feature_taproot.py @@ -1638,6 +1638,7 @@ class TaprootTest(BitcoinTestFramework): fail_input is None # Must be valid to be standard and (all(utxo.spender.is_standard for utxo in input_utxos)) # All inputs must be standard and tx.version in TX_STANDARD_VERSIONS # The tx version must be standard + # test doesn't cover in-mempool spends, so only TRUC_MAX_VSIZE limit is hit and not (tx.version == 3 and tx.get_vsize() > TRUC_MAX_VSIZE) # Topological standardness rules must be followed ) msg = ','.join(utxo.spender.comment + ("*" if n == fail_input else "") for n, utxo in enumerate(input_utxos))</details>
fanquake force-pushed on Oct 23, 2026stickies-v approvedstickies-v commented at 8:39 AM on October 23, 2026: contributorACK 6cbef1aa02ea1445f0a3ad9e514e98b2feca78be
fanquake force-pushed on Oct 23, 2026DrahtBot added the label CI failed on Oct 23, 2026DrahtBot removed the label CI failed on Oct 23, 2026fanquake commented at 10:17 AM on October 23, 2026: memberTook all the suggestions here.
stickies-v commented at 11:19 AM on October 23, 2026: contributorre-ACK 9d48c8af469fa63c79a3b234bc7fe9fe3a25c288
mercie-ux commented at 2:38 PM on October 23, 2026: contributorre-ACK 9d48c8af469fa63c79a3b234bc7fe9fe3a25c288
in test/functional/test_framework/messages.py:79 in 9d48c8af46 outdated
73 | @@ -74,11 +74,6 @@ 74 | 75 | WITNESS_SCALE_FACTOR = 4 76 | 77 | -DEFAULT_ANCESTOR_LIMIT = 25 # default max number of in-mempool ancestors 78 | -DEFAULT_DESCENDANT_LIMIT = 25 # default max number of in-mempool descendants 79 | -DEFAULT_CLUSTER_LIMIT = 64 # default max number of transactions in a cluster
hodlinator commented at 12:26 PM on October 27, 2026:nit: Could move comment to remaining definition of
DEFAULT_CLUSTER_LIMITin mempool_util.py.
fanquake commented at 3:35 PM on October 27, 2026:Moved.
hodlinator approvedhodlinator commented at 12:28 PM on October 27, 2026: contributorACK 9d48c8af469fa63c79a3b234bc7fe9fe3a25c288
test: use TRUC constants from mempool_util aa7d0a5b5875a611998etest: add MAX_REPLACEMENT_CANDIDATES to mempool_util
Use it in feature_rbf & mempool_package_rbf.
test: consolidate DEFAULT_CLUSTER_LIMIT to mempool_util 8f4caec8959ee579b6cbtest: move DEFAULT_ANCESTOR_LIMIT to mempool_util
Remove unused DEFAULT_DESCENDANT_LIMIT.
test: use MAX_FUTURE_BLOCK_TIME from blocktools d8cca0c110fanquake force-pushed on Oct 27, 2026stickies-v commented at 3:50 PM on October 27, 2026: contributorre-ACK d8cca0c110b264be0cca69fb4c55ebee0e8152a7
DrahtBot requested review from hodlinator on Oct 27, 2026mercie-ux commented at 4:36 PM on October 27, 2026: contributorhodlinator approvedhodlinator commented at 6:48 PM on October 27, 2026: contributorre-ACK d8cca0c110b264be0cca69fb4c55ebee0e8152a7
ContributorsLabels
This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-10-30 09:40 UTC