Add SafeMath to num type

[DavidKnott]

What's your issue about?

num is currently able to overflow in Viper`
The following code works:

def test_num():
    code = """
@public
def num_sub() -> num:
    return 1-2**256
"""
    c = get_contract(code)
    assert c.num_sub() == 1

How can it be fixed?

Added SafeMath checks to num arithmetic.

Fill this in if you know how to fix it.
In expr.py in function def arithmetic(self): Add in SafeMath LLL

Cute Animal Picture

[fubuloubu]

Safemath is a pretty easy thing, basically just assertions that the math won't overflow before you do the math e.g. assert a-b == -(b-a) for a-b (or something like that). We can probably adopt the solidity safe math library pretty easily.

It's more a strategic issue to me. Do we want to enforce all math to be safe math, therefore adding extra assertion code and costing extra gas? I believe for Viper that answer is yes, but I wanted to clarify this point.

[fubuloubu]

Also this is adding runtime errors that don't have to do with out of gas errors, and therefore hard to track (without an error log)

Speaking of error logs, what do you think about a native Viper error log on all contracts?

[fubuloubu]

Related to error logs, assert condition could become a macro for:

if cond:
     logs.Error('AssertionError', ...)
     throw

[DavidKnott]

@fubuloubu I think everything being overflow / underflow is worth the extra gas, given that so far we've been prioritizing trying to prioritize safety everywhere so far.

[jacqueswww]

@DavidKnott @fubuloubu I believe this has been implemented?

[fubuloubu]

I think so

In the future we can run optimizations that can formally verify SafeMath as unnecessary and remove it for gas savings. Wayyyy down the line though.

[jacqueswww]

Re-opening as the above test case still fails.

[fubuloubu]

2**256 shouldn't work though... It's above the uint256 limit

[tpmccallum]

Safety everywhere indeed!
Given this catastrophic integer overflow issue in a Solidity token contract https://medium.com/@peckshield/alert-new-batchoverflow-bug-in-multiple-erc20-smart-contracts-cve-2018-10299-511067db6536 and the fact that developers can write code outside of the help provided by SafeMath and other analysis tools it makes sense to enforce overflow protection at the compiler level, yeah?

[jacqueswww]

Yip exactly! Vyper's main goal is to make it maximally difficult to introduce insecure code 😉 and to focus on auditable / readable code.

[tpmccallum]

💯