Update compile command to support creating taproot descriptors
#208
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3 tasks
Pull Request Test Coverage Report for Build 16540773857Details
💛 - Coveralls |
tvpeter
requested changes
Aug 4, 2025
Comment on lines
+75
to
+76
| /// Well-known unspendable key used for Taproot descriptors when only script path is intended. | ||
| /// This is a NUMS (Nothing Up My Sleeve) point that ensures the key path cannot be used. |
There was a problem hiding this comment.
since you provided detailed comments in the usage block, this documentation seems redundant.
| // This ensures the key path is effectively disabled and only script path can be used. | ||
| // See https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#constructing-and-spending-taproot-outputs | ||
| let tree = TapTree::Leaf(Arc::new(taproot_policy)); | ||
| Descriptor::new_tr(NUMS_UNSPENDABLE_KEY.to_string(), Some(tree)) |
There was a problem hiding this comment.
I guess the NUMS_UNSPENDABLE_KEY represents XonlyPublicKey, so it is better to parse it into an XonlyPublicKey and not used as a string (since strings are used for key placeholders) to avoid type mismatch.
| // See https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#constructing-and-spending-taproot-outputs | ||
| let tree = TapTree::Leaf(Arc::new(taproot_policy)); | ||
| Descriptor::new_tr(NUMS_UNSPENDABLE_KEY.to_string(), Some(tree)) | ||
| } | ||
| _ => panic!("Invalid type"), |
There was a problem hiding this comment.
can you help and change from panic to a graceful handling of the error?
| let json_result = result.unwrap(); | ||
| let descriptor = json_result.get("descriptor").unwrap().as_str().unwrap(); | ||
| assert_eq!(descriptor, EXPECTED_AND_AB); | ||
| } |
There was a problem hiding this comment.
can you also add test cases for invalid/invalid policies and other edge cases?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Resolves #204.
Notes to the reviewers
For creating the tr descriptor, I used the NUMS pubkey proposed in BIP-341.
There is discussion about adding NUMS key to
rust-bicoin, we can use it in the future from there.Also there is BIP draft for new descriptor key expression
unspendable()for exacly this use case - we will simply use descriptortr(unspendable(), TREE).Changelog notice
Checklists
All Submissions:
cargo fmtandcargo clippybefore committingNew Features:
CHANGELOG.md