test: script_error checking in script_invalid tests #7517

pull laanwj wants to merge 3 commits into bitcoin:master from laanwj:2016_02_test_script_errors changing 2 files +685 −553

laanwj commented at 3:49 PM on February 11, 2016: member

Check the returned script_error. Add expected script_error for generated as well as custom tests.

The specific error is not part of consensus, however it could avoid unclear reporting issues such as #6862 in the future. It also makes it easier to verify that all potential error conditions are checked.

It also reveals that overflow conditions return UNKNOWN_ERROR, there appears to be no specific error code for that.

Fixes #7513.

Changes are best reviewed using git --word-diff.

test: Move non-generated script_invalid test to the correct place

This test was introduced in 9fadf1c874f938f87395495776dbae896551873d,
but accidentally added in the autogenerated area.

b0ff8572ae

laanwj added the label Tests on Feb 11, 2016
laanwj force-pushed on Feb 11, 2016
laanwj commented at 4:02 PM on February 11, 2016: member
These are not returned in any of the script tests:
- SCRIPT_ERR_CHECKMULTISIGVERIFY (OP_CHECKMULTISIGVERIFY)
- SCRIPT_ERR_CHECKSIGVERIFY (OP_CHECKSIGVERIFY)
- SCRIPT_ERR_NUMEQUALVERIFY (OP_NUMEQUALVERIFY)
- SCRIPT_ERR_NEGATIVE_LOCKTIME (OP_CHECKLOCKTIMEVERIFY - BIP65)
- SCRIPT_ERR_UNSATISFIED_LOCKTIME (OP_CHECKLOCKTIMEVERIFY - BIP65)
test: Re-introduce JSON pretty printing in test builder 2317ad7c56

test: Script_error checking in script_invalid tests

Check the returned script_error. Add expected script_error
for generated as well as custom tests.

The specific error is not part of consensus, however
it could avoid unclear reporting issues such as #6862 in the future.

Fixes #7513.

0ecb3401fe

laanwj force-pushed on Feb 11, 2016
laanwj commented at 1:09 PM on March 1, 2016: member

Ping @cfields @sipa @gmaxwell
laanwj closed this on Mar 2, 2016
theuni commented at 1:07 AM on March 3, 2016: member

@laanwj Closed on purpose? I didn't see this before, but sounds like a good idea for sure.
laanwj commented at 1:32 PM on March 3, 2016: member

@theuni Closed because I've been asking for review a few times for this, but never received any answer (neither positive nor negative) so I was assuming it was too much review work relative to some extra testing coverage.

Happy to reopen though!
laanwj reopened this on Mar 3, 2016
laanwj merged this on Mar 14, 2016
laanwj closed this on Mar 14, 2016
laanwj referenced this in commit f1ca8915bb on Mar 14, 2016
MarcoFalke referenced this in commit 7610d14fcd on May 4, 2016
MarcoFalke referenced this in commit 87129b24e1 on May 4, 2016
nomnombtc referenced this in commit 0b46f8ee1a on Nov 12, 2016
nomnombtc referenced this in commit 4554c67365 on Nov 12, 2016
nomnombtc referenced this in commit a4ec135bf5 on Nov 13, 2016
sickpig referenced this in commit b251e59aaf on Nov 14, 2016
MarcoFalke locked this on Sep 8, 2021

Contributors

laanwj

theuni

Labels

Tests

Linked (view graph)

#6862 Unclear rejection messages in master #7513 Verify script errors as part of tx_invalid tests