Skip to content

Commit 1cfb889

Browse files
julienrbrtjulienrbrt
committed
feedback
1 parent c50cb0f commit 1cfb889

4 files changed

Lines changed: 509 additions & 237 deletions

File tree

‎block/internal/syncing/syncer.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,9 @@ func (s *Syncer) syncLoop() {
273273
// no data at this height, increase DA height
274274
// we do still want to check p2p
275275
s.SetDAHeight(s.GetDAHeight() + 1)
276+
277+
// Reset backoff on success
278+
nextDARequestAt = time.Time{}
276279
} else {
277280
// Back off exactly by DA block time to avoid overloading
278281
hffDelay = s.config.DA.BlockTime.Duration

‎block/internal/syncing/syncer_backoff_test.go‎

Lines changed: 348 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,348 @@
1+
package syncing
2+
3+
import (
4+
"context"
5+
"errors"
6+
"testing"
7+
"time"
8+
9+
"github.com/ipfs/go-datastore"
10+
dssync "github.com/ipfs/go-datastore/sync"
11+
"github.com/rs/zerolog"
12+
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/mock"
14+
"github.com/stretchr/testify/require"
15+
16+
"github.com/evstack/ev-node/block/internal/cache"
17+
"github.com/evstack/ev-node/block/internal/common"
18+
coreda "github.com/evstack/ev-node/core/da"
19+
"github.com/evstack/ev-node/core/execution"
20+
"github.com/evstack/ev-node/pkg/config"
21+
"github.com/evstack/ev-node/pkg/genesis"
22+
"github.com/evstack/ev-node/pkg/store"
23+
mocks "github.com/evstack/ev-node/test/mocks/external"
24+
"github.com/evstack/ev-node/types"
25+
)
26+
27+
// TestSyncer_BackoffOnDAError verifies that the syncer implements proper backoff
28+
// behavior when encountering different types of DA layer errors.
29+
func TestSyncer_BackoffOnDAError(t *testing.T) {
30+
tests := map[string]struct {
31+
daBlockTime time.Duration
32+
error error
33+
expectsBackoff bool
34+
description string
35+
}{
36+
"generic_error_triggers_backoff": {
37+
daBlockTime: 1 * time.Second,
38+
error: errors.New("network failure"),
39+
expectsBackoff: true,
40+
description: "Generic DA errors should trigger backoff",
41+
},
42+
"height_from_future_triggers_backoff": {
43+
daBlockTime: 500 * time.Millisecond,
44+
error: coreda.ErrHeightFromFuture,
45+
expectsBackoff: true,
46+
description: "Height from future should trigger backoff",
47+
},
48+
"blob_not_found_no_backoff": {
49+
daBlockTime: 1 * time.Second,
50+
error: coreda.ErrBlobNotFound,
51+
expectsBackoff: false,
52+
description: "ErrBlobNotFound should not trigger backoff",
53+
},
54+
"zero_block_time_fallback": {
55+
daBlockTime: 0, // Should fallback to 2s
56+
error: errors.New("some error"),
57+
expectsBackoff: true,
58+
description: "Zero block time should use 2s fallback",
59+
},
60+
}
61+
62+
for name, tc := range tests {
63+
t.Run(name, func(t *testing.T) {
64+
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
65+
defer cancel()
66+
67+
// Setup syncer
68+
syncer := setupTestSyncer(t, tc.daBlockTime)
69+
syncer.ctx = ctx
70+
71+
// Setup mocks
72+
daRetriever := newMockdaRetriever(t)
73+
p2pHandler := newMockp2pHandler(t)
74+
syncer.daRetriever = daRetriever
75+
syncer.p2pHandler = p2pHandler
76+
77+
headerStore := mocks.NewMockStore[*types.SignedHeader](t)
78+
headerStore.On("Height").Return(uint64(0)).Maybe()
79+
syncer.headerStore = headerStore
80+
81+
dataStore := mocks.NewMockStore[*types.Data](t)
82+
dataStore.On("Height").Return(uint64(0)).Maybe()
83+
syncer.dataStore = dataStore
84+
85+
var callTimes []time.Time
86+
callCount := 0
87+
88+
// First call - returns test error
89+
daRetriever.On("RetrieveFromDA", mock.Anything, uint64(100)).
90+
Run(func(args mock.Arguments) {
91+
callTimes = append(callTimes, time.Now())
92+
callCount++
93+
}).
94+
Return(nil, tc.error).Once()
95+
96+
if tc.expectsBackoff {
97+
// Second call should be delayed due to backoff
98+
daRetriever.On("RetrieveFromDA", mock.Anything, uint64(100)).
99+
Run(func(args mock.Arguments) {
100+
callTimes = append(callTimes, time.Now())
101+
callCount++
102+
// Cancel to end test
103+
cancel()
104+
}).
105+
Return(nil, coreda.ErrBlobNotFound).Once()
106+
} else {
107+
// For ErrBlobNotFound, DA height should increment
108+
daRetriever.On("RetrieveFromDA", mock.Anything, uint64(101)).
109+
Run(func(args mock.Arguments) {
110+
callTimes = append(callTimes, time.Now())
111+
callCount++
112+
cancel()
113+
}).
114+
Return(nil, coreda.ErrBlobNotFound).Once()
115+
}
116+
117+
// Run sync loop
118+
done := make(chan struct{})
119+
go func() {
120+
defer close(done)
121+
syncer.syncLoop()
122+
}()
123+
124+
<-done
125+
126+
// Verify behavior
127+
if tc.expectsBackoff {
128+
require.Len(t, callTimes, 2, "should make exactly 2 calls with backoff")
129+
130+
timeBetweenCalls := callTimes[1].Sub(callTimes[0])
131+
expectedDelay := tc.daBlockTime
132+
if expectedDelay == 0 {
133+
expectedDelay = 2 * time.Second
134+
}
135+
136+
assert.GreaterOrEqual(t, timeBetweenCalls, expectedDelay-50*time.Millisecond,
137+
"second call should be delayed by backoff duration (expected ~%v, got %v)",
138+
expectedDelay, timeBetweenCalls)
139+
} else {
140+
assert.GreaterOrEqual(t, callCount, 2, "should continue without significant delay")
141+
if len(callTimes) >= 2 {
142+
timeBetweenCalls := callTimes[1].Sub(callTimes[0])
143+
assert.Less(t, timeBetweenCalls, 100*time.Millisecond,
144+
"should not have backoff delay for ErrBlobNotFound")
145+
}
146+
}
147+
})
148+
}
149+
}
150+
151+
// TestSyncer_BackoffResetOnSuccess verifies that backoff is properly reset
152+
// after a successful DA retrieval, allowing the syncer to continue at normal speed.
153+
func TestSyncer_BackoffResetOnSuccess(t *testing.T) {
154+
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
155+
defer cancel()
156+
157+
syncer := setupTestSyncer(t, 1*time.Second)
158+
syncer.ctx = ctx
159+
160+
addr, pub, signer := buildSyncTestSigner(t)
161+
gen := syncer.genesis
162+
163+
daRetriever := newMockdaRetriever(t)
164+
p2pHandler := newMockp2pHandler(t)
165+
syncer.daRetriever = daRetriever
166+
syncer.p2pHandler = p2pHandler
167+
168+
headerStore := mocks.NewMockStore[*types.SignedHeader](t)
169+
headerStore.On("Height").Return(uint64(0)).Maybe()
170+
syncer.headerStore = headerStore
171+
172+
dataStore := mocks.NewMockStore[*types.Data](t)
173+
dataStore.On("Height").Return(uint64(0)).Maybe()
174+
syncer.dataStore = dataStore
175+
176+
var callTimes []time.Time
177+
178+
// First call - error (should trigger backoff)
179+
daRetriever.On("RetrieveFromDA", mock.Anything, uint64(100)).
180+
Run(func(args mock.Arguments) {
181+
callTimes = append(callTimes, time.Now())
182+
}).
183+
Return(nil, errors.New("temporary failure")).Once()
184+
185+
// Second call - success (should reset backoff and increment DA height)
186+
_, header := makeSignedHeaderBytes(t, gen.ChainID, 1, addr, pub, signer, nil, nil)
187+
data := &types.Data{
188+
Metadata: &types.Metadata{
189+
ChainID: gen.ChainID,
190+
Height: 1,
191+
Time: uint64(time.Now().UnixNano()),
192+
},
193+
}
194+
event := common.DAHeightEvent{
195+
Header: header,
196+
Data: data,
197+
DaHeight: 100,
198+
HeaderDaIncludedHeight: 100,
199+
}
200+
201+
daRetriever.On("RetrieveFromDA", mock.Anything, uint64(100)).
202+
Run(func(args mock.Arguments) {
203+
callTimes = append(callTimes, time.Now())
204+
}).
205+
Return([]common.DAHeightEvent{event}, nil).Once()
206+
207+
// Third call - should happen immediately after success (DA height incremented to 101)
208+
daRetriever.On("RetrieveFromDA", mock.Anything, uint64(101)).
209+
Run(func(args mock.Arguments) {
210+
callTimes = append(callTimes, time.Now())
211+
cancel()
212+
}).
213+
Return(nil, coreda.ErrBlobNotFound).Once()
214+
215+
// Start process loop to handle events
216+
go syncer.processLoop()
217+
218+
// Run sync loop
219+
done := make(chan struct{})
220+
go func() {
221+
defer close(done)
222+
syncer.syncLoop()
223+
}()
224+
225+
<-done
226+
227+
require.Len(t, callTimes, 3, "should make exactly 3 calls")
228+
229+
// Verify backoff between first and second call
230+
delay1to2 := callTimes[1].Sub(callTimes[0])
231+
assert.GreaterOrEqual(t, delay1to2, 950*time.Millisecond,
232+
"should have backed off between error and success (got %v)", delay1to2)
233+
234+
// Verify no backoff between second and third call (backoff reset)
235+
delay2to3 := callTimes[2].Sub(callTimes[1])
236+
assert.Less(t, delay2to3, 100*time.Millisecond,
237+
"should continue immediately after success (got %v)", delay2to3)
238+
}
239+
240+
// TestSyncer_BackoffBehaviorIntegration tests the complete backoff flow:
241+
// error -> backoff delay -> recovery -> normal operation.
242+
func TestSyncer_BackoffBehaviorIntegration(t *testing.T) {
243+
// Test simpler backoff behavior: error -> backoff -> success -> continue
244+
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
245+
defer cancel()
246+
247+
syncer := setupTestSyncer(t, 500*time.Millisecond)
248+
syncer.ctx = ctx
249+
250+
daRetriever := newMockdaRetriever(t)
251+
p2pHandler := newMockp2pHandler(t)
252+
syncer.daRetriever = daRetriever
253+
syncer.p2pHandler = p2pHandler
254+
255+
headerStore := mocks.NewMockStore[*types.SignedHeader](t)
256+
headerStore.On("Height").Return(uint64(0)).Maybe()
257+
syncer.headerStore = headerStore
258+
259+
dataStore := mocks.NewMockStore[*types.Data](t)
260+
dataStore.On("Height").Return(uint64(0)).Maybe()
261+
syncer.dataStore = dataStore
262+
263+
var callTimes []time.Time
264+
265+
// First call - error (triggers backoff)
266+
daRetriever.On("RetrieveFromDA", mock.Anything, uint64(100)).
267+
Run(func(args mock.Arguments) {
268+
callTimes = append(callTimes, time.Now())
269+
}).
270+
Return(nil, errors.New("network error")).Once()
271+
272+
// Second call - should be delayed due to backoff
273+
daRetriever.On("RetrieveFromDA", mock.Anything, uint64(100)).
274+
Run(func(args mock.Arguments) {
275+
callTimes = append(callTimes, time.Now())
276+
}).
277+
Return(nil, coreda.ErrBlobNotFound).Once()
278+
279+
// Third call - should continue without delay (DA height incremented)
280+
daRetriever.On("RetrieveFromDA", mock.Anything, uint64(101)).
281+
Run(func(args mock.Arguments) {
282+
callTimes = append(callTimes, time.Now())
283+
cancel()
284+
}).
285+
Return(nil, coreda.ErrBlobNotFound).Once()
286+
287+
go syncer.processLoop()
288+
289+
done := make(chan struct{})
290+
go func() {
291+
defer close(done)
292+
syncer.syncLoop()
293+
}()
294+
295+
<-done
296+
297+
require.Len(t, callTimes, 3, "should make exactly 3 calls")
298+
299+
// First to second call should be delayed (backoff)
300+
delay1to2 := callTimes[1].Sub(callTimes[0])
301+
assert.GreaterOrEqual(t, delay1to2, 450*time.Millisecond,
302+
"should have backoff delay between first and second call (got %v)", delay1to2)
303+
304+
// Second to third call should be immediate (no backoff after ErrBlobNotFound)
305+
delay2to3 := callTimes[2].Sub(callTimes[1])
306+
assert.Less(t, delay2to3, 100*time.Millisecond,
307+
"should continue immediately after ErrBlobNotFound (got %v)", delay2to3)
308+
}
309+
310+
func setupTestSyncer(t *testing.T, daBlockTime time.Duration) *Syncer {
311+
t.Helper()
312+
313+
ds := dssync.MutexWrap(datastore.NewMapDatastore())
314+
st := store.New(ds)
315+
cm, err := cache.NewManager(config.DefaultConfig(), st, zerolog.Nop())
316+
require.NoError(t, err)
317+
318+
addr, _, _ := buildSyncTestSigner(t)
319+
320+
cfg := config.DefaultConfig()
321+
cfg.DA.BlockTime.Duration = daBlockTime
322+
cfg.DA.StartHeight = 100
323+
324+
gen := genesis.Genesis{
325+
ChainID: "test-chain",
326+
InitialHeight: 1,
327+
StartTime: time.Now().Add(-time.Hour), // Start in past
328+
ProposerAddress: addr,
329+
}
330+
331+
syncer := NewSyncer(
332+
st,
333+
execution.NewDummyExecutor(),
334+
nil,
335+
cm,
336+
common.NopMetrics(),
337+
cfg,
338+
gen,
339+
nil,
340+
nil,
341+
zerolog.Nop(),
342+
common.DefaultBlockOptions(),
343+
make(chan error, 1),
344+
)
345+
346+
require.NoError(t, syncer.initializeState())
347+
return syncer
348+
}

0 commit comments

Comments
 (0)