Solidity: Provide messages for internal reverts and option to remove.

[thegostep]

Description

abi.decode() reverts but does not return an error message on failure.

Error: VM Exception while processing transaction: revert

Adding a revert message like "abi.decode() input too short" would help with debugging.

related to #4224

Environment

• Compiler version: 0.5.11

[chriseth]

We decided against revert messages for compiler-generated errors because they are horribly expensive. We could, though, add them with a compiler option. This is related to https://github.com/ethereum/solidity/issues/6251

There could be three levels:

• debug: add revert reason strings for reverts generated by the compiler
• default: do not add revert reason strings for reverts generated by the compiler
• compact: remove all reason strings, also the ones supplied by the user

[thegostep]

I like this a lot!

Compact would also be helpful to avoid hitting code size limit.

[ekpyron]

@MicahZoltu suggested to also have the compiler change require(amount < 5); to require(amount < 5, "amount < 5"); - that would be a nice addition to the "debug mode".

EDIT: to discuss: would this need a separate command line option or is the debug mode fine? Should there be more annotations in the strings, like file name, line number, contract name, function name, etc.?

[thegostep]

Should there be more annotations in the strings, like file name, line number, contract name, function name, etc.?

+1 on this

[Marenz]

we have implemented this now, haven't we? @chriseth

[ekpyron]

@Marenz No - we have options for adding more of them and removing them, but e.g. abi.decode still doesn't actually produce any.

[leonardoalt]

Eventually (hopefully soon) ABIEncoderV2 will be the default and the current default one removed. Regarding encoding/decoding, should this then be done only for ABIEncoderV2?

[MicahZoltu]

Is the cost of the revert string in contract size or runtime gas costs? If runtime gas costs, is the cost only if the reversion happens, or do you have to pay the cost even if the revert isn't hit (for example, does require(true, "reason") cost the same amount to execute as require(true)?

[chriseth]

@MicahZoltu I don't think this feature will be used in production. The costs are mostly bytecode size, i.e. deploy-time costs.

[chriseth]

@leonardoalt this is also about division by zero and others. If it is too complicated to implement, it is fine to only do abiv2

[leonardoalt]

It's not too complicated, but definitely more work, so wanted to check before

[MicahZoltu]

@chriseth I definitely would use debug mode in production. Troubleshooting end-user errors with no useful messages is a _huge_ pain. Deployment costs are almost always insignificant to me, what I care about is runtime costs.

[chriseth]

@MicahZoltu ok, fair point! We should still ensure that the messages are not unnecessarily long.