fix(consensus/XDPoS): stabilize VerifyHeaders across v1-v2 switch, fix #2138 XFN-12

consensus/XDPoS/XDPoS.go

@@ -44,6 +44,48 @@ const (
 	newRoundChanSize    = 1
 )
 
+type verifyChainReader struct {
+	consensus.ChainReader
+	batchByHash   map[common.Hash]*types.Header
+	batchByNumber map[uint64]*types.Header
+}
+
+func newVerifyChainReader(chain consensus.ChainReader, capacity int) *verifyChainReader {
+	return &verifyChainReader{
+		ChainReader:   chain,
+		batchByHash:   make(map[common.Hash]*types.Header, capacity),
+		batchByNumber: make(map[uint64]*types.Header, capacity),
+	}
+}
+
+func (r *verifyChainReader) addHeader(header *types.Header) {
+	r.batchByHash[header.Hash()] = header
+	if header.Number != nil {
+		r.batchByNumber[header.Number.Uint64()] = header
+	}
+}
+
+func (r *verifyChainReader) GetHeader(hash common.Hash, number uint64) *types.Header {
+	if header, ok := r.batchByHash[hash]; ok && header.Number != nil && header.Number.Uint64() == number {
+		return header
+	}
+	return r.ChainReader.GetHeader(hash, number)
+}
+
+func (r *verifyChainReader) GetHeaderByHash(hash common.Hash) *types.Header {
+	if header, ok := r.batchByHash[hash]; ok {
+		return header
+	}
+	return r.ChainReader.GetHeaderByHash(hash)
+}
+
+func (r *verifyChainReader) GetHeaderByNumber(number uint64) *types.Header {
+	if header, ok := r.batchByNumber[number]; ok {
+		return header
+	}
+	return r.ChainReader.GetHeaderByNumber(number)
+}
+
 func (x *XDPoS) SigHash(header *types.Header) (hash common.Hash) {
 	switch x.config.BlockConsensusVersion(header.Number) {
 	case params.ConsensusEngineVersion2:
@@ -214,30 +256,25 @@ func (x *XDPoS) VerifyHeader(chain consensus.ChainReader, header *types.Header,
 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))
+	go func() {
+		verifyChain := newVerifyChainReader(chain, len(headers))
+		for i, header := range headers {
+			var err error
+			switch x.config.BlockConsensusVersion(header.Number) {
+			case params.ConsensusEngineVersion2:
+				err = x.EngineV2.VerifyHeader(verifyChain, header, fullVerifies[i])
+			default: // Default "v1"
+				err = x.EngineV1.VerifyHeader(verifyChain, header, fullVerifies[i])
+			}
+			verifyChain.addHeader(header)
 
-	// 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))
-
-	for i, header := range headers {
-		switch x.config.BlockConsensusVersion(header.Number) {
-		case params.ConsensusEngineVersion2:
-			v2headers = append(v2headers, header)
-			v2fullVerifies = append(v2fullVerifies, fullVerifies[i])
-		default: // Default "v1"
-			v1headers = append(v1headers, header)
-			v1fullVerifies = append(v1fullVerifies, fullVerifies[i])
+			select {
+			case <-abort:
+				return
+			case results <- err:
+			}
 		}
-	}
-
-	if v1headers != nil {
-		x.EngineV1.VerifyHeaders(chain, v1headers, v1fullVerifies, abort, results)
-	}
-	if v2headers != nil {
-		x.EngineV2.VerifyHeaders(chain, v2headers, v2fullVerifies, abort, results)
-	}
+	}()
 
 	return abort, results
 }

consensus/XDPoS/XDPoS_test.go

@@ -1,9 +1,12 @@
 package XDPoS
 
 import (
+	"math/big"
 	"testing"
 
+	"github.com/XinFinOrg/XDPoSChain/common"
 	"github.com/XinFinOrg/XDPoSChain/core/rawdb"
+	"github.com/XinFinOrg/XDPoSChain/core/types"
 	"github.com/XinFinOrg/XDPoSChain/params"
 	"github.com/stretchr/testify/assert"
 )
@@ -16,3 +19,61 @@ func TestAdaptorShouldShareDbWithV1Engine(t *testing.T) {
 	assert := assert.New(t)
 	assert.Equal(engine.EngineV1.GetDb(), engine.GetDb())
 }
+
+type testChainReader struct {
+	headersByHash   map[common.Hash]*types.Header
+	headersByNumber map[uint64]*types.Header
+}
+
+func (m *testChainReader) Config() *params.ChainConfig {
+	return params.TestXDPoSMockChainConfig
+}
+
+func (m *testChainReader) CurrentHeader() *types.Header {
+	return nil
+}
+
+func (m *testChainReader) GetHeader(hash common.Hash, number uint64) *types.Header {
+	header, ok := m.headersByHash[hash]
+	if !ok || header == nil || header.Number == nil || header.Number.Uint64() != number {
+		return nil
+	}
+	return header
+}
+
+func (m *testChainReader) GetHeaderByNumber(number uint64) *types.Header {
+	return m.headersByNumber[number]
+}
+
+func (m *testChainReader) GetHeaderByHash(hash common.Hash) *types.Header {
+	return m.headersByHash[hash]
+}
+
+func (m *testChainReader) GetBlock(hash common.Hash, number uint64) *types.Block {
+	return nil
+}
+
+func TestVerifyChainReaderReturnsBatchHeaderByNumber(t *testing.T) {
+	base := &testChainReader{
+		headersByHash:   make(map[common.Hash]*types.Header),
+		headersByNumber: make(map[uint64]*types.Header),
+	}
+
+	fallback := &types.Header{Number: big.NewInt(100), Extra: []byte{0x01}}
+	base.headersByHash[fallback.Hash()] = fallback
+	base.headersByNumber[100] = fallback
+
+	reader := newVerifyChainReader(base, 2)
+	batchHeader := &types.Header{Number: big.NewInt(100), Extra: []byte{0x02, 0x03}}
+	reader.addHeader(batchHeader)
+
+	got := reader.GetHeaderByNumber(100)
+	if got != batchHeader {
+		t.Fatalf("expected batch header to be returned by number lookup")
+	}
+
+	got = reader.GetHeader(batchHeader.Hash(), 100)
+	if got != batchHeader {
+		t.Fatalf("expected batch header to be returned by hash+number lookup")
+	}
+}

consensus/tests/engine_v2_tests/adaptor_test.go

@@ -6,12 +6,61 @@ import (
 	"testing"
 
 	"github.com/XinFinOrg/XDPoSChain/common"
+	"github.com/XinFinOrg/XDPoSChain/consensus"
 	"github.com/XinFinOrg/XDPoSChain/consensus/XDPoS"
 	"github.com/XinFinOrg/XDPoSChain/core/types"
 	"github.com/XinFinOrg/XDPoSChain/params"
 	"github.com/stretchr/testify/assert"
 )
 
+func TestAdaptorVerifyHeadersKeepsInputOrderAcrossConsensusSwitch(t *testing.T) {
+	blockchain, _, block900, signer, signFn, _ := PrepareXDCTestBlockChainForV2Engine(t, 900, params.TestXDPoSMockChainConfig, nil)
+	adaptor := blockchain.Engine().(*XDPoS.XDPoS)
+
+	goodV1Header := blockchain.GetHeaderByNumber(900)
+	if goodV1Header == nil {
+		t.Fatal("missing v1 header at block 900")
+	}
+
+	// Build the first v2 header without inserting it into DB.
+	badV2Header := types.CopyHeader(CreateBlock(blockchain, params.TestXDPoSMockChainConfig, block900, 901, 1, signer.Hex(), signer, signFn, nil, nil, "").Header())
+	badV2Header.Validator = nil
+
+	headers := []*types.Header{goodV1Header, badV2Header}
+	fullVerifies := []bool{false, false}
+	abort, results := adaptor.VerifyHeaders(blockchain, headers, fullVerifies)
+	defer close(abort)
+
+	err0 := <-results
+	err1 := <-results
+
+	assert.NoError(t, err0)
+	assert.ErrorIs(t, err1, consensus.ErrNoValidatorSignatureV2)
+	assert.NotErrorIs(t, err1, consensus.ErrUnknownAncestor)
+}
+
+func TestAdaptorVerifyHeadersHandlesConsecutiveUnwrittenHeaders(t *testing.T) {
+	blockchain, _, block900, signer, signFn, _ := PrepareXDCTestBlockChainForV2Engine(t, 900, params.TestXDPoSMockChainConfig, nil)
+	adaptor := blockchain.Engine().(*XDPoS.XDPoS)
+
+	// Build two consecutive v2 headers without inserting them into DB.
+	block901 := CreateBlock(blockchain, params.TestXDPoSMockChainConfig, block900, 901, 1, signer.Hex(), signer, signFn, nil, nil, "")
+	block902 := CreateBlock(blockchain, params.TestXDPoSMockChainConfig, block901, 902, 2, signer.Hex(), signer, signFn, nil, nil, "")
+
+	headers := []*types.Header{block901.Header(), block902.Header()}
+	fullVerifies := []bool{true, true}
+	abort, results := adaptor.VerifyHeaders(blockchain, headers, fullVerifies)
+	defer close(abort)
+
+	err0 := <-results
+	err1 := <-results
+
+	assert.NoError(t, err0)
+	assert.NoError(t, err1)
+	assert.NotErrorIs(t, err0, consensus.ErrUnknownAncestor)
+	assert.NotErrorIs(t, err1, consensus.ErrUnknownAncestor)
+}
+
 func TestAdaptorShouldGetAuthorForDifferentConsensusVersion(t *testing.T) {
 	blockchain, _, currentBlock, signer, signFn, _ := PrepareXDCTestBlockChainForV2Engine(t, 900, params.TestXDPoSMockChainConfig, nil)
 	adaptor := blockchain.Engine().(*XDPoS.XDPoS)