create taproot descriptor template #840
Conversation
There was a problem hiding this comment.
Review ACK 242f5bd
Everything looks good to me. Just one observation.
src/descriptor/template.rs
Outdated
| Bip49(prvkey, KeychainKind::External).build(Network::Bitcoin), | ||
| true, | ||
| false, | ||
| false, | ||
| Network::Regtest, |
There was a problem hiding this comment.
I was a bit confused with this at first. Why a Mainnet descriptor matches with a Regtest address? That's because the first network is used for setting the 2nd derivation index of Bipxx, and the second network is used for the address prefix.
As a result, if someone actually tries with the same Xpriv but builds with build(Network::Regtest) and derives the addresses, they will not match up with the test vector.
So does it make sense to change the first network into regtest in the build call, or change the second network to mainnet? The first option is a much smaller changeset and can be done in a separate PR.
There was a problem hiding this comment.
@rajarshimaitra , you are right this is a bit confusing. I will opt for your first option. I will option a small PR for it before end of the week. I will open an issue that helps me keep track of it.
|
@vladimirfomene please rebase to pickup the new MSRV change from #842. |
|
Would be good to get this one in before merging bdk_core work. |
|
I will work on this tonight, I will be ready tomorrow |
vladimirfomene
force-pushed
the
add-bip-86-template
branch
2 times, most recently
from
fb8d20d to
6470776
Compare
|
Thanks for all your work on this but we've gotten to the point where I'd like to hold off on merging any new features and only merge critical bug fixes to the Can you rebase this after the new |
|
Hey, can you please rebase this one on master? |
|
I see we didn't get merged in before the big bdk 1.0 switch. It It looks like it's ready to go, can you rebase this or update it for the new I was looking for this because @reez and I want to use a TR template for the iOS example app we're doing. |
|
After this is merged to master I'll get it back-ported to a maintenance release. |
vladimirfomene
force-pushed
the
add-bip-86-template
branch
from
6470776 to
50ac75e
Compare
This PR solves bitcoindevkit#836. This PR adds a P2TR descriptor template and a BIP86 taproot descriptor template. With this, users can now create a taproot descriptor with templates.
vladimirfomene
force-pushed
the
add-bip-86-template
branch
from
50ac75e to
e30919b
Compare
|
@notmandatory , this is good to go. I have created an issue #992 for fixing the issue raised by @rajarshimaitra |
7587f16 feat(descriptor): backport from master branch new taproot descriptor template (BIP86) (Steve Myers) 177c96d Create taproot descriptor template (Vladimir Fomene) Pull request description: ### Description This PR solves #836 for the release/0.28 branch. This PR adds a P2TR descriptor template and a BIP86 taproot descriptor template. With this, users can now create a taproot descriptor with templates. ### Notes to the reviewers The commit from #840 is cherry-picked from the `master` branch to the `release/0.28` branch without any changes. ### Changelog notice Add taproot descriptor template (BIP-86). ### Checklists #### All Submissions: * [x] I've signed all my commits * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) * [x] I ran `cargo fmt` and `cargo clippy` before committing #### New Features: * [x] I've added tests for the new feature * [x] I've added docs for the new feature ACKs for top commit: danielabrozzoni: utACK 7587f16 Tree-SHA512: e5b07473e27bba8ca5ec58854fa318c5a82cb67ce751d352ef17e9926dd08f8e6b7a720a77170b6f6f018c58ed9f8741cee396dc8e8721f4022c33ef2904815f
Summary This patch release backports (from the BDK 1.0 dev branch) a fix for a bug in the policy condition calculation and adds a new taproot single key descriptor template (BIP-86). The policy condition calculation bug can cause issues when a policy subtree fails due to missing info even if it's not selected when creating a new transaction, errors on unused policy paths are now ignored. Fixed - Backported bitcoindevkit#932 fix for policy condition calculation bitcoindevkit#1008 Added - Backported bitcoindevkit#840 taproot descriptor template (BIP-86) bitcoindevkit#1033
Description
This PR solves #836. This PR adds a P2TR
descriptor template and a BIP86 taproot
descriptor template. With this, users
can now create a taproot descriptor with templates.
Checklists
All Submissions:
cargo fmtandcargo clippybefore committingNew Features:
Bugfixes: