Skip to content

Add sighash tests from data file#3975

Merged
laanwj merged 3 commits intobitcoin:masterfrom
maraoz:test/sighash
Apr 1, 2014
Merged

Add sighash tests from data file#3975
laanwj merged 3 commits intobitcoin:masterfrom
maraoz:test/sighash

Conversation

@maraoz
Copy link
Copy Markdown
Contributor

@maraoz maraoz commented Mar 28, 2014

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
Copy link
Copy Markdown
Contributor Author

maraoz commented Mar 28, 2014

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

Comment thread src/test/sighash_tests.cpp Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps catch and report deserialization errors here?

@sipa
Copy link
Copy Markdown
Member

sipa commented Mar 29, 2014

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
Copy link
Copy Markdown
Member

laanwj commented Mar 31, 2014

ACK

@maraoz
Copy link
Copy Markdown
Contributor Author

maraoz commented Mar 31, 2014

updated with fixes suggested by @sipa

@BitcoinPullTester
Copy link
Copy Markdown

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
Copy link
Copy Markdown
Contributor

jgarzik commented Apr 1, 2014

ACK

laanwj added a commit that referenced this pull request Apr 1, 2014
81bfb5a add checks for deserialization errors (Manuel Araoz)
232aa9e Add code generating data/sighash.json test data (Manuel Araoz)
43cb418 Add sighash tests from data file (Manuel Araoz)
@laanwj laanwj merged commit 81bfb5a into bitcoin:master Apr 1, 2014
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants