Satisfied seller replacement, trading of zero amounts

[dexX7]

It was observed that offers, which have some units left for sale, but no more desire, are added back to the orderbook, as seller_replacement. This results in a faulty unit price, which causes orders, with inversed currency pair, likely to be matched against that faulty offer. Further, such matches result in a valid trade, where no tokens are actually traded.

Gathered logs:

• http://pastie.org/pastes/10097075/text?key=zyhapet9kp6npfj93w9grg
• http://pastie.org/pastes/10100648/text?key=9cueyytgdv61v79sad8xw
• https://gist.githubusercontent.com/dexX7/1853bfe4d58a2595334f/raw/4617bf94f897184ce824a400b6eeab8ae5400f01/faultytrade.log

Trade sequences:

• https://docs.google.com/spreadsheets/d/1PCxICIyw95aMoOHX1RTzrH9-OYbIF0_bkbZmwjAHcGw/edit?usp=sharing

A2 ends up with for sale = 0.00000049 and desired = 0.00000000:

A1: offer 0.00005100 TMSC  for 0.00000051 TDiv1
A2: offer 1.00000000 TDiv1 for 0.01000000 TMSC   <-- fills A1 complely
A3: offer 0.00999999 TMSC  for 0.99999900 TDiv1  <-- is filled by A2, A2 now has 0.00000049 for sale, but 0.00000000 desired
A4: offer 0.00000049 TMSC  for 0.00000049 TDiv1  <-- matches with A2, but it's a zero amount trade

RPC output with some extra info (using gettrade_MP, getorderbook_MP):

• https://gist.githubusercontent.com/dexX7/45097058b357cc82d5ce/raw/

Debug log, with some extra output for the desired amount:

• https://gist.githubusercontent.com/dexX7/abe123c5e37f722aae9b/raw/

The desired amount is calculated based on unit price and amount available, but it comes down to:

amount desired = amount left for sale * unit price
               = 0.00000049 * 0.01
               = 0.0000000049
               = 0

---

To conclude, this issue is unrelated to honoring the original unit price, but the total amount of tokens still desired, based on the original unit price, is less than one willet.

It is thinkable to avoid adding such offers back to the orderbook, which appears to resolve the initial issue, however, this introduces the need for additional adjustments, outside of the meta DEx core logic, to reflect the "satisfaction" of that order. In particular, when using gettrade_MP to get information about the order, the status is reported as "cancelled part filled". Is there anything else, which should be taken care of, or might be affected, assuming such offers were not added back to the orderbook?

Ping @marv-engine (to confirm proper handling and the logic), @m21 (potential mitigation and implementation pitfalls), @zathras-crypto (for the RPC results).

[marvgmail]

the goal of a sell order is to sell the full number of tokens for sale, which may result in receiving more tokens than the original amount desired, so the remaining amount desired is not a determining factor for whether or not the sell order is still active.

@dexX7 @zathras-crypto @m21 does that help?

[dexX7]

@marvgmail: an example:

Let's assume there is an offer with an unit price of 0.1, which gets partially filled. After the partial fill, there is an amount of 0.0000'0004 units left fore sale, and due to the unit price of 0.1, the amount desired of the updated offer would be 0.0000'0004 * 0.1 = 0.0000'0000'4, which is less than the minimal tradable amount of 0.0000'0001.

What should happen? Three outcomes:

1. Insert an updated offer of 0.0000'0004 for 0.0000'0000
2. Insert an updated offer of 0.0000'0004 for 0.0000'0001
3. Don't insert an updated offer, but consider the offer as filled

The first outcome is probably not what we want, but the second neither, because this could result in a lot of 1 willet/satoshi leftovers, which are offered at an higher price, due to the rounding up.

[zathras-crypto]

@dexX7 @marvgmail the remaining trade after a partial fill should be subject to the same rules as any other trade. What I mean by this is that there is nothing to stop you listing a trade with an amount desired of 1 willet, so that behaviour should not be restricted at the partial trade level either.

I'm not so concerned about 'leftovers' - that was definitely an issue in DEx v1 but due to automatic order matching wouldn't those leftovers be scooped up and traded (assuming a suitable counter trade). What I'm getting at is I see it as less of an issue of a bunch of leftover open trades, and more of an issue of a bunch of 1 willet completed trades cluttering things up. Not ideal in either circumstance.

Basically we should always be deriving the unit price from the original amounts, and the amount desired calculated on the fly from still_left_forsale * the price. As @dexX7 notes above this would mean we are basically doing 4 * 0.1 and we need an integer result, thus the amount desired can either be 0 or 1.

There has been a lot of talk (and the spec explicitly states) that the price cannot change so whilst I don't object to partial trades with amount desired of 1 willet, I do object to the unit price being changed (more than doubled since it is now 4 for sale/1 desired, thus price is 0.25). Thus in my book this rules out option 2 also (option 1 is already ruled out because it breaks the rules of allowed trades).

That leaves option 3, which seems the cleanest from a math PoV (return the unsold tokens to the seller and consider the offer filled) but this still strikes me as awkward. Thinking purely in laymans terms - if I sell 12.00000000 SPT and the order is filled, when I see that only 11.99999996 SPT was actually sold I may think there is a bug.

I don't have any better suggestions at the mo, and it may have to be option 3 combined with user education/appropriate interface messaging.

[dexX7]

wouldn't those leftovers be scooped up and traded

No, because they are more expensive:

unit price = 0.1
amount for sale = 0.0000100
amount desired  = 0.0000010

amount for sale = 0.0000004 (leftover)
amount desired  = 0.0000001 (rounded to the minimum tradable amount)
=> unit price = 0.25

Thinking purely in laymans terms ... this still strikes me as awkward

There are three routes to handle it in general, see the first post in OmniLayer/spec#173. The logic applied in the spec is route B: "amounts are updated based on amounts sold and amount desired adjusted based on earlier unit price", and this has the side effect of "... but user B only wanted to buy 15 MSC, not 17 MSC!", if any better-than-expected fill/trade is involved.

It's probably a matter of perspective.

Unfortunally the issue remains, but it's simply shifted: assuming no overpriced order is created with the leftover (case 3 from above), then the orderbook remains uncluttered, but the user ends up with that tiny amount he can't really use. In my opinion this is still the better outcome, but sort of weird nevertheless.

[marv-engine]

@dexX7 if I understand your example, it appears you are recalculating the
unit price based on the amounts remaining. That won't happen. So offers
will match with exactly the same terms regardless how much they have
remaining to sell.

Logically, the amount desired is not used after the original unit price is
computed. Also think of the amount desired as the minimum amount desired.
It's ok to get more than that.

On Tuesday, April 28, 2015, dexX7 notifications@github.com wrote:

wouldn't those leftovers be scooped up and traded

No, because they are more expensive:

unit price = 0.1
amount for sale = 0.0000100
amount desired = 0.0000010

amount for sale = 0.0000004 (leftover)
amount desired = 0.0000001 (rounded to the minimum tradable amount)
=> unit price = 0.25

Thinking purely in laymans terms ... this still strikes me as awkward

There are three routes to handle it in general, see the first post in
OmniLayer/spec#173
OmniLayer/spec#173. The logic applied in
the spec is route B: "amounts are updated based on amounts sold and amount
desired adjusted based on earlier unit price", and this has the side effect
of "... but user B only wanted to buy 15 MSC, not 17 MSC!", if any
better-than-expected fill/trade is involved.

It's probably a matter of perspective.

Unfortunally the issue remains, but it's simply shifted: assuming no
overpriced order is created with the leftover (case 3 from above), then the
orderbook remains uncluttered, but the user ends up with that tiny amount
he can't really use. In my opinion this is still the better outcome, but
sort of weird nevertheless.

—
Reply to this email directly or view it on GitHub
#21 (comment).

Marv Schneider
marv@engine.co

[dexX7]

@marv-engine: I mentioned the updated and more expensive unit price as consequence, if the leftover amount of 0.00000004, combined with the original unit price of 0.1, would result in a trade, where the traded amount is rounded to the minimal amount of 0.00000001.

Given the example, what would you say is the expected outcome?

[zathras-crypto]

Unfortunally the issue remains ... case 3 from above

Yeah at this stage the only viable option is case 3 - I was hoping for some inspiration on some other way to handle it but none has been forthcoming so far :(

[marv-engine]

@dexX7 Here's another excerpt from the spec:

Notes on rounding, with me (the new order) purchasing from Bob (the existing order):

1. First determine how many representable (indivisible) tokens I can purchase from Bob (using Bob's unit price)
   • This implies rounding down, since rounding up is impossible (would require more money than I have)
   • Example: if Bob has 9 indivisible tokens for sale, and I can afford 8.9 of them, round down to 8
2. If the amount I would have to pay to buy Bob's tokens at his price is fractional, always round UP the amount I have to pay
   • This will always be better for Bob. Rounding in the other direction will always be impossible (would violate Bob's required price)
   • If the resulting adjusted unit price is higher than my price, the orders did not really match (no representable fill can be made)
   • Example: if those 8 tokens would cost me 15.1 indivisible tokens, I must pay 16 tokens, or NO SALE

So, as indicated, the effective unit price in your example is now 0.25. This means the order for the remaining 0.00000004 will stay in the book until someone offers to buy 0.00000004 at a unit price of at least 0.25, or the seller modifies or cancels the order.

This effect will happen whenever this type of rounding occurs. The magnitude of the effect will depend on the specific numbers.

I don't like option 3 because a user may be selling in order to liquidate all his holdings of that property. Option 3 would prevent that from happening. I guess if it does happen, the user could explicitly issue another sell order at a unit price of 0.25, but why force the user to do that? If nothing else, that flow would move his sell order from the oldest at that effective price to the newest.

[dexX7]

Thanks @marv-engine, this helps a lot, and I noticed the example from above is no exceptional situation (in the context of the core logic), which I initially assumed, due to the bias of the current implementation without original unit price and "truncated" amount desired.

The following sequence should cover this situation:

Order	For Sale	Desired	Unit Price	Inverse Unit Price	Match?
A	0.00000006 MSC	0.00000006 SPX	1.00000000 SPX/MSC	1.00000000 MSC/SPX	No
B	0.00000010 SPX	0.00000001 MSC	0.10000000 MSC/SPX	10.00000000 SPX/MSC	Yes
C	0.00000001 MSC	0.00000010 SPX	10.00000000 SPX/MSC	0.10000000 MSC/SPX	No
D	0.00000001 MSC	0.00000004 SPX	4.00000000 SPX/MSC	0.25000000 MSC/SPX	Yes

1. When A is added, then there is no other order to match against, and this results in a new offering.

2. When B is added, then B is matched against A, and A is filled completely by B, but there is an amount left for sale of 0.00000004 SPX @ 0.1 MSC/SPX.

3. When C is added, then C and B are not matched, due to the minimal tradable amount of 0.00000001 MSC, which implies an effective unit price of at least 0.25 MSC/SPX to make the trade happen.

4. When D is added, then D and B are matched, and both orders are filled completely.

After this trading sequence, order A, B and D were filled, but C is still unmatched, and should be listed in the orderbook.

[zathras-crypto]

So, as indicated, the effective unit price in your example is now 0.25

This is the part that confuses me - so much discussion about how the unit price cannot change, but this implies that a change from a unit price of 0.1 to a unit price of 0.25 is fine?

Am I missing something guys?

[dexX7]

Let's define:

The unit price is the result of the original amount desired / original amount for sale, and should not change, while the effective unit price is the result of the actually traded amounts, or amounts to be traded.

1. Alice offers 20 apples for 10 oranges (unit price 0.5)
2. Bob offers 12 oranges for 17 apples (unit price ~1.4, reciprocal ~0.7)
3. Alice's and Bob's orders match (0.5 < ~0.7)

The outcome would be to move 17 apples from Alice to Bob, in return for 17 * 0.5 = 8.5 oranges from Bob to Alice.

Because they want to trade only full fruits, the number of oranges is rounded up to 9 oranges. This new effective unit price of 9/17 = ~0.529 is acceptable for both, and therefore the trade happens.

[zathras-crypto]

OK, I think I see where you're going. Perhaps I just have UI work stuck in my head where I'm constantly looking at unit prices we provide to the user. In your example:

unit price = 0.1
amount for sale = 0.0000100
amount desired  = 0.0000010

amount for sale = 0.0000004 (leftover)
amount desired  = 0.0000001 (rounded to the minimum tradable amount)
=> unit price = 0.25

I was thinking we would thus need to display the trade before partial match as a unit price of 0.1, and the trade after partial match as 0.25, thus in my mind we were "changing the unit price".

If we maintain a unit price of 0.1 in our interactions with the user I just want to make sure there are no cases where we are providing inaccurate info.

[marv-engine]

I was thinking we would thus need to display the trade before partial match as a unit price of 0.1, and the trade after partial match as 0.25, thus in my mind we were "changing the unit price".

this could be handled by the UI - e.g. indicate those matched orders where the effective unit price is different than the original unit price.

[dexX7]

I was thinking we would thus need to display the trade before partial match as a unit price of 0.1, and the trade after partial match as 0.25, thus in my mind we were "changing the unit price".

I agree that the effective unit price should be shown in the UI, because this is what matters for the counterparty.

This raises another question: let's assume prices are shown with up to 8 decimals in the UI, but the digits at positions 9+ are not 0, how would that be handled? Consider the following prices, and let's ignore the amounts up for sale for the sake of an example:

• 1.00000000[00....] SPX/MSC
• 1.00000000[01....] SPX/MSC
• 1.00000000[99....] SPX/MSC

I tend to think it would be less unexpected, if all prices with "hidden" digits are rounded up to the next satoshi/willet, whether the effective price is 1.00000000[00....000001] or 1.00000000[99....99999].

[dexX7]

I believe this issue is very close to being resolved by the currently pending/close-to-be-ready changes.

[marv-engine]

The effective unit price is known only after a match is made, because it depends on how many tokens are remaining for sale and how many would be exchanged as a result of the matching process.

As for the number of decimal digits of the unit price, the smallest unit price is 0.000000000000000000000000001084 (0.00000001 tokens desired / 9,223,372,036,854,775,807 tokens for sale).

Unless I'm missing something (which is entirely possible), we shouldn't round the price in the order book, but we should show the effective unit price after a match is made.

[dexX7]

As for the number of decimal digits of the unit pricem the smallest unit price is ...

Not all rational numbers have a finite number of decimal digits (take 1/3 for example), and the minimal distance between two rational numbers is not necessarily 1/MAX, so 30 decimal digits won't be enough to compare offers by unit price.

I created #31 to discuss the interface, especially given that I assume there is more to discuss in this context rather sooner than later. :)

[zathras-crypto]

If it's feasible, I think it would be great to only 'expose' an 8 digit price. More decimals can be used internally sure, but looking at Marv's example:
0.000000000000000000000000001084 (0.00000001 tokens desired / 9,223,372,036,854,775,807 tokens for sale)
Showing a unit price of 0.00000001 seems just incorrect.

[dexX7]

Let's go down the rabbit hole..

1. Faulty rounding:

The newly added assertion, to guarantee a successful exchange of tokens, is violated in an attempt to transfer 0 tokens, when C and B are matched in the trade sequence from above: #21 (comment)

This is because the rounding of xToInt64() is faulty, in particular: adding 0.5 to the floating number does not ceil the number, and 0.499... [+0.5 to ceil] is still 0 after the conversion [should be 1].

2. Trading more than available:

After the rounding is fixed, the assertion is once again violated:

B has 0.00000004 SPX @ 0.1 MSC/SPX left for sale. Because the desired amount would be less than one satoshi, the amount is ceiled to 0.00000001. Based on the desired amount of 0.00000001, and an unit price of 0.1 MSC/SPX, the amount to trade would be 0.00000010 SPX. When trying to un-reserve that amount, it fails, because only 0.00000004 SPX are reserved.

3. Trading at a too expensive effective price:

Given that the effective price does not necessarily equal the original prices, there must be a check to confirm, whether it's still an accepted price and not too expensive. If this is the case, the execution should be stopped.

I started with oc-0.10-mdex-zero-amounts-dirty and the relevant commits are:

• b520fb5: round up, if there is any leftover
• 7624e79: the additional trading constraints

[zathras-crypto]

Let's go down the rabbit hole..

Thanks so much for this mate - can I just ask a quick question? Things like

This is because the rounding of xToInt64() is faulty

isn't xToInt64() based on using XDOUBLE, which you were replacing with boost::rational? Sorry if I've got the wrong end of the stick but what's the plan for this :)

[dexX7]

Isn't xToInt64() based on using XDOUBLE, which you were replacing with boost::rational?

Yes, but I'd like to replace it as late as possible. All the tests, i.e. the test plan and the others, don't fail, even with XDOUBLE (and neither with boost::rational). I'd like to iron out the core logic first, and ideally see XDOUBLE fail at least once, to have a test case to show that XDOUBLE is not precise enough.

I added a few more assertions, and it currently fails on testnet. It would be great, if someone could double check them, because I assume it might as well be an error on my end:

// before the trade
assert(0 < pold->getAmountRemaining());
assert(0 < pnew->getAmountRemaining());
assert(pnew->getProperty() != pnew->getDesProperty());
assert(pnew->getProperty() == pold->getDesProperty());
assert(pold->getProperty() == pnew->getDesProperty());
assert(pold->unitPrice() <= pnew->inversePrice());
assert(pnew->unitPrice() <= pold->inversePrice());

// inbetween, to provide the context
XDOUBLE xEffectivePrice = XDOUBLE(seller_amountGot) / XDOUBLE(buyer_amountGot);

// after the trade
assert(xEffectivePrice >= pold->unitPrice());    // << ??
assert(xEffectivePrice <= pnew->inversePrice()); // << ??
assert(0 <= seller_amountStillForSale);
assert(0 <= buyer_amountStillForSale);
// amountOffered referrs to the amount offered at the beginning of the trade
assert(seller_amountOffered == seller_amountStillForSale + buyer_amountGot);
assert(buyer_amountOffered == buyer_amountStillForSale + seller_amountGot);

[zathras-crypto]

Yes, but I'd like to replace it as late as possible

OK thanks, that makes sense.

it currently fails on testnet

I can indeed confirm that. I started working to clean up the UI branch and disabled the asserts temporarily to test some changes. Would it be better to hold off on the UI stuff and join you in tracing through MetaDEx assert failures?

[dexX7]

Thanks, I'm going through it step by step, and I roughly have an understanding of the issues. I think it's probably best to disable the assertions and focus on the UI, assuming there is still something to do. - I'll post, if I reach a dead end. :)

I noticed you were using XDOUBLE in a RPC function, and there is probably more need for this in the UI. I suggest to move xToString() into the header, and avoid any direct use of XDOUBLE, if possible. This makes it probably easier to switch the datatype (e.g. boost::rational).

[dexX7]

Testnet fully processed!

An additional check was required:

if (buyer_amountGot == 0) {
    if (msc_debug_metadex1) file_log(
        "-- stopping trade execution, because buyer has not even enough "
        "tokens to purchase 1 unit\n");
    ++iitt;
    continue;
}

... and finally replacing the floats, which was more complicated than anticipated, because boost::rational<int64_t> actually overflows internally in some multiplications and divisions, and I ended up using boost::rational<int128_t>. I'm convinced it works, and it does, what it should do, but it feels overly complex.

I'll consolidate and cleanup everything over the next day, and then we should very, very closely double-check the logic. It's a bit unfortunate that the calculation doesn't follow the example of the spec, and sort of goes backwards.

Maybe I'm going to test an alternative implementation of the core (it's within those \\\\\), and ideally both routes come to similar results, basically serving as confirmation.

[zathras-crypto]

Exceptional work as always my friend - with thanks :)

[dexX7]

There is an interesting trade, where the alternative implementation differs:

A offers 25 SPX for 2.50 MSC @ >= 0.10 MSC/SPX
B offers 0.55 MSC for  5 SPX @ <= 0.11 MSC/SPX

Outcome 1:

A gets 0.55 MSC and sells 5 SPX
B pays 0.55 MSC and buys  5 SPX @ 0.11 MSC/SPX

A has 20 SPX left for sale, B has nothing left for sale.

Outcome 2:

A gets 0.50 MSC and sells 5 SPX
B pays 0.50 MSC and buys  5 SPX @ 0.10 MSC/SPX

A has 20 SPX left for sale, B has 0.05 MSC left for sale.

[zathras-crypto]

I noticed you were using XDOUBLE

Yeah I actually use XDOUBLE both in the RPC layer and the UI, because sometimes we need to divide by COIN in order to handle divisibility for the trade correctly. Agree it would be nice not to but if the non-primary part of the trade (ie the property that is not MSC/TMSC) is not divisible, we need to manipulate it.

Example:

             type: 21 (MetaDEx token trade)
         property: 13 (SP token: 13)
            value: 10
 desired property: 1 (MSC)
    desired value: 2.00000000
           action: 1

Without dividing by COIN:

"unitprice" : "20000000.00000000000000000000000000000000000000000000000000",

With dividing by COIN:

"unitprice" : "0.20000000000000000000000000000000000000000000000000",

[dexX7]

@zathras-crypto: yes, makes sense. :) My point was just: let's put all this "somewhere" in one place, so it's easier to edit globally.

I'm unfortunally away until tomorrow, but the logic seems to be correct now. Still up to do: cleanup (mostly getting rid of all the debug output), maybe optimization.

• omnicore-0.0.10...dexX7:oc-0.10-mdex-zero-amounts-dirty-rational-alt

Before the release the assertions should probably be removed, to prevent shutting down the whole network (if there is a violation).

[dexX7]

I have currently 4 trades, or trade sequences, which cover most edge cases. It would be great, if we could review the outcomes next week. If this turns out to be solid, so is the core logic. :)