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

1 stars 1 forks source link

Gas Optimizations #152

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Gas Optimizations

1. Initialising with default values

Impact

When variables are declared it is has default values, avoiding explicit intializing variables with default values can save gas

Proof of concept

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L181

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

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

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

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

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

Mitigation

remove intialization of default values

2. Unnecessary variable

Function return values that are used once can be used directly instead of storing it in a temporary variable

Proof of concept

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L435

    uint256 nft_value = _getNFTValueETH(_nftIndex);
    return (nft_value * _ethPriceUSD()) / 1 ether;

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L503

    uint256 asset_value = _getNFTValueUSD(_nftIndex);
    return
        (asset_value * settings.creditLimitRate.numerator) /
        settings.creditLimitRate.denominator;

Mitigation

The function can be directly used in the expression

3. Adding unchecked can save gas

Impact

Using unchecked in expression that cant overflow/underflow can avoid default overflow/underflow checks introduced in 0.8, which can save gas

Proof of concept

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

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/farming/yVaultLPFarming.sol#L119-L125

    require(balanceOf[msg.sender] >= _amount, "insufficient_amount");

    _update();
    _withdrawReward(msg.sender);

    balanceOf[msg.sender] -= _amount;
    totalStaked -= _amount;

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

    if (vaultBalance < backingTokens) {
        uint256 toWithdraw = backingTokens - vaultBalance;
        controller.withdraw(address(token), toWithdraw);
    }

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

    uint256 asset_value = _getNFTValueUSD(_nftIndex);
    return
        (asset_value * settings.liquidationLimitRate.numerator) /
        settings.liquidationLimitRate.denominator;

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L780

    uint256 paidPrincipal = _amount > debtInterest 
        ? _amount - debtInterest
        : 0;

Mitigation

Add unchecked block to above statements

4. Revert string > 32 bytes

Impact

Revert strings that are greater than 32 bytes will increase deployment costs as well gas when require condition is met, reducing strings to 32 bytes will save gas

refer:https://blog.polymath.network/solidity-tips-and-tricks-to-save-gas-and-reduce-bytecode-size-c44580b218e6#c17b

Proof of concept

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L394

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/tokens/JPEG.sol#L23

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/tokens/StableCoin.sol#L41

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/tokens/StableCoin.sol#L55

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/tokens/StableCoin.sol#L69

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

Mitigation

Reduce strings to 32 bytes or use custom error introduced in 0.8.4 https://blog.soliditylang.org/2021/04/21/custom-errors/

5. Storage variables can be cached to save gas

Storage variables that are re-read in the same block can be cached in a local variables to prevent re-read and save gas

Proof of concept

totalDebtAmount in

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L585-L590

debtAmount in

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/FungibleAssetVaultForDAO.sol#L179-L188

collateralAmount and collateralAsset in

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/FungibleAssetVaultForDAO.sol#L193-L206

accRewardPerShare in https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/farming/yVaultLPFarming.sol#L179-L183

Mitigation

Storage variables can be cached in a local variable

6. > can be replaced with !=

> is less gas efficient when compared to != with uint variables in require statement, with optimizer enabled

Proof of concept

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L278

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L365

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L462

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L882

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L926

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/FungibleAssetVaultForDAO.sol#L108

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/FungibleAssetVaultForDAO.sol#L142

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/FungibleAssetVaultForDAO.sol#L164

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

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

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/lock/JPEGLock.sol#L40

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

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

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

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

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

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

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

Mitigation

> can be replaced by !=

7. Rearrange condition in if statement

Condition in if statement can be rearranged to shortcircuit evaluation

Proof of concept

_useInsurance can be placed first

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L719

    if (position.borrowType == BorrowType.USE_INSURANCE || _useInsurance) {

staked > 0 can be placed first

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

    if (block.number > lastRewardBlock && staked > 0) {
        (rewardShare, ) = _computeUpdate();
    }

Mitigation

Condition can be re-arranged

8. library functions not used

In JPEGStacking.sol library SafeERC20Upgradeable is imported, but the functions are not used

Proof of concept

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

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

Mitigation

functions can be replaced with safeERC20 functions

9. statements can be re-arranged to save gas

statements can be rearranged to prevent storage operation or revert early to save gas

Proof of concept

In LPFarming:set if statement can be re-arranged to prevent writing to storage

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

uint256 prevAllocPoint = poolInfo[_pid].allocPoint;
poolInfo[_pid].allocPoint = _allocPoint;
if (prevAllocPoint != _allocPoint) { 
    totalAllocPoint = totalAllocPoint - prevAllocPoint + _allocPoint;
}

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L682-L687

    require(
        msg.sender == positionOwner[_nftIndex] ||
            address(0) == positionOwner[_nftIndex],
        "unauthorized"
    );
    require(_amount > 0, "invalid_amount")