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

Open
code423n4 opened this issue Jun 3, 2022 · 1 comment
Open

Gas Optimizations #90

code423n4 opened this issue Jun 3, 2022 · 1 comment
Labels
bug Something isn't working G (Gas Optimization) resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

  1. Don't change storage variable needlessly
    contracts\BkdLocker.sol#L144
    Currently it replaces stashedWithdraws[i] even if "i" equals to "stashedWithdraws.length - 1".
    And it calculates stashedWithdraws.length every time after pop right, you can reduce gas by changing the length variable.
    You can replace L144 like below.

--length;
if(i != length) {
    stashedWithdraws[i] = stashedWithdraws[length];
}

  1. Needless conditions.
    contracts\tokenomics\InflationManager.sol#L575
    contracts\tokenomics\InflationManager.sol#L589
    contracts\tokenomics\InflationManager.sol#L602

From the logic of the contract, "totalKeeperPoolWeight" will be always non-negative.
Even if this value could be negative, it will be revoked with underflow error before.

  1. Use != 0 instead of > 0 for uint variables
    contracts\BkdLocker.sol#L91
    contracts\BkdLocker.sol#L92
    contracts\BkdLocker.sol#L137
    contracts\BkdLocker.sol#L139
    contracts\BkdLocker.sol#L254
    contracts\BkdLocker.sol#L301
    contracts\tokenomics\AmmGauge.sol#L88
    contracts\tokenomics\AmmGauge.sol#L104
    contracts\tokenomics\AmmGauge.sol#L125
    contracts\tokenomics\AmmGauge.sol#L147
    contracts\tokenomics\FeeBurner.sol#L117
    contracts\tokenomics\KeeperGauge.sol#L140
    contracts\tokenomics\LpGauge.sol#L68
    contracts\tokenomics\LpGauge.sol#L114
    contracts\tokenomics\VestedEscrow.sol#L84
    contracts\RewardHandler.sol#L63

  2. Change storage to memory if possible
    contracts\BkdLocker.sol#L251

  3. An array’s length should be cached to save gas in for-loops
    contracts\StakerVault.sol#L259
    contracts\tokenomics\FeeBurner.sol#L56
    contracts\tokenomics\InflationManager.sol#L116
    contracts\tokenomics\VestedEscrow.sol#L94
    contracts\zaps\PoolMigrationZap.sol#L22
    contracts\zaps\PoolMigrationZap.sol#L39

  4. Usage of unchecked can reduce the gas cost
    contracts\StakerVault.sol#L387

allowances[src][msg.sender] = allowance.uncheckedSub(unstaked);

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Jun 3, 2022
code423n4 added a commit that referenced this issue Jun 3, 2022
@chase-manning chase-manning added sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) labels Jun 6, 2022
@GalloDaSballo
Copy link
Collaborator

Don't change storage variable needlessly

I don't believe the suggestion will work, in lack of POC I'm not giving it points
The suggested change goes out of bounds (last element of array is array.length - 1)

Needless conditions.

Saves 3 gas
3 * 3 = 9

Use != 0 instead of > 0 for uint variables

Only for Requires, listed below

contracts\BkdLocker.sol#L91
contracts\BkdLocker.sol#L92
contracts\BkdLocker.sol#L137
contracts\tokenomics\AmmGauge.sol#L104
contracts\tokenomics\AmmGauge.sol#L125
contracts\tokenomics\KeeperGauge.sol#L140
contracts\tokenomics\VestedEscrow.sol#L84

3 * 7 = 21

Change storage to memory if possible

In lack of POC I can't give it any points

An array’s length should be cached to save gas in for-loops

3 per instance

3 * 6 = 18

Usage of unchecked can reduce the gas cost

Would save 20 gas

Total Gas Saved
68

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working G (Gas Optimization) resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

3 participants