Skip to content
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

Add tr(KEY) support #24

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Zero-1729
Copy link

For brief context, I have been using this library for work with descriptors in a separate ts-based project, and I noticed it didn't handle parsing single-key taproot descriptors tr(KEY).

This PR adds support for parsing single-key taproot descriptors (i.e., tr(KEY)). I've used the existing p2tr and regex pattern definitions. Happy to make any adjustments to fit the repo code style or any other related changes.

@landabaso
Copy link
Member

Hi @Zero-1729,

Thank you so much for this PR! I'm thrilled to see contributors joining in. :)

I'll need a bit of time to review it thoroughly. Have you implemented this as a proof of concept, or is it already functioning well within your project?

From a quick look, I believe we'll need some basic tests. This ensures that the functionality works as intended and helps prevent unintentional breakages in future releases.

@Zero-1729
Copy link
Author

Zero-1729 commented Sep 18, 2023

@landabaso Thanks for working on the library.

I'Il need a bit of time to review it thoroughly. Have you implemented this as a proof of concept, or is it already
functioning well within your project?

Yes, it works as expected. I use the library to parse descriptors and modify their formats; it returns all the useful bits of the descriptor (e.g., key, key path, fingerprint, etc.) for other helper functions to wrangle and create private or public external and internal versions of a given descriptor.

From a quick look, I believe we'll need some basic tests. This ensures that the functionality works as intended and helps
prevent unintentional breakages in future releases.

Fair point, l'Il include some test cases.

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