rpc: Handle -named argument parsing where '=' character is used

zaidmstrr commented at 11:43 am on June 27, 2025: contributor

The [PR #31375](https://github.com/bitcoin/bitcoin/pull/31375) got merged and enables -named by default in the bitcoin rpc interface; bitcoin rpc corresponds to bitcoin-cli -named as it’s just a wrapper. Now, the problem arises when we try to parse the positional paramater which might contain “=” character. This splits the parameter into two parts first, before the “=” character, which treats this as the parameter name, but the other half is mostly passed as an empty string. Here, the first part of the string is an unknown parameter name; thus, an error is thrown. These types of errors are only applicable to those RPCs which might contain the = character as a parameter. Some examples are finalizepsbt, decodepsbt, verifymessage etc.

This is the one example of the error in finalizepsbt RPC:

0./bitcoin-cli -named -regtest finalizepsbt cHNidP8BAJoCAAAAAqvNEjSrzRI0q80SNKvNEjSrzRI0q80SNKvNEjSrzRI0AAAAAAD9////NBLNqzQSzas0Es2rNBLNqzQSzas0Es2rNBLNqzQSzasBAAAAAP3///8CoIYBAAAAAAAWABQVQBGVs/sqFAmC8HZ8O+g1htqivkANAwAAAAAAFgAUir7MzgyzDnRMjdkVa7d+Dwr07jsAAAAAAAAAAAA=
1error code: -8
2error message:
3Unknown named parameter cHNidP8BAJoCAAAAAqvNEjSrzRI0q80SNKvNEjSrzRI0q80SNKvNEjSrzRI0AAAAAAD9////NBLNqzQSzas0Es2rNBLNqzQSzas0Es2rNBLNqzQSzasBAAAAAP3///8CoIYBAAAAAAAWABQVQBGVs/sqFAmC8HZ8O+g1htqivkANAwAAAAAAFgAUir7MzgyzDnRMjdkVa7d+Dwr07jsAAAAAAAAAAAA

This PR fixes this by adding a vRPCStringParams table that identifies parameters that need special handling in -named parameter mode. The parser now recognises these parameters and handles strings with “=” char correctly, preventing them from being incorrectly split as parameter assignments.

DrahtBot added the label RPC/REST/ZMQ on Jun 27, 2025

DrahtBot commented at 11:43 am on June 27, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32821.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	ryanofsky, yuvicc
Stale ACK	BrandonOdiwuor

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32784 (wallet: derivehdkey RPC to get xpub at arbitrary path by Sjors)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

in src/rpc/client.cpp:319 in 14e2125ee0 outdated

310@@ -311,6 +311,26 @@ static const CRPCConvertParam vRPCConvertParams[] =
311     { "addnode", 2, "v2transport" },
312     { "addconnection", 2, "v2transport" },
313 };
314+
315+/**
316+ * Specify a (method, idx, name) here if the argument is a string RPC
317+ * parameter that should be recognized as a known parameter name but NOT
318+ * converted from JSON. This is primarily used for Base64-encoded parameters
319+ * that might contain '=' padding characters which could interfere with

maflcko commented at 11:58 am on June 27, 2025:

not sure about limiting this to base64. There are many other args that can hold values with an = in them. For example, labels.

zaidmstrr commented at 12:18 pm on June 27, 2025:

I think you are right; I will generalise it to any string args which might contain =

Sjors commented at 12:02 pm on June 27, 2025: member

I ran into this issue in #32784 while editing the multisig tutorial. 14e2125ee0297f0cb046ca91b6ead20052f24678 fixes it.

ryanofsky commented at 12:20 pm on June 27, 2025: contributor

Concept ACK 14e2125ee0297f0cb046ca91b6ead20052f24678. Thanks for the fix!

If possible it would be good to add python tests to check the new behavior and I also agree with comment that this change doesn’t need to be limited to base64 parameters and could probably be extended to other string parameters. But maybe the “If parameter is unknown for a given known string method, then treat the whole string as positional” behavior might have to be dropped in that case.

I think this fix is little different from my suggestion in #31375 (review) because I was thinking of continuing to treat unknown parameters as named not positional, but maybe this approach is better. Would have to think about it. The implementation is also a little different than what I had in mind because I was thinking of extending the vRPCConvertParams table with type information instead of adding a separate table, but your approach might be simpler.

zaidmstrr commented at 2:09 pm on June 27, 2025: contributor

If possible it would be good to add python tests to check the new behavior

Sure, I’m also thinking of adding a python test, but the main problem is, this issue is only reproducible when there exists = padding or in between, so while creating a PSBT, I need to look at how I can achieve this. Or maybe by calling RPCs with a label.

I also agree with #32821 (review) that this change doesn’t need to be limited to base64 parameters and could probably be extended to other string parameters. But maybe the “If parameter is unknown for a given known string method, then treat the whole string as positional” behavior might have to be dropped in that case.

To extend this feature to any string parameters, we simply need to add an entry in the vRPCStringParams table. I tested with getnewaddress which requires a label (optionally) and it’s working fine. Without adding the entry in vRPCStringParams it doesn’t work if we provide a label that consists of the = char in it.

And the line you mentioned to remove is problematic if we remove it, as it’s only got invoked if the RPC method is correct and matches with our table. And if the parameter name is not known then it’s invoked; otherwise not. This is safe to do because we already make sure that the calling method is present in the table. Otherwise, if the user passed the correct method name but an incorrect parameter, then the targeted RPC will fail with an error.

Sjors commented at 2:28 pm on June 27, 2025: member

If possible it would be good to add python tests to check the new behavior

Sure, I’m also thinking of adding a python test, but the main problem is, this issue is only reproducible when there exists = padding or in between, so while creating a PSBT, I need to look at how I can achieve this.

You could just manually generate a random PSBT on regtest, hardcode it into a test and call analyzepsbt. It doesn’t matter if it’s unspendable.

Or you could even just call these methods with abc=efg since it doesn’t even need to be a validly encoded psbt.

zaidmstrr force-pushed on Jun 28, 2025

zaidmstrr commented at 6:59 pm on June 28, 2025: contributor

I generalised this change to any string parameter which might contain the = char in it. I also added the functional test for most of the RPCs I included in the new table. For any new RPC method which might need to be parsed this way with -named enabled, one can add the new entry of that RPC in the vRPCStringParams table.

zaidmstrr renamed this:
~~rpc: Handle -named argument parsing where Base64 encoding is used~~
rpc: Handle -named argument parsing where '=' character is used
on Jun 29, 2025

yuvicc commented at 2:24 pm on June 29, 2025: contributor

Concept ACK

0./build/bin/bitcoin-cli -named  finalizepsbt cHNidP8BAJoCAAAAAqvNEjSrzRI0q80SNKvNEjSrzRI0q80SNKvNEjSrzRI0AAAAAAD9////NBLNqzQSzas0Es2rNBLNqzQSzas0Es2rNBLNqzQSzasBAAAAAP3///8CoIYBAAAAAAAWABQVQBGVs/sqFAmC8HZ8O=g1htqivkANAwAAAAAAFgAUir7MzgyzDnRMjdkVa7d+Dwr07jsAAAAAAAAAAAA=
1
2error code: -8
3error message:
4Unknown named parameter cHNidP8BAJoCAAAAAqvNEjSrzRI0q80SNKvNEjSrzRI0q80SNKvNEjSrzRI0AAAAAAD9////NBLNqzQSzas0Es2rNBLNqzQSzas0Es2rNBLNqzQSzasBAAAAAP3///8CoIYBAAAAAAAWABQVQBGVs/sqFAmC8HZ8O

Commit 59fcefcd1f490f8ae276150421cbd895a10f3d4a

0./build/bin//bitcoin-cli -named  finalizepsbt cHNidP8BAJoCAAAAAqvNEjSrzRI0q80SNKvNEjSrzRI0q80SNKvNEjSrzRI0AAAAAAD9////NBLNqzQSzas0Es2rNBLNqzQSzas0Es2rNBLNqzQSzasBAAAAAP3///8CoIYBAAAAAAAWABQVQBGVs/sqFAmC8HZ8O+g1htqivkANAwAAAAAAFgAUir7MzgyzDnRMjdkVa7d+Dwr07jsAAAAAAAAAAAA=
1{
2  "psbt": "cHNidP8BAJoCAAAAAqvNEjSrzRI0q80SNKvNEjSrzRI0q80SNKvNEjSrzRI0AAAAAAD9////NBLNqzQSzas0Es2rNBLNqzQSzas0Es2rNBLNqzQSzasBAAAAAP3///8CoIYBAAAAAAAWABQVQBGVs/sqFAmC8HZ8O+g1htqivkANAwAAAAAAFgAUir7MzgyzDnRMjdkVa7d+Dwr07jsAAAAAAAAAAAA=",
3  "complete": false
4}

Will review the code soon!

zaidmstrr requested review from maflcko on Jun 29, 2025

BrandonOdiwuor commented at 5:07 pm on June 29, 2025: contributor

Tested ACK e246741db1f2f056fac32d0f3967f55306e78b49

Confirmed that commit 59fcefcd1f490f8ae276150421cbd895a10f3d4a correctly handles named argument parsing for parameters containing = characters—an issue currently reproducible on master.

<commit: 59fcefcd1f490f8ae276150421cbd895a10f3d4a>

<branch: master>

DrahtBot requested review from ryanofsky on Jun 29, 2025

in src/rpc/client.cpp:362 in e246741db1 outdated

329+    { "walletprocesspsbt", 0, "psbt"},
330+    { "descriptorprocesspsbt", 0, "psbt"},
331+    { "utxoupdatepsbt", 0, "psbt"},
332+    { "verifymessage", 1, "signature"},
333+    { "getnewaddress", 0, "label"},
334+};

maflcko commented at 6:34 am on June 30, 2025:

this list is incomplete. Also, I wonder how this should be maintained. There needs to be an automated check that the list is up-to-date, like for the other list.

zaidmstrr commented at 11:37 am on June 30, 2025:

Can you please tell me about the automated check you are referring to? I saw a test in test/functional/rpc_help.py which does similar things.

maflcko commented at 12:59 pm on June 30, 2025:

Can you please tell me about the automated check you are referring to? I saw a test in test/functional/rpc_help.py which does similar things.

Yes, that is the one I was referring to.

maflcko commented at 1:43 pm on July 3, 2025:

Also, it doesn’t seem to be working right now anyway, see the CI failure (needs rebase)

zaidmstrr commented at 9:08 pm on July 3, 2025:

Rebased and fixed the previous CI error. But now CI is showing a different error I’m investigating for this one.

DrahtBot added the label CI failed on Jul 3, 2025

rpc: Handle -named argument parsing where Base64 encoding is used e98666c522

zaidmstrr force-pushed on Jul 3, 2025

test: Add functional tests for named argument parsing 03c1f0d604

zaidmstrr force-pushed on Jul 4, 2025

Sjors commented at 6:35 am on July 4, 2025: member

From CI:

0assert not "non_witness_utxo" in mining_node.decodepsbt(signed_psbt["psbt"])["inputs"][0]

I didn’t check the code, but that’s usually a cryptic way of saying that signing the psbt failed.

zaidmstrr commented at 9:38 am on July 4, 2025: contributor

Signing the PSBT works fine while running locally, and removing non-witness UTXO from the PSBT.

maflcko commented at 10:34 am on July 4, 2025: member

Signing the PSBT works fine while running locally, and removing non-witness UTXO from the PSBT.

What command did you use to run locally?

zaidmstrr commented at 11:07 am on July 4, 2025: contributor

What command did you use to run locally?

I first checked by adding two log statements in rpc_psbt.py the first log to check the psbt before signing and the second log after walletprocesspsbt where CI is reporting an error. And then manually parsing the result with the help of decodepsbt to check whether the PSBT afterwalletprocesspsbt contains non-witness UTXO, but it doesn’t in my case, so the test passed successfully.

I also ran all the functional test cases with —extended and all tests passed.

maflcko commented at 11:22 am on July 4, 2025: member

you’ll have to use --usecli to run the tests, otherwise the cli is not used

Prabhat1308 commented at 3:40 pm on July 7, 2025: contributor

Trying to debug the issue I see some irregularities in the how the test behaves in rpc mode and cli mode.

 0
 1diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py
 2index a241978b5f..f92e8d6e88 100755
 3--- a/test/functional/rpc_psbt.py
 4+++ b/test/functional/rpc_psbt.py
 5@@ -135,8 +135,64 @@ class PSBTTest(BitcoinTestFramework):
 6         assert "non_witness_utxo" in mining_node.decodepsbt(psbt_new.to_base64())["inputs"][0]
 7
 8         # Have the offline node sign the PSBT (which will remove the non-witness UTXO)
 9+
10+        decoded_before = mining_node.decodepsbt(psbt_new.to_base64())
11+
12+        self.log.info(f"=== DEBUG: CLI Mode = {self.options.usecli} ===")
13+
14+        # Log address and input details
15+        offline_addr_info = offline_node.getaddressinfo(offline_addr)
16+        self.log.info(f"Offline address: {offline_addr}")
17+        self.log.info(f"Address type: {offline_addr_info.get('type', 'unknown')}")
18+
19+        decoded_before = mining_node.decodepsbt(psbt_new.to_base64())
20+
21+        self.log.info(f"=== DEBUG: CLI Mode = {self.options.usecli} ===")
22+
23+        # Log address and input details
24+        offline_addr_info = offline_node.getaddressinfo(offline_addr)
25+        self.log.info(f"Offline address: {offline_addr}")
26+        self.log.info(f"Address type: {offline_addr_info.get('type', 'unknown')}")
27+        self.log.info(f"Script type: {offline_addr_info.get('script', 'unknown')}")
28+        self.log.info(f"Is witness: {offline_addr_info.get('iswitness', False)}")
29+        self.log.info(f"Witness version: {offline_addr_info.get('witness_version', 'none')}")
30+        self.log.info(f"Witness program: {offline_addr_info.get('witness_program', 'none')}")
31+
32+        # Log PSBT input details before signing
33+        self.log.info(f"BEFORE signing - input[0] keys: {list(decoded_before['inputs'][0].keys())}")
34+        self.log.info(f"BEFORE signing - has non_witness_utxo: {'non_witness_utxo' in decoded_before['inputs'][0]}")
35+        self.log.info(f"BEFORE signing - has witness_utxo: {'witness_utxo' in decoded_before['inputs'][0]}")
36+
37+        # Check witness_utxo script details
38+        if 'witness_utxo' in decoded_before['inputs'][0]:
39+            witness_utxo = decoded_before['inputs'][0]['witness_utxo']
40+            self.log.info(f"BEFORE signing - witness_utxo script type: {witness_utxo['scriptPubKey'].get('type', 'unknown')}")
41+            self.log.info(f"BEFORE signing - witness_utxo hex: {witness_utxo['scriptPubKey'].get('hex', 'none')}")
42+
43+        # Log the exact walletprocesspsbt call being made
44+        psbt_b64 = psbt_new.to_base64()
45+        self.log.info(f"walletprocesspsbt call: offline_node.walletprocesspsbt('{psbt_b64[:50]}...', True, 'ALL', True, True)")
46+
47         signed_psbt = offline_node.walletprocesspsbt(psbt_new.to_base64())
48-        assert not "non_witness_utxo" in mining_node.decodepsbt(signed_psbt["psbt"])["inputs"][0]
49+        decoded_after = mining_node.decodepsbt(signed_psbt["psbt"])
50+
51+        # Log PSBT input details after signing
52+        self.log.info(f"AFTER signing - input[0] keys: {list(decoded_after['inputs'][0].keys())}")
53+        self.log.info(f"AFTER signing - has non_witness_utxo: {'non_witness_utxo' in decoded_after['inputs'][0]}")
54+        self.log.info(f"AFTER signing - has witness_utxo: {'witness_utxo' in decoded_after['inputs'][0]}")
55+        self.log.info(f"walletprocesspsbt result complete: {signed_psbt.get('complete', 'not present')}")
56+
57+        # Check for Taproot vs regular segwit indicators
58+        has_taproot_derivs = 'taproot_bip32_derivs' in decoded_before['inputs'][0] or 'taproot_bip32_derivs' in decoded_after['inputs'][0]
59+        has_taproot_key = 'taproot_internal_key' in decoded_before['inputs'][0] or 'taproot_internal_key' in decoded_after['inputs'][0]
60+        has_regular_derivs = 'bip32_derivs' in decoded_before['inputs'][0] or 'bip32_derivs' in decoded_after['inputs'][0]
61+
62+        self.log.info(f"Input type detection - Taproot derivs: {has_taproot_derivs}, Taproot key: {has_taproot_key}, Regular derivs: {has_regular_derivs}")
63+
64+        # Log the actual walletprocesspsbt call details for comparison
65+        if self.options.usecli:
66+            self.log.info("Using CLI mode - parameters passed as separate arguments")
67+        else:
68+            self.log.info("Using RPC mode - parameters passed as JSON")
69+
70+        # More detailed assertion with context
71+        if "non_witness_utxo" in decoded_after["inputs"][0]:
72+            self.log.error("FAILURE: non_witness_utxo should be removed for segwit v1+ inputs!")
73+            self.log.error(f"This suggests the input is being treated as segwit v0 instead of v1+ in CLI mode")
74+
75+

 02025-07-07T15:20:33.852283Z TestFramework (INFO): Check that non-witness UTXOs are removed for segwit v1+ inputs
 12025-07-07T15:20:34.004885Z TestFramework (INFO): === DEBUG: CLI Mode = True ===
 22025-07-07T15:20:34.008380Z TestFramework (INFO): Offline address: bcrt1q2skwujcxhaawtf0hlqawfmk3gclpdz24a84znw
 32025-07-07T15:20:34.008437Z TestFramework (INFO): Address type: unknown
 42025-07-07T15:20:34.008459Z TestFramework (INFO): Script type: unknown
 52025-07-07T15:20:34.008479Z TestFramework (INFO): Is witness: True
 62025-07-07T15:20:34.008497Z TestFramework (INFO): Witness version: 0
 72025-07-07T15:20:34.008513Z TestFramework (INFO): Witness program: 542cee4b06bf7ae5a5f7f83ae4eed1463e168955
 82025-07-07T15:20:34.008532Z TestFramework (INFO): BEFORE signing - input[0] keys: ['witness_utxo', 'non_witness_utxo', 'bip32_derivs']
 92025-07-07T15:20:34.008550Z TestFramework (INFO): BEFORE signing - has non_witness_utxo: True
102025-07-07T15:20:34.008565Z TestFramework (INFO): BEFORE signing - has witness_utxo: True
112025-07-07T15:20:34.008579Z TestFramework (INFO): BEFORE signing - witness_utxo script type: witness_v0_keyhash
122025-07-07T15:20:34.008595Z TestFramework (INFO): BEFORE signing - witness_utxo hex: 0014542cee4b06bf7ae5a5f7f83ae4eed1463e168955
132025-07-07T15:20:34.008688Z TestFramework (INFO): walletprocesspsbt call: offline_node.walletprocesspsbt('cHNidP8BAFICAAAAAQVChcz/DF+d62weFt4XJAYjd8cLM81FZV...', True, 'ALL', True, True)
142025-07-07T15:20:34.016020Z TestFramework (INFO): AFTER signing - input[0] keys: ['witness_utxo', 'non_witness_utxo', 'final_scriptwitness']
152025-07-07T15:20:34.016093Z TestFramework (INFO): AFTER signing - has non_witness_utxo: True
162025-07-07T15:20:34.016117Z TestFramework (INFO): AFTER signing - has witness_utxo: True
172025-07-07T15:20:34.016135Z TestFramework (INFO): walletprocesspsbt result complete: True
182025-07-07T15:20:34.016155Z TestFramework (INFO): Input type detection - Taproot derivs: False, Taproot key: False, Regular derivs: True
192025-07-07T15:20:34.016174Z TestFramework (INFO): Using CLI mode - parameters passed as separate arguments
202025-07-07T15:20:34.016193Z TestFramework (ERROR): FAILURE: non_witness_utxo should be removed for segwit v1+ inputs!
212025-07-07T15:20:34.016209Z TestFramework (ERROR): This suggests the input is being treated as segwit v0 instead of v1+ in CLI mode
222025-07-07T15:20:34.016234Z TestFramework (ERROR): Assertion failed

 0
 12025-07-07T15:19:10.430768Z TestFramework (INFO): === DEBUG: CLI Mode = False ===
 22025-07-07T15:19:10.431566Z TestFramework (INFO): Offline address: bcrt1pxmju835wnuw3z5thftuz84q8rt96vqd97jaxwdn2uqlxwwka923q8fcsp8
 32025-07-07T15:19:10.431613Z TestFramework (INFO): Address type: unknown
 42025-07-07T15:19:10.431645Z TestFramework (INFO): Script type: unknown
 52025-07-07T15:19:10.431672Z TestFramework (INFO): Is witness: True
 62025-07-07T15:19:10.431699Z TestFramework (INFO): Witness version: 1
 72025-07-07T15:19:10.431729Z TestFramework (INFO): Witness program: 36e5c3c68e9f1d1151774af823d4071acba601a5f4ba67366ae03e673add2aa2
 82025-07-07T15:19:10.431760Z TestFramework (INFO): BEFORE signing - input[0] keys: ['witness_utxo', 'non_witness_utxo', 'taproot_bip32_derivs', 'taproot_internal_key']
 92025-07-07T15:19:10.431789Z TestFramework (INFO): BEFORE signing - has non_witness_utxo: True
102025-07-07T15:19:10.431831Z TestFramework (INFO): BEFORE signing - has witness_utxo: True
112025-07-07T15:19:10.431857Z TestFramework (INFO): BEFORE signing - witness_utxo script type: witness_v1_taproot
122025-07-07T15:19:10.431881Z TestFramework (INFO): BEFORE signing - witness_utxo hex: 512036e5c3c68e9f1d1151774af823d4071acba601a5f4ba67366ae03e673add2aa2
132025-07-07T15:19:10.431929Z TestFramework (INFO): walletprocesspsbt call: offline_node.walletprocesspsbt('cHNidP8BAF4CAAAAAVteg2JVDVvQHLSH4zmmgu7l4bYJdyIoFq...', True, 'ALL', True, True)
142025-07-07T15:19:10.433261Z TestFramework (INFO): AFTER signing - input[0] keys: ['witness_utxo', 'final_scriptwitness']
152025-07-07T15:19:10.433305Z TestFramework (INFO): AFTER signing - has non_witness_utxo: False
162025-07-07T15:19:10.433336Z TestFramework (INFO): AFTER signing - has witness_utxo: True
172025-07-07T15:19:10.433364Z TestFramework (INFO): walletprocesspsbt result complete: True
182025-07-07T15:19:10.433399Z TestFramework (INFO): Input type detection - Taproot derivs: True, Taproot key: True, Regular derivs: False
192025-07-07T15:19:10.433426Z TestFramework (INFO): Using RPC mode - parameters passed as JSON

inputs are being treated differently in the cli and non-cli mode
even the address being generated in rpc mode is different (larger as compared to cli mode)
non-witness utxo remains in cli mode.

rpc: Handle -named argument parsing where ‘=’ character is used #32821

Code Coverage & Benchmarks

Reviews

Conflicts