Skip to content

Add support for x-only keys in descriptor #298

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
merged 3 commits into from
Feb 17, 2022

Conversation

sanket1729
Copy link
Member

@sanket1729 sanket1729 commented Feb 14, 2022

.

@@ -45,6 +45,9 @@ pub enum ScriptContextError {
/// Only Compressed keys allowed under current descriptor
/// Segwitv0 fragments do not allow uncompressed pubkeys
CompressedOnly(String),
/// XOnly keys are only allowed in Tap context
/// The first element is key, and second element is current script context
XOnlyKeysNotAllowed(String, String),
Copy link
Member

Choose a reason for hiding this comment

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

In cbd02d4:

I think the second String could be a &'static str

@@ -427,7 +460,10 @@ impl DescriptorPublicKey {
self
}

/// Computes the public key corresponding to this descriptor key
/// Computes the public key corresponding to this descriptor key.
/// When deriving from an XOnlyPublicKey, it adds the default 0x02 y-ordinate
Copy link
Member

Choose a reason for hiding this comment

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

In ce1425e:

typo y-ordinate

/// Derive a [`Descriptor`] with a concrete [`bitcoin::PublicKey`] at a given index
/// Removes all extended pubkeys and wildcards from the descriptor and only leaves
/// concrete [`bitcoin::PublicKey`]. All [`crate::XOnlyKey`]s are converted to [`bitcoin::PublicKey`]
/// by adding a default(0x02) y-ordinate. For [`crate::descriptor::Tr`] descriptor,
Copy link
Member

Choose a reason for hiding this comment

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

In f2d7bde:

y-ordinate again. Is this a term that I don't know? :)

@apoelstra
Copy link
Member

Done reviewing f2d7bde

@sanket1729
Copy link
Member Author

Addressed the comments.

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 602e94f

@apoelstra apoelstra merged commit 6a89bd0 into rust-bitcoin:master Feb 17, 2022
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.

2 participants