Skip to content

Commit 7bac45a

Browse files
tuxcanflytuxcanfly
committed
fix(manager): refactor getRemainingSleep
1 parent 11b43ae commit 7bac45a

2 files changed

Lines changed: 163 additions & 176 deletions

File tree

‎block/manager.go‎

Lines changed: 84 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ import (
3535
"github.com/rollkit/rollkit/types"
3636
)
3737

38+
// defaultLazyBufferTime is the additional time to wait to accumulate transactions
39+
// in lazy mode
40+
const defaultLazyBufferTime = 1 * time.Second
41+
3842
// defaultDABlockTime is used only if DABlockTime is not configured for manager
3943
const defaultDABlockTime = 15 * time.Second
4044

@@ -347,6 +351,34 @@ func (m *Manager) IsDAIncluded(hash types.Hash) bool {
347351
return m.blockCache.isDAIncluded(hash.String())
348352
}
349353

354+
// getRemainingSleep calculates the remaining sleep time based on config and a start time.
355+
func (m *Manager) getRemainingSleep(start time.Time) time.Duration {
356+
elapsed := time.Since(start)
357+
interval := m.conf.BlockTime
358+
359+
if m.conf.LazyAggregator {
360+
if m.buildingBlock {
361+
interval = m.conf.BlockTime
362+
// defaultLazyBufferTime is used to give time for transactions to
363+
// accumulate if we are coming out of a period of inactivity. If we
364+
// had recently produced a block (i.e. within the block time) then
365+
// we will sleep for the remaining time within the block time
366+
// interval.
367+
if elapsed >= interval {
368+
return defaultLazyBufferTime
369+
}
370+
} else {
371+
interval = m.conf.LazyBlockTime
372+
}
373+
}
374+
375+
if elapsed < interval {
376+
return interval - elapsed
377+
}
378+
379+
return 0
380+
}
381+
350382
// AggregationLoop is responsible for aggregating transactions into rollup-blocks.
351383
func (m *Manager) AggregationLoop(ctx context.Context) {
352384
initialHeight := uint64(m.genesis.InitialHeight)
@@ -376,71 +408,69 @@ func (m *Manager) AggregationLoop(ctx context.Context) {
376408
// In Lazy Aggregator mode, blocks are built only when there are
377409
// transactions or every LazyBlockTime.
378410
if m.conf.LazyAggregator {
379-
// start is used to track the start time of the block production period
380-
var start time.Time
381-
382-
// lazyTimer is used to signal when a block should be built in
383-
// lazy mode to signal that the chain is still live during long
384-
// periods of inactivity.
385-
lazyTimer := time.NewTimer(0)
386-
defer lazyTimer.Stop()
387-
for {
388-
select {
389-
case <-ctx.Done():
390-
return
391-
// the txsAvailable channel is signalled when Txns become available
392-
// in the mempool, or after transactions remain in the mempool after
393-
// building a block.
394-
case _, ok := <-m.txsAvailable:
395-
if ok && !m.buildingBlock {
396-
// set the buildingBlock flag to prevent multiple calls to reset the timer
397-
m.buildingBlock = true
398-
// Reset the block timer based on the block time and the default sleep.
399-
// The default sleep is used to give time for transactions to accumulate
400-
// if we are coming out of a period of inactivity. If we had recently
401-
// produced a block (i.e. within the block time) then we will sleep for
402-
// the remaining time within the block time interval.
403-
blockTimer.Reset(m.getRemainingSleep(start))
411+
m.lazyAggregationLoop(ctx, blockTimer)
412+
return
413+
}
404414

405-
}
406-
continue
407-
case <-lazyTimer.C:
408-
case <-blockTimer.C:
409-
}
410-
// Define the start time for the block production period
411-
start = time.Now()
412-
err := m.publishBlock(ctx)
413-
if err != nil && ctx.Err() == nil {
414-
m.logger.Error("error while publishing block", "error", err)
415+
m.normalAggregationLoop(ctx, blockTimer)
416+
}
417+
418+
func (m *Manager) lazyAggregationLoop(ctx context.Context, blockTimer *time.Timer) {
419+
// start is used to track the start time of the block production period
420+
start := time.Now()
421+
// lazyTimer is used to signal when a block should be built in
422+
// lazy mode to signal that the chain is still live during long
423+
// periods of inactivity.
424+
lazyTimer := time.NewTimer(0)
425+
defer lazyTimer.Stop()
426+
427+
for {
428+
select {
429+
case <-ctx.Done():
430+
return
431+
// the txsAvailable channel is signalled when Txns become available
432+
// in the mempool, or after transactions remain in the mempool after
433+
// building a block.
434+
case _, ok := <-m.txsAvailable:
435+
if ok && !m.buildingBlock {
436+
// set the buildingBlock flag to prevent multiple calls to reset the time
437+
m.buildingBlock = true
438+
// Reset the block timer based on the block time.
439+
blockTimer.Reset(m.getRemainingSleep(start))
415440
}
416-
// unset the buildingBlocks flag
417-
m.buildingBlock = false
418-
// Reset the lazyTimer to produce a block even if there
419-
// are no transactions as a way to signal that the chain
420-
// is still live. Default sleep is set to 0 because care
421-
// about producing blocks on time vs giving time for
422-
// transactions to accumulate.
423-
lazyTimer.Reset(m.getRemainingSleep(start))
441+
continue
442+
case <-lazyTimer.C:
443+
case <-blockTimer.C:
424444
}
445+
// Define the start time for the block production period
446+
start = time.Now()
447+
if err := m.publishBlock(ctx); err != nil && ctx.Err() == nil {
448+
m.logger.Error("error while publishing block", "error", err)
449+
}
450+
// unset the buildingBlocks flag
451+
m.buildingBlock = false
452+
// Reset the lazyTimer to produce a block even if there
453+
// are no transactions as a way to signal that the chain
454+
// is still live.
455+
lazyTimer.Reset(m.getRemainingSleep(start))
425456
}
457+
}
426458

427-
// Normal Aggregator mode
459+
func (m *Manager) normalAggregationLoop(ctx context.Context, blockTimer *time.Timer) {
428460
for {
429461
select {
430462
case <-ctx.Done():
431463
return
432464
case <-blockTimer.C:
465+
// Define the start time for the block production period
466+
start := time.Now()
467+
if err := m.publishBlock(ctx); err != nil && ctx.Err() == nil {
468+
m.logger.Error("error while publishing block", "error", err)
469+
}
470+
// Reset the blockTimer to signal the next block production
471+
// period based on the block time.
472+
blockTimer.Reset(m.getRemainingSleep(start))
433473
}
434-
start := time.Now()
435-
err := m.publishBlock(ctx)
436-
if err != nil && ctx.Err() == nil {
437-
m.logger.Error("error while publishing block", "error", err)
438-
}
439-
// Reset the blockTimer to signal the next block production
440-
// period based on the block time. Default sleep is set to 0
441-
// because care about producing blocks on time vs giving time
442-
// for transactions to accumulate.
443-
blockTimer.Reset(m.getRemainingSleep(start))
444474
}
445475
}
446476

@@ -759,34 +789,6 @@ func (m *Manager) fetchBlock(ctx context.Context, daHeight uint64) (da.ResultRet
759789
return blockRes, err
760790
}
761791

762-
// getRemainingSleep calculates the remaining sleep time based on config and a start time.
763-
func (m *Manager) getRemainingSleep(start time.Time) time.Duration {
764-
// Initialize the sleep duration to the default sleep duration to cover
765-
// the case where more time has past than the interval duration.
766-
sleepDuration := time.Duration(0)
767-
// Calculate the time elapsed since the start time.
768-
elapse := time.Since(start)
769-
770-
interval := m.conf.BlockTime
771-
if m.conf.LazyAggregator {
772-
// If it's in lazy aggregator mode and is buildingBlock, reset sleepDuration for
773-
// blockTimer, else reset interval for lazyTimer
774-
if m.buildingBlock {
775-
sleepDuration = m.conf.BlockTime
776-
} else {
777-
interval = m.conf.LazyBlockTime
778-
}
779-
}
780-
781-
// If less time has elapsed than the interval duration, calculate the
782-
// remaining time to sleep.
783-
if elapse < interval {
784-
sleepDuration = interval - elapse
785-
}
786-
787-
return sleepDuration
788-
}
789-
790792
func (m *Manager) getSignature(header types.Header) (*types.Signature, error) {
791793
// note: for compatibility with tendermint light client
792794
consensusVote := header.MakeCometBFTVote()

‎block/manager_test.go‎

Lines changed: 79 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -536,110 +536,95 @@ func Test_publishBlock_ManagerNotProposer(t *testing.T) {
536536
require.ErrorIs(err, ErrNotProposer)
537537
}
538538

539-
func Test_getRemainingSleep_OverTheInterval(t *testing.T) {
540-
now := time.Now()
541-
blockTime := 1 * time.Second
542-
lazyBlockTime := 10 * time.Second
543-
544-
// start height over the interval in the past
545-
farPastStart := now.Add(-blockTime - 1)
546-
farPastStartLazy := now.Add(-lazyBlockTime - 1)
547-
548-
var tests = []struct {
549-
name string
550-
start time.Time
551-
blockTime time.Duration
552-
lazyBlockTime time.Duration
553-
lazyAggregator bool
554-
buildingBlock bool
555-
expectedSleep time.Duration
539+
func TestManager_getRemainingSleep(t *testing.T) {
540+
tests := []struct {
541+
name string
542+
manager *Manager
543+
start time.Time
544+
expectedSleep time.Duration
556545
}{
557546
{
558-
"nil case",
559-
time.Time{}, 0, 0,
560-
false, false,
561-
0,
562-
},
563-
{"normal aggregator mode",
564-
farPastStart, blockTime, lazyBlockTime,
565-
false, false,
566-
0,
547+
name: "Normal aggregation, elapsed < interval",
548+
manager: &Manager{
549+
conf: config.BlockManagerConfig{
550+
BlockTime: 10 * time.Second,
551+
LazyBlockTime: 20 * time.Second,
552+
LazyAggregator: false,
553+
},
554+
},
555+
start: time.Now().Add(-5 * time.Second),
556+
expectedSleep: 5 * time.Second,
567557
},
568-
{"aggregator mode - building block",
569-
farPastStart, blockTime, lazyBlockTime,
570-
true, true,
571-
blockTime,
558+
{
559+
name: "Normal aggregation, elapsed >= interval",
560+
manager: &Manager{
561+
conf: config.BlockManagerConfig{
562+
BlockTime: 10 * time.Second,
563+
LazyBlockTime: 20 * time.Second,
564+
LazyAggregator: false,
565+
},
566+
},
567+
start: time.Now().Add(-15 * time.Second),
568+
expectedSleep: 0,
572569
},
573-
{"aggregator mode - not building block",
574-
farPastStartLazy, blockTime, lazyBlockTime,
575-
true, false,
576-
0,
570+
{
571+
name: "Lazy aggregation, building block, elapsed < interval",
572+
manager: &Manager{
573+
conf: config.BlockManagerConfig{
574+
BlockTime: 10 * time.Second,
575+
LazyBlockTime: 20 * time.Second,
576+
LazyAggregator: true,
577+
},
578+
buildingBlock: true,
579+
},
580+
start: time.Now().Add(-5 * time.Second),
581+
expectedSleep: 5 * time.Second,
577582
},
578-
}
579-
580-
for _, test := range tests {
581-
m := Manager{
582-
conf: config.BlockManagerConfig{
583-
BlockTime: test.blockTime,
584-
LazyBlockTime: test.lazyBlockTime,
585-
LazyAggregator: test.lazyAggregator,
583+
{
584+
name: "Lazy aggregation, building block, elapsed >= interval",
585+
manager: &Manager{
586+
conf: config.BlockManagerConfig{
587+
BlockTime: 10 * time.Second,
588+
LazyBlockTime: 20 * time.Second,
589+
LazyAggregator: true,
590+
},
591+
buildingBlock: true,
586592
},
587-
buildingBlock: test.buildingBlock,
588-
}
589-
590-
assert.Equalf(t, test.expectedSleep, m.getRemainingSleep(test.start), "test case: %s", test.name)
591-
592-
}
593-
594-
}
595-
596-
func Test_getRemainingSleep_OverInterval(t *testing.T) {
597-
now := time.Now()
598-
blockTime := 1 * time.Second
599-
lazyBlockTime := 10 * time.Second
600-
601-
// start height over interval in the past
602-
start := now.Add(-blockTime / 2)
603-
startLazy := now.Add(-lazyBlockTime / 2)
604-
605-
var tests = []struct {
606-
name string
607-
start time.Time
608-
blockTime time.Duration
609-
lazyBlockTime time.Duration
610-
lazyAggregator bool
611-
buildingBlock bool
612-
compareTime time.Duration
613-
}{
614-
{"normal aggregator mode",
615-
start, blockTime, lazyBlockTime,
616-
false, false,
617-
blockTime,
593+
start: time.Now().Add(-15 * time.Second),
594+
expectedSleep: defaultLazyBufferTime,
618595
},
619-
{"aggregator mode - building block",
620-
start, blockTime, lazyBlockTime,
621-
true, true,
622-
blockTime,
596+
{
597+
name: "Lazy aggregation, not building block, elapsed < interval",
598+
manager: &Manager{
599+
conf: config.BlockManagerConfig{
600+
BlockTime: 10 * time.Second,
601+
LazyBlockTime: 20 * time.Second,
602+
LazyAggregator: true,
603+
},
604+
buildingBlock: false,
605+
},
606+
start: time.Now().Add(-5 * time.Second),
607+
expectedSleep: 15 * time.Second,
623608
},
624-
{"aggregator mode - not building block",
625-
startLazy, blockTime, lazyBlockTime,
626-
true, false,
627-
lazyBlockTime,
609+
{
610+
name: "Lazy aggregation, not building block, elapsed >= interval",
611+
manager: &Manager{
612+
conf: config.BlockManagerConfig{
613+
BlockTime: 10 * time.Second,
614+
LazyBlockTime: 20 * time.Second,
615+
LazyAggregator: true,
616+
},
617+
buildingBlock: false,
618+
},
619+
start: time.Now().Add(-25 * time.Second),
620+
expectedSleep: 0,
628621
},
629622
}
630623

631-
for _, test := range tests {
632-
m := Manager{
633-
conf: config.BlockManagerConfig{
634-
BlockTime: test.blockTime,
635-
LazyBlockTime: test.lazyBlockTime,
636-
LazyAggregator: test.lazyAggregator,
637-
},
638-
buildingBlock: test.buildingBlock,
639-
}
640-
641-
assert.Truef(t, test.compareTime > m.getRemainingSleep(test.start), "test case: %s", test.name)
642-
624+
for _, tt := range tests {
625+
t.Run(tt.name, func(t *testing.T) {
626+
actualSleep := tt.manager.getRemainingSleep(tt.start)
627+
assert.GreaterOrEqual(t, tt.expectedSleep, actualSleep)
628+
})
643629
}
644-
645630
}

0 commit comments

Comments
 (0)