Page MenuHomePhabricator

Add support for SPLIT opcodes
ClosedPublic

Authored by deadalnix on Mar 30 2018, 14:42.

Details

Reviewers
movrcx
jasonbcox
Group Reviewers
Restricted Project
Restricted Owners Package(Owns No Changed Paths)
Maniphest Tasks
T308: Adds OP_SPLIT functionality into interpreter.cpp
Commits
rSTAGINGa39b04fae804: Add support for SPLIT opcodes
rABCa39b04fae804: Add support for SPLIT opcodes
Summary

Add handling for SPLIT opcodes when the monolith script flag is enabled.

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

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
opcodesplit
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 2207
Build 2558: Bitcoin ABC Buildbot (legacy)
Build 2557: arc lint + arc unit

Event Timeline

jasonbcox requested changes to this revision.Mar 30 2018, 16:00
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/script/interpreter.cpp
1279 ↗(On Diff #3341)

Rename size -> position to be inline with the comment for OP_SPLIT

src/test/data/script_tests.json
869 ↗(On Diff #3341)

Does this really return SPLIT_RANGE? I'm not seeing this in the implementation. If it does somehow, i think this error should be captured more explicitly when position is read off the stack.

src/test/monolith_opcodes.cpp
411 ↗(On Diff #3341)

Add a test for negative position value

This revision now requires changes to proceed.Mar 30 2018, 16:00
src/test/data/script_tests.json
869 ↗(On Diff #3341)

-1 is a very large value when considered as unsigned.

Add C++ test case for negative value and rename size to position.

This revision is now accepted and ready to land.Mar 30 2018, 16:30
This revision was automatically updated to reflect the committed changes.