fix(consensus/XDPoS): fix mixed v1/v2 VerifyHeaders ancestor lookup and result ordering, fix XFN-12

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

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)
+}

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)

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)
+		}
+	}
+}