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

Exclude non executed BSQ trade #2937

Merged
merged 1 commit into from Jul 12, 2019

Conversation

@sqrrm
Copy link
Member

commented Jul 5, 2019

No description provided.

@sqrrm sqrrm requested review from ripcurlx and devinbileck Jul 5, 2019

@devinbileck

This comment has been minimized.

Copy link
Member

commented Jul 5, 2019

I am not familiar with the disputed trade nor its impact and the need for this. I assume it was a typo when creating the offer? Is this going to be common practice to exclude specific trades? Would it not be better to try to prevent these types of mistakes in the first place within the UI?

@sqrrm

This comment has been minimized.

Copy link
Member Author

commented Jul 6, 2019

It's misleading to include non completed trades in the trade statistics. Generally it doesn't matter much but for BSQ this is the only price feed there is. If we're going to use this price feed for future reference to ask for compensation it needs to reflect a the price of BSQ as correctly as possible.

I don't really like adding exclusions like this, it's centralized and fragile. However, we also can't rely on an average price over the last months if it's too easy to paint the tape, and in this case without even using any paint.

I'm open to suggestions, perhaps excluding trades that seem to fall far outside the previous average, much like exchanges cancel trades during flash crashes and similarly. In this case it's only about showing a proper history though so it's not as serious as cancelling trades, just not accounting for them when displaying historical data.

@devinbileck

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

I assume this is the trade in question?
image

I agree that it makes sense to not show incomplete trades, but perhaps we need to implement a proper mechanism for excluding incomplete trades rather than hard coding specific trades or making other assumptions.

And with regards to determining BSQ price for compensation request purposes, I don't think we should be calculating it based on the average price but instead on the median price. That way outlier trades like this would have minimal influence on the calculated price.

@sqrrm

This comment has been minimized.

Copy link
Member Author

commented Jul 8, 2019

That's a good point to use median, I'm basically ok to just show incomplete trades as it is now as long as it's not easy to paint the tape for the purpose of manipulating the BSQ price.

I don't think it should be possible to automate excluding incomplete trades, it's not information that's necessary to share so if we currently have some way to find that out we should work on making that impossible. We have the arbitrators for now, but with a protocol without arbitrators it wouldn't necessarily even be visible at all. Using the arbitrators is also not great in itself as it's a centralized point.

@devinbileck

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

Since the trade statistics candle popup already contains the average, perhaps we should add the median. Since I assume that is how we will be determining the price.
image

@ripcurlx
Copy link
Member

left a comment

utACK

@ripcurlx ripcurlx merged commit e2bb538 into bisq-network:master Jul 12, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.