From 1cfadfa4e0ddda8e8cfc7dee858b541adfad3813 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Thu, 18 Jan 2024 10:44:40 +0100 Subject: [PATCH] validation: log which peer sent us a header Supports -logips Since 2c3a90f663a61ee147d785167a2902494d81d34d we log received headers. For compact blocks we also log which peer sent it (e5ce8576349d404c466b2f4cab1ca7bf920904b2), but not for regular headers. That required an additional refactor, which this commit provides, to move the logging from validation to net_processing. This also reduces the number of log entries (under default configuration) per compact block header from 3 to 2: one for the header and one for the connected tip. --- src/net_processing.cpp | 42 +++++++++++++++++++++++++++++++++++++----- src/validation.cpp | 17 ----------------- src/validation.h | 1 + 3 files changed, 38 insertions(+), 22 deletions(-) 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);