From 3a75bef4af868f321b9b4f063404a7e2ac2318d9 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Thu, 4 Apr 2019 16:45:12 -0400
Subject: [PATCH] Merge #15654: net: Remove unused unsanitized user agent
 string CNode::strSubVer

fa8548c5d1 net: Remove unused unsanitized user agent string CNode::strSubVer (MarcoFalke)

Pull request description:

  I fail to see a use case for this unsanitized byte array. In fact this can easily be confused with `cleanSubVer` and be displayed to the user (or logged) by a simple typo that is hard to find in review.

  Further reading: https://btcinformation.org/en/developer-reference#version

ACKs for commit fa8548:
  promag:
    utACK fa8548c, good catch.
  practicalswift:
    utACK fa8548c5d13957f57f9b1e20e03002600962f7f0
  sipa:
    utACK fa8548c5d13957f57f9b1e20e03002600962f7f0

Tree-SHA512: 3c3ff1504d1583ad099df9a6aa761458a82ec48a58ef7aaa9b5679a5281dd1b59036ba2932ed708488951a565b669a3083ef70be5a58472ff8677b971162ae2f
---
 src/net.cpp            |  1 -
 src/net.h              | 12 ++++++------
 src/net_processing.cpp |  7 +++----
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/src/net.cpp b/src/net.cpp
index 4e156bf1af..7dfdcdf3d4 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -3899,7 +3899,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
     nVersion = 0;
     nNumWarningsSkipped = 0;
     nLastWarningTime = 0;
-    strSubVer = "";
     fWhitelisted = false;
     fOneShot = false;
     m_manual_connection = false;
diff --git a/src/net.h b/src/net.h
index 87944ad83c..b60692615f 100644
--- a/src/net.h
+++ b/src/net.h
@@ -62,7 +62,7 @@ static const unsigned int MAX_LOCATOR_SZ = 101;
 static const unsigned int MAX_ADDR_TO_SEND = 1000;
 /** Maximum length of incoming protocol messages (no message over 3 MiB is currently acceptable). */
 static const unsigned int MAX_PROTOCOL_MESSAGE_LENGTH = 3 * 1024 * 1024;
-/** Maximum length of strSubVer in `version` message */
+/** Maximum length of the user agent string in `version` message */
 static const unsigned int MAX_SUBVERSION_LENGTH = 256;
 /** Maximum number of automatic outgoing nodes */
 static const int MAX_OUTBOUND_CONNECTIONS = 8;
@@ -872,11 +872,11 @@ public:
     const CAddress addrBind;
     std::atomic<int> nNumWarningsSkipped;
     std::atomic<int> nVersion;
-    // strSubVer is whatever byte array we read from the wire. However, this field is intended
-    // to be printed out, displayed to humans in various forms and so on. So we sanitize it and
-    // store the sanitized version in cleanSubVer. The original should be used when dealing with
-    // the network or wire types and the cleaned string used when displayed or logged.
-    std::string strSubVer GUARDED_BY(cs_SubVer), cleanSubVer GUARDED_BY(cs_SubVer);
+    /**
+     * cleanSubVer is a sanitized string of the user agent byte array we read
+     * from the wire. This cleaned string can safely be logged or displayed.
+     */
+    std::string cleanSubVer GUARDED_BY(cs_SubVer){};
     CCriticalSection cs_SubVer; // used for both cleanSubVer and strSubVer
     bool fWhitelisted; // This peer can bypass DoS banning.
     bool fFeeler; // If true this node is being used as a short lived feeler.
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 98ab2a28a6..bd69e544ea 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2156,7 +2156,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
         ServiceFlags nServices;
         int nVersion;
         int nSendVersion;
-        std::string strSubVer;
         std::string cleanSubVer;
         int nStartingHeight = -1;
         bool fRelay = true;
@@ -2193,6 +2192,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
         if (!vRecv.empty())
             vRecv >> addrFrom >> nNonce;
         if (!vRecv.empty()) {
+            std::string strSubVer;
             vRecv >> LIMITED_STRING(strSubVer, MAX_SUBVERSION_LENGTH);
             cleanSubVer = SanitizeString(strSubVer);
         }
@@ -2238,9 +2238,9 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
             PushNodeVersion(pfrom, connman, GetAdjustedTime());
 
         if (Params().NetworkIDString() == CBaseChainParams::DEVNET) {
-            if (strSubVer.find(strprintf("devnet=%s", gArgs.GetDevNetName())) == std::string::npos) {
+            if (cleanSubVer.find(strprintf("devnet=%s", gArgs.GetDevNetName())) == std::string::npos) {
                 LOCK(cs_main);
-                LogPrintf("connected to wrong devnet. Reported version is %s, expected devnet name is %s\n", strSubVer, gArgs.GetDevNetName());
+                LogPrintf("connected to wrong devnet. Reported version is %s, expected devnet name is %s\n", cleanSubVer, gArgs.GetDevNetName());
                 if (!pfrom->fInbound)
                     Misbehaving(pfrom->GetId(), 100); // don't try to connect again
                 else
@@ -2257,7 +2257,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
         pfrom->SetAddrLocal(addrMe);
         {
             LOCK(pfrom->cs_SubVer);
-            pfrom->strSubVer = strSubVer;
             pfrom->cleanSubVer = cleanSubVer;
         }
         pfrom->nStartingHeight = nStartingHeight;