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

consensus/XDPoS/api_test.go

@@ -101,6 +101,10 @@ func (m *configChainMock) GetBlock(hash common.Hash, number uint64) *types.Block
 	return types.NewBlockWithHeader(header)
 }
 
+func (m *configChainMock) HasBlock(hash common.Hash, number uint64) bool {
+	return m.GetBlock(hash, number) != nil
+}
+
 var _ consensus.ChainReader = (*configChainMock)(nil)
 
 func TestCalculateSignersVote(t *testing.T) {

consensus/XDPoS/engines/engine_v2/engine.go

@@ -834,6 +834,27 @@ 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, err := consensus.ShouldHandleProposedBlock(chain, blockHeader)
+	if err != nil {
+		log.Error("[ProposedBlockHandler] cannot judge proposed block", "err", err, "number", blockHeader.Number, "hash", blockHeader.Hash())
+		return err
+	}
+	if !ok {
+		consensus.SkipLogLevel(reason)("[ProposedBlockHandler] skip block before processQC", "reason", reason, "hash", blockHeader.Hash(), "number", blockHeader.Number, "canonicalHash", canonicalHash)
+		return nil
+	}
+
 	// Generate blockInfo
 	blockInfo := &types.BlockInfo{
 		Hash:   blockHeader.Hash(),
@@ -856,6 +877,18 @@ 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, err = consensus.ShouldHandleProposedBlock(chain, blockHeader)
+		if err != nil {
+			log.Error("[ProposedBlockHandler] cannot judge proposed block", "err", err, "number", blockHeader.Number, "hash", blockHeader.Hash())
+			return err
+		}
+		if !ok {
+			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/snapshot_test.go

@@ -109,6 +109,7 @@ func (c *repairTestChain) GetHeaderByNumber(number uint64) *types.Header {
 func (c *repairTestChain) GetHeader(common.Hash, uint64) *types.Header { return nil }
 func (c *repairTestChain) GetHeaderByHash(common.Hash) *types.Header   { return nil }
 func (c *repairTestChain) GetBlock(common.Hash, uint64) *types.Block   { return nil }
+func (c *repairTestChain) HasBlock(common.Hash, uint64) bool           { return false }
 
 func (c *repairTestChain) StateAt(common.Hash) (*state.StateDB, error) {
 	c.stateCalls++

consensus/XDPoS/engines/engine_v2/vote_test.go

@@ -95,6 +95,10 @@ func (m *MockChainReader) GetBlock(hash common.Hash, number uint64) *types.Block
 	return nil
 }
 
+func (m *MockChainReader) HasBlock(hash common.Hash, number uint64) bool {
+	return false
+}
+
 // TestVerifyVoteMessage_VoteRoundTooOld tests that votes with rounds below
 // the current round are rejected immediately
 func TestVerifyVoteMessage_VoteRoundTooOld(t *testing.T) {
@@ -122,3 +126,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/XDPoS/verify_chain_reader_test.go

@@ -59,6 +59,14 @@ func (s *stubChainReader) GetBlock(hash common.Hash, number uint64) *types.Block
 	return s.blocksByHashNo[blockKey{hash: hash, number: number}]
 }
 
+func (s *stubChainReader) HasBlock(hash common.Hash, number uint64) bool {
+	if s.blocksByHashNo == nil {
+		return false
+	}
+	_, ok := s.blocksByHashNo[blockKey{hash: hash, number: number}]
+	return ok
+}
+
 func TestNewVerifyChainReaderWithNilChainReturnsNilSafeReader(t *testing.T) {
 	reader := NewVerifyHeadersChainReader(nil, []*types.Header{{Number: big.NewInt(1)}}, nil).(*verifyChainReader)
 	assert.NotNil(t, reader)

consensus/proposed_block.go

@@ -0,0 +1,132 @@
+// 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 (
+	"fmt"
+
+	"github.com/XinFinOrg/XDPoSChain/common"
+	"github.com/XinFinOrg/XDPoSChain/core/types"
+	"github.com/XinFinOrg/XDPoSChain/log"
+)
+
+// 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 an 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"
+)
+
+// 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.
+// A non-nil error means the chain does not implement BlockStorer, so the
+// storage half cannot run at all: rather than guessing and silently dropping
+// QC processing and voting for every block, the caller is told outright.
+func ShouldHandleProposedBlock(chain CanonicalChain, header *types.Header) (bool, SkipReason, common.Hash, error) {
+	canonical := chain.GetHeaderByNumber(header.Number.Uint64())
+	if canonical == nil {
+		return false, SkipNoCanonicalHeader, common.Hash{}, nil
+	}
+	if canonical.Hash() != header.Hash() {
+		return false, SkipNonCanonical, canonical.Hash(), nil
+	}
+	storer, ok := chain.(BlockStorer)
+	if !ok {
+		return false, "", common.Hash{}, fmt.Errorf("chain %T does not implement consensus.BlockStorer; the storage half of the proposed-block judgment cannot run", chain)
+	}
+	if !storer.HasBlock(header.Hash(), header.Number.Uint64()) {
+		return false, SkipBodyNotStored, canonical.Hash(), nil
+	}
+	return true, "", canonical.Hash(), nil
+}
+
+// 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.
+// 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.
+func SkipLogLevel(reason SkipReason) func(msg string, ctx ...interface{}) {
+	switch reason {
+	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
+	}
+}