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

pull luke-jr wants to merge 5 commits into bitcoin:25.x from luke-jr:descr_disallow_hybrid_and_uncompr_pubkeys-25.1 changing 4 files +172 −14

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.

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

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

descriptors: Move InferScript's pubkey validity checks to InferPubkey
Github-Pull: #28602 Rebased-From: 37b9b734770e855b9beff3b5085125f1420dd072
549401a089

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

test: Unit test for inferring scripts with hybrid and uncompressed keys
Github-Pull: #28602 Rebased-From: 74c77825e5ab68bfa575dad86444506c43ef6c06
98208a813c
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.
DrahtBot added the label Backport on Oct 18, 2023
DrahtBot added the label CI failed on Oct 18, 2023
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?
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.
achow101 commented at 2:49 PM on October 20, 2023: member

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

Closing for now.
fanquake closed this on Oct 20, 2023
bitcoin locked this on Oct 19, 2024

Contributors

Labels

Linked (view graph)

#28587 descriptors: disallow hybrid public keys #28602 descriptors: Disallow hybrid and uncompressed keys when inferring

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

Code Coverage

Reviews