Skip to content

Add helpers to convert to DescriptorPublicKey and Descriptor<DefinitePublicKey> data structures #818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

philipr-za
Copy link
Contributor

@philipr-za philipr-za commented May 12, 2025

closes #817

I added the common cases of converting from Descriptor<PublicKey> and Descriptor<XOnlyPublicKey> to Descriptor<DefinitePublicKey>. Not sure if this is considered too specific but it feels like a use case that will be very common.

@apoelstra
Copy link
Member

In 5937090:

I think this is too specific. You're the first person I've encountered who wound up with XOnlyPublicKeys (which is a low level secp256k1 type) and wanted to use it in a PSBT like this.

The From impls on the keys themselves are fine.

I agree that it would be nice if we had a .map_pk function or something that was a simpler version of .translate_pk that assumed the hash types were all the same and that the mapping function were infallible. Then you could do descriptor.map_pk(DefiniteDescriptorKey::from).

@philipr-za
Copy link
Contributor Author

In 5937090:

I think this is too specific. You're the first person I've encountered who wound up with XOnlyPublicKeys (which is a low level secp256k1 type) and wanted to use it in a PSBT like this.

The From impls on the keys themselves are fine.

I agree that it would be nice if we had a .map_pk function or something that was a simpler version of .translate_pk that assumed the hash types were all the same and that the mapping function were infallible. Then you could do descriptor.map_pk(DefiniteDescriptorKey::from).

Fair enough, I will drop those impl's and run rustfmt

@philipr-za philipr-za force-pushed the add-from-to-descriptorpublickey branch from 5937090 to 8b248f6 Compare May 13, 2025 14:14
@philipr-za
Copy link
Contributor Author

Removed the Descriptor impl's. Had to make the new constructor public on DefiniteDescriptorKey to allow for the translate_pk to work I left in the unit test to prove the application of these From impl's to the use case in the issue.

@apoelstra
Copy link
Member

Looks like the formatter is still not happy. But concept ACK.

@philipr-za philipr-za force-pushed the add-from-to-descriptorpublickey branch from 8b248f6 to a00c993 Compare May 13, 2025 14:40
Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK a00c993; successfully ran local tests

@apoelstra
Copy link
Member

BTW if you want to use this today, can you do a backport PR to the 12.x branch (along with a minor version bump)?

@apoelstra apoelstra merged commit 6d1da92 into rust-bitcoin:master May 13, 2025
31 checks passed
philipr-za added a commit to philipr-za/rust-miniscript that referenced this pull request May 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Only public API for constructing DefiniteDescriptorKey or DefinitePublicKey is from_str(...)?
2 participants