test: fix test_limit_enforcement_package
What the developer wrote
Authored by Greg Sanders
test: fix test_limit_enforcement_package
The current test has a couple issues:
1) the parent_tx_good is regenerating the exact same
transaction that is already in the cluster, so it's
resulting in no replacements on submission
2) once fixed, the additional fee needs to be allocated to
the parent transaction in the package, not the child. If the
RBF fees are allocated to the child, this triggers the
package RBF logic, which requires no in-mempool ancestors to
be present.
Fix the bug and add a few assertions to protect against
regressions.
What changed, and why it matters
This commit fixes a bug in a Bitcoin Core functional test, not in the actual Bitcoin network code. The test was accidentally creating the same transaction twice and putting fees on the wrong transaction, so it wasn't really testing what it was supposed to test. The fix makes the test exercise the intended cluster-size-limit enforcement behavior correctly. There is no security issue in production software here.
No security action required. Treat as normal test maintenance / review.
Security signals we found
No production code modified
Test-only change in functional test suite
Fixes test logic bug rather than implementation vulnerability
Adds regression assertions
Evidence from the diff
The change is confined to test/functional/mempool_cluster.py. It corrects test_limit_enforcement_package: parent_tx_good was previously regenerating an identical transaction to one already in the mempool cluster, causing no replacement; the fee was also placed on the child, which would invoke package RBF ancestor restrictions. The patch adds a locktime and higher fee to the parent, moves the fee to the parent, and adds assertions verifying the expected replacements. A second assertion is added in test_cluster_count_limit to ensure the replacer is not already in the mempool. This is purely a test-hardening change.
Changed components
test/functional/mempool_cluster.pyInspect captured patch +13 / −4
diff --git a/test/functional/mempool_cluster.py b/test/functional/mempool_cluster.py
index c8d09c21..2719951e 100755
--- a/test/functional/mempool_cluster.py
+++ b/test/functional/mempool_cluster.py
@@ -128,7 +128,8 @@ class MempoolClusterTest(BitcoinTestFramework):
def test_limit_enforcement_package(self, cluster_submitted):
node = self.nodes[0]
- # Create a package from the second to last transaction. This shouldn't work because the effect is 64 + 2 - 1 = 65
+ # Create a package from the second to last transaction.
+ # This shouldn't work because the effect is {max_cluster_count} + 2 - 1 = {max_cluster_count} + 1
last_utxo = cluster_submitted[-2]["new_utxo"]
fee_to_beat = cluster_submitted[-1]["fee"]
# We do not use package RBF here because it has additional restrictions on mempool ancestors.
@@ -141,15 +142,21 @@ class MempoolClusterTest(BitcoinTestFramework):
assert child_tx_bad["txid"] not in node.getrawmempool()
assert_equal(result_parent_only["package_msg"], "transaction failed")
assert_equal(result_parent_only["tx-results"][child_tx_bad["wtxid"]]["error"], "too-large-cluster")
+ assert_equal(result_parent_only["replaced-transactions"], [cluster_submitted[-1]["txid"]])
- # Now, create a package from the second to last transaction. This should work because the effect is 64 + 2 - 2 = 64
+ # Now, create a package from the third to last transaction.
+ # This should work because the effect is {max_cluster_count} + 2 - 2 = {max_cluster_count}
third_to_last_utxo = cluster_submitted[-3]["new_utxo"]
- parent_tx_good = self.wallet.create_self_transfer(utxo_to_spend=third_to_last_utxo)
- child_tx_good = self.wallet.create_self_transfer(utxo_to_spend=parent_tx_good["new_utxo"], fee=fee_to_beat * 5)
+ # Tweak locktime to not recreate same tx as its meant to replace, fee needs to be even higher
+ parent_tx_good = self.wallet.create_self_transfer(utxo_to_spend=third_to_last_utxo, locktime=1, fee=fee_to_beat * 10)
+ child_tx_good = self.wallet.create_self_transfer(utxo_to_spend=parent_tx_good["new_utxo"])
+ assert parent_tx_good["txid"] != cluster_submitted[-2]["txid"]
+ assert child_tx_good["txid"] != parent_tx_bad["txid"]
result_both_good = node.submitpackage([parent_tx_good["hex"], child_tx_good["hex"]], maxfeerate=0)
assert_equal(result_both_good["package_msg"], "success")
assert parent_tx_good["txid"] in node.getrawmempool()
assert child_tx_good["txid"] in node.getrawmempool()
+ assert_equal(set(result_both_good["replaced-transactions"]), set([parent_tx_bad["txid"], cluster_submitted[-2]["txid"]]))
@cleanup
def test_cluster_count_limit(self, max_cluster_count):
@@ -292,7 +299,9 @@ class MempoolClusterTest(BitcoinTestFramework):
utxos_to_replace.append(confirmed_utxo)
tx_replacer = self.wallet.create_self_transfer_multi(utxos_to_spend=utxos_to_replace)
+ assert tx_replacer["txid"] not in node.getrawmempool()
tx_replacer_sponsor = self.wallet.create_self_transfer(utxo_to_spend=tx_replacer["new_utxos"][0], fee=fee_rbf_decimal * 2)
+
node.submitpackage([tx_replacer["hex"], tx_replacer_sponsor["hex"]], maxfeerate=0)
assert tx_replacer["txid"] in node.getrawmempool()
assert tx_replacer_sponsor["txid"] in node.getrawmempool()
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.