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

Open
code423n4 opened this issue May 25, 2022 · 0 comments
Open

Gas Optimizations #299

code423n4 opened this issue May 25, 2022 · 0 comments
Labels
bug Something isn't working duplicate This issue or pull request already exists G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

Gas Optimizations

Error messages length

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met. Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.
An even better and gas efficient approach will be to use Solidity Custom Errors instead of revert strings.

Aura.sol

Pre-deploy calculation in Aura's constructor

Calculate this calculation before the deployment to save gas:
reductionPerCliff = EMISSIONS_MAX_SUPPLY.div(totalCliffs);

AuraBalRewardPool.sol

Save return value of totalSupply instead of calling it twice in the rewardPerToken function

function rewardPerToken() public view returns (uint256) {
    if (totalSupply() == 0) {
        return rewardPerTokenStored;
    }
    return
        rewardPerTokenStored.add(
            lastTimeRewardApplicable().sub(lastUpdateTime).mul(rewardRate).mul(1e18).div(totalSupply())
        );
}

Save SLOADs

  • save the return value of rewardPerToken instead of accessing the rewardPerTokenStored variable twice in the updateReward modifier

    modifier updateReward(address account) {
        rewardPerTokenStored = rewardPerToken();
        lastUpdateTime = lastTimeRewardApplicable();
        if (account != address(0)) {
            rewards[account] = earned(account);
            userRewardPerTokenPaid[account] = rewardPerTokenStored;
        }
        _;
    }
    
  • cache duration in the initialiseRewards function

    function initialiseRewards() external returns (bool) {
        require(msg.sender == rewardManager || block.timestamp > startTime, "!authorized");
        require(rewardRate == 0, "!one time");
    
        uint256 rewardsAvailable = rewardToken.balanceOf(address(this));
        require(rewardsAvailable > 0, "!balance");
    
        rewardRate = rewardsAvailable.div(duration);
    
        lastUpdateTime = block.timestamp;
        periodFinish = block.timestamp.add(duration);
    
        emit RewardAdded(rewardsAvailable);
    
        return true;
    }

redundant initialization

Variables in solidity are already initialized to their default value, and initializing them to the same value actually costs more gas. So the initialization of storage variables to 0 is redundant.

uint256 public pendingPenalty = 0;
// ...
uint256 public periodFinish = 0;
uint256 public rewardRate = 0;

Make the calculation simpler

Replace (reward * 2) / 10; with reward / 5; in the getReward function

AuraLocker.sol

save SLOADs

  • cache rewardsDuration in this calculation instead of accessing it twice:
    uint256 currentEpoch = block.timestamp.div(rewardsDuration).mul(rewardsDuration);
    You can do this in multiple places in the code, for example _checkpointEpoch, constructor and _lock.

2 optimizations for the updateReward function

  1. Save userBalance.locked to a memory variable. This will save an SLOAD.
  2. Cache rewardData[token]
modifier updateReward(address _account) {
    {
        Balances storage userBalance = balances[_account];
        uint256 rewardTokensLength = rewardTokens.length;
        for (uint256 i = 0; i < rewardTokensLength; i++) {
            address token = rewardTokens[i];
            uint256 newRewardPerToken = _rewardPerToken(token);
            rewardData[token].rewardPerTokenStored = newRewardPerToken.to96();
            rewardData[token].lastUpdateTime = _lastTimeRewardApplicable(rewardData[token].periodFinish).to32();
            if (_account != address(0)) {
                userData[_account][token] = UserData({
                    rewardPerTokenPaid: newRewardPerToken.to128(),
                    rewards: _earned(_account, token, userBalance.locked).to128()
                });
            }
        }
    }
    _;
}

Cache rewardData[_rewardsToken] in the _rewardPerToken function

function _rewardPerToken(address _rewardsToken) internal view returns (uint256) {
    if (lockedSupply == 0) {
        return rewardData[_rewardsToken].rewardPerTokenStored;
    }
    return
        uint256(rewardData[_rewardsToken].rewardPerTokenStored).add(
            _lastTimeRewardApplicable(rewardData[_rewardsToken].periodFinish)
                .sub(rewardData[_rewardsToken].lastUpdateTime)
                .mul(rewardData[_rewardsToken].rewardRate)
                .mul(1e18)
                .div(lockedSupply)
        );
}

Cache rewardTokens in the getReward function

function getReward(address _account, bool _stake) public nonReentrant updateReward(_account) {
    uint256 rewardTokensLength = rewardTokens.length;
    for (uint256 i; i < rewardTokensLength; i++) {
        address _rewardsToken = rewardTokens[i];
        uint256 reward = userData[_account][_rewardsToken].rewards;
        if (reward > 0) {
            userData[_account][_rewardsToken].rewards = 0;
            if (_rewardsToken == cvxCrv && _stake && _account == msg.sender) {
                IRewardStaking(cvxcrvStaking).stakeFor(_account, reward);
            } else {
                IERC20(_rewardsToken).safeTransfer(_account, reward);
            }
            emit RewardPaid(_account, _rewardsToken, reward);
        }
    }
}

Cache rdata in the _notifyReward function

function _notifyReward(address _rewardsToken, uint256 _reward) internal updateReward(address(0)) {
    RewardData storage rdata = rewardData[_rewardsToken];

    if (block.timestamp >= rdata.periodFinish) {
        rdata.rewardRate = _reward.div(rewardsDuration).to96();
    } else {
        uint256 remaining = uint256(rdata.periodFinish).sub(block.timestamp);
        uint256 leftover = remaining.mul(rdata.rewardRate);
        rdata.rewardRate = _reward.add(leftover).div(rewardsDuration).to96();
    }

    rdata.lastUpdateTime = block.timestamp.to32();
    rdata.periodFinish = block.timestamp.add(rewardsDuration).to32();

    emit RewardAdded(_rewardsToken, _reward);
}

Loop optimizations

Loops can be optimized in several ways. Let's take for example the loop in the _processExpiredLocks function.

  1. Use nextUnlockIndex instead of i (you increment both of them anyway and they have the same value)
  2. cache locks[i] instead of accessing the array's element multiple times in every iteration
  3. use prefix increment (++i) instead of postfix increment (i++)
  4. move the index incrementation to inside of the loop
  5. use unchecked on the loop index incrementation

This is the old code:

for (uint256 i = nextUnlockIndex; i < length; i++) {
    //unlock time must be less or equal to time
    if (locks[i].unlockTime > expiryTime) break;

    //add to cumulative amounts
    locked = locked.add(locks[i].amount);

    //check for kick reward
    //each epoch over due increases reward
    if (_checkDelay > 0) {
        uint256 currentEpoch = block.timestamp.sub(_checkDelay).div(rewardsDuration).mul(rewardsDuration);
        uint256 epochsover = currentEpoch.sub(uint256(locks[i].unlockTime)).div(rewardsDuration);
        uint256 rRate = AuraMath.min(kickRewardPerEpoch.mul(epochsover + 1), denominator);
        reward = reward.add(uint256(locks[i].amount).mul(rRate).div(denominator));
    }
    //set next unlock index
    nextUnlockIndex++;
}

And this is the code after the suggested changes:

for ( ; nextUnlockIndex < length; ) {
    LockedBalance memory lock_i = locks[nextUnlockIndex];

    //unlock time must be less or equal to time
    if (lock_i.unlockTime > expiryTime) break;

    //add to cumulative amounts
    locked = locked.add(lock_i.amount);

    //check for kick reward
    //each epoch over due increases reward
    if (_checkDelay > 0) {
        uint256 currentEpoch = block.timestamp.sub(_checkDelay).div(rewardsDuration).mul(rewardsDuration);
        uint256 epochsover = currentEpoch.sub(uint256(lock_i.unlockTime)).div(rewardsDuration);
        uint256 rRate = AuraMath.min(kickRewardPerEpoch.mul(epochsover + 1), denominator);
        reward = reward.add(uint256(lock_i.amount).mul(rRate).div(denominator));
    }
    //set next unlock index
    unchecked {
        ++nextUnlockIndex;
    }
}

Loop optimizations like this can be done to more loops in the code. For example, you can also optimize the loop in the updateReward modifier:

  1. Redundant initialization of the loop index. Variables in solidity are already initialized to their default value, and initializing them to the same value actually costs more gas. So for example in this loop, the code can be optimized using uint i; instead of uint i = 0;.
  2. cache rewardData[token]
  3. use prefix increment (++i) instead of postfix increment (i++)
  4. move the index incrementation to inside of the loop
  5. use unchecked on the loop index incrementation

This is the old code:

for (uint256 i = 0; i < rewardTokensLength; i++) {
    address token = rewardTokens[i];
    uint256 newRewardPerToken = _rewardPerToken(token);
    rewardData[token].rewardPerTokenStored = newRewardPerToken.to96();
    rewardData[token].lastUpdateTime = _lastTimeRewardApplicable(rewardData[token].periodFinish).to32();
    if (_account != address(0)) {
        userData[_account][token] = UserData({
            rewardPerTokenPaid: newRewardPerToken.to128(),
            rewards: _earned(_account, token, userBalance.locked).to128()
        });
    }
}

And this is the code after applying the suggestions:

for (uint256 i; i < rewardTokensLength; ) {
    address token = rewardTokens[i];
    uint256 newRewardPerToken = _rewardPerToken(token);

    // caching reward data to stack (memory)
    RewardData memory data = rewardData[token];
    
    data.rewardPerTokenStored = newRewardPerToken.to96();
    data.lastUpdateTime = _lastTimeRewardApplicable(data.periodFinish).to32();
    if (_account != address(0)) {
        userData[_account][token] = UserData({
            rewardPerTokenPaid: newRewardPerToken.to128(),
            rewards: _earned(_account, token, userBalance.locked).to128()
        });
    }

    // saving the new reward data to storage 
    rewardData[token] = data;

    unchecked {
        ++i;
    }
}

Optimize calculations by removing unnecessary overflow checks

Let's take a look at the _lock function. In there, both the user's balance and the locked supply is being incremented:

//add user balances
uint112 lockAmount = _amount.to112();
bal.locked = bal.locked.add(lockAmount);

//add to total supplies
lockedSupply = lockedSupply.add(_amount);

But we know that bal.locked <= lockedSupply, so we can know that if bal.locked will overflow, lockedSupply will overflow too, so we can use unchecked on bal.locked to save some gas.

Another example is in the _processExpiredLocks, where the locked supply and user's balance are being decremented. Because we know that bal.locked <= lockedSupply, we can use unchecked on the subtraction from the locked supply because if it will underflow the subtraction will underflow too.

//update user balances and total supplies
userBalance.locked = userBalance.locked.sub(locked);
lockedSupply = lockedSupply.sub(locked);

redundant initialization on line 540

Initializing this variable to zero will cost more gas than just declaring the variable (the variable's value will be set to zero by default)
uint256 unlocksSinceLatestCkpt = 0;

AuraVestingEscrow.sol

Use unchecked in the constructor and in _totalVestedOf

In the constructor, we know that endTime > startTime, so totalTime = endTime - startTime; won't underflow.

In the _totalVestedOf we know that _time >= startTime, so uint256 elapsed = _time - startTime; won't underflow.

save an SLOAD

Cache startTime in the _totalVestedOf function

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);
}
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels May 25, 2022
code423n4 added a commit that referenced this issue May 25, 2022
@0xMaharishi 0xMaharishi added the duplicate This issue or pull request already exists label May 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate This issue or pull request already exists G (Gas Optimization)
Projects
None yet
Development

No branches or pull requests

2 participants