[25.x] descriptors: Disallow hybrid keys, and disallow uncompressed keys when inferring #28677

1. 
   luke-jr commented at 8:39 pm on October 18, 2023: member

   Backport of #28587 and #28602

   The latter is not quite trivial, and notably the final commit required a change from h to ' for hardened marker.

2. descriptors: disallow hybrid public keys

   The descriptor documentation (doc/descriptors.md) and BIP380 explicitly
   require that hex-encoded public keys start with 02 or 03 (compressed) or
   04 (uncompressed). However, the current parsing/inference code permit 06
   and 07 (hybrid) encoding as well. Fix this.
   
   Github-Pull: #28587
   Rebased-From: c1e6c542af6d89a499e2a65465865aec651c4d67

   ae3a6ab04f
3. descriptors: Check result of InferPubkey

   InferPubkey can return a nullptr, so check it's result before continuing
   with creating the inferred descriptor.
   
   Github-Pull: #28602
   Rebased-From: b7485f11ab3a0f1860b261f222362af3301e0781

   cc6d6f301b
4. descriptors: Move InferScript's pubkey validity checks to InferPubkey

   Github-Pull: #28602
   Rebased-From: 37b9b734770e855b9beff3b5085125f1420dd072

   549401a089
5. test: Scripts with hybrid pubkeys are migrated to watchonly

   Descriptors disallows hybrid pubkeys. Anything with hybrid pubkeys
   should becomes a raw() descriptor that shows up in the watchonly wallet.
   
   Github-Pull: #28602
   Rebased-From: f895f97014ac5fac46d27725c1ea7decf7ff76d4

   3ca5d9b6ef
6. test: Unit test for inferring scripts with hybrid and uncompressed keys

   Github-Pull: #28602
   Rebased-From: 74c77825e5ab68bfa575dad86444506c43ef6c06

   98208a813c
7. 
   DrahtBot commented at 8:39 pm on October 18, 2023: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

8. DrahtBot added the label Backport on Oct 18, 2023
9. DrahtBot added the label CI failed on Oct 18, 2023
10. 
    darosior commented at 6:57 am on October 19, 2023: member

    Is it really worth it? Most likely nobody is using descriptors with hybrid keys. And if anybody is it’d just prevent them to upgrade to the point release with all the actual bugfixes we may backport in the future?
11. 
    fanquake commented at 10:23 am on October 20, 2023: member

    Not convinced about backporting these. They’ve also missed 25.1, and it’d be a bit weird to put these into a 25.2. cc @achow101.
12. 
    achow101 commented at 2:49 pm on October 20, 2023: member

    I agree with @darosior.
13. 
    fanquake commented at 3:00 pm on October 20, 2023: member

    Closing for now.
14. fanquake closed this on Oct 20, 2023

    ---

15. bitcoin locked this on Oct 19, 2024

Contributors
luke-jr DrahtBot darosior fanquake achow101

Labels
Backport CI failed