fix converttopsbt permitsigdata arg, add basic test #14356
Merged
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
maflcko
reviewed
Oct 1, 2018
| @@ -107,6 +107,9 @@ def run_test(self): | |||
| # Make sure that a psbt with signatures cannot be converted | |||
| signedtx = self.nodes[0].signrawtransactionwithwallet(rawtx['hex']) | |||
| assert_raises_rpc_error(-22, "TX decode failed", self.nodes[0].converttopsbt, signedtx['hex']) | |||
| assert_raises_rpc_error(-22, "TX decode failed", self.nodes[0].converttopsbt, signedtx['hex'], False) | |||
| # Unless we allow it to convert and strip signatures | |||
| self.nodes[0].converttopsbt(signedtx['hex'], True) | |||
There was a problem hiding this comment.
Nit: could use named args permitsigdata=True/False?
|
Backport would be appreciated.
…On Sun, Sep 30, 2018, 9:06 PM MarcoFalke ***@***.***> wrote:
***@***.**** commented on this pull request.
Is this for backport?
------------------------------
In test/functional/rpc_psbt.py
<#14356 (comment)>:
> @@ -107,6 +107,9 @@ def run_test(self):
# Make sure that a psbt with signatures cannot be converted
signedtx = self.nodes[0].signrawtransactionwithwallet(rawtx['hex'])
assert_raises_rpc_error(-22, "TX decode failed", self.nodes[0].converttopsbt, signedtx['hex'])
+ assert_raises_rpc_error(-22, "TX decode failed", self.nodes[0].converttopsbt, signedtx['hex'], False)
+ # Unless we allow it to convert and strip signatures
+ self.nodes[0].converttopsbt(signedtx['hex'], True)
Nit: could use named args permitsigdata=True/False?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#14356 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFgC00wnXt81XHgEIjyE4tn73YFdXSBgks5ugWqRgaJpZM4XAxPO>
.
|
|
utACK 88a79cb |
|
utACK 88a79cb. |
jimmysong
reviewed
Oct 1, 2018
| @@ -1669,7 +1669,7 @@ UniValue converttopsbt(const JSONRPCRequest& request) | |||
|
|
|||
| // Remove all scriptSigs and scriptWitnesses from inputs | |||
| for (CTxIn& input : tx.vin) { | |||
| if ((!input.scriptSig.empty() || !input.scriptWitness.IsNull()) && (request.params[1].isNull() || (!request.params[1].isNull() && request.params[1].get_bool()))) { | |||
| if ((!input.scriptSig.empty() || !input.scriptWitness.IsNull()) && !permitsigdata) { | |||
There was a problem hiding this comment.
Would it make sense to have the !request.params[1].isNull() in line 1661 above?
|
utACK 88a79cb |
|
utACK 88a79cb |
laanwj
added a commit
that referenced
this pull request
Nov 12, 2018
88a79cb fix converttopsbt permitsigdata arg, add basic test (Gregory Sanders) Pull request description: The final check for extraneous sigdata has a flipped boolean, resulting in incorrect behavior. Resolves #14355 Tree-SHA512: 5157a74b8ddebd7d836fba96765c4d7ed15a73d4289817353d3566a0f6803bd4bbc3f936735c517c7a83a6cbdb4052b9c61d23f6cc4ad00a6077278cd51adbd4
maflcko
pushed a commit
to maflcko/bitcoin-core
that referenced
this pull request
Dec 5, 2018
Github-Pull: bitcoin#14356 Rebased-From: 88a79cb
deadalnix
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
May 11, 2020
Summary: The final check for extraneous sigdata has a flipped boolean, resulting in incorrect behavior. --- This is a backport of Core [[bitcoin/bitcoin#14356 | PR14356]] Test Plan: ninja check ./test_runner.py rpc_psbt Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D6026
UdjinM6
pushed a commit
to UdjinM6/dash
that referenced
this pull request
Jul 15, 2021
88a79cb fix converttopsbt permitsigdata arg, add basic test (Gregory Sanders) Pull request description: The final check for extraneous sigdata has a flipped boolean, resulting in incorrect behavior. Resolves bitcoin#14355 Tree-SHA512: 5157a74b8ddebd7d836fba96765c4d7ed15a73d4289817353d3566a0f6803bd4bbc3f936735c517c7a83a6cbdb4052b9c61d23f6cc4ad00a6077278cd51adbd4
UdjinM6
pushed a commit
to UdjinM6/dash
that referenced
this pull request
Jul 15, 2021
88a79cb fix converttopsbt permitsigdata arg, add basic test (Gregory Sanders) Pull request description: The final check for extraneous sigdata has a flipped boolean, resulting in incorrect behavior. Resolves bitcoin#14355 Tree-SHA512: 5157a74b8ddebd7d836fba96765c4d7ed15a73d4289817353d3566a0f6803bd4bbc3f936735c517c7a83a6cbdb4052b9c61d23f6cc4ad00a6077278cd51adbd4
UdjinM6
added a commit
to dashpay/dash
that referenced
this pull request
Jul 16, 2021
Merge bitcoin#14356: fix converttopsbt permitsigdata arg, add basic test
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
The final check for extraneous sigdata has a flipped boolean, resulting in incorrect behavior.
Resolves #14355