address comments

Author: colinlyguo

rollup/abi/validium_abi.go

@@ -107,8 +107,7 @@ func action(ctx *cli.Context) error {
 	}
 
 	chunkProposer := watcher.NewChunkProposer(subCtx, cfg.L2Config.ChunkProposerConfig, minCodecVersion, genesis.Config, db, registry)
-	batchProposer := watcher.NewBatchProposer(subCtx, cfg.L2Config.BatchProposerConfig, minCodecVersion, genesis.Config, db, registry)
-	batchProposer.SetValidiumMode(cfg.L2Config.RelayerConfig.ValidiumMode)
+	batchProposer := watcher.NewBatchProposer(subCtx, cfg.L2Config.BatchProposerConfig, minCodecVersion, genesis.Config, db, cfg.L2Config.RelayerConfig.ValidiumMode, registry)
 	bundleProposer := watcher.NewBundleProposer(subCtx, cfg.L2Config.BundleProposerConfig, minCodecVersion, genesis.Config, db, registry)
 
 	l2watcher := watcher.NewL2WatcherClient(subCtx, l2client, cfg.L2Config.Confirmations, cfg.L2Config.L2MessageQueueAddress, cfg.L2Config.WithdrawTrieRootSlot, genesis.Config, db, registry)

rollup/cmd/rollup_relayer/app/app.go

@@ -1005,6 +1005,7 @@ func (r *Layer2Relayer) constructCommitBatchPayloadValidium(batchesToSubmit []*d
 
 	// Get the commitment from the batch data: for validium mode, we use the last L2 block hash as the commitment to the off-chain data
 	// Get the last chunk from the last batch to find the end block hash
+	// TODO: This is a temporary solution, we might use a larger commitment in the future
 	if len(batchesToSubmit) == 0 {
 		return nil, 0, 0, fmt.Errorf("no batches to submit")
 	}

rollup/internal/controller/relayer/l2_relayer.go

@@ -54,7 +54,7 @@ type BatchProposer struct {
 }
 
 // NewBatchProposer creates a new BatchProposer instance.
-func NewBatchProposer(ctx context.Context, cfg *config.BatchProposerConfig, minCodecVersion encoding.CodecVersion, chainCfg *params.ChainConfig, db *gorm.DB, reg prometheus.Registerer) *BatchProposer {
+func NewBatchProposer(ctx context.Context, cfg *config.BatchProposerConfig, minCodecVersion encoding.CodecVersion, chainCfg *params.ChainConfig, db *gorm.DB, validiumMode bool, reg prometheus.Registerer) *BatchProposer {
 	log.Info("new batch proposer", "batchTimeoutSec", cfg.BatchTimeoutSec, "maxBlobSize", maxBlobSize, "maxUncompressedBatchBytesSize", cfg.MaxUncompressedBatchBytesSize)
 
 	p := &BatchProposer{
@@ -65,7 +65,7 @@ func NewBatchProposer(ctx context.Context, cfg *config.BatchProposerConfig, minC
 		l2BlockOrm:      orm.NewL2Block(db),
 		cfg:             cfg,
 		replayMode:      false, // default is false, set to true when using proposer tool
-		validiumMode:    false, // default is false, set to true when using validium mode
+		validiumMode:    validiumMode,
 		minCodecVersion: minCodecVersion,
 		chainCfg:        chainCfg,
 
@@ -130,11 +130,6 @@ func (p *BatchProposer) SetReplayDB(replayDB *gorm.DB) {
 	p.replayMode = true
 }
 
-// SetValidiumMode sets the validium mode for the BatchProposer.
-func (p *BatchProposer) SetValidiumMode(validiumMode bool) {
-	p.validiumMode = validiumMode
-}
-
 // TryProposeBatch tries to propose a new batches.
 func (p *BatchProposer) TryProposeBatch() {
 	p.batchProposerCircleTotal.Inc()

rollup/internal/controller/watcher/batch_proposer.go

@@ -100,7 +100,7 @@ func testBatchProposerLimitsCodecV7(t *testing.T) {
 				DarwinV2Time:   new(uint64),
 				EuclidTime:     new(uint64),
 				EuclidV2Time:   new(uint64),
-			}, db, nil)
+			}, db, false /* rollup mode */, nil)
 			bp.TryProposeBatch()
 
 			batches, err := batchOrm.GetBatches(context.Background(), map[string]interface{}{}, []string{}, 0)
@@ -178,7 +178,7 @@ func testBatchProposerBlobSizeLimitCodecV7(t *testing.T) {
 		MaxChunksPerBatch:             math.MaxInt32,
 		BatchTimeoutSec:               math.MaxUint32,
 		MaxUncompressedBatchBytesSize: math.MaxUint64,
-	}, encoding.CodecV7, chainConfig, db, nil)
+	}, encoding.CodecV7, chainConfig, db, false /* rollup mode */, nil)
 
 	for i := 0; i < 2; i++ {
 		bp.TryProposeBatch()
@@ -246,7 +246,7 @@ func testBatchProposerMaxChunkNumPerBatchLimitCodecV7(t *testing.T) {
 		MaxChunksPerBatch:             45,
 		BatchTimeoutSec:               math.MaxUint32,
 		MaxUncompressedBatchBytesSize: math.MaxUint64,
-	}, encoding.CodecV7, chainConfig, db, nil)
+	}, encoding.CodecV7, chainConfig, db, false /* rollup mode */, nil)
 	bp.TryProposeBatch()
 
 	batches, err := batchOrm.GetBatches(context.Background(), map[string]interface{}{}, []string{}, 0)
@@ -335,7 +335,7 @@ func testBatchProposerUncompressedBatchBytesLimitCodecV8(t *testing.T) {
 		MaxChunksPerBatch:             math.MaxInt32,  // No chunk count limit
 		BatchTimeoutSec:               math.MaxUint32, // No timeout limit
 		MaxUncompressedBatchBytesSize: 4 * 1024,       // 4KiB limit
-	}, encoding.CodecV8, chainConfig, db, nil)
+	}, encoding.CodecV8, chainConfig, db, false /* rollup mode */, nil)
 
 	bp.TryProposeBatch()
 

rollup/internal/controller/watcher/batch_proposer_test.go

@@ -103,7 +103,7 @@ func testBundleProposerLimitsCodecV7(t *testing.T) {
 				MaxChunksPerBatch:             math.MaxInt32,
 				BatchTimeoutSec:               0,
 				MaxUncompressedBatchBytesSize: math.MaxUint64,
-			}, encoding.CodecV7, chainConfig, db, nil)
+			}, encoding.CodecV7, chainConfig, db, false /* rollup mode */, nil)
 
 			cp.TryProposeChunk()  // chunk1 contains block1
 			bap.TryProposeBatch() // batch1 contains chunk1

rollup/internal/controller/watcher/bundle_proposer_test.go

@@ -125,7 +125,7 @@ func NewProposerTool(ctx context.Context, cancel context.CancelFunc, cfg *config
 
 	chunkProposer := NewChunkProposer(ctx, cfg.L2Config.ChunkProposerConfig, minCodecVersion, chainCfg, db, nil)
 	chunkProposer.SetReplayDB(dbForReplay)
-	batchProposer := NewBatchProposer(ctx, cfg.L2Config.BatchProposerConfig, minCodecVersion, chainCfg, db, nil)
+	batchProposer := NewBatchProposer(ctx, cfg.L2Config.BatchProposerConfig, minCodecVersion, chainCfg, db, false /* rollup mode */, nil)
 	batchProposer.SetReplayDB(dbForReplay)
 	bundleProposer := NewBundleProposer(ctx, cfg.L2Config.BundleProposerConfig, minCodecVersion, chainCfg, db, nil)
 

rollup/internal/controller/watcher/proposer_tool.go

@@ -125,19 +125,28 @@ type BatchMetadata struct {
 }
 
 // encodeBatchHeaderValidium encodes batch header for validium mode and returns both encoded bytes and hash
-func encodeBatchHeaderValidium(b *encoding.Batch, codecVersion encoding.CodecVersion) ([]byte, common.Hash) {
+func encodeBatchHeaderValidium(b *encoding.Batch, codecVersion encoding.CodecVersion) ([]byte, common.Hash, error) {
 	if b == nil {
-		return nil, common.Hash{}
+		return nil, common.Hash{}, fmt.Errorf("batch is nil, version: %v, index: %v", codecVersion, b.Index)
 	}
 
+	if len(b.Blocks) == 0 {
+		return nil, common.Hash{}, fmt.Errorf("batch contains no blocks, version: %v, index: %v", codecVersion, b.Index)
+	}
+
+	// For validium mode, use the last block hash as commitment to the off-chain data
+	// TODO: This is a temporary solution, we might use a larger commitment in the future
+	lastBlock := b.Blocks[len(b.Blocks)-1]
+	commitment := lastBlock.Header.Hash()
+
 	// Batch header field sizes
 	const (
 		versionSize      = 1
 		indexSize        = 8
 		parentHashSize   = 32
 		stateRootSize    = 32
 		withdrawRootSize = 32
-		commitmentSize   = 32
+		commitmentSize   = 32 // TODO: 32 bytes for now, might use larger commitment in the future
 
 		// Total size of validium batch header
 		validiumBatchHeaderSize = versionSize + indexSize + parentHashSize + stateRootSize + withdrawRootSize + commitmentSize
@@ -160,17 +169,10 @@ func encodeBatchHeaderValidium(b *encoding.Batch, codecVersion encoding.CodecVer
 	copy(batchBytes[parentHashOffset:parentHashOffset+parentHashSize], b.ParentBatchHash[0:parentHashSize])                // parentBatchHash
 	copy(batchBytes[stateRootOffset:stateRootOffset+stateRootSize], b.StateRoot().Bytes()[0:stateRootSize])                // postStateRoot
 	copy(batchBytes[withdrawRootOffset:withdrawRootOffset+withdrawRootSize], b.WithdrawRoot().Bytes()[0:withdrawRootSize]) // postWithdrawRoot
-
-	// For validium mode, use the last block hash as commitment to the off-chain data
-	var commitment common.Hash
-	if len(b.Blocks) > 0 {
-		lastBlock := b.Blocks[len(b.Blocks)-1]
-		commitment = lastBlock.Header.Hash()
-	}
-	copy(batchBytes[commitmentOffset:commitmentOffset+commitmentSize], commitment[0:commitmentSize]) // data commitment
+	copy(batchBytes[commitmentOffset:commitmentOffset+commitmentSize], commitment[0:commitmentSize])                       // data commitment
 
 	hash := crypto.Keccak256Hash(batchBytes)
-	return batchBytes, hash
+	return batchBytes, hash, nil
 }
 
 // GetBatchMetadata retrieves the metadata of a batch.
@@ -195,12 +197,15 @@ func GetBatchMetadata(batch *encoding.Batch, codecVersion encoding.CodecVersion,
 
 	// If this function is used in Validium, we encode the batch header differently.
 	if validiumMode {
-		batchMeta.BatchBytes, batchMeta.BatchHash = encodeBatchHeaderValidium(batch, codecVersion)
+		batchMeta.BatchBytes, batchMeta.BatchHash, err = encodeBatchHeaderValidium(batch, codecVersion)
+		if err != nil {
+			return nil, fmt.Errorf("failed to encode batch header for validium, version: %v, index: %v, err: %w", codecVersion, batch.Index, err)
+		}
 	}
 
 	batchMeta.BatchBlobDataProof, err = daBatch.BlobDataProofForPointEvaluation()
 	if err != nil {
-		return nil, fmt.Errorf("failed to get blob data proof, version: %v, err: %w", codecVersion, err)
+		return nil, fmt.Errorf("failed to get blob data proof, version: %v, index: %v, err: %w", codecVersion, batch.Index, err)
 	}
 
 	numChunks := len(batch.Chunks)

rollup/internal/utils/utils.go

@@ -128,7 +128,7 @@ func testCommitBatchAndFinalizeBundleCodecV7(t *testing.T) {
 		MaxChunksPerBatch:             math.MaxInt32,
 		BatchTimeoutSec:               300,
 		MaxUncompressedBatchBytesSize: math.MaxUint64,
-	}, encoding.CodecV7, chainConfig, db, nil)
+	}, encoding.CodecV7, chainConfig, db, false /* rollup mode */, nil)
 
 	bup := watcher.NewBundleProposer(context.Background(), &config.BundleProposerConfig{
 		MaxBatchNumPerBundle: 2,