Skip to content

Coinbase CSV plugin#66

Closed
iloveitaly wants to merge 2 commits into
eprbell:mainfrom
iloveitaly:coinbase-csv
Closed

Coinbase CSV plugin#66
iloveitaly wants to merge 2 commits into
eprbell:mainfrom
iloveitaly:coinbase-csv

Conversation

@iloveitaly

@iloveitaly iloveitaly commented Aug 1, 2022

Copy link
Copy Markdown
Contributor

No description provided.

@eprbell eprbell mentioned this pull request Aug 1, 2022
@eprbell

eprbell commented Aug 1, 2022

Copy link
Copy Markdown
Owner

Thanks for submitting a PR! I will review this in the next few days, but meanwhile check my comments on unique_id in #60 (which apply here too).

@eprbell eprbell left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks to me this is parsing the "Reports / Visit Taxes / Raw Transactions Report" CSV file: is this correct? I looked at that file and it seems quite incomplete. In my generated file there are some Coinbase to Coinbase Pro transfers, some Coinbase Pro to Coinbase ones, but not all of them. Also external transfers seem to be missing altogether. As it is, I think may be too incomplete as a data source for RP2.

A good test is to generate the RP2 report with the CSV plugin and then with the REST plugin and then compare the results (you can use ods_diff.py utility to compare ODS files).

Coinbase also generates another CSV file ("Reports / Transaction History"), which is also missing data: e.g. Coinbase Pro transfers and transaction hash.

If the data source is too incomplete I think it may be best to just direct users to the REST plugin.

transaction_id: str = line[self.__TRANSACTION_ID_INDEX].strip()
transaction_type: str = line[self.__TRANSACTION_TYPE_INDEX].strip()

# TODO unsure how to handle conversions, I think this is only for ETH2

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check how the REST Coinbase plugin does it (look for "swap" in the code)

if transaction_type == _CONVERT:
continue

# there is no timezone information in the CSV, so we assume UTC

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is (at least in mine): the Z at the end of the timestamp means UTC

@eprbell

eprbell commented Aug 26, 2022

Copy link
Copy Markdown
Owner

Since we already have a REST plugin and Coinbase CSV are fairly incomplete this is probably not needed.

@eprbell eprbell closed this Aug 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants