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

2 stars 0 forks source link

Gas Optimizations #192

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

> 0 is less efficient than != 0 for uint in require condition

Ref: https://twitter.com/GalloDaSballo/status/1485430908165443590

$ grep -Rn "> 0" ./contracts/ | grep require
./contracts/MasterChef.sol:186:        require(user.amount > 0, "MasterChef: nothing to withdraw");
./contracts/StakingRewards.sol:94:        require(amount > 0, "Cannot stake 0");
./contracts/StakingRewards.sol:108:        require(amount > 0, "Cannot withdraw 0");

Use custom errors

Solidity ^0.8.4 allow the use of custom errors to optimize gas usage. https://blog.soliditylang.org/2021/04/21/custom-errors/

Variables that could be set immutable

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

    uint256 public startLiquidity;
    uint256 public step;

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

    IERC20 public rewardsToken;
    IERC20 public stakingToken;
    address public rewardsDistribution;

Use uint256 instead of bool

Use uint(1) instead of bool(true) to save gas by avoiding masking ops https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/Shelter.sol#L17

    mapping(IERC20 => mapping(address => bool)) public override claimed;

Cache block.timestamp + GRACE_PERIOD to activated[_token]

Since we are only interested in activated[_token] + GRACE_PERIOD, might as well cache it when we call activate. https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/Shelter.sol#L38

    function activate(IERC20 _token) external override onlyClient {
        activated[_token] = block.timestamp + GRACE_PERIOD;
        savedTokens[_token] = _token.balanceOf(address(this));
        emit ShelterActivated(_token);
    }

    function deactivate(IERC20 _token) external override onlyClient {
        require(activated[_token] != 0 && activated[_token] > block.timestamp, "too late");
        activated[_token] = 0;
        savedTokens[_token] = 0;
        _token.safeTransfer(msg.sender, _token.balanceOf(address(this)));
        emit ShelterDeactivated(_token);
    }

    function withdraw(IERC20 _token, address _to) external override {
        require(activated[_token] != 0 && activated[_token] < block.timestamp, "shelter not activated");
        uint256 amount = savedTokens[_token] * client.shareOf(_token, msg.sender) / client.totalShare(_token);
        claimed[_token][_to] = true;
        emit ExitShelter(_token, msg.sender, _to, amount);
        _token.safeTransfer(_to, amount);
    }

Unchecked safe math

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

            total.usdm += _deposits.usdm;
            user.usdm += _deposits.usdm;

can be changed to

            total.usdm += _deposits.usdm;
            unchecked{user.usdm += _deposits.usdm;}

assuming user.usdm always <= total.usdm

similarly

            total.pool3 += _deposits.pool3;
            user.pool3 += _deposits.pool3;

can be changed to

            total.pool3 += _deposits.pool3;
            unchecked{user.pool3 += _deposits.pool3;}

float multiplication optimization

// out = x * y unchecked{/} z
function fmul(uint256 x, uint256 y, uint256 z) internal pure returns(uint256 out){
assembly{
    if iszero(eq(div(mul(x,y),x),y)) {revert(0,0)}
    out := div(mul(x,y),z)
}
}

can be applied to floating multiplication such as in https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/ConvexStakingWrapper.sol#L180

            d_reward = (d_reward * 4) / 5;
GalloDaSballo commented 2 years ago

0 is less efficient than != 0 for uint in require condition 9 gas

Use custom errors Not useful without explanation

Variables that could be set immutable 5 * 2100 -> 10500

Use uint256 instead of bool 3 gas I believe

Cache block.timestamp + GRACE_PERIOD to activated[_token] Would save 3 gas on withdraw

Unchecked safe math would save 40 gas

float multiplication optimization Not exactly sure if it will save more than just using unchecked, in lack of math / poc will give it 20 gas

Gas Saved 10575

Report is short and sweet, I appreciate it