websocket/gateio: Support multi connection management and integrate with GateIO #1580

shazbert · 2024-07-15T05:56:25Z

WebSocket

Multiple Connections for Asset Endpoints

Feature: Implemented the ability to spawn multiple WebSocket connections for different asset endpoints.
Design Decision:
- Opted for a separate subscription holder per connection due to import cycle issues with a unified subscription manager.
- This allows for future scalability when exchange constraints limit subscriptions per connection.

Connection-Coupled Functionality

Approach: Coupled specific functionality with the connection itself, along with its own subscription holder.
Benefit: Enables tracking multiple connections efficiently, facilitating scaling when exchanges restrict subscriptions per connection.

Reader Routine in Streaming Package

Implementation: Created a reader routine in the streaming package to avoid code duplication.
Future Work: This will initiate a purge across wrappers to standardize this routine.

Context Awareness

Progress: Began integrating context awareness throughout the WebSocket functionality.
Enhancement: Added a DialContext method to WebsocketConnection to leverage the supported context.

Gate.io Enhancements

WebSocket Support

Added Support:
- Futures
- Dated Futures
- Options
Note: Margin, cross margin, and BTC-margined dated futures not included due to PR size and complexity.

Bug Fixes

Delivery Futures Handling:
- Fixed issue where all available pairs were fetched instead of only enabled pairs.
Payload Generation:
- Addressed issues in generateDeliveryFuturesPayload where different settlements were handled incorrectly.
- Separated connections for different settlements to flatten functions for consistency.

🐛 fix:

Type of change

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

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
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.

[I ran it 🏃 instead] go test ./... -race
golangci-lint run
Test X

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation and regenerated documentation via the documentation tool
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally and on Github Actions with my changes
Any dependent changes have been merged and published in downstream modules

codecov · 2024-07-15T06:04:53Z

Codecov Report

Attention: Patch coverage is 70.05848% with 256 lines in your changes missing coverage. Please review.

Project coverage is 36.93%. Comparing base (910cdaa) to head (2a62643).
Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
exchanges/stream/websocket.go	84.13%	55 Missing and 24 partials ⚠️
exchanges/gateio/gateio_ws_futures.go	43.54%	33 Missing and 2 partials ⚠️
exchanges/gateio/gateio_ws_delivery_futures.go	11.76%	30 Missing ⚠️
exchanges/gateio/gateio_websocket.go	60.00%	14 Missing and 6 partials ⚠️
exchanges/gateio/gateio_wrapper.go	67.74%	14 Missing and 6 partials ⚠️
exchanges/gateio/gateio_ws_option.go	23.80%	15 Missing and 1 partial ⚠️
exchanges/stream/websocket_connection.go	50.00%	14 Missing and 1 partial ⚠️
exchanges/okx/okx_websocket.go	0.00%	6 Missing ⚠️
engine/rpcserver.go	0.00%	5 Missing ⚠️
exchanges/bitfinex/bitfinex_websocket.go	40.00%	1 Missing and 2 partials ⚠️
... and 14 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1580      +/-   ##
==========================================
+ Coverage   36.83%   36.93%   +0.09%     
==========================================
  Files         413      414       +1     
  Lines      180457   180478      +21     
==========================================
+ Hits        66472    66654     +182     
+ Misses     106185   105999     -186     
- Partials     7800     7825      +25

Files with missing lines	Coverage Δ
exchanges/binance/binance_wrapper.go	`38.48% <100.00%> (+0.07%)`	⬆️
exchanges/binanceus/binanceus_wrapper.go	`40.89% <100.00%> (ø)`
exchanges/bitfinex/bitfinex_wrapper.go	`37.66% <100.00%> (ø)`
exchanges/bithumb/bithumb_wrapper.go	`29.68% <100.00%> (ø)`
exchanges/bitmex/bitmex_wrapper.go	`38.85% <100.00%> (ø)`
exchanges/bitstamp/bitstamp_wrapper.go	`53.23% <100.00%> (ø)`
exchanges/btcmarkets/btcmarkets_wrapper.go	`32.77% <100.00%> (ø)`
exchanges/btse/btse_wrapper.go	`43.72% <100.00%> (ø)`
exchanges/bybit/bybit_wrapper.go	`43.20% <100.00%> (ø)`
exchanges/coinbasepro/coinbasepro_wrapper.go	`34.41% <100.00%> (ø)`
... and 37 more

... and 9 files with indirect coverage changes

…larity on purpose. Change connections map to point to candidate to track subscriptions for future dynamic connections holder and drop struct ConnectionDetails.

gloriousCode

I'll only offer very light commentary right now.

There are things I'd do differently from this first look, but it doesn't work to comment in that way because that could blow out having an implementation where this looks to try and minimise the amount of changes while having backwards compatability. Also, this works

I'll try and focus on things to fix up and any holes I find

exchanges/stream/websocket_types.go

exchanges/stream/websocket_connection.go

exchanges/gateio/gateio_ws_delivery_futures.go

exchanges/binance/binance_websocket.go

exchanges/gateio/gateio_websocket.go

exchanges/stream/websocket.go

exchanges/stream/websocket_connection.go

…le to echo back ws data as it can be easily reviewed _test file side.

…ad of waiting for a time period, add sleep inline with echo handler as this is really quick and wanted to ensure that latency is handing correctly

gloriousCode

Thanks for addressing those issues! I think there's a potential double-shut/close happening

gloriousCode · 2024-09-30T02:02:19Z

exchanges/stream/websocket.go

+
+// monitorTraffic monitors to see if there has been traffic within the trafficTimeout time window. If there is no traffic
+// the connection is shutdown and will be reconnected by the connectionMonitor routine.
+func (w *Websocket) monitorTraffic() func() bool {


I tried simulating a disconnect on Binance and got the following:

panic: close of closed channel goroutine 950 [running]: github.com/thrasher-corp/gocryptotrader/exchanges/stream.(*WebsocketConnection).Shutdown(0x14000b12000) /Users/WIF/go/src/github.com/thrasher-corp/gocryptotrader/exchanges/stream/websocket_connection.go:275 +0x70 github.com/thrasher-corp/gocryptotrader/exchanges/stream.(*Websocket).shutdown(0x14000264800) /Users/WIF/go/src/github.com/thrasher-corp/gocryptotrader/exchanges/stream/websocket.go:553 +0x364 github.com/thrasher-corp/gocryptotrader/exchanges/stream.(*Websocket).Shutdown(0x0?) /Users/WIF/go/src/github.com/thrasher-corp/gocryptotrader/exchanges/stream/websocket.go:512 +0xb8 github.com/thrasher-corp/gocryptotrader/exchanges/stream.(*Websocket).observeTraffic.func1() /Users/WIF/go/src/github.com/thrasher-corp/gocryptotrader/exchanges/stream/websocket.go:1233 +0x24 created by github.com/thrasher-corp/gocryptotrader/exchanges/stream.(*Websocket).observeTraffic in goroutine 427 /Users/WIF/go/src/github.com/thrasher-corp/gocryptotrader/exchanges/stream/websocket.go:1232 +0x1b4 exit status 2

This does not occur on master branch. I have not been able to re-do this

I had a different recreation on closing the application. In this instance I had both GateIO and Binance enabled

^C[INFO] | SYNC | 30/09/2024 12:09:56 | GateIO websocket BTC-USDT SPOT ORDERBOOK: Bids len: 1 Amount: 2.933270 BTC. Total value: 189864.407233 Asks len: 1 Amount: 0.045910 BTC. Total value: 2971.66248 [INFO] | LOG | 30/09/2024 12:09:56 | Captured interrupt, shutdown requested. [DEBUG] | LOG | 30/09/2024 12:09:56 | Engine shutting down.. [DEBUG] | ORDER | 30/09/2024 12:09:56 | Order manager shutting down... [DEBUG] | PORTFOLIO | 30/09/2024 12:09:56 | Portfolio manager shutting down... [DEBUG] | CONNECTION | 30/09/2024 12:09:56 | Connection manager shutting down... [DEBUG] | CONNECTION | 30/09/2024 12:09:56 | Connection manager stopped. [DEBUG] | PORTFOLIO | 30/09/2024 12:09:56 | Portfolio manager shutdown. [DEBUG] | ORDER | 30/09/2024 12:09:56 | Order manager shutdown. [DEBUG] | DISPATCH | 30/09/2024 12:09:56 | Dispatch manager shutting down... [DEBUG] | DISPATCH | 30/09/2024 12:09:56 | Dispatch manager shutdown [DEBUG] | EXCHANGE | 30/09/2024 12:09:56 | Currency state manager shutting down... [DEBUG] | EXCHANGE | 30/09/2024 12:09:56 | Currency state manager shutdown. panic: close of closed channel goroutine 677 [running]: github.com/thrasher-corp/gocryptotrader/exchanges/stream.(*WebsocketConnection).Shutdown(0x14000c06000) /Users/WIF/go/src/github.com/thrasher-corp/gocryptotrader/exchanges/stream/websocket_connection.go:275 +0x70 github.com/thrasher-corp/gocryptotrader/exchanges/stream.(*Websocket).shutdown(0x140001fc800) /Users/WIF/go/src/github.com/thrasher-corp/gocryptotrader/exchanges/stream/websocket.go:553 +0x364 github.com/thrasher-corp/gocryptotrader/exchanges/stream.(*Websocket).Shutdown(0x14000406be0?) /Users/WIF/go/src/github.com/thrasher-corp/gocryptotrader/exchanges/stream/websocket.go:512 +0xb8 github.com/thrasher-corp/gocryptotrader/exchanges.(*Base).Shutdown(0x1400035a008) /Users/WIF/go/src/github.com/thrasher-corp/gocryptotrader/exchanges/exchange.go:1644 +0x2c github.com/thrasher-corp/gocryptotrader/engine.(*ExchangeManager).Shutdown.func1(0x14000cfe040, 0x14000cfe030, {0x1017f2e20, 0x1400035a008}) /Users/WIF/go/src/github.com/thrasher-corp/gocryptotrader/engine/exchange_manager.go:159 +0x3c created by github.com/thrasher-corp/gocryptotrader/engine.(*ExchangeManager).Shutdown in goroutine 1 /Users/WIF/go/src/github.com/thrasher-corp/gocryptotrader/engine/exchange_manager.go:158 +0x1f8 exit status 2

gloriousCode

tACK!

…dress

thrasher-

Been testing this for a few hours and appears to be working well! Just some unused vars with the changes of this PR

exchanges/stream/websocket.go

thrasher-

Thanks for making those changes!

shazbert added 5 commits July 14, 2024 16:13

gateio: Add multi asset websocket support WIP.

64c24ab

meow

f509399

Add tests and shenanigans

feed04e

integrate flushing and for enabling/disabling pairs from rpc shenanigans

31a26c0

some changes

e1f2f7a

shazbert added review me This pull request is ready for review pending response awaiting a response from the author blocked high priority labels Jul 15, 2024

shazbert requested review from gbjk and a team July 15, 2024 05:56

shazbert self-assigned this Jul 15, 2024

linter: fixes strikes again.

76524cc

gloriousCode removed the pending response awaiting a response from the author label Jul 15, 2024

Change name ConnectionAssociation -> ConnectionCandidate for better c…

640e82e

…larity on purpose. Change connections map to point to candidate to track subscriptions for future dynamic connections holder and drop struct ConnectionDetails.

gloriousCode requested changes Jul 16, 2024

View reviewed changes

Ryan O'Hara-Reid added 6 commits July 16, 2024 12:42

Add subscription tests (state functional)

3a0440d

glorious:nits + proxy handling

c7d2b62

Spelling

fc281ee

linter: fixerino

eaa44ba

instead of nil, dont do nil.

a8debf9

clean up nils

16b0e22

shazbert removed the blocked label Jul 16, 2024

shazbert requested a review from gloriousCode July 16, 2024 04:51

cya nils

32252b2

This was referenced Jul 16, 2024

stream: multiple connections management [draft] #1504

Closed

stream: shift websocket reader and dialer code from wrapper to stream package #1493

Closed

Ryan O'Hara-Reid added 2 commits July 17, 2024 13:13

don't need to set URL or check if its running

7c5d9c3

stop ping handler routine leak

2d2f872

Ryan O'Hara-Reid added 11 commits September 25, 2024 12:17

spells

10230b0

cant do what I did

4e02c4a

protect race due to routine.

7be80a0

update testURL

b51cf2f

use mock websocket connection instead of test URL's

ff7ae03

linter: fix

1040153

remove url because its throwing errors on CI builds

f529bd2

connections drop all the time, don't need to worry about not being ab…

5d0a7f7

…le to echo back ws data as it can be easily reviewed _test file side.

remove another superfluous url thats not really set up for this

56cb431

spawn overwatch routine when there is no errors, inline checker inste…

ca4999e

…ad of waiting for a time period, add sleep inline with echo handler as this is really quick and wanted to ensure that latency is handing correctly

linter: fixerino uperino

4240a0a

gloriousCode requested changes Sep 30, 2024

View reviewed changes

glorious: panix

8d6febc

gloriousCode approved these changes Sep 30, 2024

View reviewed changes

Ryan O'Hara-Reid added 8 commits October 1, 2024 14:04

Merge branch 'master' into gateio_ws

3df82ef

Merge branch 'master' into gateio_ws

7ff07d1

linter: things

3a24640

whoops

1eef208

Merge branch 'master' into gateio_ws

261b577

defer lock and use functions that don't require locking in SetProxyAd…

1d0dc25

…dress

Merge branch 'master' into gateio_ws

86cd5f9

lint: fix

81bda15

gloriousCode added the gcrc GloriousCode Review Complete label Oct 9, 2024

thrasher- requested changes Oct 10, 2024

View reviewed changes

exchanges/stream/websocket.go Outdated Show resolved Hide resolved

exchanges/stream/websocket.go Outdated Show resolved Hide resolved

thrasher: nits

2a62643

thrasher- changed the title ~~websocket/gateio: Multiple connection management and gateio integration.~~ websocket/gateio: Support multiple connection management and integrate with GateIO Oct 10, 2024

thrasher- changed the title ~~websocket/gateio: Support multiple connection management and integrate with GateIO~~ websocket/gateio: Support multi connection management and integrate with GateIO Oct 10, 2024

thrasher- approved these changes Oct 10, 2024

View reviewed changes

thrasher- merged commit ac731ce into thrasher-corp:master Oct 10, 2024
10 of 13 checks passed

gbjk mentioned this pull request Dec 26, 2024

Subscription and Websocket multiplexing #1764

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

websocket/gateio: Support multi connection management and integrate with GateIO #1580

websocket/gateio: Support multi connection management and integrate with GateIO #1580

shazbert commented Jul 15, 2024 •

edited

Loading

codecov bot commented Jul 15, 2024 •

edited

Loading

gloriousCode left a comment •

edited

Loading

gloriousCode left a comment

gloriousCode Sep 30, 2024

gloriousCode Sep 30, 2024 •

edited

Loading

gloriousCode left a comment

thrasher- left a comment

thrasher- left a comment

websocket/gateio: Support multi connection management and integrate with GateIO #1580

websocket/gateio: Support multi connection management and integrate with GateIO #1580

Conversation

shazbert commented Jul 15, 2024 • edited Loading

WebSocket

Multiple Connections for Asset Endpoints

Connection-Coupled Functionality

Reader Routine in Streaming Package

Context Awareness

Gate.io Enhancements

WebSocket Support

Bug Fixes

Type of change

How has this been tested

Checklist

codecov bot commented Jul 15, 2024 • edited Loading

Codecov Report

gloriousCode left a comment • edited Loading

Choose a reason for hiding this comment

gloriousCode left a comment

Choose a reason for hiding this comment

gloriousCode Sep 30, 2024

Choose a reason for hiding this comment

gloriousCode Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

gloriousCode left a comment

Choose a reason for hiding this comment

thrasher- left a comment

Choose a reason for hiding this comment

thrasher- left a comment

Choose a reason for hiding this comment

shazbert commented Jul 15, 2024 •

edited

Loading

codecov bot commented Jul 15, 2024 •

edited

Loading

gloriousCode left a comment •

edited

Loading

gloriousCode Sep 30, 2024 •

edited

Loading