Automatic script test generation and checksig tests #4960

pull sipa wants to merge 1 commits into bitcoin:master from sipa:canscript2 changing 8 files +413 −49

sipa commented at 5:29 PM on September 22, 2014: member

This is a replacement/extension of #4949, which also adds code for automatically generating the json tests.

The test generation is always executed (to prevent bitrot), and verified as if it was read from the json file itself (to make sure it remains consistent with the code), but not written out by default.

The reason for actually bothering to still have code to convert these to JSON is because the JSON tests are also used by other implementations.
sipa commented at 7:54 PM on September 22, 2014: member

Rationale for this: makes it trivial to add more tests (which I plan to do as part of BIP62 implementation).
laanwj commented at 8:22 AM on September 23, 2014: member

If you do this it should also verify that the JSON data matches the generated data, to make sure that they don't go out of sync.
laanwj added the label Tests on Sep 23, 2014
sipa commented at 3:29 PM on September 23, 2014: member

That's not possible without deterministic signing, which OpenSSL does not support afaik.
sipa force-pushed on Sep 23, 2014
sipa commented at 6:29 PM on September 23, 2014: member

Rebased on top #4949.
laanwj commented at 9:26 AM on September 24, 2014: member

Then there's nothing to make sure that the JSON files are updated after changing the tests, that worries me. May as well remove them from the repository then, and have people generate them on demand.
sipa commented at 9:33 AM on September 24, 2014: member

I'm fine with adding comparison code between the generated tests and the JSON tests once deterministic signing is available (libsecp256k1 should allow this), but that's no option now.

Before that time, the generated tests are also performed independently as if they were in JSON. The only reason they are actually also stored in JSON is because other implementations use them too.
sipa force-pushed on Sep 26, 2014
Add automatic script test generation, and actual checksig tests 8138cbea3c
sipa force-pushed on Sep 26, 2014
sipa commented at 8:03 PM on September 26, 2014: member

@laanwj Added code to check that at least the comments and scriptPubKeys are identical between the JSON code and the autogenerated tests.
BitcoinPullTester commented at 8:16 PM on September 26, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4960_8138cbea3c405e142d70b43b6c452e1738de3332/ 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.
laanwj commented at 7:48 AM on September 27, 2014: member

@sipa Nice! Indeed, that is enough to verify that the generator needs to be re-run when tests are added/removed.
in src/core_write.cpp:None in 8138cbea3c
```
  16 | @@ -17,6 +17,41 @@
  17 |  
  18 |  using namespace std;
  19 |  
  20 | +string FormatScript(const CScript& script)
```
Diapolo commented at 12:03 PM on September 27, 2014:

Could that return a const string?

laanwj commented at 1:56 PM on September 27, 2014:

The constness of return types (if they're not references or pointers) doesn't matter.
btcdrak commented at 1:48 PM on September 27, 2014: contributor

Good idea.
sipa commented at 8:24 PM on September 27, 2014: member

Can I have some ACKs here? @laanwj @gavinandresen @gmaxwell @jgarzik?

I'm working on implementing BIP62, but with too many ongoing changes to the script and test code, it's hard to know what to base on.
laanwj commented at 3:22 PM on September 29, 2014: member

ACK
gmaxwell commented at 4:03 PM on September 29, 2014: contributor

utACK
sipa merged this on Sep 29, 2014
sipa closed this on Sep 29, 2014
sipa referenced this in commit 2884b9a4ae on Sep 29, 2014
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#4949 Add script tests with actual signatures #5003 Fix vector out of bounds in script tests