Backd Tokenomics contest - robee's results

More efficient Struct packing of Market in the contract ComptrollerStorage.sol

    The following structs could change the order of their stored elements to decrease memory uses.
    and number of occupied slots. Therefore will save gas at every store and load from memory.
    

In ComptrollerStorage.sol, Market is optimized to: 3 slots from: 4 slots. The new order of types (you choose the actual variables): 1. uint256 2. mapping 3. bool 4. bool

Unnecessary equals boolean

Boolean variables can be checked within conditionals directly without the use of equality operators to true/false.

Code instances:

    RoleManager.sol, 137: account == addressProvider.getAddress(AddressProviderKeys._POOL_FACTORY_KEY, false);
    BkdTriHopCvx.sol, 169: if (_lpBalance() == 0) return false;
    Vault.sol, 647: if (address(strategy) == address(0)) return false;
    CvxCrvRewardsLocker.sol, 283: if (IERC20(CVX_CRV).balanceOf(address(this)) == 0) return false;
    Vault.sol, 434: if (address(strategy) == address(0)) return false;

State variables that could be set immutable

In the following files there are state variables that could be set immutable to save gas.

Code instances:

    _decimals in LpToken.sol
    minter in LpToken.sol
    token in StakerVault.sol
    minWithdrawalDelay in VaultReserve.sol

Unused state variables

Unused state variables are gas consuming at deployment (since they are located in storage) and are a bad code practice. Removing those variables will decrease deployment gas cost and improve code quality. This is a full list of all the unused storage variables we found in your code base.

Code instances:

    Errors.sol, INVALID_TOKEN_TO_REMOVE
    AddressProviderKeys.sol, _REWARD_HANDLER_KEY
    Errors.sol, ADDRESS_NOT_ACTION
    Errors.sol, INVALID_INDEX
    InterestRateModel.sol, isInterestRateModel

Unused declared local variables

Unused local variables are gas consuming, since the initial value assignment costs gas. And are a bad code practice. Removing those variables will decrease the gas cost and improve code quality. This is a full list of all the unused storage variables we found in your code base.

Code instances:

    PoolMigrationZap.sol, migrate, ethValue_
    RewardHandler.sol, burnFees, ethBalance
    FeeBurner.sol, _depositInPool, ethBalance_

Caching array length can save gas

Caching the array length is more gas efficient. This is because access to a local variable in solidity is more efficient than query storage / calldata / memory. We recommend to change from:

for (uint256 i=0; i<array.length; i++) { ... }

to:

uint len = array.length  
for (uint256 i=0; i<len; i++) { ... }

Code instances:

    StakerVault.sol, actions, 259
    InflationManager.sol, stakerVaults, 116
    PoolMigrationZap.sol, newPools_, 22
    VestedEscrow.sol, amounts, 94
    PoolMigrationZap.sol, oldPoolAddresses_, 39

Prefix increments are cheaper than postfix increments

Prefix increments are cheaper than postfix increments. Further more, using unchecked {++x} is even more gas efficient, and the gas saving accumulates every iteration and can make a real change There is no risk of overflow caused by increamenting the iteration index in for loops (the ++i in for (uint256 i = 0; i < numIterations; ++i)). But increments perform overflow checks that are not necessary in this case. These functions use not using prefix increments (++x) or not using the unchecked keyword:

Code instance:

    just change to unchecked: PoolMigrationZap.sol, i, 22

Unnecessary default assignment

Unnecessary default assignments, you can just declare and it will save gas and have the same meaning.

Code instances:

    Vault.sol (L#43) : uint256 internal constant _INITIAL_STRATEGIST_FEE = 0.1e18;
    Vault.sol (L#46) : uint256 public constant MAX_PERFORMANCE_FEE = 0.5e18;
    Vault.sol (L#42) : uint256 internal constant _INITIAL_RESERVE_FEE = 0.01e18;
    CvxCrvRewardsLocker.sol (L#43) : int128 private constant _CRV_INDEX = 0;
    Vault.sol (L#44) : uint256 internal constant _INITIAL_PERFORMANCE_FEE = 0; 

Rearrange state variables

You can change the order of the storage variables to decrease memory uses.

Code instances:

In VestedEscrow.sol,rearranging the storage fields can optimize to: 8 slots from: 9 slots. The new order of types (you choose the actual variables): 1. IERC20 2. uint256 3. uint256 4. uint256 5. uint256 6. uint256 7. address 8. bool 9. address

In KeeperGauge.sol,rearranging the storage fields can optimize to: 3 slots from: 4 slots. The new order of types (you choose the actual variables): 1. IController 2. uint256 3. address 4. uint48 5. bool

Use bytes32 instead of string to save gas whenever possible

Use bytes32 instead of string to save gas whenever possible.
String is a dynamic data structure and therefore is more gas consuming then bytes32.

Code instances:

    Errors.sol (L22), string internal constant INVALID_IMPLEMENTATION = "invalid pool implementation for given coin";
    Errors.sol (L59), string internal constant FAILED_MINT = "mint failed";
    Errors.sol (L62), string internal constant NOTHING_TO_CLAIM = "there is no claimable balance";
    Errors.sol (L55), string internal constant UNDERLYING_NOT_SUPPORTED = "underlying token not supported";
    Errors.sol (L91), string internal constant STRATEGY_DOES_NOT_EXIST = "Strategy does not exist";

Short the following require messages

The following require messages are of length more than 32 and we think are short enough to short them into exactly 32 characters such that it will be placed in one slot of memory and the require function will cost less gas. The list:

Code instance:

    Solidity file: Minter.sol, In line 150, Require message length to shorten: 38, The message: Maximum non-inflation amount exceeded.

Use != 0 instead of > 0

Using != 0 is slightly cheaper than > 0. (see https://github.com/code-423n4/2021-12-maple-findings/issues/75 for similar issue)

Code instances:

    AmmConvexGauge.sol, 171: change 'amount > 0' to 'amount != 0'
    StakerVault.sol, 323: change 'balance > 0' to 'balance != 0'
    LiquidityPool.sol, 469: change 'underlyingAmount > 0' to 'underlyingAmount != 0'
    InflationManager.sol, 602: change 'totalAmmTokenWeight > 0' to 'totalAmmTokenWeight != 0'
    Controller.sol, 107: change 'balance > 0' to 'balance != 0'

Unnecessary cast

Code instances:

    IController PoolFactory.sol.constructor - unnecessary casting IController(_controller)
    IController LpGauge.sol.constructor - unnecessary casting IController(_controller)
    address CompoundHandler.sol._repayAnyDebt - unnecessary casting address(ctoken)
    IController LiquidityPool.sol.constructor - unnecessary casting IController(_controller)

Use unchecked to save gas for certain additive calculations that cannot overflow

You can use unchecked in the following calculations since there is no risk to overflow:

Code instances:

    BkdLocker.sol (L#124) - stashedGovTokens[msg.sender].push( WithdrawStash(block.timestamp + currentUInts256[_WITHDRAW_DELAY], amount) );
    Minter.sol (L#188) - totalAvailableToNow += (currentTotalInflation * (block.timestamp - lastEvent));
    AmmGauge.sol (L#89) - ammStakedIntegral_ += (controller.inflationManager().getAmmRateForToken(ammToken) * (block.timestamp - uint256(ammLastUpdated))).scaledDiv(totalStaked);
    BkdLocker.sol (L#276) - newBoost += (block.timestamp - lastUpdated[user]) .scaledDiv(currentUInts256[_INCREASE_PERIOD]) .scaledMul(maxBoost - startBoost);
    LpGauge.sol (L#69) - poolStakedIntegral_ += (inflationManager.getLpRateForStakerVault(address(stakerVault)) * (block.timestamp - poolLastUpdate)).scaledDiv(poolTotalStaked);

Consider inline the following functions to save gas

You can inline the following functions instead of writing a specific function to save gas.
(see https://github.com/code-423n4/2021-11-nested-findings/issues/167 for a similar issue.)

Code instances

    ScaledMath.sol, scaledDiv, { return (a * DECIMAL_SCALE) / b; }
    ExponentialNoError.sol, lessThanOrEqualExp, { return left.mantissa <= right.mantissa; }
    Preparable.sol, _prepare, { return _prepare(key, value, _MIN_DELAY); }
    AddressProviderHelpers.sol, getSafeRewardHandler, { return provider.getAddress(AddressProviderKeys._REWARD_HANDLER_KEY, false); }
    TopUpKeeperHelper.sol, _positionToTopup, { return TopupData(user, position.account, position.protocol, position.record); }

Inline one time use functions

The following functions are used exactly once. Therefore you can inline them and save gas and improve code clearness.

Code instances:

    ExponentialNoError.sol, sub_
    BkdTriHopCvx.sol, _minLpAccepted
    FeeBurner.sol, _swapperRouter
    LiquidityPool.sol, _doTransferIn
    CompoundHandler.sol, _getAccountBorrowsAndSupply

Cache powers of 10 used several times

You calculate the power of 10 every time you use it instead of caching it once as a constant variable and using it instead. Fix the following code lines:

Code instances:

DecimalScale.sol, 22 : You should cache the used power of 10 as constant state variable since it's used several times (4): return value / 10**(_DECIMALS - decimals);

DecimalScale.sol, 12 : You should cache the used power of 10 as constant state variable since it's used several times (4): return value * 10**(_DECIMALS - decimals);

DecimalScale.sol, 20 : You should cache the used power of 10 as constant state variable since it's used several times (4): return value * 10**(decimals - _DECIMALS);

DecimalScale.sol, 10 : You should cache the used power of 10 as constant state variable since it's used several times (4): return value / 10**(decimals - _DECIMALS);

Change if -> revert pattern to require

Change if -> revert pattern to 'require' to save gas and improve code quality, if (some_condition) { revert(revert_message) }

to: require(!some_condition, revert_message)

In the following locations:

Code instance:

    CTokenRegistry.sol, 62

Do not cache msg.sender

We recommend not to cache msg.sender since calling it is 2 gas while reading a variable is more.

Code instance:

    https://github.com/code-423n4/2022-05-backd/tree/main/protocol/contracts/tokenomics/VestedEscrow.sol#L63