For descriptor pubkey parse errors, include context information #24462

pull Empact wants to merge 1 commits into bitcoin:master from Empact:2022-03-descriptor-pubkey-context changing 2 files +38 −20

Empact commented at 4:50 PM on March 2, 2022: member

This adds readily-available context information to the error string, for further disambiguation.

This is a revival of #16123 which was largely addressed in #16542.

Note 'Multi:' is used rather than 'multi():' as it also encompasses 'sortedmulti():'
Empact commented at 4:51 PM on March 2, 2022: member

/cc @achow101
DrahtBot added the label Descriptors on Mar 2, 2022
DrahtBot commented at 2:52 AM on March 3, 2022: member


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #22838 (descriptors: Be able to specify change and receiving in a single descriptor string by achow101)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

For descriptor pubkey parse errors, include context information

Note 'Multi:' is used rather than 'multi():' as it also encompasses 'sortedmulti():'

9b52672700

Empact force-pushed on Mar 3, 2022
achow101 commented at 5:46 PM on March 3, 2022: member

ACK 9b526727000509dc6ef90f2ce6a9049edebf959c

in src/script/descriptor.cpp:1094 in 9b52672700

1089 | @@ -1081,7 +1090,10 @@ std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const
1090 |              }
1091 |              auto arg = Expr(expr);
1092 |              auto pk = ParsePubkey(key_exp_index, arg, ctx, out, error);
1093 | -            if (!pk) return nullptr;
1094 | +            if (!pk) {
1095 | +                error = strprintf("Multi: %s", error);

luke-jr commented at 1:17 AM on March 8, 2022:

I don't understand why this can't (or shouldn't) be more specific?

achow101 commented at 2:39 PM on March 11, 2022:

I think it's just that it would result in a fairly large if block to get the right function name when Multi: is sufficient to get the point across. There can only be one multi in a descriptor anyways, so it should be obvious where the error is.

luke-jr commented at 8:01 PM on March 11, 2022:

                error = strprintf("%smulti: %s", sorted_multi ? "sorted" : "", error);

theStack approved
theStack commented at 6:13 PM on March 19, 2022: member

ACK 9b526727000509dc6ef90f2ce6a9049edebf959c
MarcoFalke merged this on Mar 23, 2022
MarcoFalke closed this on Mar 23, 2022
sidhujag referenced this in commit 75d92f0334 on Mar 24, 2022
DrahtBot locked this on Mar 23, 2023

Contributors

Labels

Linked (view graph)

#16123 Return error information on descriptor parse error #16542 Return more specific errors about invalid descriptors

For descriptor pubkey parse errors, include context information #24462

Conflicts