Plugin for binance.us csv tax report #65

iloveitaly · 2022-08-01T14:26:07Z

CSV import plugin for binance.us tax report. There's some TODOs which I'd love to resolve if folks have tips.

By the way, the cctx plugin has been working great for me.

eprbell · 2022-08-01T17:20:35Z

Awesome to hear that @macanudo527's hard work is starting to pay off: thanks for trying his Binance plugin! I will review this in the next few days, but meanwhile check my comments on unique_id in #60 (which apply here too).

macanudo527 · 2022-08-03T02:23:00Z

Has the CCXT plugin worked for Binance.us? Just to be clear, I've tested / worked with it for Binance.COM. Hopefully we can get it finalized and I can add official support for your plugins.

eprbell

Since I don't have a Binance account I can't tell if the CSV file is as complete as the REST output: @macanudo527 could you also review this plugin, since you know much more than I do about Binance?

I'm particularly curious about a couple of things:

does the transaction id contain the transaction hash or is it an internal binance id (see https://github.com/eprbell/dali-rp2/blob/main/docs/developer_faq.md#how-to-fill-the-unique-id-field)?
is the CSV as complete as the REST API or is it missing data (e.g. the Coinbase CSV files are missing a lot of data)?

@iloveitaly, your efforts on CSV plugins are bringing up an interesting design discussion. CSV files are sometimes incomplete (not always, e.g. Trezor is complete AFAIK): when is it worth having a CSV plugin? Here's what I'm thinking so far (input and comments are welcome):

if there is a REST plugin and the CSV files are incomplete, then the CSV plugin can probably be skipped
if the there is a REST plugin and the CSV files are complete, then CSV plugin is OK to have (it's an alternative)
if there is no REST plugin, CSV plugin is good to have regardless of whether CSV files are complete or not (it's better than nothing)

Thanks everyone!

src/dali/plugin/input/csv/binance.py

eprbell · 2022-08-03T16:08:23Z

src/dali/plugin/input/csv/binance.py

+                                    "assets": line[self.__BASE_ASSET_INDEX].strip(),
+                                    "crypto_out_no_fee": line[self.__BASE_ASSET_AMOUNT_INDEX].strip(),
+                                    "spot_price": line[self.__BASE_ASSET_AMOUNT_SPOT_PRICE_INDEX].strip(),
+                                    # TODO it's unclear if `crypto_fee` is the USD amount of the crypto-paid fee


See my other comments on fees above.

eprbell · 2022-08-07T18:24:10Z

I formalized the CSV vs REST discussion we started here into a FAQ: https://github.com/eprbell/dali-rp2/blob/main/docs/developer_faq.md#should-i-implement-a-csv-or-a-rest-data-loader-plugin

According to the reasoning in the FAQ, this CSV plugin may fall in one of these three cases:

bullet 2: REST plugin already implemented (the binance.com REST plugin also covers binance.us). The binance.us CSV files are NOT complete;
bullet 3: REST plugin already implemented (the binance.com REST plugin also covers binance.us). The binance.us CSV files are complete;
bullet 4: REST plugin NOT implemented (the binance.com REST plugin doesn't cover binance.us and we would need a different REST plugin for binance.us).

If bullet 3 or 4 are true we can proceed with the binance.us CSV plugin, but if bullet 2 is true we should abandon it.

So in order to decide, the questions to be answered are:

does the binance.com REST plugin also cover binance.us (or is the binance.us REST plugin trivially derived from the binance.com one, once it's finished)?
are the binance.us CSV files complete and correct? Based on the code it seems incomplete because unique_id is UNKNOWN.

@macanudo527, can you help with the above questions?

macanudo527 · 2022-08-11T01:44:28Z

Sorry, I guess Binance.US and Binance.com have very different CSV export processes. For .com, I have to export each kind of transaction separately.

So, to get deposits, I have to go -> [Wallet] -> [Fiat & Spot] -> [Transaction History] (which defaults to deposit history) -> [Export Deposit History]

And that will give me the deposit address, txnId, amount, asset, etc..., so basically, everything you need for IntraTransactions to resolve. Is it different for .US? That exported file does not include buys or sells, I have to go to a different menu item to get trades. I don't have access to .US to test.

eprbell · 2022-08-12T21:12:32Z

Ok, thanks for the info. Are the txids in binance.com CSVs actual transaction hashes or are they internal Binance ids?

iloveitaly · 2022-08-13T16:19:18Z

The transaction IDs (excluding USD deposits) only have 8 characters, which isn't enough for a txn hash right? My knowledge of the underlying network hashes is very fuzzy.

I think it's safe to say that the unique_id is unknown in this plugin.

does the binance.com REST plugin also cover binance.us (or is the binance.us REST plugin trivially derived from the binance.com one, once it's finished)?

From my experience with my crypto bot (https://github.com/iloveitaly/crypto-index-fund-bot) the APIs between .com & .us are materially different. I can't confirm if .us is a subset of .com, but from what I've seen .us is a fork which has diverged from .com in enough ways that they should be thought of as separate exchanges. I could be wrong.

Sorry, I guess Binance.US and Binance.com have very different CSV export processes. For .com, I have to export each kind of transaction separately.

Yup, binance.us has a special tax report which includes all transactions.

eprbell · 2022-08-14T17:31:03Z

The transaction IDs (excluding USD deposits) only have 8 characters, which isn't enough for a txn hash right? My knowledge of the underlying network hashes is very fuzzy.

Right: it's too short to be a transaction hash (at least for BTC, ETH and many others).

I think it's safe to say that the unique_id is unknown in this plugin.

does the binance.com REST plugin also cover binance.us (or is the binance.us REST plugin trivially derived from the binance.com one, once it's finished)?

From my experience with my crypto bot (https://github.com/iloveitaly/crypto-index-fund-bot) the APIs between .com & .us are materially different. I can't confirm if .us is a subset of .com, but from what I've seen .us is a fork which has diverged from .com in enough ways that they should be thought of as separate exchanges. I could be wrong.

Ok, however we use CCXT as a foundation and @macanudo527 said the CCXT binance.us class is a subclass of the CCXT binance.com one, so for our purposes we can probably consider .us a subset of .com.

Since the .us CSV is incomplete (no hashes) and the CCXT binance.us REST plugin is probably relatively simple to obtain from the .com one (or from its abstract superclass), I'm leaning toward skipping the .us CSV plugin and just focus on the REST version.

Thoughts?

iloveitaly · 2022-08-16T13:40:17Z

That seems like a reasonable approach! My gut is the .us API started as a subset of .com but has meaningfully diverged—even if the spec looks/is the same, I have a sense that the implementation/nuances of the API are different. Using the API makes sense, but I don't have the time to help here now that I have the CSV importer working :(

I've fixed in the mypy errors here in case you want to merge this in!

eprbell · 2022-08-16T18:02:04Z

That's good thinking: let's keep this open. If we find out that the REST version of the binance.us plugin isn't as easy to obtain as we're currently thinking we can merge this PR instead. Thanks for helping out and driving the discussion in interesting directions!

TheGreatJoules · 2023-04-07T05:21:56Z

hey @eprbell what is the status on this PR can this be merged? were you able to determine if the REST version of the binance.us was easily obtainable?

eprbell · 2023-04-07T05:49:49Z

hey @eprbell what is the status on this PR can this be merged? were you able to determine if the REST version of the binance.us was easily obtainable?

@gbtorrance has recently started working on the REST version of the Binance.us data loader. The reason I preferred waiting instead of merging this is that apparently Binance.us CSV files are missing transaction hashes, which are essential information. This makes transfer transactions unresolvable and as a result the missing information needs to be edited manually by users.

Could you try this version with your taxes and let us know how it goes? If you find it is useful enough we might go ahead and merge it, while the REST variant is being worked on.

TheGreatJoules · 2023-04-07T06:28:15Z

Thanks for the update, ah gotcha. Yeah I was planning on testing this one out however I pulled this PR and attempted to install via the dev page https://github.com/eprbell/dali-rp2/blob/main/README.dev.md but I keep running into this error:

import errors ImportError: cannot import name 'to_string' from 'rp2.configuration'.

Not sure if this is a known issue or one isolated to this PR as this is my first time looking at this repo but will take a closer look over this weekend.

eprbell · 2023-04-07T14:50:45Z

That means it's not finding RP2: are you sure you're not missing some of the setup steps?

A few questions:

which OS are you using?
did you do the activate step?
can you type: rp2_us --help?
can you type: dali_us --help?
can you type pytest: do the unit tests run to completion?
can you paste the entire error with stack and everything?

iloveitaly · 2023-09-17T13:27:03Z

@eprbell just pushed some updates here! What are your thoughts on getting this merged?

eprbell · 2023-09-24T15:49:39Z

@eprbell just pushed some updates here! What are your thoughts on getting this merged?

The Binance.us plugin didn't happen yet, so I'm OK with proceeding with this. I'll review the code in the next few days. Thanks for following up!

eprbell mentioned this pull request Aug 1, 2022

Nexo plugin #60

Merged

eprbell requested changes Aug 3, 2022

View reviewed changes

iloveitaly force-pushed the binance-csv-plugin branch from 8eb40ef to 6029bfd Compare August 7, 2022 11:52

eprbell mentioned this pull request Aug 9, 2022

Binance.com (not US) plugin #45

Merged

iloveitaly added 6 commits August 16, 2022 07:34

Binance csv tax report ingestion

8252249

Indicate .us extension

b19ad1d

Represent fees as separate transactions

3beab4a

Handle fees and spot price calculation properly

293a123

Linting fixes

254bcdd

Typing fixes

b766b6b

iloveitaly force-pushed the binance-csv-plugin branch from 7a67a8e to b766b6b Compare August 16, 2022 13:38

adding unique txn ids for improved auditability

7e6b857

Merge branch 'main' into binance-csv-plugin

e0a22c9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plugin for binance.us csv tax report #65

Plugin for binance.us csv tax report #65

iloveitaly commented Aug 1, 2022

eprbell commented Aug 1, 2022

macanudo527 commented Aug 3, 2022

eprbell left a comment

eprbell Aug 3, 2022

eprbell commented Aug 7, 2022 •

edited

macanudo527 commented Aug 11, 2022

eprbell commented Aug 12, 2022

iloveitaly commented Aug 13, 2022

eprbell commented Aug 14, 2022

iloveitaly commented Aug 16, 2022

eprbell commented Aug 16, 2022

TheGreatJoules commented Apr 7, 2023

eprbell commented Apr 7, 2023 •

edited

TheGreatJoules commented Apr 7, 2023

eprbell commented Apr 7, 2023

iloveitaly commented Sep 17, 2023

eprbell commented Sep 24, 2023

Plugin for binance.us csv tax report #65

Are you sure you want to change the base?

Plugin for binance.us csv tax report #65

Conversation

iloveitaly commented Aug 1, 2022

eprbell commented Aug 1, 2022

macanudo527 commented Aug 3, 2022

eprbell left a comment

Choose a reason for hiding this comment

eprbell Aug 3, 2022

Choose a reason for hiding this comment

eprbell commented Aug 7, 2022 • edited

macanudo527 commented Aug 11, 2022

eprbell commented Aug 12, 2022

iloveitaly commented Aug 13, 2022

eprbell commented Aug 14, 2022

iloveitaly commented Aug 16, 2022

eprbell commented Aug 16, 2022

TheGreatJoules commented Apr 7, 2023

eprbell commented Apr 7, 2023 • edited

TheGreatJoules commented Apr 7, 2023

eprbell commented Apr 7, 2023

iloveitaly commented Sep 17, 2023

eprbell commented Sep 24, 2023

eprbell commented Aug 7, 2022 •

edited

eprbell commented Apr 7, 2023 •

edited