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

2 stars 0 forks source link

Gas Optimizations #211

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

C4-001 : Adding unchecked directive can save gas

Impact - Gas Optimization

Using the unchecked keyword to avoid redundant arithmetic underflow/overflow checks to save gas when an underflow/overflow cannot happen. E.g. 'unchecked' can be applied in the following lines of code since there are require statements before to ensure the arithmetic operations would not cause an integer underflow or overflow. For the arithmetic operations that will never over/underflow, using the unchecked directive (Solidity v0.8 has default overflow/underflow checks) can save some gas from the unnecessary internal over/underflow checks.

Proof of Concept

  1. Navigate to the following contract function and lines.
https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/StakingRewards.sol#L145

https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/StakingRewards.sol#L161

https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/ConvexStakingWrapper.sol#L279

Tools Used

Code Review

Recommended Mitigation Steps

Consider applying unchecked arithmetic where overflow/underflow is not possible.

C4-002 : > 0 can be replaced with != 0 for gas optimization

Impact - Gas Optimization

!= 0 is a cheaper operation compared to > 0, when dealing with uint.

Proof of Concept

  1. Navigate to the following contract function and lines.
https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/MasterChef.sol#L162

https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/MasterChef.sol#L170

https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/MasterChef.sol#L194

https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/ConvexStakingWrapper.sol#L219

Tools Used

Code Review

Recommended Mitigation Steps

Use "!=0" instead of ">0" for the gas optimization.

C4-003 : ++i is more gas efficient than i++ in loops forwarding

Impact - Gas Optimization

++i is more gas efficient than i++ in loops forwarding.

Proof of Concept

  1. Navigate to the following contracts.
https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/ConcurRewardPool.sol#L35

Tools Used

Code Review

Recommended Mitigation Steps

It is recommend to use unchecked{++i} and change i declaration to uint256.

C4-004 : Cache array length in for loops can save gas

Impact - Gas Optimization

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.

Caching the array length in the stack saves around 3 gas per iteration.

Proof of Concept

  1. Navigate to the following contracts.
https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/ConcurRewardPool.sol#L35

Tools Used

Code Review

Recommended Mitigation Steps

Consider to cache array length.

C4-005 : Less than 256 uints are not gas efficient

Impact - Gas Optimization

Lower than uint256 size storage instance variables are actually less gas efficient. E.g. using uint16 does not give any efficiency, actually, it is the opposite as EVM operates on default of 256-bit values so uint16 is more expensive in this case as it needs a conversion. It only gives improvements in cases where you can pack variables together, e.g. structs.

Proof of Concept

  1. Navigate to the following contracts.
https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/MasterChef.sol#L43

Tools Used

None

Recommended Mitigation Steps

Consider to review all uint types. Change them with uint256 If the integer is not necessary to present with uint16.`

C4-006 : State variables could be declared constant

Impact - Gas Optimization

State variables that never change can be declared constant. This can greatly reduce gas costs.

Proof of Concept

  1. Navigate to the following contract variables.
https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/MasterChef.sol#L50

https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/MasterChef.sol#L56

https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/USDMPegRecovery.sol#L22

Tools Used

Code Review

Recommended Mitigation Steps

Add the constant keyword for state variables whose value never change.

C4-007 : Immutable Variables

Impact - Gas Optimization

'immutable' greatly reduces gas costs. There are variables that do not change so they can be marked as immutable to greatly improve the gas costs.

Proof of Concept

  1. For instance : https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/MasterChef.sol#L54
  2. https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/VoteProxy.sol#L9
  3. https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/StakingRewards.sol#L20
  4. https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/StakingRewards.sol#L19

Tools Used

Code Review

Recommended Mitigation Steps

Mark variables as immutable.

C4-008 : There is no need to assign default values to variables

Impact - Gas Optimization

When a variable is declared solidity assigns the default value. In case the contract assigns the value again, it costs extra gas.

Example: uint x = 0 costs more gas than uint x without having any different functionality.

Proof of Concept

https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/StakingRewards.sol#L21

https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/StakingRewards.sol#L22

https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/ConcurRewardPool.sol#L35

Tools Used

Code Review

Recommended Mitigation Steps

uint x = 0 costs more gas than uint x without having any different functionality.

C4-009 : SafeMath Is Not Required After Solidity 0.8.x

Impact

SafeMath library functions are not always used in arithmetic operations in the contracts, which could potentially cause integer underflow/overflows. Although in the reference lines of code, there are upper limits on the variables to ensure an integer underflow/overflow could not happen, using SafeMath is always a best practice, which prevents underflow/overflows completely (even if there were no assumptions on the variables) and increases code consistency as well.

Proof of Concept

  1. Navigate to the following contracts.
https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/MasterChef.sol#L14
  1. SafeMath functions are not used in the every functionality.

Tools Used

Code Review

Recommended Mitigation Steps

Consider using the SafeMath library functions in the referenced lines of code.

C4-010 : Cache external call results can save gas

Impact

Every call to an external contract costs a decent amount of gas. For optimization of gas usage, external call results should be cached if they are being used for more than one time.

Proof of Concept

  1. Navigate to the following contracts.
https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/ConcurRewardPool.sol#L37

Tools Used

Code Review

Recommended Mitigation Steps

Cache external call for the gas optimization. Example can be seen from below.

function _allowTokenIfNecessary(address token) internal {
    address lendingPool = address(_lendingPool());
    if (IERC20(token).allowance(address(this), lendingPool) < type(uint256).max / 2) {
        IERC20(token).approve(lendingPool, type(uint256).max);
    }
}

C4-011 : Redundant Import

Impact - Gas Optimization

Safemath is an unnecessary import in all contracts since it is used solely for development. It can therefore be removed.

Proof of Concept

  1. Navigate to the following contracts.
https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/MasterChef.sol#L10

Tools Used

Code Review

Recommended Mitigation Steps

Consider to delete redundant import.

C4-012 : Gas Optimization on the Public Functions

Impact

This does not directly impact the smart contract in anyway besides cost. This is a gas optimization to reduce cost of smart contract. Calling each function, we can see that the public function uses 496 gas, while the external function uses only 261.

Proof of Concept

According to Slither Analyzer documentation (https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-external), there are functions in the contract that are never called. These functions should be declared as external in order to save gas.

Slither Detector:

external-function:

https://github.com/code-423n4/2022-02-concur/blob/02d286253cd5570d4e595527618366f77627cdaf/contracts/ConvexStakingWrapper.sol#L93

Tools Used

Slither

Recommended Mitigation Steps

  1. Get Smart Contracts from the Repository.
  2. Create a python virtual environment with a stable python version.
  3. Install Slither Analyzer on the python VEM.
  4. Run Slither against all contracts.
  5. Define public functions as an external for the gas optimization.
GalloDaSballo commented 2 years ago

C4-001 : Adding unchecked directive can save gas

3 * 20 = 60

C4-002 : > 0 can be replaced with != 0 for gas optimization

6 per instance = 24

C4-003 : ++i is more gas efficient than i++ in loops forwarding

3

C4-004 : Cache array length in for loops can save gas

3

C4-005 : Less than 256 uints are not gas efficient

No Poc = No Points

C4-006 : State variables could be declared constant

3 variables without further detail, will give one Cold SLOAD per var = 6300

C4-007 : Immutable Variables

Same as above, 4 * 2100 = 8400

C4-008 : There is no need to assign default values to variables

2 * 100 = 200 3 for MSTORE

C4-009 : SafeMath Is Not Required After Solidity 0.8.x

No deets = No points

C4-010 : Cache external call results can save gas

Nope

C4-011 : Redundant Import

Doesn't save gas

C4-012 : Gas Optimization on the Public Functions

Savings here should be because of the dispatcher for that reason and lack of detailed POC I'm not going to add the gas savings

Overall the report feels like a direct output from C4 + one finding which was manually typed.

That said the report is fluent to read. Would ask the warden to add links to the findings as links rather than as code to make it even more convenient to check and address them

Total Gas Saved: 14993