feat(relayer): remove 'skipped' proving/rollup status and related code/tests (#642)

.github/workflows/roller.yml

@@ -29,6 +29,25 @@ jobs:
     if: github.event.pull_request.draft == false
     runs-on: ubuntu-latest
     steps:
+    - name: Install Go
+      uses: actions/setup-go@v2
+      with:
+        go-version: 1.19.x
+    - name: Checkout code
+      uses: actions/checkout@v2
+    - name: Test
+      run: |
+        go test -tags="mock_prover" -v -coverprofile=coverage.txt ./...
+    - name: Upload coverage reports to Codecov
+      uses: codecov/codecov-action@v3
+      env:
+        CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
+      with:
+        flags: roller
+  compile:
+    if: github.event_name == 'push' # will only be triggered when pushing to main & staging & develop & alpha
+    runs-on: ubuntu-latest
+    steps:
     - uses: actions-rs/toolchain@v1
       with:
         toolchain: nightly-2022-12-10
@@ -47,13 +66,6 @@ jobs:
     - name: Test
       run: |
         make roller
-        go test -tags="mock_prover" -v -coverprofile=coverage.txt ./...
-    - name: Upload coverage reports to Codecov
-      uses: codecov/codecov-action@v3
-      env:
-        CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
-      with:
-        flags: roller
   check:
     if: github.event.pull_request.draft == false
     runs-on: ubuntu-latest

bridge/internal/controller/relayer/l2_relayer.go

@@ -29,7 +29,6 @@ var (
 	bridgeL2BatchesCommittedTotalCounter          = gethMetrics.NewRegisteredCounter("bridge/l2/batches/committed/total", metrics.ScrollRegistry)
 	bridgeL2BatchesFinalizedConfirmedTotalCounter = gethMetrics.NewRegisteredCounter("bridge/l2/batches/finalized/confirmed/total", metrics.ScrollRegistry)
 	bridgeL2BatchesCommittedConfirmedTotalCounter = gethMetrics.NewRegisteredCounter("bridge/l2/batches/committed/confirmed/total", metrics.ScrollRegistry)
-	bridgeL2BatchesSkippedTotalCounter            = gethMetrics.NewRegisteredCounter("bridge/l2/batches/skipped/total", metrics.ScrollRegistry)
 )
 
 // Layer2Relayer is responsible for
@@ -394,15 +393,6 @@ func (r *Layer2Relayer) ProcessPendingBatches() {
 
 // ProcessCommittedBatches submit proof to layer 1 rollup contract
 func (r *Layer2Relayer) ProcessCommittedBatches() {
-	// set skipped batches in a single db operation
-	if count, err := r.batchOrm.UpdateSkippedBatches(r.ctx); err != nil {
-		log.Error("UpdateSkippedBatches failed", "err", err)
-		// continue anyway
-	} else if count > 0 {
-		bridgeL2BatchesSkippedTotalCounter.Inc(int64(count))
-		log.Info("Skipping batches", "count", count)
-	}
-
 	// retrieves the earliest batch whose rollup status is 'committed'
 	fields := map[string]interface{}{
 		"rollup_status": types.RollupCommitted,
@@ -430,11 +420,6 @@ func (r *Layer2Relayer) ProcessCommittedBatches() {
 		// It's an intermediate state. The roller manager received the proof but has not verified
 		// the proof yet. We don't roll up the proof until it's verified.
 		return
-	case types.ProvingTaskFailed, types.ProvingTaskSkipped:
-		// note: this is covered by UpdateSkippedBatches, but we keep it for completeness's sake
-		if err = r.batchOrm.UpdateRollupStatus(r.ctx, hash, types.RollupFinalizationSkipped); err != nil {
-			log.Warn("UpdateRollupStatus failed", "hash", hash, "err", err)
-		}
 	case types.ProvingTaskVerified:
 		log.Info("Start to roll up zk proof", "hash", hash)
 		success := false
@@ -454,8 +439,8 @@ func (r *Layer2Relayer) ProcessCommittedBatches() {
 		defer func() {
 			// TODO: need to revisit this and have a more fine-grained error handling
 			if !success {
-				log.Info("Failed to upload the proof, change rollup status to FinalizationSkipped", "hash", hash)
-				if err = r.batchOrm.UpdateRollupStatus(r.ctx, hash, types.RollupFinalizationSkipped); err != nil {
+				log.Info("Failed to upload the proof, change rollup status to RollupFinalizeFailed", "hash", hash)
+				if err = r.batchOrm.UpdateRollupStatus(r.ctx, hash, types.RollupFinalizeFailed); err != nil {
 					log.Warn("UpdateRollupStatus failed", "hash", hash, "err", err)
 				}
 			}

bridge/internal/controller/relayer/l2_relayer_test.go

@@ -90,7 +90,7 @@ func testL2RelayerProcessCommittedBatches(t *testing.T) {
 	statuses, err := batchOrm.GetRollupStatusByHashList(context.Background(), []string{batch.Hash})
 	assert.NoError(t, err)
 	assert.Equal(t, 1, len(statuses))
-	assert.Equal(t, types.RollupFinalizationSkipped, statuses[0])
+	assert.Equal(t, types.RollupFinalizeFailed, statuses[0])
 
 	err = batchOrm.UpdateRollupStatus(context.Background(), batch.Hash, types.RollupCommitted)
 	assert.NoError(t, err)
@@ -108,67 +108,6 @@ func testL2RelayerProcessCommittedBatches(t *testing.T) {
 	assert.Equal(t, types.RollupFinalizing, statuses[0])
 }
 
-func testL2RelayerSkipBatches(t *testing.T) {
-	db := setupL2RelayerDB(t)
-	defer database.CloseDB(db)
-
-	l2Cfg := cfg.L2Config
-	relayer, err := NewLayer2Relayer(context.Background(), l2Cli, db, l2Cfg.RelayerConfig, false)
-	assert.NoError(t, err)
-
-	batchOrm := orm.NewBatch(db)
-	createBatch := func(rollupStatus types.RollupStatus, provingStatus types.ProvingStatus) string {
-		batch, err := batchOrm.InsertBatch(context.Background(), 0, 1, chunkHash1.Hex(), chunkHash2.Hex(), []*types.Chunk{chunk1, chunk2})
-		assert.NoError(t, err)
-
-		err = batchOrm.UpdateRollupStatus(context.Background(), batch.Hash, rollupStatus)
-		assert.NoError(t, err)
-
-		proof := &message.AggProof{
-			Proof:     []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31},
-			FinalPair: []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31},
-		}
-		err = batchOrm.UpdateProofByHash(context.Background(), batch.Hash, proof, 100)
-		assert.NoError(t, err)
-		err = batchOrm.UpdateProvingStatus(context.Background(), batch.Hash, provingStatus)
-		assert.NoError(t, err)
-		return batch.Hash
-	}
-
-	skipped := []string{
-		createBatch(types.RollupCommitted, types.ProvingTaskSkipped),
-		createBatch(types.RollupCommitted, types.ProvingTaskFailed),
-	}
-
-	notSkipped := []string{
-		createBatch(types.RollupPending, types.ProvingTaskSkipped),
-		createBatch(types.RollupCommitting, types.ProvingTaskSkipped),
-		createBatch(types.RollupFinalizing, types.ProvingTaskSkipped),
-		createBatch(types.RollupFinalized, types.ProvingTaskSkipped),
-		createBatch(types.RollupPending, types.ProvingTaskFailed),
-		createBatch(types.RollupCommitting, types.ProvingTaskFailed),
-		createBatch(types.RollupFinalizing, types.ProvingTaskFailed),
-		createBatch(types.RollupFinalized, types.ProvingTaskFailed),
-		createBatch(types.RollupCommitted, types.ProvingTaskVerified),
-	}
-
-	relayer.ProcessCommittedBatches()
-
-	for _, id := range skipped {
-		statuses, err := batchOrm.GetRollupStatusByHashList(context.Background(), []string{id})
-		assert.NoError(t, err)
-		assert.Equal(t, 1, len(statuses))
-		assert.Equal(t, types.RollupFinalizationSkipped, statuses[0])
-	}
-
-	for _, id := range notSkipped {
-		statuses, err := batchOrm.GetRollupStatusByHashList(context.Background(), []string{id})
-		assert.NoError(t, err)
-		assert.Equal(t, 1, len(statuses))
-		assert.NotEqual(t, types.RollupFinalizationSkipped, statuses[0])
-	}
-}
-
 func testL2RelayerRollupConfirm(t *testing.T) {
 	db := setupL2RelayerDB(t)
 	defer database.CloseDB(db)

bridge/internal/controller/relayer/relayer_test.go

@@ -97,7 +97,6 @@ func TestFunctions(t *testing.T) {
 	t.Run("TestCreateNewRelayer", testCreateNewRelayer)
 	t.Run("TestL2RelayerProcessPendingBatches", testL2RelayerProcessPendingBatches)
 	t.Run("TestL2RelayerProcessCommittedBatches", testL2RelayerProcessCommittedBatches)
-	t.Run("TestL2RelayerSkipBatches", testL2RelayerSkipBatches)
 	t.Run("TestL2RelayerRollupConfirm", testL2RelayerRollupConfirm)
 	t.Run("TestL2RelayerGasOracleConfirm", testL2RelayerGasOracleConfirm)
 	t.Run("TestLayer2RelayerProcessGasPriceOracle", testLayer2RelayerProcessGasPriceOracle)

bridge/internal/orm/batch.go

@@ -280,25 +280,6 @@ func (o *Batch) InsertBatch(ctx context.Context, startChunkIndex, endChunkIndex
 	return &newBatch, nil
 }
 
-// UpdateSkippedBatches updates the skipped batches in the database.
-func (o *Batch) UpdateSkippedBatches(ctx context.Context) (uint64, error) {
-	provingStatusList := []interface{}{
-		int(types.ProvingTaskSkipped),
-		int(types.ProvingTaskFailed),
-	}
-
-	db := o.db.WithContext(ctx)
-	db = db.Model(&Batch{})
-	db = db.Where("rollup_status", int(types.RollupCommitted))
-	db = db.Where("proving_status IN (?)", provingStatusList)
-
-	result := db.Update("rollup_status", int(types.RollupFinalizationSkipped))
-	if result.Error != nil {
-		return 0, fmt.Errorf("Batch.UpdateSkippedBatches error: %w", result.Error)
-	}
-	return uint64(result.RowsAffected), nil
-}
-
 // UpdateL2GasOracleStatusAndOracleTxHash updates the L2 gas oracle status and transaction hash for a batch.
 func (o *Batch) UpdateL2GasOracleStatusAndOracleTxHash(ctx context.Context, hash string, status types.GasOracleStatus, txHash string) error {
 	updateFields := make(map[string]interface{})

bridge/internal/orm/orm_test.go

@@ -202,27 +202,6 @@ func TestBatchOrm(t *testing.T) {
 	assert.Equal(t, types.RollupPending, rollupStatus[0])
 	assert.Equal(t, types.RollupPending, rollupStatus[1])
 
-	err = batchOrm.UpdateProvingStatus(context.Background(), batchHash1, types.ProvingTaskSkipped)
-	assert.NoError(t, err)
-	err = batchOrm.UpdateRollupStatus(context.Background(), batchHash1, types.RollupCommitted)
-	assert.NoError(t, err)
-	err = batchOrm.UpdateProvingStatus(context.Background(), batchHash2, types.ProvingTaskFailed)
-	assert.NoError(t, err)
-	err = batchOrm.UpdateRollupStatus(context.Background(), batchHash2, types.RollupCommitted)
-	assert.NoError(t, err)
-
-	count, err = batchOrm.UpdateSkippedBatches(context.Background())
-	assert.NoError(t, err)
-	assert.Equal(t, uint64(2), count)
-
-	count, err = batchOrm.UpdateSkippedBatches(context.Background())
-	assert.NoError(t, err)
-	assert.Equal(t, uint64(0), count)
-
-	batch, err := batchOrm.GetBatchByIndex(context.Background(), 1)
-	assert.NoError(t, err)
-	assert.Equal(t, types.RollupFinalizationSkipped, types.RollupStatus(batch.RollupStatus))
-
 	err = batchOrm.UpdateProvingStatus(context.Background(), batchHash2, types.ProvingTaskVerified)
 	assert.NoError(t, err)
 

common/types/db.go

@@ -138,8 +138,6 @@ const (
 	ProvingStatusUndefined ProvingStatus = iota
 	// ProvingTaskUnassigned : proving_task is not assigned to be proved
 	ProvingTaskUnassigned
-	// ProvingTaskSkipped : proving_task is skipped for proof generation
-	ProvingTaskSkipped
 	// ProvingTaskAssigned : proving_task is assigned to be proved
 	ProvingTaskAssigned
 	// ProvingTaskProved : proof has been returned by prover
@@ -154,8 +152,6 @@ func (ps ProvingStatus) String() string {
 	switch ps {
 	case ProvingTaskUnassigned:
 		return "unassigned"
-	case ProvingTaskSkipped:
-		return "skipped"
 	case ProvingTaskAssigned:
 		return "assigned"
 	case ProvingTaskProved:
@@ -210,8 +206,6 @@ const (
 	RollupFinalizing
 	// RollupFinalized : finalize transaction is confirmed to layer1
 	RollupFinalized
-	// RollupFinalizationSkipped : batch finalization is skipped
-	RollupFinalizationSkipped
 	// RollupCommitFailed : rollup commit transaction confirmed but failed
 	RollupCommitFailed
 	// RollupFinalizeFailed : rollup finalize transaction is confirmed but failed
@@ -230,8 +224,6 @@ func (s RollupStatus) String() string {
 		return "RollupFinalizing"
 	case RollupFinalized:
 		return "RollupFinalized"
-	case RollupFinalizationSkipped:
-		return "RollupFinalizationSkipped"
 	case RollupCommitFailed:
 		return "RollupCommitFailed"
 	case RollupFinalizeFailed:

common/types/db_test.go

@@ -52,11 +52,6 @@ func TestProvingStatus(t *testing.T) {
 			ProvingTaskUnassigned,
 			"unassigned",
 		},
-		{
-			"ProvingTaskSkipped",
-			ProvingTaskSkipped,
-			"skipped",
-		},
 		{
 			"ProvingTaskAssigned",
 			ProvingTaskAssigned,

common/version/version.go

@@ -5,7 +5,7 @@ import (
 	"runtime/debug"
 )
 
-var tag = "v4.0.17"
+var tag = "v4.0.18"
 
 var commit = func() string {
 	if info, ok := debug.ReadBuildInfo(); ok {

database/migrate/migrations/00004_chunk.sql

@@ -35,7 +35,7 @@ create table chunk
 );
 
 comment
-on column chunk.proving_status is 'undefined, unassigned, skipped, assigned, proved, verified, failed';
+on column chunk.proving_status is 'undefined, unassigned, assigned, proved, verified, failed';
 
 create unique index chunk_index_uindex
 on chunk (index);

database/migrate/migrations/00005_batch.sql

@@ -49,10 +49,10 @@ comment
 on column batch.chunk_proofs_status is 'undefined, pending, ready';
 
 comment
-on column batch.proving_status is 'undefined, unassigned, skipped, assigned, proved, verified, failed';
+on column batch.proving_status is 'undefined, unassigned, assigned, proved, verified, failed';
 
 comment
-on column batch.rollup_status is 'undefined, pending, committing, committed, finalizing, finalized, finalization_skipped, commit_failed, finalize_failed';
+on column batch.rollup_status is 'undefined, pending, committing, committed, finalizing, finalized, commit_failed, finalize_failed';
 
 -- +goose StatementEnd