Skip to content

Commit 4c7c0bc

Browse files
authored
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
1 parent 4fc1bf0 commit 4c7c0bc

File tree

2 files changed

+15
-12
lines changed

2 files changed

+15
-12
lines changed

exchanges/subscription/template.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,13 @@ func expandTemplate(e iExchange, s *Subscription, ap assetPairs, assets asset.It
104104

105105
subs := List{}
106106

107+
if len(s.Pairs) != 0 {
108+
// We deliberately do not check Availability of sub Pairs because users have edge cases to subscribe to non-existent pairs
109+
for a := range ap {
110+
ap[a] = s.Pairs
111+
}
112+
}
113+
107114
switch s.Asset {
108115
case asset.All:
109116
subCtx.AssetPairs = ap
@@ -118,15 +125,6 @@ func expandTemplate(e iExchange, s *Subscription, ap assetPairs, assets asset.It
118125
}
119126
}
120127

121-
if len(s.Pairs) != 0 {
122-
for a, pairs := range subCtx.AssetPairs {
123-
if err := pairs.ContainsAll(s.Pairs, true); err != nil { //nolint:govet // Shadow, or gocritic will complain sloppyReassign
124-
return nil, err
125-
}
126-
subCtx.AssetPairs[a] = s.Pairs
127-
}
128-
}
129-
130128
buf := &bytes.Buffer{}
131129
if err := t.Execute(buf, subCtx); err != nil { //nolint:govet // Shadow, or gocritic will complain sloppyReassign
132130
return nil, err

exchanges/subscription/template_test.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,18 @@ func TestExpandTemplates(t *testing.T) {
8484
}
8585
equalLists(t, exp, got)
8686

87+
// Users can specify pairs which aren't available, even across diverse assets
88+
// 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
89+
p := currency.Pairs{currency.NewPairWithDelimiter("BEAR", "PEAR", "🐻")}
90+
got, err = List{{Channel: "expand-pairs", Asset: asset.All, Pairs: p}}.ExpandTemplates(e)
91+
require.NoError(t, err, "Must not error with fictional pairs")
92+
exp = List{{Channel: "expand-pairs", QualifiedChannel: "spot-PEARBEAR-expand-pairs@0", Asset: asset.Spot, Pairs: p}}
93+
equalLists(t, exp, got)
94+
8795
// Error cases
8896
_, err = List{{Channel: "nil"}}.ExpandTemplates(e)
8997
assert.ErrorIs(t, err, errInvalidTemplate, "Should get correct error on nil template")
9098

91-
_, err = List{{Channel: "single-channel", Asset: asset.Spot, Pairs: currency.Pairs{currency.NewPairWithDelimiter("NOPE", "POPE", "🐰")}}}.ExpandTemplates(e)
92-
assert.ErrorIs(t, err, currency.ErrPairNotContainedInAvailablePairs, "Should error correctly when pair not available")
93-
9499
e.tpl = "errors.tmpl"
95100

96101
_, err = List{{Channel: "error1"}}.ExpandTemplates(e)

0 commit comments

Comments
 (0)