From 1111b80df84aa7bc72fbddcc7bafde43f0835d90 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 28 May 2016 15:32:30 +0200 Subject: [PATCH] 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 --- src/net.cpp | 107 ++++++++++++++++++++++++++---------------------- src/net.h | 10 +++++ src/rpc/net.cpp | 95 ++++++++++-------------------------------- 3 files changed, 91 insertions(+), 121 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index a390eca776..4d27db7604 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1616,68 +1616,79 @@ void ThreadOpenConnections() } } -void ThreadOpenAddedConnections() +std::vector GetAddedNodeInfo() { + std::vector ret; + + std::list lAddresses(0); { LOCK(cs_vAddedNodes); - vAddedNodes = mapMultiArgs["-addnode"]; + ret.reserve(vAddedNodes.size()); + BOOST_FOREACH(const std::string& strAddNode, vAddedNodes) + lAddresses.push_back(strAddNode); } - if (HaveNameProxy()) { - while(true) { - std::list lAddresses(0); - { - LOCK(cs_vAddedNodes); - 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; } - BOOST_FOREACH(const std::string& strAddNode, lAddresses) { - CAddress addr; - CSemaphoreGrant grant(*semOutbound); - OpenNetworkConnection(addr, false, &grant, strAddNode.c_str()); - MilliSleep(500); + if (!pnode->addrName.empty()) { + mapConnectedByName[pnode->addrName] = std::make_pair(pnode->fInbound, static_cast(pnode->addr)); } - MilliSleep(120000); // Retry every 2 minutes } } + 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() +{ + { + LOCK(cs_vAddedNodes); + vAddedNodes = mapMultiArgs["-addnode"]; + } + 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), false, &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), false, &grant); - MilliSleep(500); - } MilliSleep(120000); // Retry every 2 minutes } } diff --git a/src/net.h b/src/net.h index 2aaca4888f..54a86d88dc 100644 --- a/src/net.h +++ b/src/net.h @@ -818,4 +818,14 @@ public: /** Return a timestamp in the future (in microseconds) for exponentially distributed events. */ int64_t PoissonNextSend(int64_t nNow, int average_interval_seconds); +struct AddedNodeInfo +{ + std::string strAddedNode; + CService resolvedAddress; + bool fConnected; + bool fInbound; +}; + +std::vector GetAddedNodeInfo(); + #endif // BITCOIN_NET_H diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index cae964e46d..b85c7b2e1a 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -271,25 +271,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:8333\", (string) The bitcoin server host and port\n" + " \"address\" : \"192.168.0.201:8333\", (string) The bitcoin server IP and port we're connected to\n" " \"connected\" : \"outbound\" (string) connection, inbound or outbound\n" " }\n" - " ,...\n" " ]\n" " }\n" " ,...\n" @@ -300,83 +297,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)); - ret.push_back(obj); - } - } + UniValue ret(UniValue::VARR); - 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); }