Num256 with natural expressions. #355

jacqueswww · 2017-09-20T21:53:04Z

- What I did

Added support for num256 to be evaluated without using builtin num256_xxx functions.

- How I did it

I started looking at basic evaluations off the following expressions:

def _num256_sub(x: num256, y: num256) -> num256:
    return x - y

And the compiler didn't understand num256 without the num256_xxx functions.
Currently num256 -> decimal -> num operations are not handled yet but I felt this could be a seperate PR instead.

- How to verify it

Check the test, one can also evaluate the LLL output with the following example (or similar):

def multiply(x: num256, y: num256) -> num256:
    return x * y


def multiply_func(x: num256, y: num256) -> num256:
    return num256_mul(x, y)

- Description for the changelog

Allows evaluating of basic arithmetic expressions using uint256 and uint256.

- Cute Animal Picture

…int256.

fubuloubu

Looks pretty good, encompasses all the changes I would expect

fubuloubu · 2017-09-21T17:45:14Z

viper/parser.py

        if o.typ.typ == 'num':
            return LLLnode.from_list(['clamp', ['mload', MINNUM_POS], o, ['mload', MAXNUM_POS]], typ=o.typ, pos=getpos(expr))
        elif o.typ.typ == 'decimal':
            return LLLnode.from_list(['clamp', ['mload', MINDECIMAL_POS], o, ['mload', MAXDECIMAL_POS]], typ=o.typ, pos=getpos(expr))
+        elif o.typ.typ == 'num256':
+            return o


We can return o directly here because there is no need for an overflow check? Is that correct?

@fubuloubu yes that was my understanding, ie. the full 32byte has been occupied by a single variable, therefore can't overflow. But I could be wrong ? I worked from the LLL output of num256_ functions - they definitely don't have any sort of clamping - If we do require it, let me know and I will add it :)

@jacqueswww I believe you should add clamping so that this test case results in TransactionFailed (right now it passes). Clamping here is necessary because it checks user input (which can result in a number more than 2**256-1).
"""

def test_num_256_overflow(): num256_code = """ def _num256_mul(x: num256, y: num256) -> num256: return num256_mul(x, y) c = get_contract_with_gas_estimation(num256_code) x = 2 ** 256 - 1 assert c._num256_mul(x, x) == 1

Perfect @DavidKnott!

Great stuff thanks for the info, will add soonest ;)

fubuloubu · 2017-09-21T17:57:56Z

viper/parser.py

@@ -627,6 +627,8 @@ def parse_expr(expr, context):
            new_unit = combine_units(left.typ.unit, right.typ.unit)


Actually about L623 above. Does it make sense not to implement cross-type operations? I believe it might for num256|decimal, as we might have data loss there, but num|num256 can occur fine withouth data loss (implicit conversion of num to num256).

Same below L656 (in updated file) for division. Does it make sense not to have cross-type division? I'm leaning towards yes, but discuss.

@fubuloubu sure I was going to do cross-type operations as a seperate PR. For the time being the compiler will throw an error with any type of cross-type operation. If you prefer I can do num | num256 as part of this PR/branch ?

I think doing it as a separate PR would work best. There are some subtle considerations that I think warrant a second PR. I'll add an issue to remind ourselves of this.

Cool thanks :) 👍

fubuloubu · 2017-09-21T18:01:00Z

viper/parser.py

@@ -645,6 +647,8 @@ def parse_expr(expr, context):
            new_unit = combine_units(left.typ.unit, right.typ.unit, div=True)
            if rtyp == 'num':
                o = LLLnode.from_list(['sdiv', left, ['clamp_nonzero', right]], typ=BaseType(ltyp, new_unit), pos=getpos(expr))
+            elif ltyp == rtyp == 'num256':
+                o = LLLnode.from_list(['div', left, right], typ=BaseType('num256', new_unit), pos=getpos(expr))


There's a potential for data loss here. I'm not sure if there's a way to avoid it (such as above for num, which also has potential data loss). Discuss.

And by data loss, I mean decimal point flooring (or does it round? @DavidKnott )

jacqueswww · 2017-09-29T09:43:57Z

Closing as this is to be replaced by a specific type or decorator that casts (and clamps) to num.

jacqueswww added 2 commits September 20, 2017 23:46

Allows evaluating of basic arithmetic expressions using uint256 and u…

31da01f

…int256.

Basic test for uint256 and uint256 arithmetic expressions.

584c7ca

fubuloubu reviewed Sep 21, 2017

View reviewed changes

pdaian mentioned this pull request Sep 28, 2017

Add solidity-uint compatibility example, ERC20 #366

Merged

jacqueswww closed this Sep 29, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Num256 with natural expressions. #355

Num256 with natural expressions. #355

jacqueswww commented Sep 20, 2017 •

edited

Loading

fubuloubu left a comment

fubuloubu Sep 21, 2017

jacqueswww Sep 21, 2017 •

edited

Loading

DavidKnott Sep 21, 2017

fubuloubu Sep 21, 2017

jacqueswww Sep 21, 2017 •

edited

Loading

fubuloubu Sep 21, 2017

jacqueswww Sep 21, 2017

fubuloubu Sep 21, 2017

jacqueswww Sep 21, 2017

fubuloubu Sep 21, 2017

fubuloubu Sep 21, 2017

jacqueswww commented Sep 29, 2017 •

edited

Loading

		@@ -627,6 +627,8 @@ def parse_expr(expr, context):
		new_unit = combine_units(left.typ.unit, right.typ.unit)

Num256 with natural expressions. #355

Num256 with natural expressions. #355

Conversation

jacqueswww commented Sep 20, 2017 • edited Loading

- What I did

- How I did it

- How to verify it

- Description for the changelog

- Cute Animal Picture

fubuloubu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacqueswww Sep 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacqueswww Sep 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacqueswww commented Sep 29, 2017 • edited Loading

jacqueswww commented Sep 20, 2017 •

edited

Loading

jacqueswww Sep 21, 2017 •

edited

Loading

jacqueswww Sep 21, 2017 •

edited

Loading

jacqueswww commented Sep 29, 2017 •

edited

Loading