Page MenuHomePhabricator

Adds Bitwise functionality into interpreter.cpp
AbandonedPublic

Authored by schancel on Feb 15 2018, 00:42.

Details

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

Adds flag for enabling monolith opcodes and re-implements OP_AND, OP_OR, and OP_XOR.

Co-authored-by: Joshua Yabut <yabut.joshua@gmail.com>
Co-authored-by: Marcos Mayorga <mm@mm-studios.com>
Co-authored-by: Daniel Connolly <daniel@dconnolly.com>

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint SkippedExcuse: Unnecessary changes to previous code.
Unit
No Unit Test Coverage
Build Status
Buildable 2131
Build 2406: Bitcoin ABC Buildbot (legacy)
Build 2405: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
danconnolly edited the test plan for this revision. (Show Details)Mar 10 2018, 14:57
danconnolly updated this revision to Diff 3143.Mar 10 2018, 18:11
danconnolly edited the test plan for this revision. (Show Details)

reorganize D1090

danconnolly updated this revision to Diff 3150.Mar 12 2018, 09:00

moved flag from parent diff into this one

danconnolly edited the summary of this revision. (Show Details)Mar 12 2018, 09:13
danconnolly added inline comments.Mar 12 2018, 09:25
test/functional/test_framework/script.py
344 ↗(On Diff #3150)

this should not be here, will fix. still getting used to phabricator

danconnolly updated this revision to Diff 3151.Mar 12 2018, 10:34

remove erroneously included changes to script.py

danconnolly edited the summary of this revision. (Show Details)Mar 12 2018, 10:37
deadalnix requested changes to this revision.Mar 13 2018, 13:50
deadalnix added inline comments.
src/Makefile.test.include
64 ↗(On Diff #3151)

op_codes.

src/script/interpreter.cpp
351 ↗(On Diff #3151)

You should have prepared this flag with negative testing. That makes it very clear what new behavior is added to the flag with each diff. It would also ensure that all your diffs do not need to depend on each other, which would ensure you move much faster.

808 ↗(On Diff #3151)

This is terrible. i original asked for C++11 style loops (so not this) but it turns out that the index is used in multiple container, so that's not even a good idea.

src/script/script_error.cpp
50 ↗(On Diff #3151)

If this is an error about input length, then an error code specifying so would be more appropriate.

src/test/op_code.cpp
11 ↗(On Diff #3151)

nope

19 ↗(On Diff #3151)

Other tests do have an infrastructure to generate json tests and then run them. If you are going to go that road, it might as well be useful and generate reusable test cases.

48 ↗(On Diff #3151)

Use explicit names.

70 ↗(On Diff #3151)

dito.

158 ↗(On Diff #3151)

There is no check for empty elements.

More generally, build a set of element and expected result when you run them. Run through various set of flags (mandatory, standard and nothing seems like good ideas) and check they fail. Do the same with the monolith flag in addition, check they give the expected result. Both way as binary ops are commutative.

Make sure the large vectors are filled with various data combination at the start and end so you exercise cases where possible signed stack items would show up.

265 ↗(On Diff #3151)

I'm not sure where this test is going. If we want to have a set of test cases, script_tests.json seems like the appropriate place to put them. Obviously, having a custom test would allow to grill thing more exhaustively, so that's a good thing, but this test doesn't do this. We should probably have both.

This revision now requires changes to proceed.Mar 13 2018, 13:50
movrcx updated this revision to Diff 3191.Mar 15 2018, 16:18

Rename op_code.cpp to op_codes.cpp

movrcx updated this revision to Diff 3192.Mar 15 2018, 16:19

Rename op_code.cpp to op_codes.cpp

movrcx marked an inline comment as done.Mar 15 2018, 16:22
macros updated this revision to Diff 3193.Mar 15 2018, 16:58

Updated for loops

macros updated this revision to Diff 3194.Mar 15 2018, 17:03

xor typo

movrcx marked an inline comment as done.Mar 15 2018, 17:07
movrcx updated this revision to Diff 3195.Mar 15 2018, 17:12

Update Bitwise error message

movrcx updated this revision to Diff 3196.Mar 15 2018, 17:15

Fixup: Bitwise error message

Harbormaster completed remote builds in B2083: Diff 3195.
movrcx updated this revision to Diff 3197.Mar 15 2018, 17:22
movrcx marked an inline comment as done.

Update op_codes.cpp to use SCRIPT_ERR_INVALID_BITWISE_LENGTH

movrcx updated this revision to Diff 3198.Mar 15 2018, 17:56

Rename op_code.cpp file to op_codes.cpp

movrcx updated this revision to Diff 3222.Mar 16 2018, 01:35

Cleaning up due to D1202

movrcx updated this revision to Diff 3248.Mar 17 2018, 01:23

Adjust input order

movrcx updated this revision to Diff 3252.Mar 17 2018, 02:14

Rebased off master

schancel commandeered this revision.Mar 18 2018, 01:38
schancel abandoned this revision.
schancel added a reviewer: movrcx.