Details

Reviewers

jasonbcox
schancel

Group Reviewers

Restricted Project

Maniphest Tasks

T304: Adds Bitwise functionality into interpreter.cpp

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

Branch

arcpatch-D1211

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 2150
Build 2444: Bitcoin ABC Buildbot (legacy)
Build 2443: arc lint + arc unit

Event Timeline

deadalnix created this revision.Mar 18 2018, 01:13

Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 18 2018, 01:13

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

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

deadalnix added inline comments.Mar 18 2018, 01:27

src/test/monolith_opcodes.cpp
239 ↗	(On Diff #3266)	TODO: Replace 520 by the constant giving the max element size here.

deadalnix added inline comments.Mar 18 2018, 01:28

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

schancel commandeered this revision.Mar 18 2018, 01:34

schancel added a reviewer: deadalnix.

Rebase and update

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging passed.

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

Harbormaster completed remote builds in B2142: Diff 3266.Mar 18 2018, 01:40

Fix spelling and use MAX_SCRIPT_ELEMENT_SIZE

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

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging passed.

Harbormaster completed remote builds in B2143: Diff 3267.Mar 18 2018, 01:56

schancel added inline comments.Mar 18 2018, 01:57

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 :*(