Skip to content

New Coinbase Pro Exchange Adapter#120

Closed
DavidHuertas wants to merge 2 commits intogazbert:masterfrom
DavidHuertas:master
Closed

New Coinbase Pro Exchange Adapter#120
DavidHuertas wants to merge 2 commits intogazbert:masterfrom
DavidHuertas:master

Conversation

@DavidHuertas
Copy link
Copy Markdown
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
Copy Markdown
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 self-requested a review August 20, 2019 20:56
@gazbert gazbert assigned DavidHuertas and unassigned gazbert Aug 20, 2019
@DavidHuertas
Copy link
Copy Markdown
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
Copy Markdown
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.

2 participants