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

ZoraModuleManager.sol Gas Optimization #178

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TradMod
Copy link

@TradMod TradMod commented Aug 22, 2022

Description & Motivation and Context

Most of the Zora-V3 contracts are using require statements for reverting errors. Which is not a very gas-efficient way to revert errors. The require statements stores Strings which costs a lot of Gas (deploying + function Calling & Reverting).
And as the protocol aims to be Gas Efficient, Then it would be much better to not use require statements to revert the errors.
Instead, use Custom Errors. Which is a new solidity feature (introduced in 0.8.*)
Custom errors do the same thing but cost much less gas than the require statements.
For more info read this

How has this been tested?

Firstly to show that, the Custom Errors lowers the deploying Costs I tested before & after deploying gas cost tests on Remix IDE. It's saving 199117 Gas

Before:
zoraModuleManagerRemixBefore

After:
zoraModuleManagerRemixAfter

And after that, to get an estimate of how much function Calling Gasis saving I did before & after tests on VS Code using Foundry.

Before:
zoraModuleManagerBefore

After:
zoraModuleManagerAfter

Even tho, 2 foundry tests are failing but we can see that custom errors are saving functions calling Gas As well.
And the 2 functions that are failing are just because they were not expecting those lines of errors.
Even tho, I changed the tests' revert text as well (similar to the expected error) but it's still failing don't know why, so please anyone from the Zora team pls guide me on what more changes I need to make.

Checklist:

My changes do not require any of the followings:

  • The module includes tests written for Foundry
  • The module is documented with NATSPEC
  • The documentation includes UML Diagrams for external and public functions
  • The module is a Hyperstructure

1 contract fixed of issue #177 :)
Thanks,
AB Dee.

@iainnash
Copy link
Contributor

When this protocol was launched, custom errors had pretty poor support in tooling so we opted to use string errors. Our newer code uses custom errors. The largest difference is deployment cost which isn't as much of a concern for a protocol like this.

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

Successfully merging this pull request may close these issues.

2 participants