opendexnetwork / opendex.network

Website 👋
https://opendex.network
GNU Affero General Public License v3.0
19 stars 10 forks source link

Remove is_buy flag from the protocol #21

Open cjdelisle opened 4 years ago

cjdelisle commented 4 years ago

Since buy of BTC/LTC is precisely the same as a sell of LTC/BTC, the protocol allows for two ways to express the same intent, which gives rise to potential bugs in implementation.

If the is_buy flag were removed and every operation were considered a buy, then there would be no way for implementations to not match BTC/LTC buys with LTC/BTC buys, which should obviously be matched.

The down-side of this is that in order to compute whether two orders match, you need to perform floating point division which might round incorrectly and cause you to try to make an invalid deal. But implementations should verify before performing a take that what they're requesting is what is being offered, which they already have the capability to do.

cjdelisle commented 4 years ago

If double price were replaced with uint64 offer_quantity then the sticky business of float math is removed entirely. Floating point is known to be a source of non-deterministic behavior: https://stackoverflow.com/questions/27149894/does-any-floating-point-intensive-code-produce-bit-exact-results-in-any-x86-base

kilrau commented 4 years ago

What's your take on this? @hatmer @LePremierHomme

hatmer commented 4 years ago

This simplification of the protocol improves clarity.

LePremierHomme commented 3 years ago

Sorry for the delay.

The buy/sell side reduction to the pair definition make sense to me, although i'm not sure it's commonly used in trading platforms. It might actually make more sense to have this as a DEX, because no one is dictating the pairs, which is problematic because AFAIK, in XUD, a buy of BTC/LTC will indeed not get matched with a buy of LTC/BTC, as they are defined as distinct pairs.

The price floating point wasn't an issue so far because it is only used in comparisons. If we'll need to "reverse" it by performing a division this is indeed problematic and we'll need to use some fixed point solution.