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

Solidity 0.8.4 Custom Errors #2839

Closed
dmihal opened this issue Aug 30, 2021 · 13 comments · Fixed by #4261
Closed

Solidity 0.8.4 Custom Errors #2839

dmihal opened this issue Aug 30, 2021 · 13 comments · Fixed by #4261
Assignees
Labels
breaking change Changes that break backwards compatibility of the public API. feature New contracts, functions, or helpers.

Comments

@dmihal
Copy link

dmihal commented Aug 30, 2021

Solidity 0.8.4 introduced "custom errors", which are named errors that can be thrown similar to how events are emitted.

These bring many benefits: primarily the gas savings of not needing to embed revert strings in the bytecode, as well as the ability to include data in the error.

More details:
https://blog.soliditylang.org/2021/04/21/custom-errors/

I would love to see these errors added to OZ contracts.

@Amxx
Copy link
Collaborator

Amxx commented Aug 31, 2021

Hello @dmihal

This is definitely on our roadmap!

Since you are interested by this change, I have a few questions for you.

  • Do you inspect the content of revert messages? Show it to users?
  • What library do you use to perform/process web3 calls? Ethers? Web3js?
  • Do you think all reverts should use custom errors, or should we keep some revert strings?

@Amxx Amxx added the feature New contracts, functions, or helpers. label Aug 31, 2021
@Amxx Amxx self-assigned this Aug 31, 2021
@dmihal
Copy link
Author

dmihal commented Sep 1, 2021

Thanks @Amxx!

  • I see revert messages as being useful in 2 places:
    • Unit tests, to check whether the correct error was thrown
    • Etherscan, to see why a transaction failed. AFAIK, Etherscan doesn't show custom error messages yet, but I'm sure it's on their roadmap
  • I use Ethers.js
  • I believe all reverts should switch to custom errors

@frangio
Copy link
Contributor

frangio commented Sep 2, 2021

We really want to do this but strictly want Truffle/Web3.js to support custom errors before we change to them. Otherwise, the change would be a downgrade in Dev Experience for that subset of users. 🙁

@PaulRBerg
Copy link
Contributor

PaulRBerg commented Sep 2, 2021

Casting my support for this as well. Custom errors are a game-changer for Solidity development. It would be fantastic if OpenZeppelin supported them!

For reference, here are a couple of projects I built or contributed to that use 100% only custom errors:

Do you inspect the content of revert messages? Show it to users?

We do, both in development and in production. In development, out frontend devs find it useful when there is a revert reason or custom error when something goes awry. In production, we filter the error we get from the JSON-RPC provider, and if it's not a bespoke error like the user not signing the tx, we display it in the frontend.

What library do you use to perform/process web3 calls? Ethers? Web3js?

Ethers.js

Do you think all reverts should use custom errors, or should we keep some revert strings?

100% custom errors.

@PaulRBerg
Copy link
Contributor

We really want to do this but strictly want Truffle/Web3.js to support custom errors before we change to them

This might one of those nasty catch-22 situations. Truffle/ Web3.js might also be waiting after the most popular smart contract libraries (cough cough OpenZeppelin) to add support for custom errors before they do it.

Should someone who knows the main contributors behind Truffle/ Web3.js tag them in this discussion?

@dmihal
Copy link
Author

dmihal commented Sep 3, 2021

Web3.js is part of ChainSafe now, I'll tag @spacesailor24 & @GregTheGreek to see if they have any thoughts

@GregTheGreek
Copy link

We're trying to get our TypeScript rewrite finished (we're just about there) and thus have put a feature freeze on 1.x until it's done.

@spacesailor24
Copy link

@dmihal this issue is tracking this though

@frangio frangio added the on hold Put on hold for some reason that must be specified in a comment. label Sep 16, 2021
@frangio frangio mentioned this issue Sep 16, 2021
1 task
@Amxx Amxx mentioned this issue Oct 18, 2021
@Amxx Amxx added this to the 4.8 milestone Jun 8, 2022
@Amxx Amxx removed this from the 4.8 milestone Aug 16, 2022
@ernestognw
Copy link
Member

ernestognw commented Feb 3, 2023

There's now an EIP (6093) for standard custom errors.
We're planning to use a similar rationale to move revert strings to custom errors in OpenZeppelin Contracts. Feedback and discussion is appreciated.

lhemerly added a commit to lhemerly/openzeppelin-contracts that referenced this issue Mar 25, 2023
Fixes OpenZeppelin#2839

Add feature custom error to ERC20

Testing is raising errors for multicall, ERC777 and ERC 4626 because I have not added the feature to them yet, and they are using ERC20 behavior. Wanted to make sure the design decisions I made for the feature and testing are OK so I can move forward to the other contracts

- [ X ] Tests
- [ ] Documentation
- [ ] Changeset entry (run `npx changeset add`)
@ernestognw ernestognw added this to the 5.0 milestone May 16, 2023
@ernestognw ernestognw added breaking change Changes that break backwards compatibility of the public API. and removed on hold Put on hold for some reason that must be specified in a comment. labels May 16, 2023
@frangio
Copy link
Contributor

frangio commented Jun 15, 2023

Fixed in #4261.

@PaulRBerg
Copy link
Contributor

Fixed in #4261.

Prettt cool. Will this be released in V4.x, or is it slated for V5?

@DragonDev1906
Copy link

Prettt cool. Will this be released in V4.x, or is it slated for V5?

5.0 (see the Milestone), since it is a breaking change on the ABI of basically all contracts.

@0xnook
Copy link

0xnook commented Jun 16, 2023

Fixed in #4261.

Awesome change, will this be done for OpenZeppelin/openzeppelin-contracts-upgradeable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Changes that break backwards compatibility of the public API. feature New contracts, functions, or helpers.
Projects
None yet
9 participants