-
Notifications
You must be signed in to change notification settings - Fork 38.8k
wallet: Precompute Txdata after setting PSBT inputs' UTXOs #25590
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
Conversation
test/functional/rpc_psbt.py
Outdated
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.
for reviewers: without the fix, this would have to be called twice in a row
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 12f5f8e0b726373109fae9356be2fcb5ea3c608e
fixes the issue I was having
If we are given a PSBT that is missing one or more input UTXOs, our PrecomputedTransactionData will be incorrect and missing information that it should otherwise have, and therefore we may not produce a signature when we should. To avoid this problem, we can do the precomputation after we have set the UTXOs the wallet is able to set for the PSBT. Also adds a test for this behavior.
12f5f8e to
d2ed976
Compare
|
reACK d2ed976 |
|
ACK d2ed976 |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
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 test does not fail without the fix, is this normal?
|
@aureleoules I'm fairly sure I tested that the test did fail without the fix (don't forget to recompile after removing the fix). Can you try again? |
|
My bad @Sjors, I didn't run the test with |
|
ACK d2ed976. |
…uts' UTXOs d2ed976 wallet: Precompute Txdata after setting PSBT inputs' UTXOs (Andrew Chow) Pull request description: If we are given a PSBT that is missing one or more input UTXOs, our PrecomputedTransactionData will be incorrect and missing information that it should otherwise have, and therefore we may not produce a signature when we should. To avoid this problem, we can do the precomputation after we have set the UTXOs the wallet is able to set for the PSBT. Also adds a test for this behavior. ACKs for top commit: instagibbs: reACK bitcoin@d2ed976 Sjors: ACK d2ed976 aureleoules: ACK d2ed976. Tree-SHA512: 71beb6c7946096e82cfca83f36277302aa9e69d27b4f6d73d7d8f2f9f0ea1c0d653e846fa6aebee5e4763f56f950b4481240e953f6a2412caa84908d519171e1
If we are given a PSBT that is missing one or more input UTXOs, our
PrecomputedTransactionData will be incorrect and missing information
that it should otherwise have, and therefore we may not produce a
signature when we should. To avoid this problem, we can do the
precomputation after we have set the UTXOs the wallet is able to set for
the PSBT.
Also adds a test for this behavior.