diff options
-rw-r--r-- | src/rpc/misc.cpp | 10 | ||||
-rw-r--r-- | src/rpc/util.cpp | 5 | ||||
-rw-r--r-- | src/script/descriptor.cpp | 127 | ||||
-rw-r--r-- | src/script/descriptor.h | 2 | ||||
-rw-r--r-- | src/test/descriptor_tests.cpp | 10 | ||||
-rw-r--r-- | src/wallet/rpcdump.cpp | 5 | ||||
-rwxr-xr-x | test/functional/wallet_importmulti.py | 2 |
7 files changed, 110 insertions, 51 deletions
diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index d1e9682416..e48ed8db75 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -150,9 +150,10 @@ UniValue getdescriptorinfo(const JSONRPCRequest& request) 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")); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid descriptor, %s", error)); } UniValue result(UniValue::VOBJ); @@ -199,9 +200,10 @@ UniValue deriveaddresses(const JSONRPCRequest& request) } 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")); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid descriptor, %s", error)); } if (!desc->IsRange() && request.params.size() > 1) { diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index de90276677..52b2b94852 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -717,9 +717,10 @@ std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, Fl throw JSONRPCError(RPC_INVALID_PARAMETER, "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.cpp b/src/script/descriptor.cpp index d2b370b65d..d9b0cfa000 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -690,7 +690,7 @@ std::vector<Span<const char>> Split(const Span<const char>& sp, char sep) } /** Parse a key path, being passed a split list of elements (the first element is ignored). */ -NODISCARD bool ParseKeyPath(const std::vector<Span<const char>>& split, KeyPath& out) +NODISCARD bool ParseKeyPath(const std::vector<Span<const char>>& split, KeyPath& out, std::string& error) { for (size_t i = 1; i < split.size(); ++i) { Span<const char> elem = split[i]; @@ -700,14 +700,17 @@ NODISCARD bool ParseKeyPath(const std::vector<Span<const char>>& split, KeyPath& hardened = true; } uint32_t p; - if (!ParseUInt32(std::string(elem.begin(), elem.end()), &p) || p > 0x7FFFFFFFUL) return false; + 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)); } return true; } /** Parse a public key that excludes origin information. */ -std::unique_ptr<PubkeyProvider> ParsePubkeyInner(const Span<const char>& sp, bool permit_uncompressed, FlatSigningProvider& out) +std::unique_ptr<PubkeyProvider> ParsePubkeyInner(const Span<const char>& sp, bool permit_uncompressed, FlatSigningProvider& out, std::string& error) { auto split = Split(sp, '/'); std::string str(split[0].begin(), split[0].end()); @@ -726,7 +729,10 @@ std::unique_ptr<PubkeyProvider> ParsePubkeyInner(const Span<const char>& sp, boo } CExtKey extkey = DecodeExtKey(str); CExtPubKey extpubkey = DecodeExtPubKey(str); - if (!extkey.key.IsValid() && !extpubkey.pubkey.IsValid()) return nullptr; + if (!extkey.key.IsValid() && !extpubkey.pubkey.IsValid()) { + error = strprintf("key '%s' is not valid", str); + return nullptr; + } KeyPath path; DeriveType type = DeriveType::NO; if (split.back() == MakeSpan("*").first(1)) { @@ -736,7 +742,7 @@ std::unique_ptr<PubkeyProvider> ParsePubkeyInner(const Span<const char>& sp, boo split.pop_back(); type = DeriveType::HARDENED; } - if (!ParseKeyPath(split, path)) return nullptr; + if (!ParseKeyPath(split, path, error)) return nullptr; if (extkey.key.IsValid()) { extpubkey = extkey.Neuter(); out.keys.emplace(extpubkey.pubkey.GetID(), extkey.key); @@ -745,43 +751,55 @@ std::unique_ptr<PubkeyProvider> ParsePubkeyInner(const Span<const char>& sp, boo } /** Parse a public key including origin information (if enabled). */ -std::unique_ptr<PubkeyProvider> ParsePubkey(const Span<const char>& sp, bool permit_uncompressed, FlatSigningProvider& out) +std::unique_ptr<PubkeyProvider> ParsePubkey(const Span<const char>& sp, bool permit_uncompressed, FlatSigningProvider& out, std::string& error) { auto origin_split = Split(sp, ']'); - if (origin_split.size() > 2) return nullptr; - if (origin_split.size() == 1) return ParsePubkeyInner(origin_split[0], permit_uncompressed, out); - if (origin_split[0].size() < 1 || origin_split[0][0] != '[') return nullptr; + 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], permit_uncompressed, 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) return nullptr; + 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)) return nullptr; + if (!IsHex(fpr_hex)) { + error = strprintf("Fingerprint '%s' is not hex", fpr_hex); + return nullptr; + } auto fpr_bytes = ParseHex(fpr_hex); KeyOriginInfo info; 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)) return nullptr; - auto provider = ParsePubkeyInner(origin_split[1], permit_uncompressed, out); + if (!ParseKeyPath(slash_split, info.path, error)) return nullptr; + auto provider = ParsePubkeyInner(origin_split[1], permit_uncompressed, out, error); if (!provider) return nullptr; return MakeUnique<OriginPubkeyProvider>(std::move(info), std::move(provider)); } /** Parse a script in a particular context. */ -std::unique_ptr<DescriptorImpl> ParseScript(Span<const char>& sp, ParseScriptContext ctx, FlatSigningProvider& out) +std::unique_ptr<DescriptorImpl> ParseScript(Span<const char>& sp, ParseScriptContext ctx, FlatSigningProvider& out, std::string& error) { auto expr = Expr(sp); if (Func("pk", expr)) { - auto pubkey = ParsePubkey(expr, ctx != ParseScriptContext::P2WSH, out); + auto pubkey = ParsePubkey(expr, ctx != ParseScriptContext::P2WSH, out, error); if (!pubkey) return nullptr; return MakeUnique<PKDescriptor>(std::move(pubkey)); } if (Func("pkh", expr)) { - auto pubkey = ParsePubkey(expr, ctx != ParseScriptContext::P2WSH, out); + auto pubkey = ParsePubkey(expr, ctx != ParseScriptContext::P2WSH, out, error); if (!pubkey) return nullptr; return MakeUnique<PKHDescriptor>(std::move(pubkey)); } if (ctx == ParseScriptContext::TOP && Func("combo", expr)) { - auto pubkey = ParsePubkey(expr, true, out); + auto pubkey = ParsePubkey(expr, true, out, error); if (!pubkey) return nullptr; return MakeUnique<ComboDescriptor>(std::move(pubkey)); } @@ -789,51 +807,70 @@ std::unique_ptr<DescriptorImpl> ParseScript(Span<const char>& sp, ParseScriptCon auto threshold = Expr(expr); uint32_t thres; std::vector<std::unique_ptr<PubkeyProvider>> providers; - if (!ParseUInt32(std::string(threshold.begin(), threshold.end()), &thres)) return nullptr; + 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)) return nullptr; + if (!Const(",", expr)) { + error = strprintf("multi: expected ',', got '%c'", expr[0]); + return nullptr; + } auto arg = Expr(expr); - auto pk = ParsePubkey(arg, ctx != ParseScriptContext::P2WSH, out); + auto pk = ParsePubkey(arg, ctx != ParseScriptContext::P2WSH, out, error); if (!pk) return nullptr; script_size += pk->GetSize() + 1; providers.emplace_back(std::move(pk)); } if (providers.size() < 1 || providers.size() > 16 || thres < 1 || thres > providers.size()) return nullptr; if (ctx == ParseScriptContext::TOP) { - if (providers.size() > 3) return nullptr; // Not more than 3 pubkeys for raw multisig + if (providers.size() > 3) { + 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) return nullptr; // Enforce P2SH script size limit + if (script_size + 3 > 520) { + error = strprintf("P2SH script is too large, %d bytes is larger than 520 bytes", script_size + 3); + return nullptr; + } } return MakeUnique<MultisigDescriptor>(thres, std::move(providers)); } if (ctx != ParseScriptContext::P2WSH && Func("wpkh", expr)) { - auto pubkey = ParsePubkey(expr, false, out); + auto pubkey = ParsePubkey(expr, false, out, error); if (!pubkey) return nullptr; return MakeUnique<WPKHDescriptor>(std::move(pubkey)); } 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; return MakeUnique<SHDescriptor>(std::move(desc)); } if (ctx != ParseScriptContext::P2WSH && Func("wsh", expr)) { - auto desc = ParseScript(expr, ParseScriptContext::P2WSH, out); + auto desc = ParseScript(expr, ParseScriptContext::P2WSH, out, error); if (!desc || expr.size()) return nullptr; return MakeUnique<WSHDescriptor>(std::move(desc)); } if (ctx == ParseScriptContext::TOP && Func("addr", expr)) { CTxDestination dest = DecodeDestination(std::string(expr.begin(), expr.end())); - if (!IsValidDestination(dest)) return nullptr; + if (!IsValidDestination(dest)) { + error = "Address is not valid"; + return nullptr; + } return MakeUnique<AddressDescriptor>(std::move(dest)); } if (ctx == ParseScriptContext::TOP && Func("raw", expr)) { std::string str(expr.begin(), expr.end()); - if (!IsHex(str)) return nullptr; + if (!IsHex(str)) { + error = "Raw script is not hex"; + return nullptr; + } auto bytes = ParseHex(str); return MakeUnique<RawDescriptor>(CScript(bytes.begin(), bytes.end())); } + error = strprintf("%s is not a valid descriptor function", std::string(expr.begin(), expr.end())); return nullptr; } @@ -915,29 +952,44 @@ std::unique_ptr<DescriptorImpl> InferScript(const CScript& script, ParseScriptCo } // namespace /** Check a descriptor checksum, and update desc to be the checksum-less part. */ -bool CheckChecksum(Span<const char>& sp, bool require_checksum, std::string* out_checksum = nullptr) +bool CheckChecksum(Span<const char>& sp, bool require_checksum, std::string& error, std::string* out_checksum = nullptr) { auto check_split = Split(sp, '#'); - if (check_split.size() > 2) return false; // Multiple '#' symbols - if (check_split.size() == 1 && require_checksum) return false; // Missing checksum + if (check_split.size() > 2) { + error = "Multiple '#' symbols"; + return false; + } + if (check_split.size() == 1 && require_checksum){ + error = "Missing checksum"; + return false; + } if (check_split.size() == 2) { - if (check_split[1].size() != 8) return false; // Unexpected length for checksum + if (check_split[1].size() != 8) { + error = strprintf("Expected 8 character checksum, not %u characters", check_split[1].size()); + return false; + } } auto checksum = DescriptorChecksum(check_split[0]); - if (checksum.empty()) return false; // Invalid characters in payload + if (checksum.empty()) { + error = "Invalid characters in payload"; + return false; + } if (check_split.size() == 2) { - if (!std::equal(checksum.begin(), checksum.end(), check_split[1].begin())) return false; // Checksum mismatch + if (!std::equal(checksum.begin(), checksum.end(), check_split[1].begin())) { + error = strprintf("Provided checksum '%s' does not match computed checksum '%s'", std::string(check_split[1].begin(), check_split[1].end()), checksum); + return false; + } } if (out_checksum) *out_checksum = std::move(checksum); sp = check_split[0]; return true; } -std::unique_ptr<Descriptor> Parse(const std::string& descriptor, FlatSigningProvider& out, bool require_checksum) +std::unique_ptr<Descriptor> Parse(const std::string& descriptor, FlatSigningProvider& out, std::string& error, bool require_checksum) { Span<const char> sp(descriptor.data(), descriptor.size()); - if (!CheckChecksum(sp, require_checksum)) return nullptr; - auto ret = ParseScript(sp, ParseScriptContext::TOP, out); + if (!CheckChecksum(sp, require_checksum, error)) return nullptr; + auto ret = ParseScript(sp, ParseScriptContext::TOP, out, error); if (sp.size() == 0 && ret) return std::unique_ptr<Descriptor>(std::move(ret)); return nullptr; } @@ -945,8 +997,9 @@ std::unique_ptr<Descriptor> Parse(const std::string& descriptor, FlatSigningProv std::string GetDescriptorChecksum(const std::string& descriptor) { std::string ret; + std::string error; Span<const char> sp(descriptor.data(), descriptor.size()); - if (!CheckChecksum(sp, false, &ret)) return ""; + if (!CheckChecksum(sp, false, error, &ret)) return ""; return ret; } diff --git a/src/script/descriptor.h b/src/script/descriptor.h index eae1e262cd..0195ca0939 100644 --- a/src/script/descriptor.h +++ b/src/script/descriptor.h @@ -79,7 +79,7 @@ struct Descriptor { * If a parse error occurs, or the checksum is missing/invalid, or anything * else is wrong, nullptr is returned. */ -std::unique_ptr<Descriptor> Parse(const std::string& descriptor, FlatSigningProvider& out, bool require_checksum = false); +std::unique_ptr<Descriptor> Parse(const std::string& descriptor, FlatSigningProvider& out, std::string& error, bool require_checksum = false); /** Get the checksum for a descriptor. * diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index f5bda7d5e6..6296634d28 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -16,8 +16,9 @@ namespace { 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); } @@ -62,10 +63,11 @@ void Check(const std::string& prv, const std::string& pub, int flags, const std: { FlatSigningProvider keys_priv, keys_pub; std::set<std::vector<uint32_t>> 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 index a905cc0c55..fad0aefabd 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1098,9 +1098,10 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID 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 index e19c7919a9..4a8fcbf6d0 100755 --- a/test/functional/wallet_importmulti.py +++ b/test/functional/wallet_importmulti.py @@ -552,7 +552,7 @@ class ImportMultiTest(BitcoinTestFramework): "keys": [key.privkey]}, success=False, error_code=-5, - error_message="Descriptor is invalid") + error_message="Descriptor is invalid, Missing checksum") # Test importing of a P2SH-P2WPKH address via descriptor + private key key = get_key(self.nodes[0]) |