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

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

Gas Optimizations #75

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. Unused contracts and libraries are imported.
    https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/tokenomics/KeeperGauge.sol#L4

Recommendation:
Remove unused libraries.

  1. Checkpoint executed twice when kill the gauge.
    https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/tokenomics/InflationManager.sol#L427-L428

https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/tokenomics/InflationManager.sol#L461-L462

InflationManager call poolCheckPoint(), then call kill() function, but inside of kill() function, poolCheckPoint() will be executed again.

Recommendation:
Remove poolCheckPoint() call before kill.

  1. Cache array length before loop to reduce gas cost
    Cache length of array and use it in the loop is a good solution to reduce gas.
    Other wise, it will always trying to load length from storage in every loop.

https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/tokenomics/InflationManager.sol#L116

https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/access/RoleManager.sol#L82

Recommendation
Cache length of array and use it in loop.

  1. Use !=0 instead of >0 for uint non zero check.
    In most of places, there is something like a > 0 for non-zero check.
    For Uint variables, there is no negative value, so it’s enough to change to a != 0.

Ex.
https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/tokenomics/KeeperGauge.sol#L140

Recommendation:
Use non-equal operator instead of greater operator for non-zero check.

  1. lastEvent in Minter.sol can be update multiple times.
    https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/tokenomics/Minter.sol#L222

_mint function update lastEvent twice – first update in Line 222, and second update inside executeInflationRateUpdate()

Recommendation:
Optimize code to avoid multiple updates.

  1. Use interface contract for external calls.
    https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/tokenomics/Minter.sol#L56

There is a IBkdToken interface, but now it is using BkdToken (implementation contract) for external calls.
This increase contract size.

Recommendation:
Use interface for external calls.

@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

Unused contracts and libraries are imported.

Doesn't save gas

Checkpoint executed twice when kill the gauge.

Finding is valid, but the warden didn't specify how much gas is saved, I'll give it 200 gas, although it probably would have saved more (next time do the math yourself)

Cache array length before loop to reduce gas cost

Saves 3 gas per instance
6 gas

Use !=0 instead of >0 for uint non zero check.

Only in require, with solidity < 0.8.13, with optimizer on
3 gas

_mint function update lastEvent twice – first update in Line 222, and second update inside executeInflationRateUpdate()

Setting storage to same value costs 100 gas as per ArrowGlacier, this would save 100 gas

## Use interface contract for external calls.
I believe the optimizer removes all unused code reducing the excessive code by a lot

Total Gas Saved
309

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