[bitfinex] Added some methods of authenticated api #209

Mikelle · 2018-07-10T10:44:04Z

Added web socket support for getting orders, pre trades and trades, balaces updates

…icated websocket

badgerwithagun · 2018-07-16T08:19:37Z

Nice, @Mikelle! 👍

I'm quite excited by this development; my own application really struggles due to poll limits on exchanges for private authenticated data, so adding streaming support would make a big difference.

Two questions I think we need to answer:

It's a significant decision in principle to extend the scope of xchange-stream to cover authenticated streaming data, so we need @dozd's thoughts on this.
Your changes are very BFX focused, rather than introducing a generic API we can extend to cover other exchanges. That's fine, but I think we need to introduce a generic API so that we can introduce other exchanges later. Assuming @dozd is in favour in principle, would you be willing to work together to define a suitable API?

badgerwithagun · 2018-07-16T08:24:01Z

Hmmm, crossover with #160.

@bryantharris - your thought here?
@dozd - can we get #160 merged?

bryantharris · 2018-07-26T05:39:44Z

@badgerwithagun I think the right approach is a two layered approach the same as the REST XChange project that this streaming project is based on.

Basically, layer 0 is exchange specific supporting 1:1 streaming events as to what is supported on the exchange ws API.
Then a layer 1, which would represent generic support, which is implemented using layer 0. This would be the consistent API to the extent there is commonality shared across all streaming implementations.

You'd typically use the common API but could always drop to the lower level if there were specific event messages you wanted to follow.

With respect to authenticated streaming, I give a resounding thumbs up that it should be supported. To me the primary advantage of authenticated streaming is real time updates to order status changes without polling.

I get viewing tickers (non authenticated), but the real power seems to be in having the stream be authenticated and receiving private events IMHO.

Mikelle · 2018-08-06T15:46:00Z

@badgerwithagun @bryantharris So what should I do to make you accept the pull request? Develop new generic API for authenticated streaming?

badgerwithagun · 2018-08-13T23:53:48Z

@Mikelle: thanks for sticking with this. It would make sense to follow the same approach as @bryantharris has followed on #160 for trades: that is, if we are authenticated, then subscribe to the private trades stream and supply both Trades and UserTrades in the existing trades stream API.

However, when it comes to orders, we currently lack any generic API, so you will need to define a new one. I think the Orderbook stream API is the closest pattern to follow (maintain a snapshot of the user's order list in memory and keep that updated from the exchange stream). The level 0 API underlying that would be the actual stream of changes.

I don't have the power to approve change requests though; just helping out. There are others on here who are closer to the code who would have clearer ideas.

pchertalev · 2018-10-05T14:13:05Z

So we are waiting for approve pool request of #160 for using the same approach for Bitfinex, right?

badgerwithagun

This is a really nice-looking PR. Great work.

If it could just be adjusted to fit into the existing patterns for authentication and L1 APIs, I reckon it's good for a merge.

Do you have time to follow this up?

badgerwithagun · 2018-10-08T20:33:06Z

...ge-bitfinex/src/main/java/info/bitrich/xchangestream/bitfinex/BitfinexStreamingExchange.java

@@ -57,4 +59,28 @@ public StreamingMarketDataService getStreamingMarketDataService() {
    @Override
    public void useCompressedMessages(boolean compressedMessages) { streamingService.useCompressedMessages(compressedMessages); }

+    public Completable connectToAuthenticated() {


Is it possible to make this connection/authentication transparent, so it occurs automatically if authentication details are available, as we do for GDAX?

Done on my uplift fork

badgerwithagun · 2018-10-08T20:34:38Z

...ge-bitfinex/src/main/java/info/bitrich/xchangestream/bitfinex/BitfinexStreamingExchange.java


    public BitfinexStreamingExchange() {
        this.streamingService = new BitfinexStreamingService(API_URI);
+        this.streamingAuthenticatedDataService = new BitfinexStreamingRawService(API_URI);


Do we need to have a second connection to BFX for the authenticated stream, or is it possible to just authenticate the existing connection?

According to the BFX documentation, it very much looks like this is the case, so I'm going to try it.

badgerwithagun · 2018-10-08T20:37:49Z

...-bitfinex/src/main/java/info/bitrich/xchangestream/bitfinex/BitfinexStreamingRawService.java

+        }
+    }
+
+    public Observable<BitfinexWebSocketAuthOrder> getAuthenticatedOrders() {


There's an existing pattern for exposing L1 (exchange specific) methods. Example: BinanceStreamingMarketDataService.getRawTicker(). Could we move these to the StreamingMarketDataService? e.g. getRawAuthenticatedOrders()?

badgerwithagun · 2018-10-08T20:40:15Z

...-bitfinex/src/main/java/info/bitrich/xchangestream/bitfinex/BitfinexStreamingRawService.java

+        return subjectPreTrade.share();
+    }
+
+    public Observable<BitfinexWebSocketAuthTrade> getAuthenticatedTrades() {


I'd like it if we could do as we do for GDAX, and merge the authenticated trades (as UserTrade) with the public trades (as Trades) and return them in the getTrades() stream. However, just implementing the L1 API is valuable on its own, so if you don't fancy doing this, that's fine. We can raise an issue for it and see if someone else picks it up (I might).

badgerwithagun · 2018-10-08T20:41:44Z

...-bitfinex/src/main/java/info/bitrich/xchangestream/bitfinex/BitfinexStreamingRawService.java

@@ -0,0 +1,303 @@
+package info.bitrich.xchangestream.bitfinex;


I've not reviewed this code - I suspect it might move around quite a lot based on my other comments.

Will review properly once the structure is right.

badgerwithagun · 2018-10-08T20:49:58Z

...ge-bitfinex/src/test/java/info/bitrich/xchangestream/bitfinex/BitfinexManualAuthExample.java

+        exchange.setCredentials("API-KEY", "API-SECRET");
+
+        exchange.connectToAuthenticated().blockingAwait();
+        exchange.getStreamingAuthenticatedDataService().getAuthenticatedTrades().subscribe(


SO, to clarify my earlier comments, this would be more "standard" like this:

ExchangeSpecification specification = StreamingExchangeFactory.INSTANCE.createExchange(BitfinexStreamingExchange.class.getName()).getDefaultExchangeSpecification(); specification.setApiKey("API-KEY"); specification.setSecretKey("API-SECRET"); BitfinexStreamingExchange exchange = (BitfinexStreamingExchange)StreamingExchangeFactory.INSTANCE.createExchange(specification); exchange.getStreamingMarketDataService().getRawAuthenticatedTrades().subscribe( ... );

So, use the existing authentication details provided by the exchange specification, authenticate by default if the authentication details are available, and use L1 "raw" methods on the StreamingMarketDataService itself.

Done on my uplift fork

davidjirovec · 2018-11-05T09:32:38Z

Any plans on merging this?

Flemingjp · 2018-11-05T09:35:26Z

@davidjirocev If it's tested and reviewed, then most likely.

davidjirovec · 2018-11-05T10:15:40Z

I'm running this branch a few months :)

Flemingjp · 2018-11-06T11:09:10Z

@Mikelle any chance you could work on the comments @badgerwithagun made?

badgerwithagun · 2019-01-27T11:14:54Z

I'm going to fork this PR and bring it in line with the work I'm doing to commonise support for authenticated streams across multiple exchanges (see the thread on #246 for details).

Once #246 is merged I'll be able to resubmit.

badgerwithagun · 2019-01-27T11:26:28Z

OK, the uplift of this PR to resolve conflicts, answer the review comments and bring it in line with other exchanges is now underway and can be tracked on https://github.com/badgerwithagun/xchange-stream/tree/bitfinex-authenticated.

badgerwithagun · 2019-01-27T11:49:54Z

@Mikelle, is there a reason why you used a separate NettyStreamingService for the authenticated stream, rather than just sending an AUTH on the existing stream?

badgerwithagun · 2019-01-27T14:57:17Z

OK, I have created my modified version as #273. This covers all my review issues.

I guess this one can be closed if everyone's happy with the modifications.

badgerwithagun · 2019-01-28T14:57:32Z

Closing this in the absence of any complaints. Can re-open if necessary.

Please head over to #273.

Mikelle and others added 4 commits June 28, 2018 19:08

[bitfinex] Added authenticated channels orders and trades for authent…

e9cac4f

…icated websocket

[bitfinex] added getters for fee and feeCurrency

50d3629

[bitfinex] added getters for BitfinexWebSocketAuthOrder

4d5422b

[bitfinex] added method for getting balance

3cbb337

[bitfinex] fixed wrong objects parsing

88ad616

Mikelle changed the title ~~[bitfinex] Added some method of authenticated api~~ [bitfinex] Added some methods of authenticated api Aug 15, 2018

Mikelle and others added 3 commits August 24, 2018 18:19

[bitfinex] fixed wrong authentication mechanism

e0b4d0e

Defect: Bitstamp orderbook timestamp is not populated - FIXED

7845b2e

Merge branch 'develop' into develop

bbcea8d

badgerwithagun suggested changes Oct 8, 2018

View reviewed changes

Flemingjp added the enhancement label Oct 8, 2018

badgerwithagun mentioned this pull request Oct 9, 2018

[HitBTC] Is there a way for me to know whether my trades get executed ? #229

Closed

badgerwithagun mentioned this pull request Jan 27, 2019

[bitfinex] Added some methods of authenticated api (version 2) #273

Merged

badgerwithagun mentioned this pull request Jan 27, 2019

[WORK IN PROGRESS] Generic authenticated stream API #274

Closed

8 tasks

badgerwithagun closed this Jan 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[bitfinex] Added some methods of authenticated api #209

[bitfinex] Added some methods of authenticated api #209

Mikelle commented Jul 10, 2018

badgerwithagun commented Jul 16, 2018

badgerwithagun commented Jul 16, 2018

bryantharris commented Jul 26, 2018

Mikelle commented Aug 6, 2018

badgerwithagun commented Aug 13, 2018

pchertalev commented Oct 5, 2018

badgerwithagun left a comment

badgerwithagun Oct 8, 2018

badgerwithagun Jan 27, 2019

badgerwithagun Oct 8, 2018

badgerwithagun Jan 27, 2019

badgerwithagun Oct 8, 2018

badgerwithagun Oct 8, 2018

badgerwithagun Oct 8, 2018

badgerwithagun Oct 8, 2018

badgerwithagun Jan 27, 2019

davidjirovec commented Nov 5, 2018

Flemingjp commented Nov 5, 2018

davidjirovec commented Nov 5, 2018

Flemingjp commented Nov 6, 2018

badgerwithagun commented Jan 27, 2019

badgerwithagun commented Jan 27, 2019

badgerwithagun commented Jan 27, 2019

badgerwithagun commented Jan 27, 2019

badgerwithagun commented Jan 28, 2019

		@@ -0,0 +1,303 @@
		package info.bitrich.xchangestream.bitfinex;

[bitfinex] Added some methods of authenticated api #209

[bitfinex] Added some methods of authenticated api #209

Conversation

Mikelle commented Jul 10, 2018

badgerwithagun commented Jul 16, 2018

badgerwithagun commented Jul 16, 2018

bryantharris commented Jul 26, 2018

Mikelle commented Aug 6, 2018

badgerwithagun commented Aug 13, 2018

pchertalev commented Oct 5, 2018

badgerwithagun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidjirovec commented Nov 5, 2018

Flemingjp commented Nov 5, 2018

davidjirovec commented Nov 5, 2018

Flemingjp commented Nov 6, 2018

badgerwithagun commented Jan 27, 2019

badgerwithagun commented Jan 27, 2019

badgerwithagun commented Jan 27, 2019

badgerwithagun commented Jan 27, 2019

badgerwithagun commented Jan 28, 2019