Skip to content

Commit c325f61

Browse files
achow101achow101
committed
Return an error from descriptor Parse that gives more information about what failed
1 parent 7a960ba commit c325f61

File tree

7 files changed

+110
-51
lines changed

7 files changed

+110
-51
lines changed

‎src/rpc/misc.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,10 @@ UniValue getdescriptorinfo(const JSONRPCRequest& request)
150150
RPCTypeCheck(request.params, {UniValue::VSTR});
151151

152152
FlatSigningProvider provider;
153-
auto desc = Parse(request.params[0].get_str(), provider);
153+
std::string error;
154+
auto desc = Parse(request.params[0].get_str(), provider, error);
154155
if (!desc) {
155-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid descriptor"));
156+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid descriptor, %s", error));
156157
}
157158

158159
UniValue result(UniValue::VOBJ);
@@ -199,9 +200,10 @@ UniValue deriveaddresses(const JSONRPCRequest& request)
199200
}
200201

201202
FlatSigningProvider key_provider;
202-
auto desc = Parse(desc_str, key_provider, /* require_checksum = */ true);
203+
std::string error;
204+
auto desc = Parse(desc_str, key_provider, error, /* require_checksum = */ true);
203205
if (!desc) {
204-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid descriptor"));
206+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid descriptor, %s", error));
205207
}
206208

207209
if (!desc->IsRange() && request.params.size() > 1) {

‎src/rpc/util.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -717,9 +717,10 @@ std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, Fl
717717
throw JSONRPCError(RPC_INVALID_PARAMETER, "Scan object needs to be either a string or an object");
718718
}
719719

720-
auto desc = Parse(desc_str, provider);
720+
std::string error;
721+
auto desc = Parse(desc_str, provider, error);
721722
if (!desc) {
722-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid descriptor '%s'", desc_str));
723+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid descriptor '%s', %s", desc_str, error));
723724
}
724725
if (!desc->IsRange()) {
725726
range.first = 0;

‎src/script/descriptor.cpp

Lines changed: 90 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,7 @@ std::vector<Span<const char>> Split(const Span<const char>& sp, char sep)
690690
}
691691

692692
/** Parse a key path, being passed a split list of elements (the first element is ignored). */
693-
NODISCARD bool ParseKeyPath(const std::vector<Span<const char>>& split, KeyPath& out)
693+
NODISCARD bool ParseKeyPath(const std::vector<Span<const char>>& split, KeyPath& out, std::string& error)
694694
{
695695
for (size_t i = 1; i < split.size(); ++i) {
696696
Span<const char> elem = split[i];
@@ -700,14 +700,17 @@ NODISCARD bool ParseKeyPath(const std::vector<Span<const char>>& split, KeyPath&
700700
hardened = true;
701701
}
702702
uint32_t p;
703-
if (!ParseUInt32(std::string(elem.begin(), elem.end()), &p) || p > 0x7FFFFFFFUL) return false;
703+
if (!ParseUInt32(std::string(elem.begin(), elem.end()), &p) || p > 0x7FFFFFFFUL) {
704+
error = strprintf("Key path value %u is out of range", p);
705+
return false;
706+
}
704707
out.push_back(p | (((uint32_t)hardened) << 31));
705708
}
706709
return true;
707710
}
708711

709712
/** Parse a public key that excludes origin information. */
710-
std::unique_ptr<PubkeyProvider> ParsePubkeyInner(const Span<const char>& sp, bool permit_uncompressed, FlatSigningProvider& out)
713+
std::unique_ptr<PubkeyProvider> ParsePubkeyInner(const Span<const char>& sp, bool permit_uncompressed, FlatSigningProvider& out, std::string& error)
711714
{
712715
auto split = Split(sp, '/');
713716
std::string str(split[0].begin(), split[0].end());
@@ -726,7 +729,10 @@ std::unique_ptr<PubkeyProvider> ParsePubkeyInner(const Span<const char>& sp, boo
726729
}
727730
CExtKey extkey = DecodeExtKey(str);
728731
CExtPubKey extpubkey = DecodeExtPubKey(str);
729-
if (!extkey.key.IsValid() && !extpubkey.pubkey.IsValid()) return nullptr;
732+
if (!extkey.key.IsValid() && !extpubkey.pubkey.IsValid()) {
733+
error = strprintf("key '%s' is not valid", str);
734+
return nullptr;
735+
}
730736
KeyPath path;
731737
DeriveType type = DeriveType::NO;
732738
if (split.back() == MakeSpan("*").first(1)) {
@@ -736,7 +742,7 @@ std::unique_ptr<PubkeyProvider> ParsePubkeyInner(const Span<const char>& sp, boo
736742
split.pop_back();
737743
type = DeriveType::HARDENED;
738744
}
739-
if (!ParseKeyPath(split, path)) return nullptr;
745+
if (!ParseKeyPath(split, path, error)) return nullptr;
740746
if (extkey.key.IsValid()) {
741747
extpubkey = extkey.Neuter();
742748
out.keys.emplace(extpubkey.pubkey.GetID(), extkey.key);
@@ -745,95 +751,126 @@ std::unique_ptr<PubkeyProvider> ParsePubkeyInner(const Span<const char>& sp, boo
745751
}
746752

747753
/** Parse a public key including origin information (if enabled). */
748-
std::unique_ptr<PubkeyProvider> ParsePubkey(const Span<const char>& sp, bool permit_uncompressed, FlatSigningProvider& out)
754+
std::unique_ptr<PubkeyProvider> ParsePubkey(const Span<const char>& sp, bool permit_uncompressed, FlatSigningProvider& out, std::string& error)
749755
{
750756
auto origin_split = Split(sp, ']');
751-
if (origin_split.size() > 2) return nullptr;
752-
if (origin_split.size() == 1) return ParsePubkeyInner(origin_split[0], permit_uncompressed, out);
753-
if (origin_split[0].size() < 1 || origin_split[0][0] != '[') return nullptr;
757+
if (origin_split.size() > 2) {
758+
error = "Multiple ']' characters found for a single pubkey";
759+
return nullptr;
760+
}
761+
if (origin_split.size() == 1) return ParsePubkeyInner(origin_split[0], permit_uncompressed, out, error);
762+
if (origin_split[0].size() < 1 || origin_split[0][0] != '[') {
763+
error = strprintf("Key origin expected but not found, got '%s' instead", std::string(origin_split[0].begin(), origin_split[0].end()));
764+
return nullptr;
765+
}
754766
auto slash_split = Split(origin_split[0].subspan(1), '/');
755-
if (slash_split[0].size() != 8) return nullptr;
767+
if (slash_split[0].size() != 8) {
768+
error = strprintf("Fingerprint is not 4 bytes (%u characters instead of 8 characters)", slash_split[0].size());
769+
return nullptr;
770+
}
756771
std::string fpr_hex = std::string(slash_split[0].begin(), slash_split[0].end());
757-
if (!IsHex(fpr_hex)) return nullptr;
772+
if (!IsHex(fpr_hex)) {
773+
error = strprintf("Fingerprint '%s' is not hex", fpr_hex);
774+
return nullptr;
775+
}
758776
auto fpr_bytes = ParseHex(fpr_hex);
759777
KeyOriginInfo info;
760778
static_assert(sizeof(info.fingerprint) == 4, "Fingerprint must be 4 bytes");
761779
assert(fpr_bytes.size() == 4);
762780
std::copy(fpr_bytes.begin(), fpr_bytes.end(), info.fingerprint);
763-
if (!ParseKeyPath(slash_split, info.path)) return nullptr;
764-
auto provider = ParsePubkeyInner(origin_split[1], permit_uncompressed, out);
781+
if (!ParseKeyPath(slash_split, info.path, error)) return nullptr;
782+
auto provider = ParsePubkeyInner(origin_split[1], permit_uncompressed, out, error);
765783
if (!provider) return nullptr;
766784
return MakeUnique<OriginPubkeyProvider>(std::move(info), std::move(provider));
767785
}
768786

769787
/** Parse a script in a particular context. */
770-
std::unique_ptr<DescriptorImpl> ParseScript(Span<const char>& sp, ParseScriptContext ctx, FlatSigningProvider& out)
788+
std::unique_ptr<DescriptorImpl> ParseScript(Span<const char>& sp, ParseScriptContext ctx, FlatSigningProvider& out, std::string& error)
771789
{
772790
auto expr = Expr(sp);
773791
if (Func("pk", expr)) {
774-
auto pubkey = ParsePubkey(expr, ctx != ParseScriptContext::P2WSH, out);
792+
auto pubkey = ParsePubkey(expr, ctx != ParseScriptContext::P2WSH, out, error);
775793
if (!pubkey) return nullptr;
776794
return MakeUnique<PKDescriptor>(std::move(pubkey));
777795
}
778796
if (Func("pkh", expr)) {
779-
auto pubkey = ParsePubkey(expr, ctx != ParseScriptContext::P2WSH, out);
797+
auto pubkey = ParsePubkey(expr, ctx != ParseScriptContext::P2WSH, out, error);
780798
if (!pubkey) return nullptr;
781799
return MakeUnique<PKHDescriptor>(std::move(pubkey));
782800
}
783801
if (ctx == ParseScriptContext::TOP && Func("combo", expr)) {
784-
auto pubkey = ParsePubkey(expr, true, out);
802+
auto pubkey = ParsePubkey(expr, true, out, error);
785803
if (!pubkey) return nullptr;
786804
return MakeUnique<ComboDescriptor>(std::move(pubkey));
787805
}
788806
if (Func("multi", expr)) {
789807
auto threshold = Expr(expr);
790808
uint32_t thres;
791809
std::vector<std::unique_ptr<PubkeyProvider>> providers;
792-
if (!ParseUInt32(std::string(threshold.begin(), threshold.end()), &thres)) return nullptr;
810+
if (!ParseUInt32(std::string(threshold.begin(), threshold.end()), &thres)) {
811+
error = strprintf("multi threshold %u out of range", thres);
812+
return nullptr;
813+
}
793814
size_t script_size = 0;
794815
while (expr.size()) {
795-
if (!Const(",", expr)) return nullptr;
816+
if (!Const(",", expr)) {
817+
error = strprintf("multi: expected ',', got '%c'", expr[0]);
818+
return nullptr;
819+
}
796820
auto arg = Expr(expr);
797-
auto pk = ParsePubkey(arg, ctx != ParseScriptContext::P2WSH, out);
821+
auto pk = ParsePubkey(arg, ctx != ParseScriptContext::P2WSH, out, error);
798822
if (!pk) return nullptr;
799823
script_size += pk->GetSize() + 1;
800824
providers.emplace_back(std::move(pk));
801825
}
802826
if (providers.size() < 1 || providers.size() > 16 || thres < 1 || thres > providers.size()) return nullptr;
803827
if (ctx == ParseScriptContext::TOP) {
804-
if (providers.size() > 3) return nullptr; // Not more than 3 pubkeys for raw multisig
828+
if (providers.size() > 3) {
829+
error = strprintf("Cannot %u pubkeys in bare multisig; only at most 3 pubkeys", providers.size());
830+
return nullptr;
831+
}
805832
}
806833
if (ctx == ParseScriptContext::P2SH) {
807-
if (script_size + 3 > 520) return nullptr; // Enforce P2SH script size limit
834+
if (script_size + 3 > 520) {
835+
error = strprintf("P2SH script is too large, %d bytes is larger than 520 bytes", script_size + 3);
836+
return nullptr;
837+
}
808838
}
809839
return MakeUnique<MultisigDescriptor>(thres, std::move(providers));
810840
}
811841
if (ctx != ParseScriptContext::P2WSH && Func("wpkh", expr)) {
812-
auto pubkey = ParsePubkey(expr, false, out);
842+
auto pubkey = ParsePubkey(expr, false, out, error);
813843
if (!pubkey) return nullptr;
814844
return MakeUnique<WPKHDescriptor>(std::move(pubkey));
815845
}
816846
if (ctx == ParseScriptContext::TOP && Func("sh", expr)) {
817-
auto desc = ParseScript(expr, ParseScriptContext::P2SH, out);
847+
auto desc = ParseScript(expr, ParseScriptContext::P2SH, out, error);
818848
if (!desc || expr.size()) return nullptr;
819849
return MakeUnique<SHDescriptor>(std::move(desc));
820850
}
821851
if (ctx != ParseScriptContext::P2WSH && Func("wsh", expr)) {
822-
auto desc = ParseScript(expr, ParseScriptContext::P2WSH, out);
852+
auto desc = ParseScript(expr, ParseScriptContext::P2WSH, out, error);
823853
if (!desc || expr.size()) return nullptr;
824854
return MakeUnique<WSHDescriptor>(std::move(desc));
825855
}
826856
if (ctx == ParseScriptContext::TOP && Func("addr", expr)) {
827857
CTxDestination dest = DecodeDestination(std::string(expr.begin(), expr.end()));
828-
if (!IsValidDestination(dest)) return nullptr;
858+
if (!IsValidDestination(dest)) {
859+
error = "Address is not valid";
860+
return nullptr;
861+
}
829862
return MakeUnique<AddressDescriptor>(std::move(dest));
830863
}
831864
if (ctx == ParseScriptContext::TOP && Func("raw", expr)) {
832865
std::string str(expr.begin(), expr.end());
833-
if (!IsHex(str)) return nullptr;
866+
if (!IsHex(str)) {
867+
error = "Raw script is not hex";
868+
return nullptr;
869+
}
834870
auto bytes = ParseHex(str);
835871
return MakeUnique<RawDescriptor>(CScript(bytes.begin(), bytes.end()));
836872
}
873+
error = strprintf("%s is not a valid descriptor function", std::string(expr.begin(), expr.end()));
837874
return nullptr;
838875
}
839876

@@ -915,38 +952,54 @@ std::unique_ptr<DescriptorImpl> InferScript(const CScript& script, ParseScriptCo
915952
} // namespace
916953

917954
/** Check a descriptor checksum, and update desc to be the checksum-less part. */
918-
bool CheckChecksum(Span<const char>& sp, bool require_checksum, std::string* out_checksum = nullptr)
955+
bool CheckChecksum(Span<const char>& sp, bool require_checksum, std::string& error, std::string* out_checksum = nullptr)
919956
{
920957
auto check_split = Split(sp, '#');
921-
if (check_split.size() > 2) return false; // Multiple '#' symbols
922-
if (check_split.size() == 1 && require_checksum) return false; // Missing checksum
958+
if (check_split.size() > 2) {
959+
error = "Multiple '#' symbols";
960+
return false;
961+
}
962+
if (check_split.size() == 1 && require_checksum){
963+
error = "Missing checksum";
964+
return false;
965+
}
923966
if (check_split.size() == 2) {
924-
if (check_split[1].size() != 8) return false; // Unexpected length for checksum
967+
if (check_split[1].size() != 8) {
968+
error = strprintf("Expected 8 character checksum, not %u characters", check_split[1].size());
969+
return false;
970+
}
925971
}
926972
auto checksum = DescriptorChecksum(check_split[0]);
927-
if (checksum.empty()) return false; // Invalid characters in payload
973+
if (checksum.empty()) {
974+
error = "Invalid characters in payload";
975+
return false;
976+
}
928977
if (check_split.size() == 2) {
929-
if (!std::equal(checksum.begin(), checksum.end(), check_split[1].begin())) return false; // Checksum mismatch
978+
if (!std::equal(checksum.begin(), checksum.end(), check_split[1].begin())) {
979+
error = strprintf("Provided checksum '%s' does not match computed checksum '%s'", std::string(check_split[1].begin(), check_split[1].end()), checksum);
980+
return false;
981+
}
930982
}
931983
if (out_checksum) *out_checksum = std::move(checksum);
932984
sp = check_split[0];
933985
return true;
934986
}
935987

936-
std::unique_ptr<Descriptor> Parse(const std::string& descriptor, FlatSigningProvider& out, bool require_checksum)
988+
std::unique_ptr<Descriptor> Parse(const std::string& descriptor, FlatSigningProvider& out, std::string& error, bool require_checksum)
937989
{
938990
Span<const char> sp(descriptor.data(), descriptor.size());
939-
if (!CheckChecksum(sp, require_checksum)) return nullptr;
940-
auto ret = ParseScript(sp, ParseScriptContext::TOP, out);
991+
if (!CheckChecksum(sp, require_checksum, error)) return nullptr;
992+
auto ret = ParseScript(sp, ParseScriptContext::TOP, out, error);
941993
if (sp.size() == 0 && ret) return std::unique_ptr<Descriptor>(std::move(ret));
942994
return nullptr;
943995
}
944996

945997
std::string GetDescriptorChecksum(const std::string& descriptor)
946998
{
947999
std::string ret;
1000+
std::string error;
9481001
Span<const char> sp(descriptor.data(), descriptor.size());
949-
if (!CheckChecksum(sp, false, &ret)) return "";
1002+
if (!CheckChecksum(sp, false, error, &ret)) return "";
9501003
return ret;
9511004
}
9521005

‎src/script/descriptor.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ struct Descriptor {
7979
* If a parse error occurs, or the checksum is missing/invalid, or anything
8080
* else is wrong, nullptr is returned.
8181
*/
82-
std::unique_ptr<Descriptor> Parse(const std::string& descriptor, FlatSigningProvider& out, bool require_checksum = false);
82+
std::unique_ptr<Descriptor> Parse(const std::string& descriptor, FlatSigningProvider& out, std::string& error, bool require_checksum = false);
8383

8484
/** Get the checksum for a descriptor.
8585
*

‎src/test/descriptor_tests.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@ namespace {
1616
void CheckUnparsable(const std::string& prv, const std::string& pub)
1717
{
1818
FlatSigningProvider keys_priv, keys_pub;
19-
auto parse_priv = Parse(prv, keys_priv);
20-
auto parse_pub = Parse(pub, keys_pub);
19+
std::string error;
20+
auto parse_priv = Parse(prv, keys_priv, error);
21+
auto parse_pub = Parse(pub, keys_pub, error);
2122
BOOST_CHECK_MESSAGE(!parse_priv, prv);
2223
BOOST_CHECK_MESSAGE(!parse_pub, pub);
2324
}
@@ -62,10 +63,11 @@ void Check(const std::string& prv, const std::string& pub, int flags, const std:
6263
{
6364
FlatSigningProvider keys_priv, keys_pub;
6465
std::set<std::vector<uint32_t>> left_paths = paths;
66+
std::string error;
6567

6668
// Check that parsing succeeds.
67-
auto parse_priv = Parse(MaybeUseHInsteadOfApostrophy(prv), keys_priv);
68-
auto parse_pub = Parse(MaybeUseHInsteadOfApostrophy(pub), keys_pub);
69+
auto parse_priv = Parse(MaybeUseHInsteadOfApostrophy(prv), keys_priv, error);
70+
auto parse_pub = Parse(MaybeUseHInsteadOfApostrophy(pub), keys_pub, error);
6971
BOOST_CHECK(parse_priv);
7072
BOOST_CHECK(parse_pub);
7173

‎src/wallet/rpcdump.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,9 +1098,10 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID
10981098

10991099
const std::string& descriptor = data["desc"].get_str();
11001100
FlatSigningProvider keys;
1101-
auto parsed_desc = Parse(descriptor, keys, /* require_checksum = */ true);
1101+
std::string error;
1102+
auto parsed_desc = Parse(descriptor, keys, error, /* require_checksum = */ true);
11021103
if (!parsed_desc) {
1103-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Descriptor is invalid");
1104+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Descriptor is invalid, %s", error));
11041105
}
11051106

11061107
have_solving_data = parsed_desc->IsSolvable();

‎test/functional/wallet_importmulti.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -552,7 +552,7 @@ def run_test(self):
552552
"keys": [key.privkey]},
553553
success=False,
554554
error_code=-5,
555-
error_message="Descriptor is invalid")
555+
error_message="Descriptor is invalid, Missing checksum")
556556

557557
# Test importing of a P2SH-P2WPKH address via descriptor + private key
558558
key = get_key(self.nodes[0])

0 commit comments

Comments
 (0)