From ec982d47b48ee43d3f3f0341733e08cff6287fd2 Mon Sep 17 00:00:00 2001 From: Daniel Liu <139250065@qq.com> Date: Fri, 6 Mar 2026 18:36:16 +0800 Subject: [PATCH 1/2] fix(consensus/XDPoS): stabilize VerifyHeaders across v1-v2 switch, fix #2138 XFN-12 Problem - Mixed v1/v2 batches around the switch boundary could produce non-deterministic verification behavior. - The adaptor previously relied on version-bucket concurrency and did not expose in-batch headers for GetHeaderByNumber lookups. - In practice this could surface as intermittent sync failures and misleading BAD BLOCK attribution near the switch height. Root cause - Verify result ordering and read visibility assumptions were too weak for mixed-boundary batches. - Engine helpers may query headers by number during verification, but batch context only shadowed hash-based lookups. Fix - Keep VerifyHeaders emission strictly aligned with input order. - Introduce verifyChainReader and shadow both hash and number lookups with in-batch headers: - GetHeader(hash, number) - GetHeaderByHash(hash) - GetHeaderByNumber(number) - This preserves deterministic per-header verification and avoids stale canonical reads during boundary processing. Tests - Add TestAdaptorVerifyHeadersKeepsInputOrderAcrossConsensusSwitch. - Add TestVerifyChainReaderReturnsBatchHeaderByNumber. Validation - go test ./consensus/XDPoS/... --- consensus/XDPoS/XDPoS.go | 81 ++++++++++++++----- consensus/XDPoS/XDPoS_test.go | 61 ++++++++++++++ .../tests/engine_v2_tests/adaptor_test.go | 49 +++++++++++ 3 files changed, 169 insertions(+), 22 deletions(-) diff --git a/consensus/XDPoS/XDPoS.go b/consensus/XDPoS/XDPoS.go index 0edf37312005..41800ccd4d8b 100644 --- a/consensus/XDPoS/XDPoS.go +++ b/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 } diff --git a/consensus/XDPoS/XDPoS_test.go b/consensus/XDPoS/XDPoS_test.go index c4c555d49403..0c4b27cd836a 100644 --- a/consensus/XDPoS/XDPoS_test.go +++ b/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") + } +} diff --git a/consensus/tests/engine_v2_tests/adaptor_test.go b/consensus/tests/engine_v2_tests/adaptor_test.go index 449b2c0e61ad..12ff9de58a78 100644 --- a/consensus/tests/engine_v2_tests/adaptor_test.go +++ b/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) From 25ea6194ac9b0a0b433a86aa79d1c55505b58766 Mon Sep 17 00:00:00 2001 From: Daniel Liu <139250065@qq.com> Date: Wed, 11 Mar 2026 02:52:28 +0800 Subject: [PATCH 2/2] debug --- consensus/XDPoS/XDPoS.go | 55 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/consensus/XDPoS/XDPoS.go b/consensus/XDPoS/XDPoS.go index 41800ccd4d8b..31649f4cf6ce 100644 --- a/consensus/XDPoS/XDPoS.go +++ b/consensus/XDPoS/XDPoS.go @@ -209,6 +209,16 @@ func (x *XDPoS) UpdateParams(header *types.Header) { } func (x *XDPoS) Initial(chain consensus.ChainReader, header *types.Header) error { + if x.config != nil && x.config.V2 != nil && x.config.V2.SwitchBlock != nil && header != nil && header.Number != nil { + switchBlock := x.config.V2.SwitchBlock.Uint64() + number := header.Number.Uint64() + switch { + case number == switchBlock: + log.Info("[Initial] still v1 at switch block, v2 initializes from switch+1", "number", number, "switchBlock", switchBlock, "hash", header.Hash()) + case number == switchBlock+1: + log.Info("[Initial] activating v2 initialization", "number", number, "switchBlock", switchBlock, "hash", header.Hash(), "parent", header.ParentHash) + } + } switch x.config.BlockConsensusVersion(header.Number) { case params.ConsensusEngineVersion2: return x.EngineV2.Initial(chain, header) @@ -242,6 +252,13 @@ func (x *XDPoS) Author(header *types.Header) (common.Address, error) { // VerifyHeader checks whether a header conforms to the consensus rules. func (x *XDPoS) VerifyHeader(chain consensus.ChainReader, header *types.Header, fullVerify bool) error { + if x.config != nil && x.config.V2 != nil && x.config.V2.SwitchBlock != nil && header != nil && header.Number != nil { + switchBlock := x.config.V2.SwitchBlock.Uint64() + number := header.Number.Uint64() + if number == switchBlock+1 { + log.Info("[VerifyHeader] v2 verification activated at switch boundary", "number", number, "switchBlock", switchBlock, "hash", header.Hash(), "parent", header.ParentHash) + } + } switch x.config.BlockConsensusVersion(header.Number) { case params.ConsensusEngineVersion2: return x.EngineV2.VerifyHeader(chain, header, fullVerify) @@ -258,14 +275,52 @@ func (x *XDPoS) VerifyHeaders(chain consensus.ChainReader, headers []*types.Head results := make(chan error, len(headers)) go func() { verifyChain := newVerifyChainReader(chain, len(headers)) + switchBlock := uint64(0) + if x.config != nil && x.config.V2 != nil && x.config.V2.SwitchBlock != nil { + switchBlock = x.config.V2.SwitchBlock.Uint64() + } for i, header := range headers { var err error + version := x.config.BlockConsensusVersion(header.Number) + if switchBlock > 0 && version == params.ConsensusEngineVersion2 && header.Number != nil && header.Number.Uint64() == switchBlock+1 { + log.Info("[VerifyHeaders] v2 verification activated in batch", "index", i, "number", header.Number.Uint64(), "switchBlock", switchBlock, "hash", header.Hash(), "parent", header.ParentHash) + } + if switchBlock > 0 && header.Number != nil { + number := header.Number.Uint64() + if number+2 >= switchBlock && number <= switchBlock+2 { + log.Debug("[VerifyHeaders] switch-boundary header", + "index", i, + "number", number, + "hash", header.Hash(), + "parent", header.ParentHash, + "selectedVersion", version, + "switchBlock", switchBlock, + "extraLen", len(header.Extra), + "validatorLen", len(header.Validator), + "fullVerify", fullVerifies[i], + ) + } + } 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]) } + if err != nil { + log.Warn("[VerifyHeaders] header verification failed", + "index", i, + "number", header.Number, + "hash", header.Hash(), + "parent", header.ParentHash, + "selectedVersion", version, + "switchBlock", switchBlock, + "extraLen", len(header.Extra), + "validatorLen", len(header.Validator), + "fullVerify", fullVerifies[i], + "err", err, + ) + } verifyChain.addHeader(header) select {