You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We currently rely on filledAmount[orderUid] to determine if a fill-or-kill order was executed or not. However, if sellAmount is zero (and buyAmount is zero, as noted below) and the order is executed then filledAmount does not change, which means that the order can be executed again. This is problematic as the fee would be taken multiple times from the user.
This is not critical for normal user orders as we don't generate zero-sell-amount orders in the interface. It's however unlikely but possible that a user creates an order selling nothing for a fee and would be affected by this issue. Most importantly, it could cause bugs in contracts that rely on filledAmount as well, see for example the ETH-flow contract case.
In the current code, this is not an issue for partially fillable orders.
The text was updated successfully, but these errors were encountered:
Looking at the code, this only affects orders where sellAmount == 0 && buyAmount == 0, and AFAICT affects both SELL and BUY orders. Specifically, we have:
The following assertion:
require(
order.sellAmount.mul(sellPrice) >= order.buyAmount.mul(buyPrice),
"GPv2: limit price not respected"
);
This means, if sellAmount == 0, then 1. will only not revert if buyAmount * buyPrice == 0, which means buyAmount == 0 or buyPrice == 0.
fedgiac
changed the title
Zero-sell-amount fill-or-kill orders can be executed more than once
Zero-amount fill-or-kill orders can be executed more than once
Oct 5, 2022
We currently rely on
filledAmount[orderUid]
to determine if a fill-or-kill order was executed or not. However, ifsellAmount
is zero (andbuyAmount
is zero, as noted below) and the order is executed thenfilledAmount
does not change, which means that the order can be executed again. This is problematic as the fee would be taken multiple times from the user.This is not critical for normal user orders as we don't generate zero-sell-amount orders in the interface. It's however unlikely but possible that a user creates an order selling nothing for a fee and would be affected by this issue. Most importantly, it could cause bugs in contracts that rely on
filledAmount
as well, see for example the ETH-flow contract case.In the current code, this is not an issue for partially fillable orders.
The text was updated successfully, but these errors were encountered: