Calculation of `z` in function `encodeOrder` can lead to future errors

[barakman]

The calculation of z in function encodeOrder (see here) can lead to errors which are overlooked during strategy creation (or update), but will emerge during runtime, i.e., when the strategy is traded at some future point:

z : H === M || y === 0n ? y : (y * (H - L)) / (M - L)

The first problem here is that when y == 0, this code overlooks the potential problem of H > M == L, which is an invalid strategy configuration:

• When M == L, you'd want to abort with an exception of some sort, which the check y == 0 masks out because the division by M - L (i.e., by 0) is never executed
• When M != L, the check y == 0 can be safely omitted, because the result will be the same (0) with and without this check

I suspect that the check y == 0 was added specifically in order to avoid the division by 0, but that exception is actually desired here, as explained above.

The second problem here is that the value of z might come out larger than the maximum permitted in the contract (2^128-1).
That by itself is fine, because the strategy-creation contract function will not get executed (or will revert upon execution).
But there might be a combination of L, H and M which initially gives a valid value of z, but at some point in the future, trading the strategy will start reverting because the contract function will attempt to set the value of z to be higher than the maximum.
In other words, the marginal rate of the order will never be able to reach its lowest rate.

This undesired scenario can be detected ahead of time, via something like:

if y * (H - L) >= 2 ** 128 then raise an exception