Lack of incentives for users to match a trade if order sender fee is negative

Summary

Even though currently, JOJO is the only one allowed to call approveTrade(), the protocol aims to decentralise matching mechanism in the future.

The issue here is that in a decentralised, self running system, no rationale user would want to match a trade if they do not make any fees, much less a negative fee.

Vulnerability Detail

We see the if result.orderSenderFee < 0 check. orderSenderFee can possibly be negative, which will discourage anyone to want to help match trades. This currently only works as JOJO is the only order matcher, and as the admins of the protocol, have an intrinsic incentives to want to help match trades. However, when the system progresses to a decentralised one in the future, this will no longer be the case.

    function approveTrade(address orderSender, bytes calldata tradeData)
        external
        onlyRegisteredPerp
        returns (
            address[] memory, // traderList
            int256[] memory, // paperChangeList
            int256[] memory // creditChangeList
        )
    {
        require(
            state.validOrderSender[orderSender],
            Errors.INVALID_ORDER_SENDER
        );

        /*
            parse tradeData
            Pass in all orders and their signatures that need to be matched.
            Also, pass in the amount you want to fill each order.
        */
        (
            Types.Order[] memory orderList,
            bytes[] memory signatureList,
            uint256[] memory matchPaperAmount
        ) = abi.decode(tradeData, (Types.Order[], bytes[], uint256[]));
        bytes32[] memory orderHashList = new bytes32[](orderList.length);

        // validate all orders
        for (uint256 i = 0; i < orderList.length; ) {
            Types.Order memory order = orderList[i];
            bytes32 orderHash = EIP712._hashTypedDataV4(
                domainSeparator,
                Trading._structHash(order)
            );
            orderHashList[i] = orderHash;
            address recoverSigner = ECDSA.recover(orderHash, signatureList[i]);
            // requirements
            require(
                recoverSigner == order.signer ||
                    state.operatorRegistry[order.signer][recoverSigner],
                Errors.INVALID_ORDER_SIGNATURE
            );
            require(
                Trading._info2Expiration(order.info) >= block.timestamp,
                Errors.ORDER_EXPIRED
            );
            require(
                (order.paperAmount < 0 && order.creditAmount > 0) ||
                    (order.paperAmount > 0 && order.creditAmount < 0),
                Errors.ORDER_PRICE_NEGATIVE
            );
            require(order.perp == msg.sender, Errors.PERP_MISMATCH);
            require(
                i == 0 || order.signer != orderList[0].signer,
                Errors.ORDER_SELF_MATCH
            );
            state.orderFilledPaperAmount[orderHash] += matchPaperAmount[i];
            require(
                state.orderFilledPaperAmount[orderHash] <=
                    int256(orderList[i].paperAmount).abs(),
                Errors.ORDER_FILLED_OVERFLOW
            );
            unchecked {
                ++i;
            }
        }

        Types.MatchResult memory result = Trading._matchOrders(
            state,
            orderHashList,
            orderList,
            matchPaperAmount
        );

        // charge fee
        state.primaryCredit[orderSender] += result.orderSenderFee;
        // if orderSender pay fees to traders, check if orderSender is safe
        if (result.orderSenderFee < 0) {
            require(
                Liquidation._isSolidSafe(state, orderSender),
                Errors.ORDER_SENDER_NOT_SAFE
            );
        }

        return (
            result.traderList,
            result.paperChangeList,
            result.creditChangeList
        );
    }
}

Impact

Lack of incentive to match trades when order sender fee is negative can prevent protocol from being used.

Code Snippet

https://github.com/sherlock-audit/2023-04-jojo/blob/main/smart-contract-EVM/contracts/impl/JOJOExternal.sol#L103C14-L192

Tool used

Manual Review

Recommendation

Consider having only positive fees for order matchers

sherlock-audit / 2023-04-jojo-judging

yixxas - Lack of incentives for users to match a trade if order sender fee is negative #383