eprbell / rp2

Privacy-focused, free, open-source cryptocurrency tax calculator for multiple countries: it handles multiple coins/exchanges and computes long/short-term capital gains, cost bases, in/out lot relationships/fractioning, and account balances. It supports FIFO, LIFO, HIFO and it outputs in form 8949 format. It has a programmable plugin architecture
https://pypi.org/project/rp2/
Apache License 2.0
256 stars 42 forks source link

Few minor suggestions #75

Open topcoderasdf opened 1 year ago

topcoderasdf commented 1 year ago

Thank you for this library, I've found it extremely useful. I just have few minor suggestions


Fee only transactions are introduced to address issues #16 and #4. I was wondering whether something like fee, fee_currency, fee_fiat_value, fee_fiat_value_currency could be used instead. With this, there is no need to create two transactions for a single transaction. Also, I found current fee structure design slightly confusing as if I am correct, one can pass either only crypto_fee or fiat_fee, but not both, which is a deviation from the current library standard. Throughout the library, the keyword fiat is used to address fiat value and both crypto and fiat related params get populated. E.g crypto_in and fiat_in related params all get populated instead of either only crypto_in or fiat_in getting populated. I think using something like fee, fee_currency, fee_fiat_value, fee_fiat_value_currency, will help the fee structure also adhere to the current practice and make the code easier to follow.


Currently there are three main keywords, asset, crypto, and fiat. However, I think using only asset and fiat seems quite fine. So I was wondering whether we could just use asset, asset_in, asset_sent, etc instead of having both asset and crypto_in, crypto_sent, etc. This will also make code easier to follow for a few corner cases where crypto_in or crypto_out gets populated even when asset is a fiat currency

eprbell commented 1 year ago

Thanks for the feedback and I'm really glad RP2 is useful to you (please consider leaving a star on RP2 and DaLI's pages, if you'd like to support these projects)!

Here are my thoughts on the issues you raised:

  1. fee parameters: fees are one of the subtlest concepts in the design of the RP2 API. Read this FAQ to learn about the different possible fee scenarios that RP2 tries to express. The guiding design principle of RP2 is: powerful, low-level primitives that can be assembled in patterns to express any tax scenario (or as many as reasonably possible). Ease of use is also a goal in RP2, but robustness and power are higher priority goals. In practical terms, and going back to your question:
    • expressing fees in a different currency to allow one single transaction instead of two. This was considered during the fee discussions, and the conclusion was that this feature would belong in a higher layer of abstraction, which belongs to a different project of the RP2 Ecosystem (see the "high-level interface" bullet point here: https://github.com/eprbell/rp2/wiki/RP2-Ecosystem). Expressing this scenario as two transactions is in line with the design principles of RP2 (powerful low-level primitives). Again, I'm very much in favor of exploring higher-level abstractions, but they should be built on top of the existing, robust, lower-level engine (I see it as layers in a network protocol, or like Bitcoin and Lightning, if you prefer a crypto metaphor :-) ).
    • the fiat fee and crypto fee are both needed in the API, because in some cases exchanges provide both values and they don't match (read the FAQ I mentioned above for more on this): in this specific case the user needs to fill in both crypto and fiat values. In general the fiat field is always required because it's needed to compute taxes, so if the fee is provided as crypto only, RP2 converts it to fiat automatically.
  2. remove crypto and leave only asset and fiat. This is a valid point, however it would be a fairly big change with many ramifications (in RP2, DaLI and automated tests) offering relatively small advantages: it would clarify the corner case of fiat being expressed as "crypto" (which, you're right, can be a bit confusing), but that case can also be addressed with documentation. This is to say: I will review PRs if somebody wants to do this, but it's not a high priority task.