openzeppelin-contracts icon indicating copy to clipboard operation
openzeppelin-contracts copied to clipboard

Solidity 0.8.4 Custom Errors

Open dmihal opened this issue 2 years ago • 9 comments

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.

dmihal avatar Aug 30 '21 19:08 dmihal

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 avatar Aug 31 '21 07:08 Amxx

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

dmihal avatar Sep 01 '21 22:09 dmihal

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. :slightly_frowning_face:

frangio avatar Sep 02 '21 15:09 frangio

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:

  • https://github.com/hifi-finance/prb-math
  • https://github.com/paulrberg/prb-contracts (see IOwnable and Erc20)
  • https://github.com/hifi-finance/hifi (in particular, see the protocol package)

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 avatar Sep 02 '21 20:09 PaulRBerg

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?

PaulRBerg avatar Sep 02 '21 20:09 PaulRBerg

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

dmihal avatar Sep 03 '21 10:09 dmihal

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.

GregTheGreek avatar Sep 03 '21 10:09 GregTheGreek

@dmihal this issue is tracking this though

spacesailor24 avatar Sep 04 '21 01:09 spacesailor24

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.

ernestognw avatar Feb 03 '23 15:02 ernestognw

Fixed in https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4261.

frangio avatar Jun 15 '23 02:06 frangio

Fixed in https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4261.

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

PaulRBerg avatar Jun 15 '23 07:06 PaulRBerg

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.

DragonDev1906 avatar Jun 15 '23 08:06 DragonDev1906

Fixed in #4261.

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

0xnook avatar Jun 16 '23 21:06 0xnook