descriptor: check if `rawtr` has only one key. #25827

pull w0xlt wants to merge 1 commits into bitcoin:master from w0xlt:expr_rawtr changing 2 files +29 −0
  1. w0xlt commented at 5:45 AM on August 12, 2022: contributor

    If I understand rawtr descriptor correctly, it should only allow rawtr(KEY), not rawtr(KEY1, KEY2, ...) or other concatenations.

    On master branch, rawtr(KEY1, KEY2, ...) will produce the rawtr(KEY1) descriptor ignoring the KEY2, ... with no error messages or warnings.

    For example, the code below will print rawtr(tprv8ZgxMBicQKsPefef2Doobbq3xTCaVTHcDn6me82KSXY1vY9AJAWD5u7SDM4XGLfc4EoXRMFrJKpp6HNmQWA3FTMRQeEmMJYJ9RPqe9ne2hU/*)#lx9qryfh for the supposedly invalid descriptor rawtr(tprv8ZgxMBicQKsPefef2Doobbq3xTCaVTHcDn6me82KSXY1vY9AJAWD5u7SDM4XGLfc4EoXRMFrJKpp6HNmQWA3FTMRQeEmMJYJ9RPqe9ne2hU/*, tprv8ZgxMBicQKsPezQ2KGArMRovTEbCGxaLgBgaVcTvEx8mby8ogX2bgC4HBapH4yMwrz2FpoCuA17eocuUVMgEP6fnm83YpwSDTFrumw42bny/*)

            self.nodes[1].createwallet(wallet_name="rawtr_multi", descriptors=True, blank=True)
            rawtr_multi = self.nodes[1].get_wallet_rpc("rawtr_multi")
            rawtr_multi_desc = "rawtr(tprv8ZgxMBicQKsPefef2Doobbq3xTCaVTHcDn6me82KSXY1vY9AJAWD5u7SDM4XGLfc4EoXRMFrJKpp6HNmQWA3FTMRQeEmMJYJ9RPqe9ne2hU/*, tprv8ZgxMBicQKsPezQ2KGArMRovTEbCGxaLgBgaVcTvEx8mby8ogX2bgC4HBapH4yMwrz2FpoCuA17eocuUVMgEP6fnm83YpwSDTFrumw42bny/*)#uv78hkt0"
            result = rawtr_multi.importdescriptors([{"desc": rawtr_multi_desc, "active": True, "timestamp": "now"}])
    
            print(rawtr_multi.listdescriptors(True))
    

    This PR adds a check that prevents rawtr descriptors from being created if more than one key is entered, shows an error message, and adds a test for this case.

  2. MarcoFalke added this to the milestone 24.0 on Aug 12, 2022
  3. darosior commented at 7:08 AM on August 12, 2022: member

    Concept ACK

  4. fanquake requested review from instagibbs on Aug 12, 2022
  5. sipa commented at 12:40 PM on August 12, 2022: member

    Concept ACK

  6. achow101 commented at 3:41 PM on August 12, 2022: member

    Concept ACK

    For the test, I would actually prefer it to be a CheckUnparseable unit test rather than a functional test.

  7. brunoerg commented at 4:02 PM on August 12, 2022: contributor

    Concept ACK

    I agree on @achow101 about the test.

  8. w0xlt force-pushed on Aug 17, 2022
  9. descriptor: check if `rawtr` has only one key. 416ceb8661
  10. w0xlt force-pushed on Aug 17, 2022
  11. w0xlt commented at 4:56 PM on August 17, 2022: contributor

    #25827 (comment) addressed in https://github.com/bitcoin/bitcoin/pull/25827/commits/416ceb8661117235c76f2985512473ebbc64956b.

    Also added success cases for rawtr since there weren't any.

  12. DrahtBot commented at 7:42 AM on August 18, 2022: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    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.

  13. achow101 commented at 5:16 PM on August 18, 2022: member

    ACK 416ceb8661117235c76f2985512473ebbc64956b

  14. sipa commented at 7:08 PM on August 18, 2022: member

    ACK 416ceb8661117235c76f2985512473ebbc64956b

  15. achow101 merged this on Aug 18, 2022
  16. achow101 closed this on Aug 18, 2022

  17. sidhujag referenced this in commit 5bc5d566b1 on Aug 18, 2022
  18. bitcoin locked this on Aug 18, 2023


instagibbs

Milestone
24.0


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: 2026-04-21 00:13 UTC

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