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

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

Gas Optimizations #125

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

Issue: Require message is too long
Explanation: The require revert strings referenced below can be shortened to 32 characters or fewer (as shown) to save gas

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/AddressProvider.sol#L296

        require(!_stakerVaults.contains(token), Error.STAKER_VAULT_EXISTS);

Change the referenced error message from a staker vault already exists for the token to token already has staker vault

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/BkdLocker.sol#L59

        require(currentUInts256[_START_BOOST] == 0, Error.CONTRACT_INITIALIZED);

Change the referenced error message from contract can only be initialized once to contract already initialized

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

        require(_minter != address(0), Error.INVALID_MINTER);

Change the referenced error message from the minter address of the LP token and the pool address do not match to minter and pool address mismatch

The same error message occurs in the lines referenced below:

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/Minter.sol#L72

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/Minter.sol#L73

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/Minter.sol#L74

Example:

        require(_annualInflationDecayAmm < ScaledMath.ONE, Error.INVALID_PARAMETER_VALUE);

Change the referenced error message from invalid parameter value attempted to invalid parameter value attempt

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/Minter.sol#L150-L153

        require(
            issuedNonInflationSupply + amount <= nonInflationDistribution,
            "Maximum non-inflation amount exceeded."
        );

Change error message to Max non-inflation amt exceeded

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/utils/Preparable.sol#L110

        require(block.timestamp >= deadline, Error.DEADLINE_NOT_REACHED);

Change the referenced error message from deadline has not been reached yet to deadline not yet reached

Issue: Should use != 0 instead of > 0 in a require statement if variable is an unsigned integer (uint)

Explanation: != 0 should be used where possible since > 0 costs more gas

The same require occurs in all three lines below:

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/BkdLocker.sol#L91

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/AmmGauge.sol#L104

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/AmmGauge.sol#L125

        require(amount > 0, Error.INVALID_AMOUNT);

Change amount > 0 to amount != 0 in each case

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/BkdLocker.sol#L92

        require(totalLockedBoosted > 0, Error.NOT_ENOUGH_FUNDS);

Change totalLockedBoosted > 0 to totalLockedBoosted != 0

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/BkdLocker.sol#L137

        require(length > 0, "No entries");

Change length > 0 to length != 0

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

        require(totalClaimable > 0, Error.ZERO_TRANSFER_NOT_ALLOWED);

Change totalClaimable > 0 to totalClaimable != 0

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/VestedEscrow.sol#L84

        require(unallocatedSupply > 0, "No reward tokens in contract");

Change unallocatedSupply > 0 to unallocatedSupply != 0

Issue: Variables should not be initialized to their default values

Explanation: For example, initialization of booleans to their default value of false is unnecessary and costs gas

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

        bool keeperGaugeExists = false;

Change to bool keeperGaugeExists;

Issue: Array length should not be looked up in every iteration of a for loop

Explanation: Calculating the array length costs gas

Recommendation: Read the length of the array from memory before executing the loop

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

        for (uint256 i; i < roles.length; i = i.uncheckedInc()) {

Recommendation:

        uint256 totalRolesLength = roles.length; 
        for (uint256 i; i < totalRolesLength; i = i.uncheckedInc()) {

Similarly for the seven for loops referenced below:

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/RewardHandler.sol#L42

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/StakerVault.sol#L259

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/FeeBurner.sol#L56

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

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/VestedEscrow.sol#L94

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/zaps/PoolMigrationZap.sol#L22

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/zaps/PoolMigrationZap.sol#L39

@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

Issue: Require message is too long

6 * 6 = 36

Issue: Should use != 0 instead of > 0 in a require statement if variable is an unsigned integer (uint)

Saves 3 gas per instance
7 * 3 = 21

Issue: Variables should not be initialized to their default values

3

Issue: Array length should not be looked up in every iteration of a for loop

3 per instance
8 * 3 = 24

Total Gas Saved
84

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