Merge pull request #2561 from Pythonix/split-dns-resolve

net: Turn net structures into dumb storage classes (backport)

src/Makefile.am

@@ -153,6 +153,7 @@ GRIDCOIN_CORE_H = \
     miner.h \
     mruset.h \
     netbase.h \
+    netaddress.h \
     net.h \
     node/blockstorage.h \
     pbkdf2.h \
@@ -265,6 +266,7 @@ GRIDCOIN_CORE_CPP = addrdb.cpp \
     main.cpp \
     miner.cpp \
     netbase.cpp \
+    netaddress.cpp \
     net.cpp \
     node/blockstorage.cpp \
     node/ui_interface.cpp \

src/Makefile.test.include

@@ -32,6 +32,7 @@ GRIDCOIN_TESTS =\
 	test/checkpoints_tests.cpp \
 	test/dos_tests.cpp \
 	test/accounting_tests.cpp \
+	test/addrman_tests.cpp \
 	test/allocator_tests.cpp \
 	test/base32_tests.cpp \
 	test/base58_tests.cpp \
@@ -59,6 +60,7 @@ GRIDCOIN_TESTS =\
 	test/mruset_tests.cpp \
 	test/multisig_tests.cpp \
 	test/netbase_tests.cpp \
+	test/net_tests.cpp \
 	test/random_tests.cpp \
 	test/rpc_tests.cpp \
 	test/sanity_tests.cpp \

src/addrman.cpp

@@ -330,39 +330,47 @@ void CAddrMan::Attempt_(const CService &addr, int64_t nTime)
     info.nAttempts++;
 }
 
-CAddress CAddrMan::Select_()
+CAddrInfo CAddrMan::Select_(bool newOnly)
 {
     if (size() == 0)
-        return CAddress();
+        return CAddrInfo();
+
+    if (newOnly && nNew == 0)
+        return CAddrInfo();
 
     // Use a 50% chance for choosing between tried and new table entries.
-    if (nTried > 0 && (nNew == 0 || GetRandInt(2) == 0)) {
+    if (!newOnly &&
+       (nTried > 0 && (nNew == 0 || RandomInt(2) == 0))) {
         // use a tried node
         double fChanceFactor = 1.0;
         while (1) {
-            int nKBucket = GetRandInt(ADDRMAN_TRIED_BUCKET_COUNT);
-            int nKBucketPos = GetRandInt(ADDRMAN_BUCKET_SIZE);
-            if (vvTried[nKBucket][nKBucketPos] == -1)
-                continue;
+            int nKBucket = RandomInt(ADDRMAN_TRIED_BUCKET_COUNT);
+            int nKBucketPos = RandomInt(ADDRMAN_BUCKET_SIZE);
+            while (vvTried[nKBucket][nKBucketPos] == -1) {
+                nKBucket = (nKBucket + insecure_rand()) % ADDRMAN_TRIED_BUCKET_COUNT;
+                nKBucketPos = (nKBucketPos + insecure_rand()) % ADDRMAN_BUCKET_SIZE;
+            }
             int nId = vvTried[nKBucket][nKBucketPos];
             assert(mapInfo.count(nId) == 1);
             CAddrInfo& info = mapInfo[nId];
-            if (GetRandInt(1 << 30) < fChanceFactor * info.GetChance() * (1 << 30))
+            if (RandomInt(1 << 30) < fChanceFactor * info.GetChance() * (1 << 30))
                 return info;
             fChanceFactor *= 1.2;
         }
     } else {
         // use a new node
         double fChanceFactor = 1.0;
         while (1) {
-            int nUBucket = GetRandInt(ADDRMAN_NEW_BUCKET_COUNT);
-            int nUBucketPos = GetRandInt(ADDRMAN_BUCKET_SIZE);
-            if (vvNew[nUBucket][nUBucketPos] == -1)
-                continue;
+            int nUBucket = RandomInt(ADDRMAN_NEW_BUCKET_COUNT);
+            int nUBucketPos = RandomInt(ADDRMAN_BUCKET_SIZE);
+            while (vvNew[nUBucket][nUBucketPos] == -1) {
+                nUBucket = (nUBucket + insecure_rand()) % ADDRMAN_NEW_BUCKET_COUNT;
+                nUBucketPos = (nUBucketPos + insecure_rand()) % ADDRMAN_BUCKET_SIZE;
+            }
             int nId = vvNew[nUBucket][nUBucketPos];
             assert(mapInfo.count(nId) == 1);
             CAddrInfo& info = mapInfo[nId];
-            if (GetRandInt(1 << 30) < fChanceFactor * info.GetChance() * (1 << 30))
+            if (RandomInt(1 << 30) < fChanceFactor * info.GetChance() * (1 << 30))
                 return info;
             fChanceFactor *= 1.2;
         }
@@ -488,3 +496,25 @@ void CAddrMan::Connected_(const CService &addr, int64_t nTime)
     if (nTime - info.nTime > nUpdateInterval)
         info.nTime = nTime;
 }
+
+void CAddrMan::SetServices_(const CService& addr, ServiceFlags nServices)
+{
+    CAddrInfo* pinfo = Find(addr);
+
+    // if not found, bail out
+    if (!pinfo)
+        return;
+
+    CAddrInfo& info = *pinfo;
+
+    // check whether we are talking about the exact same CService (including same port)
+    if (info != addr)
+        return;
+
+    // update info
+    info.nServices = nServices;
+}
+
+int CAddrMan::RandomInt(int nMax){
+    return GetRandInt(nMax);
+}

src/addrman.h

@@ -171,9 +171,6 @@ class CAddrMan
     // critical section to protect the inner data structures
     mutable CCriticalSection cs;
 
-    //! secret key to randomize bucket select with
-    uint256 nKey;
-
     //! last used nId
     int nIdCount;
 
@@ -200,6 +197,12 @@ class CAddrMan
 
 protected:
 
+    //! secret key to randomize bucket select with
+    uint256 nKey;
+
+    //! Source of random numbers for randomization in inner loops
+    FastRandomContext insecure_rand;
+
     // Find an entry.
     CAddrInfo* Find(const CNetAddr& addr, int* pnId = nullptr);
 
@@ -228,8 +231,11 @@ class CAddrMan
     // Mark an entry as attempted to connect.
     void Attempt_(const CService &addr, int64_t nTime);
 
-    //! Select an address to connect to.
-    CAddress Select_();
+    //! Select an address to connect to, if newOnly is set to true, only the new table is selected from.
+    CAddrInfo Select_(bool newOnly);
+
+    //! Wraps GetRandInt to allow tests to override RandomInt and make it determinismistic.
+    virtual int RandomInt(int nMax);
 
 #ifdef DEBUG_ADDRMAN
     // Perform consistency check. Returns an error code or zero.
@@ -242,6 +248,9 @@ class CAddrMan
     // Mark an entry as currently-connected-to.
     void Connected_(const CService &addr, int64_t nTime);
 
+    //! Update an entry's service bits.
+    void SetServices_(const CService &addr, ServiceFlags nServices);
+
 public:
     /**
      * serialized format:
@@ -458,7 +467,7 @@ class CAddrMan
     }
 
     //! Return the number of (unique) addresses in all tables.
-    int size()
+    size_t size()
     {
         LOCK(cs); // TODO: Cache this in an atomic to avoid this overhead
         return vRandom.size();
@@ -522,14 +531,16 @@ class CAddrMan
         Check();
     }
 
-    //! Choose an address to connect to.
-    CAddress Select()
+    /**
+     * Choose an address to connect to.
+     */
+    CAddrInfo Select(bool newOnly = false)
     {
-        CAddress addrRet;
+        CAddrInfo addrRet;
         {
             LOCK(cs);
             Check();
-            addrRet = Select_();
+            addrRet = Select_(newOnly);
             Check();
         }
         return addrRet;
@@ -557,6 +568,14 @@ class CAddrMan
         Check();
     }
 
+    void SetServices(const CService &addr, ServiceFlags nServices)
+    {
+        LOCK(cs);
+        Check();
+        SetServices_(addr, nServices);
+        Check();
+    }
+
 };
 
 #endif // BITCOIN_ADDRMAN_H

src/init.cpp

@@ -1082,7 +1082,7 @@ bool AppInit2(ThreadHandlerPtr threads)
     CService addrProxy;
     bool fProxy = false;
     if (gArgs.IsArgSet("-proxy")) {
-        addrProxy = CService(gArgs.GetArg("-proxy", ""), 9050);
+        CService addrProxy(LookupNumeric(gArgs.GetArg("-proxy", "").c_str(), 9050));
         if (!addrProxy.IsValid())
             return InitError(strprintf(_("Invalid -proxy address: '%s'"), gArgs.GetArg("-proxy", "")));
 
@@ -1091,17 +1091,17 @@ bool AppInit2(ThreadHandlerPtr threads)
         if (!IsLimited(NET_IPV6))
             SetProxy(NET_IPV6, addrProxy);
         SetNameProxy(addrProxy);
-
         fProxy = true;
     }
 
     // -tor can override normal proxy, -notor disables Tor entirely
     if (gArgs.IsArgSet("-tor") && (fProxy || gArgs.IsArgSet("-tor"))) {
-        CService addrOnion;
-        if (!gArgs.IsArgSet("-tor"))
+        proxyType addrOnion;
+        if (!gArgs.IsArgSet("-tor")) {
             addrOnion = addrProxy;
-        else
-            addrOnion = CService(gArgs.GetArg("-tor", ""), 9050);
+        } else {
+            CService addrProxy(LookupNumeric(gArgs.GetArg("-tor", "").c_str(), 9050));
+        }
         if (!addrOnion.IsValid())
             return InitError(strprintf(_("Invalid -tor address: '%s'"), gArgs.GetArg("-tor", "")));
         SetProxy(NET_TOR, addrOnion);
@@ -1144,10 +1144,11 @@ bool AppInit2(ThreadHandlerPtr threads)
     {
         for (auto const& strAddr : gArgs.GetArgs("-externalip"))
         {
-            CService addrLocal(strAddr, GetListenPort(), fNameLookup);
-            if (!addrLocal.IsValid())
+            CService addrLocal;
+            if (Lookup(strAddr.c_str(), addrLocal, GetListenPort(), fNameLookup) && addrLocal.IsValid())
+                AddLocal(addrLocal, LOCAL_MANUAL);
+            else
                 return InitError(strprintf(_("Cannot resolve -externalip address: '%s'"), strAddr));
-            AddLocal(CService(strAddr, GetListenPort(), fNameLookup), LOCAL_MANUAL);
         }
     }
 

src/net.cpp

@@ -64,7 +64,7 @@ CCriticalSection cs_mapLocalHost;
 std::map<CNetAddr, LocalServiceInfo> mapLocalHost;
 static bool vfLimited[NET_MAX] GUARDED_BY(cs_mapLocalHost) = {};
 static CNode* pnodeLocalHost = nullptr;
-CAddress addrSeenByPeer(CService("0.0.0.0", 0), nLocalServices);
+CAddress addrSeenByPeer(LookupNumeric("0.0.0.0", 0), nLocalServices);
 uint64_t nLocalHostNonce = 0;
 
 
@@ -484,7 +484,7 @@ bool CNode::DisconnectNode(NodeId id)
 void CNode::PushVersion()
 {
     int64_t nTime = GetAdjustedTime();
-    CAddress addrYou = (addr.IsRoutable() && !IsProxy(addr) ? addr : CAddress(CService("0.0.0.0",0)));
+    CAddress addrYou = (addr.IsRoutable() && !IsProxy(addr) ? addr : CAddress(LookupNumeric("0.0.0.0", 0)));
     CAddress addrMe = CAddress(CService(), nLocalServices);
     GetRandBytes((unsigned char*)&nLocalHostNonce, sizeof(nLocalHostNonce));
     LogPrint(BCLog::LogFlags::NET, "send version message: version %d, blocks=%d, us=%s, them=%s, peer=%s",
@@ -1179,8 +1179,11 @@ void ThreadMapPort2(void* parg)
             {
                 if(externalIPAddress[0])
                 {
-                    LogPrintf("UPnP: ExternalIPAddress = %s", externalIPAddress);
-                    AddLocal(CNetAddr(externalIPAddress), LOCAL_UPNP);
+                    CNetAddr resolved;
+                    if(LookupHost(externalIPAddress, resolved, false)) {
+                        LogPrintf("UPnP: ExternalIPAddress = %s\n", resolved.ToString().c_str());
+                        AddLocal(resolved, LOCAL_UPNP);
+                    }
                 }
                 else
                     LogPrintf("UPnP: GetExternalIPAddress not successful.");
@@ -1311,11 +1314,11 @@ void ThreadDNSAddressSeed2(void* parg)
             if (HaveNameProxy()) {
                 AddOneShot(seed[1]);
             } else {
-                vector<CNetAddr> vaddr;
+                vector<CNetAddr> vIPs;
                 vector<CAddress> vAdd;
-                if (LookupHost(seed[1], vaddr))
+                if (LookupHost(seed[1], vIPs, 0, true))
                 {
-                    for (auto const& ip : vaddr)
+                    for (auto const& ip : vIPs)
                     {
                         int nOneDay = 24*3600;
                         CAddress addr = CAddress(CService(ip, GetDefaultPort()));
@@ -1324,20 +1327,22 @@ void ThreadDNSAddressSeed2(void* parg)
                         found++;
                     }
                 }
-                addrman.Add(vAdd, CNetAddr(seed[0], true));
+                // TODO: The seed name resolve may fail, yielding an IP of [::], which results in
+                // addrman assigning the same source to results from different seeds.
+                // This should switch to a hard-coded stable dummy IP for each seed name, so that the
+                // resolve is not required at all.
+                if (!vIPs.empty()) {
+                    CService seedSource;
+                    Lookup(seed[0], seedSource, 0, true);
+                    addrman.Add(vAdd, seedSource);
+                }
             }
         }
     }
 
     LogPrint(BCLog::LogFlags::NET, "%d addresses found from DNS seeds", found);
 }
 
-
-
-
-
-
-
 unsigned int pnSeed[] =
 {
     0xdf4bd379, 0x7934d29b, 0x26bc02ad, 0x7ab743ad, 0x0ab3a7bc,
@@ -1527,7 +1532,7 @@ void ThreadOpenConnections2(void* parg)
             return;
 
         // Add seed nodes
-        if (addrman.size()==0 && (GetAdjustedTime() - nStart > 60) && !fTestNet)
+        if (addrman.size() == 0 && (GetAdjustedTime() - nStart > 60) && !fTestNet)
         {
             std::vector<CAddress> vAdd;
             for (const auto& seed : pnSeed)
@@ -1543,7 +1548,9 @@ void ThreadOpenConnections2(void* parg)
                 addr.nTime = GetAdjustedTime() - GetRand(nOneWeek) - nOneWeek;
                 vAdd.push_back(addr);
             }
-            addrman.Add(vAdd, CNetAddr("127.0.0.1"));
+            CNetAddr local;
+            LookupHost("127.0.0.1", local, false);
+            addrman.Add(vAdd, local);
         }
 
         //
@@ -1952,7 +1959,7 @@ void static Discover()
     if (gethostname(pszHostName, sizeof(pszHostName)) != SOCKET_ERROR)
     {
         vector<CNetAddr> vaddr;
-        if (LookupHost(pszHostName, vaddr))
+        if (LookupHost(pszHostName, vaddr, 0, true))
         {
             for (auto const& addr : vaddr)
             {
@@ -2010,7 +2017,7 @@ void StartNode(void* parg)
               nMaxOutbound, max_connections);
 
     if (pnodeLocalHost == nullptr)
-        pnodeLocalHost = new CNode(INVALID_SOCKET, CAddress(CService("127.0.0.1", 0), nLocalServices));
+        pnodeLocalHost = new CNode(INVALID_SOCKET, CAddress(LookupNumeric("127.0.0.1", 0), nLocalServices));
 
     Discover();