Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Kraken Margin Trading #120
base: main
Are you sure you want to change the base?
Kraken Margin Trading #120
Changes from 7 commits
6371b45
4df06c6
c587456
4146d77
1c1622e
82dc726
c8eec27
f68171c
24d5b76
5f3f3b1
2ce428a
d5c2f5b
a3d980b
8434f60
c8f023a
013f7b2
f0345c2
6b6c5b4
ee16bd5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When possible, it might be nicer to have classes
MarginStart
,MarginFee
,MarginClose
instead.MarginClose
should have afees: list[MarginFee]
variable. The fees must be linked to the MarginClose for tax evaluation.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The thing is that Kraken logs do not provide starts and closes of margins. Instead, the already computed gains or losses are logged. Therefore, we don't have to / can't compute the gains/losses ourselves.
An example can be found here: #97
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the labels from
InterestReportEntry
make sense here? All your MarginGain/MarginLoss.change are positive. So that the report should state that you always receive ("Erhalten am") coins. I believe that you have to update the labels. Also "Wert in EUR" doesn't make sense as we do not evaluate the value of the "bought" coins.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to
TaxReportEntry
."Wert in EUR" makes sense for gains/losses/fees received/paid in crypto coins. However, since it's the same as "Gewinn/Verlust in EUR", I removed the column.
Not sure about the date (as it could be gain or loss), what do you think about "Erhalten oder ausgegeben am"?