Differential D1598

OP_MUL implementation
AbandonedPublic
Actions

Authored by danconnolly on Jul 25 2018, 22:13.

Tags

None

Subscribers

Details

Reviewers

None

Group Reviewers

Restricted Project

Summary

Implementation of OP_MUL for the magnetic upgrade. The specification for this opcode is here.

Overflow: The operands to the OP_MUL opcode must be valid numerical values, otherwise the opcode will fail. Numerical values are limited to 32-bits. The result of the OP_MUL opcode can be longer than 32-bits. A result of that size would not be considered a valid numerical value but it is a valid byte sequence. The product of two maximum sized numerical values will fit in a 64-bit integer and will not produce an overflow in the implementation. Tests exist in script_tests.json for these scenarios, currently at lines 1059-1060 & 1070-1071.

Test Plan

make check

Diff Detail

Branch

opcodes_mul

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 2961
Build 4021: arc lint + arc unit

Event Timeline

danconnolly created this revision.Jul 25 2018, 22:13

Harbormaster completed remote builds in B2961: Diff 4427.Jul 25 2018, 22:13

danconnolly added parent revisions: D1592: Add SCRIPT_ENABLE_MAGNETIC_OPCODES flag to gate opcodes activation for the Magnetic upgrade., D1594: Rename monolith_opcodes.cpp to opcode_tests.cpp, D1593: Expand IsOpcodeDisabled() function to prepare for implementation of re-enabled opcodes in Magnetic update..Jul 25 2018, 22:15

add negative zero test

Harbormaster completed remote builds in B2962: Diff 4428.Jul 25 2018, 22:40

danconnolly planned changes to this revision.Jul 28 2018, 12:42

This comment was removed by danconnolly.

danconnolly edited parent revisions, added: D1631: Enable the magnetic opcode changes in the opcode tests; removed: D1594: Rename monolith_opcodes.cpp to opcode_tests.cpp, D1592: Add SCRIPT_ENABLE_MAGNETIC_OPCODES flag to gate opcodes activation for the Magnetic upgrade..Aug 8 2018, 22:48

danconnolly edited the summary of this revision. (Show Details)

danconnolly added a reviewer: Restricted Project.

refactor based on re-organizing prior diffs

Harbormaster completed remote builds in B3046: Diff 4540.Aug 8 2018, 23:00

schancel added a subscriber: schancel.Aug 9 2018, 01:07

schancel added inline comments.

src/script/interpreter.cpp
758 ↗	(On Diff #4540)	I was under the impression there was some overflow issue that needed to be addressed?

danconnolly edited the summary of this revision. (Show Details)Aug 9 2018, 07:56

danconnolly edited the summary of this revision. (Show Details)

danconnolly marked an inline comment as done.Aug 9 2018, 07:58

danconnolly added inline comments.

src/script/interpreter.cpp
758 ↗	(On Diff #4540)	I've updated the revision description to describe the potential for overflow and the related tests.

danconnolly marked 2 inline comments as done.Aug 9 2018, 07:58

danconnolly abandoned this revision.Sep 4 2018, 09:29

Revision Contents
Changeset List

Path

Size

src/

script/

interpreter.cpp

7 lines

7 lines

test/

data/

script_tests.json

28 lines

opcode_tests.cpp

37 lines

Diff 4427

src/script/interpreter.cpp

Loading...

src/script/script.h

Loading...

src/test/data/script_tests.json

Loading...

src/test/opcode_tests.cpp

Loading...