-
Notifications
You must be signed in to change notification settings - Fork 38.7k
rpc: Handle -named argument parsing where '=' character is used #32821
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32821. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
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. LLM Linter (✨ experimental)Possible typos and grammar issues:
drahtbot_id_5_m |
|
I ran into this issue in #32784 while editing the multisig tutorial. 14e2125ee0297f0cb046ca91b6ead20052f24678 fixes it. |
|
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 (comment) 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. |
Sure, I'm also thinking of adding a python test, but the main problem is, this issue is only reproducible when there exists
To extend this feature to any string parameters, we simply need to add an entry in the 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. |
You could just manually generate a random PSBT on regtest, hardcode it into a test and call Or you could even just call these methods with |
|
I generalised this change to any string parameter which might contain the |
|
Concept ACK Commit 59fcefcd1f490f8ae276150421cbd895a10f3d4a Will review the code soon! |
BrandonOdiwuor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
537c120 to
03c1f0d
Compare
|
From CI: I didn't check the code, but that's usually a cryptic way of saying that signing the psbt failed. |
|
Signing the PSBT works fine while running locally, and removing non-witness UTXO from the PSBT. |
What command did you use to run locally? |
I first checked by adding two log statements in I also ran all the functional test cases with |
|
you'll have to use |
|
Trying to debug the issue I see some irregularities in the how the test behaves in diffdiff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py
index a241978b5f..f92e8d6e88 100755
--- a/test/functional/rpc_psbt.py
+++ b/test/functional/rpc_psbt.py
@@ -135,8 +135,64 @@ class PSBTTest(BitcoinTestFramework):
assert "non_witness_utxo" in mining_node.decodepsbt(psbt_new.to_base64())["inputs"][0]
# Have the offline node sign the PSBT (which will remove the non-witness UTXO)
+
+ decoded_before = mining_node.decodepsbt(psbt_new.to_base64())
+
+ self.log.info(f"=== DEBUG: CLI Mode = {self.options.usecli} ===")
+
+ # Log address and input details
+ offline_addr_info = offline_node.getaddressinfo(offline_addr)
+ self.log.info(f"Offline address: {offline_addr}")
+ self.log.info(f"Address type: {offline_addr_info.get('type', 'unknown')}")
+
+ decoded_before = mining_node.decodepsbt(psbt_new.to_base64())
+
+ self.log.info(f"=== DEBUG: CLI Mode = {self.options.usecli} ===")
+
+ # Log address and input details
+ offline_addr_info = offline_node.getaddressinfo(offline_addr)
+ self.log.info(f"Offline address: {offline_addr}")
+ self.log.info(f"Address type: {offline_addr_info.get('type', 'unknown')}")
+ self.log.info(f"Script type: {offline_addr_info.get('script', 'unknown')}")
+ self.log.info(f"Is witness: {offline_addr_info.get('iswitness', False)}")
+ self.log.info(f"Witness version: {offline_addr_info.get('witness_version', 'none')}")
+ self.log.info(f"Witness program: {offline_addr_info.get('witness_program', 'none')}")
+
+ # Log PSBT input details before signing
+ self.log.info(f"BEFORE signing - input[0] keys: {list(decoded_before['inputs'][0].keys())}")
+ self.log.info(f"BEFORE signing - has non_witness_utxo: {'non_witness_utxo' in decoded_before['inputs'][0]}")
+ self.log.info(f"BEFORE signing - has witness_utxo: {'witness_utxo' in decoded_before['inputs'][0]}")
+
+ # Check witness_utxo script details
+ if 'witness_utxo' in decoded_before['inputs'][0]:
+ witness_utxo = decoded_before['inputs'][0]['witness_utxo']
+ self.log.info(f"BEFORE signing - witness_utxo script type: {witness_utxo['scriptPubKey'].get('type', 'unknown')}")
+ self.log.info(f"BEFORE signing - witness_utxo hex: {witness_utxo['scriptPubKey'].get('hex', 'none')}")
+
+ # Log the exact walletprocesspsbt call being made
+ psbt_b64 = psbt_new.to_base64()
+ self.log.info(f"walletprocesspsbt call: offline_node.walletprocesspsbt('{psbt_b64[:50]}...', True, 'ALL', True, True)")
+
signed_psbt = offline_node.walletprocesspsbt(psbt_new.to_base64())
- assert not "non_witness_utxo" in mining_node.decodepsbt(signed_psbt["psbt"])["inputs"][0]
+ decoded_after = mining_node.decodepsbt(signed_psbt["psbt"])
+
+ # Log PSBT input details after signing
+ self.log.info(f"AFTER signing - input[0] keys: {list(decoded_after['inputs'][0].keys())}")
+ self.log.info(f"AFTER signing - has non_witness_utxo: {'non_witness_utxo' in decoded_after['inputs'][0]}")
+ self.log.info(f"AFTER signing - has witness_utxo: {'witness_utxo' in decoded_after['inputs'][0]}")
+ self.log.info(f"walletprocesspsbt result complete: {signed_psbt.get('complete', 'not present')}")
+
+ # Check for Taproot vs regular segwit indicators
+ has_taproot_derivs = 'taproot_bip32_derivs' in decoded_before['inputs'][0] or 'taproot_bip32_derivs' in decoded_after['inputs'][0]
+ has_taproot_key = 'taproot_internal_key' in decoded_before['inputs'][0] or 'taproot_internal_key' in decoded_after['inputs'][0]
+ has_regular_derivs = 'bip32_derivs' in decoded_before['inputs'][0] or 'bip32_derivs' in decoded_after['inputs'][0]
+
+ self.log.info(f"Input type detection - Taproot derivs: {has_taproot_derivs}, Taproot key: {has_taproot_key}, Regular derivs: {has_regular_derivs}")
+
+ # Log the actual walletprocesspsbt call details for comparison
+ if self.options.usecli:
+ self.log.info("Using CLI mode - parameters passed as separate arguments")
+ else:
+ self.log.info("Using RPC mode - parameters passed as JSON")
+
+ # More detailed assertion with context
+ if "non_witness_utxo" in decoded_after["inputs"][0]:
+ self.log.error("FAILURE: non_witness_utxo should be removed for segwit v1+ inputs!")
+ self.log.error(f"This suggests the input is being treated as segwit v0 instead of v1+ in CLI mode")
+
+
with cli2025-07-07T15:20:33.852283Z TestFramework (INFO): Check that non-witness UTXOs are removed for segwit v1+ inputs
2025-07-07T15:20:34.004885Z TestFramework (INFO): === DEBUG: CLI Mode = True ===
2025-07-07T15:20:34.008380Z TestFramework (INFO): Offline address: bcrt1q2skwujcxhaawtf0hlqawfmk3gclpdz24a84znw
2025-07-07T15:20:34.008437Z TestFramework (INFO): Address type: unknown
2025-07-07T15:20:34.008459Z TestFramework (INFO): Script type: unknown
2025-07-07T15:20:34.008479Z TestFramework (INFO): Is witness: True
2025-07-07T15:20:34.008497Z TestFramework (INFO): Witness version: 0
2025-07-07T15:20:34.008513Z TestFramework (INFO): Witness program: 542cee4b06bf7ae5a5f7f83ae4eed1463e168955
2025-07-07T15:20:34.008532Z TestFramework (INFO): BEFORE signing - input[0] keys: ['witness_utxo', 'non_witness_utxo', 'bip32_derivs']
2025-07-07T15:20:34.008550Z TestFramework (INFO): BEFORE signing - has non_witness_utxo: True
2025-07-07T15:20:34.008565Z TestFramework (INFO): BEFORE signing - has witness_utxo: True
2025-07-07T15:20:34.008579Z TestFramework (INFO): BEFORE signing - witness_utxo script type: witness_v0_keyhash
2025-07-07T15:20:34.008595Z TestFramework (INFO): BEFORE signing - witness_utxo hex: 0014542cee4b06bf7ae5a5f7f83ae4eed1463e168955
2025-07-07T15:20:34.008688Z TestFramework (INFO): walletprocesspsbt call: offline_node.walletprocesspsbt('cHNidP8BAFICAAAAAQVChcz/DF+d62weFt4XJAYjd8cLM81FZV...', True, 'ALL', True, True)
2025-07-07T15:20:34.016020Z TestFramework (INFO): AFTER signing - input[0] keys: ['witness_utxo', 'non_witness_utxo', 'final_scriptwitness']
2025-07-07T15:20:34.016093Z TestFramework (INFO): AFTER signing - has non_witness_utxo: True
2025-07-07T15:20:34.016117Z TestFramework (INFO): AFTER signing - has witness_utxo: True
2025-07-07T15:20:34.016135Z TestFramework (INFO): walletprocesspsbt result complete: True
2025-07-07T15:20:34.016155Z TestFramework (INFO): Input type detection - Taproot derivs: False, Taproot key: False, Regular derivs: True
2025-07-07T15:20:34.016174Z TestFramework (INFO): Using CLI mode - parameters passed as separate arguments
2025-07-07T15:20:34.016193Z TestFramework (ERROR): FAILURE: non_witness_utxo should be removed for segwit v1+ inputs!
2025-07-07T15:20:34.016209Z TestFramework (ERROR): This suggests the input is being treated as segwit v0 instead of v1+ in CLI mode
2025-07-07T15:20:34.016234Z TestFramework (ERROR): Assertion failedwithout cli2025-07-07T15:19:10.430768Z TestFramework (INFO): === DEBUG: CLI Mode = False ===
2025-07-07T15:19:10.431566Z TestFramework (INFO): Offline address: bcrt1pxmju835wnuw3z5thftuz84q8rt96vqd97jaxwdn2uqlxwwka923q8fcsp8
2025-07-07T15:19:10.431613Z TestFramework (INFO): Address type: unknown
2025-07-07T15:19:10.431645Z TestFramework (INFO): Script type: unknown
2025-07-07T15:19:10.431672Z TestFramework (INFO): Is witness: True
2025-07-07T15:19:10.431699Z TestFramework (INFO): Witness version: 1
2025-07-07T15:19:10.431729Z TestFramework (INFO): Witness program: 36e5c3c68e9f1d1151774af823d4071acba601a5f4ba67366ae03e673add2aa2
2025-07-07T15:19:10.431760Z TestFramework (INFO): BEFORE signing - input[0] keys: ['witness_utxo', 'non_witness_utxo', 'taproot_bip32_derivs', 'taproot_internal_key']
2025-07-07T15:19:10.431789Z TestFramework (INFO): BEFORE signing - has non_witness_utxo: True
2025-07-07T15:19:10.431831Z TestFramework (INFO): BEFORE signing - has witness_utxo: True
2025-07-07T15:19:10.431857Z TestFramework (INFO): BEFORE signing - witness_utxo script type: witness_v1_taproot
2025-07-07T15:19:10.431881Z TestFramework (INFO): BEFORE signing - witness_utxo hex: 512036e5c3c68e9f1d1151774af823d4071acba601a5f4ba67366ae03e673add2aa2
2025-07-07T15:19:10.431929Z TestFramework (INFO): walletprocesspsbt call: offline_node.walletprocesspsbt('cHNidP8BAF4CAAAAAVteg2JVDVvQHLSH4zmmgu7l4bYJdyIoFq...', True, 'ALL', True, True)
2025-07-07T15:19:10.433261Z TestFramework (INFO): AFTER signing - input[0] keys: ['witness_utxo', 'final_scriptwitness']
2025-07-07T15:19:10.433305Z TestFramework (INFO): AFTER signing - has non_witness_utxo: False
2025-07-07T15:19:10.433336Z TestFramework (INFO): AFTER signing - has witness_utxo: True
2025-07-07T15:19:10.433364Z TestFramework (INFO): walletprocesspsbt result complete: True
2025-07-07T15:19:10.433399Z TestFramework (INFO): Input type detection - Taproot derivs: True, Taproot key: True, Regular derivs: False
2025-07-07T15:19:10.433426Z TestFramework (INFO): Using RPC mode - parameters passed as JSON
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review ACK ad6d740623f01bbe90cbf328590ccfae602c2a3f, just updating documentation and converting class into a namespace since last review. I like the new example in the RPCConvertNamedValues documentation.
I left some comments below, but feel free to ignore them, they are just explaining previous suggestions not adding anything new.
ryanofsky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review ACK 6730abb137eed82073814bdb8dbd577e79520b59. Just some comments and indentation changed since last review. Thanks for considering all the suggestions
ryanofsky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review ACK 7dd85d13e22f14940cce9ed9a5bbc2afc5c5c2f4. Only change since last review is applying int->size_t and documentation tweaks. Thanks for the update!
|
Rebased. And addressed the changes done in PR #33230. |
ryanofsky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review ACK 24391ed7804a724a62034b21150c89e45ac9b625. Just rebased after #33230
Prabhat1308
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review ACK 24391ed
Just left a few non-blocking nits
ryanofsky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review ACK f53dbbc. Just applied comment & test suggestions since last review
kannapoix
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review ACK: f53dbbc
achow101
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK f53dbbc
| # This should be treated as positional and fail with decode error, not parameter error | ||
| assert_raises_rpc_error(-22, "TX decode failed invalid base64", node.cli("-named", "finalizepsbt", unknown_psbt_param).send_cli) | ||
|
|
||
| self.log.info("PSBT parameter handling test completed successfully") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In f53dbbc "test: Add functional tests for named argument parsing"
nit: Logging at the end of the test case is unnecessary
| addr_info = node.getaddressinfo(address) | ||
| assert_equal(addr_info.get('labels', []), [equals_label]) | ||
|
|
||
| self.log.info("getnewaddress label parameter handling test completed successfully") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In f53dbbc "test: Add functional tests for named argument parsing"
nit: Logging at the end of the test case is unnecessary
| const CRPCConvertParam* named_param{rpc_convert::FromName(strMethod, name)}; | ||
| if (!named_param) { | ||
| const CRPCConvertParam* positional_param = rpc_convert::FromPosition(strMethod, positional_args.size()); | ||
| UniValue parsed_value; | ||
| if (positional_param && positional_param->format == ParamFormat::JSON && parsed_value.read(s)) { | ||
| positional_args.push_back(std::move(parsed_value)); | ||
| continue; | ||
| } else if (positional_param && positional_param->format == ParamFormat::STRING) { | ||
| positional_args.push_back(s); | ||
| continue; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think this heuristic makes sense, because it is already incomplete with the merge of this pull, and it is unclear how to even maintain it going forward. I don't think it is a good use of time to think about every single named argument, whether it can be a string, and whether the string may theoretically contain a =.
It would be better to just fully list all named args names in the client. Then, just check if the string starts with the named arg name. If yes, it is a named arg. If not, it is not.
For example, the current mapping is incomplete, because I can not pass random strings to echo:
$ ./bld-cmake/bin/bitcoin-cli -named echo random_string_=1_
error code: -8
error message:
Unknown named parameter random_string_
Just replacing the heuristic with a trivial and exact check seems like it would be less code logic, less edge cases, and less to manually maintain?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, with the PR#31375, the -named is enabled by default, and the idea is to pass the arguments in the named way or the positional way, without explicitly specifying for both of them. But the problem was that previously some Base64 or even some potential file paths might contain an = char in them. This can falsify the above condition of enabling the -named by default because if someone is using positional argument parsing, then the = in some arguments makes it force them as named parameters. For example, suppose someone created a wallet where -named is enabled by default using: ./bitcoin-cli createwallet "my=wallet" internally this will be passed as a -named argument and the user will get the error about "Unknown named parameter my" even though the user passed the argument by position. That's why I think your suggested approach might not work here, but you can clarify that if you think so or if I'm missing any point.
. I don't think it is a good use of time to think about every single named argument, whether it can be a string, and whether the string may theoretically contain a
=.
Well, I think we can theoretically be assured that the input of some args might contain an equal char or not. Like, the blockhash contains hex characters which are guaranteed not to contain an = char , same for int and bools. But things which contain strings, like file_paths, wallet_names, and labels, can contain an = char.
For example, the current mapping is incomplete, because I can not pass random strings to
echo:
Thanks for catching this error, we can solve this just by listing the echo method in the vRPCConvertParams table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This name=value syntax is inherently ambiguous. There was already a heuristic being used to interpret it before this pull, and now a better heuristic is implemented. I think the new parsing code is simpler and less confusing than the previous code, and it is definitely better documented and tested. This PR is fixing a real issue with bitcoin-cli and base64 which makes PBST methods difficult to use, reported #33286 (comment) and other places. I don't think the current heuristic should be much work to maintain: it can just be left alone, and if someone finds a case that isn't working well they can open a PR for consideration.
I don't think I understand your example with echo because the echo RPC should be unaffected by this PR.
I do think your suggestion to "list all named args names in the client" is reasonable, and it is compatible with this PR. It would just have its own advantages and disadvantages. An advantage is that it would avoid the need to distinguish between string parameters allowed to contain '=' (like filenames, labels, and base64 strings), and other types of strings (like enum values). Another advantage is it could allow dropping a few lines of code in RPCConvertNamedValues. But it would make the parameter table larger and potentially more difficult to maintain. Also if RPCConvertNamedValues is changed it could cause more problems if bitcoin-cli and bitcoind used are not from the same build. If you feel that approach is better, you could implement it, but that approach doesn't match what previous code was doing and shouldn't change the fact that this PR should be a strict improvement.
I think another good way forward could be to provide an unambiguous way to distinguish named and positional parameters. For example, I believe #33540 might do that by allowing named parameters to be passed without the -named option if they are prefixed with -. A function-style syntax like #20273 could also be more convenient and unambiguous.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example, suppose someone created a wallet where -named is enabled by default using:
./bitcoin-cli createwallet "my=wallet"internally this will be passed as a -named argument and the user will get the error about "Unknown named parameter my" even though the user passed the argument by position. That's why I think your suggested approach might not work here, but you can clarify that if you think so or if I'm missing any point.
I don't understand why my suggestion does not work. my= is not a valid named argument for createwallet, so the check "my=wallet".starts_with("wallet_name=") evaluates to false for all args, and the argument is correctly identified as a positional arg. Conversely, if you were passing wallet_name=bla, the check would also correctly identify the named arg.
Thanks for catching this error, we can solve this just by listing the
echomethod in the vRPCConvertParams table.
Sure, but my point is that this is not a good use of developer or reviewer time, to manually go out and evaluate each arg, when it can trivially be done by a simple check (see above).
I don't think the current heuristic should be much work to maintain: it can just be left alone, and if someone finds a case that isn't working well they can open a PR for consideration.
Sure, by my primary point is not that the heuristic is hard to maintain, but that the list of stuff that may or may not have an = in it has to be maintained manually, when there is no need to.
I don't think I understand your example with
echobecause theechoRPC should be unaffected by this PR.
Correct. This is my point. I think all RPCs should be consistent and correct. It doesn't seem a good use of time to go though each one one-by-one and fix it manually.
But it would make the parameter table larger and potentially more difficult to maintain.
Why would that be? it should be trivial to auto-generate it, or at the least check it via the rpc_help.py test.
Also if
RPCConvertNamedValuesis changed it could cause more problems ifbitcoin-cliandbitcoindused are not from the same build.
I don't think it is generally supported to mix and match them. Also, worsening the UX for people who keep them in sync does not seem like a good trade-off to accommodate people who mix incompatible binaries.
I think another good way forward could be to provide an unambiguous way to distinguish named and positional parameters. For example, I believe #33540 might do that by allowing named parameters to be passed without the
-namedoption if they are prefixed with-. A function-style syntax like #20273 could also be more convenient and unambiguous.
Sure, but those seem orthogonal, because the existing arg passing will stay around even after those approaches are taken?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why my suggestion does not work.
my=is not a valid named argument forcreatewallet, so the check"my=wallet".starts_with("wallet_name=")evaluates to false for all args, and the argument is correctly identified as a positional arg. Conversely, if you were passingwallet_name=bla, the check would also correctly identify the named arg.
That's what something similar to the current code is doing internally but with more checks and conditions. So one of the reasons your approach might fail is there is no way to tell whether after treating the arguments as positional (where -named is enabled) we need to convert them as JSON or STRING or JSON_OR_STRING. If we convert or parse them as JSON but they needed to be parsed as STRING you will see an error. For this, you can easily reproduce this by changing the type of wallet_name to JSON instead of STRING in the current code. Now, without specifying the types like JSON, STRING and JSON_OR_STRING, it's hard to parse them in their correct expected form. And this PR fixes that in addition to the check you mentioned earlier.
The second case I can think of in which your approach might fail is if the user passed ./bitcoin-cli createwallet -named passphrase="secret" wallet_name="my=wallet" . Here with your approach "passphrase=secret".starts_with("wallet_name=") becomes false, and you treat all of them as positional, which leads to incorrect results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re: #32821 (comment)
Thanks for the reply. I think you have all the facts right, and I accept all the points you are making, even if we might prefer different tradeoffs & possible solutions. My point was just that RPCConvertNamedValues was using a poor heuristic before this PR, and now it is using a better one, without requiring every parameter to be listed in the table. If you do want to list every parameter in the table and simplify the heuristic, removing a few lines of code, that seems ok to me. I think the only downside would be tighter coupling between bitcoin-cli and bitcoind and maybe worse behavior between mismatched versions. But I wouldn't have any objection.
I mostly hope it is clear that this PR is strict improvement. It makes PSBT methods work reliably, simplifies code, improves documentation and tests. It does require string parameters that contain = to be listed in the table for them benefit from this PR. Otherwise, as you pointed out with echo, if the parameters are not listed, they won't be recognized and the behavior is just the same as before.
0690514d4f Merge bitcoin/bitcoin#33770: init: Require explicit -asmap filename b2f88b53e0 Merge bitcoin/bitcoin#33286: doc: update multisig tutorial to use multipath descriptors 313cdd2bfb Merge bitcoin/bitcoin#33915: test: Retry download in get_previous_releases.py 17072f7005 Merge bitcoin/bitcoin#33912: clang-format: Set PackConstructorInitializers: CurrentLine 6b2d17b132 Merge bitcoin/bitcoin#33888: ci: Re-enable LINT_CI_SANITY_CHECK_COMMIT_SIG ac71df4338 Merge bitcoin/bitcoin#33870: refactor: remove incorrect lifetimebounds 6cdb51c14e Merge bitcoin/bitcoin#33887: doc: Improve CI docs on env and qemu-user-static 29c37651c7 Merge bitcoin/bitcoin#33880: test: Fix race condition in IPC interface block progation test 32368cd3e9 Merge bitcoin/bitcoin#33905: ci: Consistenly only cache on the default branch e55c49f851 Merge bitcoin/bitcoin#33851: depends: update xcb-util packages to latest versions a07bd8415d Merge bitcoin/bitcoin#33824: ci: Enable experimental kernel stuff in most CI tasks via `dev-mode` f541b92cf2 depends: expat 2.7.3 fad06f3bb4 test: retry download in get_previous_releases.py 2ebf4356e6 depends: libxcb 1.17.0 ba7ac870a3 depends: xcb_proto 1.17.0 fad0c76d0a clang-format: Set PackConstructorInitializers: CurrentLine 42d0692f91 depends: libxcb-util-cursor 0.1.6 25b85919ab depends: libxcb 1.15 d129384ca9 depends: libxcb-util-wm 0.4.2 0b857ae9e5 depends: libxcb-util-renderutil 0.3.10 35e50488b2 depends: libxcb-util-keysyms 0.4.1 74b68ad28b depends: libxcb-util-image 0.4.1 5bc0dde85d depends: libxcb-util 0.4.1 8d07292c28 depends: libXau 1.0.12 1af46cff94 Merge bitcoin/bitcoin#33896: clang-format: Set InsertNewlineAtEOF: true 27ac11ea0a Merge bitcoin/bitcoin#33867: kernel: handle null or empty directories in implementation 2578e6fc0f test: Fix race condition in IPC interface block propagation test 288b8c30be doc: Drop (default: none) from -i2psam description 509dc91db1 Merge bitcoin/bitcoin#33026: test, refactor: Embedded ASMap [1/3]: Selected minor preparatory work b126f98194 Merge bitcoin-core/gui#910: Added test coverage for qt gui#901 console history filter 7d7b829c36 Merge bitcoin-core/gui#908: Remove HD seed reference from blank wallet tooltip 53b72372da Merge bitcoin/bitcoin#31734: miniscript: account for all `StringType` variants in `Miniscriptdescriptor::ToString()` a7f9bbe4c5 Merge bitcoin/bitcoin#32821: rpc: Handle -named argument parsing where '=' character is used 55555db055 doc: Add missing --platform=linux to docker build command fa0ce4c148 ci: Re-enable LINT_CI_SANITY_CHECK_COMMIT_SIG faa0973de2 ci: [refactor] Rename CIRRUS_PR env var to LINT_CI_IS_PR fa411f938e ci: Consistenly only cache on the default branch 552eb90071 doc: CI - Describe qemu-user-static usage 2afbbddee5 doc: CI - Clarify how important `env -i` is and why 2444488f6a Merge bitcoin/bitcoin#33894: net: Remove unused `local_socket_bytes` variable in `CConnman::GetAddresses()` fa1bf6818f clang-format: Set InsertNewlineAtEOF: true 115d298a9f Merge bitcoin/bitcoin#33872: init: completely remove `-maxorphantx` option a90f3922ff Merge bitcoin/bitcoin#32419: psbt: clarify PSBT, PSBTInput, PSBTOutput unserialization flows 4d893c0f46 net: Remove unused `local_socket_bytes` variable in `CConnman::GetAddresses()` fa1dacaebe ci: Move lint exec snippet to stand-alone py file ead849c9f1 Merge bitcoin/bitcoin#33886: test: Remove tests violating hardened std::span c03081fdb4 Merge bitcoin/bitcoin#33776: ci: Lint follow-ups fadb4f63cb test: Remove tests violating hardened std::span 6e21558160 Merge bitcoin/bitcoin#33869: refactor: Avoid -W*-whitespace in git archive c8715aca95 Merge bitcoin/bitcoin#33247: build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings ee5de407e3 Merge bitcoin/bitcoin#33537: guix: build `bitcoin-qt` with static libxcb & utils 024a787350 Merge bitcoin/bitcoin#33876: doc: Update NetBSD Build Guide c66e988754 Merge bitcoin/bitcoin#33865: cmake: Specify Windows plugin path in `test_bitcoin-qt` property c29eaeeaf9 doc: Update NetBSD Build Guide 7f318e1dd0 test: Add better coverage for Autofile size() e221b25246 Merge bitcoin/bitcoin#33860: depends: drop Qt patches b7af960eb8 refactor: Add AutoFile::size ec0f75862e refactor: Modernize logging in util/asmap.cpp 606a251e0a tests: add unit test vectors for asmap interpreter 6657bcbdb4 kernel: allow null data_directory 0aebdac95d init: completely remove `-maxorphantx` option 99d012ec80 refactor: return reference instead of pointer f743e6c5dd refactor: add missing LIFETIMEBOUND annotation for parameter fa95353902 ci: Run macos tasks in a git archive, not git checkout 141117f5e8 refactor: remove incorrect LIFETIMEBOUND annotations fae3618fd6 ci: Annotate all check runs with the pull request number faf05d637d ci: Retry lint image building once after failure 96963b888e depends: static libxcb ad06843fab depends: avoid qdbusviewer in Qt build 6848ed56dc depends: apply Qt patches to fix static libxcb use dfde31f2ec Merge bitcoin/bitcoin#33864: scripted-diff: fix leftover references to `policy/fees.h` 5f1b016beb depends: static libxcb-util-image 98a2fbbe70 depends: static libxkbcommon 1412baf772 depends: static libxcb-util-wm a4009dadf4 depends: static libxcb-keysyms bcfb8679b3 depends: static libxcb-render-util faf99ae379 refactor: Avoid -W*-whitespace in git archive 2594d5a189 build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings 310e4979b3 qt: Added test coverage for qt gui#901 console history filter 0dd8d5c237 cmake: Specify Windows plugin path in `test_bitcoin-qt` property b0a3887154 scripted-diff: fix leftover references to `policy/fees.h` 48d4b936e0 Merge bitcoin/bitcoin#33511: init: Fix Ctrl-C shutdown hangs during wait calls 3c3c6adb72 Merge bitcoin/bitcoin#33745: mining: check witness commitment in submitBlock e652b69b8d Merge bitcoin/bitcoin#33003: test: add option to skip large re-org test in feature_block 3789215f73 Merge bitcoin/bitcoin#33724: refactor: Return uint64_t from GetSerializeSize d4e2a45833 Merge bitcoin/bitcoin#33750: doc: document fingerprinting risk when operating node on multiple networks 47618446a0 Merge bitcoin/bitcoin#33853: kernel: Allow null arguments for serialized data 3e9aca6f1b depends: drop qtbase-moc-ignore-gcc-macro.patch qt patch fac4f6de28 ci: Rewrite lint task Bash snippet to Python fa0d37a579 ci: Rewrite Bash to check inputs to Python 0da5a82700 depends: drop unused qt patch fae83611b8 ci: [refactor] Use --preset=dev-mode in mac_native task fadb67b4b4 ci: [refactor] Base nowallet task on --preset=dev-mode 6666980e86 ci: Enable bitcoin-chainstate and test_bitcoin-qt in win64 task d0da953773 Merge bitcoin/bitcoin#32482: build: add `-W*-whitespace` f450761f83 Merge bitcoin/bitcoin#33842: build: Bump g++ minimum supported version to 12 faff7b2312 ci: Enable experimental kernel stuff in i686 task fa1632eecf ci: Enable experimental kernel stuff in mac-cross tasks fad10ff7c9 ci: Enable experimental kernel stuff in armhf task fa9d67c13d ci: Enable experimental kernel stuff in Alpine task fab3fb8302 ci: Enable experimental kernel stuff in s390x task fa7da8a646 ci: Enable experimental kernel stuff in valgrind task fa9c2973d6 ci: Enable experimental kernel stuff in TSan task fad30d4395 ci: Enable experimental kernel stuff in MSan task fa9f29a4a7 doc: Recommend latest Debian stable or Ubuntu LTS fa1711ee0d doc: Add GCC-12 min release notes faa8be75c9 ci: Enable experimental kernel stuff in G++-12 task (previous releases) fabce97b30 test: Remove gccbug_90348 test case fa3854e432 test: Remove unused fs::create_directories test fa9dacdbde util: [refactor] Remove unused create_directories workaround 138726a6f8 Merge bitcoin/bitcoin#33850: depends: drop qtbase_avoid_native_float16 qt patch 1c3d5c8ffd Merge bitcoin/bitcoin#33840: test: [refactor] Use reference over ptr to chainman 40dcbf580d build: add -Wtrailing-whitespace=any a3ac59a431 ci: Enable experimental kernel stuff in ASan task 5b89956eeb kernel: Allow null arguments for serialized data 169f93d2ac depends: drop qtbase_avoid_native_float16 qt patch d7659cd7e6 build: add -Wleading-whitespace=spaces d86650220a cmake: Disable `-Wtrailing-whitespace` warnings for RCC-generated files aabc5ca6ed cmake: Switch from AUTORCC to `qt6_add_resources` 25ae14c339 subprocess: replace tab with space 0c2b9dadd5 scripted-diff: remove whitespace in sha256_sse4.cpp 4da084fbc9 scripted-diff: change whitespace to spaces in univalue e6caf150b3 ci: add moreutils to lint job a7e8067610 Merge bitcoin/bitcoin#33181: guix: build for Linux HOSTS with `-static-libgcc` b354d1ce5c Merge bitcoin/bitcoin#33820: kernel: trim Chain interface fa807f78ae build: Bump g++ minimum supported version to 12 a4e96cae7d Merge bitcoin/bitcoin#33042: refactor: inline constant return values from `dbwrapper` write methods 8c2710b041 Merge bitcoin/bitcoin#32517: rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response 1fe851a478 Merge bitcoin/bitcoin#32180: p2p: Advance pindexLastCommonBlock early after connecting blocks 5f0303b93f Merge bitcoin/bitcoin#33443: log: reduce excessive "rolling back/forward" messages during block replay f4903dddc9 Merge bitcoin/bitcoin#33433: Bugfix: QA: rpc_bind: Skip nonloopback test if no such address is found 7a4901c902 test, refactor: Fix `-Warray-bounds` warning 06e9458869 Merge bitcoin/bitcoin#32856: Update `minisketch` subtree 66978a1a95 kernel: remove btck_chain_get_tip 4dd7e6dc48 kernel: remove btck_chain_get_genesis faf2759c8c test: [refactor] Use reference over ptr to chainman 490cb056f6 Merge bitcoin/bitcoin#33785: util: Allow `Assert` (et al.) in contexts without __func__ dcd0099a76 Merge bitcoin/bitcoin#33826: scripted-diff: Remove obsolete comment 01adbbcd9c Merge bitcoin/bitcoin#33827: doc: Correct `pkgin` command usage on NetBSD eec21bc7c8 Merge bitcoin/bitcoin#33536: doc: reference fuzz coverage steps in quick-start 035f934e02 Merge bitcoin/bitcoin#33823: ci: Use cmake --preset=dev-mode in test-each-commit task ddd2afac10 Merge bitcoin/bitcoin#33676: interfaces: enable cancelling running `waitNext` calls dee7eec643 doc: mention coverage build in quickstart section 0698c6b494 doc: Correct `pkgin` command usage on NetBSD 36724205fc scripted-diff: Remove obsolete comment ca1ce52a0f Merge bitcoin/bitcoin#33825: refactor: Add missing include in bitcoinkernel_wrapper.h fa1e8d8bad refactor: Add missing include in bitcoinkernel_wrapper.h fa6db67369 ci: [refactor] Extract build_dir constant in ci-test-each-commit-exec.py fa95e6cdc1 ci: Use cmake --preset=dev-mode in test-each-commit task 513a0da2e0 Merge bitcoin/bitcoin#33818: ci: Extend tidy job to cover kernel code 5d0a40d607 ci: Extend tidy job to cover kernel code 93e79181da Merge bitcoin/bitcoin#33786: script: remove dead code in `CountWitnessSigOps` 3c4bec6223 Merge bitcoin/bitcoin#33782: test: remove obsolete `get_{key,multisig}` helpers from wallet_util.py 4b12beedae Merge bitcoin/bitcoin#33793: test: move create_malleated_version() to messages.py for reuse 0b45e6db10 Merge bitcoin/bitcoin#33789: doc: add cmake help option in Windows build docs 2b9c351198 Merge bitcoin/bitcoin#33768: refactor: remove dead branches in `SingletonClusterImpl` fad6efd3be refactor: Use STR_INTERNAL_BUG macro where possible fada379589 doc: Remove unused bugprone-lambda-function-name suppression f06c6e1898 guix: build for Linux HOSTS with -static-libgcc 1bdf4695b0 guix: patch store paths out of libunwind 078a72c35f guix: move static-libc++ into CMAKE_EXE_LINKER_FLAGS flags 2bd155e6ee test: move create_malleated_version() to messages.py for reuse 9577daa3b8 doc: Add cmake help option in Windows build instructions fae1d99651 refactor: Use const reference to std::source_location fa5fbcd615 util: Allow Assert() in contexts without __func__ 24bcad3d4d refactor: remove dead code in `CountWitnessSigOps` ec8516ceb7 test: remove obsolete `get_{key,multisig}` helpers from wallet_util.py 2d23820ee1 refactor: remove dead branches in `SingletonClusterImpl` e346ecae83 Add eclipse, partitioning, and fingerprinting note to i2p.md f6ec3519a3 init: Require explicit -asmap filename 6eaa00fe20 test: clarify submitBlock() mutates the template 862bd43283 mining: ensure witness commitment check in submitBlock 00d1b6ef4b doc: clarify UpdateUncommittedBlockStructures 929f69d0ff qt: Remove HD seed reference from blank wallet tooltip 19a6a3e75e Add eclipse, partitioning, and fingerprinting note in tor.md fa6c0bedd3 refactor: Return uint64_t from GetSerializeSize fad0c8680e refactor: Use uint64_t over size_t for serialized-size values fa4f388fc9 refactor: Use fixed size ints over (un)signed ints for serialized values 060bb55508 rpc: add decoded tx details to gettransaction with extra wallet fields ad1c3bdba5 [move only] move DecodeTxDoc() to a common util file for sharing d633db5416 rpc: add "ischange: true" in wallet gettransaction decoded tx output fa01f38e53 move-only: Move CBlockFileInfo to kernel namespace fa2bbc9e4c refactor: [rpc] Remove cast when reporting serialized size fa364af89b test: Remove outdated comment dcb56fd4cb interfaces: add interruptWait method de7c3587cd doc: Update add checksum instructions in tutorial c235aa468b Update minisketch subtree to latest upstream 4543a3bde2 Squashed 'src/minisketch/' changes from ea8f66b1ea..d1bd01e189 01cc20f330 test: improve coverage for a resolved stalling situation 9af6daf07e test: remove magic number when checking for blocks that have arrived 3069d66dca p2p: During block download, adjust pindexLastCommonBlock better 2a46e94a16 doc: Update multisig-tutorial.md to use multipath descriptors c25a5e670b init: Signal m_tip_block_cv on Ctrl-C f53dbbc505 test: Add functional tests for named argument parsing 694f04e2bd rpc: Handle -named argument parsing where '=' character is used 6a29f79006 test: Test SIGTERM handling during waitforblockheight call 1fc7a81f1f log: reduce excessive messages during block replay 79b4c276e7 Bugfix: QA: rpc_bind: Skip nonloopback test if no such address is found 743abbcbde refactor: inline constant return value of `BlockTreeDB::WriteBatchSync` and `BlockManager::WriteBlockIndexDB` and `BlockTreeDB::WriteFlag` e030240e90 refactor: inline constant return value of `CDBWrapper::Erase` and `BlockTreeDB::WriteReindexing` cdab9480e9 refactor: inline constant return value of `CDBWrapper::Write` d1847cf5b5 refactor: inline constant return value of `TxIndex::DB::WriteTxs` 50b63a5698 refactor: inline constant return value of `CDBWrapper::WriteBatch` 8810642b57 test: add option to skip large re-org test in feature_block d31158d364 psbt: clarify PSBT, PSBTInput, PSBTOutput unserialization flows 28a4fcb03c test: check listdescriptors do not return a mix of hardened derivation marker 975783cb79 descriptor: account for all StringType in MiniscriptDescriptor::ToStringHelper() git-subtree-dir: depend/bitcoin git-subtree-split: 0690514d4f72aac251ee0b876cded9187d42c63e
…356041e58d1 6356041e58d1 Merge bitcoin/bitcoin#33972: cmake: Make `BUILD_KERNEL_TEST` depend on `BUILD_KERNEL_LIB` 7d7cb1bb48f1 Merge bitcoin/bitcoin#33971: cmake: Set `WITH_ZMQ` to `ON` in Windows presets fe1815d48f0c cmake: Make `BUILD_KERNEL_TEST` depend on `BUILD_KERNEL_LIB` 49c672853503 cmake: Set `WITH_ZMQ` to `ON` in Windows presets f6acbef1084e Merge bitcoin/bitcoin#33764: ci: Add Windows + UCRT jobs for cross-compiling and native testing 808f1d972be3 Merge bitcoin/bitcoin#32009: contrib: turn off compression of macOS SDK to fix determinism (across distros) 4de26b111f4d Merge bitcoin/bitcoin#33514: ci: clear out space on CentOS, depends, gui GHA job 38c8474d0d77 Merge bitcoin/bitcoin#33914: Change Parse descriptor argument to string_view 4b25b274de66 Merge bitcoin/bitcoin#33951: test: check for output to stdout in `TestShell` test 52230a7f697f test: check for output to stdout in `TestShell` test 85d058dc537e Merge bitcoin/bitcoin#33946: interfaces: remove redundant mempool lock in ChainImpl::isInMempool() e07e57368e9f ci: clear out space on centos job 79d6e874e1da Merge bitcoin/bitcoin#32587: test: Fix reorg patterns in tests to use proper fork-based approach e249ea7da6c2 Merge bitcoin/bitcoin#33945: depends: latest config.guess & config.sub 3e01b5d0e7be contrib: rename gen-sdk to gen-sdk.py c1213a35abed macdeploy: disable compression in macOS gen-sdk script a33d03454508 contrib: more selectively pick files for macOS SDK 70d9e8f0a15d fix: reorg behaviour in mempool tests to match real one 540ed333f6c8 Move the create_empty_fork method to the test framework's blocktools.py module to enable reuse across multiple tests. 2909655fba91 fix: remove redundant mempool lock in ChainImpl::isInMempool() d5ed4ba9d862 Merge bitcoin/bitcoin#33906: depends: Add patch for Windows11Style plugin 3e4355314b1a depends: latest config.sub 04eb84fe3f73 depends: latest config.guess b30262dcaa28 Merge bitcoin/bitcoin#33903: ci: Remove redundant busybox option 1a5f1eb08067 Merge bitcoin/bitcoin#33921: doc: clarify and cleanup macOS fuzzing notes 72cb8cef9778 Merge bitcoin/bitcoin#33862: txgraph: drop move assignment operator ade0397f59f2 txgraph: drop move assignment operator 5336bcd57849 Merge bitcoin/bitcoin#33855: kernel: add btck_block_tree_entry_equals 4f65a1c5db84 Merge bitcoin/bitcoin#33917: clang-format: Set Bitcoin Core IncludeCategories 902717b66dc1 Merge bitcoin/bitcoin#33918: depends: Update Qt download link 68ab2b65bfac Merge bitcoin/bitcoin#33919: ci: Run GUI unit tests in cross-Windows task 7e129b644ec9 Merge bitcoin/bitcoin#33893: test: add `-alertnotify` test for large work invalid chain warning 5fe753b56f45 Merge bitcoin/bitcoin#32655: depends: sqlite 3.50.4; switch to autosetup ff8c2f37497f Merge bitcoin/bitcoin#33932: ci: Use latest Xcode that the minimum macOS version allows fa283d28e261 Merge bitcoin/bitcoin#33629: Cluster mempool 2e27bd9c3af9 ci: Add Windows + UCRT jobs for cross-compiling and native testing 238c1c8933b1 Merge bitcoin-core/gui#914: Revert "gui, qt: brintToFront workaround for Wayland" 8343a9ffcc75 test: add `-alertnotify` test for large work invalid chain warning c34bc01b2ff2 doc: clarify and cleanup macOS fuzzing notes fa9537cde101 ci: Use latest Xcode that the minimum macOS version allows 17cf9ff7efdb Use cluster size limit for -maxmempool bound, and allow -maxmempool=0 in general 315e43e5d86c Sanity check `GetFeerateDiagram()` in CTxMemPool::check() de2e9a24c40e test: extend package rbf functional test to larger clusters 4ef4ddb504e5 doc: update policy/packages.md for new package acceptance logic 79f73ad713a8 Add check that GetSortedScoreWithTopology() agrees with CompareMiningScoreWithTopology() a86ac1176817 Update comments for CTxMemPool class 9567eaa66da8 Invoke TxGraph::DoWork() at appropriate times 0690514d4f72 Merge bitcoin/bitcoin#33770: init: Require explicit -asmap filename b2f88b53e0ec Merge bitcoin/bitcoin#33286: doc: update multisig tutorial to use multipath descriptors 313cdd2bfb71 Merge bitcoin/bitcoin#33915: test: Retry download in get_previous_releases.py bd130db994e2 ci: Rename items specific to Windows + MSVCRT 0672e727bf1d Revert "gui, qt: brintToFront workaround for Wayland" 17072f70051d Merge bitcoin/bitcoin#33912: clang-format: Set PackConstructorInitializers: CurrentLine fa7ea497c3ef ci: Run GUI unit tests in cross-Windows task fa0fee44a89c ci: Remove redundant busybox option fa102ec69fae doc: Shorten ci name fa7e222a2326 clang-format: Set Bitcoin Core IncludeCategories 222222378048 doc: Remove bash -c wrapper 6b2d17b13220 Merge bitcoin/bitcoin#33888: ci: Re-enable LINT_CI_SANITY_CHECK_COMMIT_SIG ac71df43383a Merge bitcoin/bitcoin#33870: refactor: remove incorrect lifetimebounds 6cdb51c14eba Merge bitcoin/bitcoin#33887: doc: Improve CI docs on env and qemu-user-static 50cbde3295b4 depends: Update Qt download link 29c37651c74b Merge bitcoin/bitcoin#33880: test: Fix race condition in IPC interface block progation test 32368cd3e9f3 Merge bitcoin/bitcoin#33905: ci: Consistenly only cache on the default branch e55c49f85143 Merge bitcoin/bitcoin#33851: depends: update xcb-util packages to latest versions a07bd8415df4 Merge bitcoin/bitcoin#33824: ci: Enable experimental kernel stuff in most CI tasks via `dev-mode` c0bfe72f6e1f Change Parse descriptor argument to string_view f541b92cf2bb depends: expat 2.7.3 fad06f3bb436 test: retry download in get_previous_releases.py 2ebf4356e63d depends: libxcb 1.17.0 ba7ac870a32a depends: xcb_proto 1.17.0 fad0c76d0a10 clang-format: Set PackConstructorInitializers: CurrentLine 42d0692f9131 depends: libxcb-util-cursor 0.1.6 25b85919ab62 depends: libxcb 1.15 d129384ca97f depends: libxcb-util-wm 0.4.2 0b857ae9e555 depends: libxcb-util-renderutil 0.3.10 35e50488b25a depends: libxcb-util-keysyms 0.4.1 74b68ad28ba2 depends: libxcb-util-image 0.4.1 5bc0dde85d74 depends: libxcb-util 0.4.1 8d07292c286f depends: libXau 1.0.12 1af46cff9478 Merge bitcoin/bitcoin#33896: clang-format: Set InsertNewlineAtEOF: true 27ac11ea0a27 Merge bitcoin/bitcoin#33867: kernel: handle null or empty directories in implementation 2578e6fc0f4a test: Fix race condition in IPC interface block propagation test 288b8c30be42 doc: Drop (default: none) from -i2psam description 509dc91db143 Merge bitcoin/bitcoin#33026: test, refactor: Embedded ASMap [1/3]: Selected minor preparatory work b126f981943d Merge bitcoin-core/gui#910: Added test coverage for qt gui#901 console history filter 7d7b829c36b7 Merge bitcoin-core/gui#908: Remove HD seed reference from blank wallet tooltip 8558902e576e depends: Add patch for Windows11Style plugin 53b72372da91 Merge bitcoin/bitcoin#31734: miniscript: account for all `StringType` variants in `Miniscriptdescriptor::ToString()` a7f9bbe4c5e7 Merge bitcoin/bitcoin#32821: rpc: Handle -named argument parsing where '=' character is used 55555db055b5 doc: Add missing --platform=linux to docker build command fa0ce4c1486b ci: Re-enable LINT_CI_SANITY_CHECK_COMMIT_SIG faa0973de296 ci: [refactor] Rename CIRRUS_PR env var to LINT_CI_IS_PR fa411f938e47 ci: Consistenly only cache on the default branch 6c5c44f77405 test: add functional test for new cluster mempool RPCs 72f60c877e00 doc: Update mempool_replacements.md to reflect feerate diagram checks 21693f031a53 Expose cluster information via rpc 72e74e0d4228 fuzz: try to add more code coverage for mempool fuzzing f107417490ab bench: add more mempool benchmarks 7976eb1ae77a Avoid violating mempool policy limits in tests 84de685cf7ee Stop tracking parents/children outside of txgraph 88672e205ba1 Rewrite GatherClusters to use the txgraph implementation 1ca4f01090cf Fix miniminer_tests to work with cluster limits 1902111e0f20 Eliminate CheckPackageLimits, which no longer does anything 3a646ec46264 Rework RBF and TRUC validation 19b8479868e5 Make getting parents/children a function of the mempool, not a mempool entry 5560913e51af Rework truc_policy to use descendants, not children a4458d6c4062 Use txgraph to calculate descendants c8b6f70d6492 Use txgraph to calculate ancestors 241a3e666b59 Simplify ancestor calculation functions b9cec7f0a1e0 Make removeConflicts private 0402e6c78080 Remove unused limits from CalculateMemPoolAncestors 08be765ac26a Remove mempool logic designed to maintain ancestor/descendant state fc4e3e6bc122 Remove unused members from CTxMemPoolEntry ff3b398d124b mempool: eliminate accessors to mempool entry ancestor/descendant cached state b9a2039f5122 Eliminate use of cached ancestor data in miniminer_tests and truc_policy ba09fc9774d5 mempool: Remove unused function CalculateDescendantMaximum 8e49477e86b3 wallet: Replace max descendant count with cluster_count e031085fd464 Eliminate Single-Conflict RBF Carve Out cf3ab8e1d0a2 Stop enforcing descendant size/count limits 89ae38f48965 test: remove rbf carveout test from mempool_limit.py c0bd04d18fdf Calculate descendant information for mempool RPC output on-the-fly bdcefb8a8b06 Use mempool/txgraph to determine if a tx has descendants 69e1eaa6ed22 Add test case for cluster size limits to TRUC logic 9cda64b86c59 Stop enforcing ancestor size/count limits 1f93227a84a5 Remove dependency on cached ancestor data in mini-miner 9fbe0a4ac26c rpc: Calculate ancestor data from scratch for mempool rpc calls 7961496dda2e Reimplement GetTransactionAncestry() to not rely on cached data feceaa42e8eb Remove CTxMemPool::GetSortedDepthAndScore 21b5cea588a7 Use cluster linearization for transaction relay sort order 6445aa7d9755 Remove the ancestor and descendant indices from the mempool 216e69372903 Implement new RBF logic for cluster mempool ff8f115dec6e policy: Remove CPFP carveout rule c3f1afc934e6 test: rewrite PopulateMempool to not violate mempool policy (cluster size) limits 47ab32fdb158 Select transactions for blocks based on chunk feerate dec138d1ddc7 fuzz: remove comparison between mini_miner block construction and miner 6c2bceb200aa bench: rewrite ComplexMemPool to not create oversized clusters 1ad4590f6385 Limit mempool size based on chunk feerate b11c89cab210 Rework miner_tests to not require large cluster limit 95a8297d481e Check cluster limits when using -walletrejectlongchains 95762e675959 Do not allow mempool clusters to exceed configured limits edb3e7cdf636 [test] rework/delete feature_rbf tests requiring large clusters 435fd5671116 test: update feature_rbf.py replacement test 34e32985e811 Add new (unused) limits for cluster size/count 838d7e355366 Add transactions to txgraph, but without cluster dependencies 552eb90071fd doc: CI - Describe qemu-user-static usage 2afbbddee550 doc: CI - Clarify how important `env -i` is and why 2444488f6ad3 Merge bitcoin/bitcoin#33894: net: Remove unused `local_socket_bytes` variable in `CConnman::GetAddresses()` fa1bf6818f09 clang-format: Set InsertNewlineAtEOF: true 115d298a9fa3 Merge bitcoin/bitcoin#33872: init: completely remove `-maxorphantx` option a90f3922ff7d Merge bitcoin/bitcoin#32419: psbt: clarify PSBT, PSBTInput, PSBTOutput unserialization flows 4d893c0f4605 net: Remove unused `local_socket_bytes` variable in `CConnman::GetAddresses()` fa1dacaebe5d ci: Move lint exec snippet to stand-alone py file ead849c9f177 Merge bitcoin/bitcoin#33886: test: Remove tests violating hardened std::span c03081fdb467 Merge bitcoin/bitcoin#33776: ci: Lint follow-ups fadb4f63cb0f test: Remove tests violating hardened std::span 6e2155816058 Merge bitcoin/bitcoin#33869: refactor: Avoid -W*-whitespace in git archive c8715aca95d0 Merge bitcoin/bitcoin#33247: build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings ee5de407e369 Merge bitcoin/bitcoin#33537: guix: build `bitcoin-qt` with static libxcb & utils 024a7873500e Merge bitcoin/bitcoin#33876: doc: Update NetBSD Build Guide c66e98875439 Merge bitcoin/bitcoin#33865: cmake: Specify Windows plugin path in `test_bitcoin-qt` property c29eaeeaf937 doc: Update NetBSD Build Guide 7f318e1dd049 test: Add better coverage for Autofile size() e221b2524659 Merge bitcoin/bitcoin#33860: depends: drop Qt patches b7af960eb82f refactor: Add AutoFile::size ec0f75862e67 refactor: Modernize logging in util/asmap.cpp 606a251e0a31 tests: add unit test vectors for asmap interpreter 6657bcbdb4d0 kernel: allow null data_directory 0aebdac95da9 init: completely remove `-maxorphantx` option 99d012ec80a4 refactor: return reference instead of pointer f743e6c5dd38 refactor: add missing LIFETIMEBOUND annotation for parameter fa95353902b7 ci: Run macos tasks in a git archive, not git checkout 141117f5e8b4 refactor: remove incorrect LIFETIMEBOUND annotations fae3618fd6c8 ci: Annotate all check runs with the pull request number faf05d637d67 ci: Retry lint image building once after failure 96963b888e5a depends: static libxcb ad06843fab06 depends: avoid qdbusviewer in Qt build 6848ed56dc5f depends: apply Qt patches to fix static libxcb use dfde31f2ec1f Merge bitcoin/bitcoin#33864: scripted-diff: fix leftover references to `policy/fees.h` 5f1b016bebd2 depends: static libxcb-util-image 98a2fbbe70b8 depends: static libxkbcommon 1412baf77295 depends: static libxcb-util-wm a4009dadf466 depends: static libxcb-keysyms bcfb8679b3ba depends: static libxcb-render-util faf99ae37963 refactor: Avoid -W*-whitespace in git archive 2594d5a189e5 build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings 310e4979b36c qt: Added test coverage for qt gui#901 console history filter 0dd8d5c237e2 cmake: Specify Windows plugin path in `test_bitcoin-qt` property b0a38871546d scripted-diff: fix leftover references to `policy/fees.h` 48d4b936e09f Merge bitcoin/bitcoin#33511: init: Fix Ctrl-C shutdown hangs during wait calls 3c3c6adb7260 Merge bitcoin/bitcoin#33745: mining: check witness commitment in submitBlock e652b69b8da4 Merge bitcoin/bitcoin#33003: test: add option to skip large re-org test in feature_block 3e9aca6f1b52 depends: drop qtbase-moc-ignore-gcc-macro.patch qt patch fac4f6de28e7 ci: Rewrite lint task Bash snippet to Python fa0d37a57985 ci: Rewrite Bash to check inputs to Python 0da5a82700e9 depends: drop unused qt patch fae83611b8ef ci: [refactor] Use --preset=dev-mode in mac_native task fadb67b4b4e1 ci: [refactor] Base nowallet task on --preset=dev-mode 6666980e8653 ci: Enable bitcoin-chainstate and test_bitcoin-qt in win64 task 096924d39d64 kernel: add btck_block_tree_entry_equals faff7b231246 ci: Enable experimental kernel stuff in i686 task fa1632eecf58 ci: Enable experimental kernel stuff in mac-cross tasks fad10ff7c923 ci: Enable experimental kernel stuff in armhf task fa9d67c13d0d ci: Enable experimental kernel stuff in Alpine task fab3fb83026e ci: Enable experimental kernel stuff in s390x task fa7da8a646ed ci: Enable experimental kernel stuff in valgrind task fa9c2973d60b ci: Enable experimental kernel stuff in TSan task fad30d439502 ci: Enable experimental kernel stuff in MSan task d5ed9cb3eb52 Add accessor for sigops-adjusted weight 1bf3b513966e Add sigops adjusted weight calculator c18c68a950d3 Create a txgraph inside CTxMemPool 29a94d5b2f26 Make CTxMemPoolEntry derive from TxGraph::Ref 92b0079fe386 Allow moving CTxMemPoolEntry objects, disallow copying f6ec3519a330 init: Require explicit -asmap filename 6eaa00fe2020 test: clarify submitBlock() mutates the template 862bd432837e mining: ensure witness commitment check in submitBlock 00d1b6ef4b12 doc: clarify UpdateUncommittedBlockStructures 929f69d0ff29 qt: Remove HD seed reference from blank wallet tooltip 1db74914706f depends: sqlite 3.50.4 286f3e49c84c guix: sqlite wants tcl de7c3587cd45 doc: Update add checksum instructions in tutorial 6c73e4744837 mempool: Store iterators into mapTx in mapNextTx 51430680ecb7 Allow moving an Epoch::Marker 2a46e94a1600 doc: Update multisig-tutorial.md to use multipath descriptors c25a5e670b27 init: Signal m_tip_block_cv on Ctrl-C f53dbbc5057b test: Add functional tests for named argument parsing 694f04e2bd34 rpc: Handle -named argument parsing where '=' character is used 6a29f79006a9 test: Test SIGTERM handling during waitforblockheight call 8810642b571e test: add option to skip large re-org test in feature_block d31158d3646f psbt: clarify PSBT, PSBTInput, PSBTOutput unserialization flows 28a4fcb03c0f test: check listdescriptors do not return a mix of hardened derivation marker 975783cb79e9 descriptor: account for all StringType in MiniscriptDescriptor::ToStringHelper() git-subtree-dir: libbitcoinkernel-sys/bitcoin git-subtree-split: 6356041e58d1ba86695e2e7c219c68ee5abe583f
30367c1b3e kernel: Add support for block headers 4eda3a00da kernel: Add Handle/View pattern for BlockValidationState 0690514d4f Merge bitcoin/bitcoin#33770: init: Require explicit -asmap filename b2f88b53e0 Merge bitcoin/bitcoin#33286: doc: update multisig tutorial to use multipath descriptors 313cdd2bfb Merge bitcoin/bitcoin#33915: test: Retry download in get_previous_releases.py 17072f7005 Merge bitcoin/bitcoin#33912: clang-format: Set PackConstructorInitializers: CurrentLine 6b2d17b132 Merge bitcoin/bitcoin#33888: ci: Re-enable LINT_CI_SANITY_CHECK_COMMIT_SIG ac71df4338 Merge bitcoin/bitcoin#33870: refactor: remove incorrect lifetimebounds 6cdb51c14e Merge bitcoin/bitcoin#33887: doc: Improve CI docs on env and qemu-user-static 29c37651c7 Merge bitcoin/bitcoin#33880: test: Fix race condition in IPC interface block progation test 32368cd3e9 Merge bitcoin/bitcoin#33905: ci: Consistenly only cache on the default branch e55c49f851 Merge bitcoin/bitcoin#33851: depends: update xcb-util packages to latest versions a07bd8415d Merge bitcoin/bitcoin#33824: ci: Enable experimental kernel stuff in most CI tasks via `dev-mode` f541b92cf2 depends: expat 2.7.3 fad06f3bb4 test: retry download in get_previous_releases.py 2ebf4356e6 depends: libxcb 1.17.0 ba7ac870a3 depends: xcb_proto 1.17.0 fad0c76d0a clang-format: Set PackConstructorInitializers: CurrentLine 42d0692f91 depends: libxcb-util-cursor 0.1.6 25b85919ab depends: libxcb 1.15 d129384ca9 depends: libxcb-util-wm 0.4.2 0b857ae9e5 depends: libxcb-util-renderutil 0.3.10 35e50488b2 depends: libxcb-util-keysyms 0.4.1 74b68ad28b depends: libxcb-util-image 0.4.1 5bc0dde85d depends: libxcb-util 0.4.1 8d07292c28 depends: libXau 1.0.12 1af46cff94 Merge bitcoin/bitcoin#33896: clang-format: Set InsertNewlineAtEOF: true 27ac11ea0a Merge bitcoin/bitcoin#33867: kernel: handle null or empty directories in implementation 2578e6fc0f test: Fix race condition in IPC interface block propagation test 288b8c30be doc: Drop (default: none) from -i2psam description 509dc91db1 Merge bitcoin/bitcoin#33026: test, refactor: Embedded ASMap [1/3]: Selected minor preparatory work b126f98194 Merge bitcoin-core/gui#910: Added test coverage for qt gui#901 console history filter 7d7b829c36 Merge bitcoin-core/gui#908: Remove HD seed reference from blank wallet tooltip 53b72372da Merge bitcoin/bitcoin#31734: miniscript: account for all `StringType` variants in `Miniscriptdescriptor::ToString()` a7f9bbe4c5 Merge bitcoin/bitcoin#32821: rpc: Handle -named argument parsing where '=' character is used 55555db055 doc: Add missing --platform=linux to docker build command fa0ce4c148 ci: Re-enable LINT_CI_SANITY_CHECK_COMMIT_SIG faa0973de2 ci: [refactor] Rename CIRRUS_PR env var to LINT_CI_IS_PR fa411f938e ci: Consistenly only cache on the default branch 552eb90071 doc: CI - Describe qemu-user-static usage 2afbbddee5 doc: CI - Clarify how important `env -i` is and why 2444488f6a Merge bitcoin/bitcoin#33894: net: Remove unused `local_socket_bytes` variable in `CConnman::GetAddresses()` fa1bf6818f clang-format: Set InsertNewlineAtEOF: true 115d298a9f Merge bitcoin/bitcoin#33872: init: completely remove `-maxorphantx` option a90f3922ff Merge bitcoin/bitcoin#32419: psbt: clarify PSBT, PSBTInput, PSBTOutput unserialization flows 4d893c0f46 net: Remove unused `local_socket_bytes` variable in `CConnman::GetAddresses()` fa1dacaebe ci: Move lint exec snippet to stand-alone py file ead849c9f1 Merge bitcoin/bitcoin#33886: test: Remove tests violating hardened std::span c03081fdb4 Merge bitcoin/bitcoin#33776: ci: Lint follow-ups fadb4f63cb test: Remove tests violating hardened std::span 6e21558160 Merge bitcoin/bitcoin#33869: refactor: Avoid -W*-whitespace in git archive c8715aca95 Merge bitcoin/bitcoin#33247: build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings ee5de407e3 Merge bitcoin/bitcoin#33537: guix: build `bitcoin-qt` with static libxcb & utils 024a787350 Merge bitcoin/bitcoin#33876: doc: Update NetBSD Build Guide c29eaeeaf9 doc: Update NetBSD Build Guide 7f318e1dd0 test: Add better coverage for Autofile size() b7af960eb8 refactor: Add AutoFile::size ec0f75862e refactor: Modernize logging in util/asmap.cpp 606a251e0a tests: add unit test vectors for asmap interpreter 6657bcbdb4 kernel: allow null data_directory 0aebdac95d init: completely remove `-maxorphantx` option 99d012ec80 refactor: return reference instead of pointer f743e6c5dd refactor: add missing LIFETIMEBOUND annotation for parameter fa95353902 ci: Run macos tasks in a git archive, not git checkout 141117f5e8 refactor: remove incorrect LIFETIMEBOUND annotations fae3618fd6 ci: Annotate all check runs with the pull request number faf05d637d ci: Retry lint image building once after failure 96963b888e depends: static libxcb ad06843fab depends: avoid qdbusviewer in Qt build 6848ed56dc depends: apply Qt patches to fix static libxcb use 5f1b016beb depends: static libxcb-util-image 98a2fbbe70 depends: static libxkbcommon 1412baf772 depends: static libxcb-util-wm a4009dadf4 depends: static libxcb-keysyms bcfb8679b3 depends: static libxcb-render-util faf99ae379 refactor: Avoid -W*-whitespace in git archive 2594d5a189 build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings 310e4979b3 qt: Added test coverage for qt gui#901 console history filter fac4f6de28 ci: Rewrite lint task Bash snippet to Python fa0d37a579 ci: Rewrite Bash to check inputs to Python fae83611b8 ci: [refactor] Use --preset=dev-mode in mac_native task fadb67b4b4 ci: [refactor] Base nowallet task on --preset=dev-mode 6666980e86 ci: Enable bitcoin-chainstate and test_bitcoin-qt in win64 task faff7b2312 ci: Enable experimental kernel stuff in i686 task fa1632eecf ci: Enable experimental kernel stuff in mac-cross tasks fad10ff7c9 ci: Enable experimental kernel stuff in armhf task fa9d67c13d ci: Enable experimental kernel stuff in Alpine task fab3fb8302 ci: Enable experimental kernel stuff in s390x task fa7da8a646 ci: Enable experimental kernel stuff in valgrind task fa9c2973d6 ci: Enable experimental kernel stuff in TSan task fad30d4395 ci: Enable experimental kernel stuff in MSan task f6ec3519a3 init: Require explicit -asmap filename 929f69d0ff qt: Remove HD seed reference from blank wallet tooltip de7c3587cd doc: Update add checksum instructions in tutorial 2a46e94a16 doc: Update multisig-tutorial.md to use multipath descriptors f53dbbc505 test: Add functional tests for named argument parsing 694f04e2bd rpc: Handle -named argument parsing where '=' character is used d31158d364 psbt: clarify PSBT, PSBTInput, PSBTOutput unserialization flows 28a4fcb03c test: check listdescriptors do not return a mix of hardened derivation marker 975783cb79 descriptor: account for all StringType in MiniscriptDescriptor::ToStringHelper() git-subtree-dir: bitcoinkernel/bitcoin git-subtree-split: 30367c1b3e7e94ed59d0a3ee816da14a3ee0424f


Addresses comment and this.
The PR #31375 got merged and enables
-namedby default in thebitcoin rpcinterface;bitcoin rpccorresponds tobitcoin-cli -namedas 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 arefinalizepsbt,decodepsbt,verifymessageetc.This is the one example of the error in
finalizepsbtRPC:This PR fixes this by updating the
vRPCConvertParamstable that identifies parameters that need special handling in-namedparameter mode. The parser now recognises these parameters and handles strings with "=" char correctly, preventing them from being incorrectly split as parameter assignments.