Merge #13622: Remove mapRequest tracking that just effects Qt display.

laanwj · laanwj · commit e538a952d5f0 · 2018-07-11T16:36:15.000+02:00
beef7ec Remove useless mapRequest tracking that just effects Qt display. (Matt Corallo) Pull request description: I thought we had removed this a long time ago, TBH, its really confusing feedback to users that we display whether a tx was broadcast to immediate neighbor nodes, given that has little indication of whether the tx propagated very far. Tree-SHA512: c9d0808fb60146919bb78d0860ec2193601966c944887eaae7837408422f7e85dfdb306407a613200cdd4726aec66da18df618ebc6a8cfe8650bf08d4a8dc155
diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp
@@ -93,7 +93,6 @@ WalletTxStatus MakeWalletTxStatus(const CWalletTx& wtx)
     result.block_height = (block ? block->nHeight : std::numeric_limits<int>::max()),
     result.blocks_to_maturity = wtx.GetBlocksToMaturity();
     result.depth_in_main_chain = wtx.GetDepthInMainChain();
-    result.request_count = wtx.GetRequestCount();
     result.time_received = wtx.nTimeReceived;
     result.lock_time = wtx.tx->nLockTime;
     result.is_final = CheckFinalTx(*wtx.tx);
diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
@@ -346,7 +346,6 @@ struct WalletTxStatus
     int block_height;
     int blocks_to_maturity;
     int depth_in_main_chain;
-    int request_count;
     unsigned int time_received;
     uint32_t lock_time;
     bool is_final;
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -1264,9 +1264,6 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm
             if (!push) {
                 vNotFound.push_back(inv);
             }
-
-            // Track requests for our stuff.
-            GetMainSignals().Inventory(inv.hash);
         }
     } // release cs_main
 
@@ -1965,9 +1962,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
                     pfrom->AskFor(inv);
                 }
             }
-
-            // Track requests for our stuff
-            GetMainSignals().Inventory(inv.hash);
         }
     }
 
diff --git a/src/qt/guiconstants.h b/src/qt/guiconstants.h
@@ -27,8 +27,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
@@ -37,8 +37,6 @@ QString TransactionDesc::FormatTxStatus(const interfaces::WalletTx& wtx, const i
         int nDepth = status.depth_in_main_chain;
         if (nDepth < 0)
             return tr("conflicted with a transaction with %1 confirmations").arg(-nDepth);
-        else if (adjustedTime - status.time_received > 2 * 60 && status.request_count == 0)
-            return tr("%1/offline").arg(nDepth);
         else if (nDepth == 0)
             return tr("0/unconfirmed, %1").arg((inMempool ? tr("in memory pool") : tr("not in memory pool"))) + (status.is_abandoned ? ", "+tr("abandoned") : "");
         else if (nDepth < 6)
@@ -68,14 +66,6 @@ QString TransactionDesc::toHTML(interfaces::Node& node, interfaces::Wallet& wall
     CAmount nNet = nCredit - nDebit;
 
     strHTML += "<b>" + tr("Status") + ":</b> " + FormatTxStatus(wtx, status, inMempool, numBlocks, adjustedTime);
-    int nRequests = status.request_count;
-    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 += "<br>";
 
     strHTML += "<b>" + tr("Date") + ":</b> " + (nTime ? GUIUtil::dateTimeStr(nTime) : "") + "<br>";
diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp
@@ -195,10 +195,6 @@ void TransactionRecord::updateStatus(const interfaces::WalletTxStatus& wtx, int
             if (wtx.is_in_main_chain)
             {
                 status.matures_in = wtx.blocks_to_maturity;
-
-                // Check if the block was requested by anyone
-                if (adjustedTime - wtx.time_received > 2 * 60 && wtx.request_count == 0)
-                    status.status = TransactionStatus::MaturesWarning;
             }
             else
             {
@@ -216,10 +212,6 @@ void TransactionRecord::updateStatus(const interfaces::WalletTxStatus& wtx, int
         {
             status.status = TransactionStatus::Conflicted;
         }
-        else if (adjustedTime - wtx.time_received > 2 * 60 && wtx.request_count == 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
@@ -25,22 +25,20 @@ 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 {
         Confirmed,          /**< Have 6 or more confirmations (normal tx) or fully mature (mined tx) **/
         /// 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
@@ -286,9 +286,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;
@@ -307,9 +304,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;
@@ -447,8 +441,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:
@@ -471,7 +463,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
@@ -25,7 +25,6 @@ struct MainSignalsInstance {
     boost::signals2::signal<void (const std::shared_ptr<const CBlock> &)> BlockDisconnected;
     boost::signals2::signal<void (const CTransactionRef &)> TransactionRemovedFromMempool;
     boost::signals2::signal<void (const CBlockLocator &)> ChainStateFlushed;
-    boost::signals2::signal<void (const uint256 &)> Inventory;
     boost::signals2::signal<void (int64_t nBestBlockTime, CConnman* connman)> Broadcast;
     boost::signals2::signal<void (const CBlock&, const CValidationState&)> BlockChecked;
     boost::signals2::signal<void (const CBlockIndex *, const std::shared_ptr<const CBlock>&)> NewPoWValidBlock;
@@ -80,7 +79,6 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) {
     g_signals.m_internals->BlockDisconnected.connect(boost::bind(&CValidationInterface::BlockDisconnected, pwalletIn, _1));
     g_signals.m_internals->TransactionRemovedFromMempool.connect(boost::bind(&CValidationInterface::TransactionRemovedFromMempool, pwalletIn, _1));
     g_signals.m_internals->ChainStateFlushed.connect(boost::bind(&CValidationInterface::ChainStateFlushed, pwalletIn, _1));
-    g_signals.m_internals->Inventory.connect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1));
     g_signals.m_internals->Broadcast.connect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1, _2));
     g_signals.m_internals->BlockChecked.connect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2));
     g_signals.m_internals->NewPoWValidBlock.connect(boost::bind(&CValidationInterface::NewPoWValidBlock, pwalletIn, _1, _2));
@@ -89,7 +87,6 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) {
 void UnregisterValidationInterface(CValidationInterface* pwalletIn) {
     g_signals.m_internals->BlockChecked.disconnect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2));
     g_signals.m_internals->Broadcast.disconnect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1, _2));
-    g_signals.m_internals->Inventory.disconnect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1));
     g_signals.m_internals->ChainStateFlushed.disconnect(boost::bind(&CValidationInterface::ChainStateFlushed, pwalletIn, _1));
     g_signals.m_internals->TransactionAddedToMempool.disconnect(boost::bind(&CValidationInterface::TransactionAddedToMempool, pwalletIn, _1));
     g_signals.m_internals->BlockConnected.disconnect(boost::bind(&CValidationInterface::BlockConnected, pwalletIn, _1, _2, _3));
@@ -105,7 +102,6 @@ void UnregisterAllValidationInterfaces() {
     }
     g_signals.m_internals->BlockChecked.disconnect_all_slots();
     g_signals.m_internals->Broadcast.disconnect_all_slots();
-    g_signals.m_internals->Inventory.disconnect_all_slots();
     g_signals.m_internals->ChainStateFlushed.disconnect_all_slots();
     g_signals.m_internals->TransactionAddedToMempool.disconnect_all_slots();
     g_signals.m_internals->BlockConnected.disconnect_all_slots();
@@ -171,12 +167,6 @@ void CMainSignals::ChainStateFlushed(const CBlockLocator &locator) {
     });
 }
 
-void CMainSignals::Inventory(const uint256 &hash) {
-    m_internals->m_schedulerClient.AddToProcessQueue([hash, this] {
-        m_internals->Inventory(hash);
-    });
-}
-
 void CMainSignals::Broadcast(int64_t nBestBlockTime, CConnman* connman) {
     m_internals->Broadcast(nBestBlockTime, connman);
 }
diff --git a/src/validationinterface.h b/src/validationinterface.h
@@ -117,12 +117,6 @@ class CValidationInterface {
      * Called on a background thread.
      */
     virtual void ChainStateFlushed(const CBlockLocator &locator) {}
-    /**
-     * Notifies listeners about an inventory item being seen on the network.
-     *
-     * Called on a background thread.
-     */
-    virtual void Inventory(const uint256 &hash) {}
     /** Tells listeners to broadcast their data. */
     virtual void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) {}
     /**
@@ -173,7 +167,6 @@ class CMainSignals {
     void BlockConnected(const std::shared_ptr<const CBlock> &, const CBlockIndex *pindex, const std::shared_ptr<const std::vector<CTransactionRef>> &);
     void BlockDisconnected(const std::shared_ptr<const CBlock> &);
     void ChainStateFlushed(const CBlockLocator &);
-    void Inventory(const uint256 &);
     void Broadcast(int64_t nBestBlockTime, CConnman* connman);
     void BlockChecked(const CBlock&, const CValidationState&);
     void NewPoWValidBlock(const CBlockIndex *, const std::shared_ptr<const CBlock>&);
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
@@ -1532,45 +1532,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())
-            {
-                std::map<uint256, int>::const_iterator mi = pwallet->mapRequestCount.find(hashBlock);
-                if (mi != pwallet->mapRequestCount.end())
-                    nRequests = (*mi).second;
-            }
-        }
-        else
-        {
-            // Did anyone request this transaction?
-            std::map<uint256, int>::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())
-                {
-                    std::map<uint256, int>::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;
-}
-
 // Helper for producing a max-sized low-S signature (eg 72 bytes)
 bool CWallet::DummySignInput(CTxIn &tx_in, const CTxOut &txout) const
 {
@@ -3145,9 +3106,6 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve
             }
         }
 
-        // Track how many getdata requests our transaction gets
-        mapRequestCount[wtxNew.GetHash()] = 0;
-
         // Get the inserted-CWalletTx from mapWallet so that the
         // fInMempool flag is cached properly
         CWalletTx& wtx = mapWallet.at(wtxNew.GetHash());
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
@@ -486,7 +486,6 @@ class CWalletTx : public CMerkleTx
     bool IsTrusted() const;
 
     int64_t GetTxTime() const;
-    int GetRequestCount() const;
 
     // RelayWalletTransaction may only be called if fBroadcastTransactions!
     bool RelayWalletTransaction(CConnman* connman);
@@ -820,7 +819,6 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface
 
     int64_t nOrderPosNext = 0;
     uint64_t nAccountingEntryNumber = 0;
-    std::map<uint256, int> mapRequestCount;
 
     std::map<CTxDestination, CAddressBookData> mapAddressBook;
 
@@ -1064,16 +1062,6 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface
 
     const std::string& GetLabelName(const CScript& scriptPubKey) const;
 
-    void Inventory(const uint256 &hash) override
-    {
-        {
-            LOCK(cs_wallet);
-            std::map<uint256, int>::iterator mi = mapRequestCount.find(hash);
-            if (mi != mapRequestCount.end())
-                (*mi).second++;
-        }
-    }
-
     void GetScriptForMining(std::shared_ptr<CReserveScript> &script);
 
     unsigned int GetKeyPoolSize() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet)

Original file line number	Diff line number	Diff line change
`@@ -1264,9 +1264,6 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm`
`1264`	`1264`	`if (!push) {`
`1265`	`1265`	`vNotFound.push_back(inv);`
`1266`	`1266`	`}`
`1267`		`-`
`1268`		`- // Track requests for our stuff.`
`1269`		`- GetMainSignals().Inventory(inv.hash);`
`1270`	`1267`	`}`
`1271`	`1268`	`} // release cs_main`
`1272`	`1269`
`@@ -1965,9 +1962,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr`
`1965`	`1962`	`pfrom->AskFor(inv);`
`1966`	`1963`	`}`
`1967`	`1964`	`}`
`1968`		`-`
`1969`		`- // Track requests for our stuff`
`1970`		`- GetMainSignals().Inventory(inv.hash);`
`1971`	`1965`	`}`
`1972`	`1966`	`}`
`1973`	`1967`
Original file line number	Diff line number	Diff line change
`@@ -195,10 +195,6 @@ void TransactionRecord::updateStatus(const interfaces::WalletTxStatus& wtx, int`
`195`	`195`	`if (wtx.is_in_main_chain)`
`196`	`196`	`{`
`197`	`197`	`status.matures_in = wtx.blocks_to_maturity;`
`198`		`-`
`199`		`- // Check if the block was requested by anyone`
`200`		`- if (adjustedTime - wtx.time_received > 2 * 60 && wtx.request_count == 0)`
`201`		`- status.status = TransactionStatus::MaturesWarning;`
`202`	`198`	`}`
`203`	`199`	`else`
`204`	`200`	`{`
`@@ -216,10 +212,6 @@ void TransactionRecord::updateStatus(const interfaces::WalletTxStatus& wtx, int`
`216`	`212`	`{`
`217`	`213`	`status.status = TransactionStatus::Conflicted;`
`218`	`214`	`}`
`219`		`- else if (adjustedTime - wtx.time_received > 2 * 60 && wtx.request_count == 0)`
`220`		`- {`
`221`		`- status.status = TransactionStatus::Offline;`
`222`		`- }`
`223`	`215`	`else if (status.depth == 0)`
`224`	`216`	`{`
`225`	`217`	`status.status = TransactionStatus::Unconfirmed;`