For descriptor pubkey parse errors, include context information #24462
Conversation
|
/cc @achow101 |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
Note 'Multi:' is used rather than 'multi():' as it also encompasses 'sortedmulti():'
Empact
force-pushed
the
2022-03-descriptor-pubkey-context
branch
from
3470ee4 to
9b52672
Compare
|
ACK 9b52672 |
| auto pk = ParsePubkey(key_exp_index, arg, ctx, out, error); | ||
| if (!pk) return nullptr; | ||
| if (!pk) { | ||
| error = strprintf("Multi: %s", error); |
There was a problem hiding this comment.
I don't understand why this can't (or shouldn't) be more specific?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
| error = strprintf("Multi: %s", error); | |
| error = strprintf("%smulti: %s", sorted_multi ? "sorted" : "", error); |
?
…ext information 9b52672 For descriptor pubkey parse errors, include context information (Ben Woosley) Pull request description: This adds readily-available context information to the error string, for further disambiguation. This is a revival of bitcoin#16123 which was largely addressed in bitcoin#16542. Note 'Multi:' is used rather than 'multi():' as it also encompasses 'sortedmulti():' ACKs for top commit: achow101: ACK 9b52672 theStack: ACK 9b52672 Tree-SHA512: 96533ea8c3ac7010f9b62e75b4bd20b65aff843030eb91c7a88312975acecaaf17909b7d1841f45edc86dbf7fa402d208adb85f0673bd79b857dbebacb8c9395
6 participants
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():'