Skip to content

Commit

Permalink
Add SHL Opcode for Constantinople
Browse files Browse the repository at this point in the history 
Relates to ethereum#1104
  • Loading branch information
@cburgdorf
cburgdorf committed Jul 31, 2018
1 parent 222ef9f commit 46f7a9d
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 4 deletions.
3 changes: 3 additions & 0 deletions eth/constants.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
UINT256 = 'uint256'
BYTES = 'bytes'

CONST_256 = 256
CONST_0 = 0

UINT_256_MAX = 2**256 - 1
UINT_256_CEILING = 2**256
UINT_255_MAX = 2**255 - 1
Expand Down
29 changes: 28 additions & 1 deletion eth/vm/forks/constantinople/opcodes.py
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,35 @@
import copy
from cytoolz import (
merge
)

from eth import (
constants
)
from eth.vm import (
mnemonics,
opcode_values,
)
from eth.vm.forks.byzantium.opcodes import (
BYZANTIUM_OPCODES
)
from eth.vm.logic import (
arithmetic
)
from eth.vm.opcode import (
as_opcode
)


UPDATED_OPCODES = {
opcode_values.SHL: as_opcode(
logic_fn=arithmetic.shl,
mnemonic=mnemonics.SHL,
gas_cost=constants.GAS_VERYLOW,
),
}

CONSTANTINOPLE_OPCODES = copy.deepcopy(BYZANTIUM_OPCODES)
CONSTANTINOPLE_OPCODES = merge(
copy.deepcopy(BYZANTIUM_OPCODES),
UPDATED_OPCODES,
)
16 changes: 16 additions & 0 deletions eth/vm/logic/arithmetic.py
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from BitVector import BitVector
from cytoolz import (
curry,
)
Expand Down Expand Up @@ -177,3 +178,18 @@ def signextend(computation):
result = value

computation.stack_push(result)


def shl(computation):
"""
Bitwise left shift
"""
shift_length, value = computation.stack_pop(num_items=2, type_hint=constants.UINT256)
bit_vector = BitVector(intVal=value, size=constants.CONST_256)

This comment has been minimized.

Sign in to view

Copy link
@cburgdorf

cburgdorf Jul 31, 2018 •
edited
Loading

Author Owner

I guess we don't want that extra lib but I had trouble getting all test cases to work without it. The reason for this goes as follows. Consider the hex code 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff as defined in the test which represents the number 115792089237316195423570985008687907853269984665640564039457584007913129639935

Now let's shift this to the left.

>>> 115792089237316195423570985008687907853269984665640564039457584007913129639935 << 1
231584178474632390847141970017375815706539969331281128078915168015826259279870

This isn't what the tests were expecting and we have to look into the binary representation to understand why. So, 115792089237316195423570985008687907853269984665640564039457584007913129639935 is represented as 1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111 (exactly 256 bits).

When I say 115792089237316195423570985008687907853269984665640564039457584007913129639935 << 1 what I actually mean to get is 1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111110 (bits moved left, last bit filled with a new 0, keeping bits at 256.

However, what I get instead is 11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111110, which if we count the bits make 257 bits.

As I think about it, it makes sense because Python doesn't actually now that I want to deal with a type that is restricted to 256 bits so it just keeps growing it.

I'm sure there are plenty of clever (and performant) ways to get this done without that extra lib but I first wanted to get all tests passing.

This comment has been minimized.

Sign in to view

Copy link
@cburgdorf

cburgdorf Jul 31, 2018

Author Owner

@pipermerriam @carver just paging you for this wall of bits ;)

This comment has been minimized.

Sign in to view

Copy link
@carver

carver Jul 31, 2018 •
edited
Loading

Yeah, let's avoid the new dependency.

Any problems with something like?

result = 115792089237316195423570985008687907853269984665640564039457584007913129639935 << 1
if result > UINT256_MAX:
  return result % UINT256_MAX
else:
  return result

This comment has been minimized.

Sign in to view

Copy link
@cburgdorf

cburgdorf Jul 31, 2018

Author Owner

That doesn't cut it. That will return 0 in this case. But we want the following as defined in the EIP.

115792089237316195423570985008687907853269984665640564039457584007913129639934
                                                                             ^

However, this one seems do do the trick #1 (comment)

(value << shift_length) & constants.UINT_256_MAX

This comment has been minimized.

Sign in to view

Copy link
@carver

carver Jul 31, 2018

Oh yeah, had an off-by-one error: result % (UINT256_MAX + 1) should do it as well. But I suspect & UINT_256_MAX is faster (and maybe even easier to read). Which appears to be correct:

In [23]: %timeit result % (UINT_256_MAX + 1)
158 ns ± 2.16 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)

In [24]: %timeit result & UINT_256_MAX
58.7 ns ± 3.46 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)

This comment has been minimized.

Sign in to view

Copy link
@cburgdorf

cburgdorf Jul 31, 2018

Author Owner

🎉 And after reading https://en.wikipedia.org/wiki/Bitwise_operation#AND I even kinda understand that magic. Yay, I love working with you geniuses, I'm learning lots of new tricks!

if shift_length >= constants.CONST_256:
result = constants.CONST_0
else:
result = bit_vector.shift_left(shift_length).int_val()

computation.stack_push(result)
1 change: 1 addition & 0 deletions eth/vm/mnemonics.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
MULMOD = 'MULMOD'
EXP = 'EXP'
SIGNEXTEND = 'SIGNEXTEND'
SHL = 'SHL'
#
# Comparisons
#
Expand Down
1 change: 1 addition & 0 deletions eth/vm/opcode_values.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
XOR = 0x18
NOT = 0x19
BYTE = 0x1a
SHL = 0x1b


#
Expand Down
1 change: 1 addition & 0 deletions setup.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

deps = {
'eth': [
"BitVector>=3.4.8, <4.0.0",
"cryptography>=2.0.3,<3.0.0",
"cytoolz>=0.9.0,<1.0.0",
"eth-bloom>=1.0.0,<2.0.0",
Expand Down
91 changes: 88 additions & 3 deletions tests/core/opcodes/test_opcodes.py
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
import pytest

from eth_utils import (
decode_hex,
encode_hex,
to_canonical_address,
int_to_big_endian,
)

from eth import (
constants
)
from eth.utils.padding import (
pad32
)
from eth.vm import (
opcode_values
)
from eth.vm.forks.byzantium.opcodes import (
BYZANTIUM_OPCODES
)
from eth.vm.forks import (
ConstantinopleVM,
ByzantiumVM,
SpuriousDragonVM,
TangerineWhistleVM,
Expand Down Expand Up @@ -98,3 +102,84 @@ def test_mul(vm_class, val1, val2, expected):
assert result == expected


@pytest.mark.parametrize(
# Testcases from https://github.com/ethereum/EIPs/blob/master/EIPS/eip-145.md#shl-shift-left
'vm_class, val1, val2, expected',
(
(
ConstantinopleVM,
'0x0000000000000000000000000000000000000000000000000000000000000001',
'0x00',
'0x0000000000000000000000000000000000000000000000000000000000000001',
),
(
ConstantinopleVM,
'0x0000000000000000000000000000000000000000000000000000000000000001',
'0x01',
'0x0000000000000000000000000000000000000000000000000000000000000002',
),
(
ConstantinopleVM,
'0x0000000000000000000000000000000000000000000000000000000000000001',
'0xff',
'0x8000000000000000000000000000000000000000000000000000000000000000',
),
(
ConstantinopleVM,
'0x0000000000000000000000000000000000000000000000000000000000000001',
'0x0100',
'0x0000000000000000000000000000000000000000000000000000000000000000',
),
(
ConstantinopleVM,
'0x0000000000000000000000000000000000000000000000000000000000000001',
'0x0101',
'0x0000000000000000000000000000000000000000000000000000000000000000',
),
(
ConstantinopleVM,
'0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff',
'0x00',
'0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff',
),
(
ConstantinopleVM,
'0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff',
'0x01',
'0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe',
),
(
ConstantinopleVM,
'0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff',
'0xff',
'0x8000000000000000000000000000000000000000000000000000000000000000',
),
(
ConstantinopleVM,
'0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff',
'0x0100',
'0x0000000000000000000000000000000000000000000000000000000000000000',
),
(
ConstantinopleVM,
'0x0000000000000000000000000000000000000000000000000000000000000000',
'0x01',
'0x0000000000000000000000000000000000000000000000000000000000000000',
),
(
ConstantinopleVM,
'0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff',
'0x01',
'0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe',
),
)
)
def test_shl(vm_class, val1, val2, expected):
computation = prepare_computation(vm_class)
computation.stack_push(decode_hex(val1))
computation.stack_push(decode_hex(val2))
computation.opcodes[opcode_values.SHL](computation)

result = computation.stack_pop(type_hint=constants.UINT256)

assert encode_hex(pad32(int_to_big_endian(result))) == expected

0 comments on commit 46f7a9d

Please sign in to comment.