Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix largest part of GUI lockups with large wallets #3155

Merged
merged 15 commits into from
Oct 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/qt/addresstablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -421,11 +421,21 @@ bool AddressTableModel::removeRows(int row, int count, const QModelIndex &parent
/* Look up label for address in address book, if not found return empty string.
*/
QString AddressTableModel::labelForAddress(const QString &address) const
{
CBitcoinAddress address_parsed(address.toStdString());
return labelForAddress(address_parsed);
}

QString AddressTableModel::labelForAddress(const CBitcoinAddress &address) const
{
return labelForDestination(address.Get());
}

QString AddressTableModel::labelForDestination(const CTxDestination &dest) const
{
{
LOCK(wallet->cs_wallet);
CBitcoinAddress address_parsed(address.toStdString());
std::map<CTxDestination, CAddressBookData>::iterator mi = wallet->mapAddressBook.find(address_parsed.Get());
std::map<CTxDestination, CAddressBookData>::iterator mi = wallet->mapAddressBook.find(dest);
if (mi != wallet->mapAddressBook.end())
{
return QString::fromStdString(mi->second.name);
Expand Down
4 changes: 4 additions & 0 deletions src/qt/addresstablemodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#ifndef BITCOIN_QT_ADDRESSTABLEMODEL_H
#define BITCOIN_QT_ADDRESSTABLEMODEL_H

#include "base58.h"

#include <QAbstractTableModel>
#include <QStringList>

Expand Down Expand Up @@ -66,6 +68,8 @@ class AddressTableModel : public QAbstractTableModel
/* Look up label for address in address book, if not found return empty string.
*/
QString labelForAddress(const QString &address) const;
QString labelForAddress(const CBitcoinAddress &address) const;
QString labelForDestination(const CTxDestination &dest) const;

/* Look up row index of an address in the model.
Return -1 if not found.
Expand Down
6 changes: 3 additions & 3 deletions src/qt/transactiondesc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,14 @@ QString TransactionDesc::toHTML(CWallet *wallet, CWalletTx &wtx, TransactionReco
if (nNet > 0)
{
// Credit
if (CBitcoinAddress(rec->address).IsValid())
if (rec->address.IsValid())
{
CTxDestination address = CBitcoinAddress(rec->address).Get();
CTxDestination address = rec->txDest;
if (wallet->mapAddressBook.count(address))
{
strHTML += "<b>" + tr("From") + ":</b> " + tr("unknown") + "<br>";
strHTML += "<b>" + tr("To") + ":</b> ";
strHTML += GUIUtil::HtmlEscape(rec->address);
strHTML += GUIUtil::HtmlEscape(rec->strAddress);
QString addressOwned = (::IsMine(*wallet, address) == ISMINE_SPENDABLE) ? tr("own address") : tr("watch-only");
if (!wallet->mapAddressBook[address].name.empty())
strHTML += " (" + addressOwned + ", " + tr("label") + ": " + GUIUtil::HtmlEscape(wallet->mapAddressBook[address].name) + ")";
Expand Down
10 changes: 5 additions & 5 deletions src/qt/transactionfilterproxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ const QDateTime TransactionFilterProxy::MAX_DATE = QDateTime::fromTime_t(0xFFFFF

TransactionFilterProxy::TransactionFilterProxy(QObject *parent) :
QSortFilterProxyModel(parent),
dateFrom(MIN_DATE),
dateTo(MAX_DATE),
dateFrom(MIN_DATE.toTime_t()),
dateTo(MAX_DATE.toTime_t()),
addrPrefix(),
typeFilter(COMMON_TYPES),
watchOnlyFilter(WatchOnlyFilter_All),
Expand All @@ -35,7 +35,7 @@ bool TransactionFilterProxy::filterAcceptsRow(int sourceRow, const QModelIndex &
QModelIndex index = sourceModel()->index(sourceRow, 0, sourceParent);

int type = index.data(TransactionTableModel::TypeRole).toInt();
QDateTime datetime = index.data(TransactionTableModel::DateRole).toDateTime();
qint64 datetime = index.data(TransactionTableModel::DateRoleInt).toLongLong();
bool involvesWatchAddress = index.data(TransactionTableModel::WatchonlyRole).toBool();
bool lockedByInstantSend = index.data(TransactionTableModel::InstantSendRole).toBool();
QString address = index.data(TransactionTableModel::AddressRole).toString();
Expand Down Expand Up @@ -67,8 +67,8 @@ bool TransactionFilterProxy::filterAcceptsRow(int sourceRow, const QModelIndex &

void TransactionFilterProxy::setDateRange(const QDateTime &from, const QDateTime &to)
{
this->dateFrom = from;
this->dateTo = to;
this->dateFrom = from.toTime_t();
this->dateTo = to.toTime_t();
invalidateFilter();
}

Expand Down
4 changes: 2 additions & 2 deletions src/qt/transactionfilterproxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ class TransactionFilterProxy : public QSortFilterProxyModel
bool filterAcceptsRow(int source_row, const QModelIndex & source_parent) const;

private:
QDateTime dateFrom;
QDateTime dateTo;
qint64 dateFrom;
qint64 dateTo;
QString addrPrefix;
quint32 typeFilter;
WatchOnlyFilter watchOnlyFilter;
Expand Down
60 changes: 44 additions & 16 deletions src/qt/transactionrecord.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,20 +58,22 @@ QList<TransactionRecord> TransactionRecord::decomposeTransaction(const CWallet *
{
// Received by Dash Address
sub.type = TransactionRecord::RecvWithAddress;
sub.address = CBitcoinAddress(address).ToString();
sub.strAddress = CBitcoinAddress(address).ToString();
}
else
{
// Received by IP connection (deprecated features), or a multisignature or other non-simple transaction
sub.type = TransactionRecord::RecvFromOther;
sub.address = mapValue["from"];
sub.strAddress = mapValue["from"];
}
if (wtx.IsCoinBase())
{
// Generated
sub.type = TransactionRecord::Generated;
}

sub.address.SetString(sub.strAddress);
sub.txDest = sub.address.Get();
parts.append(sub);
}
}
Expand Down Expand Up @@ -119,7 +121,7 @@ QList<TransactionRecord> TransactionRecord::decomposeTransaction(const CWallet *
TransactionRecord sub(hash, nTime);
// Payment to self by default
sub.type = TransactionRecord::SendToSelf;
sub.address = "";
sub.strAddress = "";

if(mapValue["DS"] == "1")
{
Expand All @@ -128,12 +130,12 @@ QList<TransactionRecord> TransactionRecord::decomposeTransaction(const CWallet *
if (ExtractDestination(wtx.tx->vout[0].scriptPubKey, address))
{
// Sent to Dash Address
sub.address = CBitcoinAddress(address).ToString();
sub.strAddress = CBitcoinAddress(address).ToString();
}
else
{
// Sent to IP, or other non-address transaction like OP_EVAL
sub.address = mapValue["to"];
sub.strAddress = mapValue["to"];
}
}
else
Expand Down Expand Up @@ -162,6 +164,8 @@ QList<TransactionRecord> TransactionRecord::decomposeTransaction(const CWallet *

sub.debit = -(nDebit - nChange);
sub.credit = nCredit - nChange;
sub.address.SetString(sub.strAddress);
sub.txDest = sub.address.Get();
parts.append(sub);
parts.last().involvesWatchAddress = involvesWatchAddress; // maybe pass to TransactionRecord as constructor argument
}
Expand Down Expand Up @@ -205,13 +209,13 @@ QList<TransactionRecord> TransactionRecord::decomposeTransaction(const CWallet *
{
// Sent to Dash Address
sub.type = TransactionRecord::SendToAddress;
sub.address = CBitcoinAddress(address).ToString();
sub.strAddress = CBitcoinAddress(address).ToString();
}
else
{
// Sent to IP, or other non-address transaction like OP_EVAL
sub.type = TransactionRecord::SendToOther;
sub.address = mapValue["to"];
sub.strAddress = mapValue["to"];
}

if(mapValue["DS"] == "1")
Expand All @@ -228,6 +232,9 @@ QList<TransactionRecord> TransactionRecord::decomposeTransaction(const CWallet *
}
sub.debit = -nValue;

sub.address.SetString(sub.strAddress);
sub.txDest = sub.address.Get();

parts.append(sub);
}
}
Expand All @@ -244,9 +251,10 @@ QList<TransactionRecord> TransactionRecord::decomposeTransaction(const CWallet *
return parts;
}

void TransactionRecord::updateStatus(const CWalletTx &wtx, int numISLocks, int chainLockHeight)
void TransactionRecord::updateStatus(const CWalletTx &wtx, int chainLockHeight)
{
AssertLockHeld(cs_main);
AssertLockHeld(wtx.GetWallet()->cs_wallet);
// Determine transaction status

// Find the block the tx is in
Expand All @@ -264,9 +272,20 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx, int numISLocks, int c
status.countsForBalance = wtx.IsTrusted() && !(wtx.GetBlocksToMaturity() > 0);
status.depth = wtx.GetDepthInMainChain();
status.cur_num_blocks = chainActive.Height();
status.cachedNumISLocks = numISLocks;
status.cachedChainLockHeight = chainLockHeight;

bool oldLockedByChainLocks = status.lockedByChainLocks;
if (!status.lockedByChainLocks) {
status.lockedByChainLocks = wtx.IsChainLocked();
}

auto addrBookIt = wtx.GetWallet()->mapAddressBook.find(this->txDest);
if (addrBookIt == wtx.GetWallet()->mapAddressBook.end()) {
status.label = "";
} else {
status.label = QString::fromStdString(addrBookIt->second.name);
}

if (!CheckFinalTx(wtx))
{
if (wtx.tx->nLockTime < LOCKTIME_THRESHOLD)
Expand Down Expand Up @@ -307,7 +326,17 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx, int numISLocks, int c
}
else
{
status.lockedByInstantSend = wtx.IsLockedByInstantSend();
// The IsLockedByInstantSend call is quite expensive, so we only do it when a state change is actually possible.
if (status.lockedByChainLocks) {
if (oldLockedByChainLocks != status.lockedByChainLocks) {
status.lockedByInstantSend = wtx.IsLockedByInstantSend();
} else {
status.lockedByInstantSend = false;
}
} else if (!status.lockedByInstantSend) {
status.lockedByInstantSend = wtx.IsLockedByInstantSend();
}

if (status.depth < 0)
{
status.status = TransactionStatus::Conflicted;
Expand All @@ -322,7 +351,7 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx, int numISLocks, int c
if (wtx.isAbandoned())
status.status = TransactionStatus::Abandoned;
}
else if (status.depth < RecommendedNumConfirmations && !wtx.IsChainLocked())
else if (status.depth < RecommendedNumConfirmations && !status.lockedByChainLocks)
{
status.status = TransactionStatus::Confirming;
}
Expand All @@ -331,15 +360,14 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx, int numISLocks, int c
status.status = TransactionStatus::Confirmed;
}
}

status.needsUpdate = false;
}

bool TransactionRecord::statusUpdateNeeded(int numISLocks, int chainLockHeight)
bool TransactionRecord::statusUpdateNeeded(int chainLockHeight)
{
AssertLockHeld(cs_main);
return status.cur_num_blocks != chainActive.Height()
|| status.cachedNumISLocks != numISLocks
|| status.cachedChainLockHeight != chainLockHeight;
return status.cur_num_blocks != chainActive.Height() || status.needsUpdate
|| (!status.lockedByChainLocks && status.cachedChainLockHeight != chainLockHeight);
}

QString TransactionRecord::getTxID() const
Expand Down
36 changes: 25 additions & 11 deletions src/qt/transactionrecord.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "amount.h"
#include "uint256.h"
#include "base58.h"

#include <QList>
#include <QString>
Expand All @@ -20,9 +21,9 @@ class TransactionStatus
{
public:
TransactionStatus():
countsForBalance(false), lockedByInstantSend(false), sortKey(""),
countsForBalance(false), lockedByInstantSend(false), lockedByChainLocks(false), sortKey(""),
matures_in(0), status(Offline), depth(0), open_for(0), cur_num_blocks(-1),
cachedNumISLocks(-1), cachedChainLockHeight(-1)
cachedChainLockHeight(-1), needsUpdate(false)
{ }

enum Status {
Expand All @@ -45,8 +46,12 @@ class TransactionStatus
bool countsForBalance;
/// Transaction was locked via InstantSend
bool lockedByInstantSend;
/// Transaction was locked via ChainLocks
bool lockedByChainLocks;
/// Sorting key based on status
std::string sortKey;
/// Label
QString label;
UdjinM6 marked this conversation as resolved.
Show resolved Hide resolved

/** @name Generated (mined) transactions
@{*/
Expand All @@ -65,10 +70,10 @@ class TransactionStatus
/** Current number of blocks (to know whether cached status is still valid) */
int cur_num_blocks;

//** Know when to update transaction for IS-locks **/
int cachedNumISLocks;
//** Know when to update transaction for chainlocks **/
int cachedChainLockHeight;

bool needsUpdate;
UdjinM6 marked this conversation as resolved.
Show resolved Hide resolved
};

/** UI model for a transaction. A core transaction can be represented by multiple UI transactions if it has
Expand Down Expand Up @@ -98,22 +103,28 @@ class TransactionRecord
static const int RecommendedNumConfirmations = 6;

TransactionRecord():
hash(), time(0), type(Other), address(""), debit(0), credit(0), idx(0)
hash(), time(0), type(Other), strAddress(""), debit(0), credit(0), idx(0)
{
address = CBitcoinAddress(strAddress);
txDest = address.Get();
}

TransactionRecord(uint256 _hash, qint64 _time):
hash(_hash), time(_time), type(Other), address(""), debit(0),
hash(_hash), time(_time), type(Other), strAddress(""), debit(0),
credit(0), idx(0)
{
address = CBitcoinAddress(strAddress);
txDest = address.Get();
}

TransactionRecord(uint256 _hash, qint64 _time,
Type _type, const std::string &_address,
Type _type, const std::string &_strAddress,
const CAmount& _debit, const CAmount& _credit):
hash(_hash), time(_time), type(_type), address(_address), debit(_debit), credit(_credit),
hash(_hash), time(_time), type(_type), strAddress(_strAddress), debit(_debit), credit(_credit),
idx(0)
{
address = CBitcoinAddress(strAddress);
txDest = address.Get();
}

/** Decompose CWallet transaction to model transaction records.
Expand All @@ -126,7 +137,10 @@ class TransactionRecord
uint256 hash;
qint64 time;
Type type;
std::string address;
std::string strAddress;
CBitcoinAddress address;
CTxDestination txDest;

CAmount debit;
CAmount credit;
/**@}*/
Expand All @@ -148,11 +162,11 @@ class TransactionRecord

/** Update status from core wallet tx.
*/
void updateStatus(const CWalletTx &wtx, int numISLocks, int chainLockHeight);
void updateStatus(const CWalletTx &wtx, int chainLockHeight);

/** Return whether a status update is needed.
*/
bool statusUpdateNeeded(int numISLocks, int chainLockHeight);
bool statusUpdateNeeded(int chainLockHeight);
};

#endif // BITCOIN_QT_TRANSACTIONRECORD_H
Loading