Skip to content

Commit 4679f31

Browse files
b-l-u-eb-l-u-e
committed
fix: improve bitcoin RPC argument handling for JSON and mixed arguments
1 parent b76cb94 commit 4679f31

File tree

3 files changed

+170
-20
lines changed

3 files changed

+170
-20
lines changed

‎src/warnet/bitcoin.py

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import json
12
import os
23
import re
34
import shlex
@@ -39,29 +40,34 @@ def rpc(tank: str, method: str, params: list[str], namespace: Optional[str]):
3940

4041

4142
def _rpc(tank: str, method: str, params: list[str], namespace: Optional[str] = None):
42-
# bitcoin-cli should be able to read bitcoin.conf inside the container
43-
# so no extra args like port, chain, username or password are needed
4443
namespace = get_default_namespace_or(namespace)
4544

4645
if params:
47-
# Check if this looks like a JSON argument (starts with [ or {)
48-
param_str = " ".join(params)
49-
if param_str.strip().startswith("[") or param_str.strip().startswith("{"):
50-
# For JSON arguments, ensure it's passed as a single argument
51-
# Remove any extra quotes that might have been added by the shell
52-
param_str = param_str.strip()
53-
if (
54-
param_str.startswith("'")
55-
and param_str.endswith("'")
56-
or param_str.startswith('"')
57-
and param_str.endswith('"')
58-
):
59-
param_str = param_str[1:-1]
60-
cmd = f"kubectl -n {namespace} exec {tank} --container {BITCOINCORE_CONTAINER} -- bitcoin-cli {method} {shlex.quote(param_str)}"
61-
else:
62-
# For non-JSON arguments, use simple space joining
63-
cmd = f"kubectl -n {namespace} exec {tank} --container {BITCOINCORE_CONTAINER} -- bitcoin-cli {method} {param_str}"
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}"
6469
else:
70+
# Handle commands with no parameters
6571
cmd = f"kubectl -n {namespace} exec {tank} --container {BITCOINCORE_CONTAINER} -- bitcoin-cli {method}"
6672

6773
return run_command(cmd)

‎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']}"

‎test/test_bitcoin_rpc_args.py

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

0 commit comments

Comments
 (0)