-
Notifications
You must be signed in to change notification settings - Fork 37.7k
Require a public key to be retrieved when signing a P2PKH input #14689
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
This deserves a regression test. Have a |
utACK otherwise 409564a |
If we do not have the public key for a P2PKH input, we should not continue to attempt to sign for it.
409564a
to
6b8d86d
Compare
@instagibbs I added a test |
I can confirm this test fails on master. It also fixes the downstream issue I was seeing. |
just to note this issue happens for any native "keyhash" type including native segwit, and this fixes that |
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. Coverage
Updated at: 2018-11-08T22:33:52.852772. |
utACK 6b8d86d |
1 similar comment
utACK 6b8d86d |
utACK 6b8d86d |
…KH input 6b8d86d Require a public key to be retrieved when signing a P2PKH input (Andrew Chow) Pull request description: If we do not have the public key for a P2PKH input, we should not continue to attempt to sign for it. This fixes a problem where a PSBT with a P2PKH output would include invalid BIP 32 derivation paths that are missing the public key. Tree-SHA512: 850d5e74c06833da937d5bf0348bd134180be7167b6f9b9cecbf09f75e3543fbad60d0abbc0b9afdfa51ce165aa36168849f24a7c5abf1e75f37ce8f9a13d127
…erialization 4e4de10 Throw error if CPubKey is invalid during PSBT keypath serialization (Gregory Sanders) Pull request description: Related to #14689 We should catch this error before attempting to deserialize it later. Tree-SHA512: d2f3ea7f363818ac70c81ee988231b2bb50d055b6919f7bff3f27120c85a7048bfa183efae33e23e6b81d684bcb8bb81e5b209abb3acbcaff1d88014f4f1aa93
Backport? |
Marked for backport. |
# Test that psbts with p2pkh outputs are created properly | ||
p2pkh = self.nodes[0].getnewaddress(address_type='legacy') | ||
psbt = self.nodes[1].walletcreatefundedpsbt([], [{p2pkh : 1}], 0, {"includeWatching" : True}, True) | ||
self.nodes[0].decodepsbt(psbt['psbt']) |
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.
I tried to backport, but the test passes even without the code changes.
Mind taking a look to backport this?
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.
@MarcoFalke I believe the issue only arose after #14424 (which is also marked for backport), does the test fail if you backport that at the same time?
…P2PKH input Summary: If we do not have the public key for a P2PKH input, we should not continue to attempt to sign for it. --- Depends on D6028 This is a backport of Core [[bitcoin/bitcoin#14689 | PR14689]] Test Plan: cmake .. -GNinja -DENABLE_WERROR=ON ninja check-all Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D6029
…ypath serialization 4e4de10 Throw error if CPubKey is invalid during PSBT keypath serialization (Gregory Sanders) Pull request description: Related to bitcoin#14689 We should catch this error before attempting to deserialize it later. Tree-SHA512: d2f3ea7f363818ac70c81ee988231b2bb50d055b6919f7bff3f27120c85a7048bfa183efae33e23e6b81d684bcb8bb81e5b209abb3acbcaff1d88014f4f1aa93
…ypath serialization 4e4de10 Throw error if CPubKey is invalid during PSBT keypath serialization (Gregory Sanders) Pull request description: Related to bitcoin#14689 We should catch this error before attempting to deserialize it later. Tree-SHA512: d2f3ea7f363818ac70c81ee988231b2bb50d055b6919f7bff3f27120c85a7048bfa183efae33e23e6b81d684bcb8bb81e5b209abb3acbcaff1d88014f4f1aa93
merge bitcoin#16117, bitcoin#18358, bitcoin#17383, bitcoin#21052, bitcoin#14424, bitcoin#15159, bitcoin#14689, bitcoin#14978, partial bitcoin#16908, bitcoin#14978, bitcoin#13932: Auxillary Backports
…g a P2PKH input
…ypath serialization 4e4de10 Throw error if CPubKey is invalid during PSBT keypath serialization (Gregory Sanders) Pull request description: Related to bitcoin#14689 We should catch this error before attempting to deserialize it later. Tree-SHA512: d2f3ea7f363818ac70c81ee988231b2bb50d055b6919f7bff3f27120c85a7048bfa183efae33e23e6b81d684bcb8bb81e5b209abb3acbcaff1d88014f4f1aa93
…ypath serialization 4e4de10 Throw error if CPubKey is invalid during PSBT keypath serialization (Gregory Sanders) Pull request description: Related to bitcoin#14689 We should catch this error before attempting to deserialize it later. Tree-SHA512: d2f3ea7f363818ac70c81ee988231b2bb50d055b6919f7bff3f27120c85a7048bfa183efae33e23e6b81d684bcb8bb81e5b209abb3acbcaff1d88014f4f1aa93
If we do not have the public key for a P2PKH input, we should not continue to attempt to sign for it.
This fixes a problem where a PSBT with a P2PKH output would include invalid BIP 32 derivation paths that are missing the public key.