Change effect of assert to invalid opcode.

[chriseth]

No description provided.

[chriseth]

This might need also updates for the tests.

[axic]

This will break the possibility to return the assert reason as a text, because that is only possible via revert.

[chriseth]

@pirapira this might also be of interest to you.

[pirapira]

@chriseth for me, it's fine if assertion failures are translated into 1) the invalid opcode, or 2) REVERT with data somehow saying "assertion failure".

[pirapira]

The phrase "internal conditions" might not make sense to everyone. What about "If contracts are written so that assert should never fail at runtime."

[chriseth]

I would like to convey when to use assert and when to use throw. How would you write that?

[pirapira]

An assert should be a sanity check that is expected to pass always. An assert failure should detect coding mistakes (e.g. when the owner of something has changed without the owners' permission).

throw should catch everything else that goes wrong at runtime (e.g. when the caller supplies wrong data; it's not time yet to call this function).

[wuestholz]

@chriseth @pirapira Maybe it would be helpful to explain who is responsible for ensuring that there are no such exceptions: "assert is used to express safety conditions that the contract should ensure. Consequently, the author of the contract is responsible for avoiding any assertion violations. In contrast, throw can be used to express that some input is not considered valid. It is the responsibilty of the client/caller, as opposed to the contract's author, to ensure that this does not happen. Formal analysis tools can be used to verify that no execution of a contract leads to an assertion violation assuming the client/caller provided valid inputs.".

Does that capture what you want to express?

[pirapira]

@wuestholz yes, that sounds accurate. Thank you.

[pirapira]

I don't think the discussion with @axis is over. If assert() failures always mean the programmers' mistakes, this PR makes more sense. If assert() is sometimes used to tell the caller's mistakes at runtime, @axic's point stands.

[chriseth]

@pirapira indeed. I think in the end we perhaps need two different asserts.

[wuestholz]

@chriseth @pirapira @axic I don't think two different asserts are necessary. I tried to elaborate on the difference in a comment above.

In my opinion, an assertion violation should be "blamed" on the author of the contract where the violation happened. The client/caller of the contract shouldn't need to know about all the possible things that may go wrong (e.g., division by zero) in the callee contract.

In contrast, an exception that is thrown is similar to a precondition violation. The client/caller should be "blamed" in this case and should be given a chance to react by providing a different input or by propagating the exception up the call stack.

In short, use a throw if the author/programmer wants to blame the client if the condition does not hold. Use an assert if the author/programmer considers it his or her responsibility to ensure that a condition always holds.

In a nutshell, this is the main idea behind Design by Contract (https://en.wikipedia.org/wiki/Design_by_contract).

[chriseth]

@wuestholz yes, this is also what I currently have in mind. The question is whether we want to shorten if (!x) revert(reason) into a single function.

[pirapira]

Then we can have

• if (!x) revert (reason) for user's mistakes
• if (!x) waaah for programmer's mistakes

Assertions can be defined in a library, so they do not need to be defined in the language.

[wuestholz]

@chriseth @pirapira Great that we seem to agree on the semantics. :)

Now about the syntax: I agree that one could define such things as a library. However, I think chances are higher that people make use of this if the language supports it natively. Given my background in program analysis assume(cond, msg) and assert(cond) would be obvious choices. However, I understand that I'm not very representative. ;)

Assertions should be familiar to most programmers and I don't see a reason to reinvent the wheel here. However, most programmers will not be familiar with assume-statements. Maybe something like require(cond, msg) would be more intuitive, in particular if someone is familiar with Design by Contract.

Another reason for such native language support is that tools can extract such conditions more easily (e.g., to generate documentation that includes preconditions for procedures).

[chriseth]

"Decision" of team meeting: Remove the global assert function for the next release. After that, we will have

1. revert("reason") which uses the new revert opcode
2. some function to check external conditions which is a shorthand for if(!condition) revert(reason)
3. some function to check internal failures which will use 0xfe and potentially provide a reason.

The actual naming of 2 and 3 has to be discussed.

[chriseth]

@axic @pirapira ready for review.

[wuestholz]

@chriseth Thank you for the update! Please let me know if you would like my input once you're ready to design the changes for the next release. I'm very interested in finding a good solution for these issues and would be glad to help.

[axic]

Can we roll back the first commit?

The second commit also needs to adjust the changelog.

Then again if we make a decision on naming on Tuesday/Wednesday, we might as well keep this PR open until that point.

[chriseth]

@wuestholz yes, your input would be very valuable! We agreed that the goal would be to have two built-in functions:
The first is used to check for invalid input / preconditions / user error. It would allow an error message that is handed back to the caller using the revert opcode.
The second is used to check for internal logic / programming errors (like verifying that after you sent money from A to B, A has less money than before). It does not necessarily have the ability to provide a message and would cause an invalid opcode (different from the revert opcode).

The question is: How to name those two functions?

[wuestholz]

@chriseth Great! I think that makes a lot of sense and is consistent with what I suggested above. As I said above, adding a require(cond, msg) statement (for the first use case) and an assert(cond) statement (for the second use case) would seem quite intuitive to me. However, I'd be happy to think about other suggestions. For instance, I could also imagine ensure(cond) for the second use case. What do you think?

[pirapira]

Looks good to me. Now the code is properly waiting for a decision on names.