diff --git a/src/bitcoin.cpp b/src/bitcoin.cpp index a8ebab982fe..37b6fdcac22 100644 --- a/src/bitcoin.cpp +++ b/src/bitcoin.cpp @@ -91,9 +91,8 @@ int main(int argc, char* argv[]) // Since "bitcoin rpc" is a new interface that doesn't need to be // backward compatible, enable -named by default so it is convenient // for callers to use a mix of named and unnamed parameters. Callers - // can override this by specifying -nonamed, but should not need to - // unless they are passing string values containing '=' characters - // as unnamed parameters. + // can override this by specifying -nonamed, but it handles parameters + // that contain '=' characters, so -nonamed should rarely be needed. args.emplace_back("-named"); } else if (cmd.command == "wallet") { args.emplace_back("bitcoin-wallet"); diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index e462a26d97e..77e5ec08052 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -12,19 +12,45 @@ #include #include +//! Specify whether parameter should be parsed by bitcoin-cli as a JSON value, +//! or passed unchanged as a string, or a combination of both. +enum ParamFormat { JSON, STRING, JSON_OR_STRING }; + class CRPCConvertParam { public: std::string methodName; //!< method whose params want conversion int paramIdx; //!< 0-based idx of param to convert std::string paramName; //!< parameter name - bool also_string{false}; //!< The parameter is also a string + ParamFormat format{ParamFormat::JSON}; //!< parameter format }; // clang-format off /** - * Specify a (method, idx, name) here if the argument is a non-string RPC - * argument and needs to be converted from JSON. + * Specify a (method, idx, name, format) here if the argument is a non-string RPC + * argument and needs to be converted from JSON, or if it is a string argument + * passed to a method that accepts '=' characters in any string arguments. + * + * JSON parameters need to be listed here to make bitcoin-cli parse command line + * arguments as JSON, instead of passing them as raw strings. `JSON` and + * `JSON_OR_STRING` formats both make `bitcoin-cli` attempt to parse the + * argument as JSON. But if parsing fails, the former triggers an error while + * the latter falls back to passing the argument as a raw string. This is + * useful for arguments like hash_or_height, allowing invocations such as + * `bitcoin-cli getblockstats ` without needing to quote the hash string + * as JSON (`'""'`). + * + * String parameters that may contain an '=' character (e.g. base64 strings, + * filenames, or labels) need to be listed here with format `ParamFormat::STRING` + * to make bitcoin-cli treat them as positional parameters when `-named` is used. + * This prevents `bitcoin-cli` from splitting strings like "my=wallet" into a named + * argument "my" and value "wallet" when the whole string is intended to be a + * single positional argument. And if one string parameter is listed for a method, + * other string parameters for that method need to be listed as well so bitcoin-cli + * does not make the opposite mistake and pass other arguments by position instead of + * name because it does not recognize their names. See \ref RPCConvertNamedValues + * for more information on how named and positional arguments are distinguished with + * -named. * * @note Parameter indexes start from 0. */ @@ -32,6 +58,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { { "setmocktime", 0, "timestamp" }, { "mockscheduler", 0, "delta_time" }, + { "utxoupdatepsbt", 0, "psbt", ParamFormat::STRING }, { "utxoupdatepsbt", 1, "descriptors" }, { "generatetoaddress", 0, "nblocks" }, { "generatetoaddress", 2, "maxtries" }, @@ -41,16 +68,21 @@ static const CRPCConvertParam vRPCConvertParams[] = { "generateblock", 2, "submit" }, { "getnetworkhashps", 0, "nblocks" }, { "getnetworkhashps", 1, "height" }, + { "sendtoaddress", 0, "address", ParamFormat::STRING }, { "sendtoaddress", 1, "amount" }, + { "sendtoaddress", 2, "comment", ParamFormat::STRING }, + { "sendtoaddress", 3, "comment_to", ParamFormat::STRING }, { "sendtoaddress", 4, "subtractfeefromamount" }, { "sendtoaddress", 5 , "replaceable" }, { "sendtoaddress", 6 , "conf_target" }, + { "sendtoaddress", 7, "estimate_mode", ParamFormat::STRING }, { "sendtoaddress", 8, "avoid_reuse" }, { "sendtoaddress", 9, "fee_rate"}, { "sendtoaddress", 10, "verbose"}, { "settxfee", 0, "amount" }, { "getreceivedbyaddress", 1, "minconf" }, { "getreceivedbyaddress", 2, "include_immature_coinbase" }, + { "getreceivedbylabel", 0, "label", ParamFormat::STRING }, { "getreceivedbylabel", 1, "minconf" }, { "getreceivedbylabel", 2, "include_immature_coinbase" }, { "listreceivedbyaddress", 0, "minconf" }, @@ -70,20 +102,27 @@ static const CRPCConvertParam vRPCConvertParams[] = { "waitforblockheight", 1, "timeout" }, { "waitforblock", 1, "timeout" }, { "waitfornewblock", 0, "timeout" }, + { "listtransactions", 0, "label", ParamFormat::STRING }, { "listtransactions", 1, "count" }, { "listtransactions", 2, "skip" }, { "listtransactions", 3, "include_watchonly" }, + { "walletpassphrase", 0, "passphrase", ParamFormat::STRING }, { "walletpassphrase", 1, "timeout" }, { "getblocktemplate", 0, "template_request" }, + { "listsinceblock", 0, "blockhash", ParamFormat::STRING }, { "listsinceblock", 1, "target_confirmations" }, { "listsinceblock", 2, "include_watchonly" }, { "listsinceblock", 3, "include_removed" }, { "listsinceblock", 4, "include_change" }, + { "listsinceblock", 5, "label", ParamFormat::STRING }, + { "sendmany", 0, "dummy", ParamFormat::STRING }, { "sendmany", 1, "amounts" }, { "sendmany", 2, "minconf" }, + { "sendmany", 3, "comment", ParamFormat::STRING }, { "sendmany", 4, "subtractfeefrom" }, { "sendmany", 5 , "replaceable" }, { "sendmany", 6 , "conf_target" }, + { "sendmany", 7, "estimate_mode", ParamFormat::STRING }, { "sendmany", 8, "fee_rate"}, { "sendmany", 9, "verbose" }, { "deriveaddresses", 1, "range" }, @@ -170,10 +209,14 @@ static const CRPCConvertParam vRPCConvertParams[] = { "walletcreatefundedpsbt", 3, "max_tx_weight"}, { "walletcreatefundedpsbt", 4, "bip32derivs" }, { "walletcreatefundedpsbt", 5, "version" }, + { "walletprocesspsbt", 0, "psbt", ParamFormat::STRING }, { "walletprocesspsbt", 1, "sign" }, + { "walletprocesspsbt", 2, "sighashtype", ParamFormat::STRING }, { "walletprocesspsbt", 3, "bip32derivs" }, { "walletprocesspsbt", 4, "finalize" }, + { "descriptorprocesspsbt", 0, "psbt", ParamFormat::STRING }, { "descriptorprocesspsbt", 1, "descriptors"}, + { "descriptorprocesspsbt", 2, "sighashtype", ParamFormat::STRING }, { "descriptorprocesspsbt", 3, "bip32derivs" }, { "descriptorprocesspsbt", 4, "finalize" }, { "createpsbt", 0, "inputs" }, @@ -183,16 +226,19 @@ static const CRPCConvertParam vRPCConvertParams[] = { "createpsbt", 4, "version" }, { "combinepsbt", 0, "txs"}, { "joinpsbts", 0, "txs"}, + { "finalizepsbt", 0, "psbt", ParamFormat::STRING }, { "finalizepsbt", 1, "extract"}, { "converttopsbt", 1, "permitsigdata"}, { "converttopsbt", 2, "iswitness"}, { "gettxout", 1, "n" }, { "gettxout", 2, "include_mempool" }, { "gettxoutproof", 0, "txids" }, - { "gettxoutsetinfo", 1, "hash_or_height", /*also_string=*/true }, + { "gettxoutsetinfo", 1, "hash_or_height", ParamFormat::JSON_OR_STRING }, { "gettxoutsetinfo", 2, "use_index"}, + { "dumptxoutset", 0, "path", ParamFormat::STRING }, + { "dumptxoutset", 1, "type", ParamFormat::STRING }, { "dumptxoutset", 2, "options" }, - { "dumptxoutset", 2, "rollback", /*also_string=*/true }, + { "dumptxoutset", 2, "rollback", ParamFormat::JSON_OR_STRING }, { "lockunspent", 0, "unlock" }, { "lockunspent", 1, "transactions" }, { "lockunspent", 2, "persistent" }, @@ -239,6 +285,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "simulaterawtransaction", 0, "rawtxs" }, { "simulaterawtransaction", 1, "options" }, { "simulaterawtransaction", 1, "include_watchonly"}, + { "importmempool", 0, "filepath", ParamFormat::STRING }, { "importmempool", 1, "options" }, { "importmempool", 1, "apply_fee_delta_priority" }, { "importmempool", 1, "use_current_time" }, @@ -247,7 +294,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "listdescriptors", 0, "private" }, { "verifychain", 0, "checklevel" }, { "verifychain", 1, "nblocks" }, - { "getblockstats", 0, "hash_or_height", /*also_string=*/true }, + { "getblockstats", 0, "hash_or_height", ParamFormat::JSON_OR_STRING }, { "getblockstats", 1, "stats" }, { "pruneblockchain", 0, "height" }, { "keypoolrefill", 0, "newsize" }, @@ -299,14 +346,20 @@ static const CRPCConvertParam vRPCConvertParams[] = { "echojson", 9, "arg9" }, { "rescanblockchain", 0, "start_height"}, { "rescanblockchain", 1, "stop_height"}, + { "createwallet", 0, "wallet_name", ParamFormat::STRING }, { "createwallet", 1, "disable_private_keys"}, { "createwallet", 2, "blank"}, + { "createwallet", 3, "passphrase", ParamFormat::STRING }, { "createwallet", 4, "avoid_reuse"}, { "createwallet", 5, "descriptors"}, { "createwallet", 6, "load_on_startup"}, { "createwallet", 7, "external_signer"}, + { "restorewallet", 0, "wallet_name", ParamFormat::STRING }, + { "restorewallet", 1, "backup_file", ParamFormat::STRING }, { "restorewallet", 2, "load_on_startup"}, + { "loadwallet", 0, "filename", ParamFormat::STRING }, { "loadwallet", 1, "load_on_startup"}, + { "unloadwallet", 0, "wallet_name", ParamFormat::STRING }, { "unloadwallet", 1, "load_on_startup"}, { "getnodeaddresses", 0, "count"}, { "addpeeraddress", 1, "port"}, @@ -315,72 +368,108 @@ static const CRPCConvertParam vRPCConvertParams[] = { "stop", 0, "wait" }, { "addnode", 2, "v2transport" }, { "addconnection", 2, "v2transport" }, + { "decodepsbt", 0, "psbt", ParamFormat::STRING }, + { "analyzepsbt", 0, "psbt", ParamFormat::STRING}, + { "verifymessage", 1, "signature", ParamFormat::STRING }, + { "verifymessage", 2, "message", ParamFormat::STRING }, + { "getnewaddress", 0, "label", ParamFormat::STRING }, + { "getnewaddress", 1, "address_type", ParamFormat::STRING }, + { "backupwallet", 0, "destination", ParamFormat::STRING }, + { "echoipc", 0, "arg", ParamFormat::STRING }, + { "encryptwallet", 0, "passphrase", ParamFormat::STRING }, + { "getaddressesbylabel", 0, "label", ParamFormat::STRING }, + { "loadtxoutset", 0, "path", ParamFormat::STRING }, + { "migratewallet", 0, "wallet_name", ParamFormat::STRING }, + { "migratewallet", 1, "passphrase", ParamFormat::STRING }, + { "setlabel", 1, "label", ParamFormat::STRING }, + { "signmessage", 1, "message", ParamFormat::STRING }, + { "signmessagewithprivkey", 1, "message", ParamFormat::STRING }, + { "walletpassphrasechange", 0, "oldpassphrase", ParamFormat::STRING }, + { "walletpassphrasechange", 1, "newpassphrase", ParamFormat::STRING }, }; // clang-format on /** Parse string to UniValue or throw runtime_error if string contains invalid JSON */ -static UniValue Parse(std::string_view raw, bool also_string) +static UniValue Parse(std::string_view raw, ParamFormat format = ParamFormat::JSON) { UniValue parsed; if (!parsed.read(raw)) { - if (!also_string) throw std::runtime_error(tfm::format("Error parsing JSON: %s", raw)); - return raw; + if (format != ParamFormat::JSON_OR_STRING) throw std::runtime_error(tfm::format("Error parsing JSON: %s", raw)); + return UniValue(std::string(raw)); } return parsed; } -class CRPCConvertTable +namespace rpc_convert { -private: - std::map, bool> members; - std::map, bool> membersByName; - -public: - CRPCConvertTable(); - - /** Return arg_value as UniValue, and first parse it if it is a non-string parameter */ - UniValue ArgToUniValue(std::string_view arg_value, const std::string& method, int param_idx) - { - const auto& it = members.find({method, param_idx}); - if (it != members.end()) { - return Parse(arg_value, it->second); - } - return arg_value; - } - - /** Return arg_value as UniValue, and first parse it if it is a non-string parameter */ - UniValue ArgToUniValue(std::string_view arg_value, const std::string& method, const std::string& param_name) - { - const auto& it = membersByName.find({method, param_name}); - if (it != membersByName.end()) { - return Parse(arg_value, it->second); - } - return arg_value; - } -}; - -CRPCConvertTable::CRPCConvertTable() +const CRPCConvertParam* FromPosition(std::string_view method, size_t pos) { - for (const auto& cp : vRPCConvertParams) { - members.emplace(std::make_pair(cp.methodName, cp.paramIdx), cp.also_string); - membersByName.emplace(std::make_pair(cp.methodName, cp.paramName), cp.also_string); - } + auto it = std::ranges::find_if(vRPCConvertParams, [&](const auto& p) { + return p.methodName == method && p.paramIdx == static_cast(pos); + }); + + return it == std::end(vRPCConvertParams) ? nullptr : &*it; } -static CRPCConvertTable rpcCvtTable; +const CRPCConvertParam* FromName(std::string_view method, std::string_view name) +{ + auto it = std::ranges::find_if(vRPCConvertParams, [&](const auto& p) { + return p.methodName == method && p.paramName == name; + }); + return it == std::end(vRPCConvertParams) ? nullptr : &*it; +} +} // namespace rpc_convert + +static UniValue ParseParam(const CRPCConvertParam* param, std::string_view raw) +{ + // Only parse parameters which have the JSON or JSON_OR_STRING format; otherwise, treat them as strings. + return (param && (param->format == ParamFormat::JSON || param->format == ParamFormat::JSON_OR_STRING)) ? Parse(raw, param->format) : UniValue(std::string(raw)); +} + +/** + * Convert command lines arguments to params object when -named is disabled. + */ UniValue RPCConvertValues(const std::string &strMethod, const std::vector &strParams) { UniValue params(UniValue::VARR); - for (unsigned int idx = 0; idx < strParams.size(); idx++) { - std::string_view value{strParams[idx]}; - params.push_back(rpcCvtTable.ArgToUniValue(value, strMethod, idx)); + for (std::string_view s : strParams) { + params.push_back(ParseParam(rpc_convert::FromPosition(strMethod, params.size()), s)); } return params; } +/** + * Convert command line arguments to params object when -named is enabled. + * + * The -named syntax accepts named arguments in NAME=VALUE format, as well as + * positional arguments without names. The syntax is inherently ambiguous if + * names are omitted and values contain '=', so a heuristic is used to + * disambiguate: + * + * - Arguments that do not contain '=' are treated as positional parameters. + * + * - Arguments that do contain '=' are assumed to be named parameters in + * NAME=VALUE format except for two special cases: + * + * 1. The case where NAME is not a known parameter name, and the next + * positional parameter requires a JSON value, and the argument parses as + * JSON. E.g. ["list", "with", "="]. + * + * 2. The case where NAME is not a known parameter name and the next + * positional parameter requires a string value. E.g. "my=wallet". + * + * For example, the command `bitcoin-cli -named createwallet "my=wallet"`, + * the parser initially sees "my=wallet" and attempts to process it as a + * parameter named "my". When it finds that "my" is not a valid named parameter + * parameter for this method, it falls back to checking the rule for the + * next available positional parameter (index 0). Because it finds the rule + * that this parameter is a ParamFormat::STRING, it correctly treats the entire + * "my=wallet" as a single positional string, successfully creating a + * wallet with that literal name. + */ UniValue RPCConvertNamedValues(const std::string &strMethod, const std::vector &strParams) { UniValue params(UniValue::VOBJ); @@ -388,18 +477,31 @@ UniValue RPCConvertNamedValues(const std::string &strMethod, const std::vectorformat == 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; + } + } + // Intentionally overwrite earlier named values with later ones as a // convenience for scripts and command line users that want to merge // options. - params.pushKV(name, rpcCvtTable.ArgToUniValue(value, strMethod, name)); + params.pushKV(name, ParseParam(named_param, value)); } if (!positional_args.empty()) { diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index 734f3110af3..1d917749a56 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -94,8 +94,30 @@ class TestBitcoinCli(BitcoinTestFramework): assert re.match(rf"^{re.escape(self.config['environment']['CLIENT_NAME'])} client.+services nwl2?$", det[0]) assert not any(line.startswith("Local services:") for line in det) + def test_echojson_positional_equals(self): + """Test JSON parameter parsing containing '=' with -named echojson""" + self.log.info("Test JSON parameter parsing containing '=' is handled correctly with -named") + + # This should be treated as a positional JSON argument, not as a named + result = self.nodes[0].cli("-named", "echojson", '["key=value"]').send_cli() + assert_equal(result, [["key=value"]]) + + result = self.nodes[0].cli("-named", "echojson", '["key=value", "another=test"]').send_cli() + assert_equal(result, [["key=value", "another=test"]]) + + result = self.nodes[0].cli("-named", "echojson", '["data=test"]', "42").send_cli() + expected = [["data=test"], 42] + assert_equal(result, expected) + + # This should be treated as a named parameter, as arg0 and arg1 are valid parameter names + result = self.nodes[0].cli("-named", "echojson", 'arg0=["data=test"]', 'arg1=42').send_cli() + expected = [["data=test"], 42] + assert_equal(result, expected) + def run_test(self): """Main test logic""" + self.test_echojson_positional_equals() + self.generate(self.nodes[0], BLOCKS) self.log.info("Compare responses from getblockchaininfo RPC and `bitcoin-cli getblockchaininfo`") diff --git a/test/functional/rpc_help.py b/test/functional/rpc_help.py index 02279ad2f7f..74d34ddca1a 100755 --- a/test/functional/rpc_help.py +++ b/test/functional/rpc_help.py @@ -17,30 +17,40 @@ def parse_string(s): assert s[-1] == '"' return s[1:-1] - def process_mapping(fname): """Find and parse conversion table in implementation file `fname`.""" cmds = [] + string_params = [] in_rpcs = False with open(fname, "r", encoding="utf8") as f: for line in f: line = line.rstrip() if not in_rpcs: - if line == 'static const CRPCConvertParam vRPCConvertParams[] =': + if re.match(r'static const CRPCConvertParam vRPCConvertParams\[] =', line): in_rpcs = True else: if line.startswith('};'): in_rpcs = False elif '{' in line and '"' in line: - m = re.search(r'{ *("[^"]*"), *([0-9]+) *, *("[^"]*") *(, \/\*also_string=\*\/true *)?},', line) - assert m, 'No match to table expression: %s' % line - name = parse_string(m.group(1)) - idx = int(m.group(2)) - argname = parse_string(m.group(3)) - cmds.append((name, idx, argname)) - assert not in_rpcs and cmds - return cmds + # Match lines with ParamFormat::STRING + m_string = re.search(r'{ *("[^"]*") *, *([0-9]+) *, *("[^"]*") *, *ParamFormat::STRING *},?', line) + if m_string: + name = parse_string(m_string.group(1)) + idx = int(m_string.group(2)) + argname = parse_string(m_string.group(3)) + string_params.append((name, idx, argname)) + continue + # Match lines with ParamFormat::JSON and ParamFormat::JSON_OR_STRING + m_json = re.search(r'{ *("[^"]*") *, *([0-9]+) *, *("[^"]*") *(?:, *ParamFormat::(JSON_OR_STRING|JSON))? *},?', line) + if m_json: + name = parse_string(m_json.group(1)) + idx = int(m_json.group(2)) + argname = parse_string(m_json.group(3)) + cmds.append((name, idx, argname)) + + assert not in_rpcs + return cmds, string_params class HelpRpcTest(BitcoinTestFramework): def set_test_params(self): @@ -49,6 +59,7 @@ class HelpRpcTest(BitcoinTestFramework): def run_test(self): self.test_client_conversion_table() + self.test_client_string_conversion_table() self.test_categories() self.dump_help() if self.is_wallet_compiled(): @@ -56,7 +67,7 @@ class HelpRpcTest(BitcoinTestFramework): def test_client_conversion_table(self): file_conversion_table = os.path.join(self.config["environment"]["SRCDIR"], 'src', 'rpc', 'client.cpp') - mapping_client = process_mapping(file_conversion_table) + mapping_client, _ = process_mapping(file_conversion_table) # Ignore echojson in client table mapping_client = [m for m in mapping_client if m[0] != 'echojson'] @@ -85,6 +96,29 @@ class HelpRpcTest(BitcoinTestFramework): # Only allow dummy and psbt to fail consistency check assert argname in ['dummy', "psbt"], ('WARNING: conversion mismatch for argument named %s (%s)' % (argname, list(zip(all_methods_by_argname[argname], converts_by_argname[argname])))) + def test_client_string_conversion_table(self): + file_conversion_table = os.path.join(self.config["environment"]["SRCDIR"], 'src', 'rpc', 'client.cpp') + _, string_params_client = process_mapping(file_conversion_table) + mapping_server = self.nodes[0].help("dump_all_command_conversions") + server_tuples = {tuple(m[:3]) for m in mapping_server} + + # Filter string parameters based on wallet compilation status + if self.is_wallet_compiled(): + # Check that every entry in string parameters exists on the server + stale_entries = [entry for entry in string_params_client if entry not in server_tuples] + if stale_entries: + raise AssertionError(f"String parameters contains entries not present on the server: {stale_entries}") + filtered_string_params = string_params_client + else: + available_string_params = [entry for entry in string_params_client if entry in server_tuples] + filtered_string_params = available_string_params + + # Validate that all entries are legitimate server parameters + server_method_param_tuples = {(m[0], m[1], m[2]) for m in mapping_server} + invalid_entries = [entry for entry in filtered_string_params if entry not in server_method_param_tuples] + if invalid_entries: + raise AssertionError(f"String parameters contains invalid entries: {invalid_entries}") + def test_categories(self): node = self.nodes[0] @@ -128,6 +162,5 @@ class HelpRpcTest(BitcoinTestFramework): self.restart_node(0, extra_args=['-nowallet=1']) assert 'getnewaddress ( "label" "address_type" )' in self.nodes[0].help('getnewaddress') - if __name__ == '__main__': HelpRpcTest(__file__).main() diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 5acaaa71a6a..3943abcec40 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -369,6 +369,43 @@ class PSBTTest(BitcoinTestFramework): changepos = psbtx["changepos"] assert_equal(decoded_psbt["tx"]["vout"][changepos]["scriptPubKey"]["type"], expected_type) + def test_psbt_named_parameter_handling(self): + """Test that PSBT Base64 parameters with '=' padding are handled correctly in -named mode""" + self.log.info("Testing PSBT Base64 parameter handling with '=' padding characters") + node = self.nodes[0] + psbt_with_padding = "cHNidP8BAJoCAAAAAqvNEjSrzRI0q80SNKvNEjSrzRI0q80SNKvNEjSrzRI0AAAAAAD9////NBLNqzQSzas0Es2rNBLNqzQSzas0Es2rNBLNqzQSzasBAAAAAP3///8CoIYBAAAAAAAWABQVQBGVs/sqFAmC8HZ8O+g1htqivkANAwAAAAAAFgAUir7MzgyzDnRMjdkVa7d+Dwr07jsAAAAAAAAAAAA=" + + # Test decodepsbt with explicit named parameter containing '=' padding + result = node.cli("-named", "decodepsbt", f"psbt={psbt_with_padding}").send_cli() + assert 'tx' in result + + # Test decodepsbt with positional argument containing '=' padding + result = node.cli("-named", "decodepsbt", psbt_with_padding).send_cli() + assert 'tx' in result + + # Test analyzepsbt with positional argument containing '=' padding + result = node.cli("-named", "analyzepsbt", psbt_with_padding).send_cli() + assert 'inputs' in result + + # Test finalizepsbt with positional argument containing '=' padding + result = node.cli("-named", "finalizepsbt", psbt_with_padding, "extract=true").send_cli() + assert 'complete' in result + + # Test walletprocesspsbt with positional argument containing '=' padding + result = node.cli("-named", "walletprocesspsbt", psbt_with_padding).send_cli() + assert 'complete' in result + + # Test utxoupdatepsbt with positional argument containing '=' padding + result = node.cli("-named", "utxoupdatepsbt", psbt_with_padding).send_cli() + assert isinstance(result, str) and len(result) > 0 + + # Test that unknown parameter with '=' gets treated as positional and return error + unknown_psbt_param = "unknown_param_data=more_data=" + # 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") + def run_test(self): # Create and fund a raw tx for sending 10 BTC psbtx1 = self.nodes[0].walletcreatefundedpsbt([], {self.nodes[2].getnewaddress():10})['psbt'] @@ -1211,6 +1248,7 @@ class PSBTTest(BitcoinTestFramework): if not self.options.usecli: self.test_sighash_mismatch() self.test_sighash_adding() + self.test_psbt_named_parameter_handling() if __name__ == '__main__': PSBTTest(__file__).main() diff --git a/test/functional/wallet_labels.py b/test/functional/wallet_labels.py index 068a4980db7..4a752486782 100755 --- a/test/functional/wallet_labels.py +++ b/test/functional/wallet_labels.py @@ -50,6 +50,27 @@ class WalletLabelsTest(BitcoinTestFramework): for rpc_call in rpc_calls: assert_raises_rpc_error(-11, "Invalid label name", *rpc_call, label="*") + def test_label_named_parameter_handling(self): + """Test that getnewaddress with labels containing '=' characters is handled correctly in -named mode""" + self.log.info("Test getnewaddress label parameter handling") + node = self.nodes[0] + + # Test getnewaddress with explicit named parameter containing '=' + label_with_equals = "wallet=wallet" + result = node.cli("-named", "getnewaddress", f"label={label_with_equals}").send_cli() + address = result.strip() + addr_info = node.getaddressinfo(address) + assert_equal(addr_info.get('labels', []), [label_with_equals]) + + self.log.info("Test bitcoin-cli -named passes parameter containing '=' by position if it does not specify a known parameter name and is in a string position") + equals_label = "my=label" + result = node.cli("-named", "getnewaddress", equals_label).send_cli() + address = result.strip() + addr_info = node.getaddressinfo(address) + assert_equal(addr_info.get('labels', []), [equals_label]) + + self.log.info("getnewaddress label parameter handling test completed successfully") + def run_test(self): # Check that there's no UTXO on the node node = self.nodes[0] @@ -159,6 +180,7 @@ class WalletLabelsTest(BitcoinTestFramework): change_label(node, labels[2].addresses[0], labels[2], labels[2]) self.invalid_label_name_test() + self.test_label_named_parameter_handling() # This is a descriptor wallet test because of segwit v1+ addresses self.log.info('Check watchonly labels')