From fac12ebf4f3b77b05112d2b00f8d3f4669621a4c Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Tue, 16 Jun 2020 06:40:40 -0400
Subject: net: Avoid redundant and confusing FAILED log
Every `return false` is preceeded by a detailed debug log message to
explain that a disconnect or misbehavior happened. Logging another
generic "FAILED" message seems redundant.
Also, the size of the message and the message type has already been
logged and is thus redundant as well.
Finally, claiming that message processing FAILED seems odd, because the
message was fully processed to the point where it was concluded that the
peer should be either disconnected or marked as misbehaving.
---
src/net_processing.cpp | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
(limited to 'src')
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index aa5c9668e3..8019343fd8 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1793,7 +1793,7 @@ bool static ProcessHeadersMessage(CNode& pfrom, CConnman* connman, ChainstateMan
for (const CBlockHeader& header : headers) {
if (!hashLastBlock.IsNull() && header.hashPrevBlock != hashLastBlock) {
Misbehaving(pfrom.GetId(), 20, "non-continuous headers sequence");
- return false;
+ return true;
}
hashLastBlock = header.GetHash();
}
@@ -1809,7 +1809,7 @@ bool static ProcessHeadersMessage(CNode& pfrom, CConnman* connman, ChainstateMan
if (!chainman.ProcessNewBlockHeaders(headers, state, chainparams, &pindexLast)) {
if (state.IsInvalid()) {
MaybePunishNodeForBlock(pfrom.GetId(), state, via_compact_block, "invalid header received");
- return false;
+ return true;
}
}
@@ -2221,7 +2221,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
{
LOCK(cs_main);
Misbehaving(pfrom.GetId(), 1);
- return false;
+ return true;
}
int64_t nTime;
@@ -2247,14 +2247,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 true;
}
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 true;
}
if (!vRecv.empty())
@@ -2381,7 +2381,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
// Must have a version message before anything else
LOCK(cs_main);
Misbehaving(pfrom.GetId(), 1);
- return false;
+ return true;
}
// At this point, the outgoing message serialization version can't change.
@@ -2429,7 +2429,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
// Must have a verack message before anything else
LOCK(cs_main);
Misbehaving(pfrom.GetId(), 1);
- return false;
+ return true;
}
if (msg_type == NetMsgType::ADDR) {
@@ -2446,7 +2446,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
{
LOCK(cs_main);
Misbehaving(pfrom.GetId(), 20, strprintf("message addr size() = %u", vAddr.size()));
- return false;
+ return true;
}
// Store the new addresses
@@ -2522,7 +2522,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 true;
}
// We won't accept tx inv's if we're in blocks-only mode, or this is a
@@ -2588,7 +2588,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 true;
}
LogPrint(BCLog::NET, "received getdata (%u invsz) peer=%d\n", vInv.size(), pfrom.GetId());
@@ -3246,7 +3246,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 true;
}
headers.resize(nCount);
for (unsigned int n = 0; n < nCount; n++) {
--
cgit v1.2.3
From fa1904e5f0d164fbcf41398f9ebbaafe82c28419 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Tue, 16 Jun 2020 06:58:06 -0400
Subject: net: Remove dead logging code
fRet is never false, so the dead code can be removed and the return type
can be made void
---
src/net_processing.cpp | 191 +++++++++++++++++++-------------------
src/test/fuzz/process_message.cpp | 17 +++-
2 files changed, 112 insertions(+), 96 deletions(-)
(limited to 'src')
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 8019343fd8..a7a90adf3f 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1746,14 +1746,14 @@ inline void static SendBlockTransactions(const CBlock& block, const BlockTransac
connman->PushMessage(&pfrom, msgMaker.Make(nSendFlags, 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;
@@ -1786,14 +1786,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 true;
+ return;
}
hashLastBlock = header.GetHash();
}
@@ -1809,7 +1809,7 @@ bool static ProcessHeadersMessage(CNode& pfrom, CConnman* connman, ChainstateMan
if (!chainman.ProcessNewBlockHeaders(headers, state, chainparams, &pindexLast)) {
if (state.IsInvalid()) {
MaybePunishNodeForBlock(pfrom.GetId(), state, via_compact_block, "invalid header received");
- return true;
+ return;
}
}
@@ -1925,7 +1925,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, std::list<CTransactionRef>& removed_txn) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans)
@@ -2205,13 +2205,23 @@ static void ProcessGetCFCheckPt(CNode& pfrom, CDataStream& vRecv, const CChainPa
connman.PushMessage(&pfrom, std::move(msg));
}
-bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, ChainstateManager& chainman, CTxMemPool& mempool, 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,
+ 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());
if (gArgs.IsArgSet("-dropmessagestest") && GetRand(gArgs.GetArg("-dropmessagestest", 0)) == 0)
{
LogPrintf("dropmessagestest DROPPING RECV MESSAGE\n");
- return true;
+ return;
}
@@ -2221,7 +2231,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
{
LOCK(cs_main);
Misbehaving(pfrom.GetId(), 1);
- return true;
+ return;
}
int64_t nTime;
@@ -2247,14 +2257,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 true;
+ 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 true;
+ return;
}
if (!vRecv.empty())
@@ -2274,7 +2284,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())
@@ -2374,14 +2384,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 true;
+ return;
}
// At this point, the outgoing message serialization version can't change.
@@ -2422,14 +2432,14 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion));
}
pfrom.fSuccessfullyConnected = true;
- return true;
+ return;
}
if (!pfrom.fSuccessfullyConnected) {
// Must have a verack message before anything else
LOCK(cs_main);
Misbehaving(pfrom.GetId(), 1);
- return true;
+ return;
}
if (msg_type == NetMsgType::ADDR) {
@@ -2438,15 +2448,15 @@ 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("message addr size() = %u", vAddr.size()));
- return true;
+ return;
}
// Store the new addresses
@@ -2456,7 +2466,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
@@ -2483,13 +2493,13 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
pfrom.fGetAddr = false;
if (pfrom.fOneShot)
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) {
@@ -2512,7 +2522,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
State(pfrom.GetId())->fSupportsDesiredCmpctVersion = (nCMPCTBLOCKVersion == 1);
}
}
- return true;
+ return;
}
if (msg_type == NetMsgType::INV) {
@@ -2522,7 +2532,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 true;
+ return;
}
// We won't accept tx inv's if we're in blocks-only mode, or this is a
@@ -2542,7 +2552,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
for (CInv &inv : vInv)
{
if (interruptMsgProc)
- return true;
+ return;
bool fAlreadyHave = AlreadyHave(inv, mempool);
LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId());
@@ -2566,7 +2576,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
if (fBlocksOnly) {
LogPrint(BCLog::NET, "transaction (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.hash.ToString(), pfrom.GetId());
pfrom.fDisconnect = true;
- return true;
+ return;
} else if (!fAlreadyHave && !fImporting && !fReindex && !::ChainstateActive().IsInitialBlockDownload()) {
RequestTx(State(pfrom.GetId()), inv.hash, current_time);
}
@@ -2578,7 +2588,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
LogPrint(BCLog::NET, "getheaders (%d) %s to peer=%d\n", pindexBestHeader->nHeight, best_block->ToString(), pfrom.GetId());
}
- return true;
+ return;
}
if (msg_type == NetMsgType::GETDATA) {
@@ -2588,7 +2598,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 true;
+ return;
}
LogPrint(BCLog::NET, "received getdata (%u invsz) peer=%d\n", vInv.size(), pfrom.GetId());
@@ -2599,7 +2609,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, interruptMsgProc);
- return true;
+ return;
}
if (msg_type == NetMsgType::GETBLOCKS) {
@@ -2610,7 +2620,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
@@ -2667,7 +2677,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
break;
}
}
- return true;
+ return;
}
if (msg_type == NetMsgType::GETBLOCKTXN) {
@@ -2683,7 +2693,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);
@@ -2691,7 +2701,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) {
@@ -2708,7 +2718,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;
@@ -2716,7 +2726,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) {
@@ -2727,13 +2737,13 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
if (locator.vHave.size() > MAX_LOCATOR_SZ) {
LogPrint(BCLog::NET, "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);
if (::ChainstateActive().IsInitialBlockDownload() && !pfrom.HasPermission(PF_NOBAN)) {
LogPrint(BCLog::NET, "Ignoring getheaders from peer=%d because node is in initial block download\n", pfrom.GetId());
- return true;
+ return;
}
CNodeState *nodestate = State(pfrom.GetId());
@@ -2743,12 +2753,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
@@ -2783,7 +2793,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
// in the SendMessages logic.
nodestate->pindexBestHeaderSent = pindex ? pindex : ::ChainActive().Tip();
connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::HEADERS, vHeaders));
- return true;
+ return;
}
if (msg_type == NetMsgType::TX) {
@@ -2794,7 +2804,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
{
LogPrint(BCLog::NET, "transaction sent in violation of protocol peer=%d\n", pfrom.GetId());
pfrom.fDisconnect = true;
- return true;
+ return;
}
CTransactionRef ptx;
@@ -2925,7 +2935,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
state.ToString());
MaybePunishNodeForTx(pfrom.GetId(), state);
}
- return true;
+ return;
}
if (msg_type == NetMsgType::CMPCTBLOCK)
@@ -2933,7 +2943,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;
@@ -2948,7 +2958,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(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexBestHeader), uint256()));
- return true;
+ return;
}
if (!LookupBlockIndex(cmpctblock.header.GetHash())) {
@@ -2961,7 +2971,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
if (!chainman.ProcessNewBlockHeaders({cmpctblock.header}, state, chainparams, &pindex)) {
if (state.IsInvalid()) {
MaybePunishNodeForBlock(pfrom.GetId(), state, /*via_compact_block*/ true, "invalid header via cmpctblock");
- return true;
+ return;
}
}
@@ -2999,7 +3009,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
@@ -3010,17 +3020,17 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom), 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;
if (IsWitnessEnabled(pindex->pprev, chainparams.GetConsensus()) && !nodestate->fSupportsDesiredCmpctVersion) {
// Don't bother trying to process compact blocks from v1 peers
// after segwit activates.
- return true;
+ return;
}
// We want to be a bit conservative just to be extra careful about DoS
@@ -3035,7 +3045,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;
}
}
@@ -3044,13 +3054,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 of whitelist
Misbehaving(pfrom.GetId(), 100, strprintf("Peer %d sent us invalid compact block\n", 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 | GetFetchFlags(pfrom), cmpctblock.header.GetHash());
connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv));
- return true;
+ return;
}
BlockTransactionsRequest req;
@@ -3078,7 +3088,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);
@@ -3093,7 +3103,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
std::vector<CInv> vInv(1);
vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom), 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;
@@ -3146,7 +3156,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
MarkBlockAsReceived(pblock->GetHash());
}
}
- return true;
+ return;
}
if (msg_type == NetMsgType::BLOCKTXN)
@@ -3154,7 +3164,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;
@@ -3169,7 +3179,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;
@@ -3177,7 +3187,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 of whitelist
Misbehaving(pfrom.GetId(), 100, strprintf("Peer %d sent us invalid compact block/non-matching block transactions\n", pfrom.GetId()));
- return true;
+ return;
} else if (status == READ_STATUS_FAILED) {
// Might have collided, fall back to getdata now :(
std::vector<CInv> invs;
@@ -3228,7 +3238,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
mapBlockSource.erase(pblock->GetHash());
}
}
- return true;
+ return;
}
if (msg_type == NetMsgType::HEADERS)
@@ -3236,7 +3246,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
// 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;
@@ -3246,7 +3256,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 true;
+ return;
}
headers.resize(nCount);
for (unsigned int n = 0; n < nCount; n++) {
@@ -3262,7 +3272,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>();
@@ -3290,7 +3300,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) {
@@ -3301,18 +3311,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;
@@ -3324,7 +3334,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) {
@@ -3335,7 +3345,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))
@@ -3345,14 +3355,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_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) {
@@ -3373,7 +3383,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
// return very quickly.
connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::PONG, nonce));
}
- return true;
+ return;
}
if (msg_type == NetMsgType::PONG) {
@@ -3429,13 +3439,13 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
if (bPingFinished) {
pfrom.nPingNonceSent = 0;
}
- return true;
+ return;
}
if (msg_type == NetMsgType::FILTERLOAD) {
if (!(pfrom.GetLocalServices() & NODE_BLOOM)) {
pfrom.fDisconnect = true;
- return true;
+ return;
}
CBloomFilter filter;
vRecv >> filter;
@@ -3452,13 +3462,13 @@ 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) {
if (!(pfrom.GetLocalServices() & NODE_BLOOM)) {
pfrom.fDisconnect = true;
- return true;
+ return;
}
std::vector<unsigned char> vData;
vRecv >> vData;
@@ -3480,21 +3490,21 @@ 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.GetLocalServices() & NODE_BLOOM)) {
pfrom.fDisconnect = true;
- return true;
+ return;
}
if (pfrom.m_tx_relay == nullptr) {
- return true;
+ return;
}
LOCK(pfrom.m_tx_relay->cs_filter);
pfrom.m_tx_relay->pfilter = nullptr;
pfrom.m_tx_relay->fRelayTxes = true;
- return true;
+ return;
}
if (msg_type == NetMsgType::FEEFILTER) {
@@ -3507,22 +3517,22 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
}
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) {
@@ -3547,12 +3557,12 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec
}
}
}
- 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)
@@ -3660,11 +3670,8 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
return fMoreWork;
}
- // Process message
- bool fRet = false;
- try
- {
- fRet = ProcessMessage(*pfrom, msg_type, vRecv, msg.m_time, chainparams, m_chainman, m_mempool, connman, m_banman, interruptMsgProc);
+ try {
+ ProcessMessage(*pfrom, msg_type, vRecv, msg.m_time, chainparams, m_chainman, m_mempool, connman, m_banman, interruptMsgProc);
if (interruptMsgProc)
return false;
if (!pfrom->vRecvGetData.empty())
@@ -3675,10 +3682,6 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
LogPrint(BCLog::NET, "%s(%s, %u bytes): Unknown exception caught\n", __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
index 211a84b5f2..2fa751b987 100644
--- a/src/test/fuzz/process_message.cpp
+++ b/src/test/fuzz/process_message.cpp
@@ -30,7 +30,17 @@
#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, 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,
+ CConnman* connman,
+ BanMan* banman,
+ const std::atomic<bool>& interruptMsgProc);
namespace {
@@ -77,7 +87,10 @@ void test_one_input(const std::vector<uint8_t>& buffer)
connman.AddTestNode(p2p_node);
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.connman.get(), 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.connman.get(), g_setup->m_node.banman.get(),
+ std::atomic<bool>{false});
} catch (const std::ios_base::failure&) {
}
SyncWithValidationInterfaceQueue();
--
cgit v1.2.3