Skip to content

Commit

Permalink
Rework addnode behaviour
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@sipa @Fuzzbawls
sipa authored and Fuzzbawls committed May 23, 2020
1 parent e0239f8 commit cdc1d9f
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 116 deletions.
109 changes: 58 additions & 51 deletions src/net.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1472,71 +1472,78 @@ void ThreadOpenConnections()
}
}

void ThreadOpenAddedConnections()
std::vector<AddedNodeInfo> GetAddedNodeInfo()
{
std::vector<AddedNodeInfo> ret;

std::list<std::string> lAddresses(0);
{
LOCK(cs_vAddedNodes);
vAddedNodes = mapMultiArgs["-addnode"];
ret.reserve(vAddedNodes.size());
for (std::string& strAddNode : vAddedNodes)
lAddresses.push_back(strAddNode);
}

if (HaveNameProxy()) {
while (true) {
std::list<std::string> lAddresses(0);
{
LOCK(cs_vAddedNodes);
for (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<CService, bool> mapConnected;
std::map<std::string, std::pair<bool, CService> > mapConnectedByName;
{
LOCK(cs_vNodes);
for (const CNode* pnode : vNodes) {
if (pnode->addr.IsValid()) {
mapConnected[pnode->addr] = pnode->fInbound;
}
for (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<const CService&>(pnode->addr));
}
MilliSleep(120000); // Retry every 2 minutes
}
}

for (unsigned int i = 0; true; i++) {
std::list<std::string> lAddresses(0);
{
LOCK(cs_vAddedNodes);
for (std::string& strAddNode : vAddedNodes)
lAddresses.push_back(strAddNode);
for (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});
}
}
}

std::list<std::vector<CService> > lservAddressesToAdd(0);
for (std::string& strAddNode : lAddresses) {
std::vector<CService> vservNode(0);
if (Lookup(strAddNode.c_str(), vservNode, Params().GetDefaultPort(), fNameLookup, 0)) {
lservAddressesToAdd.push_back(vservNode);
{
LOCK(cs_setservAddNodeAddresses);
for (CService& serv : vservNode)
setservAddNodeAddresses.insert(serv);
}
return ret;
}

void ThreadOpenAddedConnections()
{
{
LOCK(cs_vAddedNodes);
vAddedNodes = mapMultiArgs["-addnode"];
}

for (unsigned int i = 0; true; i++) {
std::vector<AddedNodeInfo> 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);
}
}
// 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);
for (CNode* pnode : vNodes)
for (std::list<std::vector<CService> >::iterator it = lservAddressesToAdd.begin(); it != lservAddressesToAdd.end(); it++)
for (CService& addrNode : *(it))
if (pnode->addr == addrNode) {
it = lservAddressesToAdd.erase(it);
it--;
break;
}
}
for (std::vector<CService>& 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
}
}
Expand Down
9 changes: 9 additions & 0 deletions src/net.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -777,4 +777,13 @@ class CBanDB

void DumpBanlist();

struct AddedNodeInfo {
std::string strAddedNode;
CService resolvedAddress;
bool fConnected;
bool fInbound;
};

std::vector<AddedNodeInfo> GetAddedNodeInfo();

#endif // BITCOIN_NET_H
90 changes: 25 additions & 65 deletions src/rpc/net.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2009-2014 The Bitcoin developers
// Copyright (c) 2009-2015 The Bitcoin developers
// Copyright (c) 2014-2015 The Dash developers
// Copyright (c) 2015-2019 The PIVX developers
// Distributed under the MIT software license, see the accompanying
Expand Down Expand Up @@ -238,27 +238,24 @@ UniValue getaddednodeinfo(const UniValue& params, bool fHelp)
{
if (fHelp || params.size() < 1 || params.size() > 2)
throw std::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:51472\", (string) The pivx server host and port\n"
" \"address\" : \"192.168.0.201:51472\", (string) The pivx server IP and port we're connected to\n"
" \"connected\" : \"outbound\" (string) connection, inbound or outbound\n"
" }\n"
" ,...\n"
" ]\n"
" }\n"
" ,...\n"
Expand All @@ -267,72 +264,35 @@ UniValue getaddednodeinfo(const UniValue& params, bool fHelp)
"\nExamples:\n" +
HelpExampleCli("getaddednodeinfo", "true") + HelpExampleCli("getaddednodeinfo", "true \"192.168.0.201\"") + HelpExampleRpc("getaddednodeinfo", "true, \"192.168.0.201\""));

bool fDns = params[0].get_bool();

std::list<std::string> laddedNodes(0);
if (params.size() == 1) {
LOCK(cs_vAddedNodes);
for (std::string& strAddNode : vAddedNodes)
laddedNodes.push_back(strAddNode);
} else {
std::string strNode = params[1].get_str();
LOCK(cs_vAddedNodes);
for (std::string& strAddNode : vAddedNodes)
if (strAddNode == strNode) {
laddedNodes.push_back(strAddNode);
std::vector<AddedNodeInfo> vInfo = GetAddedNodeInfo();

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) {
for (std::string& strAddNode : laddedNodes) {
UniValue obj(UniValue::VOBJ);
obj.push_back(Pair("addednode", strAddNode));
ret.push_back(obj);
}
return ret;
}

std::list<std::pair<std::string, std::vector<CService> > > laddedAddreses(0);
for (std::string& strAddNode : laddedNodes) {
std::vector<CService> vservNode(0);
if (Lookup(strAddNode.c_str(), vservNode, Params().GetDefaultPort(), fNameLookup, 0))
laddedAddreses.push_back(std::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));
}
}
UniValue ret(UniValue::VARR);

LOCK(cs_vNodes);
for (std::list<std::pair<std::string, std::vector<CService> > >::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;
for (CService& addrNode : it->second) {
bool fFound = false;
UniValue node(UniValue::VOBJ);
node.push_back(Pair("address", addrNode.ToString()));
for (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);
}
Expand Down

0 comments on commit cdc1d9f

Please sign in to comment.