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
  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

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-21 15:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me