JPEG'd contest - IllIllI's results

Low Risk Issues

Missing checks for address(0x0) when assigning values to address state variables

1. File: contracts/escrow/NFTEscrow.sol (line 38)

        nftAddress = _nftAddress;

2. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 79)

        collateralAsset = _collateralAsset;

Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions

See this link for a description of this storage variable. While some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.

1. File: contracts/helpers/CryptoPunksHelper.sol (line 16)

contract CryptoPunksHelper is NFTEscrow, OwnableUpgradeable {

2. File: contracts/helpers/EtherRocksHelper.sol (line 16)

contract EtherRocksHelper is NFTEscrow, OwnableUpgradeable {

Non-critical Issues

public functions not called by the contract should be declared external instead

Contracts are allowed to override their parents' functions and change the visibility from external to public.

1. File: contracts/vaults/yVault/Controller.sol (line 151)

    function withdraw(IERC20 _token, uint256 _amount) public {

2. File: contracts/vaults/yVault/yVault.sol (line 115)

    function setFarmingPool(address _farm) public onlyOwner {

constants should be defined rather than using magic numbers

1. File: contracts/escrow/NFTEscrow.sol (line 98)

                bytes1(0xff),

2. File: contracts/vaults/NFTVault.sol (line 465)

            decimals > 18

3. File: contracts/vaults/NFTVault.sol (line 466)

                ? uint256(answer) / 10**(decimals - 18)

4. File: contracts/vaults/NFTVault.sol (line 467)

                : uint256(answer) * 10**(18 - decimals);

5. File: contracts/vaults/NFTVault.sol (line 593)

        uint256 interestPerSec = interestPerYear / 365 days;

6. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 112)

            decimals > 18

7. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 113)

                ? uint256(answer) / 10**(decimals - 18)

8. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 114)

                : uint256(answer) * 10**(18 - decimals);

9. File: contracts/vaults/yVault/yVault.sol (line 196)

        return (balance() * 1e18) / supply;

10. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 126)

        require(_curveConfig.pusdIndex < 4, "INVALID_PUSD_CURVE_INDEX");

11. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 127)

        require(_curveConfig.usdcIndex < 4, "INVALID_USDC_CURVE_INDEX");

12. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 343)

                    abi.encodePacked(weth, uint24(500), usdc),

13. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 368)

            10**12;

14. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 371)

        uint256[4] memory liquidityAmounts = [uint256(0), 0, 0, 0];

15. File: contracts/farming/yVaultLPFarming.sol (line 94)

            1e36;

16. File: contracts/farming/yVaultLPFarming.sol (line 172)

        newAccRewardsPerShare = accRewardPerShare + newRewards * 1e36 / totalStaked;

17. File: contracts/farming/yVaultLPFarming.sol (line 179)

            (accRewardPerShare - userLastAccRewardPerShare[account])) / 1e36;

18. File: contracts/farming/LPFarming.sol (line 196)

                1e36 *

19. File: contracts/farming/LPFarming.sol (line 207)

            1e36;

20. File: contracts/farming/LPFarming.sol (line 307)

            1e36 *

21. File: contracts/farming/LPFarming.sol (line 319)

            1e36;

22. File: contracts/lock/JPEGLock.sol (line 33)

        lockTime = 365 days;

Use a more recent version of solidity

Use a solidity version of at least 0.8.4 to get bytes.concat() instead of abi.encodePacked(<bytes>,<bytes>) Use a solidity version of at least 0.8.12 to get string.concat() instead of abi.encodePacked(<str>,<str>)

1. File: contracts/escrow/NFTEscrow.sol (line 2)

pragma solidity ^0.8.0;

2. File: contracts/vaults/yVault/yVault.sol (line 2)

pragma solidity ^0.8.0;

3. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 2)

pragma solidity ^0.8.0;

Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18)

1. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 368)

            10**12;

Non-library/interface files should use fixed compiler versions, not floating ones

1. File: contracts/escrow/NFTEscrow.sol (line 2)

pragma solidity ^0.8.0;

2. File: contracts/vaults/NFTVault.sol (line 2)

pragma solidity ^0.8.0;

3. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 2)

pragma solidity ^0.8.0;

4. File: contracts/vaults/yVault/Controller.sol (line 2)

pragma solidity ^0.8.0;

5. File: contracts/vaults/yVault/yVault.sol (line 2)

pragma solidity ^0.8.0;

6. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 2)

pragma solidity ^0.8.0;

7. File: contracts/tokens/StableCoin.sol (line 2)

pragma solidity ^0.8.0;

8. File: contracts/tokens/JPEG.sol (line 2)

pragma solidity ^0.8.0;

9. File: contracts/staking/JPEGStaking.sol (line 2)

pragma solidity ^0.8.0;

10. File: contracts/farming/yVaultLPFarming.sol (line 2)

pragma solidity ^0.8.0;

11. File: contracts/farming/LPFarming.sol (line 2)

pragma solidity ^0.8.0;

12. File: contracts/lock/JPEGLock.sol (line 2)

pragma solidity ^0.8.0;

13. File: contracts/helpers/CryptoPunksHelper.sol (line 2)

pragma solidity ^0.8.0;

14. File: contracts/helpers/EtherRocksHelper.sol (line 2)

pragma solidity ^0.8.0;

File is missing NatSpec

1. File: contracts/tokens/JPEG.sol (line 0)

// SPDX-License-Identifier: GPL-3.0

NatSpec is incomplete

1. File: contracts/escrow/NFTEscrow.sol (lines 56-62)

    /// @dev Virtual function, should return the `payload` to use in {FlashEscrow}'s constructor
    /// @param _idx The index of the NFT that's going to be sent to the {FlashEscrow} instance
    function _encodeFlashEscrowPayload(uint256 _idx)
        internal
        view
        virtual
        returns (bytes memory);

Missing: @return

2. File: contracts/vaults/yVault/Controller.sol (lines 24-25)

    /// @param _feeAddress The address to send fees to
    constructor(address _jpeg, address _feeAddress) {

Missing: @param _jpeg

3. File: contracts/vaults/yVault/yVault.sol (lines 37-49)

    /// @param _token The token managed by this vault
    /// @param _controller The JPEG'd strategies controller
    constructor(
        address _token,
        address _controller,
        Rate memory _availableTokensRate
    )
        ERC20(
            string(
                abi.encodePacked("JPEG\xE2\x80\x99d ", ERC20(_token).name())
            ),
            string(abi.encodePacked("JPEGD", ERC20(_token).symbol()))
        )

Missing: @param _availableTokensRate

4. File: contracts/farming/yVaultLPFarming.sol (lines 175-177)

    /// @dev Updates `account`'s claimable rewards by adding pending rewards
    /// @param account The account to update
    function _withdrawReward(address account) internal returns (uint256) {

Missing: @return

5. File: contracts/farming/LPFarming.sol (lines 260-269)

    /// @dev Normalizes `blockNumber` to fit within the bounds of an epoch.
    /// This is done to ensure that no rewards are distributed for staking outside of an epoch without modifying the reward logic.
    /// For example:
    /// `blockNumber` is 1100, the epoch's `endBlock` is 1000. In this case the function would return 1000. If this value were to be used
    /// in the {_updatePool} function, where the pool's `lastRewardBlock` is 990, only the rewards from block 990 to block 1000 would be distributed
    /// @return Normalized `blockNumber`
    function _normalizeBlockNumber(uint256 blockNumber)
        internal
        view
        returns (uint256)

Missing: @param blockNumber

6. File: contracts/farming/LPFarming.sol (lines 313-315)

    /// @dev Updates `msg.sender`'s claimable rewards by adding pending rewards from `_pid`
    /// @param _pid The pool to withdraw rewards from
    function _withdrawReward(uint256 _pid) internal returns (uint256) {

Missing: @return

7. File: contracts/helpers/CryptoPunksHelper.sol (lines 94-100)

    /// @dev The {transferPunk} function is used as the escrow's payload.
    /// @param _idx The index of the punk that's going to be transferred using {NFTEscrow}
    function _encodeFlashEscrowPayload(uint256 _idx)
        internal
        view
        override
        returns (bytes memory)

Missing: @return

8. File: contracts/helpers/EtherRocksHelper.sol (lines 99-105)

    /// @dev The {giftRock} function is used as the escrow's payload.
    /// @param _idx The index of the rock that's going to be transferred using {NFTEscrow}
    function _encodeFlashEscrowPayload(uint256 _idx)
        internal
        view
        override
        returns (bytes memory)

Missing: @return

Event is missing indexed fields

Each event should use three indexed fields if there are three or more fields

1. File: contracts/vaults/NFTVault.sol (lines 23-27)

    event Borrowed(
        address indexed owner,
        uint256 indexed index,
        uint256 amount
    );

2. File: contracts/vaults/NFTVault.sol (line 28)

    event Repaid(address indexed owner, uint256 indexed index, uint256 amount);

3. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 31)

    event Deposit(address indexed user, uint256 depositAmount);

4. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 32)

    event Borrow(address indexed user, uint256 borrowAmount);

5. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 33)

    event Repay(address indexed user, uint256 repayAmount);

6. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 34)

    event Withdraw(address indexed user, uint256 withdrawAmount);

7. File: contracts/vaults/yVault/yVault.sol (line 20)

    event Deposit(address indexed depositor, uint256 wantAmount);

8. File: contracts/vaults/yVault/yVault.sol (line 21)

    event Withdrawal(address indexed withdrawer, uint256 wantAmount);

9. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 25)

    event Harvested(uint256 wantEarned);

10. File: contracts/staking/JPEGStaking.sol (line 14)

    event Stake(address indexed user, uint256 amount);

11. File: contracts/staking/JPEGStaking.sol (line 15)

    event Unstake(address indexed user, uint256 amount);

12. File: contracts/farming/yVaultLPFarming.sol (line 18)

    event Deposit(address indexed user, uint256 amount);

13. File: contracts/farming/yVaultLPFarming.sol (line 19)

    event Withdraw(address indexed user, uint256 amount);

14. File: contracts/farming/yVaultLPFarming.sol (line 20)

    event Claim(address indexed user, uint256 rewards);

15. File: contracts/farming/LPFarming.sol (line 20)

    event Deposit(address indexed user, uint256 indexed pid, uint256 amount);

16. File: contracts/farming/LPFarming.sol (line 21)

    event Withdraw(address indexed user, uint256 indexed pid, uint256 amount);

17. File: contracts/farming/LPFarming.sol (line 22)

    event Claim(address indexed user, uint256 indexed pid, uint256 amount);

18. File: contracts/farming/LPFarming.sol (line 23)

    event ClaimAll(address indexed user, uint256 amount);

19. File: contracts/lock/JPEGLock.sol (line 14)

    event Lock(address indexed user, uint256 indexed nftIndex, uint256 amount);

20. File: contracts/lock/JPEGLock.sol (line 15)

    event Unlock(address indexed user, uint256 indexed nftIndex, uint256 amount);

The nonReentrant modifier should occur before all other modifiers

This is a best-practice to protect against reentrancy in other modifiers

1. File: contracts/vaults/NFTVault.sol (line 679)

    ) external validNFTIndex(_nftIndex) nonReentrant {

2. File: contracts/vaults/NFTVault.sol (line 759)

        nonReentrant

3. File: contracts/vaults/NFTVault.sol (line 834)

        nonReentrant

4. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 163)

    function borrow(uint256 amount) external onlyRole(WHITELISTED_ROLE) nonReentrant {

5. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 179)

    function repay(uint256 amount) external onlyRole(WHITELISTED_ROLE) nonReentrant {

6. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 193)

    function withdraw(uint256 amount) external onlyRole(WHITELISTED_ROLE) nonReentrant {

7. File: contracts/lock/JPEGLock.sol (line 53)

    ) external onlyOwner nonReentrant {

Typos

1. File: contracts/escrow/NFTEscrow.sol (line 30)

/// This is an alternative to the classic "reservation" method, which requires users to call 3 functions in a specifc order (making the process non atomic)

specifc

2. File: contracts/vaults/NFTVault.sol (line 117)

        //The standard OZ ERC721 implementation of ownerOf reverts on a non existing nft isntead of returning address(0)

isntead

3. File: contracts/vaults/NFTVault.sol (line 625)

    /// @notice Returns data relative to a postition, existing or not

postition

4. File: contracts/vaults/NFTVault.sol (line 673)

    /// @param _useInsurance Whereter to open an insured position. In case the position has already been opened previously,

Whereter

5. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 50)

    /// instead of fetching it everytime to save gas

everytime

6. File: contracts/vaults/yVault/Controller.sol (line 128)

    /// @notice Allows members of the `STRATEGIST_ROLE` to withdraw tokens stuck in this constract

constract

Not using the named return variables when a function returns, wastes deployment gas

1. File: contracts/escrow/NFTEscrow.sol (line 106)

        return (salt, predictedAddress);

2. File: contracts/escrow/NFTEscrow.sol (line 106)

        return (salt, predictedAddress);

require()/revert() strings longer than 32 bytes cost extra gas

1. File: contracts/vaults/NFTVault.sol (lines 391-395)

        require(
            _liquidationLimitRate.numerator * _creditLimitRate.denominator >
                _creditLimitRate.numerator * _liquidationLimitRate.denominator,
            "credit_rate_exceeds_or_equals_liquidation_rate"
        );

2. File: contracts/tokens/StableCoin.sol (lines 39-42)

        require(
            hasRole(MINTER_ROLE, _msgSender()),
            "StableCoin: must have minter role to mint"
        );

3. File: contracts/tokens/StableCoin.sol (lines 53-56)

        require(
            hasRole(PAUSER_ROLE, _msgSender()),
            "StableCoin: must have pauser role to pause"
        );

4. File: contracts/tokens/StableCoin.sol (lines 67-70)

        require(
            hasRole(PAUSER_ROLE, _msgSender()),
            "StableCoin: must have pauser role to unpause"
        );

5. File: contracts/tokens/JPEG.sol (lines 21-24)

        require(
            hasRole(MINTER_ROLE, _msgSender()),
            "JPEG: must have minter role to mint"
        );

Use a more recent version of solidity

Use a solidity version of at least 0.8.2 to get compiler automatic inlining Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value

1. File: contracts/escrow/NFTEscrow.sol (line 2)

pragma solidity ^0.8.0;

2. File: contracts/vaults/NFTVault.sol (line 2)

pragma solidity ^0.8.0;

3. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 2)

pragma solidity ^0.8.0;

4. File: contracts/vaults/yVault/Controller.sol (line 2)

pragma solidity ^0.8.0;

5. File: contracts/vaults/yVault/yVault.sol (line 2)

pragma solidity ^0.8.0;

6. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 2)

pragma solidity ^0.8.0;

7. File: contracts/tokens/StableCoin.sol (line 2)

pragma solidity ^0.8.0;

8. File: contracts/tokens/JPEG.sol (line 2)

pragma solidity ^0.8.0;

9. File: contracts/staking/JPEGStaking.sol (line 2)

pragma solidity ^0.8.0;

10. File: contracts/farming/yVaultLPFarming.sol (line 2)

pragma solidity ^0.8.0;

11. File: contracts/farming/LPFarming.sol (line 2)

pragma solidity ^0.8.0;

12. File: contracts/lock/JPEGLock.sol (line 2)

pragma solidity ^0.8.0;

13. File: contracts/helpers/CryptoPunksHelper.sol (line 2)

pragma solidity ^0.8.0;

14. File: contracts/helpers/EtherRocksHelper.sol (line 2)

pragma solidity ^0.8.0;

Using bools for storage incurs overhead

    // Booleans are more expensive than uint256 or any type that takes up a full
    // word because each write operation emits an extra SLOAD to first read the
    // slot's contents, replace the bits taken up by the boolean, and then write
    // back. This is the compiler's defense against contract upgrades and
    // pointer aliasing, and it cannot be disabled.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27

1. File: contracts/vaults/NFTVault.sol (line 91)

    bool public daoFloorOverride;

2. File: contracts/vaults/NFTVault.sol (line 93)

    bool public useFallbackOracle;

3. File: contracts/vaults/yVault/Controller.sol (line 22)

    mapping(IERC20 => mapping(IStrategy => bool)) public approvedStrategies;

4. File: contracts/vaults/yVault/yVault.sol (line 35)

    mapping(address => bool) public whitelistedContracts;

5. File: contracts/farming/yVaultLPFarming.sol (line 37)

    mapping(address => bool) public whitelistedContracts;

6. File: contracts/farming/LPFarming.sol (line 73)

    mapping(address => bool) public whitelistedContracts;

Use delete rather than assigning zero to a mapping to get a gas refund

1. File: contracts/vaults/NFTVault.sol (line 380)

        pendingNFTValueETH[_nftIndex] = 0;

2. File: contracts/farming/yVaultLPFarming.sol (line 141)

        userPendingRewards[msg.sender] = 0;

3. File: contracts/farming/LPFarming.sol (line 340)

        userRewards[msg.sender] = 0;

4. File: contracts/farming/LPFarming.sol (line 357)

        userRewards[msg.sender] = 0;

Using > 0 costs more gas than != 0 when used on a uint in a require() statement

1. File: contracts/vaults/NFTVault.sol (line 278)

        require(_newFloor > 0, "Invalid floor");

2. File: contracts/vaults/NFTVault.sol (line 365)

        require(pendingValue > 0, "no_pending_value");

3. File: contracts/vaults/NFTVault.sol (line 687)

        require(_amount > 0, "invalid_amount");

4. File: contracts/vaults/NFTVault.sol (line 764)

        require(_amount > 0, "invalid_amount");

5. File: contracts/vaults/NFTVault.sol (line 770)

        require(debtAmount > 0, "position_not_borrowed");

6. File: contracts/vaults/NFTVault.sol (line 882)

        require(position.liquidatedAt > 0, "not_liquidated");

7. File: contracts/vaults/NFTVault.sol (line 926)

        require(position.liquidatedAt > 0, "not_liquidated");

8. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 142)

        require(amount > 0, "invalid_amount");

9. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 164)

        require(amount > 0, "invalid_amount");

10. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 180)

        require(amount > 0, "invalid_amount");

11. File: contracts/vaults/yVault/yVault.sol (line 143)

        require(_amount > 0, "INVALID_AMOUNT");

12. File: contracts/vaults/yVault/yVault.sol (line 167)

        require(_shares > 0, "INVALID_AMOUNT");

13. File: contracts/vaults/yVault/yVault.sol (line 170)

        require(supply > 0, "NO_TOKENS_DEPOSITED");

14. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 334)

            require(wethBalance > 0, "NOOP");

15. File: contracts/staking/JPEGStaking.sol (line 32)

        require(_amount > 0, "invalid_amount");

16. File: contracts/farming/yVaultLPFarming.sol (line 101)

        require(_amount > 0, "invalid_amount");

17. File: contracts/farming/yVaultLPFarming.sol (line 118)

        require(_amount > 0, "invalid_amount");

18. File: contracts/farming/yVaultLPFarming.sol (line 139)

        require(rewards > 0, "no_reward");

19. File: contracts/farming/LPFarming.sol (line 114)

        require(_rewardPerBlock > 0, "Invalid reward per block");

20. File: contracts/farming/LPFarming.sol (line 218)

        require(_amount > 0, "invalid_amount");

21. File: contracts/farming/LPFarming.sol (line 239)

        require(_amount > 0, "invalid_amount");

22. File: contracts/farming/LPFarming.sol (line 337)

        require(rewards > 0, "no_reward");

23. File: contracts/farming/LPFarming.sol (line 354)

        require(rewards > 0, "no_reward");

24. File: contracts/lock/JPEGLock.sol (line 40)

        require(_newTime > 0, "Invalid lock time");

<array>.length should not be looked up in every loop of a for-loop

Even memory arrays incur the overhead of bit tests and bit shifts to calculate the array length

1. File: contracts/vaults/NFTVault.sol (line 181)

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

2. File: contracts/vaults/NFTVault.sol (line 184)

            for (uint256 j = 0; j < initializer.nfts.length; j++) {

3. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 145)

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

4. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 319)

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

5. File: contracts/farming/LPFarming.sol (line 348)

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

It costs more gas to initialize variables to zero than to let the default of zero be applied

1. File: contracts/vaults/NFTVault.sol (line 181)

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

2. File: contracts/vaults/NFTVault.sol (line 184)

            for (uint256 j = 0; j < initializer.nfts.length; j++) {

3. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 145)

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

4. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 231)

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

5. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 319)

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

6. File: contracts/farming/LPFarming.sol (line 281)

        for (uint256 pid = 0; pid < length; ++pid) {

7. File: contracts/farming/LPFarming.sol (line 348)

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

State variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second access of a state variable within a function. Less obvious optimizations include having local storage variables of mappings within state variable mappings or mappings within state variable structs, having local storage variables of structs within mappings, or having local caches of state variable contracts/addresses.

1. File: contracts/vaults/NFTVault.sol (line 814)

            nftContract.safeTransferFrom(address(this), msg.sender, _nftIndex);

2. File: contracts/vaults/NFTVault.sol (line 590)

        uint256 interestPerYear = (totalDebtAmount *

3. File: contracts/vaults/NFTVault.sol (line 740)

                totalDebtAmount;

4. File: contracts/vaults/NFTVault.sol (line 739)

            uint256 plusPortion = (totalDebtPortion * _amount) /

5. File: contracts/vaults/NFTVault.sol (line 242)

        settings.creditLimitRate = _creditLimitRate;

6. File: contracts/vaults/NFTVault.sol (line 257)

        settings.liquidationLimitRate = _liquidationLimitRate;

7. File: contracts/vaults/NFTVault.sol (line 369)

            settings.creditLimitRate.denominator) *

8. File: contracts/vaults/NFTVault.sol (line 506)

            settings.creditLimitRate.denominator;

9. File: contracts/vaults/NFTVault.sol (line 520)

            settings.liquidationLimitRate.denominator;

10. File: contracts/vaults/NFTVault.sol (line 592)

            settings.debtInterestApr.denominator;

11. File: contracts/vaults/NFTVault.sol (line 714)

            settings.organizationFeeRate.numerator) /

12. File: contracts/vaults/NFTVault.sol (line 895)

            settings.insuranceLiquidationPenaltyRate.numerator) /

13. File: contracts/vaults/NFTVault.sol (line 533)

        uint256 principal = positions[_nftIndex].debtPrincipal;

14. File: contracts/vaults/NFTVault.sol (line 637)

        uint256 debtAmount = positions[_nftIndex].liquidatedAt > 0

15. File: contracts/vaults/NFTVault.sol (line 684)

                address(0) == positionOwner[_nftIndex],

16. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 148)

            IERC20Upgradeable(collateralAsset).safeTransferFrom(

17. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 203)

            IERC20Upgradeable(collateralAsset).safeTransfer(msg.sender, amount);

18. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 106)

        uint8 decimals = oracle.decimals();

19. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 135)

            creditLimitRate.denominator;

20. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 196)

        uint256 creditLimit = getCreditLimit(collateralAmount - amount);

21. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 182)

        amount = amount > debtAmount ? debtAmount : amount;

22. File: contracts/vaults/yVault/yVault.sol (line 78)

            controller.balanceOf(address(token));

23. File: contracts/vaults/yVault/yVault.sol (line 132)

        controller.earn(address(token), _bal);

24. File: contracts/vaults/yVault/yVault.sol (line 179)

            controller.withdraw(address(token), toWithdraw);

25. File: contracts/vaults/yVault/yVault.sol (line 132)

        controller.earn(address(token), _bal);

26. File: contracts/vaults/yVault/yVault.sol (line 189)

        controller.withdrawJPEG(address(token), farm);

27. File: contracts/vaults/yVault/yVault.sol (line 125)

            availableTokensRate.denominator;

28. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 252)

        want.safeIncreaseAllowance(address(convex.booster), balance);

29. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 277)

        uint256 balance = want.balanceOf(address(this));

30. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 296)

        balance = want.balanceOf(address(this));

31. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 233)

            if (address(jpeg) == extraReward.rewardToken()) {

32. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 306)

        jpeg.safeTransfer(_to, jpeg.balanceOf(address(this)));

33. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 343)

                    abi.encodePacked(weth, uint24(500), usdc),

34. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 356)

        uint256 usdcBalance = usdc.balanceOf(address(this));

35. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 353)

        StrategyConfig memory strategy = strategyConfig;

36. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 360)

            performanceFee.denominator;

37. File: contracts/farming/yVaultLPFarming.sol (line 149)

        jpeg.safeTransfer(msg.sender, rewards);

38. File: contracts/farming/yVaultLPFarming.sol (line 183)

        userLastAccRewardPerShare[account] = accRewardPerShare;

39. File: contracts/farming/LPFarming.sol (line 130)

            jpeg.safeTransferFrom(

40. File: contracts/farming/LPFarming.sol (line 120)

            (epoch.endBlock - _blockNumber());

41. File: contracts/farming/LPFarming.sol (line 271)

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

42. File: contracts/farming/LPFarming.sol (line 163)

        poolInfo[_pid].allocPoint = _allocPoint;

43. File: contracts/farming/LPFarming.sol (line 324)

        user.lastAccRewardPerShare = poolInfo[_pid].accRewardPerShare;

44. File: contracts/farming/LPFarming.sol (line 145)

        totalAllocPoint = totalAllocPoint + _allocPoint;

45. File: contracts/farming/LPFarming.sol (line 165)

            totalAllocPoint = totalAllocPoint - prevAllocPoint + _allocPoint;

Using calldata instead of memory for read-only arguments in external functions saves gas

1. File: contracts/vaults/NFTVault.sol (line 146)

        NFTCategoryInitializer[] memory _typeInitializers,

2. File: contracts/vaults/NFTVault.sol (line 148)

        VaultSettings memory _settings

3. File: contracts/vaults/NFTVault.sol (line 212)

    function setDebtInterestApr(Rate memory _debtInterestApr)

4. File: contracts/vaults/NFTVault.sol (line 222)

    function setValueIncreaseLockRate(Rate memory _valueIncreaseLockRate)

5. File: contracts/vaults/NFTVault.sol (line 232)

    function setCreditLimitRate(Rate memory _creditLimitRate)

6. File: contracts/vaults/NFTVault.sol (line 247)

    function setLiquidationLimitRate(Rate memory _liquidationLimitRate)

7. File: contracts/vaults/NFTVault.sol (line 290)

    function setOrganizationFeeRate(Rate memory _organizationFeeRate)

8. File: contracts/vaults/NFTVault.sol (line 300)

    function setInsurancePurchaseRate(Rate memory _insurancePurchaseRate)

9. File: contracts/vaults/NFTVault.sol (line 311)

        Rate memory _insuranceLiquidationPenaltyRate

10. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 70)

        Rate memory _creditLimitRate

++i/i++ should be unchecked{++i}/unchecked{++i} when it is not possible for them to overflow, as is the case when used in for- and while-loops

1. File: contracts/vaults/NFTVault.sol (line 181)

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

2. File: contracts/vaults/NFTVault.sol (line 184)

            for (uint256 j = 0; j < initializer.nfts.length; j++) {

3. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 145)

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

4. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 231)

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

5. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 319)

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

6. File: contracts/farming/LPFarming.sol (line 281)

        for (uint256 pid = 0; pid < length; ++pid) {

7. File: contracts/farming/LPFarming.sol (line 348)

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

++i costs less gas than ++i, especially when it's used in for-loops (--i/i-- too)

1. File: contracts/vaults/NFTVault.sol (line 181)

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

2. File: contracts/vaults/NFTVault.sol (line 184)

            for (uint256 j = 0; j < initializer.nfts.length; j++) {

3. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 145)

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

4. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 231)

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

5. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 319)

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

6. File: contracts/farming/LPFarming.sol (line 348)

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

Splitting require() statements that use && saves gas

See this issue for an example

1. File: contracts/escrow/NFTEscrow.sol (lines 86-89)

        require(
            _owner != address(this) && _owner != address(0),
            "NFTEscrow: invalid_owner"
        );

2. File: contracts/vaults/NFTVault.sol (lines 401-404)

        require(
            rate.denominator > 0 && rate.denominator >= rate.numerator,
            "invalid_rate"
        );

3. File: contracts/vaults/FungibleAssetVaultForDAO.sol (lines 93-98)

        require(
            _creditLimitRate.denominator > 0 &&
                //denominator can be equal to the numerator in some cases (stablecoins used as collateral)
                _creditLimitRate.denominator >= _creditLimitRate.numerator,
            "invalid_rate"
        );

4. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 194)

        require(amount > 0 && amount <= collateralAmount, "invalid_amount");

5. File: contracts/vaults/yVault/yVault.sol (lines 99-102)

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

6. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (lines 181-185)

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

7. File: contracts/staking/JPEGStaking.sol (lines 45-48)

        require(
            _amount > 0 && _amount <= balanceOf(msg.sender),
            "invalid_amount"
        );

Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Use a larger size then downcast where needed

1. File: contracts/vaults/NFTVault.sol (line 55)

        uint128 numerator;

2. File: contracts/vaults/NFTVault.sol (line 56)

        uint128 denominator;

3. File: contracts/vaults/NFTVault.sol (line 460)

        uint8 decimals = aggregator.decimals();

4. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 37)

        uint128 numerator;

5. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 38)

        uint128 denominator;

6. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 106)

        uint8 decimals = oracle.decimals();

7. File: contracts/vaults/yVault/yVault.sol (line 24)

        uint128 numerator;

8. File: contracts/vaults/yVault/yVault.sol (line 25)

        uint128 denominator;

9. File: contracts/vaults/yVault/yVault.sol (line 70)

    function decimals() public view virtual override returns (uint8) {

10. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 28)

        uint128 numerator;

11. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 29)

        uint128 denominator;

abi.encode() is less efficient than abi.encodePacked()

1. File: contracts/escrow/NFTEscrow.sol (line 52)

                abi.encode(nftAddress, _encodeFlashEscrowPayload(_idx))

Expressions for constant values such as a call to keccak256(), should use immutable rather than constant

See this issue for a detail description of the issue

1. File: contracts/vaults/NFTVault.sol (line 71)

    bytes32 public constant DAO_ROLE = keccak256("DAO_ROLE");

2. File: contracts/vaults/NFTVault.sol (line 72)

    bytes32 public constant LIQUIDATOR_ROLE = keccak256("LIQUIDATOR_ROLE");

3. File: contracts/vaults/NFTVault.sol (line 74)

    bytes32 public constant CUSTOM_NFT_HASH = keccak256("CUSTOM");

4. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 41)

    bytes32 public constant WHITELISTED_ROLE = keccak256("WHITELISTED_ROLE");

5. File: contracts/vaults/yVault/Controller.sol (line 15)

    bytes32 public constant STRATEGIST_ROLE = keccak256("STRATEGIST_ROLE");

6. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 66)

    bytes32 public constant STRATEGIST_ROLE = keccak256("STRATEGIST_ROLE");

7. File: contracts/tokens/StableCoin.sol (line 22)

    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");

8. File: contracts/tokens/StableCoin.sol (line 23)

    bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");

9. File: contracts/tokens/JPEG.sol (line 10)

    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");

Using private rather than public for constants, saves gas

If needed, the value can be read from the verified contract source code

1. File: contracts/vaults/NFTVault.sol (line 71)

    bytes32 public constant DAO_ROLE = keccak256("DAO_ROLE");

2. File: contracts/vaults/NFTVault.sol (line 72)

    bytes32 public constant LIQUIDATOR_ROLE = keccak256("LIQUIDATOR_ROLE");

3. File: contracts/vaults/NFTVault.sol (line 74)

    bytes32 public constant CUSTOM_NFT_HASH = keccak256("CUSTOM");

4. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 41)

    bytes32 public constant WHITELISTED_ROLE = keccak256("WHITELISTED_ROLE");

5. File: contracts/vaults/yVault/Controller.sol (line 15)

    bytes32 public constant STRATEGIST_ROLE = keccak256("STRATEGIST_ROLE");

6. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 66)

    bytes32 public constant STRATEGIST_ROLE = keccak256("STRATEGIST_ROLE");

7. File: contracts/tokens/StableCoin.sol (line 22)

    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");

8. File: contracts/tokens/StableCoin.sol (line 23)

    bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");

9. File: contracts/tokens/JPEG.sol (line 10)

    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");

Don't compare boolean expressions to boolean literals

if (<x> == true) => if (<x>), if (<x> == false) => if (!<x>)

1. File: contracts/vaults/yVault/Controller.sol (line 87)

            approvedStrategies[_token][_strategy] == true,

Duplicated require()/revert() checks should be refactored to a modifier or function

1. File: contracts/vaults/NFTVault.sol (line 764)

        require(_amount > 0, "invalid_amount");

2. File: contracts/vaults/NFTVault.sol (line 767)

        require(position.liquidatedAt == 0, "liquidated");

3. File: contracts/vaults/NFTVault.sol (line 805)

        require(msg.sender == positionOwner[_nftIndex], "unauthorized");

4. File: contracts/vaults/NFTVault.sol (line 926)

        require(position.liquidatedAt > 0, "not_liquidated");

5. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 164)

        require(amount > 0, "invalid_amount");

6. File: contracts/vaults/yVault/Controller.sol (line 73)

        require(address(_token) != address(0), "INVALID_TOKEN");

7. File: contracts/vaults/yVault/Controller.sol (line 74)

        require(address(_strategy) != address(0), "INVALID_STRATEGY");

8. File: contracts/vaults/yVault/Controller.sol (line 164)

        require(msg.sender == vaults[_token], "NOT_VAULT");

9. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 292)

        require(vault != address(0), "ZERO_VAULT"); // additional protection so we don't burn the funds

10. File: contracts/farming/yVaultLPFarming.sol (line 118)

        require(_amount > 0, "invalid_amount");

11. File: contracts/farming/LPFarming.sol (line 239)

        require(_amount > 0, "invalid_amount");

12. File: contracts/farming/LPFarming.sol (line 354)

        require(rewards > 0, "no_reward");

Stack variable used as a cheaper cache for a state variable is only used once

If the variable is only accessed once, it's cheaper to use the state variable directly that one time

1. File: contracts/farming/yVaultLPFarming.sol (line 81)

        uint256 staked = totalStaked;

State variables only set in the constructor should be declared immutable

1. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 75)

    DexConfig public dexConfig;

2. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 76)

    CurveConfig public curveConfig;

3. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 77)

    ConvexConfig public convexConfig;

4. File: contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol (line 78)

    StrategyConfig public strategyConfig;

require() or revert() statements that check input arguments should be at the top of the function

Checks that involve constants should come before checks that involve state variables

1. File: contracts/vaults/NFTVault.sol (line 687)

        require(_amount > 0, "invalid_amount");

2. File: contracts/vaults/NFTVault.sol (line 764)

        require(_amount > 0, "invalid_amount");

3. File: contracts/vaults/yVault/Controller.sol (line 49)

        require(_vault != address(0), "INVALID_VAULT");

4. File: contracts/farming/LPFarming.sol (line 243)

        require(user.amount >= _amount, "insufficient_amount");

Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

1. File: contracts/vaults/NFTVault.sol (lines 203-205)

    function setBorrowAmountCap(uint256 _borrowAmountCap)
        external
        onlyRole(DAO_ROLE)

2. File: contracts/vaults/NFTVault.sol (lines 212-214)

    function setDebtInterestApr(Rate memory _debtInterestApr)
        external
        onlyRole(DAO_ROLE)

3. File: contracts/vaults/NFTVault.sol (lines 222-224)

    function setValueIncreaseLockRate(Rate memory _valueIncreaseLockRate)
        external
        onlyRole(DAO_ROLE)

4. File: contracts/vaults/NFTVault.sol (lines 232-234)

    function setCreditLimitRate(Rate memory _creditLimitRate)
        external
        onlyRole(DAO_ROLE)

5. File: contracts/vaults/NFTVault.sol (lines 247-249)

    function setLiquidationLimitRate(Rate memory _liquidationLimitRate)
        external
        onlyRole(DAO_ROLE)

6. File: contracts/vaults/NFTVault.sol (lines 262-264)

    function toggleFallbackOracle(bool _useFallback)
        external
        onlyRole(DAO_ROLE)

7. File: contracts/vaults/NFTVault.sol (line 271)

    function setJPEGLockTime(uint256 _newLockTime) external onlyRole(DAO_ROLE) {

8. File: contracts/vaults/NFTVault.sol (line 277)

    function overrideFloor(uint256 _newFloor) external onlyRole(DAO_ROLE) {

9. File: contracts/vaults/NFTVault.sol (line 284)

    function disableFloorOverride() external onlyRole(DAO_ROLE) {

10. File: contracts/vaults/NFTVault.sol (lines 290-292)

    function setOrganizationFeeRate(Rate memory _organizationFeeRate)
        external
        onlyRole(DAO_ROLE)

11. File: contracts/vaults/NFTVault.sol (lines 300-302)

    function setInsurancePurchaseRate(Rate memory _insurancePurchaseRate)
        external
        onlyRole(DAO_ROLE)

12. File: contracts/vaults/NFTVault.sol (lines 310-312)

    function setInsuranceLiquidationPenaltyRate(
        Rate memory _insuranceLiquidationPenaltyRate
    ) external onlyRole(DAO_ROLE) {

13. File: contracts/vaults/NFTVault.sol (lines 321-324)

    function setNFTType(uint256 _nftIndex, bytes32 _type)
        external
        validNFTIndex(_nftIndex)
        onlyRole(DAO_ROLE)

14. File: contracts/vaults/NFTVault.sol (lines 336-338)

    function setNFTTypeValueETH(bytes32 _type, uint256 _amountETH)
        external
        onlyRole(DAO_ROLE)

15. File: contracts/vaults/NFTVault.sol (lines 347-350)

    function setPendingNFTValueETH(uint256 _nftIndex, uint256 _amountETH)
        external
        validNFTIndex(_nftIndex)
        onlyRole(DAO_ROLE)

16. File: contracts/vaults/NFTVault.sol (lines 830-834)

    function liquidate(uint256 _nftIndex)
        external
        onlyRole(LIQUIDATOR_ROLE)
        validNFTIndex(_nftIndex)
        nonReentrant

17. File: contracts/vaults/NFTVault.sol (line 944)

    function collect() external nonReentrant onlyRole(DAO_ROLE) {

18. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 92)

    function setCreditLimitRate(Rate memory _creditLimitRate) public onlyRole(DEFAULT_ADMIN_ROLE) {

19. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 141)

    function deposit(uint256 amount) external payable onlyRole(WHITELISTED_ROLE) {

20. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 163)

    function borrow(uint256 amount) external onlyRole(WHITELISTED_ROLE) nonReentrant {

21. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 179)

    function repay(uint256 amount) external onlyRole(WHITELISTED_ROLE) nonReentrant {

22. File: contracts/vaults/FungibleAssetVaultForDAO.sol (line 193)

    function withdraw(uint256 amount) external onlyRole(WHITELISTED_ROLE) nonReentrant {

23. File: contracts/vaults/yVault/Controller.sol (lines 33-35)

    function setFeeAddress(address _feeAddress)
        public
        onlyRole(DEFAULT_ADMIN_ROLE)

24. File: contracts/vaults/yVault/Controller.sol (lines 44-46)

    function setVault(IERC20 _token, address _vault)
        external
        onlyRole(STRATEGIST_ROLE)

25. File: contracts/vaults/yVault/Controller.sol (lines 56-58)

    function approveStrategy(IERC20 _token, IStrategy _strategy)
        external
        onlyRole(DEFAULT_ADMIN_ROLE)