[stream-core] Replacing getBalance endpoint to getWallet endpoint on StreamingAccountService
makarid opened this issue · 6 comments
Hello, i would like to ask you if it is ok to add a new getWallet() endpoint on StreamingAccountService for websocket connections which return all the balances from every currency. @badgerwithagun can you please tell me if you are ok with this? Thanks
I'm a bit uncomfortable with adding any endpoint that is unlikely to get widespread exchange support and where the data can already easily be obtained from an existing API.
It's easy to achieve with the existing API by:
- Iterating the available currencies using
Exchange.getExchangeMetaData() - Subscribing to all these currencies (using
ProductSubscriptionBuilderwhen connecting) - Opening all the streams and combining them, into the single stream you want. You could probably do this very easily with a single
Observablechain, like this:
return Observable.fromIterable(exchange.getExchangeMetaData().getCurrencies())
.concatMap(streamingAccountService::getBalanceChanges);
I've not checked the above. I have a funny feeling that concatMap won't actually work here and you need to use merge instead. Would need to have a play to be sure. Either way - I'm sure it's easy - this is what Reactive Streams are designed to do.
This approach has the advantage of working even if the exchange(s) you are calling don't support getting all currencies at once.
It sounds inefficient, but it shouldn't be; a "correct" StreamingAccountService implementation should subscribe to the full "wallet" anyway and then split out the individual balances changes as separate dependent streams.. As a result, the net result is still that you are subscribing to the full "wallet" on the websocket.
Thank a lot for your response. But why i have to do all these when i can just add a new endpoint which no one is obliged to use but everyone can implement if they want? In my project i want to get the Wallet.class and not Balance.class. So even if i use your suggestion i then must convert Balance to Wallet which as i remember i cannot do that with just calling toList() on the stream chain because the stream never ends. So if i just add a new endpoint to StreamingAccountService my trouble ends there. Also because of your recent PRs (thanks again for that) which you have convert the endpoints on core interfaces to use the default key, it is much easier to add endpoints without breaking anyone's implementation.
You are welcome to submit a PR to put a getWallet method on the exchanges where you want the support (a level 2 API). There are lots of examples of this in xchange-stream where individual exchanges support more features than the core API. This same design approach is taken by XChange. All you then need to do is cast to the specific exchange type(s) and call the method directly.
However, the core APIs on the interfaces (level 1 API) are intended to be a lowest-common-denominator subset of the features that almost all exchanges could support. The most important thing is interoperability. If using the L1 APIs you shouldn't have to worry about differences between exchanges. Yes, sometimes that means the support is less powerful than the exchange supports. If that's an issue, the caller can use a L2 API instead at the expense of losing that standardisation.
See OpenGL Vs Vulkan or DirectX 11 Vs 12.
I totally understand that. But as you said, L1 API must be the lowest-common-denominator and in this endpoint the lowest common denominator is a Wallet class instead of Balance. A Wallet.class can have many balances which is more abstract than Balance.class, it is one level higher hierarchical. I also saw the Binance implementation. In order to return a Balance you must first getAllBalances and then filter for a currency. As i see it, this isn't the lowest level.
But not Bitfinex or Poloniex. For example.
The lowest level is definitely individual balance updates, even if a lot of exchanges do provide wallet-level APIs.
OK, nevermind i will find a solution. Thanks for your help