From 958aef21b0097f0ba4f0698b8a1328d097471679 Mon Sep 17 00:00:00 2001 From: Daniel Liu <139250065@qq.com> Date: Mon, 16 Mar 2026 16:16:35 +0800 Subject: [PATCH] fix(consensus/XDPoS): fix mixed v1/v2 VerifyHeaders ancestor lookup and result ordering, fix XFN-12 Background: Mixed v1/v2 VerifyHeaders batches could fail with ErrUnknownAncestor when the first v2 header depended on an in-flight parent header not yet persisted to DB. Root cause: - In mixed batches, v2 ancestor lookup could miss parents that existed only in the input batch. - The adaptor previously let EngineV1 and EngineV2 write to a shared results channel concurrently, creating ordering/mapping ambiguity. Changes: - Add verifyChainReader to overlay in-batch headers/blocks for GetHeader/GetHeaderByNumber/GetHeaderByHash/GetBlock, with fallback to canonical chain data. - Use verifyChainReader in mixed v1/v2 VerifyHeaders paths. - Split mixed verification into per-engine result channels and fan-in deterministically (all v1 first, then all v2). - Make newVerifyChainReader always return a non-nil, nil-safe reader. Tests: - Add verify_chain_reader unit tests for: - number lookup shadowing, - parent resolution from in-batch headers, - nil-safe constructor behavior, - mixed nil-chain no-panic, - deterministic mixed result order (v1 then v2). - Add engine_v2 regression test to verify mixed headers pass even when GetHeader(parentHash, number) is masked. Impact: Fixes XFN-12 and stabilizes mixed-batch verification behavior across v1->v2 boundary handling and result emission. --- consensus/XDPoS/XDPoS.go | 63 ++++++-- consensus/XDPoS/verify_chain_reader.go | 99 ++++++++++++ consensus/XDPoS/verify_chain_reader_test.go | 143 ++++++++++++++++++ .../engine_v2_tests/verify_header_test.go | 49 ++++++ 4 files changed, 340 insertions(+), 14 deletions(-) create mode 100644 consensus/XDPoS/verify_chain_reader.go create mode 100644 consensus/XDPoS/verify_chain_reader_test.go diff --git a/consensus/XDPoS/XDPoS.go b/consensus/XDPoS/XDPoS.go index 0edf37312005..cd8284597533 100644 --- a/consensus/XDPoS/XDPoS.go +++ b/consensus/XDPoS/XDPoS.go @@ -210,33 +210,68 @@ func (x *XDPoS) VerifyHeader(chain consensus.ChainReader, header *types.Header, // VerifyHeaders is similar to VerifyHeader, but verifies a batch of headers. The // method returns a quit channel to abort the operations and a results channel to -// retrieve the async verifications (the order is that of the input slice). +// retrieve the async verifications. For mixed v1/v2 inputs, results are emitted +// in deterministic consensus order: all v1 results first, then all v2 results. func (x *XDPoS) VerifyHeaders(chain consensus.ChainReader, headers []*types.Header, fullVerifies []bool) (chan<- struct{}, <-chan error) { abort := make(chan struct{}) results := make(chan error, len(headers)) // Split the headers list into v1 and v2 buckets - var v1headers []*types.Header - var v2headers []*types.Header - v1fullVerifies := make([]bool, 0, len(headers)) - v2fullVerifies := make([]bool, 0, len(headers)) + var v1Headers []*types.Header + var v2Headers []*types.Header + v1FullVerifies := make([]bool, 0, len(headers)) + v2FullVerifies := make([]bool, 0, len(headers)) for i, header := range headers { switch x.config.BlockConsensusVersion(header.Number) { case params.ConsensusEngineVersion2: - v2headers = append(v2headers, header) - v2fullVerifies = append(v2fullVerifies, fullVerifies[i]) + v2Headers = append(v2Headers, header) + v2FullVerifies = append(v2FullVerifies, fullVerifies[i]) default: // Default "v1" - v1headers = append(v1headers, header) - v1fullVerifies = append(v1fullVerifies, fullVerifies[i]) + v1Headers = append(v1Headers, header) + v1FullVerifies = append(v1FullVerifies, fullVerifies[i]) } } - if v1headers != nil { - x.EngineV1.VerifyHeaders(chain, v1headers, v1fullVerifies, abort, results) - } - if v2headers != nil { - x.EngineV2.VerifyHeaders(chain, v2headers, v2fullVerifies, abort, results) + v1Count := len(v1Headers) + v2Count := len(v2Headers) + if v1Count != 0 && v2Count == 0 { + x.EngineV1.VerifyHeaders(chain, v1Headers, v1FullVerifies, abort, results) + } else if v1Count == 0 && v2Count != 0 { + x.EngineV2.VerifyHeaders(chain, v2Headers, v2FullVerifies, abort, results) + } else if v1Count != 0 && v2Count != 0 { + v1Results := make(chan error, v1Count) + v2Results := make(chan error, v2Count) + verifyChain := newVerifyChainReader(chain, headers) + x.EngineV1.VerifyHeaders(verifyChain, v1Headers, v1FullVerifies, abort, v1Results) + x.EngineV2.VerifyHeaders(verifyChain, v2Headers, v2FullVerifies, abort, v2Results) + + go func() { + for range v1Count { + select { + case <-abort: + return + case err := <-v1Results: + select { + case <-abort: + return + case results <- err: + } + } + } + for range v2Count { + select { + case <-abort: + return + case err := <-v2Results: + select { + case <-abort: + return + case results <- err: + } + } + } + }() } return abort, results diff --git a/consensus/XDPoS/verify_chain_reader.go b/consensus/XDPoS/verify_chain_reader.go new file mode 100644 index 000000000000..ccd35d1f208d --- /dev/null +++ b/consensus/XDPoS/verify_chain_reader.go @@ -0,0 +1,99 @@ +package XDPoS + +import ( + "github.com/XinFinOrg/XDPoSChain/common" + "github.com/XinFinOrg/XDPoSChain/consensus" + "github.com/XinFinOrg/XDPoSChain/core/types" + "github.com/XinFinOrg/XDPoSChain/params" +) + +// verifyChainReader shadows chain lookups with headers in the current verify batch. +// This keeps verification deterministic when deep consensus paths query ancestors +// that are in-flight and not written to DB yet. +type verifyChainReader struct { + chain consensus.ChainReader + headersByHash map[common.Hash]*types.Header + headersByNumber map[uint64]*types.Header + blocksByHashNo map[hashAndNumber]*types.Block +} + +type hashAndNumber struct { + hash common.Hash + number uint64 +} + +func newVerifyChainReader(chain consensus.ChainReader, headers []*types.Header) *verifyChainReader { + reader := &verifyChainReader{ + chain: chain, + headersByHash: make(map[common.Hash]*types.Header, len(headers)), + headersByNumber: make(map[uint64]*types.Header, len(headers)), + blocksByHashNo: make(map[hashAndNumber]*types.Block, len(headers)), + } + for _, header := range headers { + if header == nil || header.Number == nil { + continue + } + h := header.Hash() + n := header.Number.Uint64() + reader.headersByHash[h] = header + if _, exists := reader.headersByNumber[n]; !exists { + reader.headersByNumber[n] = header + } + reader.blocksByHashNo[hashAndNumber{hash: h, number: n}] = types.NewBlockWithHeader(header) + } + return reader +} + +func (r *verifyChainReader) Config() *params.ChainConfig { + if r.chain == nil { + return nil + } + return r.chain.Config() +} + +func (r *verifyChainReader) CurrentHeader() *types.Header { + if r.chain == nil { + return nil + } + return r.chain.CurrentHeader() +} + +func (r *verifyChainReader) GetHeader(hash common.Hash, number uint64) *types.Header { + if header, ok := r.headersByHash[hash]; ok && header.Number != nil && header.Number.Uint64() == number { + return header + } + if r.chain == nil { + return nil + } + return r.chain.GetHeader(hash, number) +} + +func (r *verifyChainReader) GetHeaderByNumber(number uint64) *types.Header { + if header, ok := r.headersByNumber[number]; ok { + return header + } + if r.chain == nil { + return nil + } + return r.chain.GetHeaderByNumber(number) +} + +func (r *verifyChainReader) GetHeaderByHash(hash common.Hash) *types.Header { + if header, ok := r.headersByHash[hash]; ok { + return header + } + if r.chain == nil { + return nil + } + return r.chain.GetHeaderByHash(hash) +} + +func (r *verifyChainReader) GetBlock(hash common.Hash, number uint64) *types.Block { + if block, ok := r.blocksByHashNo[hashAndNumber{hash: hash, number: number}]; ok { + return block + } + if r.chain == nil { + return nil + } + return r.chain.GetBlock(hash, number) +} diff --git a/consensus/XDPoS/verify_chain_reader_test.go b/consensus/XDPoS/verify_chain_reader_test.go new file mode 100644 index 000000000000..8f1b91a81b86 --- /dev/null +++ b/consensus/XDPoS/verify_chain_reader_test.go @@ -0,0 +1,143 @@ +package XDPoS + +import ( + "math/big" + "testing" + + "github.com/XinFinOrg/XDPoSChain/common" + "github.com/XinFinOrg/XDPoSChain/consensus" + "github.com/XinFinOrg/XDPoSChain/core/rawdb" + "github.com/XinFinOrg/XDPoSChain/core/types" + "github.com/XinFinOrg/XDPoSChain/params" + "github.com/stretchr/testify/assert" +) + +type stubChainReader struct { + config *params.ChainConfig + headersByHash map[common.Hash]*types.Header + headersByNumber map[uint64]*types.Header +} + +func (s *stubChainReader) Config() *params.ChainConfig { return s.config } + +func (s *stubChainReader) CurrentHeader() *types.Header { return nil } + +func (s *stubChainReader) GetHeader(hash common.Hash, number uint64) *types.Header { + header := s.GetHeaderByHash(hash) + if header == nil || header.Number == nil || header.Number.Uint64() != number { + return nil + } + return header +} + +func (s *stubChainReader) GetHeaderByNumber(number uint64) *types.Header { + if s.headersByNumber == nil { + return nil + } + return s.headersByNumber[number] +} + +func (s *stubChainReader) GetHeaderByHash(hash common.Hash) *types.Header { + if s.headersByHash == nil { + return nil + } + return s.headersByHash[hash] +} + +func (s *stubChainReader) GetBlock(common.Hash, uint64) *types.Block { return nil } + +func TestNewVerifyChainReaderWithNilChainReturnsNilSafeReader(t *testing.T) { + reader := newVerifyChainReader(nil, []*types.Header{{Number: big.NewInt(1)}}) + assert.NotNil(t, reader) + assert.Nil(t, reader.Config()) + assert.Nil(t, reader.CurrentHeader()) + assert.Nil(t, reader.GetHeaderByNumber(2)) + assert.Nil(t, reader.GetHeaderByHash(common.Hash{})) + assert.Nil(t, reader.GetHeader(common.Hash{}, 2)) + assert.Nil(t, reader.GetBlock(common.Hash{}, 2)) +} + +func TestVerifyChainReaderShadowsHeaderByNumber(t *testing.T) { + baseHeader := &types.Header{Number: big.NewInt(100), Time: 1} + batchHeader := &types.Header{Number: big.NewInt(100), Time: 2} + + base := &stubChainReader{ + config: params.TestXDPoSMockChainConfig, + headersByHash: map[common.Hash]*types.Header{baseHeader.Hash(): baseHeader}, + headersByNumber: map[uint64]*types.Header{100: baseHeader}, + } + + reader := newVerifyChainReader(base, []*types.Header{batchHeader}) + resolved := reader.GetHeaderByNumber(100) + assert.NotNil(t, resolved) + assert.Equal(t, batchHeader.Hash(), resolved.Hash()) +} + +func TestVerifyChainReaderResolvesParentFromBatch(t *testing.T) { + parent := &types.Header{Number: big.NewInt(900), Time: 1} + child := &types.Header{Number: big.NewInt(901), ParentHash: parent.Hash(), Time: 2} + + base := &stubChainReader{ + config: params.TestXDPoSMockChainConfig, + headersByHash: map[common.Hash]*types.Header{}, + headersByNumber: map[uint64]*types.Header{}, + } + + reader := newVerifyChainReader(base, []*types.Header{parent, child}) + resolved := reader.GetHeader(child.ParentHash, child.Number.Uint64()-1) + assert.NotNil(t, resolved) + assert.Equal(t, parent.Hash(), resolved.Hash()) +} + +func TestVerifyHeadersMixedWithNilChainDoesNotPanic(t *testing.T) { + database := rawdb.NewMemoryDatabase() + config := params.TestXDPoSMockChainConfig + engine := New(config, database) + + headers := []*types.Header{ + {Number: big.NewInt(900)}, + {Number: big.NewInt(901), Validator: make([]byte, 65)}, + } + fullVerifies := []bool{false, false} + + assert.NotPanics(t, func() { + abort, results := engine.VerifyHeaders(nil, headers, fullVerifies) + defer close(abort) + + err1 := <-results + err2 := <-results + _ = err1 + _ = err2 + }) +} + +func TestVerifyHeadersMixedEmitsV1ThenV2(t *testing.T) { + database := rawdb.NewMemoryDatabase() + config := params.TestXDPoSMockChainConfig + engine := New(config, database) + + base := &stubChainReader{ + config: config, + headersByNumber: map[uint64]*types.Header{ + 450: {Number: big.NewInt(450)}, + 900: {Number: big.NewInt(900)}, + }, + } + + headers := []*types.Header{ + {Number: big.NewInt(900)}, + {Number: big.NewInt(901)}, + } + fullVerifies := []bool{false, false} + + abort, results := engine.VerifyHeaders(base, headers, fullVerifies) + defer close(abort) + + err1 := <-results + err2 := <-results + + assert.NoError(t, err1) + assert.Error(t, err2) +} + +var _ consensus.ChainReader = (*stubChainReader)(nil) diff --git a/consensus/tests/engine_v2_tests/verify_header_test.go b/consensus/tests/engine_v2_tests/verify_header_test.go index 96466b58557c..7037bbbb6246 100644 --- a/consensus/tests/engine_v2_tests/verify_header_test.go +++ b/consensus/tests/engine_v2_tests/verify_header_test.go @@ -17,6 +17,19 @@ import ( "github.com/stretchr/testify/assert" ) +type maskingChainReader struct { + consensus.ChainReader + maskedHash common.Hash + maskedNumber uint64 +} + +func (m *maskingChainReader) GetHeader(hash common.Hash, number uint64) *types.Header { + if hash == m.maskedHash && number == m.maskedNumber { + return nil + } + return m.ChainReader.GetHeader(hash, number) +} + func TestShouldVerifyBlock(t *testing.T) { b, err := json.Marshal(params.TestXDPoSMockChainConfig) assert.Nil(t, err) @@ -485,3 +498,39 @@ func TestShouldVerifyHeadersEvenIfParentsNotYetWrittenIntoDB(t *testing.T) { } } } + +func TestShouldVerifyMixedHeadersWhenParentLookupByHashIsMasked(t *testing.T) { + skipLongInShortMode(t) + b, err := json.Marshal(params.TestXDPoSMockChainConfig) + assert.Nil(t, err) + + var config params.ChainConfig + err = json.Unmarshal(b, &config) + assert.Nil(t, err) + + blockchain, _, _, _, _, _ := PrepareXDCTestBlockChainForV2Engine(t, 910, &config, nil) + adaptor := blockchain.Engine().(*XDPoS.XDPoS) + + // Build a mixed v1/v2 input where the first v2 header (901) depends on v1 parent (900). + headers := []*types.Header{ + blockchain.GetBlockByNumber(900).Header(), + blockchain.GetBlockByNumber(901).Header(), + } + fullVerifies := []bool{true, true} + + maskedChain := &maskingChainReader{ + ChainReader: blockchain, + maskedHash: headers[0].Hash(), + maskedNumber: headers[0].Number.Uint64(), + } + + _, results := adaptor.VerifyHeaders(maskedChain, headers, fullVerifies) + for i := 0; i < len(headers); i++ { + select { + case result := <-results: + assert.Nil(t, result) + case <-time.After(5 * time.Second): + t.Fatalf("timed out waiting for verify result %d", i) + } + } +}