Update `compile` command to support creating taproot descriptors by va-an · Pull Request #208 · bitcoindevkit/bdk-cli

va-an · 2025-07-26T14:26:40Z

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 descriptor tr(unspendable(), TREE).

Changelog notice

Checklists

All Submissions:

I've signed all my commits
I followed the contribution guidelines
I ran cargo fmt and cargo clippy before committing

New Features:

I've added tests for the new feature
I've added docs for the new feature
I've updated CHANGELOG.md

coveralls · 2025-07-26T14:32:16Z

Pull Request Test Coverage Report for Build 17544445237

Details

59 of 67 (88.06%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+6.5%) to 8.628%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/utils.rs	0	2	0.0%
src/handlers.rs	59	65	90.77%

Totals
Change from base Build 17324807959:	6.5%
Covered Lines:	105
Relevant Lines:	1217

💛 - Coveralls

tvpeter

Thank you for working on this @va-an .

I have left a few comments for you.
Thank you

src/handlers.rs

tvpeter

@va-an Please rebase so I can test.

Thank you.

src/handlers.rs

chaitika · 2025-08-29T19:53:40Z

src/handlers.rs

+        assert!(EXPECTED_PK_A.contains(NUMS_UNSPENDABLE_KEY_HEX));
+        assert!(EXPECTED_AND_AB.contains(NUMS_UNSPENDABLE_KEY_HEX));


I am wondering how much value these assert statements add when we are in control of the expected values ?

Agreed, fixed!

tvpeter · 2025-09-04T11:11:00Z

@va-an , can you please make some time to update this PR? It will be great if it can get in the next release in few days.

tvpeter

Thank you @va-an for updating the PR.

I left minor comments.

src/handlers.rs

tvpeter

tACK 04d0301

Thank you @va-an for working on this.

nit: If you can squash your commits into one or two messages, it would be nice.

va-an · 2025-09-08T08:26:36Z

tACK 04d0301

Thank you @va-an for working on this.

nit: If you can squash your commits into one or two messages, it would be nice.

Right, forgot about rebase.
Did it, now there's 1 commit in the branch.

tvpeter

Thank you @va-an

va-an · 2025-09-08T10:44:31Z

Thank you @va-an

Thank you for your patience and help!

notmandatory · 2025-09-08T14:53:21Z

src/handlers.rs

+            // 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 xonly_public_key = XOnlyPublicKey::from_str(NUMS_UNSPENDABLE_KEY_HEX)


I believe technically the NUMS_UNSPENDABLE_KEY_HEX is not supposed to be used as is (privacy concerns), though for now it's fine. Per BIP-0431, should add an issue to do the following:

"In order to avoid leaking the information that key path spending is not possible it is recommended to pick a fresh integer r in the range 0...n-1 uniformly at random and use H + rG as internal key. It is possible to prove that this internal key does not have a known discrete logarithm with respect to G by revealing r to a verifier who can then reconstruct how the internal key was created."

notmandatory

utACK 3bcec61

github-project-automation bot added this to BDK-CLI Jul 26, 2025

va-an mentioned this pull request Jul 26, 2025

chore(clippy): fix clippy warnings #205

Closed

3 tasks

tvpeter requested changes Aug 4, 2025

View reviewed changes

src/handlers.rs Outdated Show resolved Hide resolved

src/handlers.rs Outdated Show resolved Hide resolved

src/handlers.rs Outdated Show resolved Hide resolved

src/handlers.rs Show resolved Hide resolved

va-an requested a review from tvpeter August 14, 2025 18:13

tvpeter added this to the CLI 2.0.0 milestone Aug 25, 2025

tvpeter reviewed Aug 28, 2025

View reviewed changes

src/handlers.rs Outdated Show resolved Hide resolved

chaitika reviewed Aug 29, 2025

View reviewed changes

tvpeter requested changes Sep 6, 2025

View reviewed changes

src/handlers.rs Outdated Show resolved Hide resolved

src/handlers.rs Show resolved Hide resolved

notmandatory assigned va-an Sep 6, 2025

va-an requested review from chaitika and tvpeter September 7, 2025 19:52

tvpeter approved these changes Sep 7, 2025

View reviewed changes

feat(compile): add tr option for compile cmd

3bcec61

va-an force-pushed the feat/add-tr-to-compile-cmd branch from 04d0301 to 3bcec61 Compare September 8, 2025 08:21

va-an requested a review from tvpeter September 8, 2025 08:26

tvpeter approved these changes Sep 8, 2025

View reviewed changes

notmandatory reviewed Sep 8, 2025

View reviewed changes

notmandatory approved these changes Sep 8, 2025

View reviewed changes

tvpeter mentioned this pull request Sep 8, 2025

NUMS_UNSPENDABLE_KEY_HEX is not supposed to be used as is (privacy concerns) #218

Open

tvpeter merged commit a821a79 into bitcoindevkit:master Sep 8, 2025
5 checks passed

github-project-automation bot moved this to Done in BDK-CLI Sep 8, 2025

va-an deleted the feat/add-tr-to-compile-cmd branch October 9, 2025 07:51

		assert!(EXPECTED_PK_A.contains(NUMS_UNSPENDABLE_KEY_HEX));
		assert!(EXPECTED_AND_AB.contains(NUMS_UNSPENDABLE_KEY_HEX));

Conversation

va-an commented Jul 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

New Features:

Uh oh!

coveralls commented Jul 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 17544445237

Details

💛 - Coveralls

Uh oh!

tvpeter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tvpeter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chaitika Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

va-an Sep 7, 2025

Choose a reason for hiding this comment

Uh oh!

tvpeter commented Sep 4, 2025

Uh oh!

tvpeter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tvpeter left a comment

Choose a reason for hiding this comment

Uh oh!

va-an commented Sep 8, 2025

Uh oh!

tvpeter left a comment

Choose a reason for hiding this comment

Uh oh!

va-an commented Sep 8, 2025

Uh oh!

notmandatory Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

notmandatory left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

va-an commented Jul 26, 2025 •

edited

Loading

coveralls commented Jul 26, 2025 •

edited

Loading