Skip to content

Commit 2727200

Browse files
tac0turtletac0turtle
tac0turtle
and
tac0turtle
authored
test: increase test coverage (block manager) (#2219)
<!-- Please read and fill out this form before submitting your PR. Please make sure you have reviewed our contributors guide before submitting your first PR. NOTE: PR titles should follow semantic commits: https://www.conventionalcommits.org/en/v1.0.0/ --> ## Overview This pr adds more test cases to the block manager. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced robust block aggregation, retrieval, synchronization, and header submission loops for improved block management and data availability integration. - Added flexible block publishing logic with support for both normal and lazy aggregation modes. - Enhanced error handling and retry mechanisms for header submission and retrieval. - **Bug Fixes** - Improved handling of edge cases such as empty data hashes and duplicate events during synchronization. - **Refactor** - Refactored signing logic to use a signer abstraction instead of direct private key usage. - Streamlined manager structure by removing proposer-related logic and redundant methods for clarity and maintainability. - Changed encoding of batch data length prefixes and block height metadata from big endian to little endian for consistency. - Renamed genesis start height field to better reflect its timestamp semantics. - **Tests** - Added comprehensive unit and integration tests for aggregation, publishing, retrieval, synchronization, and store loops. - Introduced new mocks and utilities to facilitate robust testing. - Removed some deprecated tests and replaced them with focused unit tests. - **Chores** - Updated `.gitignore` to ignore coverage output files and adjusted code coverage configuration. - Added scripts and make targets for test coverage reporting and mock generation. - Improved test execution commands to include coverage reporting. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: tac0turtle <you@example.com>
1 parent addcb75 commit 2727200

28 files changed

Lines changed: 3608 additions & 1186 deletions

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,4 @@ types/pb/tendermint
1111
build
1212
.aider*
1313
.DS_Store
14+
coverage.out

‎block/aggregation.go‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
package block
2+
3+
import (
4+
"context"
5+
"time"
6+
)
7+
8+
// AggregationLoop is responsible for aggregating transactions into rollup-blocks.
9+
func (m *Manager) AggregationLoop(ctx context.Context) {
10+
initialHeight := m.genesis.InitialHeight //nolint:gosec
11+
height, err := m.store.Height(ctx)
12+
if err != nil {
13+
m.logger.Error("error while getting store height", "error", err)
14+
return
15+
}
16+
var delay time.Duration
17+
18+
// TODO(tzdybal): double-check when https://github.com/celestiaorg/rollmint/issues/699 is resolved
19+
if height < initialHeight {
20+
delay = time.Until(m.genesis.GenesisDAStartTime.Add(m.config.Node.BlockTime.Duration))
21+
} else {
22+
lastBlockTime := m.getLastBlockTime()
23+
delay = time.Until(lastBlockTime.Add(m.config.Node.BlockTime.Duration))
24+
}
25+
26+
if delay > 0 {
27+
m.logger.Info("Waiting to produce block", "delay", delay)
28+
time.Sleep(delay)
29+
}
30+
31+
// blockTimer is used to signal when to build a block based on the
32+
// rollup block time. A timer is used so that the time to build a block
33+
// can be taken into account.
34+
blockTimer := time.NewTimer(0)
35+
defer blockTimer.Stop()
36+
37+
// Lazy Aggregator mode.
38+
// In Lazy Aggregator mode, blocks are built only when there are
39+
// transactions or every LazyBlockTime.
40+
if m.config.Node.LazyAggregator {
41+
m.lazyAggregationLoop(ctx, blockTimer)
42+
return
43+
}
44+
45+
m.normalAggregationLoop(ctx, blockTimer)
46+
}
47+
48+
func (m *Manager) lazyAggregationLoop(ctx context.Context, blockTimer *time.Timer) {
49+
// lazyTimer triggers block publication even during inactivity
50+
lazyTimer := time.NewTimer(0)
51+
defer lazyTimer.Stop()
52+
53+
for {
54+
select {
55+
case <-ctx.Done():
56+
return
57+
58+
case <-lazyTimer.C:
59+
case <-blockTimer.C:
60+
}
61+
62+
// Reset the start time
63+
start := time.Now()
64+
65+
// Attempt to publish the block regardless of activity
66+
if err := m.publishBlock(ctx); err != nil && ctx.Err() == nil {
67+
m.logger.Error("error while publishing block", "error", err)
68+
}
69+
70+
// Reset both timers for the next aggregation window
71+
lazyTimer.Reset(getRemainingSleep(start, m.config.Node.LazyBlockTime.Duration))
72+
blockTimer.Reset(getRemainingSleep(start, m.config.Node.BlockTime.Duration))
73+
}
74+
}
75+
76+
func (m *Manager) normalAggregationLoop(ctx context.Context, blockTimer *time.Timer) {
77+
for {
78+
select {
79+
case <-ctx.Done():
80+
return
81+
case <-blockTimer.C:
82+
// Define the start time for the block production period
83+
start := time.Now()
84+
85+
if err := m.publishBlock(ctx); err != nil && ctx.Err() == nil {
86+
m.logger.Error("error while publishing block", "error", err)
87+
}
88+
// Reset the blockTimer to signal the next block production
89+
// period based on the block time.
90+
blockTimer.Reset(getRemainingSleep(start, m.config.Node.BlockTime.Duration))
91+
}
92+
}
93+
}
94+
95+
func getRemainingSleep(start time.Time, interval time.Duration) time.Duration {
96+
elapsed := time.Since(start)
97+
98+
if elapsed < interval {
99+
return interval - elapsed
100+
}
101+
102+
return time.Millisecond
103+
}

‎block/aggregation_test.go‎

Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
package block
2+
3+
import (
4+
"context"
5+
"errors"
6+
"sync"
7+
"sync/atomic"
8+
"testing"
9+
"time"
10+
11+
"cosmossdk.io/log"
12+
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/mock"
14+
"github.com/stretchr/testify/require"
15+
16+
"github.com/rollkit/rollkit/pkg/cache"
17+
"github.com/rollkit/rollkit/pkg/config"
18+
genesispkg "github.com/rollkit/rollkit/pkg/genesis"
19+
"github.com/rollkit/rollkit/test/mocks"
20+
"github.com/rollkit/rollkit/types"
21+
)
22+
23+
// TestAggregationLoop_Normal_BasicInterval verifies the basic time interval logic of the normal aggregation loop.
24+
func TestAggregationLoop_Normal_BasicInterval(t *testing.T) {
25+
assert := assert.New(t)
26+
require := require.New(t)
27+
28+
blockTime := 50 * time.Millisecond
29+
waitTime := blockTime*4 + blockTime/2
30+
31+
mockStore := mocks.NewStore(t)
32+
mockStore.On("Height", mock.Anything).Return(uint64(1), nil).Maybe()
33+
mockStore.On("GetState", mock.Anything).Return(types.State{LastBlockTime: time.Now().Add(-blockTime)}, nil).Maybe()
34+
35+
mockExec := mocks.NewExecutor(t)
36+
mockSeq := mocks.NewSequencer(t)
37+
mockDAC := mocks.NewClient(t)
38+
logger := log.NewTestLogger(t)
39+
40+
m := &Manager{
41+
store: mockStore,
42+
exec: mockExec,
43+
sequencer: mockSeq,
44+
dalc: mockDAC,
45+
logger: logger,
46+
config: config.Config{
47+
Node: config.NodeConfig{
48+
BlockTime: config.DurationWrapper{Duration: blockTime},
49+
LazyAggregator: false,
50+
},
51+
DA: config.DAConfig{
52+
BlockTime: config.DurationWrapper{Duration: 1 * time.Second},
53+
},
54+
},
55+
genesis: genesispkg.Genesis{
56+
InitialHeight: 1,
57+
},
58+
lastState: types.State{
59+
LastBlockTime: time.Now().Add(-blockTime),
60+
},
61+
lastStateMtx: &sync.RWMutex{},
62+
metrics: NopMetrics(),
63+
headerCache: cache.NewCache[types.SignedHeader](),
64+
dataCache: cache.NewCache[types.Data](),
65+
}
66+
67+
var publishTimes []time.Time
68+
var publishLock sync.Mutex
69+
mockPublishBlock := func(ctx context.Context) error {
70+
publishLock.Lock()
71+
defer publishLock.Unlock()
72+
publishTimes = append(publishTimes, time.Now())
73+
m.logger.Debug("Mock publishBlock called", "time", publishTimes[len(publishTimes)-1])
74+
return nil
75+
}
76+
m.publishBlock = mockPublishBlock
77+
78+
ctx, cancel := context.WithCancel(context.Background())
79+
defer cancel()
80+
var wg sync.WaitGroup
81+
wg.Add(1)
82+
go func() {
83+
defer wg.Done()
84+
m.AggregationLoop(ctx)
85+
m.logger.Info("AggregationLoop exited")
86+
}()
87+
88+
m.logger.Info("Waiting for blocks...", "duration", waitTime)
89+
time.Sleep(waitTime)
90+
91+
m.logger.Info("Cancelling context")
92+
cancel()
93+
m.logger.Info("Waiting for WaitGroup")
94+
wg.Wait()
95+
m.logger.Info("WaitGroup finished")
96+
97+
publishLock.Lock()
98+
defer publishLock.Unlock()
99+
100+
m.logger.Info("Recorded publish times", "count", len(publishTimes), "times", publishTimes)
101+
102+
expectedCallsLow := int(waitTime/blockTime) - 1
103+
expectedCallsHigh := int(waitTime/blockTime) + 1
104+
require.GreaterOrEqualf(len(publishTimes), expectedCallsLow, "Expected at least %d calls, got %d", expectedCallsLow, len(publishTimes))
105+
require.LessOrEqualf(len(publishTimes), expectedCallsHigh, "Expected at most %d calls, got %d", expectedCallsHigh, len(publishTimes))
106+
107+
if len(publishTimes) > 1 {
108+
for i := 1; i < len(publishTimes); i++ {
109+
interval := publishTimes[i].Sub(publishTimes[i-1])
110+
m.logger.Debug("Checking interval", "index", i, "interval", interval)
111+
tolerance := blockTime / 2
112+
assert.True(WithinDuration(t, blockTime, interval, tolerance), "Interval %d (%v) not within tolerance (%v) of blockTime (%v)", i, interval, tolerance, blockTime)
113+
}
114+
}
115+
}
116+
117+
// TestAggregationLoop_Normal_PublishBlockError verifies the loop continues after publishBlock returns an error.
118+
func TestAggregationLoop_Normal_PublishBlockError(t *testing.T) {
119+
assert := assert.New(t)
120+
require := require.New(t)
121+
122+
blockTime := 50 * time.Millisecond
123+
waitTime := blockTime*4 + blockTime/2
124+
tolerance := blockTime / 2
125+
126+
mockStore := mocks.NewStore(t)
127+
mockStore.On("Height", mock.Anything).Return(uint64(1), nil).Maybe()
128+
mockStore.On("GetState", mock.Anything).Return(types.State{LastBlockTime: time.Now().Add(-blockTime)}, nil).Maybe()
129+
130+
mockExec := mocks.NewExecutor(t)
131+
mockSeq := mocks.NewSequencer(t)
132+
mockDAC := mocks.NewClient(t)
133+
134+
mockLogger := log.NewTestLogger(t)
135+
136+
// Create a basic Manager instance
137+
m := &Manager{
138+
store: mockStore,
139+
exec: mockExec,
140+
sequencer: mockSeq,
141+
dalc: mockDAC,
142+
logger: mockLogger,
143+
config: config.Config{
144+
Node: config.NodeConfig{
145+
BlockTime: config.DurationWrapper{Duration: blockTime},
146+
LazyAggregator: false,
147+
},
148+
DA: config.DAConfig{
149+
BlockTime: config.DurationWrapper{Duration: 1 * time.Second},
150+
},
151+
},
152+
genesis: genesispkg.Genesis{
153+
InitialHeight: 1,
154+
},
155+
lastState: types.State{
156+
LastBlockTime: time.Now().Add(-blockTime),
157+
},
158+
lastStateMtx: &sync.RWMutex{},
159+
metrics: NopMetrics(),
160+
headerCache: cache.NewCache[types.SignedHeader](),
161+
dataCache: cache.NewCache[types.Data](),
162+
}
163+
164+
var publishCalls atomic.Int64
165+
var publishTimes []time.Time
166+
var publishLock sync.Mutex
167+
expectedErr := errors.New("failed to publish block")
168+
169+
mockPublishBlock := func(ctx context.Context) error {
170+
callNum := publishCalls.Add(1)
171+
publishLock.Lock()
172+
publishTimes = append(publishTimes, time.Now())
173+
publishLock.Unlock()
174+
175+
if callNum == 1 {
176+
m.logger.Debug("Mock publishBlock returning error", "call", callNum)
177+
return expectedErr
178+
}
179+
m.logger.Debug("Mock publishBlock returning nil", "call", callNum)
180+
return nil
181+
}
182+
m.publishBlock = mockPublishBlock
183+
184+
ctx, cancel := context.WithCancel(context.Background())
185+
var wg sync.WaitGroup
186+
wg.Add(1)
187+
go func() {
188+
defer wg.Done()
189+
m.AggregationLoop(ctx)
190+
m.logger.Info("AggregationLoop exited")
191+
}()
192+
193+
time.Sleep(waitTime)
194+
195+
cancel()
196+
wg.Wait()
197+
198+
publishLock.Lock()
199+
defer publishLock.Unlock()
200+
201+
calls := publishCalls.Load()
202+
assert.GreaterOrEqualf(calls, int64(4), "publishBlock should have been called multiple times (around 4), but was called %d times", calls)
203+
assert.LessOrEqualf(calls, int64(5), "publishBlock should have been called multiple times (around 4-5), but was called %d times", calls)
204+
205+
require.GreaterOrEqual(len(publishTimes), 3, "Need at least 3 timestamps to check intervals after error")
206+
for i := 2; i < len(publishTimes); i++ {
207+
interval := publishTimes[i].Sub(publishTimes[i-1])
208+
WithinDuration(t, blockTime, interval, tolerance)
209+
}
210+
}

0 commit comments

Comments
 (0)