Add sighash tests from data file #3975

pull maraoz wants to merge 3 commits into bitcoin:master from maraoz:test/sighash changing 3 files +599 −5

maraoz commented at 7:33 PM on March 28, 2014: contributor

This adds SignatureHash tests derived from a data file, instead of generating random transactions like the existing test. It allows other libraries (in my case https://github.com/bitpay/bitcore) to import the test vectors knowing they pass in Bitcoin Core. This is my first contribution (and the first C++ code I wrote), so please let me know if it needs corrections.
maraoz commented at 7:39 PM on March 28, 2014: contributor

btw: the new test data was generated using the existing previous test, by adding a few lines that printed the needed data.

in src/test/sighash_tests.cpp:None in b1138911e2 outdated

 159 | +        std::string sigHashHex = test[4].get_str();
 160 | +
 161 | +        uint256 sh;
 162 | +        CDataStream stream(ParseHex(raw_tx), SER_NETWORK, PROTOCOL_VERSION);
 163 | +        CTransaction tx;
 164 | +        stream >> tx;

sipa commented at 4:29 PM on March 29, 2014:

Perhaps catch and report deserialization errors here?

sipa commented at 4:31 PM on March 29, 2014: member

Looks good to me. Would you mind including the code that was used to generate these tests (perhaps in an #if 0'ed block in sighash_tests.cpp)?
laanwj commented at 9:00 AM on March 31, 2014: member

ACK
Add sighash tests from data file 43cb41859e
Add code generating data/sighash.json test data 232aa9e034
add checks for deserialization errors 81bfb5aebf
maraoz commented at 6:12 PM on March 31, 2014: contributor

updated with fixes suggested by @sipa
BitcoinPullTester commented at 6:55 PM on March 31, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/81bfb5aebf983f999d3de12ca4adf8cd4c4a8ae0 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
jgarzik commented at 1:04 AM on April 1, 2014: contributor

ACK
laanwj referenced this in commit d8dcfb9034 on Apr 1, 2014
laanwj merged this on Apr 1, 2014
laanwj closed this on Apr 1, 2014
DrahtBot locked this on Sep 8, 2021

Contributors