code-423n4 / 2022-04-jpegd-findings

1 stars 1 forks source link

Gas Optimizations #169

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

[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.

[S] Use else if can save gas

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/farming/LPFarming.sol#L271-L275

        if (blockNumber < epoch.startBlock) return epoch.startBlock;

        if (blockNumber > epoch.endBlock) return epoch.endBlock;

        return blockNumber;

Change to else if can save gas:

        if (blockNumber < epoch.startBlock) {
            return epoch.startBlock;
        } 
        else if (blockNumber > epoch.endBlock){
            return epoch.endBlock;
        } 
        return blockNumber;

[S] 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:

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/farming/LPFarming.sol#L348-L351

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol#L145-L150

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol#L319-L331

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

Using ++i is more gas efficient than i++, especially in for loops.

For example:

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/farming/LPFarming.sol#L348-L348

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

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol#L145-L145

        for (uint256 i = 0; i < _strategyConfig.rewardTokens.length; i++) 

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol#L231-L231

        for (uint256 i = 0; i < length; i++)

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol#L319-L319

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

[G-S] Using immutable variables can save gas

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/staking/JPEGStaking.sol#L17-L18

    /// @notice The stake token, JPEG
    IERC20Upgradeable public jpeg;

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/staking/JPEGStaking.sol#L21-L26

    function initialize(IERC20Upgradeable _jpeg) external initializer {
        __ReentrancyGuard_init();
        __ERC20_init("sJPEG", "sJPEG");
        __ERC20Permit_init("sJPEG");
        jpeg = _jpeg;
    }

Considering that jpeg will never change, changing it to immutable variable instead of storage variable can save gas.

[S] Outdated versions of OpenZeppelin library

Outdated versions of OpenZeppelin library are used.

It's a best practice to use the latest version of libraries.

New versions of OpenZeppelin libraries can be more gas efficient.

For example:

ERC20.sol in @openzeppelin/contracts@4.3.1:

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/yarn.lock#L794-L797

"@openzeppelin/contracts@^4.0.0":
  version "4.3.1"
  resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-4.3.1.tgz#c01f791ce6c9d3989ac1a643267501dbe336b9e3"
  integrity sha512-QjgbPPlmDK2clK1hzjw2ROfY8KA5q+PfhDUUxZFEBCZP9fi6d5FuNoh/Uq0oCTMEKPmue69vhX2jcl0N/tFKGw==

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.3.1/contracts/token/ERC20/ERC20.sol#L154-L160

    function transferFrom(
        address sender,
        address recipient,
        uint256 amount
    ) public virtual override returns (bool) {
        _transfer(sender, recipient, amount);

        uint256 currentAllowance = _allowances[sender][_msgSender()];
        require(currentAllowance >= amount, "ERC20: transfer amount exceeds allowance");
        unchecked {
            _approve(sender, _msgSender(), currentAllowance - amount);
        }

        return true;
    }

A gas optimization upgrade has been added to @openzeppelin/contracts@4.5.0:

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.5.0/contracts/token/ERC20/ERC20.sol#L163-L165

    function transferFrom(
        address from,
        address to,
        uint256 amount
    ) public virtual override returns (bool) {
        address spender = _msgSender();
        _spendAllowance(from, spender, amount);
        _transfer(from, to, amount);
        return true;
    }

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.5.0/contracts/token/ERC20/ERC20.sol#L330-L342

    function _spendAllowance(
        address owner,
        address spender,
        uint256 amount
    ) internal virtual {
        uint256 currentAllowance = allowance(owner, spender);
        if (currentAllowance != type(uint256).max) {
            require(currentAllowance >= amount, "ERC20: insufficient allowance");
            unchecked {
                _approve(owner, spender, currentAllowance - amount);
            }
        }
    }