semashkinvg / Bitmex.NET

Wrapper for BitMEX.com REST & WebSocket API
MIT License
52 stars 26 forks source link

Support multiple accounts on REST interface #20

Closed tekr closed 6 years ago

tekr commented 6 years ago

The current implementation requires separate BitmexApiService instances for each account, as account details (in BitmexAuthorization) are passed in at construction time. This in turn means separate TCP connections for each account.

Providing an alternate interface where account details are passed with a request would allow any number of accounts to share a single connection.

semashkinvg commented 6 years ago

@tekr good point! If you provide a very elegant way to do that, I will accept your pull request. but I think you can handle it without any changes. When I designed the structure I expected handling this case by registration the components with the following way. This example uses UnityContainer but you can implement absolutely the same behaviour using any other DI framework.

           // authorization1 - this instance contains apikey1 and secret1
            Container.RegisterInstance<IBitmexAuthorization>("AuthorizationAccount1", authorization1);
            Container.RegisterType<IBitmexApiProxy, BitmexApiProxy>($"BitmexApiProxyAccount1",
                new InjectionConstructor(new ResolvedParameter<IBitmexAuthorization>($"AuthorizationAccount1")));
            Container.RegisterType<IBitmexApiService, BitmexApiService>($"BitmexApiServiceAccount1",
                new InjectionConstructor(new ResolvedParameter<IBitmexApiProxy>($"BitmexApiProxyAccount1")));

           // authorization2 - uses another credencials
            Container.RegisterInstance<IBitmexAuthorization>("AuthorizationAccount2", authorization2);
            Container.RegisterType<IBitmexApiProxy, BitmexApiProxy>($"BitmexApiProxyAccount2",
                new InjectionConstructor(new ResolvedParameter<IBitmexAuthorization>($"AuthorizationAccount2")));
            Container.RegisterType<IBitmexApiService, BitmexApiService>($"BitmexApiServiceAccount2",
                new InjectionConstructor(new ResolvedParameter<IBitmexApiProxy>($"BitmexApiProxyAccount2")));

let me know if it covers you case

tekr commented 6 years ago

@semashkinvg The current implementation requires each account to have its own TCP connection. I.e. 10 accounts = 10 TCP connections. However it's fairly straightforward to support multiple accounts on one connection given that account details are specified on every REST request. There are at least a couple of ways to do it:

1) Make the IBitmexAuthorization parameter optional in the BitmexApiService constructor, and add an optional parameter for it to IBitmexApiService.Execute(). It can then be specified in the constructor (as before) for single-account usage, or alternatively in the Execute() calls for multi-account usage.

2) Add [I]BitmexMultiAccountApiService, similar to the existing [I]BitmexApiService except that IBitmexAuthorization is moved from the constructor to the Execute() method.

The first approach has the advantage of less code/duplication, but would need to rely on run-time checks to ensure an IBitmexAuthorization has been specified in either the constructor or Execute() call.

The second approach allows compile-time checking, but at the cost of some code duplication.

Do you have a preference? I'm happy to go with either approach.

tekr commented 6 years ago

NB. Commits 51f0996 and 7c99c8f referenced above are implementations of options 1 and 2 respectively. Let me know if you're happy with one of them and I'll raise a PR.

semashkinvg commented 6 years ago

@tekr wow thanks, I will have a look tonight

tekr commented 6 years ago

So.. not keen on adopting either approach?

In my current production setup I'm using 7c99c8f (i.e. a build with [I]BitmexMultiAccountApiService added). Happy to keep maintaining that fork if need be as it's not much extra code. I just figured some others may find it useful.