fix(eth,consensus): gate the proposed-block handler on canonicality and storage

importBlockResults handed the tail of every batch to the proposed-block
handler as long as InsertChain returned nil. A nil error does not mean
the tail is canonical: a fork batch is written as side-chain entries, and
a parked tail is not written at all. The engine did not make up for it:
processQC updates highestQuorumCert, lockQuorumCert and the commit block
before its own existence check, and an existence check cannot tell a
reorged-away block from a canonical one, because the fork stays in the
database. The fetcher and the miner call the same handler and had no gate
at all. The miner is not gated separately: it hands the *core.BlockChain
it already holds to the shared handler, so the engine's canonicality and
storage re-checks cover that path too — a block WriteBlockWithState files
as a side-chain entry is skipped, and no processQC or vote runs on it.
Canonicality is not enough on its own either: it is a property of
the header, and the fast sync header phase marks a height canonical
before its body lands, so a node could processQC and vote for a block
that only ever existed as a header. A master node could therefore end up
voting for, and committing state against, a block it had just reorged
away.

Judge the block once, in one place. consensus.ShouldHandleProposedBlock
reports whether a header is the canonical block at its height and whether
its body is stored, together with the reason it must be skipped and the
canonical hash at that height, both for the skip log. It takes a minimal
CanonicalChain (GetHeaderByNumber alone) that both consensus.ChainReader
and the downloader's BlockChain satisfy, so the callers cannot drift into
two diverging judgments; on the downloader side that costs one addition of
GetHeaderByNumber to its BlockChain interface. The storage half goes
through the optional capability interface consensus.BlockStorer, whose
single method is HasBlock rather than GetBlock: only existence matters,
and GetBlock would read and RLP-decode the whole body of every imported
block. core.BlockChain implements BlockStorer — a compile-time assertion
keeps it from dropping off the type — while a HeaderChain deliberately
does not, so handing a header-only chain to the judgment fails it loudly
as the SkipUnjudgeable skip reason instead of every block silently
failing as not stored. The judgment has no error channel: every outcome
is a skip reason, so each caller collapses to a single !ok branch. The
BlockStorer assertion runs before any chain read, so the unjudgeable
skip is reported at every height: a header-only chain must not get a
silent "not stored" from the canonicality half for the heights it
happens to answer.

The downloader gates importBlockResults on that judgment instead of
open-coding the same rule out of HasBlock and GetCanonicalHash, and logs
the skip with its reason and the canonical hash it observed. The v2 engine
re-checks the same judgment twice inside ProposedBlockHandler: in front of
processQC, and again right before sendVote, because x.lock serialises the
handler but not InsertChain, so a reorg can still land between the two
checks. A judgment that cannot run — a chain type that lost the
BlockStorer half to wrapping or replacement — surfaces as the
SkipUnjudgeable skip, which the handler logs and returns as nil: the
fetcher's import loop treats any handler error as an import failure and
would suppress the broadcast of an already imported block, so a skip
must never surface as an error. That skip means the node stops
processing QCs and voting outright — in production the chain always
implements BlockStorer, so it is a wiring bug. The judgment itself counts
every such skip in a metrics counter (consensus/unjudgeable-proposed-block)
at the single return all callers share, so the liveness halt is observable
beyond the logs on every call site: the downloader's pre-filter hits the
same return before its handler would ever run, where an engine-side-only
counter would stay silent exactly where the wiring bug is easiest to trip.

consensus.SkipLogLevel grades the skips by reason:
SkipUnjudgeable is the one Error — not an observation about the block but
a wiring bug that will skip every block — SkipNonCanonical is a genuine
reorg race and stays at Warn, while the sync-phase skips are routine and
stay at Info so they do not drown the level in noise. The downloader's
own skip log stays at Info on purpose: it is the pre-filter for the
routine cases and correctness rests on the handler's re-checks, so
grading it too would fire a Warn for every fork tail of a sync; its
unjudgeable branch is an explicit Error instead of SkipLogLevel.

The engine's two re-checks now share one counter of their own
(consensus/skipped-proposed-block), incremented at the gate that declined
the block — at most once per block, since a first-gate skip returns
before the second gate. It deliberately counts only the engine gates and
not the downloader's pre-filter: fork tails arriving there are routine
sync noise logged at Info, while a block that got past the pre-filter and
was still declined is exactly the anomaly logs alone could bury. The
reason stays in the skip log; one counter for all reasons keeps the
metric surface flat. And because the same proposal can reach the shared
judgment through both the fetcher and the downloader paths, the
unjudgeable counter's increments are judgment counts, not deduplicated
blocks — its comment says so now. The window comment also names what the
second check leaves open: sendVote itself reads the chain again via
getEpochSwitchInfo before signing and broadcasting, so the residual
window spans that read plus the signature and the broadcast. The fetcher
gate's comment now cross-references the engine side — canonicality is
covered there, so the two gates interlock rather than duplicate — and
points at the post-pivot full import in processFastSyncContent, the
downloader call site the ungated closure's justification rests on.

The fetcher is the one caller whose fast sync calls must not reach the
handler at all. While snapSync runs, it discards propagated blocks before
executing them, so a body already written by the fast sync receipt phase
would pass both halves of the judgment — canonical and stored — and drive
processQC and the vote path on a block whose state transition was never
validated. Its callback therefore carries the same snapSync guard the
inserter and prepare closures already have, instead of asking the
judgment a question the judgment cannot answer. The guard sits in a
fetcher-specific wrapper and not in the shared closure: the same closure
also feeds the downloader, whose fast sync handler calls run after the
pivot commit, on blocks InsertChain has fully executed, and snapSync
stays set until the whole Synchronise returns. Behind that flag the
callback also requires the block's full state, keyed by the block hash
(HasBlockAndFullState) rather than the state root: an empty block's root
can repeat its parent's, so a root-keyed check would leak the parent's
executed state to a block whose import was discarded. Unlike the
judgment's storage half — which reads HasBlock to avoid decoding whole
bodies — this check deliberately pays HasBlockAndFullState's GetBlock
body read plus OpenTrie: proposed blocks arrive at consensus cadence,
orders of magnitude rarer than sync-path blocks, so the fuller check is
affordable here and closes the unexecuted-state hole the cheap HasBlock
check would leave open. Its skips are graded by what they mean: the
fast-sync flag skip is routine and stays at Debug, but a state-half skip
still halts QC processing and voting for that block, so it logs at Warn
and increments its own counter (eth/skipped-proposed-block-state)
— the same logs-alone-could-bury-it discipline as the unjudgeable
counter, so a persistent post-sync liveness stall is observable beyond
the logs.

Tests:

- TestShouldHandleProposedBlock covers the five outcomes of the shared
  judgment plus the header-only chain at an absent height, whose
  unjudgeable skip is the point of the entry-side assertion; the two
  storerless cases also pin that the judgment increments
  unjudgeableProposedBlock exactly once per skip, and
  TestHeaderChainShouldHandleProposedBlock pins that the header chain's
  interface stubs never pass it at any height.
- TestProposedBlockHandlerSkipsNonCanonicalBlock,
  TestProposedBlockHandlerSkipsReorgedBlockBeforeProcessQC,
  TestProposedBlockHandlerDropsVoteForReorgedBlock and
  TestProposedBlockHandlerSkipsBlockWithoutBody cover the two engine
  re-checks: highestQuorumCert, lockQC, the timeout certificate, the
  voted round and the commit block must stay untouched and no vote may be
  broadcast. The two reorg tests inject the reorg through a ChainReader
  wrapper that serves a fork header once N judgment gates have completed —
  anchored on the gate boundary, where each gate that passes the
  canonicality check ends in exactly one HasBlock of the watched height —
  so reads added in front of or between the handler's gates stay truthful
  and cannot move where the injection lands, instead of coupling the test
  to the handler's read ordinals.
- TestProposedBlockHandlerGradesSkipLogLevelByReason pins the log level
  of each skip reason.
- TestProposedBlockHandlerSkipsUnjudgeableChain feeds the handler a
  ChainReader wrapper without the BlockStorer half — the shape a wrapped
  or replaced chain type presents — and pins that the chain skips as
  SkipUnjudgeable, logged at Error and returned as nil, not surfaced to a
  caller that would treat it as an import failure. No vote may be
  broadcast either: that assertion waits out a two-second timeout window
  rather than checking BroadcastCh non-blockingly, because a vote travels
  through broadcastToBftChannel's own goroutine and an immediate
  default-branch check would pass vacuously even if a vote were sent.
- TestFetcherSkipsProposedBlockHandlerDuringFastSync pins the fetcher
  gate: with snapSync set the fetcher's callback is inert and logs the
  skip, and the same call in full sync reaches the closure without the
  skip log. It also reads the handler back off the fetcher through the
  test-only HandleProposedBlock accessor and requires it to be the gated
  closure itself, so swapping the wiring back to the bare
  handleProposedBlock fails loudly instead of staying green. The
  state-half skip is pinned with a block forged for real: its header and
  body are written straight into the database via rawdb so the
  hash-keyed GetBlock half finds it, while its root exists nowhere in
  the state database so HasFullState's OpenTrie is the half that fails —
  mutating the current header's root alone would change its hash and
  fail at the GetBlock half instead, never exercising the state check —
  and the counter's one increment per skip is asserted.
- TestImportBlockResultsProposedBlockHandler covers six downloader
  shapes: a parked tail, a fully imported batch, a stored fork batch
  re-delivered after the local chain grew past it, a head advanced past
  the canonical tail by a concurrent import, a heavier fork that stays
  canonical, and a fast sync height that is canonical without a body.
- downloadTester gains a canonical number-to-hash table, picked by total
  difficulty the way the real chain resolves a reorg, plus hooks to park
  a batch tail and to extend it after the insert.
  TestStoreBlockCleansStaleCanonicalMarkers,
  TestRollbackClearsCanonicalMarkers and
  TestInsertChainErrorReportsPosition pin the parts of that table the
  downloader tests rely on.
- TestShouldNotSendVoteMsgIfBlockNotExtendedFromAncestor no longer
  proposes a forked block, which the new entry re-check now
  short-circuits; it proposes a canonical block below the locked ancestor
  instead. The fork case moved to
  TestShouldNotSendVoteMsgIfCanonicalBlockNotExtendedFromForkedAncestor,
  where the parent walk of isExtendingFromAncestor actually runs.

Author: gzliudan

consensus/XDPoS/engines/engine_v2/engine.go

@@ -27,11 +27,27 @@ import (
 	"github.com/XinFinOrg/XDPoSChain/core/vm"
 	"github.com/XinFinOrg/XDPoSChain/ethdb"
 	"github.com/XinFinOrg/XDPoSChain/log"
+	"github.com/XinFinOrg/XDPoSChain/metrics"
 	"github.com/XinFinOrg/XDPoSChain/params"
 	"github.com/XinFinOrg/XDPoSChain/trie"
 	"golang.org/x/sync/errgroup"
 )
 
+// skippedProposedBlock counts skips at this engine's two gates (before
+// processQC and before sendVote): the judgment declined a block that had
+// already reached the consensus handler, so the node does not process QC
+// or vote on it. The downloader's pre-filter runs the same judgment on
+// routine sync fork tails and logs those at Info without counting — that
+// noise is expected — while these gates only fire for a block that got
+// past the pre-filter and was still declined, so a persistently growing
+// counter is a liveness stall, the same "logs alone could bury it"
+// discipline as unjudgeableProposedBlock and the fetcher's
+// eth/skipped-proposed-block-state. A single block is counted at most once
+// here: a first-gate skip returns before the second gate. The reason
+// stays in the skip log; one counter for all reasons keeps the metric
+// surface flat.
+var skippedProposedBlock = metrics.NewRegisteredCounter("consensus/skipped-proposed-block", nil)
+
 type XDPoS_v2 struct {
 	chainConfig *params.ChainConfig // Chain & network configuration
 
@@ -834,6 +850,36 @@ func (x *XDPoS_v2) ProposedBlockHandler(chain consensus.ChainReader, blockHeader
 		return err
 	}
 
+	// Re-check canonicality and storage at two points. x.lock only serializes
+	// this handler, while the chain is written under the import lock of
+	// BlockChain.InsertChain, so the callers' own gates (downloader,
+	// fetcher, miner) cannot make these checks atomic with a write: a
+	// concurrent reorg can land anywhere in between. The first check
+	// directly protects processQC, which updates highestQuorumCert,
+	// lockQuorumCert and the commit block before its own existence check;
+	// the second check right before sendVote keeps the unguarded window of
+	// the vote down to the broadcast itself. An existence check cannot tell
+	// a reorged-away block from a canonical one, since a fork stays in the
+	// database as side entries.
+	ok, reason, canonicalHash := consensus.ShouldHandleProposedBlock(chain, blockHeader)
+	if !ok {
+		skippedProposedBlock.Inc(1)
+		consensus.SkipLogLevel(reason)("[ProposedBlockHandler] skip block before processQC", "reason", reason, "hash", blockHeader.Hash(), "number", blockHeader.Number, "canonicalHash", canonicalHash)
+		// An unjudgeable skip here is the node stopping voting: with the BlockStorer
+		// half off the chain type, every proposed block lands in the judgment's
+		// SkipUnjudgeable return and processQC and sendVote never run again. In
+		// production the chain always implements BlockStorer, so this is a wiring
+		// bug (a wrapped or replaced chain type) most likely introduced by a
+		// refactor. The judgment itself counts it in the unjudgeableProposedBlock
+		// metric — shared with the downloader's pre-filter, which hits the same
+		// return before its handler would ever run — so the liveness halt is
+		// observable beyond the logs on every call site.
+		// Return nil, never an error: the fetcher's import loop treats any
+		// handler error as an import failure and suppresses the broadcast
+		// of an already imported block.
+		return nil
+	}
+
 	// Generate blockInfo
 	blockInfo := &types.BlockInfo{
 		Hash:   blockHeader.Hash(),
@@ -856,6 +902,15 @@ func (x *XDPoS_v2) ProposedBlockHandler(chain consensus.ChainReader, blockHeader
 		return err
 	}
 	if verified {
+		// x.lock does not block InsertChain, so a reorg can still land
+		// between the processQC re-check and the broadcast; drop the vote if
+		// the block has been reorged away meanwhile.
+		ok, reason, canonicalHash = consensus.ShouldHandleProposedBlock(chain, blockHeader)
+		if !ok {
+			skippedProposedBlock.Inc(1)
+			consensus.SkipLogLevel(reason)("[ProposedBlockHandler] skip vote for reorged block", "reason", reason, "hash", blockHeader.Hash(), "number", blockHeader.Number, "canonicalHash", canonicalHash)
+			return nil
+		}
 		return x.sendVote(chain, blockInfo)
 	}
 

consensus/XDPoS/engines/engine_v2/vote_test.go

@@ -122,3 +122,63 @@ func TestVerifyVoteMessage_VoteRoundTooOld(t *testing.T) {
 	assert.False(t, verified, "Should return false for vote with round < currentRound")
 	assert.NoError(t, err, "Should not return an error for old round votes")
 }
+
+// blockInfoOf turns a header into the BlockInfo shape the voting rule and
+// forensics pass around. The round is irrelevant to isExtendingFromAncestor,
+// which only walks hashes and numbers.
+func blockInfoOf(h *types.Header) *types.BlockInfo {
+	return &types.BlockInfo{Hash: h.Hash(), Number: h.Number}
+}
+
+// TestIsExtendingFromAncestor covers the parent walk of the HotStuff voting
+// rule at the rule layer. The handler-level tests cannot reach the positive
+// branch anymore: since ProposedBlockHandler gates on canonicality, a
+// proposed block on the locked ancestor's own chain that passes the gate
+// always outranks the lockQC round and returns before the walk (see
+// TestShouldNotSendVoteMsgIfCanonicalBlockNotExtendedFromForkedAncestor),
+// and the forensics caller's positive path is only exercised by a skipped
+// test. Both branches of the walk are safety-critical — a false positive
+// lets a node vote off the locked chain, a false negative stalls it — so
+// the walk itself gets direct coverage here.
+func TestIsExtendingFromAncestor(t *testing.T) {
+	// 1 <- 2 <- 3, with 2' a same-height fork of 2.
+	mockChain := NewMockChainReader()
+	h1 := &types.Header{Number: big.NewInt(1)}
+	h2 := &types.Header{Number: big.NewInt(2), ParentHash: h1.Hash()}
+	h3 := &types.Header{Number: big.NewInt(3), ParentHash: h2.Hash()}
+	forkH2 := &types.Header{Number: big.NewInt(2), ParentHash: h1.Hash(), Coinbase: common.BytesToAddress([]byte{0x02})}
+	for _, h := range []*types.Header{h1, h2, h3} {
+		mockChain.AddHeader(h)
+	}
+	engine := &XDPoS_v2{}
+
+	// Positive branch: the walk runs two hops down the parent chain and
+	// lands exactly on the locked ancestor.
+	extended, err := engine.isExtendingFromAncestor(mockChain, blockInfoOf(h3), blockInfoOf(h1))
+	assert.NoError(t, err)
+	assert.True(t, extended, "h3 extends the locked ancestor h1")
+
+	// Negative branch with the walk executed: h3's parent chain bottoms out
+	// at h1, not at the forked ancestor 2', so the final hash comparison
+	// rejects the block. This is the geometry
+	// TestShouldNotSendVoteMsgIfCanonicalBlockNotExtendedFromForkedAncestor
+	// exercises through verifyVotingRule.
+	extended, err = engine.isExtendingFromAncestor(mockChain, blockInfoOf(h3), blockInfoOf(forkH2))
+	assert.NoError(t, err)
+	assert.False(t, extended, "h3 does not extend the forked ancestor 2'")
+
+	// Zero-iteration mismatch: the proposed block sits below the locked
+	// ancestor, so the walk never runs and only the direct hash comparison
+	// can reject it. This is the geometry of
+	// TestShouldNotSendVoteMsgIfBlockNotExtendedFromAncestor.
+	extended, err = engine.isExtendingFromAncestor(mockChain, blockInfoOf(h1), blockInfoOf(h2))
+	assert.NoError(t, err)
+	assert.False(t, extended, "h1 is below the locked ancestor h2")
+
+	// A missing parent aborts the walk with an error instead of silently
+	// reporting false: the proposed block's own header is not in the chain.
+	missing := &types.BlockInfo{Hash: common.StringToHash("missing"), Number: big.NewInt(3)}
+	extended, err = engine.isExtendingFromAncestor(mockChain, missing, blockInfoOf(h1))
+	assert.Error(t, err)
+	assert.False(t, extended)
+}

consensus/proposed_block.go

@@ -0,0 +1,177 @@
+// Copyright 2026 The go-ethereum Authors
+// This file is part of the go-ethereum library.
+//
+// The go-ethereum library is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// The go-ethereum library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
+
+package consensus
+
+import (
+	"github.com/XinFinOrg/XDPoSChain/common"
+	"github.com/XinFinOrg/XDPoSChain/core/types"
+	"github.com/XinFinOrg/XDPoSChain/log"
+	"github.com/XinFinOrg/XDPoSChain/metrics"
+)
+
+// unjudgeableProposedBlock counts the proposed-block judgment skips caused by
+// a chain whose type hides the BlockStorer half (SkipUnjudgeable). The counter
+// lives here, at the single judgment site every caller shares, so the liveness
+// halt is observable on all of them: the downloader's pre-filter hits the same
+// return before its handler would ever run, and an engine-side-only counter
+// would stay silent exactly where the wiring bug is easiest to trip. In
+// production the chain always implements the interface, so a non-zero value
+// is a wiring bug — a wrapped or replaced chain type — that skips every
+// proposed block, halting QC processing and voting; logs alone could bury it.
+// Each increment is a judgment, not a block: the same proposal can reach the
+// judgment through both the fetcher and the downloader paths, so interpret
+// the counter as judgment counts, not deduplicated blocks.
+var unjudgeableProposedBlock = metrics.NewRegisteredCounter("consensus/unjudgeable-proposed-block", nil)
+
+// CanonicalChain is the subset of ChainReader needed for the canonicality half
+// of the judgment whether a proposed block should be handled by the consensus
+// engine.
+type CanonicalChain interface {
+	// GetHeaderByNumber retrieves a block header from the database by number.
+	GetHeaderByNumber(number uint64) *types.Header
+}
+
+// BlockStorer is the optional capability the judgment needs for its storage
+// half: whether a block is present in the database. Chains that only carry
+// headers (e.g. *core.HeaderChain) deliberately do not implement it — handing
+// one to ShouldHandleProposedBlock surfaces as a SkipUnjudgeable skip, graded
+// Error, instead of every block silently failing as "not stored".
+type BlockStorer interface {
+	// HasBlock reports whether a block with the given hash and number is
+	// stored in the database.
+	HasBlock(hash common.Hash, number uint64) bool
+}
+
+// SkipReason describes why a proposed block header was rejected by
+// ShouldHandleProposedBlock. The empty value means the header was accepted.
+type SkipReason string
+
+// The reasons ShouldHandleProposedBlock can report for rejecting a header.
+const (
+	// SkipNoCanonicalHeader: no canonical header exists at the height.
+	SkipNoCanonicalHeader SkipReason = "no canonical header at height"
+
+	// SkipNonCanonical: the header is not the canonical block at its height.
+	SkipNonCanonical SkipReason = "non-canonical"
+
+	// SkipBodyNotStored: the canonical block's body has not landed in the
+	// database yet, e.g. during the fast sync header phase.
+	SkipBodyNotStored SkipReason = "block body not stored"
+
+	// SkipUnjudgeable: the chain does not implement BlockStorer, so the
+	// judgment cannot run at all. It is not a judgment about the block but
+	// about the chain: in production the node's chain always implements the
+	// interface, so this reason means a wiring bug — a wrapped or replaced
+	// chain type — and the node will skip every block, stopping processQC
+	// and voting outright. That liveness halt is why it is graded Error.
+	SkipUnjudgeable SkipReason = "unjudgeable"
+)
+
+// ShouldHandleProposedBlock reports whether a proposed block header should
+// reach the consensus handler, together with the reason it should not and the
+// canonical hash at its height, both meant for skip logging. The header must
+// be the canonical block at its height — a fork stays in the database as a
+// side entry, so a mere existence check cannot tell a reorged-away block from
+// a canonical one — and its body must be stored, since the fast sync header
+// phase marks a height canonical before its body lands. HasBlock keeps the
+// storage check off the full-block read path: GetBlock would pull and decode
+// the whole body on every judge call, while only its existence matters here.
+// The skip reason is one of the exported SkipReason constants; the empty
+// reason means the header was accepted.
+// The two chain reads are not atomic: a concurrent reorg can land between
+// GetHeaderByNumber and the storage check, and again between a caller's
+// checks and its effects on chain state. The v2 engine re-checks before
+// processQC and before sendVote; what window remains is recorded there:
+// after the second check, sendVote itself reads the chain again via
+// getEpochSwitchInfo before signing and broadcasting, so the residual
+// window spans that read plus the signature and the broadcast.
+// A chain that does not implement BlockStorer cannot be judged at all:
+// rather than guessing and silently dropping QC processing and voting for
+// every block, the judgment fails it loudly as SkipUnjudgeable. The
+// BlockStorer assertion runs before any chain read, so the skip is
+// reported at every height — a header-only chain must not get a silent
+// "not stored" from the canonicality half for heights it happens to answer.
+// Every SkipUnjudgeable skip increments the unjudgeableProposedBlock counter
+// here — the one point all callers share — so the wiring-bug liveness halt is
+// observable even where a caller (the downloader pre-filter) never reaches
+// its handler.
+func ShouldHandleProposedBlock(chain CanonicalChain, header *types.Header) (bool, SkipReason, common.Hash) {
+	storer, ok := chain.(BlockStorer)
+	if !ok {
+		// The single inc point for the wiring-bug skip: every caller (downloader
+		// pre-filter, engine gates) funnels through this return, so the counter
+		// observes the liveness halt on all of them. See unjudgeableProposedBlock.
+		unjudgeableProposedBlock.Inc(1)
+		return false, SkipUnjudgeable, common.Hash{}
+	}
+	canonical := chain.GetHeaderByNumber(header.Number.Uint64())
+	if canonical == nil {
+		return false, SkipNoCanonicalHeader, common.Hash{}
+	}
+	if canonical.Hash() != header.Hash() {
+		return false, SkipNonCanonical, canonical.Hash()
+	}
+	if !storer.HasBlock(header.Hash(), header.Number.Uint64()) {
+		return false, SkipBodyNotStored, canonical.Hash()
+	}
+	return true, "", canonical.Hash()
+}
+
+// SkipLogLevel maps a skip reason to the level its skip should be logged at.
+// Grade the skip by what the reason means once the handler is reached.
+// SkipUnjudgeable is the one Error: it is not a judgment about the block
+// but a wiring bug that will skip every block and halt QC processing and
+// voting, so it must surface beyond the Info/Warn discipline below.
+// SkipNonCanonical means another block already claims this height, and
+// SkipNoCanonicalHeader means no canonical marker exists at all: the fast
+// sync header phase marks heights canonical, so a marker can only be
+// missing post-sync — a fork growing above the local head, a reorged-away
+// tip being re-delivered, or a reorg racing the handler between its
+// checks. Both are reorg-race observations a Warn is reserved for.
+// SkipBodyNotStored is the one routine skip instead: the fast sync header
+// phase marks a height canonical before its body lands, and a Warn per
+// header-only height would drown the level in noise. Info for the
+// expected, Warn for the anomalous — the same discipline the downloader's
+// expected fork-tail skip already follows. The empty reason means the
+// header was accepted and should never reach a skip log; a caller passing
+// it has misused this helper, so it grades like the routine Info skips
+// rather than inflating the Warn level reserved for anomalies. Any other
+// unregistered reason is graded Warn on purpose: a skip reason that was
+// never classified is most likely a future anomalous one whose registration
+// was forgotten, and an Info would bury it exactly where it hurts most.
+// SkipUnjudgeable, finally, is graded Error and sits outside the Info/Warn
+// discipline: it is not an observation about the block at all.
+func SkipLogLevel(reason SkipReason) func(msg string, ctx ...interface{}) {
+	switch reason {
+	case SkipUnjudgeable:
+		// Not an observation about the block but a wiring bug that skips
+		// every block and halts QC processing and voting — a liveness
+		// incident that must be surfaced at Error.
+		return log.Error
+	case SkipNonCanonical, SkipNoCanonicalHeader:
+		return log.Warn
+	case "":
+		// The empty reason means "accepted" (see SkipReason); grade a
+		// misused accept like the routine skips, not like the anomalies.
+		return log.Info
+	case SkipBodyNotStored:
+		return log.Info
+	default:
+		// Unregistered reason: grade it as the anomaly it most likely is.
+		return log.Warn
+	}
+}