From 90373b711e533c4d1aca3d88c054ce2743fcf77c Mon Sep 17 00:00:00 2001 From: Patrick Lodder Date: Mon, 13 Jun 2022 16:23:36 +0200 Subject: [PATCH] wallet: remove misleading information about transaction state Qt wallets display information to users about the broadcast state of transactions, without truly knowing the actual state. The fact that a peer has requested details for a transaction does not mean it was accepted to their mempool or relayed to any other peers and miners, because the only way to test if the transaction can be accepted is by requesting and processing it. We remove the "offline" and "maturity warning" statuses, which saves large wallets memory and processing time. Backported from: beef7ec4 Original author: Matt Corallo --- src/net_processing.cpp | 5 ---- src/qt/guiconstants.h | 2 -- src/qt/transactiondesc.cpp | 10 -------- src/qt/transactionrecord.cpp | 8 ------ src/qt/transactionrecord.h | 4 +-- src/qt/transactiontablemodel.cpp | 9 ------- src/validationinterface.cpp | 5 ---- src/validationinterface.h | 3 --- src/wallet/wallet.cpp | 42 -------------------------------- src/wallet/wallet.h | 19 +-------------- 10 files changed, 2 insertions(+), 105 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 2544c2d83..38fa071ff 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1141,9 +1141,6 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam } } - // Track requests for our stuff. - GetMainSignals().Inventory(inv.hash); - if (inv.type == MSG_BLOCK || inv.type == MSG_FILTERED_BLOCK || inv.type == MSG_CMPCT_BLOCK || inv.type == MSG_WITNESS_BLOCK) break; } @@ -1585,8 +1582,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr pfrom->AskFor(inv); } - // Track requests for our stuff - GetMainSignals().Inventory(inv.hash); } if (!vToFetch.empty()) diff --git a/src/qt/guiconstants.h b/src/qt/guiconstants.h index 48cd4fb90..7004c5db3 100644 --- a/src/qt/guiconstants.h +++ b/src/qt/guiconstants.h @@ -29,8 +29,6 @@ static const bool DEFAULT_SPLASHSCREEN = true; #define COLOR_BAREADDRESS QColor(140, 140, 140) /* Transaction list -- TX status decoration - open until date */ #define COLOR_TX_STATUS_OPENUNTILDATE QColor(64, 64, 255) -/* Transaction list -- TX status decoration - offline */ -#define COLOR_TX_STATUS_OFFLINE QColor(192, 192, 192) /* Transaction list -- TX status decoration - danger, tx needs attention */ #define COLOR_TX_STATUS_DANGER QColor(200, 100, 100) /* Transaction list -- TX status decoration - default color */ diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index fa02501dc..5c300a2e8 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -37,8 +37,6 @@ QString TransactionDesc::FormatTxStatus(const CWalletTx& wtx) int nDepth = wtx.GetDepthInMainChain(); if (nDepth < 0) return tr("conflicted with a transaction with %1 confirmations").arg(-nDepth); - else if (GetAdjustedTime() - wtx.nTimeReceived > 2 * 60 && wtx.GetRequestCount() == 0) - return tr("%1/offline").arg(nDepth); else if (nDepth == 0) return tr("0/unconfirmed, %1").arg((wtx.InMempool() ? tr("in memory pool") : tr("not in memory pool"))) + (wtx.isAbandoned() ? ", "+tr("abandoned") : ""); else if (nDepth < 6) @@ -62,14 +60,6 @@ QString TransactionDesc::toHTML(CWallet *wallet, CWalletTx &wtx, TransactionReco CAmount nNet = nCredit - nDebit; strHTML += "" + tr("Status") + ": " + FormatTxStatus(wtx); - int nRequests = wtx.GetRequestCount(); - if (nRequests != -1) - { - if (nRequests == 0) - strHTML += tr(", has not been successfully broadcast yet"); - else if (nRequests > 0) - strHTML += tr(", broadcast through %n node(s)", "", nRequests); - } strHTML += "
"; strHTML += "" + tr("Date") + ": " + (nTime ? GUIUtil::dateTimeStr(nTime) : "") + "
"; diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index a9d9b6887..9fee17ae4 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -212,10 +212,6 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx) if (wtx.IsInMainChain()) { status.matures_in = wtx.GetBlocksToMaturity(); - - // Check if the block was requested by anyone - if (GetAdjustedTime() - wtx.nTimeReceived > 2 * 60 && wtx.GetRequestCount() == 0) - status.status = TransactionStatus::MaturesWarning; } else { @@ -233,10 +229,6 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx) { status.status = TransactionStatus::Conflicted; } - else if (GetAdjustedTime() - wtx.nTimeReceived > 2 * 60 && wtx.GetRequestCount() == 0) - { - status.status = TransactionStatus::Offline; - } else if (status.depth == 0) { status.status = TransactionStatus::Unconfirmed; diff --git a/src/qt/transactionrecord.h b/src/qt/transactionrecord.h index 5aabbbffa..96ae3f553 100644 --- a/src/qt/transactionrecord.h +++ b/src/qt/transactionrecord.h @@ -21,7 +21,7 @@ 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(Unconfirmed), depth(0), open_for(0), cur_num_blocks(-1) { } enum Status { @@ -29,14 +29,12 @@ public: /// Normal (sent/received) transactions OpenUntilDate, /**< Transaction not yet final, waiting for date */ OpenUntilBlock, /**< Transaction not yet final, waiting for block */ - Offline, /**< Not sent to any other nodes **/ Unconfirmed, /**< Not yet mined into a block **/ Confirming, /**< Confirmed, but waiting for the recommended number of confirmations **/ Conflicted, /**< Conflicts with other transaction or mempool **/ Abandoned, /**< Abandoned from the wallet **/ /// Generated (mined) transactions Immature, /**< Mined but waiting for maturity */ - MaturesWarning, /**< Transaction will likely not mature because no nodes have confirmed */ NotAccepted /**< Mined but not accepted */ }; diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 57d6787c2..7e6e726d3 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -307,9 +307,6 @@ QString TransactionTableModel::formatTxStatus(const TransactionRecord *wtx) cons case TransactionStatus::OpenUntilDate: status = tr("Open until %1").arg(GUIUtil::dateTimeStr(wtx->status.open_for)); break; - case TransactionStatus::Offline: - status = tr("Offline"); - break; case TransactionStatus::Unconfirmed: status = tr("Unconfirmed"); break; @@ -328,9 +325,6 @@ QString TransactionTableModel::formatTxStatus(const TransactionRecord *wtx) cons case TransactionStatus::Immature: status = tr("Immature (%1 confirmations, will be available after %2)").arg(wtx->status.depth).arg(wtx->status.depth + wtx->status.matures_in); break; - case TransactionStatus::MaturesWarning: - status = tr("This block was not received by any other nodes and will probably not be accepted!"); - break; case TransactionStatus::NotAccepted: status = tr("Generated but not accepted"); break; @@ -468,8 +462,6 @@ QVariant TransactionTableModel::txStatusDecoration(const TransactionRecord *wtx) case TransactionStatus::OpenUntilBlock: case TransactionStatus::OpenUntilDate: return COLOR_TX_STATUS_OPENUNTILDATE; - case TransactionStatus::Offline: - return COLOR_TX_STATUS_OFFLINE; case TransactionStatus::Unconfirmed: return QIcon(":/icons/transaction_0"); case TransactionStatus::Abandoned: @@ -492,7 +484,6 @@ QVariant TransactionTableModel::txStatusDecoration(const TransactionRecord *wtx) int part = (wtx->status.depth * 4 / total) + 1; return QIcon(QString(":/icons/transaction_%1").arg(part)); } - case TransactionStatus::MaturesWarning: case TransactionStatus::NotAccepted: return QIcon(":/icons/transaction_0"); default: diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index c34d7d326..a26c18081 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -27,8 +27,6 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) { pwalletIn, boost::placeholders::_1)); g_signals.SetBestChain.connect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, boost::placeholders::_1)); - g_signals.Inventory.connect(boost::bind(&CValidationInterface::Inventory, - pwalletIn, boost::placeholders::_1)); g_signals.Broadcast.connect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, boost::placeholders::_1, boost::placeholders::_2)); g_signals.BlockChecked.connect(boost::bind(&CValidationInterface::BlockChecked, @@ -54,8 +52,6 @@ void UnregisterValidationInterface(CValidationInterface* pwalletIn) { g_signals.Broadcast.disconnect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, boost::placeholders::_1, boost::placeholders::_2)); - g_signals.Inventory.disconnect(boost::bind(&CValidationInterface::Inventory, - pwalletIn, boost::placeholders::_1)); g_signals.SetBestChain.disconnect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, boost::placeholders::_1)); g_signals.UpdatedTransaction.disconnect(boost::bind(&CValidationInterface::UpdatedTransaction, @@ -78,7 +74,6 @@ void UnregisterAllValidationInterfaces() { g_signals.ScriptForMining.disconnect_all_slots(); g_signals.BlockChecked.disconnect_all_slots(); g_signals.Broadcast.disconnect_all_slots(); - g_signals.Inventory.disconnect_all_slots(); g_signals.SetBestChain.disconnect_all_slots(); g_signals.UpdatedTransaction.disconnect_all_slots(); g_signals.SyncTransaction.disconnect_all_slots(); diff --git a/src/validationinterface.h b/src/validationinterface.h index a2e76f203..c98695da6 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -36,7 +36,6 @@ protected: virtual void SyncTransaction(const CTransaction &tx, const CBlockIndex *pindex, int posInBlock) {} virtual void SetBestChain(const CBlockLocator &locator) {} virtual void UpdatedTransaction(const uint256 &hash) {} - virtual void Inventory(const uint256 &hash) {} virtual void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) {} virtual void BlockChecked(const CBlock&, const CValidationState&) {} virtual void GetScriptForMining(boost::shared_ptr&) {}; @@ -65,8 +64,6 @@ struct CMainSignals { boost::signals2::signal UpdatedTransaction; /** Notifies listeners of a new active block chain. */ boost::signals2::signal SetBestChain; - /** Notifies listeners about an inventory item being seen on the network. */ - boost::signals2::signal Inventory; /** Tells listeners to broadcast their data. */ boost::signals2::signal Broadcast; /** Notifies listeners of a block validation result */ diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index bda823415..66283dc73 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1417,45 +1417,6 @@ int64_t CWalletTx::GetTxTime() const return n ? n : nTimeReceived; } -int CWalletTx::GetRequestCount() const -{ - // Returns -1 if it wasn't being tracked - int nRequests = -1; - { - LOCK(pwallet->cs_wallet); - if (IsCoinBase()) - { - // Generated block - if (!hashUnset()) - { - map::const_iterator mi = pwallet->mapRequestCount.find(hashBlock); - if (mi != pwallet->mapRequestCount.end()) - nRequests = (*mi).second; - } - } - else - { - // Did anyone request this transaction? - map::const_iterator mi = pwallet->mapRequestCount.find(GetHash()); - if (mi != pwallet->mapRequestCount.end()) - { - nRequests = (*mi).second; - - // How about the block it's in? - if (nRequests == 0 && !hashUnset()) - { - map::const_iterator _mi = pwallet->mapRequestCount.find(hashBlock); - if (_mi != pwallet->mapRequestCount.end()) - nRequests = (*_mi).second; - else - nRequests = 1; // If it's in someone else's block it must have got out - } - } - } - } - return nRequests; -} - void CWalletTx::GetAmounts(list& listReceived, list& listSent, CAmount& nFee, string& strSentAccount, const isminefilter& filter) const { @@ -2870,9 +2831,6 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CCon } } - // Track how many getdata requests our transaction gets - mapRequestCount[wtxNew.GetHash()] = 0; - if (fBroadcastTransactions) { // Broadcast diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index c8e58a154..8d35a727d 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -394,7 +394,6 @@ public: bool IsTrusted() const; int64_t GetTxTime() const; - int GetRequestCount() const; bool RelayWalletTransaction(CConnman* connman); @@ -668,7 +667,6 @@ public: TxItems wtxOrdered; int64_t nOrderPosNext; - std::map mapRequestCount; std::map mapAddressBook; @@ -876,23 +874,8 @@ public: void UpdatedTransaction(const uint256 &hashTx) override; - void Inventory(const uint256 &hash) override - { - { - LOCK(cs_wallet); - std::map::iterator mi = mapRequestCount.find(hash); - if (mi != mapRequestCount.end()) - (*mi).second++; - } - } - void GetScriptForMining(boost::shared_ptr &script) override; - void ResetRequestCount(const uint256 &hash) override - { - LOCK(cs_wallet); - mapRequestCount[hash] = 0; - }; - + unsigned int GetKeyPoolSize() { AssertLockHeld(cs_wallet); // setKeyPool