Vyper: VIP: Disallow side-effecting expressions in assert

[charles-cooper]

Simple Summary

Ensure assertions don't have side effects.

Abstract

Assertions should not have any side effects. This also applies to the new assertion type proposed in https://github.com/ethereum/vyper/issues/711.
(Note: memory safety is guaranteed by Vyper's memory model. If Vyper were to have reference types however, the compiler would have to ensure that function calls do not modify memory outside of the function frame).

Motivation

Check out the following misleading function

user_balances: map(address, wei)
balance: wei
def get_balance() -> wei {
  bal: wei = self.balance;
  self.balance = 0 # Uh oh!!
  return bal
}
def accept_funds() {
  self.balance += msg.value
  self.user_balances[msg.sender] += msg.value
  assert self.user_balances[msg.sender] <= self.get_balance()
}

The main issue for readability is that an assertion could be true before it has been asserted but not after!

Specification

Add constraint to any assertion which ensures that the asserting expression has no side effects. I believe the only case in which this can happen is if the asserting expression calls a non-constant or payable function, so, this boils down to checking that if any function calls are made they must be to constant functions. (Note: in Vyper @constant implies non-payable, but I am not sure if this is ensured by the ABI).

EDIT: I realized that even if a function's ABI tells us that it is constant or non-payable, we can't prove it unless it is a function we are currently compiling (like a private function). So we must also use STATICCALL. This lets us use a very straightforward implementation (although the error messages might be confusing) - set context.is_constant to True while compiling the assertion test.

Backwards Compatibility

This could weed out existing programs which have side-effecting asserts. However, I think this is a feature!

Copyright

Copyright and related rights waived via CC0

[jakerockland]

Probably worth adding "VIP: " to the title here as this seems like a noteworthy functional change, I like this idea a lot @charles-cooper 👍

[charles-cooper]

@jakerockland good point - updated, thanks!

[fubuloubu]

On board with this, good catch!

[jacqueswww]

As discussed, this proposal is approved.

[vbuterin]

Support this! In general, side effects inside of things that don't look like they're state-changing expressions are IMO un-Vyperic and should be banned with prejudice :smile:

[jacqueswww]

This has been merged, closing.

[fubuloubu]

As noted in #1468, it seems like a special exception should be made for external calls that return values after modifications are made.