Gas Optimizations

[code423n4]

FINDINGS

Comparisons: != is more efficient than > in require

!= 0 costs less gas compared to > 0 for unsigned integers in require statements with the optimizer enabled (6 gas)

Proof: While it may seem that > 0 is cheaper than !=, this is only true without the optimizer enabled and outside a require statement. If you enable the optimizer at 10k AND you're in a require statement, this will save gas. You can see this tweet for more proofs:

I suggest changing > 0 with != 0 here:

File: BkdLocker.sol line 91

require(amount > 0, Error.INVALID_AMOUNT);

Since amount is a uint256, it means it's value can never be less than 0 so the test > 0 is essentially testing that amount is not equal to 0 a

Other Instances File: BkdLocker.sol line 92

require(totalLockedBoosted > 0, Error.NOT_ENOUGH_FUNDS);

File: BkdLocker.sol line 137

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

File: VestedEscrow.sol line 84

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

File: AmmGauge.sol line 104

require(amount > 0, Error.INVALID_AMOUNT);

File: AmmGauge.sol line 125

require(amount > 0, Error.INVALID_AMOUNT);

use shorter revert strings(less than 32 bytes) or use custom errors

File: Minter.sol line 150

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

Cache the length of arrays in loops

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack. Caching the array length in the stack saves around 3 gas per iteration. Therefore, it’s possible to save a significant amount of gas especially when the length is significantly big.

Here, I suggest storing the array’s length in a variable before the for-loop, and use it instead:

File:StakerVault.sol line 259

function getStakedByActions() external view override returns (uint256) {
        address[] memory actions = addressProvider.allActions();
        uint256 total;
        for (uint256 i; i < actions.length; i = i.uncheckedInc()) {
            total += balances[actions[i]];
        }
        return total;
    }

The line to modify:

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

Other Instance to change File: RewardHandler.sol line 35-55

        IBkdLocker bkdLocker = IBkdLocker(addressProvider.getBKDLocker());
        IFeeBurner feeBurner = addressProvider.getFeeBurner();
        address targetLpToken = bkdLocker.rewardToken();
        address[] memory pools = addressProvider.allPools();
        uint256 ethBalance = address(this).balance;
        address[] memory tokens = new address[](pools.length);
        for (uint256 i; i < pools.length; i = i.uncheckedInc()) {
            ILiquidityPool pool = ILiquidityPool(pools[i]);
            address underlying = pool.getUnderlying();
            if (underlying != address(0)) {
                _approve(underlying, address(feeBurner));
            }
            tokens[i] = underlying;
        }
        feeBurner.burnToTarget{value: ethBalance}(tokens, targetLpToken);
        uint256 burnedAmount = IERC20(targetLpToken).balanceOf(address(this));
        IERC20(targetLpToken).safeApprove(address(bkdLocker), burnedAmount);
        bkdLocker.depositFees(burnedAmount);
        emit Burned(targetLpToken, burnedAmount);
    }

The specific line to change here

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

File: PoolMigrationZap.sol line 22

for (uint256 i; i < newPools_.length; ++i) {

Something similar to my propasal was implemented on this contract as shown in the function below. File:Controller.sol line 121-130

        // solhint-disable-previous-line ordering
        uint256 totalEthRequired;
        address[] memory actions = addressProvider.allActions();
        uint256 numActions = actions.length;
        for (uint256 i; i < numActions; i = i.uncheckedInc()) {
            totalEthRequired += IAction(actions[i]).getEthRequiredForGas(payer);
        }
        return totalEthRequired;
    }

Note the array length was cached to numActions

Also Implemented on here: File:InflationManager.sol line 96-100

        uint256 length = liquidityPools.length;
        for (uint256 i; i < length; i = i.uncheckedInc()) {
            _removeKeeperGauge(address(liquidityPools[i]));
        }

Using unchecked blocks to save gas

Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn't possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked block: see official docs

File: VestedEscrow.sol line 63

totalTime = endtime_ - starttime_;

The above line cannot overflow due to the check on the line shown below File: VestedEscrow.sol line 58

require(endtime_ > starttime_, "end must be greater");

In this contract we have a library that can aid in having unchecked arithmetics that is libraries/UncheckedMath.sol imported on line 21

With the help of this library we can modify our arithmetic to

totalTime = endtime_.uncheckedSub(starttime_);

A similar approach was implemented in the file below. File: StakerVault.sol lines 229-230

if (actionLockedBalances[account] > amount) {
            actionLockedBalances[account] = actionLockedBalances[account].uncheckedSub(amount);

Other Instances to modify File: VestedEscrow.sol line 155

uint256 elapsed = _time - startTime;

The above cannot underflow due to the check on line 152 which ensures that time is greater than startTime

if (_time < startTime) {
            return 0;
        }
        uint256 elapsed = _time - startTime;
        return Math.min((locked * elapsed) / totalTime, locked);

This uint256 elapsed = _time - startTime; should be changed to uint256 elapsed = _time.uncheckedSub(startTime);

File: StakerVault.sol line 124

balances[msg.sender] -= amount;

The above line cannot underflow due the check on line 113

require(balances[msg.sender] >= amount, Error.INSUFFICIENT_BALANCE);

The checks ensures that balances[msg.sender] is greater or equal to amount therefore balances[msg.sender]-amount will never underflow.

[GalloDaSballo]

Comparisons: != is more efficient than > in require

3 Per instance 6 * 3 = 18

use shorter revert strings(less than 32 bytes) or use custom errors

6 gas

Cache the length of arrays in loops

3 per instance

3 * 3 = 9

Using unchecked blocks to save gas

Saves 20 gas per instance 4 * 20 = 80

Total Gas Saved 113