diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -192,8 +192,7 @@ std::string error; auto desc = Parse(request.params[0].get_str(), provider, error); if (!desc) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, - strprintf("Invalid descriptor, %s", error)); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, error); } UniValue result(UniValue::VOBJ); @@ -257,8 +256,7 @@ std::string error; auto desc = Parse(desc_str, key_provider, error, /* require_checksum = */ true); if (!desc) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, - strprintf("Invalid descriptor, %s", error)); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, error); } if (!desc->IsRange() && request.params.size() > 1) { diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -756,9 +756,7 @@ std::string error; auto desc = Parse(desc_str, provider, error); if (!desc) { - throw JSONRPCError( - RPC_INVALID_ADDRESS_OR_KEY, - strprintf("Invalid descriptor '%s', %s", desc_str, error)); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, error); } if (!desc->IsRange()) { range.first = 0; diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -810,8 +810,11 @@ hardened = true; } uint32_t p; - if (!ParseUInt32(std::string(elem.begin(), elem.end()), &p) || - p > 0x7FFFFFFFUL) { + if (!ParseUInt32(std::string(elem.begin(), elem.end()), &p)) { + error = strprintf("Key path value '%s' is not a valid uint32", + std::string(elem.begin(), elem.end()).c_str()); + return false; + } else if (p > 0x7FFFFFFFUL) { error = strprintf("Key path value %u is out of range", p); return false; } @@ -826,6 +829,10 @@ std::string &error) { auto split = Split(sp, '/'); std::string str(split[0].begin(), split[0].end()); + if (str.size() == 0) { + error = "No key provided"; + return nullptr; + } if (split.size() == 1) { if (IsHex(str)) { std::vector data = ParseHex(str); @@ -833,6 +840,8 @@ if (pubkey.IsFullyValid()) { return std::make_unique(pubkey); } + error = strprintf("Pubkey '%s' is invalid", str); + return nullptr; } CKey key = DecodeSecret(str); if (key.IsValid()) { @@ -881,9 +890,9 @@ return ParsePubkeyInner(origin_split[0], out, error); } if (origin_split[0].size() < 1 || origin_split[0][0] != '[') { - error = strprintf( - "Key origin expected but not found, got '%s' instead", - std::string(origin_split[0].begin(), origin_split[0].end())); + error = strprintf("Key origin start '[ character expected but not " + "found, got '%c' instead", + origin_split[0][0]); return nullptr; } auto slash_split = Split(origin_split[0].subspan(1), '/'); @@ -941,6 +950,9 @@ return nullptr; } return std::make_unique(std::move(pubkey)); + } else if (ctx != ParseScriptContext::TOP && Func("combo", expr)) { + error = "Cannot have combo in non-top level"; + return nullptr; } if (Func("multi", expr)) { auto threshold = Expr(expr); @@ -948,13 +960,15 @@ std::vector> providers; if (!ParseUInt32(std::string(threshold.begin(), threshold.end()), &thres)) { - error = strprintf("multi threshold %u out of range", thres); + error = strprintf( + "Multi threshold '%s' is not valid", + std::string(threshold.begin(), threshold.end()).c_str()); return nullptr; } size_t script_size = 0; while (expr.size()) { if (!Const(",", expr)) { - error = strprintf("multi: expected ',', got '%c'", expr[0]); + error = strprintf("Multi: expected ',', got '%c'", expr[0]); return nullptr; } auto arg = Expr(expr); @@ -965,14 +979,26 @@ script_size += pk->GetSize() + 1; providers.emplace_back(std::move(pk)); } - if (providers.size() < 1 || providers.size() > 16 || thres < 1 || - thres > providers.size()) { + if (providers.size() < 1 || providers.size() > 16) { + error = strprintf("Cannot have %u keys in multisig; must have " + "between 1 and 16 keys, inclusive", + providers.size()); + return nullptr; + } else if (thres < 1) { + error = strprintf( + "Multisig threshold cannot be %d, must be at least 1", thres); + return nullptr; + } else if (thres > providers.size()) { + error = + strprintf("Multisig threshold cannot be larger than the number " + "of keys; threshold is %d but only %u keys specified", + thres, providers.size()); return nullptr; } if (ctx == ParseScriptContext::TOP) { if (providers.size() > 3) { - error = strprintf("Cannot %u pubkeys in bare multisig; only at " - "most 3 pubkeys", + error = strprintf("Cannot have %u pubkeys in bare multisig; " + "only at most 3 pubkeys", providers.size()); return nullptr; } @@ -994,6 +1020,9 @@ return nullptr; } return std::make_unique(std::move(desc)); + } else if (ctx != ParseScriptContext::TOP && Func("sh", expr)) { + error = "Cannot have sh in non-top level"; + return nullptr; } if (ctx == ParseScriptContext::TOP && Func("addr", expr)) { CTxDestination dest = @@ -1014,6 +1043,10 @@ return std::make_unique( CScript(bytes.begin(), bytes.end())); } + if (ctx == ParseScriptContext::P2SH) { + error = "A function is needed within P2SH"; + return nullptr; + } error = strprintf("%s is not a valid descriptor function", std::string(expr.begin(), expr.end())); return nullptr; diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1270,8 +1270,7 @@ auto parsed_desc = Parse(descriptor, keys, error, /* require_checksum = */ true); if (!parsed_desc) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, - strprintf("Descriptor is invalid, %s", error)); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, error); } have_solving_data = parsed_desc->IsSolvable(); diff --git a/test/functional/rpc_deriveaddresses.py b/test/functional/rpc_deriveaddresses.py --- a/test/functional/rpc_deriveaddresses.py +++ b/test/functional/rpc_deriveaddresses.py @@ -14,7 +14,7 @@ self.supports_cli = 1 def run_test(self): - assert_raises_rpc_error(-5, "Invalid descriptor", + assert_raises_rpc_error(-5, "Missing checksum", self.nodes[0].deriveaddresses, "a") descriptor = "pkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/0)#rdfjd0a9" @@ -23,7 +23,7 @@ descriptor = descriptor[:-9] assert_raises_rpc_error(-5, - "Invalid descriptor", + "Missing checksum", self.nodes[0].deriveaddresses, descriptor) diff --git a/test/functional/wallet_importmulti.py b/test/functional/wallet_importmulti.py --- a/test/functional/wallet_importmulti.py +++ b/test/functional/wallet_importmulti.py @@ -477,7 +477,7 @@ "label": "Descriptor import test"}, success=False, error_code=-5, - error_message='Descriptor is invalid, Missing checksum') + error_message='Missing checksum') xpriv = "tprv8ZgxMBicQKsPeuVhWwi6wuMQGfPKi9Li5GtX35jVNknACgqe3CY4g5xgkfDDJcmtF7o1QnxWDRYw4H5P26PXq7sbcUkEqeR4fg3Kxp2tigg" # hdkeypath=m/0'/0'/0' and 1'