merge bitcoin#19293: Avoid redundant and confusing FAILED log

src/net_processing.cpp

@@ -2007,14 +2007,14 @@ inline void static SendBlockTransactions(const CBlock& block, const BlockTransac
     connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCKTXN, resp));
 }
 
-bool static ProcessHeadersMessage(CNode& pfrom, CConnman& connman, ChainstateManager& chainman, CTxMemPool& mempool, const std::vector<CBlockHeader>& headers, const CChainParams& chainparams, bool via_compact_block)
+static void ProcessHeadersMessage(CNode& pfrom, CConnman& connman, ChainstateManager& chainman, CTxMemPool& mempool, const std::vector<CBlockHeader>& headers, const CChainParams& chainparams, bool via_compact_block)
 {
     const CNetMsgMaker msgMaker(pfrom.GetSendVersion());
     size_t nCount = headers.size();
 
     if (nCount == 0) {
         // Nothing interesting. Stop asking this peers for more headers.
-        return true;
+        return;
     }
 
     bool received_new_header = false;
@@ -2049,14 +2049,14 @@ bool static ProcessHeadersMessage(CNode& pfrom, CConnman& connman, ChainstateMan
             if (nodestate->nUnconnectingHeaders % MAX_UNCONNECTING_HEADERS == 0) {
                 Misbehaving(pfrom.GetId(), 20);
             }
-            return true;
+            return;
         }
 
         uint256 hashLastBlock;
         for (const CBlockHeader& header : headers) {
             if (!hashLastBlock.IsNull() && header.hashPrevBlock != hashLastBlock) {
                 Misbehaving(pfrom.GetId(), 20, "non-continuous headers sequence");
-                return false;
+                return;
             }
             hashLastBlock = header.GetHash();
         }
@@ -2073,7 +2073,7 @@ bool static ProcessHeadersMessage(CNode& pfrom, CConnman& connman, ChainstateMan
     if (!chainman.ProcessNewBlockHeaders(headers, state, chainparams, &pindexLast, &first_invalid_header)) {
         if (state.IsInvalid()) {
             MaybePunishNode(pfrom.GetId(), state, via_compact_block, "invalid header received");
-            return false;
+            return;
         }
     }
 
@@ -2188,7 +2188,7 @@ bool static ProcessHeadersMessage(CNode& pfrom, CConnman& connman, ChainstateMan
         }
     }
 
-    return true;
+    return;
 }
 
 void static ProcessOrphanTx(CConnman& connman, CTxMemPool& mempool, std::set<uint256>& orphan_work_set) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans)
@@ -2547,9 +2547,18 @@ std::pair<bool /*ret*/, bool /*do_return*/> static ValidateDSTX(CCoinJoinBroadca
     return {true, false};
 }
 
-bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, int64_t nTimeReceived,
-                    const CChainParams& chainparams, ChainstateManager& chainman, CTxMemPool& mempool,
-                    LLMQContext& llmq_ctx, CConnman& connman, BanMan* banman, const std::atomic<bool>& interruptMsgProc)
+void ProcessMessage(
+    CNode& pfrom,
+    const std::string& msg_type,
+    CDataStream& vRecv,
+    int64_t nTimeReceived,
+    const CChainParams& chainparams,
+    ChainstateManager& chainman,
+    CTxMemPool& mempool,
+    LLMQContext& llmq_ctx,
+    CConnman& connman,
+    BanMan* banman,
+    const std::atomic<bool>& interruptMsgProc)
 {
     LogPrint(BCLog::NET, "received: %s (%u bytes) peer=%d\n", SanitizeString(msg_type), vRecv.size(), pfrom.GetId());
     statsClient.inc("message.received." + SanitizeString(msg_type), 1.0f);
@@ -2557,7 +2566,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
     if (gArgs.IsArgSet("-dropmessagestest") && GetRand(gArgs.GetArg("-dropmessagestest", 0)) == 0)
     {
         LogPrintf("dropmessagestest DROPPING RECV MESSAGE\n");
-        return true;
+        return;
     }
 
     if (!(pfrom.GetLocalServices() & NODE_BLOOM) &&
@@ -2566,7 +2575,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
     {
         LOCK(cs_main);
         Misbehaving(pfrom.GetId(), 100);
-        return false;
+        return;
     }
 
     if (msg_type == NetMsgType::VERSION) {
@@ -2575,7 +2584,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         {
             LOCK(cs_main);
             Misbehaving(pfrom.GetId(), 1);
-            return false;
+            return;
         }
 
         int64_t nTime;
@@ -2601,14 +2610,14 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         {
             LogPrint(BCLog::NET, "peer=%d does not offer the expected services (%08x offered, %08x expected); disconnecting\n", pfrom.GetId(), nServices, GetDesirableServiceFlags(nServices));
             pfrom.fDisconnect = true;
-            return false;
+            return;
         }
 
         if (nVersion < MIN_PEER_PROTO_VERSION) {
             // disconnect from peers older than this proto version
             LogPrint(BCLog::NET, "peer=%d using obsolete version %i; disconnecting\n", pfrom.GetId(), nVersion);
             pfrom.fDisconnect = true;
-            return false;
+            return;
         }
 
         if (!vRecv.empty())
@@ -2638,7 +2647,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                     if (!fMasternodeMode) {
                         LogPrint(BCLog::NET_NETCONN, "but we're not a masternode, disconnecting\n");
                         pfrom.fDisconnect = true;
-                        return true;
+                        return;
                     }
                 }
             }
@@ -2648,7 +2657,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         {
             LogPrintf("connected to self at %s, disconnecting\n", pfrom.addr.ToString());
             pfrom.fDisconnect = true;
-            return true;
+            return;
         }
 
         if (pfrom.fInbound && addrMe.IsRoutable())
@@ -2669,7 +2678,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                 else
                     Misbehaving(pfrom.GetId(), 1); // whover connected, might just have made a mistake, don't ban him immediately
                 pfrom.fDisconnect = true;
-                return true;
+                return;
             }
         }
 
@@ -2758,14 +2767,14 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
             assert(pfrom.fInbound == false);
             pfrom.fDisconnect = true;
         }
-        return true;
+        return;
     }
 
     if (pfrom.nVersion == 0) {
         // Must have a version message before anything else
         LOCK(cs_main);
         Misbehaving(pfrom.GetId(), 1);
-        return false;
+        return;
     }
 
     // At this point, the outgoing message serialization version can't change.
@@ -2818,29 +2827,29 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         }
 
         pfrom.fSuccessfullyConnected = true;
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::SENDADDRV2) {
         if (pfrom.GetSendVersion() < ADDRV2_PROTO_VERSION) {
             // Ignore previous implementations
-            return true;
+            return;
         }
         if (pfrom.fSuccessfullyConnected) {
             // Disconnect peers that send SENDADDRV2 message after VERACK; this
             // must be negotiated between VERSION and VERACK.
             pfrom.fDisconnect = true;
-            return false;
+            return;
         }
         pfrom.m_wants_addrv2 = true;
-        return true;
+        return;
     }
 
     if (!pfrom.fSuccessfullyConnected) {
         // Must have a verack message before anything else
         LOCK(cs_main);
         Misbehaving(pfrom.GetId(), 1);
-        return false;
+        return;
     }
 
     if (pfrom.nTimeFirstMessageReceived == 0) {
@@ -2852,7 +2861,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         if (pfrom.m_masternode_probe_connection && !pfrom.fFirstMessageIsMNAUTH) {
             LogPrint(BCLog::NET, "connection is a masternode probe but first received message is not MNAUTH, peer=%d\n", pfrom.GetId());
             pfrom.fDisconnect = true;
-            return false;
+            return;
         }
     }
 
@@ -2860,7 +2869,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
     if (fBlocksOnly && NetMessageViolatesBlocksOnly(msg_type)) {
         LogPrint(BCLog::NET, "%s sent in violation of protocol peer=%d\n", msg_type, pfrom.GetId());
         pfrom.fDisconnect = true;
-        return false;
+        return;
     }
 
     if (msg_type == NetMsgType::ADDR || msg_type == NetMsgType::ADDRV2) {
@@ -2878,16 +2887,16 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
 
         // Don't want addr from older versions unless seeding
         if (pfrom.nVersion < CADDR_TIME_VERSION && connman.GetAddressCount() > 1000)
-            return true;
+            return;
 
         if (!pfrom.IsAddrRelayPeer()) {
-            return true;
+            return;
         }
         if (vAddr.size() > 1000)
         {
             LOCK(cs_main);
             Misbehaving(pfrom.GetId(), 20, strprintf("%s message size = %u", msg_type, vAddr.size()));
-            return false;
+            return;
         }
 
         // Store the new addresses
@@ -2897,7 +2906,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         for (CAddress& addr : vAddr)
         {
             if (interruptMsgProc)
-                return true;
+                return;
 
             // We only bother storing full nodes, though this may include
             // things which we would not make an outbound connection to, in
@@ -2925,19 +2934,19 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         if (pfrom.fOneShot)
             pfrom.fDisconnect = true;
         statsClient.gauge("peers.knownAddresses", connman.GetAddressCount(), 1.0f);
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::SENDHEADERS) {
         LOCK(cs_main);
         State(pfrom.GetId())->fPreferHeaders = true;
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::SENDHEADERS2) {
         LOCK(cs_main);
         State(pfrom.GetId())->fPreferHeadersCompressed = true;
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::SENDCMPCT) {
@@ -2950,7 +2959,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
             State(pfrom.GetId())->fPreferHeaderAndIDs = fAnnounceUsingCMPCTBLOCK;
             State(pfrom.GetId())->fSupportsDesiredCmpctVersion = true;
         }
-        return true;
+        return;
     }
 
 
@@ -2959,7 +2968,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         bool b;
         vRecv >> b;
         pfrom.fSendDSQueue = b;
-        return true;
+        return;
     }
 
 
@@ -2967,7 +2976,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         bool b;
         vRecv >> b;
         pfrom.fSendRecSigs = b;
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::INV) {
@@ -2977,7 +2986,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         {
             LOCK(cs_main);
             Misbehaving(pfrom.GetId(), 20, strprintf("message inv size() = %u", vInv.size()));
-            return false;
+            return;
         }
 
         LOCK(cs_main);
@@ -2992,7 +3001,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
             }
 
             if (interruptMsgProc)
-                return true;
+                return;
 
             bool fAlreadyHave = AlreadyHave(inv, mempool, llmq_ctx);
             LogPrint(BCLog::NET, "got inv: %s  %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId());
@@ -3038,7 +3047,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                 if (fBlocksOnly && NetMessageViolatesBlocksOnly(inv.GetCommand())) {
                     LogPrint(BCLog::NET, "%s (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.GetCommand(), inv.hash.ToString(), pfrom.GetId());
                     pfrom.fDisconnect = true;
-                    return true;
+                    return;
                 } else if (!fAlreadyHave) {
                     if (fBlocksOnly && (inv.type == MSG_ISLOCK || inv.type == MSG_ISDLOCK)) {
                         if (pfrom.GetRecvVersion() <= ADDRV2_PROTO_VERSION) {
@@ -3049,7 +3058,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                         // Peers with newer versions should never send us these invs when we are in blocks-relay-only mode
                         LogPrint(BCLog::NET, "%s (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.GetCommand(), inv.hash.ToString(), pfrom.GetId());
                         pfrom.fDisconnect = true;
-                        return true;
+                        return;
                     }
                     bool allowWhileInIBD = allowWhileInIBDObjs.count(inv.type);
                     if (allowWhileInIBD || !chainman.ActiveChainstate().IsInitialBlockDownload()) {
@@ -3058,7 +3067,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                 }
             }
         }
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::GETDATA) {
@@ -3068,7 +3077,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         {
             LOCK(cs_main);
             Misbehaving(pfrom.GetId(), 20, strprintf("message getdata size() = %u", vInv.size()));
-            return false;
+            return;
         }
 
         LogPrint(BCLog::NET, "received getdata (%u invsz) peer=%d\n", vInv.size(), pfrom.GetId());
@@ -3079,7 +3088,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
 
         pfrom.vRecvGetData.insert(pfrom.vRecvGetData.end(), vInv.begin(), vInv.end());
         ProcessGetData(pfrom, chainparams, connman, mempool, llmq_ctx, interruptMsgProc);
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::GETBLOCKS) {
@@ -3090,7 +3099,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         if (locator.vHave.size() > MAX_LOCATOR_SZ) {
             LogPrint(BCLog::NET, "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
@@ -3149,7 +3158,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                 break;
             }
         }
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::GETBLOCKTXN) {
@@ -3165,7 +3174,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         }
         if (recent_block) {
             SendBlockTransactions(*recent_block, req, pfrom, connman);
-            return true;
+            return;
         }
 
         LOCK(cs_main);
@@ -3173,7 +3182,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         const CBlockIndex* pindex = LookupBlockIndex(req.blockhash);
         if (!pindex || !(pindex->nStatus & BLOCK_HAVE_DATA)) {
             LogPrint(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) {
@@ -3190,7 +3199,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
             inv.hash = req.blockhash;
             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;
@@ -3198,7 +3207,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         assert(ret);
 
         SendBlockTransactions(block, req, pfrom, connman);
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::GETHEADERS || msg_type == NetMsgType::GETHEADERS2) {
@@ -3209,13 +3218,13 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         if (locator.vHave.size() > MAX_LOCATOR_SZ) {
             LogPrint(BCLog::NET, "%s locator size %lld > %d, disconnect peer=%d\n", msg_type, locator.vHave.size(), MAX_LOCATOR_SZ, pfrom.GetId());
             pfrom.fDisconnect = true;
-            return true;
+            return;
         }
 
         LOCK(cs_main);
         if (::ChainstateActive().IsInitialBlockDownload() && !pfrom.HasPermission(PF_NOBAN)) {
             LogPrint(BCLog::NET, "Ignoring %s from peer=%d because node is in initial block download\n", msg_type, pfrom.GetId());
-            return true;
+            return;
         }
 
         CNodeState *nodestate = State(pfrom.GetId());
@@ -3225,12 +3234,12 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
             // If locator is null, return the hashStop block
             pindex = LookupBlockIndex(hashStop);
             if (!pindex) {
-                return true;
+                return;
             }
 
             if (!BlockRequestAllowed(pindex, chainparams.GetConsensus())) {
                 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;
+                return;
             }
         }
         else
@@ -3281,7 +3290,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                 return compressible_header;
             });
         }
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::TX || msg_type == NetMsgType::DSTX || msg_type == NetMsgType::LEGACYTXLOCKREQUEST) {
@@ -3316,7 +3325,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
            uint256 hashTx = tx.GetHash();
            const auto& [bRet, bDoReturn] = ValidateDSTX(dstx, hashTx);
            if (bDoReturn) {
-               return bRet;
+               return;
            }
         }
 
@@ -3439,7 +3448,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
             MaybePunishNode(pfrom.GetId(), state, /*via_compact_block*/ false);
             llmq_ctx.isman->TransactionRemovedFromMempool(ptx);
         }
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::CMPCTBLOCK)
@@ -3447,7 +3456,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         // Ignore cmpctblock received while importing
         if (fImporting || fReindex) {
             LogPrint(BCLog::NET, "Unexpected cmpctblock message received from peer %d\n", pfrom.GetId());
-            return true;
+            return;
         }
 
         CBlockHeaderAndShortTxIDs cmpctblock;
@@ -3462,7 +3471,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
             // Doesn't connect (or is genesis), instead of DoSing in AcceptBlockHeader, request deeper headers
             if (!::ChainstateActive().IsInitialBlockDownload())
                 connman.PushMessage(&pfrom, msgMaker.Make((pfrom.nServices & NODE_HEADERS_COMPRESSED) ? NetMsgType::GETHEADERS2 : NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexBestHeader), uint256()));
-            return true;
+            return;
         }
 
         if (!LookupBlockIndex(cmpctblock.header.GetHash())) {
@@ -3475,7 +3484,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         if (!chainman.ProcessNewBlockHeaders({cmpctblock.header}, state, chainparams, &pindex)) {
             if (state.IsInvalid()) {
                 MaybePunishNode(pfrom.GetId(), state, /*via_compact_block*/ true, "invalid header via cmpctblock");
-                return true;
+                return;
             }
         }
 
@@ -3513,7 +3522,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         bool fAlreadyInFlight = blockInFlightIt != mapBlocksInFlight.end();
 
         if (pindex->nStatus & BLOCK_HAVE_DATA) // Nothing to do here
-            return true;
+            return;
 
         if (pindex->nChainWork <= ::ChainActive().Tip()->nChainWork || // We know something better
                 pindex->nTx != 0) { // We had this block at some point, but pruned it
@@ -3524,12 +3533,12 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                 vInv[0] = CInv(MSG_BLOCK, cmpctblock.header.GetHash());
                 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 DoS
         // possibilities in compact block processing...
@@ -3543,7 +3552,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                     else {
                         // The block was already in flight using compact blocks from the same peer
                         LogPrint(BCLog::NET, "Peer sent us compact block we were already syncing!\n");
-                        return true;
+                        return;
                     }
                 }
 
@@ -3552,13 +3561,13 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                 if (status == READ_STATUS_INVALID) {
                     MarkBlockAsReceived(pindex->GetBlockHash()); // Reset in-flight state in case Misbehaving does not result in a disconnect
                     Misbehaving(pfrom.GetId(), 100, strprintf("Peer %d sent us invalid compact block", pfrom.GetId()));
-                    return true;
+                    return;
                 } else if (status == READ_STATUS_FAILED) {
                     // Duplicate txindexes, the block is now in-flight, so just request it
                     std::vector<CInv> vInv(1);
                     vInv[0] = CInv(MSG_BLOCK, cmpctblock.header.GetHash());
                     connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv));
-                    return true;
+                    return;
                 }
 
                 BlockTransactionsRequest req;
@@ -3586,7 +3595,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                 ReadStatus status = tempBlock.InitData(cmpctblock, vExtraTxnForCompact);
                 if (status != READ_STATUS_OK) {
                     // TODO: don't ignore failures
-                    return true;
+                    return;
                 }
                 std::vector<CTransactionRef> dummy;
                 status = tempBlock.FillBlock(*pblock, dummy);
@@ -3601,7 +3610,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                 std::vector<CInv> vInv(1);
                 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
                 fRevertToHeaderProcessing = true;
@@ -3654,7 +3663,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                 MarkBlockAsReceived(pblock->GetHash());
             }
         }
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::BLOCKTXN)
@@ -3662,7 +3671,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         // Ignore blocktxn received while importing
         if (fImporting || fReindex) {
             LogPrint(BCLog::NET, "Unexpected blocktxn message received from peer %d\n", pfrom.GetId());
-            return true;
+            return;
         }
 
         BlockTransactions resp;
@@ -3677,7 +3686,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
             if (it == mapBlocksInFlight.end() || !it->second.second->partialBlock ||
                     it->second.first != pfrom.GetId()) {
                 LogPrint(BCLog::NET, "Peer %d sent us block transactions for block we weren't expecting\n", pfrom.GetId());
-                return true;
+                return;
             }
 
             PartiallyDownloadedBlock& partialBlock = *it->second.second->partialBlock;
@@ -3685,7 +3694,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
             if (status == READ_STATUS_INVALID) {
                 MarkBlockAsReceived(resp.blockhash); // Reset in-flight state in case Misbehaving does not result in a disconnect
                 Misbehaving(pfrom.GetId(), 100, strprintf("Peer %d sent us invalid compact block/non-matching block transactions", pfrom.GetId()));
-                return true;
+                return;
             } else if (status == READ_STATUS_FAILED) {
                 // Might have collided, fall back to getdata now :(
                 std::vector<CInv> invs;
@@ -3735,14 +3744,14 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                 mapBlockSource.erase(pblock->GetHash());
             }
         }
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::HEADERS || msg_type == NetMsgType::HEADERS2) {
         // Ignore headers received while importing
         if (fImporting || fReindex) {
             LogPrint(BCLog::NET, "Unexpected headers message received from peer %d\n", pfrom.GetId());
-            return true;
+            return;
         }
 
         std::vector<CBlockHeader> headers;
@@ -3752,7 +3761,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         if (nCount > MAX_HEADERS_RESULTS) {
             LOCK(cs_main);
             Misbehaving(pfrom.GetId(), 20, strprintf("headers message size = %u", nCount));
-            return false;
+            return;
         }
 
         if (msg_type == NetMsgType::HEADERS) {
@@ -3779,7 +3788,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         // Ignore block received while importing
         if (fImporting || fReindex) {
             LogPrint(BCLog::NET, "Unexpected block message received from peer %d\n", pfrom.GetId());
-            return true;
+            return;
         }
 
         std::shared_ptr<CBlock> pblock = std::make_shared<CBlock>();
@@ -3806,7 +3815,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
             LOCK(cs_main);
             mapBlockSource.erase(pblock->GetHash());
         }
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::GETADDR) {
@@ -3817,18 +3826,18 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         // the getaddr message mitigates the attack.
         if (!pfrom.fInbound) {
             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 waste
         //  and discourage addr stamping of INV announcements.
         if (pfrom.fSentAddr) {
             LogPrint(BCLog::NET, "Ignoring repeated \"getaddr\". peer=%d\n", pfrom.GetId());
-            return true;
+            return;
         }
         pfrom.fSentAddr = true;
 
@@ -3840,7 +3849,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                 pfrom.PushAddress(addr, insecure_rand);
             }
         }
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::MEMPOOL) {
@@ -3851,7 +3860,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                 LogPrint(BCLog::NET, "mempool request with bloom filters disabled, disconnect peer=%d\n", pfrom.GetId());
                 pfrom.fDisconnect = true;
             }
-            return true;
+            return;
         }
 
         if (connman.OutboundTargetReached(false) && !pfrom.HasPermission(PF_MEMPOOL))
@@ -3861,14 +3870,14 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                 LogPrint(BCLog::NET, "mempool request with bandwidth limit reached, disconnect peer=%d\n", pfrom.GetId());
                 pfrom.fDisconnect = true;
             }
-            return true;
+            return;
         }
 
         if (!pfrom.m_block_relay_only_peer) {
             LOCK(pfrom.m_tx_relay->cs_tx_inventory);
             pfrom.m_tx_relay->fSendMempool = true;
         }
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::PING) {
@@ -3886,7 +3895,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         // seconds to respond to each, the 5th 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) {
@@ -3942,7 +3951,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         if (bPingFinished) {
             pfrom.nPingNonceSent = 0;
         }
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::FILTERLOAD) {
@@ -3961,7 +3970,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
             pfrom.m_tx_relay->pfilter.reset(new CBloomFilter(filter));
             pfrom.m_tx_relay->fRelayTxes = true;
         }
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::FILTERADD) {
@@ -3985,19 +3994,19 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
             LOCK(cs_main);
             Misbehaving(pfrom.GetId(), 100);
         }
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::FILTERCLEAR) {
         if (pfrom.m_block_relay_only_peer) {
-            return true;
+            return;
         }
         LOCK(pfrom.m_tx_relay->cs_filter);
         if (pfrom.GetLocalServices() & NODE_BLOOM) {
             pfrom.m_tx_relay->pfilter = nullptr;
         }
         pfrom.m_tx_relay->fRelayTxes = true;
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::GETMNLISTDIFF) {
@@ -4014,22 +4023,22 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
             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;
+        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;
     }
 
 
@@ -4037,7 +4046,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         // we have never requested this
         LOCK(cs_main);
         Misbehaving(pfrom.GetId(), 100, strprintf("received not-requested mnlistdiff. peer=%d", pfrom.GetId()));
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::GETQUORUMROTATIONINFO) {
@@ -4054,14 +4063,14 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
             strError = strprintf("getquorumrotationinfo failed for size(baseBlockHashes)=%d, blockRequestHash=%s. error=%s", cmd.baseBlockHashes.size(), cmd.blockRequestHash.ToString(), strError);
             Misbehaving(pfrom.GetId(), 1, strError);
         }
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::QUORUMROTATIONINFO) {
         // we have never requested this
         LOCK(cs_main);
         Misbehaving(pfrom.GetId(), 100, strprintf("received not-requested quorumrotationinfo. peer=%d", pfrom.GetId()));
-        return true;
+        return;
     }
 
     if (msg_type == NetMsgType::NOTFOUND) {
@@ -4086,7 +4095,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
                 }
             }
         }
-        return true;
+        return;
     }
 
     bool found = false;
@@ -4119,13 +4128,13 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
         llmq_ctx.sigman->ProcessMessage(&pfrom, msg_type, vRecv);
         llmq_ctx.clhandler->ProcessMessage(&pfrom, msg_type, vRecv);
         llmq_ctx.isman->ProcessMessage(&pfrom, msg_type, vRecv);
-        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::MaybeDiscourageAndDisconnect(CNode& pnode)
@@ -4200,11 +4209,8 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
     // Message size
     unsigned int nMessageSize = msg.m_message_size;
 
-    // Process message
-    bool fRet = false;
-    try
-    {
-        fRet = ProcessMessage(*pfrom, msg_type, msg.m_recv, msg.m_time, chainparams, m_chainman, m_mempool, *m_llmq_ctx, *connman, m_banman, interruptMsgProc);
+    try {
+        ProcessMessage(*pfrom, msg_type, msg.m_recv, msg.m_time, chainparams, m_chainman, m_mempool, *m_llmq_ctx, *connman, m_banman, interruptMsgProc);
         if (interruptMsgProc)
             return false;
         if (!pfrom->vRecvGetData.empty())
@@ -4234,10 +4240,6 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
         PrintExceptionContinue(std::current_exception(), "ProcessMessages()");
     }
 
-    if (!fRet) {
-        LogPrint(BCLog::NET, "%s(%s, %u bytes) FAILED peer=%d\n", __func__, SanitizeString(msg_type), nMessageSize, pfrom->GetId());
-    }
-
     LOCK(cs_main);
     MaybeDiscourageAndDisconnect(*pfrom);
 

src/test/fuzz/process_message.cpp

@@ -36,7 +36,18 @@
 #include <string>
 #include <vector>
 
-bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, ChainstateManager& chainman, CTxMemPool& mempool, LLMQContext& llmq_ctx, CConnman& connman, BanMan* banman, const std::atomic<bool>& interruptMsgProc);
+void ProcessMessage(
+    CNode& pfrom,
+    const std::string& msg_type,
+    CDataStream& vRecv,
+    int64_t nTimeReceived,
+    const CChainParams& chainparams,
+    ChainstateManager& chainman,
+    CTxMemPool& mempool,
+    LLMQContext& llmq_ctx,
+    CConnman& connman,
+    BanMan* banman,
+    const std::atomic<bool>& interruptMsgProc);
 
 namespace {
 const TestingSetup* g_setup;
@@ -72,7 +83,7 @@ void fuzz_target(const std::vector<uint8_t>& buffer, const std::string& LIMIT_TO
     p2p_node.SetSendVersion(PROTOCOL_VERSION);
     g_setup->m_node.peer_logic->InitializeNode(&p2p_node);
     try {
-        (void)ProcessMessage(p2p_node, random_message_type, random_bytes_data_stream, GetTimeMillis(), Params(), *g_setup->m_node.chainman, *g_setup->m_node.mempool, *g_setup->m_node.llmq_ctx, *g_setup->m_node.connman, g_setup->m_node.banman.get(), std::atomic<bool>{false});
+        ProcessMessage(p2p_node, random_message_type, random_bytes_data_stream, GetTimeMillis(), Params(), *g_setup->m_node.chainman, *g_setup->m_node.mempool, *g_setup->m_node.llmq_ctx, *g_setup->m_node.connman, g_setup->m_node.banman.get(), std::atomic<bool>{false});
     } catch (const std::ios_base::failure& e) {
     }
     SyncWithValidationInterfaceQueue();