From fdf1af79e520e5567b13e93cd8ecf918c976133e Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 14 Oct 2019 11:28:16 +0200 Subject: [PATCH 01/15] Remove unnecessary tracking of IS lock count --- src/qt/transactionrecord.cpp | 6 ++---- src/qt/transactionrecord.h | 8 +++----- src/qt/transactiontablemodel.cpp | 14 ++------------ src/qt/transactiontablemodel.h | 3 --- src/qt/walletmodel.cpp | 2 -- 5 files changed, 7 insertions(+), 26 deletions(-) diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index 205bc14daf68..f07a0fa46197 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -244,7 +244,7 @@ QList 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); // Determine transaction status @@ -264,7 +264,6 @@ 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; if (!CheckFinalTx(wtx)) @@ -334,11 +333,10 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx, int numISLocks, int c } -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; } diff --git a/src/qt/transactionrecord.h b/src/qt/transactionrecord.h index 3ad6556fd0a8..8393a42ea0f0 100644 --- a/src/qt/transactionrecord.h +++ b/src/qt/transactionrecord.h @@ -22,7 +22,7 @@ class TransactionStatus TransactionStatus(): countsForBalance(false), lockedByInstantSend(false), sortKey(""), matures_in(0), status(Offline), depth(0), open_for(0), cur_num_blocks(-1), - cachedNumISLocks(-1), cachedChainLockHeight(-1) + cachedChainLockHeight(-1) { } enum Status { @@ -65,8 +65,6 @@ 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; }; @@ -148,11 +146,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 diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 03b2dbf4156c..15293c20cd9d 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -193,13 +193,13 @@ class TransactionTablePriv if(lockMain) { TRY_LOCK(wallet->cs_wallet, lockWallet); - if(lockWallet && (rec->statusUpdateNeeded(parent->getNumISLocks(), parent->getChainLockHeight()))) + if(lockWallet && (rec->statusUpdateNeeded(parent->getChainLockHeight()))) { std::map::iterator mi = wallet->mapWallet.find(rec->hash); if(mi != wallet->mapWallet.end()) { - rec->updateStatus(mi->second, parent->getNumISLocks(), parent->getChainLockHeight()); + rec->updateStatus(mi->second, parent->getChainLockHeight()); } } } @@ -281,10 +281,6 @@ void TransactionTableModel::updateConfirmations() Q_EMIT dataChanged(index(0, ToAddress), index(priv->size()-1, ToAddress)); } -void TransactionTableModel::updateNumISLocks(int numISLocks) -{ - cachedNumISLocks = numISLocks; -} void TransactionTableModel::updateChainLockHeight(int chainLockHeight) { @@ -292,16 +288,10 @@ void TransactionTableModel::updateChainLockHeight(int chainLockHeight) updateConfirmations(); } -int TransactionTableModel::getNumISLocks() const -{ - return cachedNumISLocks; -} - int TransactionTableModel::getChainLockHeight() const { return cachedChainLockHeight; } - int TransactionTableModel::rowCount(const QModelIndex &parent) const { Q_UNUSED(parent); diff --git a/src/qt/transactiontablemodel.h b/src/qt/transactiontablemodel.h index 81c94dcfc62c..e7f6a6553db9 100644 --- a/src/qt/transactiontablemodel.h +++ b/src/qt/transactiontablemodel.h @@ -85,9 +85,7 @@ class TransactionTableModel : public QAbstractTableModel QVariant headerData(int section, Qt::Orientation orientation, int role) const; QModelIndex index(int row, int column, const QModelIndex & parent = QModelIndex()) const; bool processingQueuedTransactions() { return fProcessingQueuedTransactions; } - void updateNumISLocks(int numISLocks); void updateChainLockHeight(int chainLockHeight); - int getNumISLocks() const; int getChainLockHeight() const; private: @@ -97,7 +95,6 @@ class TransactionTableModel : public QAbstractTableModel TransactionTablePriv *priv; bool fProcessingQueuedTransactions; const PlatformStyle *platformStyle; - int cachedNumISLocks; int cachedChainLockHeight; void subscribeToCoreSignals(); diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 780e95275b38..e9006a3b8837 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -194,8 +194,6 @@ void WalletModel::updateNumISLocks() { fForceCheckBalanceChanged = true; cachedNumISLocks++; - if (transactionTableModel) - transactionTableModel->updateNumISLocks(cachedNumISLocks); } void WalletModel::updateChainLockHeight(int chainLockHeight) From 4a4834674ce1942615179e72f8464554f56bc069 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Tue, 15 Oct 2019 16:01:43 +0200 Subject: [PATCH 02/15] Track lockedByChainLocks in TransactionRecord --- src/qt/transactionrecord.cpp | 5 +++-- src/qt/transactionrecord.h | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index f07a0fa46197..d4e6b72a168a 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -266,6 +266,8 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx, int chainLockHeight) status.cur_num_blocks = chainActive.Height(); status.cachedChainLockHeight = chainLockHeight; + status.lockedByChainLocks = wtx.IsChainLocked(); + if (!CheckFinalTx(wtx)) { if (wtx.tx->nLockTime < LOCKTIME_THRESHOLD) @@ -321,7 +323,7 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx, int chainLockHeight) 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; } @@ -330,7 +332,6 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx, int chainLockHeight) status.status = TransactionStatus::Confirmed; } } - } bool TransactionRecord::statusUpdateNeeded(int chainLockHeight) diff --git a/src/qt/transactionrecord.h b/src/qt/transactionrecord.h index 8393a42ea0f0..b698b3ca11f3 100644 --- a/src/qt/transactionrecord.h +++ b/src/qt/transactionrecord.h @@ -20,7 +20,7 @@ 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), cachedChainLockHeight(-1) { } @@ -45,6 +45,8 @@ 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; From c261fca361fa8405c16a94fed3b235b3bb77cbde Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Tue, 15 Oct 2019 16:10:18 +0200 Subject: [PATCH 03/15] Only update record when the TX was not ChainLocked before --- src/qt/transactionrecord.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index d4e6b72a168a..159386d7c514 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -338,7 +338,7 @@ bool TransactionRecord::statusUpdateNeeded(int chainLockHeight) { AssertLockHeld(cs_main); return status.cur_num_blocks != chainActive.Height() - || status.cachedChainLockHeight != chainLockHeight; + || (!status.lockedByChainLocks && status.cachedChainLockHeight != chainLockHeight); } QString TransactionRecord::getTxID() const From 5f18ee092ef63f6bb742d6d1631445b89d62c4c2 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Wed, 24 May 2017 17:09:01 +0200 Subject: [PATCH 04/15] [Qt] make sure transaction table entry gets updated after bump --- src/qt/transactionrecord.cpp | 3 ++- src/qt/transactionrecord.h | 2 ++ src/qt/transactiontablemodel.cpp | 4 ++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index 159386d7c514..609103d3307a 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -332,12 +332,13 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx, int chainLockHeight) status.status = TransactionStatus::Confirmed; } } + status.needsUpdate = false; } bool TransactionRecord::statusUpdateNeeded(int chainLockHeight) { AssertLockHeld(cs_main); - return status.cur_num_blocks != chainActive.Height() + return status.cur_num_blocks != chainActive.Height() || status.needsUpdate || (!status.lockedByChainLocks && status.cachedChainLockHeight != chainLockHeight); } diff --git a/src/qt/transactionrecord.h b/src/qt/transactionrecord.h index b698b3ca11f3..b4e30abfecb5 100644 --- a/src/qt/transactionrecord.h +++ b/src/qt/transactionrecord.h @@ -69,6 +69,8 @@ class TransactionStatus //** Know when to update transaction for chainlocks **/ int cachedChainLockHeight; + + bool needsUpdate; }; /** UI model for a transaction. A core transaction can be represented by multiple UI transactions if it has diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 15293c20cd9d..549902470a07 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -167,6 +167,10 @@ class TransactionTablePriv case CT_UPDATED: // Miscellaneous updates -- nothing to do, status update will take care of this, and is only computed for // visible transactions. + for (int i = lowerIndex; i < upperIndex; i++) { + TransactionRecord *rec = &cachedWallet[i]; + rec->status.needsUpdate = true; + } break; } } From e70a2d1ab7fff9794d9fbdaeee89d6503c444ed8 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Tue, 15 Oct 2019 16:12:58 +0200 Subject: [PATCH 05/15] Emit dataChanged for CT_UPDATED transactions --- src/qt/transactiontablemodel.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 549902470a07..fd81476782b7 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -171,6 +171,7 @@ class TransactionTablePriv TransactionRecord *rec = &cachedWallet[i]; rec->status.needsUpdate = true; } + Q_EMIT parent->dataChanged(parent->index(lowerIndex, TransactionTableModel::Status), parent->index(upperIndex, TransactionTableModel::Status)); break; } } From 144d8be5213a9637af6c21bc26a9d40d21b47907 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Tue, 15 Oct 2019 17:34:26 +0200 Subject: [PATCH 06/15] Don't invoke updateConfirmations directly and let pollBalanceChanged handle it --- src/qt/transactiontablemodel.cpp | 1 - src/qt/walletmodel.cpp | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index fd81476782b7..bdff369fb957 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -290,7 +290,6 @@ void TransactionTableModel::updateConfirmations() void TransactionTableModel::updateChainLockHeight(int chainLockHeight) { cachedChainLockHeight = chainLockHeight; - updateConfirmations(); } int TransactionTableModel::getChainLockHeight() const diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index e9006a3b8837..9ac6210be788 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -200,6 +200,8 @@ void WalletModel::updateChainLockHeight(int chainLockHeight) { if (transactionTableModel) transactionTableModel->updateChainLockHeight(chainLockHeight); + // Number and status of confirmations might have changed (WalletModel::pollBalanceChanged handles this as well) + fForceCheckBalanceChanged = true; } int WalletModel::getNumISLocks() const From 9cea642e774bb7d49d6ec27b124ff62f69bb5aac Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Tue, 15 Oct 2019 16:15:47 +0200 Subject: [PATCH 07/15] Use plain seconds since epoch comparison in TransactionFilterProxy::filterAcceptsRow The QDateTime::operator< calls inside TransactionFilterProxy::filterAcceptsRow turned out to be the slowest part in the UI when many TXs are inside the wallet. DateRoleInt allows us to request the plain seconds since epoch which we then use to compare against dateFrom/dateTo, which are also both stored as seconds since epoch now. --- src/qt/transactionfilterproxy.cpp | 10 +++++----- src/qt/transactionfilterproxy.h | 4 ++-- src/qt/transactiontablemodel.cpp | 2 ++ src/qt/transactiontablemodel.h | 2 ++ 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/qt/transactionfilterproxy.cpp b/src/qt/transactionfilterproxy.cpp index e4eff3e3fc14..b50fe7923ef4 100644 --- a/src/qt/transactionfilterproxy.cpp +++ b/src/qt/transactionfilterproxy.cpp @@ -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), @@ -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(); @@ -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(); } diff --git a/src/qt/transactionfilterproxy.h b/src/qt/transactionfilterproxy.h index ed25b29c99c8..026ca31c9efd 100644 --- a/src/qt/transactionfilterproxy.h +++ b/src/qt/transactionfilterproxy.h @@ -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; diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index bdff369fb957..de912a93b2f9 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -654,6 +654,8 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const return rec->type; case DateRole: return QDateTime::fromTime_t(static_cast(rec->time)); + case DateRoleInt: + return qint64(rec->time); case WatchonlyRole: return rec->involvesWatchAddress; case WatchonlyDecorationRole: diff --git a/src/qt/transactiontablemodel.h b/src/qt/transactiontablemodel.h index e7f6a6553db9..e4dd52eac453 100644 --- a/src/qt/transactiontablemodel.h +++ b/src/qt/transactiontablemodel.h @@ -45,6 +45,8 @@ class TransactionTableModel : public QAbstractTableModel TypeRole = Qt::UserRole, /** Date and time this transaction was created */ DateRole, + /** Date and time this transaction was created in MSec since epoch */ + DateRoleInt, /** Watch-only boolean */ WatchonlyRole, /** Watch-only icon */ From 066945658ec26e8565af7813eed8ab8e650d2e66 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Tue, 15 Oct 2019 18:42:01 +0200 Subject: [PATCH 08/15] Implement AddressTableModel::labelForDestination This one avoids converting from string to CBitcoinAddress and calling .Get() on the result. --- src/qt/addresstablemodel.cpp | 14 ++++++++++++-- src/qt/addresstablemodel.h | 4 ++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index 2a6fb40cae8d..56d6df93557a 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -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::iterator mi = wallet->mapAddressBook.find(address_parsed.Get()); + std::map::iterator mi = wallet->mapAddressBook.find(dest); if (mi != wallet->mapAddressBook.end()) { return QString::fromStdString(mi->second.name); diff --git a/src/qt/addresstablemodel.h b/src/qt/addresstablemodel.h index d04b95ebaeb3..f55921bc6dd0 100644 --- a/src/qt/addresstablemodel.h +++ b/src/qt/addresstablemodel.h @@ -5,6 +5,8 @@ #ifndef BITCOIN_QT_ADDRESSTABLEMODEL_H #define BITCOIN_QT_ADDRESSTABLEMODEL_H +#include "base58.h" + #include #include @@ -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. From 3d1df7772a9da1b58ae1d029569eb514a7b1f24c Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Tue, 15 Oct 2019 18:44:33 +0200 Subject: [PATCH 09/15] Also store CBitcoinAddress object and CTxDestination in TransactionRecord This avoids frequent and slow conversion --- src/qt/transactiondesc.cpp | 6 +++--- src/qt/transactionrecord.cpp | 21 ++++++++++++++------- src/qt/transactionrecord.h | 18 ++++++++++++++---- src/qt/transactiontablemodel.cpp | 18 +++++++++--------- 4 files changed, 40 insertions(+), 23 deletions(-) diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index a3d8706c78a7..6c10ffd22dcc 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -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 += "" + tr("From") + ": " + tr("unknown") + "
"; strHTML += "" + tr("To") + ": "; - 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) + ")"; diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index 609103d3307a..90dcb8b851ff 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -58,13 +58,13 @@ QList 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()) { @@ -72,6 +72,8 @@ QList TransactionRecord::decomposeTransaction(const CWallet * sub.type = TransactionRecord::Generated; } + sub.address.SetString(sub.strAddress); + sub.txDest = sub.address.Get(); parts.append(sub); } } @@ -119,7 +121,7 @@ QList 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") { @@ -128,12 +130,12 @@ QList 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 @@ -162,6 +164,8 @@ QList 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 } @@ -205,13 +209,13 @@ QList 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") @@ -228,6 +232,9 @@ QList TransactionRecord::decomposeTransaction(const CWallet * } sub.debit = -nValue; + sub.address.SetString(sub.strAddress); + sub.txDest = sub.address.Get(); + parts.append(sub); } } diff --git a/src/qt/transactionrecord.h b/src/qt/transactionrecord.h index b4e30abfecb5..00ab39651343 100644 --- a/src/qt/transactionrecord.h +++ b/src/qt/transactionrecord.h @@ -7,6 +7,7 @@ #include "amount.h" #include "uint256.h" +#include "base58.h" #include #include @@ -100,22 +101,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, 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(_address), debit(_debit), credit(_credit), idx(0) { + address = CBitcoinAddress(strAddress); + txDest = address.Get(); } /** Decompose CWallet transaction to model transaction records. @@ -128,7 +135,10 @@ class TransactionRecord uint256 hash; qint64 time; Type type; - std::string address; + std::string strAddress; + CBitcoinAddress address; + CTxDestination txDest; + CAmount debit; CAmount credit; /**@}*/ diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index de912a93b2f9..d8d221cfbc4b 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -443,15 +443,15 @@ QString TransactionTableModel::formatTxToAddress(const TransactionRecord *wtx, b switch(wtx->type) { case TransactionRecord::RecvFromOther: - return QString::fromStdString(wtx->address) + watchAddress; + return QString::fromStdString(wtx->strAddress) + watchAddress; case TransactionRecord::RecvWithAddress: case TransactionRecord::RecvWithPrivateSend: case TransactionRecord::SendToAddress: case TransactionRecord::Generated: case TransactionRecord::PrivateSend: - return lookupAddress(wtx->address, tooltip) + watchAddress; + return lookupAddress(wtx->strAddress, tooltip) + watchAddress; case TransactionRecord::SendToOther: - return QString::fromStdString(wtx->address) + watchAddress; + return QString::fromStdString(wtx->strAddress) + watchAddress; case TransactionRecord::SendToSelf: default: return tr("(n/a)") + watchAddress; @@ -469,7 +469,7 @@ QVariant TransactionTableModel::addressColor(const TransactionRecord *wtx) const case TransactionRecord::PrivateSend: case TransactionRecord::RecvWithPrivateSend: { - QString label = walletModel->getAddressTableModel()->labelForAddress(QString::fromStdString(wtx->address)); + QString label = walletModel->getAddressTableModel()->labelForAddress(QString::fromStdString(wtx->strAddress)); if(label.isEmpty()) return COLOR_BAREADDRESS; } break; @@ -667,9 +667,9 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const case LongDescriptionRole: return priv->describe(rec, walletModel->getOptionsModel()->getDisplayUnit()); case AddressRole: - return QString::fromStdString(rec->address); + return QString::fromStdString(rec->strAddress); case LabelRole: - return walletModel->getAddressTableModel()->labelForAddress(QString::fromStdString(rec->address)); + return walletModel->getAddressTableModel()->labelForAddress(QString::fromStdString(rec->strAddress)); case AmountRole: return qint64(rec->credit + rec->debit); case TxIDRole: @@ -681,7 +681,7 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const case TxPlainTextRole: { QString details; - QString txLabel = walletModel->getAddressTableModel()->labelForAddress(QString::fromStdString(rec->address)); + QString txLabel = walletModel->getAddressTableModel()->labelForAddress(QString::fromStdString(rec->strAddress)); details.append(formatTxDate(rec)); details.append(" "); @@ -691,7 +691,7 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const details.append(formatTxType(rec)); details.append(" "); } - if(!rec->address.empty()) { + if(!rec->strAddress.empty()) { if(txLabel.isEmpty()) details.append(tr("(no label)") + " "); else { @@ -699,7 +699,7 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const details.append(txLabel); details.append(") "); } - details.append(QString::fromStdString(rec->address)); + details.append(QString::fromStdString(rec->strAddress)); details.append(" "); } details.append(formatTxAmount(rec, false, BitcoinUnits::separatorNever)); From 8256622d95dfed01b5ddf09cca54f048451d073e Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Tue, 15 Oct 2019 18:45:43 +0200 Subject: [PATCH 10/15] Use labelForDestination when possible This avoids unnecessary conversions --- src/qt/transactiontablemodel.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index d8d221cfbc4b..def33fe9fe23 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -469,7 +469,7 @@ QVariant TransactionTableModel::addressColor(const TransactionRecord *wtx) const case TransactionRecord::PrivateSend: case TransactionRecord::RecvWithPrivateSend: { - QString label = walletModel->getAddressTableModel()->labelForAddress(QString::fromStdString(wtx->strAddress)); + QString label = walletModel->getAddressTableModel()->labelForDestination(wtx->txDest); if(label.isEmpty()) return COLOR_BAREADDRESS; } break; @@ -669,7 +669,7 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const case AddressRole: return QString::fromStdString(rec->strAddress); case LabelRole: - return walletModel->getAddressTableModel()->labelForAddress(QString::fromStdString(rec->strAddress)); + return walletModel->getAddressTableModel()->labelForDestination(rec->txDest); case AmountRole: return qint64(rec->credit + rec->debit); case TxIDRole: @@ -681,7 +681,7 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const case TxPlainTextRole: { QString details; - QString txLabel = walletModel->getAddressTableModel()->labelForAddress(QString::fromStdString(rec->strAddress)); + QString txLabel = walletModel->getAddressTableModel()->labelForDestination(rec->txDest); details.append(formatTxDate(rec)); details.append(" "); From 51adc068fffbba4575b5f2e70aefa20a0dbf802f Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Tue, 15 Oct 2019 18:52:41 +0200 Subject: [PATCH 11/15] Don't set fForceCheckBalanceChanged to true when IS lock is received We already do this through updateTransaction(), which is also called when an IS lock is received for one of our own TXs. --- src/qt/walletmodel.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 9ac6210be788..0abd177e6a46 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -192,7 +192,6 @@ void WalletModel::updateTransaction() void WalletModel::updateNumISLocks() { - fForceCheckBalanceChanged = true; cachedNumISLocks++; } From bd205c0f751c5e6554cc6f77592716c84db3775e Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Tue, 15 Oct 2019 19:02:19 +0200 Subject: [PATCH 12/15] Only update lockedByChainLocks and lockedByInstantSend when a change is possible lockedByChainLocks can never get back to false, so no need to re-check it. Same with lockedByInstantSend, except when a ChainLock overrides it. --- src/qt/transactionrecord.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index 90dcb8b851ff..d3c97066895f 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -273,7 +273,10 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx, int chainLockHeight) status.cur_num_blocks = chainActive.Height(); status.cachedChainLockHeight = chainLockHeight; - status.lockedByChainLocks = wtx.IsChainLocked(); + bool oldLockedByChainLocks = status.lockedByChainLocks; + if (!status.lockedByChainLocks) { + status.lockedByChainLocks = wtx.IsChainLocked(); + } if (!CheckFinalTx(wtx)) { @@ -315,7 +318,17 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx, int chainLockHeight) } 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; From 3b44576fe004bda33bb02b58be8afbe920727b7a Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Wed, 16 Oct 2019 01:10:32 +0200 Subject: [PATCH 13/15] Hold and update label in TransactionRecord Instead of looking it up in data() --- src/qt/transactionrecord.cpp | 8 ++++++++ src/qt/transactionrecord.h | 2 ++ src/qt/transactiontablemodel.cpp | 35 ++++++++++++++++++++++++++++++-- src/qt/transactiontablemodel.h | 2 ++ src/wallet/wallet.h | 5 +++++ 5 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index d3c97066895f..c2d180093e60 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -254,6 +254,7 @@ QList TransactionRecord::decomposeTransaction(const CWallet * 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 @@ -278,6 +279,13 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx, int chainLockHeight) 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) diff --git a/src/qt/transactionrecord.h b/src/qt/transactionrecord.h index 00ab39651343..b01db16a3657 100644 --- a/src/qt/transactionrecord.h +++ b/src/qt/transactionrecord.h @@ -50,6 +50,8 @@ class TransactionStatus bool lockedByChainLocks; /// Sorting key based on status std::string sortKey; + /// Label + QString label; /** @name Generated (mined) transactions @{*/ diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index def33fe9fe23..23c758df41a7 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -176,6 +176,19 @@ class TransactionTablePriv } } + void updateAddressBook(const QString& address, const QString& label, bool isMine, const QString& purpose, int status) + { + std::string address2 = address.toStdString(); + int index = 0; + for (auto& rec : cachedWallet) { + if (rec.strAddress == address2) { + rec.status.needsUpdate = true; + Q_EMIT parent->dataChanged(parent->index(index, TransactionTableModel::Status), parent->index(index, TransactionTableModel::Status)); + } + index++; + } + } + int size() { return cachedWallet.size(); @@ -276,6 +289,12 @@ void TransactionTableModel::updateTransaction(const QString &hash, int status, b priv->updateWallet(updated, status, showTransaction); } +void TransactionTableModel::updateAddressBook(const QString& address, const QString& label, bool isMine, + const QString& purpose, int status) +{ + priv->updateAddressBook(address, label, isMine, purpose, status); +} + void TransactionTableModel::updateConfirmations() { // Blocks came in since last poll. @@ -669,7 +688,7 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const case AddressRole: return QString::fromStdString(rec->strAddress); case LabelRole: - return walletModel->getAddressTableModel()->labelForDestination(rec->txDest); + return rec->status.label; case AmountRole: return qint64(rec->credit + rec->debit); case TxIDRole: @@ -681,7 +700,7 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const case TxPlainTextRole: { QString details; - QString txLabel = walletModel->getAddressTableModel()->labelForDestination(rec->txDest); + QString txLabel = rec->status.label; details.append(formatTxDate(rec)); details.append(" "); @@ -813,6 +832,16 @@ static void NotifyTransactionChanged(TransactionTableModel *ttm, CWallet *wallet notification.invoke(ttm); } +static void NotifyAddressBookChanged(TransactionTableModel *ttm, CWallet *wallet, const CTxDestination &address, const std::string &label, bool isMine, const std::string &purpose, ChangeType status) +{ + QMetaObject::invokeMethod(ttm, "updateAddressBook", Qt::QueuedConnection, + Q_ARG(QString, QString::fromStdString(CBitcoinAddress(address).ToString())), + Q_ARG(QString, QString::fromStdString(label)), + Q_ARG(bool, isMine), + Q_ARG(QString, QString::fromStdString(purpose)), + Q_ARG(int, (int)status)); +} + static void ShowProgress(TransactionTableModel *ttm, const std::string &title, int nProgress) { if (nProgress == 0) @@ -838,6 +867,7 @@ void TransactionTableModel::subscribeToCoreSignals() { // Connect signals to wallet wallet->NotifyTransactionChanged.connect(boost::bind(NotifyTransactionChanged, this, _1, _2, _3)); + wallet->NotifyAddressBookChanged.connect(boost::bind(NotifyAddressBookChanged, this, _1, _2, _3, _4, _5, _6)); wallet->ShowProgress.connect(boost::bind(ShowProgress, this, _1, _2)); } @@ -845,5 +875,6 @@ void TransactionTableModel::unsubscribeFromCoreSignals() { // Disconnect signals from wallet wallet->NotifyTransactionChanged.disconnect(boost::bind(NotifyTransactionChanged, this, _1, _2, _3)); + wallet->NotifyAddressBookChanged.disconnect(boost::bind(NotifyAddressBookChanged, this, _1, _2, _3, _4, _5, _6)); wallet->ShowProgress.disconnect(boost::bind(ShowProgress, this, _1, _2)); } diff --git a/src/qt/transactiontablemodel.h b/src/qt/transactiontablemodel.h index e4dd52eac453..16669ac2ef5f 100644 --- a/src/qt/transactiontablemodel.h +++ b/src/qt/transactiontablemodel.h @@ -118,6 +118,8 @@ class TransactionTableModel : public QAbstractTableModel public Q_SLOTS: /* New transaction, or transaction changed status */ void updateTransaction(const QString &hash, int status, bool showTransaction); + void updateAddressBook(const QString &address, const QString &label, + bool isMine, const QString &purpose, int status); void updateConfirmations(); void updateDisplayUnit(); /** Updates the column title to "Amount (DisplayUnit)" and emits headerDataChanged() signal for table headers to react. */ diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index c877af059969..65b978b50a78 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -484,6 +484,11 @@ class CWalletTx : public CMerkleTx MarkDirty(); } + const CWallet* GetWallet() const + { + return pwallet; + } + //! filter decides which addresses will count towards the debit CAmount GetDebit(const isminefilter& filter) const; CAmount GetCredit(const isminefilter& filter) const; From 275725cdbbeb39204bd4e1d2b93adf5955c66fd5 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Wed, 16 Oct 2019 08:29:07 +0200 Subject: [PATCH 14/15] Review suggestions --- src/qt/transactionrecord.h | 6 +++--- src/qt/transactiontablemodel.cpp | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/qt/transactionrecord.h b/src/qt/transactionrecord.h index b01db16a3657..309f3fd005f4 100644 --- a/src/qt/transactionrecord.h +++ b/src/qt/transactionrecord.h @@ -23,7 +23,7 @@ class TransactionStatus TransactionStatus(): countsForBalance(false), lockedByInstantSend(false), lockedByChainLocks(false), sortKey(""), matures_in(0), status(Offline), depth(0), open_for(0), cur_num_blocks(-1), - cachedChainLockHeight(-1) + cachedChainLockHeight(-1), needsUpdate(false) { } enum Status { @@ -118,9 +118,9 @@ class TransactionRecord } 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), strAddress(_address), debit(_debit), credit(_credit), + hash(_hash), time(_time), type(_type), strAddress(_strAddress), debit(_debit), credit(_credit), idx(0) { address = CBitcoinAddress(strAddress); diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 23c758df41a7..6598b0757710 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -315,6 +315,7 @@ int TransactionTableModel::getChainLockHeight() const { return cachedChainLockHeight; } + int TransactionTableModel::rowCount(const QModelIndex &parent) const { Q_UNUSED(parent); From 7aa074f009498d5f5218480c1e4d4dd98c7c27a6 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Sat, 19 Oct 2019 10:42:49 +0200 Subject: [PATCH 15/15] Use proper columns in dataChanged call in updateAddressBook --- src/qt/transactiontablemodel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 6598b0757710..75fa2f43571f 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -183,7 +183,7 @@ class TransactionTablePriv for (auto& rec : cachedWallet) { if (rec.strAddress == address2) { rec.status.needsUpdate = true; - Q_EMIT parent->dataChanged(parent->index(index, TransactionTableModel::Status), parent->index(index, TransactionTableModel::Status)); + Q_EMIT parent->dataChanged(parent->index(index, TransactionTableModel::ToAddress), parent->index(index, TransactionTableModel::ToAddress)); } index++; }