ccxws icon indicating copy to clipboard operation
ccxws copied to clipboard

Refactor to make functionality changes easier

Open bmancini55 opened this issue 4 years ago • 8 comments

It is a non-trivial task to understand what needs to be done to implement a new exchange or to modify an existing exchange for additional functionality.

To name a few of the variables that add complexity:

  • number of subscriptions per socket allowed (one, X, unlimited) -> this results in different base classes: BasicClient (single socket) and BasicMultiClient (many sockets) or some hybrid where a new socket is added every 20 subscriptions (we're looking at you Bibox). Sometimes they are limited to a single market but you can subscribe to multiple feeds for a single market from the socket.
  • room vs single data point subscriptions -> Poloinex and Gemini use a "room" approach that gives trades, candles, order book, etc for a single market from a single subscription. Most other exchanges have a single subscription per type of data
  • sub/unsub message vs connection URL -> Binance has the distinct pleasure of using a URL to provide data so you need to restart the connection via a new URL when you want to sub/unsub. Most other exchanges allow you to send a sub/unsub message.
  • number of subscriptions per time period (throttling via semaphore is used) -> some exchange allow you to make 400 subscriptions, but limit the number you are allowed to make in a given 10s or 1m. This requires customized throttling.
  • socket technologies (raw ws, signalr, socket.io, pusher, etc)
  • message compression (uncompressed, deflate, deflate raw, gzip)
  • authentication -> currently there is a different base client for handling exchanges that use authentication
  • L2 orderbook snapshots via REST -> some exchange get the L2 orderbook snapshot via REST requests
  • Ping/Pong logic

A number of exchanges, such as Coinbase and Bitfinex are fairly simple. Other exchanges such as Binance, Gemini, and Poloniex are completely one off. Others such as Bibox are a s***show because they only allow 20 markets per socket.

Some work has already been done to make things more uniform. The creation of a uniform testing system in #86 helped ensure there is consistency across clients with similar functionality.

The first step of this issue is going to be to document the peculiarities and commonalities of each exchange supported by this library. This will help determine what needs to be done to remove one-off solutions in each library and provide some consistency (if possible).

bmancini55 avatar Mar 26 '20 21:03 bmancini55

What are you thinking for a migration approach? Refactoring all exchanges at once could require a ton of work so perhaps it might be appropriate to create a new v2 branch that doesn't guarantee to support every exchange out of the gate, otherwise you could migrate one exchange at a time but this might make the codebase even more confusing in the interim as you'd have some exchanges on the old paradigm and some on the new paradigm.

I personally am more invested in #148 than in refactoring the entire codebase, so anything that could allow me to contribute to private feeds for the few exchanges I'm using (6 atm) ASAP would be my vote.

evan-coygo avatar Mar 26 '20 23:03 evan-coygo

I think it would be a piecemeal approach where exchanges are migrated to the new pattern. The challenge with that is that you can end up with another emergent design. That's why I think a good starting point is collecting information on exchange capabilities. That part is also required for #148, so there is some overlap in that information collection.

I'm not sure a refactor would result in a cleaner code base, but it's worth documenting the functionality and brainstorming on designs.

bmancini55 avatar Apr 02 '20 15:04 bmancini55

Hello,

I'm working in a solution that creates subclients transparently for exchanges that are limited by subscriptions per socket. This aims to solve Kucoin and Bibox subscription limit problem.

I would to know if anyone is working on this. Anyone can tell me?

fbadiola avatar Aug 21 '20 13:08 fbadiola

Hi @fbadiola, thanks for asking. I've been updating notes in the wiki as I'm fixing issues (https://github.com/altangent/ccxws/wiki/Refactor-Research). It's a bit of a mess that needs to be organized and I need to do a brain-dump of my plans.

The biggest bang for the buck refactor is going to around obtaining a socket for a subscription. Right now that's a mixed bag on how it works across various exchanges. I'm thinking this process should be a black-box async process specific to an exchange's requirements. Abstracting this part of the process away allows for single sockets, multi sockets, authorization, REST token initialization, limits per socket, sockets per market, etc.

Once a socket is obtained it is paired with the subscription command (type, market, arguments) for the duration of the subscription lifetime. This can be used to handle socket events by treating the subscription command and socket as first class arguments in the various handlers.

That's the theory at least. At this point most exchanges are using BasicClient or MultiClient. The major work will be creating a new BasicClient type that supports this functionality and migrating existing clients to it. This should ultimately make the library much easier to extend.

bmancini55 avatar Aug 21 '20 15:08 bmancini55

Yes, I'm agree but I'm thinking in other approach that maybe allows iterate this refactor without impact in other clients at moment. I would like you tell me ur opinion about it:

My idea is create a Clients that enhances other clients. e.g.

LoadBalanceClient.create((clientOptions) => new KuCoinClient(clientOptions), { maxSubscriptions: 100 })

LoadBalanceClient accepts different strategies to balance it (FillHoles, RoundRobin...). It's a client wrapper that fulfills the interface of any other client but limits the maximum subscriptions per client.

In the same way, we could made other implementation like e.g. RateLimitSubscriptionClient that fills with differents strategies like LeakyBucket, TokenBucket...

All of them we can be composed:

const KucoinBalancedClient = LoadBalanceClient.create((clientOptions) => new KuCoinClient(clientOptions), { maxSubscriptions: 100 })

const KucoinBalancedAndRateLimitedClient = RateLimitClient.create((clientOptions) => new KucoinBalancedClient(clientOptions), { strategy: (params) => new LeakyBucketStrategy(params) });

IMHO, this creates a composable architecture to enhance clients without affect their implementation and allow reuse more code.

How about?

fbadiola avatar Aug 21 '20 15:08 fbadiola

Cool! Thanks so much for sharing your ideas! I certainly agree that we need to move complexity towards composition, there are a couple of ways we should be able to do that, so it's awesome to discuss and explore.

If you haven't already, take a look at BasicMultiClient. It is essentially a decorator over BasicClient that adds functionality to split subscriptions into different sockets. It was an early attempt to solve this problem, but I don't believe using a decorator is the right tool for the client level. IMO the number of variables affecting various sub-systems of the client have the potential to create a large amount of combinatorial complexity and potential for cross-talk.

In patterns terms, what I am proposing is treating the new base client as a facade. That is we use a simple interface that abstracts complex sub-systems.

One of those subsystems would be obtaining a socket. The rest of the code inside the facade doesn't care about the rules used to obtain a socket, only that when it's needed a socket can be retrieved.

IMO what you're proposing is perfect for composing functionality of the subsystems themselves. For example Kucoin uses a multisocket strategy with a limiter.

Sorry I can't be more concrete with code. I'm typing from my phone due to an unfortunate water on laptop issue this morning haha.

bmancini55 avatar Aug 21 '20 16:08 bmancini55

Yep, I think we're absolutely align. My decorators proposal is on top of clients (subsystems) and it doesn't affect other under the hood changes.

Obviously, BasicClient responsability should be splitted into smaller parts. Facade is a good approach for this. If u wish, i can help u with these tasks.

BTW, my objetive only affect to enhance clients without update them. If u want, I send a PR as soon as I will have got decorators approach that doesnt affect actual codebase and we can iterate about this when BasicClient refactor done.

Sorry I can't be more concrete with code. I'm typing from my phone due to an unfortunate water on laptop issue this morning haha.

No problem! Ouch!!! Water and laptops are no good idea... hahaha

P.S:

I certainly agree that we need to move complexity towards composition, there are a couple of ways we should be able to do that, so it's awesome to discuss and explore.

Yeah, there're a lot of ways. I like discuss about architecture :rofl:

fbadiola avatar Aug 21 '20 17:08 fbadiola

Thanks for the PR, will take a look once I'm I'm back up and running.

bmancini55 avatar Aug 22 '20 20:08 bmancini55