Resolve Withdrawals/Deposits

[Griffsano]

Closes #4

Dependencies that should be finished first

• Close PR Detailed export #98: This PR contains many changes in the balancing/taxation loop. Merging would be easier if this is done first. Once we separate balancing and taxation in two loops, we might get many merge conflicts and have to resolve them manually . However, not strictly required.

To-do list

• Split up balancing and taxation in two different loops
• Add additional variable acquire_platform or similar to operations
• Change price fetching logic accordingly (use the correct acquire_platform or platform)
• Implement read-in of config files to match withdrawals and deposits (similar to Airdrop #115)
• Resolve deposits and withdrawals based on matching file
• Move coins between platforms, balancing for each platform (happens before taxation)

[jhoogstraat]

If you provide a rough outline of how the solution is going to look, I am more than happy to help to implement this feature.

[provinzio]

Hey @jhoogstraat, awesome that you want to help. Does this give you the idea?

[Griffsano]

Hey @jhoogstraat, thanks for helping! I added a to-do list in the top-comment above, based on the discussion in #4. Let me know if this makes sense to you.
I think the most challenging part is resolving the deposits/withdrawals, as you need to consider all platforms simultaneously. Idea: Balance each platform until a deposit occurs, then pause and start/continue with the platform of the matching withdrawal (you can't continue after the deposit until you know which exact coin was transferred and where it originates).
I can implement the read-in of the deposits/withdrawals matching files, as I already have a concept for this and did something similar for the Airdrops.

[jhoogstraat]

You can find the work in progress on my fork
It does not work in the current state, but at least the general idea should be clear.
I must admit that with the limited time I had with the code, it is more difficult to understand than I thought.

Hopefully I got the distinction between "balancing" and "taxation" right. As I said, this is just a rudimentary implementation that is missing many things.

The approach basically goes through each platform and memorizes the "sold coins" when a withdrawal emerges (aka the reduction of the balance). In a second loop, the sold coins from the withdrawals are appended to the associated deposit. So deposits are regarded as buys and withdrawals as sells on the respective balance queues.
After that, it's the same taxation evaluation as before. What's also missing currently is the calculation of the correct gains of deposited coins (when they are sold).

[provinzio]

I think it should be possible to balance the platforms and evaluate the taxation at the same time.

Implementing this in the evaluation requires this to be easily refactor able in case we want to evaluate the taxation for another country.

Is it necessary to add the new DepositMatch class instead of adding a new variable to all coins, which states where the coin is currently (besides the old variable for where the coin was bought)?

[jhoogstraat]

Wasn't the goal to separate the balancing from taxation? Or what do you mean by "at the same time"?

Should the withdrawals/deposits better be evaluated beforehand (in Book, altering the list of operations?).

The new class is not strictly necessary. It currently just serves as a container for the rows in the wd.csv file.
It allows to uniquely identify the related operations (deposit and withdrawal), so the operations can be matched.

[provinzio]

Wasn't the goal to separate the balancing from taxation? Or what do you mean by "at the same time"?

Should the withdrawals/deposits better be evaluated beforehand (in Book, altering the list of operations?).

The new class is not strictly necessary. It currently just serves as a container for the rows in the wd.csv file. It allows to uniquely identify the related operations (deposit and withdrawal), so the operations can be matched.

Good points. Keep on the good work.

[jhoogstraat]

Thanks!
Maybe I can clarify the thought process a bit more. Please correct me, if I am wrong.

The goal is for every operation to specify which coins exactly are exchanged via FIFO or LIFO queue. That would be equal to a tuple[Operation, list[CoinSold]] for each operation.
That is the "balancing" in my understanding. After that the "taxation" goes though all tuples and can calculate the p/l of the operation.

That is basically how it works now, not respecting staking and potentially other operations.

[provinzio]

I think balancing means the part when the buys/sells are evaluated.

E.g. buy 2 coins. Sell 1 coin. Sell 1 coin. Has operations buy(2), buy(1), sell(1), withdraw(2).
And another platform might have deposit(2), sell(2).

The missing information is the link between the withdraw and deposit operation. So our first goal is to match deposit and withdraw operations and the second goal is to identify the coins with buy times and buy prices. The information about the moved coins could be saved in the deposit operation.

On tax evaluation. The withdraw operation will remove coins from the queue (on withdraw. We do not care where the coins get moved to). The deposit operation will add/"buy" the coins which where identified earlier with the buy times and buy prices. This might be your tuple of sold coins. A tuple of operation and sold coins shouldn't be necessary as the sold coins know their operation (not sure about this).

[jhoogstraat]

Thanks for clarifying.
I think my current implementation is not far off from the solution discussed in #4.
I am evaluating the operation chain in a separate evaluate_coin method, before localized taxation kicks in.
By doing this, I was able to slim down the localized taxation method quite a bit, as it does not have to evaluate the chain itself, but just the profits/losses of each operation (its single responsibility). This leads to fewer sources of errors.

Withdrawals are handled as "selling" the coin on the specific platform queue, while deposits are just "buys" on the queue.
To evaluate withdrawals/deposits, it's just going through all operations and updating the specific coins transferred for deposits.
Maybe that can be handled inside the first iteration through all operations, but I am not sure on that.

As history cannot be changed, we could even cache the balancing to disk.

[provinzio]

Saving the withdrawal/deposit links might be a good idea. But keep in mind, that I could add a missing account statement after the first run. So the saved link should also save the latest transaction. If an even older account statement file is added later on, the links might have to be recalculated.

[provinzio]

some more ideas from me to the current implementation and my current favorite approach

I think that it's quite rare, that you have withdrawal/deposits in your history, which can not be matched automatically/easily

e.g.
deposit 5 btc requires a withdrawal of 5 btc + maybe fees

in my case: i rarly withdraw/deposit coins. for me it might be enough to just search for a deposit/withdrawal match by coin in an appropiate time frame.

using a file to match deposit/withdrawals is in my eyes over the top. i'd recommend to stick to a basic matching algo.

# get all deposit/withdrawal operations
# group by coin
# sort by time

If deposit/withdrawals come in the pattern: withdrawal, deposit, withdrawal, deposit; matching should be trivial. (check the change to be sure)

[jhoogstraat]

I agree. I'll implement the matching algo in a new branch first. After that we can implement the balancing part. Especially because you are refactoring the BalanceQueue right now.

That means I'll implement book.resolve_deposits() and save the source platform on the deposits. Is it really enough to known the source platform when handling a deposit during balancing?

[provinzio]

I agree. I'll implement the matching algo in a new branch first. After that we can implement the balancing part. Especially because you are refactoring the BalanceQueue right now.

Balancing won't be an issue as soon as the matching is done. I'll add the balancing and evaluation afterwards.

Is it really enough to known the source platform when handling a deposit during balancing?

I'd recommend to save the Withdrawal operation to each Deposit. We'll require much more information than just the source platform. Also the handling will be much easier.

[provinzio]

...and raise a warning when a Withdrawal matches to no Deposit. Make sure that each Withdrawal matches only to one Deposit, raise a warning when a Deposit has no Withdrawal.

[jhoogstraat]

Ok, I implemented the most basic matching algorithm I could think of (matching attributes 'coin' and 'change'). We can build on this now.
Branch: https://github.com/jhoogstraat/CoinTaxman/commits/resolve-deposits-2

Does anyone have statements with some transfers in them? Doesn't matter if they are made up, as long as all transactions are valid.

Transfers between coinbase and coinbase_pro are not reported in the respective transfers, but also feeless and thus tax-irrelevant.
But how is per-platform balancing done then?

3rd, warning are almost invisible using the debug log level. At least for me.

[provinzio]

Difficult question. Let's keep the tax evaluation separate from this PR.

Might be enough to create your own transaction history. Make sure to account for withdrawal fees. E.g. withdrawal 100. Deposit 97

So we will need a matching Algo which looks for same coin and the same change or a bit lower change

[jhoogstraat]

Maybe coinbase and coinbase_pro can be considered the same "platform" from a legal perspective? What makes a platform a platform?

Anyhow, yes, the change might be lower for the deposit, and we have to account for that. I don't know common fee rates, so maybe go with withdrawal change - 5% as a minimum?

[provinzio]

Yeah why not. Just keep something like that or lower.

If it's not enough. Issues will be raised.
Perhaps start with 1%

[jhoogstraat]

Done. How would you like to merge this?

[provinzio]

Might be easiest when merged into my working branch instead of master.

[jhoogstraat]

The match-fees branch? I can rebase and squash my changes onto the branch, so you can just cherry pick the commit. I can also create a new pr if you prefer.

[provinzio]

Yes.

Whatever you like most. You can reuse this branch if you like rebasing or resetting this.

[jhoogstraat]

I am not the owner of this branch, so I don't think I can do that.

[provinzio]

In that case, just create a new branch.