[BUG][GUI] Fix random double/triple transaction record issue

random-zebra · random-zebra · commit 7562b6310db9 · 2021-09-12T13:56:33.000+02:00
This bug is caused by the fact that, if at startup the wallet has more
than 4000 (SINGLE_THREAD_MAX_TXES_SIZE) transaction records, then
chachedWallet is loaded not properly ordered, therfore the binary search
(used later in updateWallet, to check whether a tx is already in the
model) is unreliable.

When the wallet has more than 4000 records, in fact, the list of records
in TransactionTablePriv::refreshWallet() is split in batches, each one
processed in a separate thread, processing the last batch in the main
thread at the beginning, thus without preserving the original order (by
hash) of walletTxes. Further, if the wallet has more than 200k records
(MAX_AMOUNT_LOADED_RECORDS), then the list is also sorted by date before
being trimmed and split in batches.

Fix this by re-sorting the cachedWallet list by hash at the end of the
multi-threaded update.
x
diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp
@@ -104,9 +104,7 @@ class TransactionTablePriv
 
             // First check if the amount of txs exceeds the UI limit
             if (txesSize > MAX_AMOUNT_LOADED_RECORDS) {
-                // Sort the txs by date just to be really really sure that them are ordered.
-                // (this extra calculation should be removed in the future if can ensure that
-                // txs are stored in order in the db, which is what should be happening)
+                // Sort the txs by date
                 sort(walletTxes.begin(), walletTxes.end(),
                         [](const CWalletTx & a, const CWalletTx & b) -> bool {
                          return a.GetTxTime() > b.GetTxTime();
@@ -153,6 +151,10 @@ class TransactionTablePriv
                     nFirstLoadedTxTime = convertRes.nFirstLoadedTxTime;
                 }
             }
+
+            // Now that all records have been cached, sort them by tx hash
+            std::sort(cachedWallet.begin(), cachedWallet.end(), TxLessThan());
+
         } else {
             // Single thread flow
             ConvertTxToVectorResult convertRes = convertTxToRecords(wallet, walletTxes);
