From ada5a067c75f19a724cc054286ecf2254e5dbe8f Mon Sep 17 00:00:00 2001 From: Tom Harding Date: Thu, 26 Jun 2014 18:31:05 -0700 Subject: [PATCH] UI to alert of respend attempt affecting wallet. Respend transactions that conflict with transactions already in the wallet are added to it. They are not displayed unless they also involve the wallet, or get into a block. If they do not involve the wallet, they continue not to affect balance. Transactions that involve the wallet, and have conflicting non-equivalent transactions, are highlighted in red. When the conflict first occurs, a modal dialog is thrown. CWallet::SyncMetaData is changed to sync only to equivalent transactions. When a conflict is added to the wallet, counter nConflictsReceived is incremented. This acts like a change in active block height for the purpose of triggering UI updates. --- src/qt/guiconstants.h | 4 ++++ src/qt/transactionfilterproxy.cpp | 4 ++-- src/qt/transactionrecord.cpp | 10 +++++++--- src/qt/transactionrecord.h | 13 ++++++++++-- src/qt/transactiontablemodel.cpp | 18 ++++++++++++----- src/qt/walletmodel.cpp | 8 ++++++++ src/ui_interface.h | 3 ++- src/wallet.cpp | 33 +++++++++++++++++++++++++------ src/wallet.h | 15 ++++++++++++-- 9 files changed, 87 insertions(+), 21 deletions(-) diff --git a/src/qt/guiconstants.h b/src/qt/guiconstants.h index 5ae4bc833..44e361e1e 100644 --- a/src/qt/guiconstants.h +++ b/src/qt/guiconstants.h @@ -23,6 +23,10 @@ static const int STATUSBAR_ICONSIZE = 16; #define COLOR_NEGATIVE QColor(255, 0, 0) /* Transaction list -- bare address (without label) */ #define COLOR_BAREADDRESS QColor(140, 140, 140) +/* Transaction list -- has conflicting transactions */ +#define COLOR_HASCONFLICTING Qt::white; +/* Transaction list -- has conflicting transactions - background */ +#define COLOR_HASCONFLICTING_BG QColor(192, 0, 0) /* Tooltips longer than this (in characters) are converted into rich text, so that they can be word-wrapped. diff --git a/src/qt/transactionfilterproxy.cpp b/src/qt/transactionfilterproxy.cpp index f9546fddb..729302978 100644 --- a/src/qt/transactionfilterproxy.cpp +++ b/src/qt/transactionfilterproxy.cpp @@ -24,7 +24,7 @@ TransactionFilterProxy::TransactionFilterProxy(QObject *parent) : typeFilter(ALL_TYPES), minAmount(0), limitRows(-1), - showInactive(true) + showInactive(false) { } @@ -39,7 +39,7 @@ bool TransactionFilterProxy::filterAcceptsRow(int sourceRow, const QModelIndex & qint64 amount = llabs(index.data(TransactionTableModel::AmountRole).toLongLong()); int status = index.data(TransactionTableModel::StatusRole).toInt(); - if(!showInactive && status == TransactionStatus::Conflicted) + if(!showInactive && status == TransactionStatus::Conflicted && type == TransactionRecord::Other) return false; if(!(TYPE(type) & typeFilter)) return false; diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index eec2b57e8..21f1b7356 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -170,6 +170,8 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx) status.depth = wtx.GetDepthInMainChain(); status.cur_num_blocks = chainActive.Height(); + status.hasConflicting = false; + if (!IsFinalTx(wtx, chainActive.Height() + 1)) { if (wtx.nLockTime < LOCKTIME_THRESHOLD) @@ -213,6 +215,7 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx) if (status.depth < 0) { status.status = TransactionStatus::Conflicted; + status.hasConflicting = !(wtx.GetConflicts(false).empty()); } else if (GetAdjustedTime() - wtx.nTimeReceived > 2 * 60 && wtx.GetRequestCount() == 0) { @@ -221,6 +224,7 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx) else if (status.depth == 0) { status.status = TransactionStatus::Unconfirmed; + status.hasConflicting = !(wtx.GetConflicts(false).empty()); } else if (status.depth < RecommendedNumConfirmations) { @@ -231,13 +235,13 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx) status.status = TransactionStatus::Confirmed; } } - } -bool TransactionRecord::statusUpdateNeeded() +bool TransactionRecord::statusUpdateNeeded(int64_t nConflictsReceived) { AssertLockHeld(cs_main); - return status.cur_num_blocks != chainActive.Height(); + return (status.cur_num_blocks != chainActive.Height() || + status.cur_num_conflicts != nConflictsReceived); } QString TransactionRecord::getTxID() const diff --git a/src/qt/transactionrecord.h b/src/qt/transactionrecord.h index af6fd403b..4c2847144 100644 --- a/src/qt/transactionrecord.h +++ b/src/qt/transactionrecord.h @@ -20,7 +20,8 @@ class TransactionStatus public: TransactionStatus(): countsForBalance(false), sortKey(""), - matures_in(0), status(Offline), depth(0), open_for(0), cur_num_blocks(-1) + matures_in(0), status(Offline), hasConflicting(false), depth(0), open_for(0), cur_num_blocks(-1), + cur_num_conflicts(-1) { } enum Status { @@ -51,6 +52,10 @@ public: /** @name Reported status @{*/ Status status; + + // Has conflicting transactions spending same prevout + bool hasConflicting; + qint64 depth; qint64 open_for; /**< Timestamp if status==OpenUntilDate, otherwise number of additional blocks that need to be mined before @@ -59,6 +64,10 @@ public: /** Current number of blocks (to know whether cached status is still valid) */ int cur_num_blocks; + + /** Number of conflicts received into wallet as of last status update */ + int64_t cur_num_conflicts; + }; /** UI model for a transaction. A core transaction can be represented by multiple UI transactions if it has @@ -133,7 +142,7 @@ public: /** Return whether a status update is needed. */ - bool statusUpdateNeeded(); + bool statusUpdateNeeded(int64_t nConflictsReceived); }; #endif // TRANSACTIONRECORD_H diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index b9fcd0d6b..eb5c3abdb 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -168,8 +168,7 @@ public: parent->endRemoveRows(); break; case CT_UPDATED: - // Miscellaneous updates -- nothing to do, status update will take care of this, and is only computed for - // visible transactions. + emit parent->dataChanged(parent->index(lowerIndex, parent->Status), parent->index(upperIndex-1, parent->Amount)); break; } } @@ -190,20 +189,21 @@ public: // stuck if the core is holding the locks for a longer time - for // example, during a wallet rescan. // - // If a status update is needed (blocks came in since last check), - // update the status of this transaction from the wallet. Otherwise, + // If a status update is needed (blocks or conflicts came in since last check), + // update the status of this transaction from the wallet. Otherwise, // simply re-use the cached status. TRY_LOCK(cs_main, lockMain); if(lockMain) { TRY_LOCK(wallet->cs_wallet, lockWallet); - if(lockWallet && rec->statusUpdateNeeded()) + if(lockWallet && rec->statusUpdateNeeded(wallet->nConflictsReceived)) { std::map::iterator mi = wallet->mapWallet.find(rec->hash); if(mi != wallet->mapWallet.end()) { rec->updateStatus(mi->second); + rec->status.cur_num_conflicts = wallet->nConflictsReceived; } } } @@ -363,6 +363,8 @@ QString TransactionTableModel::formatTxType(const TransactionRecord *wtx) const return tr("Payment to yourself"); case TransactionRecord::Generated: return tr("Mined"); + case TransactionRecord::Other: + return tr("Other"); default: return QString(); } @@ -535,7 +537,13 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const return formatTooltip(rec); case Qt::TextAlignmentRole: return column_alignments[index.column()]; + case Qt::BackgroundColorRole: + if (rec->status.hasConflicting) + return COLOR_HASCONFLICTING_BG; + break; case Qt::ForegroundRole: + if (rec->status.hasConflicting) + return COLOR_HASCONFLICTING; // Non-confirmed (but not immature) as transactions are grey if(!rec->status.countsForBalance && rec->status.status != TransactionStatus::Immature) { diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 2f633a26c..3d6f2ab9b 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -138,6 +138,14 @@ void WalletModel::checkBalanceChanged() void WalletModel::updateTransaction(const QString &hash, int status) { + if (status == CT_GOT_CONFLICT) + { + emit message(tr("Conflict Received"), + tr("WARNING: Transaction may never be confirmed. Its input was seen being spent by another transaction on the network. Wait for confirmation!"), + CClientUIInterface::MSG_WARNING); + return; + } + if(transactionTableModel) transactionTableModel->updateTransaction(hash, status); diff --git a/src/ui_interface.h b/src/ui_interface.h index e1a3e04e1..e9fcd91d4 100644 --- a/src/ui_interface.h +++ b/src/ui_interface.h @@ -21,7 +21,8 @@ enum ChangeType { CT_NEW, CT_UPDATED, - CT_DELETED + CT_DELETED, + CT_GOT_CONFLICT }; /** Signals for UI communication. */ diff --git a/src/wallet.cpp b/src/wallet.cpp index f6fd6e958..79cc5ba3c 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -256,7 +256,7 @@ bool CWallet::SetMaxVersion(int nVersion) return true; } -set CWallet::GetConflicts(const uint256& txid) const +set CWallet::GetConflicts(const uint256& txid, bool includeEquivalent) const { set result; AssertLockHeld(cs_wallet); @@ -274,7 +274,8 @@ set CWallet::GetConflicts(const uint256& txid) const continue; // No conflict if zero or one spends range = mapTxSpends.equal_range(txin.prevout); for (TxSpends::const_iterator it = range.first; it != range.second; ++it) - result.insert(it->second); + if (includeEquivalent || !wtx.IsEquivalentTo(mapWallet.at(it->second))) + result.insert(it->second); } return result; } @@ -303,6 +304,7 @@ void CWallet::SyncMetaData(pair range) const uint256& hash = it->second; CWalletTx* copyTo = &mapWallet[hash]; if (copyFrom == copyTo) continue; + if (!copyFrom->IsEquivalentTo(*copyTo)) continue; copyTo->mapValue = copyFrom->mapValue; copyTo->vOrderForm = copyFrom->vOrderForm; // fTimeReceivedIsTxTime not copied on purpose @@ -588,6 +590,20 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet) // Notify UI of new or updated transaction NotifyTransactionChanged(this, hash, fInsertedNew ? CT_NEW : CT_UPDATED); + // Notifications for existing transactions that now have conflicts with this one + if (fInsertedNew) + { + BOOST_FOREACH(const uint256& conflictHash, wtxIn.GetConflicts(false)) + { + CWalletTx& txConflict = mapWallet[conflictHash]; + NotifyTransactionChanged(this, conflictHash, CT_UPDATED); //Updates UI table + if (IsFromMe(txConflict) || IsMine(txConflict)) + { + NotifyTransactionChanged(this, conflictHash, CT_GOT_CONFLICT); //Throws dialog + } + } + } + // notify an external script when a wallet transaction comes in or is updated std::string strCmd = GetArg("-walletnotify", ""); @@ -610,7 +626,12 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl AssertLockHeld(cs_wallet); bool fExisted = mapWallet.count(tx.GetHash()); if (fExisted && !fUpdate) return false; - if (fExisted || IsMine(tx) || IsFromMe(tx)) + + bool fIsConflicting = IsConflicting(tx); + if (fIsConflicting) + nConflictsReceived++; + + if (fExisted || IsMine(tx) || IsFromMe(tx) || fIsConflicting) { CWalletTx wtx(this,tx); // Get merkle branch if transaction was found in a block @@ -896,7 +917,7 @@ void CWallet::ReacceptWalletTransactions() int nDepth = wtx.GetDepthInMainChain(); - if (!wtx.IsCoinBase() && nDepth < 0) + if (!wtx.IsCoinBase() && nDepth < 0 && (IsMine(wtx) || IsFromMe(wtx))) { // Try to add to memory pool LOCK(mempool.cs); @@ -916,13 +937,13 @@ void CWalletTx::RelayWalletTransaction() } } -set CWalletTx::GetConflicts() const +set CWalletTx::GetConflicts(bool includeEquivalent) const { set result; if (pwallet != NULL) { uint256 myHash = GetHash(); - result = pwallet->GetConflicts(myHash); + result = pwallet->GetConflicts(myHash, includeEquivalent); result.erase(myHash); } return result; diff --git a/src/wallet.h b/src/wallet.h index 8494ce9a3..1c2512d67 100644 --- a/src/wallet.h +++ b/src/wallet.h @@ -141,6 +141,9 @@ public: MasterKeyMap mapMasterKeys; unsigned int nMasterKeyMaxID; + // Increment to cause UI refresh, similar to new block + int64_t nConflictsReceived; + CWallet() { SetNull(); @@ -163,6 +166,7 @@ public: nNextResend = 0; nLastResend = 0; nTimeFirstKey = 0; + nConflictsReceived = 0; } std::map mapWallet; @@ -305,6 +309,13 @@ public: { return (GetDebit(tx) > 0); } + bool IsConflicting(const CTransaction& tx) const + { + BOOST_FOREACH(const CTxIn& txin, tx.vin) + if (mapTxSpends.count(txin.prevout)) + return true; + return false; + } int64_t GetDebit(const CTransaction& tx) const { int64_t nDebit = 0; @@ -377,7 +388,7 @@ public: int GetVersion() { LOCK(cs_wallet); return nWalletVersion; } // Get wallet transactions that conflict with given transaction (spend same outputs) - std::set GetConflicts(const uint256& txid) const; + std::set GetConflicts(const uint256& txid, bool includeEquivalent) const; /** Address book entry changed. * @note called with lock cs_wallet held. @@ -699,7 +710,7 @@ public: void RelayWalletTransaction(); - std::set GetConflicts() const; + std::set GetConflicts(bool includeEquivalent=true) const; };