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

0 stars 0 forks source link

Gas Optimizations #205

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

1. Cache array length outside of loop

Impact

Caching the array length outside a loop saves reading it on each iteration, as long as the array's length is not changed during the loop.

Proof of Concept

Gauge.sol:353:        for (uint i = 0; i < tokens.length; i++) {
Gauge.sol:426:        for (uint i; i < length; i++) {
Minter.sol:57:        for (uint i = 0; i < claimants.length; i++) {
Pair.sol:257:        for (uint i = 0; i < _prices.length; i++) {
Pair.sol:276:        for (; i < length; i+=window) {
RewardsDistributor.sol:301:        for (uint i = 0; i < _tokenIds.length; i++) {
Router.sol:90:        for (uint i = 0; i < routes.length; i++) {
Router.sol:316:        for (uint i = 0; i < routes.length; i++) {
Voter.sol:76:        for (uint i = 0; i < _tokens.length; i++) {
Voter.sol:266:        for (uint i = 0; i < _gauges.length; i++) {
Voter.sol:304:        for (uint i = 0; i < _gauges.length; i++) {
Voter.sol:310:        for (uint i = 0; i < _gauges.length; i++) {
Voter.sol:346:        for (uint x = 0; x < _gauges.length; x++) {
VotingEscrow.sol:1146:        for (uint i = 0; i < _tokenIds.length; i++) {
VotingEscrow.sol:1193:        for (uint i = 0; i < _tokenIds.length; i++) {
VotingEscrow.sol:1225:                for (uint i = 0; i < srcRepOld.length; i++) {
VotingEscrow.sol:1249:                for (uint i = 0; i < dstRepOld.length; i++) {
VotingEscrow.sol:1295:                for (uint i = 0; i < srcRepOld.length; i++) {
VotingEscrow.sol:1320:                for (uint i = 0; i < dstRepOld.length; i++) {
governance/L2Governor.sol:326:        for (uint256 i = 0; i < targets.length; ++i) {
governance/L2Governor.sol:343:            for (uint256 i = 0; i < targets.length; ++i) {

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to cache the array length outside of the for loop.

2. Long revert error messages

Impact

Shortening revert error messages to fit in 32 bytes will decrease gas costs for deployment and gas costs when the revert condition has been met.

Proof of Concept

Router.sol:341:        require(amounts[amounts.length - 1] >= amountOutMin, 'Router: INSUFFICIENT_OUTPUT_AMOUNT');
Router.sol:356:        require(amounts[amounts.length - 1] >= amountOutMin, 'Router: INSUFFICIENT_OUTPUT_AMOUNT');
Router.sol:371:        require(amounts[amounts.length - 1] >= amountOutMin, 'Router: INSUFFICIENT_OUTPUT_AMOUNT');
Router.sol:384:        require(amounts[amounts.length - 1] >= amountOutMin, 'Router: INSUFFICIENT_OUTPUT_AMOUNT');
Router.sol:406:        require(success, 'TransferHelper: ETH_TRANSFER_FAILED');
VotingEscrow.sol:774:        require(_locked.end > block.timestamp, 'Cannot add to expired lock. Withdraw');
VotingEscrow.sol:786:        require(unlock_time > block.timestamp, 'Can only lock until time in the future');
VotingEscrow.sol:821:        require(_locked.end > block.timestamp, 'Cannot add to expired lock. Withdraw');
--
VotingEscrow.sol:1245:                require(
VotingEscrow.sol-1246-                    dstRepOld.length + 1 <= MAX_DELEGATES,
VotingEscrow.sol-1247-                    "dstRep would have too many tokenIds"
VotingEscrow.sol-1248-                );
--
VotingEscrow.sol:1315:                require(
VotingEscrow.sol-1316-                    dstRepOld.length + ownerTokenCount <= MAX_DELEGATES,
VotingEscrow.sol-1317-                    "dstRep would have too many tokenIds"
VotingEscrow.sol-1318-                );
--
VotingEscrow.sol:1378:        require(
VotingEscrow.sol-1379-            signatory != address(0),
VotingEscrow.sol-1380-            "VotingEscrow::delegateBySig: invalid signature"
VotingEscrow.sol-1381-        );
--
VotingEscrow.sol:1382:        require(
VotingEscrow.sol-1383-            nonce == nonces[signatory]++,
VotingEscrow.sol-1384-            "VotingEscrow::delegateBySig: invalid nonce"
VotingEscrow.sol-1385-        );
--
VotingEscrow.sol:1386:        require(
VotingEscrow.sol-1387-            block.timestamp <= expiry,
VotingEscrow.sol-1388-            "VotingEscrow::delegateBySig: signature expired"
VotingEscrow.sol-1389-        );

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to decrease revert messages to maximum 32 bytes. Alternatively custom error types should be used.

3. Use custom errors instead of revert strings to save gas

Impact

Usage of custom errors reduces the gas cost.

Proof of Concept

Contract that could be using custom errors:

Gauge.sol
Voter.sol
Velo.sol
RewardsDistributor.sol
Minter.sol
Router.sol
factories/GaugeFactory.sol
factories/PairFactory.sol
VeloGovernor.sol
governance/L2GovernorVotesQuorumFraction.sol
governance/L2Governor.sol
governance/L2GovernorCountingSimple.sol
VotingEscrow.sol
Pair.sol
Bribe.sol
redeem/RedemptionSender.sol
redeem/RedemptionReceiver.sol
PairFees.sol

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to add custom errors to listed contracts.

4. ++i/--i costs less gas compared to i++, i += 1, i-- or i -= 1

Impact

++i or --i costs less gas compared to i++, i += 1, i-- or i -= 1 for unsigned integer as pre-increment/pre-decrement is cheaper (about 5 gas per iteration).

Proof of Concept

Gauge.sol:179:        for (uint i = 0; i < numRewards; i++) {
Gauge.sol:353:        for (uint i = 0; i < tokens.length; i++) {
Gauge.sol:405:        for (uint i = _startIndex; i < _endIndex; i++) {
Gauge.sol:426:        for (uint i; i < length; i++) {
Gauge.sol:448:            for (uint i = _startIndex; i < _endIndex; i++) {
Gauge.sol:484:            for (uint i = _startIndex; i < _endIndex; i++) {
Minter.sol:57:        for (uint i = 0; i < claimants.length; i++) {
Pair.sol:257:        for (uint i = 0; i < _prices.length; i++) {
Pair.sol:389:        for (uint i = 0; i < 255; i++) {
RewardsDistributor.sol:75:        for (uint i = 0; i < 20; i++) {
RewardsDistributor.sol:105:        for (uint i = 0; i < 128; i++) {
RewardsDistributor.sol:121:        for (uint i = 0; i < 128; i++) {
RewardsDistributor.sol:148:        for (uint i = 0; i < 20; i++) {
RewardsDistributor.sol:195:        for (uint i = 0; i < 50; i++) {
RewardsDistributor.sol:252:        for (uint i = 0; i < 50; i++) {
RewardsDistributor.sol:301:        for (uint i = 0; i < _tokenIds.length; i++) {
Router.sol:90:        for (uint i = 0; i < routes.length; i++) {
Router.sol:316:        for (uint i = 0; i < routes.length; i++) {
VelodromeLibrary.sol:24:        for (uint i = 0; i < 255; i++) {
Voter.sol:76:        for (uint i = 0; i < _tokens.length; i++) {
Voter.sol:103:        for (uint i = 0; i < _poolVoteCnt; i ++) {
Voter.sol:128:        for (uint i = 0; i < _poolCnt; i ++) {
Voter.sol:143:        for (uint i = 0; i < _poolCnt; i++) {
Voter.sol:147:        for (uint i = 0; i < _poolCnt; i++) {
Voter.sol:266:        for (uint i = 0; i < _gauges.length; i++) {
Voter.sol:272:        for (uint i = start; i < end; i++) {
Voter.sol:304:        for (uint i = 0; i < _gauges.length; i++) {
Voter.sol:310:        for (uint i = 0; i < _gauges.length; i++) {
Voter.sol:340:        for (uint x = start; x < finish; x++) {
Voter.sol:346:        for (uint x = 0; x < _gauges.length; x++) {
VotingEscrow.sol:137:            digits++;
VotingEscrow.sol:1146:        for (uint i = 0; i < _tokenIds.length; i++) {
VotingEscrow.sol:1193:        for (uint i = 0; i < _tokenIds.length; i++) {
VotingEscrow.sol:1225:                for (uint i = 0; i < srcRepOld.length; i++) {
VotingEscrow.sol:1249:                for (uint i = 0; i < dstRepOld.length; i++) {
VotingEscrow.sol:1295:                for (uint i = 0; i < srcRepOld.length; i++) {
VotingEscrow.sol:1320:                for (uint i = 0; i < dstRepOld.length; i++) {
VotingEscrow.sol:1325:                for (uint i = 0; i < ownerTokenCount; i++) {
RewardsDistributor.sol:199:                user_epoch += 1;
RewardsDistributor.sol:256:                user_epoch += 1;
VotingEscrow.sol:453:        ownerToNFTokenCount[_to] += 1;
VotingEscrow.sol:655:                _epoch += 1;
libraries/Math.sol:29:                x += 1;
VotingEscrow.sol:142:            digits -= 1;
VotingEscrow.sol:514:        ownerToNFTokenCount[_from] -= 1;

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to use ++i or --i instead of i++, i += 1, i-- or i -= 1 to increment value of an unsigned integer variable.

5. Obsolete overflow/underflow check

Impact

Starting from solidity 0.8.0 there is built-in check for overflows/underflows. This mechanism automatically checks if the variable overflows or underflows and throws an error. Multiple contracts use increments that cannot overflow but consume additional gas for checks.

Proof of Concept

Gauge.sol:179:        for (uint i = 0; i < numRewards; i++) {
Gauge.sol:353:        for (uint i = 0; i < tokens.length; i++) {
Gauge.sol:405:        for (uint i = _startIndex; i < _endIndex; i++) {
Gauge.sol:426:        for (uint i; i < length; i++) {
Gauge.sol:448:            for (uint i = _startIndex; i < _endIndex; i++) {
Gauge.sol:484:            for (uint i = _startIndex; i < _endIndex; i++) {
Minter.sol:57:        for (uint i = 0; i < claimants.length; i++) {
Pair.sol:257:        for (uint i = 0; i < _prices.length; i++) {
Pair.sol:389:        for (uint i = 0; i < 255; i++) {
Pair.sol:481:                keccak256(abi.encode(PERMIT_TYPEHASH, owner, spender, value, nonces[owner]++, deadline))
RewardsDistributor.sol:75:        for (uint i = 0; i < 20; i++) {
RewardsDistributor.sol:105:        for (uint i = 0; i < 128; i++) {
RewardsDistributor.sol:121:        for (uint i = 0; i < 128; i++) {
RewardsDistributor.sol:148:        for (uint i = 0; i < 20; i++) {
RewardsDistributor.sol:195:        for (uint i = 0; i < 50; i++) {
RewardsDistributor.sol:252:        for (uint i = 0; i < 50; i++) {
RewardsDistributor.sol:301:        for (uint i = 0; i < _tokenIds.length; i++) {
Router.sol:90:        for (uint i = 0; i < routes.length; i++) {
Router.sol:316:        for (uint i = 0; i < routes.length; i++) {
VelodromeLibrary.sol:24:        for (uint i = 0; i < 255; i++) {
Voter.sol:76:        for (uint i = 0; i < _tokens.length; i++) {
Voter.sol:103:        for (uint i = 0; i < _poolVoteCnt; i ++) {
Voter.sol:128:        for (uint i = 0; i < _poolCnt; i ++) {
Voter.sol:143:        for (uint i = 0; i < _poolCnt; i++) {
Voter.sol:147:        for (uint i = 0; i < _poolCnt; i++) {
Voter.sol:266:        for (uint i = 0; i < _gauges.length; i++) {
Voter.sol:272:        for (uint i = start; i < end; i++) {
Voter.sol:304:        for (uint i = 0; i < _gauges.length; i++) {
Voter.sol:310:        for (uint i = 0; i < _gauges.length; i++) {
Voter.sol:340:        for (uint x = start; x < finish; x++) {
Voter.sol:346:        for (uint x = 0; x < _gauges.length; x++) {
VotingEscrow.sol:137:            digits++;
VotingEscrow.sol:632:            for (uint i = 0; i < 255; ++i) {
VotingEscrow.sol:789:        ++tokenId;
VotingEscrow.sol:886:        for (uint i = 0; i < 128; ++i) {
VotingEscrow.sol:942:        for (uint i = 0; i < 128; ++i) {
VotingEscrow.sol:1017:        for (uint i = 0; i < 255; ++i) {
VotingEscrow.sol:1146:        for (uint i = 0; i < _tokenIds.length; i++) {
VotingEscrow.sol:1193:        for (uint i = 0; i < _tokenIds.length; i++) {
VotingEscrow.sol:1225:                for (uint i = 0; i < srcRepOld.length; i++) {
VotingEscrow.sol:1249:                for (uint i = 0; i < dstRepOld.length; i++) {
VotingEscrow.sol:1295:                for (uint i = 0; i < srcRepOld.length; i++) {
VotingEscrow.sol:1320:                for (uint i = 0; i < dstRepOld.length; i++) {
VotingEscrow.sol:1325:                for (uint i = 0; i < ownerTokenCount; i++) {
VotingEscrow.sol:1383:            nonce == nonces[signatory]++,
governance/L2Governor.sol:326:        for (uint256 i = 0; i < targets.length; ++i) {
governance/L2Governor.sol:343:            for (uint256 i = 0; i < targets.length; ++i) {
RewardsDistributor.sol:199:                user_epoch += 1;
RewardsDistributor.sol:256:                user_epoch += 1;
VotingEscrow.sol:453:        ownerToNFTokenCount[_to] += 1;
VotingEscrow.sol:655:                _epoch += 1;
libraries/Math.sol:29:                x += 1;
VotingEscrow.sol:142:            digits -= 1;
VotingEscrow.sol:514:        ownerToNFTokenCount[_from] -= 1;

Recommended Mitigation Steps

It is recommended wrap incrementing with unchecked block, for example: unchecked { ++i } or unchecked { --i }.

6. No need to explicitly initialize variables with default values

Impact

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for addresses). Explicitly initializing it with its default value is an anti-pattern and waste of gas.

Proof of Concept

Gauge.sol:179:        for (uint i = 0; i < numRewards; i++) {
Gauge.sol:222:        uint lower = 0;
Gauge.sol:254:        uint lower = 0;
Gauge.sol:286:        uint lower = 0;
Gauge.sol:353:        for (uint i = 0; i < tokens.length; i++) {
Gauge.sol:481:        uint reward = 0;
Gauge.sol:551:        uint tokenId = 0;
Minter.sol:57:        for (uint i = 0; i < claimants.length; i++) {
Pair.sol:20:    uint public totalSupply = 0;
Pair.sol:61:    uint public index0 = 0;
Pair.sol:62:    uint public index1 = 0;
Pair.sol:257:        for (uint i = 0; i < _prices.length; i++) {
Pair.sol:273:        uint nextIndex = 0;
Pair.sol:274:        uint index = 0;
Pair.sol:389:        for (uint i = 0; i < 255; i++) {
RewardsDistributor.sol:73:        uint next_week = 0;
RewardsDistributor.sol:75:        for (uint i = 0; i < 20; i++) {
RewardsDistributor.sol:103:        uint _min = 0;
RewardsDistributor.sol:105:        for (uint i = 0; i < 128; i++) {
RewardsDistributor.sol:119:        uint _min = 0;
RewardsDistributor.sol:121:        for (uint i = 0; i < 128; i++) {
RewardsDistributor.sol:148:        for (uint i = 0; i < 20; i++) {
RewardsDistributor.sol:154:                int128 dt = 0;
RewardsDistributor.sol:170:        uint user_epoch = 0;
RewardsDistributor.sol:171:        uint to_distribute = 0;
RewardsDistributor.sol:195:        for (uint i = 0; i < 50; i++) {
RewardsDistributor.sol:227:        uint user_epoch = 0;
RewardsDistributor.sol:228:        uint to_distribute = 0;
RewardsDistributor.sol:252:        for (uint i = 0; i < 50; i++) {
RewardsDistributor.sol:299:        uint total = 0;
RewardsDistributor.sol:301:        for (uint i = 0; i < _tokenIds.length; i++) {
Router.sol:90:        for (uint i = 0; i < routes.length; i++) {
Router.sol:112:        uint _totalSupply = 0;
Router.sol:316:        for (uint i = 0; i < routes.length; i++) {
Velo.sol:9:    uint public totalSupply = 0;
VelodromeLibrary.sol:24:        for (uint i = 0; i < 255; i++) {
Voter.sol:76:        for (uint i = 0; i < _tokens.length; i++) {
Voter.sol:101:        uint256 _totalWeight = 0;
Voter.sol:103:        for (uint i = 0; i < _poolVoteCnt; i ++) {
Voter.sol:128:        for (uint i = 0; i < _poolCnt; i ++) {
Voter.sol:139:        uint256 _totalVoteWeight = 0;
Voter.sol:140:        uint256 _totalWeight = 0;
Voter.sol:141:        uint256 _usedWeight = 0;
Voter.sol:143:        for (uint i = 0; i < _poolCnt; i++) {
Voter.sol:147:        for (uint i = 0; i < _poolCnt; i++) {
Voter.sol:266:        for (uint i = 0; i < _gauges.length; i++) {
Voter.sol:304:        for (uint i = 0; i < _gauges.length; i++) {
Voter.sol:310:        for (uint i = 0; i < _gauges.length; i++) {
Voter.sol:346:        for (uint x = 0; x < _gauges.length; x++) {
VotingEscrow.sol:584:        int128 old_dslope = 0;
VotingEscrow.sol:585:        int128 new_dslope = 0;
VotingEscrow.sol:622:        uint block_slope = 0; // dblock/dt
VotingEscrow.sol:632:            for (uint i = 0; i < 255; ++i) {
VotingEscrow.sol:636:                int128 d_slope = 0;
VotingEscrow.sol:884:        uint _min = 0;
VotingEscrow.sol:886:        for (uint i = 0; i < 128; ++i) {
VotingEscrow.sol:940:        uint _min = 0;
VotingEscrow.sol:942:        for (uint i = 0; i < 128; ++i) {
VotingEscrow.sol:960:        uint d_block = 0;
VotingEscrow.sol:961:        uint d_t = 0;
VotingEscrow.sol:996:        uint dt = 0;
VotingEscrow.sol:1017:        for (uint i = 0; i < 255; ++i) {
VotingEscrow.sol:1019:            int128 d_slope = 0;
VotingEscrow.sol:1145:        uint votes = 0;
VotingEscrow.sol:1146:        for (uint i = 0; i < _tokenIds.length; i++) {
VotingEscrow.sol:1168:        uint32 lower = 0;
VotingEscrow.sol:1192:        uint votes = 0;
VotingEscrow.sol:1193:        for (uint i = 0; i < _tokenIds.length; i++) {
VotingEscrow.sol:1225:                for (uint i = 0; i < srcRepOld.length; i++) {
VotingEscrow.sol:1249:                for (uint i = 0; i < dstRepOld.length; i++) {
VotingEscrow.sol:1295:                for (uint i = 0; i < srcRepOld.length; i++) {
VotingEscrow.sol:1320:                for (uint i = 0; i < dstRepOld.length; i++) {
VotingEscrow.sol:1325:                for (uint i = 0; i < ownerTokenCount; i++) {
governance/L2Governor.sol:326:        for (uint256 i = 0; i < targets.length; ++i) {
governance/L2Governor.sol:343:            for (uint256 i = 0; i < targets.length; ++i) {
libraries/Math.sol:23:        uint256 x = 0;

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to remove explicit initializations with default values.

7. Use != 0 instead of > 0 for unsigned integer comparison

Impact

When dealing with unsigned integer types, comparisons with != 0 are cheaper than with > 0.

Proof of Concept

Bribe.sol:42:      require(amount > 0);
Bribe.sol:86:    if (rewardPerEpoch > 0) {
Bribe.sol:93:      require(token.code.length > 0);
Bribe.sol:100:      require(token.code.length > 0);
Gauge.sol:140:        if (claimed0 > 0 || claimed1 > 0) {
Gauge.sol:144:            if (_fees0 / DURATION > 0) {
Gauge.sol:151:            if (_fees1 / DURATION > 0) {
Gauge.sol:182:            if (epochRewards > 0) {
Gauge.sol:306:        if (_nCheckPoints > 0 && checkpoints[account][_nCheckPoints - 1].timestamp == _timestamp) {
Gauge.sol:309:            bool prevVoteStatus = (_nCheckPoints > 0) ? checkpoints[account][_nCheckPoints].voted : false;
Gauge.sol:318:        if (_nCheckPoints > 0 && rewardPerTokenCheckpoints[token][_nCheckPoints - 1].timestamp == timestamp) {
Gauge.sol:330:        if (_nCheckPoints > 0 && supplyCheckpoints[_nCheckPoints - 1].timestamp == _timestamp) {
Gauge.sol:359:            if (_reward > 0) _safeTransfer(tokens[i], account, _reward);
Gauge.sol:407:            if (sp0.supply > 0) {
Gauge.sol:447:        if (_endIndex > 0) {
Gauge.sol:450:                if (sp0.supply > 0) {
Gauge.sol:461:        if (sp.supply > 0) {
Gauge.sol:483:        if (_endIndex > 0) {
Gauge.sol:512:        require(amount > 0);
Gauge.sol:520:        if (tokenId > 0) {
Gauge.sol:563:        if (tokenId > 0) {
Gauge.sol:592:        require(amount > 0);
Gauge.sol:613:        require(rewardRate[token] > 0);
Gauge.sol:665:        require(token.code.length > 0);
Gauge.sol:672:        require(token.code.length > 0);
Gauge.sol:679:        require(token.code.length > 0);
Pair.sol:140:        if (claimed0 > 0 || claimed1 > 0) {
Pair.sol:154:        if (_ratio > 0) {
Pair.sol:164:        if (_ratio > 0) {
Pair.sol:174:        if (_supplied > 0) {
Pair.sol:183:            if (_delta0 > 0) {
Pair.sol:187:            if (_delta1 > 0) {
Pair.sol:207:        if (timeElapsed > 0 && _reserve0 != 0 && _reserve1 != 0) {
Pair.sol:303:        require(liquidity > 0, 'ILM'); // Pair: INSUFFICIENT_LIQUIDITY_MINTED
Pair.sol:322:        require(amount0 > 0 && amount1 > 0, 'ILB'); // Pair: INSUFFICIENT_LIQUIDITY_BURNED
Pair.sol:336:        require(amount0Out > 0 || amount1Out > 0, 'IOA'); // Pair: INSUFFICIENT_OUTPUT_AMOUNT
Pair.sol:345:        if (amount0Out > 0) _safeTransfer(_token0, to, amount0Out); // optimistically transfer tokens
Pair.sol:346:        if (amount1Out > 0) _safeTransfer(_token1, to, amount1Out); // optimistically transfer tokens
Pair.sol:347:        if (data.length > 0) IPairCallee(to).hook(msg.sender, amount0Out, amount1Out, data); // callback, used for flash loans
Pair.sol:353:        require(amount0In > 0 || amount1In > 0, 'IIA'); // Pair: INSUFFICIENT_INPUT_AMOUNT
Pair.sol:356:        if (amount0In > 0) _update0(amount0In * PairFactory(factory).getFee(stable) / 10000); // accrue fees for token0 and move them out of pool
Pair.sol:357:        if (amount1In > 0) _update1(amount1In * PairFactory(factory).getFee(stable) / 10000); // accrue fees for token1 and move them out of pool
Pair.sol:522:        require(token.code.length > 0);
PairFees.sol:20:        require(token.code.length > 0);
PairFees.sol:29:        if (amount0 > 0) _safeTransfer(token0, recipient, amount0);
PairFees.sol:30:        if (amount1 > 0) _safeTransfer(token1, recipient, amount1);
Router.sol:58:        require(amountA > 0, 'Router: INSUFFICIENT_AMOUNT');
Router.sol:59:        require(reserveA > 0 && reserveB > 0, 'Router: INSUFFICIENT_LIQUIDITY');
Router.sol:410:        require(token.code.length > 0);
Router.sol:417:        require(token.code.length > 0);
Voter.sol:111:                if (_votes > 0) {
Voter.sol:167:        if (_usedWeight > 0) IVotingEscrow(_ve).voting(_tokenId);
Voter.sol:227:        if (tokenId > 0) IVotingEscrow(_ve).attach(tokenId);
Voter.sol:239:        if (tokenId > 0) IVotingEscrow(_ve).detach(tokenId);
Voter.sol:259:        if (_ratio > 0) {
Voter.sol:289:        if (_supplied > 0) {
Voter.sol:294:            if (_delta > 0) {
Voter.sol:322:        if (_claimable > IGauge(_gauge).left(base) && _claimable / DURATION > 0) {
Voter.sol:352:        require(token.code.length > 0);
VotingEscrow.sol:369:        return size > 0;
VotingEscrow.sol:591:            if (old_locked.end > block.timestamp && old_locked.amount > 0) {
VotingEscrow.sol:595:            if (new_locked.end > block.timestamp && new_locked.amount > 0) {
VotingEscrow.sol:614:        if (_epoch > 0) {
VotingEscrow.sol:742:        // value == 0 (extend lock) or value > 0 (add to lock or extend lock)
VotingEscrow.sol:772:        require(_value > 0); // dev: need non-zero value
VotingEscrow.sol:773:        require(_locked.amount > 0, 'No existing lock found');
VotingEscrow.sol:785:        require(_value > 0); // dev: need non-zero value
VotingEscrow.sol:819:        assert(_value > 0); // dev: need non-zero value
VotingEscrow.sol:820:        require(_locked.amount > 0, 'No existing lock found');
VotingEscrow.sol:835:        require(_locked.amount > 0, 'Nothing is locked');
VotingEscrow.sol:1214:        if (srcRep != dstRep && _tokenId > 0) {
VotingEscrow.sol:1217:                uint[] storage srcRepOld = srcRepNum > 0
VotingEscrow.sol:1237:                uint[] storage dstRepOld = dstRepNum > 0
VotingEscrow.sol:1269:            _nCheckPoints > 0 &&
VotingEscrow.sol:1287:                uint[] storage srcRepOld = srcRepNum > 0
VotingEscrow.sol:1307:                uint[] storage dstRepOld = dstRepNum > 0
governance/L2Governor.sol:262:        require(targets.length > 0, "Governor: empty proposal");
libraries/Math.sol:24:        for (uint256 y = 1 << 255; y > 0; y >>= 3) {

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to use != 0 instead of > 0.

8. Pack integer values

Impact

Packing integer variables into storage slots saves gas.

Proof of Concept

VotingEscrow.sol:

Pair.sol:

Gauge.sol:

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to pack listed values in order to consume less storage and in effect less gas.

9. Use scientific notation

Impact

Multiple contracts are using math exponent calculation to express big numbers. This consumes additional gas and its better to use scienfic notation.

Proof of Concept

Gauge.sol:36:    uint internal constant PRECISION = 10 ** 18;
Pair.sol:30:    uint internal constant MINIMUM_LIQUIDITY = 10**3;
Router.sol:21:    uint internal constant MINIMUM_LIQUIDITY = 10**3;

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to use scientific notation, for example: 1e18.

10. Obsolete function logic

Impact

Contract Velo.sol in its constructor is executing function _mint with argument of 0 which does not affect storage in any way thus can be removed to save the gas.

Proof of Concept

Velo.sol:

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to remove obsolete function logic.

GalloDaSballo commented 2 years ago

Packing

The warding has shown a use of packing that would save 5k in the average case (non-zero to non-zero)

pack ts as uint64 - https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L34 pack blk as uint64 - https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L35

The rest of the findings would save less than 1k gas

GalloDaSballo commented 2 years ago

The report could benefit by: