Merge bitcoin/bitcoin#32290: test: allow all functional tests to be run or skipped with --usecli

666016e ci: use --usecli in one of the CI jobs (Martin Zumsande)
7ea248a test: Disable several (sub)tests with cli (Martin Zumsande)
f420b63 test: skip subtests that check for wrong types with cli (Martin Zumsande)
6530d00 test: add function to convert to json for height_or_hash params (Martin Zumsande)
54d2872 test: Don't send empty named args with cli (Martin Zumsande)
cca4220 test: convert tuple to json for cli (Martin Zumsande)
af34e98 test: make rpc_psbt.py usable with --usecli (Martin Zumsande)
8f8ce9e test: rename .rpc to ._rpc and remove unnecessary uses (Martin Zumsande)
5b08885 test: enable functional tests with large rpc args for cli (Martin Zumsande)
7d5352a test: use -stdin for large rpc commands (Martin Zumsande)
6c364e0 test: Enable various tests for usage with cli (Martin Zumsande)

Pull request description:

  Fixes #32264

  I looked into all current failures listed in the issue, as well all tests that are already disabled for the cli with `self.supports_cli = False`. There are several reasons why existing tests fail  with `--usecli` on many systems, the most important ones are:

  - Most common reason is that the test executes a RPC call with a large arg that exceeds `MAX_ARG_STRLEN` of the OS, which is usually 128kb on linux: This is fixed by using `-stdin` for these large calls (idea by 0xB10C)
  - they test specifically the rpc interface - nothing to do there except disabling.
  - Some functional test submit wrong types to params on purpose to test the error message (which is different when using the cli) - deactivated these specific subtests locally for the cli when there is just one or two of them, deactivated the entire tests when there are more spots
  - When python sets `None` for an arg, the cli converts this to 'null' in `arg_to_cli`. This is fine e.g. for boolean args, but doesn't work for strings where it's interpreted as the string 'null'. Bypass this for named args by not including args in case the value is `None` for the cli is used (it's effectively the same as leaving the optional arg out).
  -  the `height_or_hash` param used in some RPC needs to be converted to a JSON (effectively adding full quotes).
  - Some tests were marked with `self.supports_cli = False` in the past but run fine on master today - enabled those.

  In total, this PR fixes all tests that fail on master and reduces the number of tests that are deactivated (`self.supports_cli = False`) from 40 to 21.
  It also adds `--usecli` to one CI job (multiprocess, i686, DEBUG) to detect regressions.

ACKs for top commit:
  maflcko:
    re-ACK 666016e 🔀
  pinheadmz:
    re-ACK 666016e

Tree-SHA512: 7a1efd212649ca100b236a1239294d40ecd36e2720e3b173a230b14545bb40b135111db7fed8a0d1448120f5387da146a03f1912e2028c8d03a0b6a3ca8761b0

Author: fanquake

ci/test/00_setup_env_i686_multiprocess.sh

@@ -13,7 +13,7 @@ export CI_IMAGE_PLATFORM="linux/amd64"
 export PACKAGES="llvm clang g++-multilib"
 export DEP_OPTS="DEBUG=1 MULTIPROCESS=1"
 export GOAL="install"
-export TEST_RUNNER_EXTRA="--v2transport"
+export TEST_RUNNER_EXTRA="--v2transport --usecli"
 export BITCOIN_CONFIG="\
  -DCMAKE_BUILD_TYPE=Debug \
  -DCMAKE_C_COMPILER='clang;-m32' \

test/functional/feature_assumeutxo.py

@@ -481,7 +481,7 @@ def check_dump_output(output):
 
         # Use a hash instead of a height
         prev_snap_hash = n0.getblockhash(prev_snap_height)
-        dump_output5 = n0.dumptxoutset('utxos5.dat', rollback=prev_snap_hash)
+        dump_output5 = n0.dumptxoutset('utxos5.dat', rollback=self.convert_to_json_for_cli(prev_snap_hash))
         assert_equal(sha256sum_file(dump_output4['path']), sha256sum_file(dump_output5['path']))
 
         # Ensure n0 is back at the tip

test/functional/feature_coinstatsindex.py

@@ -41,7 +41,6 @@ class CoinStatsIndexTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 2
-        self.supports_cli = False
         self.extra_args = [
             [],
             ["-coinstatsindex"]
@@ -104,7 +103,7 @@ def _test_coin_stats_index(self):
             assert_equal(res0, res2)
 
             # Fetch old stats by hash
-            res3 = index_node.gettxoutsetinfo(hash_option, res0['bestblock'])
+            res3 = index_node.gettxoutsetinfo(hash_option, self.convert_to_json_for_cli(res0['bestblock']))
             del res3['block_info'], res3['total_unspendable_amount']
             res3.pop('muhash', None)
             assert_equal(res0, res3)
@@ -243,7 +242,7 @@ def _test_coin_stats_index(self):
         assert_equal(res12, res10)
 
         self.log.info("Test obtaining info for a non-existent block hash")
-        assert_raises_rpc_error(-5, "Block not found", index_node.gettxoutsetinfo, hash_type="none", hash_or_height="ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", use_index=True)
+        assert_raises_rpc_error(-5, "Block not found", index_node.gettxoutsetinfo, hash_type="none", hash_or_height=self.convert_to_json_for_cli("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"), use_index=True)
 
     def _test_use_index_option(self):
         self.log.info("Test use_index option for nodes running the index")
@@ -278,7 +277,7 @@ def _test_reorg_index(self):
         assert_not_equal(res["muhash"], res_invalid["muhash"])
 
         # Test that requesting reorged out block by hash is still returning correct results
-        res_invalid2 = index_node.gettxoutsetinfo(hash_type='muhash', hash_or_height=reorg_block)
+        res_invalid2 = index_node.gettxoutsetinfo(hash_type='muhash', hash_or_height=self.convert_to_json_for_cli(reorg_block))
         assert_equal(res_invalid2["muhash"], res_invalid["muhash"])
         assert_not_equal(res["muhash"], res_invalid2["muhash"])
 

test/functional/feature_csv_activation.py

@@ -100,7 +100,6 @@ def set_test_params(self):
         self.extra_args = [[
             f'-testactivationheight=csv@{CSV_ACTIVATION_HEIGHT}',
         ]]
-        self.supports_cli = False
 
     def create_self_transfer_from_utxo(self, input_tx):
         utxo = self.miniwallet.get_utxo(txid=input_tx.txid_hex, mark_as_spent=False)

test/functional/feature_index_prune.py

@@ -67,7 +67,7 @@ def run_test(self):
         for node in filter_nodes:
             assert_greater_than(len(node.getblockfilter(tip)['filter']), 0)
         for node in stats_nodes:
-            assert node.gettxoutsetinfo(hash_type="muhash", hash_or_height=tip)['muhash']
+            assert node.gettxoutsetinfo(hash_type="muhash", hash_or_height=self.convert_to_json_for_cli(tip))['muhash']
 
         self.generate(self.nodes[0], 500)
         self.sync_index(height=700)
@@ -85,14 +85,14 @@ def run_test(self):
         for node in filter_nodes:
             assert_greater_than(len(node.getblockfilter(tip)['filter']), 0)
         for node in stats_nodes:
-            assert node.gettxoutsetinfo(hash_type="muhash", hash_or_height=tip)['muhash']
+            assert node.gettxoutsetinfo(hash_type="muhash", hash_or_height=self.convert_to_json_for_cli(tip))['muhash']
 
         self.log.info("check if we can access the blockfilter and coinstats of a pruned block")
         height_hash = self.nodes[0].getblockhash(2)
         for node in filter_nodes:
             assert_greater_than(len(node.getblockfilter(height_hash)['filter']), 0)
         for node in stats_nodes:
-            assert node.gettxoutsetinfo(hash_type="muhash", hash_or_height=height_hash)['muhash']
+            assert node.gettxoutsetinfo(hash_type="muhash", hash_or_height=self.convert_to_json_for_cli(height_hash))['muhash']
 
         # mine and sync index up to a height that will later be the pruneheight
         self.generate(self.nodes[0], 51)
@@ -106,7 +106,7 @@ def run_test(self):
             assert_raises_rpc_error(-1, msg, node.getblockfilter, height_hash)
         for node in stats_nodes:
             msg = "Querying specific block heights requires coinstatsindex"
-            assert_raises_rpc_error(-8, msg, node.gettxoutsetinfo, "muhash", height_hash)
+            assert_raises_rpc_error(-8, msg, node.gettxoutsetinfo, "muhash", self.convert_to_json_for_cli(height_hash))
 
         self.generate(self.nodes[0], 749)
 

test/functional/feature_maxuploadtarget.py

@@ -52,7 +52,6 @@ def set_test_params(self):
         self.extra_args = [[
             f"-maxuploadtarget={UPLOAD_TARGET_MB}M",
         ]]
-        self.supports_cli = False
 
     def assert_uploadtarget_state(self, *, target_reached, serve_historical_blocks):
         """Verify the node's current upload target state via the `getnettotals` RPC call."""

test/functional/feature_pruning.py

@@ -69,7 +69,6 @@ class PruneTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 6
-        self.supports_cli = False
         self.uses_wallet = None
 
         # Create nodes 0 and 1 to mine.

test/functional/feature_rbf.py

@@ -33,7 +33,6 @@ def set_test_params(self):
             [
             ],
         ]
-        self.supports_cli = False
         self.uses_wallet = None
 
     def run_test(self):

test/functional/mempool_limit.py

@@ -31,7 +31,6 @@ def set_test_params(self):
         self.extra_args = [[
             "-maxmempool=5",
         ]]
-        self.supports_cli = False
 
     def test_rbf_carveout_disallowed(self):
         node = self.nodes[0]

test/functional/mining_basic.py

@@ -65,7 +65,6 @@ def set_test_params(self):
             ["-fastprune", "-prune=1"]
         ]
         self.setup_clean_chain = True
-        self.supports_cli = False
 
     def mine_chain(self):
         self.log.info('Create some old blocks')