From 44025982af1da491ea652cfbbb0c654ed1014200 Mon Sep 17 00:00:00 2001 From: Gareth Kirwan Date: Sun, 22 Dec 2024 09:30:02 +0700 Subject: [PATCH 1/3] Kucoin: Fix Margin-only subs As a GCT user with spot and margin assets enabled, but only margin asset enabled websocket subscriptions, I should still get subscriptions for all the pairs in margin which are also in spot Currently it only works when spot subscriptions are enabled. Otherwise the spot pairs are ignored. Fixes #1755 --- README.md | 10 +- .../exchanges_templates/kucoin.tmpl | 3 + engine/currency_state_manager.md | 74 +++++------ exchanges/kucoin/README.md | 3 + exchanges/kucoin/kucoin_test.go | 122 ++++++++++-------- exchanges/kucoin/kucoin_websocket.go | 71 ++++++---- 6 files changed, 166 insertions(+), 117 deletions(-) diff --git a/README.md b/README.md index 16819ec0f39..b76c4bc3ade 100644 --- a/README.md +++ b/README.md @@ -148,11 +148,11 @@ Binaries will be published once the codebase reaches a stable condition. |User|Contribution Amount| |--|--| -| [thrasher-](https://github.com/thrasher-) | 700 | -| [shazbert](https://github.com/shazbert) | 345 | -| [dependabot[bot]](https://github.com/apps/dependabot) | 317 | -| [gloriousCode](https://github.com/gloriousCode) | 234 | -| [gbjk](https://github.com/gbjk) | 93 | +| [thrasher-](https://github.com/thrasher-) | 703 | +| [shazbert](https://github.com/shazbert) | 355 | +| [dependabot[bot]](https://github.com/apps/dependabot) | 331 | +| [gloriousCode](https://github.com/gloriousCode) | 236 | +| [gbjk](https://github.com/gbjk) | 107 | | [dependabot-preview[bot]](https://github.com/apps/dependabot-preview) | 88 | | [xtda](https://github.com/xtda) | 47 | | [lrascao](https://github.com/lrascao) | 27 | diff --git a/cmd/documentation/exchanges_templates/kucoin.tmpl b/cmd/documentation/exchanges_templates/kucoin.tmpl index 1eb9ca856c9..1ae61cc437c 100644 --- a/cmd/documentation/exchanges_templates/kucoin.tmpl +++ b/cmd/documentation/exchanges_templates/kucoin.tmpl @@ -23,6 +23,9 @@ Default Authenticated Subscriptions: Subscriptions are subject to enabled assets and pairs. +Margin subscriptions for ticker, orderbook and All trades are merged into Spot subscriptions because duplicates are not allowed, +unless Spot subscription does not exist, i.e. Spot asset not enabled, or subscription configured only for Margin + Limitations: - 100 symbols per subscription - 300 symbols per connection diff --git a/engine/currency_state_manager.md b/engine/currency_state_manager.md index 5e938e72fab..cb6a44d9a57 100644 --- a/engine/currency_state_manager.md +++ b/engine/currency_state_manager.md @@ -1,22 +1,22 @@ -# GoCryptoTrader package Currency state manager - - - - -[![Build Status](https://github.com/thrasher-corp/gocryptotrader/actions/workflows/tests.yml/badge.svg?branch=master)](https://github.com/thrasher-corp/gocryptotrader/actions/workflows/tests.yml) -[![Software License](https://img.shields.io/badge/License-MIT-orange.svg?style=flat-square)](https://github.com/thrasher-corp/gocryptotrader/blob/master/LICENSE) -[![GoDoc](https://godoc.org/github.com/thrasher-corp/gocryptotrader?status.svg)](https://godoc.org/github.com/thrasher-corp/gocryptotrader/engine/currency_state_manager) -[![Coverage Status](https://codecov.io/gh/thrasher-corp/gocryptotrader/graph/badge.svg?token=41784B23TS)](https://codecov.io/gh/thrasher-corp/gocryptotrader) -[![Go Report Card](https://goreportcard.com/badge/github.com/thrasher-corp/gocryptotrader)](https://goreportcard.com/report/github.com/thrasher-corp/gocryptotrader) - - -This currency_state_manager package is part of the GoCryptoTrader codebase. - -## This is still in active development - -You can track ideas, planned features and what's in progress on our [GoCryptoTrader Kanban board](https://github.com/orgs/thrasher-corp/projects/3). - -Join our slack to discuss all things related to GoCryptoTrader! [GoCryptoTrader Slack](https://join.slack.com/t/gocryptotrader/shared_invite/enQtNTQ5NDAxMjA2Mjc5LTc5ZDE1ZTNiOGM3ZGMyMmY1NTAxYWZhODE0MWM5N2JlZDk1NDU0YTViYzk4NTk3OTRiMDQzNGQ1YTc4YmRlMTk) +# GoCryptoTrader package Currency state manager + + + + +[![Build Status](https://github.com/thrasher-corp/gocryptotrader/actions/workflows/tests.yml/badge.svg?branch=master)](https://github.com/thrasher-corp/gocryptotrader/actions/workflows/tests.yml) +[![Software License](https://img.shields.io/badge/License-MIT-orange.svg?style=flat-square)](https://github.com/thrasher-corp/gocryptotrader/blob/master/LICENSE) +[![GoDoc](https://godoc.org/github.com/thrasher-corp/gocryptotrader?status.svg)](https://godoc.org/github.com/thrasher-corp/gocryptotrader/engine/currency_state_manager) +[![Coverage Status](https://codecov.io/gh/thrasher-corp/gocryptotrader/graph/badge.svg?token=41784B23TS)](https://codecov.io/gh/thrasher-corp/gocryptotrader) +[![Go Report Card](https://goreportcard.com/badge/github.com/thrasher-corp/gocryptotrader)](https://goreportcard.com/report/github.com/thrasher-corp/gocryptotrader) + + +This currency_state_manager package is part of the GoCryptoTrader codebase. + +## This is still in active development + +You can track ideas, planned features and what's in progress on our [GoCryptoTrader Kanban board](https://github.com/orgs/thrasher-corp/projects/3). + +Join our slack to discuss all things related to GoCryptoTrader! [GoCryptoTrader Slack](https://join.slack.com/t/gocryptotrader/shared_invite/enQtNTQ5NDAxMjA2Mjc5LTc5ZDE1ZTNiOGM3ZGMyMmY1NTAxYWZhODE0MWM5N2JlZDk1NDU0YTViYzk4NTk3OTRiMDQzNGQ1YTc4YmRlMTk) ## Current Features for Currency state manager + The state manager keeps currency states up to date, which include: @@ -27,22 +27,22 @@ Join our slack to discuss all things related to GoCryptoTrader! [GoCryptoTrader + This allows for an internal state check to compliment internal and external strategies. - -## Contribution - -Please feel free to submit any pull requests or suggest any desired features to be added. - -When submitting a PR, please abide by our coding guidelines: - -+ Code must adhere to the official Go [formatting](https://golang.org/doc/effective_go.html#formatting) guidelines (i.e. uses [gofmt](https://golang.org/cmd/gofmt/)). -+ Code must be documented adhering to the official Go [commentary](https://golang.org/doc/effective_go.html#commentary) guidelines. -+ Code must adhere to our [coding style](https://github.com/thrasher-corp/gocryptotrader/blob/master/doc/coding_style.md). -+ Pull requests need to be based on and opened against the `master` branch. - -## Donations - - - -If this framework helped you in any way, or you would like to support the developers working on it, please donate Bitcoin to: - + +## Contribution + +Please feel free to submit any pull requests or suggest any desired features to be added. + +When submitting a PR, please abide by our coding guidelines: + ++ Code must adhere to the official Go [formatting](https://golang.org/doc/effective_go.html#formatting) guidelines (i.e. uses [gofmt](https://golang.org/cmd/gofmt/)). ++ Code must be documented adhering to the official Go [commentary](https://golang.org/doc/effective_go.html#commentary) guidelines. ++ Code must adhere to our [coding style](https://github.com/thrasher-corp/gocryptotrader/blob/master/doc/coding_style.md). ++ Pull requests need to be based on and opened against the `master` branch. + +## Donations + + + +If this framework helped you in any way, or you would like to support the developers working on it, please donate Bitcoin to: + ***bc1qk0jareu4jytc0cfrhr5wgshsq8282awpavfahc*** diff --git a/exchanges/kucoin/README.md b/exchanges/kucoin/README.md index 3439d186f84..b027535efcd 100644 --- a/exchanges/kucoin/README.md +++ b/exchanges/kucoin/README.md @@ -41,6 +41,9 @@ Default Authenticated Subscriptions: Subscriptions are subject to enabled assets and pairs. +Margin subscriptions for ticker, orderbook and All trades are merged into Spot subscriptions because duplicates are not allowed, +unless Spot subscription does not exist, i.e. Spot asset not enabled, or subscription configured only for Margin + Limitations: - 100 symbols per subscription - 300 symbols per connection diff --git a/exchanges/kucoin/kucoin_test.go b/exchanges/kucoin/kucoin_test.go index 86c3ac38b33..40566e24671 100644 --- a/exchanges/kucoin/kucoin_test.go +++ b/exchanges/kucoin/kucoin_test.go @@ -2266,52 +2266,53 @@ func TestGenerateSubscriptions(t *testing.T) { // Only in Spot: BTC-USDT, ETH-USDT // In Both: ETH-BTC, LTC-USDT // Only in Margin: TRX-BTC, SOL-USDC - subPairs := currency.Pairs{} - for _, pp := range [][]string{ - {"BTC", "USDT", "-"}, {"ETH", "BTC", "-"}, {"ETH", "USDT", "-"}, {"LTC", "USDT", "-"}, // Spot - {"ETH", "BTC", "-"}, {"LTC", "USDT", "-"}, {"SOL", "USDC", "-"}, {"TRX", "BTC", "-"}, // Margin - {"ETH", "USDCM", ""}, {"SOL", "USDTM", ""}, {"XBT", "USDCM", ""}, // Futures + pairs := map[string]currency.Pairs{} + for a, ss := range map[string][]string{ + "spot": {"BTC-USDT", "ETH-BTC", "ETH-USDT", "LTC-USDT"}, + "margin": {"ETH-BTC", "LTC-USDT", "SOL-USDC", "TRX-BTC"}, + "futures": {"ETHUSDCM", "SOLUSDTM", "XBTUSDCM"}, } { - subPairs = append(subPairs, currency.NewPairWithDelimiter(pp[0], pp[1], pp[2])) + for _, s := range ss { + p, err := currency.NewPairFromString(s) + require.NoError(t, err, "NewPairFromString must not error") + pairs[a] = pairs[a].Add(p) + } } + pairs["both"] = common.SortStrings(pairs["spot"].Add(pairs["margin"]...)) exp := subscription.List{ - {Channel: subscription.TickerChannel, Asset: asset.Spot, Pairs: subPairs[0:4], QualifiedChannel: "/market/ticker:" + subPairs[0:4].Join()}, - {Channel: subscription.TickerChannel, Asset: asset.Margin, Pairs: subPairs[6:8], QualifiedChannel: "/market/ticker:" + subPairs[6:8].Join()}, - {Channel: subscription.TickerChannel, Asset: asset.Futures, Pairs: subPairs[8:], QualifiedChannel: "/contractMarket/tickerV2:" + subPairs[8:].Join()}, - {Channel: subscription.OrderbookChannel, Asset: asset.Spot, Pairs: subPairs[0:4], QualifiedChannel: "/spotMarket/level2Depth5:" + subPairs[0:4].Join(), - Interval: kline.HundredMilliseconds}, - {Channel: subscription.OrderbookChannel, Asset: asset.Margin, Pairs: subPairs[6:8], QualifiedChannel: "/spotMarket/level2Depth5:" + subPairs[6:8].Join(), + {Channel: subscription.TickerChannel, Asset: asset.Spot, Pairs: pairs["both"], QualifiedChannel: "/market/ticker:" + pairs["both"].Join()}, + {Channel: subscription.TickerChannel, Asset: asset.Futures, Pairs: pairs["futures"], QualifiedChannel: "/contractMarket/tickerV2:" + pairs["futures"].Join()}, + {Channel: subscription.OrderbookChannel, Asset: asset.Spot, Pairs: pairs["both"], QualifiedChannel: "/spotMarket/level2Depth5:" + pairs["both"].Join(), Interval: kline.HundredMilliseconds}, - {Channel: subscription.OrderbookChannel, Asset: asset.Futures, Pairs: subPairs[8:], QualifiedChannel: "/contractMarket/level2Depth5:" + subPairs[8:].Join(), + {Channel: subscription.OrderbookChannel, Asset: asset.Futures, Pairs: pairs["futures"], QualifiedChannel: "/contractMarket/level2Depth5:" + pairs["futures"].Join(), Interval: kline.HundredMilliseconds}, - {Channel: subscription.AllTradesChannel, Asset: asset.Spot, Pairs: subPairs[0:4], QualifiedChannel: "/market/match:" + subPairs[0:4].Join()}, - {Channel: subscription.AllTradesChannel, Asset: asset.Margin, Pairs: subPairs[6:8], QualifiedChannel: "/market/match:" + subPairs[6:8].Join()}, + {Channel: subscription.AllTradesChannel, Asset: asset.Spot, Pairs: pairs["both"], QualifiedChannel: "/market/match:" + pairs["both"].Join()}, } subs, err := ku.generateSubscriptions() - assert.NoError(t, err, "generateSubscriptions must not error") + require.NoError(t, err, "generateSubscriptions must not error") testsubs.EqualLists(t, exp, subs) ku.Websocket.SetCanUseAuthenticatedEndpoints(true) var loanPairs currency.Pairs - loanCurrs := common.SortStrings(subPairs[0:8].GetCurrencies()) + loanCurrs := common.SortStrings(pairs["both"].GetCurrencies()) for _, c := range loanCurrs { loanPairs = append(loanPairs, currency.Pair{Base: c}) } exp = append(exp, subscription.List{ - {Asset: asset.Futures, Channel: futuresTradeOrderChannel, QualifiedChannel: "/contractMarket/tradeOrders", Pairs: subPairs[8:]}, - {Asset: asset.Futures, Channel: futuresStopOrdersLifecycleEventChannel, QualifiedChannel: "/contractMarket/advancedOrders", Pairs: subPairs[8:]}, - {Asset: asset.Futures, Channel: futuresAccountBalanceEventChannel, QualifiedChannel: "/contractAccount/wallet", Pairs: subPairs[8:]}, - {Asset: asset.Margin, Channel: marginPositionChannel, QualifiedChannel: "/margin/position", Pairs: subPairs[4:8]}, + {Asset: asset.Futures, Channel: futuresTradeOrderChannel, QualifiedChannel: "/contractMarket/tradeOrders", Pairs: pairs["futures"]}, + {Asset: asset.Futures, Channel: futuresStopOrdersLifecycleEventChannel, QualifiedChannel: "/contractMarket/advancedOrders", Pairs: pairs["futures"]}, + {Asset: asset.Futures, Channel: futuresAccountBalanceEventChannel, QualifiedChannel: "/contractAccount/wallet", Pairs: pairs["futures"]}, + {Asset: asset.Margin, Channel: marginPositionChannel, QualifiedChannel: "/margin/position", Pairs: pairs["margin"]}, {Asset: asset.Margin, Channel: marginLoanChannel, QualifiedChannel: "/margin/loan:" + loanCurrs.Join(), Pairs: loanPairs}, {Channel: accountBalanceChannel, QualifiedChannel: "/account/balance"}, }...) subs, err = ku.generateSubscriptions() - assert.NoError(t, err, "generateSubscriptions with Auth must not error") + require.NoError(t, err, "generateSubscriptions with Auth must not error") testsubs.EqualLists(t, exp, subs) } @@ -2320,21 +2321,16 @@ func TestGenerateTickerAllSub(t *testing.T) { ku := testInstance(t) //nolint:govet // Intentional shadow to avoid future copy/paste mistakes avail, err := ku.GetAvailablePairs(asset.Spot) - assert.NoError(t, err, "GetAvailablePairs must not error") - for i := 0; i <= 10; i++ { - err = ku.CurrencyPairs.EnablePair(asset.Spot, avail[i]) - assert.NoError(t, common.ExcludeError(err, currency.ErrPairAlreadyEnabled), "EnablePair must not error") - } - - enabled, err := ku.GetEnabledPairs(asset.Spot) - assert.NoError(t, err, "GetEnabledPairs must not error") + require.NoError(t, err, "GetAvailablePairs must not error") + err = ku.CurrencyPairs.StorePairs(asset.Spot, avail[:11], true) + require.NoError(t, err, "StorePairs must not error") ku.Features.Subscriptions = subscription.List{{Channel: subscription.TickerChannel, Asset: asset.Spot}} exp := subscription.List{ - {Channel: subscription.TickerChannel, Asset: asset.Spot, QualifiedChannel: "/market/ticker:all", Pairs: enabled}, + {Channel: subscription.TickerChannel, Asset: asset.Spot, QualifiedChannel: "/market/ticker:all", Pairs: avail[:11]}, } subs, err := ku.generateSubscriptions() - assert.NoError(t, err, "generateSubscriptions with Auth must not error") + require.NoError(t, err, "generateSubscriptions with Auth must not error") testsubs.EqualLists(t, exp, subs) } @@ -2353,7 +2349,7 @@ func TestGenerateOtherSubscriptions(t *testing.T) { ku.Features.Subscriptions = subscription.List{s} got, err := ku.generateSubscriptions() assert.NoError(t, err, "generateSubscriptions should not error") - assert.Len(t, got, 1, "Should generate just one sub") + require.Len(t, got, 1, "Must generate just one sub") assert.NotEmpty(t, got[0].QualifiedChannel, "Qualified Channel should not be empty") if got[0].Channel == subscription.CandlesChannel { assert.Equal(t, "/market/candles:BTC-USDT_4hour,ETH-BTC_4hour,ETH-USDT_4hour,LTC-USDT_4hour", got[0].QualifiedChannel, "QualifiedChannel should be correct") @@ -2361,6 +2357,28 @@ func TestGenerateOtherSubscriptions(t *testing.T) { } } +// TestGenerateMarginSubscriptions is a regression test for #1755 and ensures margin subscriptions work without spot subs +func TestGenerateMarginSubscriptions(t *testing.T) { + t.Parallel() + + ku := testInstance(t) //nolint:govet // Intentional shadow to avoid future copy/paste mistakes + + avail, err := ku.GetAvailablePairs(asset.Spot) + require.NoError(t, err, "GetAvailablePairs must not error storing spot pairs") + avail = common.SortStrings(avail) + err = ku.CurrencyPairs.StorePairs(asset.Margin, avail[:6], true) + require.NoError(t, err, "StorePairs must not error storing margin pairs") + err = ku.CurrencyPairs.StorePairs(asset.Spot, avail[:3], true) + require.NoError(t, err, "StorePairs must not error storing spot pairs") + + ku.Features.Subscriptions = subscription.List{{Channel: subscription.TickerChannel, Asset: asset.Margin}} + subs, err := ku.Features.Subscriptions.ExpandTemplates(ku) + require.NoError(t, err, "ExpandTemplates must not error") + require.Len(t, subs, 1, "Must generate just one sub") + assert.Equal(t, asset.Margin, subs[0].Asset, "Asset should be correct") + assert.Equal(t, "/market/ticker:"+avail[:6].Join(), subs[0].QualifiedChannel, "QualifiedChannel should be correct") +} + // TestCheckSubscriptions ensures checkSubscriptions upgrades user config correctly func TestCheckSubscriptions(t *testing.T) { t.Parallel() @@ -2934,8 +2952,8 @@ func TestSubscribeBatches(t *testing.T) { } subs, err := ku.generateSubscriptions() - assert.NoError(t, err, "generateSubscriptions must not error") - assert.Len(t, subs, len(ku.Features.Subscriptions), "Must generate batched subscriptions") + require.NoError(t, err, "generateSubscriptions must not error") + require.Len(t, subs, len(ku.Features.Subscriptions), "Must generate batched subscriptions") err = ku.Subscribe(subs) assert.NoError(t, err, "Subscribe to small batches should not error") @@ -2953,32 +2971,32 @@ func TestSubscribeBatchLimit(t *testing.T) { testexch.SetupWs(t, ku) avail, err := ku.GetAvailablePairs(asset.Spot) - assert.NoError(t, err, "GetAvailablePairs must not error") + require.NoError(t, err, "GetAvailablePairs must not error") err = ku.CurrencyPairs.StorePairs(asset.Spot, avail[:299], true) - assert.NoError(t, err, "StorePairs must not error") + require.NoError(t, err, "StorePairs must not error") ku.Features.Subscriptions = subscription.List{{Asset: asset.Spot, Channel: subscription.AllTradesChannel}} subs, err := ku.generateSubscriptions() - assert.NoError(t, err, "generateSubscriptions must not error") - assert.Len(t, subs, 3, "Must get 3 subs") + require.NoError(t, err, "generateSubscriptions must not error") + require.Len(t, subs, 3, "Must get 3 subs") err = ku.Subscribe(subs) - assert.NoError(t, err, "Subscribe must not error") + require.NoError(t, err, "Subscribe must not error") err = ku.Unsubscribe(subs) - assert.NoError(t, err, "Unsubscribe must not error") + require.NoError(t, err, "Unsubscribe must not error") err = ku.CurrencyPairs.StorePairs(asset.Spot, avail[:320], true) - assert.NoError(t, err, "StorePairs must not error") + require.NoError(t, err, "StorePairs must not error") ku.Features.Subscriptions = subscription.List{{Asset: asset.Spot, Channel: subscription.AllTradesChannel}} subs, err = ku.generateSubscriptions() - assert.NoError(t, err, "generateSubscriptions must not error") - assert.Len(t, subs, 4, "Must get 4 subs") + require.NoError(t, err, "generateSubscriptions must not error") + require.Len(t, subs, 4, "Must get 4 subs") err = ku.Subscribe(subs) - assert.ErrorContains(t, err, "exceed max subscription count limitation of 300 per session", "Subscribe to MarketSnapshot must error above connection symbol limit") + assert.ErrorContains(t, err, "exceed max subscription count limitation of 300 per session", "Subscribe to MarketSnapshot should error above connection symbol limit") } func TestSubscribeTickerAll(t *testing.T) { @@ -2989,20 +3007,20 @@ func TestSubscribeTickerAll(t *testing.T) { testexch.SetupWs(t, ku) avail, err := ku.GetAvailablePairs(asset.Spot) - assert.NoError(t, err, "GetAvailablePairs must not error") + require.NoError(t, err, "GetAvailablePairs must not error") err = ku.CurrencyPairs.StorePairs(asset.Spot, avail[:500], true) - assert.NoError(t, err, "StorePairs must not error") + require.NoError(t, err, "StorePairs must not error") ku.Features.Subscriptions = subscription.List{{Asset: asset.Spot, Channel: subscription.TickerChannel}} subs, err := ku.generateSubscriptions() - assert.NoError(t, err, "generateSubscriptions must not error") - assert.Len(t, subs, 1, "Must generate one subscription") - assert.Equal(t, "/market/ticker:all", subs[0].QualifiedChannel, "QualifiedChannel must be correct") + require.NoError(t, err, "generateSubscriptions must not error") + require.Len(t, subs, 1, "Must generate one subscription") + assert.Equal(t, "/market/ticker:all", subs[0].QualifiedChannel, "QualifiedChannel should be correct") err = ku.Subscribe(subs) - assert.NoError(t, err, "Subscribe to must not error") + assert.NoError(t, err, "Subscribe to should not error") } func TestSeedLocalCache(t *testing.T) { @@ -3957,7 +3975,7 @@ func TestGetCurrencyTradeURL(t *testing.T) { func testInstance(tb testing.TB) *Kucoin { tb.Helper() kucoin := new(Kucoin) - assert.NoError(tb, testexch.Setup(kucoin), "Test instance Setup must not error") + require.NoError(tb, testexch.Setup(kucoin), "Test instance Setup must not error") kucoin.obm = &orderbookManager{ state: make(map[currency.Code]map[currency.Code]map[asset.Item]*update), jobs: make(chan job, maxWSOrderbookJobs), diff --git a/exchanges/kucoin/kucoin_websocket.go b/exchanges/kucoin/kucoin_websocket.go index e364b2b84fc..24ce6640cee 100644 --- a/exchanges/kucoin/kucoin_websocket.go +++ b/exchanges/kucoin/kucoin_websocket.go @@ -1070,11 +1070,8 @@ func (ku *Kucoin) generateSubscriptions() (subscription.List, error) { func (ku *Kucoin) GetSubscriptionTemplate(_ *subscription.Subscription) (*template.Template, error) { return template.New("master.tmpl"). Funcs(template.FuncMap{ - "channelName": channelName, - "removeSpotFromMargin": func(s *subscription.Subscription, ap map[asset.Item]currency.Pairs) string { - spotPairs, _ := ku.GetEnabledPairs(asset.Spot) - return removeSpotFromMargin(s, ap, spotPairs) - }, + "channelName": channelName, + "mergeMarginPairs": ku.mergeMarginPairs, "isCurrencyChannel": isCurrencyChannel, "isSymbolChannel": isSymbolChannel, "channelInterval": channelInterval, @@ -1686,13 +1683,41 @@ func channelName(s *subscription.Subscription, a asset.Item) string { return s.Channel } -// removeSpotFromMargin removes spot pairs from margin pairs in the supplied AssetPairs map for subscriptions to non-margin endpoints -func removeSpotFromMargin(s *subscription.Subscription, ap map[asset.Item]currency.Pairs, spotPairs currency.Pairs) string { +// mergeMarginPairs merges margin pairs into spot pairs for shared subs (ticker, orderbook, etc) if Spot asset and sub are enabled, +// because Kucoin errors on duplicate pairs in separate subs, and doesn't have separate subs for spot and margin +func (ku *Kucoin) mergeMarginPairs(s *subscription.Subscription, ap map[asset.Item]currency.Pairs) string { if strings.HasPrefix(s.Channel, "/margin") { return "" } - if p, ok := ap[asset.Margin]; ok { - ap[asset.Margin] = p.Remove(spotPairs...) + wantKey := &subscription.IgnoringAssetKey{Subscription: s} + switch s.Asset { + case asset.All: + marginPairs, _ := ku.GetEnabledPairs(asset.Margin) + ap[asset.Spot] = common.SortStrings(ap[asset.Spot].Add(marginPairs...)) + ap[asset.Margin] = currency.Pairs{} + case asset.Spot: + // If there's a margin sub then we should merge the pairs into spot + hasMarginSub := slices.ContainsFunc(ku.Features.Subscriptions, func(sB *subscription.Subscription) bool { + if sB.Asset != asset.Margin && sB.Asset != asset.All { + return false + } + return wantKey.Match(&subscription.IgnoringAssetKey{Subscription: sB}) + }) + if hasMarginSub { + marginPairs, _ := ku.GetEnabledPairs(asset.Margin) + ap[asset.Spot] = common.SortStrings(ap[asset.Spot].Add(marginPairs...)) + } + case asset.Margin: + // If there's a spot sub, all margin pairs are already merged, so empty the margin pairs + hasSpotSub := slices.ContainsFunc(ku.Features.Subscriptions, func(sB *subscription.Subscription) bool { + if sB.Asset != asset.Spot && sB.Asset != asset.All { + return false + } + return wantKey.Match(&subscription.IgnoringAssetKey{Subscription: sB}) + }) + if hasSpotSub { + ap[asset.Margin] = currency.Pairs{} + } } return "" } @@ -1749,27 +1774,27 @@ func joinPairsWithInterval(b currency.Pairs, s *subscription.Subscription) strin } const subTplText = ` -{{- removeSpotFromMargin $.S $.AssetPairs -}} +{{- mergeMarginPairs $.S $.AssetPairs }} {{- if isCurrencyChannel $.S }} - {{ channelName $.S $.S.Asset -}} : {{- (assetCurrencies $.S $.AssetPairs).Join -}} + {{- channelName $.S $.S.Asset -}} : {{- (assetCurrencies $.S $.AssetPairs).Join }} {{- else if isSymbolChannel $.S }} - {{ range $asset, $pairs := $.AssetPairs }} + {{- range $asset, $pairs := $.AssetPairs }} {{- with $name := channelName $.S $asset }} - {{- if and (eq $name "/market/ticker") (gt (len $pairs) 10) -}} + {{- if and (eq $name "/market/ticker") (gt (len $pairs) 10) }} {{- $name -}} :all - {{- with $i := channelInterval $.S -}}_{{- $i -}}{{- end -}} - {{- $.BatchSize -}} {{ len $pairs }} - {{- else -}} - {{- range $b := batch $pairs 100 -}} - {{- $name -}} : {{- joinPairsWithInterval $b $.S -}} - {{ $.PairSeparator }} - {{- end -}} + {{- with $i := channelInterval $.S }}_{{ $i }}{{ end }} + {{- $.BatchSize }} {{- len $pairs }} + {{- else }} + {{- range $b := batch $pairs 100 }} + {{- $name -}} : {{- joinPairsWithInterval $b $.S }} + {{- $.PairSeparator }} + {{- end }} {{- $.BatchSize -}} 100 {{- end }} {{- end }} - {{ $.AssetSeparator }} + {{- $.AssetSeparator }} {{- end }} -{{- else -}} - {{ channelName $.S $.S.Asset }} +{{- else }} + {{- channelName $.S $.S.Asset }} {{- end }} ` From a7e1f47fa44222f57768ab76c14fe909c6bf4f8c Mon Sep 17 00:00:00 2001 From: Gareth Kirwan Date: Tue, 31 Dec 2024 12:31:07 +0700 Subject: [PATCH 2/3] fixup! Kucoin: Fix Margin-only subs --- exchanges/kucoin/kucoin_test.go | 6 ++++++ exchanges/kucoin/kucoin_websocket.go | 8 +++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/exchanges/kucoin/kucoin_test.go b/exchanges/kucoin/kucoin_test.go index 40566e24671..29902888fe4 100644 --- a/exchanges/kucoin/kucoin_test.go +++ b/exchanges/kucoin/kucoin_test.go @@ -2377,6 +2377,12 @@ func TestGenerateMarginSubscriptions(t *testing.T) { require.Len(t, subs, 1, "Must generate just one sub") assert.Equal(t, asset.Margin, subs[0].Asset, "Asset should be correct") assert.Equal(t, "/market/ticker:"+avail[:6].Join(), subs[0].QualifiedChannel, "QualifiedChannel should be correct") + + err = ku.CurrencyPairs.SetAssetEnabled(asset.Margin, false) + require.NoError(t, err, "SetAssetEnabled must not error") + ku.Features.Subscriptions = subscription.List{{Channel: subscription.TickerChannel, Asset: asset.All}} + _, err = ku.Features.Subscriptions.ExpandTemplates(ku) + require.NoError(t, err, "mergeMarginPairs must not cause errAssetRecords by adding an empty asset when margin is disabled") } // TestCheckSubscriptions ensures checkSubscriptions upgrades user config correctly diff --git a/exchanges/kucoin/kucoin_websocket.go b/exchanges/kucoin/kucoin_websocket.go index 24ce6640cee..35922f268e7 100644 --- a/exchanges/kucoin/kucoin_websocket.go +++ b/exchanges/kucoin/kucoin_websocket.go @@ -1692,9 +1692,11 @@ func (ku *Kucoin) mergeMarginPairs(s *subscription.Subscription, ap map[asset.It wantKey := &subscription.IgnoringAssetKey{Subscription: s} switch s.Asset { case asset.All: - marginPairs, _ := ku.GetEnabledPairs(asset.Margin) - ap[asset.Spot] = common.SortStrings(ap[asset.Spot].Add(marginPairs...)) - ap[asset.Margin] = currency.Pairs{} + if _, marginEnabled := ap[asset.Margin]; marginEnabled { + marginPairs, _ := ku.GetEnabledPairs(asset.Margin) + ap[asset.Spot] = common.SortStrings(ap[asset.Spot].Add(marginPairs...)) + ap[asset.Margin] = currency.Pairs{} + } case asset.Spot: // If there's a margin sub then we should merge the pairs into spot hasMarginSub := slices.ContainsFunc(ku.Features.Subscriptions, func(sB *subscription.Subscription) bool { From d4ac07b70eeefd98f7a1f1443ef7b88f761b11ec Mon Sep 17 00:00:00 2001 From: Gareth Kirwan Date: Tue, 31 Dec 2024 13:06:36 +0700 Subject: [PATCH 3/3] fixup! Kucoin: Fix Margin-only subs --- exchanges/kucoin/kucoin_test.go | 15 ++++++++++++--- exchanges/kucoin/kucoin_websocket.go | 4 +++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/exchanges/kucoin/kucoin_test.go b/exchanges/kucoin/kucoin_test.go index 29902888fe4..aab3357280d 100644 --- a/exchanges/kucoin/kucoin_test.go +++ b/exchanges/kucoin/kucoin_test.go @@ -2378,11 +2378,20 @@ func TestGenerateMarginSubscriptions(t *testing.T) { assert.Equal(t, asset.Margin, subs[0].Asset, "Asset should be correct") assert.Equal(t, "/market/ticker:"+avail[:6].Join(), subs[0].QualifiedChannel, "QualifiedChannel should be correct") - err = ku.CurrencyPairs.SetAssetEnabled(asset.Margin, false) + require.NoError(t, ku.CurrencyPairs.SetAssetEnabled(asset.Margin, false), "SetAssetEnabled Spot must not error") require.NoError(t, err, "SetAssetEnabled must not error") ku.Features.Subscriptions = subscription.List{{Channel: subscription.TickerChannel, Asset: asset.All}} - _, err = ku.Features.Subscriptions.ExpandTemplates(ku) - require.NoError(t, err, "mergeMarginPairs must not cause errAssetRecords by adding an empty asset when margin is disabled") + subs, err = ku.Features.Subscriptions.ExpandTemplates(ku) + require.NoError(t, err, "mergeMarginPairs must not cause errAssetRecords by adding an empty asset when Margin is disabled") + require.NotEmpty(t, subs, "ExpandTemplates must return some subs") + + require.NoError(t, ku.CurrencyPairs.SetAssetEnabled(asset.Margin, true), "SetAssetEnabled Margin must not error") + require.NoError(t, ku.CurrencyPairs.SetAssetEnabled(asset.Spot, false), "SetAssetEnabled Spot must not error") + require.NoError(t, ku.CurrencyPairs.SetAssetEnabled(asset.Futures, false), "SetAssetEnabled Futures must not error") + ku.Features.Subscriptions = subscription.List{{Channel: subscription.TickerChannel, Asset: asset.All}} + subs, err = ku.Features.Subscriptions.ExpandTemplates(ku) + require.NoError(t, err, "mergeMarginPairs must not cause errAssetRecords by adding an empty asset when Spot is disabled") + require.NotEmpty(t, subs, "ExpandTemplates must return some subs") } // TestCheckSubscriptions ensures checkSubscriptions upgrades user config correctly diff --git a/exchanges/kucoin/kucoin_websocket.go b/exchanges/kucoin/kucoin_websocket.go index 35922f268e7..a1c43bb68d5 100644 --- a/exchanges/kucoin/kucoin_websocket.go +++ b/exchanges/kucoin/kucoin_websocket.go @@ -1692,7 +1692,9 @@ func (ku *Kucoin) mergeMarginPairs(s *subscription.Subscription, ap map[asset.It wantKey := &subscription.IgnoringAssetKey{Subscription: s} switch s.Asset { case asset.All: - if _, marginEnabled := ap[asset.Margin]; marginEnabled { + _, marginEnabled := ap[asset.Margin] + _, spotEnabled := ap[asset.Spot] + if marginEnabled && spotEnabled { marginPairs, _ := ku.GetEnabledPairs(asset.Margin) ap[asset.Spot] = common.SortStrings(ap[asset.Spot].Add(marginPairs...)) ap[asset.Margin] = currency.Pairs{}