Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add exchange Coinbene support #366

Open
wants to merge 14 commits into
base: master
from

Conversation

@MadCozBadd
Copy link
Collaborator

commented Oct 8, 2019

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Please delete options that are not relevant and add an x in [] as item is complete.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Tested with golangci-lint and basic error tests

Please also consider improving test coverage whilst working on a certain package

  • Test A
  • Test B

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation and regenerated documentation via the documentation tool
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally and on Travis with my changes
  • Any dependent changes have been merged and published in downstream modules
Copy link
Collaborator

left a comment

Awesome work, just did a quick first pass.

exchanges/coinbene/coinbene.go Outdated Show resolved Hide resolved
exchanges/coinbene/coinbene.go Outdated Show resolved Hide resolved
exchanges/coinbene/coinbene.go Outdated Show resolved Hide resolved
exchanges/coinbene/coinbene.go Outdated Show resolved Hide resolved
exchanges/coinbene/coinbene_test.go Show resolved Hide resolved
exchanges/coinbene/coinbene_wrapper.go Outdated Show resolved Hide resolved
exchanges/coinbene/coinbene_wrapper.go Outdated Show resolved Hide resolved
exchanges/coinbene/coinbene_wrapper.go Outdated Show resolved Hide resolved
exchanges/coinbene/coinbene_wrapper.go Outdated Show resolved Hide resolved
testdata/configtest.json Outdated Show resolved Hide resolved
@thrasher- thrasher- changed the title Coinbene Add exchange Coinbene support Oct 8, 2019
Copy link
Collaborator

left a comment

Nice work @MadCozBadd ! Just a few nits:

config_example.json Show resolved Hide resolved
exchanges/coinbene/README.md Outdated Show resolved Hide resolved
exchanges/coinbene/README.md Outdated Show resolved Hide resolved
exchanges/coinbene/README.md Outdated Show resolved Hide resolved
exchanges/coinbene/README.md Outdated Show resolved Hide resolved
exchanges/coinbene/coinbene_wrapper.go Outdated Show resolved Hide resolved
testdata/configtest.json Outdated Show resolved Hide resolved
testdata/configtest.json Outdated Show resolved Hide resolved
testdata/configtest.json Show resolved Hide resolved
testdata/configtest.json Outdated Show resolved Hide resolved
Copy link

left a comment

LGTM

@shazbert

This comment has been minimized.

Copy link
Collaborator

commented Oct 9, 2019

Bump default enabled exchanges in const config.
Please add tests for wrapper functions.

Copy link
Collaborator

left a comment

Cool stuff! Some nits from an initial pass centre around parameters and types.

exchanges/coinbene/coinbene_wrapper.go Outdated Show resolved Hide resolved
exchanges/coinbene/coinbene.go Outdated Show resolved Hide resolved
exchanges/coinbene/coinbene_wrapper.go Outdated Show resolved Hide resolved
exchanges/coinbene/coinbene_wrapper.go Outdated Show resolved Hide resolved
exchanges/coinbene/coinbene_wrapper.go Outdated Show resolved Hide resolved
exchanges/coinbene/coinbene_wrapper.go Outdated Show resolved Hide resolved
exchanges/coinbene/coinbene_wrapper.go Outdated Show resolved Hide resolved
Copy link
Member

left a comment

I have had a quick initial look over great work so far.

A few minor issues that cause broken functionality that need to be addressed

exchanges/coinbene/coinbene_types.go Outdated Show resolved Hide resolved
exchanges/coinbene/coinbene.go Outdated Show resolved Hide resolved
var preSign string
if len(params) != 0 {
m := make(map[string]string)
for k, v := range params {

This comment has been minimized.

Copy link
@xtda

xtda Oct 9, 2019

Member

the url package has a function called ParseQuery that takes a url encoded string and returns a map

exchanges/coinbene/coinbene.go Outdated Show resolved Hide resolved
MadCozBadd added 2 commits Oct 10, 2019
Copy link
Collaborator

left a comment

Thanks for making those changes. I've reopened some of the previously closed nits as they're still valid. I've added a nit for rate limits. Please review

exchanges/coinbene/coinbene.go Outdated Show resolved Hide resolved
config_example.json Show resolved Hide resolved
exchanges/coinbene/README.md Outdated Show resolved Hide resolved
testdata/configtest.json Outdated Show resolved Hide resolved
MadCozBadd added 2 commits Oct 17, 2019
)

const (
coinbeneWsURL = "wss://ws-contract.coinbene.vip/openapi/ws"

This comment has been minimized.

Copy link
@xtda

xtda Oct 17, 2019

Member
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.ABTETH.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.ABTUSDT.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.ADIETH.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.ADKBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.ADNBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.AIDBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.AITUSDT.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.ALIETH.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.ALXETH.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.APLETH.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.ATXBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.B2GBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.B91USDT.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.BATBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.BEZBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.BGCUSDT.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.BKGBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.BNTBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.BOAUSDT.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.BTTUSDT.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.BVTETH.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.C3WETH.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.CANETH.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.CCCETH.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.CCEUSDT.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.CFTUSDT.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.CLOBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.CMTETH.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.CMTUSDT.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.CNNBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.CNNETH.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.CNNUSDT.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.CPCBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.CRNBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.CVCBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.CXCUSDT.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.CXPBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.DCAETH.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.DCTBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.DGDBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.DTAETH.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.DUCBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.DVCETH.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.EBCBTC.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.EBCETH.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.EBCUSDT.100. Code: 10503
[ERROR]: 2019/10/17 13:07:31 routines.go exchange Coinbene websocket error - Message: unsupported topic orderBook.ECABTC.100. Code: 10503

As mentioned before this websocket is not designed for their regular exchange but for

https://www.coinbene.com/contract/usdt/BTC-SWAP
https://www.coinbene.com/contract/btc/BTCUSDT

How does this impact with usual orderbook/ticker requirements etc? @thrasher- might be able to shed some light on

This comment has been minimized.

Copy link
@gloriousCode

gloriousCode Oct 17, 2019

Collaborator

I feel the best approach would be to read the error, determine the subscription and remove it from the subscribed pairs.

Specific currencies for specific asset types is a feature of engine as is REST/Websocket data retrieval switching and could be adjust there post engine-merge, or post this branch rebased into engine. Pairs supported can be retrievable via http://openapi-contract.coinbene.com/api/swap/v2/market/tickers in the FetchTradablePairs funcs

This comment has been minimized.

Copy link
@thrasher-

thrasher- Oct 17, 2019

Collaborator

There's currently no way in master to abstract a spot pair from a swap deriv contract. Will make the necessary changes on engine as it supports multiple asset types when merging this in and can leave websocket disabled by default

Copy link

left a comment

LGTM

Copy link

left a comment

LGTM

Copy link
Collaborator

left a comment

Getting closer! Thanks for fixing some of the previous nits, just have some more:

| m1kola | https://github.com/m1kola
| cavapoo2 | https://github.com/cavapoo2
| zeldrinn | https://github.com/zeldrinn

This comment has been minimized.

Copy link
@thrasher-

thrasher- Oct 17, 2019

Collaborator

Please preserve the original contributors

README.md Outdated

This comment has been minimized.

Copy link
@thrasher-

thrasher- Oct 17, 2019

Collaborator

Please preserve the original contributors

@@ -752,6 +752,52 @@
}
]
},
{
"name": "Coinbene",
"enabled": false,

This comment has been minimized.

Copy link
@thrasher-

thrasher- Oct 17, 2019

Collaborator

Please enable this exchange by default (but will need a warning for using a non HTTPs endpoint)

exchanges/coinbene/README.md Outdated Show resolved Hide resolved
exchanges/coinbene/coinbene_websocket.go Outdated Show resolved Hide resolved
testdata/configtest.json Outdated Show resolved Hide resolved
Copy link

left a comment

LGTM

Copy link

left a comment

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.