Gdax adding user channel #160

bryantharris · 2018-04-06T17:22:58Z

I noticed gdax only was accessing public information (tickers, trades) and wasn't in anyway retrieving user information (such as fills or order changes).

Receiving real time fill information is one of the most compelling reasons to use streaming.

The changes I've made are 100% backwards compatible. However, if you include user credentials when connecting to the stream, the stream will now connect to the 'user' channel and will receive fill information.

This means that some Trade object will now be UserTrade objects, which includes info tying the trade back to the original Order. UserTrade extends Trade so this is backwards compatible, however now you can detect these UserTrade objects and use the information to track when trades execute.

Also, the implementation approach of the streaming for GDAX (Possibly others haven't looked) doesn't mirror the 2 level implementation approach used in the XChange project.

In XChange There's a level 1 API which 100% mirrors the raw exposed API of the exchange. Then there's a level 2 API, which is implemented in terms of calls to the Level 1 API to expose a common/universal interface.

I think xchange-stream could benefit from a similar implementation approach. GDAX supports events for live updates to a user's Orders as they progress through to actually trading. If there's no generic call back for receiving Order changes, it would be nice for all GDAX low level events to have raw callbacks to allow you to connect in to all the capabilities of the raw API, which may not have an equivalent at the generic level.

The current implementation just supports a level 2 (generic) API directly off the messages coming off the websocket.

If there's interest, I can start morphing the implementation to be a two level approach, exposing 100% of GDAXs streaming API in a Level 1 (Raw) API, and maintain the existing Level 2 (generic) API.

I think overall, this change applied to all the subproject would improve the flexibility and usability of the API.

… from commit 74f5c40)

… from commit 74f5c40) (reverted from commit 50a77e6)

derDeidra · 2018-05-06T23:20:58Z

+1, would like to see gdax fill support

dozd · 2018-05-10T13:16:29Z

xchange-gdax/src/main/java/info/bitrich/xchangestream/gdax/GDAXStreamingService.java


    private WebSocketClientHandler.WebSocketMessageHandler channelInactiveHandler = null;

-    public GDAXStreamingService(String apiUrl) {
+    public GDAXStreamingService(String apiUrl, GDAXStreamingExchange exchange) {


I am not a big fan of cyclic class dependencies like this. There should contract "parent" <- "child" in this scenario. So if StreamingService needs something in particular, give it to it. But don't give it main object that holds everything - that leads to spaghetti.

dozd · 2018-05-10T13:16:48Z

xchange-gdax/src/main/java/info/bitrich/xchangestream/gdax/GDAXStreamingService.java

@@ -25,11 +30,13 @@
    private static final String SHARE_CHANNEL_NAME = "ALL";
    private final Map<String, Observable<JsonNode>> subscriptions = new HashMap<>();
    private ProductSubscription product = null;
+    GDAXStreamingExchange exchange;


Use private final

dozd · 2018-05-10T13:26:14Z

xchange-gdax/src/main/java/info/bitrich/xchangestream/gdax/GDAXStreamingService.java

+        GDAXWebsocketAuthData authData = null;
+        if ( exchangeSpec.getApiKey() != null ) {
+            GDAXAccountServiceRaw rawAccountService = (GDAXAccountServiceRaw) exchange.getAccountService();
+            authData = rawAccountService.getWebsocketAuthData();


I would prefer not to include getWebsocketAuthData call into this particual method as well xchange-stream. User can call this by her own. Just let the user to pass websocet auth data to the xchange-stream (probably in connect() method?). By this you will have easier time with solving cyclic dependency issue as well as more flexible usage of the library.

dozd · 2018-05-10T13:29:51Z

Hello, just two minor findings with the PR. Thanks for that. I think that two level approach is the better way to go as you proposed :)

bryantharris · 2018-05-16T14:10:37Z

Okay, I'll take a look, sorry I got the sense you weren't going to look at the pull request so I stopped checking so frequently. I'll make the proposed changes and then perhaps will consider the two layer approach that the base project XChange uses. If you like it, shouldn't be too much work to one by one refactor the other projects.

It gives the maximum flexibility of a simple generic API as well as 100% exchange support.

…nnect() call.

bryantharris · 2018-05-16T20:44:33Z

Changes made based off of feedback.

bryantharris

Changes made.

bryantharris

These changes have been made for several months now.

badgerwithagun · 2018-08-13T20:40:04Z

develop merges into this branch fine, but I don't have permission to do so.

@dozd, can you please confirm that the changes @bryantharris made match what you were expecting? The changes look OK from here.

I'm just about to run some basic testing and if it looks good, I think should be merged.

badgerwithagun · 2018-09-17T16:57:03Z

xchange-gdax/src/main/java/info/bitrich/xchangestream/gdax/GDAXStreamingService.java

        ObjectMapper objectMapper = new ObjectMapper();
        return objectMapper.writeValueAsString(subscribeMessage);
    }

    @Override
    public String getUnsubscribeMessage(String channelName) throws IOException {
        GDAXWebSocketSubscriptionMessage subscribeMessage =
-                new GDAXWebSocketSubscriptionMessage(UNSUBSCRIBE, new String[]{"level2", "matches", "ticker"});
+                new GDAXWebSocketSubscriptionMessage(UNSUBSCRIBE, new String[]{"level2", "matches", "ticker"}, authData);


Should this unsubscribe message also include "user" if authData != null? Not seen any side-effects myself in use but it seems symmetrical.

badgerwithagun · 2018-09-17T17:00:23Z

xchange-gdax/src/main/java/info/bitrich/xchangestream/gdax/GDAXStreamingMarketDataService.java

-                            Trades adaptedTrades = adaptTrades(new GDAXTrade[]{s.toGDAXTrade()}, currencyPair);
+                            Trades adaptedTrades = null;
+                            if ( s.getUserId() != null )
+                                adaptedTrades = adaptTradeHistory(new GDAXFill[]{s.toGDAXFill()});


I'm working on something similar for Binance right now, and this bit gave me pause. Are you getting duplicated trades appearing where you get both the Trade from the matches channel and the UserTrade from the user channel?

I certainly do with Binance and it itched. The only way I could think of to resolve the issue looks something like this:

// To avoid duplication of user and public trades, we delay the public trades by one second // and skip any trade ids which have recently been streamed as user trades. This isn't // perfect, of course... Cache<String, ? super Trade> recentTrades = CacheBuilder.newBuilder().maximumSize(50).build(); return publicTrades .concatMap(t -> Observable.just(t).delay(1, TimeUnit.SECONDS)) .skipWhile(t -> recentTrades.asMap().containsKey(t.getId())) .mergeWith( getUserTrades() .filter(t -> t.getCurrencyPair().equals(currencyPair)) .doOnNext(t -> recentTrades.put(t.getId(), t)) );

I think the code replaces the Trade object with a UserTrade object so there wouldn't be a duplicate.

Honestly in my usage (external to this project) I'm just looking for the fills coming in (UserTrade) so it never hurt my bot if there was or wasn't a Trade reported via the regular trade feed.

I suppose if the record came in via the trade feed with no user information there would be a duplicate.

badgerwithagun · 2018-09-24T09:18:24Z

Been working with this a lot and have found something else: if the socket disconnects, the reconnection attempt uses an out-of-date timestamp for the resubscribe:

INFO  [2018-09-24 09:16:05,320] info.bitrich.xchangestream.gdax.GDAXStreamingService: Reopening websocket because it was closed by the host
INFO  [2018-09-24 09:16:07,524] info.bitrich.xchangestream.gdax.GDAXStreamingService: Connecting to wss://ws-feed-public.sandbox.pro.coinbase.com:-1
INFO  [2018-09-24 09:16:07,525] info.bitrich.xchangestream.gdax.GDAXStreamingService: Registering GDAXWebSocketClientHandler
INFO  [2018-09-24 09:16:08,051] info.bitrich.xchangestream.service.netty.WebSocketClientHandler: WebSocket Client connected!
WARN  [2018-09-24 09:16:08,051] info.bitrich.xchangestream.gdax.GDAXStreamingService: Resubscribing channels
DEBUG [2018-09-24 09:16:08,051] info.bitrich.xchangestream.gdax.GDAXStreamingService: Sending message: {"type":"subscribe","channels":[{"name":"ticker","product_ids":["BTC-USD"]}],"signature":"REDACTED","key":"REDACTED","passphrase":"REDACTED","timestamp":"REDACTED"}
DEBUG [2018-09-24 09:16:08,185] info.bitrich.xchangestream.service.netty.JsonNettyStreamingService: Received message: {"type":"error","message":"Sorry, you could not be authenticated: Bad Request","reason":"request timestamp expired"}

Seeing if I can fix this now.

badgerwithagun · 2018-09-24T09:42:06Z

Yup. Fixed. @bryantharris: I can't merge my changes into your branch, but if you could grab them from https://github.com/badgerwithagun/xchange-stream.git and see if you're happy that'd be great. I'm not worried about the UserTrade/Trade duplication for now; may fix that with another PR.

bryantharris · 2018-09-27T23:28:59Z

@badgerwithagun Are you asking me to review the changes on your repo or to merge them in with mine? The bug you found sounds like one that probably existed in the main code as my changes were unrelated right? Happy to review it or to try to merge it.

Though the real question is, is there any interest in pulling in my changes at all. I mean I think I submitted these like 6 months ago. I've been using them in a bot during that time and I know they work, but it just doesn't feel like there's an appetite for user specific events in this project based on the utter lack of interest in folding in what are ultimately some pretty minor changes.

badgerwithagun · 2018-09-28T05:53:52Z

@bryantharris: I'm in the same position as you. I'm not able to merge PRs, just trying to help. Unless @dozd gives me merge permission, all I can do is to test and review.

I appreciate that it's frustrating.

I have a slew of PRs of my own that I'm waiting to get merged and am actively using yours daily, so would very much like it merged. It's getting harder and harder to keep my fork synchronised with the main repo.

badgerwithagun · 2018-10-06T08:44:12Z

@bryantharris - just realised I didn't answer your question. No, the bug was introduced by your code (at least here). Auth was never previously needed by the websocket handler code. Your code re-uses the same authData for all reconnections even after it has expired. My change just simply makes it derive new authData for each reconnection. Should make your bot more stable.

Don't know if you're following #224 but there are quite a few people trying to get this stuff moving. In the meantime there are at least three other people that really want your PR - it's a great solution - so it'd be awesome if you could stick with it for a bit longer.

.

badgerwithagun

There is an issue with reconnection as noted above, but I will submit that as another PR. Approved.

bryantharris added 4 commits April 6, 2018 10:03

[gdax] Enabling of 'user' channel to receive fill messages.

74f5c40

Merge remote-tracking branch 'upstream/develop' into develop

78808fb

[gdax] Enabling of 'user' channel to receive fill messages. (reverted…

50a77e6

… from commit 74f5c40)

[gdax] Enabling of 'user' channel to receive fill messages. (reverted…

beb0a9d

… from commit 74f5c40) (reverted from commit 50a77e6)

dozd self-requested a review May 10, 2018 13:01

dozd self-assigned this May 10, 2018

dozd previously requested changes May 10, 2018

View reviewed changes

[gdax] Refactored obtaining of Websocket Auth Data to occur in the co…

a280d61

…nnect() call.

bryantharris mentioned this pull request May 19, 2018

GDAX listens to fills #173

Closed

bryantharris commented May 21, 2018

View reviewed changes

bryantharris commented Jul 11, 2018

View reviewed changes

badgerwithagun mentioned this pull request Jul 16, 2018

[bitfinex] Added some methods of authenticated api #209

Closed

badgerwithagun reviewed Sep 17, 2018

View reviewed changes

badgerwithagun mentioned this pull request Sep 17, 2018

Help with xchange-stream maintenance #181

Closed

Merge branch 'develop' into gdax_adding_user_channel

54638bb

badgerwithagun mentioned this pull request Oct 4, 2018

Project Continuation #224

Closed

badgerwithagun added a commit to badgerwithagun/xchange-stream that referenced this pull request Oct 6, 2018

Merge PR bitrich-info#160

f697697

badgerwithagun mentioned this pull request Oct 8, 2018

Fix gdax disconnect NPE/Bitmex execution reports/bug-fix rollup #191

Merged

Merge branch 'develop' into gdax_adding_user_channel

d361c85

Flemingjp approved these changes Oct 8, 2018

View reviewed changes

badgerwithagun approved these changes Oct 8, 2018

View reviewed changes

dozd merged commit 76a56b9 into bitrich-info:develop Oct 8, 2018

Flemingjp added the enhancement label Oct 9, 2018

badgerwithagun mentioned this pull request Oct 9, 2018

[Binance] Support for execution reports / UserTrades / fills #231

Closed

badgerwithagun mentioned this pull request Oct 28, 2018

[Binance] Support for execution reports / UserTrades / fills #246

Merged

badgerwithagun mentioned this pull request Jan 27, 2019

[WORK IN PROGRESS] Generic authenticated stream API #274

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gdax adding user channel #160

Gdax adding user channel #160

bryantharris commented Apr 6, 2018

derDeidra commented May 6, 2018

dozd May 10, 2018

dozd May 10, 2018

dozd May 10, 2018

dozd commented May 10, 2018

bryantharris commented May 16, 2018

bryantharris commented May 16, 2018

bryantharris left a comment

bryantharris left a comment

badgerwithagun commented Aug 13, 2018

badgerwithagun Sep 17, 2018

badgerwithagun Sep 17, 2018 •

edited

Loading

bryantharris Sep 18, 2018

badgerwithagun commented Sep 24, 2018

badgerwithagun commented Sep 24, 2018

bryantharris commented Sep 27, 2018

badgerwithagun commented Sep 28, 2018

badgerwithagun commented Oct 6, 2018

badgerwithagun left a comment

Gdax adding user channel #160

Gdax adding user channel #160

Conversation

bryantharris commented Apr 6, 2018

derDeidra commented May 6, 2018

dozd May 10, 2018

Choose a reason for hiding this comment

dozd May 10, 2018

Choose a reason for hiding this comment

dozd May 10, 2018

Choose a reason for hiding this comment

dozd commented May 10, 2018

bryantharris commented May 16, 2018

bryantharris commented May 16, 2018

bryantharris left a comment

Choose a reason for hiding this comment

bryantharris left a comment

Choose a reason for hiding this comment

badgerwithagun commented Aug 13, 2018

badgerwithagun Sep 17, 2018

Choose a reason for hiding this comment

badgerwithagun Sep 17, 2018 • edited Loading

Choose a reason for hiding this comment

bryantharris Sep 18, 2018

Choose a reason for hiding this comment

badgerwithagun commented Sep 24, 2018

badgerwithagun commented Sep 24, 2018

bryantharris commented Sep 27, 2018

badgerwithagun commented Sep 28, 2018

badgerwithagun commented Oct 6, 2018

badgerwithagun left a comment

Choose a reason for hiding this comment

badgerwithagun Sep 17, 2018 •

edited

Loading