txscript: add/group stack op code tests. #1350

dnldd · 2018-07-16T23:25:09Z

This is work towards #1331

davecgh

Still more to review, but here is some initial feedback you can work on in the mean time.

davecgh · 2018-07-24T08:43:37Z

txscript/data/script_tests.json

@@ -421,6 +421,7 @@

 ["IF test coverage"],
 ["1", "DUP IF ENDIF", "NONE", "OK"],
+["NOP", "IF 1 ENDIF", "NONE", "ERR_INVALID_STACK_OPERATION"],


This is a duplicate of line 469.

davecgh · 2018-07-24T08:44:20Z

txscript/data/script_tests.json

@@ -504,6 +507,7 @@
 ["0 1", "NOTIF IF 1 ELSE 0 ENDIF ENDIF", "NONE", "ERR_EVAL_FALSE"],
 ["1 1", "NOTIF IF 1 ELSE 0 ENDIF ELSE IF 0 ELSE 1 ENDIF ENDIF", "NONE", "ERR_EVAL_FALSE"],
 ["0 0", "NOTIF IF 1 ELSE 0 ENDIF ELSE IF 0 ELSE 1 ENDIF ENDIF", "NONE", "ERR_EVAL_FALSE"],
+["NOP", "NOTIF 1 ENDIF", "NONE", "ERR_INVALID_STACK_OPERATION"],


This is a duplicate of line 512.

davecgh · 2018-07-24T08:45:08Z

txscript/data/script_tests.json

@@ -530,6 +534,7 @@
 ["1", "VERIFY 1", "NONE", "OK"],
 ["1 0x05 0x01 0x00 0x00 0x00 0x00", "VERIFY", "NONE", "OK", "values >4 bytes can be cast to boolean"],
 ["0", "VERIFY 1", "NONE", "ERR_VERIFY"],
+["NOP", "VERIFY 1", "NONE", "ERR_INVALID_STACK_OPERATION"],


Duplicate of line 540.

davecgh · 2018-07-24T08:51:38Z

txscript/data/script_tests.json

+["STACK opcode tests"],
+
+["TOALTSTACK test coverage"],
+["0", "TOALTSTACK 1", "NONE", "OK"],


None of the tests in this section actually ensure the data was pushed to the alt stack nor removed from the primary stack, so they really aren't testing anything. I see that the first one was already there, but, honestly, it's a pretty useless test, so I think it should be changed along with all of the ones modeled on it.

I see that you do address the round-tripping in the next section for FROMALTSTACK although it doesn't address the clearing of the primary data stack. So, I would suggest combining the two and doing the following:

["0", "TOALTSTACK DEPTH 0 EQUALVERIFY FROMALTSTACK 0 EQUAL", "NONE", "OK", "OP_0 must be able to round trip alt data stack"], ["DATA_1 0x7a", "TOALTSTACK DEPTH 0 EQUALVERIFY FROMALTSTACK DATA_1 0x7a EQUAL", "NONE", "OK", "1-byte pushes must be able to round trip alt data stack"], ["DATA_2 0x7a{2}", "TOALTSTACK DEPTH 0 EQUALVERIFY FROMALTSTACK DATA_2 0x7a{2} EQUAL", "NONE", "OK", "2-byte pushes must be able to round trip alt data stack"], ...

Also, I don't see any tests here that ensure the alt stack is cleared in between the public key script and a p2sh redeem script. There is one further down that tests the transition from the signature script to the pubkey script.

davecgh · 2018-07-24T09:13:46Z

txscript/data/script_tests.json

+["DATA_73 0x7a{73}", "TOALTSTACK 1", "NONE", "OK", "Push DATA_73 to ALTSTACK"],
+["DATA_74 0x7a{74}", "TOALTSTACK 1", "NONE", "OK", "Push DATA_74 to ALTSTACK"],
+["DATA_75 0x7a{75}", "TOALTSTACK 1", "NONE", "OK", "Push DATA_75 to ALTSTACK"],
+["NOP", "TOALTSTACK 1", "NONE", "ERR_INVALID_STACK_OPERATION"],


There should be a test similar to this, but which is in the same script since the alt stack is cleared between scripts.

davecgh · 2018-07-24T18:42:59Z

txscript/data/script_tests.json

+["2DROP test coverge"],
+["0 0", "2DROP 1", "NONE", "OK"],
+["OP_2 OP_4", "2DROP 1", "NONE", "OK"],
+["DATA_15 0x7a{15}  DATA_7 0x7a{7}", "2DROP 1", "NONE", "OK"],


Extra space in here. Also, needs a comment describing what is being tested since it was added in this PR.

davecgh · 2018-07-24T18:45:08Z

txscript/data/script_tests.json

+
+["2DROP test coverge"],
+["0 0", "2DROP 1", "NONE", "OK"],
+["OP_2 OP_4", "2DROP 1", "NONE", "OK"],


Numbers should just be written plain instead of using OP_#.

Also, needs a comment describing what is being tested since it was added in this PR.

davecgh · 2018-07-24T18:52:09Z

txscript/data/script_tests.json

+
+["2DUP test coverge"],
+["0 1", "2DUP", "NONE", "OK"],
+["OP_2 OP_4", "2DUP 2DROP ADD 6 EQUAL", "NONE", "OK"],


I would suggest performing the addition check before the drop because otherwise you're not actually testing the values were duplicated properly, only that the stack ended up with 2 extra items that are immediately dropped.

e.g.

["2 4", "2DUP ADD 6 EQUALVERIFY 2DROP 1", "NONE", "OK", "2DUP must duplicate top two stack items"],

Also, needs a comment describing what is being tested since it was added in this PR.

davecgh · 2018-07-24T18:55:41Z

txscript/data/script_tests.json

+["2DUP test coverge"],
+["0 1", "2DUP", "NONE", "OK"],
+["OP_2 OP_4", "2DUP 2DROP ADD 6 EQUAL", "NONE", "OK"],
+["OP_2 OP_2", "2DUP ADD ADD ADD 8 EQUAL", "NONE", "OK"],


I would suggest using two different numbers to help ensure the code isn't just duplicating the top stack entry twice and instead is actually duplicating both entries.

Also, needs a comment describing what is being tested since it was added in this PR.

davecgh · 2018-07-24T22:00:40Z

txscript/data/script_tests.json

+
+["3DUP test coverge"],
+["0 0 1", "3DUP", "NONE", "OK"],
+["OP_1 OP_1 OP_1", "3DUP ADD ADD ADD ADD ADD 6 EQUAL", "NONE", "OK"],


As mentioned elsewhere, please use plain numbers instead of OP_#.

Also, needs a comment describing what is being tested since it was added in this PR.

davecgh · 2018-07-24T22:16:57Z

txscript/data/script_tests.json

+["1", "2 3 2OVER 1", "NONE", "ERR_INVALID_STACK_OPERATION"],
+["1 1 1", "2OVER", "NONE", "ERR_INVALID_STACK_OPERATION"],
+["0 1 0", "2OVER 1", "NONE", "ERR_INVALID_STACK_OPERATION", "2OVER requires two inputs"],
+["", "2OVER 1", "NONE", "ERR_INVALID_STACK_OPERATION", "2OVER requires two inputs"],


Same as above with comment being wrong.

Also, I would suggest reording these so that the number of arguments supplied increase in order.

e.g. 0 args, 1 arg (needs a test), 2 args (needs a test), then the 3 arg tests.

davecgh · 2018-07-24T22:18:23Z

txscript/data/script_tests.json

+["1 1 1", "2OVER", "NONE", "ERR_INVALID_STACK_OPERATION"],
+["0 1 0", "2OVER 1", "NONE", "ERR_INVALID_STACK_OPERATION", "2OVER requires two inputs"],
+["", "2OVER 1", "NONE", "ERR_INVALID_STACK_OPERATION", "2OVER requires two inputs"],
+["'z'{2049} 'a'{8} 0 0", "2OVER", "NONE", "ERR_PUSH_SIZE", "Exceed max allowed input size"],


The problem is not exceeding an input size, rather it's a data push that is too large, and that's tested several other places.

davecgh · 2018-07-24T22:19:07Z

txscript/data/script_tests.json

+["1 2 3 4 5 <3DUP>{173}", "1 2 3 4 5 6 <3DUP>{165}", "NONE", "ERR_STACK_SIZE", ">1,024 stack size"],
+["1 2 3 4 5 <3DUP>{172}", "1 TOALTSTACK 2 TOALTSTACK 3 4 5 6 <3DUP>{166}", "NONE", "ERR_STACK_SIZE", ">1,024 stack+altstack size"],
+["PUSHDATA4 0x01400000 0x62{16385}", "3DUP 1", "NONE", "ERR_SCRIPT_SIZE", "Exceed max allowed script size"],
+["PUSHDATA4 0x01200000 0x62{8193}", "3DUP 1", "NONE", "ERR_PUSH_SIZE", "Exceed max allowed input size"],


The problem is not exceeding an input size, rather it's a data push that is too large, and that's tested several other places.

davecgh · 2018-07-24T22:26:29Z

txscript/data/script_tests.json

+["25 24 23 22 21 20", "2ROT 2ROT 22 EQUAL", "NONE", "OK"],
+["25 24 23 22 21 20", "2ROT 2ROT 2ROT 20 EQUAL", "NONE", "OK"],
+["1 1 1 1 1", "2ROT", "NONE", "ERR_INVALID_STACK_OPERATION"],
+["0 1 0 0", "2ROT 1", "NONE", "ERR_INVALID_STACK_OPERATION", "2ROT requires six inputs"],


Same as above regarding differentiating the comments, ordering the tests by the number of input args, and testing 0, 1, 2, 3, 4, and 5 inputs.

davecgh · 2018-07-24T22:28:07Z

txscript/data/script_tests.json

+["1 1 1 1 1", "2ROT", "NONE", "ERR_INVALID_STACK_OPERATION"],
+["0 1 0 0", "2ROT 1", "NONE", "ERR_INVALID_STACK_OPERATION", "2ROT requires six inputs"],
+["0 1", "2ROT 1", "NONE", "ERR_INVALID_STACK_OPERATION", "2ROT requires six inputs"],
+["", "2ROT 1", "NONE", "ERR_INVALID_STACK_OPERATION", "2ROT requires six inputs"],


All of these tests only operate on numeric data, however, the 2ROT opcode works with arbitrary data, so please add tests which work with non-numeric data as well. They can follow the ["25 24 23 22 21 20", ...] series of tests, but using non-numeric data pushes instead.

Also, they should use EQUALVERIFY and the necessary mix of 3DROP/2DROP/DROP opcodes to consume all remaining data one the stack after the EQUALVERIFY followed by 1 so they adhere to the CLEANSTACK requirements. In fact, please add that requirement to all of the expected passing tests you add to ensure that's the case as the original issue requested.

davecgh

There are several tests added which aren't CLEANSTACK, but I didn't call them all out since it's mentioned inline in a specific comment.

davecgh · 2018-07-24T23:16:47Z

txscript/data/script_tests.json

+["IFDUP", "DEPTH 0 EQUAL", "NONE", "ERR_INVALID_STACK_OPERATION"],
+["NOP", "IFDUP 1", "NONE", "ERR_INVALID_STACK_OPERATION"],
+["", "IFDUP 1", "NONE", "ERR_INVALID_STACK_OPERATION", "IFDUP requires an input"],
+["'z'{2049}", "IFDUP 1", "NONE", "ERR_PUSH_SIZE", "Exceed max allowed input size"],


As mentioned elsewhere, the issue here is not exceeding an input size, rather it's a data push that is too large, and that's tested several other places.

davecgh · 2018-07-24T23:22:36Z

txscript/data/script_tests.json

+
+["DUP test coverge"],
+["1", "DUP", "NONE", "OK"],
+["1", "DUP ADD 2 EQUAL ", "NONE", "OK"],


Needs a comment describing what is being tested since it was added in this PR.

davecgh · 2018-07-24T23:24:34Z

txscript/data/script_tests.json

+["DUP", "DEPTH 0 EQUAL", "NONE", "ERR_INVALID_STACK_OPERATION"],
+["1", "DUP 1 ADD 2 EQUALVERIFY 0 EQUAL", "NONE", "ERR_EVAL_FALSE"],
+["", "DUP 1", "NONE", "ERR_INVALID_STACK_OPERATION", "DUP requires an input"],
+["'z'{2049}", "DUP 1", "NONE", "ERR_PUSH_SIZE", "Exceed max allowed input size"],


As mentioned elsewhere, the issue here is not exceeding an input size, rather it's a data push that is too large, and that's tested several other places.

davecgh · 2018-07-24T23:24:42Z

txscript/data/script_tests.json

+["0 1", "OVER DEPTH 3 EQUALVERIFY", "NONE", "ERR_EVAL_FALSE"],
+["1", "OVER 1", "NONE", "ERR_INVALID_STACK_OPERATION", "OVER requires two inputs"],
+["", "OVER 1", "NONE", "ERR_INVALID_STACK_OPERATION", "OVER requires two inputs"],
+["'z'{2049} 0", "OVER", "NONE", "ERR_PUSH_SIZE", "Exceed max allowed input size"],


As mentioned elsewhere, the issue here is not exceeding an input size, rather it's a data push that is too large, and that's tested several other places.

davecgh · 2018-07-24T23:24:49Z

txscript/data/script_tests.json

+["1 0", "TUCK DEPTH 3 EQUALVERIFY SWAP 2DROP", "NONE", "ERR_EVAL_FALSE"],
+["1", "TUCK 1", "NONE", "ERR_INVALID_STACK_OPERATION", "TUCK requires two inputs"],
+["", "TUCK 1", "NONE", "ERR_INVALID_STACK_OPERATION", "TUCK requires two inputs"],
+["0 'z'{2049}", "TUCK", "NONE", "ERR_PUSH_SIZE", "Exceed max allowed input size"],


As mentioned elsewhere, the issue here is not exceeding an input size, rather it's a data push that is too large, and that's tested several other places.

davecgh · 2018-07-24T23:41:55Z

txscript/data/script_tests.json

+["1", "SWAP 1", "NONE", "ERR_INVALID_STACK_OPERATION", "SWAP requires two inputs"],
+["", "SWAP 1", "NONE", "ERR_INVALID_STACK_OPERATION", "SWAP requires two inputs"],
+
+["TUCK test coverage"],


s/coverge/coverage/

davecgh · 2018-07-24T23:43:50Z

txscript/data/script_tests.json

+
+["OVER test coverge"],
+["1 0", "OVER", "NONE", "OK"],
+["1 0", "OVER ADD ADD 2 EQUAL", "NONE", "OK"],


Needs a comment describing what is being tested since it was added in this PR.

davecgh · 2018-07-24T23:44:42Z

txscript/data/script_tests.json

+["1", "OVER", "NONE", "ERR_INVALID_STACK_OPERATION"],
+["0 1", "OVER DEPTH 3 EQUALVERIFY", "NONE", "ERR_EVAL_FALSE"],
+["1", "OVER 1", "NONE", "ERR_INVALID_STACK_OPERATION", "OVER requires two inputs"],
+["", "OVER 1", "NONE", "ERR_INVALID_STACK_OPERATION", "OVER requires two inputs"],


As mentioned elsewhere, needs a unique comment

davecgh · 2018-07-25T01:34:34Z

txscript/data/script_tests.json

+["PICK test coverge"],
+["1 0 0 0 3", "PICK", "NONE", "OK"],
+["1 0", "PICK", "NONE", "OK"],
+["1 0", "PICK DEPTH 2 EQUAL", "NONE", "OK"],


Needs a comment describing what is being tested since it was added in this PR.

davecgh · 2018-07-25T02:05:53Z

txscript/data/script_tests.json

+["1 0 0 0 3", "PICK", "NONE", "OK"],
+["1 0", "PICK", "NONE", "OK"],
+["1 0", "PICK DEPTH 2 EQUAL", "NONE", "OK"],
+["1 0", "PICK ADD 2 EQUAL", "NONE", "OK"],


Needs a comment describing what is being tested since it was added in this PR.

dnldd force-pushed the stack_tests branch 5 times, most recently from a2c4ba4 to b0c69ff Compare July 19, 2018 18:56

dnldd changed the title ~~[wip]txscript: add/group stack op code tests.~~ txscript: add/group stack op code tests. Jul 19, 2018

dnldd force-pushed the stack_tests branch 2 times, most recently from 02a5971 to 342f711 Compare July 20, 2018 16:57

davecgh added this to the 1.3.0 milestone Jul 20, 2018

dnldd force-pushed the stack_tests branch 3 times, most recently from ff25de1 to 7166947 Compare July 23, 2018 01:36

davecgh requested changes Jul 24, 2018

View reviewed changes

davecgh requested changes Jul 25, 2018

View reviewed changes

dnldd force-pushed the stack_tests branch 3 times, most recently from 736d6e1 to 88ae4c8 Compare July 25, 2018 23:53

txscript: add/group stack op code tests.

f60d7d4

dnldd force-pushed the stack_tests branch from 88ae4c8 to f60d7d4 Compare July 26, 2018 04:17

davecgh approved these changes Jul 26, 2018

View reviewed changes

davecgh merged commit 34a33e1 into decred:master Jul 26, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

txscript: add/group stack op code tests. #1350

txscript: add/group stack op code tests. #1350

dnldd commented Jul 16, 2018

davecgh left a comment

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 24, 2018 •

edited

Loading

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh left a comment

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 24, 2018

davecgh Jul 25, 2018

davecgh Jul 25, 2018

txscript: add/group stack op code tests. #1350

txscript: add/group stack op code tests. #1350

Conversation

dnldd commented Jul 16, 2018

davecgh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davecgh Jul 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davecgh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davecgh Jul 24, 2018 •

edited

Loading