gupadhyaya · Oct 18, 2024 · Oct 4, 2024 · Oct 4, 2024 · Oct 4, 2024 · Oct 4, 2024
diff --git a/block/cache_test.go b/block/cache_test.go
@@ -9,6 +9,7 @@ import (
 )
 
 func TestBlockCache(t *testing.T) {
+	chainId := "TestBlockCache"
 	require := require.New(t)
 	// Create new HeaderCache and DataCache and verify not nil
 	hc := NewHeaderCache()
@@ -19,7 +20,7 @@ func TestBlockCache(t *testing.T) {
 
 	// Test setBlock and getBlock
 	height, nTxs := uint64(1), 2
-	header, data := types.GetRandomBlock(height, nTxs)
+	header, data := types.GetRandomBlock(height, nTxs, chainId)
 	hc.setHeader(height, header)
 	gotHeader := hc.getHeader(height)
 	require.NotNil(gotHeader, "getHeader should return non-nil after setHeader")
@@ -30,7 +31,7 @@ func TestBlockCache(t *testing.T) {
 	require.Equal(data, gotData)
 
 	// Test overwriting a block
-	header1, data1 := types.GetRandomBlock(height, nTxs)
+	header1, data1 := types.GetRandomBlock(height, nTxs, chainId)
 	hc.setHeader(height, header1)
 	gotHeader1 := hc.getHeader(height)
 	require.NotNil(gotHeader1, "getHeader should return non-nil after overwriting a header")

diff --git a/block/manager.go b/block/manager.go
@@ -3,7 +3,6 @@ package block
 import (
 	"bytes"
 	"context"
-	"crypto/sha256"
 	"encoding/binary"
 	"encoding/hex"
 	"errors"
@@ -81,6 +80,9 @@ const DAIncludedHeightKey = "da included height"
 // dataHashForEmptyTxs to be used while only syncing headers from DA and no p2p to get the Data for no txs scenarios, the syncing can proceed without getting stuck forever.
 var dataHashForEmptyTxs = []byte{110, 52, 11, 156, 255, 179, 122, 152, 156, 165, 68, 230, 187, 120, 10, 44, 120, 144, 29, 63, 179, 55, 56, 118, 133, 17, 163, 6, 23, 175, 160, 29}
 
+// ErrNoBatch indicate no batch is available for creating block
+var ErrNoBatch = errors.New("no batch to process")
+
 // NewHeaderEvent is used to pass header and DA height to headerInCh
 type NewHeaderEvent struct {
 	Header   *types.SignedHeader
@@ -415,27 +417,35 @@ func (m *Manager) BatchRetrieveLoop(ctx context.Context) {
 		case <-batchTimer.C:
 			// Define the start time for the block production period
 			start := time.Now()
-			batch, batchTime, err := m.seqClient.GetNextBatch(ctx, m.lastBatchHash)
+			res, err := m.seqClient.GetNextBatch(ctx, sequencing.GetNextBatchRequest{RollupId: []byte(m.genesis.ChainID), LastBatchHash: m.lastBatchHash})
 			if err != nil && ctx.Err() == nil {
 				m.logger.Error("error while retrieving batch", "error", err)
 			}
-			// Add the batch to the batch queue
-			if batch != nil && batch.Transactions != nil {
-				m.bq.AddBatch(BatchWithTime{batch, batchTime})
-				// Calculate the hash of the batch and store it for the next batch retrieval
-				batchBytes, err := batch.Marshal()
-				if err != nil {
-					m.logger.Error("error while marshaling batch", "error", err)
+			if res != nil {
+				batch := res.Batch
+				batchTime := res.Timestamp
+				// Add the batch to the batch queue
+				if batch != nil {
+					// Calculate the hash of the batch and store it for the next batch retrieval
+					h, err := batch.Hash()
+					if err == nil {
+						// add batch to the queue even if its empty (no txs)
+						m.bq.AddBatch(BatchWithTime{batch, batchTime})
+						// update lastBatchHash only if the batch has actual txs
+						if batch.Transactions != nil {
+							m.lastBatchHash = h
+						}
+					} else {
+						m.logger.Error("error while hashing batch", "error", err)
+					}
 				}
-				h := sha256.Sum256(batchBytes)
-				m.lastBatchHash = h[:]
 			}
 			// Reset the batchTimer to signal the next batch production
 			// period based on the batch retrieval time.
 			remainingSleep := time.Duration(0)
 			elapsed := time.Since(start)
-			if elapsed < defaultBatchRetrievalInterval {
-				remainingSleep = defaultBatchRetrievalInterval - elapsed
+			if elapsed < m.conf.BlockTime {
+				remainingSleep = m.conf.BlockTime - elapsed
 			}
 			batchTimer.Reset(remainingSleep)
 		}
@@ -984,16 +994,17 @@ func (m *Manager) getSignature(header types.Header) (*types.Signature, error) {
 	return &signature, nil
 }
 
-func (m *Manager) getTxsFromBatch() cmtypes.Txs {
+func (m *Manager) getTxsFromBatch() (cmtypes.Txs, *time.Time, error) {
 	batch := m.bq.Next()
 	if batch == nil {
-		return make(cmtypes.Txs, 0)
+		// batch is nil when there is nothing to process
+		return nil, nil, ErrNoBatch
 	}
 	txs := make(cmtypes.Txs, 0, len(batch.Transactions))
 	for _, tx := range batch.Transactions {
 		txs = append(txs, tx)
 	}
-	return txs
+	return txs, &batch.Time, nil
 }
 
 func (m *Manager) publishBlock(ctx context.Context) error {
@@ -1056,7 +1067,11 @@ func (m *Manager) publishBlock(ctx context.Context) error {
 			return fmt.Errorf("failed to load extended commit for height %d: %w", height, err)
 		}
 
-		header, data, err = m.createBlock(newHeight, lastSignature, lastHeaderHash, extendedCommit, m.getTxsFromBatch())
+		txs, timestamp, err := m.getTxsFromBatch()
+		if err != nil {
+			return fmt.Errorf("failed to get transactions from batch: %w", err)
+		}
+		header, data, err = m.createBlock(newHeight, lastSignature, lastHeaderHash, extendedCommit, txs, *timestamp)
 		if err != nil {
 			return err
 		}
@@ -1397,10 +1412,10 @@ func (m *Manager) getLastBlockTime() time.Time {
 	return m.lastState.LastBlockTime
 }
 
-func (m *Manager) createBlock(height uint64, lastSignature *types.Signature, lastHeaderHash types.Hash, extendedCommit abci.ExtendedCommitInfo, txs cmtypes.Txs) (*types.SignedHeader, *types.Data, error) {
+func (m *Manager) createBlock(height uint64, lastSignature *types.Signature, lastHeaderHash types.Hash, extendedCommit abci.ExtendedCommitInfo, txs cmtypes.Txs, timestamp time.Time) (*types.SignedHeader, *types.Data, error) {
 	m.lastStateMtx.RLock()
 	defer m.lastStateMtx.RUnlock()
-	return m.executor.CreateBlock(height, lastSignature, extendedCommit, lastHeaderHash, m.lastState, txs)
+	return m.executor.CreateBlock(height, lastSignature, extendedCommit, lastHeaderHash, m.lastState, txs, timestamp)
 }
 
 func (m *Manager) applyBlock(ctx context.Context, header *types.SignedHeader, data *types.Data) (types.State, *abci.ResponseFinalizeBlock, error) {

diff --git a/block/manager_test.go b/block/manager_test.go
@@ -31,6 +31,7 @@ import (
 	goDA "github.com/rollkit/go-da"
 	goDAMock "github.com/rollkit/go-da/mocks"
 	goDATest "github.com/rollkit/go-da/test"
+	"github.com/rollkit/go-sequencing"
 
 	seqGRPC "github.com/rollkit/go-sequencing/proxy/grpc"
 	"github.com/rollkit/rollkit/config"
@@ -85,9 +86,9 @@ func getManager(t *testing.T, backend goDA.DA) *Manager {
 
 func TestInitialStateClean(t *testing.T) {
 	require := require.New(t)
-	genesisDoc, _ := types.GetGenesisWithPrivkey(types.DefaultSigningKeyType)
+	genesisDoc, _ := types.GetGenesisWithPrivkey(types.DefaultSigningKeyType, "TestInitialStateClean")
 	genesis := &cmtypes.GenesisDoc{
-		ChainID:       "myChain",
+		ChainID:       "TestInitialStateClean",
 		InitialHeight: 1,
 		Validators:    genesisDoc.Validators,
 		AppHash:       []byte("app hash"),
@@ -102,16 +103,16 @@ func TestInitialStateClean(t *testing.T) {
 
 func TestInitialStateStored(t *testing.T) {
 	require := require.New(t)
-	genesisDoc, _ := types.GetGenesisWithPrivkey(types.DefaultSigningKeyType)
+	genesisDoc, _ := types.GetGenesisWithPrivkey(types.DefaultSigningKeyType, "TestInitialStateStored")
 	valset := types.GetRandomValidatorSet()
 	genesis := &cmtypes.GenesisDoc{
-		ChainID:       "myChain",
+		ChainID:       "TestInitialStateStored",
 		InitialHeight: 1,
 		Validators:    genesisDoc.Validators,
 		AppHash:       []byte("app hash"),
 	}
 	sampleState := types.State{
-		ChainID:         "myChain",
+		ChainID:         "TestInitialStateStored",
 		InitialHeight:   1,
 		LastBlockHeight: 100,
 		Validators:      valset,
@@ -179,16 +180,16 @@ func TestHandleEmptyDataHash(t *testing.T) {
 
 func TestInitialStateUnexpectedHigherGenesis(t *testing.T) {
 	require := require.New(t)
-	genesisDoc, _ := types.GetGenesisWithPrivkey(types.DefaultSigningKeyType)
+	genesisDoc, _ := types.GetGenesisWithPrivkey(types.DefaultSigningKeyType, "TestInitialStateUnexpectedHigherGenesis")
 	valset := types.GetRandomValidatorSet()
 	genesis := &cmtypes.GenesisDoc{
-		ChainID:       "myChain",
+		ChainID:       "TestInitialStateUnexpectedHigherGenesis",
 		InitialHeight: 2,
 		Validators:    genesisDoc.Validators,
 		AppHash:       []byte("app hash"),
 	}
 	sampleState := types.State{
-		ChainID:         "myChain",
+		ChainID:         "TestInitialStateUnexpectedHigherGenesis",
 		InitialHeight:   1,
 		LastBlockHeight: 0,
 		Validators:      valset,
@@ -282,7 +283,7 @@ func TestSubmitBlocksToMockDA(t *testing.T) {
 			m.store = store.New(kvStore)
 
 			var blobs [][]byte
-			header, data := types.GetRandomBlock(1, 5)
+			header, data := types.GetRandomBlock(1, 5, "TestSubmitBlocksToMockDA")
 			blob, err := header.MarshalBinary()
 			require.NoError(t, err)
 
@@ -457,15 +458,16 @@ func TestSubmitBlocksToMockDA(t *testing.T) {
 
 // Test_submitBlocksToDA_BlockMarshalErrorCase1: A itself has a marshalling error. So A, B and C never get submitted.
 func Test_submitBlocksToDA_BlockMarshalErrorCase1(t *testing.T) {
+	chainId := "Test_submitBlocksToDA_BlockMarshalErrorCase1"
 	assert := assert.New(t)
 	require := require.New(t)
 	ctx := context.Background()
 
 	m := getManager(t, goDATest.NewDummyDA())
 
-	header1, data1 := types.GetRandomBlock(uint64(1), 5)
-	header2, data2 := types.GetRandomBlock(uint64(2), 5)
-	header3, data3 := types.GetRandomBlock(uint64(3), 5)
+	header1, data1 := types.GetRandomBlock(uint64(1), 5, chainId)
+	header2, data2 := types.GetRandomBlock(uint64(2), 5, chainId)
+	header3, data3 := types.GetRandomBlock(uint64(3), 5, chainId)
 
 	store := mocks.NewStore(t)
 	invalidateBlockHeader(header1)
@@ -491,15 +493,16 @@ func Test_submitBlocksToDA_BlockMarshalErrorCase1(t *testing.T) {
 // Test_submitBlocksToDA_BlockMarshalErrorCase2: A and B are fair blocks, but C has a marshalling error
 // - Block A and B get submitted to DA layer not block C
 func Test_submitBlocksToDA_BlockMarshalErrorCase2(t *testing.T) {
+	chainId := "Test_submitBlocksToDA_BlockMarshalErrorCase2"
 	assert := assert.New(t)
 	require := require.New(t)
 	ctx := context.Background()
 
 	m := getManager(t, goDATest.NewDummyDA())
 
-	header1, data1 := types.GetRandomBlock(uint64(1), 5)
-	header2, data2 := types.GetRandomBlock(uint64(2), 5)
-	header3, data3 := types.GetRandomBlock(uint64(3), 5)
+	header1, data1 := types.GetRandomBlock(uint64(1), 5, chainId)
+	header2, data2 := types.GetRandomBlock(uint64(2), 5, chainId)
+	header3, data3 := types.GetRandomBlock(uint64(3), 5, chainId)
 
 	store := mocks.NewStore(t)
 	invalidateBlockHeader(header3)
@@ -552,7 +555,7 @@ func Test_isProposer(t *testing.T) {
 		{
 			name: "Signing key matches genesis proposer public key",
 			args: func() args {
-				genesisData, privKey := types.GetGenesisWithPrivkey(types.DefaultSigningKeyType)
+				genesisData, privKey := types.GetGenesisWithPrivkey(types.DefaultSigningKeyType, "Test_isProposer")
 				s, err := types.NewFromGenesisDoc(genesisData)
 				require.NoError(err)
 				signingKey, err := types.PrivKeyToSigningKey(privKey)
@@ -568,7 +571,7 @@ func Test_isProposer(t *testing.T) {
 		{
 			name: "Signing key does not match genesis proposer public key",
 			args: func() args {
-				genesisData, _ := types.GetGenesisWithPrivkey(types.DefaultSigningKeyType)
+				genesisData, _ := types.GetGenesisWithPrivkey(types.DefaultSigningKeyType, "Test_isProposer")
 				s, err := types.NewFromGenesisDoc(genesisData)
 				require.NoError(err)
 
@@ -586,7 +589,7 @@ func Test_isProposer(t *testing.T) {
 		{
 			name: "No validators found in genesis",
 			args: func() args {
-				genesisData, privKey := types.GetGenesisWithPrivkey(types.DefaultSigningKeyType)
+				genesisData, privKey := types.GetGenesisWithPrivkey(types.DefaultSigningKeyType, "Test_isProposer")
 				genesisData.Validators = nil
 				s, err := types.NewFromGenesisDoc(genesisData)
 				require.NoError(err)
@@ -684,14 +687,15 @@ func TestManager_publishBlock(t *testing.T) {
 	lastState.NextValidators = cmtypes.NewValidatorSet(validators)
 	lastState.LastValidators = cmtypes.NewValidatorSet(validators)
 
+	chainId := "TestManager_publishBlock"
 	mpool := mempool.NewCListMempool(cfg.DefaultMempoolConfig(), proxy.NewAppConnMempool(client, proxy.NopMetrics()), 0)
 	seqClient := seqGRPC.NewClient()
 	require.NoError(seqClient.Start(
 		MockSequencerAddress,
 		grpc.WithTransportCredentials(insecure.NewCredentials()),
 	))
-	mpoolReaper := mempool.NewCListMempoolReaper(mpool, []byte("test"), seqClient, logger)
-	executor := state.NewBlockExecutor(vKey.PubKey().Address(), "test", mpool, mpoolReaper, proxy.NewAppConnConsensus(client, proxy.NopMetrics()), nil, 100, logger, state.NopMetrics())
+	mpoolReaper := mempool.NewCListMempoolReaper(mpool, []byte(chainId), seqClient, logger)
+	executor := state.NewBlockExecutor(vKey.PubKey().Address(), chainId, mpool, mpoolReaper, proxy.NewAppConnConsensus(client, proxy.NopMetrics()), nil, 100, logger, state.NopMetrics())
 
 	signingKey, err := types.PrivKeyToSigningKey(vKey)
 	require.NoError(err)
@@ -704,7 +708,7 @@ func TestManager_publishBlock(t *testing.T) {
 		store:        mockStore,
 		logger:       mockLogger,
 		genesis: &cmtypes.GenesisDoc{
-			ChainID:       "myChain",
+			ChainID:       chainId,
 			InitialHeight: 1,
 			AppHash:       []byte("app hash"),
 		},
@@ -720,7 +724,7 @@ func TestManager_publishBlock(t *testing.T) {
 	t.Run("height should not be updated if saving block responses fails", func(t *testing.T) {
 		mockStore.On("Height").Return(uint64(0))
 		signature := types.Signature([]byte{1, 1, 1})
-		header, data, err := executor.CreateBlock(0, &signature, abci.ExtendedCommitInfo{}, []byte{}, lastState, cmtypes.Txs{})
+		header, data, err := executor.CreateBlock(0, &signature, abci.ExtendedCommitInfo{}, []byte{}, lastState, cmtypes.Txs{}, time.Now())
 		require.NoError(err)
 		require.NotNil(header)
 		require.NotNil(data)
@@ -921,3 +925,53 @@ func TestNormalAggregationLoop(t *testing.T) {
 	// Wait for the function to complete or timeout
 	<-ctx.Done()
 }
+
+func TestGetTxsFromBatch_NoBatch(t *testing.T) {
+	// Mocking a manager with an empty batch queue
+	m := &Manager{
+		bq: &BatchQueue{queue: nil}, // No batch available
+	}
+
+	// Call the method and assert the results
+	txs, timestamp, err := m.getTxsFromBatch()
+
+	// Assertions
+	assert.Nil(t, txs, "Transactions should be nil when no batch exists")
+	assert.Nil(t, timestamp, "Timestamp should be nil when no batch exists")
+	assert.Equal(t, ErrNoBatch, err, "Expected ErrNoBatch error")
+}
+
+func TestGetTxsFromBatch_EmptyBatch(t *testing.T) {
+	// Mocking a manager with an empty batch
+	m := &Manager{
+		bq: &BatchQueue{queue: []BatchWithTime{
+			{Batch: &sequencing.Batch{Transactions: nil}, Time: time.Now()},
+		}},
+	}
+
+	// Call the method and assert the results
+	txs, timestamp, err := m.getTxsFromBatch()
+
+	// Assertions
+	require.NoError(t, err, "Expected no error for empty batch")
+	assert.Empty(t, txs, "Transactions should be empty when batch has no transactions")
+	assert.NotNil(t, timestamp, "Timestamp should not be nil for empty batch")
+}
+
+func TestGetTxsFromBatch_ValidBatch(t *testing.T) {
+	// Mocking a manager with a valid batch
+	m := &Manager{
+		bq: &BatchQueue{queue: []BatchWithTime{
+			{Batch: &sequencing.Batch{Transactions: [][]byte{[]byte("tx1"), []byte("tx2")}}, Time: time.Now()},
+		}},
+	}
+
+	// Call the method and assert the results
+	txs, timestamp, err := m.getTxsFromBatch()
+
+	// Assertions
+	require.NoError(t, err, "Expected no error for valid batch")
+	assert.Len(t, txs, 2, "Expected 2 transactions")
+	assert.NotNil(t, timestamp, "Timestamp should not be nil for valid batch")
+	assert.Equal(t, cmtypes.Txs{cmtypes.Tx([]byte("tx1")), cmtypes.Tx([]byte("tx2"))}, txs, "Transactions do not match")
+}