dougdellolio / coinbasepro-csharp

The unofficial .NET/C# client library for the Coinbase Pro/GDAX API
MIT License
193 stars 90 forks source link

[Feature] Ability to add and remove subscriptions on the fly #129

Closed BradForsythe closed 5 years ago

BradForsythe commented 6 years ago

It's useful on occasion to add/remove a subscription on the fly without having to close and reopen the socket with a new subscription list. Example: let's say you are subscribing to BTC-USD and LTC-USD. If your BTC book is out of date or Gdax drops on of the product subscriptions (it happens...), it would be good to be able to "remove" and "add" just the one subscription. Especially if you have 7 level 2 product feeds. Some of the books are HUGE!

Having the Start/Stop on the socket with the sub list passed in is simple and clean so I propose to leave that and add in a few methods : add and remove to the socket.

@dougdellolio what do you think? If everyone seems ok with that, I'll do a PR in the next week or so.

dougdellolio commented 6 years ago

@BradForsythe cant believe gdax drops product subscriptions randomly........ would this just give the user the ability to remove and add subscriptions manually or can we also build it to do this automatically (giving the user the option)? it would also check if the book is out dated automatically and re-subscribe

sounds good to me!

BradForsythe commented 6 years ago

@dougdellolio That's a good idea to auto re-subscribe (as an option). I currently do this outside of GDAXSharp, since I am watching for feed loss and the status of my L2 mini book . We can monitor feed status inside of GDAXSharp and auto re-subscribe, that could be cool, but there isn't an L2book inside of GDAXSharp (right?) ? An L2book might be a nice feature addition for a later update.

Yes, I have been running for hours and all of a sudden we stop getting one of the feeds!!! And sometimes, we get a partial snapshot or no snapshot for a product on request. Could be network related or something other than GDAX.

I have also had "service unavailable" for order entry during high volume periods, that is GDAX. So I have external logic dealing with the various order failures. But that all might be app specific and a different subject anyway.

dougdellolio commented 6 years ago

you can view a snapshot of L2 orderbook by subscribing to Level2 channel type and the OnSnapShotReceived eventhandler. or are you referring to something else when you say it being inside this library?

the order failure bit sounds like it could be app specific

BradForsythe commented 6 years ago

Right, subscribe to L2 channel, get the snapshot (which for BTC maybe 30k+ bids / 30k+ asks), then consume the L2updates and apply to your own product level book (order book). Keeping the full book involves searching your book to know where to apply the L2update. This burns a lot of clock cycles (even with clever searching) and then times it by the number of products you have. Alternatively, you can keep a smaller mini book and resync on certain conditions. This gives you more time to spend on your algo and get the order out the door,

Either way, could be interesting to add an OrderBookManager to keep the product level order book (order book) based on the subscription feed. This orderbook manager would have methods to get best bid/ask price/size, level search, aggregation of sizes, etc. A topic for another feature...

BradForsythe commented 6 years ago

Still planning on getting back to this....in the middle of some other things for a week or so...

alexhiggins732 commented 6 years ago

This should be pretty easy to implement. The official node client shows it is a matter of sending an unsubscribe and subscribe request. https://github.com/coinbase/gdax-node#user-content-websocket-client. I will look into making a pull request for this change.

websocket.unsubscribe({ channels: ['full'] });

websocket.subscribe({ product_ids: ['LTC-USD'], channels: ['ticker', 'user'] });

alexhiggins732 commented 6 years ago

Pull request submitted. https://github.com/dougdellolio/coinbasepro-csharp/pull/146

alexhiggins732 commented 5 years ago

I believe this can be closed. #146 has been merged.

dougdellolio commented 5 years ago

👍