eprbell / dali-rp2

DaLI (Data Loader Interface) is a data loader and input generator for RP2 (https://pypi.org/project/rp2), the privacy-focused, free, open-source cryptocurrency tax calculator: DaLI removes the need to manually prepare RP2 input files. Just like RP2, DaLI is also free, open-source and it prioritizes user privacy.
https://pypi.org/project/dali-rp2/
Apache License 2.0
63 stars 42 forks source link

Transaction Resolver and Unique Id #85

Open topcoderasdf opened 1 year ago

topcoderasdf commented 1 year ago

When populating unique_id, if it is not a blockchain address, I think we should use <plugin>::<id>, instead of current <id> format just to be safe. While it is very unlikely to happen, there could be a case where two different transactions from two different exchanges end up sharing an identical internal id.

eprbell commented 1 year ago

This already came up when designing the unique id logic. There are 2 problems with this proposal:

topcoderasdf commented 1 year ago

For the first point, I don't quite understand why we need to programmatically distinguish transaction hashes from internal ids. Current design already takes either internal ids or hashes.

For the second point, I actually would like to know whether Coinbase Pro api and Coinbase api assign same internal id for off-chain transfers between Coinbase and Coinbase Pro. My guess is that they don't as every transaction will get a unique internal id. I can't verify it right now as I have just deposited money into my Coinbase Pro account I will need to wait a few days for my pending deposits to be transferrable. So I think it would be nice if someone can actually check if current dali-rp2 is able to resolve off-chain transfer between Coinbase and Coinbase Pro without transaction hints.

If Coinbase and Coinbase Pro actually do assign same internal id, I think we can still create a mapping table that instructs transaction resolver to treat coinbase_pro and coinbase as basically the same plugin during the resolving process. I don't think there will be that many exceptions similar to Coinbase and Coinbase Pro like off-chain transfer. The vast majority will be on-chain transfers and for those we don't add <plugin>.

This is not an urgent problem, but I am mentioning this just for peace of mind

eprbell commented 1 year ago

The vast majority will be on-chain transfers and for those we don't add <plugin>.

This means you need a way to distinguish on-chain transfers from other kinds of transfers (which was my first point).

If Coinbase and Coinbase Pro actually do assign same internal id, I think we can still create a mapping table that instructs transaction resolver to treat coinbase_pro and coinbase as basically the same plugin during the resolving process. I don't think there will be that many exceptions similar to Coinbase and Coinbase Pro like off-chain transfer.

Regarding the second point, the CB-CBPro scenario was just an example: the issue is that adding a prefix to the unique id breaks the transaction resolver. Adding a mapping table goes against the design principle that data loaders are self-contained and independent of each-other.

I appreciate the discussion though: if we find a way to do this that doesn't violate data loader independence and doesn't break the transaction resolver we can consider/expolre it.