-
Notifications
You must be signed in to change notification settings - Fork 38.8k
test: fix script_p2sh_tests OP_PUSHBACK2/4 missing #15140
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
Fixes commit 6b25f29 where opcodes where lost in translation.
| BOOST_CHECK(CScript(direct.begin(), direct.end()).IsPayToScriptHash()); | ||
|
|
||
| //---------><----- cut here | ||
| // orig version fixed by re-adding OP_PUSHDATA2/4 |
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.
Can you explain what cut here and orig version means? I think those are confusion to test readers.
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.
Oh, sorry my commit comment was a bit short.
I've made two versions of the fix since I didn't know what you prefer.
The orig version is clearer in what it fixes.
The block below that is perhaps less code, though not perhaps the quickest to understand at-a-glance.
| BOOST_CHECK(!CScript(script.begin(), script.end()).IsPayToScriptHash()); | ||
| } | ||
|
|
||
| //---------><----- cut here |
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.
Could you either please remove the comments here or remove the whole second hunk. I think the fixup should be standalone and minimal.
|
utACK first version |
|
ACK, sorry this was my oversight |
|
Good catch, sorry for missing the Also, please squash the two commits. |
|
This has been waiting for the author for more than half a year. Closing with "up for grabs". |
5710dad test: fix script_p2sh_tests OP_PUSHBACK2/4 missing (kodslav) Pull request description: Cleans up #15140 which fixes commit 6b25f29 where opcodes were lost in translation. ACKs for top commit: laanwj: code review ACK 5710dad Tree-SHA512: 3f7fbcaf0dd199626d9ec9fdf3c5b5c5c2a91c4cfe81fae5b1d5662a48e52cf4bd27c94f8f42ebdfe7a076c5d600ada5661a6902b03eb5dc3dc953f4524345ac
5710dad test: fix script_p2sh_tests OP_PUSHBACK2/4 missing (kodslav) Pull request description: Cleans up bitcoin#15140 which fixes commit 6b25f29 where opcodes were lost in translation. ACKs for top commit: laanwj: code review ACK 5710dad Tree-SHA512: 3f7fbcaf0dd199626d9ec9fdf3c5b5c5c2a91c4cfe81fae5b1d5662a48e52cf4bd27c94f8f42ebdfe7a076c5d600ada5661a6902b03eb5dc3dc953f4524345ac
5710dad test: fix script_p2sh_tests OP_PUSHBACK2/4 missing (kodslav) Pull request description: Cleans up bitcoin#15140 which fixes commit 6b25f29 where opcodes were lost in translation. ACKs for top commit: laanwj: code review ACK 5710dad Tree-SHA512: 3f7fbcaf0dd199626d9ec9fdf3c5b5c5c2a91c4cfe81fae5b1d5662a48e52cf4bd27c94f8f42ebdfe7a076c5d600ada5661a6902b03eb5dc3dc953f4524345ac
5710dad test: fix script_p2sh_tests OP_PUSHBACK2/4 missing (kodslav) Pull request description: Cleans up bitcoin#15140 which fixes commit 6b25f29 where opcodes were lost in translation. ACKs for top commit: laanwj: code review ACK 5710dad Tree-SHA512: 3f7fbcaf0dd199626d9ec9fdf3c5b5c5c2a91c4cfe81fae5b1d5662a48e52cf4bd27c94f8f42ebdfe7a076c5d600ada5661a6902b03eb5dc3dc953f4524345ac
5710dad test: fix script_p2sh_tests OP_PUSHBACK2/4 missing (kodslav) Pull request description: Cleans up bitcoin#15140 which fixes commit 6b25f29 where opcodes were lost in translation. ACKs for top commit: laanwj: code review ACK 5710dad Tree-SHA512: 3f7fbcaf0dd199626d9ec9fdf3c5b5c5c2a91c4cfe81fae5b1d5662a48e52cf4bd27c94f8f42ebdfe7a076c5d600ada5661a6902b03eb5dc3dc953f4524345ac
Fixes commit 6b25f29 where opcodes where probably lost in translation.
Made two versions, the looped test using the '<<' operator also (stream api).