Solidity: Support reason string in assert

Created on 13 Feb 2017  ·  44Comments  ·  Source: ethereum/solidity

Depends on EIP 140 (REVERT) to be fully accepted.

assert(thing, "Thing is invalid");

would basically translate to

if (!thing) revert("Thing is invalid");

(Issue split off #1130.)

feature language design

Most helpful comment

@stevenroose it is under review and will hopefully go with the next release.

All 44 comments

Should this issue be renamed to Support reason string in require()?

Yes, this refers to require and not assert. This issue was created before we settled on the require name, sorry for this confusion.

Thought that was why, I just wanted to clarify. Will there be any limitations on the data which can be given to revert(data) and require(bool, data)?

This is yet to be decided. I think revert might have a few overloaded versions, but the native one is revert(bytes) as that is how the opcode is implemented.

I hope there will be a way to either agree on an encoding scheme for error codes + messages or to include such a scheme in the ABI or Natspec. If this happens, it could have an error code and optional message.

Perhaps we should just do a quick and easy version now:

Error return data is either empty or is an ABI-encoding with the signature (uint,uint,string) where the first argument is always 1, the second argument can be used for an error code and the third argument for a reason string.

Future versions of this specification have to increment the version number.

This also forces the first 31 bytes to be zero which leaves ample room for further "selectors" (for example, the first four bytes could encode a (non-zero) function selector in the future).

Very excited to follow this closely :-D, I am not sure how to contribute, but I'm going to try :-D

To support tracking of assert, we could consider pushing a "source location identifier" (to be defined) just before INVALID:

PUSH ....
INVALID

This way a debugger with having access to the sources could figure out which assertion terminated.

@axic That is a genius idea and very forward thinking! +1000; Too often we are forced into printf-style debugging with solidity and while passing reason strings around is a good step, it still doesn't allow us to utilize modern debuggers with breakpoints, traces, watches, etc. Op-code additions are a serious thing, I imagine, and we should aim for the moon here to get it right.

@axic +1 Yeah I think that would be just as if not more useful than the "message" or "reason" strings requested in various issues. How difficult would it be to implement "source location identifiers" and is there anything I can do towards making this a reality? This is the sort of thing that's going to save so much time the quicker it happens the better.

To support tracking of assert, we could consider pushing a "source location identifier" (to be defined) just before INVALID:

An idea regarding source location identifier is to reuse parts of the source mapping: <16bits source index><32bit offset><16bit length>, which should fit into 5 bytes including the PUSH opcode.

e.g. PUSH 0001000004200024 means source index 1, offset 0x42 and length 0x24.

The debugger then needs to look up the source mapping (emitted by the compiler) to find the actual code in source.

@axic You mean offset 0x420? Or PUSH 00 01 00 0000 4200 24?..

EDIT: I'm not sure I understand how that fits into 5 bytes.

Sorry, it was late. PUSH 0001 00000042 0024 is the correct one and it takes 9 bytes :)

Perhaps a much better optimisations would be the following: insert a unique index for each assert and have a translation table (assertionMap) for source locations emitted by the compiler.

Then again, if debugging is needed, the sources could be recompiled with the same settings to figure out which assertion terminated.

This should only pose an overhead of 2 or 3 bytes per assertions assuming there are only <=2048 assertions in the input.

+1

-1 only on the implementation details.

My understanding of proposal is that ABI will be extended to include a mapping of . And compiler will put a return value on the stack during crash. And the client can translate those strings if necessary to any end-user language.

I don't see why an additional value needs to be put on the stack before revert. This seems unnecessary.

Alternate proposal:

The client can already see where the program crashed by checking the program counter. Simply add a mapping of for assertion operations having names. Or possibly and if you want a normal form database. This saves adding and additional code to programs. Also, you can add assertion language retroactively to existing contracts!

Of course the compiler will need to treat these special REVERT assembly instructions specially: you cannot optimize by merging code paths if it results in two distinct "colors" of REVERT instructions being merged.

@fulldecent:

I don't see why an additional value needs to be put on the stack before revert. This seems unnecessary.

A REVERT always consumes two stack items - to determine memory location and length of data to be returned. The actual return value data comes from memory.

The client can already see where the program crashed by checking the program counter. Simply add a mapping of for assertion operations having names. (...)

(Also described by @axic a few comments back.)

This requires every client that wants human-readable error messages to have this mapping/database. If it's supposed to be distributed with the ABI, that allows for easy translation (to other human languages) of the error messages; but it also means the error messages can be spoofed.

More importantly, relying on the program counter like this will not work nicely in loops; and, in general, only helps to determine _where_ it failed, but not _why_.

Consider, for example, a loop that goes through N addresses and performs a check on every element. REVERT returning a program counter will not be able to indicate which address failed the check.


EDIT: EIP issue 838 (no-status) might be relevant.

@fulldecent this is meant to be read both by humans and by smart contracts. Smart contracts do not easily have access to the internals of another smart contract, especially if the error code is forwarded ("backwarded"?) across multiple call frames. Yet a future specification might standardize the encoding of the error data and the caller can act accordingly.

Also note that programs do not "crash" anymore since byzantium, they just return data together with a failure flag.

This is _only_ about revert, not about assert.

Ah sorry, I actually confused this issue with the pull request of a similar name. Still, I think we are not totally talking about the same thing. Perhaps we should have a call to clarify?

When an error message is returned where can this be seen? Will it be part of the eth_getTransactionReceipt response? Or part of the trace_block?

@cleanunicorn it will not be part of the receipt. We have to check with the client developers to make it accessible. Tracing will certainly show it.

@chriseth So what is the time schedule for this to be implemented?

I just refactored all our contracts from throw to require, revert and assert following this guide from ConsenSys and in there they seemed to imply that it would be supported once the syntax was rolled out. So I just assumed it was there, giving me a ton of Error: Wrong argument count for function call: 2 arguments given but expected 1. :)

@stevenroose it is under review and will hopefully go with the next release.

Started writing Stack Overflow question.
Found the related: https://ethereum.stackexchange.com/a/30066/2524
Arrived here...

Totally...

Sometimes transaction execution fails.

Sometimes programmers put a lot of require, making sure everything works just fine.

If something fails, the error message is generic:

  • invalid opcode
  • revert

Is there a way to pass additional parameter to require with the message?

Docs: http://solidity.readthedocs.io/en/v0.4.21/control-structures.html?#error-handling-assert-require-revert-and-exceptions


Something like assert (now deprecated) or strictAssert accepting optional parameter message

https://nodejs.org/api/assert.html#assert_assert_strictequal_actual_expected_message
assert.strictEqual(actual, expected[, message])

Implemented support in Remix (but no ETA when that will be merged and released on the non-alpha version): https://github.com/ethereum/remix/pull/760

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)

It would be a great idea, I'm still stucking on debugging.

Changed the title again, I thought we had a different issue for require.

Above, we were discussing if it returned strings should calculatable at runtime or if compiletime is good enough.

I still don't see why runtime is necessary. For-loops are an exceedingly rare commodity in this world.

Also, the ABI can't be spoofed, it is generatable by anybody that has access to the source code.

@fulldecent If require and assert would throw to us a reason, it could be helpful to develop and maintenance smart contracts. So, it might useful at runtime.

Is the revert() error message a Solidity thing or a Web3.js issue? The latter potentially being a problem with tool like the Web3.js library not being updated yet to retrieve the error code/msg return at the low-level Ethereum interface, and bubble it up back to the caller?

Also, can someone sort out the following rumors I've heard? Is an error code supported by _revert()_ or an error message, or both? I read somewhere while scouring the Web that the error message idea was dropped because of concerns there could be DDOS-like attacks against the Ethereum network, from people spamming the network with method calls that would easily trigger a revert() and due to the much large byte handling issues involved with strings over a simple error byte return, could put a lot of stress on the network. Can someone shed some light on the actual facts here?

If strings error messages are still being considered, what effect does this have on the _stipend_ as far as the error message possibly not being propagated because of a lack of _gas_?

Incorrectly metered resource consumption ("DDOS") is not a concern for the language, but only for the VM, and I don't think it is an issue here. Currently, strings are implemented (note that this has been finalized for Solidity) as error messages with a possible extension to allow any ABI-encoded data.

Not sure what exactly you mean with "effect [...] on the stipend". This mechanism uses returndata which is mostly paid for by the caller.

@chiro-hiro Here is exactly how the alternate proposal works:

Contract code

pragma solidity ^0.4.21;
contract Math {
    function addPositiveNumbers(int addend, int otherAddend) external pure returns (int sum) {
        require(addend >= 0, "Parameter one must be positive");
        require(otherAddend >= 0, "Parameter two must be positive");
        sum = addend + otherAddend;
        assert(sum > addend, "I thought adding was monotonic!");
    }
}

Byte code

123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef01234

somethingsomethingsomethingsomethingREVERTsomethingsomethingsomethingREVERTsomREVERT

ABI

[
  ...
  "revertStrings": {
    "0x25": "Parameter one must be positive",
    "0x46": "Parameter two must be positive",
    "0x4f": "I thought adding was monotonic!"
  },
  ...
]

Discussion

Yes, you do have access to the revert reasons at run time, not compile time. However at compile time there is no need to place english-language strings into the byte code.

Also my approach allows you to extend this concept by adding a file math-zh.po:

#: Math: addPositiveNumbers:4
msgid "Parameter one must be positive"
msgstr "参数一必须是正数"
msgid "Parameter two must be positive"
msgstr "参数二必须是正数"
msgid "I thought adding was monotonic!"
msgstr "我不知道怎么开发区块链软件!"

@fulldecent It's make scene, an offset of message could be better than put a string inside opcode. It's less annoying to deal with Unicode encoding.

It seems this was added to the language but I can't find a way on obtaining the messages. Does anyone hasmore information about this?

E.g. see my test contract with source code on the Ropsten node:
https://ropsten.etherscan.io/address/0x8e663a720295c7518fba5ae3d4c4655dca4ddaa0

Last transaction failed but I can't find the message that should be written by it.
https://ropsten.etherscan.io/tx/0x8111735fcf2d2310e1a7a6d16419e5e35bfd410c43b456003f51ab88ee2328a8

@fulldecent @axic

@devedse with web3js or at least with the latest truffle (version 5) you can get the revert reason from the exception e.g.

try {
  await contract.myMethodCall();
} catch (error) {
  // error.reason now populated with an REVERT reason
  console.log("Failure reason", error.reason);
}

Oh so cool!

@jamesmorgan, I see, I assume however that you can't do this on transactions that have been executed on the blockchain though?

@jamesmorgan I can see the reason field, but no matter what I do, it is always undefined.

Any progress in that? I still can't see revert messegas on ethereum testnet (while it works very fine on TRON). The error I got has this format:

Transaction has been reverted by the EVM:
{
  "blockHash": "0xcb70ea5a9ca990504cd3dcb5577c50f25d004d3e959f584a728906f63e019953",
  "blockNumber": 4057666,
  "contractAddress": null,
  "cumulativeGasUsed": 92226,
  "from": "0x143aa1f77937a3d5b81cbedd62584dddfbf0fe2a",
  "gasUsed": 66752,
  "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
  "status": false,
  "to": "0x7649b36218e573aad06112228f540aee670aaabb",
  "transactionHash": "0xc6e25f993de08f0f66f23ea77e7e49119d43698b21ff3273d8cc1999e3080e9c",
  "transactionIndex": 1,
  "events": {}
}

@mscherer82 there is no indication why it should not work. Perhaps you are using a client that does not support it?

Did you set the testnet to be byzantium-compatible?

Here is a bash script to fetch revert reason:

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

Was this page helpful?
0 / 5 - 0 ratings