Gas Optimizations

[code423n4]

[S]: Suggested optimation, save a decent amount of gas without compromising readability;
[M]: Minor optimation, the amount of gas saved is minor, change when you see fit;
[N]: Non-preferred, the amount of gas saved is at cost of readability, only apply when gas saving is a top priority.

[S1] Cache external call result in the stack can save gas

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.

For example:

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

function addRewards(uint256 _pid) public {
    address mainPool = IRewardStaking(convexBooster)
        .poolInfo(_pid)
        .crvRewards;
    if (rewards[_pid].length == 0) {
        pids[IRewardStaking(convexBooster).poolInfo(_pid).lptoken] = _pid;
        // ...
    }
    // ...
}

IRewardStaking(convexBooster).poolInfo(_pid) can be cached to avoid an extra external call.

[S2] Cache external call result in storage can save gas

For the unchanged results of an external call that will be reused multiple times, cache and read from storage rather than initiate a fresh external call can save gas.

Instances include:

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/ConvexStakingWrapper.sol#L237-L239

IERC20 lpToken = IERC20(
    IRewardStaking(convexPool[_pid]).poolInfo(_pid).lptoken
);

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/ConvexStakingWrapper.sol#L264-L266

IERC20 lpToken = IERC20(
    IRewardStaking(convexPool[_pid]).poolInfo(_pid).lptoken
);

lpToken of _pid can be cached when addRewards() to avoid extra external calls.

[S3] `SafeMath is no longer needed

SafeMath is no longer needed starting with Solidity 0.8. The compiler now has built in overflow checking.

Removing SafeMath can save some gas.

Instances include:

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/MasterChef.sol#L89-L89

totalAllocPoint = totalAllocPoint.add(_allocationPoints);

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/MasterChef.sol#L109-L109

return _to.sub(_from);

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/MasterChef.sol#L120-L121

uint concurReward = multiplier.mul(concurPerBlock).mul(pool.allocPoint).div(totalAllocPoint);
accConcurPerShare = accConcurPerShare.add(concurReward.mul(_concurShareMultiplier).div(lpSupply));

[S4] Change unnecessary storage variables to constants can save gas

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

uint private _concurShareMultiplier = 1e18;
uint private _perMille = 1000; // 100%

Some storage variables include _concurShareMultiplier, _perMille will never be changed and they should not be.

Changing them to constant can save gas.

[M5] Setting bool variables to false is redundant

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/MasterChef.sol#L204-L204

bool transferSuccess = false;

Setting bool variables to false is redundant as they default to false.

See https://docs.soliditylang.org/en/v0.8.11/control-structures.html#default-value

[S6] Using immutable variable can save gas

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/MasterChef.sol#L52-L71

uint public startBlock;
uint public endBlock; // The block number when mining starts.
IERC20 public concur;

uint private _concurShareMultiplier = 1e18;
uint private _perMille = 1000; // 100%

constructor(IERC20 _concur, uint _startBlock, uint _endBlock) Ownable() {
    startBlock = _startBlock;
    endBlock = _endBlock;
    concur = _concur;
    // ...
}

Considering that startBlock, endBlock and concur will never change, changing them to immutable variables instead of storages variable can save gas.

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/StakingRewards.sol#L19-L20

    IERC20 public rewardsToken;
    IERC20 public stakingToken;

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/StakingRewards.sol#L37-L47

    constructor(
        address _rewardsDistribution,
        address _rewardsToken,
        address _stakingToken,
        MasterChef _masterChef
    ) {
        rewardsToken = IERC20(_rewardsToken);
        stakingToken = IERC20(_stakingToken);
        rewardsDistribution = _rewardsDistribution;
        masterChef = _masterChef;
    }

Considering that rewardsToken and stakingToken will never change, changing them to immutable variables instead of storages variable can save gas.

[M7] Use short reason strings can save gas

Every reason string takes at least 32 bytes.

Use short reason strings that fits in 32 bytes or it will become more expensive.

Instances include:

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/StakingRewards.sol#L179-L182

require(
    block.timestamp > periodFinish,
    "Previous rewards period must be complete before changing the duration for the new period"
);

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/StakingRewards.sol#L191-L194

require(
    block.timestamp > periodFinish,
    "Previous rewards period must be complete before changing the duration for the new period"
);

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/StakingRewards.sol#L137-L140

require(
    msg.sender == rewardsDistribution,
    "Caller is not RewardsDistribution contract"
);

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/StakingRewards.sol#L170-L173

require(
    tokenAddress != address(stakingToken),
    "Cannot withdraw the staking token"
);

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/MasterChef.sol#L210-L210

require(transferSuccess, "safeConcurTransfer: transfer failed");

[M8] Setting uint256 variables to 0 is redundant

Setting uint256 variables to 0 is redundant as they default to 0.

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

    uint256 public periodFinish = 0;
    uint256 public rewardRate = 0;

Recommendation

Change to uint256 public periodFinish; uint256 public rewardRate; can make the code simpler and save some gas.

[M9] Adding unchecked directive can save gas

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.

For example:

1. https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/StakingRewards.sol#L88-L101

function stake(uint256 amount)
    external
    nonReentrant
    whenNotPaused
    updateReward(msg.sender)
{
    require(amount > 0, "Cannot stake 0");
    _totalSupply += amount;
    _balances[msg.sender] += amount;
    stakingToken.safeTransferFrom(msg.sender, address(this), amount);
    uint256 pid = masterChef.pid(address(stakingToken));
    masterChef.deposit(msg.sender, pid, amount);
    emit Staked(msg.sender, amount);
}

_balances[msg.sender] += amount will never overflow if _totalSupply += amount; does not revert.

2. https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/StakingRewards.sol#L103-L115

function withdraw(uint256 amount)
    public
    nonReentrant
    updateReward(msg.sender)
{
    require(amount > 0, "Cannot withdraw 0");
    _totalSupply -= amount;
    _balances[msg.sender] -= amount;
    stakingToken.safeTransfer(msg.sender, amount);
    uint256 pid = masterChef.pid(address(stakingToken));
    masterChef.withdraw(msg.sender, pid, amount);
    emit Withdrawn(msg.sender, amount);
}

_totalSupply -= amount will never underflow if _balances[msg.sender] -= amount; does not underflow revert.

Therefore it can be changed to for gas saving:

    _balances[msg.sender] -= amount;
    unchecked {
        _totalSupply -= amount;
    }

[M10] "> 0" is less efficient than "!= 0" for unsigned integers

It is cheaper to use != 0 than > 0 for uint256.

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/StakingRewards.sol#L94-L94

require(amount > 0, "Cannot stake 0");

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/StakingRewards.sol#L108-L108

require(amount > 0, "Cannot withdraw 0");

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/StakingRewards.sol#L119-L119

if (reward > 0) {

[M11] ++i is more efficient than i++

Using ++i is more gas efficient than i++, especially in a loop.

For example:

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/ConvexStakingWrapper.sol#L121-L121

for (uint256 i = 0; i < extraCount; i++) {

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

for (uint256 i = 0; i < rewardCount; i++) {

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

for (uint256 i = 0; i < _tokens.length; i++) {

[S12] Reuse existing external call's cache can save gas

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/ConvexStakingWrapper.sol#L120-L139

uint256 extraCount = IRewardStaking(mainPool).extraRewardsLength();
for (uint256 i = 0; i < extraCount; i++) {
    address extraPool = IRewardStaking(mainPool).extraRewards(i);
    address extraToken = IRewardStaking(extraPool).rewardToken();
    if (extraToken == cvx) {
        //no-op for cvx, crv rewards
        rewards[_pid][CVX_INDEX].pool = extraPool;
    } else if (registeredRewards[_pid][extraToken] == 0) {
        //add new token to list
        rewards[_pid].push(
            RewardType({
                token: IRewardStaking(extraPool).rewardToken(),
                pool: extraPool,
                integral: 0,
                remaining: 0
            })
        );
        registeredRewards[_pid][extraToken] = rewards[_pid].length; //mark registered at index+1
    }
}

IRewardStaking(extraPool).rewardToken() at L131 is already cached in the local variable extraToken at L123.

Reusing the cached local variable in the stack instead of initiating an external call again can save gas.

[N13] Unnecessary checked arithmetic in for loops

There is no risk of overflow caused by increamenting the iteration index in for loops (the i++ in for for (uint256 i = 0; i < rewardCount; i++)).

Increments perform overflow checks that are not necessary in this case.

Recommendation

Surround the increment expressions with an unchecked { ... } block to avoid the default overflow checks. For example, change the for loop:

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

for (uint256 i = 0; i < rewardCount; i++) {
    _calcRewardIntegral(_pid, i, _account, depositedBalance, supply);
}

to:

for (uint256 i = 0; i < rewardCount;) {
    _calcRewardIntegral(_pid, i, _account, depositedBalance, supply);
    unchecked { ++i; }
}

[M14] Cache array length in for loops can save gas

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.

Instances include:

• ConcurRewardPool.sol#claimRewards()

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

[GalloDaSballo]

[S1] Cache external call result in the stack can save gas

Would save 100 gas for STATICALL + the cost of reading from Storage again, let's say another 100, at the cost of 6 for MSTORE + MLOAD = 194 gas saved

[S2] Cache external call result in storage can save gas

Would save 91 gas (First call costs 6 more, second costs 97 less)

[S3] `SafeMath is no longer needed

In contrast to other findings the warden has listed all instances that are unnecessary, for thus reason I will add the gas savings to this report
8 instances, 20 gas per instance = 160 gas saved

[S4] Change unnecessary storage variables to constants can save gas

Because the warden didn't list the other savings, I'll give one COLD SLOAD per variable
2100 * 2 = 4200

[M5] Setting bool variables to false is redundant

3 gas

[S6] Using immutable variable can save gas

Similar to S4
5 * 2100 = 10500

[M7] Use short reason strings can save gas

5 * 2500 per discussion on other reports
12500

[M8] Setting uint256 variables to 0 is redundant

200

[M9] Adding unchecked directive can save gas

2 * 20 = 40

[M10] "> 0" is less efficient than "!= 0" for unsigned integers

Only for require
2 * 6 = 12

[M11] ++i is more efficient than i++

3 * 3 = 9

[S12] Reuse existing external call's cache can save gas

91 gas saved (97 - 6)

[N13] Unnecessary checked arithmetic in for loops

20 gas

[M14] Cache array length in for loops can save gas

3 gas

Overall the report is short and sweet, the formatting is excellent and there's a little more detail than in other reports.
Would have liked to see the storage to constant gas savings explicitly shown as that would have made the report as close to complete as it could have been.

Total Gas Saved:
28023