Gas Optimizations

[code423n4]

Gas Optimizations Report

For-Loops: Cache array length outside of loops

Reading an 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.

For example:

for (uint256 i; i < arr.length; ++i) {}

can be changed to:

uint256 len = arr.length;
for (uint256 i; i < len; ++i) {}

Consider making the following change to these lines:

protocol/contracts/RewardHandler.sol:
  42:        for (uint256 i; i < pools.length; i = i.uncheckedInc()) {

protocol/contracts/StakerVault.sol:
 259:        for (uint256 i; i < actions.length; i = i.uncheckedInc()) {

protocol/contracts/tokenomics/VestedEscrow.sol:
  94:        for (uint256 i; i < amounts.length; i = i.uncheckedInc()) {

protocol/contracts/tokenomics/InflationManager.sol:
 116:        for (uint256 i; i < stakerVaults.length; i = i.uncheckedInc()) {

protocol/contracts/tokenomics/FeeBurner.sol:
  56:        for (uint256 i; i < tokens_.length; i = i.uncheckedInc()) {

protocol/contracts/zaps/PoolMigrationZap.sol:
  22:        for (uint256 i; i < newPools_.length; ++i) {
  39:        for (uint256 i; i < oldPoolAddresses_.length; ) {

protocol/contracts/access/RoleManager.sol:
  82:        for (uint256 i; i < roles.length; i = i.uncheckedInc()) {

For-Loops: Index increments can be left unchecked

From Solidity v0.8 onwards, all arithmetic operations come with implicit overflow and underflow checks.

In for-loops, as it is impossible for the index to overflow, it can be left unchecked to save gas every iteration.

For example, the code below:

for (uint256 i; i < numIterations; ++i) {  
    // ...  
}  

can be changed to:

for (uint256 i; i < numIterations;) {  
    // ...  
    unchecked { ++i; }  
}  

Consider making the following change to these lines:

protocol/contracts/zaps/PoolMigrationZap.sol:
  22:        for (uint256 i; i < newPools_.length; ++i) {

Arithmetics: ++i costs less gas compared to i++ or i += 1

++i costs less gas compared to i++ or i += 1 for unsigned integers, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.

i++ increments i and returns the initial value of i. Which means:

uint i = 1;  
i++; // == 1 but i == 2  

But ++i returns the actual incremented value:

uint i = 1;  
++i; // == 2 and i == 2 too, so no need for a temporary variable  

In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2, thus it costs more gas.

The same logic applies for --i and i--.

Consider using ++i instead of i++ or i += 1 in the following instances:

protocol/contracts/tokenomics/KeeperGauge.sol:
  59:        epoch++;
  98:        epoch++;

Arithmetics: Use != 0 instead of > 0 for unsigned integers

uint will never go below 0. Thus, > 0 is gas inefficient in comparisons as checking if != 0 is sufficient and costs less gas.

Consider changing > 0 to != 0 in these lines:

protocol/contracts/BkdLocker.sol:
  91:        require(amount > 0, Error.INVALID_AMOUNT);
  92:        require(totalLockedBoosted > 0, Error.NOT_ENOUGH_FUNDS);
 137:        require(length > 0, "No entries");
 139:        while (i > 0) {
 254:        if (userBalance > 0) {
 301:        if (userBalance > 0) {

protocol/contracts/RewardHandler.sol:
  63:        if (IERC20(token).allowance(address(this), spender) > 0) return;

protocol/contracts/tokenomics/AmmGauge.sol:
  88:        if (!killed && totalStaked > 0) {
 104:        require(amount > 0, Error.INVALID_AMOUNT);
 125:        require(amount > 0, Error.INVALID_AMOUNT);
 147:        if (totalStaked > 0) {

protocol/contracts/tokenomics/AmmConvexGauge.sol:
 107:        if (!killed && totalStaked > 0) {
 129:        if (!killed && totalStaked > 0) {
 158:        require(amount > 0, Error.INVALID_AMOUNT);
 171:        require(amount > 0, Error.INVALID_AMOUNT);
 197:        if (totalStaked > 0) {

protocol/contracts/tokenomics/VestedEscrow.sol:
  84:        require(unallocatedSupply > 0, "No reward tokens in contract");

protocol/contracts/tokenomics/InflationManager.sol:
 575:        totalKeeperPoolWeight = totalKeeperPoolWeight > 0 ? totalKeeperPoolWeight : 0;
 589:        totalLpPoolWeight = totalLpPoolWeight > 0 ? totalLpPoolWeight : 0;
 602:        totalAmmTokenWeight = totalAmmTokenWeight > 0 ? totalAmmTokenWeight : 0;

protocol/contracts/tokenomics/LpGauge.sol:
  68:        if (poolTotalStaked > 0) {
 114:        if (poolTotalStaked > 0) {

protocol/contracts/tokenomics/FeeBurner.sol:
 117:        if (IERC20(token_).allowance(address(this), spender_) > 0) return;

protocol/contracts/tokenomics/KeeperGauge.sol:
 140:        require(totalClaimable > 0, Error.ZERO_TRANSFER_NOT_ALLOWED);

Errors: Use custom errors instead of revert strings

Since Solidity v0.8.4, custom errors should be used instead of revert strings due to:

• Cheaper deployment cost
• Lower runtime cost upon revert

Taken from Custom Errors in Solidity:

Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them.

Custom errors can be defined using of the error statement, both inside or outside of contracts.

Instances where custom errors can be used instead:

protocol/contracts/BkdLocker.sol:
 137:        require(length > 0, "No entries");

protocol/contracts/tokenomics/Minter.sol:
 100:        require(address(token) == address(0), "Token already set!");
 105:        require(lastEvent == 0, "Inflation has already started.");
 220:        require(newTotalMintedToNow <= totalAvailableToNow, "Mintable amount exceeded");

protocol/contracts/tokenomics/VestedEscrow.sol:
  57:        require(starttime_ >= block.timestamp, "start must be future");
  58:        require(endtime_ > starttime_, "end must be greater");
  82:        require(!initializedSupply, "Supply already initialized once");
  84:        require(unallocatedSupply > 0, "No reward tokens in contract");
  91:        require(initializedSupply, "Supply must be initialized");

protocol/contracts/tokenomics/InflationManager.sol:
  95:        require(!weightBasedKeeperDistributionDeactivated, "Weight-based dist. deactivated.");
 265:        require(length == weights.length, "Invalid length of arguments");
 315:        require(_ammGauges.contains(token), "amm gauge not found");
 365:        require(length == weights.length, "Invalid length of arguments");
 367:        require(_ammGauges.contains(tokens[i]), "amm gauge not found");

protocol/contracts/tokenomics/VestedEscrowRevocable.sol:
  53:        require(revokedTime[_recipient] == 0, "Recipient already revoked");
  54:        require(_recipient != treasury, "Treasury cannot be revoked!");

protocol/contracts/tokenomics/FeeBurner.sol:
  49:        require(tokens_.length != 0, "No tokens to burn");

protocol/contracts/zaps/PoolMigrationZap.sol:
  56:        require(lpTokenAmount_ != 0, "No LP Tokens");
  57:        require(oldPool_.getWithdrawalFee(msg.sender, lpTokenAmount_) == 0, "withdrawal fee not 0");

Unnecessary initialization of variables with default values

Uninitialized variables are assigned with a default value depending on its type:

• uint: 0
• bool: false
• address: address(0)

Thus, explicitly initializing a variable with its default value costs unnecesary gas. For example, the following code:

bool b = false;
address c = address(0);
uint256 a = 0;

can be changed to:

uint256 a;
bool b;
address c;

Consider declaring the following lines without explicitly setting a value:

protocol/contracts/tokenomics/InflationManager.sol:
 412:        bool keeperGaugeExists = false;

Unnecessary definition of variables

Some variables are defined even though they are only used once in their respective functions. Not defining these variables can help to reduce gas cost and contract size.

Instances include:

protocol/contracts/BkdLocker.sol:
 150:        uint256 newTotal = balances[msg.sender] - totalAvailableToWithdraw;

protocol/contracts/RewardHandler.sol:
  40:        uint256 ethBalance = address(this).balance;

protocol/contracts/tokenomics/AmmConvexGauge.sol:
 153:        uint256[3] memory allRewards = [bkdRewards, crvRewards, cvxRewards];

protocol/contracts/tokenomics/VestedEscrow.sol:
 155:        uint256 elapsed = _time - startTime;

protocol/contracts/tokenomics/KeeperGauge.sol:
 112:        uint256 timeElapsed = block.timestamp - uint256(lastUpdated);

Storage variables should be declared immutable when possible

If a storage variable is assigned only in the constructor, it should be declared as immutable. This would help to reduce gas costs as calls to immutable variables are much cheaper than regular state variables, as seen from the Solidity Docs:

Compared to regular state variables, the gas costs of constant and immutable variables are much lower. Immutable variables are evaluated once at construction time and their value is copied to all the places in the code where they are accessed.

Consider declaring these variables as immutable:

protocol/contracts/StakerVault.sol:
  48:        address public token;

protocol/contracts/tokenomics/VestedEscrow.sol:
  39:        uint256 public totalTime;

Variables declared as constant are expressions, not constants

Due to how constant variables are implemented (replacements at compile-time), an expression assigned to a constant variable is recomputed each time that the variable is used, which wastes some gas.

If the variable was immutable instead: the calculation would only be done once at deploy time (in the constructor), and then the result would be saved and read directly at runtime rather than being recalculated.

See: ethereum/solidity#9232:

Consequences: each usage of a “constant” costs ~100 gas more on each access (it is still a little better than storing the result in storage, but not much). since these are not real constants, they can’t be referenced from a real constant environment (e.g. from assembly, or from another library)

protocol/contracts/tokenomics/FeeBurner.sol:
  25:        address private constant _WETH = address(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);

protocol/contracts/utils/CvxMintAmount.sol:
  10:        uint256 private constant _CLIFF_SIZE = 100000 * 1e18;
  12:        uint256 private constant _MAX_SUPPLY = 100000000 * 1e18;

  13:        IERC20 private constant _CVX_TOKEN =
  14:                IERC20(address(0x4e3FBD56CD56c3e72c1403e103b45Db9da5B9D2B));

Change these expressions from constant to immutable and implement the calculation in the constructor. Alternatively, hardcode these values in the constants and add a comment to say how the value was calculated.

[GalloDaSballo]

For-Loops: Cache array length outside of loops

3 * 8 = 24

For-Loops: Index increments can be left unchecked

Would save 20 gas

Arithmetics: ++i costs less gas compared to i++ or i += 1

Saves 5 gas per 2 = 10

Arithmetics: Use != 0 instead of > 0 for unsigned integers

Only for requires, only if solidity < 0.8.13 with optimizer on, saves 3 gas

9 * 3 = 27

Errors: Use custom errors instead of revert strings

Cannot quantify unless you sent POC with before and after

Unnecessary initialization of variables with default values

Saves 3 gas

Unnecessary definition of variables

Saves 6 gas (MSTORE = 3 + MLOAD = 3) per instance 5 * 6 = 30

Storage variables should be declared immutable when possible

Agree, will save 2.1k per first SLOAD, in lack of more details I'll give it 1 SLOAD each 2100 * 2 = 4200

Variables declared as constant are expressions, not constants

No longer true

Total Gas Saved 4314