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

Pricenode tweak: completely remove Coinmate and cex.io as data providers #4422

Merged

Conversation

wiz
Copy link
Contributor

@wiz wiz commented Aug 21, 2020

Based on user feedback, these exchanges are too illiquid and do not accurately reflect the true price of the market. After research, I found the user's feedback to be correct and this PR removes both exchanges.

Based on user feedback, these exchanges are too illiquid and do not
accurately reflect the true price of the market. After research, I found
the user's feedback to be correct and this PR removes both exchanges.
@mrosseel
Copy link
Contributor

Wouldn't it be cleaner to set the exchange weights to zero? In this way when the exchange gains relevance or is useful for another crypto pair it's still available. Don't want to block this PR, just something to keep in mind for the future

@wiz
Copy link
Contributor Author

wiz commented Aug 21, 2020

@mrosseel Yes, @cd2357 will likely re-add these exchanges with a new configuration in another PR later, this PR was urgently done to fix $DASH price from being 1% off which caused one user to notice

Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

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

utACK

@sqrrm sqrrm merged commit 06cfe39 into bisq-network:master Aug 21, 2020
@sqrrm
Copy link
Member

sqrrm commented Aug 21, 2020

I think this can be deployed pretty fast as long as price node operators coordinate.

@cd2357
Copy link
Contributor

cd2357 commented Aug 21, 2020

How about just disabling them, instead of removing?

They can be easily disabled by commenting out the @Component annotation + evtl adding a short comment hinting that they're temporarily disabled until rate weights are avialable.

@cd2357
Copy link
Contributor

cd2357 commented Aug 21, 2020

Too late, I didn't notice its already merged.

@sqrrm
Copy link
Member

sqrrm commented Aug 21, 2020

If you want to, just revert this PR and disable as you suggest. Sounds more appropriate.

@ripcurlx ripcurlx added this to the v1.3.8 milestone Aug 24, 2020
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.

5 participants