Page MenuHomePhabricator

Add support for AND, OR and XOR opcodes
ClosedPublic

Authored by deadalnix on Mar 18 2018, 01:13.

Details

Reviewers
jasonbcox
schancel
Group Reviewers
Restricted Project
Maniphest Tasks
Restricted Maniphest Task
Commits
rSTAGING3e8288e5b42e: Add support for AND, OR and XOR opcodes
rABC3e8288e5b42e: Add support for AND, OR and XOR opcodes
Summary

These opcodes are gated by a flag and compute what you'd expect for
binary ops. Added cases to the json test cases. In addition, added a
new unit tests that check if the opcodes behave in the expected way
in a large variety of situations.

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>
Co-authored-by: Shammah Chancellor <shammah.chancellor@bitcoinabc.org>

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jasonbcox requested changes to this revision.Mar 18 2018, 01:23
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/test/monolith_opcodes.cpp
21 ↗(On Diff #3266)

Bitwize -> Bitwise. many instances of this throughout the file.

34 ↗(On Diff #3266)

ar -> are

67 ↗(On Diff #3266)

ar -> are

89 ↗(On Diff #3266)

Thank you for the more comprehensive tests!

This revision now requires changes to proceed.Mar 18 2018, 01:23
src/test/monolith_opcodes.cpp
239 ↗(On Diff #3266)

TODO: Replace 520 by the constant giving the max element size here.

src/test/monolith_opcodes.cpp
140 ↗(On Diff #3266)

dito

schancel added a reviewer: deadalnix.

Fix spelling and use MAX_SCRIPT_ELEMENT_SIZE

src/test/data/script_tests.json
858 ↗(On Diff #3267)

Anybody know if this is necessary to make these run?

// Uncomment if you want to output updated JSON tests.
// #define UPDATE_JSON_TESTS

This is a really screwy comment to leave for posterity :*(

schancel edited the test plan for this revision. (Show Details)
This revision is now accepted and ready to land.Mar 18 2018, 02:55
schancel added a task: Restricted Maniphest Task.Mar 18 2018, 03:45
deadalnix edited reviewers, added: schancel; removed: deadalnix.
This revision was automatically updated to reflect the committed changes.