Merge bitcoin/bitcoin#26656: tests: Improve runtime of some tests when --enable-debug

1647a11f39 tests: Reorder longer running tests in test_runner (Andrew Chow)
ff6c9fe027 tests: Whitelist test p2p connection in rpc_packages (Andrew Chow)
8c20796aac tests: Use waitfornewblock for work queue test in interface_rpc (Andrew Chow)
6c872d5e65 tests: Initialize sigops draining script with bytes in feature_taproot (Andrew Chow)
544cbf776c tests: Use batched RPC in feature_fee_estimation (Andrew Chow)
4ad7272f8b 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

test/functional/feature_fee_estimation.py

@@ -23,7 +23,7 @@ from test_framework.wallet import MiniWallet
 
 
 def small_txpuzzle_randfee(
-    wallet, from_node, conflist, unconflist, amount, min_fee, fee_increment
+    wallet, from_node, conflist, unconflist, amount, min_fee, fee_increment, batch_reqs
 ):
     """Create and send a transaction with a random fee using MiniWallet.
 
@@ -57,8 +57,11 @@ def small_txpuzzle_randfee(
     tx.vout[0].nValue = int((total_in - amount - fee) * COIN)
     tx.vout.append(deepcopy(tx.vout[0]))
     tx.vout[1].nValue = int(amount * COIN)
+    tx.rehash()
+    txid = tx.hash
+    tx_hex = tx.serialize().hex()
 
-    txid = from_node.sendrawtransaction(hexstring=tx.serialize().hex(), maxfeerate=0)
+    batch_reqs.append(from_node.sendrawtransaction.get_request(hexstring=tx_hex, maxfeerate=0))
     unconflist.append({"txid": txid, "vout": 0, "value": total_in - amount - fee})
     unconflist.append({"txid": txid, "vout": 1, "value": amount})
 
@@ -115,13 +118,12 @@ def check_estimates(node, fees_seen):
     check_smart_estimates(node, fees_seen)
 
 
-def send_tx(wallet, node, utxo, feerate):
-    """Broadcast a 1in-1out transaction with a specific input and feerate (sat/vb)."""
-    return wallet.send_self_transfer(
-        from_node=node,
+def make_tx(wallet, utxo, feerate):
+    """Create a 1in-1out transaction with a specific input and feerate (sat/vb)."""
+    return wallet.create_self_transfer(
         utxo_to_spend=utxo,
         fee_rate=Decimal(feerate * 1000) / COIN,
-    )['txid']
+    )
 
 
 class EstimateFeeTest(BitcoinTestFramework):
@@ -156,6 +158,7 @@ class EstimateFeeTest(BitcoinTestFramework):
         # resorting to tx's that depend on the mempool when those run out
         for _ in range(numblocks):
             random.shuffle(self.confutxo)
+            batch_sendtx_reqs = []
             for _ in range(random.randrange(100 - 50, 100 + 50)):
                 from_index = random.randint(1, 2)
                 (tx_bytes, fee) = small_txpuzzle_randfee(
@@ -166,9 +169,12 @@ class EstimateFeeTest(BitcoinTestFramework):
                     Decimal("0.005"),
                     min_fee,
                     min_fee,
+                    batch_sendtx_reqs,
                 )
                 tx_kbytes = tx_bytes / 1000.0
                 self.fees_per_kb.append(float(fee) / tx_kbytes)
+            for node in self.nodes:
+                node.batch(batch_sendtx_reqs)
             self.sync_mempools(wait=0.1)
             mined = mining_node.getblock(self.generate(mining_node, 1)[0], True)["tx"]
             # update which txouts are confirmed
@@ -245,14 +251,20 @@ class EstimateFeeTest(BitcoinTestFramework):
         assert_greater_than_or_equal(len(utxos), 250)
         for _ in range(5):
             # Broadcast 45 low fee transactions that will need to be RBF'd
+            txs = []
             for _ in range(45):
                 u = utxos.pop(0)
-                txid = send_tx(self.wallet, node, u, low_feerate)
+                tx = make_tx(self.wallet, u, low_feerate)
                 utxos_to_respend.append(u)
-                txids_to_replace.append(txid)
+                txids_to_replace.append(tx["txid"])
+                txs.append(tx)
             # Broadcast 5 low fee transaction which don't need to
             for _ in range(5):
-                send_tx(self.wallet, node, utxos.pop(0), low_feerate)
+                tx = make_tx(self.wallet, utxos.pop(0), low_feerate)
+                txs.append(tx)
+            batch_send_tx = [node.sendrawtransaction.get_request(tx["hex"]) for tx in txs]
+            for n in self.nodes:
+                n.batch(batch_send_tx)
             # Mine the transactions on another node
             self.sync_mempools(wait=0.1, nodes=[node, miner])
             for txid in txids_to_replace:
@@ -261,7 +273,12 @@ class EstimateFeeTest(BitcoinTestFramework):
             # RBF the low-fee transactions
             while len(utxos_to_respend) > 0:
                 u = utxos_to_respend.pop(0)
-                send_tx(self.wallet, node, u, high_feerate)
+                tx = make_tx(self.wallet, u, high_feerate)
+                node.sendrawtransaction(tx["hex"])
+                txs.append(tx)
+            dec_txs = [res["result"] for res in node.batch([node.decoderawtransaction.get_request(tx["hex"]) for tx in txs])]
+            self.wallet.scan_txs(dec_txs)
+
 
         # Mine the last replacement txs
         self.sync_mempools(wait=0.1, nodes=[node, miner])

test/functional/feature_taproot.py

@@ -1292,7 +1292,7 @@ class TaprootTest(BitcoinTestFramework):
         # It is not impossible to fit enough tapscript sigops to hit the old 80k limit without
         # busting txin-level limits. We simply have to account for the p2pk outputs in all
         # transactions.
-        extra_output_script = CScript([OP_CHECKSIG]*((MAX_BLOCK_SIGOPS_WEIGHT - sigops_weight) // WITNESS_SCALE_FACTOR))
+        extra_output_script = CScript(bytes([OP_CHECKSIG]*((MAX_BLOCK_SIGOPS_WEIGHT - sigops_weight) // WITNESS_SCALE_FACTOR)))
 
         coinbase_tx = create_coinbase(self.lastblockheight + 1, pubkey=cb_pubkey, extra_output_script=extra_output_script, fees=fees)
         block = create_block(self.tip, coinbase_tx, self.lastblocktime + 1, txlist=txs)

test/functional/interface_rpc.py

@@ -25,7 +25,7 @@ def expect_http_status(expected_http_status, expected_rpc_code,
 def test_work_queue_getblock(node, got_exceeded_error):
     while not got_exceeded_error:
         try:
-            node.cli('getrpcinfo').send_cli()
+            node.cli("waitfornewblock", "500").send_cli()
         except subprocess.CalledProcessError as e:
             assert_equal(e.output, 'error: Server response: Work queue depth exceeded\n')
             got_exceeded_error.append(True)

test/functional/rpc_packages.py

@@ -29,6 +29,7 @@ class RPCPackagesTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
         self.setup_clean_chain = True
+        self.extra_args = [["-whitelist=noban@127.0.0.1"]] # noban speeds up tx relay
 
     def assert_testres_equal(self, package_hex, testres_expected):
         """Shuffle package_hex and assert that the testmempoolaccept result matches testres_expected. This should only

test/functional/test_framework/wallet.py

@@ -141,6 +141,10 @@ class MiniWallet:
             if out['scriptPubKey']['hex'] == self._scriptPubKey.hex():
                 self._utxos.append(self._create_utxo(txid=tx["txid"], vout=out["n"], value=out["value"], height=0))
 
+    def scan_txs(self, txs):
+        for tx in txs:
+            self.scan_tx(tx)
+
     def sign_tx(self, tx, fixed_length=True):
         """Sign tx that has been created by MiniWallet in P2PK mode"""
         assert_equal(self._mode, MiniWalletMode.RAW_P2PK)

test/functional/test_runner.py

@@ -91,56 +91,81 @@ EXTENDED_SCRIPTS = [
 BASE_SCRIPTS = [
     # Scripts that are run by default.
     # Longest test should go first, to favor running tests in parallel
-    'wallet_hd.py --legacy-wallet',
-    'wallet_hd.py --descriptors',
-    'wallet_backup.py --legacy-wallet',
-    'wallet_backup.py --descriptors',
     # vv Tests less than 5m vv
-    'mining_getblocktemplate_longpoll.py',
-    'feature_maxuploadtarget.py',
+    'feature_fee_estimation.py',
+    'feature_taproot.py',
     'feature_block.py',
+    # vv Tests less than 2m vv
+    'mining_getblocktemplate_longpoll.py',
+    'p2p_segwit.py',
+    'feature_maxuploadtarget.py',
+    'mempool_updatefromblock.py',
+    'mempool_persist.py --descriptors',
+    # vv Tests less than 60s vv
+    'rpc_psbt.py --legacy-wallet',
+    'rpc_psbt.py --descriptors',
     'wallet_fundrawtransaction.py --legacy-wallet',
     'wallet_fundrawtransaction.py --descriptors',
-    'p2p_compactblocks.py',
-    'p2p_compactblocks_blocksonly.py',
+    'wallet_bumpfee.py --legacy-wallet',
+    'wallet_bumpfee.py --descriptors',
+    'wallet_import_rescan.py --legacy-wallet',
+    'wallet_backup.py --legacy-wallet',
+    'wallet_backup.py --descriptors',
     'feature_segwit.py --legacy-wallet',
     'feature_segwit.py --descriptors',
-    # vv Tests less than 2m vv
+    'p2p_tx_download.py',
+    'wallet_avoidreuse.py --legacy-wallet',
+    'wallet_avoidreuse.py --descriptors',
+    'feature_abortnode.py',
+    'wallet_address_types.py --legacy-wallet',
+    'wallet_address_types.py --descriptors',
     'wallet_basic.py --legacy-wallet',
     'wallet_basic.py --descriptors',
-    'wallet_labels.py --legacy-wallet',
-    'wallet_labels.py --descriptors',
-    'p2p_segwit.py',
+    'feature_maxtipage.py',
+    'wallet_multiwallet.py --legacy-wallet',
+    'wallet_multiwallet.py --descriptors',
+    'wallet_multiwallet.py --usecli',
+    'p2p_dns_seeds.py',
+    'wallet_groups.py --legacy-wallet',
+    'wallet_groups.py --descriptors',
+    'p2p_blockfilters.py',
+    'feature_assumevalid.py',
+    'wallet_taproot.py --descriptors',
+    'feature_bip68_sequence.py',
+    'rpc_packages.py',
+    'rpc_bind.py --ipv4',
+    'rpc_bind.py --ipv6',
+    'rpc_bind.py --nonloopback',
+    'p2p_headers_sync_with_minchainwork.py',
+    'p2p_feefilter.py',
+    'feature_csv_activation.py',
+    'p2p_sendheaders.py',
+    'wallet_listtransactions.py --legacy-wallet',
+    'wallet_listtransactions.py --descriptors',
+    # vv Tests less than 30s vv
+    'p2p_invalid_messages.py',
+    'rpc_createmultisig.py',
     'p2p_timeouts.py',
-    'p2p_tx_download.py',
-    'mempool_updatefromblock.py',
     'wallet_dump.py --legacy-wallet',
-    'feature_taproot.py',
     'rpc_signer.py',
     'wallet_signer.py --descriptors',
-    # vv Tests less than 60s vv
-    'p2p_sendheaders.py',
     'wallet_importmulti.py --legacy-wallet',
     'mempool_limit.py',
     'rpc_txoutproof.py',
     'wallet_listreceivedby.py --legacy-wallet',
     'wallet_listreceivedby.py --descriptors',
     'wallet_abandonconflict.py --legacy-wallet',
-    'p2p_dns_seeds.py',
     'wallet_abandonconflict.py --descriptors',
-    'feature_csv_activation.py',
-    'wallet_address_types.py --legacy-wallet',
-    'wallet_address_types.py --descriptors',
-    'feature_bip68_sequence.py',
-    'p2p_feefilter.py',
-    'rpc_packages.py',
     'feature_reindex.py',
-    'feature_abortnode.py',
-    # vv Tests less than 30s vv
+    'wallet_labels.py --legacy-wallet',
+    'wallet_labels.py --descriptors',
+    'p2p_compactblocks.py',
+    'p2p_compactblocks_blocksonly.py',
+    'wallet_hd.py --legacy-wallet',
+    'wallet_hd.py --descriptors',
     'wallet_keypool_topup.py --legacy-wallet',
     'wallet_keypool_topup.py --descriptors',
     'wallet_fast_rescan.py --descriptors',
-    'feature_fee_estimation.py',
     'interface_zmq.py',
     'rpc_invalid_address_message.py',
     'interface_bitcoin_cli.py --legacy-wallet',
@@ -158,20 +183,12 @@ BASE_SCRIPTS = [
     'rpc_misc.py',
     'interface_rest.py',
     'mempool_spend_coinbase.py',
-    'wallet_avoidreuse.py --legacy-wallet',
-    'wallet_avoidreuse.py --descriptors',
     'wallet_avoid_mixing_output_types.py --descriptors',
     'mempool_reorg.py',
-    'mempool_persist.py --descriptors',
     'p2p_block_sync.py',
-    'wallet_multiwallet.py --legacy-wallet',
-    'wallet_multiwallet.py --descriptors',
-    'wallet_multiwallet.py --usecli',
     'wallet_createwallet.py --legacy-wallet',
     'wallet_createwallet.py --usecli',
     'wallet_createwallet.py --descriptors',
-    'wallet_listtransactions.py --legacy-wallet',
-    'wallet_listtransactions.py --descriptors',
     'wallet_watchonly.py --legacy-wallet',
     'wallet_watchonly.py --usecli --legacy-wallet',
     'wallet_reorgsrestore.py',
@@ -181,8 +198,6 @@ BASE_SCRIPTS = [
     'interface_usdt_net.py',
     'interface_usdt_utxocache.py',
     'interface_usdt_validation.py',
-    'rpc_psbt.py --legacy-wallet',
-    'rpc_psbt.py --descriptors',
     'rpc_users.py',
     'rpc_whitelist.py',
     'feature_proxy.py',
@@ -190,13 +205,10 @@ BASE_SCRIPTS = [
     'wallet_signrawtransactionwithwallet.py --legacy-wallet',
     'wallet_signrawtransactionwithwallet.py --descriptors',
     'rpc_signrawtransactionwithkey.py',
-    'p2p_headers_sync_with_minchainwork.py',
     'rpc_rawtransaction.py --legacy-wallet',
-    'wallet_groups.py --legacy-wallet',
     'wallet_transactiontime_rescan.py --descriptors',
     'wallet_transactiontime_rescan.py --legacy-wallet',
     'p2p_addrv2_relay.py',
-    'wallet_groups.py --descriptors',
     'p2p_compactblocks_hb.py',
     'p2p_disconnect_ban.py',
     'rpc_decodescript.py',
@@ -212,7 +224,6 @@ BASE_SCRIPTS = [
     'wallet_keypool.py --descriptors',
     'wallet_descriptor.py --descriptors',
     'wallet_miniscript.py --descriptors',
-    'feature_maxtipage.py',
     'p2p_nobloomfilter_messages.py',
     'p2p_filter.py',
     'rpc_setban.py',
@@ -220,9 +231,7 @@ BASE_SCRIPTS = [
     'mining_prioritisetransaction.py',
     'p2p_invalid_locator.py',
     'p2p_invalid_block.py',
-    'p2p_invalid_messages.py',
     'p2p_invalid_tx.py',
-    'feature_assumevalid.py',
     'example_test.py',
     'wallet_txn_doublespend.py --legacy-wallet',
     'wallet_multisig_descriptor_psbt.py --descriptors',
@@ -238,7 +247,6 @@ BASE_SCRIPTS = [
     'feature_rbf.py',
     'mempool_packages.py',
     'mempool_package_onemore.py',
-    'rpc_createmultisig.py',
     'mempool_package_limits.py',
     'feature_versionbits_warning.py',
     'rpc_preciousblock.py',
@@ -255,18 +263,12 @@ BASE_SCRIPTS = [
     'feature_nulldummy.py',
     'mempool_accept.py',
     'mempool_expiry.py',
-    'wallet_import_rescan.py --legacy-wallet',
     'wallet_import_with_label.py --legacy-wallet',
     'wallet_importdescriptors.py --descriptors',
     'wallet_upgradewallet.py --legacy-wallet',
-    'rpc_bind.py --ipv4',
-    'rpc_bind.py --ipv6',
-    'rpc_bind.py --nonloopback',
     'wallet_crosschain.py',
     'mining_basic.py',
     'feature_signet.py',
-    'wallet_bumpfee.py --legacy-wallet',
-    'wallet_bumpfee.py --descriptors',
     'wallet_implicitsegwit.py --legacy-wallet',
     'rpc_named_arguments.py',
     'feature_startupnotify.py',
@@ -297,7 +299,6 @@ BASE_SCRIPTS = [
     'wallet_sendall.py --legacy-wallet',
     'wallet_sendall.py --descriptors',
     'wallet_create_tx.py --descriptors',
-    'wallet_taproot.py --descriptors',
     'wallet_inactive_hdchains.py --legacy-wallet',
     'p2p_fingerprint.py',
     'feature_uacomment.py',
@@ -310,7 +311,6 @@ BASE_SCRIPTS = [
     'p2p_add_connections.py',
     'feature_bind_port_discover.py',
     'p2p_unrequested_blocks.py',
-    'p2p_blockfilters.py',
     'p2p_message_capture.py',
     'feature_includeconf.py',
     'feature_addrman.py',

test/functional/wallet_import_rescan.py

@@ -179,7 +179,16 @@ class ImportRescanTest(BitcoinTestFramework):
 
         # Create one transaction on node 0 with a unique amount for
         # each possible type of wallet import RPC.
+        last_variants = []
         for i, variant in enumerate(IMPORT_VARIANTS):
+            if i % 10 == 0:
+                blockhash = self.generate(self.nodes[0], 1)[0]
+                conf_height = self.nodes[0].getblockcount()
+                timestamp = self.nodes[0].getblockheader(blockhash)["time"]
+                for var in last_variants:
+                    var.confirmation_height = conf_height
+                    var.timestamp = timestamp
+                last_variants.clear()
             variant.label = "label {} {}".format(i, variant)
             variant.address = self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress(
                 label=variant.label,
@@ -188,9 +197,15 @@ class ImportRescanTest(BitcoinTestFramework):
             variant.key = self.nodes[1].dumpprivkey(variant.address["address"])
             variant.initial_amount = get_rand_amount()
             variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount)
-            self.generate(self.nodes[0], 1)  # Generate one block for each send
-            variant.confirmation_height = self.nodes[0].getblockcount()
-            variant.timestamp = self.nodes[0].getblockheader(self.nodes[0].getbestblockhash())["time"]
+            last_variants.append(variant)
+
+        blockhash = self.generate(self.nodes[0], 1)[0]
+        conf_height = self.nodes[0].getblockcount()
+        timestamp = self.nodes[0].getblockheader(blockhash)["time"]
+        for var in last_variants:
+            var.confirmation_height = conf_height
+            var.timestamp = timestamp
+        last_variants.clear()
 
         # Generate a block further in the future (past the rescan window).
         assert_equal(self.nodes[0].getrawmempool(), [])
@@ -217,11 +232,14 @@ class ImportRescanTest(BitcoinTestFramework):
                 variant.check()
 
         # Create new transactions sending to each address.
-        for variant in IMPORT_VARIANTS:
+        for i, variant in enumerate(IMPORT_VARIANTS):
+            if i % 10 == 0:
+                blockhash = self.generate(self.nodes[0], 1)[0]
+                conf_height = self.nodes[0].getblockcount() + 1
             variant.sent_amount = get_rand_amount()
             variant.sent_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.sent_amount)
-            self.generate(self.nodes[0], 1)  # Generate one block for each send
-            variant.confirmation_height = self.nodes[0].getblockcount()
+            variant.confirmation_height = conf_height
+        self.generate(self.nodes[0], 1)
 
         assert_equal(self.nodes[0].getrawmempool(), [])
         self.sync_all()