Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

txscript: add p2sh opcode tests. #1381

Merged
merged 1 commit into from
Sep 11, 2018
Merged

txscript: add p2sh opcode tests. #1381

merged 1 commit into from
Sep 11, 2018

Conversation

dnldd
Copy link
Member

@dnldd dnldd commented Aug 5, 2018 •
edited
Loading

This is work towards #1331.

@davecgh davecgh added this to the 1.4.0 milestone Aug 8, 2018
davecgh
davecgh requested changes Sep 10, 2018
View reviewed changes
txscript/data/script_tests.json Outdated

["OP_[2...16] test coverage"],
["OP_2", "2 EQUAL", "NONE", "OK", "Push 2"],
["DATA_1 OP_2", "HASH160 DATA_20 0x21d15fc0bc32237b0475986afbe89b4bd9109eba EQUAL", "NONE", "OK", "OP_2 P2SH"],
Copy link
Member

Choose a reason for hiding this comment

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

These aren't really testing the opcodes other than that they're non-zero. They should all be similar to:

["DATA_3 OP_2 2 EQUAL", "HASH160 DATA_20 0x21...... EQUAL", "NONE",  "OK", "OP_2 P2SH"]

txscript/data/script_tests.json Outdated

["Control opcode tests"],

["NOP test coverage"],
["NOP", "NOP 1", "NONE", "OK"],
["DATA_2 DATA_1 NOP", "HASH160 DATA_20 0x746dc3dac3603258451d1c6f9410796740981baf EQUAL", "NONE", "OK", "NOP P2SH"],
Copy link
Member

Choose a reason for hiding this comment

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

Why is the DATA_1 here? Doesn't look like you're intending to push the NOP to the stack.

txscript/data/script_tests.json Outdated
@@ -448,6 +544,7 @@
["'' 1", "IF SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ENDIF 0x14 0x68ca4fec736264c13b859bac43d5173df6871682 EQUAL", "NONE", "OK"],
["0", "IF 1 IF RETURN ELSE RETURN ELSE RETURN ENDIF ELSE 1 IF 1 ELSE RETURN ELSE 1 ENDIF ELSE RETURN ENDIF ADD 2 EQUAL", "NONE", "OK", "Nested ELSE ELSE"],
["1 0x01 0x80", "IF 0 ENDIF", "NONE", "OK", "negative 0 is false"],
["DATA_6 0 DATA_4 IF ELSE 1 ENDIF", "HASH160 DATA_20 0x6df4bbb67fca39714081eaed3bb8a5c2107fdfc1 EQUAL", "NONE", "OK", "IF P2SH"],
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't look like it's doing what you intend. It's not actually testing the if because it's just pushing those opcodes as if they were bytes.

It looks like it should be:

["0 DATA_4 IF ELSE 1 ENDIF", "HASH160 DATA_20 0x.... EQUAL", "NONE", "OK", "IF P2SH"]

That way the actual redeem script is IF ELSE 1 ENDIF and it will pull the 0 from the data stack from the signature script.

txscript/data/script_tests.json Outdated
@@ -505,6 +602,7 @@
["0", "NOTIF 1 ENDIF", "NONE", "OK"],
["0", "NOTIF 1 ELSE 0 ELSE 1 ENDIF ADD 2 EQUAL", "NONE", "OK"],
["'' 0", "NOTIF SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ELSE ELSE SHA1 ENDIF 0x14 0x68ca4fec736264c13b859bac43d5173df6871682 EQUAL", "NONE", "OK"],
["DATA_5 0 DATA_3 NOTIF 1 ENDIF", "HASH160 DATA_20 0xa700df9fa14ea731f15a256b9f1b13ebab0584cf EQUAL", "NONE", "OK", "NOTIF P2SH"],
Copy link
Member

Choose a reason for hiding this comment

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

Same here. This is not actually testing the NOTIF.

["0 DATA_3 NOTIF 1 ENDIF", "HASH160 DATA_20 0x... EQUAL", "NONE", "OK", "NOTIF P2SH"],

txscript/data/script_tests.json Outdated
["0", "VERIFY 1", "NONE", "ERR_VERIFY"],
["1", "VERIFY", "NONE", "ERR_EMPTY_STACK"],
["1", "VERIFY 0", "NONE", "ERR_EVAL_FALSE"],
["NOP", "VERIFY 1", "NONE", "ERR_INVALID_STACK_OPERATION", "VERIFY requires an input"],

["RETURN test coverage"],
["0", "IF RETURN ENDIF 1", "NONE", "OK", "RETURN only works if executed"],
["DATA_6 0 DATA_4 IF RETURN ENDIF 1", "HASH160 DATA_20 0xb92f457e8bdf98a0e1a1980c3792f2f14f83a661 EQUAL", "NONE", "OK", "RETURN P2SH"],
Copy link
Member

Choose a reason for hiding this comment

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

Same deal here.

txscript/data/script_tests.json Outdated
@@ -644,6 +744,7 @@
["DATA_75 0x7a{75}", "TOALTSTACK DEPTH 0 EQUALVERIFY FROMALTSTACK DATA_75 0x7a{75} EQUAL", "NONE", "OK", "75-byte pushes must be able to round trip alt data stack"],
["PUSHDATA1 0x4c 0x7a{76}", "TOALTSTACK DEPTH 0 EQUALVERIFY FROMALTSTACK PUSHDATA1 0x4c 0x7a{76} EQUAL", "NONE", "OK", "76-byte pushes must be able to round trip alt data stack"],
["PUSHDATA2 0x0001 0x7a{256}", "TOALTSTACK DEPTH 0 EQUALVERIFY FROMALTSTACK PUSHDATA2 0x0001 0x7a{256} EQUAL", "NONE", "OK", "256-byte pushes must be able to round trip alt data stack"],
["DATA_11 10 0 11 DATA_7 TOALTSTACK DROP FROMALTSTACK ADD 21 EQUAL", "HASH160 DATA_20 0xd855c91d7d6b2c4f6dc680418978f83868e08560 EQUAL", "NONE", "OK", "FROMALTSTACK/TOALTSTACK P2SH"],
Copy link
Member

Choose a reason for hiding this comment

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

Same. All this is testing is a push to the data stack, not the actual redeem script you want.

I'll refrain from commenting on the rest, but a lot of the others below are incorrect in the same way.

@davecgh davecgh mentioned this pull request Sep 10, 2018
Merged
@dnldd dnldd force-pushed the p2sh-tests branch 6 times, most recently from 40057c4 to dcbcc5e Compare September 11, 2018 14:09
davecgh
davecgh requested changes Sep 11, 2018
View reviewed changes
txscript/data/script_tests.json Outdated

["DROP test coverage"],
["NOP", "DROP 1", "NONE", "ERR_INVALID_STACK_OPERATION", "DROP requires an item on the stack and NOP must not be treated as one"],
["", "DROP 1", "NONE", "ERR_INVALID_STACK_OPERATION", "DROP requires an item on the stack, the stack has no items"],
["DROP", "DEPTH 0 EQUAL", "NONE", "ERR_INVALID_STACK_OPERATION", "DROP requires an item on the stack, the stack has no items"],
["0", "DROP 1", "CLEANSTACK", "OK"],
["0 DROP", "DEPTH 0 EQUAL", "CLEANSTACK", "OK"],
["0", "DUP 1 ADD 1 EQUALVERIFY 0 EQUAL", "CLEANSTACK", "OK"],
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this was unintentionally deleted.

@davecgh davecgh merged commit e0222e3 into decred:master Sep 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davecgh davecgh davecgh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.4.0
Development

Successfully merging this pull request may close these issues.
2 participants
@dnldd @davecgh