diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b7d0f5360de..206d7144055 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1202,6 +1202,8 @@ private: void AddAddressKnown(Peer& peer, const CAddress& addr) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); void PushAddress(Peer& peer, const CAddress& addr) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); + + void LogBlockHeader(const CBlockIndex& index, const CNode& peer, bool via_compact_block); }; const CNodeState* PeerManagerImpl::State(NodeId pnode) const @@ -3174,7 +3176,8 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, // Now process all the headers. BlockValidationState state; - if (!m_chainman.ProcessNewBlockHeaders(headers, /*min_pow_checked=*/true, state, &pindexLast)) { + const bool processed = m_chainman.ProcessNewBlockHeaders(headers, /*min_pow_checked=*/true, state, &pindexLast); + if (!processed) { if (state.IsInvalid()) { MaybePunishNodeForBlock(pfrom.GetId(), state, via_compact_block, "invalid header received"); return; @@ -3182,6 +3185,10 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, } assert(pindexLast); + if (processed && received_new_header) { + LogBlockHeader(*pindexLast, pfrom, /*via_compact_block=*/false); + } + // Consider fetching more headers if we are not using our headers-sync mechanism. if (nCount == m_opts.max_headers_result && !have_headers_sync) { // Headers message had its maximum size; the peer may have more headers. @@ -3705,6 +3712,32 @@ void PeerManagerImpl::ProcessCompactBlockTxns(CNode& pfrom, Peer& peer, const Bl return; } +void PeerManagerImpl::LogBlockHeader(const CBlockIndex& index, const CNode& peer, bool via_compact_block) { + // To prevent log spam, this function should only be called after it was determined that a + // header is both new and valid. + // + // These messages are valuable for detecting potential selfish mining behavior; + // if multiple displacing headers are seen near simultaneously across many + // nodes in the network, this might be an indication of selfish mining. + // In addition it can be used to identify peers which send us a header, but + // don't followup with a complete and valid (compact) block. + // Having this log by default when not in IBD ensures broad availability of + // this data in case investigation is merited. + const auto msg = strprintf( + "Saw new %sheader hash=%s height=%d peer=%d%s", + via_compact_block ? "cmpctblock " : "", + index.GetBlockHash().ToString(), + index.nHeight, + peer.GetId(), + fLogIPs ? strprintf(" peeraddr=%s", peer.addr.ToStringAddrPort()) : "" + ); + if (m_chainman.IsInitialBlockDownload()) { + LogDebug(BCLog::VALIDATION, "%s\n", msg); + } else { + LogInfo("%s\n", msg); + } +} + void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, DataStream& vRecv, const std::chrono::microseconds time_received, const std::atomic& interruptMsgProc) @@ -4764,9 +4797,10 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } } + // If AcceptBlockHeader returned true, it set pindex + assert(pindex); if (received_new_header) { - LogInfo("Saw new cmpctblock header hash=%s peer=%d\n", - blockhash.ToString(), pfrom.GetId()); + LogBlockHeader(*pindex, pfrom, /*via_compact_block=*/true); } bool fProcessBLOCKTXN = false; @@ -4782,8 +4816,6 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, { LOCK(cs_main); - // If AcceptBlockHeader returned true, it set pindex - assert(pindex); UpdateBlockAvailability(pfrom.GetId(), pindex->GetBlockHash()); CNodeState *nodestate = State(pfrom.GetId()); diff --git a/src/validation.cpp b/src/validation.cpp index 38fcda55af6..b7c960f306e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4364,23 +4364,6 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida if (ppindex) *ppindex = pindex; - // Since this is the earliest point at which we have determined that a - // header is both new and valid, log here. - // - // These messages are valuable for detecting potential selfish mining behavior; - // if multiple displacing headers are seen near simultaneously across many - // nodes in the network, this might be an indication of selfish mining. Having - // this log by default when not in IBD ensures broad availability of this data - // in case investigation is merited. - const auto msg = strprintf( - "Saw new header hash=%s height=%d", hash.ToString(), pindex->nHeight); - - if (IsInitialBlockDownload()) { - LogPrintLevel(BCLog::VALIDATION, BCLog::Level::Debug, "%s\n", msg); - } else { - LogPrintf("%s\n", msg); - } - return true; } diff --git a/src/validation.h b/src/validation.h index 6bc8a14de95..2eec29b750e 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1220,6 +1220,7 @@ public: * @param[in] min_pow_checked True if proof-of-work anti-DoS checks have been done by caller for headers chain * @param[out] state This may be set to an Error state if any error occurred processing them * @param[out] ppindex If set, the pointer will be set to point to the last new block index object for the given headers + * @returns false if AcceptBlockHeader fails on any of the headers, true otherwise (including if headers were already known) */ bool ProcessNewBlockHeaders(std::span headers, bool min_pow_checked, BlockValidationState& state, const CBlockIndex** ppindex = nullptr) LOCKS_EXCLUDED(cs_main);