diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -189,10 +189,11 @@ RPCTypeCheck(request.params, {UniValue::VSTR}); FlatSigningProvider provider; - auto desc = Parse(request.params[0].get_str(), provider); + 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")); + strprintf("Invalid descriptor, %s", error)); } UniValue result(UniValue::VOBJ); @@ -253,10 +254,11 @@ } FlatSigningProvider key_provider; - auto desc = Parse(desc_str, key_provider, /* require_checksum = */ true); + 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")); + strprintf("Invalid descriptor, %s", 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 @@ -753,10 +753,12 @@ "Scan object needs to be either a string or an object"); } - auto desc = Parse(desc_str, provider); + std::string error; + auto desc = Parse(desc_str, provider, error); if (!desc) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, - strprintf("Invalid descriptor '%s'", desc_str)); + throw JSONRPCError( + RPC_INVALID_ADDRESS_OR_KEY, + strprintf("Invalid descriptor '%s', %s", desc_str, error)); } if (!desc->IsRange()) { range.first = 0; diff --git a/src/script/descriptor.h b/src/script/descriptor.h --- a/src/script/descriptor.h +++ b/src/script/descriptor.h @@ -91,7 +91,7 @@ * else is wrong, nullptr is returned. */ std::unique_ptr Parse(const std::string &descriptor, - FlatSigningProvider &out, + FlatSigningProvider &out, std::string &error, bool require_checksum = false); /** diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -800,7 +800,7 @@ * ignored). */ NODISCARD bool ParseKeyPath(const std::vector> &split, - KeyPath &out) { + KeyPath &out, std::string &error) { for (size_t i = 1; i < split.size(); ++i) { Span elem = split[i]; bool hardened = false; @@ -812,6 +812,7 @@ uint32_t p; if (!ParseUInt32(std::string(elem.begin(), elem.end()), &p) || p > 0x7FFFFFFFUL) { + error = strprintf("Key path value %u is out of range", p); return false; } out.push_back(p | (uint32_t(hardened) << 31)); @@ -821,7 +822,8 @@ /** Parse a public key that excludes origin information. */ std::unique_ptr ParsePubkeyInner(const Span &sp, - FlatSigningProvider &out) { + FlatSigningProvider &out, + std::string &error) { auto split = Split(sp, '/'); std::string str(split[0].begin(), split[0].end()); if (split.size() == 1) { @@ -842,6 +844,7 @@ CExtKey extkey = DecodeExtKey(str); CExtPubKey extpubkey = DecodeExtPubKey(str); if (!extkey.key.IsValid() && !extpubkey.pubkey.IsValid()) { + error = strprintf("key '%s' is not valid", str); return nullptr; } KeyPath path; @@ -854,7 +857,7 @@ split.pop_back(); type = DeriveType::HARDENED; } - if (!ParseKeyPath(split, path)) { + if (!ParseKeyPath(split, path, error)) { return nullptr; } if (extkey.key.IsValid()) { @@ -867,24 +870,33 @@ /** Parse a public key including origin information (if enabled). */ std::unique_ptr ParsePubkey(const Span &sp, - FlatSigningProvider &out) { + FlatSigningProvider &out, + std::string &error) { auto origin_split = Split(sp, ']'); if (origin_split.size() > 2) { + error = "Multiple ']' characters found for a single pubkey"; return nullptr; } if (origin_split.size() == 1) { - return ParsePubkeyInner(origin_split[0], out); + 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())); return nullptr; } auto slash_split = Split(origin_split[0].subspan(1), '/'); if (slash_split[0].size() != 8) { + error = strprintf("Fingerprint is not 4 bytes (%u characters instead " + "of 8 characters)", + slash_split[0].size()); return nullptr; } std::string fpr_hex = std::string(slash_split[0].begin(), slash_split[0].end()); if (!IsHex(fpr_hex)) { + error = strprintf("Fingerprint '%s' is not hex", fpr_hex); return nullptr; } auto fpr_bytes = ParseHex(fpr_hex); @@ -892,10 +904,10 @@ static_assert(sizeof(info.fingerprint) == 4, "Fingerprint must be 4 bytes"); assert(fpr_bytes.size() == 4); std::copy(fpr_bytes.begin(), fpr_bytes.end(), info.fingerprint); - if (!ParseKeyPath(slash_split, info.path)) { + if (!ParseKeyPath(slash_split, info.path, error)) { return nullptr; } - auto provider = ParsePubkeyInner(origin_split[1], out); + auto provider = ParsePubkeyInner(origin_split[1], out, error); if (!provider) { return nullptr; } @@ -906,24 +918,25 @@ /** Parse a script in a particular context. */ std::unique_ptr ParseScript(Span &sp, ParseScriptContext ctx, - FlatSigningProvider &out) { + FlatSigningProvider &out, + std::string &error) { auto expr = Expr(sp); if (Func("pk", expr)) { - auto pubkey = ParsePubkey(expr, out); + auto pubkey = ParsePubkey(expr, out, error); if (!pubkey) { return nullptr; } return std::make_unique(std::move(pubkey)); } if (Func("pkh", expr)) { - auto pubkey = ParsePubkey(expr, out); + auto pubkey = ParsePubkey(expr, out, error); if (!pubkey) { return nullptr; } return std::make_unique(std::move(pubkey)); } if (ctx == ParseScriptContext::TOP && Func("combo", expr)) { - auto pubkey = ParsePubkey(expr, out); + auto pubkey = ParsePubkey(expr, out, error); if (!pubkey) { return nullptr; } @@ -935,15 +948,17 @@ std::vector> providers; if (!ParseUInt32(std::string(threshold.begin(), threshold.end()), &thres)) { + error = strprintf("multi threshold %u out of range", thres); return nullptr; } size_t script_size = 0; while (expr.size()) { if (!Const(",", expr)) { + error = strprintf("multi: expected ',', got '%c'", expr[0]); return nullptr; } auto arg = Expr(expr); - auto pk = ParsePubkey(arg, out); + auto pk = ParsePubkey(arg, out, error); if (!pk) { return nullptr; } @@ -956,13 +971,17 @@ } if (ctx == ParseScriptContext::TOP) { if (providers.size() > 3) { - // Not more than 3 pubkeys for raw multisig + error = strprintf("Cannot %u pubkeys in bare multisig; only at " + "most 3 pubkeys", + providers.size()); return nullptr; } } if (ctx == ParseScriptContext::P2SH) { if (script_size + 3 > 520) { - // Enforce P2SH script size limit + error = strprintf("P2SH script is too large, %d bytes is " + "larger than 520 bytes", + script_size + 3); return nullptr; } } @@ -970,7 +989,7 @@ std::move(providers)); } if (ctx == ParseScriptContext::TOP && Func("sh", expr)) { - auto desc = ParseScript(expr, ParseScriptContext::P2SH, out); + auto desc = ParseScript(expr, ParseScriptContext::P2SH, out, error); if (!desc || expr.size()) { return nullptr; } @@ -980,6 +999,7 @@ CTxDestination dest = DecodeDestination(std::string(expr.begin(), expr.end()), Params()); if (!IsValidDestination(dest)) { + error = "Address is not valid"; return nullptr; } return std::make_unique(std::move(dest)); @@ -987,12 +1007,15 @@ if (ctx == ParseScriptContext::TOP && Func("raw", expr)) { std::string str(expr.begin(), expr.end()); if (!IsHex(str)) { + error = "Raw script is not hex"; return nullptr; } auto bytes = ParseHex(str); return std::make_unique( CScript(bytes.begin(), bytes.end())); } + error = strprintf("%s is not a valid descriptor function", + std::string(expr.begin(), expr.end())); return nullptr; } @@ -1068,31 +1091,36 @@ * Check a descriptor checksum, and update desc to be the checksum-less part. */ bool CheckChecksum(Span &sp, bool require_checksum, - std::string *out_checksum = nullptr) { + std::string &error, std::string *out_checksum = nullptr) { auto check_split = Split(sp, '#'); if (check_split.size() > 2) { - // Multiple '#' symbols + error = "Multiple '#' symbols"; return false; } if (check_split.size() == 1 && require_checksum) { - // Missing checksum + error = "Missing checksum"; return false; } if (check_split.size() == 2) { if (check_split[1].size() != 8) { - // Unexpected length for checksum + error = + strprintf("Expected 8 character checksum, not %u characters", + check_split[1].size()); return false; } } auto checksum = DescriptorChecksum(check_split[0]); if (checksum.empty()) { - // Invalid characters in payload + error = "Invalid characters in payload"; return false; } if (check_split.size() == 2) { if (!std::equal(checksum.begin(), checksum.end(), check_split[1].begin())) { - // Checksum mismatch + error = strprintf( + "Provided checksum '%s' does not match computed checksum '%s'", + std::string(check_split[1].begin(), check_split[1].end()), + checksum); return false; } } @@ -1104,13 +1132,13 @@ } std::unique_ptr Parse(const std::string &descriptor, - FlatSigningProvider &out, + FlatSigningProvider &out, std::string &error, bool require_checksum) { Span sp(descriptor.data(), descriptor.size()); - if (!CheckChecksum(sp, require_checksum)) { + if (!CheckChecksum(sp, require_checksum, error)) { return nullptr; } - auto ret = ParseScript(sp, ParseScriptContext::TOP, out); + auto ret = ParseScript(sp, ParseScriptContext::TOP, out, error); if (sp.size() == 0 && ret) { return std::unique_ptr(std::move(ret)); } @@ -1119,8 +1147,9 @@ std::string GetDescriptorChecksum(const std::string &descriptor) { std::string ret; + std::string error; Span sp(descriptor.data(), descriptor.size()); - if (!CheckChecksum(sp, false, &ret)) { + if (!CheckChecksum(sp, false, error, &ret)) { return ""; } return ret; diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -18,8 +18,9 @@ void CheckUnparsable(const std::string &prv, const std::string &pub) { FlatSigningProvider keys_priv, keys_pub; - auto parse_priv = Parse(prv, keys_priv); - auto parse_pub = Parse(pub, keys_pub); + std::string error; + auto parse_priv = Parse(prv, keys_priv, error); + auto parse_pub = Parse(pub, keys_pub, error); BOOST_CHECK_MESSAGE(!parse_priv, prv); BOOST_CHECK_MESSAGE(!parse_pub, pub); } @@ -78,10 +79,12 @@ const std::set> &paths = ONLY_EMPTY) { FlatSigningProvider keys_priv, keys_pub; std::set> left_paths = paths; + std::string error; // Check that parsing succeeds. - auto parse_priv = Parse(MaybeUseHInsteadOfApostrophy(prv), keys_priv); - auto parse_pub = Parse(MaybeUseHInsteadOfApostrophy(pub), keys_pub); + auto parse_priv = + Parse(MaybeUseHInsteadOfApostrophy(prv), keys_priv, error); + auto parse_pub = Parse(MaybeUseHInsteadOfApostrophy(pub), keys_pub, error); BOOST_CHECK(parse_priv); BOOST_CHECK(parse_pub); diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1266,9 +1266,12 @@ const std::string &descriptor = data["desc"].get_str(); FlatSigningProvider keys; - auto parsed_desc = Parse(descriptor, keys, /* require_checksum = */ true); + std::string error; + auto parsed_desc = + Parse(descriptor, keys, error, /* require_checksum = */ true); if (!parsed_desc) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Descriptor is invalid"); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, + strprintf("Descriptor is invalid, %s", error)); } have_solving_data = parsed_desc->IsSolvable(); 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') + error_message='Descriptor is invalid, Missing checksum') xpriv = "tprv8ZgxMBicQKsPeuVhWwi6wuMQGfPKi9Li5GtX35jVNknACgqe3CY4g5xgkfDDJcmtF7o1QnxWDRYw4H5P26PXq7sbcUkEqeR4fg3Kxp2tigg" # hdkeypath=m/0'/0'/0' and 1'