Implement revert reasons

[cburgdorf]

What is wrong?

#74 adds support for revert but it does not yet add support revert reasons. We should support revert reasons.

How can it be fixed

I haven't given that much thought. I guess it boils down to reserving some space for revert reasons and then calling rever(start, end) with the correct memory offsets but I would have to take a closer look.

[cburgdorf]

Now that #342 landed, we do have support to revert with properly Error(string) encoded revert messages.

However, we only support it in assert statements so far as in assert false, "Meh, super bad".

We do not yet support it with revert. Looking into this, it got me thinking again whether revert should actually be a keyword or a function.

Fun fact, in Solidity revert can be used both as keyword and a function. Citing from the docs:

For backards-compatibility reasons, there is also the revert() function, which uses parentheses and accepts a string:

When you use it as a keyword you can provide a custom error as in revert CustomError(arg1, arg2);
However, you can not revert with just a string using the keyword syntax, e.g. the following does NOT work revert "foo".

A while back I created an issue that argued to turn revert into a library function but now I'm starting to question that. It seems that Solidity has the function only for backwards compatibility and one could also argue that in traditional functions raise is usually a keyword. I'm sure there are a tons of pros and cons for either direction but I'm not having them at the top of my head 😅

Does anyone have opinions about that?

[cburgdorf]

I've found some really interesting discussions from Solidity regarding that:

• Typed errors / exceptions solidity#7877
• Move away from INVALID and use custom error types instead. solidity#9824
• https://gist.github.com/christianparpart/8dbfafca45306d102eb115485aa48fe3

Main take away is that Solidity seems to be focused on deprecating the revert() function syntax and will use revert as keyword. Afaik they are also looking to introduce an error keyword to define custom errors.

In practice, one could define an error like this:

error LoginError:
    required_rights_group: u256

Based on that type, an ABI selector of LoginError(uint256) could automatically be inferred.

One could then use it as revert LoginError(5)
The error that solidity currently uses to propagate revert("not enough ether") (and that we now also use for assert statements #342) would basically boil down to a builtin error defined as:

error Error:
    message: string

I think it would be reasonable to follow down the same path. However, it could also be interesting to explore an interface / trait based solution. That said, interfaces/traits seem to be still further away and adding an error type would be relatively easy to achieve in the short term.

One thing to discuss would be if we still want to support revert "some string" as a shorthand for revert Error("some string") (the lowering pass could take care of that) but my immediate reaction would be to NOT support that because it would be some form of special compiler magic that might confuse people.

To sum up, what I think we should do short term is:

• Add a new keyword / type error
• Add a new Error error with only a message field (Open question: would that message be limited in size?) as a pre-defined type in the runtime
• Change revert to optionally revert with an instance of an error type (so both revert and revert Error("foo") would be legal

@g-r-a-n-t What do you think?

[cburgdorf]

Just leaving this here as another reference: https://blog.soliditylang.org/2021/04/21/custom-errors/

[cburgdorf]

@g-r-a-n-t @sbillig any thoughts on implementing the proposed custom error mechanics. I think it's very mvp worthy and sentiment that I get from twitter is that people seem to be quite happy with it.

[tfalencar]

Hi! I'm not a language-expert, but I'm excited about Fe, and I hope my input can be useful:

One thing to discuss would be if we still want to support revert "some string" as a shorthand for revert Error("some string") (the lowering pass could take care of that) but my immediate reaction would be to NOT support that because it would be some form of special compiler magic that might confuse people.

+1 , I'd also vote to just drop the old syntax.

Some interesting analysis / discussions on this topic from other Swift and other languages: https://github.com/apple/swift/blob/main/docs/ErrorHandlingRationale.rst#id49

Looking into this, it got me thinking again whether revert should actually be a keyword or a function.

My personal opinion, would be to treat it as a keyword. I kinda read "revert" as the equivalent to "throw" in swift for instance (https://docs.swift.org/swift-book/LanguageGuide/ErrorHandling.html) ; Having it as a statement might make it more familiar to other people too.
Maybe a "reverts" keyword could be used in the function signature (i.e., after parameters) to express that this function can call revert 🤔 .. if you are considering try/catch expressions?