Remove unused MaybeSetAddrName

Summary: Backport of [[bitcoin/bitcoin#22782 | core#22782]].

Test Plan:
Enable tracing, then:
  ninja all check-all

Reviewers: #bitcoin_abc, PiRK

Reviewed By: #bitcoin_abc, PiRK

Differential Revision: https://reviews.bitcoinabc.org/D12637

Author: MarcoFalke

doc/tracing.md

@@ -147,7 +147,7 @@ For example:
 ```C++
 TRACE6(net, inbound_message,
     pnode->GetId(),
-    pnode->GetAddrName().c_str(),
+    pnode->m_addr_name.c_str(),
     pnode->ConnectionTypeAsString().c_str(),
     sanitizedType.c_str(),
     msg.data.size(),

src/net.cpp

@@ -332,7 +332,7 @@ CNode *CConnman::FindNode(const CSubNet &subNet) {
 CNode *CConnman::FindNode(const std::string &addrName) {
     LOCK(cs_vNodes);
     for (CNode *pnode : vNodes) {
-        if (pnode->GetAddrName() == addrName) {
+        if (pnode->m_addr_name == addrName) {
             return pnode;
         }
     }
@@ -424,13 +424,10 @@ CNode *CConnman::ConnectNode(CAddress addrConnect, const char *pszDest,
             }
             // It is possible that we already have a connection to the IP/port
             // pszDest resolved to. In that case, drop the connection that was
-            // just created, and return the existing CNode instead. Also store
-            // the name we used to connect in that CNode, so that future
-            // FindNode() calls to that name catch this early.
+            // just created.
             LOCK(cs_vNodes);
             CNode *pnode = FindNode(static_cast<CService>(addrConnect));
             if (pnode) {
-                pnode->MaybeSetAddrName(std::string(pszDest));
                 LogPrintf("Failed to open new connection, already connected\n");
                 return nullptr;
             }
@@ -561,18 +558,6 @@ std::string CNode::ConnectionTypeAsString() const {
     assert(false);
 }
 
-std::string CNode::GetAddrName() const {
-    LOCK(cs_addrName);
-    return addrName;
-}
-
-void CNode::MaybeSetAddrName(const std::string &addrNameIn) {
-    LOCK(cs_addrName);
-    if (addrName.empty()) {
-        addrName = addrNameIn;
-    }
-}
-
 CService CNode::GetAddrLocal() const {
     LOCK(cs_addrLocal);
     return addrLocal;
@@ -612,7 +597,7 @@ void CNode::copyStats(CNodeStats &stats) {
     stats.m_last_block_time = m_last_block_time;
     stats.m_connected = m_connected;
     stats.nTimeOffset = nTimeOffset;
-    stats.addrName = GetAddrName();
+    stats.m_addr_name = m_addr_name;
     stats.nVersion = nVersion;
     {
         LOCK(cs_SubVer);
@@ -2506,7 +2491,7 @@ std::vector<AddedNodeInfo> CConnman::GetAddedNodeInfo() {
             if (pnode->addr.IsValid()) {
                 mapConnected[pnode->addr] = pnode->IsInboundConn();
             }
-            std::string addrName = pnode->GetAddrName();
+            std::string addrName{pnode->m_addr_name};
             if (!addrName.empty()) {
                 mapConnectedByName[std::move(addrName)] =
                     std::make_pair(pnode->IsInboundConn(),
@@ -3456,8 +3441,10 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, SOCKET hSocketIn,
              const CAddress &addrBindIn, const std::string &addrNameIn,
              ConnectionType conn_type_in, bool inbound_onion)
     : m_connected(GetTime<std::chrono::seconds>()), addr(addrIn),
-      addrBind(addrBindIn), m_inbound_onion(inbound_onion),
-      nKeyedNetGroup(nKeyedNetGroupIn),
+      addrBind(addrBindIn), m_addr_name{addrNameIn.empty()
+                                            ? addr.ToStringIPPort()
+                                            : addrNameIn},
+      m_inbound_onion(inbound_onion), nKeyedNetGroup(nKeyedNetGroupIn),
       m_tx_relay(conn_type_in != ConnectionType::BLOCK_RELAY
                      ? std::make_unique<TxRelay>()
                      : nullptr),
@@ -3473,15 +3460,15 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, SOCKET hSocketIn,
         assert(conn_type_in == ConnectionType::INBOUND);
     }
     hSocket = hSocketIn;
-    addrName = addrNameIn == "" ? addr.ToStringIPPort() : addrNameIn;
 
     for (const std::string &msg : getAllNetMessageTypes()) {
         mapRecvBytesPerMsgCmd[msg] = 0;
     }
     mapRecvBytesPerMsgCmd[NET_MESSAGE_COMMAND_OTHER] = 0;
 
     if (fLogIPs) {
-        LogPrint(BCLog::NET, "Added connection to %s peer=%d\n", addrName, id);
+        LogPrint(BCLog::NET, "Added connection to %s peer=%d\n", m_addr_name,
+                 id);
     } else {
         LogPrint(BCLog::NET, "Added connection peer=%d\n", id);
     }
@@ -3509,7 +3496,7 @@ void CConnman::PushMessage(CNode *pnode, CSerializedNetMsg &&msg) {
         CaptureMessage(pnode->addr, msg.m_type, msg.data, /*incoming=*/false);
     }
 
-    TRACE6(net, outbound_message, pnode->GetId(), pnode->GetAddrName().c_str(),
+    TRACE6(net, outbound_message, pnode->GetId(), pnode->m_addr_name.c_str(),
            pnode->ConnectionTypeAsString().c_str(), msg.m_type.c_str(),
            msg.data.size(), msg.data.data());
 

src/net.h

@@ -292,7 +292,7 @@ struct CNodeStats {
     std::chrono::seconds m_last_block_time;
     std::chrono::seconds m_connected;
     int64_t nTimeOffset;
-    std::string addrName;
+    std::string m_addr_name;
     int nVersion;
     std::string cleanSubVer;
     bool fInbound;
@@ -490,6 +490,7 @@ class CNode {
     const CAddress addr;
     // Bind address of our side of the connection
     const CAddress addrBind;
+    const std::string m_addr_name;
     //! Whether this peer is an inbound onion, i.e. connected via our Tor onion
     //! service.
     const bool m_inbound_onion;
@@ -774,9 +775,6 @@ class CNode {
     // Used only by SocketHandler thread
     std::list<CNetMessage> vRecvMsg;
 
-    mutable RecursiveMutex cs_addrName;
-    std::string addrName GUARDED_BY(cs_addrName);
-
     // Our address, as reported by the peer
     CService addrLocal GUARDED_BY(cs_addrLocal);
     mutable RecursiveMutex cs_addrLocal;
@@ -872,10 +870,6 @@ class CNode {
 
     ServiceFlags GetLocalServices() const { return nLocalServices; }
 
-    std::string GetAddrName() const;
-    //! Sets the addrName only if it was not previously set
-    void MaybeSetAddrName(const std::string &addrNameIn);
-
     std::string ConnectionTypeAsString() const;
 };
 

src/net_processing.cpp

@@ -1612,7 +1612,6 @@ void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) {
 
 void PeerManagerImpl::InitializeNode(const Config &config, CNode *pnode) {
     CAddress addr = pnode->addr;
-    std::string addrName = pnode->GetAddrName();
     NodeId nodeid = pnode->GetId();
     {
         LOCK(cs_main);
@@ -5867,7 +5866,7 @@ bool PeerManagerImpl::ProcessMessages(const Config &config, CNode *pfrom,
     }
     CNetMessage &msg(msgs.front());
 
-    TRACE6(net, inbound_message, pfrom->GetId(), pfrom->GetAddrName().c_str(),
+    TRACE6(net, inbound_message, pfrom->GetId(), pfrom->m_addr_name.c_str(),
            pfrom->ConnectionTypeAsString().c_str(), msg.m_command.c_str(),
            msg.m_recv.size(), msg.m_recv.data());
 

src/qt/peertablemodel.cpp

@@ -28,7 +28,7 @@ bool NodeLessThan::operator()(const CNodeCombinedStats &left,
         case PeerTableModel::NetNodeId:
             return pLeft->nodeid < pRight->nodeid;
         case PeerTableModel::Address:
-            return pLeft->addrName.compare(pRight->addrName) < 0;
+            return pLeft->m_addr_name.compare(pRight->m_addr_name) < 0;
         case PeerTableModel::Network:
             return pLeft->m_network < pRight->m_network;
         case PeerTableModel::Ping:
@@ -151,7 +151,7 @@ QVariant PeerTableModel::data(const QModelIndex &index, int role) const {
                 // prepend to peer address down-arrow symbol for inbound
                 // connection and up-arrow for outbound connection
                 return QString(rec->nodeStats.fInbound ? "↓ " : "↑ ") +
-                       QString::fromStdString(rec->nodeStats.addrName);
+                       QString::fromStdString(rec->nodeStats.m_addr_name);
             case Network:
                 return GUIUtil::NetworkToQString(rec->nodeStats.m_network);
             case Ping:

src/qt/rpcconsole.cpp

@@ -1260,8 +1260,8 @@ void RPCConsole::updateDetailWidget() {
         clientModel->getPeerTableModel()->getNodeStats(
             selected_rows.first().row());
     // update the detail ui with latest node information
-    QString peerAddrDetails(QString::fromStdString(stats->nodeStats.addrName) +
-                            " ");
+    QString peerAddrDetails(
+        QString::fromStdString(stats->nodeStats.m_addr_name) + " ");
     peerAddrDetails +=
         tr("(peer id: %1)").arg(QString::number(stats->nodeStats.nodeid));
     if (!stats->nodeStats.addrLocal.empty()) {

src/rpc/net.cpp

@@ -235,7 +235,7 @@ static RPCHelpMan getpeerinfo() {
                 bool fStateStats =
                     node.peerman->GetNodeStateStats(stats.nodeid, statestats);
                 obj.pushKV("id", stats.nodeid);
-                obj.pushKV("addr", stats.addrName);
+                obj.pushKV("addr", stats.m_addr_name);
                 if (stats.addrBind.IsValid()) {
                     obj.pushKV("addrbind", stats.addrBind.ToString());
                 }

src/test/fuzz/net.cpp

@@ -66,27 +66,22 @@ void test_one_input(const std::vector<uint8_t> &buffer) {
                 break;
             }
             case 1: {
-                node.MaybeSetAddrName(
-                    fuzzed_data_provider.ConsumeRandomLengthString(32));
-                break;
-            }
-            case 2: {
                 CNodeStats stats;
                 node.copyStats(stats);
                 break;
             }
-            case 3: {
+            case 2: {
                 const CNode *add_ref_node = node.AddRef();
                 assert(add_ref_node == &node);
                 break;
             }
-            case 4: {
+            case 3: {
                 if (node.GetRefCount() > 0) {
                     node.Release();
                 }
                 break;
             }
-            case 5: {
+            case 4: {
                 const std::optional<CInv> inv_opt =
                     ConsumeDeserializable<CInv>(fuzzed_data_provider);
                 if (!inv_opt) {
@@ -96,12 +91,12 @@ void test_one_input(const std::vector<uint8_t> &buffer) {
                 node.AddKnownTx(txid);
                 break;
             }
-            case 6: {
+            case 5: {
                 const TxId &txid = TxId(ConsumeUInt256(fuzzed_data_provider));
                 node.PushTxInventory(txid);
                 break;
             }
-            case 7: {
+            case 6: {
                 const std::optional<CService> service_opt =
                     ConsumeDeserializable<CService>(fuzzed_data_provider);
                 if (!service_opt) {
@@ -110,7 +105,7 @@ void test_one_input(const std::vector<uint8_t> &buffer) {
                 node.SetAddrLocal(*service_opt);
                 break;
             }
-            case 8: {
+            case 7: {
                 const std::vector<uint8_t> b =
                     ConsumeRandomLengthByteVector(fuzzed_data_provider);
                 bool complete;
@@ -121,7 +116,6 @@ void test_one_input(const std::vector<uint8_t> &buffer) {
     }
 
     (void)node.GetAddrLocal();
-    (void)node.GetAddrName();
     (void)node.GetId();
     (void)node.GetLocalNonce();
     (void)node.GetLocalServices();