Fix JSON parameter parsing in warnet bitcoin rpc command

src/warnet/bitcoin.py

@@ -1,5 +1,7 @@
+import json
 import os
 import re
+import shlex
 import sys
 from datetime import datetime
 from io import BytesIO
@@ -23,9 +25,9 @@ def bitcoin():
 @bitcoin.command(context_settings={"ignore_unknown_options": True})
 @click.argument("tank", type=str)
 @click.argument("method", type=str)
-@click.argument("params", type=str, nargs=-1)  # this will capture all remaining arguments
+@click.argument("params", type=click.UNPROCESSED, nargs=-1)  # get raw unprocessed arguments
 @click.option("--namespace", default=None, show_default=True)
-def rpc(tank: str, method: str, params: str, namespace: Optional[str]):
+def rpc(tank: str, method: str, params: list[str], namespace: Optional[str]):
     """
     Call bitcoin-cli <method> [params] on <tank pod name>
     """
@@ -37,14 +39,37 @@ def rpc(tank: str, method: str, params: str, namespace: Optional[str]):
     print(result)
 
 
-def _rpc(tank: str, method: str, params: str, namespace: Optional[str] = None):
-    # bitcoin-cli should be able to read bitcoin.conf inside the container
-    # so no extra args like port, chain, username or password are needed
+def _rpc(tank: str, method: str, params: list[str], namespace: Optional[str] = None):
     namespace = get_default_namespace_or(namespace)
+
     if params:
-        cmd = f"kubectl -n {namespace} exec {tank} --container {BITCOINCORE_CONTAINER} -- bitcoin-cli {method} {' '.join(map(str, params))}"
+        # First, try to join all parameters into a single string.
+        full_param_str = " ".join(params)
+
+        try:
+            # Heuristic: if the string looks like a JSON object/array, try to parse it.
+            # This handles the `signet_test` case where one large JSON argument was split
+            # by the shell into multiple params.
+            if full_param_str.strip().startswith(("[", "{")):
+                json.loads(full_param_str)
+                # SUCCESS: The params form a single, valid JSON object.
+                # Quote the entire reconstructed string as one argument.
+                param_str = shlex.quote(full_param_str)
+            else:
+                # It's not a JSON object, so it must be multiple distinct arguments.
+                # Raise an error to fall through to the individual quoting logic.
+                raise ValueError
+        except (json.JSONDecodeError, ValueError):
+            # FAILURE: The params are not one single JSON object.
+            # This handles the `rpc_test` case with mixed arguments.
+            # Quote each parameter individually to preserve them as separate arguments.
+            param_str = " ".join(shlex.quote(p) for p in params)
+
+        cmd = f"kubectl -n {namespace} exec {tank} --container {BITCOINCORE_CONTAINER} -- bitcoin-cli {method} {param_str}"
     else:
+        # Handle commands with no parameters
         cmd = f"kubectl -n {namespace} exec {tank} --container {BITCOINCORE_CONTAINER} -- bitcoin-cli {method}"
+
     return run_command(cmd)
 
 

test/signet_test.py

@@ -34,7 +34,7 @@ def setup_network(self):
     def check_signet_miner(self):
         self.warnet("bitcoin rpc miner createwallet miner")
         self.warnet(
-            f"bitcoin rpc miner importdescriptors '{json.dumps(self.signer_data['descriptors'])}'"
+            f"bitcoin rpc miner importdescriptors {json.dumps(self.signer_data['descriptors'])}"
         )
         self.warnet(
             f"run resources/scenarios/signet_miner.py --tank=0 generate --max-blocks=8 --min-nbits --address={self.signer_data['address']['address']}"

test/test_bitcoin_rpc_args.py

@@ -0,0 +1,144 @@
+import shlex
+import sys
+from pathlib import Path
+from unittest.mock import patch
+
+# Import TestBase for consistent test structure
+from test_base import TestBase
+
+# Import _rpc from warnet.bitcoin and run_command from warnet.process
+sys.path.insert(0, str(Path(__file__).parent.parent / "src"))
+from warnet.bitcoin import _rpc
+
+# Edge cases to test
+EDGE_CASES = [
+    # (params, expected_cmd_suffix, should_fail)
+    (
+        ['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]'],
+        ["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]'],
+        False,
+    ),
+    (
+        ['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1"],
+        ["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1"],
+        False,
+    ),
+    (
+        ['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "economical"],
+        ["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "economical"],
+        False,
+    ),
+    (
+        ['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "'economical'"],
+        ["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "'economical'"],
+        False,
+    ),
+    (
+        ['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", '"economical"'],
+        ["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", '"economical"'],
+        False,
+    ),
+    (
+        ['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco nomical"],
+        ["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco nomical"],
+        False,
+    ),
+    (
+        ['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco'nomical"],
+        ["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco'nomical"],
+        False,
+    ),
+    (
+        ['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", 'eco"nomical'],
+        ["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", 'eco"nomical'],
+        False,
+    ),
+    (
+        ['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco$nomical"],
+        ["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco$nomical"],
+        False,
+    ),
+    (
+        ['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco;nomical"],
+        ["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco;nomical"],
+        False,
+    ),
+    (
+        ['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco|nomical"],
+        ["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco|nomical"],
+        False,
+    ),
+    # Malformed JSON (should fail gracefully)
+    (
+        [
+            '[{"desc":"wpkh(tprv8ZgxMBicQKsPfH87iaMtrpzTkWiyFDW7SVWqfsKAhtyEBEqMV6ctPdtc5pNrb2FpSmPcDe8NrxEouUnWj1ud7LT1X1hB1XHKAgB2Z5Z4u2s/84h/1h/0h/0/*)#5j6mshps","timestamp":0,"active":true,"internal":false,"range":[0,999],"next":0,"next_index":0}'
+        ],  # Missing closing bracket
+        [
+            "importdescriptors",
+            '[{"desc":"wpkh(tprv8ZgxMBicQKsPfH87iaMtrpzTkWiyFDW7SVWqfsKAhtyEBEqMV6ctPdtc5pNrb2FpSmPcDe8NrxEouUnWj1ud7LT1X1hB1XHKAgB2Z5Z4u2s/84h/1h/0h/0/*)#5j6mshps","timestamp":0,"active":true,"internal":false,"range":[0,999],"next":0,"next_index":0}',
+        ],
+        True,  # Should fail due to malformed JSON
+    ),
+    # Unicode in descriptors
+    (
+        [
+            '[{"desc":"wpkh(tprv8ZgxMBicQKsPfH87iaMtrpzTkWiyFDW7SVWqfsKAhtyEBEqMV6ctPdtc5pNrb2FpSmPcDe8NrxEouUnWj1ud7LT1X1hB1XHKAgB2Z5Z4u2s/84h/1h/0h/0/*)#5j6mshps","timestamp":0,"active":true,"internal":false,"range":[0,999],"next":0,"next_index":0,"label":"测试"}'
+        ],
+        [
+            "importdescriptors",
+            '[{"desc":"wpkh(tprv8ZgxMBicQKsPfH87iaMtrpzTkWiyFDW7SVWqfsKAhtyEBEqMV6ctPdtc5pNrb2FpSmPcDe8NrxEouUnWj1ud7LT1X1hB1XHKAgB2Z5Z4u2s/84h/1h/0h/0/*)#5j6mshps","timestamp":0,"active":true,"internal":false,"range":[0,999],"next":0,"next_index":0,"label":"测试"}',
+        ],
+        False,
+    ),
+    # Long descriptor (simulate, should not crash, may fail)
+    (
+        [
+            "[{'desc':'wpkh([d34db33f/84h/0h/0h/0/0]xpub6CUGRUonZSQ4TWtTMmzXdrXDtypWKiKp...','range':[0,1000]}]"
+        ],
+        [
+            "send",
+            "[{'desc':'wpkh([d34db33f/84h/0h/0h/0/0]xpub6CUGRUonZSQ4TWtTMmzXdrXDtypWKiKp...','range':[0,1000]}]",
+        ],
+        False,  # Updated to False since it now works correctly
+    ),
+    # Empty params
+    ([], ["send"], False),
+]
+
+
+class BitcoinRPCRPCArgsTest(TestBase):
+    def __init__(self):
+        super().__init__()
+        self.tank = "tank-0027"
+        self.namespace = "default"
+        self.captured_cmds = []
+
+    def run_test(self):
+        self.log.info("Testing bitcoin _rpc argument handling edge cases")
+        for params, expected_suffix, should_fail in EDGE_CASES:
+            # Extract the method from the expected suffix
+            method = expected_suffix[0]
+
+            with patch("warnet.bitcoin.run_command") as mock_run_command:
+                mock_run_command.return_value = "MOCKED"
+                try:
+                    _rpc(self.tank, method, params, self.namespace)
+                    called_args = mock_run_command.call_args[0][0]
+                    self.captured_cmds.append(called_args)
+                    # Parse the command string into arguments for comparison
+                    parsed_args = shlex.split(called_args)
+                    assert parsed_args[-len(expected_suffix) :] == expected_suffix, (
+                        f"Params: {params} | Got: {parsed_args[-len(expected_suffix) :]} | Expected: {expected_suffix}"
+                    )
+                    if should_fail:
+                        self.log.info(f"Expected failure for params: {params}, but succeeded.")
+                except Exception as e:
+                    if not should_fail:
+                        raise AssertionError(f"Unexpected failure for params: {params}: {e}") from e
+                    self.log.info(f"Expected failure for params: {params}: {e}")
+        self.log.info("All edge case argument tests passed.")
+
+
+if __name__ == "__main__":
+    test = BitcoinRPCRPCArgsTest()
+    test.run_test()