Skip to content

Commit bd13d6b

Browse files
author
MarcoFalke
committed
Merge bitcoin/bitcoin#26656: tests: Improve runtime of some tests when --enable-debug
1647a11 tests: Reorder longer running tests in test_runner (Andrew Chow) ff6c9fe tests: Whitelist test p2p connection in rpc_packages (Andrew Chow) 8c20796 tests: Use waitfornewblock for work queue test in interface_rpc (Andrew Chow) 6c872d5 tests: Initialize sigops draining script with bytes in feature_taproot (Andrew Chow) 544cbf7 tests: Use batched RPC in feature_fee_estimation (Andrew Chow) 4ad7272 tests: reduce number of generated blocks for wallet_import_rescan (Andrew Chow) Pull request description: When configured with `--enable-debug`, many tests become dramatically slower. These slow downs are particularly noticed in tests that generate a lot of blocks in separate calls, make a lot of RPC calls, or send a lot of data from the test framework's P2P connection. This PR aims to improve the runtime of some of the slower tests and improve the overall runtime of the test runner. This has improved the runtime of the test runner from ~400s to ~140s on my computer. The slowest test by far was `wallet_import_rescan.py`. This was taking ~320s. Most of that time was spent waiting for blocks to be mined and then synced to the other nodes. It was generating a new block for every new transaction it was creating in a setup loop. However it is not necessary to have one tx per block. By mining a block only every 10 txs, the runtime is improved to ~61s. The second slowest test was `feature_fee_estimation.py`. This test spends most of its time waiting for RPCs to respond. I was able to improve its runtime by batching RPC requests. This has improved the runtime from ~201s to ~140s. In `feature_taproot.py`, the test was constructing a Python `CScript` using a very large list of `OP_CHECKSIG`s. The constructor for the Python implementation of `CScript` was iterating this list in order to create a `bytes` from it even though a `bytes` could be created from it without iterating. By making the `bytes` before passing it into the constructor, we are able to improve this test's runtime from ~131s to ~106s. Although `interface_rpc.py` was not typically a slow test, I found that it would occasionally have a super long runtime. It typically takes ~7s, but I have observed it taking >400s to run on occasion. This longer runtime occurs more often when `--enable-debug`. This long runtime was caused by the "exceeding work queue" test which is really just trying to trigger a race condition. In this test, it would create a few threads and try an RPC in a loop in the hopes that eventually one of the RPCs would be added to the work queue while another was processing. It used `getrpcinfo` for this, but this function is fairly fast. I believe what was happening was that with `--enable-debug`, all of the code for receiving the RPC would often take longer to run than the RPC itself, so the majority of the requests would succeed, until we got lucky after 10's of thousands of requests. By changing this to use a slow RPC, the race condition can be triggered more reliably, and much sooner as well. I've used `waitfornewblock` with a 500ms timeout. This improves the runtime to ~3s consistently. The last test I've changed was `rpc_packages.py`. This test was one of the higher runtime variability tests. The main source of this variation appears to be waiting for the test node to relay a transaction to the test framework's P2P connection. By whitelisting that peer, the variability is reduced to nearly 0. Lastly, I've reordered the tests in `test_runner.py` to account for the slower runtimes when configured with `--enable-debug`. Some of the slow tests I've looked at were listed as being fast which was causing overall `test_runner.py` runtime to be extended. This change makes the test runner's runtime be bounded by the slowest test (currently `feature_fee_estimation.py` with my usual config (`-j 60`). ACKs for top commit: willcl-ark: ACK 1647a11 Tree-SHA512: 529e0da4bc51f12c78a40d6d70b3a492b97723c96a3526148c46943d923c118737b32d2aec23d246392e50ab48013891ef19fe6205bf538b61b70d4f16a203eb
2 parents 8b05f13 + 1647a11 commit bd13d6b

File tree

7 files changed

+110
-70
lines changed

7 files changed

+110
-70
lines changed

test/functional/feature_fee_estimation.py

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323

2424

2525
def small_txpuzzle_randfee(
26-
wallet, from_node, conflist, unconflist, amount, min_fee, fee_increment
26+
wallet, from_node, conflist, unconflist, amount, min_fee, fee_increment, batch_reqs
2727
):
2828
"""Create and send a transaction with a random fee using MiniWallet.
2929
@@ -57,8 +57,11 @@ def small_txpuzzle_randfee(
5757
tx.vout[0].nValue = int((total_in - amount - fee) * COIN)
5858
tx.vout.append(deepcopy(tx.vout[0]))
5959
tx.vout[1].nValue = int(amount * COIN)
60+
tx.rehash()
61+
txid = tx.hash
62+
tx_hex = tx.serialize().hex()
6063

61-
txid = from_node.sendrawtransaction(hexstring=tx.serialize().hex(), maxfeerate=0)
64+
batch_reqs.append(from_node.sendrawtransaction.get_request(hexstring=tx_hex, maxfeerate=0))
6265
unconflist.append({"txid": txid, "vout": 0, "value": total_in - amount - fee})
6366
unconflist.append({"txid": txid, "vout": 1, "value": amount})
6467

@@ -115,13 +118,12 @@ def check_estimates(node, fees_seen):
115118
check_smart_estimates(node, fees_seen)
116119

117120

118-
def send_tx(wallet, node, utxo, feerate):
119-
"""Broadcast a 1in-1out transaction with a specific input and feerate (sat/vb)."""
120-
return wallet.send_self_transfer(
121-
from_node=node,
121+
def make_tx(wallet, utxo, feerate):
122+
"""Create a 1in-1out transaction with a specific input and feerate (sat/vb)."""
123+
return wallet.create_self_transfer(
122124
utxo_to_spend=utxo,
123125
fee_rate=Decimal(feerate * 1000) / COIN,
124-
)['txid']
126+
)
125127

126128

127129
class EstimateFeeTest(BitcoinTestFramework):
@@ -156,6 +158,7 @@ def transact_and_mine(self, numblocks, mining_node):
156158
# resorting to tx's that depend on the mempool when those run out
157159
for _ in range(numblocks):
158160
random.shuffle(self.confutxo)
161+
batch_sendtx_reqs = []
159162
for _ in range(random.randrange(100 - 50, 100 + 50)):
160163
from_index = random.randint(1, 2)
161164
(tx_bytes, fee) = small_txpuzzle_randfee(
@@ -166,9 +169,12 @@ def transact_and_mine(self, numblocks, mining_node):
166169
Decimal("0.005"),
167170
min_fee,
168171
min_fee,
172+
batch_sendtx_reqs,
169173
)
170174
tx_kbytes = tx_bytes / 1000.0
171175
self.fees_per_kb.append(float(fee) / tx_kbytes)
176+
for node in self.nodes:
177+
node.batch(batch_sendtx_reqs)
172178
self.sync_mempools(wait=0.1)
173179
mined = mining_node.getblock(self.generate(mining_node, 1)[0], True)["tx"]
174180
# update which txouts are confirmed
@@ -245,14 +251,20 @@ def sanity_check_rbf_estimates(self, utxos):
245251
assert_greater_than_or_equal(len(utxos), 250)
246252
for _ in range(5):
247253
# Broadcast 45 low fee transactions that will need to be RBF'd
254+
txs = []
248255
for _ in range(45):
249256
u = utxos.pop(0)
250-
txid = send_tx(self.wallet, node, u, low_feerate)
257+
tx = make_tx(self.wallet, u, low_feerate)
251258
utxos_to_respend.append(u)
252-
txids_to_replace.append(txid)
259+
txids_to_replace.append(tx["txid"])
260+
txs.append(tx)
253261
# Broadcast 5 low fee transaction which don't need to
254262
for _ in range(5):
255-
send_tx(self.wallet, node, utxos.pop(0), low_feerate)
263+
tx = make_tx(self.wallet, utxos.pop(0), low_feerate)
264+
txs.append(tx)
265+
batch_send_tx = [node.sendrawtransaction.get_request(tx["hex"]) for tx in txs]
266+
for n in self.nodes:
267+
n.batch(batch_send_tx)
256268
# Mine the transactions on another node
257269
self.sync_mempools(wait=0.1, nodes=[node, miner])
258270
for txid in txids_to_replace:
@@ -261,7 +273,12 @@ def sanity_check_rbf_estimates(self, utxos):
261273
# RBF the low-fee transactions
262274
while len(utxos_to_respend) > 0:
263275
u = utxos_to_respend.pop(0)
264-
send_tx(self.wallet, node, u, high_feerate)
276+
tx = make_tx(self.wallet, u, high_feerate)
277+
node.sendrawtransaction(tx["hex"])
278+
txs.append(tx)
279+
dec_txs = [res["result"] for res in node.batch([node.decoderawtransaction.get_request(tx["hex"]) for tx in txs])]
280+
self.wallet.scan_txs(dec_txs)
281+
265282

266283
# Mine the last replacement txs
267284
self.sync_mempools(wait=0.1, nodes=[node, miner])

test/functional/feature_taproot.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1292,7 +1292,7 @@ def block_submit(self, node, txs, msg, err_msg, cb_pubkey=None, fees=0, sigops_w
12921292
# It is not impossible to fit enough tapscript sigops to hit the old 80k limit without
12931293
# busting txin-level limits. We simply have to account for the p2pk outputs in all
12941294
# transactions.
1295-
extra_output_script = CScript([OP_CHECKSIG]*((MAX_BLOCK_SIGOPS_WEIGHT - sigops_weight) // WITNESS_SCALE_FACTOR))
1295+
extra_output_script = CScript(bytes([OP_CHECKSIG]*((MAX_BLOCK_SIGOPS_WEIGHT - sigops_weight) // WITNESS_SCALE_FACTOR)))
12961296

12971297
coinbase_tx = create_coinbase(self.lastblockheight + 1, pubkey=cb_pubkey, extra_output_script=extra_output_script, fees=fees)
12981298
block = create_block(self.tip, coinbase_tx, self.lastblocktime + 1, txlist=txs)

test/functional/interface_rpc.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def expect_http_status(expected_http_status, expected_rpc_code,
2525
def test_work_queue_getblock(node, got_exceeded_error):
2626
while not got_exceeded_error:
2727
try:
28-
node.cli('getrpcinfo').send_cli()
28+
node.cli("waitfornewblock", "500").send_cli()
2929
except subprocess.CalledProcessError as e:
3030
assert_equal(e.output, 'error: Server response: Work queue depth exceeded\n')
3131
got_exceeded_error.append(True)

test/functional/rpc_packages.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ class RPCPackagesTest(BitcoinTestFramework):
2929
def set_test_params(self):
3030
self.num_nodes = 1
3131
self.setup_clean_chain = True
32+
self.extra_args = [["[email protected]"]] # noban speeds up tx relay
3233

3334
def assert_testres_equal(self, package_hex, testres_expected):
3435
"""Shuffle package_hex and assert that the testmempoolaccept result matches testres_expected. This should only

test/functional/test_framework/wallet.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,10 @@ def scan_tx(self, tx):
141141
if out['scriptPubKey']['hex'] == self._scriptPubKey.hex():
142142
self._utxos.append(self._create_utxo(txid=tx["txid"], vout=out["n"], value=out["value"], height=0))
143143

144+
def scan_txs(self, txs):
145+
for tx in txs:
146+
self.scan_tx(tx)
147+
144148
def sign_tx(self, tx, fixed_length=True):
145149
"""Sign tx that has been created by MiniWallet in P2PK mode"""
146150
assert_equal(self._mode, MiniWalletMode.RAW_P2PK)

test/functional/test_runner.py

Lines changed: 51 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -91,56 +91,81 @@
9191
BASE_SCRIPTS = [
9292
# Scripts that are run by default.
9393
# Longest test should go first, to favor running tests in parallel
94-
'wallet_hd.py --legacy-wallet',
95-
'wallet_hd.py --descriptors',
96-
'wallet_backup.py --legacy-wallet',
97-
'wallet_backup.py --descriptors',
9894
# vv Tests less than 5m vv
95+
'feature_fee_estimation.py',
96+
'feature_taproot.py',
97+
'feature_block.py',
98+
# vv Tests less than 2m vv
9999
'mining_getblocktemplate_longpoll.py',
100+
'p2p_segwit.py',
100101
'feature_maxuploadtarget.py',
101-
'feature_block.py',
102+
'mempool_updatefromblock.py',
103+
'mempool_persist.py --descriptors',
104+
# vv Tests less than 60s vv
105+
'rpc_psbt.py --legacy-wallet',
106+
'rpc_psbt.py --descriptors',
102107
'wallet_fundrawtransaction.py --legacy-wallet',
103108
'wallet_fundrawtransaction.py --descriptors',
104-
'p2p_compactblocks.py',
105-
'p2p_compactblocks_blocksonly.py',
109+
'wallet_bumpfee.py --legacy-wallet',
110+
'wallet_bumpfee.py --descriptors',
111+
'wallet_import_rescan.py --legacy-wallet',
112+
'wallet_backup.py --legacy-wallet',
113+
'wallet_backup.py --descriptors',
106114
'feature_segwit.py --legacy-wallet',
107115
'feature_segwit.py --descriptors',
108-
# vv Tests less than 2m vv
116+
'p2p_tx_download.py',
117+
'wallet_avoidreuse.py --legacy-wallet',
118+
'wallet_avoidreuse.py --descriptors',
119+
'feature_abortnode.py',
120+
'wallet_address_types.py --legacy-wallet',
121+
'wallet_address_types.py --descriptors',
109122
'wallet_basic.py --legacy-wallet',
110123
'wallet_basic.py --descriptors',
111-
'wallet_labels.py --legacy-wallet',
112-
'wallet_labels.py --descriptors',
113-
'p2p_segwit.py',
124+
'feature_maxtipage.py',
125+
'wallet_multiwallet.py --legacy-wallet',
126+
'wallet_multiwallet.py --descriptors',
127+
'wallet_multiwallet.py --usecli',
128+
'p2p_dns_seeds.py',
129+
'wallet_groups.py --legacy-wallet',
130+
'wallet_groups.py --descriptors',
131+
'p2p_blockfilters.py',
132+
'feature_assumevalid.py',
133+
'wallet_taproot.py --descriptors',
134+
'feature_bip68_sequence.py',
135+
'rpc_packages.py',
136+
'rpc_bind.py --ipv4',
137+
'rpc_bind.py --ipv6',
138+
'rpc_bind.py --nonloopback',
139+
'p2p_headers_sync_with_minchainwork.py',
140+
'p2p_feefilter.py',
141+
'feature_csv_activation.py',
142+
'p2p_sendheaders.py',
143+
'wallet_listtransactions.py --legacy-wallet',
144+
'wallet_listtransactions.py --descriptors',
145+
# vv Tests less than 30s vv
146+
'p2p_invalid_messages.py',
147+
'rpc_createmultisig.py',
114148
'p2p_timeouts.py',
115-
'p2p_tx_download.py',
116-
'mempool_updatefromblock.py',
117149
'wallet_dump.py --legacy-wallet',
118-
'feature_taproot.py',
119150
'rpc_signer.py',
120151
'wallet_signer.py --descriptors',
121-
# vv Tests less than 60s vv
122-
'p2p_sendheaders.py',
123152
'wallet_importmulti.py --legacy-wallet',
124153
'mempool_limit.py',
125154
'rpc_txoutproof.py',
126155
'wallet_listreceivedby.py --legacy-wallet',
127156
'wallet_listreceivedby.py --descriptors',
128157
'wallet_abandonconflict.py --legacy-wallet',
129-
'p2p_dns_seeds.py',
130158
'wallet_abandonconflict.py --descriptors',
131-
'feature_csv_activation.py',
132-
'wallet_address_types.py --legacy-wallet',
133-
'wallet_address_types.py --descriptors',
134-
'feature_bip68_sequence.py',
135-
'p2p_feefilter.py',
136-
'rpc_packages.py',
137159
'feature_reindex.py',
138-
'feature_abortnode.py',
139-
# vv Tests less than 30s vv
160+
'wallet_labels.py --legacy-wallet',
161+
'wallet_labels.py --descriptors',
162+
'p2p_compactblocks.py',
163+
'p2p_compactblocks_blocksonly.py',
164+
'wallet_hd.py --legacy-wallet',
165+
'wallet_hd.py --descriptors',
140166
'wallet_keypool_topup.py --legacy-wallet',
141167
'wallet_keypool_topup.py --descriptors',
142168
'wallet_fast_rescan.py --descriptors',
143-
'feature_fee_estimation.py',
144169
'interface_zmq.py',
145170
'rpc_invalid_address_message.py',
146171
'interface_bitcoin_cli.py --legacy-wallet',
@@ -158,20 +183,12 @@
158183
'rpc_misc.py',
159184
'interface_rest.py',
160185
'mempool_spend_coinbase.py',
161-
'wallet_avoidreuse.py --legacy-wallet',
162-
'wallet_avoidreuse.py --descriptors',
163186
'wallet_avoid_mixing_output_types.py --descriptors',
164187
'mempool_reorg.py',
165-
'mempool_persist.py --descriptors',
166188
'p2p_block_sync.py',
167-
'wallet_multiwallet.py --legacy-wallet',
168-
'wallet_multiwallet.py --descriptors',
169-
'wallet_multiwallet.py --usecli',
170189
'wallet_createwallet.py --legacy-wallet',
171190
'wallet_createwallet.py --usecli',
172191
'wallet_createwallet.py --descriptors',
173-
'wallet_listtransactions.py --legacy-wallet',
174-
'wallet_listtransactions.py --descriptors',
175192
'wallet_watchonly.py --legacy-wallet',
176193
'wallet_watchonly.py --usecli --legacy-wallet',
177194
'wallet_reorgsrestore.py',
@@ -181,22 +198,17 @@
181198
'interface_usdt_net.py',
182199
'interface_usdt_utxocache.py',
183200
'interface_usdt_validation.py',
184-
'rpc_psbt.py --legacy-wallet',
185-
'rpc_psbt.py --descriptors',
186201
'rpc_users.py',
187202
'rpc_whitelist.py',
188203
'feature_proxy.py',
189204
'feature_syscall_sandbox.py',
190205
'wallet_signrawtransactionwithwallet.py --legacy-wallet',
191206
'wallet_signrawtransactionwithwallet.py --descriptors',
192207
'rpc_signrawtransactionwithkey.py',
193-
'p2p_headers_sync_with_minchainwork.py',
194208
'rpc_rawtransaction.py --legacy-wallet',
195-
'wallet_groups.py --legacy-wallet',
196209
'wallet_transactiontime_rescan.py --descriptors',
197210
'wallet_transactiontime_rescan.py --legacy-wallet',
198211
'p2p_addrv2_relay.py',
199-
'wallet_groups.py --descriptors',
200212
'p2p_compactblocks_hb.py',
201213
'p2p_disconnect_ban.py',
202214
'rpc_decodescript.py',
@@ -212,17 +224,14 @@
212224
'wallet_keypool.py --descriptors',
213225
'wallet_descriptor.py --descriptors',
214226
'wallet_miniscript.py --descriptors',
215-
'feature_maxtipage.py',
216227
'p2p_nobloomfilter_messages.py',
217228
'p2p_filter.py',
218229
'rpc_setban.py',
219230
'p2p_blocksonly.py',
220231
'mining_prioritisetransaction.py',
221232
'p2p_invalid_locator.py',
222233
'p2p_invalid_block.py',
223-
'p2p_invalid_messages.py',
224234
'p2p_invalid_tx.py',
225-
'feature_assumevalid.py',
226235
'example_test.py',
227236
'wallet_txn_doublespend.py --legacy-wallet',
228237
'wallet_multisig_descriptor_psbt.py --descriptors',
@@ -238,7 +247,6 @@
238247
'feature_rbf.py',
239248
'mempool_packages.py',
240249
'mempool_package_onemore.py',
241-
'rpc_createmultisig.py',
242250
'mempool_package_limits.py',
243251
'feature_versionbits_warning.py',
244252
'rpc_preciousblock.py',
@@ -255,18 +263,12 @@
255263
'feature_nulldummy.py',
256264
'mempool_accept.py',
257265
'mempool_expiry.py',
258-
'wallet_import_rescan.py --legacy-wallet',
259266
'wallet_import_with_label.py --legacy-wallet',
260267
'wallet_importdescriptors.py --descriptors',
261268
'wallet_upgradewallet.py --legacy-wallet',
262-
'rpc_bind.py --ipv4',
263-
'rpc_bind.py --ipv6',
264-
'rpc_bind.py --nonloopback',
265269
'wallet_crosschain.py',
266270
'mining_basic.py',
267271
'feature_signet.py',
268-
'wallet_bumpfee.py --legacy-wallet',
269-
'wallet_bumpfee.py --descriptors',
270272
'wallet_implicitsegwit.py --legacy-wallet',
271273
'rpc_named_arguments.py',
272274
'feature_startupnotify.py',
@@ -297,7 +299,6 @@
297299
'wallet_sendall.py --legacy-wallet',
298300
'wallet_sendall.py --descriptors',
299301
'wallet_create_tx.py --descriptors',
300-
'wallet_taproot.py --descriptors',
301302
'wallet_inactive_hdchains.py --legacy-wallet',
302303
'p2p_fingerprint.py',
303304
'feature_uacomment.py',
@@ -310,7 +311,6 @@
310311
'p2p_add_connections.py',
311312
'feature_bind_port_discover.py',
312313
'p2p_unrequested_blocks.py',
313-
'p2p_blockfilters.py',
314314
'p2p_message_capture.py',
315315
'feature_includeconf.py',
316316
'feature_addrman.py',

0 commit comments

Comments
 (0)