From 2f51951d03cc1f8917e0fc931dce674f9bfedaf5 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Mon, 6 Oct 2025 16:07:19 -0400 Subject: [PATCH 1/2] p2p: Add warning message when receiving headers for blocks cached as invalid Currently, if database corruption leads to a block being marked as invalid incorrectly, we can get stuck in an infinite headerssync loop with no indication what went wrong or how to fix it. With the added log message, users will receive an explicit warning after each failed headerssync attempt with an outbound peer. --- src/net_processing.cpp | 7 +++++++ src/validation.cpp | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 9cab2461763..c539f4a5180 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2955,6 +2955,13 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, state, &pindexLast)}; if (!processed) { if (state.IsInvalid()) { + if (!pfrom.IsInboundConn() && state.GetResult() == BlockValidationResult::BLOCK_CACHED_INVALID) { + // Warn user if outgoing peers send us headers of blocks that we previously marked as invalid. + LogWarning("%s (received from peer=%i). " + "If this happens with all peers, consider database corruption (that -reindex may fix) " + "or a potential consensus incompatibility.", + state.GetDebugMessage(), pfrom.GetId()); + } MaybePunishNodeForBlock(pfrom.GetId(), state, via_compact_block, "invalid header received"); return; } diff --git a/src/validation.cpp b/src/validation.cpp index 73b3e91dd20..e1d6babfd0e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4244,7 +4244,8 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida *ppindex = pindex; if (pindex->nStatus & BLOCK_FAILED_MASK) { LogDebug(BCLog::VALIDATION, "%s: block %s is marked invalid\n", __func__, hash.ToString()); - return state.Invalid(BlockValidationResult::BLOCK_CACHED_INVALID, "duplicate-invalid"); + return state.Invalid(BlockValidationResult::BLOCK_CACHED_INVALID, "duplicate-invalid", + strprintf("block %s was previously marked invalid", hash.ToString())); } return true; } From 4b4711369880369729893ba7baef11ba2a36cf4b Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Mon, 6 Oct 2025 15:50:31 -0400 Subject: [PATCH 2/2] validation: Reword CheckForkWarningConditions and call it also during IBD and at startup The existing IBD disable was added at a time when CheckForkWarningConditions did also sophisticated fork detection that could lead to false positives during IBD (55ed3f14751206fc87f0cbf8cb4e223efacef338). The fork detection logic doesn't exist anymore (since fa62304c9760f0de9838e56150008816e7a9bacb), so the IBD check is no longer necessary. Displaying the log at startup will help node operators diagnose the problem better. Also unify log message and alert warning text, since a long invalid chain could be due to chainstate corruption or an actual consensus incompatibility with peers. Previously the log assumed the former and the alert the latter. --- src/validation.cpp | 11 +++++------ test/functional/feature_notifications.py | 3 +-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index e1d6babfd0e..2f71c2c571a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1958,18 +1958,15 @@ void Chainstate::CheckForkWarningConditions() { AssertLockHeld(cs_main); - // Before we get past initial download, we cannot reliably alert about forks - // (we assume we don't get stuck on a fork before finishing our initial sync) - // Also not applicable to the background chainstate - if (m_chainman.IsInitialBlockDownload() || this->GetRole() == ChainstateRole::BACKGROUND) { + if (this->GetRole() == ChainstateRole::BACKGROUND) { return; } if (m_chainman.m_best_invalid && m_chainman.m_best_invalid->nChainWork > m_chain.Tip()->nChainWork + (GetBlockProof(*m_chain.Tip()) * 6)) { - LogWarning("Found invalid chain at least ~6 blocks longer than our best chain. Chain state database corruption likely."); + LogWarning("Found invalid chain more than 6 blocks longer than our best chain. This could be due to database corruption or consensus incompatibility with peers."); m_chainman.GetNotifications().warningSet( kernel::Warning::LARGE_WORK_INVALID_CHAIN, - _("Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade.")); + _("Warning: Found invalid chain more than 6 blocks longer than our best chain. This could be due to database corruption or consensus incompatibility with peers.")); } else { m_chainman.GetNotifications().warningUnset(kernel::Warning::LARGE_WORK_INVALID_CHAIN); } @@ -4638,6 +4635,8 @@ bool Chainstate::LoadChainTip() /*verification_progress=*/m_chainman.GuessVerificationProgress(tip)); } + CheckForkWarningConditions(); + return true; } diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py index 519d80b9abf..c08bf07fb8d 100755 --- a/test/functional/feature_notifications.py +++ b/test/functional/feature_notifications.py @@ -25,8 +25,7 @@ FILE_CHARS_DISALLOWED = '/\\?%*:|"<>' if platform.system() == 'Windows' else '/' UNCONFIRMED_HASH_STRING = 'unconfirmed' LARGE_WORK_INVALID_CHAIN_WARNING = ( - "Warning: We do not appear to fully agree with our peers " # Exclamation mark removed by SanitizeString in AlertNotify - "You may need to upgrade, or other nodes may need to upgrade." + "Warning: Found invalid chain more than 6 blocks longer than our best chain. This could be due to database corruption or consensus incompatibility with peers." )