Resolve ops in edge cases

provinzio / CoinTaxman

Calculate your taxes from cryptocurrency gains

GNU Affero General Public License v3.0

140 stars 31 forks source link

Resolve ops in edge cases #136

Open jhoogstraat opened 2 years ago

jhoogstraat commented 2 years ago

This is the pr for issue #135.

It's not finished yet, but I wanted to allow for feedback early on.

Implemented edge cases:

[x] Two pairs at a particular utc_time via a bridge coin
[ ] Orders not being fulfilled at once, leading to Buy/Sell ops later on (can this even happen? Buy/Sell come in pairs, no?)
[x] Commissions / Withdrawals / Deposits / Airdrop / CoinLendInterest not relevant for trade linking
[x] BNB small asset exchange at multiple utc_times (buy/sell 1 second apart for me)
[ ] Fees paid (partly) in BNB in the double_buy_sell_pair case

Currently, the is_binance_bnb_small_asset_transfer case catches almost everything that was not handled by the other cases. I tried my best to make sure that only ops that really are small asset exchanges get handled (assert that the buy coin is BNB).

jhoogstraat commented 2 years ago

I merged the two methods for resolving fees and trades into one. This reduces duplication by quite a bit. It also enables us to resolve fees easily when there are two buy/sell-pairs at any given utc_time. I know that separating the two methods is good practice, but I think in this case it adds too much duplication and complexity.

I am not sure how to account for fees paid in bnb when there is more than one buy op... I added a BUG to the code at the relevant line. How can we detect which bnb-fee op belongs to which buy op?

provinzio commented 2 years ago

I merged the two methods for resolving fees and trades into one.

Make sure that resolving fees and resolving trades can be something different. E.g. the fee could also belong to a deposit

I am not sure how to account for fees paid in bnb when there is more than one buy op... I added a BUG to the code at the relevant line. How can we detect which bnb-fee op belongs to which buy op?

Split the BNB fee according to the "value" of the sell. We can use the price in eur to estimate that

jhoogstraat commented 2 years ago

Make sure that resolving fees and resolving trades can be something different. E.g. the fee could also belong to a deposit

True. Maybe have fees done in multiple passes? What do you prefer / think is better?

Split the BNB fee according to the "value" of the sell. We can use the price in eur to estimate that

Like calculating the expected fee and find a combination of fee ops matches the calculated fee best? Fees can be quite different depending on volume on Binance. Don't know about other platforms.

provinzio commented 2 years ago

True. Maybe have fees done in multiple passes? What do you prefer / think is better?

Matching of deposit/withdrawals, coin lend/staking start and end, trading pairs and finally matching everything with fees should be possible in the same loop

In that case we might have to group and match everything before the "merge of equal operations"

jhoogstraat commented 2 years ago

Im am first going to concentrate on trades with fees. We can add lending/staking, etc. later.

provinzio commented 2 years ago

Hey @jhoogstraat,

I went through the code and did some formatting. Didn't wanted to bother you with that. I merged the newest main into the branch, fixed the linting errors and added some more assert checks.

Changes look good to me. I'd love when you could proof read my commits. Is the PR ready to be merged? Looking forward to your reply. For now, I'll mark this as draft to keep an overview. :)