More to the point it has *empty* trading splits that quite reasonably get removed and not re-created when the cloned transaction is committed. How did you make a transaction with empty trading splits?

The simple work-around would be to edit, dirty, and commit the original transaction as the first step of cloning it.

I seriously cannot remember... definitely created via UI. Still, anything would be better than a g_assert fault.

Don't run with G_DEBUG=fatal-criticals or fatal-warnings if you don't want the assert--but if you do you'll get a segfault at Transaction.c:2802 instead because xaccTransReverse doesn't null check trans. Some will argue that's worse. ;-)

BTW, there's a memory leak there, do you see it?

(In reply to John Ralls from comment #3)
> Don't run with G_DEBUG=fatal-criticals or fatal-warnings if you don't want
> the assert--but if you do you'll get a segfault at Transaction.c:2802

2802? Why?

> instead because xaccTransReverse doesn't null check trans. Some will argue
> that's worse. ;-)
> 
> BTW, there's a memory leak there, do you see it?

The GValue not being unset? Maybe, I'd need to re-check valgrind.

(In reply to John Ralls from comment #3)
> Don't run with G_DEBUG=fatal-criticals or fatal-warnings if you don't want
> the assert--but if you do you'll get a segfault at Transaction.c:2802
> instead because xaccTransReverse doesn't null check trans. Some will argue
> that's worse. ;-)
> 
> BTW, there's a memory leak there, do you see it?

Or the GncGUID not being guid_free()d.

> 2802? Why?

Now 2825.

I guess I was unclear: The leak is in xaccTransClone when the list lengths are unequal, not in xaccTransReverse.

(In reply to John Ralls from comment #3)
> Don't run with G_DEBUG=fatal-criticals or fatal-warnings if you don't want
> the assert--but if you do you'll get a segfault at Transaction.c:2802
> instead because xaccTransReverse doesn't null check trans. Some will argue
> that's worse. ;-)

I don't actually run with these fatal-critical warnings. Here's my RUN script:

------------------------------
ninja -j 3 && ninja install
cd ../maint$1-install/bin
# gdb -ex run --args ./gnucash --logto stdout
./gnucash --extra --logto stdout
------------------------------

And here's the segfault crash:

------------------------------
* 23:20:10 ERROR <gnc.engine> xaccTransClone: assertion 'g_list_length (to->splits) == g_list_length (from->splits)' failed
./run-maint.sh: line 10:  5947 Segmentation fault      (core dumped) ./gnucash --extra --logto stdout
------------------------------

> 
> BTW, there's a memory leak there, do you see it?

I'll leave it to you how to fix it, I don't see any leak!

> I don't actually run with these fatal-critical warnings.
G_DEBUG is an environment variable. If it's not set then the assert in g_log doesn't run and is only informational.

> ./run-maint.sh: line 10:  5947 Segmentation fault      (core dumped) ./gnucash --extra --logto stdout

That's your run script, it's not telling you anything about the actual crash location. Switch it to run the gdb line and get a stack trace of a few frames to see the actual crash information.

> I'll leave it to you how to fix it, I don't see any leak!

Transaction *
xaccTransClone (const Transaction *from)
{
    Transaction *to = xaccTransCloneNoKvp (from); // Allocates a Transaction
    GList *lfrom, *lto;

    xaccTransBeginEdit (to);
    qof_instance_copy_kvp (QOF_INSTANCE (to), QOF_INSTANCE (from));
    g_return_val_if_fail (g_list_length (to->splits) == g_list_length (from->splits),
                          NULL); // Returns nullptr without freeing `to`.

> I'll leave it to you how to fix it

Done.