Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

compile_lll.py: buggy translation of LLL operator sha3_32 #448

Closed
daejunpark opened this issue Nov 9, 2017 · 0 comments
Closed

compile_lll.py: buggy translation of LLL operator sha3_32 #448

daejunpark opened this issue Nov 9, 2017 · 0 comments

Comments

@daejunpark
Copy link
Contributor

What's your issue about?

The translation of LLL operator sha3_32 in compile_lll.py seems to be buggy.

For example, an LLL expression [sha3_32 0] is translated to EVM opcodes, PUSH1, 0, PUSH1, 192, MSTORE, PUSH1, 192, PUSH1, 32, SHA3, which perhaps should not. AFAIU, the translation logic should be that it first stores 0 in a predefined memory location (here 192), and then computes SHA3 hash of memory range of starting location 192 and width 32 (i.e., MEM[192...223]). But in the generated EVM code, it computes SHA3 hash of memory range of starting location 32 and width 192 (i.e., MEM[32...223]):
https://github.com/ethereum/viper/blob/b1fe12ddd8706ef3340a103916e3d3f2cc95ceef/viper/compile_lll.py#L203

Here is why it works until now. Fortunately (or unfortunately) the unintended memory range MEM[32...191] that is prepended to the value to be hashed (i.e., MEM[192...223]) is fixed over all Viper programs for now. They are pre-occupied to store the five size limits used to clamp values: ADDRSIZE, MAXNUM, MINNUM, MAXDECIMAL, MINDECIMAL:
https://github.com/ethereum/viper/blob/b1fe12ddd8706ef3340a103916e3d3f2cc95ceef/viper/utils.py#L76-L80

Here is a minimal example to reproduce. For the following Viper program:

balance: num[num]
def foo():
    self.balance[0] = 1

The intermediate LLL program is:

[seq,   
  [return,
    0,    
    [lll, 
      [seq, 
        [seq,
          [mstore, 28, [calldataload, 0]],
          [mstore, 32, 1461501637330902918203684832716283019655932542976],
          [mstore, 64, 170141183460469231731687303715884105727],
          [mstore, 96, -170141183460469231731687303715884105728],
          [mstore, 128, 1701411834604692317316873037158841057270000000000],
          [mstore, 160, -1701411834604692317316873037158841057280000000000]],
        # Line 3
        [if,
          [eq, [mload, 0], 3264763256 <foo>],
          [seq,
            pass,
            [assert, [iszero, callvalue]],
            # Line 4
            [sstore, [add, [sha3_32, 0 <self.balance>], 0], 1], 
            # Line 3
            stop]]],
      0]]]

The final bytecode generated (in opcode form):

['PUSH2', 0, 189, 'JUMP', 'PUSH1', 0, 'CALLDATALOAD', 'PUSH1', 28, 'MSTORE', 'PUSH21', 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 'PUSH1', 32, 'MSTORE', 'PUSH16', 127, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 'PUSH1', 64, 'MSTORE', 'PUSH32', 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 128, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 'PUSH1', 96, 'MSTORE', 'PUSH21', 1, 42, 5, 241, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 253, 171, 244, 28, 0, 'PUSH1', 128, 'MSTORE', 'PUSH32', 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 254, 213, 250, 14, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 'PUSH1', 160, 'MSTORE', 'PUSH4', 194, 152, 85, 120, 'PUSH1', 0, 'MLOAD', 'EQ', 'ISZERO', 'PUSH2', 0, 184, 'JUMPI', 'CALLVALUE', 'ISZERO', 'ISZERO', 'PC', 'JUMPI', 'PUSH1', 1, 'PUSH1', 0, 'PUSH1', 0, 'PUSH1', 192, 'MSTORE', 'PUSH1', 192, 'PUSH1', 32, 'SHA3', 'ADD', 'SSTORE', 'STOP', 'JUMPDEST', 'JUMPDEST', 'PUSH2', 0, 4, 'PUSH2', 0, 189, 'SUB', 'PUSH2', 0, 4, 'PUSH1', 0, 'CODECOPY', 'PUSH2', 0, 4, 'PUSH2', 0, 189, 'SUB', 'PUSH1', 0, 'RETURN']

How can it be fixed?

Simply change the following line:
https://github.com/ethereum/viper/blob/b1fe12ddd8706ef3340a103916e3d3f2cc95ceef/viper/compile_lll.py#L203
to:

o.extend(['PUSH1', MemoryPositions.FREE_VAR_SPACE, 'MSTORE', 'PUSH1', 32, 'PUSH1', MemoryPositions.FREE_VAR_SPACE, 'SHA3']) 

Cute Animal Picture

               __   __
              __ \ / __
             /  \ | /  \
                 \|/
            _,.---v---._
   /\__/\  /            \
   \_  _/ /              \
     \ \_|           @ __|
  hjw \                \_
  `97  \     ,__/       /
     ~~~`~~~~~~~~~~~~~~/~~~~
@daejunpark daejunpark changed the title Buggy translation of LLL operator sha3_32 in compile_lll.py compile_lll.py: buggy translation of LLL operator sha3_32 Nov 9, 2017
DavidKnott added a commit that referenced this issue Nov 16, 2017
fix issue #448: buggy translation of LLL operator `sha3_32`
Dexaran added a commit to Dexaran/viper that referenced this issue Dec 25, 2017
* Added get_logs method to return a list of logs with optional filtering; changed get_log fixture to use last entry in get_logs list for specific log name

* WIP overhaul of types

* Draft of viper-by-example.

* fixes voting with delegation example

* use falsey-ness of 0x0000000000000000000000000000000000000000

* use falsey-ness of 0x0000000000000000000000000000000000000000 (reverted from commit 21cc53a)

* use falsey-ness of 0x0000000000000000000000000000000000000000

* iff --> if and only if

* Adds basic for loop for in memory list.

* Adds test for literal list.

* Adds for i in list support from storage and list literals.

* Adds support for i in list of other BaseTypes.

* Add StructureException testcase for iterating over a nested list.

* Allow skip of the event name

* Change function to func

* Changed get_log() to get_last_log(), removed optional log name from usages

* Make corrections according to suggestions.

* Minor typo.

* Adds basic bytes as map keys support.

* Adds support for mapping literal bytes.

* fix the problems when install viper in Ubuntu16.04

* Adds test for long than 32 bytes keys.

* Adds test for mismatched bytes size map assign.

* Adds variable name to i_pos, so it doesn't conflict.

* Adds exceptions for altering lists whilst iterating them.

* Fixe for altering list in iteration exception check.

* Adds tests for assert x in list

* Added reference types.

* Add the packaging metadata to build the viper snap

* Improves ownership test - to be more realistic.

* Modified assert_tx_failed() to remove reference to tester as tester reference inside setup_transaction_tests.py was referencing the same module

* Add 65 and 25 gas to CLAMP and ASSERT pseudo opcodes, based on test_decimals.py

* Various changes to gas estimation.

- Fixes with statement estimation to total all underlying args.
- Adds add_gas parameter to LLLNode to allow more advanced gas calculations to
be assigned to an LLLNode, see make_byte_array_copier as an example.

* Replace get_contract with get_contract_with_gas_estimation.

* Adds test for keys with a very long byte length.

* Removed compound calling, small touchups

* Minor spelling fix

* Remove empty test

* Refactor code parsing

* Fix typo in folder name

* Make pseudo opcode gas estimate a single value.

* Add test for colored gas estimate output when using -f ir.

* Remove tester from assert_tx_failed.

* Move test_company file into proper folder

* Fix constant clamp checks / change constant clamp checks to compile time

* Add fixture to directly test LLL

* Add clamp tests at the LLL level

* Break out `test_parser.py` into test specific files

* Change to assert_tx_failed

* Add staticcall opcode for constant function calls

* Test constant external function calls

* Remove repetitive assert_tx_failed

* Create Viper VIP template

* Add link to VIP template to the issue template

* Improve rlp decoding gas estimation

* Add VIP link to contributing docs

* fix issue vyperlang#453

* use clamp constants for literals as well

* fix num_min_literal parsing failure

* fix test coverage

* fix issue vyperlang#448: compile_lll.py: buggy translation of LLL operator `sha3_32`

* add sha3_32 compilation test

* Add checks preventing assigning values on defining types.

* Fix gas estimation error

* Fix repeat LLL with <= 0 iteration

* Add test for repat lll

* Add depth check to with "set" statement

* Require function visibility declaration

* Add tests for public decorator

* Add public decorator to tests (vyperlang#474)

* Document function visibility requirements

* Clarifies expectation in error message

This would have helped me when I saw this error learning Viper for the first time.

* Note internal or public decorator requirement

* Fixes test with for function visibility.

* Add the install instructions for the Viper snap

This requries vyperlang#445, and enabling the continuous delivery on https://build.snapcraft.io

* Allow for gas estimation tests for constants

* Add gas estimation for create_with_code_of

* ad logs to company.v.py with tests

* document event in structure of a contract

* move events before global declarations

* remove unused import

* types.py: Remove duplicated method

Fix vyperlang#488

* Adds support -f bytecode_runtime.

* cleans up test_logs

* Use revert opcode for assert statements

* Show lineno of base_type_conversion error

Fix vyperlang#498

* Show lineno of undeclared function visibility

Fix vyperlang#489

* Adds basic test for bytecode_runtime generation.

* add clamp_nonzero to modulo arithemetic

* Add modulo tests and refactor simple auction example

* Use `with` to decrease repetition

* Change vipercoin from returning false to throwing

* Add check to as_num256 to make sure number is greater than 0

* Improve contributing .rst

- Fixes a link that wasn't displaying correctly.
- Makes some instructions more concise.
- Changes old `:ref:building-from-source`, to link to installation instructions.

* Added testing and deployment doc

* Fix augassignment checks

* Test for in list invalids

* Remove repetitive error and fix typo

* Begin adding built in erc20 external call functionality

* Add implied erc20 token abi fucntionality

* Add on chain market maker example

* Test implied erc20 abi functionality

* Update on chain market maker and erc20 abi

* Change @internal decorator to @Private decorator.

* Add support for logging a variable of list type.

* Adds tests for logging of list types.

* Fix for log packing on list variable.

* Add error if log has wrong number of arguments

* Add log test and improve bytes as input

* Improve variable naming / fix bytes as input

* Clean up tests

* Adds Fix vyperlang#531. Check return type of lists match.

* Adds more tests for logging of lists.

* Adds Fix vyperlang#504 for clamping decimal literals correctly.

* Use revert for clamps

* Fix exponents with units

* Create a contract data type

* Remove .python-version, update .gitignore

* Improve vipercoin example: link out to resources describing attack vectors

* Test the contract data type

* Update version of ethereum

* Improves gas estimate on internal call.

* Removed suicide keyword

* Carry over add_gas_estimate in optimizer.

* Adds gas estimation for internal calls.

* Added test for invalid keyword

* not official a language > officially

* Changing notes to be indented blocks starting on the line after .. note:: 

https://sublime-and-sphinx-guide.readthedocs.io/en/latest/notes_warnings.html

* Adds storage list logging support.

* Adds test to check list logging is of correct (sub)type.

* Splitting comments over two lines where necessary to fit on the page on readthedocs

* Update to use conftest; fix some incorrect tests

* Update Safe Remote Purchases: line numbers, minor edits

* Updates from code review

* Add modulo math checks

* Adds support for logging bytes from storage.

* Adds exception to limit to 32 bytes of bytes type logging (restriction for the time being).

* virtualenv -p /usr/local/lib/python3.6/bin/python3 > virtualenv -p python3.6

Related: vyperlang#558

* Add a note on how to fix `fatal error: openssl/aes.h: No such file or directory` in the `make` output

* removed extraneous backtick `

* Minor edits

with ``num`` type > with the ``num`` type
crowdfunding period is over - as determined> crowdfunding period is over—as determined

* Minor edits voting

add a comma > to vote, and each
add a comma > to vote, or

* Improve for loop error

* Adds test for logging a decimal list.

* :ref:`function-calls` > :ref:`Function-calls`

* Minor edits types.rst

* Add length check to event topics

* Fix and test logging with units

* Improve type parsing error message

* Add valid call keyword list to utils

* Add length check to event topics

* Partial fix for vyperlang#590. Adds check for a minimum of one return statement.

* Fixes test that were missing return statements.

* Adds example of return checker behaving incorrectly.

* Update dockerfile to use slim image base

* Remove commented out directives

* Reduce num256_add gas cost

* Tag built image as viper

* Add built-in function docs

* Fix on chain market maker

* Add comments to on chain market maker

* Change badges from viper to vyper
anistark added a commit to anistark/viper that referenced this issue Jan 12, 2018
* Change to assert_tx_failed

* Add staticcall opcode for constant function calls

* Test constant external function calls

* Remove repetitive assert_tx_failed

* Create Viper VIP template

* Add link to VIP template to the issue template

* Improve rlp decoding gas estimation

* Add VIP link to contributing docs

* fix issue vyperlang#453

* use clamp constants for literals as well

* fix num_min_literal parsing failure

* fix test coverage

* fix issue vyperlang#448: compile_lll.py: buggy translation of LLL operator `sha3_32`

* add sha3_32 compilation test

* Add checks preventing assigning values on defining types.

* Fix gas estimation error

* Fix repeat LLL with <= 0 iteration

* Add test for repat lll

* Add depth check to with "set" statement

* Require function visibility declaration

* Add tests for public decorator

* Add public decorator to tests (vyperlang#474)

* Document function visibility requirements

* Clarifies expectation in error message

This would have helped me when I saw this error learning Viper for the first time.

* Note internal or public decorator requirement

* Fixes test with for function visibility.

* Add the install instructions for the Viper snap

This requries vyperlang#445, and enabling the continuous delivery on https://build.snapcraft.io

* Allow for gas estimation tests for constants

* Add gas estimation for create_with_code_of

* ad logs to company.v.py with tests

* document event in structure of a contract

* move events before global declarations

* remove unused import

* types.py: Remove duplicated method

Fix vyperlang#488

* Adds support -f bytecode_runtime.

* cleans up test_logs

* Use revert opcode for assert statements

* Show lineno of base_type_conversion error

Fix vyperlang#498

* Show lineno of undeclared function visibility

Fix vyperlang#489

* Adds basic test for bytecode_runtime generation.

* add clamp_nonzero to modulo arithemetic

* Add modulo tests and refactor simple auction example

* Use `with` to decrease repetition

* Change vipercoin from returning false to throwing

* Add check to as_num256 to make sure number is greater than 0

* Improve contributing .rst

- Fixes a link that wasn't displaying correctly.
- Makes some instructions more concise.
- Changes old `:ref:building-from-source`, to link to installation instructions.

* Added testing and deployment doc

* Fix augassignment checks

* Test for in list invalids

* Remove repetitive error and fix typo

* Begin adding built in erc20 external call functionality

* Add implied erc20 token abi fucntionality

* Add on chain market maker example

* Test implied erc20 abi functionality

* Update on chain market maker and erc20 abi

* Change @internal decorator to @Private decorator.

* Add support for logging a variable of list type.

* Adds tests for logging of list types.

* Fix for log packing on list variable.

* Add error if log has wrong number of arguments

* Add log test and improve bytes as input

* Improve variable naming / fix bytes as input

* Clean up tests

* Adds Fix vyperlang#531. Check return type of lists match.

* Adds more tests for logging of lists.

* Adds Fix vyperlang#504 for clamping decimal literals correctly.

* Use revert for clamps

* Fix exponents with units

* Create a contract data type

* Remove .python-version, update .gitignore

* Improve vipercoin example: link out to resources describing attack vectors

* Test the contract data type

* Update version of ethereum

* Improves gas estimate on internal call.

* Removed suicide keyword

* Carry over add_gas_estimate in optimizer.

* Adds gas estimation for internal calls.

* Added test for invalid keyword

* not official a language > officially

* Changing notes to be indented blocks starting on the line after .. note:: 

https://sublime-and-sphinx-guide.readthedocs.io/en/latest/notes_warnings.html

* Adds storage list logging support.

* Adds test to check list logging is of correct (sub)type.

* Splitting comments over two lines where necessary to fit on the page on readthedocs

* Update to use conftest; fix some incorrect tests

* Update Safe Remote Purchases: line numbers, minor edits

* Updates from code review

* Add modulo math checks

* Adds support for logging bytes from storage.

* Adds exception to limit to 32 bytes of bytes type logging (restriction for the time being).

* virtualenv -p /usr/local/lib/python3.6/bin/python3 > virtualenv -p python3.6

Related: vyperlang#558

* Add a note on how to fix `fatal error: openssl/aes.h: No such file or directory` in the `make` output

* removed extraneous backtick `

* Minor edits

with ``num`` type > with the ``num`` type
crowdfunding period is over - as determined> crowdfunding period is over—as determined

* Minor edits voting

add a comma > to vote, and each
add a comma > to vote, or

* Adds basic single list argument call functionality.

* Improve for loop error

* Adds test for logging a decimal list.

* :ref:`function-calls` > :ref:`Function-calls`

* Minor edits types.rst

* Fixes handling of static array in call.

- Also adds tests for many mixed cases of calling with a list parameter.

* Adds test for mixed bytes - list calling arguments.

* Add length check to event topics

* Fix and test logging with units

* Improve type parsing error message

* Add valid call keyword list to utils

* Add length check to event topics

* Partial fix for vyperlang#590. Adds check for a minimum of one return statement.

* Fixes test that were missing return statements.

* Adds example of return checker behaving incorrectly.

* Adds if statement block scoping.

* Add blockscoping to for loop.

* Add blockscope tests.

* Update dockerfile to use slim image base

* Remove commented out directives

* Reduce num256_add gas cost

* Tag built image as viper

* Add built-in function docs

* Fix on chain market maker

* Add comments to on chain market maker

* Change badges from viper to vyper

* Fix rlp decoding loop memory

* Fix call data copy gas estimation

* Implement continue

Add `continue` to pseudo_opcodes, LLL, and viper.

* Fix example ERC20 token

Discovered that it doesn't compile by trying to compile all of the
example contracts in `examples/`. This fixes ERC20.v.py by resolving
compiler complaints.

* Add internal call arg count check

* cleaned up linter errors under tests/compiler

* cleaned up imports and replaced boolean assertion under test_simple_auction

* fixed linter errors in tests/examples/company

* fixed linter errors on tests/examples/market_maker

* fixed linter errors under tests/exmaples/safe_remote_purchase

* cleaned up linter errors tests/examples/tokens

* fixed linter errors on tests/examples/voting

* fixed linter errors on tests/examples/wallets

* fixed linter errors in tests/parsers/features

* test_extract32.py: renamed first function to test_extract32_extraction

* fixed linter errors

* cleaned up linter errors tests/parser

* Adds test for passing list from storage.

* Make blockscopes use set() instead of list().

* specified flake8 script on travis to include tests/

* cleaned up exception block for test_test_bytes

* cleaned up bare except under test_arbitration_code

* cleaned up bare imports to handle ValueOutOfBounds exception in tests/parser/features/test_constructor.py

* cleaned up bare imports test_raw_call

* test_test_slice4: cleaned up test assertions for raised exceptions

* cleaned up bare excepts in test_extract32

* fixed linter error tests/parser/features/test_internal_call.py

* setup.cfg: set flake8 exclusion only for docs

* Fixes vyperlang#547, correct handling of units for exponent notation.

* Adds tests for unit exponents.

* edited travis script for flake8

* Fix err msg for functions without enough arguments

* Add an error message for calls to undefined top-level functions

Previously, the compiler would error ungracefully if an undefined
top-level function was called, due to an unhandled case in `Expr.call`.

The compiler now also makes suggestions when a non-existent top-level function
is mistakenly called instead of a contract method.

* Add `.gitattributes` syntax highlighting trick

* Add .gitattributes file

* Prevent mixed list type from occuring.

* Fixes test that contain mixed list types.

* pep8 fixes

* Remove num256 greater than 0 check for bytes32

* Bump version: 0.0.2 → 0.0.3
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant