Generic user channels (balance, open orders, user trades) + support for Binance, Bitfinex, Coinbase Pro #340
Conversation
…ned in bitrich-info#274. Coinbase Pro is a challenge here. It seems to need an initial snapshot. At the moment the implementation is bare-bones. Need to start working on the ordering challenge. All three seem to provide a sequence number. This will be essential for maintaining any sort of snapshot. Not done balances yet.
Also adds warning that the order change stream contains incomplete data.
… to be implemented for everything
After refactoring to move account and trade streams to separate classes, I forgot to subscribe those classes on connection.
|
Looks great, thanks for putting in the effort! Looking forward to extending your work onto some more exchanges once merged in. |
|
@pchertalev any chance of a review on this in return? |
badgerwithagun
changed the title
|
Great work @badgerwithagun ! Thanks for that. It was much needed. |
Looks great overall, not too familiar with the netty internals but the public API's are exactly what we need. |
|
Will someone please approve this PR in order to work on other exchanges implementation? |
|
Thanks for the review @nielsdraaisma. We do need am approved committer review too. @Flemingjp perhaps? |
There was a problem hiding this comment.
Looks good to me @badgerwithagun , I will give a hand extending to other exchanges and think about the Coinbase Pro ToDo's. I have used Binance and Bitfinex and so far all good.
|
Thanks @garciapd. Still need a review from an approved committer - GitHub blocks me from merging otherwise. |
|
Can i become a review or something in order to help speeding up the process? |
|
@makarid - by all means, perform a review (more reviews is good), but without signoff from an approver like @Flemingjp or @dozd this can't be merged. I'm not allowed to approve my own stuff. @dozd perhaps you could step in? |
|
@makarid If you review the PR briefly, I'll submit it afterwards |
There was a problem hiding this comment.
@badgerwithagun have you test the new changes to the NettyStreamingService.java? Have you changed any logic about reconnecting if an exception or error occurs? Thanks
|
@Flemingjp i just review it. |
|
@makarid - as much as I could. I rhave been running this code in production for.many months and it is pretty stable, but I only use a limited number of exchanges. No guarantees, but it it does seem to be more "correct" based on example Netty code I've read elsewhere which makes me feel more confident. |
Thanks a lot for your response @badgerwithagun ! |
Resolves #274
Resolves #276
Resolves #278
Resolves #283
See #274 for full description.
This is ready for merge. Note there are some TODOs around Coinbase Pro support, but it's useful as it is, so I say LGTM.