Skip to content

Commit cdc1d9f

Browse files
sipaFuzzbawls
sipa
authored and
Fuzzbawls
committed
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
1 parent e0239f8 commit cdc1d9f

File tree

3 files changed

+92
-116
lines changed

3 files changed

+92
-116
lines changed

‎src/net.cpp‎

Lines changed: 58 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1472,71 +1472,78 @@ void ThreadOpenConnections()
14721472
}
14731473
}
14741474

1475-
void ThreadOpenAddedConnections()
1475+
std::vector<AddedNodeInfo> GetAddedNodeInfo()
14761476
{
1477+
std::vector<AddedNodeInfo> ret;
1478+
1479+
std::list<std::string> lAddresses(0);
14771480
{
14781481
LOCK(cs_vAddedNodes);
1479-
vAddedNodes = mapMultiArgs["-addnode"];
1482+
ret.reserve(vAddedNodes.size());
1483+
for (std::string& strAddNode : vAddedNodes)
1484+
lAddresses.push_back(strAddNode);
14801485
}
14811486

1482-
if (HaveNameProxy()) {
1483-
while (true) {
1484-
std::list<std::string> lAddresses(0);
1485-
{
1486-
LOCK(cs_vAddedNodes);
1487-
for (std::string& strAddNode : vAddedNodes)
1488-
lAddresses.push_back(strAddNode);
1487+
1488+
// Build a map of all already connected addresses (by IP:port and by name) to inbound/outbound and resolved CService
1489+
std::map<CService, bool> mapConnected;
1490+
std::map<std::string, std::pair<bool, CService> > mapConnectedByName;
1491+
{
1492+
LOCK(cs_vNodes);
1493+
for (const CNode* pnode : vNodes) {
1494+
if (pnode->addr.IsValid()) {
1495+
mapConnected[pnode->addr] = pnode->fInbound;
14891496
}
1490-
for (std::string& strAddNode : lAddresses) {
1491-
CAddress addr;
1492-
CSemaphoreGrant grant(*semOutbound);
1493-
OpenNetworkConnection(addr, false, &grant, strAddNode.c_str());
1494-
MilliSleep(500);
1497+
if (!pnode->addrName.empty()) {
1498+
mapConnectedByName[pnode->addrName] = std::make_pair(pnode->fInbound, static_cast<const CService&>(pnode->addr));
14951499
}
1496-
MilliSleep(120000); // Retry every 2 minutes
14971500
}
14981501
}
14991502

1500-
for (unsigned int i = 0; true; i++) {
1501-
std::list<std::string> lAddresses(0);
1502-
{
1503-
LOCK(cs_vAddedNodes);
1504-
for (std::string& strAddNode : vAddedNodes)
1505-
lAddresses.push_back(strAddNode);
1503+
for (std::string& strAddNode : lAddresses) {
1504+
CService service(strAddNode, Params().GetDefaultPort());
1505+
if (service.IsValid()) {
1506+
// strAddNode is an IP:port
1507+
auto it = mapConnected.find(service);
1508+
if (it != mapConnected.end()) {
1509+
ret.push_back(AddedNodeInfo{strAddNode, service, true, it->second});
1510+
} else {
1511+
ret.push_back(AddedNodeInfo{strAddNode, CService(), false, false});
1512+
}
1513+
} else {
1514+
// strAddNode is a name
1515+
auto it = mapConnectedByName.find(strAddNode);
1516+
if (it != mapConnectedByName.end()) {
1517+
ret.push_back(AddedNodeInfo{strAddNode, it->second.second, true, it->second.first});
1518+
} else {
1519+
ret.push_back(AddedNodeInfo{strAddNode, CService(), false, false});
1520+
}
15061521
}
1522+
}
15071523

1508-
std::list<std::vector<CService> > lservAddressesToAdd(0);
1509-
for (std::string& strAddNode : lAddresses) {
1510-
std::vector<CService> vservNode(0);
1511-
if (Lookup(strAddNode.c_str(), vservNode, Params().GetDefaultPort(), fNameLookup, 0)) {
1512-
lservAddressesToAdd.push_back(vservNode);
1513-
{
1514-
LOCK(cs_setservAddNodeAddresses);
1515-
for (CService& serv : vservNode)
1516-
setservAddNodeAddresses.insert(serv);
1517-
}
1524+
return ret;
1525+
}
1526+
1527+
void ThreadOpenAddedConnections()
1528+
{
1529+
{
1530+
LOCK(cs_vAddedNodes);
1531+
vAddedNodes = mapMultiArgs["-addnode"];
1532+
}
1533+
1534+
for (unsigned int i = 0; true; i++) {
1535+
std::vector<AddedNodeInfo> vInfo = GetAddedNodeInfo();
1536+
for (const AddedNodeInfo& info : vInfo) {
1537+
if (!info.fConnected) {
1538+
CSemaphoreGrant grant(*semOutbound);
1539+
// If strAddedNode is an IP/port, decode it immediately, so
1540+
// OpenNetworkConnection can detect existing connections to that IP/port.
1541+
CService service(info.strAddedNode, Params().GetDefaultPort());
1542+
OpenNetworkConnection(CAddress(service, NODE_NONE), false, &grant, info.strAddedNode.c_str(), false);
1543+
MilliSleep(500);
15181544
}
15191545
}
1520-
// Attempt to connect to each IP for each addnode entry until at least one is successful per addnode entry
1521-
// (keeping in mind that addnode entries can have many IPs if fNameLookup)
1522-
{
1523-
LOCK(cs_vNodes);
1524-
for (CNode* pnode : vNodes)
1525-
for (std::list<std::vector<CService> >::iterator it = lservAddressesToAdd.begin(); it != lservAddressesToAdd.end(); it++)
1526-
for (CService& addrNode : *(it))
1527-
if (pnode->addr == addrNode) {
1528-
it = lservAddressesToAdd.erase(it);
1529-
it--;
1530-
break;
1531-
}
1532-
}
1533-
for (std::vector<CService>& vserv : lservAddressesToAdd) {
1534-
CSemaphoreGrant grant(*semOutbound);
1535-
/* We want -addnode to work even for nodes that don't provide all
1536-
* wanted services, so pass in nServices=NODE_NONE to CAddress. */
1537-
OpenNetworkConnection(CAddress(vserv[i % vserv.size()], NODE_NONE), false, &grant);
1538-
MilliSleep(500);
1539-
}
1546+
15401547
MilliSleep(120000); // Retry every 2 minutes
15411548
}
15421549
}

‎src/net.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -777,4 +777,13 @@ class CBanDB
777777

778778
void DumpBanlist();
779779

780+
struct AddedNodeInfo {
781+
std::string strAddedNode;
782+
CService resolvedAddress;
783+
bool fConnected;
784+
bool fInbound;
785+
};
786+
787+
std::vector<AddedNodeInfo> GetAddedNodeInfo();
788+
780789
#endif // BITCOIN_NET_H

‎src/rpc/net.cpp‎

Lines changed: 25 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) 2009-2014 The Bitcoin developers
1+
// Copyright (c) 2009-2015 The Bitcoin developers
22
// Copyright (c) 2014-2015 The Dash developers
33
// Copyright (c) 2015-2019 The PIVX developers
44
// Distributed under the MIT software license, see the accompanying
@@ -238,27 +238,24 @@ UniValue getaddednodeinfo(const UniValue& params, bool fHelp)
238238
{
239239
if (fHelp || params.size() < 1 || params.size() > 2)
240240
throw std::runtime_error(
241-
"getaddednodeinfo dns ( \"node\" )\n"
241+
"getaddednodeinfo dummy ( \"node\" )\n"
242242
"\nReturns information about the given added node, or all added nodes\n"
243243
"(note that onetry addnodes are not listed here)\n"
244-
"If dns is false, only a list of added nodes will be provided,\n"
245-
"otherwise connected information will also be available.\n"
246244

247245
"\nArguments:\n"
248-
"1. dns (boolean, required) If false, only a list of added nodes will be provided, otherwise connected information will also be available.\n"
246+
"1. dummy (boolean, required) Kept for historical purposes but ignored\n"
249247
"2. \"node\" (string, optional) If provided, return information about this specific node, otherwise all nodes are returned.\n"
250248

251249
"\nResult:\n"
252250
"[\n"
253251
" {\n"
254-
" \"addednode\" : \"192.168.0.201\", (string) The node ip address\n"
252+
" \"addednode\" : \"192.168.0.201\", (string) The node ip address or name (as provided to addnode)\n"
255253
" \"connected\" : true|false, (boolean) If connected\n"
256-
" \"addresses\" : [\n"
254+
" \"addresses\" : [ (list of objects) Only when connected = true\n"
257255
" {\n"
258-
" \"address\" : \"192.168.0.201:51472\", (string) The pivx server host and port\n"
256+
" \"address\" : \"192.168.0.201:51472\", (string) The pivx server IP and port we're connected to\n"
259257
" \"connected\" : \"outbound\" (string) connection, inbound or outbound\n"
260258
" }\n"
261-
" ,...\n"
262259
" ]\n"
263260
" }\n"
264261
" ,...\n"
@@ -267,72 +264,35 @@ UniValue getaddednodeinfo(const UniValue& params, bool fHelp)
267264
"\nExamples:\n" +
268265
HelpExampleCli("getaddednodeinfo", "true") + HelpExampleCli("getaddednodeinfo", "true \"192.168.0.201\"") + HelpExampleRpc("getaddednodeinfo", "true, \"192.168.0.201\""));
269266

270-
bool fDns = params[0].get_bool();
271-
272-
std::list<std::string> laddedNodes(0);
273-
if (params.size() == 1) {
274-
LOCK(cs_vAddedNodes);
275-
for (std::string& strAddNode : vAddedNodes)
276-
laddedNodes.push_back(strAddNode);
277-
} else {
278-
std::string strNode = params[1].get_str();
279-
LOCK(cs_vAddedNodes);
280-
for (std::string& strAddNode : vAddedNodes)
281-
if (strAddNode == strNode) {
282-
laddedNodes.push_back(strAddNode);
267+
std::vector<AddedNodeInfo> vInfo = GetAddedNodeInfo();
268+
269+
if (params.size() == 2) {
270+
bool found = false;
271+
for (const AddedNodeInfo& info : vInfo) {
272+
if (info.strAddedNode == params[1].get_str()) {
273+
vInfo.assign(1, info);
274+
found = true;
283275
break;
284276
}
285-
if (laddedNodes.size() == 0)
277+
}
278+
if (!found) {
286279
throw JSONRPCError(RPC_CLIENT_NODE_NOT_ADDED, "Error: Node has not been added.");
287-
}
288-
289-
UniValue ret(UniValue::VARR);
290-
if (!fDns) {
291-
for (std::string& strAddNode : laddedNodes) {
292-
UniValue obj(UniValue::VOBJ);
293-
obj.push_back(Pair("addednode", strAddNode));
294-
ret.push_back(obj);
295280
}
296-
return ret;
297281
}
298282

299-
std::list<std::pair<std::string, std::vector<CService> > > laddedAddreses(0);
300-
for (std::string& strAddNode : laddedNodes) {
301-
std::vector<CService> vservNode(0);
302-
if (Lookup(strAddNode.c_str(), vservNode, Params().GetDefaultPort(), fNameLookup, 0))
303-
laddedAddreses.push_back(std::make_pair(strAddNode, vservNode));
304-
else {
305-
UniValue obj(UniValue::VOBJ);
306-
obj.push_back(Pair("addednode", strAddNode));
307-
obj.push_back(Pair("connected", false));
308-
UniValue addresses(UniValue::VARR);
309-
obj.push_back(Pair("addresses", addresses));
310-
}
311-
}
283+
UniValue ret(UniValue::VARR);
312284

313-
LOCK(cs_vNodes);
314-
for (std::list<std::pair<std::string, std::vector<CService> > >::iterator it = laddedAddreses.begin(); it != laddedAddreses.end(); it++) {
285+
for (const AddedNodeInfo& info : vInfo) {
315286
UniValue obj(UniValue::VOBJ);
316-
obj.push_back(Pair("addednode", it->first));
317-
287+
obj.push_back(Pair("addednode", info.strAddedNode));
288+
obj.push_back(Pair("connected", info.fConnected));
318289
UniValue addresses(UniValue::VARR);
319-
bool fConnected = false;
320-
for (CService& addrNode : it->second) {
321-
bool fFound = false;
322-
UniValue node(UniValue::VOBJ);
323-
node.push_back(Pair("address", addrNode.ToString()));
324-
for (CNode* pnode : vNodes)
325-
if (pnode->addr == addrNode) {
326-
fFound = true;
327-
fConnected = true;
328-
node.push_back(Pair("connected", pnode->fInbound ? "inbound" : "outbound"));
329-
break;
330-
}
331-
if (!fFound)
332-
node.push_back(Pair("connected", "false"));
333-
addresses.push_back(node);
290+
if (info.fConnected) {
291+
UniValue address(UniValue::VOBJ);
292+
address.push_back(Pair("address", info.resolvedAddress.ToString()));
293+
address.push_back(Pair("connected", info.fInbound ? "inbound" : "outbound"));
294+
addresses.push_back(address);
334295
}
335-
obj.push_back(Pair("connected", fConnected));
336296
obj.push_back(Pair("addresses", addresses));
337297
ret.push_back(obj);
338298
}

0 commit comments

Comments
 (0)