exchanges/websocket: Implement subscription configuration

[gbjk]

• Move Subscriptions to it's own package
  • This allows the small type to be imported from both config and from
    stream without an import cycle, so we don't have to repeat ourselves
• Rename Subscription.Currency to Subscription.Pair
• Add Subscription Configuration
• Binance support for Subscription Configuration
• Kucoin support for Subscription Configuration

Type of change

Please delete options that are not relevant and add an x in [] as item is complete.

• New feature (non-breaking change which adds functionality)
• This change requires a documentation update

How has this been tested

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration and
also consider improving test coverage whilst working on a certain feature or package.

• go test ./... -race
• golangci-lint run

[shazbert]

Just a quick check, code and running looks good. 👍

[gloriousCode]

I've only done a small review of the overall concept, I'm happy to see it works. A few things I've picked up and then I can get into the nitty gritty

[shazbert]

Something that came up while testing.

[gbjk]

@thrasher- I've pulled in the abstracted form of testExch.TestInstance which was prototyped in #1358 and then built out for other exchanges.
That's allowed us to isolate the kucoin sub testing from the pair changes introduced for GetOpenInterest testing.

Also rebased to fix the master conflict.

[codecov]

Codecov Report

Attention: 342 lines in your changes are missing coverage. Please review.

Comparison is base (301551a) 43.33% compared to head (0e4eac8) 43.39%.

❗ Current head 0e4eac8 differs from pull request most recent head 44f7c91. Consider uploading reports for the commit 44f7c91 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1394      +/-   ##
==========================================
+ Coverage   43.33%   43.39%   +0.06%     
==========================================
  Files         366      367       +1     
  Lines      147087   147096       +9     
==========================================
+ Hits        63744    63837      +93     
+ Misses      75520    75436      -84     
  Partials     7823     7823              

Files	Coverage Δ
currency/currencies.go	87.03% <100.00%> (+1.32%)	⬆️
exchanges/binance/binance.go	45.32% <ø> (ø)
exchanges/binance/binance_wrapper.go	38.22% <100.00%> (+0.13%)	⬆️
exchanges/bitfinex/bitfinex.go	22.90% <ø> (ø)
exchanges/kucoin/kucoin_types.go	60.86% <ø> (ø)
exchanges/kucoin/kucoin_wrapper.go	34.30% <100.00%> (+0.56%)	⬆️
exchanges/stream/websocket.go	82.55% <100.00%> (-0.20%)	⬇️
exchanges/bithumb/bithumb_websocket.go	42.85% <75.00%> (ø)
exchanges/subscription/subscription.go	93.33% <93.33%> (ø)
engine/rpcserver.go	38.37% <0.00%> (ø)
... and 28 more

... and 18 files with indirect coverage changes

[gbjk]

🔁 Rebased on master

[thrasher-]

All looking good when running live. Very cool stuff!! 🎉

Some areas of improvement for future expansion would be explicit interval support and validation, since ATM I can set wild figures in the config for Binance and it'll happily accept it. Definitely a user error thing, but something we should guard against.

The other thing would be that if I put one invalid subscription entry, with the rest being valid. It will correctly report the invalid subscription but no other valid subscriptions will take place and the websocket connection will be establibshed but be completely idle. So the connection will be doing nothing. We should handle this by expecting at least one valid subscription enabled (on top of the correctly reported user configured issues), otherwise disable websocket and report the issue to the user.

Last thing would be unification of fills feed / trade feed and subscription togglability with gctcli. Overall a very cool feature, great work!

[gloriousCode]

I have found myself confused about how your rebasing timelines work. I requested changes on Jan 2nd, but there is only changes done on November 22nd and Jan 16 which was after I approved my review on January 8th.

So do change requests get bundled before or after? It is more difficult to keep track of all pull request reviews and their state of code leaving me doubting what code specifically I had already reviewed. I'll do a final review tomorrow

edit: Shazbert's request here: #1394 (comment)
got pulled into a commit from 2 months ago, yet it was raised on January 8th, meaning I can't really trust the commit from November 22nd since it has changed since I reviewed it

[gbjk]

I have found myself confused about how your rebasing timelines work. I requested changes on Jan 2nd, but there is only changes done on November 22nd and Jan 16 which was after I approved my review on January 8th.

So do change requests get bundled before or after? It is more difficult to keep track of all pull request reviews and their state of code leaving me doubting what code specifically I had already reviewed. I'll do a final review tomorrow

Rebased after you approved and added gcrc. At that point all changes you'd asked for and subsequently approved get fixuped down to their respective original commits.

Then any commits added after that are left for you to see again.

Specifically the code you haven't reviewed was 94da21f and 7283b51.

edit: Shazbert's request here: #1394 (comment) got pulled into a commit from 2 months ago, yet it was raised on January 8th, meaning I can't really trust the commit from November 22nd since it has changed since I reviewed it

Yes. The commit was fixed on 8th January and merged down on 15th January ( specifically after you signalled you were done ).

The commits change, but the body of code is as you last saw it. The autosquash just moves the commits down and merges them once you've approved your changes.

As discussed previously, I'm leaving everything completely alone until each member is done, and then I merge it at that point.

Since it seems confusing, I can leave it right until the end and then rebase at the very last moment?
Normally fixups get merged when you rebase on master, but I can avoid that.

[gloriousCode]

Thank you for your explanations. I can put my reply which I messaged you here if you'd like.
I re-reviewed everything and only found the tiniest thing 😄

[gbjk]

Thank you for your explanations. I can put my reply which I messaged you here if you'd like. I re-reviewed everything and only found the tiniest thing 😄

Thank you 🙏 Ready again.

[gloriousCode]

tACK! Thanks for making those changes!

[thrasher-]

Great stuff @gbjk, awesome feature!