bybit: enable multiconnection handling across websocket endpoints #1670

shazbert · 2024-10-09T01:15:24Z

PR Description

Expand websocket handling to concurrently allow processing for CoinMarginedFutures, USDTMarginedFutures, USDCMarginedFutures, Options.
Implement dedicated connection for private data.

NOTE: This is a nice size PR for just functionality expansion, I will build outbound request functions for trading on top of this in a future PR. Here's the dependency: #1603

Fixes #

🐛 ~~DOGEUSDT wasn't being processed correctly so used MatchSymbolWithAvailablePairs because there was no delimiter.~~ This was merged upstream.

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.

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

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

…rror but websocket frame error or anything really makes the reader routine return and then connection never cycles and the buffer gets filled. * Handle reconnection via an errors.Is check which is simpler and in that scope allow for quick disconnect reconnect without waiting for connection cycle. * Dial now uses code from DialContext but just calls context.Background() * Don't allow reader to return on parse binary response error. Just output error and return a non nil response

…would hang connection reader for eternity.

gloriousCode

Nice work shazbert, I'm quite happy with a lot of the work

gloriousCode · 2025-04-28T06:52:24Z

exchanges/bybit/bybit_wrapper.go

+		Handler: func(ctx context.Context, resp []byte) error {
+			return by.wsHandleData(ctx, resp, asset.USDCMarginedFutures)
+		},
+		BespokeGenerateMessageID: by.bespokeWebsocketRequestID,


Is it not too late to update these to RequestIDGenerator: by.websocketRequestIDGenerator,/ by.getWebsocketRequestID,? 😄

gloriousCode · 2025-04-28T06:59:17Z

exchanges/bybit/bybit_websocket.go

+	return fmt.Errorf("unhandled stream data %s", string(respRaw))
+}
+
+func (by *Bybit) wsHandleAuthenticated(_ context.Context, respRaw []byte) error {


Handle authenticated what? 😄

gloriousCode · 2025-04-28T23:46:19Z

exchanges/bybit/bybit_websocket.go

+		// Below provides a way of matching an order change to a websocket request. There is no batch support for this
+		// so the first element will be used to match the order ID.
+		if id, err := jsonparser.GetString(respRaw, "data", "[0]", "orderId"); err == nil {


We don't know bybit's orderID before being sent so we can't match on that. Sure, this captures updates, but why not also match on OrderLinkID which can allow you to match on order creation?

Nice catch this was actually updated to that in the request PR built on this. 🚀 I will roll that update here.

gloriousCode · 2025-04-28T23:51:08Z

exchanges/bybit/bybit_websocket.go

+	tick := &ticker.Price{Pair: p, ExchangeName: by.Name, AssetType: assetType}
+	snapshot, err := ticker.GetTicker(by.Name, p, assetType)
+	if err == nil && resp.Type != "snapshot" {
 		// ticker updates may be partial, so we need to update the current ticker
-		tick, err = ticker.GetTicker(by.Name, p, assetType)
-		if err != nil {
-			return err
-		}
+		tick = snapshot
 	}

-	updateTicker(tick, tickResp)
+	updateTicker(tick, &tickResp)
 	tick.LastUpdated = resp.PushTimestamp.Time()

 	if err = ticker.ProcessTicker(tick); err == nil {
 		by.Websocket.DataHandler <- tick
 	}
-
 	return err


tick := &ticker.Price{Pair: p, ExchangeName: by.Name, AssetType: assetType} if resp.Type != "snapshot" { snapshot, err := ticker.GetTicker(by.Name, p, assetType) if err != nil { return err } // ticker updates may be partial, so we need to update the current ticker tick = snapshot } updateTicker(tick, &tickResp) tick.LastUpdated = resp.PushTimestamp.Time() if err = ticker.ProcessTicker(tick); err != nil { return err } by.Websocket.DataHandler <- tick return nil

I think is much cleaner, clearer and standard

exchanges/bybit/bybit_websocket.go

gloriousCode · 2025-04-28T23:55:17Z

exchanges/bybit/bybit_websocket.go

@@ -792,3 +740,162 @@ const subTplText = `
 func hasPotentialDelimiter(a asset.Item) bool {
 	return a == asset.Options || a == asset.USDCMarginedFutures
 }
+
+// TODO: Remove this function when template expansion is across all assets
+func (by *Bybit) handleSubscriptionNonTemplate(ctx context.Context, conn websocket.Connection, a asset.Item, operation string, channelsToSubscribe subscription.List) error {


Suggested change

func (by *Bybit) handleSubscriptionNonTemplate(ctx context.Context, conn websocket.Connection, a asset.Item, operation string, channelsToSubscribe subscription.List) error {

func (by *Bybit) submitSubscriptionNonTemplate(ctx context.Context, conn websocket.Connection, a asset.Item, operation string, channelsToSubscribe subscription.List) error {

Just to distinguish from handleSubscriptionsNonTemplate a bit more and a bit closer to what it does.
That or update the other name

gloriousCode · 2025-04-28T23:57:14Z

exchanges/bybit/bybit_websocket.go

+	// These channels are currently being managed by the `generateAuthSubscriptions` method for the private connection
+	// TODO: Reimplement these channels
+	// {Enabled: true, Asset: asset.Spot, Authenticated: true, Channel: subscription.MyOrdersChannel},
+	// {Enabled: true, Asset: asset.Spot, Authenticated: true, Channel: subscription.MyWalletChannel},
+	// {Enabled: true, Asset: asset.Spot, Authenticated: true, Channel: subscription.MyTradesChannel},


Suggested change

// These channels are currently being managed by the `generateAuthSubscriptions` method for the private connection

// TODO: Reimplement these channels

// {Enabled: true, Asset: asset.Spot, Authenticated: true, Channel: subscription.MyOrdersChannel},

// {Enabled: true, Asset: asset.Spot, Authenticated: true, Channel: subscription.MyWalletChannel},

// {Enabled: true, Asset: asset.Spot, Authenticated: true, Channel: subscription.MyTradesChannel},

// Authenticated channels are currently being managed by the `generateAuthSubscriptions` method for the private connection

// TODO: expand subscription template generation to handle authenticated subscriptions across all assets

gloriousCode · 2025-04-29T00:03:51Z

exchanges/bybit/bybit_websocket.go

+	var err error
+	for _, a := range assets {
+		var cp currency.Pair
+		if cp, err = by.MatchSymbolWithAvailablePairs(symbol, a, hasPotentialDelimiter(a)); err == nil {
+			return cp, a, nil
+		}
+	}
+	return currency.EMPTYPAIR, 0, err


for _, a := range assets { cp, err := by.MatchSymbolWithAvailablePairs(symbol, a, hasPotentialDelimiter(a)) if err != nil { return currency.EMPTYPAIR, 0, err } return cp, a, nil } return currency.EMPTYPAIR, 0, nil

I think its an improvement to readability to not try funny business with err handling and such

gloriousCode · 2025-04-29T00:06:10Z

exchanges/bybit/bybit_websocket.go

+	for _, channel := range []string{chanPositions, chanExecution, chanOrder, chanWallet} {
+		subscriptions = append(subscriptions, &subscription.Subscription{
+			Channel: channel,
+			Pairs:   currency.Pairs{currency.EMPTYPAIR}, // This is a placeholder, the actual pair is not required for these channels


Is pairs required to be filled? Have you tried not sending pairs?

github-actions · 2025-05-30T00:01:46Z

This PR is stale because it has been open 30 days with no activity. Please provide an update on the progress of this PR.

Copilot

Pull Request Overview

This PR enhances Bybit’s websocket handling by introducing multi-connection support for spot, options, linear (USDT/USDC margined), inverse (coin margined) futures and a dedicated authenticated connection.
Key changes:

Refactor Setup to initialize individual websocket.Connection instances for each asset type and a private connection.
Update websocket handlers to include context, unified subscription logic, and routing for authenticated messages.
Separate REST and websocket ticker types (TickerREST, TickerWebsocket) and adapt parsing logic.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
exchanges/bybit/bybit_wrapper.go	Refactor `Setup` to create multiple websocket connections.
exchanges/bybit/bybit_websocket.go	Revamp subscription/auth handlers; add context and routing logic.
exchanges/bybit/bybit_types.go	Split the old `TickerItem` into `TickerREST` and `TickerWebsocket`.
exchanges/bybit/bybit_test.go	Update websocket tests to use `DummyConnection` and context.
exchanges/bybit/bybit_options_websocket.go	Remove legacy `WsOptionsConnect`, unify subscription calls.
exchanges/bybit/bybit_linear_websocket.go	Simplify linear subscription generator and update signatures.
exchanges/bybit/bybit_inverse_websocket.go	Simplify inverse subscription generator and update signatures.
exchanges/bybit/bybit.go	Add `counter` field for message ID generation.
docs/ADD_NEW_EXCHANGE.md	Fix formatting typo.

Comments suppressed due to low confidence (1)

exchanges/bybit/bybit_test.go:3194

Currently only spot data paths are exercised in TestPushDataPublic. To ensure all connection handlers work as expected, add unit tests for wsHandleData with payloads for options, USDT/USDC margined futures, and coin‐margined futures.

func TestPushDataPublic(t *testing.T) {

Copilot · 2025-05-30T01:32:01Z

exchanges/bybit/bybit_websocket.go

@@ -351,7 +314,7 @@ func (by *Bybit) wsProcessWalletPushData(assetType asset.Item, resp []byte) erro
 			accounts = append(accounts, account.Change{
 				Exchange: by.Name,
 				Currency: currency.NewCode(result.Data[x].Coin[y].Coin),
-				Asset:    assetType,
+				Asset:    asset.Spot,


In wsProcessWalletPushData, the Asset field is hardcoded to asset.Spot. Since this handler is shared across different asset‐type connections, consider passing through or inferring the correct assetType instead of always using spot.

Suggested change

Asset: asset.Spot,

Asset: assetType,

param reduced, no assetype

Copilot · 2025-05-30T01:32:01Z

exchanges/bybit/bybit_websocket.go

+// TODO: Remove this function when template expansion is across all assets
+func (by *Bybit) handleSubscriptionNonTemplate(ctx context.Context, conn websocket.Connection, a asset.Item, operation string, channelsToSubscribe subscription.List) error {
+	payloads, err := by.handleSubscriptionsNonTemplate(conn, a, operation, channelsToSubscribe)
+	if err != nil {
+		return err
+	}
+	for _, payload := range payloads {
+		if a == asset.Options {
+			// The options connection does not send the subscription request id back with the subscription notification payload
+			// therefore the code doesn't wait for the response to check whether the subscription is successful or not.


[nitpick] The handleSubscriptionNonTemplate and handleSubscriptionsNonTemplate functions duplicate a lot of logic found in handleSubscriptions. Consider unifying these into a single, parametrized subscription helper to reduce code duplication and simplify future updates.

Suggested change

// TODO: Remove this function when template expansion is across all assets

func (by *Bybit) handleSubscriptionNonTemplate(ctx context.Context, conn websocket.Connection, a asset.Item, operation string, channelsToSubscribe subscription.List) error {

payloads, err := by.handleSubscriptionsNonTemplate(conn, a, operation, channelsToSubscribe)

if err != nil {

return err

}

for _, payload := range payloads {

if a == asset.Options {

// The options connection does not send the subscription request id back with the subscription notification payload

// therefore the code doesn't wait for the response to check whether the subscription is successful or not.

// TODO: Refactored to use processSubscriptions helper function

func (by *Bybit) handleSubscriptionNonTemplate(ctx context.Context, conn websocket.Connection, a asset.Item, operation string, channelsToSubscribe subscription.List) error {

return by.processSubscriptions(ctx, conn, a, operation, channelsToSubscribe, false)

}

func (by *Bybit) processSubscriptions(ctx context.Context, conn websocket.Connection, a asset.Item, operation string, channelsToSubscribe subscription.List, useTemplate bool) error {

var payloads []SubscriptionPayload

var err error

if useTemplate {

payloads, err = by.handleSubscriptions(conn, a, operation, channelsToSubscribe)

} else {

payloads, err = by.handleSubscriptionsNonTemplate(conn, a, operation, channelsToSubscribe)

}

if err != nil {

return err

}

for _, payload := range payloads {

if a == asset.Options {

Going to be RM'd anyway

shazbert added 30 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

linter: fixes strikes again.

76524cc

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.

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

cya nils

32252b2

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

7c5d9c3

stream match update

2f93b64

update tests

dd94e4e

linter: fix

ca597f3

glorious: nits + handle context cancellations

2e0f0ae

stop ping handler routine leak

2d2f872

Allow rollback on connect on any error across all connections

f1c3895

fix shadow jutsu

09bff6c

glorious/gk: nitters - adds in ws mock server

e66c9be

linter: fix

03de669

Merge branch 'master' into gateio_ws

eddb58b

fix deadlock on connection as the previous channel had no reader and …

8159b05

…would hang connection reader for eternity.

Merge branch 'master' into gateio_ws

4f0b42f

Merge branch 'master' into stream_match

2af934e

glorious: whooops

0e3bb31

Ryan O'Hara-Reid added 11 commits March 12, 2025 14:22

linter: fix

a96e82c

Merge branch 'master' into ws_oubtound_req_futures

ef819c9

linter: fix

75b4601

Merge branch 'master' into ws_oubtound_req_futures

e6ba733

fix merge whoopsie

8359429

Merge branch 'master' into ws_oubtound_req_futures

fbc4493

Merge branch 'master' into ws_oubtound_req_futures

bbe1286

glorious: nits

d641938

Merge branch 'master' into ws_oubtound_req_futures

1ca33df

gk: nit

ec91b66

Merge branch 'ws_oubtound_req_futures' into bybit_multi_out

b577869

shazbert removed the rebase/merge of master required label Apr 2, 2025

shazbert added 2 commits April 14, 2025 10:27

Merge branch 'master' into bybit_multi_out

9f40855

linter: fix

025eb75

shazbert added review me This pull request is ready for review and removed nomerge requires dependency This pull request is dependent on another, so it can't be merged until the dependent one is merged labels Apr 14, 2025

gloriousCode requested changes Apr 29, 2025

View reviewed changes

github-actions bot added the stale label May 30, 2025

thrasher- requested review from gbjk, thrasher-, samuael, cranktakular and Copilot May 30, 2025 01:29

Copilot AI reviewed May 30, 2025

View reviewed changes

Ryan O'Hara-Reid added 3 commits May 30, 2025 12:55

glorious: nits

82e1117

Merge branch 'master' into bybit_multi_out

ae7d0f8

linter/misc: fix and remove meows

3e08a65

shazbert added medium priority and removed stale labels May 30, 2025

	func (by *Bybit) handleSubscriptionNonTemplate(ctx context.Context, conn websocket.Connection, a asset.Item, operation string, channelsToSubscribe subscription.List) error {
	func (by *Bybit) submitSubscriptionNonTemplate(ctx context.Context, conn websocket.Connection, a asset.Item, operation string, channelsToSubscribe subscription.List) error {

bybit: enable multiconnection handling across websocket endpoints #1670

Are you sure you want to change the base?

bybit: enable multiconnection handling across websocket endpoints #1670

Uh oh!

Conversation

shazbert commented Oct 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Description

Type of change

How has this been tested

Checklist

Uh oh!

gloriousCode left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gloriousCode Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented May 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI May 30, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 30, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shazbert commented Oct 9, 2024 •

edited

Loading

gloriousCode Apr 28, 2025 •

edited

Loading