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

Open
code423n4 opened this issue Apr 27, 2022 · 0 comments
Open

Gas Optimizations #70

code423n4 opened this issue Apr 27, 2022 · 0 comments
Labels
bug Something isn't working G (Gas Optimization) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Gas optimizations

  • Use delete in the claimRewards function of the FlywheelCore contract and in the _decrementVotesUntilFree function of the ERC20MultiVotes contract in order to trigger a gas refund

  • Loops can be optimized in several ways. Let's take for example the loop in the incrementGauges function in ERC20Gauges (There is also a loop in the decrementGauges function which is similar).

    // Update gauge specific state
    for (uint256 i = 0; i < size; ) {
        address gauge = gaugeList[i];
        uint112 weight = weights[i];
        weightsSum += weight;
    
        _incrementGaugeWeight(msg.sender, gauge, weight, currentCycle);
        unchecked {
            i++;
        }
    }

    We can do multiple things here:

    1. 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 the loop above, the code can be optimized using uint i; instead of uint i = 0;.
    2. Use ++i instead of i++ to save some gas spent in every iteration.

    Let's look at another example:

    uint256 size = gaugeList.length;
    for (uint256 i = 0; i < size && (userFreeWeight + totalFreed) < weight; ) {
        address gauge = gaugeList[i];
        uint112 userGaugeWeight = getUserGaugeWeight[user][gauge];
        if (userGaugeWeight != 0) {
            // If the gauge is live (not deprecated), include its weight in the total to remove
            if (!_deprecatedGauges.contains(gauge)) {
                totalFreed += userGaugeWeight;
            }
            userFreed += userGaugeWeight;
            _decrementGaugeWeight(user, gauge, userGaugeWeight, currentCycle);
    
            unchecked {
                i++;
            }
        }
    }

    First of all, the optimizations from the loop before can be done here too. In addition, there is another optimization that can be done here - the condition of the loop contains 2 conditions - i < size && (userFreeWeight + totalFreed) < weight.
    In order to avoid calculating userFreeWeight + totalFreed in every iteration, you can calculate weight - userFreeWeight (let's assume that weightLeftToFree == weight - userFreeWeight) once and change the condition to totalFreed < weightLeftToFree. You can also use unchecked on the calculation of weightLeftToFree because you know for sure that weight > userFreeWeight (or there was no weight to free).

  • Use unchecked in _incrementDelegation and _undelegate in ERC20MultiVotes
    In the _incrementDelegation function you increment both _delegatesVotesCount[delegator][delegatee] and userDelegatedVotes[delegator]. We know for sure that _delegatesVotesCount[delegator][delegatee] <= userDelegatedVotes[delegator], because userDelegatedVotes[delegator] is incremented at least any time that _delegatesVotesCount[delegator][delegatee] (it's also incremented when delegating other delegatees), so unchecked can be used when incrementing _delegatesVotesCount[delegator][delegatee] because if it will overflow userDelegatedVotes[delegator] will overflow too.
    We can see a similar thing in the _undelegate function. In that function we decrement both _delegatesVotesCount[delegator][delegatee] and userDelegatedVotes[delegator], and for the same reason we know that if userDelegatedVotes[delegator] will underflow, _delegatesVotesCount[delegator][delegatee] will underflow too, so we can use unchecked when decrementing userDelegatedVotes[delegator].

    function _incrementDelegation(
        address delegator,
        address delegatee,
        uint256 amount
    ) internal virtual {
        // ...
    
        userDelegatedVotes[delegator] += amount;
        unchecked {
            _delegatesVotesCount[delegator][delegatee] += amount;
        }
        
        // ...
    }
    
    function _undelegate(
            address delegator,
            address delegatee,
            uint256 amount
        ) internal virtual {
            uint256 newDelegates = _delegatesVotesCount[delegator][delegatee] - amount;
            
            // ...
    
            _delegatesVotesCount[delegator][delegatee] = newDelegates;
            unchecked {
                userDelegatedVotes[delegator] -= amount;
            }
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Apr 27, 2022
code423n4 added a commit that referenced this issue Apr 27, 2022
@thomas-waite thomas-waite added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Jul 14, 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 G (Gas Optimization) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

2 participants