diff --git a/src/init.cpp b/src/init.cpp --- a/src/init.cpp +++ b/src/init.cpp @@ -455,7 +455,8 @@ strUsage += HelpMessageGroup(_("Connection options:")); strUsage += HelpMessageOpt( "-addnode=", - _("Add a node to connect to and attempt to keep the connection open")); + _("Add a node to connect to and attempt to keep the connection open " + "(see the `addnode` RPC command help for more info)")); strUsage += HelpMessageOpt( "-banscore=", strprintf( @@ -468,10 +469,10 @@ strUsage += HelpMessageOpt("-bind=", _("Bind to given address and always listen on " "it. Use [host]:port notation for IPv6")); - strUsage += - HelpMessageOpt("-connect=", - _("Connect only to the specified node(s); -noconnect or " - "-connect=0 alone to disable automatic connections")); + strUsage += HelpMessageOpt( + "-connect=", _("Connect only to the specified node(s); -connect=0 " + "disables automatic connections (the rules for this " + "peer are the same as for -addnode)")); strUsage += HelpMessageOpt("-discover", _("Discover own IP addresses (default: 1 when " "listening and no -externalip or -proxy)")); @@ -1259,7 +1260,6 @@ namespace { // Variables internal to initialization process only -ServiceFlags nRelevantServices = NODE_NETWORK; int nMaxConnections; int nUserMaxConnections; int nFD; @@ -2233,7 +2233,6 @@ CConnman::Options connOptions; connOptions.nLocalServices = nLocalServices; - connOptions.nRelevantServices = nRelevantServices; connOptions.nMaxConnections = nMaxConnections; connOptions.nMaxOutbound = std::min(MAX_OUTBOUND_CONNECTIONS, connOptions.nMaxConnections); diff --git a/src/net.h b/src/net.h --- a/src/net.h +++ b/src/net.h @@ -85,8 +85,6 @@ static const size_t DEFAULT_MAXRECEIVEBUFFER = 5 * 1000; static const size_t DEFAULT_MAXSENDBUFFER = 1 * 1000; -static const ServiceFlags REQUIRED_SERVICES = ServiceFlags(NODE_NETWORK); - // Default 24-hour ban. // NOTE: When adjusting this, update rpcnet:setban's help ("24h") static const unsigned int DEFAULT_MISBEHAVING_BANTIME = 60 * 60 * 24; @@ -127,7 +125,6 @@ struct Options { ServiceFlags nLocalServices = NODE_NONE; - ServiceFlags nRelevantServices = NODE_NONE; int nMaxConnections = 0; int nMaxOutbound = 0; int nMaxAddnode = 0; @@ -146,7 +143,6 @@ void Init(const Options &connOptions) { nLocalServices = connOptions.nLocalServices; - nRelevantServices = connOptions.nRelevantServices; nMaxConnections = connOptions.nMaxConnections; nMaxOutbound = std::min(connOptions.nMaxOutbound, connOptions.nMaxConnections); @@ -174,7 +170,7 @@ CSemaphoreGrant *grantOutbound = nullptr, const char *strDest = nullptr, bool fOneShot = false, bool fFeeler = false, - bool fAddnode = false); + bool manual_connection = false); bool CheckIncomingNonce(uint64_t nonce); bool ForNode(NodeId id, std::function func); @@ -391,9 +387,6 @@ /** Services this instance offers */ ServiceFlags nLocalServices; - /** Services this instance cares about */ - ServiceFlags nRelevantServices; - CSemaphore *semOutbound; CSemaphore *semAddnode; int nMaxConnections; @@ -518,7 +511,7 @@ int nVersion; std::string cleanSubVer; bool fInbound; - bool fAddnode; + bool m_manual_connection; int nStartingHeight; uint64_t nSendBytes; mapMsgCmdSize mapSendBytesPerMsgCmd; @@ -595,8 +588,6 @@ public: // socket std::atomic nServices; - // Services expected from a peer, otherwise it will be disconnected - ServiceFlags nServicesExpected; SOCKET hSocket; // Total size of all vSendMsg entries. size_t nSendSize; @@ -640,7 +631,7 @@ // If true this node is being used as a short lived feeler. bool fFeeler; bool fOneShot; - bool fAddnode; + bool m_manual_connection; bool fClient; const bool fInbound; std::atomic_bool fSuccessfullyConnected; diff --git a/src/net.cpp b/src/net.cpp --- a/src/net.cpp +++ b/src/net.cpp @@ -411,8 +411,6 @@ new CNode(id, nLocalServices, GetBestHeight(), hSocket, addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", false); - pnode->nServicesExpected = - ServiceFlags(addrConnect.nServices & nRelevantServices); pnode->AddRef(); return pnode; @@ -668,7 +666,7 @@ stats.cleanSubVer = cleanSubVer; } stats.fInbound = fInbound; - stats.fAddnode = fAddnode; + stats.m_manual_connection = m_manual_connection; stats.nStartingHeight = nStartingHeight; { LOCK(cs_vSend); @@ -1004,12 +1002,12 @@ continue; } NodeEvictionCandidate candidate = { - node->id, + node->GetId(), node->nTimeConnected, node->nMinPingUsecTime, node->nLastBlockTime, node->nLastTXTime, - (node->nServices & nRelevantServices) == nRelevantServices, + HasAllDesirableServiceFlags(node->nServices), node->fRelayTxes, node->pfilter != nullptr, node->addr, @@ -1684,9 +1682,9 @@ LOCK(cs_vNodes); int nRelevant = 0; for (auto pnode : vNodes) { - nRelevant += - pnode->fSuccessfullyConnected && - ((pnode->nServices & nRelevantServices) == nRelevantServices); + nRelevant += pnode->fSuccessfullyConnected && !pnode->fFeeler && + !pnode->fOneShot && !pnode->m_manual_connection && + !pnode->fInbound; } if (nRelevant >= 2) { LogPrintf("P2P peers available. Skipped DNS seeding.\n"); @@ -1706,7 +1704,8 @@ } else { std::vector vIPs; std::vector vAdd; - ServiceFlags requiredServiceBits = nRelevantServices; + ServiceFlags requiredServiceBits = + GetDesirableServiceFlags(NODE_NONE); std::string host = GetDNSHost(seed, &requiredServiceBits); CNetAddr resolveSource; if (!resolveSource.SetInternal(host)) { @@ -1773,7 +1772,8 @@ ProcessOneShot(); for (const std::string &strAddr : gArgs.GetArgs("-connect")) { CAddress addr(CService(), NODE_NONE); - OpenNetworkConnection(addr, false, nullptr, strAddr.c_str()); + OpenNetworkConnection(addr, false, nullptr, strAddr.c_str(), + false, false, true); for (int i = 0; i < 10 && i < nLoop; i++) { if (!interruptNet.sleep_for( std::chrono::milliseconds(500))) { @@ -1832,7 +1832,7 @@ { LOCK(cs_vNodes); for (CNode *pnode : vNodes) { - if (!pnode->fInbound && !pnode->fAddnode) { + if (!pnode->fInbound && !pnode->m_manual_connection) { // Netgroups for inbound and addnode peers are not excluded // because our goal here is to not use multiple of our // limited outbound slots on a single netgroup but inbound @@ -1895,20 +1895,19 @@ continue; } - // only connect to full nodes - if ((addr.nServices & REQUIRED_SERVICES) != REQUIRED_SERVICES) { + // only consider very recently tried nodes after 30 failed attempts + if (nANow - addr.nLastTry < 600 && nTries < 30) { continue; } - // only consider very recently tried nodes after 30 failed attempts - if (nANow - addr.nLastTry < 600 && nTries < 30) { + // for non-feelers, require all the services we'll want, + // for feelers, only require they be a full node (only because most + // SPV clients don't have a good address DB available) + if (!fFeeler && !HasAllDesirableServiceFlags(addr.nServices)) { continue; } - // only consider nodes missing relevant services after 40 failed - // attempts and only if less than half the outbound are up. - if ((addr.nServices & nRelevantServices) != nRelevantServices && - (nTries < 40 || nOutbound >= (nMaxOutbound >> 1))) { + if (fFeeler && !MayHaveUsefulAddressDB(addr.nServices)) { continue; } @@ -2053,7 +2052,7 @@ bool fCountFailure, CSemaphoreGrant *grantOutbound, const char *pszDest, bool fOneShot, - bool fFeeler, bool fAddnode) { + bool fFeeler, bool manual_connection) { // // Initiate outbound network connection // @@ -2086,8 +2085,8 @@ if (fFeeler) { pnode->fFeeler = true; } - if (fAddnode) { - pnode->fAddnode = true; + if (manual_connection) { + pnode->m_manual_connection = true; } m_msgproc->InitializeNode(*config, pnode); @@ -2860,7 +2859,6 @@ nLocalHostNonce(nLocalHostNonceIn), nLocalServices(nLocalServicesIn), nMyStartingHeight(nMyStartingHeightIn), nSendVersion(0) { nServices = NODE_NONE; - nServicesExpected = NODE_NONE; hSocket = hSocketIn; nRecvVersion = INIT_PROTO_VERSION; nLastSend = 0; @@ -2873,7 +2871,7 @@ strSubVer = ""; fWhitelisted = false; fOneShot = false; - fAddnode = false; + m_manual_connection = false; // set by version message fClient = false; fFeeler = false; diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1428,17 +1428,19 @@ if (!pfrom->fInbound) { connman->SetServices(pfrom->addr, nServices); } - if (pfrom->nServicesExpected & ~nServices) { - LogPrint(BCLog::NET, "peer=%d does not offer the expected services " - "(%08x offered, %08x expected); " - "disconnecting\n", - pfrom->GetId(), nServices, pfrom->nServicesExpected); + if (!pfrom->fInbound && !pfrom->fFeeler && + !pfrom->m_manual_connection && + !HasAllDesirableServiceFlags(nServices)) { + LogPrint( + BCLog::NET, "peer=%d does not offer the expected services " + "(%08x offered, %08x expected); disconnecting\n", + pfrom->GetId(), nServices, GetDesirableServiceFlags(nServices)); connman->PushMessage( pfrom, CNetMsgMaker(INIT_PROTO_VERSION) .Make(NetMsgType::REJECT, strCommand, REJECT_NONSTANDARD, strprintf("Expected to offer services %08x", - pfrom->nServicesExpected))); + GetDesirableServiceFlags(nServices)))); pfrom->fDisconnect = true; return false; } @@ -1659,7 +1661,10 @@ return true; } - if ((addr.nServices & REQUIRED_SERVICES) != REQUIRED_SERVICES) { + // We only bother storing full nodes, though this may include things + // which we would not make an outbound connection to, in part + // because we may make feeler connections to them. + if (!MayHaveUsefulAddressDB(addr.nServices)) { continue; } @@ -2995,8 +3000,8 @@ if (pnode->fWhitelisted) { LogPrintf("Warning: not punishing whitelisted peer %s!\n", pnode->addr.ToString()); - } else if (pnode->fAddnode) { - LogPrintf("Warning: not punishing addnoded peer %s!\n", + } else if (pnode->m_manual_connection) { + LogPrintf("Warning: not punishing manually-connected peer %s!\n", pnode->addr.ToString()); } else { pnode->fDisconnect = true; diff --git a/src/protocol.h b/src/protocol.h --- a/src/protocol.h +++ b/src/protocol.h @@ -304,6 +304,43 @@ // BIP process. }; +/** + * Gets the set of service flags which are "desirable" for a given peer. + * + * These are the flags which are required for a peer to support for them + * to be "interesting" to us, ie for us to wish to use one of our few + * outbound connection slots for or for us to wish to prioritize keeping + * their connection around. + * + * Relevant service flags may be peer- and state-specific in that the + * version of the peer may determine which flags are required (eg in the + * case of NODE_NETWORK_LIMITED where we seek out NODE_NETWORK peers + * unless they set NODE_NETWORK_LIMITED and we are out of IBD, in which + * case NODE_NETWORK_LIMITED suffices). + * + * Thus, generally, avoid calling with peerServices == NODE_NONE. + */ +static ServiceFlags GetDesirableServiceFlags(ServiceFlags services) { + return ServiceFlags(NODE_NETWORK); +} + +/** + * A shortcut for (services & GetDesirableServiceFlags(services)) + * == GetDesirableServiceFlags(services), ie determines whether the given + * set of service flags are sufficient for a peer to be "relevant". + */ +static inline bool HasAllDesirableServiceFlags(ServiceFlags services) { + return !(GetDesirableServiceFlags(services) & (~services)); +} + +/** + * Checks if a peer with the given service flags may be capable of having a + * robust address-storage DB. Currently an alias for checking NODE_NETWORK. + */ +static inline bool MayHaveUsefulAddressDB(ServiceFlags services) { + return services & NODE_NETWORK; +} + /** * A CService with information about it as peer. */ diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -110,7 +110,8 @@ " \"inbound\": true|false, (boolean) Inbound (true) or " "Outbound (false)\n" " \"addnode\": true|false, (boolean) Whether connection was " - "due to addnode and is using an addnode slot\n" + "due to addnode/-connect or if it was an automatic/inbound " + "connection\n" " \"startingheight\": n, (numeric) The starting height " "(block) of the peer\n" " \"banscore\": n, (numeric) The ban score\n" @@ -189,7 +190,7 @@ // characters in their ver message. obj.push_back(Pair("subver", stats.cleanSubVer)); obj.push_back(Pair("inbound", stats.fInbound)); - obj.push_back(Pair("addnode", stats.fAddnode)); + obj.push_back(Pair("addnode", stats.m_manual_connection)); obj.push_back(Pair("startingheight", stats.nStartingHeight)); if (fStateStats) { obj.push_back(Pair("banscore", statestats.nMisbehavior)); @@ -235,6 +236,10 @@ "addnode \"node\" \"add|remove|onetry\"\n" "\nAttempts add or remove a node from the addnode list.\n" "Or try a connection to a node once.\n" + "Nodes added using addnode (or -connect) are protected from DoS " + "disconnection and are not required to be\n" + "full nodes/support SegWit as other outbound peers are (though " + "such peers will not be synced from).\n" "\nArguments:\n" "1. \"node\" (string, required) The node (see getpeerinfo for " "nodes)\n" @@ -254,7 +259,8 @@ if (strCommand == "onetry") { CAddress addr; - g_connman->OpenNetworkConnection(addr, false, nullptr, strNode.c_str()); + g_connman->OpenNetworkConnection(addr, false, nullptr, strNode.c_str(), + false, false, true); return NullUniValue; }