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

Gas Optimizations: PR #1 (closed #209 to include changes in this one) #213

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

Antony-SS
Copy link

@Antony-SS Antony-SS commented Jul 12, 2023

Changes are limited to Verifier.sol, Pairing.sol, MiMC.sol, and MerkleTree.sol

I took advantage of the following methods for gas saving:

  • ++i instead of i++ everywhere in all files. Pre-increment is cheaper because it does not store a copy of i's value.
  • Replaced memory with calldata where applicable in Verifier.sol. Calldata is an immutable, temporary location where calldata can be stored that behaves similarly to memory. It stops unecessary copies being made, thus saving gas.
  • Marking variables constants where possible. Constants are turned to bytecode at compile time and save the costly sload operation when accessed from that point forwards. All changes to do with this are in MerkleTree.sol
  • Caching storage variables that are accessed more than once in a given function. Accessing storage variables is costly. It is better to save them into memory if they are going to be used more than once. All changes to do with this are in the MerkleTree.sol file
  • Adding indexed to event with value types. Small trick that is slightly more gas efficient
  • Adding unchecked to increment variables in for loops where we know they won't overflow. This saves the EVM from having to do overflow checks everytime it increments.

For specifics on the gas savings see this Notion Doc: https://antony-ss.notion.site/Antony-s-Starlight-Gas-Investigation-PR-1-4ee119fe89d24e56b41f3cc87d9b686d?pvs=4

There are some changes to the .gitignore b/c I added a test-contracts folder with all the contracts I was testing. You can choose whether or not to include this in the PR.

Addtionally, in order to get Starlight to run on my computer I had to update my "https://github.com/types" dependency, which is reflected in the changes. I would reccomend including this change.

uint public treeWidth = 2 ** treeHeight;
uint public constant zero = 0;
uint public constant treeHeight = 32;
uint public constant treeWidth = uint64(2 ** treeHeight);
Copy link
Author

@Antony-SS Antony-SS Jul 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reject the change to cast to a uint64. Accidentally left this in from when I was testing treeWidth as a uint64. Make sure to keep it as a constant though.

@MirandaWood MirandaWood self-requested a review July 17, 2023 08:04
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