Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improve TapTree API #617

Merged
merged 2 commits into from
Oct 13, 2023
Merged

Conversation

tcharding
Copy link
Member

Improve the TapTree API by making the height and combine methods public.

Note the serendipitous finding that upgrading miniscript to use bitcoin v0.31.0-rc1 requires no changes. I found these fixes doing the bdk upgrade.

While upgrading `bdk` to use bitcoin v0.31.0-rc1 and a patched version
of miniscript with the same upgrade it was found that the
`TapTree::combine` function could be used. Without this function users
are required to do the height calculation themselves.
There is no reason to keep this function private, the `height` field can
be gotten at by pattern matching so this function is just a convenience,
might as well let users have at it.
@tcharding
Copy link
Member Author

This PR does pose the question, "should we be maintaining an invariant that height is valid for left and right?" At the moment the fields are public.

tcharding added a commit to tcharding/bdk that referenced this pull request Oct 13, 2023
- Upgrade bitcoin to to v0.31.0-rc1

`rust-miniscript` does not require any changes to upgrade to use bitcoin
v0.31.0-rc1.

Upgrade highlights a weakness in the `rust-miniscript` API,
`TapTree::combine` is useful to us, it could be public.

see: rust-bitcoin/rust-miniscript#617
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 0630491

@apoelstra apoelstra merged commit a280447 into rust-bitcoin:master Oct 13, 2023
@tcharding tcharding deleted the 10-13-tap-tree-combine branch October 15, 2023 23:21
tcharding added a commit to tcharding/rust-psbt that referenced this pull request Nov 14, 2023
Import the `psbt` module from `rust-miniscript` while tip of master was:

  a280447 Merge rust-bitcoin/rust-miniscript#617: Improve `TapTree` API

Only do changes required to get things building, however this includes
upgrade of the bitcoin dependency.
tcharding added a commit to tcharding/rust-psbt that referenced this pull request Nov 14, 2023
Import the `psbt` module from `rust-miniscript` while tip of master was:

  a280447 Merge rust-bitcoin/rust-miniscript#617: Improve `TapTree` API

Only do changes required to get things building, however this includes
upgrade of the bitcoin dependency.
tcharding added a commit to tcharding/rust-psbt that referenced this pull request Nov 15, 2023
Import the `psbt` module from `rust-miniscript` while tip of master was:

  a280447 Merge rust-bitcoin/rust-miniscript#617: Improve `TapTree` API

Only do changes required to get things building, however this includes
upgrade of the bitcoin dependency.

Feature gate the miniscript stuff because we want this crate to be a
drop in replacement for the `psbt` module in `rust-bitcoin`, users may
not want to have `rust-miniscript` in there dependency tree.
tcharding added a commit to tcharding/rust-psbt that referenced this pull request Nov 23, 2023
Import the `psbt` module from `rust-miniscript` while tip of master was:

  a280447 Merge rust-bitcoin/rust-miniscript#617: Improve `TapTree` API

Only do changes required to get things building, however this includes
upgrade of the bitcoin dependency.

Feature gate the miniscript stuff because we want this crate to be a
drop in replacement for the `psbt` module in `rust-bitcoin`, users may
not want to have `rust-miniscript` in there dependency tree.
lucky-whistle-dev added a commit to lucky-whistle-dev/rust-bsbt that referenced this pull request Apr 1, 2024
Import the `psbt` module from `rust-miniscript` while tip of master was:

  a280447 Merge rust-bitcoin/rust-miniscript#617: Improve `TapTree` API

Only do changes required to get things building, however this includes
upgrade of the bitcoin dependency.

Feature gate the miniscript stuff because we want this crate to be a
drop in replacement for the `psbt` module in `rust-bitcoin`, users may
not want to have `rust-miniscript` in there dependency tree.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants