Skip to content

Commit

Permalink
Merge #2551: [BUG][GUI] Fix random double/triple transaction record i…
Browse files Browse the repository at this point in the history
…ssue

7562b63 [BUG][GUI] Fix random double/triple transaction record issue (random-zebra)

Pull request description:

  Should put the final nail in this coffin, allowing us to finally close #1480.

  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.

ACKs for top commit:
  furszy:
    finally 🍺 , tested ACK 7562b63
  Fuzzbawls:
    utACK 7562b63

Tree-SHA512: 8a3a26125cb1f8c70596c9bacfe52f7577edc810769ff53d48226533eda40fdc5aa4deab1e19ca7f84a4daebf000a959886547f285e65cbe4a3581aaff6a7dc9
  • Loading branch information
random-zebra committed Sep 20, 2021
2 parents 7176536 + 7562b63 commit 0a644d8
Showing 1 changed file with 5 additions and 3 deletions.
8 changes: 5 additions & 3 deletions src/qt/transactiontablemodel.cpp
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 0a644d8

Please sign in to comment.