Skip to content

Commit 98d5f97

Browse files
b-l-u-epinheadmz
b-l-u-e
and
pinheadmz
authored
Fix JSON parameter parsing in warnet bitcoin rpc command (#725)
* Fix JSON parameter parsing in warnet bitcoin rpc command - Add _reconstruct_json_params() function to handle JSON parameters split by shell parsing - Update _rpc() to properly process JSON arrays and primitive values for bitcoin-cli - Fix issue where shell parsing would break JSON parameters into separate arguments - Handle edge cases like unquoted string arrays [network] -> [network] - Maintain backward compatibility with non-JSON parameters This fixes the issue where commands like: warnet bitcoin rpc tank-0000 getnetmsgstats '[network]' would fail due to shell parsing breaking the JSON array. Fixes #714 * fix: robust JSON parameter handling for warnet bitcoin rpc - Accepts JSON params with or without backslash-escaped quotes (e.g. '[http]' and '["http"]') - Wraps JSON params in single quotes for correct shell and bitcoin-cli parsing * fix: handle JSON array parameters intelligently for different RPC methods - Fix getblock '[blockhash]' issue by extracting first element from JSON arrays - Maintain backward compatibility for plain string parameters - Support JSON arrays as-is for methods like logging and importdescriptors - Extract only first element for single-parameter methods (getblock, getblockhash, gettransaction) - Extract all elements for multi-parameter methods - Fix malformed JSON patterns with escaped quotes * Simplify bitcoin RPC parameter handling with pure passthrough approach * fix: resolve JSON parsing errors in bitcoin rpc command Use click.UNPROCESSED to prevent JSON splitting, enabling proper descriptor import and fixing signet test timeout. * fix: improve bitcoin RPC argument handling for JSON and mixed arguments * run bitcoin args test --------- Co-authored-by: Matthew Zipkin <[email protected]>
1 parent db5f124 commit 98d5f97

File tree

4 files changed

+180
-7
lines changed

4 files changed

+180
-7
lines changed

‎.github/workflows/test.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ jobs:
3636
strategy:
3737
matrix:
3838
test:
39+
- bitcoin_rpc_args_test.py
3940
- conf_test.py
4041
- dag_connection_test.py
4142
- graph_test.py

‎src/warnet/bitcoin.py

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
import json
12
import os
23
import re
4+
import shlex
35
import sys
46
from datetime import datetime
57
from io import BytesIO
@@ -23,9 +25,9 @@ def bitcoin():
2325
@bitcoin.command(context_settings={"ignore_unknown_options": True})
2426
@click.argument("tank", type=str)
2527
@click.argument("method", type=str)
26-
@click.argument("params", type=str, nargs=-1) # this will capture all remaining arguments
28+
@click.argument("params", type=click.UNPROCESSED, nargs=-1) # get raw unprocessed arguments
2729
@click.option("--namespace", default=None, show_default=True)
28-
def rpc(tank: str, method: str, params: str, namespace: Optional[str]):
30+
def rpc(tank: str, method: str, params: list[str], namespace: Optional[str]):
2931
"""
3032
Call bitcoin-cli <method> [params] on <tank pod name>
3133
"""
@@ -37,14 +39,37 @@ def rpc(tank: str, method: str, params: str, namespace: Optional[str]):
3739
print(result)
3840

3941

40-
def _rpc(tank: str, method: str, params: str, namespace: Optional[str] = None):
41-
# bitcoin-cli should be able to read bitcoin.conf inside the container
42-
# so no extra args like port, chain, username or password are needed
42+
def _rpc(tank: str, method: str, params: list[str], namespace: Optional[str] = None):
4343
namespace = get_default_namespace_or(namespace)
44+
4445
if params:
45-
cmd = f"kubectl -n {namespace} exec {tank} --container {BITCOINCORE_CONTAINER} -- bitcoin-cli {method} {' '.join(map(str, params))}"
46+
# First, try to join all parameters into a single string.
47+
full_param_str = " ".join(params)
48+
49+
try:
50+
# Heuristic: if the string looks like a JSON object/array, try to parse it.
51+
# This handles the `signet_test` case where one large JSON argument was split
52+
# by the shell into multiple params.
53+
if full_param_str.strip().startswith(("[", "{")):
54+
json.loads(full_param_str)
55+
# SUCCESS: The params form a single, valid JSON object.
56+
# Quote the entire reconstructed string as one argument.
57+
param_str = shlex.quote(full_param_str)
58+
else:
59+
# It's not a JSON object, so it must be multiple distinct arguments.
60+
# Raise an error to fall through to the individual quoting logic.
61+
raise ValueError
62+
except (json.JSONDecodeError, ValueError):
63+
# FAILURE: The params are not one single JSON object.
64+
# This handles the `rpc_test` case with mixed arguments.
65+
# Quote each parameter individually to preserve them as separate arguments.
66+
param_str = " ".join(shlex.quote(p) for p in params)
67+
68+
cmd = f"kubectl -n {namespace} exec {tank} --container {BITCOINCORE_CONTAINER} -- bitcoin-cli {method} {param_str}"
4669
else:
70+
# Handle commands with no parameters
4771
cmd = f"kubectl -n {namespace} exec {tank} --container {BITCOINCORE_CONTAINER} -- bitcoin-cli {method}"
72+
4873
return run_command(cmd)
4974

5075

‎test/bitcoin_rpc_args_test.py

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
#!/usr/bin/env python3
2+
3+
import shlex
4+
import sys
5+
from pathlib import Path
6+
from unittest.mock import patch
7+
8+
# Import TestBase for consistent test structure
9+
from test_base import TestBase
10+
11+
from warnet.bitcoin import _rpc
12+
13+
# Import _rpc from warnet.bitcoin and run_command from warnet.process
14+
sys.path.insert(0, str(Path(__file__).parent.parent / "src"))
15+
16+
# Edge cases to test
17+
EDGE_CASES = [
18+
# (params, expected_cmd_suffix, should_fail)
19+
(
20+
['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]'],
21+
["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]'],
22+
False,
23+
),
24+
(
25+
['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1"],
26+
["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1"],
27+
False,
28+
),
29+
(
30+
['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "economical"],
31+
["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "economical"],
32+
False,
33+
),
34+
(
35+
['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "'economical'"],
36+
["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "'economical'"],
37+
False,
38+
),
39+
(
40+
['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", '"economical"'],
41+
["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", '"economical"'],
42+
False,
43+
),
44+
(
45+
['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco nomical"],
46+
["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco nomical"],
47+
False,
48+
),
49+
(
50+
['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco'nomical"],
51+
["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco'nomical"],
52+
False,
53+
),
54+
(
55+
['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", 'eco"nomical'],
56+
["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", 'eco"nomical'],
57+
False,
58+
),
59+
(
60+
['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco$nomical"],
61+
["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco$nomical"],
62+
False,
63+
),
64+
(
65+
['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco;nomical"],
66+
["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco;nomical"],
67+
False,
68+
),
69+
(
70+
['[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco|nomical"],
71+
["send", '[{"bcrt1qsrsmr7f77kcxggk99yp2h8yjzv29lxhet4efwn":0.1}]', "1", "eco|nomical"],
72+
False,
73+
),
74+
# Malformed JSON (should fail gracefully)
75+
(
76+
[
77+
'[{"desc":"wpkh(tprv8ZgxMBicQKsPfH87iaMtrpzTkWiyFDW7SVWqfsKAhtyEBEqMV6ctPdtc5pNrb2FpSmPcDe8NrxEouUnWj1ud7LT1X1hB1XHKAgB2Z5Z4u2s/84h/1h/0h/0/*)#5j6mshps","timestamp":0,"active":true,"internal":false,"range":[0,999],"next":0,"next_index":0}'
78+
], # Missing closing bracket
79+
[
80+
"importdescriptors",
81+
'[{"desc":"wpkh(tprv8ZgxMBicQKsPfH87iaMtrpzTkWiyFDW7SVWqfsKAhtyEBEqMV6ctPdtc5pNrb2FpSmPcDe8NrxEouUnWj1ud7LT1X1hB1XHKAgB2Z5Z4u2s/84h/1h/0h/0/*)#5j6mshps","timestamp":0,"active":true,"internal":false,"range":[0,999],"next":0,"next_index":0}',
82+
],
83+
True, # Should fail due to malformed JSON
84+
),
85+
# Unicode in descriptors
86+
(
87+
[
88+
'[{"desc":"wpkh(tprv8ZgxMBicQKsPfH87iaMtrpzTkWiyFDW7SVWqfsKAhtyEBEqMV6ctPdtc5pNrb2FpSmPcDe8NrxEouUnWj1ud7LT1X1hB1XHKAgB2Z5Z4u2s/84h/1h/0h/0/*)#5j6mshps","timestamp":0,"active":true,"internal":false,"range":[0,999],"next":0,"next_index":0,"label":"测试"}'
89+
],
90+
[
91+
"importdescriptors",
92+
'[{"desc":"wpkh(tprv8ZgxMBicQKsPfH87iaMtrpzTkWiyFDW7SVWqfsKAhtyEBEqMV6ctPdtc5pNrb2FpSmPcDe8NrxEouUnWj1ud7LT1X1hB1XHKAgB2Z5Z4u2s/84h/1h/0h/0/*)#5j6mshps","timestamp":0,"active":true,"internal":false,"range":[0,999],"next":0,"next_index":0,"label":"测试"}',
93+
],
94+
False,
95+
),
96+
# Long descriptor (simulate, should not crash, may fail)
97+
(
98+
[
99+
"[{'desc':'wpkh([d34db33f/84h/0h/0h/0/0]xpub6CUGRUonZSQ4TWtTMmzXdrXDtypWKiKp...','range':[0,1000]}]"
100+
],
101+
[
102+
"send",
103+
"[{'desc':'wpkh([d34db33f/84h/0h/0h/0/0]xpub6CUGRUonZSQ4TWtTMmzXdrXDtypWKiKp...','range':[0,1000]}]",
104+
],
105+
False, # Updated to False since it now works correctly
106+
),
107+
# Empty params
108+
([], ["send"], False),
109+
]
110+
111+
112+
class BitcoinRPCRPCArgsTest(TestBase):
113+
def __init__(self):
114+
super().__init__()
115+
self.tank = "tank-0027"
116+
self.namespace = "default"
117+
self.captured_cmds = []
118+
119+
def run_test(self):
120+
self.log.info("Testing bitcoin _rpc argument handling edge cases")
121+
for params, expected_suffix, should_fail in EDGE_CASES:
122+
# Extract the method from the expected suffix
123+
method = expected_suffix[0]
124+
125+
with patch("warnet.bitcoin.run_command") as mock_run_command:
126+
mock_run_command.return_value = "MOCKED"
127+
try:
128+
_rpc(self.tank, method, params, self.namespace)
129+
called_args = mock_run_command.call_args[0][0]
130+
self.captured_cmds.append(called_args)
131+
# Parse the command string into arguments for comparison
132+
parsed_args = shlex.split(called_args)
133+
assert parsed_args[-len(expected_suffix) :] == expected_suffix, (
134+
f"Params: {params} | Got: {parsed_args[-len(expected_suffix) :]} | Expected: {expected_suffix}"
135+
)
136+
if should_fail:
137+
self.log.info(f"Expected failure for params: {params}, but succeeded.")
138+
except Exception as e:
139+
if not should_fail:
140+
raise AssertionError(f"Unexpected failure for params: {params}: {e}") from e
141+
self.log.info(f"Expected failure for params: {params}: {e}")
142+
self.log.info("All edge case argument tests passed.")
143+
144+
145+
if __name__ == "__main__":
146+
test = BitcoinRPCRPCArgsTest()
147+
test.run_test()

‎test/signet_test.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ def setup_network(self):
3434
def check_signet_miner(self):
3535
self.warnet("bitcoin rpc miner createwallet miner")
3636
self.warnet(
37-
f"bitcoin rpc miner importdescriptors '{json.dumps(self.signer_data['descriptors'])}'"
37+
f"bitcoin rpc miner importdescriptors {json.dumps(self.signer_data['descriptors'])}"
3838
)
3939
self.warnet(
4040
f"run resources/scenarios/signet_miner.py --tank=0 generate --max-blocks=8 --min-nbits --address={self.signer_data['address']['address']}"

0 commit comments

Comments
 (0)