-
Notifications
You must be signed in to change notification settings - Fork 912
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
Elements PSBT / test fixup #4033
Merged
rustyrussell
merged 10 commits into
ElementsProject:master
from
niftynei:nifty/elements_fix
Sep 10, 2020
Merged
Elements PSBT / test fixup #4033
rustyrussell
merged 10 commits into
ElementsProject:master
from
niftynei:nifty/elements_fix
Sep 10, 2020
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustyrussell
requested changes
Sep 9, 2020
Elements requires the witness utxo to include the asset and value info, in order for the signing hash to be constructed correctly Changelog-Fixed: elementsd: PSBTs include correct witness_utxo struct for elements transactions
Not strictly necessary as technically this info is included in the witness_utxo, but nice to have
Now that we're using an 'elements' witness_utxo and issue 3998 has been resolved, this should work as expected.
We're moving away from bitcoin_tx, slowly
so we add an accurate one
This will update the fee output if it exists, rather than unilaterally adding a new one. Also, if the fee output already exists, we should make sure that it doesn't interfere with the outnums of the other outputs
In the case that you've got more than one fee output already on a psbt, we consolidate them into a single fee output (the first)
niftynei
force-pushed
the
nifty/elements_fix
branch
from
September 10, 2020 01:56
27a7a0f
to
6cf66e6
Compare
rustyrussell
approved these changes
Sep 10, 2020
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.
Ack 6cf66e6
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We should be using the
elements
tx_output, which includes the value + asset info required by the PSBT's sighash signing algorithm.This fixes that, plus adds back in some 'nice to have' extra PSET info (value + assset fields) for PSBTs constructed by
utxopsbt
/fundpsbt
.Finally, we allow the
test_sign_and_send
test to run on travis!Edit: there were some other elements failures, those should be fixed now.