Gas Optimizations

[code423n4]

1

Title: Using != is more efficient https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L114 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L218 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L239 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/staking/JPEGStaking.sol#L32 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/staking/JPEGStaking.sol#L46 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/yVault/yVault.sol#L143 Using != instead of > is more gas efficient for checking that the var is not 0

2

Title: Using ERC20.function instead of SafeERC20.function lib for jpeg https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L128 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L130 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L339 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L356 ERC20.functions is way cheaper to use. Its unnecessary to use SafeERC20 lib because jpeg is ERC20 standard token

3

Title: Using if statement instead else if https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L129 The execution on condition in L127 and L129 won't executed at the same time. Using if statement can reduce 37 gas consumption

        if (remainingRewards > newRewards) {
            jpeg.safeTransfer(msg.sender, remainingRewards - newRewards);
        }
    if (remainingRewards < newRewards) { //@audit-info: Replacing else if with if statement here
            jpeg.safeTransferFrom(
                msg.sender,
                address(this),
                newRewards - remainingRewards
        );

4

Title: gas opt in set() function https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L159 Checking that prevAllocPoint != _allocPoint early can save gas consumption (or just throw an error message earlier), than just execute all the line above it Change to:

    function set(uint256 _pid, uint256 _allocPoint) external onlyOwner {
    _massUpdatePools();
    uint256 prevAllocPoint = poolInfo[_pid].allocPoint;
        if (prevAllocPoint != _allocPoint) { //@audit-info: check here

            poolInfo[_pid].allocPoint = _allocPoint; //@audit-info: prevent SSTORE here

                totalAllocPoint = totalAllocPoint - prevAllocPoint + _allocPoint;
        }
    }

Its will prevent unnecessary SSTORE

5

Title: Using += to increase value on var https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L226 Change:

        user.amount = user.amount + _amount;

To

        user.amount += _amount;

Also at: https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L145

6

Title: Using delete statement to empty userReward https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L340 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L357 Change:

        userRewards[msg.sender] = 0;

To:

    delete userRewards[msg.sender];

7

Title: Gas improvement on returning _withdrawReward value https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L315-L327 By setting pending in function returns and deleting L326 can save gas. Change to:

    function _withdrawReward(uint256 _pid) internal returns (uint256 pending) { //@audit-info: return
        UserInfo storage user = userInfo[_pid][msg.sender];
        uint256 pending = (user.amount *
            (poolInfo[_pid].accRewardPerShare - user.lastAccRewardPerShare)) /
            1e36;
        if (pending > 0) {
            userRewards[msg.sender] += pending;
        }

        user.lastAccRewardPerShare = poolInfo[_pid].accRewardPerShare;
    //@audit-info: no return
    }

Same at: https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/yVaultLPFarming.sol#L177-L186

8

Title: Unnecessary var init with default value https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L281 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L348 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol#L231 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/NFTVault.sol#L181-L184 Declaring uint with 0 value is gas consuming. Just remove = 0

9

Title: Using unchecked and prefix increment https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L281 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L348 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol#L231 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/NFTVault.sol#L181-L184 Change to:

        for (uint256 pid = 0; pid < length;) {
            _updatePool(pid);
    unchecked{
        ++pid //@audit-info: Place here with unchecked
        };
        }

10

Title: Caching poolInfo.length can save gas https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/LPFarming.sol#L348 As the loop in _massUpdatePools which caching poolInfo.length, same method can be used in claimAll function

11

Title: Unnecessary MSTORE newReward https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/yVaultLPFarming.sol#L170 MSTORing the calculation is unnecessary and cost gas. Just pass cuerrnetBalance - previousBalance to the newAccRewardPerShare calculation

newAccRewardsPerShare = accRewardPerShare + (currentBalance - previousBalance) * 1e36 / totalStaked;

12

Title: Using msg.sender instead _msgSender() https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/tokens/StableCoin.sol#L26 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/tokens/StableCoin.sol#L40 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/tokens/StableCoin.sol#L54 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/tokens/StableCoin.sol#L68 Using msg.sender instead _msgSender() is more effective. I recommend to replace all _msgSender() with msg.sender

13

Title: Using && is less gas optimum in require https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/tokens/StableCoin.sol#L68 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/yVault/yVault.sol#L100 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/FungibleAssetVaultForDAO.sol#L93-L98 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/FungibleAssetVaultForDAO.sol#L194 Using multiple require() than && can save 15 execution gas fee, however it cost more on deployment gas fee(better use on function which are called many times):

require(
            _performanceFee.denominator > 0,
            "INVALID_RATE"
        );
require(
                _performanceFee.denominator >= _performanceFee.numerator,
            "INVALID_RATE"
        );

14

Title: Using delete statement to set mapping false https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/yVault/Controller.sol#L76 Using delete statement to set mapping with bool value to false is more effective

        delete approvedStrategies[_token][_strategy];

15

Title: Don't need to use == true to validate bool value == true https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/yVault/Controller.sol#L87 Change to:

require(
            approvedStrategies[_token][_strategy], // @audit-info remove == true
            "STRATEGY_NOT_APPROVED"
        );

16

Title: Using calldata to declare read only struct parameter https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/FungibleAssetVaultForDAO.sol#L70 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/NFTVault.sol#L212 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/NFTVault.sol#L222 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/NFTVault.sol#L232 Using calldata to store _creditLimitRate can save gas because it is read only var

17

Title: Using if statement to determine amount value https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/FungibleAssetVaultForDAO.sol#L182 Changing the code to:

if(amount > debtAmount)amount = debtAmount;

Can save execution gas fee in most case:

• if amount < debtAmount, it will save 16 gas because of avoiding MSTORE
• if amount == debtAmount, same
• if amount > debtAmount, 2 more expensive gas cost. Considering that WHITELISTED_ROLE will inputting amount is > than debtAmount is the rarest case from all cases above, this way is the most effective way

18

Title: Using unchecked to calculate collateralAmount in withdraw() https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/FungibleAssetVaultForDAO.sol#L199 amount value was checked that it won't > than collateralAmount so using unchecked can save gas:

unchecked{
    collateralAmount -= amount;
}

19

Title: Unnecessary creditLimit MSTORE in withdraw() https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/FungibleAssetVaultForDAO.sol#L196 By passing getCreditLimit(collateralAmount - amount) directly to L197 without storing it to credit limit can save gas without damaging readability of the code (The function name is clear that we are getting credit limit value):

require(getCreditLimit(collateralAmount - amount) >= debtAmount, "insufficient_credit");

20

Title: Using storage to store initializer struct var can save gas https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/NFTVault.sol#L182 Reading from storage than caching struct to memory can save execution gas cost:

            NFTCategoryInitializer storage initializer = _typeInitializers[i];

21

Title: Tight vars packing in PositionPreview struct https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/NFTVault.sol#L610-L623 Bool has 1 byte size, address has 20 bytes size (< 32). Grouping them together can save 1 slot, which can save gas:

struct PositionPreview {
        address owner;
        bool liquidatable; //@audit-info: move here
        uint256 nftIndex;
        bytes32 nftType;
        uint256 nftValueUSD;
        VaultSettings vaultSettings;
        uint256 creditLimit;
        uint256 debtPrincipal;
        uint256 debtInterest;
        BorrowType borrowType;
        uint256 liquidatedAt;
        address liquidator;
    }

22

Title: Avoiding MLOAD by using msg.sender https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/NFTVault.sol#L559-L574 It is clear that _owner in _openPosition is msg.sender. Replacing it with msg.sender can save gas Change it to:

    function _openPosition(address, uint256 _nftIndex) internal {
        nftContract.transferFrom(msg.sender, address(this), _nftIndex); //@audit-info: here

        positions[_nftIndex] = Position({
            borrowType: BorrowType.NOT_CONFIRMED,
            debtPrincipal: 0,
            debtPortion: 0,
            debtAmountForRepurchase: 0,
            liquidatedAt: 0,
            liquidator: address(0)
        });
        positionOwner[_nftIndex] = msg.sender;
        positionIndexes.add(_nftIndex);

        emit PositionOpened(msg.sender, _nftIndex);
    }