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

Branch
arcpatch-D1202_3 (branched from arcpatch-D1202_1)
Lint
Lint Skipped
Unit
No Test Coverage
Build Status
Buildable 2125
Build 2394: arc lint + arc unit

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 ↗(On Diff #3251)

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 ↗(On Diff #3251)

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 ↗(On Diff #3251)

OP_SPLIT needs tests for SCRIPT_ERR_INVALID_STACK_OPERATION and SCRIPT_ERR_INVALID_SPLIT_RANGE

855–856 ↗(On Diff #3251)

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 3246

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...