Merge bitcoin/bitcoin#29483: test, ci: add --v1transport option, add --v2transport to a CI task

ecc036c5d6 ci: add --v2transport to an existing CI job (Martin Zumsande)
3a25a575f0 test: ignore --v2transport for older versions instead of asserting (Martin Zumsande)
547aacff08 test: add -v1transport option and use it in test_runner (Martin Zumsande)

Pull request description:

  This suggests a strategy to run the functional tests with both v1 and v2 transport in the CI.

  **Status Quo:**
  There is both the global `--v2transport` option for the `test_runner.py` (not enabled by default), plus the possibility to specify `--v2transport` for particular tests, which is used for a handful of tests. Currently, when running `test_runner.py --v2transport`, these tests are run twice with the same `--v2transport` configuration, as has been noted in https://github.com/bitcoin/bitcoin/pull/29358#discussion_r1485626063, which is wasteful.

  **Suggested Change:**
  Fix this by adding a `--v1transport` option and using it in `test_runner.py`, so that irrespective of the global `--v2transport` flag, the  tests that run twice use v1 in one run and v2 in the other.
  Also add `--v2transport` to one CI task (`multiprocess, i686, DEBUG`).
  This means, that for each CI task, the majority of functional tests will run once using the global `--v2transport` option if provided, while a few selected tests will always run two times, once with `v1` and once with `v2`.

  **Rationale:**
  A simpler alternative would have been to remove all test-specific `--v2transport` commands from `test_runner.py` and just enable `--v2transport` option for a few CI tasks. I didn't do that because it would have meant that v2 would never be running in the CI for some platforms, and also be run a lot less locally by users and devs (who would have to actively enable the `--v2transport` option).

ACKs for top commit:
  tdb3:
    ACK for ecc036c5d6.
  achow101:
    ACK ecc036c5d6
  stratospher:
    ACK ecc036c.
  vasild:
    ACK ecc036c5d6

Tree-SHA512: 375b2293d49991f2fbd8e1bb646c0034004a09cee36063bc32996b721323eb19a43d7b2f36b3f9a3fdca846d74f48d8f1387565c03ef5d34b3481d2a0fe1d328

ci/test/00_setup_env_i686_multiprocess.sh

@@ -12,6 +12,7 @@ export CI_IMAGE_NAME_TAG="docker.io/amd64/ubuntu:22.04"
 export PACKAGES="llvm clang g++-multilib"
 export DEP_OPTS="DEBUG=1 MULTIPROCESS=1"
 export GOAL="install"
+export TEST_RUNNER_EXTRA="--v2transport"
 export BITCOIN_CONFIG="--enable-debug CC='clang -m32' CXX='clang++ -m32' \
 CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE'"
 export BITCOIND=bitcoin-node  # Used in functional tests

test/functional/test_framework/test_framework.py

@@ -191,6 +191,8 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
         parser.add_argument("--timeout-factor", dest="timeout_factor", type=float, help="adjust test timeouts by a factor. Setting it to 0 disables all timeouts")
         parser.add_argument("--v2transport", dest="v2transport", default=False, action="store_true",
                             help="use BIP324 v2 connections between all nodes by default")
+        parser.add_argument("--v1transport", dest="v1transport", default=False, action="store_true",
+                            help="Explicitly use v1 transport (can be used to overwrite global --v2transport option)")
 
         self.add_options(parser)
         # Running TestShell in a Jupyter notebook causes an additional -f argument
@@ -206,6 +208,8 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
         config = configparser.ConfigParser()
         config.read_file(open(self.options.configfile))
         self.config = config
+        if self.options.v1transport:
+            self.options.v2transport=False
 
         if "descriptors" not in self.options:
             # Wallet is not required by the test at all and the value of self.options.descriptors won't matter.

test/functional/test_framework/test_node.py

@@ -136,9 +136,7 @@ class TestNode():
                 self.args.append("-v2transport=1")
             else:
                 self.args.append("-v2transport=0")
-        else:
+        # if v2transport is requested via global flag but not supported for node version, ignore it
-            # v2transport requested but not supported for node
-            assert not v2transport
 
         self.cli = TestNodeCLI(bitcoin_cli, self.datadir_path)
         self.use_cli = use_cli

test/functional/test_runner.py

@@ -120,7 +120,7 @@ BASE_SCRIPTS = [
     'wallet_backup.py --legacy-wallet',
     'wallet_backup.py --descriptors',
     'feature_segwit.py --legacy-wallet',
-    'feature_segwit.py --descriptors',
+    'feature_segwit.py --descriptors --v1transport',
     'feature_segwit.py --descriptors --v2transport',
     'p2p_tx_download.py',
     'wallet_avoidreuse.py --legacy-wallet',
@@ -156,7 +156,7 @@ BASE_SCRIPTS = [
     # vv Tests less than 30s vv
     'p2p_invalid_messages.py',
     'rpc_createmultisig.py',
-    'p2p_timeouts.py',
+    'p2p_timeouts.py --v1transport',
     'p2p_timeouts.py --v2transport',
     'wallet_dump.py --legacy-wallet',
     'rpc_signer.py',
@@ -201,7 +201,7 @@ BASE_SCRIPTS = [
     'mempool_spend_coinbase.py',
     'wallet_avoid_mixing_output_types.py --descriptors',
     'mempool_reorg.py',
-    'p2p_block_sync.py',
+    'p2p_block_sync.py --v1transport',
     'p2p_block_sync.py --v2transport',
     'wallet_createwallet.py --legacy-wallet',
     'wallet_createwallet.py --usecli',
@@ -230,13 +230,13 @@ BASE_SCRIPTS = [
     'wallet_transactiontime_rescan.py --descriptors',
     'wallet_transactiontime_rescan.py --legacy-wallet',
     'p2p_addrv2_relay.py',
-    'p2p_compactblocks_hb.py',
+    'p2p_compactblocks_hb.py --v1transport',
     'p2p_compactblocks_hb.py --v2transport',
-    'p2p_disconnect_ban.py',
+    'p2p_disconnect_ban.py --v1transport',
     'p2p_disconnect_ban.py --v2transport',
     'feature_posix_fs_permissions.py',
     'rpc_decodescript.py',
-    'rpc_blockchain.py',
+    'rpc_blockchain.py --v1transport',
     'rpc_blockchain.py --v2transport',
     'rpc_deprecated.py',
     'wallet_disable.py',
@@ -246,21 +246,21 @@ BASE_SCRIPTS = [
     'p2p_getaddr_caching.py',
     'p2p_getdata.py',
     'p2p_addrfetch.py',
-    'rpc_net.py',
+    'rpc_net.py --v1transport',
     'rpc_net.py --v2transport',
     'wallet_keypool.py --legacy-wallet',
     'wallet_keypool.py --descriptors',
     'wallet_descriptor.py --descriptors',
     'p2p_nobloomfilter_messages.py',
     'p2p_filter.py',
-    'rpc_setban.py',
+    'rpc_setban.py --v1transport',
     'rpc_setban.py --v2transport',
     'p2p_blocksonly.py',
     'mining_prioritisetransaction.py',
     'p2p_invalid_locator.py',
-    'p2p_invalid_block.py',
+    'p2p_invalid_block.py --v1transport',
     'p2p_invalid_block.py --v2transport',
-    'p2p_invalid_tx.py',
+    'p2p_invalid_tx.py --v1transport',
     'p2p_invalid_tx.py --v2transport',
     'p2p_v2_transport.py',
     'p2p_v2_encrypted.py',
@@ -286,12 +286,12 @@ BASE_SCRIPTS = [
     'rpc_preciousblock.py',
     'wallet_importprunedfunds.py --legacy-wallet',
     'wallet_importprunedfunds.py --descriptors',
-    'p2p_leak_tx.py',
+    'p2p_leak_tx.py --v1transport',
     'p2p_leak_tx.py --v2transport',
     'p2p_eviction.py',
-    'p2p_ibd_stalling.py',
+    'p2p_ibd_stalling.py --v1transport',
     'p2p_ibd_stalling.py --v2transport',
-    'p2p_net_deadlock.py',
+    'p2p_net_deadlock.py --v1transport',
     'p2p_net_deadlock.py --v2transport',
     'wallet_signmessagewithaddress.py',
     'rpc_signmessagewithprivkey.py',
@@ -381,7 +381,7 @@ BASE_SCRIPTS = [
     'feature_coinstatsindex.py',
     'wallet_orphanedreward.py',
     'wallet_timelock.py',
-    'p2p_node_network_limited.py',
+    'p2p_node_network_limited.py --v1transport',
     'p2p_node_network_limited.py --v2transport',
     'p2p_permissions.py',
     'feature_blocksdir.py',