-
Notifications
You must be signed in to change notification settings - Fork 293
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.
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
txscript: add/group control op code tests. #1349
Conversation
3da278c
to
79a3c76
Compare
txscript/data/script_tests.json
Outdated
|
||
["CHECKLOCKTIMEVERIFY test coverage"], | ||
["", "CHECKLOCKTIMEVERIFY", "NONE", "ERR_EMPTY_STACK"], | ||
["1", "CHECKLOCKTIMEVERIFY", "NONE", "OK"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't add this test as it can be misleading. CLTV is already well tested via the transaction tests since it relies on fields in a transaction.
Perhaps replace it with a comment that states such.
txscript/data/script_tests.json
Outdated
["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"], | ||
["0 NOTIF", "123", "NONE", "ERR_UNBALANCED_CONDITIONAL"], | ||
["NOP", "NOTIF 1 ENDIF", "NONE", "ERR_INVALID_STACK_OPERATION"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this test was added in this PR it needs a comment for what is being tested.
txscript/data/script_tests.json
Outdated
["0 IF", "RETURN ENDIF 1", "NONE", "ERR_UNBALANCED_CONDITIONAL", "still prunable because IF/ENDIF can't span scriptSig/scriptPubKey"], | ||
|
||
["CHECKLOCKTIMEVERIFY test coverage"], | ||
["", "CHECKLOCKTIMEVERIFY", "NONE", "ERR_EMPTY_STACK"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this test was added in this PR it needs a comment for what is being tested.
txscript/data/script_tests.json
Outdated
["1 IF 1", "ENDIF", "NONE", "ERR_UNBALANCED_CONDITIONAL", "IFs don't carry over"], | ||
["1 IF", "1 ENDIF", "NONE", "ERR_UNBALANCED_CONDITIONAL", "IF/ENDIF can't span scriptSig/scriptPubKey"], | ||
["1 IF 0 ENDIF", "1 ENDIF", "NONE", "ERR_UNBALANCED_CONDITIONAL"], | ||
["NOP", "IF 1 ENDIF", "NONE", "ERR_INVALID_STACK_OPERATION"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this test was added in this PR it needs a comment for what is being tested.
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"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this test was added in this PR it needs a comment for what is being tested.
txscript/data/script_tests.json
Outdated
["1", "CHECKLOCKTIMEVERIFY", "NONE", "OK"], | ||
|
||
["CHECKSEQUENCEVERIFY test coverage"], | ||
["", "CHECKSEQUENCEVERIFY", "NONE", "ERR_EMPTY_STACK"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this test was added in this PR it needs a comment for what is being tested.
txscript/data/script_tests.json
Outdated
|
||
["CHECKSEQUENCEVERIFY test coverage"], | ||
["", "CHECKSEQUENCEVERIFY", "NONE", "ERR_EMPTY_STACK"], | ||
["1", "CHECKSEQUENCEVERIFY", "NONE", "OK"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't add this test as it can be misleading. CSV is already well tested via the transaction tests since it relies on fields in a transaction.
Perhaps replace it with a comment that states such.
txscript/data/script_tests.json
Outdated
["NOP", "NOTIF 1 ENDIF", "NONE", "ERR_INVALID_STACK_OPERATION", "They are here to catch copy-and-paste errors"], | ||
["NOP", "VERIFY 1", "NONE", "ERR_INVALID_STACK_OPERATION", "Most of them are duplicated elsewhere,"], | ||
["NOP", "TOALTSTACK 1", "NONE", "ERR_INVALID_STACK_OPERATION", "but, hey, more is always better, right?"], | ||
["NOP", "TOALTSTACK 1", "NONE", "ERR_INVALID_STACK_OPERATION"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this test was added in this PR it needs a comment for what is being tested.
This is work towards #1331