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 #47

Open
code423n4 opened this issue Feb 12, 2022 · 3 comments
Open

Gas Optimizations #47

code423n4 opened this issue Feb 12, 2022 · 3 comments
Labels
bug Something isn't working duplicate This issue or pull request already exists G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

Unused transfer function in NestedReserve

The transfer function in NestedReserve can be only called by NestedFactory, yet the factory doesn't use it. There are specific unit tests for this function but it is not used in the protocol.
Removing it will save 61414 gas on deployment costs.
Note: it is possible you are leaving it there for the future in case NestedFactory will be upgraded and you want this functionality to be possible. If so, this issue is invalid. But maybe it's just an old leftover.

Unused ETH variable in FeeSplitter

FeeSplitter contains an unused ETH variable:

    address private constant ETH = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;

You can remove it for cleanliness and to save 500 gas at deployment.

Change NestedFactory's ETH variable to be immutable

By changing this variable in NestedFactory to immutable instead of constant:

    address private constant ETH = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;

You will save 30-70 gas on various calls.

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Feb 12, 2022
code423n4 added a commit that referenced this issue Feb 12, 2022
@adrien-supizet adrien-supizet added sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue and removed sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue labels Feb 15, 2022
@adrien-supizet
Copy link
Collaborator

adrien-supizet commented Feb 16, 2022

Unused transfer function in NestedReserve

We want to keep it for use cases such as migration or staking of deposited tokens.

Unused ETH variable in FeeSplitter

duplicate #25

Change NestedFactory's ETH variable to be immutable

Duplicated #15

@adrien-supizet adrien-supizet added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Feb 16, 2022
@maximebrugel maximebrugel added duplicate This issue or pull request already exists and removed sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") labels Feb 18, 2022
@harleythedogC4
Copy link
Collaborator

My personal judgments:

  1. "Unused transfer function in NestedReserve". Valid and medium-optimization.
  2. "Unused ETH variable in FeeSplitter". Valid and small-optimization.
  3. "Change NestedFactory's ETH variable to be immutable". Valid and small-optimization.

@harleythedogC4
Copy link
Collaborator

Now, here is the methodology I used for calculating a score for each gas report. I first assigned each submission to be either small-optimization (1 point), medium-optimization (5 points) or large-optimization (10 points), depending on how useful the optimization is. The score of a gas report is the sum of these points, divided by the maximum number of points achieved by a gas report. This maximum number was 10 points, achieved by #67.

The number of points achieved by this report is 7 points.
Thus the final score of this gas report is (7/10)*100 = 70.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate This issue or pull request already exists G (Gas Optimization)
Projects
None yet
Development

No branches or pull requests

4 participants