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

add dex.decred.org exchange #1787

Merged
merged 5 commits into from
Nov 5, 2020
Merged

add dex.decred.org exchange #1787

merged 5 commits into from
Nov 5, 2020

Conversation

buck54321
Copy link
Member

@buck54321 buck54321 commented Nov 4, 2020

Add an exchanges.Exchange-implementing type for a Decred DEX. On by default.

Aggregated depth chart stacks deepest books first. Exchange colors are now static.

Add an exchange.Exchange implementing type for a Decred DEX. On
by default.

Aggregated depth chart stacks deepest books first. Exchange colors
are now static.
dcrrates/rateserver/go.sum Show resolved Hide resolved
exchanges/bot.go Show resolved Hide resolved
@buck54321 buck54321 marked this pull request as ready for review November 5, 2020 15:00
Copy link
Member

@chappjc chappjc left a comment

Choose a reason for hiding this comment

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

Looks great. I think we can get this in today. Just need to deal with book purge and a few other little things.

exchanges/exchanges.go Outdated Show resolved Hide resolved
exchanges/exchanges.go Show resolved Hide resolved
exchanges/exchanges.go Show resolved Hide resolved
exchanges/exchanges.go Show resolved Hide resolved
exchanges/exchanges.go Outdated Show resolved Hide resolved
exchanges/exchanges.go Outdated Show resolved Hide resolved
exchanges/exchanges.go Show resolved Hide resolved
@@ -3,12 +3,11 @@ module github.com/decred/dcrdata/exchanges/v2
go 1.12

require (
decred.org/dcrdex v0.1.1-beta1
Copy link
Member

Choose a reason for hiding this comment

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

I stupidly made a tag that qualified as semver and we're stuck with it. Will want to make this go for whatever we have on either the master or release-0.1 branches. Seeing as I made that dumb tag I think we have no choice but to tag v0.1.1 at the same place as release-v0.1.1, so I'll probably do that.

exchanges/exchanges.go Outdated Show resolved Hide resolved
@chappjc
Copy link
Member

chappjc commented Nov 5, 2020

Two thoughts about the "Decred Markets" table.

image

  1. Would be nice to retain some defined capitalization so that instead of "Dcrdex" it would be "DCRDEX"
  2. Instead of 0 for DCR Vol., would be better to put "-" or "N/A"

@chappjc
Copy link
Member

chappjc commented Nov 5, 2020

When selecting just the dcrdex market, something is wrong with the depth chart

image

vs.

image

@buck54321
Copy link
Member Author

buck54321 commented Nov 5, 2020

@chappjc It appears you're on an older commit. I've updated the name in the "Decred Markets" table. See what you think now.

I do see one more place I need to do that though.

@chappjc
Copy link
Member

chappjc commented Nov 5, 2020

So I was. I see "dex.decred.org" now, which makes sense since there can be any number of DCRDEXs.

image

Something is still up with the depth chart though. And the zero kinda implies there's no volume, when we just don't have it reported yet.

@buck54321
Copy link
Member Author

This is actually a pre-existing Dygraphs bug. For some reason, the left-most section is often unshaded if just one of its points is out of on the edge of the viewport. I see the same thing if I carefully zoom in on say Bittrex. It's just more apparent on dcrdex because the book only has a few price levels instead of the hundreds on other books. Once the book has more price bins, the chart will look normal.

@chappjc chappjc merged commit cc933bd into decred:master Nov 5, 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.

2 participants