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

0 stars 0 forks source link

Gas Optimizations #194

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

G01 - Using != 0 instead of > 0 in require statement with uint

!= 0 costs less gas compared to > 0 for unsigned integers in require statements with the optimizer enabled:

contracts/Bribe.sol:42  require(amount > 0); 
contracts/Bribe.sol:93  require(token.code.length > 0);  
contracts/Bribe.sol:100 require(token.code.length > 0); 
contracts/Gauge.sol:512 require(amount > 0);  contracts/Gauge.sol:592 require(amount > 0); 
contracts/Gauge.sol:613 require(rewardRate[token] > 0); 
contracts/Gauge.sol:665 require(token.code.length > 0); 
contracts/Gauge.sol:672 require(token.code.length > 0); 
contracts/Gauge.sol:679 require(token.code.length > 0); 
contracts/Pair.sol:303  require(liquidity > 0, 'ILM'); 
contracts/Pair.sol:322  require(amount0 > 0 && amount1 > 0, 'ILB');
contracts/Pair.sol:336  require(amount0Out > 0 || amount1Out > 0, 'IOA'); 
contracts/Pair.sol:522  require(token.code.length > 0); 
contracts/PairFees.sol:20   require(token.code.length > 0); 

G02 - More gas efficient for loop with ++i increment and unchecked block

Loop index increments can be written as unchecked { ++i } instead of simply i++ to save gas.

G03 - Cache array length in for

Caching the array length before for loop could save gas here:

contracts/Gauge.sol:353 for (uint i = 0; i < tokens.length; i++) {  
contracts/Minter.sol:57 for (uint i = 0; i < claimants.length; i++) { 
contracts/Pair.sol:257  for (uint i = 0; i < _prices.length; i++) { 
contracts/RewardsDistributor.sol:301    for (uint i = 0; i < _tokenIds.length; i++) { 
contracts/Router.sol:90 for (uint i = 0; i < routes.length; i++) { 
contracts/Router.sol:316    for (uint i = 0; i < routes.length; i++) { 
contracts/Voter.sol:76  for (uint i = 0; i < _tokens.length; i++) { 
contracts/Voter.sol:266 for (uint i = 0; i < _gauges.length; i++) { 
contracts/Voter.sol:304 for (uint i = 0; i < _gauges.length; i++) { 
contracts/Voter.sol:310 for (uint i = 0; i < _gauges.length; i++) { 

G04 - Avoid long revert strings

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition has been met. Next revert strings are longer than 32 bytes:

contracts/Router.sol:341    require(amounts[amounts.length - 1] >= amountOutMin, 'Router: INSUFFICIENT_OUTPUT_AMOUNT'); 
contracts/Router.sol:356    require(amounts[amounts.length - 1] >= amountOutMin, 'Router: INSUFFICIENT_OUTPUT_AMOUNT'); 
contracts/Router.sol:371    require(amounts[amounts.length - 1] >= amountOutMin, 'Router: INSUFFICIENT_OUTPUT_AMOUNT');  
contracts/Router.sol:384    require(amounts[amounts.length - 1] >= amountOutMin, 'Router: INSUFFICIENT_OUTPUT_AMOUNT'); 
contracts/Router.sol:406    require(success, 'TransferHelper: ETH_TRANSFER_FAILED');  
contracts/VotingEscrow.sol:398  revert('ERC721: transfer to non ERC721Receiver implementer'); 
contracts/VotingEscrow.sol:774  require(_locked.end > block.timestamp, 'Cannot add to expired lock. Withdraw'); 
contracts/VotingEscrow.sol:786  require(unlock_time > block.timestamp, 'Can only lock until time in the future');  
contracts/VotingEscrow.sol:821  require(_locked.end > block.timestamp, 'Cannot add to expired lock. Withdraw');  
contracts/VotingEscrow.sol:1247 "dstRep would have too many tokenIds" 
contracts/VotingEscrow.sol:1317 "dstRep would have too many tokenIds" 
contracts/VotingEscrow.sol:1380 "VotingEscrow::delegateBySig: invalid signature"  
contracts/VotingEscrow.sol:1384 "VotingEscrow::delegateBySig: invalid nonce" 
contracts/VotingEscrow.sol:1388 "VotingEscrow::delegateBySig: signature expired"

G04 - unchecked block can be used for gas efficiency of the expression that can't overflow/underflow

Check comments

contracts/Gauge.sol:144 if (_fees0 / DURATION > 0) { // could be unchecked since DURATION has non-zero constant value 
contracts/Gauge.sol:151 if (_fees1 / DURATION > 0) { // could be unchecked since DURATION has non-zero constant value 
contracts/Gauge.sol:225 uint center = upper - (upper - lower) / 2; // could be unchecked due to check on L224
contracts/Gauge.sol:605 rewardRate[token] = amount / DURATION; // could be unchecked since DURATION has non-zero constant value 
contracts/Gauge.sol:615 require(rewardRate[token] <= balance / DURATION, "Provided reward too high"); // could be unchecked since DURATION has non-zero constant value 
contracts/Gauge.sol:650 rewardRate[token] = amount / DURATION; // could be unchecked since DURATION has non-zero constant value 
contracts/Minter.sol:61 active_period = ((block.timestamp + WEEK) / WEEK) * WEEK; // could be unchecked since WEEK has non-zero constant value, dividing and multiplying on which would not lead to overflow
contracts/Minter.sol:115    _period = (block.timestamp / WEEK) * WEEK; // could be unchecked since WEEK has non-zero constant value
contracts/RewardsDistributor.sol:61 return block.timestamp / WEEK * WEEK; // could be unchecked since WEEK has non-zero constant value
contracts/RewardsDistributor.sol:72 uint this_week = t / WEEK * WEEK; // could be unchecked since WEEK has non-zero constant value
contracts/RewardsDistributor.sol:145    uint rounded_timestamp = block.timestamp / WEEK * WEEK; // could be unchecked since WEEK has non-zero constant value
contracts/RewardsDistributor.sol:278    uint _last_token_time = last_token_time / WEEK * WEEK; // could be unchecked since WEEK has non-zero constant value
contracts/RewardsDistributor.sol:285    _last_token_time = _last_token_time / WEEK * WEEK; // could be unchecked since WEEK has non-zero constant value
contracts/RewardsDistributor.sol:297    _last_token_time = _last_token_time / WEEK * WEEK; // could be unchecked since WEEK has non-zero constant value
contracts/VotingEscrow.sol:138  temp /= 10; // could be unchecked since div to non-zero value
contracts/VotingEscrow.sol:144  value /= 10; // could be unchecked since div to non-zero value
contracts/VotingEscrow.sol:631  uint t_i = (last_checkpoint / WEEK) * WEEK; // could be unchecked since WEEK has non-zero constant value
contracts/VotingEscrow.sol:1016 uint t_i = (last_point.ts / WEEK) * WEEK; // could be unchecked since WEEK has non-zero constant value
contracts/VotingEscrow.sol:1144 uint[] storage _tokenIds = checkpoints[account][nCheckpoints - 1].tokenIds; // could be unchecked due to check on L1141
contracts/VotingEscrow.sol:1159 if (checkpoints[account][nCheckpoints - 1].timestamp <= timestamp) { // could be unchecked due to check on L1155
contracts/VotingEscrow.sol:1169 uint32 upper = nCheckpoints - 1; // could be unchecked due to check on L1155
contracts/VotingEscrow.sol:1171 uint32 center = upper - (upper - lower) / 2; // could be unchecked due to check on L1170
contracts/VotingEscrow.sol:1218 ? checkpoints[srcRep][srcRepNum - 1].tokenIds // could be unchecked due to statement logic
contracts/VotingEscrow.sol:1238 ? checkpoints[dstRep][dstRepNum - 1].tokenIds // could be unchecked due to statement logic
contracts/VotingEscrow.sol:1270 checkpoints[account][_nCheckPoints - 1].timestamp == _timestamp // could be unchecked due to statement logiccontracts/VotingEscrow.sol:1288 ? checkpoints[srcRep][srcRepNum - 1].tokenIds // could be unchecked due to statement 
contracts/VotingEscrow.sol:1308 ? checkpoints[dstRep][dstRepNum - 1].tokenIds  // could be unchecked due to statement 

G05 - Variables that can be changed to immutable

    string public name;
    string public symbol;

https://github.com/code-423n4/2022-05-velodrome/blob/main/contracts/contracts/Pair.sol#L13-L14

G06 - Reachable assert statements

Due to solidity docs: Assert should only be used to test for internal errors, and to check invariants. Properly functioning code should never create a Panic, not even on invalid external input. If this happens, then there is a bug in your contract which you should fix.

Multiple assert statements represented in code should be replaced to require , for example:

    function checkpoint_token() external {
        assert(msg.sender == depositor); 
        _checkpoint_token();
    }

https://github.com/code-423n4/2022-05-velodrome/blob/main/contracts/contracts/RewardsDistributor.sol#L97-L100

GalloDaSballo commented 2 years ago

G05 - Variables that can be changed to immutable

Would save 2.1k * 2 = 4200 each time they are called

Rest would save between 100 and 500 gas

GalloDaSballo commented 2 years ago

G01 - Using != 0 instead of > 0 in require statement with uint

Not anymore

G02 - More gas efficient for loop with ++i increment and unchecked block

Saves 25 gas per instance

25

G03 - Cache array length in for

3 gas per instance 10 * 3 = 30

G04 - Avoid long revert strings

6 gas per instance 14 * 6 = 84

G04 - unchecked block can be used for gas efficiency of the expression that can't overflow/underflow

20 per instance 26 * 20 = 520

G05 - Variables that can be changed to immutable

4200 per above

G06 - Reachable assert statements

Will not save gas

Report is short and sweet, total saved: 4859