probstj / ccGains

Python package for calculating cryptocurrency trading profits and creating capital gains reports
GNU Lesser General Public License v3.0
49 stars 13 forks source link

Binance Trades TPLOC incorrect #30

Closed mpmX closed 3 years ago

mpmX commented 4 years ago

First of all, thank you for this nice tool. It saved me a lot of time and I really like the reports. However, I think the Binance Trades TPLOC is not correct. In the Trade docstring is written that the buy_amount should exclude transaction fees but the current TPLOC included fees.

Current:

TPLOC_BINANCE_TRADES = { 'kind': 2, 'dtime': 0, 'buy_currency': lambda cols: currency_for(cols, 'buy'), 'buy_amount': lambda cols: [Decimal(cols[4]), Decimal(cols[5])][(cols[2].upper() == 'SELL')], 'sell_currency': lambda cols: currency_for(cols, 'sell'), 'sell_amount': lambda cols: [Decimal(cols[4]), Decimal(cols[5])][(cols[2].upper() == 'BUY')], 'fee_currency': 7, 'fee_amount': 6, 'exchange': 'Binance', }

Suggestion:

TPLOC_BINANCE_TRADES = { 'kind': 2, 'dtime': 0, 'buy_currency': lambda cols: currency_for(cols, 'buy'), 'buy_amount': lambda cols: [Decimal(cols[4])-Decimal(cols[6]), Decimal(cols[5])-Decimal(cols[6])][(cols[2].upper() == 'SELL')], 'sell_currency': lambda cols: currency_for(cols, 'sell'), 'sell_amount': lambda cols: [Decimal(cols[4]), Decimal(cols[5])][(cols[2].upper() == 'BUY')], 'fee_currency': 7, 'fee_amount': 6, 'exchange': 'Binance' }

Sample CSV data:

Date(UTC) Market Type Price Amount Total Fee Fee Coin
2020-01-03 08:01:00 BTCEUR BUY 6244.330078125 0.16014528179783097 1000.0 0.00016014528179783097 BTC
2020-01-03 08:02:00 BTCUSDT SELL 7221.52978515625 0.15998513651603313 1155.337428532822 1.1553374285328222 USDT
2020-03-27 06:51:00 BTCUSDT BUY 6692.31005859375 0.05748797654815656 384.72736370142974 5.748797654815656e-05 BTC
2020-03-30 05:30:00 BTCUSDT SELL 6167.8701171875 0.0287152442858042 177.11189713815085 0.17711189713815084 USDT
2020-05-02 23:40:00 BTCUSDT SELL 8962.650390625 0.0287152442858042 257.36469541505534 0.25736469541505536 USDT
anson-vandoren commented 4 years ago

@mm-manu The way that Binance does fees when using BNB as the payment option is that the fees are taken out of your BNB balance, and so the Amount/Total from the imports are correct as written. Making the changes you suggest would break this import when fees are paid in BNB. I've not made any transactions on Binance without paying the fees with BNB, so I'm not sure how it's handled there. Are you saying that (using the 2020-01-03 BTCEUR BUY for example) that after buying 1000 euros worth of BTC, you ended up with something less than 6244.330078125 BTC in your account? Did you have BTC there to begin with that you paid the fee from?

Since I haven't made any trades with fees paid with other than BNB, I can't investigate this further. If Binance handles the fees differently when paid with another coin/currency, we may need to have a separate TPLOC for that, since the current one does calculate correctly for BNB fees.

probstj commented 4 years ago

Thank you very much for pointing that out! And @anson-vandoren for clarifying. I am unsure myself, since I never used Binance. So when paying fees with BNB, isn't it noted in column 7 which currency was used for the fees? Maybe we could use that info to differentiate between the two cases?

mpmX commented 4 years ago

You are right, binance seems to handle it differently when fees are paid in BNB. The sample data I provided was generated, but here are my test transactions from a new account:

Date(UTC) Market Type Price Amount Total Fee Fee Coin
2020-08-26 01:22:32 BTCUSDT SELL 11314.05 0.003124 35.34509220 0.03534509 USDT
2020-08-25 19:05:18 BTCEUR BUY 9590.48 0.003128 29.99902144 0.00000313 BTC

My available USDT is now 35.30974711. That's exactly Total - Fee.

So we could write it as this:

TPLOC_BINANCE_TRADES = { 'kind': 2, 'dtime': 0, 'buy_currency': lambda cols: currency_for(cols, 'buy'), 'buy_amount': lambda cols: [Decimal(cols[4]) if cols[7] == "BNB" else Decimal(cols[4])-Decimal(cols[6]), Decimal(cols[5]) if cols[7] == "BNB" else Decimal(cols[5])-Decimal(cols[6])][(cols[2].upper() == 'SELL')], 'sell_currency': lambda cols: currency_for(cols, 'sell'), 'sell_amount': lambda cols: [Decimal(cols[4]), Decimal(cols[5])][(cols[2].upper() == 'BUY')], 'fee_currency': 7, 'fee_amount': 6, 'exchange': 'Binance' }

However this would still be incorrect in the case when "Pay fees with BNB" is disabled but you regularly buy BNB.

Just wrote with the Binance support. They confirmed that when paying fees with BNB, Total is what you actually receive, otherwise Fee must be subtracted from Total.