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

New Coinbase Pro Exchange Adapter #120

Closed
wants to merge 2 commits into from

Conversation

DavidHuertas
Copy link
Contributor

Hi! I have been working in a fork of your BXbot project. After the migration of GDAX API to Coinbase Pro API, a new Coinbase Pro Exchange Adapter is needed (you can check it here)

It works as the other exchanges, plus a "time-server-bias" that is needed in order to make work the private calls (you can check this issue I have open in my fork: DavidHuertas#2).

I have a question to make: ¿do you have in mind to add calls to the "fees" services in order to improve fees calculation further than keeping them in the "otherConfig" section of the "exchange.yaml" config file?

Thank you in advance, regards
David Huertas

@gazbert
Copy link
Owner

gazbert commented Aug 20, 2019

Hi David

Thank you for your contribution - it's always good to see folks contributing to the project 👍

I'll try and make some time this weekend to review the PR...

I'll also deprecate the GDAX adapter in the next release, given the exchange has been migrated from GDAX to Coinbase Pro.

Regarding fees API calls - go for it! The fees in the YAML config are fallback options for when exchanges did not expose their fees via their REST APIs. I believe the Bitstamp adapter gets its fees from the exchange's REST API.

Looks like the PR build needs fixing - SpotBugs issues from a quick look ;-)

Take your time - no rush for anything :-)

gazbert

@gazbert gazbert self-assigned this Aug 20, 2019
@gazbert gazbert added this to In Progress in BX-bot Aug 20, 2019
@gazbert gazbert self-requested a review August 20, 2019 20:56
@gazbert gazbert assigned DavidHuertas and unassigned gazbert Aug 20, 2019
@DavidHuertas
Copy link
Contributor Author

Thank you so much for your answer, as soon as I have some free time I will review the different issues to make my changes pullable to your repository.

gazbert added a commit that referenced this pull request Aug 25, 2019
@DavidHuertas
Copy link
Contributor Author

I will review when I have time enough

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants