Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7][svk] Fractional trade dust left over sometimes #200

Closed
wmbutler opened this issue Jul 21, 2017 · 28 comments
Closed

[7][svk] Fractional trade dust left over sometimes #200

wmbutler opened this issue Jul 21, 2017 · 28 comments
Assignees
Labels
[3] Bug Classification indicating the existing implementation does not match the intention of the design
Milestone

Comments

@wmbutler
Copy link
Contributor

When you click on a buy/sell order on an asset paired with BTS to fill it in the UI, and execute it, sometimes the UI places the order with an error of +/-0.0001 of the asset being traded. When this remaining order for 0.0001 subsequently fills, due to the small size of the order and the fees involved it winds up executing at a much higher/lower price than listed (ie. a sale at 2.2bitCNY coming to 2.5 after fees). In some instances, the chart on the UI will draw a wick extending to the substantially higher/lower price the small order wound up executing at. This makes the chart appear deceptive, particularly to new traders who have not been using the DEX long enough to see one of these orders execute, and observe the subsequent candle wick.

@wmbutler wmbutler added the [3] Bug Classification indicating the existing implementation does not match the intention of the design label Jul 21, 2017
@happyconcepts
Copy link
Contributor

I can confirm. I watched $0.0003 execute ETH/bitUSD the other day on the chart and confirmed the trade history...yes that is 3/10,000 of one penny in value for that trade. Is this the 'sig fig' issue or else why is this dust reported?

@landry314
Copy link

landry314 commented Jul 24, 2017

Absolutely, this is an incredibly important bug to squash for two reasons:

  1. The candles are drawn with huge wicks (and sometimes, if the timing is exactly at the end of a candle's period, a huge candle body is drawn) because the little orders left over are executed way out of range and the candle drawing program is not designed to ignore these order executions. This makes the built-in chart hard to read and ugly. To fix this the candle drawing routine needs only ignore any amount less than a certain amount: 0.001 for example.

  2. When the last market trade was one of these little trades then the balance calculation for the account goes way off. Sometimes this leads to my balance displayed at the top saying I have much more or less than I actually do until another trade executes in the right range. This can really scare or confuse someone who doesn't understand why this is happening. "OMG, where did my 10,000 USD go?!" for example... it isn't good to scare users like this.

I don't know why these little amounts even exist or why they would trade so far out of range but if this is more than a GUI issue and actually an issue that is more deeply with Bitshares itself than it needs to be squashed immediately and therefore nothing would be necessary to change in the GUI. If Bitshares created this "fractional dust" on purpose to deal with something in the way it works then the GUI needs to understand it and to adjust the display properly.

These wicks and bad balance calculations make bitshares look and feel poorly written which will prevent it from really succeeding. Let's fix it up and get to the next level here!

@happyconcepts
Copy link
Contributor

For at least 20 minutes today ETH showed $400 price, dusted.

@happyconcepts
Copy link
Contributor

I believe this is a top priority bug from a UX perspective. The crypto space is uncomfortable enough for newbs without their balances recalculating for no good reason.

How about enforcing a minimum order size or even better, letting users set a filter to ignore insignificant trades in charts and trade history? @svk31 @bytemaster @wmbutler

@landry314
Copy link

@happyconcepts No user would WANT to have those wicks or a poorly calculated balance so making it a filter is not the issue. The issue will be solved from finding how to define the dust in the code and how to clear it away in the GUI.

@happyconcepts
Copy link
Contributor

happyconcepts commented Jul 25, 2017 via email

@landry314
Copy link

landry314 commented Jul 25, 2017

What? I am saying this is a bug and there is no need to add an option to users to leave in the bug once it is solved.

It may sound easy to you to simply add a filter but it is not that easy. The devs have to find out exactly how to isolate the trades in question and how to change to GUI to ignore them. It is not like they WANT to have long meaningless wicks and bad balances; they just haven't been able to fix the bug yet.

I was not minimizing you contribution. I appreciate it and I am entirely on board with your desire to improve this program.

@happyconcepts
Copy link
Contributor

happyconcepts commented Jul 26, 2017 via email

@wmbutler
Copy link
Contributor Author

OMG, between the two of you I don't know which one ruins my Sunday more.

@wmbutler wmbutler changed the title Fractional trade dust left over sometimes [2] Fractional trade dust left over sometimes Aug 4, 2017
@wmbutler wmbutler added this to the 170901 milestone Aug 4, 2017
@calvinfroedge
Copy link
Contributor

@wmbutler Can you provide exact reproduction steps for this?

@wmbutler
Copy link
Contributor Author

I've observed it but cannot reliably reproduce. @svk31 says it's caused by the fees associated with a variable length of the memo field. I'll close it since it's my issue and I can't reproduce.

@wmbutler wmbutler added question and removed [3] Bug Classification indicating the existing implementation does not match the intention of the design labels Aug 15, 2017
@wmbutler wmbutler removed this from the 170901 milestone Aug 15, 2017
@svk31
Copy link
Contributor

svk31 commented Aug 15, 2017

No I do not think it's due to fees. The dust orders execute at a different price because Bitshares has no concept of price, only ratios of amounts to buy and sell. When there's only one satoshi left, the trade has to execute at a ratio (aka price) that is different from that originally defined for the order. The actual issue here is the existence of dust orders.

I've spent countless hours on this issue and have apparently yet to figure out how to fix it, although I suspect it's now caused more by bots making incorrect orders than orders from the GUI.

@happyconcepts
Copy link
Contributor

happyconcepts commented Aug 31, 2017

This phenomenon actually occurs in real securities markets, but at what seems to be a far lower frequency than I have observed on the Dex.

When a stock trader finishes a large institutional order, for example, there is sometimes a "cleanup" print to the ticker tape for the last transaction(s) that is/are "away" from the market price. Usually market liquidity, meaning other buyers and sellers, are participating so that the price before the cleanup price again quickly becomes the market equilibrium...and the market .

Afaik the "cleanup print" phenomenon is only because market rules require disclosure of all trades...but in its defense, price transparency is a good thing for credibility of any financial marketplace.

So to not show a real trade, even if it's miniscule, is detrimental toward growing the user base imho.

This. Dust. I will try to help solve it @svk31

@wmbutler
Copy link
Contributor Author

wmbutler commented Sep 1, 2017

I'm increasing the bounty to 4 hours.

@wmbutler wmbutler changed the title [2] Fractional trade dust left over sometimes [4] Fractional trade dust left over sometimes Sep 1, 2017
@wmbutler wmbutler added this to the 170914 milestone Sep 1, 2017
@wmbutler wmbutler added [3] Bug Classification indicating the existing implementation does not match the intention of the design and removed question labels Sep 1, 2017
@happyconcepts
Copy link
Contributor

Thanks, I would like to work this with @svk31 -- can we push the delivery milestone out to Oct 1? I anticipate it is going to take some testing time especially.

@wmbutler
Copy link
Contributor Author

wmbutler commented Sep 4, 2017

I'm bumping this to 7 hours but leaving it in the 170914 Sprint. I'll assign it to you but you will need to work out @svk31 cut based upon time spent.

@wmbutler wmbutler changed the title [4] Fractional trade dust left over sometimes [7] Fractional trade dust left over sometimes Sep 4, 2017
@wmbutler wmbutler changed the title [7] Fractional trade dust left over sometimes [7][happyconcepts] Fractional trade dust left over sometimes Sep 4, 2017
@svk31
Copy link
Contributor

svk31 commented Sep 5, 2017

I'm not convinced this is fixeable in the GUI. In all the testing I've done in my last iteration on this issue (there have been many) I never managed to create any new dust orders after implementing the new market classes. It's possible the GUI still causes it in some cases but if so we need actual reproducible test cases, without them this issue is impossible to fix.

And even though it may be fixed in the GUI, others might cause them elsewhere, especially bots making partial fills, and the real issue may actually lie with the bitshares-core code as discussed in these issues: bitshares/bitshares-core#342 and bitshares/bitshares-core#132

@svk31
Copy link
Contributor

svk31 commented Sep 5, 2017

OK I decided to look at this after all, and was able to recreate the situation on the testnet. To reproduce:

  1. create a sell order
  2. click the sell order in the orderbook
  3. modify the buy order quantity and execute the partial match
  4. click the sell order again and try to execute a full match, it should leave a satoshi dust

Looking at how to fix it.

@landry314
Copy link

Wow, you figured out how to reproduce, @svk31! I am always clicking and reclicking, changing, playing with the orderbook, so it was happening a lot to me. This is a very high priority bug imho because if fixed it will solve the wick problem and start to make the chart usable. Also, the jumping account value calculation which takes in the last market order to calculate will be fixed too. Looking forward to this bug squashing.

@svk31 svk31 self-assigned this Sep 6, 2017
@svk31 svk31 changed the title [7][happyconcepts] Fractional trade dust left over sometimes [7][svk] Fractional trade dust left over sometimes Sep 6, 2017
@wmbutler
Copy link
Contributor Author

wmbutler commented Sep 6, 2017

@happyconcepts leaving this assigned to @svk31 only, FYI.

@happyconcepts
Copy link
Contributor

@wmbutler thanks for the heads-up...the issue is in good hands :)

@svk31
Copy link
Contributor

svk31 commented Sep 11, 2017

Should be fixed with my latest commit but I've been wrong about this before, will close this issue but we should keep it in mind and reopen after the next release if it still happens.

@svk31 svk31 closed this as completed Sep 11, 2017
@wmbutler
Copy link
Contributor Author

Winning!!

@landry314
Copy link

Seems this wasn't actually fixed... guessing we are waiting to integrate trading view as discussed in issue #361 but will this fix the problem? Because if the same fractional trading dust data is being fed into trading view, I imagine the same problem with occur.

Since posting in closed issues doesn't seem to work well, I am going to tag you guys. I can open an identical issue to this one but... it just seems silly when we can reopen this one...

@svk31, @wmbutler...

@svk31
Copy link
Contributor

svk31 commented Oct 19, 2017

Why does it seem it wasn't fixed? We can only fix it in our GUI, bots and anyone using other interfaces are likely to create dust orders also, I've seen bots do it many times and there's nothing I can do about that.

Tradingview charts will change nothing here, what needs to happen is for bitshares-core to filter out dust orders when creating market history. Ideally it would also just cancel any orders that only had dust left which cannot fill at the defined price, which is what's causing these issues in the first place..

@landry314
Copy link

landry314 commented Oct 19, 2017

Ok, there seem to be a few issues in the core github that touch on this like the ones you tagged above in #200 (comment) and maybe issue 287 too...

Should I create an issue in the github for the core specifically about this problem? It would not be so technical as I am not as familiar with the code as you are but I could do it and reference this issue here then see what happens... I don't want to create a duplicate though.

This is a very important issue as having a trading platform with usable charts is a big plus and regardless of if we are using react or tradingview, those wicks make things very difficult. You need to see REAL trading ranges to trade properly. Also, as I have mentioned, it causes the Account Value to jump around sometimes as it is taking the last trade to calculate and when the last trade is out of range, the Account Value goes out of range and becomes a very poor estimate.

@abitmore
Copy link
Member

abitmore commented Nov 2, 2017

More discussion in bitshares/bitshares-core#449, and I proposed a fix in bitshares/bitshares-core#455. Please check.

@abitmore
Copy link
Member

abitmore commented Nov 2, 2017

Actually there are two issues in this ticket:

  • the chart. Described in previous comment.
  • the dust. This is somehow related to the rounding issue. IMHO UI's (not only bitshares-ui but also other UI's, bots and etc) should create orders more accurately to avoid leaving dusts, but we can't prevent people from submitting their own orders which may still leave dusts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[3] Bug Classification indicating the existing implementation does not match the intention of the design
Projects
None yet
Development

No branches or pull requests

6 participants