code-423n4 / 2022-05-velodrome-findings

0 stars 0 forks source link

Gas Optimizations #216

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

No need to initialize variables with default values

If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type). If you explicitly initialize it with its default value, you are just wasting gas. No need to initialize variables with their defaults (save some 3 gas for every variable )

router.sol line 112

        uint _totalSupply = 0;

File: Voter.sol line 139-141

        uint256 _totalVoteWeight = 0;
        uint256 _totalWeight = 0;
        uint256 _usedWeight = 0;

Other Instances:

File: RewardsDistributor.sol line 73 File: RewardsDistributor.sol line 103 File: RewardsDistributor.sol line 119 File: RewardsDistributor.sol line 170-171 File: RewardsDistributor.sol line 227-228 File: RewardsDistributor.sol line 299

File: gauge.sol line 222 File: gauge.sol line 254 File: gauge.sol line 286 File: Gauge.sol line 481 File: Gauge.sol line 551

File: Pair.sol line 61-62 File: Pair.sol line 61-62 File: Pair.sol line 273-274

File: Voter.sol line 101 File: Voter.sol line 101

File: VotingEscrow.sol line 622

File: Velo.sol line 9

Most of the require statement have strings more than 32 bits

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and also 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

File: Router.sol line 341

        require(amounts[amounts.length - 1] >= amountOutMin, 'Router: INSUFFICIENT_OUTPUT_AMOUNT');

Other Instances: File: Router.sol line 356 File: Router.sol line 384 File: Router.sol line 406

FIle: VotingEscrow.sol line 398 FIle: VotingEscrow.sol line 774 FIle: VotingEscrow.sol line 786 FIle: VotingEscrow.sol line 821 FIle: VotingEscrow.sol line 1245 FIle: VotingEscrow.sol line 1315 File: VotingEscrow.sol Line 1378 FIle: VotingEscrow.sol line 1382 FIle: VotingEscrow.sol line 1386

Splitting require() statements that use && saves gas

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 3 GAS per &&:

File: Pair.sol line 338

        require(amount0Out < _reserve0 && amount1Out < _reserve1, 'IL'); // Pair: INSUFFICIENT_LIQUIDITY

The above can be written as follows to save some gas(3 gas)

        require(amount0Out < _reserve0 ,'IL');
        require( amount1Out < _reserve1, 'IL'); 

Other Instances

File: Pair.sol line 322 File: Pair.sol line 344 File: Pair.sol line 485

File: VotingEscrow.sol line 307 File: VotingEscrow.sol line 846 File: VotingEscrow.sol line 1085

File: Router.sol line 59

File: Voter.sol line 194

File: RedemptionReceiver.sol line 79

>0 is Less efficient than != 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:

File: Gauge.sol line 512

        require(amount > 0);

Other Instances

File: Gauge.sol line 592

File: Bribe.sol line 42

File: Router.sol line 58

File: VotingEscrow.sol line 772 File: VotingEscrow.sol line 785

File: Pair.sol line 322 File: Pair.sol line 303 File: Pair.sol line 336

Use Require instead of Assert to save some gas on some functions

The bigger difference between the two keyword is that assert when condition is false tend to consume all gas remaining gas and reverts all the changes made. In reverse, require when the condition is false refund all the remaining gas fees we offered to pay beyond reverts all the changes.

Gas Efficiency

• assert(false) compiles to 0xfe, an invalid opcode, thus using up all remaining gas, this because the assert function creates an error type Panic(uint256).

• require(false) compiles to 0xfd, the revert opcode, thus refunding the remaining gas.

File: VotingEscrow.sol line 262

            function setApprovalForAll(address _operator, bool _approved) external {
        // Throws if `_operator` is the `msg.sender`
        assert(_operator != msg.sender);
        ownerToOperators[msg.sender][_operator] = _approved;
        emit ApprovalForAll(msg.sender, _operator, _approved);
    }

Note that in the above function , the condition being checked is that the user input _operator is not equal to msg.sender.

File: VotingEscrow.sol line 819

        assert(_value > 0); // dev: need non-zero value

We should use require instead of assert as we are validating the user input _value

Other Instances:

File: VotingEscrow.sol line 447

File: VotingEscrow.sol line 464

File: VotingEscrow.sol line 508

Use Unchecked blocks for operations that cannot underflow

File: Gauge.sol line 223

 uint upper = nCheckpoints - 1;

The line 208 ensures that nCheckpoints is greater than 0 therefore our operation nCheckpoints - 1 would never over/underflow

We can wrap that operation with **unchecked block like below to save some gas

 unchecked{ 
         uint upper = nCheckpoints - 1;
}

Use Unchecked block

File: Pair.sol line 351-352

uint amount0In = _balance0 > _reserve0 - amount0Out ? _balance0 - (_reserve0 - amount0Out) : 0;
uint amount1In = _balance1 > _reserve1 - amount1Out ? _balance1 - (_reserve1 - amount1Out) : 0;

We first check that _balance0 is greater than _reserve0 - amount0Out) and if that's the case we then run _balance0 - (_reserve0 - amount0Out) meaning we can never get an underflow at _balance0 - (_reserve0 - amount0Out)

We can also use uncheck block on the first part of the check ie _balance0 > _reserve0 - amount0Out since _reserve0 - amount0Out cannot underflow due to the line 338 here which ensures that amount0Out is greater than_reserve0`

require(amount0Out < _reserve0 && amount1Out < _reserve1, 'IL'); // Pair: INSUFFICIENT_LIQUIDITY

Unchecked blocks for operations that cannot underflow

Router.sol line 230

 if (msg.value > amountETH) _safeTransferETH(msg.sender, msg.value - amountETH);

The msg,value - amountETH should be wrapped with unchecked block since it cannot underflow due to the check, msg.value > amountEth

Use Unchecked block

File: Gauge.sol line 584-588

    function left(address token) external view returns (uint) {
        if (block.timestamp >= periodFinish[token]) return 0;
        uint _remaining = periodFinish[token] - block.timestamp;
        return _remaining * rewardRate[token];
    }

The line 586 uint _remaining = periodFinish[token] - block.timestamp; cannot underflow due to the check on line 585

Use Unchecked block

File: Gauge.sol line 607

            uint _remaining = periodFinish[token] - block.timestamp;

The above cannot underflow/overflow due to the check on line 603 ie

        if (block.timestamp >= periodFinish[token]) {
            _safeTransferFrom(token, msg.sender, address(this), amount);
            rewardRate[token] = amount / DURATION;
        } else {
            uint _remaining = periodFinish[token] - block.timestamp;

The code would not reach the else block if block.timestamp >= periodFinish[token]

Use Unchecked block

File: Gauge.sol line 652

            uint _remaining = periodFinish[token] - block.timestamp;

Use Unchecked block

File: Gauge.sol line 214

            return (nCheckpoints - 1);

Use Unchecked block

File: Gauge.sol line 246

            return (nCheckpoints - 1);

Use Unchecked block

File: Gauge.sol line 255

            return (nCheckpoints - 1);

Use Unchecked block

File: Gauge.sol line 287

            return (nCheckpoints - 1);

Use Unchecked block

File: Minter.sol line 124

 if (_balanceOf < _required) {
                _velo.mint(address(this), _required - _balanceOf);
            }

The line _required - _balanceOf cannot underflow due the check if (_balanceOf < _required)

The Increments in for loops can be unchecked

File: Gauge.sol line 179

    function deliverBribes() external lock {
        require(msg.sender == voter);
        IBribe sb = IBribe(bribe);
        uint bribeStart = block.timestamp - (block.timestamp % (7 days)) + BRIBE_LAG;
        uint numRewards = sb.rewardsListLength();

        for (uint i = 0; i < numRewards; i++) {
            address token = sb.rewards(i);
            uint epochRewards = sb.deliverReward(token, bribeStart);
            if (epochRewards > 0) {
                _notifyBribeAmount(token, epochRewards, bribeStart);
            }
        }
    }

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 numRewards<= 2256 - 1. Therefore, the theoretical maximum value of i to enter the for-loop body is 2256 - 2. This means that the i++ in the for loop can never overflow. Regardless, the overflow checks are performed by the compiler.

for (uint i = 0; i < numRewards; i = unchecked_inc(i)) {
    // do something that doesn't change the value of i
}

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

Note that it’s important that the call to unchecked_inc is inlined which is possible for solidity versions starting from 0.8.2.

Gas savings: roughly speaking this can save 30-40 gas per loop iteration. For lengthy loops, this can be significant!

Other Instance: File: Gauge.sol 424

An arrays Length should be cached to save gas in for loops

Reading arrays length at each iteration of the loop takes 6 gas (3 for mloads and 3 to place memory offset in the stack Caching the array length in the stack saves around 3 gas per iteration

File: Voter.sol line 74


    function initialize(address[] memory _tokens, address _minter) external {
        require(msg.sender == minter);
        for (uint i = 0; i < _tokens.length; i++) {
            _whitelist(_tokens[i]);
        }
        minter = _minter;
    }

Modify the above function to:

  function initialize(address[] memory _tokens, address _minter) external {
        require(msg.sender == minter);
        uint length = _tokens.length;
        for (uint i = 0; i < length; i++) {
            _whitelist(_tokens[i]);
        }
        minter = _minter;
    }

Other Instances: File: Router.sol line 90 File: Router.sol line 316

GalloDaSballo commented 2 years ago

Would save between 500 and 2k gas