From 7f88a67d5bb962166f8e135c04a699511d966c1e Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 13 Dec 2017 05:09:57 +0100 Subject: [PATCH 01/18] Merge #11363: net: Split socket create/connect 3830b6e net: use CreateSocket for binds (Cory Fields) df3bcf8 net: pass socket closing responsibility up to caller for outgoing connections (Cory Fields) 9e3b2f5 net: Move IsSelectableSocket check into socket creation (Cory Fields) 1729c29 net: split socket creation out of connection (Cory Fields) Pull request description: Requirement for #11227. We'll need to create sockets and perform the actual connect in separate steps, so break them up. #11227 adds an RAII wrapper around connection attempts, as a belt-and-suspenders in case a CloseSocket is missed. Tree-SHA512: de675bb718cc56d68893c303b8057ca062c7431eaa17ae7c4829caed119fa3f15b404d8f52aca22a6bca6e73a26fb79e898b335d090ab015bf6456cf417fc694 --- src/net.cpp | 71 +++++++++++++++++++------------------------------ src/netbase.cpp | 70 ++++++++++++++++++++++-------------------------- src/netbase.h | 5 ++-- 3 files changed, 63 insertions(+), 83 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index c111a411ae138..f710d2b002dc1 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -434,40 +434,48 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo if (addrConnect.IsValid()) { bool proxyConnectionFailed = false; - if (GetProxy(addrConnect.GetNetwork(), proxy)) + if (GetProxy(addrConnect.GetNetwork(), proxy)) { + hSocket = CreateSocket(proxy.proxy); + if (hSocket == INVALID_SOCKET) { + return nullptr; + } connected = ConnectThroughProxy(proxy, addrConnect.ToStringIP(), addrConnect.GetPort(), hSocket, nConnectTimeout, &proxyConnectionFailed); - else // no proxy needed (none set for target network) + } else { + // no proxy needed (none set for target network) + hSocket = CreateSocket(addrConnect); + if (hSocket == INVALID_SOCKET) { + return nullptr; + } 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)) { + hSocket = CreateSocket(proxy.proxy); + if (hSocket == INVALID_SOCKET) { + return nullptr; + } 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(); - uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); - CAddress addr_bind = GetBindAddress(hSocket); - CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", false); - pnode->AddRef(); - - - return pnode; + if (!connected) { + CloseSocket(hSocket); + return nullptr; } - return nullptr; + // Add node + NodeId id = GetNewNodeId(); + uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); + CAddress addr_bind = GetBindAddress(hSocket); + CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", false); + pnode->AddRef(); + + return pnode; } void CConnman::DumpBanlist() @@ -2311,44 +2319,21 @@ bool CConnman::BindListenPort(const CService &addrBind, std::string& strError, b return false; } - SOCKET hListenSocket = socket(((struct sockaddr*)&sockaddr)->sa_family, SOCK_STREAM, IPPROTO_TCP); + SOCKET hListenSocket = CreateSocket(addrBind); if (hListenSocket == INVALID_SOCKET) { strError = strprintf("Error: Couldn't open socket for incoming connections (socket returned error %s)", NetworkErrorString(WSAGetLastError())); LogPrintf("%s\n", strError); return false; } - if (!IsSelectableSocket(hListenSocket)) - { - strError = "Error: Couldn't create a listenable socket for incoming connections"; - LogPrintf("%s\n", strError); - return false; - } - - #ifndef WIN32 -#ifdef SO_NOSIGPIPE - // Different way of disabling SIGPIPE on BSD - setsockopt(hListenSocket, SOL_SOCKET, SO_NOSIGPIPE, (void*)&nOne, sizeof(int)); -#endif // Allow binding if the port is still in TIME_WAIT state after // the program was closed and restarted. setsockopt(hListenSocket, SOL_SOCKET, SO_REUSEADDR, (void*)&nOne, sizeof(int)); - // Disable Nagle's algorithm - setsockopt(hListenSocket, IPPROTO_TCP, TCP_NODELAY, (void*)&nOne, sizeof(int)); #else setsockopt(hListenSocket, SOL_SOCKET, SO_REUSEADDR, (const char*)&nOne, sizeof(int)); - setsockopt(hListenSocket, IPPROTO_TCP, TCP_NODELAY, (const char*)&nOne, sizeof(int)); #endif - // Set to non-blocking, incoming connections will also inherit this - if (!SetSocketNonBlocking(hListenSocket, true)) { - CloseSocket(hListenSocket); - strError = strprintf("BindListenPort: Setting listening socket to non-blocking failed, error %s\n", NetworkErrorString(WSAGetLastError())); - LogPrintf("%s\n", strError); - return false; - } - // some systems don't have IPV6_V6ONLY but are always v6only; others do have the option // and enable it by default or not. Try to enable it, if possible. if (addrBind.IsIPv6()) { diff --git a/src/netbase.cpp b/src/netbase.cpp index f35eed7ff1424..c1db993561d81 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -317,12 +317,11 @@ std::string Socks5ErrorString(uint8_t err) } /** Connect using SOCKS5 (as described in RFC1928) */ -static bool Socks5(const std::string& strDest, int port, const ProxyCredentials *auth, SOCKET& hSocket) +static bool Socks5(const std::string& strDest, int port, const ProxyCredentials *auth, const SOCKET& hSocket) { IntrRecvError recvr; LogPrint(BCLog::NET, "SOCKS5 connecting %s\n", strDest); if (strDest.size() > 255) { - CloseSocket(hSocket); return error("Hostname too long"); } // Accepted authentication methods @@ -338,17 +337,14 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials } ssize_t ret = send(hSocket, (const char*)vSocks5Init.data(), vSocks5Init.size(), MSG_NOSIGNAL); if (ret != (ssize_t)vSocks5Init.size()) { - CloseSocket(hSocket); return error("Error sending to proxy"); } uint8_t pchRet1[2]; if ((recvr = InterruptibleRecv(pchRet1, 2, SOCKS5_RECV_TIMEOUT, hSocket)) != IntrRecvError::OK) { - CloseSocket(hSocket); LogPrintf("Socks5() connect to %s:%d failed: InterruptibleRecv() timeout or other failure\n", strDest, port); return false; } if (pchRet1[0] != SOCKSVersion::SOCKS5) { - CloseSocket(hSocket); return error("Proxy failed to initialize"); } if (pchRet1[1] == SOCKS5Method::USER_PASS && auth) { @@ -363,23 +359,19 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials vAuth.insert(vAuth.end(), auth->password.begin(), auth->password.end()); ret = send(hSocket, (const char*)vAuth.data(), vAuth.size(), MSG_NOSIGNAL); if (ret != (ssize_t)vAuth.size()) { - CloseSocket(hSocket); return error("Error sending authentication to proxy"); } LogPrint(BCLog::PROXY, "SOCKS5 sending proxy authentication %s:%s\n", auth->username, auth->password); uint8_t pchRetA[2]; if ((recvr = InterruptibleRecv(pchRetA, 2, SOCKS5_RECV_TIMEOUT, hSocket)) != IntrRecvError::OK) { - CloseSocket(hSocket); return error("Error reading proxy authentication response"); } if (pchRetA[0] != 0x01 || pchRetA[1] != 0x00) { - CloseSocket(hSocket); return error("Proxy authentication unsuccessful"); } } else if (pchRet1[1] == SOCKS5Method::NOAUTH) { // Perform no authentication } else { - CloseSocket(hSocket); return error("Proxy requested wrong authentication method %02x", pchRet1[1]); } std::vector vSocks5; @@ -393,12 +385,10 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials vSocks5.push_back((port >> 0) & 0xFF); ret = send(hSocket, (const char*)vSocks5.data(), vSocks5.size(), MSG_NOSIGNAL); if (ret != (ssize_t)vSocks5.size()) { - CloseSocket(hSocket); return error("Error sending to proxy"); } uint8_t pchRet2[4]; if ((recvr = InterruptibleRecv(pchRet2, 4, SOCKS5_RECV_TIMEOUT, hSocket)) != IntrRecvError::OK) { - CloseSocket(hSocket); if (recvr == IntrRecvError::Timeout) { /* If a timeout happens here, this effectively means we timed out while connecting * to the remote node. This is very common for Tor, so do not print an @@ -409,17 +399,14 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials } } if (pchRet2[0] != SOCKSVersion::SOCKS5) { - CloseSocket(hSocket); return error("Proxy failed to accept request"); } if (pchRet2[1] != SOCKS5Reply::SUCCEEDED) { // Failures to connect to a peer that are not proxy errors - CloseSocket(hSocket); LogPrintf("Socks5() connect to %s:%d failed: %s\n", strDest, port, Socks5ErrorString(pchRet2[1])); return false; } if (pchRet2[2] != 0x00) { // Reserved field must be 0 - CloseSocket(hSocket); return error("Error: malformed proxy response"); } uint8_t pchRet3[256]; @@ -431,41 +418,42 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials { recvr = InterruptibleRecv(pchRet3, 1, SOCKS5_RECV_TIMEOUT, hSocket); if (recvr != IntrRecvError::OK) { - CloseSocket(hSocket); return error("Error reading from proxy"); } int nRecv = pchRet3[0]; recvr = InterruptibleRecv(pchRet3, nRecv, SOCKS5_RECV_TIMEOUT, hSocket); break; } - default: CloseSocket(hSocket); return error("Error: malformed proxy response"); + default: return error("Error: malformed proxy response"); } if (recvr != IntrRecvError::OK) { - CloseSocket(hSocket); return error("Error reading from proxy"); } if ((recvr = InterruptibleRecv(pchRet3, 2, SOCKS5_RECV_TIMEOUT, hSocket)) != IntrRecvError::OK) { - CloseSocket(hSocket); return error("Error reading from proxy"); } LogPrint(BCLog::NET, "SOCKS5 connected %s\n", strDest); return true; } -bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int nTimeout) +SOCKET CreateSocket(const CService &addrConnect) { - hSocketRet = INVALID_SOCKET; - struct sockaddr_storage sockaddr; socklen_t len = sizeof(sockaddr); if (!addrConnect.GetSockAddr((struct sockaddr*)&sockaddr, &len)) { - LogPrintf("Cannot connect to %s: unsupported network\n", addrConnect.ToString()); - return false; + LogPrintf("Cannot create socket for %s: unsupported network\n", addrConnect.ToString()); + return INVALID_SOCKET; } SOCKET hSocket = socket(((struct sockaddr*)&sockaddr)->sa_family, SOCK_STREAM, IPPROTO_TCP); if (hSocket == INVALID_SOCKET) - return false; + return INVALID_SOCKET; + + if (!IsSelectableSocket(hSocket)) { + CloseSocket(hSocket); + LogPrintf("Cannot create connection: non-selectable socket created (fd >= FD_SETSIZE ?)\n"); + return INVALID_SOCKET; + } #ifdef SO_NOSIGPIPE int set = 1; @@ -479,9 +467,23 @@ bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int // Set to non-blocking if (!SetSocketNonBlocking(hSocket, true)) { CloseSocket(hSocket); - return error("ConnectSocketDirectly: Setting socket to non-blocking failed, error %s\n", NetworkErrorString(WSAGetLastError())); + LogPrintf("ConnectSocketDirectly: Setting socket to non-blocking failed, error %s\n", NetworkErrorString(WSAGetLastError())); } + return hSocket; +} +bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocket, int nTimeout) +{ + struct sockaddr_storage sockaddr; + socklen_t len = sizeof(sockaddr); + if (hSocket == INVALID_SOCKET) { + LogPrintf("Cannot connect to %s: invalid socket\n", addrConnect.ToString()); + return false; + } + if (!addrConnect.GetSockAddr((struct sockaddr*)&sockaddr, &len)) { + LogPrintf("Cannot connect to %s: unsupported network\n", addrConnect.ToString()); + return false; + } if (connect(hSocket, (struct sockaddr*)&sockaddr, len) == SOCKET_ERROR) { int nErr = WSAGetLastError(); @@ -496,13 +498,11 @@ bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int if (nRet == 0) { LogPrint(BCLog::NET, "connection to %s timeout\n", addrConnect.ToString()); - CloseSocket(hSocket); return false; } if (nRet == SOCKET_ERROR) { LogPrintf("select() for %s failed: %s\n", addrConnect.ToString(), NetworkErrorString(WSAGetLastError())); - CloseSocket(hSocket); return false; } socklen_t nRetSize = sizeof(nRet); @@ -513,13 +513,11 @@ bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int #endif { LogPrintf("getsockopt() for %s failed: %s\n", addrConnect.ToString(), NetworkErrorString(WSAGetLastError())); - CloseSocket(hSocket); return false; } if (nRet != 0) { LogPrintf("connect() to %s failed after select(): %s\n", addrConnect.ToString(), NetworkErrorString(nRet)); - CloseSocket(hSocket); return false; } } @@ -530,12 +528,9 @@ bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int #endif { LogPrintf("connect() to %s failed: %s\n", addrConnect.ToString(), NetworkErrorString(WSAGetLastError())); - CloseSocket(hSocket); return false; } } - - hSocketRet = hSocket; return true; } @@ -587,9 +582,8 @@ bool IsProxy(const CNetAddr &addr) { return false; } -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, const SOCKET& hSocket, int nTimeout, bool *outProxyConnectionFailed) { - SOCKET hSocket = INVALID_SOCKET; // first connect to proxy server if (!ConnectSocketDirectly(proxy.proxy, hSocket, nTimeout)) { if (outProxyConnectionFailed) @@ -601,14 +595,14 @@ bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int ProxyCredentials random_auth; static std::atomic_int counter(0); random_auth.username = random_auth.password = strprintf("%i", counter++); - if (!Socks5(strDest, (unsigned short)port, &random_auth, hSocket)) + if (!Socks5(strDest, (unsigned short)port, &random_auth, hSocket)) { return false; + } } else { - if (!Socks5(strDest, (unsigned short)port, 0, hSocket)) + if (!Socks5(strDest, (unsigned short)port, 0, hSocket)) { return false; + } } - - hSocketRet = hSocket; return true; } bool LookupSubNet(const char* pszName, CSubNet& ret) diff --git a/src/netbase.h b/src/netbase.h index ef7fc5199d2ad..0e71d5d2c1ae9 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -52,8 +52,9 @@ bool Lookup(const char *pszName, CService& addr, int portDefault, bool fAllowLoo bool Lookup(const char *pszName, std::vector& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions); CService LookupNumeric(const char *pszName, int portDefault = 0); bool LookupSubNet(const char *pszName, CSubNet& subnet); -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); +SOCKET CreateSocket(const CService &addrConnect); +bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocketRet, int nTimeout); +bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const 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 */ From c2611736692281ebb9800dc39eeb518f71dd0b54 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Sat, 25 Aug 2018 17:57:05 +0200 Subject: [PATCH 02/18] Merge #13946: p2p: Clarify control flow in ProcessMessage fa6c3dea420b6c50c164ccc34f4e9e8a7d9a8022 p2p: Clarify control flow in ProcessMessage() (MarcoFalke) Pull request description: `ProcessMessage` is effectively a massive switch case construct. In the past there were attempts to clarify the control flow in `ProcessMessage()` by moving each case into a separate static function (see #9608). It was closed because it wasn't clear if moving each case into a function was the right approach. Though, we can quasi treat each case as a function by adding a return statement to each case. (Can be seen as a continuation of bugfix #13162) This patch does exactly that. Also note that this patch is a subset of previous approaches such as #9608 and #10145. Review suggestion: `git diff HEAD~ --function-context` Tree-SHA512: 91f6106840de2f29bb4f10d27bae0616b03a91126e6c6013479e1dd79bee53f22a78902b631fe85517dd5dc0fa7239939b4fefc231851a13c819458559f6c201 --- src/net_processing.cpp | 166 +++++++++++++++++++---------------------- 1 file changed, 75 insertions(+), 91 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 91c7158694450..9fd39c4279c28 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1790,8 +1790,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } } - else if (strCommand == NetMsgType::VERSION) - { + if (strCommand == NetMsgType::VERSION) { // Each connection can only send one version message if (pfrom->nVersion != 0) { @@ -1961,9 +1960,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr return true; } - - else if (pfrom->nVersion == 0) - { + if (pfrom->nVersion == 0) { // Must have a version message before anything else LOCK(cs_main); Misbehaving(pfrom->GetId(), 1); @@ -2028,18 +2025,17 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } pfrom->fSuccessfullyConnected = true; + return true; } - else if (!pfrom->fSuccessfullyConnected) - { + if (!pfrom->fSuccessfullyConnected) { // Must have a verack message before anything else LOCK(cs_main); Misbehaving(pfrom->GetId(), 1); return false; } - else if (strCommand == NetMsgType::ADDR) - { + if (strCommand == NetMsgType::ADDR) { std::vector vAddr; vRecv >> vAddr; @@ -2085,17 +2081,16 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr pfrom->fGetAddr = false; if (pfrom->fOneShot) pfrom->fDisconnect = true; + return true; } - else if (strCommand == NetMsgType::SENDHEADERS) - { + if (strCommand == NetMsgType::SENDHEADERS) { LOCK(cs_main); State(pfrom->GetId())->fPreferHeaders = true; + return true; } - - else if (strCommand == NetMsgType::SENDCMPCT) - { + if (strCommand == NetMsgType::SENDCMPCT) { bool fAnnounceUsingCMPCTBLOCK = false; uint64_t nCMPCTBLOCKVersion = 1; vRecv >> fAnnounceUsingCMPCTBLOCK >> nCMPCTBLOCKVersion; @@ -2105,6 +2100,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr State(pfrom->GetId())->fPreferHeaderAndIDs = fAnnounceUsingCMPCTBLOCK; State(pfrom->GetId())->fSupportsDesiredCmpctVersion = true; } + return true; } @@ -2122,9 +2118,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr pfrom->fSendRecSigs = b; } - - else if (strCommand == NetMsgType::INV) - { + if (strCommand == NetMsgType::INV) { std::vector vInv; vRecv >> vInv; if (vInv.size() > MAX_INV_SZ) @@ -2218,11 +2212,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // Track requests for our stuff GetMainSignals().Inventory(inv.hash); } + return true; } - - else if (strCommand == NetMsgType::GETDATA) - { + if (strCommand == NetMsgType::GETDATA) { std::vector vInv; vRecv >> vInv; if (vInv.size() > MAX_INV_SZ) @@ -2240,11 +2233,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr pfrom->vRecvGetData.insert(pfrom->vRecvGetData.end(), vInv.begin(), vInv.end()); ProcessGetData(pfrom, chainparams.GetConsensus(), connman, interruptMsgProc); + return true; } - - else if (strCommand == NetMsgType::GETBLOCKS) - { + if (strCommand == NetMsgType::GETBLOCKS) { CBlockLocator locator; uint256 hashStop; vRecv >> locator >> hashStop; @@ -2301,11 +2293,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr break; } } + return true; } - - else if (strCommand == NetMsgType::GETBLOCKTXN) - { + if (strCommand == NetMsgType::GETBLOCKTXN) { BlockTransactionsRequest req; vRecv >> req; @@ -2351,11 +2342,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr assert(ret); SendBlockTransactions(block, req, pfrom, connman); + return true; } - - else if (strCommand == NetMsgType::GETHEADERS) - { + if (strCommand == NetMsgType::GETHEADERS) { CBlockLocator locator; uint256 hashStop; vRecv >> locator >> hashStop; @@ -2413,11 +2403,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // in the SendMessages logic. nodestate->pindexBestHeaderSent = pindex ? pindex : chainActive.Tip(); connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::HEADERS, vHeaders)); + return true; } - - else if (strCommand == NetMsgType::TX || strCommand == NetMsgType::DSTX || strCommand == NetMsgType::LEGACYTXLOCKREQUEST) - { + if (strCommand == NetMsgType::TX || strCommand == NetMsgType::DSTX || strCommand == NetMsgType::LEGACYTXLOCKREQUEST) { // Stop processing the transaction early if // We are in blocks only mode and peer is either not whitelisted or whitelistrelay is off if (!fRelayTxes && (!pfrom->fWhitelisted || !gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY))) @@ -2605,9 +2594,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr Misbehaving(pfrom->GetId(), nDoS); } } + return true; } - else if (strCommand == NetMsgType::CMPCTBLOCK && !fImporting && !fReindex) // Ignore blocks received while importing + if (strCommand == NetMsgType::CMPCTBLOCK && !fImporting && !fReindex) // Ignore blocks received while importing { CBlockHeaderAndShortTxIDs cmpctblock; vRecv >> cmpctblock; @@ -2819,10 +2809,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr MarkBlockAsReceived(pblock->GetHash()); } } - + return true; } - else if (strCommand == NetMsgType::BLOCKTXN && !fImporting && !fReindex) // Ignore blocks received while importing + if (strCommand == NetMsgType::BLOCKTXN && !fImporting && !fReindex) // Ignore blocks received while importing { BlockTransactions resp; vRecv >> resp; @@ -2895,10 +2885,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr mapBlockSource.erase(pblock->GetHash()); } } + return true; } - - else if (strCommand == NetMsgType::HEADERS && !fImporting && !fReindex) // Ignore headers received while importing + if (strCommand == NetMsgType::HEADERS && !fImporting && !fReindex) // Ignore headers received while importing { std::vector headers; @@ -2923,7 +2913,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr return ProcessHeadersMessage(pfrom, connman, headers, chainparams, should_punish); } - else if (strCommand == NetMsgType::BLOCK && !fImporting && !fReindex) // Ignore blocks received while importing + if (strCommand == NetMsgType::BLOCK && !fImporting && !fReindex) // Ignore blocks received while importing { std::shared_ptr pblock = std::make_shared(); vRecv >> *pblock; @@ -2949,11 +2939,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LOCK(cs_main); mapBlockSource.erase(pblock->GetHash()); } + return true; } - - else if (strCommand == NetMsgType::GETADDR) - { + if (strCommand == NetMsgType::GETADDR) { // This asymmetric behavior for inbound and outbound connections was introduced // to prevent a fingerprinting attack: an attacker can send specific fake addresses // to users' AddrMan and later request them by sending getaddr messages. @@ -2977,11 +2966,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr FastRandomContext insecure_rand; for (const CAddress &addr : vAddr) pfrom->PushAddress(addr, insecure_rand); + return true; } - - else if (strCommand == NetMsgType::MEMPOOL) - { + if (strCommand == NetMsgType::MEMPOOL) { if (!(pfrom->GetLocalServices() & NODE_BLOOM) && !pfrom->fWhitelisted) { LogPrint(BCLog::NET, "mempool request with bloom filters disabled, disconnect peer=%d\n", pfrom->GetId()); @@ -2998,11 +2986,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LOCK(pfrom->cs_inventory); pfrom->fSendMempool = true; + return true; } - - else if (strCommand == NetMsgType::PING) - { + if (strCommand == NetMsgType::PING) { if (pfrom->nVersion > BIP0031_VERSION) { uint64_t nonce = 0; @@ -3020,11 +3007,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // return very quickly. connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::PONG, nonce)); } + return true; } - - else if (strCommand == NetMsgType::PONG) - { + if (strCommand == NetMsgType::PONG) { int64_t pingUsecEnd = nTimeReceived; uint64_t nonce = 0; size_t nAvail = vRecv.in_avail(); @@ -3077,11 +3063,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (bPingFinished) { pfrom->nPingNonceSent = 0; } + return true; } - - else if (strCommand == NetMsgType::FILTERLOAD) - { + if (strCommand == NetMsgType::FILTERLOAD) { CBloomFilter filter; vRecv >> filter; @@ -3099,11 +3084,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr pfrom->pfilter->UpdateEmptyFull(); pfrom->fRelayTxes = true; } + return true; } - - else if (strCommand == NetMsgType::FILTERADD) - { + if (strCommand == NetMsgType::FILTERADD) { std::vector vData; vRecv >> vData; @@ -3124,21 +3108,21 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LOCK(cs_main); Misbehaving(pfrom->GetId(), 100); } + return true; } - - else if (strCommand == NetMsgType::FILTERCLEAR) - { + if (strCommand == NetMsgType::FILTERCLEAR) { LOCK(pfrom->cs_filter); if (pfrom->GetLocalServices() & NODE_BLOOM) { delete pfrom->pfilter; pfrom->pfilter = new CBloomFilter(); } pfrom->fRelayTxes = true; + return true; } - else if (strCommand == NetMsgType::GETMNLISTDIFF) { + if (strCommand == NetMsgType::GETMNLISTDIFF) { CGetSimplifiedMNListDiff cmd; vRecv >> cmd; @@ -3152,57 +3136,57 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LogPrint(BCLog::NET, "getmnlistdiff failed for baseBlockHash=%s, blockHash=%s. error=%s\n", cmd.baseBlockHash.ToString(), cmd.blockHash.ToString(), strError); Misbehaving(pfrom->GetId(), 1); } + return true; } - else if (strCommand == NetMsgType::MNLISTDIFF) { + if (strCommand == NetMsgType::MNLISTDIFF) { // we have never requested this LOCK(cs_main); Misbehaving(pfrom->GetId(), 100); LogPrint(BCLog::NET, "received not-requested mnlistdiff. peer=%d\n", pfrom->GetId()); + return true; } - else if (strCommand == NetMsgType::NOTFOUND) { + if (strCommand == NetMsgType::NOTFOUND) { // We do not care about the NOTFOUND message, but logging an Unknown Command // message would be undesirable as we transmit it ourselves. + return true; } - else { - bool found = false; - const std::vector &allMessages = getAllNetMessageTypes(); - for (const std::string msg : allMessages) { - if(msg == strCommand) { - found = true; - break; - } + bool found = false; + const std::vector &allMessages = getAllNetMessageTypes(); + for (const std::string msg : allMessages) { + if(msg == strCommand) { + found = true; + break; } + } - if (found) - { - //probably one the extensions + if (found) + { + //probably one the extensions #ifdef ENABLE_WALLET - privateSendClient.ProcessMessage(pfrom, strCommand, vRecv, *connman); + privateSendClient.ProcessMessage(pfrom, strCommand, vRecv, *connman); #endif // ENABLE_WALLET - privateSendServer.ProcessMessage(pfrom, strCommand, vRecv, *connman); - sporkManager.ProcessSpork(pfrom, strCommand, vRecv, *connman); - masternodeSync.ProcessMessage(pfrom, strCommand, vRecv); - governance.ProcessMessage(pfrom, strCommand, vRecv, *connman); - CMNAuth::ProcessMessage(pfrom, strCommand, vRecv, *connman); - llmq::quorumBlockProcessor->ProcessMessage(pfrom, strCommand, vRecv, *connman); - llmq::quorumDKGSessionManager->ProcessMessage(pfrom, strCommand, vRecv, *connman); - llmq::quorumSigSharesManager->ProcessMessage(pfrom, strCommand, vRecv, *connman); - llmq::quorumSigningManager->ProcessMessage(pfrom, strCommand, vRecv, *connman); - llmq::chainLocksHandler->ProcessMessage(pfrom, strCommand, vRecv, *connman); - llmq::quorumInstantSendManager->ProcessMessage(pfrom, strCommand, vRecv, *connman); - } - else - { - // Ignore unknown commands for extensibility - LogPrint(BCLog::NET, "Unknown command \"%s\" from peer=%d\n", SanitizeString(strCommand), pfrom->GetId()); - } + privateSendServer.ProcessMessage(pfrom, strCommand, vRecv, *connman); + sporkManager.ProcessSpork(pfrom, strCommand, vRecv, *connman); + masternodeSync.ProcessMessage(pfrom, strCommand, vRecv); + governance.ProcessMessage(pfrom, strCommand, vRecv, *connman); + CMNAuth::ProcessMessage(pfrom, strCommand, vRecv, *connman); + llmq::quorumBlockProcessor->ProcessMessage(pfrom, strCommand, vRecv, *connman); + llmq::quorumDKGSessionManager->ProcessMessage(pfrom, strCommand, vRecv, *connman); + llmq::quorumSigSharesManager->ProcessMessage(pfrom, strCommand, vRecv, *connman); + llmq::quorumSigningManager->ProcessMessage(pfrom, strCommand, vRecv, *connman); + llmq::chainLocksHandler->ProcessMessage(pfrom, strCommand, vRecv, *connman); + llmq::quorumInstantSendManager->ProcessMessage(pfrom, strCommand, vRecv, *connman); + return true; } + // Ignore unknown commands for extensibility + LogPrint(BCLog::NET, "Unknown command \"%s\" from peer=%d\n", SanitizeString(strCommand), pfrom->GetId()); + return true; } From 88197c74146f013d084e397c668294a9d4ef6e46 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 9 Oct 2017 16:10:29 +0200 Subject: [PATCH 03/18] Merge #11448: [gui] reset addrProxy/addrSeparateProxyTor if colon char missing ce2418f [gui] reset addrProxy/addrSeparateProxyTor if colon char missing (Cristian Mircea Messel) Pull request description: If addrProxy or addrSeparateProxyTor do not have a colon in the string somewhere in the QSettings storage, then attempting to open the options dialog will cause the entire program to crash. Fixes #11209 Tree-SHA512: 2d9e6987cf05af3f41033290b61d00920f7fe4a65bea7efd96ed417a8ca7866d248f091e09947cc8aad3a6a4aa8b7777211cfff7f379a62188be50df2c46d4b2 --- src/qt/optionsmodel.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index dc57e7781c086..b9e146cb6d821 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -168,7 +168,7 @@ void OptionsModel::Init(bool resetSettings) if (!settings.contains("fUseProxy")) settings.setValue("fUseProxy", false); - if (!settings.contains("addrProxy")) + if (!settings.contains("addrProxy") || !settings.value("addrProxy").toString().contains(':')) settings.setValue("addrProxy", "127.0.0.1:9050"); // Only try to set -proxy, if user has enabled fUseProxy if (settings.value("fUseProxy").toBool() && !gArgs.SoftSetArg("-proxy", settings.value("addrProxy").toString().toStdString())) @@ -178,7 +178,7 @@ void OptionsModel::Init(bool resetSettings) if (!settings.contains("fUseSeparateProxyTor")) settings.setValue("fUseSeparateProxyTor", false); - if (!settings.contains("addrSeparateProxyTor")) + if (!settings.contains("addrSeparateProxyTor") || !settings.value("addrSeparateProxyTor").toString().contains(':')) settings.setValue("addrSeparateProxyTor", "127.0.0.1:9050"); // Only try to set -onion, if user has enabled fUseSeparateProxyTor if (settings.value("fUseSeparateProxyTor").toBool() && !gArgs.SoftSetArg("-onion", settings.value("addrSeparateProxyTor").toString().toStdString())) From 94e99441c5bdc51a18e76f8e013252e1982db325 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 24 Jan 2018 13:01:14 +0100 Subject: [PATCH 04/18] Merge #11512: Use GetDesireableServiceFlags in seeds, dnsseeds, fixing static seed adding 2b839ab Update chainparams comment for more info on service bits per dnsseed (Matt Corallo) 62e7642 Fall back to oneshot for DNS Seeds which don't support filtering. (Matt Corallo) 51ae766 Use GetDesireableServiceFlags in static seeds, document this. (Matt Corallo) fb6f6b1 bluematt's testnet-seed now supports x9 (and is just a static list) (Matt Corallo) Pull request description: 4440710 broke inserting entries into addrman from dnsseeds which did not support service bits, as well as static seeds. Static seeds were already being filtered by UA for 0.13.1+ (ie NODE_WITNESS), so simply changing the default service bits to include NODE_WITNESS (and updating docs appropriately) is sufficient. For DNS Seeds, not supporting NODE_WITNESS is no longer useful, so instead use non-filtering seeds as oneshot hosts irrespective of named proxy. I've set my testnet-seed to also support x9, though because it is simply a static host, it may be useful to leave the support off so that it is used as a oneshot to get addresses from a live node instead. I'm fine with either. Tree-SHA512: 3f17d4d2b0b84d876981c962d2b44cb0c8f95f52c56a48c6b35fd882f6d7a40805f320ec452985a1c0b34aebddb1922709156c3ceccd1b9f8363fd7cb537d21d --- src/chainparams.cpp | 11 ++++++++--- src/chainparams.h | 11 +++-------- src/net.cpp | 26 +++++++++----------------- src/protocol.h | 10 +++++++++- 4 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index b1ac902c6d091..8944e7375e4f1 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -331,8 +331,13 @@ class CMainParams : public CChainParams { assert(consensus.hashGenesisBlock == uint256S("0x00000ffd590b1485b3caadc19b22e6379c733355108f107a430458cdf3407ab6")); assert(genesis.hashMerkleRoot == uint256S("0xe0028eb9648db56b1ac77cf090b99048a8007e2bb64b68f092c03c7f56a662c7")); - vSeeds.emplace_back("dnsseed.dash.org", true); - vSeeds.emplace_back("dnsseed.dashdot.io", true); + // Note that of those which support the service bits prefix, most only support a subset of + // possible options. + // This is fine at runtime as we'll fall back to using them as a oneshot if they dont support the + // service bits we want, but we should get them updated to support all service bits wanted by any + // release ASAP to avoid it where possible. + vSeeds.emplace_back("dnsseed.dash.org"); + vSeeds.emplace_back("dnsseed.dashdot.io"); // Dash addresses start with 'X' base58Prefixes[PUBKEY_ADDRESS] = std::vector(1,76); @@ -510,7 +515,7 @@ class CTestNetParams : public CChainParams { vSeeds.clear(); // nodes with support for servicebits filtering should be at the top - vSeeds.emplace_back("testnet-seed.dashdot.io", true); + vSeeds.emplace_back("testnet-seed.dashdot.io"); // Just a static list of stable node(s), only supports x9 // Testnet Dash addresses start with 'y' base58Prefixes[PUBKEY_ADDRESS] = std::vector(1,140); diff --git a/src/chainparams.h b/src/chainparams.h index ac957a3868cd5..545912b56053f 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -14,12 +14,6 @@ #include #include -struct CDNSSeedData { - std::string host; - bool supportsServiceBitsFiltering; - CDNSSeedData(const std::string &strHost, bool supportsServiceBitsFilteringIn) : host(strHost), supportsServiceBitsFiltering(supportsServiceBitsFilteringIn) {} -}; - struct SeedSpec6 { uint8_t addr[16]; uint16_t port; @@ -78,7 +72,8 @@ class CChainParams bool AllowMultiplePorts() const { return fAllowMultiplePorts; } /** Return the BIP70 network string (main, test or regtest) */ std::string NetworkIDString() const { return strNetworkID; } - const std::vector& DNSSeeds() const { return vSeeds; } + /** Return the list of hostnames to look up for DNS seeds */ + const std::vector& DNSSeeds() const { return vSeeds; } const std::vector& Base58Prefix(Base58Type type) const { return base58Prefixes[type]; } int ExtCoinType() const { return nExtCoinType; } const std::vector& FixedSeeds() const { return vFixedSeeds; } @@ -103,7 +98,7 @@ class CChainParams CMessageHeader::MessageStartChars pchMessageStart; int nDefaultPort; uint64_t nPruneAfterHeight; - std::vector vSeeds; + std::vector vSeeds; std::vector base58Prefixes[MAX_BASE58_TYPES]; int nExtCoinType; std::string strNetworkID; diff --git a/src/net.cpp b/src/net.cpp index f710d2b002dc1..bf92220fd3561 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -148,7 +148,7 @@ static std::vector convertSeed6(const std::vector &vSeedsIn for (const auto& seed_in : vSeedsIn) { struct in6_addr ip; memcpy(&ip, seed_in.addr, sizeof(ip)); - CAddress addr(CService(ip, seed_in.port), NODE_NETWORK); + CAddress addr(CService(ip, seed_in.port), GetDesirableServiceFlags(NODE_NONE)); addr.nTime = GetTime() - GetRand(nOneWeek) - nOneWeek; vSeedsOut.push_back(addr); } @@ -1723,18 +1723,6 @@ void MapPort(bool) -static std::string GetDNSHost(const CDNSSeedData& data, ServiceFlags* requiredServiceBits) -{ - //use default host for non-filter-capable seeds or if we use the default service bits (NODE_NETWORK) - if (!data.supportsServiceBitsFiltering || *requiredServiceBits == NODE_NETWORK) { - *requiredServiceBits = NODE_NETWORK; - return data.host; - } - - return strprintf("x%x.%s", *requiredServiceBits, data.host); -} - - void CConnman::ThreadDNSAddressSeed() { // goal: only query DNS seeds if address need is acute @@ -1757,22 +1745,22 @@ void CConnman::ThreadDNSAddressSeed() } } - const std::vector &vSeeds = Params().DNSSeeds(); + const std::vector &vSeeds = Params().DNSSeeds(); int found = 0; LogPrintf("Loading addresses from DNS seeds (could take a while)\n"); - for (const CDNSSeedData &seed : vSeeds) { + for (const std::string &seed : vSeeds) { if (interruptNet) { return; } if (HaveNameProxy()) { - AddOneShot(seed.host); + AddOneShot(seed); } else { std::vector vIPs; std::vector vAdd; ServiceFlags requiredServiceBits = GetDesirableServiceFlags(NODE_NONE); - std::string host = GetDNSHost(seed, &requiredServiceBits); + std::string host = strprintf("x%x.%s", requiredServiceBits, seed); CNetAddr resolveSource; if (!resolveSource.SetInternal(host)) { continue; @@ -1788,6 +1776,10 @@ void CConnman::ThreadDNSAddressSeed() found++; } addrman.Add(vAdd, resolveSource); + } else { + // We now avoid directly using results from DNS Seeds which do not support service bit filtering, + // instead using them as a oneshot to get nodes with our desired service bits. + AddOneShot(seed); } } } diff --git a/src/protocol.h b/src/protocol.h index 4bf0cdf00a69f..fd97cad5bf74e 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -320,7 +320,15 @@ enum ServiceFlags : uint64_t { * 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. + * Thus, generally, avoid calling with peerServices == NODE_NONE, unless + * state-specific flags must absolutely be avoided. When called with + * peerServices == NODE_NONE, the returned desirable service flags are + * guaranteed to not change dependant on state - ie they are suitable for + * use when describing peers which we know to be desirable, but for which + * we do not have a confirmed set of service flags. + * + * If the NODE_NONE return value is changed, contrib/seeds/makeseeds.py + * should be updated appropriately to filter for the same nodes. */ static ServiceFlags GetDesirableServiceFlags(ServiceFlags services) { return ServiceFlags(NODE_NETWORK); From fccf28b6a0c44cd2d1ef7dea9f5f596d6d208625 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 29 Jan 2018 14:26:20 +0100 Subject: [PATCH 05/18] Merge #11577: Fix warnings (-Wsign-compare) when building with DEBUG_ADDRMAN 6eddd43 Fix warnings when building with DEBUG_ADDRMAN (practicalswift) Pull request description: Fix warnings when building with `DEBUG_ADDRMAN`. Warnings prior to this commit: ``` addrman.cpp:390:24: warning: comparison of integers of different signs: 'size_type' (aka 'unsigned long') and 'int' [-Wsign-compare] if (vRandom.size() != nTried + nNew) ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~ addrman.cpp:411:52: warning: comparison of integers of different signs: 'int' and 'size_type' (aka 'unsigned long') [-Wsign-compare] if (info.nRandomPos < 0 || info.nRandomPos >= vRandom.size() || vRandom[info.nRandomPos] != n) ~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~ addrman.cpp:419:25: warning: comparison of integers of different signs: 'size_type' (aka 'unsigned long') and 'int' [-Wsign-compare] if (setTried.size() != nTried) ~~~~~~~~~~~~~~~ ^ ~~~~~~ addrman.cpp:421:23: warning: comparison of integers of different signs: 'size_type' (aka 'unsigned long') and 'int' [-Wsign-compare] if (mapNew.size() != nNew) ~~~~~~~~~~~~~ ^ ~~~~ 4 warnings generated. ``` Tree-SHA512: 0316faecfe95066d2c9a0b6b3960086e43824f21a67086a895ea45fbce1327f8d6df5945fe923c2dbe4efce430bc1384d515d317c3930d97d24965e507cf734d --- src/addrman.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index b4d4526f77a8e..ef26f90d73ccf 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -404,7 +404,7 @@ int CAddrMan::Check_() std::set setTried; std::map mapNew; - if (vRandom.size() != nTried + nNew) + if (vRandom.size() != (size_t)(nTried + nNew)) return -7; for (std::map::iterator it = mapInfo.begin(); it != mapInfo.end(); it++) { @@ -425,7 +425,7 @@ int CAddrMan::Check_() } if (mapAddr[info] != n) return -5; - if (info.nRandomPos < 0 || info.nRandomPos >= vRandom.size() || vRandom[info.nRandomPos] != n) + if (info.nRandomPos < 0 || (size_t)info.nRandomPos >= vRandom.size() || vRandom[info.nRandomPos] != n) return -14; if (info.nLastTry < 0) return -6; @@ -433,9 +433,9 @@ int CAddrMan::Check_() return -8; } - if (setTried.size() != nTried) + if (setTried.size() != (size_t)nTried) return -9; - if (mapNew.size() != nNew) + if (mapNew.size() != (size_t)nNew) return -10; for (int n = 0; n < ADDRMAN_TRIED_BUCKET_COUNT; n++) { From f4fb2c1906e4deef2b12736b0ddac464017164eb Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 11 Dec 2017 16:44:02 +0100 Subject: [PATCH 06/18] Merge #11583: Do not make it trivial for inbound peers to generate log entries be9f38c Do not make it trivial for inbound peers to generate log entries (Matt Corallo) Pull request description: Based on #11580 because I'm lazy. We should generally avoid writing to debug.log unconditionally for inbound peers which misbehave (the peer being about to be banned being an exception, since they cannot do this twice). Tree-SHA512: 8e59c8d08d00b1527951b30f4842d010a4c2fc440503ade112baa2c1b9afd0e0d1c5c2df83dde25183a242af45089cf9b9f873b71796771232ffb6c5fc6cc0cc --- src/net.cpp | 2 +- src/net_processing.cpp | 31 ++++++++++++++++++------------- src/util.h | 4 ++++ 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index bf92220fd3561..f7a3f9d7bc3b9 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1199,7 +1199,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { if (IsBanned(addr) && !whitelisted) { - LogPrintf("%s (banned)\n", strDropped); + LogPrint(BCLog::NET, "%s (banned)\n", strDropped); CloseSocket(hSocket); return; } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 9fd39c4279c28..548f56e6046ec 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1167,7 +1167,7 @@ void static ProcessGetBlockData(CNode* pfrom, const Consensus::Params& consensus if (mi != mapBlockIndex.end()) { send = BlockRequestAllowed(mi->second, consensusParams); if (!send) { - LogPrintf("%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId()); + LogPrint(BCLog::NET,"%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId()); } } const CNetMsgMaker msgMaker(pfrom->GetSendVersion()); @@ -1832,7 +1832,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (nVersion < MIN_PEER_PROTO_VERSION) { // disconnect from peers older than this proto version - LogPrintf("peer=%d using obsolete version %i; disconnecting\n", pfrom->GetId(), nVersion); + LogPrint(BCLog::NET, "peer=%d using obsolete version %i; disconnecting\n", pfrom->GetId(), nVersion); connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_OBSOLETE, strprintf("Version must be %d or greater", MIN_PEER_PROTO_VERSION))); pfrom->fDisconnect = true; @@ -1943,7 +1943,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (fLogIPs) remoteAddr = ", peeraddr=" + pfrom->addr.ToString(); - LogPrintf("receive version message: %s: version %d, blocks=%d, us=%s, peer=%d%s\n", + LogPrint(BCLog::NET, "receive version message: %s: version %d, blocks=%d, us=%s, peer=%d%s\n", cleanSubVer, pfrom->nVersion, pfrom->nStartingHeight, addrMe.ToString(), pfrom->GetId(), remoteAddr); @@ -1978,6 +1978,9 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // Mark this node as currently connected, so we update its timestamp later. LOCK(cs_main); State(pfrom->GetId())->fCurrentlyConnected = true; + LogPrintf("New outbound peer connected: version: %d, blocks=%d, peer=%d%s\n", + pfrom->nVersion.load(), pfrom->nStartingHeight, pfrom->GetId(), + (fLogIPs ? strprintf(", peeraddr=%s", pfrom->addr.ToString()) : "")); } if (pfrom->nVersion >= LLMQS_PROTO_VERSION) { @@ -2316,7 +2319,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr BlockMap::iterator it = mapBlockIndex.find(req.blockhash); if (it == mapBlockIndex.end() || !(it->second->nStatus & BLOCK_HAVE_DATA)) { - LogPrintf("Peer %d sent us a getblocktxn for a block we don't have", pfrom->GetId()); + LogPrint(BCLog::NET, "Peer %d sent us a getblocktxn for a block we don't have", pfrom->GetId()); return true; } @@ -2367,7 +2370,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr pindex = (*mi).second; if (!BlockRequestAllowed(pindex, chainparams.GetConsensus())) { - LogPrintf("%s: ignoring request from peer=%i for old block header that isn't in the main chain\n", __func__, pfrom->GetId()); + LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block header that isn't in the main chain\n", __func__, pfrom->GetId()); return true; } } @@ -2625,10 +2628,12 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr int nDoS; if (state.IsInvalid(nDoS)) { if (nDoS > 0) { + LogPrintf("Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId()); LOCK(cs_main); Misbehaving(pfrom->GetId(), nDoS); + } else { + LogPrint(BCLog::NET, "Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId()); } - LogPrintf("Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId()); return true; } } @@ -3270,7 +3275,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic& inter msg.SetVersion(pfrom->GetRecvVersion()); // Scan for message start if (memcmp(msg.hdr.pchMessageStart, chainparams.MessageStart(), CMessageHeader::MESSAGE_START_SIZE) != 0) { - LogPrintf("PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", SanitizeString(msg.hdr.GetCommand()), pfrom->GetId()); + LogPrint(BCLog::NET, "PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", SanitizeString(msg.hdr.GetCommand()), pfrom->GetId()); pfrom->fDisconnect = true; return false; } @@ -3279,7 +3284,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic& inter CMessageHeader& hdr = msg.hdr; if (!hdr.IsValid(chainparams.MessageStart())) { - LogPrintf("PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", SanitizeString(hdr.GetCommand()), pfrom->GetId()); + LogPrint(BCLog::NET, "PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", SanitizeString(hdr.GetCommand()), pfrom->GetId()); return fMoreWork; } std::string strCommand = hdr.GetCommand(); @@ -3292,7 +3297,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic& inter const uint256& hash = msg.GetMessageHash(); if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0) { - LogPrintf("%s(%s, %u bytes): CHECKSUM ERROR expected %s was %s\n", __func__, + LogPrint(BCLog::NET, "%s(%s, %u bytes): CHECKSUM ERROR expected %s was %s\n", __func__, SanitizeString(strCommand), nMessageSize, HexStr(hash.begin(), hash.begin()+CMessageHeader::CHECKSUM_SIZE), HexStr(hdr.pchChecksum, hdr.pchChecksum+CMessageHeader::CHECKSUM_SIZE)); @@ -3315,17 +3320,17 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic& inter if (strstr(e.what(), "end of data")) { // Allow exceptions from under-length message on vRecv - LogPrintf("%s(%s, %u bytes): Exception '%s' caught, normally caused by a message being shorter than its stated length\n", __func__, SanitizeString(strCommand), nMessageSize, e.what()); + LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught, normally caused by a message being shorter than its stated length\n", __func__, SanitizeString(strCommand), nMessageSize, e.what()); } else if (strstr(e.what(), "size too large")) { // Allow exceptions from over-long size - LogPrintf("%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what()); + LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what()); } else if (strstr(e.what(), "non-canonical ReadCompactSize()")) { // Allow exceptions from non-canonical encoding - LogPrintf("%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what()); + LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught\n", __func__, SanitizeString(strCommand), nMessageSize, e.what()); } else { @@ -3336,7 +3341,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic& inter } if (!fRet) { - LogPrintf("%s(%s, %u bytes) FAILED peer=%d\n", __func__, SanitizeString(strCommand), nMessageSize, pfrom->GetId()); + LogPrint(BCLog::NET, "%s(%s, %u bytes) FAILED peer=%d\n", __func__, SanitizeString(strCommand), nMessageSize, pfrom->GetId()); } LOCK(cs_main); diff --git a/src/util.h b/src/util.h index 9ff425d32005a..fe89a9ead1fac 100644 --- a/src/util.h +++ b/src/util.h @@ -184,6 +184,10 @@ template static inline void MarkUsed(const T& t, c MarkUsed(args...); } +// Be conservative when using LogPrintf/error or other things which +// unconditionally log to debug.log! It should not be the case that an inbound +// peer can fill up a users disk with debug.log entries. + #ifdef USE_COVERAGE #define LogPrintf(...) do { MarkUsed(__VA_ARGS__); } while(0) #define LogPrint(category, ...) do { MarkUsed(__VA_ARGS__); } while(0) From 500a60a8fe26db98f3ab16de225650099e1859a5 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 30 Nov 2017 11:32:51 +0100 Subject: [PATCH 07/18] Merge #11744: net: Add missing locks in net.{cpp,h} bfb0c0a Add Clang thread safety analysis annotations (practicalswift) 63f21d2 net: Add missing locks in net.{cpp,h} (practicalswift) Pull request description: Add missing locks in `net.{cpp,h}`: * writing variable `nTotalBytesRecv` requires holding mutex `cs_totalBytesRecv` exclusively * writing variables `nTotalBytesSent`, `nMaxOutboundTotalBytesSentInCycle` and `nMaxOutboundCycleStartTime` require holding mutex `cs_totalBytesSent` exclusively * writing variables `nMaxOutboundTimeframe` and `nMaxOutboundLimit` require holding mutex `cs_totalBytesSent` exclusively * writing variable `vAddedNodes` requires holding mutex `cs_vAddedNodes` exclusively Tree-SHA512: 54a5b4bc6dc6f404dacf403af2ddd7b2214cc0a17d1d32a282def1c6b536105dada56bfabbc8606f56755f2d24874abba09913b51c8d13b0f2b000149551f0b0 --- src/net.cpp | 14 ++++++++++---- src/net.h | 26 ++++++++++++++++---------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index f7a3f9d7bc3b9..8ac6ef37c50c2 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2504,10 +2504,16 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) { Init(connOptions); - nTotalBytesRecv = 0; - nTotalBytesSent = 0; - nMaxOutboundTotalBytesSentInCycle = 0; - nMaxOutboundCycleStartTime = 0; + { + LOCK(cs_totalBytesRecv); + nTotalBytesRecv = 0; + } + { + LOCK(cs_totalBytesSent); + nTotalBytesSent = 0; + nMaxOutboundTotalBytesSentInCycle = 0; + nMaxOutboundCycleStartTime = 0; + } if (fListen && !InitBinds(connOptions.vBinds, connOptions.vWhiteBinds)) { if (clientInterface) { diff --git a/src/net.h b/src/net.h index 35dd40738b36e..57c405d2dce05 100644 --- a/src/net.h +++ b/src/net.h @@ -185,10 +185,16 @@ class CConnman m_msgproc = connOptions.m_msgproc; nSendBufferMaxSize = connOptions.nSendBufferMaxSize; nReceiveFloodSize = connOptions.nReceiveFloodSize; - nMaxOutboundTimeframe = connOptions.nMaxOutboundTimeframe; - nMaxOutboundLimit = connOptions.nMaxOutboundLimit; + { + LOCK(cs_totalBytesSent); + nMaxOutboundTimeframe = connOptions.nMaxOutboundTimeframe; + nMaxOutboundLimit = connOptions.nMaxOutboundLimit; + } vWhitelistedRange = connOptions.vWhitelistedRange; - vAddedNodes = connOptions.m_added_nodes; + { + LOCK(cs_vAddedNodes); + vAddedNodes = connOptions.m_added_nodes; + } } CConnman(uint64_t seed0, uint64_t seed1); @@ -510,14 +516,14 @@ class CConnman // Network usage totals CCriticalSection cs_totalBytesRecv; CCriticalSection cs_totalBytesSent; - uint64_t nTotalBytesRecv; - uint64_t nTotalBytesSent; + uint64_t nTotalBytesRecv GUARDED_BY(cs_totalBytesRecv); + uint64_t nTotalBytesSent GUARDED_BY(cs_totalBytesSent); // outbound limit & stats - uint64_t nMaxOutboundTotalBytesSentInCycle; - uint64_t nMaxOutboundCycleStartTime; - uint64_t nMaxOutboundLimit; - uint64_t nMaxOutboundTimeframe; + uint64_t nMaxOutboundTotalBytesSentInCycle GUARDED_BY(cs_totalBytesSent); + uint64_t nMaxOutboundCycleStartTime GUARDED_BY(cs_totalBytesSent); + uint64_t nMaxOutboundLimit GUARDED_BY(cs_totalBytesSent); + uint64_t nMaxOutboundTimeframe GUARDED_BY(cs_totalBytesSent); // Whitelisted ranges. Any node connecting from these is automatically // whitelisted (as well as those connecting to whitelisted binds). @@ -535,7 +541,7 @@ class CConnman CAddrMan addrman; std::deque vOneShots; CCriticalSection cs_vOneShots; - std::vector vAddedNodes; + std::vector vAddedNodes GUARDED_BY(cs_vAddedNodes); CCriticalSection cs_vAddedNodes; std::vector vPendingMasternodes; std::map, std::set> masternodeQuorumNodes; // protected by cs_vPendingMasternodes From 49549390af740804a4143a06d141cd932037d44b Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 6 Feb 2018 12:34:17 +0100 Subject: [PATCH 08/18] Merge #12218: net: Move misbehaving logging to net logging category d3a185a net: Move misbehaving logging to net logging category (Wladimir J. van der Laan) Pull request description: This moves the error messages for misbehavior (when available) into the line that reports the misbehavior, as well as moves the logging to the `net` category. This is a continuation of #11583 and avoids serious-looking errors due to misbehaving peers. As it is impossible to correlate the `peer=X` numbers to specific incoming connections now without enabling the `net` category, it doesn't really help to see these messages by default. To do this, Misbehaving() gains an optional `message` argument. E.g. change: 2018-01-18 16:02:27 Misbehaving: x.x.x.x:62174 peer=164603 (80 -> 100) BAN THRESHOLD EXCEEDED 2018-01-18 16:02:27 ERROR: non-continuous headers sequence to 2018-01-18 16:02:27 Misbehaving: x.x.x.x:62174 peer=164603 (80 -> 100) BAN THRESHOLD EXCEEDED: non-continuous headers sequence When there is a category for "important" net messages (see #12219 ), we should move it there. Tree-SHA512: 51c97e9a649bf5409f2fd4625fa1243a036e9c9de6037bb064244207408c2e0eb025e3af80866df673cdc006b8f35dc4078d074033f0d4c6a73bbb03949a269f --- src/net_processing.cpp | 45 +++++++++++++++++++++--------------------- src/net_processing.h | 2 +- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 548f56e6046ec..827ffce6bc819 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -776,7 +776,7 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphansSize) void static ProcessOrphanTx(CConnman* connman, std::set& orphan_work_set) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans); // Requires cs_main. -void Misbehaving(NodeId pnode, int howmuch) +void Misbehaving(NodeId pnode, int howmuch, const std::string& message) { if (howmuch == 0) return; @@ -787,12 +787,13 @@ void Misbehaving(NodeId pnode, int howmuch) state->nMisbehavior += howmuch; int banscore = gArgs.GetArg("-banscore", DEFAULT_BANSCORE_THRESHOLD); + std::string message_prefixed = message.empty() ? "" : (": " + message); if (state->nMisbehavior >= banscore && state->nMisbehavior - howmuch < banscore) { - LogPrintf("%s: %s peer=%d (%d -> %d) BAN THRESHOLD EXCEEDED\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior); + LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d) BAN THRESHOLD EXCEEDED%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed); state->fShouldBan = true; } else - LogPrintf("%s: %s peer=%d (%d -> %d)\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior); + LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d)%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed); } // Requires cs_main. @@ -1448,8 +1449,7 @@ inline void static SendBlockTransactions(const CBlock& block, const BlockTransac for (size_t i = 0; i < req.indexes.size(); i++) { if (req.indexes[i] >= block.vtx.size()) { LOCK(cs_main); - Misbehaving(pfrom->GetId(), 100); - LogPrintf("Peer %d sent us a getblocktxn with out-of-bounds tx indices", pfrom->GetId()); + Misbehaving(pfrom->GetId(), 100, strprintf("Peer %d sent us a getblocktxn with out-of-bounds tx indices", pfrom->GetId())); return; } resp.txn[i] = block.vtx[req.indexes[i]]; @@ -1505,8 +1505,8 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::ve uint256 hashLastBlock; for (const CBlockHeader& header : headers) { if (!hashLastBlock.IsNull() && header.hashPrevBlock != hashLastBlock) { - Misbehaving(pfrom->GetId(), 20); - return error("non-continuous headers sequence"); + Misbehaving(pfrom->GetId(), 20, "non-continuous headers sequence"); + return false; } hashLastBlock = header.GetHash(); } @@ -1525,7 +1525,9 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::ve if (state.IsInvalid(nDoS)) { LOCK(cs_main); if (nDoS > 0) { - Misbehaving(pfrom->GetId(), nDoS); + Misbehaving(pfrom->GetId(), nDoS, "invalid header received"); + } else { + LogPrint(BCLog::NET, "peer=%d: invalid header received\n", pfrom->GetId()); } if (punish_duplicate_invalid && mapBlockIndex.find(first_invalid_header.GetHash()) != mapBlockIndex.end()) { // Goal: don't allow outbound peers to use up our outbound @@ -1561,7 +1563,7 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::ve // etc), and not just the duplicate-invalid case. pfrom->fDisconnect = true; } - return error("invalid header received"); + return false; } } @@ -2048,8 +2050,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (vAddr.size() > 1000) { LOCK(cs_main); - Misbehaving(pfrom->GetId(), 20); - return error("message addr size() = %u", vAddr.size()); + Misbehaving(pfrom->GetId(), 20, strprintf("message addr size() = %u", vAddr.size())); + return false; } // Store the new addresses @@ -2127,8 +2129,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (vInv.size() > MAX_INV_SZ) { LOCK(cs_main); - Misbehaving(pfrom->GetId(), 20); - return error("message inv size() = %u", vInv.size()); + Misbehaving(pfrom->GetId(), 20, strprintf("message inv size() = %u", vInv.size())); + return false; } bool fBlocksOnly = !fRelayTxes; @@ -2224,8 +2226,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (vInv.size() > MAX_INV_SZ) { LOCK(cs_main); - Misbehaving(pfrom->GetId(), 20); - return error("message getdata size() = %u", vInv.size()); + Misbehaving(pfrom->GetId(), 20, strprintf("message getdata size() = %u", vInv.size())); + return false; } LogPrint(BCLog::NET, "received getdata (%u invsz) peer=%d\n", vInv.size(), pfrom->GetId()); @@ -2628,9 +2630,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr int nDoS; if (state.IsInvalid(nDoS)) { if (nDoS > 0) { - LogPrintf("Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId()); LOCK(cs_main); - Misbehaving(pfrom->GetId(), nDoS); + Misbehaving(pfrom->GetId(), nDoS, strprintf("Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId())); } else { LogPrint(BCLog::NET, "Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId()); } @@ -2710,8 +2711,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr ReadStatus status = partialBlock.InitData(cmpctblock, vExtraTxnForCompact); if (status == READ_STATUS_INVALID) { MarkBlockAsReceived(pindex->GetBlockHash()); // Reset in-flight state in case of whitelist - Misbehaving(pfrom->GetId(), 100); - LogPrintf("Peer %d sent us invalid compact block\n", pfrom->GetId()); + Misbehaving(pfrom->GetId(), 100, strprintf("Peer %d sent us invalid compact block\n", pfrom->GetId())); return true; } else if (status == READ_STATUS_FAILED) { // Duplicate txindexes, the block is now in-flight, so just request it @@ -2838,8 +2838,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr ReadStatus status = partialBlock.FillBlock(*pblock, resp.txn); if (status == READ_STATUS_INVALID) { MarkBlockAsReceived(resp.blockhash); // Reset in-flight state in case of whitelist - Misbehaving(pfrom->GetId(), 100); - LogPrintf("Peer %d sent us invalid compact block/non-matching block transactions\n", pfrom->GetId()); + Misbehaving(pfrom->GetId(), 100, strprintf("Peer %d sent us invalid compact block/non-matching block transactions\n", pfrom->GetId())); return true; } else if (status == READ_STATUS_FAILED) { // Might have collided, fall back to getdata now :( @@ -2901,8 +2900,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr unsigned int nCount = ReadCompactSize(vRecv); if (nCount > MAX_HEADERS_RESULTS) { LOCK(cs_main); - Misbehaving(pfrom->GetId(), 20); - return error("headers message size = %u", nCount); + Misbehaving(pfrom->GetId(), 20, strprintf("headers message size = %u", nCount)); + return false; } headers.resize(nCount); for (unsigned int n = 0; n < nCount; n++) { diff --git a/src/net_processing.h b/src/net_processing.h index ead114e9d9dff..02583baab7435 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -81,7 +81,7 @@ struct CNodeStateStats { /** Get statistics from node state */ bool GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats); /** Increase a node's misbehavior score. */ -void Misbehaving(NodeId nodeid, int howmuch); +void Misbehaving(NodeId nodeid, int howmuch, const std::string& message=""); bool IsBanned(NodeId nodeid); #endif // BITCOIN_NET_PROCESSING_H From 211c8a3a5b33ca8c8a72ca80a321758f701c0c2f Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 1 Feb 2018 19:21:05 +0100 Subject: [PATCH 09/18] Merge #12326: net: initialize socket to avoid closing random fd's 96dbd38 net: initialize socket to avoid closing random fd's (Cory Fields) Pull request description: An excellent spot by @david60. Even if it isn't causing the fd issue we're looking for, this should be fixed. Tree-SHA512: 062a8f2cdd39d895213e1263dbd7b8391473ddaea2f93c82c211a9bb6ea6744d48a6c84c8ff804b16b865d14145492635c500a9fd138d0988fee5e4f719ebb91 --- src/net.cpp | 2 +- src/netbase.cpp | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/net.cpp b/src/net.cpp index 8ac6ef37c50c2..bcf459a836906 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -429,7 +429,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo // Connect bool connected = false; - SOCKET hSocket; + SOCKET hSocket = INVALID_SOCKET; proxyType proxy; if (addrConnect.IsValid()) { bool proxyConnectionFailed = false; diff --git a/src/netbase.cpp b/src/netbase.cpp index c1db993561d81..948f8f333bf13 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -686,6 +686,9 @@ bool CloseSocket(SOCKET& hSocket) #else int ret = close(hSocket); #endif + if (ret) { + LogPrintf("Socket close failed: %d. Error: %s\n", hSocket, NetworkErrorString(WSAGetLastError())); + } hSocket = INVALID_SOCKET; return ret != SOCKET_ERROR; } From 03576217e5cd3ff9e6d704020d687d12374590e2 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 2 Feb 2018 09:50:12 +0100 Subject: [PATCH 10/18] Merge #12329: net: don't retry failed oneshot connections forever 660f5f1 net: don't retry failed oneshot connections forever (Cory Fields) Pull request description: As introduced by (my suggestion, sorry, in) #11512, failed dns resolves end up as oneshots. But failed oneshots are re-added as oneshots, so we need to make sure that we're not queuing these up forever after failed resolves. Rather than trying to differentiate, I think we should just not re-add failed oneshots and be done with it. Maybe @sipa can shed a light on what the original intention was. Tree-SHA512: 2dfe35dabfb6354c315cf6f8ae42971765d36575e685662caae7ed8f9dea9472c6fb1fd5e62ec35301550b74b6613a54265e90fca2a6618544f78dacaac4d4fd fix 12329 backport Signed-off-by: Pasta fix 12329 backport Signed-off-by: Pasta fix 12329 backport Signed-off-by: Pasta --- src/net.cpp | 25 +++++++++++-------------- src/net.h | 4 ++-- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index bcf459a836906..a186fc0b1bd61 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1828,8 +1828,7 @@ void CConnman::ProcessOneShot() CAddress addr; CSemaphoreGrant grant(*semOutbound, true); if (grant) { - if (!OpenNetworkConnection(addr, false, &grant, strDest.c_str(), true)) - AddOneShot(strDest); + OpenNetworkConnection(addr, false, &grant, strDest.c_str(), true); } } @@ -2199,36 +2198,36 @@ void CConnman::ThreadOpenMasternodeConnections() } // if successful, this moves the passed grant to the constructed node -bool CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound, const char *pszDest, bool fOneShot, bool fFeeler, bool manual_connection, bool fConnectToMasternode) +void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound, const char *pszDest, bool fOneShot, bool fFeeler, bool manual_connection, bool fConnectToMasternode) { // // Initiate outbound network connection // if (interruptNet) { - return false; + return; } if (!fNetworkActive) { - return false; + return; } if (!pszDest) { // banned or exact match? if (IsBanned(addrConnect) || FindNode(addrConnect.ToStringIPPort())) - return false; + return; // local and not a connection to itself? bool fAllowLocal = Params().AllowMultiplePorts() && addrConnect.GetPort() != GetListenPort(); if (!fAllowLocal && IsLocal(addrConnect)) - return false; + return; // if multiple ports for same IP are allowed, search for IP:PORT match, otherwise search for IP-only match if ((!Params().AllowMultiplePorts() && FindNode((CNetAddr)addrConnect)) || (Params().AllowMultiplePorts() && FindNode((CService)addrConnect))) - return false; + return; } else if (FindNode(std::string(pszDest))) - return false; + return; CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure); if (!pnode) - return false; + return; if (grantOutbound) grantOutbound->MoveTo(pnode->grantOutbound); if (fOneShot) @@ -2245,12 +2244,10 @@ bool CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai LOCK(cs_vNodes); vNodes.push_back(pnode); } - - return true; } -bool CConnman::OpenMasternodeConnection(const CAddress &addrConnect) { - return OpenNetworkConnection(addrConnect, false, nullptr, nullptr, false, false, false, true); +void CConnman::OpenMasternodeConnection(const CAddress &addrConnect) { + OpenNetworkConnection(addrConnect, false, nullptr, nullptr, false, false, false, true); } void CConnman::ThreadMessageHandler() diff --git a/src/net.h b/src/net.h index 57c405d2dce05..0d829fabfa7a6 100644 --- a/src/net.h +++ b/src/net.h @@ -204,8 +204,8 @@ class CConnman void Interrupt(); bool GetNetworkActive() const { return fNetworkActive; }; void SetNetworkActive(bool active); - bool OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound = nullptr, const char *strDest = nullptr, bool fOneShot = false, bool fFeeler = false, bool manual_connection = false, bool fConnectToMasternode = false); - bool OpenMasternodeConnection(const CAddress& addrConnect); + void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound = nullptr, const char *strDest = nullptr, bool fOneShot = false, bool fFeeler = false, bool manual_connection = false, bool fConnectToMasternode = false); + void OpenMasternodeConnection(const CAddress& addrConnect); bool CheckIncomingNonce(uint64_t nonce); struct CFullyConnectedOnly { From 12a98098249a95f017a5160b8175acdf44bad9ec Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 6 Feb 2018 12:19:15 +0100 Subject: [PATCH 11/18] Merge #12342: Extend #11583 to include "version handshake timeout" message c887f87 Extend #11583 to include the most common message generated by non-contributing peers (port scanners?) 37% of the log default log entries for a node that has been up for ~24hrs was "version handshake timeout..." (Clem Taylor) Pull request description: 37% of the default log entries for a node that has been up for ~24hrs was "version handshake timeout..." Tree-SHA512: dceeee5d55a9ff7570174aeb63faac9beda239087220522adefef7ed11e0eeffa008ca28726011247c8834c1a222d37817baf895635ab874a95ebc435959070e --- src/net.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net.cpp b/src/net.cpp index a186fc0b1bd61..079742d34ae49 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1562,7 +1562,7 @@ void CConnman::ThreadSocketHandler() } else if (!pnode->fSuccessfullyConnected) { - LogPrintf("version handshake timeout from %d\n", pnode->GetId()); + LogPrint(BCLog::NET, "version handshake timeout from %d\n", pnode->GetId()); pnode->fDisconnect = true; } } From a3ea0e93ef3ef497025fcb27b781245c7daef5b3 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 7 Mar 2018 17:40:32 +0100 Subject: [PATCH 12/18] Merge #12626: Limit the number of IPs addrman learns from each DNS seeder 46e7f800b Limit the number of IPs we use from each DNS seeder (e0) Pull request description: A risk exists where a malicious DNS seeder eclipses a node by returning an enormous number of IP addresses. In this commit we mitigate this risk by limiting the number of IP addresses addrman learns to 256 per DNS seeder. As discussed with @theuni Tree-SHA512: 949e870765b1470200f2c650341d9e3308a973a7d1a6e557b944b0a2b8ccda49226fc8c4ff7d2a05e5854c4014ec0b67e37a3f2287556fe7dfa2048ede1f2e6f --- src/net.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/net.cpp b/src/net.cpp index 079742d34ae49..4db146ee65d9e 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1765,7 +1765,8 @@ void CConnman::ThreadDNSAddressSeed() if (!resolveSource.SetInternal(host)) { continue; } - if (LookupHost(host.c_str(), vIPs, 0, true)) + unsigned int nMaxIPs = 256; // Limits number of IPs learned from a DNS seed + if (LookupHost(host.c_str(), vIPs, nMaxIPs, true)) { for (const CNetAddr& ip : vIPs) { From e7beff5bcd8b72f24ed31078060cce82c49a0c5d Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 19 Apr 2018 14:24:26 +0200 Subject: [PATCH 13/18] Merge #12855: net: Minor accumulated cleanups 2c084a6 net: Minor accumulated cleanups (Thomas Snider) Pull request description: From now-derelict larger changes I had been working on, here are a series of DRY refactors/cleanups. Net loss of 35 lines of code - a small step in the good fight. In particular I think operator!= should only ever be implemented as a negation of operator==. Lower chance for errors, and removes the possibility of divergent behavior. Tree-SHA512: 58bf4b542a4e8e5bc465b508aaa16e9ab51448c3f9bee52cd9db0a64a5c6c5a13e4b4286d0a5aa864934fc58064799f6a88a40a87154fd3a4bd731a72e254393 --- src/compat.h | 6 ++++++ src/dashd.cpp | 5 +---- src/net.cpp | 27 ++++++++++--------------- src/netaddress.cpp | 50 +++++++++++----------------------------------- src/netaddress.h | 10 +++++----- src/netbase.cpp | 6 +----- 6 files changed, 36 insertions(+), 68 deletions(-) diff --git a/src/compat.h b/src/compat.h index b73c7478a4158..a3b647294aae8 100644 --- a/src/compat.h +++ b/src/compat.h @@ -86,6 +86,12 @@ typedef unsigned int SOCKET; size_t strnlen( const char *start, size_t max_len); #endif // HAVE_DECL_STRNLEN +#ifndef WIN32 +typedef void* sockopt_arg_type; +#else +typedef char* sockopt_arg_type; +#endif + bool static inline IsSelectableSocket(const SOCKET& s) { #ifdef WIN32 return true; diff --git a/src/dashd.cpp b/src/dashd.cpp index 8db07c1ddb186..e2cc8decc1f5d 100644 --- a/src/dashd.cpp +++ b/src/dashd.cpp @@ -44,12 +44,9 @@ void WaitForShutdown(boost::thread_group* threadGroup) { - bool fShutdown = ShutdownRequested(); - // Tell the main threads to shutdown. - while (!fShutdown) + while (!ShutdownRequested()) { MilliSleep(200); - fShutdown = ShutdownRequested(); } if (threadGroup) { diff --git a/src/net.cpp b/src/net.cpp index 4db146ee65d9e..cdb5a46abbcca 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2067,23 +2067,25 @@ std::vector CConnman::GetAddedNodeInfo() for (const std::string& strAddNode : lAddresses) { CService service(LookupNumeric(strAddNode.c_str(), Params().GetDefaultPort())); + AddedNodeInfo addedNode{strAddNode, CService(), false, false}; 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}); + addedNode.resolvedAddress = service; + addedNode.fConnected = true; + addedNode.fInbound = it->second; } } 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}); + addedNode.resolvedAddress = it->second.second; + addedNode.fConnected = true; + addedNode.fInbound = it->second.first; } } + ret.emplace_back(std::move(addedNode)); } return ret; @@ -2316,23 +2318,16 @@ bool CConnman::BindListenPort(const CService &addrBind, std::string& strError, b LogPrintf("%s\n", strError); return false; } -#ifndef WIN32 + // Allow binding if the port is still in TIME_WAIT state after // the program was closed and restarted. - setsockopt(hListenSocket, SOL_SOCKET, SO_REUSEADDR, (void*)&nOne, sizeof(int)); -#else - setsockopt(hListenSocket, SOL_SOCKET, SO_REUSEADDR, (const char*)&nOne, sizeof(int)); -#endif + setsockopt(hListenSocket, SOL_SOCKET, SO_REUSEADDR, (sockopt_arg_type)&nOne, sizeof(int)); // some systems don't have IPV6_V6ONLY but are always v6only; others do have the option // and enable it by default or not. Try to enable it, if possible. if (addrBind.IsIPv6()) { #ifdef IPV6_V6ONLY -#ifdef WIN32 - setsockopt(hListenSocket, IPPROTO_IPV6, IPV6_V6ONLY, (const char*)&nOne, sizeof(int)); -#else - setsockopt(hListenSocket, IPPROTO_IPV6, IPV6_V6ONLY, (void*)&nOne, sizeof(int)); -#endif + setsockopt(hListenSocket, IPPROTO_IPV6, IPV6_V6ONLY, (sockopt_arg_type)&nOne, sizeof(int)); #endif #ifdef WIN32 int nProtLevel = PROTECTION_LEVEL_UNRESTRICTED; diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 2429cea352032..b4c5a1802d882 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -21,7 +21,7 @@ static const unsigned char g_internal_prefix[] = { 0xFD, 0x6B, 0x88, 0xC0, 0x87, bool fAllowPrivateNet = DEFAULT_ALLOWPRIVATENET; -void CNetAddr::Init() +CNetAddr::CNetAddr() { memset(ip, 0, sizeof(ip)); scopeId = 0; @@ -74,11 +74,6 @@ bool CNetAddr::SetSpecial(const std::string &strName) return false; } -CNetAddr::CNetAddr() -{ - Init(); -} - CNetAddr::CNetAddr(const struct in_addr& ipv4Addr) { SetRaw(NET_IPV4, (const uint8_t*)&ipv4Addr); @@ -304,11 +299,6 @@ bool operator==(const CNetAddr& a, const CNetAddr& b) return (memcmp(a.ip, b.ip, 16) == 0); } -bool operator!=(const CNetAddr& a, const CNetAddr& b) -{ - return (memcmp(a.ip, b.ip, 16) != 0); -} - bool operator<(const CNetAddr& a, const CNetAddr& b) { return (memcmp(a.ip, b.ip, 16) < 0); @@ -483,14 +473,8 @@ int CNetAddr::GetReachabilityFrom(const CNetAddr *paddrPartner) const } } -void CService::Init() +CService::CService() : port(0) { - port = 0; -} - -CService::CService() -{ - Init(); } CService::CService(const CNetAddr& cip, unsigned short portIn) : CNetAddr(cip), port(portIn) @@ -539,11 +523,6 @@ bool operator==(const CService& a, const CService& b) return (CNetAddr)a == (CNetAddr)b && a.port == b.port; } -bool operator!=(const CService& a, const CService& b) -{ - return (CNetAddr)a != (CNetAddr)b || a.port != b.port; -} - bool operator<(const CService& a, const CService& b) { return (CNetAddr)a < (CNetAddr)b || ((CNetAddr)a == (CNetAddr)b && a.port < b.port); @@ -682,16 +661,16 @@ bool CSubNet::Match(const CNetAddr &addr) const static inline int NetmaskBits(uint8_t x) { switch(x) { - case 0x00: return 0; break; - case 0x80: return 1; break; - case 0xc0: return 2; break; - case 0xe0: return 3; break; - case 0xf0: return 4; break; - case 0xf8: return 5; break; - case 0xfc: return 6; break; - case 0xfe: return 7; break; - case 0xff: return 8; break; - default: return -1; break; + case 0x00: return 0; + case 0x80: return 1; + case 0xc0: return 2; + case 0xe0: return 3; + case 0xf0: return 4; + case 0xf8: return 5; + case 0xfc: return 6; + case 0xfe: return 7; + case 0xff: return 8; + default: return -1; } } @@ -743,11 +722,6 @@ bool operator==(const CSubNet& a, const CSubNet& b) return a.valid == b.valid && a.network == b.network && !memcmp(a.netmask, b.netmask, 16); } -bool operator!=(const CSubNet& a, const CSubNet& b) -{ - return !(a==b); -} - bool operator<(const CSubNet& a, const CSubNet& b) { return (a.network < b.network || (a.network == b.network && memcmp(a.netmask, b.netmask, 16) < 0)); diff --git a/src/netaddress.h b/src/netaddress.h index 0e2f6e6b88e87..0a4af5e4084b9 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -39,15 +39,16 @@ class CNetAddr public: CNetAddr(); explicit CNetAddr(const struct in_addr& ipv4Addr); - void Init(); void SetIP(const CNetAddr& ip); + private: /** * Set raw IPv4 or IPv6 address (in network byte order) * @note Only NET_IPV4 and NET_IPV6 are allowed for network. */ void SetRaw(Network network, const uint8_t *data); + public: /** * Transform an arbitrary string into a non-routable ipv6 address. * Useful for mapping resolved addresses back to their source. @@ -88,7 +89,7 @@ class CNetAddr bool GetIn6Addr(struct in6_addr* pipv6Addr) const; friend bool operator==(const CNetAddr& a, const CNetAddr& b); - friend bool operator!=(const CNetAddr& a, const CNetAddr& b); + friend bool operator!=(const CNetAddr& a, const CNetAddr& b) { return !(a == b); } friend bool operator<(const CNetAddr& a, const CNetAddr& b); ADD_SERIALIZE_METHODS; @@ -125,7 +126,7 @@ class CSubNet bool IsValid() const; friend bool operator==(const CSubNet& a, const CSubNet& b); - friend bool operator!=(const CSubNet& a, const CSubNet& b); + friend bool operator!=(const CSubNet& a, const CSubNet& b) { return !(a == b); } friend bool operator<(const CSubNet& a, const CSubNet& b); ADD_SERIALIZE_METHODS; @@ -149,13 +150,12 @@ class CService : public CNetAddr CService(const CNetAddr& ip, unsigned short port); CService(const struct in_addr& ipv4Addr, unsigned short port); explicit CService(const struct sockaddr_in& addr); - void Init(); void SetPort(unsigned short portIn); unsigned short GetPort() const; bool GetSockAddr(struct sockaddr* paddr, socklen_t *addrlen) const; bool SetSockAddr(const struct sockaddr* paddr); friend bool operator==(const CService& a, const CService& b); - friend bool operator!=(const CService& a, const CService& b); + friend bool operator!=(const CService& a, const CService& b) { return !(a == b); } friend bool operator<(const CService& a, const CService& b); std::vector GetKey() const; std::string ToString(bool fUseGetnameinfo = true) const; diff --git a/src/netbase.cpp b/src/netbase.cpp index 948f8f333bf13..8ecae47f185c8 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -506,11 +506,7 @@ bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocket, i return false; } socklen_t nRetSize = sizeof(nRet); -#ifdef WIN32 - if (getsockopt(hSocket, SOL_SOCKET, SO_ERROR, (char*)(&nRet), &nRetSize) == SOCKET_ERROR) -#else - if (getsockopt(hSocket, SOL_SOCKET, SO_ERROR, &nRet, &nRetSize) == SOCKET_ERROR) -#endif + if (getsockopt(hSocket, SOL_SOCKET, SO_ERROR, (sockopt_arg_type)&nRet, &nRetSize) == SOCKET_ERROR) { LogPrintf("getsockopt() for %s failed: %s\n", addrConnect.ToString(), NetworkErrorString(WSAGetLastError())); return false; From 0255027fb698739db3c9bdbe23d8d314b75898c6 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 28 Jan 2020 13:01:46 +0300 Subject: [PATCH 14/18] Fix "\n"s --- src/net_processing.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 827ffce6bc819..b3391d3459847 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2321,7 +2321,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr BlockMap::iterator it = mapBlockIndex.find(req.blockhash); if (it == mapBlockIndex.end() || !(it->second->nStatus & BLOCK_HAVE_DATA)) { - LogPrint(BCLog::NET, "Peer %d sent us a getblocktxn for a block we don't have", pfrom->GetId()); + LogPrint(BCLog::NET, "Peer %d sent us a getblocktxn for a block we don't have\n", pfrom->GetId()); return true; } @@ -2631,7 +2631,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (state.IsInvalid(nDoS)) { if (nDoS > 0) { LOCK(cs_main); - Misbehaving(pfrom->GetId(), nDoS, strprintf("Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId())); + Misbehaving(pfrom->GetId(), nDoS, strprintf("Peer %d sent us invalid header via cmpctblock", pfrom->GetId())); } else { LogPrint(BCLog::NET, "Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId()); } @@ -2711,7 +2711,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr ReadStatus status = partialBlock.InitData(cmpctblock, vExtraTxnForCompact); if (status == READ_STATUS_INVALID) { MarkBlockAsReceived(pindex->GetBlockHash()); // Reset in-flight state in case of whitelist - Misbehaving(pfrom->GetId(), 100, strprintf("Peer %d sent us invalid compact block\n", pfrom->GetId())); + Misbehaving(pfrom->GetId(), 100, strprintf("Peer %d sent us invalid compact block", pfrom->GetId())); return true; } else if (status == READ_STATUS_FAILED) { // Duplicate txindexes, the block is now in-flight, so just request it @@ -2838,7 +2838,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr ReadStatus status = partialBlock.FillBlock(*pblock, resp.txn); if (status == READ_STATUS_INVALID) { MarkBlockAsReceived(resp.blockhash); // Reset in-flight state in case of whitelist - Misbehaving(pfrom->GetId(), 100, strprintf("Peer %d sent us invalid compact block/non-matching block transactions\n", pfrom->GetId())); + Misbehaving(pfrom->GetId(), 100, strprintf("Peer %d sent us invalid compact block/non-matching block transactions", pfrom->GetId())); return true; } else if (status == READ_STATUS_FAILED) { // Might have collided, fall back to getdata now :( From 6eb6298bc2d32a9bf7273ddce94d2f321ae0eee2 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 28 Jan 2020 13:03:14 +0300 Subject: [PATCH 15/18] More of 12218 for Dash-specific code --- src/evo/mnauth.cpp | 14 ++++++++++---- src/net_processing.cpp | 7 +++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/evo/mnauth.cpp b/src/evo/mnauth.cpp index 724e4ba3ab74e..03f566d7baaa5 100644 --- a/src/evo/mnauth.cpp +++ b/src/evo/mnauth.cpp @@ -63,13 +63,19 @@ void CMNAuth::ProcessMessage(CNode* pnode, const std::string& strCommand, CDataS } if (fAlreadyHaveMNAUTH) { LOCK(cs_main); - Misbehaving(pnode->GetId(), 100); + Misbehaving(pnode->GetId(), 100, "duplicate mnauth"); + return; + } + + if (mnauth.proRegTxHash.IsNull()) { + LOCK(cs_main); + Misbehaving(pnode->GetId(), 100, "empty mnauth proRegTxHash"); return; } if (mnauth.proRegTxHash.IsNull() || !mnauth.sig.IsValid()) { LOCK(cs_main); - Misbehaving(pnode->GetId(), 100); + Misbehaving(pnode->GetId(), 100, "invalid mnauth signature"); return; } @@ -80,7 +86,7 @@ void CMNAuth::ProcessMessage(CNode* pnode, const std::string& strCommand, CDataS // in case he was unlucky and not up to date, just let him be connected as a regular node, which gives him // a chance to get up-to-date and thus realize by himself that he's not a MN anymore. We still give him a // low DoS score. - Misbehaving(pnode->GetId(), 10); + Misbehaving(pnode->GetId(), 10, "missing mnauth masternode"); return; } @@ -95,7 +101,7 @@ void CMNAuth::ProcessMessage(CNode* pnode, const std::string& strCommand, CDataS LOCK(cs_main); // Same as above, MN seems to not know about his fate yet, so give him a chance to update. If this is a // malicious actor (DoSing us), we'll ban him soon. - Misbehaving(pnode->GetId(), 10); + Misbehaving(pnode->GetId(), 10, "mnauth signature verification failed"); return; } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b3391d3459847..9cf7519d0d68b 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3137,8 +3137,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (BuildSimplifiedMNListDiff(cmd.baseBlockHash, cmd.blockHash, mnListDiff, strError)) { connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::MNLISTDIFF, mnListDiff)); } else { - LogPrint(BCLog::NET, "getmnlistdiff failed for baseBlockHash=%s, blockHash=%s. error=%s\n", cmd.baseBlockHash.ToString(), cmd.blockHash.ToString(), strError); - Misbehaving(pfrom->GetId(), 1); + strError = strprintf("getmnlistdiff failed for baseBlockHash=%s, blockHash=%s. error=%s", cmd.baseBlockHash.ToString(), cmd.blockHash.ToString(), strError); + Misbehaving(pfrom->GetId(), 1, strError); } return true; } @@ -3147,8 +3147,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (strCommand == NetMsgType::MNLISTDIFF) { // we have never requested this LOCK(cs_main); - Misbehaving(pfrom->GetId(), 100); - LogPrint(BCLog::NET, "received not-requested mnlistdiff. peer=%d\n", pfrom->GetId()); + Misbehaving(pfrom->GetId(), 100, strprintf("received not-requested mnlistdiff. peer=%d", pfrom->GetId())); return true; } From 67c8484e20da9310e425df028287795a9822fe5c Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 28 Jan 2020 13:04:47 +0300 Subject: [PATCH 16/18] More of 11583 for Dash-specific code --- src/governance/governance.cpp | 24 ++++++++++++------------ src/llmq/quorums_blockprocessor.cpp | 12 ++++++------ src/llmq/quorums_chainlocks.cpp | 2 +- src/llmq/quorums_dkgsessionhandler.cpp | 12 ++++++------ src/llmq/quorums_instantsend.cpp | 2 +- src/llmq/quorums_signing.cpp | 2 +- src/llmq/quorums_signing_shares.cpp | 16 ++++++++-------- src/spork.cpp | 8 ++++---- src/txmempool.cpp | 22 +++++++++++----------- 9 files changed, 50 insertions(+), 50 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 15ade9c28623e..f402038812cb0 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -154,7 +154,7 @@ void CGovernanceManager::ProcessMessage(CNode* pfrom, const std::string& strComm LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECT -- Received object: %s\n", strHash); if (!AcceptObjectMessage(nHash)) { - LogPrintf("MNGOVERNANCEOBJECT -- Received unrequested object: %s\n", strHash); + LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECT -- Received unrequested object: %s\n", strHash); return; } @@ -168,7 +168,7 @@ void CGovernanceManager::ProcessMessage(CNode* pfrom, const std::string& strComm bool fRateCheckBypassed = false; if (!MasternodeRateCheck(govobj, true, false, fRateCheckBypassed)) { - LogPrintf("MNGOVERNANCEOBJECT -- masternode rate check failed - %s - (current block height %d) \n", strHash, nCachedBlockHeight); + LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECT -- masternode rate check failed - %s - (current block height %d) \n", strHash, nCachedBlockHeight); return; } @@ -180,7 +180,7 @@ void CGovernanceManager::ProcessMessage(CNode* pfrom, const std::string& strComm if (fRateCheckBypassed && fIsValid) { if (!MasternodeRateCheck(govobj, true)) { - LogPrintf("MNGOVERNANCEOBJECT -- masternode rate check failed (after signature verification) - %s - (current block height %d)\n", strHash, nCachedBlockHeight); + LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECT -- masternode rate check failed (after signature verification) - %s - (current block height %d)\n", strHash, nCachedBlockHeight); return; } } @@ -190,7 +190,7 @@ void CGovernanceManager::ProcessMessage(CNode* pfrom, const std::string& strComm AddPostponedObject(govobj); LogPrintf("MNGOVERNANCEOBJECT -- Not enough fee confirmations for: %s, strError = %s\n", strHash, strError); } else { - LogPrintf("MNGOVERNANCEOBJECT -- Governance object is invalid - %s\n", strError); + LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECT -- Governance object is invalid - %s\n", strError); // apply node's ban score Misbehaving(pfrom->GetId(), 20); } @@ -292,7 +292,7 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, CConnman // MAKE SURE THIS OBJECT IS OK if (!govobj.IsValidLocally(strError, true)) { - LogPrintf("CGovernanceManager::AddGovernanceObject -- invalid governance object - %s - (nCachedBlockHeight %d) \n", strError, nCachedBlockHeight); + LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- invalid governance object - %s - (nCachedBlockHeight %d) \n", strError, nCachedBlockHeight); return; } @@ -303,7 +303,7 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, CConnman auto objpair = mapObjects.emplace(nHash, govobj); if (!objpair.second) { - LogPrintf("CGovernanceManager::AddGovernanceObject -- already have governance object %s\n", nHash.ToString()); + LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- already have governance object %s\n", nHash.ToString()); return; } @@ -747,13 +747,13 @@ bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bo std::string strHash = govobj.GetHash().ToString(); if (nTimestamp < nNow - 2 * nSuperblockCycleSeconds) { - LogPrintf("CGovernanceManager::MasternodeRateCheck -- object %s rejected due to too old timestamp, masternode = %s, timestamp = %d, current time = %d\n", + LogPrint(BCLog::GOBJECT, "CGovernanceManager::MasternodeRateCheck -- object %s rejected due to too old timestamp, masternode = %s, timestamp = %d, current time = %d\n", strHash, masternodeOutpoint.ToStringShort(), nTimestamp, nNow); return false; } if (nTimestamp > nNow + MAX_TIME_FUTURE_DEVIATION) { - LogPrintf("CGovernanceManager::MasternodeRateCheck -- object %s rejected due to too new (future) timestamp, masternode = %s, timestamp = %d, current time = %d\n", + LogPrint(BCLog::GOBJECT, "CGovernanceManager::MasternodeRateCheck -- object %s rejected due to too new (future) timestamp, masternode = %s, timestamp = %d, current time = %d\n", strHash, masternodeOutpoint.ToStringShort(), nTimestamp, nNow); return false; } @@ -779,7 +779,7 @@ bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bo return true; } - LogPrintf("CGovernanceManager::MasternodeRateCheck -- Rate too high: object hash = %s, masternode = %s, object timestamp = %d, rate = %f, max rate = %f\n", + LogPrint(BCLog::GOBJECT, "CGovernanceManager::MasternodeRateCheck -- Rate too high: object hash = %s, masternode = %s, object timestamp = %d, rate = %f, max rate = %f\n", strHash, masternodeOutpoint.ToStringShort(), nTimestamp, dRate, dMaxRate); if (fUpdateFailStatus) { @@ -806,7 +806,7 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, ostr << "CGovernanceManager::ProcessVote -- Old invalid vote " << ", MN outpoint = " << vote.GetMasternodeOutpoint().ToStringShort() << ", governance object hash = " << nHashGovobj.ToString(); - LogPrintf("%s\n", ostr.str()); + LogPrint(BCLog::GOBJECT, "%s\n", ostr.str()); exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20); LEAVE_CRITICAL_SECTION(cs); return false; @@ -821,7 +821,7 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, if (cmmapOrphanVotes.Insert(nHashGovobj, vote_time_pair_t(vote, GetAdjustedTime() + GOVERNANCE_ORPHAN_EXPIRATION_TIME))) { LEAVE_CRITICAL_SECTION(cs); RequestGovernanceObject(pfrom, nHashGovobj, connman); - LogPrintf("%s\n", ostr.str()); + LogPrint(BCLog::GOBJECT, "%s\n", ostr.str()); return false; } @@ -862,7 +862,7 @@ void CGovernanceManager::CheckPostponedObjects(CConnman& connman) if (govobj.IsValidLocally(strError, false)) { AddGovernanceObject(govobj, connman); } else { - LogPrintf("CGovernanceManager::CheckPostponedObjects -- %s invalid\n", nHash.ToString()); + LogPrint(BCLog::GOBJECT, "CGovernanceManager::CheckPostponedObjects -- %s invalid\n", nHash.ToString()); } } else if (fMissingConfirmations) { diff --git a/src/llmq/quorums_blockprocessor.cpp b/src/llmq/quorums_blockprocessor.cpp index bb5a3d88f41d7..893f78f9f0c3e 100644 --- a/src/llmq/quorums_blockprocessor.cpp +++ b/src/llmq/quorums_blockprocessor.cpp @@ -41,14 +41,14 @@ void CQuorumBlockProcessor::ProcessMessage(CNode* pfrom, const std::string& strC if (qc.IsNull()) { LOCK(cs_main); - LogPrintf("CQuorumBlockProcessor::%s -- null commitment from peer=%d\n", __func__, pfrom->GetId()); + LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- null commitment from peer=%d\n", __func__, pfrom->GetId()); Misbehaving(pfrom->GetId(), 100); return; } if (!Params().GetConsensus().llmqs.count((Consensus::LLMQType)qc.llmqType)) { LOCK(cs_main); - LogPrintf("llmq""CQuorumBlockProcessor::%s -- invalid commitment type %d from peer=%d\n", __func__, + LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- invalid commitment type %d from peer=%d\n", __func__, qc.llmqType, pfrom->GetId()); Misbehaving(pfrom->GetId(), 100); return; @@ -61,7 +61,7 @@ void CQuorumBlockProcessor::ProcessMessage(CNode* pfrom, const std::string& strC { LOCK(cs_main); if (!mapBlockIndex.count(qc.quorumHash)) { - LogPrintf("CQuorumBlockProcessor::%s -- unknown block %s in commitment, peer=%d\n", __func__, + LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- unknown block %s in commitment, peer=%d\n", __func__, qc.quorumHash.ToString(), pfrom->GetId()); // can't really punish the node here, as we might simply be the one that is on the wrong chain or not // fully synced @@ -69,14 +69,14 @@ void CQuorumBlockProcessor::ProcessMessage(CNode* pfrom, const std::string& strC } pquorumIndex = mapBlockIndex[qc.quorumHash]; if (chainActive.Tip()->GetAncestor(pquorumIndex->nHeight) != pquorumIndex) { - LogPrintf("CQuorumBlockProcessor::%s -- block %s not in active chain, peer=%d\n", __func__, + LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- block %s not in active chain, peer=%d\n", __func__, qc.quorumHash.ToString(), pfrom->GetId()); // same, can't punish return; } int quorumHeight = pquorumIndex->nHeight - (pquorumIndex->nHeight % params.dkgInterval); if (quorumHeight != pquorumIndex->nHeight) { - LogPrintf("CQuorumBlockProcessor::%s -- block %s is not the first block in the DKG interval, peer=%d\n", __func__, + LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- block %s is not the first block in the DKG interval, peer=%d\n", __func__, qc.quorumHash.ToString(), pfrom->GetId()); Misbehaving(pfrom->GetId(), 100); return; @@ -103,7 +103,7 @@ void CQuorumBlockProcessor::ProcessMessage(CNode* pfrom, const std::string& strC if (!qc.Verify(members, true)) { LOCK(cs_main); - LogPrintf("CQuorumBlockProcessor::%s -- commitment for quorum %s:%d is not valid, peer=%d\n", __func__, + LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- commitment for quorum %s:%d is not valid, peer=%d\n", __func__, qc.quorumHash.ToString(), qc.llmqType, pfrom->GetId()); Misbehaving(pfrom->GetId(), 100); return; diff --git a/src/llmq/quorums_chainlocks.cpp b/src/llmq/quorums_chainlocks.cpp index 8dee4a5de2b7c..e83ea6b557031 100644 --- a/src/llmq/quorums_chainlocks.cpp +++ b/src/llmq/quorums_chainlocks.cpp @@ -121,7 +121,7 @@ void CChainLocksHandler::ProcessNewChainLock(NodeId from, const llmq::CChainLock uint256 requestId = ::SerializeHash(std::make_pair(CLSIG_REQUESTID_PREFIX, clsig.nHeight)); uint256 msgHash = clsig.blockHash; if (!quorumSigningManager->VerifyRecoveredSig(Params().GetConsensus().llmqTypeChainLocks, clsig.nHeight, requestId, msgHash, clsig.sig)) { - LogPrintf("CChainLocksHandler::%s -- invalid CLSIG (%s), peer=%d\n", __func__, clsig.ToString(), from); + LogPrint(BCLog::CHAINLOCKS, "CChainLocksHandler::%s -- invalid CLSIG (%s), peer=%d\n", __func__, clsig.ToString(), from); if (from != -1) { LOCK(cs_main); Misbehaving(from, 10); diff --git a/src/llmq/quorums_dkgsessionhandler.cpp b/src/llmq/quorums_dkgsessionhandler.cpp index c5e7e83c45a16..9af746b6c70df 100644 --- a/src/llmq/quorums_dkgsessionhandler.cpp +++ b/src/llmq/quorums_dkgsessionhandler.cpp @@ -32,7 +32,7 @@ void CDKGPendingMessages::PushPendingMessage(NodeId from, CDataStream& vRecv) if (messagesPerNode[from] >= maxMessagesPerNode) { // TODO ban? - LogPrintf("CDKGPendingMessages::%s -- too many messages, peer=%d\n", __func__, from); + LogPrint(BCLog::LLMQ_DKG, "CDKGPendingMessages::%s -- too many messages, peer=%d\n", __func__, from); return; } messagesPerNode[from]++; @@ -403,7 +403,7 @@ bool ProcessPendingMessageBatch(CDKGSession& session, CDKGPendingMessages& pendi for (const auto& p : msgs) { if (!p.second) { - LogPrintf("%s -- failed to deserialize message, peer=%d\n", __func__, p.first); + LogPrint(BCLog::LLMQ_DKG, "%s -- failed to deserialize message, peer=%d\n", __func__, p.first); { LOCK(cs_main); Misbehaving(p.first, 100); @@ -421,13 +421,13 @@ bool ProcessPendingMessageBatch(CDKGSession& session, CDKGPendingMessages& pendi bool ban = false; if (!session.PreVerifyMessage(hash, msg, ban)) { if (ban) { - LogPrintf("%s -- banning node due to failed preverification, peer=%d\n", __func__, p.first); + LogPrint(BCLog::LLMQ_DKG, "%s -- banning node due to failed preverification, peer=%d\n", __func__, p.first); { LOCK(cs_main); Misbehaving(p.first, 100); } } - LogPrintf("%s -- skipping message due to failed preverification, peer=%d\n", __func__, p.first); + LogPrint(BCLog::LLMQ_DKG, "%s -- skipping message due to failed preverification, peer=%d\n", __func__, p.first); continue; } hashes.emplace_back(hash); @@ -441,7 +441,7 @@ bool ProcessPendingMessageBatch(CDKGSession& session, CDKGPendingMessages& pendi if (!badNodes.empty()) { LOCK(cs_main); for (auto nodeId : badNodes) { - LogPrintf("%s -- failed to verify signature, peer=%d\n", __func__, nodeId); + LogPrint(BCLog::LLMQ_DKG, "%s -- failed to verify signature, peer=%d\n", __func__, nodeId); Misbehaving(nodeId, 100); } } @@ -455,7 +455,7 @@ bool ProcessPendingMessageBatch(CDKGSession& session, CDKGPendingMessages& pendi bool ban = false; session.ReceiveMessage(hashes[i], msg, ban); if (ban) { - LogPrintf("%s -- banning node after ReceiveMessage failed, peer=%d\n", __func__, nodeId); + LogPrint(BCLog::LLMQ_DKG, "%s -- banning node after ReceiveMessage failed, peer=%d\n", __func__, nodeId); LOCK(cs_main); Misbehaving(nodeId, 100); badNodes.emplace(nodeId); diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 141bb95b8ef66..8682a99752e0d 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -843,7 +843,7 @@ std::unordered_set CInstantSendManager::ProcessPendingInstantSendLocks( auto& islock = p.second.second; if (batchVerifier.badMessages.count(hash)) { - LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: invalid sig in islock, peer=%d\n", __func__, + LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txid=%s, islock=%s: invalid sig in islock, peer=%d\n", __func__, islock.txid.ToString(), hash.ToString(), nodeId); badISLocks.emplace(hash); continue; diff --git a/src/llmq/quorums_signing.cpp b/src/llmq/quorums_signing.cpp index e1f9535fbcc82..86989d335cb31 100644 --- a/src/llmq/quorums_signing.cpp +++ b/src/llmq/quorums_signing.cpp @@ -644,7 +644,7 @@ bool CSigningManager::ProcessPendingRecoveredSigs(CConnman& connman) if (batchVerifier.badSources.count(nodeId)) { LOCK(cs_main); - LogPrintf("CSigningManager::%s -- invalid recSig from other node, banning peer=%d\n", __func__, nodeId); + LogPrint(BCLog::LLMQ, "CSigningManager::%s -- invalid recSig from other node, banning peer=%d\n", __func__, nodeId); Misbehaving(nodeId, 100); continue; } diff --git a/src/llmq/quorums_signing_shares.cpp b/src/llmq/quorums_signing_shares.cpp index 74f585966f6db..e4b6e303d0e54 100644 --- a/src/llmq/quorums_signing_shares.cpp +++ b/src/llmq/quorums_signing_shares.cpp @@ -240,7 +240,7 @@ void CSigSharesManager::ProcessMessage(CNode* pfrom, const std::string& strComma std::vector msgs; vRecv >> msgs; if (msgs.size() > MAX_MSGS_CNT_QSIGSESANN) { - LogPrintf("CSigSharesManager::%s -- too many announcements in QSIGSESANN message. cnt=%d, max=%d, node=%d\n", __func__, msgs.size(), MAX_MSGS_CNT_QSIGSESANN, pfrom->GetId()); + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- too many announcements in QSIGSESANN message. cnt=%d, max=%d, node=%d\n", __func__, msgs.size(), MAX_MSGS_CNT_QSIGSESANN, pfrom->GetId()); BanNode(pfrom->GetId()); return; } @@ -254,7 +254,7 @@ void CSigSharesManager::ProcessMessage(CNode* pfrom, const std::string& strComma std::vector msgs; vRecv >> msgs; if (msgs.size() > MAX_MSGS_CNT_QSIGSHARESINV) { - LogPrintf("CSigSharesManager::%s -- too many invs in QSIGSHARESINV message. cnt=%d, max=%d, node=%d\n", __func__, msgs.size(), MAX_MSGS_CNT_QSIGSHARESINV, pfrom->GetId()); + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- too many invs in QSIGSHARESINV message. cnt=%d, max=%d, node=%d\n", __func__, msgs.size(), MAX_MSGS_CNT_QSIGSHARESINV, pfrom->GetId()); BanNode(pfrom->GetId()); return; } @@ -268,7 +268,7 @@ void CSigSharesManager::ProcessMessage(CNode* pfrom, const std::string& strComma std::vector msgs; vRecv >> msgs; if (msgs.size() > MAX_MSGS_CNT_QGETSIGSHARES) { - LogPrintf("CSigSharesManager::%s -- too many invs in QGETSIGSHARES message. cnt=%d, max=%d, node=%d\n", __func__, msgs.size(), MAX_MSGS_CNT_QGETSIGSHARES, pfrom->GetId()); + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- too many invs in QGETSIGSHARES message. cnt=%d, max=%d, node=%d\n", __func__, msgs.size(), MAX_MSGS_CNT_QGETSIGSHARES, pfrom->GetId()); BanNode(pfrom->GetId()); return; } @@ -286,7 +286,7 @@ void CSigSharesManager::ProcessMessage(CNode* pfrom, const std::string& strComma totalSigsCount += bs.sigShares.size(); } if (totalSigsCount > MAX_MSGS_TOTAL_BATCHED_SIGS) { - LogPrintf("CSigSharesManager::%s -- too many sigs in QBSIGSHARES message. cnt=%d, max=%d, node=%d\n", __func__, msgs.size(), MAX_MSGS_TOTAL_BATCHED_SIGS, pfrom->GetId()); + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- too many sigs in QBSIGSHARES message. cnt=%d, max=%d, node=%d\n", __func__, msgs.size(), MAX_MSGS_TOTAL_BATCHED_SIGS, pfrom->GetId()); BanNode(pfrom->GetId()); return; } @@ -494,12 +494,12 @@ bool CSigSharesManager::PreVerifyBatchedSigShares(NodeId nodeId, const CSigShare } if (quorumMember >= session.quorum->members.size()) { - LogPrintf("CSigSharesManager::%s -- quorumMember out of bounds\n", __func__); + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- quorumMember out of bounds\n", __func__); retBan = true; return false; } if (!session.quorum->qc.validMembers[quorumMember]) { - LogPrintf("CSigSharesManager::%s -- quorumMember not valid\n", __func__); + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- quorumMember not valid\n", __func__); retBan = true; return false; } @@ -627,7 +627,7 @@ bool CSigSharesManager::ProcessPendingSigShares(CConnman& connman) auto& v = p.second; if (batchVerifier.badSources.count(nodeId)) { - LogPrintf("CSigSharesManager::%s -- invalid sig shares from other node, banning peer=%d\n", + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- invalid sig shares from other node, banning peer=%d\n", __func__, nodeId); // this will also cause re-requesting of the shares that were sent by this node BanNode(nodeId); @@ -747,7 +747,7 @@ void CSigSharesManager::TryRecoverSig(const CQuorumCPtr& quorum, const uint256& cxxtimer::Timer t(true); CBLSSignature recoveredSig; if (!recoveredSig.Recover(sigSharesForRecovery, idsForRecovery)) { - LogPrintf("CSigSharesManager::%s -- failed to recover signature. id=%s, msgHash=%s, time=%d\n", __func__, + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- failed to recover signature. id=%s, msgHash=%s, time=%d\n", __func__, id.ToString(), msgHash.ToString(), t.count()); return; } diff --git a/src/spork.cpp b/src/spork.cpp index 06d0500335fa7..add1e71c9428a 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -137,7 +137,7 @@ void CSporkManager::ProcessSpork(CNode* pfrom, const std::string& strCommand, CD if (spork.nTimeSigned > GetAdjustedTime() + 2 * 60 * 60) { LOCK(cs_main); - LogPrintf("CSporkManager::ProcessSpork -- ERROR: too far into the future\n"); + LogPrint(BCLog::SPORK, "CSporkManager::ProcessSpork -- ERROR: too far into the future\n"); Misbehaving(pfrom->GetId(), 100); return; } @@ -150,7 +150,7 @@ void CSporkManager::ProcessSpork(CNode* pfrom, const std::string& strCommand, CD // (and even if it would, spork order can't be guaranteed anyway). if (!spork.GetSignerKeyID(keyIDSigner, !fSpork6IsActive) || !setSporkPubKeyIDs.count(keyIDSigner)) { LOCK(cs_main); - LogPrintf("CSporkManager::ProcessSpork -- ERROR: invalid signature\n"); + LogPrint(BCLog::SPORK, "CSporkManager::ProcessSpork -- ERROR: invalid signature\n"); Misbehaving(pfrom->GetId(), 100); return; } @@ -397,7 +397,7 @@ bool CSporkMessage::CheckSignature(const CKeyID& pubKeyId, bool fSporkSixActive) if (!CHashSigner::VerifyHash(hash, pubKeyId, vchSig, strError)) { // Note: unlike for many other messages when SPORK_6_NEW_SIGS is ON sporks with sigs in old format // and newer timestamps should not be accepted, so if we failed here - that's it - LogPrintf("CSporkMessage::CheckSignature -- VerifyHash() failed, error: %s\n", strError); + LogPrint(BCLog::SPORK, "CSporkMessage::CheckSignature -- VerifyHash() failed, error: %s\n", strError); return false; } } else { @@ -409,7 +409,7 @@ bool CSporkMessage::CheckSignature(const CKeyID& pubKeyId, bool fSporkSixActive) // (and even if it would, spork order can't be guaranteed anyway). uint256 hash = GetSignatureHash(); if (!CHashSigner::VerifyHash(hash, pubKeyId, vchSig, strError)) { - LogPrintf("CSporkMessage::CheckSignature -- VerifyHash() failed, error: %s\n", strError); + LogPrint(BCLog::SPORK, "CSporkMessage::CheckSignature -- VerifyHash() failed, error: %s\n", strError); return false; } } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 1a012fab7b128..d3d5d45992d7a 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -854,7 +854,7 @@ void CTxMemPool::removeProTxSpentCollateralConflicts(const CTransaction &tx) } else { // Should not happen as we track referencing TXs in addUnchecked/removeUnchecked. // But lets be on the safe side and not run into an endless loop... - LogPrintf("%s: ERROR: found invalid TX ref in mapProTxRefs, proTxHash=%s, txHash=%s", __func__, proTxHash.ToString(), it->second.ToString()); + LogPrint(BCLog::MEMPOOL, "%s: ERROR: found invalid TX ref in mapProTxRefs, proTxHash=%s, txHash=%s\n", __func__, proTxHash.ToString(), it->second.ToString()); mapProTxRefs.erase(it); } } @@ -899,7 +899,7 @@ void CTxMemPool::removeProTxConflicts(const CTransaction &tx) if (tx.nType == TRANSACTION_PROVIDER_REGISTER) { CProRegTx proTx; if (!GetTxPayload(tx, proTx)) { - LogPrintf("%s: ERROR: Invalid transaction payload, tx: %s", __func__, tx.ToString()); + LogPrint(BCLog::MEMPOOL, "%s: ERROR: Invalid transaction payload, tx: %s", __func__, tx.ToString()); return; } @@ -917,7 +917,7 @@ void CTxMemPool::removeProTxConflicts(const CTransaction &tx) } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { CProUpServTx proTx; if (!GetTxPayload(tx, proTx)) { - LogPrintf("%s: ERROR: Invalid transaction payload, tx: %s", __func__, tx.ToString()); + LogPrint(BCLog::MEMPOOL, "%s: ERROR: Invalid transaction payload, tx: %s", __func__, tx.ToString()); return; } @@ -930,7 +930,7 @@ void CTxMemPool::removeProTxConflicts(const CTransaction &tx) } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { CProUpRegTx proTx; if (!GetTxPayload(tx, proTx)) { - LogPrintf("%s: ERROR: Invalid transaction payload, tx: %s", __func__, tx.ToString()); + LogPrint(BCLog::MEMPOOL, "%s: ERROR: Invalid transaction payload, tx: %s", __func__, tx.ToString()); return; } @@ -939,7 +939,7 @@ void CTxMemPool::removeProTxConflicts(const CTransaction &tx) } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) { CProUpRevTx proTx; if (!GetTxPayload(tx, proTx)) { - LogPrintf("%s: ERROR: Invalid transaction payload, tx: %s", __func__, tx.ToString()); + LogPrint(BCLog::MEMPOOL, "%s: ERROR: Invalid transaction payload, tx: %s", __func__, tx.ToString()); return; } @@ -1245,7 +1245,7 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { if (tx.nType == TRANSACTION_PROVIDER_REGISTER) { CProRegTx proTx; if (!GetTxPayload(tx, proTx)) { - LogPrintf("%s: ERROR: Invalid transaction payload, tx: %s", __func__, tx.ToString()); + LogPrint(BCLog::MEMPOOL, "%s: ERROR: Invalid transaction payload, tx: %s", __func__, tx.ToString()); return true; // i.e. can't decode payload == conflict } if (mapProTxAddresses.count(proTx.addr) || mapProTxPubKeyIDs.count(proTx.keyIDOwner) || mapProTxBlsPubKeyHashes.count(proTx.pubKeyOperator.GetHash())) @@ -1264,7 +1264,7 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { CProUpServTx proTx; if (!GetTxPayload(tx, proTx)) { - LogPrintf("%s: ERROR: Invalid transaction payload, tx: %s", __func__, tx.ToString()); + LogPrint(BCLog::MEMPOOL, "%s: ERROR: Invalid transaction payload, tx: %s", __func__, tx.ToString()); return true; // i.e. can't decode payload == conflict } auto it = mapProTxAddresses.find(proTx.addr); @@ -1272,14 +1272,14 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { CProUpRegTx proTx; if (!GetTxPayload(tx, proTx)) { - LogPrintf("%s: ERROR: Invalid transaction payload, tx: %s", __func__, tx.ToString()); + LogPrint(BCLog::MEMPOOL, "%s: ERROR: Invalid transaction payload, tx: %s", __func__, tx.ToString()); return true; // i.e. can't decode payload == conflict } // this method should only be called with validated ProTxs auto dmn = deterministicMNManager->GetListAtChainTip().GetMN(proTx.proTxHash); if (!dmn) { - LogPrintf("%s: ERROR: Masternode is not in the list, proTxHash: %s", __func__, proTx.proTxHash.ToString()); + LogPrint(BCLog::MEMPOOL, "%s: ERROR: Masternode is not in the list, proTxHash: %s\n", __func__, proTx.proTxHash.ToString()); return true; // i.e. failed to find validated ProTx == conflict } // only allow one operator key change in the mempool @@ -1294,14 +1294,14 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) { CProUpRevTx proTx; if (!GetTxPayload(tx, proTx)) { - LogPrintf("%s: ERROR: Invalid transaction payload, tx: %s", __func__, tx.ToString()); + LogPrint(BCLog::MEMPOOL, "%s: ERROR: Invalid transaction payload, tx: %s", __func__, tx.ToString()); return true; // i.e. can't decode payload == conflict } // this method should only be called with validated ProTxs auto dmn = deterministicMNManager->GetListAtChainTip().GetMN(proTx.proTxHash); if (!dmn) { - LogPrintf("%s: ERROR: Masternode is not in the list, proTxHash: %s", __func__, proTx.proTxHash.ToString()); + LogPrint(BCLog::MEMPOOL, "%s: ERROR: Masternode is not in the list, proTxHash: %s\n", __func__, proTx.proTxHash.ToString()); return true; // i.e. failed to find validated ProTx == conflict } // only allow one operator key change in the mempool From 0d56f80206cf3acde6b39926d1b91ba5da26ce6a Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 28 Jan 2020 18:42:15 +0300 Subject: [PATCH 17/18] More of 13946 --- src/net_processing.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 9cf7519d0d68b..838f1f8ffb2c9 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2109,18 +2109,20 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } - else if (strCommand == NetMsgType::SENDDSQUEUE) + if (strCommand == NetMsgType::SENDDSQUEUE) { bool b; vRecv >> b; pfrom->fSendDSQueue = b; + return true; } - else if (strCommand == NetMsgType::QSENDRECSIGS) { + if (strCommand == NetMsgType::QSENDRECSIGS) { bool b; vRecv >> b; pfrom->fSendRecSigs = b; + return true; } if (strCommand == NetMsgType::INV) { @@ -2602,7 +2604,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr return true; } - if (strCommand == NetMsgType::CMPCTBLOCK && !fImporting && !fReindex) // Ignore blocks received while importing + if (strCommand == NetMsgType::CMPCTBLOCK && !fImporting && !fReindex) // Ignore blocks received while importing { CBlockHeaderAndShortTxIDs cmpctblock; vRecv >> cmpctblock; From 5cc46c34caa97f2cb0243cd2680d5e195776e00b Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 7 May 2018 12:45:33 +0200 Subject: [PATCH 18/18] Merge #13162: [net] Don't incorrectly log that REJECT messages are unknown. fad63eb [logging] Don't incorrectly log that REJECT messages are unknown. (John Newbery) Pull request description: Reject messages are logged to debug.log if NET debug logging is enabled. Because of the way the `ProcessMessages()` function is structured, processing for REJECT messages will also drop through to the default branch and incorrectly log `Unknown command "reject" from peer-?`. Fix that by exiting from `ProcessMessages()` early. without this PR: ``` 2018-05-03T17:37:00.930600Z received: reject (21 bytes) peer=0 2018-05-03T17:37:00.930620Z Reject message code 16: spammy spam 2018-05-03T17:37:00.930656Z Unknown command "reject" from peer=0 ``` with this PR: ``` 2018-05-03T17:35:04.751246Z received: reject (21 bytes) peer=0 2018-05-03T17:35:04.751274Z Reject message code 16: spammy spam ``` Tree-SHA512: 5c84c98433ab99e0db2dd481f9c2db6f87ff0d39022ff317a791737e918714bbcb4a23e81118212ed8e594ebcf098ab7f52f7fd5e21ebc3f07b1efb279b9b30b --- src/net_processing.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 838f1f8ffb2c9..776073bc62220 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1790,6 +1790,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } LogPrint(BCLog::NET, "Reject %s\n", SanitizeString(ss.str())); } + return true; } if (strCommand == NetMsgType::VERSION) {