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

as_num256 failed to reject negative arguments #469

Closed
daejunpark opened this issue Nov 15, 2017 · 7 comments
Closed

as_num256 failed to reject negative arguments #469

daejunpark opened this issue Nov 15, 2017 · 7 comments
Labels
bug Bug that shouldn't change language semantics when fixed.

Comments

@daejunpark
Copy link
Contributor

daejunpark commented Nov 15, 2017

  • viper Version: 0.0.2
  • pyethereum Version: 2.1.0
  • OS: linux
  • Python Version (python --version): 3.6

What's your issue about?

AFAIU, as_num256(x) should not be allowed when x < 0. If so, the compiler should reject at compile-time as much as possible, or insert runtime check to throw a runtime exception. However, the current compiler does not generate the runtime check even if it cannot prove x >= 0.

For example, foo() returns 2**256 - 1:

def foo() -> num256:
    return as_num256(0 - 1)

while this:

def foo() -> num256:
    return as_num256(-1)

failed to be parsed:

viper.exceptions.InvalidLiteralException: line 2: Number out of range: -1
    return as_num256(-1)
----------------------^

Similarly, bar(-1) returns 2**256 - 1:

def bar(x: num) -> num256:
    return as_num256(x)

How can it be fixed?

For each as_num256(x), generate runtime-check assert(x >= 0) when x >= 0 cannot be proved at compile-time.

Cute Animal Picture

                              ___     _,.--.,_
                           .-~   ~--"~-.   ._ "-.
                          /      ./_    Y    "-. \
                         Y       :~     !         Y
                         lq p    |     /         .|
                      _   \. .-, l    /          |j
                     ()\___) |/   \_/";          !
                      \._____.-~\  .  ~\.      ./
                                 Y_ Y_. "vr"~  T
                                 (  (    |L    j   -Row
                                 [nn[nn..][nn..]
                             ~~~~~~~~~~~~~~~~~~~~~~~
@fubuloubu
Copy link
Member

Totally on board with this. Does a runtime error get generated when x: num256 - y: num256 > x?

@daejunpark
Copy link
Contributor Author

@fubuloubu Yep,

from ethereum.tools import tester as t
from ethereum.slogging import configure_logging

#configure_logging(':trace')
s = t.Chain()
from viper import compiler

code = """
def foo(x: num256, y: num256) -> num256:
    return num256_sub(x, y)
"""

t.languages['viper'] = compiler.Compiler()

c = s.contract(code, language='viper')

print(c.foo(1,2)) # TransactionFailed

error:

Traceback (most recent call last):
  File "run.py", line 17, in <module>
    print(c.foo(1,2))
  File "/home/daejunpark/x/venv/lib/python3.6/site-packages/ethereum-2.1.0-py3.6.egg/ethereum/tools/tester.py", line 103, in kall
    gasprice=kwargs.get('gasprice', GASPRICE)
  File "/home/daejunpark/x/venv/lib/python3.6/site-packages/ethereum-2.1.0-py3.6.egg/ethereum/tools/tester.py", line 194, in tx
    output = self.direct_tx(transaction)
  File "/home/daejunpark/x/venv/lib/python3.6/site-packages/ethereum-2.1.0-py3.6.egg/ethereum/tools/tester.py", line 185, in direct_tx
    raise TransactionFailed()
ethereum.tools.tester.TransactionFailed

@fubuloubu
Copy link
Member

Fantastic! Is there a test like that? Haha

@fubuloubu
Copy link
Member

E.g. 2-1 == 1, 2-2 == 0, and 2-3 throws?

But like use really big numbers for 1, 2, and 3

@DavidKnott DavidKnott added the bug Bug that shouldn't change language semantics when fixed. label Nov 15, 2017
@Dishendramishra
Copy link

in which file num256_sub() is defined?

@DavidKnott
Copy link
Contributor

@Dishendramishra All num256 functionality is defined in functions.py

@DavidKnott
Copy link
Contributor

@daejunpark Thanks for making an issue, this has been fixed. Closing.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Bug that shouldn't change language semantics when fixed.
Projects
None yet
Development

No branches or pull requests

4 participants