code-423n4 / 2022-02-concur-findings

2 stars 0 forks source link

Gas Optimizations #247

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

GAS OPTIMIZATION

--1 -unnecessary i value set https://github.com/code-423n4/2022-02-concur/blob/main/contracts/ConcurRewardPool.sol#L35 unnecessary value set. the default value of uint is zero. just use:

uint i;

--2 -better increment https://github.com/code-423n4/2022-02-concur/blob/main/contracts/ConcurRewardPool.sol#L35 change i++ to ++i --3 -Best way to use SafeERC20.function for gas opt https://github.com/code-423n4/2022-02-concur/blob/main/contracts/ConcurRewardPool.sol#L10 by not declare:

using SafeERC20 for IERC20;

and use:

SafeERC20.safeTransfer(IERC20(_tokens[i]), msg.sender, getting);

can safe gas usage. --4 -unnecessary variable declaration -Unnecessary (uint) getting variable declaration https://github.com/code-423n4/2022-02-concur/blob/main/contracts/ConcurRewardPool.sol#L36 instead of caching reward[msg.sender][_tokens[i]] in getting, better just pass it directly into safeTransfer() function. getting was declared and called once per loop. It cost more gas.

recommended mitigation:

IERC20(_tokens[i]).safeTransfer(msg.sender, reward[msg.sender][_tokens[i]]); // L 36

ConvexStakingWrapper.sol

--5 -Using storage to declare Struct variable inside function https://github.com/code-423n4/2022-02-concur/blob/main/contracts/ConvexStakingWrapper.sol#L171 instead of caching rewardType to memory. read it directly from storage.

RewardType storage reward = rewards[_pid][_index];

--6 -Using uint instead struct https://github.com/code-423n4/2022-02-concur/blob/main/contracts/ConvexStakingWrapper.sol#L24 The Reward struct merely contain 1 property. Instead of declaring it as struct, better as a uint example: uint128 integralReward(example name); https://github.com/code-423n4/2022-02-concur/blob/main/contracts/ConvexStakingWrapper.sol#L44 then modify the userReward mapping output from Reward to uint.

MasterChef.sol

--7 -unnecessary totalAllocPoint & _pid value set https://github.com/code-423n4/2022-02-concur/blob/main/contracts/MasterChef.sol#L51 https://github.com/code-423n4/2022-02-concur/blob/main/contracts/MasterChef.sol#L129 unnecessary value set. the default value of uint is zero. just use:

uint public totalAllocPoint;

--8 -use constant for gas saving https://github.com/code-423n4/2022-02-concur/blob/main/contracts/MasterChef.sol#L57 use constant to declare _perMille & _concurShareMultiplier uint --9 -Unused library https://github.com/code-423n4/2022-02-concur/blob/main/contracts/MasterChef.sol#L15 unused SafeERC20 lib

Shelter.sol

--10 -&& is more expensive gas https://github.com/code-423n4/2022-02-concur/blob/main/contracts/Shelter.sol#L45 using multiple require() is cheaper than use &&

require(activated[_token] != 0 , "too late");
require(activated[_token] + GRACE_PERIOD > block.timestamp, "too late");

--11 -unnecessary value set https://github.com/code-423n4/2022-02-concur/blob/main/contracts/StakingRewards.sol#L21-L22 unnecessary value set. the default value of uint is zero for rewardRate & periodFinish. uint default value is 0 --12 -step declaration https://github.com/code-423n4/2022-02-concur/blob/main/contracts/USDMPegRecovery.sol#L22 setting the step value directly and use constant can save gas. then remove the line: https://github.com/code-423n4/2022-02-concur/blob/main/contracts/USDMPegRecovery.sol#L56 --13 -using storage to declare struct inside function https://github.com/code-423n4/2022-02-concur/blob/main/contracts/USDMPegRecovery.sol#L91 instead of caching Liquidity in memory. just read it directly to storage

Liquidity storage total = totalLiquidity;

it can save gas. And same for user variable right below it. --14 -unnecessary math operation https://github.com/code-423n4/2022-02-concur/blob/main/contracts/ConvexStakingWrapper.sol#L116-L117 the value of registeredRewards[_pid][crv] and registeredRewards[_pid][cvx] is fixed, Set the value(1 and 2) directly then add additional info which explain the math operation behind it:

            registeredRewards[_pid][crv] = 1; //mark registered CRV_INDEX + 1
            registeredRewards[_pid][cvx] = 2; //mark registered CVX_INDEX + 1

--15 -use calldata to store signature https://github.com/code-423n4/2022-02-concur/blob/main/contracts/EasySign.sol#L51 replace memory with calldata to save gas --16 -Using immutable to declare variable which set once at constructor https://github.com/code-423n4/2022-02-concur/blob/main/contracts/MasterChef.sol#L52-L54 startBlock, endBlock & concur are set once in constructor. Use immutable --17 -use require() to validate instead if() https://github.com/code-423n4/2022-02-concur/blob/main/contracts/MasterChef.sol#L137 by using:

    require(block.number > pool.lastRewardBlock);

to replace:

        if (block.number <= pool.lastRewardBlock) {
            return;
        }

can save gas with the same output 18-- -Unncessary multiplier (uint) declaration https://github.com/code-423n4/2022-02-concur/blob/main/contracts/MasterChef.sol#L150-L151 multiplier is called once in updatePool(). Its gas consuming to cache getMultiplier return in multiplier. Remove line 150, and change line it to:

uint concurReward = getMultiplier(pool.lastRewardBlock, block.number).mul(concurPerBlock).mul(pool.allocPoint).div(totalAllocPoint);
GalloDaSballo commented 2 years ago

-unnecessary i value set

3 gas

-better increment

3

-Best way to use SafeERC20.function for gas opt

Disagree especially with optimizer on

-unnecessary variable declaration

6 gas

-Using storage to declare Struct variable inside function

Disagree wholeheartedly as the cost of reading from storage is way higher

-Using uint instead struct

Valid but no POC on savings so 0

-unnecessary totalAllocPoint & _pid value set

103 gas saved

-use constant for gas saving

2100 * 2 = 4200

-Unused library

Won't cost more gas

-&& is more expensive gas

You're storing 2 bytes 32 (2500 gas), also reading from storage twice, are you sure this saves gas?

-unnecessary value set

200

-step declaration

2100

-using storage to declare struct inside function

Happy to disagree

-unnecessary math operation

Saves 6 gas

-use calldata to store signature

467 gas (h/t #245)

-Using immutable to declare variable which set once at constructor

2100 * 3 = 6300

-use require() to validate instead if()

Changes the functionality

-Unncessary multiplier (uint) declaration

6 gas saved

Honestly want to commend the warden for having a different opinion, formatting can be improved and most importantly would recommend they add POC when they make "storage can save gas" statements

Total Gas Saved: 13394