QA Report

[code423n4]

Low risk findings

1. filling orders with msg.value doesn’t work with batchFillOrder

in fillOrder while filling a long order the function check msg.value == order.strike to move collateral to the contract, or check msg.value == premium and pay the seller directly. This will not work with batchFill because msg.value will be a the same value across all iterations. So if user want to fill 2 short order both with weth as premium at the same time, fillOder will fail

Mitigation:

Consider refactor the fillOrder and batchFillOrder into the following:

function fillOrder(
        Order memory order,
        bytes calldata signature,
        uint256[] memory floorAssetTokenIds
    ) public payable returns (uint256 positionId) {
        _fillOrder(order, signature, floorAssetTokenIds, mgs.value)
}

function batchFillOrder(
        Order[] memory orders,
        bytes[] calldata signatures,
        uint256[][] memory floorAssetTokenIds
        uint256[] calldata values
    ) public returns (uint256[] memory positionIds) {
        require(orders.length == signatures.length, "Length mismatch in input");
        require(signatures.length == floorAssetTokenIds.length, "Length mismatch in input");

        positionIds = new uint256[](orders.length);

        uint256 totalValue;
        for (uint256 i = 0; i < orders.length; i++) {
            positionIds[i] = fillOrder(orders[i], signatures[i], floorAssetTokenIds[i], values[i]);
            totalValue += value[i]
        }

        require(totalValue == msg.value, "value mismatch");
    }

// internal _fillOrder
function _fillOrder(
        Order memory order,
        bytes calldata signature,
        uint256[] memory floorAssetTokenIds
        uint256 _value;
    ) public payable returns (uint256 positionId) {
        ...
    {use _value instead of msg.value}
    ...
}

2.order.nonce doesn’t work as its name suggested

Usually the word nonce refers to a increasing number that can be used as a way to “cancel old orders”. but in this case, nonce is purely used as “salt” to produce different id for different orders. I think it’s better that either rename this to salt , or add a check of order.nonce>last_ nonce in order validation step.

3. exercise can be marked as exeternal

[HickupHH3]

(1) is invalid: see #138