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

filter out the price data that not have enough precision. #287

Closed
wants to merge 2 commits into from
Closed

filter out the price data that not have enough precision. #287

wants to merge 2 commits into from

Conversation

btsabc
Copy link

@btsabc btsabc commented May 24, 2017

Signed-off-by: kinglaw 58291@qq.com

Signed-off-by: kinglaw <58291@qq.com>
b.close_base = trade_price.base.amount;
b.close_quote = trade_price.quote.amount;
if( b.high() < trade_price )
if(trade_price.base.amount >= 100 && trade_price.quote.amount >= 100)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although this is okay for most "popular" trading pairs, it's not ideal to be applied to all pairs, especially UIA's.

I think it's better that we record the data that how much difference is caused by rounding when matching orders, or simply record the "fair" price there, then process the data here.

@landry314
Copy link

Is this related to bitshares/bitshares-ui#200?

@pmconrad
Copy link
Contributor

The intent of this PR is unclear, but I think it is likely an attempt to fix that exact problem.

@landry314
Copy link

landry314 commented Oct 19, 2017

@abitmore, is your PR related to bitshares/bitshares-ui#200?

I am considering opening a new issue in the core for this because @svk31 thinks the issue goes deeper than the GUI and must be fixed in the core. He explains it here: bitshares/bitshares-ui#200 (comment).

I don't want to create a duplicate issue, so I ask here first, but I am happy to open a new issue and link to the past discussions and relevant issues.

The charts (long wicks mainly) and estimated account value are screwed up by these "fractional dust" trades executing out of range... it is a serious issue because we need good charts to get people to stay on the DEX.

@abitmore
Copy link
Member

@landry314 it's not "my" PR. Please go ahead and create an issue. Better to discuss there.

@abitmore
Copy link
Member

Please discuss in #449.

@abitmore
Copy link
Member

Superseded by #455, closing this one.

@abitmore abitmore closed this Nov 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants