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

Incorrect proceeds because sale price not used #65

Open
Zburatorul opened this issue Sep 15, 2021 · 3 comments
Open

Incorrect proceeds because sale price not used #65

Zburatorul opened this issue Sep 15, 2021 · 3 comments

Comments

@Zburatorul
Copy link

Consider the following ledger

2021-01-02 * "Buy 1 HOOL"
  Assets:Cash
  Assets:HOOL   1 HOOL {10 USD}

2021-01-10 price HOOL  15 USD

2021-01-13 * "Sell HOOL"
 Assets:Cash
 Assets:HOOL -1 HOOL {} @ 20 USD 
 Income:Gains  -10 USD 

The correct proceeds from the sale are clearly 20 USD.
The SQL query in the recently_sold_at_loss method uses the CONVERT method to compute the proceeds from the 01-13 sale.
However by inspection CONVERT, while lives at line 153 of core/convert.py, looks up the most recent price available rather than the sale price (which it does not have access to anyway). In the absence of a price entry that exactly agrees with the sale price, the proceeds are incorrect.

Furthermore, if no price entry exists for that commodity up until the sale date, an exception is raised because CONVERT gives up on the conversion and reports the proceeds in terms of the commodity sold, rather than USD. This leads to the Inventory object having multiple positions.

@Zburatorul Zburatorul changed the title Incorrect proceeds Incorrect proceeds because sale price not used Sep 15, 2021
@redstreet
Copy link
Owner

Makes sense, and thanks for filing this. This probably also affects cases where multiple sales are made on the same day at different prices.

I haven't looked yet, but I assume we'd have to either replace the CONVERT and do the conversion ourselves, or replace the SQL itself. If you've taken a look, ideas/PR welcome.

@Zburatorul
Copy link
Author

Zburatorul commented Oct 10, 2021

I don't know how to fix this through SQL alone, so I've only looked at doing the conversion ourselves.
In the process I ran across another issue upstream in the logic: incorrect proceeds for wallet to wallet transfers of crypto, or any in-kind transfers.

2021-01-02 * "Buy 1 HOOL"
  Assets:Cash
  Assets:Exchange:HOOL   1 HOOL {10 USD}

2021-01-03 * "Transfer from exchange to wallet"
  Assets:Exchange:HOOL   -1 HOOL {2021-01-02}
  Assets:Wallet:HOOL          1 HOOL {10 USD, 2021-01-02}

afaik, from IRS's point of view no gains are realized (unless perhaps there is a small transaction fee as common for crypto), but the SQL query will record 10 USD in proceeds. The issue of course is that number < 0 is not a sufficient criterion for distinguishing a sale from a transfer.
How do we easily identify the right subset of transactions?
It seems one has to look through every posting of the transaction and see if the currency is HOOL or something else.
If all postings contain only HOOL, then it's a zero-fee transfer and should be ignored.
If any posting involves something other than HOOL, then the transaction is at least partially a sale (possibly immediately followed by a buy if currency != USD). It's hard to see the general logic. The two simplest possibilities are:

Assets:Exchange:HOOL -1 HOOL  {2021-01-02} @ 15 USD ;today's price
Assets:Wallet:HOOL       0.99 HOOL {10 USD, 2021-01-02}  ;inherit original cost basis
Expenses:Finance          0.15 USD ; actual fee paid given today's price and HOOL amount paid as fee
Income:Realized

This should be handled as a sale of 0.01 HOOL.

Assets:Exchange:HOOL -1 HOOL  {2021-01-02} @ 15 USD ;today's price
Assets:Exchange:BAR     7 BAR {2 USD}
Expenses:Finance            1 USD ; actual fee paid
Income:Realized               -5 USD; gains of HOOL

This should be handled as a sale of 1 HOOL.
@redstreet, do you concur?

@redstreet
Copy link
Owner

Agreed. Thanks for fleshing this out. I'd noticed this long ago, and probably should've included a note somewhere at least. Yes, recently_sold_at_loss could benefit from addressing both these edge cases, and will probably require going away from SQL. PR welcome.

Context for anyone reading along: this last point is an edge case only affects the "What not to buy" analysis by sometimes producing false positives.

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

No branches or pull requests

2 participants