diff --git a/src/net_processing.h b/src/net_processing.h --- a/src/net_processing.h +++ b/src/net_processing.h @@ -138,7 +138,7 @@ /** Relay transaction to every node */ void RelayTransaction(const TxId &txid, const CConnman &connman); -bool ProcessMessage(const Config &config, CNode &pfrom, +void ProcessMessage(const Config &config, CNode &pfrom, const std::string &msg_type, CDataStream &vRecv, int64_t nTimeReceived, CTxMemPool &mempool, ChainstateManager &chainman, CConnman &connman, diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1975,7 +1975,7 @@ msgMaker.Make(nSendFlags, NetMsgType::BLOCKTXN, resp)); } -static bool ProcessHeadersMessage(const Config &config, CNode &pfrom, +static void ProcessHeadersMessage(const Config &config, CNode &pfrom, CConnman &connman, CTxMemPool &mempool, ChainstateManager &chainman, const std::vector &headers, @@ -1986,7 +1986,7 @@ if (nCount == 0) { // Nothing interesting. Stop asking this peers for more headers. - return true; + return; } bool received_new_header = false; @@ -2028,7 +2028,7 @@ // The peer is sending us many headers we can't connect. Misbehaving(pfrom, 20, "too-many-unconnected-headers"); } - return true; + return; } BlockHash hashLastBlock; @@ -2036,7 +2036,7 @@ if (!hashLastBlock.IsNull() && header.hashPrevBlock != hashLastBlock) { Misbehaving(pfrom, 20, "non-continuous headers sequence"); - return false; + return; } hashLastBlock = header.GetHash(); } @@ -2053,7 +2053,7 @@ if (state.IsInvalid()) { MaybePunishNodeForBlock(pfrom.GetId(), state, via_compact_block, "invalid header received"); - return false; + return; } } @@ -2203,8 +2203,6 @@ } } } - - return true; } void static ProcessOrphanTx(const Config &config, CConnman &connman, @@ -2520,7 +2518,7 @@ connman.PushMessage(&peer, std::move(msg)); } -bool ProcessMessage(const Config &config, CNode &pfrom, +void ProcessMessage(const Config &config, CNode &pfrom, const std::string &msg_type, CDataStream &vRecv, int64_t nTimeReceived, CTxMemPool &mempool, ChainstateManager &chainman, CConnman &connman, @@ -2531,7 +2529,7 @@ if (gArgs.IsArgSet("-dropmessagestest") && GetRand(gArgs.GetArg("-dropmessagestest", 0)) == 0) { LogPrintf("dropmessagestest DROPPING RECV MESSAGE\n"); - return true; + return; } if (!(pfrom.GetLocalServices() & NODE_BLOOM) && @@ -2540,11 +2538,10 @@ if (pfrom.nVersion >= NO_BLOOM_VERSION) { LOCK(cs_main); Misbehaving(pfrom, 100, "no-bloom-version"); - return false; } else { pfrom.fDisconnect = true; - return false; } + return; } if (msg_type == NetMsgType::VERSION) { @@ -2552,7 +2549,7 @@ if (pfrom.nVersion != 0) { LOCK(cs_main); Misbehaving(pfrom, 1, "multiple-version"); - return false; + return; } int64_t nTime; @@ -2581,7 +2578,7 @@ pfrom.GetId(), nServices, GetDesirableServiceFlags(nServices)); pfrom.fDisconnect = true; - return false; + return; } if (nVersion < MIN_PEER_PROTO_VERSION) { @@ -2590,7 +2587,7 @@ "peer=%d using obsolete version %i; disconnecting\n", pfrom.GetId(), nVersion); pfrom.fDisconnect = true; - return false; + return; } if (!vRecv.empty()) { @@ -2612,7 +2609,7 @@ LogPrintf("connected to self at %s, disconnecting\n", pfrom.addr.ToString()); pfrom.fDisconnect = true; - return true; + return; } if (pfrom.IsInboundConn() && addrMe.IsRoutable()) { @@ -2717,14 +2714,14 @@ if (pfrom.IsFeelerConn()) { pfrom.fDisconnect = true; } - return true; + return; } if (pfrom.nVersion == 0) { // Must have a version message before anything else LOCK(cs_main); Misbehaving(pfrom, 10, "missing-version"); - return false; + return; } // At this point, the outgoing message serialization version can't change. @@ -2766,14 +2763,14 @@ nCMPCTBLOCKVersion)); } pfrom.fSuccessfullyConnected = true; - return true; + return; } if (!pfrom.fSuccessfullyConnected) { // Must have a verack message before anything else LOCK(cs_main); Misbehaving(pfrom, 10, "missing-verack"); - return false; + return; } if (msg_type == NetMsgType::ADDR) { @@ -2781,14 +2778,14 @@ vRecv >> vAddr; if (!pfrom.IsAddrRelayPeer()) { - return true; + return; } if (vAddr.size() > 1000) { LOCK(cs_main); Misbehaving(pfrom, 20, strprintf("oversized-addr: message addr size() = %u", vAddr.size())); - return false; + return; } // Store the new addresses @@ -2797,7 +2794,7 @@ int64_t nSince = nNow - 10 * 60; for (CAddress &addr : vAddr) { if (interruptMsgProc) { - return true; + return; } // We only bother storing full nodes, though this may include things @@ -2839,13 +2836,13 @@ if (pfrom.IsAddrFetchConn()) { pfrom.fDisconnect = true; } - return true; + return; } if (msg_type == NetMsgType::SENDHEADERS) { LOCK(cs_main); State(pfrom.GetId())->fPreferHeaders = true; - return true; + return; } if (msg_type == NetMsgType::SENDCMPCT) { @@ -2866,7 +2863,7 @@ State(pfrom.GetId())->fSupportsDesiredCmpctVersion = true; } } - return true; + return; } if (msg_type == NetMsgType::INV) { @@ -2877,7 +2874,7 @@ Misbehaving(pfrom, 20, strprintf("oversized-inv: message inv size() = %u", vInv.size())); - return false; + return; } // We won't accept tx inv's if we're in blocks-only mode, or this is a @@ -2896,7 +2893,7 @@ for (CInv &inv : vInv) { if (interruptMsgProc) { - return true; + return; } bool fAlreadyHave = AlreadyHave(inv, mempool); @@ -2933,14 +2930,14 @@ "protocol, disconnecting peer=%d\n", txid.ToString(), pfrom.GetId()); pfrom.fDisconnect = true; - return true; + return; } else if (!fAlreadyHave && !fImporting && !fReindex && !::ChainstateActive().IsInitialBlockDownload()) { RequestTx(State(pfrom.GetId()), txid, current_time); } } } - return true; + return; } if (msg_type == NetMsgType::GETDATA) { @@ -2951,7 +2948,7 @@ Misbehaving(pfrom, 20, strprintf("too-many-inv: message getdata size() = %u", vInv.size())); - return false; + return; } LogPrint(BCLog::NET, "received getdata (%u invsz) peer=%d\n", @@ -2965,7 +2962,7 @@ pfrom.vRecvGetData.insert(pfrom.vRecvGetData.end(), vInv.begin(), vInv.end()); ProcessGetData(config, pfrom, connman, mempool, interruptMsgProc); - return true; + return; } if (msg_type == NetMsgType::GETBLOCKS) { @@ -2978,7 +2975,7 @@ "getblocks locator size %lld > %d, disconnect peer=%d\n", locator.vHave.size(), MAX_LOCATOR_SZ, pfrom.GetId()); pfrom.fDisconnect = true; - return true; + return; } // We might have announced the currently-being-connected tip using a @@ -3048,7 +3045,7 @@ break; } } - return true; + return; } if (msg_type == NetMsgType::GETBLOCKTXN) { @@ -3065,7 +3062,7 @@ } if (recent_block) { SendBlockTransactions(*recent_block, req, pfrom, connman); - return true; + return; } LOCK(cs_main); @@ -3076,7 +3073,7 @@ BCLog::NET, "Peer %d sent us a getblocktxn for a block we don't have\n", pfrom.GetId()); - return true; + return; } if (pindex->nHeight < ::ChainActive().Height() - MAX_BLOCKTXN_DEPTH) { @@ -3096,7 +3093,7 @@ pfrom.vRecvGetData.push_back(inv); // The message processing loop will go around again (without // pausing) and we'll respond then (without cs_main) - return true; + return; } CBlock block; @@ -3104,7 +3101,7 @@ assert(ret); SendBlockTransactions(block, req, pfrom, connman); - return true; + return; } if (msg_type == NetMsgType::GETHEADERS) { @@ -3117,7 +3114,7 @@ "getheaders locator size %lld > %d, disconnect peer=%d\n", locator.vHave.size(), MAX_LOCATOR_SZ, pfrom.GetId()); pfrom.fDisconnect = true; - return true; + return; } LOCK(cs_main); @@ -3127,7 +3124,7 @@ "Ignoring getheaders from peer=%d because node is in " "initial block download\n", pfrom.GetId()); - return true; + return; } CNodeState *nodestate = State(pfrom.GetId()); @@ -3136,7 +3133,7 @@ // If locator is null, return the hashStop block pindex = LookupBlockIndex(hashStop); if (!pindex) { - return true; + return; } if (!BlockRequestAllowed(pindex, chainparams.GetConsensus())) { @@ -3144,7 +3141,7 @@ "%s: ignoring request from peer=%i for old block " "header that isn't in the main chain\n", __func__, pfrom.GetId()); - return true; + return; } } else { // Find the last block the caller has in the main chain @@ -3184,7 +3181,7 @@ pindex ? pindex : ::ChainActive().Tip(); connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::HEADERS, vHeaders)); - return true; + return; } if (msg_type == NetMsgType::TX) { @@ -3198,7 +3195,7 @@ "transaction sent in violation of protocol peer=%d\n", pfrom.GetId()); pfrom.fDisconnect = true; - return true; + return; } CTransactionRef ptx; @@ -3336,7 +3333,7 @@ tx.GetHash().ToString(), pfrom.GetId(), state.ToString()); MaybePunishNodeForTx(pfrom.GetId(), state); } - return true; + return; } if (msg_type == NetMsgType::CMPCTBLOCK) { @@ -3345,7 +3342,7 @@ LogPrint(BCLog::NET, "Unexpected cmpctblock message received from peer %d\n", pfrom.GetId()); - return true; + return; } CBlockHeaderAndShortTxIDs cmpctblock; @@ -3366,7 +3363,7 @@ pindexBestHeader), uint256())); } - return true; + return; } if (!LookupBlockIndex(cmpctblock.header.GetHash())) { @@ -3382,7 +3379,7 @@ MaybePunishNodeForBlock(pfrom.GetId(), state, /*via_compact_block*/ true, "invalid header via cmpctblock"); - return true; + return; } } @@ -3426,7 +3423,7 @@ if (pindex->nStatus.hasData()) { // Nothing to do here - return true; + return; } if (pindex->nChainWork <= @@ -3444,14 +3441,14 @@ connman.PushMessage( &pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); } - return true; + return; } // If we're not close to tip yet, give up and let parallel block // fetch work its magic. if (!fAlreadyInFlight && !CanDirectFetch(chainparams.GetConsensus())) { - return true; + return; } // We want to be a bit conservative just to be extra careful about @@ -3476,7 +3473,7 @@ // blocks from the same peer. LogPrint(BCLog::NET, "Peer sent us compact block " "we were already syncing!\n"); - return true; + return; } } @@ -3490,7 +3487,7 @@ Misbehaving(pfrom, 100, strprintf("invalid-cmpctblk: Peer sent us " "invalid compact block")); - return true; + return; } else if (status == READ_STATUS_FAILED) { // Duplicate txindices, the block is now in-flight, so // just request it. @@ -3498,7 +3495,7 @@ vInv[0] = CInv(MSG_BLOCK, cmpctblock.header.GetHash()); connman.PushMessage( &pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); - return true; + return; } BlockTransactionsRequest req; @@ -3530,7 +3527,7 @@ tempBlock.InitData(cmpctblock, vExtraTxnForCompact); if (status != READ_STATUS_OK) { // TODO: don't ignore failures - return true; + return; } std::vector dummy; status = tempBlock.FillBlock(*pblock, dummy); @@ -3547,7 +3544,7 @@ vInv[0] = CInv(MSG_BLOCK, cmpctblock.header.GetHash()); connman.PushMessage( &pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); - return true; + return; } else { // If this was an announce-cmpctblock, we want the same // treatment as a header message. @@ -3610,7 +3607,7 @@ MarkBlockAsReceived(pblock->GetHash()); } } - return true; + return; } if (msg_type == NetMsgType::BLOCKTXN) { @@ -3619,7 +3616,7 @@ LogPrint(BCLog::NET, "Unexpected blocktxn message received from peer %d\n", pfrom.GetId()); - return true; + return; } BlockTransactions resp; @@ -3640,7 +3637,7 @@ "Peer %d sent us block transactions for block " "we weren't expecting\n", pfrom.GetId()); - return true; + return; } PartiallyDownloadedBlock &partialBlock = @@ -3653,7 +3650,7 @@ pfrom, 100, strprintf("invalid-cmpctblk-txns: Peer sent us invalid " "compact block/non-matching block transactions")); - return true; + return; } else if (status == READ_STATUS_FAILED) { // Might have collided, fall back to getdata now :( std::vector invs; @@ -3710,7 +3707,7 @@ mapBlockSource.erase(pblock->GetHash()); } } - return true; + return; } if (msg_type == NetMsgType::HEADERS) { @@ -3719,7 +3716,7 @@ LogPrint(BCLog::NET, "Unexpected headers message received from peer %d\n", pfrom.GetId()); - return true; + return; } std::vector headers; @@ -3732,7 +3729,7 @@ Misbehaving(pfrom, 20, strprintf("too-many-headers: headers message size = %u", nCount)); - return false; + return; } headers.resize(nCount); for (unsigned int n = 0; n < nCount; n++) { @@ -3752,7 +3749,7 @@ LogPrint(BCLog::NET, "Unexpected block message received from peer %d\n", pfrom.GetId()); - return true; + return; } std::shared_ptr pblock = std::make_shared(); @@ -3786,7 +3783,7 @@ LOCK(cs_main); mapBlockSource.erase(hash); } - return true; + return; } // Ignore avalanche requests while importing @@ -3818,7 +3815,7 @@ Misbehaving( pfrom, 20, strprintf("too-many-ava-poll: poll message size = %u", nCount)); - return false; + return; } std::vector votes; @@ -3897,7 +3894,7 @@ // Send the query to the node. g_avalanche->sendResponse( &pfrom, avalanche::Response(round, cooldown, std::move(votes))); - return true; + return; } // Ignore avalanche requests while importing @@ -3918,12 +3915,12 @@ })) { LOCK(cs_main); Misbehaving(pfrom, 100, "invalid-ava-response-signature"); - return true; + return; } std::vector updates; if (!g_avalanche->registerVotes(pfrom.GetId(), response, updates)) { - return true; + return; } if (updates.size()) { @@ -3937,8 +3934,9 @@ BlockValidationState state; ::ChainstateActive().ParkBlock(config, state, pindex); if (!state.IsValid()) { - return error("Database error: %s", - state.GetRejectReason()); + LogPrintf("ERROR: Database error: %s\n", + state.GetRejectReason()); + return; } } break; case avalanche::BlockUpdate::Status::Accepted: @@ -3958,7 +3956,7 @@ } } - return true; + return; } if (msg_type == NetMsgType::GETADDR) { @@ -3972,14 +3970,14 @@ LogPrint(BCLog::NET, "Ignoring \"getaddr\" from outbound connection. peer=%d\n", pfrom.GetId()); - return true; + return; } if (!pfrom.IsAddrRelayPeer()) { LogPrint(BCLog::NET, "Ignoring \"getaddr\" from block-relay-only connection. " "peer=%d\n", pfrom.GetId()); - return true; + return; } // Only send one GetAddr response per connection to reduce resource @@ -3987,7 +3985,7 @@ if (pfrom.fSentAddr) { LogPrint(BCLog::NET, "Ignoring repeated \"getaddr\". peer=%d\n", pfrom.GetId()); - return true; + return; } pfrom.fSentAddr = true; @@ -3999,7 +3997,7 @@ pfrom.PushAddress(addr, insecure_rand); } } - return true; + return; } if (msg_type == NetMsgType::MEMPOOL) { @@ -4012,7 +4010,7 @@ pfrom.GetId()); pfrom.fDisconnect = true; } - return true; + return; } if (connman.OutboundTargetReached(false) && @@ -4024,14 +4022,14 @@ pfrom.GetId()); pfrom.fDisconnect = true; } - return true; + return; } if (pfrom.m_tx_relay != nullptr) { LOCK(pfrom.m_tx_relay->cs_tx_inventory); pfrom.m_tx_relay->fSendMempool = true; } - return true; + return; } if (msg_type == NetMsgType::PING) { @@ -4054,7 +4052,7 @@ // ping the remote sends would appear to return very quickly. connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::PONG, nonce)); } - return true; + return; } if (msg_type == NetMsgType::PONG) { @@ -4113,7 +4111,7 @@ if (bPingFinished) { pfrom.nPingNonceSent = 0; } - return true; + return; } if (msg_type == NetMsgType::FILTERLOAD) { @@ -4130,7 +4128,7 @@ pfrom.m_tx_relay->pfilter->UpdateEmptyFull(); pfrom.m_tx_relay->fRelayTxes = true; } - return true; + return; } if (msg_type == NetMsgType::FILTERADD) { @@ -4157,19 +4155,19 @@ // code. We'll go generic. Misbehaving(pfrom, 100, "invalid-filteradd"); } - return true; + return; } if (msg_type == NetMsgType::FILTERCLEAR) { if (pfrom.m_tx_relay == nullptr) { - return true; + return; } LOCK(pfrom.m_tx_relay->cs_filter); if (pfrom.GetLocalServices() & NODE_BLOOM) { pfrom.m_tx_relay->pfilter.reset(new CBloomFilter()); } pfrom.m_tx_relay->fRelayTxes = true; - return true; + return; } if (msg_type == NetMsgType::FEEFILTER) { @@ -4183,22 +4181,22 @@ LogPrint(BCLog::NET, "received: feefilter of %s from peer=%d\n", CFeeRate(newFeeFilter).ToString(), pfrom.GetId()); } - return true; + return; } if (msg_type == NetMsgType::GETCFILTERS) { ProcessGetCFilters(pfrom, vRecv, chainparams, connman); - return true; + return; } if (msg_type == NetMsgType::GETCFHEADERS) { ProcessGetCFHeaders(pfrom, vRecv, chainparams, connman); - return true; + return; } if (msg_type == NetMsgType::GETCFCHECKPT) { ProcessGetCFCheckPt(pfrom, vRecv, chainparams, connman); - return true; + return; } if (msg_type == NetMsgType::NOTFOUND) { @@ -4227,13 +4225,13 @@ } } } - return true; + return; } // Ignore unknown commands for extensibility LogPrint(BCLog::NET, "Unknown command \"%s\" from peer=%d\n", SanitizeString(msg_type), pfrom.GetId()); - return true; + return; } bool PeerLogicValidation::CheckIfBanned(CNode &pnode) { @@ -4365,12 +4363,9 @@ return fMoreWork; } - // Process message - bool fRet = false; try { - fRet = ProcessMessage(config, *pfrom, msg_type, vRecv, msg.m_time, - m_mempool, m_chainman, *connman, m_banman, - interruptMsgProc); + ProcessMessage(config, *pfrom, msg_type, vRecv, msg.m_time, m_mempool, + m_chainman, *connman, m_banman, interruptMsgProc); if (interruptMsgProc) { return false; } @@ -4387,11 +4382,6 @@ __func__, SanitizeString(msg_type), nMessageSize); } - if (!fRet) { - LogPrint(BCLog::NET, "%s(%s, %u bytes) FAILED peer=%d\n", __func__, - SanitizeString(msg_type), nMessageSize, pfrom->GetId()); - } - LOCK(cs_main); CheckIfBanned(*pfrom); diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -113,11 +113,11 @@ connman.AddTestNode(p2p_node); g_setup->m_node.peer_logic->InitializeNode(config, &p2p_node); try { - (void)ProcessMessage( - config, p2p_node, random_message_type, random_bytes_data_stream, - GetTimeMillis(), *g_setup->m_node.mempool, - *g_setup->m_node.chainman, *g_setup->m_node.connman.get(), - g_setup->m_node.banman.get(), std::atomic{false}); + ProcessMessage(config, p2p_node, random_message_type, + random_bytes_data_stream, GetTimeMillis(), + *g_setup->m_node.mempool, *g_setup->m_node.chainman, + *g_setup->m_node.connman.get(), + g_setup->m_node.banman.get(), std::atomic{false}); } catch (const std::ios_base::failure &e) { const std::string exception_message{e.what()}; const auto p =