check that a separator is found for psbt inputs, outputs, and global map #14377
Conversation
This file contains hidden or 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
Member
|
Is this for 0.17.1 backport? |
jimmysong
reviewed
Oct 3, 2018
| if (key.empty()) return; | ||
| if (key.empty()) { | ||
| found_sep = true; | ||
| break; |
Contributor
There was a problem hiding this comment.
What's the rationale for breaking here instead of throwing the error right away?
Member
Author
There was a problem hiding this comment.
We want to throw the error if we don't get here.
| s >> key; | ||
|
|
||
| // the key is empty if that was actually a separator byte | ||
| // This is a special case for key lengths 0 as those are not allowed (except for separator) |
Contributor
There was a problem hiding this comment.
Comment may need updating?
|
|
||
| // the key is empty if that was actually a separator byte | ||
| // This is a special case for key lengths 0 as those are not allowed (except for separator) | ||
| if (key.empty()) return; |
Contributor
There was a problem hiding this comment.
same as above, comment may need updating?
|
|
||
| // the key is empty if that was actually a separator byte | ||
| // This is a special case for key lengths 0 as those are not allowed (except for separator) | ||
| if (key.empty()) break; |
Contributor
There was a problem hiding this comment.
comment may need updating?
Member
|
utACK 4fb3388. I think it should go into 0.17.1. |
Contributor
|
Contributor
|
utACK 4fb3388 |
laanwj
added a commit
that referenced
this pull request
Nov 1, 2018
…s, and global map 4fb3388 check that a separator is found for psbt inputs, outputs, and global map (Andrew Chow) Pull request description: Currently it doesn't make sure that a separator was found so PSBTs missing a trailing separator would still pass. This fixes that and adds a test case for it. It really only makes sense to check for the separator for the output maps as if an input or global map was missing a separator, the fields following it would be interpreted as belonging to the previous input or global map. However I have added the check for those two anyways to be consistent. Tree-SHA512: 50c0c08e201ba02494b369a4d36ddb73e6634eb5a4e4e201c4ef38fd2dbeea2c642b8a04d50c91615da61ecbfade37309e47431368f4b1064539c42015766b50
Member
|
This will be backported in #14780. |
sipa
pushed a commit
to sipa/bitcoin
that referenced
this pull request
Dec 3, 2018
Github-Pull: bitcoin#14377 Rebased-From: 4fb3388
maflcko
pushed a commit
that referenced
this pull request
Dec 5, 2018
7bee414 Add test for conversion from non-witness to witness UTXO (Pieter Wuille) ff56bb9 Add regression test for PSBT signing bug #14473 (Glenn Willen) db445d4 Refactor PSBTInput signing to enforce invariant (Glenn Willen) ad94165 Simplify arguments to SignPSBTInput (Glenn Willen) 39ece4f Add bool PSBTInputSigned (Glenn Willen) 70ee1f8 New PartiallySignedTransaction constructor from CTransction (Glenn Willen) a9eab08 Remove redundant txConst parameter to FillPSBT (Glenn Willen) cfdd6b2 More concise conversion of CDataStream to string (Glenn Willen) a3fe125 check that a separator is found for psbt inputs, outputs, and global map (Andrew Chow) Pull request description: This is a backport of #14588, #14377, and #14197's test to 0.17. Tree-SHA512: 07535ec69a878a63b549e5e463345e233f34662dff805202614cf2ffc896c6d1981363e6d06d02db2e02d815075ad8ebdc5f93f637052cff8c8cbe6c8dfa096a
deadalnix
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Oct 5, 2020
Summary: PR description: Currently it doesn't make sure that a separator was found so PSBTs missing a trailing separator would still pass. This fixes that and adds a test case for it. It really only makes sense to check for the separator for the output maps as if an input or global map was missing a separator, the fields following it would be interpreted as belonging to the previous input or global map. However I have added the check for those two anyways to be consistent. Backport of Core [[bitcoin/bitcoin#14377 | PR14377]] Test Plan: `ninja && test/functional/test_runner.py rpc_psbt` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, Fabien Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D7753
Munkybooty
pushed a commit
to Munkybooty/dash
that referenced
this pull request
Jul 28, 2021
… outputs, and global map 4fb3388 check that a separator is found for psbt inputs, outputs, and global map (Andrew Chow) Pull request description: Currently it doesn't make sure that a separator was found so PSBTs missing a trailing separator would still pass. This fixes that and adds a test case for it. It really only makes sense to check for the separator for the output maps as if an input or global map was missing a separator, the fields following it would be interpreted as belonging to the previous input or global map. However I have added the check for those two anyways to be consistent. Tree-SHA512: 50c0c08e201ba02494b369a4d36ddb73e6634eb5a4e4e201c4ef38fd2dbeea2c642b8a04d50c91615da61ecbfade37309e47431368f4b1064539c42015766b50
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
8 participants
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.
Currently it doesn't make sure that a separator was found so PSBTs missing a trailing separator would still pass. This fixes that and adds a test case for it.
It really only makes sense to check for the separator for the output maps as if an input or global map was missing a separator, the fields following it would be interpreted as belonging to the previous input or global map. However I have added the check for those two anyways to be consistent.