Add daIncluderTests

Manav-Aggarwal · Apr 29, 2025 · Apr 30, 2025 · Apr 30, 2025 · Apr 30, 2025 · May 2, 2025
commit a16e63b053ce93e1d80ae423840cddb3b1dc6a0d
diff --git a/block/daIncluder.go b/block/daIncluder.go
@@ -15,12 +15,12 @@ func (m *Manager) DAIncluderLoop(ctx context.Context) {
 		case <-m.daIncluderCh:
 			// proceed to check for DA inclusion
 		}
-
 		currentDAIncluded := m.GetDAIncludedHeight()
 		for {
 			nextHeight := currentDAIncluded + 1
 			header, data, err := m.store.GetBlockData(ctx, nextHeight)
 			if err != nil {
+				m.logger.Debug("no more blocks to check at this time", "height", nextHeight, "error", err)
 				// No more blocks to check at this time
 				break
 			}
@@ -31,7 +31,6 @@ func (m *Manager) DAIncluderLoop(ctx context.Context) {
 			if m.headerCache.IsDAIncluded(headerHash) && m.dataCache.IsDAIncluded(dataHash) {
 				// Both header and data are DA-included, so we can advance the height
 				if err := m.incrementDAIncludedHeight(ctx); err != nil {
-					m.logger.Error("failed to set DA included height", "height", nextHeight, "error", err)
 					break
 				}
 				currentDAIncluded = nextHeight
@@ -45,15 +44,19 @@ func (m *Manager) DAIncluderLoop(ctx context.Context) {
 
 // incrementDAIncludedHeight sets the DA included height in the store
 func (m *Manager) incrementDAIncludedHeight(ctx context.Context) error {
-	currentHeight := m.daIncludedHeight.Load()
+	currentHeight := m.GetDAIncludedHeight()
 	newHeight := currentHeight + 1
-	m.logger.Debug("setting final", "height", newHeight)
-	m.exec.SetFinal(ctx, newHeight)
 	if m.daIncludedHeight.CompareAndSwap(currentHeight, newHeight) {
-		m.logger.Debug("setting DA included height", "height", newHeight)
 		heightBytes := make([]byte, 8)
 		binary.LittleEndian.PutUint64(heightBytes, newHeight)
-		return m.store.SetMetadata(ctx, DAIncludedHeightKey, heightBytes)
+		m.logger.Debug("setting DA included height", "height", newHeight)
+		err := m.store.SetMetadata(ctx, DAIncludedHeightKey, heightBytes)
+		if err != nil {
+			m.logger.Error("failed to set DA included height", "height", newHeight, "error", err)
+			return err
+		}
+		m.logger.Debug("setting final", "height", newHeight)
+		m.exec.SetFinal(ctx, newHeight)
 	}
 	return nil
 }
diff --git a/block/daIncluder_test.go b/block/daIncluder_test.go
@@ -0,0 +1,267 @@
+package block
+
+import (
+	"context"
+	"encoding/binary"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+
+	"github.com/rollkit/rollkit/pkg/cache"
+	"github.com/rollkit/rollkit/test/mocks"
+	"github.com/rollkit/rollkit/types"
+)
+
+// newTestManager creates a Manager with mocked Store and Executor for testing DAIncluder logic.
+func newTestManager(t *testing.T) (*Manager, *mocks.Store, *mocks.Executor, *MockLogger) {
+	store := mocks.NewStore(t)
+	exec := mocks.NewExecutor(t)
+	logger := new(MockLogger)
+	logger.On("Debug", mock.Anything, mock.Anything).Maybe()
+	logger.On("Info", mock.Anything, mock.Anything).Maybe()
+	logger.On("Warn", mock.Anything, mock.Anything).Maybe()
+	logger.On("Error", mock.Anything, mock.Anything).Maybe()
+	m := &Manager{
+		store:        store,
+		headerCache:  cache.NewCache[types.SignedHeader](),
+		dataCache:    cache.NewCache[types.Data](),
+		daIncluderCh: make(chan struct{}, 1),
+		logger:       logger,
+		exec:         exec,
+		lastStateMtx: &sync.RWMutex{},
+		metrics:      NopMetrics(),
+	}
+	return m, store, exec, logger
+}
+
+// TestDAIncluderLoop_AdvancesHeightWhenBothDAIncluded verifies that the DAIncluderLoop advances the DA included height
+// when both the header and data for the next block are marked as DA-included in the caches.
+func TestDAIncluderLoop_AdvancesHeightWhenBothDAIncluded(t *testing.T) {
+	t.Parallel()
+	m, store, exec, _ := newTestManager(t)
+	startDAIncludedHeight := uint64(4)
+	expectedDAIncludedHeight := uint64(startDAIncludedHeight + 1)
+	m.daIncludedHeight.Store(startDAIncludedHeight)
+
+	header, data := types.GetRandomBlock(5, 1, "testchain")
+	headerHash := header.Hash().String()
+	dataHash := data.DACommitment().String()
+	m.headerCache.SetDAIncluded(headerHash)
+	m.dataCache.SetDAIncluded(dataHash)
+
+	store.On("GetBlockData", mock.Anything, uint64(5)).Return(header, data, nil).Once()
+	store.On("GetBlockData", mock.Anything, uint64(6)).Return(nil, nil, assert.AnError).Once()
+	heightBytes := make([]byte, 8)
+	binary.LittleEndian.PutUint64(heightBytes, expectedDAIncludedHeight)
+	store.On("SetMetadata", mock.Anything, DAIncludedHeightKey, heightBytes).Return(nil).Once()
+	exec.On("SetFinal", mock.Anything, uint64(5)).Return(nil).Once()
+
+	ctx, loopCancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
+	defer loopCancel()
+
+	var wg sync.WaitGroup
+	wg.Add(1)
+
+	go func() {
+		defer wg.Done()
+		m.DAIncluderLoop(ctx)
+	}()
+
+	m.daIncluderCh <- struct{}{}
+
+	wg.Wait()
+
+	assert.Equal(t, expectedDAIncludedHeight, m.daIncludedHeight.Load())
+	store.AssertExpectations(t)
+	exec.AssertExpectations(t)
+}
+
+// TestDAIncluderLoop_StopsWhenHeaderNotDAIncluded verifies that the DAIncluderLoop does not advance the height
+// if the header for the next block is not marked as DA-included in the cache.
+func TestDAIncluderLoop_StopsWhenHeaderNotDAIncluded(t *testing.T) {
+	t.Parallel()
+	m, store, _, _ := newTestManager(t)
+	startDAIncludedHeight := uint64(4)
+	m.daIncludedHeight.Store(startDAIncludedHeight)
+
+	header, data := types.GetRandomBlock(5, 1, "testchain")
+	// m.headerCache.SetDAIncluded(headerHash) // Not set
+	m.dataCache.SetDAIncluded(data.DACommitment().String())
+
+	store.On("GetBlockData", mock.Anything, uint64(5)).Return(header, data, nil).Once()
+
+	ctx, loopCancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
+	defer loopCancel()
+
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		m.DAIncluderLoop(ctx)
+	}()
+
+	m.daIncluderCh <- struct{}{}
+	wg.Wait()
+
+	assert.Equal(t, startDAIncludedHeight, m.GetDAIncludedHeight())
+	store.AssertExpectations(t)
+}
+
+// TestDAIncluderLoop_StopsWhenDataNotDAIncluded verifies that the DAIncluderLoop does not advance the height
+// if the data for the next block is not marked as DA-included in the cache.
+func TestDAIncluderLoop_StopsWhenDataNotDAIncluded(t *testing.T) {
+	t.Parallel()
+	m, store, _, _ := newTestManager(t)
+	startDAIncludedHeight := uint64(4)
+	m.daIncludedHeight.Store(startDAIncludedHeight)
+
+	header, data := types.GetRandomBlock(5, 1, "testchain")
+	headerHash := header.Hash().String()
+	m.headerCache.SetDAIncluded(headerHash)
+	// m.dataCache.SetDAIncluded(data.DACommitment().String()) // Not set
+
+	store.On("GetBlockData", mock.Anything, uint64(5)).Return(header, data, nil).Once()
+
+	ctx, loopCancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
+	defer loopCancel()
+
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		m.DAIncluderLoop(ctx)
+	}()
+
+	m.daIncluderCh <- struct{}{}
+	wg.Wait()
+
+	assert.Equal(t, startDAIncludedHeight, m.GetDAIncludedHeight())
+	store.AssertExpectations(t)
+}
+
+// TestDAIncluderLoop_StopsOnGetBlockDataError verifies that the DAIncluderLoop stops advancing
+// if GetBlockData returns an error for the next block height.
+func TestDAIncluderLoop_StopsOnGetBlockDataError(t *testing.T) {
+	t.Parallel()
+	m, store, _, mockLogger := newTestManager(t)
+	startDAIncludedHeight := uint64(4)
+	m.daIncludedHeight.Store(startDAIncludedHeight)
+
+	store.On("GetBlockData", mock.Anything, uint64(5)).Return(nil, nil, assert.AnError).Once()
+
+	// Expect the debug log for no more blocks to check
+	mockLogger.ExpectedCalls = nil // Clear any previous expectations
+	mockLogger.On("Debug", "no more blocks to check at this time", mock.Anything).Once()
+
+	ctx, loopCancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
+	defer loopCancel()
+
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		m.DAIncluderLoop(ctx)
+	}()
+
+	m.daIncluderCh <- struct{}{}
+	wg.Wait()
+
+	assert.Equal(t, startDAIncludedHeight, m.GetDAIncludedHeight())
+	store.AssertExpectations(t)
+	mockLogger.AssertExpectations(t)
+}
+
+// TestIncrementDAIncludedHeight_Success verifies that incrementDAIncludedHeight increments the height
+// and calls SetMetadata and SetFinal when CompareAndSwap succeeds.
+func TestIncrementDAIncludedHeight_Success(t *testing.T) {
+	t.Parallel()
+	m, store, exec, _ := newTestManager(t)
+	startDAIncludedHeight := uint64(4)
+	expectedDAIncludedHeight := uint64(startDAIncludedHeight + 1)
+	m.daIncludedHeight.Store(startDAIncludedHeight)
+
+	heightBytes := make([]byte, 8)
+	binary.LittleEndian.PutUint64(heightBytes, expectedDAIncludedHeight)
+	store.On("SetMetadata", mock.Anything, DAIncludedHeightKey, heightBytes).Return(nil).Once()
+	exec.On("SetFinal", mock.Anything, expectedDAIncludedHeight).Return(nil).Once()
+
+	err := m.incrementDAIncludedHeight(context.Background())
+	assert.NoError(t, err)
+	assert.Equal(t, expectedDAIncludedHeight, m.GetDAIncludedHeight())
+	store.AssertExpectations(t)
+	exec.AssertExpectations(t)
+}
+
+// TestIncrementDAIncludedHeight_SetMetadataError verifies that incrementDAIncludedHeight returns an error
+// if SetMetadata fails after CompareAndSwap succeeds.
+func TestIncrementDAIncludedHeight_SetMetadataError(t *testing.T) {
+	t.Parallel()
+	m, store, _, mockLogger := newTestManager(t)
+	startDAIncludedHeight := uint64(4)
+	expectedDAIncludedHeight := uint64(startDAIncludedHeight + 1)
+	m.daIncludedHeight.Store(startDAIncludedHeight)
+
+	heightBytes := make([]byte, 8)
+	binary.LittleEndian.PutUint64(heightBytes, expectedDAIncludedHeight)
+	store.On("SetMetadata", mock.Anything, DAIncludedHeightKey, heightBytes).Return(assert.AnError).Once()
+
+	// Expect the error log for failed to set DA included height
+	mockLogger.ExpectedCalls = nil // Clear any previous expectations
+	mockLogger.On("Error", "failed to set DA included height", []interface{}{"height", expectedDAIncludedHeight, "error", assert.AnError}).Once()
+	mockLogger.On("Debug", mock.Anything, mock.Anything).Maybe() // Allow other debug logs
+
+	err := m.incrementDAIncludedHeight(context.Background())
+	assert.Error(t, err)
+	store.AssertExpectations(t)
+	mockLogger.AssertExpectations(t)
+}
+
+// TestDAIncluderLoop_MultipleConsecutiveHeightsDAIncluded verifies that DAIncluderLoop advances the height
+// multiple times in a single run when several consecutive blocks are DA-included.
+func TestDAIncluderLoop_MultipleConsecutiveHeightsDAIncluded(t *testing.T) {
+	t.Parallel()
+	m, store, exec, _ := newTestManager(t)
+	startDAIncludedHeight := uint64(4)
+	numConsecutive := 10
+	numTxs := 5
+	m.daIncludedHeight.Store(startDAIncludedHeight)
+
+	headers := make([]*types.SignedHeader, numConsecutive)
+	dataBlocks := make([]*types.Data, numConsecutive)
+
+	for i := 0; i < numConsecutive; i++ {
+		height := startDAIncludedHeight + uint64(i+1)
+		headers[i], dataBlocks[i] = types.GetRandomBlock(height, numTxs, "testchain")
+		headerHash := headers[i].Hash().String()
+		dataHash := dataBlocks[i].DACommitment().String()
+		m.headerCache.SetDAIncluded(headerHash)
+		m.dataCache.SetDAIncluded(dataHash)
+		store.On("GetBlockData", mock.Anything, height).Return(headers[i], dataBlocks[i], nil).Once()
+	}
+	// Next height returns error
+	store.On("GetBlockData", mock.Anything, startDAIncludedHeight+uint64(numConsecutive+1)).Return(nil, nil, assert.AnError).Once()
+	store.On("SetMetadata", mock.Anything, DAIncludedHeightKey, mock.Anything).Return(nil).Times(numConsecutive)
+	exec.On("SetFinal", mock.Anything, mock.Anything).Return(nil).Times(numConsecutive)
+
+	expectedDAIncludedHeight := startDAIncludedHeight + uint64(numConsecutive)
+
+	ctx, loopCancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
+	defer loopCancel()
+
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		m.DAIncluderLoop(ctx)
+	}()
+
+	m.daIncluderCh <- struct{}{}
+	wg.Wait()
+
+	assert.Equal(t, expectedDAIncludedHeight, m.GetDAIncludedHeight())
+	store.AssertExpectations(t)
+	exec.AssertExpectations(t)
+}