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↗.