Assessment reports>Ostium>Low findings>Unexecutable trades might be added to ,tradesToTrigger
Category: Business Logic

Unexecutable trades might be added to tradesToTrigger

Low Severity
Low Impact
Medium Likelihood

Description

The function _getLimitOrdersToTrigger loops through the open limit orders to find the trades to execute. If the trade type is OpenOrderType.STOP, the o.targetPrice should be compared against a.price instead of priceAfterImpact; otherwise, the trade would be added to the tradesToTrigger and would fail in the following check in OstiumTradingCallbacks if o.targetPrice is greater than a.price and less than priceAfterImpact:

cancelReason = (
    o.orderType == IOstiumTradingStorage.OpenOrderType.LIMIT
        ? (o.buy ? priceAfterImpact > o.targetPrice
            : priceAfterImpact < o.targetPrice)
        : (o.buy ? uint192(a.price) < o.targetPrice
            : uint192(a.price) > o.targetPrice)
)

Impact

Some trades that could not be executed may inadvertently be included in the tradesToTrigger array, resulting in an unnecessary consumption of gas.

Recommendations

Replace the priceAfterImpact in the check in case of STOP orders to a.price:

    o.orderType == IOstiumTradingStorage.OpenOrderType.LIMIT
        ? (o.buy ? priceAfterImpact <= o.targetPrice : priceAfterImpact >= o.targetPrice)
-        : (o.buy ? priceAfterImpact >= o.targetPrice : priceAfterImpact <= o.targetPrice)
+        : (o.buy ? a.price >= o.targetPrice : a.price <= o.targetPrice)

Remediation

This issue has been acknowledged by Ostium Labs, and a fix was implemented in commit bf6d763c.

Zellic © 2024Back to top ↑