Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEATURE: [bitget] support book stream on bitget #1344

Merged
merged 1 commit into from Oct 20, 2023

Conversation

bailantaotao
Copy link
Collaborator

  1. The symbol is upper case, e.q. ETHUSDT
  2. The books channel might return less than 200 bids/asks as per symbol's orderbook various from each other; The number of bids/asks is not a fixed value and may vary in the future.
  3. The supported channels are books, books5, and books15.

Reference

https://bitgetlimited.github.io/apidoc/en/spot/#depth-channel

@bbgokarma-bot
Copy link

Welcome back! @bailantaotao, This pull request may get 869 BBG.

@bailantaotao bailantaotao force-pushed the edwin/bitget/public-stream-book branch from 556329f to f7dcefd Compare October 18, 2023 06:31
@bbgokarma-bot
Copy link

Re-estimated karma: this pull request may get 861 BBG

@codecov
Copy link

codecov bot commented Oct 18, 2023

Codecov Report

Merging #1344 (51d86ca) into main (d50e509) will increase coverage by 0.10%.
Report is 23 commits behind head on main.
The diff coverage is 28.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1344      +/-   ##
==========================================
+ Coverage   20.70%   20.81%   +0.10%     
==========================================
  Files         560      565       +5     
  Lines       40025    40265     +240     
==========================================
+ Hits         8287     8380      +93     
- Misses      31136    31276     +140     
- Partials      602      609       +7     
Files Coverage Δ
pkg/types/stream.go 0.00% <ø> (ø)
pkg/exchange/bitget/stream_callbacks.go 0.00% <0.00%> (ø)
pkg/exchange/bitget/types.go 30.76% <30.76%> (ø)
pkg/exchange/bitget/stream.go 29.46% <29.46%> (ø)

... and 6 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d50e509...51d86ca. Read the comment docs.

pkg/exchange/bitget/stream.go Outdated Show resolved Hide resolved

func (s *Stream) handleBooksEvent(o BookEvent) {
for _, book := range o.OrderBooks() {
switch {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

switch o.Type ?

pkg/exchange/bitget/stream.go Outdated Show resolved Hide resolved
pkg/exchange/bitget/stream.go Outdated Show resolved Hide resolved
Arg WebsocketArg `json:"arg"`
// "op" and "channel" are exclusive.
*WsOpEvent
*WsChannelEvent
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

embed multiple struct like this may be dangerous?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that is fine because we have a sanity check before calling the pointer struct.

func (w *WsEvent) IsOp() bool {
	return w.WsOpEvent != nil && w.WsChannelEvent == nil
}

func (w *WsEvent) IsChannel() bool {
	return w.WsOpEvent == nil && w.WsChannelEvent != nil
}

This event is only used in parseWebSocketEvent. If it does not belong to any type, it will return an error.

func (s *Stream) parseWebSocketEvent(in []byte) (interface{}, error) {
	var e WsEvent

	err := json.Unmarshal(in, &e)
	if err != nil {
		return nil, err
	}

	switch {
	case e.IsOp():
		return e.WsOpEvent, nil

	case e.IsChannel():
		switch e.Arg.Channel {
		case ChannelOrderBook, ChannelOrderBook5, ChannelOrderBook15:
			...
			return &books, nil
		}
	}

	return nil, fmt.Errorf("unhandled websocket event: %+v", string(in))
}


// internal use
Type ActionType
instId string
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this field public?

case types.DepthLevel15:
w.Channel = ChannelOrderBook15
case types.DepthLevel200:
log.Info("*** The subscription events for the order book may return fewer than 200 bids/asks at a depth of 200. ***")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to add the exchange name "bitget" in this log.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The log already with the filed

var log = logrus.WithFields(logrus.Fields{
"exchange": ID,
})

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好酷喔,現在可以顯示code了?

var books BookEvent
err = json.Unmarshal(e.WsChannelEvent.Data, &books.Events)
if err != nil {
return nil, fmt.Errorf("failed to unmarshal data into BookEvent: %+v, err: %w", string(e.WsChannelEvent.Data), err)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we log e.Arg.Channel as well? This would allow us to immediately identify which channel is problematic upon encountering the error.

@bailantaotao bailantaotao force-pushed the edwin/bitget/public-stream-book branch from f7dcefd to 3af0238 Compare October 18, 2023 08:57
@bbgokarma-bot
Copy link

Re-estimated karma: this pull request may get 980 BBG

@bailantaotao bailantaotao force-pushed the edwin/bitget/public-stream-book branch from 3af0238 to 4e4e529 Compare October 18, 2023 09:47
@bbgokarma-bot
Copy link

Re-estimated karma: this pull request may get 985 BBG

for _, subscription := range s.Subscriptions {
arg, err := convertSubscription(subscription)
if err != nil {
logger.WithError(err).Errorf("convert error, subscription: %+v", subscription)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can simply return the error here I think

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, it cannot be. The reason is that the syncSubscriptions function is invoked by both handleConnect and Unsubscribe. However, there is no error returned to the caller to inform them that an error has occurred.

handleConnect

stream.OnConnect(stream.handlerConnect)

func (s *Stream) handlerConnect() {
	if s.PublicOnly {
		// errors are handled in the syncSubscriptions, so they are skipped here.
		_ = s.syncSubscriptions(WsEventSubscribe)
	} else {
		log.Error("*** PRIVATE API NOT IMPLEMENTED ***")
	}
}

Unsubscribe

func (s *Stream) Unsubscribe() {
	// errors are handled in the syncSubscriptions, so they are skipped here.
	_ = s.syncSubscriptions(WsEventUnsubscribe)
	s.Resubscribe(func(old []types.Subscription) (new []types.Subscription, err error) {
		// clear the subscriptions
		return []types.Subscription{}, nil
	})
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 got it, thanks

return fmt.Errorf("unexpected subscription type: %v", opType)
}

logger := log.WithField("opType", opType)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opType log tag might not be so helpful?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The op type represents a subscription or unsubscription. If an error occurs, we can identify the type that is causing the problem and address it.

@bailantaotao bailantaotao force-pushed the edwin/bitget/public-stream-book branch from 4e4e529 to 843f683 Compare October 19, 2023 03:16
@bbgokarma-bot
Copy link

Re-estimated karma: this pull request may get 1040 BBG

@bbgokarma-bot
Copy link

Re-estimated karma: this pull request may get 1049 BBG

@bailantaotao bailantaotao force-pushed the edwin/bitget/public-stream-book branch from 6e48749 to c4f651a Compare October 19, 2023 05:41
@bbgokarma-bot
Copy link

Re-estimated karma: this pull request may get 1054 BBG

@bailantaotao bailantaotao force-pushed the edwin/bitget/public-stream-book branch from c4f651a to f17cf34 Compare October 19, 2023 06:03
@bbgokarma-bot
Copy link

Re-estimated karma: this pull request may get 1059 BBG

@bailantaotao bailantaotao force-pushed the edwin/bitget/public-stream-book branch from f17cf34 to 51d86ca Compare October 19, 2023 07:40
@bbgokarma-bot
Copy link

Re-estimated karma: this pull request may get 1072 BBG

@bailantaotao bailantaotao merged commit f8c47f7 into c9s:main Oct 20, 2023
4 checks passed
@bailantaotao bailantaotao deleted the edwin/bitget/public-stream-book branch October 20, 2023 06:22
@bbgokarma-bot
Copy link

Hi @bailantaotao,

Well done! 1077 BBG has been sent to your polygon wallet. Please check the following tx:

https://polygonscan.com/tx/0xb4220e34afec278399f53a49ef0f8e72deb548a98aac82c55f2b94d870e4c3e1

Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants