Page MenuHomePhabricator

Add support for NUM2BIN, and BIN2NUM opcodes
AbandonedPublic

Authored by deadalnix on Feb 15 2018, 01:24.

Details

Reviewers
jasonbcox
movrcx
schancel
Group Reviewers
Restricted Project
Restricted Owners Package(Owns No Changed Paths)
Maniphest Tasks
T309: Adds OP_BIN2NUM functionality into interpreter.cpp
Summary

Takes a minimally encoded number and converts it to binary

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

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
arcpatch-D1103_1
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 2175
Build 2494: Bitcoin ABC Buildbot (legacy)
Build 2493: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Owners added a reviewer: Restricted Owners Package.Mar 13 2018, 06:27
schancel retitled this revision from Adds OP_NUM2BIN functionality into interpreter.cpp to Add support for OP_NUM2BIN, and BIN2NUM opcodes.Mar 18 2018, 02:52
schancel added a subscriber: schancel.

Clean up of diff

Remove some code that shouldn't be there.

jasonbcox requested changes to this revision.Mar 18 2018, 03:17
jasonbcox added inline comments.
src/script/interpreter.cpp
948–963 ↗(On Diff #3276)

Code merge goof?

This revision now requires changes to proceed.Mar 18 2018, 03:17
schancel retitled this revision from Add support for OP_NUM2BIN, and BIN2NUM opcodes to Add support for NUM2BIN, and BIN2NUM opcodes.Mar 18 2018, 09:04

Use MinimalizeBigEndianArray

schancel added a reviewer: movrcx.

Fix tests, and and edge case in BIN2NUM

jasonbcox requested changes to this revision.Mar 18 2018, 22:44
jasonbcox added inline comments.
src/script/interpreter.cpp
1297 ↗(On Diff #3299)

Is this comment misplaced? The code immediately below it is just recording neg. I don't actually see any code in this function that protects against -0.

src/test/data/script_tests.json
837 ↗(On Diff #3299)

Add a test for -0 BIN2NUM

This revision now requires changes to proceed.Mar 18 2018, 22:44

Update with a couple fixes

src/script/interpreter.cpp
1304 ↗(On Diff #3299)

i is a uint8_t. This is implicitly limiting padding to 255 bytes. The spec doesn't imply any limitation other than that the result must me less than DEFAULT_MAX_NUM_BYTES

macros added a subscriber: macros.

refactored tests

deadalnix abandoned this revision.
deadalnix added a reviewer: schancel.