Page MenuHomePhabricator
Differential D1202

Pre-deployment for Monolith Opcode Deployment
AbandonedPublic
Actions

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

movrcx created this revision.Mar 15 2018, 22:57
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
movrcx updated this revision to Diff 3213.Mar 15 2018, 23:00

Fix spaces to tab in Cmake file

schancel added a subscriber: schancel.Mar 15 2018, 23:09
schancel added inline comments.
src/script/interpreter.cpp
1227 ↗(On Diff #3213)

break;

1234 ↗(On Diff #3213)

break;

schancel added a comment.Mar 15 2018, 23:18

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

schancel added a child revision: D1090: Adds Bitwise functionality into interpreter.cpp.Mar 15 2018, 23:18
movrcx updated this revision to Diff 3219.Mar 16 2018, 00:12

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

movrcx updated this revision to Diff 3220.Mar 16 2018, 00:14

Fix tab/space issue in Cmake file

teamcity added a subscriber: teamcity.Mar 16 2018, 00:46

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

Harbormaster failed remote builds in B2095: Diff 3212!Mar 16 2018, 01:00
teamcity added a comment.Mar 16 2018, 01:00

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging failed.

teamcity added a comment.Mar 16 2018, 01:00

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

teamcity added a comment.Mar 16 2018, 01:15

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging failed.

Harbormaster failed remote builds in B2096: Diff 3213!Mar 16 2018, 01:15
movrcx mentioned this in D1090: Adds Bitwise functionality into interpreter.cpp.Mar 16 2018, 01:35
teamcity added a comment.Mar 16 2018, 01:45

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

teamcity added a comment.Mar 16 2018, 01:49

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

teamcity added a comment.Mar 16 2018, 02:03

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging failed.

teamcity added a comment.Mar 16 2018, 02:03

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging failed.

Harbormaster failed remote builds in B2101: Diff 3219!Mar 16 2018, 02:04
Harbormaster failed remote builds in B2102: Diff 3220!
movrcx mentioned this in D1093: Adds OP_DIV functionality into interpreter.cpp.Mar 16 2018, 02:05
movrcx mentioned this in D1095: Adds OP_MOD functionality into interpreter.cpp.Mar 16 2018, 02:29
movrcx mentioned this in D1097: Add support for CAT, and SPLIT opcodes.Mar 16 2018, 02:37
movrcx mentioned this in D1099: Adds OP_SPLIT functionality into interpreter.cpp.Mar 16 2018, 02:49
movrcx mentioned this in D1101: Adds OP_BIN2NUM functionality into interpreter.cpp.Mar 16 2018, 02:57
movrcx mentioned this in D1103: Add support for NUM2BIN, and BIN2NUM opcodes.Mar 16 2018, 03:03
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
movrcx updated this revision to Diff 3238.Mar 16 2018, 16:53

Fix monolith_opcodes.cpp for disabled opcodes

teamcity added a comment.Mar 16 2018, 16:57

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

movrcx updated this revision to Diff 3239.Mar 16 2018, 17:06

Fixup for diff 5 (wrong commit submitted)

teamcity added a comment.Mar 16 2018, 17:12

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging failed.

Harbormaster failed remote builds in B2117: Diff 3238!Mar 16 2018, 17:12
teamcity added a comment.Mar 16 2018, 17:12

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

teamcity added a comment.Mar 16 2018, 17:28

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging failed.

Harbormaster failed remote builds in B2118: Diff 3239!Mar 16 2018, 17:28
schancel added a comment.EditedMar 16 2018, 18:58

script_test.cpp doesn't have the new error codes added in it.

This list also needs to be updated:
https://reviews.bitcoinabc.org/source/bitcoin-abc/browse/master/src/test/script_tests.cpp;0d1b49c9f37f8549540521e6e02a27c261c6da5a$54

schancel added a comment.Mar 16 2018, 19:01

Also, flags need updating here: https://reviews.bitcoinabc.org/source/bitcoin-abc/browse/master/src/test/scriptflags.cpp;0d1b49c9f37f8549540521e6e02a27c261c6da5a$16

schancel added a comment.EditedMar 16 2018, 19:04

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 a subscriber: jasonbcox.Mar 16 2018, 19:30
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.

movrcx updated this revision to Diff 3245.Mar 17 2018, 00:41

Add script json tests

teamcity added a comment.Mar 17 2018, 00:43

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

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
teamcity added a comment.Mar 17 2018, 00:59

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging failed.

Harbormaster failed remote builds in B2124: Diff 3245!Mar 17 2018, 00:59
movrcx updated this revision to Diff 3246.Mar 17 2018, 01:09

Fixup for opcode input order

Harbormaster completed remote builds in B2125: Diff 3246.Mar 17 2018, 01:09
movrcx updated this revision to Diff 3247.Mar 17 2018, 01:20

Rebase off phabricator master

teamcity added a comment.Mar 17 2018, 01:21

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

teamcity added a comment.Mar 17 2018, 01:35

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging failed.

Harbormaster failed remote builds in B2126: Diff 3247!Mar 17 2018, 01:35
movrcx updated this revision to Diff 3251.Mar 17 2018, 02:13

Undo previous diff

teamcity added a comment.Mar 17 2018, 02:14

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

teamcity added a comment.Mar 17 2018, 02:28

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging failed.

Harbormaster failed remote builds in B2130: Diff 3251!Mar 17 2018, 02:29
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 commandeered this revision.Mar 18 2018, 02:13
schancel added a reviewer: movrcx.
schancel abandoned this revision.Mar 18 2018, 02:56

Revision Contents
Changeset List

Diff 3251

View Options

src/Makefile.test.include

Loading...
View Options

src/script/interpreter.h

Loading...
View Options

src/script/interpreter.cpp

Loading...
View Options

src/script/script_error.h

Loading...
View Options

src/script/script_error.cpp

Loading...
View Options

src/test/CMakeLists.txt

Loading...
View Options

src/test/data/script_tests.json

Loading...
View Options

src/test/monolith_opcodes.cpp

Loading...
View Options

src/test/scriptflags.cpp

Loading...