bitrich-info / xchange-stream

XChange-stream is a Java library providing a simple and consistent streaming API for interacting with Bitcoin and other crypto currency exchanges via WebSocket protocol. It is build on top of of XChange library providing new interfaces for streaming API. User can subscribe for live updates via reactive streams of RxJava library.
Apache License 2.0
413 stars 222 forks source link

[Binance] implement streaming trades #120

Open daniel-cohen opened 6 years ago

daniel-cohen commented 6 years ago

API reference: https://github.com/binance-exchange/binance-official-api-docs/blob/master/web-socket-streams.md#aggregate-trade-streams

I think we can get use the aggregated steam as we're not tracking individual trades anyway.

aleks-azen commented 6 years ago

I think individual trades would be useful as well, if you want to track order completion without polling the api

daniel-cohen commented 6 years ago

@aleks-azen OK, I see what your use-case is now. Would you be open to maybe add a switch to BinanceStreamingExchange.connect to indicate whether you want the aggregated trades or single trades to come through: Observable<Trade> getTrades(CurrencyPair currencyPair, Object... args);

I'm just trying to think how we can expose both the aggTrades and single trades..

aleks-azen commented 6 years ago

My primary experience is with the gdax and binance apis. The gdax api streams trades on an order by order basis. I assume that is likely the case for most exchanges but correct me if I'm wrong. To remain consistent I would suggest adding an aditional field to the StreamingMarketDataService interface with something like the following

   default Observable<Trade> getAggregatedTrades(CurrencyPair var1, Object... var2){
        throw new NotYetImplementedForExchangeException();

The default modifier would save you the hassle of implementing the function for every class but if other exchanges support it, it can be overridden. Also just add a new list of subscription types to the ProductSubscription class. If an AggTrade is different enough from a Trade for your needs it might be worth it to make a seperate AggTrade class with different fields

daniel-cohen commented 6 years ago

@aleks-azen I think you're right and we'll need to implement both. Bitstamp seems to send individual trades too. Bitfinex doesn't send any order ids (or any ids), so it could be aggregated.) see: https://bitfinex.readme.io/v2/reference#ws-public-trades

I wasn't sure how open the owner(s) of this project are to these sort of changes (changes to the xchange-stream-core interfaces/class), so I tried to keep them to a minimum.

If I am given the OK I'm glad to implement the following:

   default Observable<Trade> getAggregatedTrades(CurrencyPair var1, Object... var2){
        throw new NotYetImplementedForExchangeException();
aleks-azen commented 6 years ago

I've made a core interface change to the base xchage-repo, as long as it makes sense it should be approved. In this case I believe it does. This repo is only a year old and there's still plenty of expansion the interfaces will need

Request for reference: https://github.com/timmolter/XChange/pull/2361/files