leoschweizer / moneymoney-coinbase-pro

Inofficial MoneyMoney extension to list Coinbase Pro balances as securities
MIT License
6 stars 4 forks source link

Bad Request Error #4

Closed qoomon closed 5 years ago

qoomon commented 5 years ago

Log says:

1> Refreshing account Coinbase Pro. 1> Web Banking Engine: Using user-supplied extension GDAX.lua version 1.00. 1> Protocol version: Web Scraping 1> Server address: https://api.gdax.com 1> Getting portfolio for Coinbase Pro... ... 1> Sending: GET https://api.coinbase.com/v2/exchange-rates?currency=ETC 22:28:05 1> HTTPS response: Bad Request 1> The server responded with the error message: Bad Request. Retrying... 1> Sending: GET https://api.coinbase.com/v2/exchange-rates?currency=ETC 22:28:06 1> HTTPS response: Bad Request 1> The API server of your bank responded with the error message: »Bad Request« Please try again later or contact the support of your bank with this error message.

leoschweizer commented 5 years ago

See https://github.com/leoschweizer/moneymoney-gdax/issues/3#issuecomment-411324815

TheSerapher commented 5 years ago

I think this may need to be revised since they started trading but it's still thrown Bad Request. My strong suspicion is that they are not going to add this new currency to the deprecated API and an update may be necessary to their new API endpoint to make this work.

 1> Sending: GET https://api.coinbase.com/v2/exchange-rates?currency=ETC
          1> HTTPS response: Bad Request
          1> The server responded with the error message: Bad Request. Retrying...
          1> Sending: GET https://api.coinbase.com/v2/exchange-rates?currency=ETC
11:39:24  1> HTTPS response: Bad Request

I have also added ETC to my account so that shouldn't be an issue.

Maybe it's time to move to their new API endpoint 😄

leoschweizer commented 5 years ago

@TheSerapher the exchange rates API is not a GDAX API but a Coinbase API and thus not subject to the deprecation at all!

TheSerapher commented 5 years ago

My bad! But isn't that the API endpoint for Coinbase and not Coinbase Pro (which is replacing GDAX)? Since Coinbase itself didn't add the new currency it can't get the new data. Hence it's causing a Bad Request error.

Maybe it would make sense to add a new plugin that supports Coinbase Pro and is named as such to avoid any confusion with other plugins?

I think calling https://api.gdax.com/accounts and then later using the Coinbase (non-PRO) API is mixing two different API endpoints for different purposes. There is a Coinbase Plugin for MoneyMoney which would be used to call that endpoint (I would assume, didn't check the plugin code).

What do you think?

leoschweizer commented 5 years ago

It is the Coinbase API, the Coinbase PRO API (which really IS the GDAX API with a new address) unfortunately does not have an exchange rates endpoint: https://docs.pro.coinbase.com (neither did the GDAX API).

So the problem won't really go away when switching the GDAX endpoints to the Coinbase Pro endpoints...

TheSerapher commented 5 years ago

So we'd need a new plugin specifically for the Coinbase Pro API if I am not mistaken. Maybe this endpoint can be used to catch a most recent trade value and use it for the Last quote in MoneyMoney:

https://api.pro.coinbase.com/products/BTC-EUR/ticker

Comparing the ticker price with the price displayed on the WebUI, they seem to match and could be used?

I wish I could give it a go but using the App Store version of MoneyMoney I am not able to disable signature checks (unless you know a way or how to create a signature?).

EDIT: Send a mail to their support - maybe I can get a license for the non-App Store App (since I already own the app anyway). Never hurts to ask :-)

leoschweizer commented 5 years ago

The ticker API could certainly be used for the time being but I wouldn't be too happy with it. Semantically, it does not express what I wan't to see in MoneyMoney - the actual worth of my cryptocurrencies in EUR. The ticker API just gives you the current bid and ask prices and sizes, which doesn't mean that you could convert your cryto assets to EUR with the current bid price (which is only the case when the current bid size is greater than your portfolio). But the Coinbase Exchange rate does mean exactly that, at least that was my understanding.

Regarding signatures: Only the author of MoneyMoney can sign extensions, but as long as you can verify that you bought the app they will happily send you a license for the non-appstore version.

TheSerapher commented 5 years ago

What if we used the ticker to fetch the current price and get the actual value based on your account portfolio. We have the amount of each coin and use the ticker to calculate the value to display in MoneMoney.

Sent them a mail and waiting for a response, may take a while the automated reply said :-)

TheSerapher commented 5 years ago

Went ahead and checked again and it seems you are already doing it in a two-step process. You fetch all accounts (CryptoCurrencies and real ones). If the currency you find is the native one in MoneyMoney (e.g. EUR), you handle it different since there is no exchange rate. For all others you fetch the current trade value from the exchange API:

https://github.com/leoschweizer/moneymoney-gdax/blob/master/GDAX.lua#L76

Instead of taking this as a fixed value, we can just use the quantity available in our Account listing:

https://github.com/leoschweizer/moneymoney-gdax/blob/master/GDAX.lua#L78

and call the ticker API to fetch the current price, then multiply by quantity to get the same result.

I'd think making this a new moneymoney-coinbasepro extension would make sense since it's not related to GDAX anymore with all changes done.

If/once I get my license I can aid in testing if you need a volunteer.

Edit: Forgot to mention that I checked the ticker price * amount in Ruby and it matches the balance shown in the WebUI when going to "My Wallets" for each coin. I would guess this would be a good way to calculate it for MoneyMoney. I didn't use the bid and ask prices, just the price which seems the be the current 'value' of the currency on the market. I'd say that's close enough for a real value :)

leoschweizer commented 5 years ago

I think you really overestimate the difference between GDAX and Coinbase Pro :wink: This was just a rebranding. The API is exactly the same. All that needs to be changed in that regard is the API base url.

That alone would however still cause the current exchange rate problem, since this calculation always did use the Coinbase API which currently causes this problem but is itself unrelated to the GDAX <-> Coinbase Pro rebranding. We could as well use a completely different source for exchange rates since this never was based on any Coinbase account specific information. It was merely used to multiply your GDAX account balances with an exchange rate to get a fiat currency.

So all that needs to be done is 1) change the API endpoints and 2) integrate another exchange rate source (e.g. the Coinbase Pro ticker API but it could be any exchange rate API) and maybe 3) rename the repository / extension :wink:

TheSerapher commented 5 years ago

I guess you are right, overthinking it it seems 😉. I just thought if we brand it a Coinbase Pro extension it would make sense to also ask the Coinbase Pro ticker even though it may not really be required to show a proper value for the coins a user has. So in short:

1) Yes 2) Coinbase Pro Ticker preferred to stay in the same "realm" 3) Yes 😄

Thanks for your feedback! I may look into it but it would be mostly a copy of the existing GDAX extension so maybe it would be best if you took it under your repository. Don't wanna steal any work you have done under a repo in my name - while keeping most of the code the same. But that'll be your call.

leoschweizer commented 5 years ago

@TheSerapher if you want to make the adjustments, that's what pull requests are for 😉

TheSerapher commented 5 years ago

You'll create a new repository or just rename this one?

I'll have to wait for a license but I can give it a go then.

leoschweizer commented 5 years ago

I would rename this repository once the changes are made, because it doesn't make sense to keep it separate (GDAX no longer exists under this name).

TheSerapher commented 5 years ago

True. I'll see what I can do!

leoschweizer commented 5 years ago

I've started a branch with the necessary adjustments. https://github.com/leoschweizer/moneymoney-gdax/tree/feature/coinbase-pro-support

Only the exchange rate API fix is missing. The work is basically done, I asked for a new signature for the changes.

TheSerapher commented 5 years ago

Awesome! Once ready I will install it and give it a go. Thanks for your work on this, makes my life a lot easier 😃

leoschweizer commented 5 years ago

The extension for Coinbase Pro is now available at https://moneymoney-app.com/extensions/

You will have to remove your GDAX account from MoneyMoney and add a new account for Coinbase Pro.

TheSerapher commented 5 years ago

Got me the license but installed the Signature version anyway. Works! Thanks a lot :-)