Page MenuHomePhabricator

Pre-deployment for Monolith Opcode Deployment
AbandonedPublic

Authored by schancel on Mar 15 2018, 22:57.

Details

Reviewers
deadalnix
jasonbcox
movrcx
Group Reviewers
Restricted Project
Restricted Owners Package(Owns No Changed Paths)
Summary

Adds scaffolding for opcode deployment

Test Plan

make test

Diff Detail

Event Timeline

Owners added a reviewer: Restricted Owners Package.Mar 15 2018, 22:57
Herald added 1 blocking reviewer(s): Restricted Project. ยท View Herald TranscriptMar 15 2018, 22:57

Fix spaces to tab in Cmake file

schancel added inline comments.
src/script/interpreter.cpp
1227 โ†—(On Diff #3213)

break;

1234 โ†—(On Diff #3213)

break;

This looks good besides the issues we discussed with the tests.

Fixup: add missing breaks, rename op_codes.cpp to monolith_opcodes.cpp

Fix tab/space issue in Cmake file

deadalnix requested changes to this revision.Mar 16 2018, 16:12
deadalnix added a subscriber: deadalnix.

This absolutely should break tests as it changes the error code for some operations.

src/script/script_error.cpp
60 โ†—(On Diff #3220)

dito

src/script/script_error.h
41 โ†—(On Diff #3220)

These error codes are not being used right now.

This revision now requires changes to proceed.Mar 16 2018, 16:12

Fix monolith_opcodes.cpp for disabled opcodes

Fixup for diff 5 (wrong commit submitted)

Rename this for clarity (in scripts_test.cpp)

static const unsigned int flags = SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC;
static const unsigned int defaultTestFlags = SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC;
jasonbcox added inline comments.
src/test/monolith_opcodes.cpp
119โ€“135 โ†—(On Diff #3239)

Not blocking for the release, but this can be rewritten to loop through these values rather than copy-pasting the test.

jasonbcox requested changes to this revision.Mar 17 2018, 00:49
jasonbcox added inline comments.
src/test/monolith_opcodes.cpp
60 โ†—(On Diff #3245)

Unnecessary comment

67 โ†—(On Diff #3245)

Remove unnecessary comment and put the } else { on the same line

105 โ†—(On Diff #3245)

Same here

This revision now requires changes to proceed.Mar 17 2018, 00:49

Fixup for opcode input order

Rebase off phabricator master

jasonbcox requested changes to this revision.Mar 17 2018, 04:42
jasonbcox added inline comments.
src/test/data/script_tests.json
846

Adding some tests as commented in the slack channel for posterity:
["0x02 0x00000011", "AND", "P2SH,STRICTENC,MONOLITH_OPCODES", "SCRIPT_ERR_INVALID_BITWISE_LENGTH"],
["0x00000011 0x02", "AND", "P2SH,STRICTENC,MONOLITH_OPCODES", "SCRIPT_ERR_INVALID_BITWISE_LENGTH"],
["0x02 0x00000011", "OR", "P2SH,STRICTENC,MONOLITH_OPCODES", "SCRIPT_ERR_INVALID_BITWISE_LENGTH"],
["0x00000011 0x02", "OR", "P2SH,STRICTENC,MONOLITH_OPCODES", "SCRIPT_ERR_INVALID_BITWISE_LENGTH"],
["0x02 0x00000011", "XOR", "P2SH,STRICTENC,MONOLITH_OPCODES", "SCRIPT_ERR_INVALID_BITWISE_LENGTH"],
["0x00000011 0x02", "XOR", "P2SH,STRICTENC,MONOLITH_OPCODES", "SCRIPT_ERR_INVALID_BITWISE_LENGTH"],

850

Adding some tests as commented in the slack channel for posterity:
["1 0", "DIV", "P2SH,STRICTENC,MONOLITH_OPCODES", "SCRIPT_ERR_DIV_BY_ZERO"],
["1 0", "MOD", "P2SH,STRICTENC,MONOLITH_OPCODES", "SCRIPT_ERR_DIV_BY_ZERO"],

853โ€“854

OP_SPLIT needs tests for SCRIPT_ERR_INVALID_STACK_OPERATION and SCRIPT_ERR_INVALID_SPLIT_RANGE

855โ€“856

BIN2NUM and NUM2BIN need tests for SCRIPT_ERR_INVALID_STACK_OPERATION

This revision now requires changes to proceed.Mar 17 2018, 04:42
schancel added a reviewer: movrcx.