eprbell / dali-rp2

DaLI (Data Loader Interface) is a data loader and input generator for RP2 (https://pypi.org/project/rp2), the privacy-focused, free, open-source cryptocurrency tax calculator: DaLI removes the need to manually prepare RP2 input files. Just like RP2, DaLI is also free, open-source and it prioritizes user privacy.
https://pypi.org/project/dali-rp2/
Apache License 2.0
67 stars 42 forks source link

Coinbase "advanced_trade_fill" type is not supported #91

Open skasab opened 2 years ago

skasab commented 2 years ago

First off, what a great project! I liked the original rp2, but the data wrangling was always the most difficult bit. This data loader is super easy to use and understand. Kudos!

One thing I ran into though is that with Coinbase introducing "advanced trading" earlier this year, some of my transactions for 2022 are labeled as "type": "advanced_trade_fill", many of which appear to be throwing errors (redacted example below)

ERROR: Unsupported transaction type (skipping):

{
   "id":"[XXXXX]",
   "type":"advanced_trade_fill",
   "status":"completed",
   "amount":{
      "amount":"[XXX]",
      "currency":"BTC"
   },
   "native_amount":{
      "amount":"[XXX]",
      "currency":"USD"
   },
   "description":null,
   "created_at":"2022-03-10T15:34:41Z",
   "updated_at":"2022-03-10T15:34:42Z",
   "resource":"transaction",
   "resource_path":"/v2/accounts/[xxxxx]/transactions/[xxxxx]",
   "instant_exchange":false,
   "advanced_trade_fill":{
      "fill_price":"[XXX]",
      "product_id":"BTC-USD",
      "order_id":"[XXX]",
      "commission":"0"
   },
   "details":{
      "title":"Filled Bitcoin on buy",
      "subtitle":null,
      "header":null,
      "health":"positive"
   },
   "hide_native_amount":false
}

I have 86 errors in my log file, all of which print out these advanced_trade_fill type transactions.

Apologies if this has been brought up before - couldn't find a similar issue on search.

As a side note - perhaps should be its own thread - is there any way to constrain the time parameters of a data pull? Ex. I wanted to avoid this issue by just pulling 2021 transactions, but it looks like there was no way to do that.

eprbell commented 2 years ago

Thanks for the kind words and for reporting a missing feature! Currently I don't have time to work on adding advanced trade to the Coinbase plugin, but if anybody wants to do it I'll be happy to review PRs (I tagged this issue as "Help Wanted" to increase visibility).

Regarding the time filter question: DaLI doesn't have one, but RP2 does. The rationale is that RP2 needs all the data to compute taxes (but it can time-filter output after computation) and DaLI just reads everything because RP2 needs everything. What would be the use case for a DaLI time filter, since you can't (or rather shouldn't) feed RP2 incomplete data? Just wanted to understand if I'm missing something here.

ghost commented 1 year ago

I think this would be a good first issue me as I'd like to start contributing to the RP2 community. Could this issue please be assigned to me? Thank you!

eprbell commented 1 year ago

Thanks for volunteering to help! Actually advanced_trade_fill was added here (I forgot to update the issue). However, while both sell and buy are implemented, only sell was tested and advanced trade conversions are not implemented at all. So I suggest the following:

If this interests you I'll assign the bug to you. Thanks!

ghost commented 1 year ago

Thanks for volunteering to help! Actually advanced_trade_fill was added here (I forgot to update the issue). However, while both sell and buy are implemented, only sell was tested and advanced trade conversions are not implemented at all. So I suggest the following:

  • start by testing the buy side, then
  • work on advanced trading conversions.

If this interests you I'll assign the bug to you. Thanks!

That definitely interests me, please assign the bug to me. After this bug, my goal is to become a regular contributor. RP2/Dali are so unique in their space, and I'd love to put some work in so they continues to improve. 👍

eprbell commented 1 year ago

Done! Welcome again and let us know if you have questions.

eprbell commented 1 year ago

Quick update on this: I managed to test advanced trade buy and found a small bug, which I fixed here: https://github.com/eprbell/dali-rp2/commit/6828f04de0f62d6a29834004ca5d4f7164594116. So all that is missing is advanced trade conversions (regular conversions were already supported).