For descriptor pubkey parse errors, include context information #24462

1. 
   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():’

2. 
   Empact commented at 4:51 pm on March 2, 2022: member

   /cc @achow101
3. DrahtBot added the label Descriptors on Mar 2, 2022
4. 
   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.

5. For descriptor pubkey parse errors, include context information

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

   9b52672700
6. Empact force-pushed on Mar 3, 2022
7. 
   achow101 commented at 5:46 pm on March 3, 2022: member

   ACK 9b526727000509dc6ef90f2ce6a9049edebf959c
8. 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:

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

   ?

9. theStack approved
10. 
    theStack commented at 6:13 pm on March 19, 2022: member

    ACK 9b526727000509dc6ef90f2ce6a9049edebf959c
11. MarcoFalke merged this on Mar 23, 2022
12. MarcoFalke closed this on Mar 23, 2022

    ---

13. sidhujag referenced this in commit 75d92f0334 on Mar 24, 2022
14. DrahtBot locked this on Mar 23, 2023

Contributors
Empact DrahtBot achow101 luke-jr theStack

Labels
Descriptors