Addition of a balance column in transaction history table #5587

SoCoCpp commented at 3:10 AM on January 1, 2015: none

I created a running balance column in the transaction table, like you would see on most financial ledgers. This came into complications with the table's column re-size fixer system. I updated that as well to support multiple columns to the right of the column you want to stretch.

Balance column details:

This was accomplished by adding a balance variable to the TransactionRecord class, but the main code which fills those variables is in (/src/qt/transactiontablemodel.cpp) TransactionTablePriv::updateWalletBalances() which is called in reaction to TransactionTablePriv::refreshWallet() and new/deleted transactions detected in TransactionTablePriv::updateWallet().

TransactionTablePriv::updateWalletBalances() builds a list of pointers to the transactions in the TransactionTablePriv::cachedWallet list, sorts them by time as they are already ordered by hash, then iterates them calculating the balance at the time that transaction came in the list.

Many changes were made to support adding the GUI column and all the features that go with that. This includes filtering support, Bitcoin units support, CSV export support, and column sorting support.

Column size fixer details:

The (src/qt/guiutil.cpp) TableViewLastColumnResizingFixer was created expecting the RecentRequestsTableModel::Message to be the main stretching column and only the RecentRequestsTableModel::Amount column to be to the right of it as well as being the right most column.

I updated this to allow you to specify the stretch column index. It handles the special cases of re-sizing the stretch column, the last column, and the second last column. Aside from restoring the same functionality with support for the additional column (and future column additions), it also restores the ability to re-size the last column from the right side. This isn't an intuitive way to do it, but there seems no benefit from preventing doing so. I think preventing this was just a side effect of the previous code.

This column size fixer is used on the transaction table (src/qt/transactionview.cpp) and the received coins table (src/qt/receivecoinsdialog.cpp).

I put together a quick video comparing the column re-sizing Qt default behavior, the original Bitcoin column size fixer, what happens to the original fixer when the new balance column is added, and how it works with the balance column and the updated column fixer.

http://youtu.be/7N-c9p-YNCQ

Added balance column to transactions table. 13ac0814af

Revised TableViewLastColumnResizingFixer to support multiple columns to the right of the stretching column. a8152fab6c

Adjusted TableViewLastColumnResizingFixer imply the last column index and not require specifying it. 758a0a7418

luke-jr commented at 3:22 AM on January 1, 2015: member

How does this sort transactions? The GUI constantly reorders my transaction list, for example...

SoCoCpp commented at 3:31 AM on January 1, 2015: none

The GUI reorders the model indexes, but not the cached transaction list.

This implementation works directly off the cached transaction list that represents your visible transactions. When this list changes it rebuilds all running balances of all visible transactions, since one can't be sure exactly where, in the list sorted by time, that a transaction addition/removal/or status updates that affect your visible transactions should be.

laanwj added the label GUI on Jan 1, 2015

laanwj commented at 11:26 AM on January 1, 2015: member

Nice!

Re: ordering: I'm not an accountant, but I'm a bit in doubt about doing this on the GUI side and computing them, based on date and time, on the fly. It seems like that if there is no permanence and ie changes in ordering can change the displayed balances.

After all, bitcoin transactions on lowest level have not date and time associated with them, the date and time shown is either A) when it was received by your client B) the corresponding block time/date, if confirmed. The former can change arbitrarily (ie through a rescan/salvagewallet), the second cannot except through chain reorganizations which are rare.

Of course, transactions in the block chain do have a normative ordering, ie by (block number,tx number). This roughly corresponds to time. You could also use the block timestamps. Note that block timestamps are not monotonically increasing, so these two are not always the same.

Maybe use that (block,tx number) order, and not display a counting balance for unconfirmed transactions?

laanwj added the label Wallet on Jan 1, 2015

luke-jr commented at 11:43 AM on January 1, 2015: member

It would look weird and confusing for the display order to be different from the balance order IMO, especially when the user is trying to sort it chronologically. Some use cases also break if we strictly use block ordering, though I do see the appeal there.

in src/qt/guiutil.cpp:None in 758a0a7418

 462 | -// Setup the resize mode, handles compatibility for Qt5 and below as the method signatures changed.
 463 | -// Refactored here for readability.
 464 | -void TableViewLastColumnResizingFixer::setViewHeaderResizeMode(int logicalIndex, QHeaderView::ResizeMode resizeMode)
 465 | +/**
 466 | + * Make sure the columns don't get wider than the viewport.
 467 | + * Also, keey the last column's size snug against the right side.

fanquake commented at 3:01 AM on January 2, 2015:

s/keey/keep

in src/qt/guiutil.cpp:None in 758a0a7418

 540 |  
 541 | -// Make sure we don't make the columns wider than the tables viewport width.
 542 | -void TableViewLastColumnResizingFixer::adjustTableColumnsWidth()
 543 | +/** 
 544 | + * Manually sets a column's width while ignoring singnals
 545 | + * the resize causes to prevent infinate loops.

fanquake commented at 3:05 AM on January 2, 2015:

s/infinate/infinite

laanwj commented at 10:38 AM on January 2, 2015: member

Luke-jr: Sure, but we have that problem anyhow as the transactions can be sorted in different ways, and that should certainly not affect the running balances. Though changing the default displayed transaction order to that may make sense too.

SoCoCpp commented at 1:14 AM on January 4, 2015: none

luke-jr and laanwj Think not just of the transactions being sorted in different ways, but also of them being filtered in different ways. I think this drives home the idea that the running balances should be the balance as of the time that transaction existed, irrespective of any sorting and filtering.

If we agreed against that point, it may slightly simplify things, as we wouldn't need to clone the transaction cache into a pointer list to sort by time and calculate balances. Instead, we could iterate the transaction model index, how they are currently sorted and filtered, using that existing list, then calculate balances.

Edit: Currently, we have the ability to filter out any transaction records, that for example, have less than a 0.01 BTC balance. If we worked off the sorted/filtered list, I don't know if we'd still have that ability.

laanwj commented at 10:43 AM on January 4, 2015: member

IMO this is most useful if the running balances are of the time the transaction was mined into the chain, and are thus static (excluding reorganizations of the chain).

You can also use this for optimization, as the value for transactions deeper in the chain will not change - barring catastrophic chain reorganization, which would be fair to trigger a full recompute.

Changes to sorting and filtering should certainly not affect them, that's visual only. Hence my reservations of doing this in the GUI in the first place. You can still do it in the GUI code, but be sure that trivial changes to user preferences (ie, filtering/sorting) don't affect it.

Thinking of it, transactions also have an 'nOrderPos' which is another ordering that could be used. It is the order in which transaction were added to the wallet. This is preferable to using time and date, but less stable than (height,tx) block chain ordering, as it is affected by when the unconfirmed transaction is first received by the client (so eg -salvagewallet will reconstruct in a different order).

sipa commented at 2:35 PM on January 4, 2015: member

I don't think it's reasonable to claim that bitcoin wallet does any real accounting (that would require an append-only ledger of credits and debits, while our transaction list can be modified, rewritten, and its ordering is not well-defined), so trying to make it (or balances computed from it) work like one isn't very useful.

Seeing how much money you approximately had at what date does seem useful, however.

laanwj commented at 2:54 PM on January 4, 2015: member

My point in the above posts is that there are various orderings that are at least reasonably well-defined:

nOrderPos tracks the order in which transactions have entered the wallet. These won't change, unless -resalvagewallet or such but in that case metadata is expected to be lost.
(block height,num) order is well-defined, unless chain reorganizations happen
block date order is well-defined, unless chain reorganizations happen

luke-jr commented at 3:38 PM on January 4, 2015: member

It's also tempting to try to abuse nSequence as a timestamp :)

jonasschnelli commented at 4:00 PM on January 9, 2015: contributor

IMO this make things more complicate and i don't see a need for this in bitcoin-core. Nevertheless i did some testing:

setgenerate true 101 gives a strange image: Is this correct? bildschirmfoto 2015-01-09 um 16 55 55

Same if i do an additional walle tx on the top: bildschirmfoto 2015-01-09 um 16 58 26

i guess it needs better handling of immature balance?

jonasschnelli commented at 10:34 AM on March 11, 2015: contributor

@SoCoCpp do you have plans to update this?

laanwj commented at 10:59 AM on March 20, 2015: member

Closing this due to inactivity. @SoCoCpp let me know if you start work on this again, then I'll reopen.

laanwj closed this on Mar 20, 2015

DrahtBot locked this on Sep 8, 2021