Page MenuHomePhabricator
Differential D1103

Add support for NUM2BIN, and BIN2NUM opcodes
AbandonedPublic
Actions

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
schancel removed a child revision: D1104: unit tests for OP_NUM2BIN.Mar 9 2018, 10:54
schancel removed parent revisions: D1094: unit test for OP_DIV, D1096: unit test for OP_MOD, D1091: unit test removal OP_LEFT/RIGHT/SUBSTR, D1092: unit test for bitwise operations (AND, OR, XOR), D1098: unit test for OP_CAT, D1100: unit tests for OP_SPLIT, D1102: unit tests for OP_BIN2NUM.
schancel removed parent revisions: D1095: Adds OP_MOD functionality into interpreter.cpp, D1093: Adds OP_DIV functionality into interpreter.cpp, D1097: Add support for CAT, and SPLIT opcodes, D1090: Adds Bitwise functionality into interpreter.cpp.Mar 9 2018, 11:02
schancel added a child revision: D1101: Adds OP_BIN2NUM functionality into interpreter.cpp.
danconnolly removed a child revision: D1101: Adds OP_BIN2NUM functionality into interpreter.cpp.Mar 12 2018, 21:07
danconnolly edited parent revisions, added: D1101: Adds OP_BIN2NUM functionality into interpreter.cpp; removed: D1099: Adds OP_SPLIT functionality into interpreter.cpp.
danconnolly edited the test plan for this revision. (Show Details)Mar 12 2018, 21:25
danconnolly updated this revision to Diff 3166.Mar 13 2018, 06:27

resubmit D1103 - OP_NUM2BIN

Owners added a reviewer: Restricted Owners Package.Mar 13 2018, 06:27
teamcity added a subscriber: teamcity.Mar 13 2018, 06:28

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

teamcity added a comment.Mar 13 2018, 06:52

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

Harbormaster completed remote builds in B2060: Diff 3166.Mar 13 2018, 06:52
danconnolly added a child revision: D1194: Set Monolith opcode enable script flag if monolith enabled.Mar 13 2018, 12:15
movrcx updated this revision to Diff 3228.Mar 16 2018, 03:03

Cleaning up due to D1202

teamcity added a comment.Mar 16 2018, 03:08

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

teamcity added a comment.Mar 16 2018, 03:26

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging failed.

Harbormaster failed remote builds in B2110: Diff 3228!Mar 16 2018, 03:27
movrcx updated this revision to Diff 3258.Mar 17 2018, 02:21

Rebase off of master

teamcity added a comment.Mar 17 2018, 03:02

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

Harbormaster completed remote builds in B2137: Diff 3258.Mar 17 2018, 03:20
teamcity added a comment.Mar 17 2018, 03:20

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

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 updated this revision to Diff 3275.Mar 18 2018, 03:06
schancel added a subscriber: schancel.

Clean up of diff

schancel updated this revision to Diff 3276.Mar 18 2018, 03:08

Remove some code that shouldn't be there.

schancel removed a child revision: D1194: Set Monolith opcode enable script flag if monolith enabled.Mar 18 2018, 03:13
schancel removed a parent revision: D1101: Adds OP_BIN2NUM functionality into interpreter.cpp.
schancel added a child revision: D1194: Set Monolith opcode enable script flag if monolith enabled.
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
teamcity added a comment.Mar 18 2018, 03:19

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

teamcity added a comment.Mar 18 2018, 03:23

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

teamcity added a comment.Mar 18 2018, 03:23

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

Harbormaster completed remote builds in B2151: Diff 3275.Mar 18 2018, 03:23
teamcity added a comment.Mar 18 2018, 03:26

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

Harbormaster completed remote builds in B2152: Diff 3276.Mar 18 2018, 03:26
schancel added a task: T309: Adds OP_BIN2NUM functionality into interpreter.cpp.Mar 18 2018, 03:44
schancel added a task: T310: Adds OP_NUM2BIN functionality into interpreter.cpp.
jasonbcox mentioned this in D1213: Attempt to add json tests for BIN2NUM and NUM2BIN, but blocked by various bugs. This diff contains a few fixes to said bugs, but doesn't solve the entire picture..Mar 18 2018, 07:20
schancel updated this revision to Diff 3283.Mar 18 2018, 09:03

Cleanup and rebase on D1214

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
teamcity added a comment.Mar 18 2018, 09:20

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

schancel updated this revision to Diff 3284.Mar 18 2018, 09:24

Trim input properly

teamcity added a comment.Mar 18 2018, 09:35

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging failed.

Harbormaster failed remote builds in B2159: Diff 3283!Mar 18 2018, 09:35
teamcity added a comment.Mar 18 2018, 09:35

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

teamcity added a comment.Mar 18 2018, 09:49

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging failed.

Harbormaster failed remote builds in B2160: Diff 3284!Mar 18 2018, 09:49
schancel added a parent revision: D1215: Add function to trim leading array zeros maintaining MSB..Mar 18 2018, 11:10
schancel updated this revision to Diff 3288.Mar 18 2018, 11:11

Use MinimalizeBigEndianArray

teamcity added a comment.Mar 18 2018, 11:17

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

Harbormaster failed remote builds in B2164: Diff 3288!Mar 18 2018, 11:32
teamcity added a comment.Mar 18 2018, 11:32

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging failed.

schancel commandeered this revision.Mar 18 2018, 20:49
schancel added a reviewer: movrcx.
schancel updated this revision to Diff 3299.Mar 18 2018, 22:13

Fix tests, and and edge case in BIN2NUM

teamcity added a comment.Mar 18 2018, 22:14

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

teamcity added a comment.Mar 18 2018, 22:34

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

Harbormaster completed remote builds in B2173: Diff 3299.Mar 18 2018, 22:34
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
schancel updated this revision to Diff 3300.Mar 19 2018, 02:37

Update with a couple fixes

teamcity added a comment.Mar 19 2018, 02:39

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

Harbormaster completed remote builds in B2174: Diff 3300.Mar 19 2018, 02:58
teamcity added a comment.Mar 19 2018, 02:58

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

shadders added inline comments.Mar 19 2018, 06:06
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

schancel updated this revision to Diff 3301.Mar 19 2018, 08:25

Add some more tests

teamcity added a comment.Mar 19 2018, 08:27

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

Harbormaster completed remote builds in B2175: Diff 3301.Mar 19 2018, 08:46
teamcity added a comment.Mar 19 2018, 08:46

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

schancel abandoned this revision.Mar 19 2018, 22:58
macros updated this revision to Diff 3324.Mar 21 2018, 17:43
macros added a subscriber: macros.

refactored tests

teamcity added a comment.Mar 21 2018, 17:45

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

teamcity added a comment.Mar 21 2018, 18:04

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

Harbormaster completed remote builds in B2192: Diff 3324.Mar 21 2018, 18:04
schancel removed a task: T310: Adds OP_NUM2BIN functionality into interpreter.cpp.Mar 22 2018, 04:55
deadalnix commandeered this revision.Mar 31 2018, 08:34
deadalnix abandoned this revision.
deadalnix added a reviewer: schancel.

Revision Contents
Changeset List

Diff 3301

View Options

src/Makefile.test.include

Loading...
View Options

src/script/interpreter.cpp

Loading...
View Options

src/script/script_error.h

Loading...
View Options

src/script/script_error.cpp

Loading...
View Options

src/test/CMakeLists.txt

Loading...
View Options

src/test/data/script_tests.json

Loading...
View Options

src/test/monolith_opcodes_type.cpp

Loading...
View Options

src/test/script_tests.cpp

Loading...