Revert with reason

[chriseth]

Part of issue #1686

Depends on #3373

I went with encoding the reason string as a function call to Erorr(string), including the function selector.

Still to do:

• the same for require()
• documentation for require()
• forwarding of the reason string by the caller together with bubbling up the error
• tests for forwarding reason string for weird cases like create and transfer.

The reason we might not want to forward the reason string is that it allows a called attacker to consume more gas in the caller (because the caller needs to copy the return data), but I think this is an acceptable "risk" given the benefit that the error string shows up in the transaction receipt.

[SilentCicero]

Looking forward to this ;)

[GriffGreen]

Me toooooooo!!!! :-D

[chriseth]

This is ready for review.

[onbjerg]

❤️ ❤️ ❤️ ❤️ This is the best!

[izqui]

Is the string returned regularly, right? In terms of forwarding the return data, would this work https://github.com/aragon/aragon-core/blob/dev/contracts/common/DelegateProxy.sol#L9

[chriseth]

Rebased.

[AnthonyAkentiev]

Great improvement!

[tcoulter]

Looking forward to implementing this in Truffle. Nice job @chriseth!

[naddison36]

Sad to see this bumped to 0.4.22. I was getting ready to change my contracts to take advantage of this new feature with the 0.4.21 release

[chriseth]

Failure in appveyor is just bytecode upload and thus can be ignored.

[chriseth]

I think prefixing a specific 4-byte sequence allows us to extend the encoding later, while at the same time it does not reduce usability since it is fixed for now. Also, it is better than just prefixing a certain number of zeros, since it already suggests away how to extend it to encode different types.

I would opt to merge it :)

[axic]

... and it doesn't support an error string.

[chriseth]

Yes, alternative to rever(), not to revert("string") :)
Do you want me to clarify that or not?

[axic]

I think it may be cleaner that way. I didn't spot it before you mentioned, of course it is clear after you did.

[axic]

This is still dead code.

[axic]

Needs test cases for built in revert bubbling:

• transfer
• contract creation
• regular foreign contract calls

i.e.

contract C {
  function f() {
    revert("failed here :(");
  }
}
contract D {
  D d = new D();
  function f() {
    return d.f();
  }
}

[chriseth]

The test case bubble_up_error_messages does it for a regular external function call, bubble_up_error_messages_through_transfer for transfer and bubble_up_error_messages_through_create for create....

[axic]

That's weird, I have seen revert_with_cause and require_with_message, but not the bubble up ones. Anyway, they look correct.

[leanthebean]

Oh wow, this is so great. Thank you!

[vsdigitall]

contract Example {
  function test (uint i) {
    require(i == 1, "ERROR_TEXT");
  }
}

How to read require/revert error string from failed transaction log? (ERROR_TEXT in example above)

[andreafspeziale]

I know that maybe is not the right place but I've not found any references or examples so +1 to @vsdigitall. I think that the reason string is in the output as hex in the failed transaction. But I've been not able to fully test this thing.

[gluk64]

Here is a bash script to fetch revert reason:

https://ethereum.stackexchange.com/a/70391/3558