feat(block): add lazy block time as additional block production trigger in lazy aggregator mode (#1643)

<!--
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

<!-- 
Please provide an explanation of the PR, including the appropriate
context,
background, goal, and rationale. If there is an issue with this
information,
please provide a tl;dr and link the issue. 

Ex: Closes #<issue number>
-->
One problem that chains in lazy aggregator mode have is that there is no
way externally to see if the chain is still live without sending new
transactions.

The lazy block time adds an additional trigger that can produce a block
during long periods of inactivity.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Introduced a new configuration option `LazyBlockTime` to customize the
time interval for block production in lazy mode when no transactions are
present.

- **Enhancements**
- Improved block production efficiency in lazy aggregation mode by
implementing new timers and sleep interval calculations.

- **Testing**
- Added comprehensive tests to ensure the correct calculation of sleep
durations in various scenarios for block production.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Author: MSevey

block/block-manager.md

@@ -63,6 +63,7 @@ Block manager configuration options:
 |BlockTime|time.Duration|time interval used for block production and block retrieval from block store ([`defaultBlockTime`][defaultBlockTime])|
 |DABlockTime|time.Duration|time interval used for both block publication to DA network and block retrieval from DA network ([`defaultDABlockTime`][defaultDABlockTime])|
 |DAStartHeight|uint64|block retrieval from DA network starts from this height|
+|LazyBlockTime|time.Duration|time interval used for block production in lazy aggregator mode even when there are no transactions ([`defaultLazyBlockTime`][defaultLazyBlockTime])|
 
 ### Block Production
 
@@ -181,6 +182,7 @@ See [tutorial] for running a multi-node network with both sequencer and non-sequ
 [maxSubmitAttempts]: https://github.com/rollkit/rollkit/blob/main/block/manager.go#L39
 [defaultBlockTime]: https://github.com/rollkit/rollkit/blob/main/block/manager.go#L35
 [defaultDABlockTime]: https://github.com/rollkit/rollkit/blob/main/block/manager.go#L32
+[defaultLazyBlockTime]: https://github.com/rollkit/rollkit/blob/main/block/manager.go#L38
 [initialBackoff]: https://github.com/rollkit/rollkit/blob/main/block/manager.go#L48
 [go-header]: https://github.com/celestiaorg/go-header
 [block-sync]: https://github.com/rollkit/rollkit/blob/main/block/block_sync.go

block/manager.go

@@ -35,6 +35,9 @@ const defaultDABlockTime = 15 * time.Second
 // defaultBlockTime is used only if BlockTime is not configured for manager
 const defaultBlockTime = 1 * time.Second
 
+// defaultLazyBlockTime is used only if LazyBlockTime is not configured for manager
+const defaultLazyBlockTime = 60 * time.Second
+
 // defaultMempoolTTL is the number of blocks until transaction is dropped from mempool
 const defaultMempoolTTL = 25
 
@@ -185,6 +188,11 @@ func NewManager(
 		conf.BlockTime = defaultBlockTime
 	}
 
+	if conf.LazyBlockTime == 0 {
+		logger.Info("Using default lazy block time", "LazyBlockTime", defaultLazyBlockTime)
+		conf.LazyBlockTime = defaultLazyBlockTime
+	}
+
 	if conf.DAMempoolTTL == 0 {
 		logger.Info("Using default mempool ttl", "MempoolTTL", defaultMempoolTTL)
 		conf.DAMempoolTTL = defaultMempoolTTL
@@ -332,31 +340,65 @@ func (m *Manager) AggregationLoop(ctx context.Context, lazy bool) {
 		time.Sleep(delay)
 	}
 
-	timer := time.NewTimer(0)
-	defer timer.Stop()
+	// blockTimer is used to signal when to build a block based on the
+	// rollup block time. A timer is used so that the time to build a block
+	// can be taken into account.
+	blockTimer := time.NewTimer(0)
+	defer blockTimer.Stop()
 
-	// Lazy Aggregator mode
+	// Lazy Aggregator mode.
+	// In Lazy Aggregator mode, blocks are built only when there are
+	// transactions or every LazyBlockTime.
 	if lazy {
+		// start is used to track the start time of the block production period
+		var start time.Time
+
+		// defaultSleep is used when coming out of a period of inactivity,
+		// to try and pull in more transactions in the first block
+		defaultSleep := time.Second
+
+		// lazyTimer is used to signal when a block should be built in
+		// lazy mode to signal that the chain is still live during long
+		// periods of inactivity.
+		lazyTimer := time.NewTimer(0)
+		defer lazyTimer.Stop()
 		for {
 			select {
 			case <-ctx.Done():
 				return
-			// the buildBlock channel is signalled when Txns become available
+			// the txsAvailable channel is signalled when Txns become available
 			// in the mempool, or after transactions remain in the mempool after
 			// building a block.
 			case _, ok := <-m.txsAvailable:
 				if ok && !m.buildingBlock {
+					// set the buildingBlock flag to prevent multiple calls to reset the timer
 					m.buildingBlock = true
-					timer.Reset(1 * time.Second)
-				}
-			case <-timer.C:
-				// build a block with all the transactions received in the last 1 second
-				err := m.publishBlock(ctx)
-				if err != nil && ctx.Err() == nil {
-					m.logger.Error("error while publishing block", "error", err)
+					// Reset the block timer based on the block time and the default sleep.
+					// The default sleep is used to give time for transactions to accumulate
+					// if we are coming out of a period of inactivity.  If we had recently
+					// produced a block (i.e. within the block time) then we will sleep for
+					// the remaining time within the block time interval.
+					blockTimer.Reset(getRemainingSleep(start, m.conf.BlockTime, defaultSleep))
+
 				}
-				m.buildingBlock = false
+				continue
+			case <-lazyTimer.C:
+			case <-blockTimer.C:
 			}
+			// Define the start time for the block production period
+			start = time.Now()
+			err := m.publishBlock(ctx)
+			if err != nil && ctx.Err() == nil {
+				m.logger.Error("error while publishing block", "error", err)
+			}
+			// unset the buildingBlocks flag
+			m.buildingBlock = false
+			// Reset the lazyTimer to produce a block even if there
+			// are no transactions as a way to signal that the chain
+			// is still live. Default sleep is set to 0 because care
+			// about producing blocks on time vs giving time for
+			// transactions to accumulate.
+			lazyTimer.Reset(getRemainingSleep(start, m.conf.LazyBlockTime, 0))
 		}
 	}
 
@@ -365,14 +407,18 @@ func (m *Manager) AggregationLoop(ctx context.Context, lazy bool) {
 		select {
 		case <-ctx.Done():
 			return
-		case <-timer.C:
+		case <-blockTimer.C:
 		}
 		start := time.Now()
 		err := m.publishBlock(ctx)
 		if err != nil && ctx.Err() == nil {
 			m.logger.Error("error while publishing block", "error", err)
 		}
-		timer.Reset(m.getRemainingSleep(start))
+		// Reset the blockTimer to signal the next block production
+		// period based on the block time. Default sleep is set to 0
+		// because care about producing blocks on time vs giving time
+		// for transactions to accumulate.
+		blockTimer.Reset(getRemainingSleep(start, m.conf.BlockTime, 0))
 	}
 }
 
@@ -687,11 +733,18 @@ func (m *Manager) fetchBlock(ctx context.Context, daHeight uint64) (da.ResultRet
 	return blockRes, err
 }
 
-func (m *Manager) getRemainingSleep(start time.Time) time.Duration {
-	publishingDuration := time.Since(start)
-	sleepDuration := m.conf.BlockTime - publishingDuration
-	if sleepDuration < 0 {
-		sleepDuration = 0
+// getRemainingSleep calculates the remaining sleep time based on an interval
+// and a start time.
+func getRemainingSleep(start time.Time, interval, defaultSleep time.Duration) time.Duration {
+	// Initialize the sleep duration to the default sleep duration to cover
+	// the case where more time has past than the interval duration.
+	sleepDuration := defaultSleep
+	// Calculate the time elapsed since the start time.
+	elapse := time.Since(start)
+	// If less time has elapsed than the interval duration, calculate the
+	// remaining time to sleep.
+	if elapse < interval {
+		sleepDuration = interval - elapse
 	}
 	return sleepDuration
 }

block/manager_test.go

@@ -479,3 +479,43 @@ func Test_publishBlock_ManagerNotProposer(t *testing.T) {
 	err := m.publishBlock(context.Background())
 	require.ErrorIs(err, ErrNotProposer)
 }
+
+func TestGetRemainingSleep(t *testing.T) {
+	now := time.Now()
+	interval := 10 * time.Second
+	defaultSleep := time.Second
+
+	// start height over the interval in the past
+	farPastStart := now.Add(-interval - 1)
+
+	// Table test in case we need to easily extend this in the future.
+	var tests = []struct {
+		name          string
+		start         time.Time
+		interval      time.Duration
+		defaultSleep  time.Duration
+		expectedSleep time.Duration
+	}{
+		{"nil case", time.Time{}, 0, 0, 0},
+		{"start over the interval in the past", farPastStart, interval, defaultSleep, defaultSleep},
+	}
+
+	for _, test := range tests {
+		assert.Equalf(t, test.expectedSleep, getRemainingSleep(test.start, test.interval, test.defaultSleep), "test case: %s", test.name)
+
+	}
+
+	// Custom handler for start is within the interval in the past since
+	// getRemainingSleep uses time.Since which results in a
+	// non-deterministic result. But we know that it should be less than
+	// timeInThePast and greater than defaultSleep based on the test setup.
+	timeInThePast := interval / 2
+
+	// start height within the interval in the past
+	pastStart := now.Add(-timeInThePast)
+
+	sleep := getRemainingSleep(pastStart, interval, defaultSleep)
+	assert.True(t, sleep <= timeInThePast)
+	assert.True(t, sleep > defaultSleep)
+
+}

config/config.go

@@ -79,6 +79,9 @@ type BlockManagerConfig struct {
 	// MaxPendingBlocks defines limit of blocks pending DA submission. 0 means no limit.
 	// When limit is reached, aggregator pauses block production.
 	MaxPendingBlocks uint64 `mapstructure:"max_pending_blocks"`
+	// LazyBlockTime defines how often new blocks are produced in lazy mode
+	// even if there are no transactions
+	LazyBlockTime time.Duration `mapstructure:"lazy_block_time"`
 }
 
 // GetNodeConfig translates Tendermint's configuration into Rollkit configuration.

config/defaults.go

@@ -23,8 +23,9 @@ var DefaultNodeConfig = NodeConfig{
 	Aggregator:     false,
 	LazyAggregator: false,
 	BlockManagerConfig: BlockManagerConfig{
-		BlockTime:   1 * time.Second,
-		DABlockTime: 15 * time.Second,
+		BlockTime:     1 * time.Second,
+		DABlockTime:   15 * time.Second,
+		LazyBlockTime: 60 * time.Second,
 	},
 	DAAddress:       "http://localhost:26658",
 	DAGasPrice:      -1,

node/full_node_integration_test.go

@@ -181,7 +181,8 @@ func TestLazyAggregator(t *testing.T) {
 		// the blocktime too short. in future, we can add a configuration
 		// in go-header syncer initialization to not rely on blocktime, but the
 		// config variable
-		BlockTime: 1 * time.Second,
+		BlockTime:     1 * time.Second,
+		LazyBlockTime: 5 * time.Second,
 	}
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -212,6 +213,9 @@ func TestLazyAggregator(t *testing.T) {
 	assert.NoError(err)
 
 	require.NoError(waitForAtLeastNBlocks(node, 4, Header))
+
+	// LazyBlockTime should trigger another block even without transactions
+	require.NoError(waitForAtLeastNBlocks(node, 5, Header))
 }
 
 // TestFastDASync verifies that nodes can sync DA blocks faster than the DA block time