-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
psbt: add support for new Taproot fields #1847
Conversation
Pull Request Test Coverage Report for Build 2258483739Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! Cool to be on the on the bleeding edge when it comes to stuff like this 🤓. Will circle back re testing between hardwarewallets/bitcoind on that lnd
PR.
btcutil/psbt/partial_input.go
Outdated
} | ||
} | ||
|
||
sort.Sort(TaprootScriptSpendSigSorter(pi.TaprootScriptSpendSig)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does the BIP actually mandate that these contents be sorted? Or is this just something we're adding to make testing easier since our serialization would be deterministic-ish?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The BIP doesn't mention it. I just followed the existing pattern of the library. But I guess it's nice to have things in a stable order? Though since we're not using maps internally but slices, the order should remain stable anyway...
Rebased and addressed all comments. |
Unit test failure seems to be a flake... Can't reproduce locally. |
Kicked things to re-run CI... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎗
Adds new Taproot specific fields to PSBT as defined in https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki.
Test vectors taken from bitcoin/bitcoin#22558.