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. DrahtBot cross-referenced this on Aug 18, 2022 from issue descriptors: Be able to specify change and receiving in a single descriptor string by achow101
  14. achow101 commented at 5:16 PM on August 18, 2022: member

    ACK 416ceb8661117235c76f2985512473ebbc64956b

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

    ACK 416ceb8661117235c76f2985512473ebbc64956b

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

  18. sidhujag referenced this in commit 5bc5d566b1 on Aug 18, 2022
  19. 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-05-20 06:53 UTC