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 thrasher-corp#1755
gbjk · Dec 23, 2024 · 6ceeeae · 6ceeeae
1 parent 50448ec
commit 6ceeeae
Show file tree

Hide file tree

Showing 6 changed files with 165 additions and 117 deletions.
diff --git 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
@@ -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
@@ -1,22 +1,22 @@
-# GoCryptoTrader package Currency state manager
-
-<img src="/common/gctlogo.png?raw=true" width="350px" height="350px" hspace="70">
-
-
-[![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
+
+<img src="/common/gctlogo.png?raw=true" width="350px" height="350px" hspace="70">
+
+
+[![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
-
-<img src="https://github.com/thrasher-corp/gocryptotrader/blob/master/web/src/assets/donate.png?raw=true" hspace="70">
-
-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
+
+<img src="https://github.com/thrasher-corp/gocryptotrader/blob/master/web/src/assets/donate.png?raw=true" hspace="70">
+
+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
@@ -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
@@ -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,14 +2349,36 @@ 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")
 		}
 	}
 }
 
+// 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),