From f5f2b93d4f79d287041ec14218310b9bdc6b565c Mon Sep 17 00:00:00 2001 From: Gareth Kirwan Date: Mon, 12 Feb 2024 15:28:55 +0700 Subject: [PATCH] Subscription: Fix Match Key no pairs candidates have pairs --- exchanges/kraken/kraken_test.go | 13 ++++++++++++- exchanges/stream/websocket.go | 3 +-- exchanges/subscription/subscription.go | 11 +++++++---- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/exchanges/kraken/kraken_test.go b/exchanges/kraken/kraken_test.go index 28aeb0323de..085573218e7 100644 --- a/exchanges/kraken/kraken_test.go +++ b/exchanges/kraken/kraken_test.go @@ -1256,6 +1256,8 @@ func TestWsSubscribe(t *testing.T) { func TestWsOrderbookSub(t *testing.T) { t.Parallel() + k := new(Kraken) + require.NoError(t, testexch.TestInstance(k), "TestInstance must not error") testexch.SetupWs(t, k) err := k.Subscribe([]subscription.Subscription{{ @@ -1289,6 +1291,10 @@ func TestWsOrderbookSub(t *testing.T) { // TestWsCandlesSub tests caPairss subscription for Timeframe params func TestWsCandlesSub(t *testing.T) { + t.Parallel() + + k := new(Kraken) + require.NoError(t, testexch.TestInstance(k), "TestInstance must not error") testexch.SetupWs(t, k) err := k.Subscribe([]subscription.Subscription{{ @@ -1322,9 +1328,14 @@ func TestWsCandlesSub(t *testing.T) { // TestWsOwnTradesSub tests the authenticated WS subscription channel for trades func TestWsOwnTradesSub(t *testing.T) { - sharedtestvalues.SkipTestIfCredentialsUnset(t, k) + t.Parallel() + + k := new(Kraken) + require.NoError(t, testexch.TestInstance(k), "TestInstance must not error") testexch.SetupWs(t, k) + sharedtestvalues.SkipTestIfCredentialsUnset(t, k) + err := k.Subscribe([]subscription.Subscription{{Channel: krakenWsOwnTrades}}) assert.NoError(t, err, "Subsrcibing to ownTrades should not error") diff --git a/exchanges/stream/websocket.go b/exchanges/stream/websocket.go index ba69b8f2c76..d115f334b97 100644 --- a/exchanges/stream/websocket.go +++ b/exchanges/stream/websocket.go @@ -970,8 +970,7 @@ func (w *Websocket) AddSuccessfulSubscriptions(channels ...subscription.Subscrip if w.subscriptions == nil { w.subscriptions = subscription.Map{} } - for _, cN := range channels { - c := cN // cN is an iteration var; Not safe to make a pointer to + for _, c := range channels { key := c.EnsureKeyed() c.State = subscription.SubscribedState w.subscriptions[key] = &c diff --git a/exchanges/subscription/subscription.go b/exchanges/subscription/subscription.go index 7cfdb670b4e..8abe3dfa03b 100644 --- a/exchanges/subscription/subscription.go +++ b/exchanges/subscription/subscription.go @@ -89,8 +89,8 @@ func (s *Subscription) EnsureKeyed() any { // Match returns the first subscription which matches the Key's Asset, Channel and Pairs // If the key provided has: -// * Empty pairs then only Subscriptions without pairs will be considered -// * >=1 pairs then Subscriptions which contain all the pairs will be considered +// 1) Empty pairs then only Subscriptions without pairs will be considered +// 2) >=1 pairs then Subscriptions which contain all the pairs will be considered func (k Key) Match(m Map) *Subscription { for anyKey, s := range m { candidate, ok := anyKey.(Key) @@ -103,8 +103,11 @@ func (k Key) Match(m Map) *Subscription { if k.Asset != candidate.Asset { continue } - if (k.Pairs == nil || len(*k.Pairs) == 0) && (candidate.Pairs == nil || len(*candidate.Pairs) == 0) { - return s + if k.Pairs == nil || len(*k.Pairs) == 0 { + if candidate.Pairs == nil || len(*candidate.Pairs) == 0 { + return s + } + continue // Case (1) - key doesn't have any pairs but candidate does } if err := candidate.Pairs.ContainsAll(*k.Pairs, true); err == nil { return s