multi: add support for DCR-USDT pair on `/markets` view #2002

ukane-philemon · 2024-08-15T02:17:56Z

Add Mexc exchange DCR-USDT pair.
Add Binance DCR-USDT pair.

Breaking Changes:

1. `ratesproto`: Rate messages are no longer `DCR-BTC` only, a new `currencyPair` field indicates which market rate is sent.
   `ExchangeSubscription` field changed from `btcIndex` to `index`.
2. `exchanges`: Aggregated chart data has been removed. This is because combining usdt and btc market bids and asks is not
   ideal and serves very little purpose. Each market has its own chart.
3. Renamed three fields on `exchanges.ExchangeBotState`:
     - `btc_index` -> `index`
     - `dcr_btc_exchanges` -> `dcr_exchanges` (this field now returns a nested map of supported markets)
     - `btc_indices` -> `indices` (this field now returns a nested map of supported indices)
4. Affected API Endpoints:
     -  `/exchangerate` (`exchanges.ExchangeRates.Exchanges` json field returns a nested map of support markets)
     -  `/exchanges` (returns modified `exchanges.ExchangeBotState`)

Closes #2001

ukane-philemon · 2024-09-23T15:12:45Z

This PR sets the foundation for adding other pairs and exchanges. As per UI, we can always improve on that if there are suggestions. If a redesign is want we want, I can get that sorted.

- Add Mexc exchange `DCR-USDT` pair. - Add Binance `DCR-USDT` pair. - Fix minor bugs Breaking Changes: 1. `ratesproto`: Rate messages are no longer `DCR-BTC` only, a new `currencyPair` field indicates which market rate is sent.  `ExchangeSubscription` field changed from `btcIndex` to `index`. 2. `exchanges`: Aggregated chart data has been removed. This is because combining usdt and btc market bids and asks is not ideal and serves very little purpose. Each market has its own chart. 3. Renamed three fields on `exchanges.ExchangeBotState`: - `btc_index` -> `index` - `dcr_btc_exchanges` -> `dcr_exchanges` (this field now returns a nested map of supported markets) - `btc_indices` -> `indices` (this field now returns a nested map of supported indices) 4. Affected API Endpoints: - `/exchangerate` (`exchanges.ExchangeRates.Exchanges` json field returns a nested map of support markets) - `/exchanges` (returns modified `exchanges.ExchangeBotState`) Signed-off-by: Philemon Ukane <ukanephilemon@gmail.com>

Signed-off-by: Philemon Ukane <ukanephilemon@gmail.com>

buck54321 · 2025-02-10T15:20:39Z

I can't speak for others, but the aggregate chart is kinda the whole point for me. Can't we just convert all depth to USD?

ukane-philemon · 2025-02-22T11:36:48Z

I can't speak for others, but the aggregate chart is kinda the whole point for me. Can't we just convert all depth to USD?

I'm expecting a review from @davecgh.

@davecgh what do you think?

davecgh

Alright, I finished the review and identified a few minor things

In terms of the overall direction, I agree with @buck54321 in that I very much like the aggregated chart and find it much more useful overall.

However, I don't see any issue with having individual markets as an option as well.

davecgh · 2025-01-10T08:45:49Z

cmd/dcrdata/internal/api/apiroutes.go

@@ -1814,12 +1814,13 @@ func (c *appContext) getCandlestickChart(w http.ResponseWriter, r *http.Request)
 	}
 	token := m.RetrieveExchangeTokenCtx(r)
 	bin := m.RetrieveStickWidthCtx(r)
-	if token == "" || bin == "" {
+	currencyPair, error := m.RetrieveCurrencyPair(r)


Seems like error shold be consistently named err like the rest.

davecgh · 2025-01-10T08:50:30Z

cmd/dcrdata/public/js/controllers/homepage_controller.js

      if (this.hasPowConvertedTarget) {
-        this.powConvertedTarget.textContent = `${humanize.twoDecimals(ex.subsidy.pow / 1e8 * xcRate)} ${btcIndex}`
+        this.powConvertedTarget.textContent = `${humanize.twoDecimals(ex.subsidy.pow / 1e8 * xcRate)} ${index}`


Since this PR is intended to serve as a foundation for for adding other pairs and exchanges, should be the conversion rate be parameterized instead of hard coded as 1e8?

davecgh · 2025-01-10T08:53:37Z

exchanges/rateserver/config.go

@@ -34,7 +34,7 @@ type config struct {
 	LogPath           string   `long:"logpath" description:"Directory to log output. ([appdir]/logs/)" env:"DCRRATES_LOG_PATH"`
 	LogLevel          string   `long:"loglevel" description:"Logging level {trace, debug, info, warn, error, critical}" env:"DCRRATES_LOG_LEVEL"`
 	DisabledExchanges string   `long:"disable-exchange" description:"Exchanges to disable. See /exchanges/exchanges.go for available exchanges. Use a comma to separate multiple exchanges" env:"DCRRATES_DISABLE_EXCHANGES"`
-	ExchangeCurrency  string   `long:"exchange-currency" description:"The default bitcoin price index. A 3-letter currency code." env:"DCRRATES_EXCHANGE_INDEX"`
+	ExchangeCurrency  string   `long:"exchange-currency" description:"The default {bitcoin, usdt} price index. A 3-letter currency code." env:"DCRRATES_EXCHANGE_INDEX"`


Not sure these should be listed inline here. It's not clear to me what it intends to set from the text. If it just said "The default price index. A 3-letter currency code.", it would make more sense imo.

davecgh · 2025-01-10T09:02:55Z

cmd/dcrdata/internal/middleware/apimiddleware.go

+
+// RetrieveCurrencyPair tries to fetch the currency pair from the request query.
+func RetrieveCurrencyPair(r *http.Request) (exchanges.CurrencyPair, error) {
+	pair := exchanges.CurrencyPair(r.URL.Query().Get("currencyPair"))


I'm not super familiar with all of this code, but this does not look like it belongs in the middleware since it's specifically getting a URL query parameter as opposed to a query path parameter.

You can see looking at the other handlers in the middleware that they use chi.URLParam. For example, for BlockHashPathAndIndexCtx, it does hash := chi.URLParam(r, "blockhash") so that it pulls the hash from the query path like /block/{hash}.

Instead, you're aiming to qet the URL query parameter ?currencyPair=FOO.

It looks to me like this instead belongs over in internal/middleware/apimiddleware.go in the appContext handlers just like how e.g. func (c *appContext) ChartTypeData and func (c *appContext) getExchanges get URL query parameters inline.

davecgh · 2025-02-07T20:00:24Z

cmd/dcrdata/internal/explorer/explorer.go

+
+		// Other DCR pairs should also provide an index price for the quote
+		// asset.
+		update.Indices[exchanges.BTCIndex.String()] = xcState.BtcPrice


Perhaps a personal preference thing, but I'd rather cases like this just cast it directly to string since it makes the performance implications much more clear.

As a case in point, I had to go look up what the String method is doing to see if it was constructing it each invocation and should be cached as a result.

On the other hand update.Indices[string(exchanges.BTCIndex)] = xcState.BtcPrice is abundantly clear.

I understand the thought process is likely to make it so the type could theoretically be changed and the String method updated so the callers are not dependent on the type, but I'm not a big fan of that because it makes it too easy to mindlessly cause massive performance implications. In contrast, casts of primitive types make it obvious exactly what the implications are each call site should the type ultimately be changed.

davecgh · 2025-02-07T20:06:57Z

cmd/dcrdata/internal/explorer/explorer.go

@@ -809,14 +816,22 @@ func (exp *explorerUI) watchExchanges() {
 	for {
 		select {
 		case update := <-xcChans.Exchange:
-			sendXcUpdate(false, update.Token, update.State)
+			sendXcUpdate(false, update.Token, update.CurrencyPair.String(), update.State)


Same type of thing here regarding the CurrencyPair. Could just directly cast it.

I won't comment on all the rest of the cases, but the same holds for them too.

davecgh · 2025-02-07T20:21:32Z

cmd/dcrdata/internal/explorer/explorer.go

+			}
+		}
+	}
+	if price == 0 {


It doesn't really matter since price will be zero when there are no sources due to above logic, but this really should be checking if nSources == 0, because it's the divisor that must be non-zero, not the dividend.

ukane-philemon mentioned this pull request Sep 18, 2024

in dcrdata "market" under DCR volume, it should show all DCR volume not just DCR/BTC #2006

Open

ukane-philemon force-pushed the dcr-usdt branch from 96d80c1 to ffce1c6 Compare September 23, 2024 15:22

ukane-philemon requested review from davecgh and chappjc September 23, 2024 15:23

update site dist

fb33fb8

Signed-off-by: Philemon Ukane <ukanephilemon@gmail.com>

ukane-philemon force-pushed the dcr-usdt branch from ffce1c6 to fb33fb8 Compare December 15, 2024 01:14

davecgh reviewed Mar 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

multi: add support for DCR-USDT pair on `/markets` view #2002

multi: add support for DCR-USDT pair on `/markets` view #2002

ukane-philemon commented Aug 15, 2024 •

edited

Loading

ukane-philemon commented Sep 23, 2024 •

edited

Loading

buck54321 commented Feb 10, 2025

ukane-philemon commented Feb 22, 2025

davecgh left a comment

davecgh Jan 10, 2025

davecgh Jan 10, 2025

davecgh Jan 10, 2025

davecgh Jan 10, 2025

davecgh Feb 7, 2025

davecgh Feb 7, 2025

davecgh Feb 7, 2025

multi: add support for DCR-USDT pair on /markets view #2002

Are you sure you want to change the base?

multi: add support for DCR-USDT pair on /markets view #2002

Conversation

ukane-philemon commented Aug 15, 2024 • edited Loading

ukane-philemon commented Sep 23, 2024 • edited Loading

buck54321 commented Feb 10, 2025

ukane-philemon commented Feb 22, 2025

davecgh left a comment

Choose a reason for hiding this comment

davecgh Jan 10, 2025

Choose a reason for hiding this comment

davecgh Jan 10, 2025

Choose a reason for hiding this comment

davecgh Jan 10, 2025

Choose a reason for hiding this comment

davecgh Jan 10, 2025

Choose a reason for hiding this comment

davecgh Feb 7, 2025

Choose a reason for hiding this comment

davecgh Feb 7, 2025

Choose a reason for hiding this comment

davecgh Feb 7, 2025

Choose a reason for hiding this comment

multi: add support for DCR-USDT pair on `/markets` view #2002

multi: add support for DCR-USDT pair on `/markets` view #2002

ukane-philemon commented Aug 15, 2024 •

edited

Loading

ukane-philemon commented Sep 23, 2024 •

edited

Loading