Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transaction log #37

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Transaction log #37

wants to merge 11 commits into from

Conversation

mdavid217
Copy link
Contributor

No description provided.

@eprbell
Copy link
Owner

eprbell commented May 13, 2022

I finally managed to commit the changes needed to distinguish default output plugins from non-default ones (default ones are specified here). The user can now define an optional generators section in the configuration file, listing the generator plugins they want to run. If this section is not specified, the default generators are ran.

I also took a look at your plugin: it's a good attempt at unifying everything, but I think it might be a bit confusing to users. The main problem I see is that the header has to make room for semantically incompatible transaction fields: while in and out transactions can be managed with amount and sign, intra ones need different fields altogether like sent/received. While the intent is clear to us, who are writing the software, I'm afraid it may be bit non-intuitive to non-developers.

I like the aspiration though. I wonder how others have solved this problem: there may be a CSV format out there that attempts to do this. Perhaps splitting intra transactions into in and out? Not sure: that may be non-intuitive in different ways. Just spitballing...

@mdavid217
Copy link
Contributor Author

mdavid217 commented May 13, 2022

I got a chance to look at the plug-in config code and it looks great. Will test it tonight.

I agree about the intra transactions; the same thoughts have gnawed at me as well that there's probably a better way to represent them. Initially I hesitated to turn one action into two rows, but the more I think about it, the more I feel it is probably the cleanest way to include them. I'll play with that a bit and see how it looks.

@eprbell
Copy link
Owner

eprbell commented May 13, 2022

Here's another idea. Maybe let's get rid of all the existing Crypto and USD fields and try with:

  • Crypto In, Crypto Out, Crypto Fee
  • USD In, USD Out, USD Fee

This way you can capture In transactions (use In and Fee fields), Out transactions (use Out and Fee fields) and Intra transactions (use In, Out and Fee fields). It's just an experiment, but I'm curious to see how it looks.

@mdavid217
Copy link
Contributor Author

Definitely worth a look. I added the two additional styles. Of the three, I'm currently liking Style 2 the most so far.

@eprbell
Copy link
Owner

eprbell commented May 14, 2022

Looking more and more promising! Style 2 is compact, but it may be a bit cryptic for users:

  • it doesn't have crypto data (except fee),
  • some fields are hard to parse (e.g. "Bob" vs "From Bob").

Style 3 is interesting but it is missing some info:

  • transactions should always have fiat fields filled out, unless fiat information is actually not available.

Finally, a few general comments:

  • perhaps it would be even easier to read if we merged the exchange and holder columns into one column with a format like Coinbase / Bob -> BlockFi / Alice (for intra) or Coinbase / Bob for in and out: this format has the advantages of being both compact and clear to read;
  • transaction type should have a "direction / type" format similar to the one in the tax_report_us output: In / Buy, Out / Sell, etc;
  • add unique id (to identify transactions): all DaLI-generated transactions have this field;
  • add notes (for user-provided context).

@mdavid217
Copy link
Contributor Author

Looking more and more promising! Style 2 is compact, but it may be a bit cryptic for users:

* it doesn't have crypto data (except fee),
* some fields are hard to parse (e.g. "Bob" vs "From Bob").

For the first bullet, I do believe all information is accounted for. For buys or sells, the crypto amount reflects the value (positive in, negative out) and for moves, the crypto in/out are on two different rows while the fee is present on the out side.

I agree with the second bullet. I could try adding a suffix instead, e.g. instead of "From Bob" I could show "Bob (Out)" or "Coinbase / Bob (Out)" in the case where the output is changed from having discrete exchange and holder fields to a single "Exchange / Holder" field.

Style 3 is interesting but it is missing some info:

* transactions should always have fiat fields filled out, unless fiat information is actually not available.

I believe the only fiat field missing was on the style 3 transaction out side. I've added that in on my local copy. If you see any other place I missed a fiat value, please let me know and I'll get it in.

Finally, a few general comments:

* perhaps it would be even easier to read if we merged the `exchange` and `holder` columns into one column with a format like `Coinbase / Bob -> BlockFi / Alice` (for intra) or `Coinbase / Bob` for in and out: this format has the advantages of being both compact and clear to read;

Hmmm, yes, I do think that'd help for readability.

* transaction type should have a "direction / type" format similar to the one in the tax_report_us output: `In / Buy`, `Out / Sell`, etc;
* add unique id (to identify transactions): all DaLI-generated transactions have this field;
* add notes (for user-provided context).

I'll incorporate all of these. I particularly like the idea of adding the flow type to the transaction type.

…e id and notes.

Tweaked fiat fee: added a missing entry and set to show only if non-zero akin to crypto fee fields.
@eprbell
Copy link
Owner

eprbell commented May 14, 2022

For the first bullet, I do believe all information is accounted for. For buys or sells, the crypto amount reflects the value (positive in, negative out) and for moves, the crypto in/out are on two different rows while the fee is present on the out side.

Ah, right. And to think I'm the one who came up with the idea of splitting transactions, and yet I got confused... I feel this may be too hard to read for regular users (and myself, apparently :-D )

I believe the only fiat field missing was on the style 3 transaction out side. I've added that in on my local copy. If you see any other place I missed a fiat value, please let me know and I'll get it in.

Initially I was thinking of missing fiat fields in intra transactions (in style 3), however, on second thought, that might be OK because fiat fields on intra transactions are probably more distracting than not (just leave the fiat fee though).

@eprbell
Copy link
Owner

eprbell commented May 17, 2022

I'll incorporate all of these. I particularly like the idea of adding the flow type to the transaction type.

Cool!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants