provinzio / CoinTaxman

Calculate your taxes from cryptocurrency gains
GNU Affero General Public License v3.0
142 stars 31 forks source link

[bug] Not enough BNB in queue to sell #61

Closed igi01 closed 2 years ago

igi01 commented 2 years ago

2021-08-15 12:06:36,024 book INFO Reading file from exchange binance at X:\CoinTaxman-main\account_statements\part-00000-154c5b63-ff2c-4615-9ed2-4d89319596e4-c000.csv 2021-08-15 12:06:36,180 book WARNING I may have missed a remark in X:\CoinTaxman-main\account_statements\part-00000-154c5b63-ff2c-4615-9ed2-4d89319596e4-c000.csv:2584: Withdraw fee is included. 2021-08-15 12:06:37,588 book WARNING I may have missed a remark in X:\CoinTaxman-main\account_statements\part-00000-154c5b63-ff2c-4615-9ed2-4d89319596e4-c000.csv:26726: Withdraw fee is included. 2021-08-15 12:06:37,588 book WARNING I may have missed a remark in X:\CoinTaxman-main\account_statements\part-00000-154c5b63-ff2c-4615-9ed2-4d89319596e4-c000.csv:26754: Withdraw fee is included. 2021-08-15 12:06:37,728 book INFO Reading file from exchange binance at X:\CoinTaxman-main\account_statements\part-00000-21a78b20-5388-477e-be15-b0617f4cb439-c000.csv 2021-08-15 12:06:38,102 book WARNING I may have missed a remark in X:\CoinTaxman-main\account_statements\part-00000-21a78b20-5388-477e-be15-b0617f4cb439-c000.csv:6259: Withdraw fee is included. 2021-08-15 12:06:38,102 book WARNING I may have missed a remark in X:\CoinTaxman-main\account_statements\part-00000-21a78b20-5388-477e-be15-b0617f4cb439-c000.csv:6260: Withdraw fee is included. 2021-08-15 12:06:38,102 book WARNING I may have missed a remark in X:\CoinTaxman-main\account_statements\part-00000-21a78b20-5388-477e-be15-b0617f4cb439-c000.csv:6261: Withdraw fee is included. 2021-08-15 12:06:38,682 book INFO Reading file from exchange binance at X:\CoinTaxman-main\account_statements\part-00000-fc60df31-720b-487d-8fe5-e37f5b43deeb-c000.csv 2021-08-15 12:06:38,682 taxman DEBUG Starting evaluation... 2021-08-15 12:06:38,870 taxman ERROR part-00000-154c5b63-ff2c-4615-9ed2-4d89319596e4-c000.csv: Line 545: Not enough BNB in queue to sell (transaction from 2021-03-19 08:44:39+00:00 on binance) This error occurs if your account statements have unmatched buy/sell positions. Have you added all your account statements of the last years? This error may also occur after deposits from unknown sources.

Traceback (most recent call last): File "X:\CoinTaxman-main\src\main.py", line 44, in main() File "X:\CoinTaxman-main\src\main.py", line 38, in main taxman.evaluate_taxation() File "X:\CoinTaxman-main\src\taxman.py", line 194, in evaluate_taxation self.__evaluate_taxation(coin, operations) File "X:\CoinTaxman-main\src\taxman.py", line 109, in _evaluate_taxation_GERMANY raise RuntimeError RuntimeError

2021-03-19 08:12:34,Spot,Transaction Related,BNBUP,-0.14000000,"" 2021-03-19 08:12:34,Spot,Fee,BNB,-0.00022759,"" 2021-03-19 08:12:34,Spot,Buy,USDT,79.92600000,"" 2021-03-19 08:44:39,Spot,Buy,BTC,0.00208771,"" 2021-03-19 08:44:39,Spot,Fee,BNB,-0.00034544,"" 2021-03-19 08:44:39,Spot,Transaction Related,BNB,-0.46000000,"" 2021-03-19 09:04:33,Spot,Transaction Related,BTCUP,-0.01000000,"" 2021-03-19 09:04:33,Spot,Transaction Related,BTCUP,-3.28000000,"" 2021-03-19 09:04:33,Spot,Fee,BNB,-0.00175041,"" 2021-03-19 09:04:33,Spot,Buy,USDT,620.02496000,"" 2021-03-19 09:04:33,Spot,Buy,USDT,1.89032000,"" 2021-03-19 09:04:33,Spot,Fee,BNB,-0.00000533,""

I tried manually to fake a BNB deposit in this history to try to fix this error:

UTC_Time,Account,Operation,Coin,Change,Remark 2021-02-11 08:57:34,Spot,Deposit,BNB,1.10664599,"" 2021-02-11 13:20:51,Spot,Commission History,BNB,0.00002547,"" 2021-02-11 18:32:00,Spot,Commission History,BNB,0.00003643,"" 2021-02-12 13:01:54,Spot,Commission History,BNB,0.00001155,""

however this was unsuccesful:

2021-08-15 12:16:03,675 taxman ERROR part-00000-154c5b63-ff2c-4615-9ed2-4d89319596e4-c000.csv: Line 546: Not enough BNB in queue to sell (transaction from 2021-03-19 08:44:39+00:00 on binance)

Is there a fix for this bug or do you have some suggestions, what i can try?

provinzio commented 2 years ago

Please check all your transactions with BNB and validate that at all times your amount of BNB is positive (you can not sell what you don't have).

scientes commented 2 years ago

also deposits and withdrawhals do not alter the process to "fake" ist you would need a buy

sebastianelsner commented 2 years ago

I am testing out this tool and I have basically the same issue, but with USDT. I know all history is there. But "make run" errors here:

Line 242: Not enough USDT in queue to sell (transaction from 2021-08-23 07:52:11+00:00 on binance)

2021-08-23 07:52:11 Spot Fee BNB -0.00002
2021-08-23 07:52:11 Spot Sell USDT 123.8
2021-08-23 07:52:11 Spot Sell EUR -105.8

As far as I understand this csv the data here is correct and reflects what I did: I "Sell" "EUR" for "USDT" on the "Spot" exchange. I did this on the "pair" on the exchange NOT in the "convert" or "buy crypto" pages, since the fees are lower on the spot. could this be the issue?

I am willing to debug myself, if you could give any pointer if this should work with this tool or maybe where to start.

provinzio commented 2 years ago

The error is telling you that you sell usdt which you don't own according to the given history. Please share a minimal history example, so we can help you.

scientes commented 2 years ago

@provinzio the error is actually that the sell is both times logged as sell but one positive one negative, and the only distinction is the - and i think he bought usdt with eur

sebastianelsner commented 2 years ago

Yes, correct , sorry if this was not clear. The part above is basically the only relevant section since there is nothing before (only the euro deposit) :D .

related thought:

I kinda thought the "Buy"/"Sell" column are not indicative of anything and mostly "informative" since paying for a coin with USDT is like:

2021-08-28 20:45:03 | Spot | Buy | USDT | -49.99202000

Its a "Buy" but its negative.

scientes commented 2 years ago

That means we can't trust binance with the buy sell but need to Look if the transaction is positive or negative

scientes commented 2 years ago

https://github.com/provinzio/CoinTaxman/blob/094748cbc1d6b1bd5842ea7d6aa032769dd2e807/src/book.py#L105

dedentation of this line should fix this but i havent tested it yet

sebastianelsner commented 2 years ago

That seems to do the trick. Now it is running through.

sebastianelsner commented 2 years ago

Thank you :)

sebastianelsner commented 2 years ago

Fixed it in my fork. Open for a PR if you want to. https://github.com/sebastianelsner/CoinTaxman/commit/53d571794daf8d343ae50b889d9630b8953f8b01

jhoogstraat commented 2 years ago

I still got a Not enough USDT to sell.

Might this be correlated? I am sure I imported all Statements.

Not sure how to debug this issue.

provinzio commented 2 years ago

I still got a Not enough USDT to sell.

Might this be correlated? I am sure I imported all Statements.

Not sure how to debug this issue.

Delete your deposit/withdrawals, check all usdt operations and sum them up in excel. If you think that everything should be fine, you can post a minimal as possible example of your transaction statement. E.g. cumulate all buy and sell positions prior to the error.

jhoogstraat commented 2 years ago

I extracted all USDT related lines (via regex) and summed them up in excel. The result almost comes down to 0 (0,25). Deposits were done in EUR and do not change the result. Coinbase does not list deposits explicitly.

I am unsure how to cumulate all buy and sell positions, as there are many types of entries (Transaction Related, Commission Fee Shared..., Buy, Fee).

provinzio commented 2 years ago

If you sum up all entries up to the point where the error occures, the sum should be negative.

Other idea: add a print message each time an usdt transaction is handled in Taxman and check what type of operation with which value has which effect.

scientes commented 2 years ago

@jhoogstraat deposits are also ignored currently in the calculation, because they are irrelevant for taxes,

igi01 commented 2 years ago

also deposits and withdrawhals do not alter the process to "fake" ist you would need a buy

Okay so if i hodled BNB over one year (tax free) and then actively traded with this (XX/BNB), this will not work? I don't really get why deposits are ignored. If I fake a buy this would mean, that this will be a taxable event, isn't it?

scientes commented 2 years ago

Deposits are not an taxable event and not needed for the tax calculation. The premise is that even if you deposit something into the exchange you have bought or earned it somewhere else. To calculate the taxes instead of the deposit we need the amount and timestamp when you bought the assets that you deposited into the exchange. The taxes are calculated on price change between the buy and the sell only.

igi01 commented 2 years ago

Yes i get the problem. However i still think deposit of tax-free coins would be nice to consider in future. For example: a friend gave me 0.1 btc, which i then did not touch for one year. So this btc is now tax free, isn't it? This 0.1 btc will now be deposited to binance and i want to actively trade different altcoins against btc, which will be a taxable event now.

scientes commented 2 years ago

yes there is an issue for a custom csv where things like that can be added, It is just not implemented yet

igi01 commented 2 years ago

Okay nice to know. Thanks :)

scientes commented 2 years ago

For the meantime changing that particular deposit into a buy would be a patch for now, or just create a new binance like file and copy that line there and change it to a buy

and the issue mentioned is #55

jhoogstraat commented 2 years ago

Well, I have compared the csv data with the data in the transaction history on binance. I have to say that the csv reporting is not in the chronologically correct order. The deduction of usdt is reported before it is available from selling another coin for usdt.

Or does this tool already take this into account?

provinzio commented 2 years ago

Interesting, so both operations have the same timestamp but are ordered so that Taxman does not work properly?

Or do you mean that the timestamps are wrong?

Or does this tool already take this into account?

I am not sure what you mean by deduction.

scientes commented 2 years ago

Operations are sorted by timestamp before evaluating taxes: https://github.com/provinzio/CoinTaxman/blob/dcf3f21594a2b867a156cf1e754283c9f63676ce/src/taxman.py#L193

Thus the Order of ocurrence with in the csv should not matter.

jhoogstraat commented 2 years ago

Interesting, so both operations have the same timestamp but are ordered so that Taxman does not work properly?

Or do you mean that the timestamps are wrong?

Or does this tool already take this into account?

I am not sure what you mean by deduction.

Exactly. Everything happened within one second, so the csv reports the same timestamp for all operations. The timestamps are correct, but not precise enough to guarantee correct chronological order. I don't know why the csv reports the wrong chronology though. Exporting the trade history shows the correct history of events (it is much more readable aswell).

With deduction I meant that Transaction Related,USDT,-14 is reported before Buy,USDT,51,"" and so not enough usdt is available (in time).

Edit: When exporting the Trade History up to Past 6 Months a .xlsx is generated. This file reports everything correctly (Selling a coin for usdt and then buying another coin with those usdt).

When exporting the same Trade History using Beyond 6 months - Custom a .csv is generated. This file reports all events in the same wrong order as the Generate all statements-csv.

I think binance is messing up here.

scientes commented 2 years ago

Yes binance Exports are sadly far from perfect , so we should maybe need an extra ordering step to ensure that buys always Happen before sells if its the Same timestamp

provinzio commented 2 years ago

@jhoogstraat Please consider to report your bug at binance

provinzio commented 2 years ago

@scientes we could sort the timestamps by time and secondly by the operation type to make sure gains come before losses. What do you think about it?

scientes commented 2 years ago

Yes we should so that

provinzio commented 2 years ago

There we go. The commit https://github.com/provinzio/CoinTaxman/commit/41d4bd6e1af23dd07cf117256e18217e4597bca6 should fix your issue @jhoogstraat.

jhoogstraat commented 2 years ago

Thanks, its working now!

One question though. As far as I understood the code, buying something with fiat is always possible, even if there is no fiat in the queue (makes sense, as you have to deposit fiat initially). But shouldn't deposits of other coins be put onto the balance queue? Otherwise deposited coins are ignored and lead to "Not enough XYZ in queue" when selling more than you have bought with fiat. Maybe my thought is wrong, but I am getting this error now.

I tried

elif isinstance(op, transaction.Deposit):
    balance.put(op)

which resulted in another "Not enough XYZ" even earlier in the csv.

scientes commented 2 years ago

Deposits are ignored, because they are not the date you bought these coins. For the tax calculation only the date the coin came into your possession is relevant (and this counts for all coins you own and not only those present on our exchange) and not the date when you deposited them into the exchange. Due to this deposits have to be ignored, if you have a deposit that come from another exchange you need to also import the data from that exchange and if you have deposits that come from an external source, you need to find the date they came into your possession on the blockchain and make a put operation in the balance queue with that date

provinzio commented 2 years ago

To determine which operations are relevant, you have to check if it is a taxable event.

Moving money from A to B ist not taxable.

Buying stuff with your home fiat (e.g. EUR for Germany) is not taxable. Selling stuff with other fiats/coins (e.g. USD or BTC in Germany) is not taxable, and buying is kind of taxable because you are giving the other fiats/coins away and might have generated taxable gains with that (relative to the EUR ratio).

To determine the taxable amount you have to check whether you generated gains or loses. To know that you have to combine the buy amount and sell amount in converted to your home fiat (e.g. EUR).

Example:

Buy 1 BTC with 1 EUR. Sell 1 BTC for 10 EUR.

--> 9 EUR gains. But is it taxable? Check if you sold it before or after 1 year.

Your scenario

I have 1 BTC Sell 1 BTC for 10 EUR

How much have you gained by it? You can not tell, because you are missing information about the origin (Airdrop, Interest, Bought (when for how much EUR?)) of your 1 BTC. To just know that it was deposited might not be enough information. And it will not be enough information. Because you have to know the buy time.

another example

your information: 1 BTC for 1 EUR 1 BTC deposited from somewhere else. Sell 1 BTC for 10 EUR

you might think that you have made 9 EUR gains. But you might have lost money with this trade when in reality it looks like this

1 BTC for 20 EUR somewhere else 1 BTC for 1 EUR 1 BTC deposited from somewhere else. Sell 1 BTC for 10 EUR

You can see here. That you lost 10 EUR instead. To bring this example to the next level: the time between your first transaction (1 BTC for 20 EUR) might be longer than 1 year ago. So you loss of 10 EUR might not be taxable at all.


Summarizing. you have to know when and how much of a coin you have become or have given. Depositing it from yourself to yourself doesn't change that it's your own property. You can not gain/loose something if you move it from your left to your right hand.

jhoogstraat commented 2 years ago

Thanks for explaining. I already guessed that the date a coin came into my possession must be relevant.

Sadly this realisation does not make the error go away... Maybe the numbers are rounded or something, I don't know. Looking at the data online does show that I sold more coin than buy - fees gives (like 0,01 EOS more).

Edit: It has to do with shared commissions. Maybe with the current sort order. I'll look into it later.