From 4c7c0bc533cbd79fbbddf62dd28b626080a3fec3 Mon Sep 17 00:00:00 2001 From: Gareth Kirwan Date: Wed, 23 Oct 2024 06:34:01 +0200 Subject: [PATCH] Subscriptions: Relax subscription validation for non-existent pairs (#1635) The subscription pairs do not need to be validated as enabled or available. The check was just belt-and-braces and didn't have a specific use-case in mind. Coinbase has a use-case for wanting to subscribe to BTC-USD when it's not enabled. Moreover, it shouldn't be our job to check this. You want a sub expanded with these pairs? Fine. Done. If it doesn't work, you can work out why --- exchanges/subscription/template.go | 16 +++++++--------- exchanges/subscription/template_test.go | 11 ++++++++--- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/exchanges/subscription/template.go b/exchanges/subscription/template.go index ab0a18b4..ae994f79 100644 --- a/exchanges/subscription/template.go +++ b/exchanges/subscription/template.go @@ -104,6 +104,13 @@ func expandTemplate(e iExchange, s *Subscription, ap assetPairs, assets asset.It subs := List{} + if len(s.Pairs) != 0 { + // We deliberately do not check Availability of sub Pairs because users have edge cases to subscribe to non-existent pairs + for a := range ap { + ap[a] = s.Pairs + } + } + switch s.Asset { case asset.All: subCtx.AssetPairs = ap @@ -118,15 +125,6 @@ func expandTemplate(e iExchange, s *Subscription, ap assetPairs, assets asset.It } } - if len(s.Pairs) != 0 { - for a, pairs := range subCtx.AssetPairs { - if err := pairs.ContainsAll(s.Pairs, true); err != nil { //nolint:govet // Shadow, or gocritic will complain sloppyReassign - return nil, err - } - subCtx.AssetPairs[a] = s.Pairs - } - } - buf := &bytes.Buffer{} if err := t.Execute(buf, subCtx); err != nil { //nolint:govet // Shadow, or gocritic will complain sloppyReassign return nil, err diff --git a/exchanges/subscription/template_test.go b/exchanges/subscription/template_test.go index f14e851e..5025b8df 100644 --- a/exchanges/subscription/template_test.go +++ b/exchanges/subscription/template_test.go @@ -84,13 +84,18 @@ func TestExpandTemplates(t *testing.T) { } equalLists(t, exp, got) + // Users can specify pairs which aren't available, even across diverse assets + // Use-case: Coinbasepro user sub for futures BTC-USD would return all BTC pairs and all USD pairs, even though BTC-USD might not be enabled or available + p := currency.Pairs{currency.NewPairWithDelimiter("BEAR", "PEAR", "🐻")} + got, err = List{{Channel: "expand-pairs", Asset: asset.All, Pairs: p}}.ExpandTemplates(e) + require.NoError(t, err, "Must not error with fictional pairs") + exp = List{{Channel: "expand-pairs", QualifiedChannel: "spot-PEARBEAR-expand-pairs@0", Asset: asset.Spot, Pairs: p}} + equalLists(t, exp, got) + // Error cases _, err = List{{Channel: "nil"}}.ExpandTemplates(e) assert.ErrorIs(t, err, errInvalidTemplate, "Should get correct error on nil template") - _, err = List{{Channel: "single-channel", Asset: asset.Spot, Pairs: currency.Pairs{currency.NewPairWithDelimiter("NOPE", "POPE", "🐰")}}}.ExpandTemplates(e) - assert.ErrorIs(t, err, currency.ErrPairNotContainedInAvailablePairs, "Should error correctly when pair not available") - e.tpl = "errors.tmpl" _, err = List{{Channel: "error1"}}.ExpandTemplates(e)