search

code-423n4 / 2022-06-notional-coop-findings

1 stars 1 forks source link

Gas Optimizations #117

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Issue: Use of '&&' within a require function

Explanation: Splitting the require() statement into separate requires instead of using '&&' saves gas

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashLogic.sol#L116-L123

        require(
            msg.sender == address(NotionalV2) &&
            // Only accept the fcash id that corresponds to the listed currency and maturity
            _id == fCashID &&
            // Protect against signed value underflows
            int256(_value) > 0,
            "Invalid"
        );

Recommendation:

        require(msg.sender == address(NotionalV2), "Error message");
        // Only accept the fcash id that corresponds to the listed currency and maturity
        require( _id == fCashID, "Error message");
        // Protect against signed value underflows
        require(int256(_value) > 0, "Invalid");

Note that an error string should be included for each require

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashLogic.sol#L129-L137

        require(
            ac.hasDebt == 0x00 &&
            assets.length == 1 &&
            EncodeDecode.encodeERC1155Id(
                assets[0].currencyId,
                assets[0].maturity,
                assets[0].assetType
            ) == fCashID
        );

Recommendation:

        require(ac.hasDebt == 0x00, "error message");
        require(assets.length == 1, "error message");
        require(EncodeDecode.encodeERC1155Id(1
                assets[0].currencyId,
                assets[0].maturity,
                assets[0].assetType
            ) == fCashID, "error message");

Again, an error string should be included for each require

Issue: Should use != 0 instead of > 0 in a require statement if variable is an unsigned integer (uint)

Explanation: != 0 should be used where possible since > 0 costs more gas

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L23

        require(underlyingExternal > 0, "Must Settle");

Change underlyingExternal > 0 to underlyingExternal != 0

Issue: Storage of uints or ints smaller than 32 bytes Explanation: Storage of uints or ints smaller than 32 bytes incurs overhead Solution: Use a larger size, then downcast where needed

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/interfaces/ISetToken.sol#L49-L55

    struct Position {
        address component;
        address module;
        int256 unit;
        uint8 positionState;
        bytes data;
    }

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/interfaces/IWrappedFCash.sol#L22

    function initialize(uint16 currencyId, uint40 maturity) external;

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/interfaces/IWrappedFCash.sol#L49

    function getDecodedID() external view returns (uint16 currencyId, uint40 maturity);

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/interfaces/IWrappedFCash.sol#L53

    function getMarketIndex() external view returns (uint8);

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/interfaces/IWrappedFCashFactory.sol#L5

    function deployWrapper(uint16 currencyId, uint40 maturity) external returns(address);

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/interfaces/IWrappedFCashFactory.sol#L6

    function computeAddress(uint16 currencyId, uint40 maturity) external view returns(address);

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashBase.sol#L35

    function initialize(uint16 currencyId, uint40 maturity) external override initializer {

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashBase.sol#L93-L95

    function getCurrencyId() public view override returns (uint16 currencyId) {
        (currencyId, /* */, /* */) = EncodeDecode.decodeERC1155Id(_fCashId);
    }

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashBase.sol#L98-L100

    function getDecodedID() public view override returns (uint16 currencyId, uint40 maturity) {
        (currencyId, maturity, /* */) = EncodeDecode.decodeERC1155Id(_fCashId);
    }

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashBase.sol#L103-L105

    function decimals() public pure override returns (uint8) {
        return 8;
    }

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashBase.sol#L109-L119

    function getMarketIndex() public view override returns (uint8) {
        (uint256 marketIndex, bool isIdiosyncratic) = DateTime.getMarketIndex(
            Constants.MAX_TRADED_MARKET_INDEX,
            getMaturity(),
            block.timestamp
        );

        if (isIdiosyncratic) return 0;
        // Market index as defined does not overflow this conversion
        return uint8(marketIndex);
    }

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L18

        uint16 currencyId = getCurrencyId();

uint16 currencyId occurs in all five identical lines referenced below:

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L31

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L100

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L120

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L139

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L157

        (uint16 currencyId, uint40 maturity) = getDecodedID();

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashLogic.sol#L222

            uint16 currencyId = getCurrencyId();

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashLogic.sol#L261

        uint16 currencyId,

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/proxy/WrappedfCashFactory.sol#L15

    event WrapperDeployed(uint16 currencyId, uint40 maturity, address wrapper);

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/proxy/WrappedfCashFactory.sol#L26-L27

    function deployWrapper(uint16 currencyId, uint40 maturity) external returns (address) {
        address _computedWrapper = computeAddress(currencyId, maturity);

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/proxy/WrappedfCashFactory.sol#L39-L41

    function computeAddress(uint16 currencyId, uint40 maturity) public view returns (address) {
        return Create2.computeAddress(SALT, keccak256(_getByteCode(currencyId, maturity)));
    }

Issue: Use of abi.encode() Explanation: abi.encode() is less efficient than abi.encodePacked() Recommendation: Change abi.encode to abi.encodePacked

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L225-L241

    function _redeemInternal(
        uint256 shares,
        address receiver,
        address owner
    ) private {
        bytes memory userData = abi.encode(
            RedeemOpts({
                redeemToUnderlying: true,
                transferfCash: false,
                receiver: receiver,
                maxImpliedRate: 0
            })
        );

        // No operator data
        _burn(owner, shares, userData, "");
    }

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashLogic.sol#L158-L162

    function redeem(uint256 amount, RedeemOpts memory opts) public override {
        bytes memory data = abi.encode(opts);
        // In this case, the owner is msg.sender based on the OZ ERC777 implementation
        burn(amount, data);
    }