Skip to content

Commit c426e0d

Browse files
author
MarcoFalke
committed
Merge bitcoin/bitcoin#22972: test: fix misleading fee unit in mempool_limit.py
2600db6 test: fix misleading fee unit in mempool_limit.py (Sebastian Falbesoner) Pull request description: The PR is a follow-up to #22543. The helper `send_large_txs` in its current interface has a fee_rate parameter, implying that it would create a transaction with exactly that rate. Unfortunately, this fee rate is only passed to MiniWallet's `create_self_transfer` method, which can't know that we append several tx outputs after, increasing the tx's vsize and decreasing it's fee rate accordingly. In our case, the fee rate is off by several orders of magnitude, as the tx's vsize changes changes from 96 to 67552 vbytes (>700x), i.e. the value passed to this function is neither really a fee rate nor an absolute fee, but something in-between, which is very confusing. It was suggested to simply in-line this helper as it's currently only used in this single test (bitcoin/bitcoin#22543 (comment), bitcoin/bitcoin#22543 (comment)), but I could imagine that this helper may also become useful for other tests and may be moved to a library (e.g. wallet.py) in the future. Clarify the interface by passing an absolute fee that is deducted in the end (and also verified, via testmempoolaccept) and also describe how we come up with the value passed. On master, the comment says that the fee rate needs to increased "massively"; this word is also removed because the fee rate only needs to be higher for the test to succeed. ACKs for top commit: stratospher: ACK 2600db6. Tree-SHA512: 0bfacc3fa87603970d86c1d0186e51511f6c20c64b0559e19e7e12a68647f79dcb4f436000dee718fd832ce6a68e3bbacacb29145e0287811f1cb03d2f316843
2 parents baa9fc9 + 2600db6 commit c426e0d

File tree

1 file changed

+19
-8
lines changed

1 file changed

+19
-8
lines changed

test/functional/mempool_limit.py

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,14 @@
77
from decimal import Decimal
88

99
from test_framework.blocktools import COINBASE_MATURITY
10+
from test_framework.messages import COIN
1011
from test_framework.test_framework import BitcoinTestFramework
11-
from test_framework.util import assert_equal, assert_greater_than, assert_raises_rpc_error, gen_return_txouts
12+
from test_framework.util import (
13+
assert_equal,
14+
assert_greater_than,
15+
assert_raises_rpc_error,
16+
gen_return_txouts,
17+
)
1218
from test_framework.wallet import MiniWallet
1319

1420

@@ -23,16 +29,19 @@ def set_test_params(self):
2329
]]
2430
self.supports_cli = False
2531

26-
def send_large_txs(self, node, miniwallet, txouts, fee_rate, tx_batch_size):
32+
def send_large_txs(self, node, miniwallet, txouts, fee, tx_batch_size):
2733
for _ in range(tx_batch_size):
28-
tx = miniwallet.create_self_transfer(from_node=node, fee_rate=fee_rate)['tx']
34+
tx = miniwallet.create_self_transfer(from_node=node, fee_rate=0, mempool_valid=False)['tx']
2935
for txout in txouts:
3036
tx.vout.append(txout)
37+
tx.vout[0].nValue -= int(fee * COIN)
38+
res = node.testmempoolaccept([tx.serialize().hex()])[0]
39+
assert_equal(res['fees']['base'], fee)
3140
miniwallet.sendrawtransaction(from_node=node, tx_hex=tx.serialize().hex())
3241

3342
def run_test(self):
3443
txouts = gen_return_txouts()
35-
node=self.nodes[0]
44+
node = self.nodes[0]
3645
miniwallet = MiniWallet(node)
3746
relayfee = node.getnetworkinfo()['relayfee']
3847

@@ -54,13 +63,15 @@ def run_test(self):
5463
self.log.info('Create a mempool tx that will be evicted')
5564
tx_to_be_evicted_id = miniwallet.send_self_transfer(from_node=node, fee_rate=relayfee)["txid"]
5665

57-
# Increase the tx fee rate massively to give the subsequent transactions a higher priority in the mempool
58-
base_fee = relayfee * 1000
66+
# Increase the tx fee rate to give the subsequent transactions a higher priority in the mempool
67+
# The tx has an approx. vsize of 65k, i.e. multiplying the previous fee rate (in sats/kvB)
68+
# by 130 should result in a fee that corresponds to 2x of that fee rate
69+
base_fee = relayfee * 130
5970

6071
self.log.info("Fill up the mempool with txs with higher fee rate")
6172
for batch_of_txid in range(num_of_batches):
62-
fee_rate=(batch_of_txid + 1) * base_fee
63-
self.send_large_txs(node, miniwallet, txouts, fee_rate, tx_batch_size)
73+
fee = (batch_of_txid + 1) * base_fee
74+
self.send_large_txs(node, miniwallet, txouts, fee, tx_batch_size)
6475

6576
self.log.info('The tx should be evicted by now')
6677
# The number of transactions created should be greater than the ones present in the mempool

0 commit comments

Comments
 (0)