Rounding errors checks create trades that can not be executed

[Dvisacker]

Solidity does not handle float arithmetic and therefore errors are introduced when numbers of different order of magnitude are multiplied divided.
The rounding error check functions prevents trades that would cause the exact traded amounts to be too far off from the original expected amount.

    /// @dev Checks if rounding error > 0.1%.
    /// @param numerator Numerator.
    /// @param denominator Denominator.
    /// @param target Value to multiply with numerator/denominator.
    /// @return Rounding error is present.
    function isRoundingError(uint numerator, uint denominator, uint target)
    public
    pure
    returns (bool)
    {
        uint remainder = mulmod(target, numerator, denominator);
        if (remainder == 0) return false;
        // No rounding error.

        uint errPercentageTimes1000000 = (remainder.mul(1000000)).div(numerator.mul(target));
        return errPercentageTimes1000000 > 1000;
    }

The isRoundingError function checks whether (numerator/denominator) x target returns a value that is close to the result that would be expected in a float multiplication and currently checks whether the error is above 0.1%

In our case:

numerator = trade amount
denominator = buy amount
target = sell amount

For example, if

• buy amount = 1000
• sell amount = 3000
• trade amount = 1000
  Then the rounding error is equal to 0.001 = 0.1%

First, if i am not mistaken, this creates drastic conditions to fill orders, for example the following order will return a rounding error event and will not be accepted

• buy amount = 1000
• sell amount = 3000
• trade amount = 500
  Then the rounding error is equal to 0.004 = 0.4%

Secondly, this creates unfillable orders such as in the following case.

1. Alice puts up a trade with

• buy amount = 1000
• sell amount = 3000

2. A trade is sent with

• amount = 990
  The rounding error is equal to 0.

3. A trade is sent that fills the remaining order.

• amount = 10
  The rounding error is equal to 0.1 = 10%.
  I think in this case no trade can fill the remaining order.

This doesn't seem like an issue for most tokens with 18 decimals. But it might be worth looking into.

I believe the main issue is that the matching-engine needs to be aware of this and remove unfillable orders appropriately.