I think that finding and renaming trading accounts is too hard, and anyway I don't like the way that GnuCash relies on names for such things.

So instead I've fixed it by looking for a trading account that uses the same commodity and ignores the name. This works perfectly for your procedure, the only action left to the user is to rename the first FOO trading account (easily identified because is balance will show as 1 BAR on the Accounts page).

Fixed for GnuCash 4.5.

Great, thank you!

I tested this and there seem to still be a few problems.

I have 5 test files related to this problem which I used to test this change.  They are all copies of my regular accounts file restored from backups over the last few years.  They demonstrate various stages in this issue. Here is my summary of the problem and what the files represent.

Prior to 0f8d9f6 changing the mnemonic (ticker symbol) for a commodity would cause a new trading account for that commodity to be created.  That would result in two trading accounts for the same commodity, but Gnucash would consistently use the one with the same name as the commodity's ticker symbol.  This seemed to work ok although it was confusing.  After that change the trading account was located using the commodity's ticker symbol regardless of the name so you didn't get a second trading account when you renamed a commodity.

However if you already had two trading accounts for a commodity when this change was made things got slightly sticky.  Because of the way the search is done Gnucash tended to switch back to the original trading account for the security.  This worked ok for new transactions although it was even more confusing.

THe real problem occurs if you edit a transaction with splits in the "wrong" trading account.  This can happen if you autofill a new transaction from one with the wrong splits and then change the shares or value in the new transaction.  Sometimes this results in a transaction with splits in both trading accounts, which still seems to work ok.  However it can also result in a transaction which Gnucash is unable to balance at all.  This is the real problem.

I had this problem with two TIAA securities.  In 2013 I renamed TIAA to TIAAtrad and in 2020 I renamed TIAAreal to QREARX.  These files are all copies of Alexander.xac at various stages in this evolution:

1Before is from before I renamed TIAAreal and has only the one older trading account for it.  TIAA has already been renamed in this file.

2During is after the TIAAreal rename but before the fix in 0f8d9f6.  It has two trading accounts and is using the new one.

3After is after the fix in 0f8d9f6 and it has switched back to the old trading account.

4Unbalanced is the version that demonstrates the transaction that can't be balanced.  Enter a new interest transaction in Assets:TIAA/CREF:Mike:401A (C8304GY-8):TIAA and let it autofill.  Then change the shares and value in the new transaction.  When you try to commit the transaction it will tell you it is unbalanced and no matter how many times you try it can't autobalance it.  This is probably because the transaction it is autofilling from has splits in both trading accounts.  Note that this is for the other commodity that was renamed in 2013.

5Fixed is after I deleted the old trading accounts and moved the splits from them into the corresponding new accounts.  This seemed to work, even for transactions that had splits in both accounts.

I loaded 4Unbalanced into version a60471c951 (the current Maint) and tried a few things.  The problem with a transaction that can't be auto-balanced still exists.  I haven't looked to see what exactly is happening.  However I think that the trading account balancing code tries to use existing trading account splits if they exist and just adjust the amounts in them.  This is probably failing if there are two or more splits for different trading accounts in the same commodity.  Perhaps it should just pitch all the trading account splits and start over in that case (or always).

There is also another new problem.  My trading account hierarchy includes an account at the path "Trading:CURRENCY:USD".  It happens that the commodity for "CURRENCY" is USD.  Nowwhen I create a new transaction that includes USD splits as well as splits in other commodities, it uses "Trading:CURRENCY" for the USD trading account.  This is wrong, it should use the child account.  It uses CURRENCY even if I make it a placeholder account which should make it read only.

This change did fix things so that renaming a commodity doesn't cause a new trading account to be created.  So things will likely work ok if someone hasn't already created a second trading account for one or more comodities prior to this change.  Even then it mostly works although it's a bit confusing.  The only really serious problem seems to be the transaction that can't be balanced.

The other problem that probably needs to be fixed is that it uses the parent account (TRADING:CURRENCY) instead of the correct trading account "TRADING:CURRENCY:USD) for trading splits in USD.

I see that I had a serious analysis failure. I thought the problem with the duplicates was down to a failure to recurse, but of course it's not.

The actual problem is that the old code cared only about the root Trading account's and the namespace placeholder account's names. It would use the transaction currency when creating those accounts and of course that might or might not be the root currency. The new code that looks for an account having the same currency when called with a different transaction currency doesn't find the top-level trading account and makes a new hierarchy. Oops.

I've written https://github.com/Gnucash/gnucash/pull/996 to partly correct that. In my limited testing it behaves a bit more rationally, but it doesn't yet fix the duplicate trading account trees caused by 0f8d9f6. For that I propose a scrub to convert the first top-level trading account and all of its placeholder sub accounts to the root currency and to delete any other top-level trading accounts, moving and combining if appropriate the actual trading accounts with splits to the top-level one.

I wondered why you were recursing, but didn't really pursue the question.  I'm not sure I completely understand your suggested scrubbing solution.  Let me explain what I thought I was doing when I wrote that code and where I made a mistake and how it could have been fixed.  This will likely be obvious to most of you, but it's good to make sure we all start from the same understanding.

The trading accounts were designed as a three level hierarchy.  The top level (which is also at the top level in the account tree) is an account called "Trading" of type "Trading".  Below that is an account for each commodity namespace.  In my case there are 8 of these with names like "CURRENCY" and "NASDAQ".  The third level is an account for each commodity under the account that has the same name as the commodity's namespace.  

The currency for the level 1 and 2 accounts is not really relevant since there should never be transactions in any of those accounts.  In my case they are all USD accounts.  The level 3 accounts are, of course, in the commodity being traded.  Initially the account name and comodity code were the same.  For example there is an account called "Trading:NASDAQ:AAPL" with commodity "AAPL".  

This is all pretty obvious, but I wanted to get it down anyway.

The original code did a specific three level search (no recursion as such).  The level 1 and 2 accounts were found by searching by name at the appropriate level.  Originally there was no dedicated account type for trading accounts so a name search was the only choice for these.  I thought about changing the top level search for "Trading" account to search by type instead of name but I don't recall if it was ever done.  That would allow localizing the name.

The level 3 accounts were also found by name and this is the source of the problem that started all this.  If the code for a commodity was changed then the trading account lookup no longer found the trading account for that commodity and a new trading account was created.  I don't think it's possible to change the name of a commodity namespace, so the probelem can't happen at level 2.  Finding the top level account by type would let you rename it.  Hence this problem is only really for the level 3 accounts.

It seems to me that there should have been two changes to fix this problem and make it less confusing.  The code that changes a commodity's code should also change the name of the trading account for the commodity if one exists.  This is mostly to make things less confusing for users if you also make the second change to look up the level 3 trading account by commodity instead of name.  It should really have been done that way in the beginning which is my mistake.  

Given that we've had this bug out there for a while and there must be other people with multiple trading accounts for some commodities, I hink it would be good now to look up trading accounts by commodity, giving preference to one with the same name as the commodity if it exists.  I.e., find one with the same name and commodity if possible or one with the same commodity but a different name if not.

As to how to fix existing files, I think it would be possible to make a scrub function that combines all trading account splits for a given commodity into a single trading account with the same name as the commodity.  If any transaction ends up with two or more splits in the same trading account after this, then merge them into one split.  I did something more or less like this by hand to my file and it seemed to fix things up.

If any of this is confusing or seems wrong let me know.

Mike,

There have been a couple of changes:

Some time ago the top-level trading account got changed to look for the translation of "Trading" so if you opened the book in a different locale you'd get a new set of trading accounts. Not so good, but that's apparently not a common occurrence and AFAIK nobody's ever complained about it.

Then a couple of months ago I decided to fix this bug and wrote 0f8d9f6 in which I changed the lookup to ignore the name and use the account type and commodity instead. That commodity part is the problem, it should apply only to the level 3 account.

Somewhat skew to that issue is that setting the level 1 and 2 accounts' currencies to the transaction currency isn't great. As long as nobody ever looks it's theoretically harmless, but it's easy enough to make it the root currency and remove the wart.

Automatically changing trading (and I suppose imbalance, opening-balance, orphan)  account names when a commodity is renamed sounds attractive but seems likely to create some ugly inter-class dependencies.

We already have API to delete an account and transfer all of its splits to another one, so scrubbing away the extra trading accounts won't be too hard.

That all makes sense.  It sounds like we basically agree.  Renaming the accounts isn't necessary, of course, so long as it always uses the correct account for these balancing splits.

I used the "delete account and move splits" feature to delete my extra trading accounts and it actually worked better than I expected.  I didn't specifically verify this, but it seemed to combine the moved split with an existing split in the destination account if one existed.

Combining splits seems an odd thing to do. Was that in the case where a single transaction had splits in two same-currency trading accounts? If so I'd speculate that re-committing the transaction fired the balance code and that cleaned them up into a new single split.

Did you test PR 996 and if so does everything now work the way you expect?

I just tested it again more carefully and deleting a duplicate trading account with transactions that have splits in both trading accounts does not combine the splits.  The transactions end up with two splits in the same trading account.  I must have done something else to cause the splits to be combined, probably for the reason you mention.

I tested PR 996 and left a comment on it.

The transaction that can't be balanced still can't be balanced.  However upon closer inspection it looks like it is balanced (by adding an unnecessary imbalance split) but GnuCash doesn't think it is balanced.  I'l attach a couple of screenshots. AfterAutofill shows the transaction that is created by auto fill.  AfterUnbalancedMessage is what we have after I change the transaction to purchase 10 shares for $10 (by autofilling again and changing the transaction in the Basic Ledger view) and letting GnuCash autobalance the transaction.  It looks to me like it's balanced, but GnuCash doesn't think so.  It balanced it by adding an unnecessary Imbalance-USD split and some strange looking Trading splits.  The two main splits for the purchase are unchanged.

Created attachment 374065 [details]
Transaction after autofill.

Created attachment 374066 [details]
After editing transaction and autobalance

I'm not able to get to the "AfterAutoFill" stage where GnuCash shows it balanced with two trading account splits. I can get it balanced with only one Trading Account or unbalanced with two. If I auto-fill using a transaction with an "old" split then I get a balanced transaction with just that TA split (and the USD one of course); proceeding to edit the split (after switching back to basic view so that GnuCash will try to balance the transaction) I get an unbalanced transaction that looks like AfterUnbalancedMessage. The thing is that I get that result with GnuCash 4.4 too.

What are you doing that I'm missing?

I know it's not the point but you should be able to delete all of the trading account splits and GnuCash will redo them when you commit the transaction.

Created attachment 374068 [details]
Screenshots showing duplication, modification, and rebalancing of a trading accounts transaction.

I think I've replicated your copy-paste imbalance issue. The behavior is the same in 4.4 and 4.5 with one small exception.

The setup is that I created a security MF2, created an account for it, and a buy transaction. This is the same as your interest transaction except for the source of the USD. Then I changed the name and symbol of the security to MF02.
Next I copied a transaction (<ctrl><cmd>C) and pasted it (<ctrl><cmd>V) into the blank transaction, changing the date (image 1), then I changed the amount and value in both asset accounts, but didn't change the trading accounts (image 2), then attempted to commit the transaction. That produced the Rebalance Transaction dialog (image 3). Interestingly it doesn't matter what you tell that dialog, it just disappears leaving you to balance the transaction. I did that by changing the amount in the Trading:Currency:USD split to match the amount in Brokerage Account and deleting the imbalance split (image 4). 

The imbalance is caused by the balancing code no longer recognizing the MF2 trading account as a trading account: It creates a new MF02 trading account and a split for it with the difference between the amounts in the asset and MF2 trading accounts and changing the amount in the USD trading account to balance the value of that new MF02 split, leaving the value of the MF2 trading account unbalanced. Attempting to commit creates an imbalance split, but GnuCash doesn't think that the transaction is balanced because one of the $2500 splits is in a trading account and the other isn't. 

All of that is with GnuCash 4.4! GnuCash 4.5 doesn't have the problem because it recognizes that Trading:Mutual Funds:MF2 is the correct trading account for MF02 and doesn't create a new trading split.

But your account already has three-trading-split transactions that you apparently rebalanced the same way. The last two images show the result of copying, pasting, and changing the amount and value of the transaction created in the first 4. These images are made with GnuCash 4.5, but 4.4 does exactly the same thing except that it adjusts the MF02 account instead of the MF2 account. The problem is exactly the same, the USD trading split is adjusted to balance whichever MF trading split gets adjusted leaving the other trading split unbalanced and so creating an imbalance split for it.

Created attachment 374069 [details]
Four-trading-split copies balances

If instead of combining the imbalance split into the trading split you change it to a fourth trading split it still balances and GnuCash is able to automatically balance future copies of it. In this screen shot the first copy was created in 4.4 and the second in 4.5. Notice that even though no pair of trading splits balance each other GnuCash was still able to balance the transaction itself.

Mike has tested my bug798093bis PR and found that it resolves his issues so I've merged it and pushed; it will be in GnuCash 4.6.

Created attachment 374112 [details]
Test file to crash Gnucash when transferring funds between different currency accounts

I have two trading accounts for the same commodity, each one with some splits. This worked well until 4.4, and in 4.5 the auto-split created a new hierarchy or selected the oldest trading account among the ones with the same commodity.
In 4.6 it Gnucash crashes. Creating any transaction with a commodity that has more than one trading account crashes Gnucash.
I have annexed a test file, a.gnucash
To reproduce the crash just add a new transfer transaction from Checking EUR to Checking USD accounts.

Paolo's also reported his problem on bug 798216 and Bob Fewell has fixed it.

Err, bug 798221.

*** Bug 798216 has been marked as a duplicate of this bug. ***