test: fix script_p2sh_tests OP_PUSHBACK2/4 missing #15140

pull kodslav wants to merge 2 commits into bitcoin:master from kodslav:test_20190110 changing 1 files +31 −4

kodslav commented at 3:19 PM on January 10, 2019: none

Fixes commit 6b25f29a91 where opcodes where probably lost in translation.

Made two versions, the looped test using the '<<' operator also (stream api).

test: fix script_p2sh_tests OP_PUSHBACK2/4 missing

Fixes commit 6b25f29a91 where opcodes where lost in translation.

d7a9f8b6ac

laanwj added the label Tests on Jan 10, 2019
in src/test/script_p2sh_tests.cpp:222 in d7a9f8b6ac outdated
```
 219 | +    direct.insert(direct.end(), 20, 0); // this is 160bit dummy
 220 |      direct.push_back(OP_EQUAL);
 221 |      BOOST_CHECK(CScript(direct.begin(), direct.end()).IsPayToScriptHash());
 222 | +
 223 | +    //---------><----- cut here
 224 | +    // orig version fixed by re-adding OP_PUSHDATA2/4
```
MarcoFalke commented at 5:22 PM on January 10, 2019:

Can you explain what cut here and orig version means? I think those are confusion to test readers.

kodslav commented at 5:40 PM on January 10, 2019:

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.
clang_format 017877d8a7

in src/test/script_p2sh_tests.cpp:241 in d7a9f8b6ac outdated

 240 |  
 241 | +    //---------><----- cut here
 242 | +
 243 | +    // orig version loopified, and using both interfaces (string and stream)
 244 | +    std::vector<std::vector<unsigned char>>
 245 | +     scripts = {{OP_HASH160, OP_PUSHDATA1, 20}, // 8bit

practicalswift commented at 9:31 AM on January 11, 2019:

Run the new code you're running through clang-format :-)

kodslav commented at 10:50 AM on January 11, 2019:

Thanks!

MarcoFalke commented at 10:01 PM on February 13, 2019: member

@domob1812

in src/test/script_p2sh_tests.cpp:258 in 017877d8a7

 257 | +        script.insert(script.end(), 20, 0);
 258 | +        script.push_back(OP_EQUAL);
 259 | +        BOOST_CHECK(!CScript(script.begin(), script.end()).IsPayToScriptHash());
 260 | +    }
 261 | +
 262 | +    //---------><----- cut here

MarcoFalke commented at 10:03 PM on February 13, 2019:

Could you either please remove the comments here or remove the whole second hunk. I think the fixup should be standalone and minimal.

sipa commented at 10:03 PM on February 13, 2019: member

utACK first version
MarcoFalke commented at 10:05 PM on February 13, 2019: member

ACK, sorry this was my oversight
domob1812 commented at 7:10 AM on February 14, 2019: contributor

Good catch, sorry for missing the OP_PUSHDATA2/4 ones in my change! utACK to the first version of the fix. Perhaps you can do the second change in a follow up, but I agree it should not be mixed with the fix.

Also, please squash the two commits.
fanquake added the label Waiting for author on Jun 26, 2019
laanwj added the label Up for grabs on Oct 25, 2019
laanwj removed the label Waiting for author on Oct 25, 2019
laanwj commented at 10:01 AM on October 25, 2019: member

This has been waiting for the author for more than half a year. Closing with "up for grabs".
laanwj closed this on Oct 25, 2019
fanquake removed the label Up for grabs on Oct 25, 2019
MarcoFalke referenced this in commit 5021ef8d7f on Nov 1, 2019
sidhujag referenced this in commit 29303d46ed on Nov 2, 2019
sidhujag referenced this in commit 15e4e1e915 on Nov 10, 2020
PastaPastaPasta referenced this in commit 45e3eb67c1 on Jun 27, 2021
PastaPastaPasta referenced this in commit 8bfb1e5df2 on Jun 28, 2021
PastaPastaPasta referenced this in commit 537946ba87 on Jun 29, 2021
PastaPastaPasta referenced this in commit 5870d2c49f on Jul 1, 2021
PastaPastaPasta referenced this in commit fba6743773 on Jul 1, 2021
PastaPastaPasta referenced this in commit 960ac7194e on Jul 14, 2021
MarcoFalke locked this on Dec 16, 2021

Contributors

Labels

Linked (view graph)

#17254 test: fix script_p2sh_tests OP_PUSHBACK2/4 missing