Skip to content
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

Switch to custom errors #61

Open
CodeSandwich opened this issue Nov 22, 2021 · 6 comments
Open

Switch to custom errors #61

CodeSandwich opened this issue Nov 22, 2021 · 6 comments

Comments

@CodeSandwich
Copy link
Collaborator

As @byterose wrote in #45:

Custom errors decrease both deploy and runtime gas costs.
Additionally all expected errors are listed in the abi file which can be used to improve the UX of contract interactions.

Introduction blog post

@CodeSandwich CodeSandwich mentioned this issue Nov 22, 2021
2 tasks
@ghost
Copy link

ghost commented Jan 10, 2022

If this issue is still of interest, I would be open to push a pr.

Custom errors are similar to NatSpec comments in the sense that they are not required but can improve the developer and user experience.

@CodeSandwich
Copy link
Collaborator Author

The user experience is the thing I'm most afraid about. AFAIU custom errors are lightweight integer IDs with optional details payloads. To know what an ID means a tool needs to look up the contract ABI, e.g. on Etherscan. How well supported is this flow across the ecosystem? E.g. if an error is thrown and your wallet software can only tell you that "an error 123456 occured", that's bad user experience. String errors are widely supported and provide everything needed to form a proper error message. They're also very easy to debug.

@ghost
Copy link

ghost commented Jan 10, 2022

I understand that the potential low ecosystem support is a concern at the moment but I am also convinced that this will be the standard for smart contract interactions in the near future. In addition to that I want to point out that these custom error messages can be used for crafting front-end notifications and would therefore support front-end developers. It is a nice addition if both deploy and runtime gas costs decrease.

On the other hand, as you said, "errors are widely supported and provide everything needed to form a proper error message".

@CodeSandwich
Copy link
Collaborator Author

The gas cost argument IMO isn't a very important one. For runtime, errors are very rarely thrown in the actually mined transactions. For deployment yes, it'll be a bit cheaper, but we're talking about 24 requires, let's say 32 characters each, which adds up to ~750 bytes. Custom errors IDs are 4 bytes each, so the savings would be in the ballpark of ~650 bytes.

I think that the biggest advantage of custom errors is what can be done with them in the UI with proper error handling, error message translations and precise details about what's wrong.

I support that change, I'm only not sure if the ecosystem is ready for it and if we'll shoot ourselves in the foot by chasing the shiny new conventions.

@ghost
Copy link

ghost commented Jan 10, 2022

Precise evaluation! Since the ecosystem moves fast and these contracts will probably be used for a significant timeframe, I suggest we wait until foundry comes up with a stable release and then come back to this issue. Otherwise it will be hard to write tests for custom errors.

@ghost
Copy link

ghost commented Feb 19, 2022

@byterose here is a svelte package that can handle custom error reverts with natspec comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant