Bug in market engine / matching cover_orders

[pmconrad]

Suppose for example on the CNY/BTS market the feed price is 0.1 CNY per BTS, there is an open bid at 0.095 CNY/BTS and a short with a limit of 0.085 CNY/BTS. Now the expiration date of a cover_order passes. What I'd expect to happen (see https://bitsharestalk.org/index.php?topic=8390.msg178760#msg178760 ) is that the cover_order is filled by the bid. What actually happens is - nothing.

The reason:

1. market_engine::get_next_bid fetches the next bid in the list. If the bid price is below the price feed and there are shorts available, it returns the next short instead - the bids will be fetched after all shorts have been processed.
2. market_engine::get_next_ask fetches the expired cover_order as expected.
3. market_engine::execute notices that _current_ask is a cover and performs some additional checks, in particular it checks if the bid price is less than 90% of the feed price. Here this is true, and in consequence _current_ask is dropped!
4. Processing continues with the next ask, if any.

[vikramrajkumar]

@theoreticalbts Please check this in the new market engine.

[theoreticalbts]

Is there an easy way to get the expiration date for a short position from the wallet? I'm using blockchain_market_list_covers but there really should be a way to get the expiration dates of my shorts without going through the blockchain or hard-coding the formula for expiration date (including grandfathering!) into whatever RPC client is requesting that information. (edit: _list_covers, was previously _list_shorts)

[bytemaster]

This is the proper behavior. We never force people to cover at less than 90% of the feed price. The order will sit there as expected until either the feed price falls or someone places a bid at 91% of the feed price.

[pmconrad]

But the bid is 95% of the feed price. Why is it not used for covering?

[vikramrajkumar]

Is there an easy way to get the expiration date for a short position from the wallet?

@theoreticalbts It looks like it should be part of the raw output of wallet_account_order_list, but it may not be included in the CLI pretty-printing.

[abitmore]

@pmconrad a suggestion: you could try to fix it yourself and submit a pull request (I believe you're a good enough developer).

[pmconrad]

I'm certainly capable of providing a fix. But this is about fiddling with market logic, and I'm unsure about what is the correct way to fix this.

IMHO get_next_bid should interleave bids and shorts ordered by execution price instead of simply preferring shorts when the next bid is below the feed. But AFAIK shorts are ordered by interest offered, which means we have conflicting goals here. And we might have to iterate through the shorts more than once, so the fix is really not so simple.

[bytemaster]

I see... what is going on here.

The short is selected... because it has the highest interest rate.
The short has a price limit of .85 which is more than 10% below the feed price.
The cover uses the limit price rather than the bid price.
If there exists a bid higher than the min(price_limit,feed_price) it should take priority.

[theoreticalbts]

First of all, @pmconrad you should be looking at develop branch. As part of my work implementing relative orders, I made additions to this code including substantial improvements to the logic merging various types of orders, see https://github.com/BitShares/bitshares/blob/18ac97f16b0da2aaad71ca2bf591441b51e03863/libraries/blockchain/market_engine.cpp#L626-830

The above comment generated some intense discussion. The fundamental issue is that you can have latent state changes -- i.e. feed updates change order semantics (whether the order executes and the price it executes at).

For each order type, you want to be able to have a "best" order of that type, such that failing to match against that best order means no other order of that type can match. Now if latent updates change the ordering within a type (i.e. two orders a, b of the same type may have a first for some feed values, b first for other feed values), you basically have to walk through every order on every feed update which will kill performance.

Latent updates can change the ordering within the short order type.

However, we can get around this by dividing shorts into "stuck" and "unstuck" categories. "Stuck" shorts execute at the price feed and are ordered based on interest rate. Unstuck shorts execute at their limit price and are ordered based on their limit price. Simply keep track of a list of stuck and unstuck shorts, and for every feed update, we only need to update shorts whose limit price the feed moved through. Which is likely small or zero.

So basically we just have to add a bunch of database indices, and in the market engine have separate merge clauses for stuck and unstuck shorts. @bytemaster is currently working on implementing that.

[bytemaster]

When sorting bids we need to create an index object of the form
{ price, type, interest, owner }

And then populate that index like so:

bid => { price, 3, 0, owner }
relative_bid => { feed * R, 2, 0, owner }
short => { min(feed,limit), 1, interest, owner }

After sorting we can iterate through bids in order. In practice, we only need to maintain the index for short orders and we can "merge sort" just in time from the bid and relative bid databases.

For Asks we have

ask => { price, 3, 0, owner }
relative_ask => { feed * R, 2, 0, owner }

Cover's are sorted by call price which is the "trigger price" and not the actual ask price. The actual ask price is the max( highest_bid_price, 0.9*feed ) if feed < call price, else max( highest_bid_price, feed ) if just expired.

[pmconrad]

Glad to see this in the hands of experts. :-)

[pmconrad]

There's another issue with covers. Don't know if the planned changes already cover it.

There's currently a margin call on the BTC/BTS market that can't be covered:

{"type":"cover_order","market_index":{"order_price":{"ratio":"0.033698020954319057","quote_asset_id":4,"base_asset_id":0},"owner":"BTSC3SgLy8Ebgp1uLq6PUrUudiMkxJM3fwYK"},"state":{"balance":217792,"limit_price":null,"last_update":"1970-01-01T00:00:00"},"collateral":143794477,"interest_rate":{"ratio":"0.","quote_asset_id":4,"base_asset_id":0},"expiration":"2015-09-25T20:59:00"}]}

Current feed price is 0.000033525036657829, i. e. it's below the limit. In my understanding, this margin call should be filled at the price feed or 10% below.
There are some shorts with a limit of less than 0.9*price_feed as described above. This leads to the ask (cover) eventually being dropped by market_engine::execute.
However, placing a short with higher interest and limit doesn't work either, because market_engine::execute checks if the short's limit is below the ask's limit (which is always the case, because the ask's limit is above the feed), and drops the short.

(Upon closer examination it should be possible the fill the cover with an unlimited short and sufficient interest at exactly the feed price.)

[abitmore]

Don't know if this has been discussed somewhere yet:
With the example in OP, in current v0.6 client, if one short with price_limit 0.91 and a higher interest than all other shorts in the market, the short will match with the cover at price 0.91. The guy could then dump some BTS to the bid order at 0.95 to get enough CNY, then cover the short order and make an instant profit.
This happens every day in the market. That's why nobody bid near the feed price.

[abitmore]

Failed to protect expired short orders by placing a short order with low limit_price and high interest rate.
(Looks like) with same interest rate, the order with higher limit_price takes priority. See this http://bitsharesblocks.com/blocks/block?id=2027457

[pmconrad]

I would think that with the same interest rate the owner address comes into play.

[abitmore]

Had a look on the develop branch, it seems the refactoring of market engine is almost finished, so issue #1315 (comment) should have been fixed already.

There is a TODO in the code which I think could be a issue:
https://github.com/BitShares/bitshares/blob/a79de7cd90859f4e0c1073988d0709571fb86095/libraries/blockchain/market_engine.cpp#L139
If _current_ask is a margin call order, the ask_price is always greater than feed_price; if _current_bid is a short order, the limit_price may be less than feed_price; according to the code, the short will be skipped at all, but it should match the ask if limit_price > feed_price * 0.9

[abitmore]

I would think that with the same interest rate the owner address comes into play.

Could be.. I placed a USD short "BTS2yD2gQTnTrdUD2X7yZWLdh6pcYUJLKARh". Will see.

//Edit:
with same interest, bigger address takes priority. So my order sucks. Anyway, it's unable to block the leaching bot by just placing one order no matter how big the address is, because the bot is creating new orders repeatedly, it will succeed sooner or later if it set same interest rate as mine.

[abitmore]

//EDIT: the comment below is incorrect. Just leave it here for reference.

In develop branch, wouldn't this line skip the short orders with limit_price < feed_price, cause they never get matched with margin call orders?
https://github.com/BitShares/bitshares/blob/a79de7cd90859f4e0c1073988d0709571fb86095/libraries/blockchain/market_engine.cpp#L76

[theoreticalbts]

This is fixed by those last two patches, confirmed fixed by MC test.