Skip to content

Commit 02517e7

Browse files
tac0turtletac0turtle
committed
address comments
1 parent 5405e52 commit 02517e7

File tree

3 files changed

+194
-47
lines changed

3 files changed

+194
-47
lines changed

‎pkg/sync/exchange_wrapper.go‎

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,17 @@ import (
44
"context"
55

66
"github.com/celestiaorg/go-header"
7-
"github.com/evstack/ev-node/pkg/store"
87
)
98

10-
type storeGetter[H header.Header[H]] func(context.Context, store.Store, header.Hash) (H, error)
11-
type storeGetterByHeight[H header.Header[H]] func(context.Context, store.Store, uint64) (H, error)
9+
// GetterFunc retrieves a header by hash from a backing store.
10+
type GetterFunc[H header.Header[H]] func(context.Context, header.Hash) (H, error)
11+
12+
// GetterByHeightFunc retrieves a header by height from a backing store.
13+
type GetterByHeightFunc[H header.Header[H]] func(context.Context, uint64) (H, error)
14+
15+
// RangeGetterFunc retrieves headers in range [from, to) from a backing store.
16+
// Returns the contiguous headers found starting from 'from', and the next height needed.
17+
type RangeGetterFunc[H header.Header[H]] func(ctx context.Context, from, to uint64) ([]H, uint64, error)
1218

1319
// P2PExchange defines the interface for the underlying P2P exchange.
1420
type P2PExchange[H header.Header[H]] interface {
@@ -19,15 +25,15 @@ type P2PExchange[H header.Header[H]] interface {
1925

2026
type exchangeWrapper[H header.Header[H]] struct {
2127
p2pExchange P2PExchange[H]
22-
daStore store.Store
23-
getter storeGetter[H]
24-
getterByHeight storeGetterByHeight[H]
28+
getter GetterFunc[H]
29+
getterByHeight GetterByHeightFunc[H]
30+
rangeGetter RangeGetterFunc[H]
2531
}
2632

2733
func (ew *exchangeWrapper[H]) Get(ctx context.Context, hash header.Hash) (H, error) {
2834
// Check DA store first
29-
if ew.daStore != nil && ew.getter != nil {
30-
if h, err := ew.getter(ctx, ew.daStore, hash); err == nil && !h.IsZero() {
35+
if ew.getter != nil {
36+
if h, err := ew.getter(ctx, hash); err == nil && !h.IsZero() {
3137
return h, nil
3238
}
3339
}
@@ -38,8 +44,8 @@ func (ew *exchangeWrapper[H]) Get(ctx context.Context, hash header.Hash) (H, err
3844

3945
func (ew *exchangeWrapper[H]) GetByHeight(ctx context.Context, height uint64) (H, error) {
4046
// Check DA store first
41-
if ew.daStore != nil && ew.getterByHeight != nil {
42-
if h, err := ew.getterByHeight(ctx, ew.daStore, height); err == nil && !h.IsZero() {
47+
if ew.getterByHeight != nil {
48+
if h, err := ew.getterByHeight(ctx, height); err == nil && !h.IsZero() {
4349
return h, nil
4450
}
4551
}
@@ -53,7 +59,39 @@ func (ew *exchangeWrapper[H]) Head(ctx context.Context, opts ...header.HeadOptio
5359
}
5460

5561
func (ew *exchangeWrapper[H]) GetRangeByHeight(ctx context.Context, from H, to uint64) ([]H, error) {
56-
return ew.p2pExchange.GetRangeByHeight(ctx, from, to)
62+
fromHeight := from.Height() + 1
63+
64+
// If no range getter, fallback entirely to P2P
65+
if ew.rangeGetter == nil {
66+
return ew.p2pExchange.GetRangeByHeight(ctx, from, to)
67+
}
68+
69+
// Try DA store first for contiguous range
70+
daHeaders, nextHeight, err := ew.rangeGetter(ctx, fromHeight, to)
71+
if err != nil {
72+
// DA store failed, fallback to P2P for entire range
73+
return ew.p2pExchange.GetRangeByHeight(ctx, from, to)
74+
}
75+
76+
// Got everything from DA
77+
if nextHeight >= to {
78+
return daHeaders, nil
79+
}
80+
81+
// Need remainder from P2P
82+
if len(daHeaders) == 0 {
83+
// Nothing from DA, get entire range from P2P
84+
return ew.p2pExchange.GetRangeByHeight(ctx, from, to)
85+
}
86+
87+
// Get remainder from P2P starting after last DA header
88+
lastDAHeader := daHeaders[len(daHeaders)-1]
89+
p2pHeaders, err := ew.p2pExchange.GetRangeByHeight(ctx, lastDAHeader, to)
90+
if err != nil {
91+
return nil, err
92+
}
93+
94+
return append(daHeaders, p2pHeaders...), nil
5795
}
5896

5997
func (ew *exchangeWrapper[H]) Start(ctx context.Context) error {

‎pkg/sync/exchange_wrapper_test.go‎

Lines changed: 77 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import (
66
"testing"
77

88
"github.com/celestiaorg/go-header"
9-
"github.com/evstack/ev-node/pkg/store"
10-
"github.com/evstack/ev-node/test/mocks"
119
extmocks "github.com/evstack/ev-node/test/mocks/external"
1210
"github.com/evstack/ev-node/types"
1311
"github.com/stretchr/testify/assert"
@@ -22,13 +20,12 @@ func TestExchangeWrapper_Get(t *testing.T) {
2220
mockEx := extmocks.NewMockP2PExchange[*types.SignedHeader](t)
2321
// Exchange should NOT be called
2422

25-
getter := func(ctx context.Context, s store.Store, h header.Hash) (*types.SignedHeader, error) {
23+
getter := func(ctx context.Context, h header.Hash) (*types.SignedHeader, error) {
2624
return expectedHeader, nil
2725
}
2826

2927
ew := &exchangeWrapper[*types.SignedHeader]{
3028
p2pExchange: mockEx,
31-
daStore: mocks.NewMockStore(t),
3229
getter: getter,
3330
}
3431

@@ -41,13 +38,12 @@ func TestExchangeWrapper_Get(t *testing.T) {
4138
mockEx := extmocks.NewMockP2PExchange[*types.SignedHeader](t)
4239
mockEx.EXPECT().Get(ctx, hash).Return(expectedHeader, nil)
4340

44-
getter := func(ctx context.Context, s store.Store, h header.Hash) (*types.SignedHeader, error) {
41+
getter := func(ctx context.Context, h header.Hash) (*types.SignedHeader, error) {
4542
return nil, errors.New("not found")
4643
}
4744

4845
ew := &exchangeWrapper[*types.SignedHeader]{
4946
p2pExchange: mockEx,
50-
daStore: mocks.NewMockStore(t),
5147
getter: getter,
5248
}
5349

@@ -56,13 +52,12 @@ func TestExchangeWrapper_Get(t *testing.T) {
5652
assert.Equal(t, expectedHeader, h)
5753
})
5854

59-
t.Run("Store Not Configured", func(t *testing.T) {
55+
t.Run("Getter Not Configured", func(t *testing.T) {
6056
mockEx := extmocks.NewMockP2PExchange[*types.SignedHeader](t)
6157
mockEx.EXPECT().Get(ctx, hash).Return(expectedHeader, nil)
6258

6359
ew := &exchangeWrapper[*types.SignedHeader]{
6460
p2pExchange: mockEx,
65-
daStore: nil, // No store
6661
getter: nil,
6762
}
6863

@@ -80,13 +75,12 @@ func TestExchangeWrapper_GetByHeight(t *testing.T) {
8075
t.Run("Hit in Store", func(t *testing.T) {
8176
mockEx := extmocks.NewMockP2PExchange[*types.SignedHeader](t)
8277

83-
getterByHeight := func(ctx context.Context, s store.Store, h uint64) (*types.SignedHeader, error) {
78+
getterByHeight := func(ctx context.Context, h uint64) (*types.SignedHeader, error) {
8479
return expectedHeader, nil
8580
}
8681

8782
ew := &exchangeWrapper[*types.SignedHeader]{
8883
p2pExchange: mockEx,
89-
daStore: mocks.NewMockStore(t),
9084
getterByHeight: getterByHeight,
9185
}
9286

@@ -99,13 +93,12 @@ func TestExchangeWrapper_GetByHeight(t *testing.T) {
9993
mockEx := extmocks.NewMockP2PExchange[*types.SignedHeader](t)
10094
mockEx.EXPECT().GetByHeight(ctx, height).Return(expectedHeader, nil)
10195

102-
getterByHeight := func(ctx context.Context, s store.Store, h uint64) (*types.SignedHeader, error) {
96+
getterByHeight := func(ctx context.Context, h uint64) (*types.SignedHeader, error) {
10397
return nil, errors.New("not found")
10498
}
10599

106100
ew := &exchangeWrapper[*types.SignedHeader]{
107101
p2pExchange: mockEx,
108-
daStore: mocks.NewMockStore(t),
109102
getterByHeight: getterByHeight,
110103
}
111104

@@ -114,3 +107,75 @@ func TestExchangeWrapper_GetByHeight(t *testing.T) {
114107
assert.Equal(t, expectedHeader, h)
115108
})
116109
}
110+
111+
func TestExchangeWrapper_GetRangeByHeight(t *testing.T) {
112+
ctx := context.Background()
113+
114+
t.Run("All from DA", func(t *testing.T) {
115+
mockEx := extmocks.NewMockP2PExchange[*types.SignedHeader](t)
116+
117+
headers := []*types.SignedHeader{
118+
{Header: types.Header{BaseHeader: types.BaseHeader{Height: 2}}},
119+
{Header: types.Header{BaseHeader: types.BaseHeader{Height: 3}}},
120+
}
121+
from := &types.SignedHeader{Header: types.Header{BaseHeader: types.BaseHeader{Height: 1}}}
122+
123+
rangeGetter := func(ctx context.Context, fromH, toH uint64) ([]*types.SignedHeader, uint64, error) {
124+
return headers, 4, nil
125+
}
126+
127+
ew := &exchangeWrapper[*types.SignedHeader]{
128+
p2pExchange: mockEx,
129+
rangeGetter: rangeGetter,
130+
}
131+
132+
result, err := ew.GetRangeByHeight(ctx, from, 4)
133+
assert.NoError(t, err)
134+
assert.Equal(t, headers, result)
135+
})
136+
137+
t.Run("Partial from DA then P2P", func(t *testing.T) {
138+
mockEx := extmocks.NewMockP2PExchange[*types.SignedHeader](t)
139+
140+
daHeaders := []*types.SignedHeader{
141+
{Header: types.Header{BaseHeader: types.BaseHeader{Height: 2}}},
142+
}
143+
p2pHeaders := []*types.SignedHeader{
144+
{Header: types.Header{BaseHeader: types.BaseHeader{Height: 3}}},
145+
}
146+
from := &types.SignedHeader{Header: types.Header{BaseHeader: types.BaseHeader{Height: 1}}}
147+
148+
rangeGetter := func(ctx context.Context, fromH, toH uint64) ([]*types.SignedHeader, uint64, error) {
149+
return daHeaders, 3, nil // only got height 2, need 3+
150+
}
151+
mockEx.EXPECT().GetRangeByHeight(ctx, daHeaders[0], uint64(4)).Return(p2pHeaders, nil)
152+
153+
ew := &exchangeWrapper[*types.SignedHeader]{
154+
p2pExchange: mockEx,
155+
rangeGetter: rangeGetter,
156+
}
157+
158+
result, err := ew.GetRangeByHeight(ctx, from, 4)
159+
assert.NoError(t, err)
160+
assert.Equal(t, append(daHeaders, p2pHeaders...), result)
161+
})
162+
163+
t.Run("No range getter fallback to P2P", func(t *testing.T) {
164+
mockEx := extmocks.NewMockP2PExchange[*types.SignedHeader](t)
165+
166+
from := &types.SignedHeader{Header: types.Header{BaseHeader: types.BaseHeader{Height: 1}}}
167+
expected := []*types.SignedHeader{
168+
{Header: types.Header{BaseHeader: types.BaseHeader{Height: 2}}},
169+
}
170+
mockEx.EXPECT().GetRangeByHeight(ctx, from, uint64(3)).Return(expected, nil)
171+
172+
ew := &exchangeWrapper[*types.SignedHeader]{
173+
p2pExchange: mockEx,
174+
rangeGetter: nil,
175+
}
176+
177+
result, err := ew.GetRangeByHeight(ctx, from, 3)
178+
assert.NoError(t, err)
179+
assert.Equal(t, expected, result)
180+
})
181+
}

0 commit comments

Comments
 (0)