-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use error strings in smart contracts #81
Comments
@loredanacirstea Do you think you can add them in this contract change (red eyes testnet 4)? I would say it's good to take advantage of the redeployment to include them. |
Yes, added the milestone now. |
Just some notes:
Aside from these mentions, I am ok with just using some error strings right now and re-evaluating them after standards are clarified, because I assume it will take a while. |
The revert error strings cannot be implemented right now in a satisfactory manner, as commented in #213 (comment) |
@loredanacirstea Was looking at the Testnet 4 milestone and was wondering why this is not there yet. I understand now. Sure let's do this. It's not a requirement right now at all anyway. |
P2 because I heard error strings would have saved our time today. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
If we recover |
Whether we optimize or not, any message that fits 32 bytes seems to compile into the samely sized runtime code. So no need to go for cryptic "a" "b" "c" style messages. |
I’ll just add somewhat recognizable comments under 32 bytes. |
This depends on #401. |
Just tried the old PR again, we're still running into code size issues. Blocked by #401 |
See #1435 for more space saving. |
Description
Since solidity 0.4.22 error strings are supported for revert and require:
Adding this to the smart contract will make it easier to debug where things go wrong in a function. It also makes the terrible work flow of removing
require
s to figure out which require is failing obsolete.Tasklist
require
sThe text was updated successfully, but these errors were encountered: