Zero-amount fill-or-kill orders can be executed more than once

[fedgiac]

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.

[nlordell]

Looking at the code, this only affects orders where sellAmount == 0 && buyAmount == 0, and AFAICT affects both SELL and BUY orders. Specifically, we have:

1. The following assertion:

   require(
       order.sellAmount.mul(sellPrice) >= order.buyAmount.mul(buyPrice),
       "GPv2: limit price not respected"
   );

2. This means, if sellAmount == 0, then 1. will only not revert if buyAmount * buyPrice == 0, which means buyAmount == 0 or buyPrice == 0.
3. We also have the following division:

   executedBuyAmount = executedSellAmount.mul(sellPrice).ceilDiv(buyPrice);

4. The division in 4. will revert if buyPrice == 0 hence buyAmount == 0 in order for 1. to not revert.

[fleupold]

Let's document this in the README of this contract (maybe we can add a "known issues" section?) and potentially add a note on https://docs.cow.fi/tutorials/how-to-submit-orders-via-the-api/4.-signing-the-order

[igorroncevic]

Updated the docs to reflect the fact that this is still an issue regardless of whether the order is partially fillable or not: cowprotocol/docs#624