Skip to content

feat: Add descriptor miniscript constructor method#1016

Open
ItoroD wants to merge 1 commit into
bitcoindevkit:masterfrom
ItoroD:add-final-miniscript-method
Open

feat: Add descriptor miniscript constructor method#1016
ItoroD wants to merge 1 commit into
bitcoindevkit:masterfrom
ItoroD:add-final-miniscript-method

Conversation

@ItoroD
Copy link
Copy Markdown
Collaborator

@ItoroD ItoroD commented May 28, 2026

Description

  • Add new_tr, new_sh_with_wsh, and new_sh_with_wpkh

Notes to the reviewers

Documentation

Changelog

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing
  • I've added exactly one changelog:* label
  • I've linked the relevant upstream docs or specs above

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

@ItoroD ItoroD requested a review from thunderbiscuit as a code owner May 28, 2026 11:43
@ItoroD ItoroD force-pushed the add-final-miniscript-method branch from 00d3bc7 to b51cdc1 Compare May 28, 2026 11:49
@ItoroD ItoroD force-pushed the add-final-miniscript-method branch from b51cdc1 to 5b09b02 Compare May 28, 2026 11:53
Copy link
Copy Markdown
Collaborator

@reez reez left a comment

Choose a reason for hiding this comment

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

Looks great overall, 2 comments

Descriptor.newWpkh(timelockConjunction)
Descriptor.newWsh(timelockConjunction)
Descriptor.newPkh(timelockConjunction)
Descriptor.newShWithWpkh(newShDescriptor.toString())
Copy link
Copy Markdown
Collaborator

@reez reez May 28, 2026

Choose a reason for hiding this comment

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

Can we split these into separate assertFailsWith blocks? I think this new newShWithWpkh case never executes because Descriptor.newBare(...) throws first in the same block.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

woaw. Good catch!

val newWpkhDescriptor = Descriptor.newWpkh("xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB")
val newShWpkhDescriptor = Descriptor.newShWpkh("xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB")
val newTrDescriptor = Descriptor.newTr("xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB","and_v(v:pk(02b4632d08485ff1df2db55b9dafd23347d1c47a457072a1e87be26896549a8737),older(50))")
println(newWpkhDescriptor)
Copy link
Copy Markdown
Collaborator

@reez reez May 28, 2026

Choose a reason for hiding this comment

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

Should we remove this debug println before merge?

- Add new_tr, new_sh_with_wsh, and new_sh_with_wpkh
@ItoroD ItoroD force-pushed the add-final-miniscript-method branch from 5b09b02 to c1a34cd Compare May 28, 2026 17:18
Copy link
Copy Markdown
Collaborator

@reez reez left a comment

Choose a reason for hiding this comment

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

ACK c1a34cd

Thanks for addressing both comments. Looks good to me pending the Android CI finishing green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants