leoschweizer / moneymoney-coinbase-pro

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

[FEATURE] Fetch purchase prices when available #10

Closed TheSerapher closed 1 year ago

TheSerapher commented 5 years ago

I have been using this extension for a while now and was looking for a way to include initial purchase prices made on Coinbase Pro. This change will allow MoneyMoney to display the initial purchase prices and in turn the current value best on the current price of the coin. Coins transferred to Coinbase and not traded on the market will also show, but they will not list any price information (since it's not available). For orders, it also takes into account when they were purchased and properly displays the date. For transactions I didn't bother fetching that information too.

The only thing I am not 100% sure about is the pagination implementation. Since orders are paginated I had to add a scraper to fetch all pages related to a product_id. In my tests it worked fine by using an artificial limit of 1 (the default is 100). I would think with a really high trade volume it probably creates too many requests and takes too long, but maybe that's OK too.

Here an example:

screenshot 2018-11-30 at 08 27 30

This PR already incorporates the fix for currencies missing ticker information.

TheSerapher commented 5 years ago

And here as requested the full feature PR.

TheSerapher commented 5 years ago

I have fixed the merge conflict and incremented to version 2 - seemed to make since for the changes done to the original version.

leoschweizer commented 5 years ago

Sorry for the lack of feedback on this, I've been quite busy lately.

Is it correct that this would display multiple balance entries for the same currency? E.g. if I buy BTC multiple times, I would get multiple entries for BTC, each showing a gain/loss compared to the current price? If this is the case, this is not really how it should work for moneymoney. We would have to calculate the average buying price, and show only one entry with that. This can of course get quite complicated if you have a mixed buy/sell transaction history...

In general, I have mixed feelings about this. Of course I understand the demand for showing gains / losses for the cryptocurrencies. But on the other hand this approach seems brittle and can impose a significant number of HTTP requests on every moneymoney refresh for frequent traders.

TheSerapher commented 5 years ago

No problem!

I think this depends. For me it would be more important to know when a specific buy is worth selling again. Same in regards for taxes. You’d have to tax based on the gain on specific sales so having those listed that way would make that fairly easy. But of course an average is also possible.

And I am with you on the request count. That really is the limiting factor here and there is sadly no great way to get the trade prices without calling each currency separately ... I think unless Coinbase offers a more convenient API endpoint to gather this information we should close this. I’ll keep it for myself and check occasionally if there is a new way to do this.

TheSerapher commented 5 years ago

So I checked their online documentation and it seems they only throttle requests if they reach a certain limit by the second: https://docs.pro.coinbase.com/#rate-limits

REST API

PUBLIC ENDPOINTS

We throttle public endpoints by IP: 3 requests per second, up to 6 requests per second in bursts.

PRIVATE ENDPOINTS

We throttle private endpoints by user ID: 5 requests per second, up to 10 requests per second in bursts.

A quick way around for me was adding a sleep before each request. I haven't found any other way around it since we have to request data page by page.

I wish they added a simple get ledger call that would automatically format everything in a way that we can ingest into MoneyMoney. Would make this a whole lot easier and they could run the sums on their end ...