Support reason string in assert

[axic]

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.)

[maurelian]

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

[axic]

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

[maurelian]

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)?

[axic]

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.

[chriseth]

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).

[GriffGreen]

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

[axic]

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.

[awgneo]

@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.

[Quazia]

@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.

[axic]

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.

[veox]

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

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

[axic]

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

[axic]

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.

[Suhail]

+1

[fulldecent]

-1 only on the implementation details.

My understanding of proposal is that ABI will be extended to include a mapping of <revertReturnValue => String>. 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 <program counter => String> for assertion operations having names. Or possibly <program counter => assert code> and <assert code => String> 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.

[NachoPal]

+1 #1686 (comment)

[chiro-hiro]

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

[chriseth]

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

[fulldecent]

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.

[chiro-hiro]

@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.

[roschler]

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?

[chriseth]

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.

[fulldecent]

@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

000000000000000011111111111111112222222222222222333333333333333344444444444444445555
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 "我不知道怎么开发区块链软件！"

[chiro-hiro]

@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.

[devedse]

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

[jamesmorgan]

@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);
}

[xinbenlv]

Oh so cool!

[devedse]

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

[simonDos]

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

[mscherer82]

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": {}
}

[chriseth]

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

[chriseth]

Did you set the testnet to be byzantium-compatible?

[gluk64]

Here is a bash script to fetch revert reason:

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

[Samboy76]

Following the above previous example, how do you access "RevertStrings" data at run-time?
Do you access RevertStrings from test javascript?
I´ve enabled "debug": {"revertStrings": "default"} in truffle-config.js.

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

Best,
Sam

[github-actions]

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

[github-actions]

Hi everyone! This issue has been automatically closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.