Gas Optimizations

[code423n4]

Findings

Remove safeMath

All the contracts in the main folder here are using the compiler version 0.8.11.

The file AuraBalRewardPool.sol uses safeMath

Safemath is no longer needed starting with solidity 0.8 since the compiler now has built in overflow checks when performing arithmetic functions and revert in case we have an underflow/overflow.

Safemath is a library that is used to prevent overflow/underflow and since these are now checked by default in the compiler the Safemath implementation can be deleted from the code which will provide gas optimization.

similar issue was found here similar issue

POC
navigate here link and see the pragma used is 0.8.11 and safeMath is used

Recommendation
For the gas optimization delete safeMath Implementaion.
Since solidity 0.8, the overflow/underflow check is implemented on the language level - it adds the validation to the bytecode during compilation.

Comparisons

> 0 is less efficient than != 0 for unsigned integers

!= 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: https://twitter.com/gzeon/status/1485428085885640706

I suggest changing > 0 with != 0 here:

AuraBalRewardPool.sol

Other instances to change:

AuraBalRewardPool.sol#L157 require(amount > 0, "RewardPool : Cannot withdraw 0");

AuraBalRewardPool.sol#L139 require(_amount > 0, "RewardPool : Cannot stake 0");

AuraBalRewardPool.sol#L121 require(_amount > 0, "RewardPool : Cannot stake 0");

AuraBalRewardPool.sol#L121 require(_rewards > 0, "No reward");

AuraBalRewardPool.sol#L121 require(_amount > 0, "Cannot stake 0");

AuraBalRewardPool.sol#L178 see here

function getReward(bool _lock) public updateReward(msg.sender) returns (bool) {
        uint256 reward = rewards[msg.sender];
        if (reward > 0) {
            rewards[msg.sender] = 0;

In the above function, since reward is declared as a uint256 , it means the value of reward will never be negative. We can therefore replace > with != which would help save some gas.

modified function

function getReward(bool _lock) public updateReward(msg.sender) returns (bool) {
        uint256 reward = rewards[msg.sender];
        if (reward != 0) {
            rewards[msg.sender] = 0;

This would help save around 3 gas.

Other places to modify:

Booster.sol line (593) link

VoterProxy.sol line (175) link

CrvDepositor.sol line(113) link

AuraClaimZap.sol line (196) link

AuraStakingProxy.sol line (177) link

AuraStakingProxy.sol line (185) link

AuraStakingProxy.sol line (207) link

Splitting require() statements that use && saves around 8 gas

File: Booster.sol line (220) link

require(lockRewards != address(0) && rewardFactory != address(0), "!initialised");

The above should be split like below.

require(lockRewards != address(0),"!initialised");
require(rewardFactory != address(0),"!initialised");

Instead of using the && operator in a single require statement to check multiple conditions,using multiple require statements with 1 condition per require statement will save 8 GAS per &&:

Other instances to modify:

File: Booster.sol line(222) link

 require(_feeToken != address(0) && _feeDistro != address(0), "!addresses");

File: Booster.sol line(314) link

require(_gauge != address(0) && _lptoken != address(0),"!param");

File: AuraStakingProxy.sol line (90) link

require(_outputBps > 9000 && _outputBps < 10000, "Invalid output bps");

File: AuraStakingProxy.sol line (159) link

require(_token != crv && _token != cvx && _token != cvxCrv, "not allowed");

File: AuraStakingProxy.sol line (203) link

require(address(_token) != crv && address(_token) != cvxCrv, "not allowed");

File: Booster.sol line (222) link

require(_feeToken != address(0) && _feeDistro != address(0), "!addresses");

File: AuraStakingProxy.sol line (90) link

require(_outputBps > 9000 && _outputBps < 10000, "Invalid output bps");

File: AuraStakingProxy.sol line (159) link

require(_token != crv && _token != cvx && _token != cvxCrv, "not allowed");

Unused named returns

File: AuraLocker.sol line (769) link

	    // Address and claimable amount of all reward tokens for the given account
    function claimableRewards(address _account) external view returns (EarnedData[] memory userRewards) {
        userRewards = new EarnedData[](rewardTokens.length);
        Balances storage userBalance = balances[_account];
        uint256 userRewardsLength = userRewards.length;
        for (uint256 i = 0; i < userRewardsLength; i++) {
            address token = rewardTokens[i];
            userRewards[i].token = token;
            userRewards[i].amount = _earned(_account, token, userBalance.locked);
        }
        return userRewards;
    }

Using both named returns and a return statement isn’t necessary in the above function.To save gas and improve code quality: consider using only one of those.

File: AuraLocker.sol line (738)link

  // Get an epoch index based on timestamp
    function findEpochId(uint256 _time) public view returns (uint256 epoch) {
        return _time.sub(epochs[0].date).div(rewardsDuration);
    }

Removing unused named returns variables can reduce gas usage (MSTOREs/MLOADs) and improve code clarity.

Use bytes32 instead of string

The string type is basically equal to bytes so we can use bytes32 as an alternative to string, as it is cheaper to store data in a bytes32 vis-à-vis string as long as the string length is less than 32.

In the following function, the string being returned is less than 32 bytes so we can save some gas by replacing string with bytes32.

The string being returned by this function is a fixed string with less than 32 bytes so we should be safe using a bytes32 data type.

File: AuraClaimZap.sol line (85) link

function getName() external pure returns (string memory) {
        return "ClaimZap V2.0";
    }

save gas by using

    function getName() external pure returns (bytes32) {
        return bytes32("ClaimZap V2.0");
    }

File: VoterProxy.sol file(L69) link

function getName() external pure returns (string memory) {
        return "BalancerVoterProxy";
    }

Modiffied version:

function getName() external pure returns (bytes32) {
        return bytes32("BalancerVoterProxy");
    }

The above would save us approximately 302 gas.

Initialization

No need to initialize variables with default values since every variable assignment in solidity costs gas.
If a variable is not set/initialized, it is assumed to have the default value of that particular data type . If you explicitly initialize it with its default value, you end up wasting gas.

File:VoterProxy.sol line (308) link

 uint256 _balance = 0;

replace above with:

uint256 _balance;

File: AuraVestedEscrow.sol line (99) link

 uint256 totalAmount = 0;

replace above with:

uint256 totalAmount;

File: AuraLocker.sol line (773) link

 for (uint256 i = 0; i < userRewardsLength; i++) {
            address token = rewardTokens[i];
            userRewards[i].token = token;
            userRewards[i].amount = _earned(_account, token, userBalance.locked);
        }

The above should be replaced with:

 for (uint256 i; i < userRewardsLength; i++) {
            address token = rewardTokens[i];
            userRewards[i].token = token;
            userRewards[i].amount = _earned(_account, token, userBalance.locked);
        }

Other Instances:

File: AuraClaimZap.sol line (143) link
File: AuraClaimZap.sol line (147) link

File: AuraClaimZap.sol line (151) link

File: AuraLocker.sol line(174) link

Use Unchecked for values that can't underflow to save gas

File: AuraVestedEscrow.sol line (157)link

    function _totalVestedOf(address _recipient, uint256 _time) internal view returns (uint256 total) {
        if (_time < startTime) {
            return 0;
        }
        uint256 locked = totalLocked[_recipient];
        uint256 elapsed = _time - startTime;
        total = AuraMath.min((locked * elapsed) / totalTime, locked);
    }

Since we've checked that time is not less than startTime see then we cannot get an underflow at uint256 elapsed = _time - startTime; so use unchecked as shown below.

function _totalVestedOf(address _recipient, uint256 _time) internal view returns (uint256 total) {
        if (_time < startTime) {
            return 0;
        }
        uint256 locked = totalLocked[_recipient];
				
        uint256 elapsed;
	 unchecked { elapsed =  _time - startTime;}
        total = AuraMath.min((locked * elapsed) / totalTime, locked);
    }

We save around 182 gas for the above.

File: AuraLocker.sol line( 484) link
The line uint256 i = len - 1; cannot underflow since we have checks that ensure that len is greater than 0. see the snippet below.

	470: uint256 len = locks.length;
  471:    require(len > 0, "Nothing to delegate");
	

We are checking the length of len is greater than 0 so it cannot /underflow overflow here and therefore the line uint256 i = len - 1; should be wrapped in in an unchecked block.

The increment in for loop post condition can be made unchecked

**Since the compiler version is 0.8.11 the default checks for overflow/underflow are implemented.

File: AuraLocker.sol line (773) link

for (uint256 i = 0; i < userRewardsLength; i++) {
            address token = rewardTokens[i];
            userRewards[i].token = token;
            userRewards[i].amount = _earned(_account, token, userBalance.locked);
        }

The for loop post condition, i.e., i++ involves checked arithmetic, which is not required. This is because the value of i is always strictly less than userRewardsLength <= 2**256 - 1. Therefore, the theoretical maximum value of i to enter the for-loop body is 2**256 - 2. This means that the i++ in the for loop can never overflow. Regardless, the overflow checks are performed by the compiler.

Unfortunately, the Solidity optimizer is not smart enough to detect this and remove the checks.

We can use the unchecked block as follows.

    // Address and claimable amount of all reward tokens for the given account
    function claimableRewards(address _account) external view returns (EarnedData[] memory userRewards) {
        userRewards = new EarnedData[](rewardTokens.length);
        Balances storage userBalance = balances[_account];
        uint256 userRewardsLength = userRewards.length;
        for (uint256 i = 0; i < userRewardsLength; i = unchecked_inc(i) {
            address token = rewardTokens[i];
            userRewards[i].token = token;
            userRewards[i].amount = _earned(_account, token, userBalance.locked);
        }
        return userRewards;
    }


function unchecked_inc(uint256 i) public returns (uint256){
   unchecked {
      return i + 1;
   }
}

We can do the above in the following instances to:

File: AuraClaimZap.sol line (143) link
File: AuraClaimZap.sol line (147) link

File: AuraClaimZap.sol line (151) link

This can save 30-40 gas per loop iteration.

Use short reason/revert strings

Error reasons string are attached to require statement to make it easier to understand why a contract call reverted. These strings, take space in the deployed bytecode . Every reason string will take atleast 32 bytes so we should ensure our string fits in 32 bytes or we may end up spending too much gas on these reasons.

File:AuraLocker.sol line(197) link

 require(_rewardsToken != address(stakingToken), "Cannot add StakingToken as reward");

The above require string contains 33 bytes which will cost us more gas during deployment and when the revert condition is met.

Consider using a shorter string(32 bytes).

An alternative to this would also be to use custom errors as they have a cheaper deployment and runtime cost when the revert condition is achieved.

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

Reference to use of custom errors: official docs

blog on custom errors