Skip to content

Commit

Permalink
Merge bitcoin#10663: net: split resolve out of connect
Browse files Browse the repository at this point in the history 
b887676 net: remove now-unused functions (Cory Fields)
45fd754 net: remove now-superfluous numeric resolve (Cory Fields)
2416dd7 net: separate resolving and conecting (Cory Fields)

Pull request description:

  This is a greatly simplified version of bitcoin#10285, which only aims to address async resolving.

  It essentially breaks up two wrapper functions for things only used in one place (ConnectSocketDirectly/ConnectThroughProxy) in favor of calling them directly. This allows us to fully handle resolves before attempting a connection, as is necessary for async connections.

  As a bonus, I believe the logic is now much easier to follow than before.

Tree-SHA512: f03f618107379edf3efe2a9f3e3677e8f075017ab140a0b4fdc3b8263e6beff148d55256263ab10bc2125ef089ca68e0d8e865beeae176f1eca544e769c976d3
  • Loading branch information
@laanwj @PastaPastaPasta
laanwj authored and PastaPastaPasta committed Dec 22, 2019
1 parent 39bbe50 commit 3a6067a
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 68 deletions.
64 changes: 41 additions & 23 deletions src/net.cpp
View file
Expand Up @@ -402,19 +402,16 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
pszDest ? 0.0 : (double)(GetAdjustedTime() - addrConnect.nTime)/3600.0);
}

// Connect
SOCKET hSocket;
bool proxyConnectionFailed = false;
if (pszDest ? ConnectSocketByName(addrConnect, hSocket, pszDest, Params().GetDefaultPort(), nConnectTimeout, &proxyConnectionFailed) :
ConnectSocket(addrConnect, hSocket, nConnectTimeout, &proxyConnectionFailed))
{
if (!IsSelectableSocket(hSocket)) {
LogPrintf("Cannot create connection: non-selectable socket created (fd >= FD_SETSIZE ?)\n");
CloseSocket(hSocket);
return nullptr;
}

if (pszDest && addrConnect.IsValid()) {
// Resolve
const int default_port = Params().GetDefaultPort();
if (pszDest) {
std::vector<CService> resolved;
if (Lookup(pszDest, resolved, default_port, fNameLookup && !HaveNameProxy(), 256) && !resolved.empty()) {
addrConnect = CAddress(resolved[GetRand(resolved.size())], NODE_NONE);
if (!addrConnect.IsValid()) {
LogPrint(BCLog::NET, "Resolver returned invalid address %s for %s", addrConnect.ToString(), pszDest);
return nullptr;
}
// 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
Expand All @@ -424,13 +421,40 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
if (pnode)
{
pnode->MaybeSetAddrName(std::string(pszDest));
CloseSocket(hSocket);
LogPrintf("Failed to open new connection, already connected\n");
return nullptr;
}
}
}

addrman.Attempt(addrConnect, fCountFailure);
// Connect
bool connected = false;
SOCKET hSocket;
proxyType proxy;
if (addrConnect.IsValid()) {
bool proxyConnectionFailed = false;

if (GetProxy(addrConnect.GetNetwork(), proxy))
connected = ConnectThroughProxy(proxy, addrConnect.ToStringIP(), addrConnect.GetPort(), hSocket, nConnectTimeout, &proxyConnectionFailed);
else // no proxy needed (none set for target network)
connected = ConnectSocketDirectly(addrConnect, hSocket, nConnectTimeout);
if (!proxyConnectionFailed) {
// If a connection to the node was attempted, and failure (if any) is not caused by a problem connecting to
// the proxy, mark this as an attempt.
addrman.Attempt(addrConnect, fCountFailure);
}
} else if (pszDest && GetNameProxy(proxy)) {
std::string host;
int port = default_port;
SplitHostPort(std::string(pszDest), port, host);
connected = ConnectThroughProxy(proxy, host, port, hSocket, nConnectTimeout, nullptr);
}
if (connected) {
if (!IsSelectableSocket(hSocket)) {
LogPrintf("Cannot create connection: non-selectable socket created (fd >= FD_SETSIZE ?)\n");
CloseSocket(hSocket);
return nullptr;
}

// Add node
NodeId id = GetNewNodeId();
Expand All @@ -441,10 +465,6 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo


return pnode;
} else if (!proxyConnectionFailed) {
// If connecting to the node failed, and failure is not caused by a problem connecting to
// the proxy, mark this as an attempt.
addrman.Attempt(addrConnect, fCountFailure);
}

return nullptr;
Expand Down Expand Up @@ -2083,11 +2103,9 @@ void CConnman::ThreadOpenAddedConnections()
// the addednodeinfo state might change.
break;
}
// If strAddedNode is an IP/port, decode it immediately, so
// OpenNetworkConnection can detect existing connections to that IP/port.
tried = true;
CService service(LookupNumeric(info.strAddedNode.c_str(), Params().GetDefaultPort()));
OpenNetworkConnection(CAddress(service, NODE_NONE), false, &grant, info.strAddedNode.c_str(), false, false, true);
CAddress addr(CService(), NODE_NONE);
OpenNetworkConnection(addr, false, &grant, info.strAddedNode.c_str(), false, false, true);
if (!interruptNet.sleep_for(std::chrono::milliseconds(500)))
return;
}
Expand Down
45 changes: 2 additions & 43 deletions src/netbase.cpp
View file
Expand Up @@ -452,7 +452,7 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials
return true;
}

bool static ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int nTimeout)
bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int nTimeout)
{
hSocketRet = INVALID_SOCKET;

Expand Down Expand Up @@ -587,7 +587,7 @@ bool IsProxy(const CNetAddr &addr) {
return false;
}

static bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed)
bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed)
{
SOCKET hSocket = INVALID_SOCKET;
// first connect to proxy server
Expand All @@ -611,47 +611,6 @@ static bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDe
hSocketRet = hSocket;
return true;
}

bool ConnectSocket(const CService &addrDest, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed)
{
proxyType proxy;
if (outProxyConnectionFailed)
*outProxyConnectionFailed = false;

if (GetProxy(addrDest.GetNetwork(), proxy))
return ConnectThroughProxy(proxy, addrDest.ToStringIP(), addrDest.GetPort(), hSocketRet, nTimeout, outProxyConnectionFailed);
else // no proxy needed (none set for target network)
return ConnectSocketDirectly(addrDest, hSocketRet, nTimeout);
}

bool ConnectSocketByName(CService &addr, SOCKET& hSocketRet, const char *pszDest, int portDefault, int nTimeout, bool *outProxyConnectionFailed)
{
std::string strDest;
int port = portDefault;

if (outProxyConnectionFailed)
*outProxyConnectionFailed = false;

SplitHostPort(std::string(pszDest), port, strDest);

proxyType proxy;
GetNameProxy(proxy);

std::vector<CService> 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();

if (!HaveNameProxy())
return false;
return ConnectThroughProxy(proxy, strDest, port, hSocketRet, nTimeout, outProxyConnectionFailed);
}

bool LookupSubNet(const char* pszName, CSubNet& ret)
{
std::string strSubnet(pszName);
Expand Down
5 changes: 3 additions & 2 deletions src/netbase.h
View file
Expand Up @@ -45,14 +45,15 @@ bool GetProxy(enum Network net, proxyType &proxyInfoOut);
bool IsProxy(const CNetAddr &addr);
bool SetNameProxy(const proxyType &addrProxy);
bool HaveNameProxy();
bool GetNameProxy(proxyType &nameProxyOut);
bool LookupHost(const char *pszName, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup);
bool LookupHost(const char *pszName, CNetAddr& addr, bool fAllowLookup);
bool Lookup(const char *pszName, CService& addr, int portDefault, bool fAllowLookup);
bool Lookup(const char *pszName, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions);
CService LookupNumeric(const char *pszName, int portDefault = 0);
bool LookupSubNet(const char *pszName, CSubNet& subnet);
bool ConnectSocket(const CService &addr, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed = nullptr);
bool ConnectSocketByName(CService &addr, SOCKET& hSocketRet, const char *pszDest, int portDefault, int nTimeout, bool *outProxyConnectionFailed = nullptr);
bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int nTimeout);
bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed);
/** Return readable error string for a network error code */
std::string NetworkErrorString(int err);
/** Close socket and set hSocket to INVALID_SOCKET */
Expand Down

0 comments on commit 3a6067a

Please sign in to comment.