Skip to content

Commit 958aef2

Browse files
committed
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.
1 parent be36b32 commit 958aef2

4 files changed

Lines changed: 340 additions & 14 deletions

File tree

consensus/XDPoS/XDPoS.go

Lines changed: 49 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -210,33 +210,68 @@ func (x *XDPoS) VerifyHeader(chain consensus.ChainReader, header *types.Header,
210210

211211
// VerifyHeaders is similar to VerifyHeader, but verifies a batch of headers. The
212212
// method returns a quit channel to abort the operations and a results channel to
213-
// retrieve the async verifications (the order is that of the input slice).
213+
// retrieve the async verifications. For mixed v1/v2 inputs, results are emitted
214+
// in deterministic consensus order: all v1 results first, then all v2 results.
214215
func (x *XDPoS) VerifyHeaders(chain consensus.ChainReader, headers []*types.Header, fullVerifies []bool) (chan<- struct{}, <-chan error) {
215216
abort := make(chan struct{})
216217
results := make(chan error, len(headers))
217218

218219
// Split the headers list into v1 and v2 buckets
219-
var v1headers []*types.Header
220-
var v2headers []*types.Header
221-
v1fullVerifies := make([]bool, 0, len(headers))
222-
v2fullVerifies := make([]bool, 0, len(headers))
220+
var v1Headers []*types.Header
221+
var v2Headers []*types.Header
222+
v1FullVerifies := make([]bool, 0, len(headers))
223+
v2FullVerifies := make([]bool, 0, len(headers))
223224

224225
for i, header := range headers {
225226
switch x.config.BlockConsensusVersion(header.Number) {
226227
case params.ConsensusEngineVersion2:
227-
v2headers = append(v2headers, header)
228-
v2fullVerifies = append(v2fullVerifies, fullVerifies[i])
228+
v2Headers = append(v2Headers, header)
229+
v2FullVerifies = append(v2FullVerifies, fullVerifies[i])
229230
default: // Default "v1"
230-
v1headers = append(v1headers, header)
231-
v1fullVerifies = append(v1fullVerifies, fullVerifies[i])
231+
v1Headers = append(v1Headers, header)
232+
v1FullVerifies = append(v1FullVerifies, fullVerifies[i])
232233
}
233234
}
234235

235-
if v1headers != nil {
236-
x.EngineV1.VerifyHeaders(chain, v1headers, v1fullVerifies, abort, results)
237-
}
238-
if v2headers != nil {
239-
x.EngineV2.VerifyHeaders(chain, v2headers, v2fullVerifies, abort, results)
236+
v1Count := len(v1Headers)
237+
v2Count := len(v2Headers)
238+
if v1Count != 0 && v2Count == 0 {
239+
x.EngineV1.VerifyHeaders(chain, v1Headers, v1FullVerifies, abort, results)
240+
} else if v1Count == 0 && v2Count != 0 {
241+
x.EngineV2.VerifyHeaders(chain, v2Headers, v2FullVerifies, abort, results)
242+
} else if v1Count != 0 && v2Count != 0 {
243+
v1Results := make(chan error, v1Count)
244+
v2Results := make(chan error, v2Count)
245+
verifyChain := newVerifyChainReader(chain, headers)
246+
x.EngineV1.VerifyHeaders(verifyChain, v1Headers, v1FullVerifies, abort, v1Results)
247+
x.EngineV2.VerifyHeaders(verifyChain, v2Headers, v2FullVerifies, abort, v2Results)
248+
249+
go func() {
250+
for range v1Count {
251+
select {
252+
case <-abort:
253+
return
254+
case err := <-v1Results:
255+
select {
256+
case <-abort:
257+
return
258+
case results <- err:
259+
}
260+
}
261+
}
262+
for range v2Count {
263+
select {
264+
case <-abort:
265+
return
266+
case err := <-v2Results:
267+
select {
268+
case <-abort:
269+
return
270+
case results <- err:
271+
}
272+
}
273+
}
274+
}()
240275
}
241276

242277
return abort, results
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package XDPoS
2+
3+
import (
4+
"github.com/XinFinOrg/XDPoSChain/common"
5+
"github.com/XinFinOrg/XDPoSChain/consensus"
6+
"github.com/XinFinOrg/XDPoSChain/core/types"
7+
"github.com/XinFinOrg/XDPoSChain/params"
8+
)
9+
10+
// verifyChainReader shadows chain lookups with headers in the current verify batch.
11+
// This keeps verification deterministic when deep consensus paths query ancestors
12+
// that are in-flight and not written to DB yet.
13+
type verifyChainReader struct {
14+
chain consensus.ChainReader
15+
headersByHash map[common.Hash]*types.Header
16+
headersByNumber map[uint64]*types.Header
17+
blocksByHashNo map[hashAndNumber]*types.Block
18+
}
19+
20+
type hashAndNumber struct {
21+
hash common.Hash
22+
number uint64
23+
}
24+
25+
func newVerifyChainReader(chain consensus.ChainReader, headers []*types.Header) *verifyChainReader {
26+
reader := &verifyChainReader{
27+
chain: chain,
28+
headersByHash: make(map[common.Hash]*types.Header, len(headers)),
29+
headersByNumber: make(map[uint64]*types.Header, len(headers)),
30+
blocksByHashNo: make(map[hashAndNumber]*types.Block, len(headers)),
31+
}
32+
for _, header := range headers {
33+
if header == nil || header.Number == nil {
34+
continue
35+
}
36+
h := header.Hash()
37+
n := header.Number.Uint64()
38+
reader.headersByHash[h] = header
39+
if _, exists := reader.headersByNumber[n]; !exists {
40+
reader.headersByNumber[n] = header
41+
}
42+
reader.blocksByHashNo[hashAndNumber{hash: h, number: n}] = types.NewBlockWithHeader(header)
43+
}
44+
return reader
45+
}
46+
47+
func (r *verifyChainReader) Config() *params.ChainConfig {
48+
if r.chain == nil {
49+
return nil
50+
}
51+
return r.chain.Config()
52+
}
53+
54+
func (r *verifyChainReader) CurrentHeader() *types.Header {
55+
if r.chain == nil {
56+
return nil
57+
}
58+
return r.chain.CurrentHeader()
59+
}
60+
61+
func (r *verifyChainReader) GetHeader(hash common.Hash, number uint64) *types.Header {
62+
if header, ok := r.headersByHash[hash]; ok && header.Number != nil && header.Number.Uint64() == number {
63+
return header
64+
}
65+
if r.chain == nil {
66+
return nil
67+
}
68+
return r.chain.GetHeader(hash, number)
69+
}
70+
71+
func (r *verifyChainReader) GetHeaderByNumber(number uint64) *types.Header {
72+
if header, ok := r.headersByNumber[number]; ok {
73+
return header
74+
}
75+
if r.chain == nil {
76+
return nil
77+
}
78+
return r.chain.GetHeaderByNumber(number)
79+
}
80+
81+
func (r *verifyChainReader) GetHeaderByHash(hash common.Hash) *types.Header {
82+
if header, ok := r.headersByHash[hash]; ok {
83+
return header
84+
}
85+
if r.chain == nil {
86+
return nil
87+
}
88+
return r.chain.GetHeaderByHash(hash)
89+
}
90+
91+
func (r *verifyChainReader) GetBlock(hash common.Hash, number uint64) *types.Block {
92+
if block, ok := r.blocksByHashNo[hashAndNumber{hash: hash, number: number}]; ok {
93+
return block
94+
}
95+
if r.chain == nil {
96+
return nil
97+
}
98+
return r.chain.GetBlock(hash, number)
99+
}
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
package XDPoS
2+
3+
import (
4+
"math/big"
5+
"testing"
6+
7+
"github.com/XinFinOrg/XDPoSChain/common"
8+
"github.com/XinFinOrg/XDPoSChain/consensus"
9+
"github.com/XinFinOrg/XDPoSChain/core/rawdb"
10+
"github.com/XinFinOrg/XDPoSChain/core/types"
11+
"github.com/XinFinOrg/XDPoSChain/params"
12+
"github.com/stretchr/testify/assert"
13+
)
14+
15+
type stubChainReader struct {
16+
config *params.ChainConfig
17+
headersByHash map[common.Hash]*types.Header
18+
headersByNumber map[uint64]*types.Header
19+
}
20+
21+
func (s *stubChainReader) Config() *params.ChainConfig { return s.config }
22+
23+
func (s *stubChainReader) CurrentHeader() *types.Header { return nil }
24+
25+
func (s *stubChainReader) GetHeader(hash common.Hash, number uint64) *types.Header {
26+
header := s.GetHeaderByHash(hash)
27+
if header == nil || header.Number == nil || header.Number.Uint64() != number {
28+
return nil
29+
}
30+
return header
31+
}
32+
33+
func (s *stubChainReader) GetHeaderByNumber(number uint64) *types.Header {
34+
if s.headersByNumber == nil {
35+
return nil
36+
}
37+
return s.headersByNumber[number]
38+
}
39+
40+
func (s *stubChainReader) GetHeaderByHash(hash common.Hash) *types.Header {
41+
if s.headersByHash == nil {
42+
return nil
43+
}
44+
return s.headersByHash[hash]
45+
}
46+
47+
func (s *stubChainReader) GetBlock(common.Hash, uint64) *types.Block { return nil }
48+
49+
func TestNewVerifyChainReaderWithNilChainReturnsNilSafeReader(t *testing.T) {
50+
reader := newVerifyChainReader(nil, []*types.Header{{Number: big.NewInt(1)}})
51+
assert.NotNil(t, reader)
52+
assert.Nil(t, reader.Config())
53+
assert.Nil(t, reader.CurrentHeader())
54+
assert.Nil(t, reader.GetHeaderByNumber(2))
55+
assert.Nil(t, reader.GetHeaderByHash(common.Hash{}))
56+
assert.Nil(t, reader.GetHeader(common.Hash{}, 2))
57+
assert.Nil(t, reader.GetBlock(common.Hash{}, 2))
58+
}
59+
60+
func TestVerifyChainReaderShadowsHeaderByNumber(t *testing.T) {
61+
baseHeader := &types.Header{Number: big.NewInt(100), Time: 1}
62+
batchHeader := &types.Header{Number: big.NewInt(100), Time: 2}
63+
64+
base := &stubChainReader{
65+
config: params.TestXDPoSMockChainConfig,
66+
headersByHash: map[common.Hash]*types.Header{baseHeader.Hash(): baseHeader},
67+
headersByNumber: map[uint64]*types.Header{100: baseHeader},
68+
}
69+
70+
reader := newVerifyChainReader(base, []*types.Header{batchHeader})
71+
resolved := reader.GetHeaderByNumber(100)
72+
assert.NotNil(t, resolved)
73+
assert.Equal(t, batchHeader.Hash(), resolved.Hash())
74+
}
75+
76+
func TestVerifyChainReaderResolvesParentFromBatch(t *testing.T) {
77+
parent := &types.Header{Number: big.NewInt(900), Time: 1}
78+
child := &types.Header{Number: big.NewInt(901), ParentHash: parent.Hash(), Time: 2}
79+
80+
base := &stubChainReader{
81+
config: params.TestXDPoSMockChainConfig,
82+
headersByHash: map[common.Hash]*types.Header{},
83+
headersByNumber: map[uint64]*types.Header{},
84+
}
85+
86+
reader := newVerifyChainReader(base, []*types.Header{parent, child})
87+
resolved := reader.GetHeader(child.ParentHash, child.Number.Uint64()-1)
88+
assert.NotNil(t, resolved)
89+
assert.Equal(t, parent.Hash(), resolved.Hash())
90+
}
91+
92+
func TestVerifyHeadersMixedWithNilChainDoesNotPanic(t *testing.T) {
93+
database := rawdb.NewMemoryDatabase()
94+
config := params.TestXDPoSMockChainConfig
95+
engine := New(config, database)
96+
97+
headers := []*types.Header{
98+
{Number: big.NewInt(900)},
99+
{Number: big.NewInt(901), Validator: make([]byte, 65)},
100+
}
101+
fullVerifies := []bool{false, false}
102+
103+
assert.NotPanics(t, func() {
104+
abort, results := engine.VerifyHeaders(nil, headers, fullVerifies)
105+
defer close(abort)
106+
107+
err1 := <-results
108+
err2 := <-results
109+
_ = err1
110+
_ = err2
111+
})
112+
}
113+
114+
func TestVerifyHeadersMixedEmitsV1ThenV2(t *testing.T) {
115+
database := rawdb.NewMemoryDatabase()
116+
config := params.TestXDPoSMockChainConfig
117+
engine := New(config, database)
118+
119+
base := &stubChainReader{
120+
config: config,
121+
headersByNumber: map[uint64]*types.Header{
122+
450: {Number: big.NewInt(450)},
123+
900: {Number: big.NewInt(900)},
124+
},
125+
}
126+
127+
headers := []*types.Header{
128+
{Number: big.NewInt(900)},
129+
{Number: big.NewInt(901)},
130+
}
131+
fullVerifies := []bool{false, false}
132+
133+
abort, results := engine.VerifyHeaders(base, headers, fullVerifies)
134+
defer close(abort)
135+
136+
err1 := <-results
137+
err2 := <-results
138+
139+
assert.NoError(t, err1)
140+
assert.Error(t, err2)
141+
}
142+
143+
var _ consensus.ChainReader = (*stubChainReader)(nil)

consensus/tests/engine_v2_tests/verify_header_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,19 @@ import (
1717
"github.com/stretchr/testify/assert"
1818
)
1919

20+
type maskingChainReader struct {
21+
consensus.ChainReader
22+
maskedHash common.Hash
23+
maskedNumber uint64
24+
}
25+
26+
func (m *maskingChainReader) GetHeader(hash common.Hash, number uint64) *types.Header {
27+
if hash == m.maskedHash && number == m.maskedNumber {
28+
return nil
29+
}
30+
return m.ChainReader.GetHeader(hash, number)
31+
}
32+
2033
func TestShouldVerifyBlock(t *testing.T) {
2134
b, err := json.Marshal(params.TestXDPoSMockChainConfig)
2235
assert.Nil(t, err)
@@ -485,3 +498,39 @@ func TestShouldVerifyHeadersEvenIfParentsNotYetWrittenIntoDB(t *testing.T) {
485498
}
486499
}
487500
}
501+
502+
func TestShouldVerifyMixedHeadersWhenParentLookupByHashIsMasked(t *testing.T) {
503+
skipLongInShortMode(t)
504+
b, err := json.Marshal(params.TestXDPoSMockChainConfig)
505+
assert.Nil(t, err)
506+
507+
var config params.ChainConfig
508+
err = json.Unmarshal(b, &config)
509+
assert.Nil(t, err)
510+
511+
blockchain, _, _, _, _, _ := PrepareXDCTestBlockChainForV2Engine(t, 910, &config, nil)
512+
adaptor := blockchain.Engine().(*XDPoS.XDPoS)
513+
514+
// Build a mixed v1/v2 input where the first v2 header (901) depends on v1 parent (900).
515+
headers := []*types.Header{
516+
blockchain.GetBlockByNumber(900).Header(),
517+
blockchain.GetBlockByNumber(901).Header(),
518+
}
519+
fullVerifies := []bool{true, true}
520+
521+
maskedChain := &maskingChainReader{
522+
ChainReader: blockchain,
523+
maskedHash: headers[0].Hash(),
524+
maskedNumber: headers[0].Number.Uint64(),
525+
}
526+
527+
_, results := adaptor.VerifyHeaders(maskedChain, headers, fullVerifies)
528+
for i := 0; i < len(headers); i++ {
529+
select {
530+
case result := <-results:
531+
assert.Nil(t, result)
532+
case <-time.After(5 * time.Second):
533+
t.Fatalf("timed out waiting for verify result %d", i)
534+
}
535+
}
536+
}

0 commit comments

Comments
 (0)