From 0aac4438e7f4e7a593107a6037fd3a0aa7fbd46e Mon Sep 17 00:00:00 2001 From: Sherlock <119709043+agwekixj@users.noreply.github.com> Date: Wed, 8 Jul 2026 23:07:45 -0700 Subject: [PATCH] Bug: Gain/Loss displayed in "Splits free" is not correct The gain/loss displayed for splits in the "Splits free" is historical. It may or may not be correct. Instead, the gain should be calculated against the selected lot for appropriate splits. Implements xaccSplitLotCapGain() to obtain the gain for the selected lot. --- gnucash/gnome/dialog-lot-viewer.c | 30 +++---- libgnucash/engine/Split.h | 18 ++-- libgnucash/engine/cap-gains.cpp | 142 ++++++++++++++++++++++++++++++ libgnucash/engine/cap-gains.h | 14 +++ 4 files changed, 172 insertions(+), 32 deletions(-) diff --git a/gnucash/gnome/dialog-lot-viewer.c b/gnucash/gnome/dialog-lot-viewer.c index be9c425e57..f336381e1f 100644 --- a/gnucash/gnome/dialog-lot-viewer.c +++ b/gnucash/gnome/dialog-lot-viewer.c @@ -200,15 +200,6 @@ lv_show_splits_in_lot (GNCLotViewer *lv) gnc_split_viewer_fill (lv, lv->split_in_lot_store, split_list); } -/* ======================================================================== */ -/* Remove all splits from the split list view */ - -static void -lv_clear_splits_in_lot (GNCLotViewer *lv) -{ - gtk_list_store_clear (lv->split_in_lot_store); -} - /* ======================================================================== */ /* Populate the free split list view */ @@ -218,9 +209,6 @@ lv_show_splits_free (GNCLotViewer *lv) SplitList *split_list, *node; SplitList *filtered_list = NULL; - /* cleanup */ - gtk_list_store_clear (lv->split_free_store); - /* get splits */ split_list = xaccAccountGetSplitList (lv->account); @@ -286,7 +274,7 @@ lv_unset_lot (GNCLotViewer *lv) gtk_text_view_set_editable (lv->lot_notes, FALSE); /* Erase the mini-view area */ - lv_clear_splits_in_lot (lv); + gtk_list_store_clear (lv->split_in_lot_store); #ifdef LOTS_READY_FOR_SHOWTIME gtk_widget_set_sensitive (GTK_WIDGET(lv->regview_button), FALSE); @@ -505,7 +493,8 @@ gnc_split_viewer_fill (GNCLotViewer *lv, GtkListStore *store, SplitList *split_l gboolean show_adjusted_amounts = gtk_toggle_button_get_active (lv->show_adjusted_amounts_checkbutton); gnc_numeric baln = gnc_numeric_zero (); - gtk_list_store_clear (lv->split_in_lot_store); + gtk_list_store_clear (store); + for (SplitList *node = split_list; node; node = node->next) { Split *split = node->data; @@ -516,7 +505,7 @@ gnc_split_viewer_fill (GNCLotViewer *lv, GtkListStore *store, SplitList *split_l gnc_commodity *currency; Transaction *trans = xaccSplitGetParent (split); time64 date = xaccTransGetDate (trans); - gnc_numeric amnt, value, gains; + gnc_numeric amnt, value, gain; GtkTreeIter iter; /* Do not show gains splits, however do show empty business splits */ @@ -556,20 +545,21 @@ gnc_split_viewer_fill (GNCLotViewer *lv, GtkListStore *store, SplitList *split_l gtk_list_store_set (store, &iter, SPLIT_COL_VALUE_DOUBLE, gnc_numeric_to_double (value), -1); /* Gains. Blank if none. */ - gains = xaccSplitGetCapGains (split); - if (gnc_numeric_zero_p (gains)) + gain = (store == lv->split_in_lot_store) ? + xaccSplitGetCapGains (split) : xaccLotFreeSplitCapGain (split, lv->selected_lot); + if (gnc_numeric_zero_p (gain)) { gainbuff[0] = 0; } else { - xaccSPrintAmount (gainbuff, gains, + xaccSPrintAmount (gainbuff, gain, gnc_commodity_print_info (currency, TRUE)); } gtk_list_store_set (store, &iter, SPLIT_COL_GAIN_LOSS, gainbuff, -1); - gtk_list_store_set (store, &iter, SPLIT_COL_GAIN_LOSS_DOUBLE, gnc_numeric_to_double (gains), -1); + gtk_list_store_set (store, &iter, SPLIT_COL_GAIN_LOSS_DOUBLE, gnc_numeric_to_double (gain), -1); - /* Balance of Gains */ + /* Balance of Amounts */ baln = gnc_numeric_add_fixed (baln, amnt); if (gnc_numeric_zero_p (baln)) { diff --git a/libgnucash/engine/Split.h b/libgnucash/engine/Split.h index f6d4733f6d..63fb1ac370 100644 --- a/libgnucash/engine/Split.h +++ b/libgnucash/engine/Split.h @@ -345,15 +345,15 @@ gnc_numeric xaccSplitGetClearedBalance (const Split *split); */ gnc_numeric xaccSplitGetReconciledBalance (const Split *split); +/** Sets the stock split adjusted amount of a split. + * @note The adjusted amount will be reset when the account is rebalanced. + */ +void xaccSplitSetAdjustedAmount (Split *split, gnc_numeric amount); + /** Returns the stock-split adjusted amount of the split in the account's commodity. */ gnc_numeric xaccSplitGetAdjustedAmount (const Split *split); -/** Sets the stock split adjusted amount of a split. Note: The adjusted amount - * will be reset when the account is rebalanced. - */ -void xaccSplitSetAdjustedAmount (Split *split, gnc_numeric amount); - /** @} */ @@ -420,14 +420,8 @@ void xaccSplitMergePeerSplits (Split *split, const Split *other_split); */ Split * xaccSplitGetOtherSplit (const Split *split); -/** The xaccIsPeerSplit() is a convenience routine that returns TRUE - * (a non-zero value) if the two splits share a common parent - * transaction, else it returns FALSE (zero). - * -gboolean xaccIsPeerSplit (const Split *split_1, const Split *split_2); -*/ /** Returns the split type, which is either the string "normal", or - * "stock-split" for a split from a stock split (pun intended? :-). */ + * "stock-split" for a split from a stock split. */ const char *xaccSplitGetType(Split *s); /** Mark a split to be of type stock split - after this, you shouldn't diff --git a/libgnucash/engine/cap-gains.cpp b/libgnucash/engine/cap-gains.cpp index 1b1241af40..269780a484 100644 --- a/libgnucash/engine/cap-gains.cpp +++ b/libgnucash/engine/cap-gains.cpp @@ -892,6 +892,148 @@ xaccSplitComputeCapGains(Split *split, Account *gain_acc) /* ============================================================== */ +gnc_numeric +xaccLotFreeSplitCapGain(Split *split, GNCLot *lot) +{ + g_return_val_if_fail (split && lot && !split->lot, gnc_numeric_zero()); + + ENTER ("(split=%p lot=%s)", split, gnc_lot_get_title(lot)); + + auto pcy = gnc_account_get_policy (gnc_lot_get_account (lot)); + auto currency = split->parent->common_currency; + + /* Not possible to have gains if the transaction currency and + * account commodity are identical. */ + if (gnc_commodity_equal (currency, + xaccAccountGetCommodity(split->acc))) + { + LEAVE ("Currency transfer, gains not possible, returning."); + return gnc_numeric_zero(); + } + + auto *esplit = gnc_lot_get_earliest_split (lot); + if (!esplit) + { + LEAVE ("Lot is empty, returning."); + return gnc_numeric_zero(); + } + + if (xaccTransGetDate (xaccSplitGetParent (esplit)) >= xaccTransGetDate (xaccSplitGetParent (split))) + { + LEAVE ("Split is too early, returning."); + return gnc_numeric_zero(); + } + + if (xaccSplitIsStockSplit (split)) + { + LEAVE ("Stock split split, returning."); + return gnc_numeric_zero(); + } + + /* If amount is zero, there's nothing to do! Amount-zero splits + * may exist if users attempted to manually record gains. */ + if (gnc_numeric_zero_p (split->amount)) return gnc_numeric_zero(); + + /* If we got to here, then the gains really do need to be recomputed. + * So start working things. */ + + /* Get the amount and value in this lot at the time of this transaction. */ + gnc_numeric lot_amount, lot_value; + gnc_lot_get_balance_before (lot, split, &lot_amount, &lot_value); + + gnc_numeric opening_amount, opening_value; + gnc_commodity *opening_currency; + pcy->PolicyGetLotOpening (pcy, lot, &opening_amount, &opening_value, + &opening_currency); + + /* Check to make sure the lot-opening currency and this split + * use the same currency */ + if (FALSE == gnc_commodity_equiv (currency, opening_currency)) + { + /* OK, the purchase and the sale were made in different currencies. + * I don't know how to compute cap gains for that. This is not + * an error. Just punt, silently. + */ + LEAVE ("Can't compute gains, mismatched commodities!"); + return gnc_numeric_zero(); + } + + /* Opening amount should be larger (or equal) to current split, + * and it should be of the opposite sign. + * XXX This should really be a part of a scrub routine that + * cleans up the lot, before we get at it! + */ + if (0 > gnc_numeric_compare (gnc_numeric_abs(lot_amount), + gnc_numeric_abs(xaccSplitGetAdjustedAmount (split)))) + { + for (auto *n = gnc_lot_get_split_list(lot); n; n = n->next) + { + auto *s = GNC_SPLIT(n->data); + PINFO ("split adj amt=%s", gnc_num_dbg_to_string(xaccSplitGetAdjustedAmount (s))); + } + return gnc_numeric_zero(); + } + if ( (gnc_numeric_negative_p(lot_amount) || + gnc_numeric_positive_p(xaccSplitGetAdjustedAmount (split))) && + (gnc_numeric_positive_p(lot_amount) || + gnc_numeric_negative_p(xaccSplitGetAdjustedAmount (split)))) + { + for (auto *n = gnc_lot_get_split_list(lot); n; n = n->next) + { + auto *s = GNC_SPLIT(n->data); + PINFO ("split adj amt=%s", gnc_num_dbg_to_string(xaccSplitGetAdjustedAmount (s))); + } + return gnc_numeric_zero(); + } + + /* The cap gains is the difference between the basis prior to the + * current split, and the current split, pro-rated for an equal + * amount of shares. + * i.e. purchase_price = lot_value / lot_amount + * cost_basis = purchase_price * current_split_amount + * cap_gain = current_split_value - cost_basis + */ + /* Fraction of the lot that this split represents: */ + auto frac = gnc_numeric_div (xaccSplitGetAdjustedAmount (split), lot_amount, + GNC_DENOM_AUTO, + GNC_HOW_DENOM_REDUCE); + /* Basis for this split: */ + auto value = gnc_numeric_mul (frac, lot_value, + gnc_numeric_denom(opening_value), + GNC_HOW_DENOM_EXACT | GNC_HOW_RND_ROUND_HALF_UP); + /* Capital gain for this split: */ + value = gnc_numeric_sub (value, split->value, + GNC_DENOM_AUTO, GNC_HOW_DENOM_FIXED); + + PINFO ("Open amt=%s val=%s; split adj amt=%s val=%s; gains=%s\n", + gnc_num_dbg_to_string (lot_amount), + gnc_num_dbg_to_string (lot_value), + gnc_num_dbg_to_string (xaccSplitGetAdjustedAmount (split)), + gnc_num_dbg_to_string (split->value), + gnc_num_dbg_to_string (value)); + + if (gnc_numeric_check (value)) + { + PERR ("Numeric overflow during gains calculation\n" + "Acct=%s Txn=%s\n" + "\tOpen amt=%s val=%s\n\tsplit amt=%s val=%s\n\tgains=%s\n", + xaccAccountGetName(split->acc), + xaccTransGetDescription(split->parent), + gnc_num_dbg_to_string (lot_amount), + gnc_num_dbg_to_string (lot_value), + gnc_num_dbg_to_string (xaccSplitGetAdjustedAmount (split)), + gnc_num_dbg_to_string (split->value), + gnc_num_dbg_to_string (value)); + return gnc_numeric_zero(); + } + + LEAVE ("(lot=%s)", gnc_lot_get_title(lot)); + + return value; +} + +/* ============================================================== */ + gnc_numeric xaccSplitGetCapGains(Split * split) { diff --git a/libgnucash/engine/cap-gains.h b/libgnucash/engine/cap-gains.h index c8ced0be1c..9b6b7606f9 100644 --- a/libgnucash/engine/cap-gains.h +++ b/libgnucash/engine/cap-gains.h @@ -187,6 +187,20 @@ gboolean xaccSplitAssign (Split *split); */ Split * xaccSplitAssignToLot (Split *split, GNCLot *lot); +/** The xaccLotFreeSplitCapGain() method returns the value of capital gain + * (if any) associated with the indicated split against the indicated + * lot. In order for there to be any capital gain, a few things must + * hold true about this split: + * (1) It must have been involved in trading (for example, + * by belonging to a stock or trading account) + * (2) It must not be assigned to a lot. + * (3) It cannot be the opening split of a lot; that + * is, it must be a matching sale of an earlier purchase + * (or vice versa). + */ +gnc_numeric xaccLotFreeSplitCapGain(Split *split, GNCLot *lot); + + /** The xaccSplitComputeCapGains() routine computes the cap gains * or losses for the indicated split. The gains are placed into * the 'gains_acct'. If the gains_acct is NULL, then the appropriate