From 9ce2b966cdea73feada8e4cbc2cfcdcbed347414 Mon Sep 17 00:00:00 2001 From: Oleg Girko Date: Fri, 14 Jul 2017 17:58:57 +0100 Subject: [PATCH] Backport Bitcoin PR#8113: Rework addnode behaviour (#1525) * Rework addnode behaviour * Use CNode::addeName to track whether a connection to a name is already open * A new connection to a previously-connected by-name addednode is only opened when the previous one closes (even if the name starts resolving to something else) * At most one connection is opened per addednode (even if the name resolves to multiple) * Unify the code between ThreadOpenAddedNodeConnections and getaddednodeinfo * Information about open connections is always returned, and the dns argument becomes a dummy * An IP address and inbound/outbound is only reported for the (at most 1) open connection * Prevent duplicate connections where one is by name and another by ip * Randomize name lookup result in ConnectSocketByName --- src/net.cpp | 138 ++++++++++++++++++++++++++++++------------------ src/net.h | 10 ++++ src/netbase.cpp | 10 ++-- src/rpc/net.cpp | 94 ++++++++------------------------- 4 files changed, 125 insertions(+), 127 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 9648d5d08..9991d9f18 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -420,6 +420,31 @@ CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fConnectToMas return NULL; } + if (pszDest && addrConnect.IsValid()) { + // 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. + CNode* pnode = FindNode((CService)addrConnect); + if (pnode) + { + // we have existing connection to this node but it was not a connection to masternode, + // change flag and add reference so that we can correctly clear it later + if(fConnectToMasternode && !pnode->fMasternode) { + pnode->AddRef(); + pnode->fMasternode = true; + } + { + LOCK(cs_vNodes); + if (pnode->addrName.empty()) { + pnode->addrName = std::string(pszDest); + } + } + CloseSocket(hSocket); + return pnode; + } + } + addrman.Attempt(addrConnect); // Add node @@ -1682,6 +1707,58 @@ void ThreadOpenConnections() } } +std::vector GetAddedNodeInfo() +{ + std::vector ret; + + std::list lAddresses(0); + { + LOCK(cs_vAddedNodes); + ret.reserve(vAddedNodes.size()); + BOOST_FOREACH(const std::string& strAddNode, vAddedNodes) + lAddresses.push_back(strAddNode); + } + + + // Build a map of all already connected addresses (by IP:port and by name) to inbound/outbound and resolved CService + std::map mapConnected; + std::map> mapConnectedByName; + { + LOCK(cs_vNodes); + for (const CNode* pnode : vNodes) { + if (pnode->addr.IsValid()) { + mapConnected[pnode->addr] = pnode->fInbound; + } + if (!pnode->addrName.empty()) { + mapConnectedByName[pnode->addrName] = std::make_pair(pnode->fInbound, static_cast(pnode->addr)); + } + } + } + + BOOST_FOREACH(const std::string& strAddNode, lAddresses) { + CService service(strAddNode, Params().GetDefaultPort()); + if (service.IsValid()) { + // strAddNode is an IP:port + auto it = mapConnected.find(service); + if (it != mapConnected.end()) { + ret.push_back(AddedNodeInfo{strAddNode, service, true, it->second}); + } else { + ret.push_back(AddedNodeInfo{strAddNode, CService(), false, false}); + } + } else { + // strAddNode is a name + auto it = mapConnectedByName.find(strAddNode); + if (it != mapConnectedByName.end()) { + ret.push_back(AddedNodeInfo{strAddNode, it->second.second, true, it->second.first}); + } else { + ret.push_back(AddedNodeInfo{strAddNode, CService(), false, false}); + } + } + } + + return ret; +} + void ThreadOpenAddedConnections() { { @@ -1689,61 +1766,20 @@ void ThreadOpenAddedConnections() vAddedNodes = mapMultiArgs["-addnode"]; } - if (HaveNameProxy()) { - while(true) { - std::list lAddresses(0); - { - LOCK(cs_vAddedNodes); - BOOST_FOREACH(const std::string& strAddNode, vAddedNodes) - lAddresses.push_back(strAddNode); - } - BOOST_FOREACH(const std::string& strAddNode, lAddresses) { - CAddress addr; - CSemaphoreGrant grant(*semOutbound); - OpenNetworkConnection(addr, &grant, strAddNode.c_str()); - MilliSleep(500); - } - MilliSleep(120000); // Retry every 2 minutes - } - } - for (unsigned int i = 0; true; i++) { - std::list lAddresses(0); - { - LOCK(cs_vAddedNodes); - BOOST_FOREACH(const std::string& strAddNode, vAddedNodes) - lAddresses.push_back(strAddNode); + std::vector vInfo = GetAddedNodeInfo(); + for (const AddedNodeInfo& info : vInfo) { + if (!info.fConnected) { + CSemaphoreGrant grant(*semOutbound); + // If strAddedNode is an IP/port, decode it immediately, so + // OpenNetworkConnection can detect existing connections to that IP/port. + CService service(info.strAddedNode, Params().GetDefaultPort()); + OpenNetworkConnection(CAddress(service, NODE_NONE), &grant, info.strAddedNode.c_str(), false); + MilliSleep(500); + } } - std::list > lservAddressesToAdd(0); - BOOST_FOREACH(const std::string& strAddNode, lAddresses) { - std::vector vservNode(0); - if(Lookup(strAddNode.c_str(), vservNode, Params().GetDefaultPort(), fNameLookup, 0)) - lservAddressesToAdd.push_back(vservNode); - } - // Attempt to connect to each IP for each addnode entry until at least one is successful per addnode entry - // (keeping in mind that addnode entries can have many IPs if fNameLookup) - { - LOCK(cs_vNodes); - BOOST_FOREACH(CNode* pnode, vNodes) - for (std::list >::iterator it = lservAddressesToAdd.begin(); it != lservAddressesToAdd.end(); it++) - BOOST_FOREACH(const CService& addrNode, *(it)) - if (pnode->addr == addrNode) - { - it = lservAddressesToAdd.erase(it); - it--; - break; - } - } - BOOST_FOREACH(std::vector& vserv, lservAddressesToAdd) - { - CSemaphoreGrant grant(*semOutbound); - /* We want -addnode to work even for nodes that don't provide all - * wanted services, so pass in nServices=NODE_NONE to CAddress. */ - OpenNetworkConnection(CAddress(vserv[i % vserv.size()], NODE_NONE), &grant); - MilliSleep(500); - } MilliSleep(120000); // Retry every 2 minutes } } diff --git a/src/net.h b/src/net.h index 6dd2ac6b3..b02c61085 100644 --- a/src/net.h +++ b/src/net.h @@ -886,4 +886,14 @@ std::vector CopyNodeVector(); void ReleaseNodeVector(const std::vector& vecNodes); +struct AddedNodeInfo +{ + std::string strAddedNode; + CService resolvedAddress; + bool fConnected; + bool fInbound; +}; + +std::vector GetAddedNodeInfo(); + #endif // BITCOIN_NET_H diff --git a/src/netbase.cpp b/src/netbase.cpp index 1ad75788a..87956d6ec 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -613,10 +613,12 @@ bool ConnectSocketByName(CService &addr, SOCKET& hSocketRet, const char *pszDest proxyType nameProxy; GetNameProxy(nameProxy); - CService addrResolved(CNetAddr(strDest, fNameLookup && !HaveNameProxy()), port); - if (addrResolved.IsValid()) { - addr = addrResolved; - return ConnectSocket(addr, hSocketRet, nTimeout); + std::vector addrResolved; + if (Lookup(strDest.c_str(), addrResolved, port, fNameLookup && !HaveNameProxy(), 256)) { + if (addrResolved.size() > 0) { + addr = addrResolved[GetRand(addrResolved.size())]; + return ConnectSocket(addr, hSocketRet, nTimeout); + } } addr = CService("0.0.0.0:0"); diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index cd848d7b7..3d58b7882 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -270,25 +270,22 @@ UniValue getaddednodeinfo(const UniValue& params, bool fHelp) { if (fHelp || params.size() < 1 || params.size() > 2) throw runtime_error( - "getaddednodeinfo dns ( \"node\" )\n" + "getaddednodeinfo dummy ( \"node\" )\n" "\nReturns information about the given added node, or all added nodes\n" "(note that onetry addnodes are not listed here)\n" - "If dns is false, only a list of added nodes will be provided,\n" - "otherwise connected information will also be available.\n" "\nArguments:\n" - "1. dns (boolean, required) If false, only a list of added nodes will be provided, otherwise connected information will also be available.\n" + "1. dummy (boolean, required) Kept for historical purposes but ignored\n" "2. \"node\" (string, optional) If provided, return information about this specific node, otherwise all nodes are returned.\n" "\nResult:\n" "[\n" " {\n" - " \"addednode\" : \"192.168.0.201\", (string) The node ip address\n" + " \"addednode\" : \"192.168.0.201\", (string) The node ip address or name (as provided to addnode)\n" " \"connected\" : true|false, (boolean) If connected\n" - " \"addresses\" : [\n" + " \"addresses\" : [ (list of objects) Only when connected = true\n" " {\n" - " \"address\" : \"192.168.0.201:9999\", (string) The dash server host and port\n" + " \"address\" : \"192.168.0.201:9999\", (string) The dash server IP and port we're connected to\n" " \"connected\" : \"outbound\" (string) connection, inbound or outbound\n" " }\n" - " ,...\n" " ]\n" " }\n" " ,...\n" @@ -299,82 +296,35 @@ UniValue getaddednodeinfo(const UniValue& params, bool fHelp) + HelpExampleRpc("getaddednodeinfo", "true, \"192.168.0.201\"") ); - bool fDns = params[0].get_bool(); + std::vector vInfo = GetAddedNodeInfo(); - list laddedNodes(0); - if (params.size() == 1) - { - LOCK(cs_vAddedNodes); - BOOST_FOREACH(const std::string& strAddNode, vAddedNodes) - laddedNodes.push_back(strAddNode); - } - else - { - string strNode = params[1].get_str(); - LOCK(cs_vAddedNodes); - BOOST_FOREACH(const std::string& strAddNode, vAddedNodes) { - if (strAddNode == strNode) - { - laddedNodes.push_back(strAddNode); + if (params.size() == 2) { + bool found = false; + for (const AddedNodeInfo& info : vInfo) { + if (info.strAddedNode == params[1].get_str()) { + vInfo.assign(1, info); + found = true; break; } } - if (laddedNodes.size() == 0) + if (!found) { throw JSONRPCError(RPC_CLIENT_NODE_NOT_ADDED, "Error: Node has not been added."); + } } UniValue ret(UniValue::VARR); - if (!fDns) - { - BOOST_FOREACH (const std::string& strAddNode, laddedNodes) { - UniValue obj(UniValue::VOBJ); - obj.push_back(Pair("addednode", strAddNode)); - ret.push_back(obj); - } - return ret; - } - list > > laddedAddreses(0); - BOOST_FOREACH(const std::string& strAddNode, laddedNodes) { - vector vservNode(0); - if(Lookup(strAddNode.c_str(), vservNode, Params().GetDefaultPort(), fNameLookup, 0)) - laddedAddreses.push_back(make_pair(strAddNode, vservNode)); - else - { - UniValue obj(UniValue::VOBJ); - obj.push_back(Pair("addednode", strAddNode)); - obj.push_back(Pair("connected", false)); - UniValue addresses(UniValue::VARR); - obj.push_back(Pair("addresses", addresses)); - } - } - - LOCK(cs_vNodes); - for (list > >::iterator it = laddedAddreses.begin(); it != laddedAddreses.end(); it++) - { + for (const AddedNodeInfo& info : vInfo) { UniValue obj(UniValue::VOBJ); - obj.push_back(Pair("addednode", it->first)); - + obj.push_back(Pair("addednode", info.strAddedNode)); + obj.push_back(Pair("connected", info.fConnected)); UniValue addresses(UniValue::VARR); - bool fConnected = false; - BOOST_FOREACH(const CService& addrNode, it->second) { - bool fFound = false; - UniValue node(UniValue::VOBJ); - node.push_back(Pair("address", addrNode.ToString())); - BOOST_FOREACH(CNode* pnode, vNodes) { - if (pnode->addr == addrNode) - { - fFound = true; - fConnected = true; - node.push_back(Pair("connected", pnode->fInbound ? "inbound" : "outbound")); - break; - } - } - if (!fFound) - node.push_back(Pair("connected", "false")); - addresses.push_back(node); + if (info.fConnected) { + UniValue address(UniValue::VOBJ); + address.push_back(Pair("address", info.resolvedAddress.ToString())); + address.push_back(Pair("connected", info.fInbound ? "inbound" : "outbound")); + addresses.push_back(address); } - obj.push_back(Pair("connected", fConnected)); obj.push_back(Pair("addresses", addresses)); ret.push_back(obj); }