Skip to content

Commit 13c8605

Browse files
georgehaocolinlyguo
andauthored
fix coordiantor assign bug (#1621)
Co-authored-by: georgehao <[email protected]> Co-authored-by: colinlyguo <[email protected]>
1 parent 228cba4 commit 13c8605

File tree

15 files changed

+142
-120
lines changed

15 files changed

+142
-120
lines changed

Makefile

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,12 @@ fmt: ## Format the code
3131
cd $(PWD)/rollup/ && go mod tidy
3232
cd $(PWD)/tests/integration-test/ && go mod tidy
3333

34-
goimports -local $(PWD)/bridge-history-api/ -w .
35-
goimports -local $(PWD)/common/ -w .
36-
goimports -local $(PWD)/coordinator/ -w .
37-
goimports -local $(PWD)/database/ -w .
38-
goimports -local $(PWD)/rollup/ -w .
39-
goimports -local $(PWD)/tests/integration-test/ -w .
34+
goimports -local scroll-tech/bridge-history-api/ -w .
35+
goimports -local scroll-tech/common/ -w .
36+
goimports -local scroll-tech/coordinator/ -w .
37+
goimports -local scroll-tech/database/ -w .
38+
goimports -local scroll-tech/rollup/ -w .
39+
goimports -local scroll-tech/tests/integration-test/ -w .
4040

4141
dev_docker: ## Build docker images for development/testing usages
4242
docker pull postgres

common/version/version.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import (
55
"runtime/debug"
66
)
77

8-
var tag = "v4.4.95"
8+
var tag = "v4.4.96"
99

1010
var commit = func() string {
1111
if info, ok := debug.ReadBuildInfo(); ok {

coordinator/internal/controller/api/auth.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func (a *AuthController) Login(c *gin.Context) (interface{}, error) {
4646

4747
hardForkNames, err := a.loginLogic.ProverHardForkName(&login)
4848
if err != nil {
49-
return "", fmt.Errorf("prover hard name failure:%w", err)
49+
return "", fmt.Errorf("prover hard fork name failure:%w", err)
5050
}
5151

5252
// check the challenge is used, if used, return failure

coordinator/internal/logic/provertask/batch_prover_task.go

Lines changed: 14 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ func (bp *BatchProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
7777
}
7878

7979
var batchTask *orm.Batch
80+
var hardForkName string
8081
for i := 0; i < 5; i++ {
8182
var getTaskError error
8283
var tmpBatchTask *orm.Batch
@@ -101,10 +102,20 @@ func (bp *BatchProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
101102
return nil, nil
102103
}
103104

105+
taskCtx.taskType = message.ProofTypeBatch
106+
taskCtx.batchTask = tmpBatchTask
107+
108+
var checkErr error
109+
hardForkName, checkErr = bp.hardForkSanityCheck(ctx, taskCtx)
110+
if checkErr != nil {
111+
log.Debug("hard fork sanity check failed", "height", getTaskParameter.ProverHeight, "err", checkErr)
112+
return nil, nil
113+
}
114+
104115
// Don't dispatch the same failing job to the same prover
105-
proverTasks, getTaskError := bp.proverTaskOrm.GetFailedProverTasksByHash(ctx.Copy(), message.ProofTypeBatch, tmpBatchTask.Hash, 2)
106-
if getTaskError != nil {
107-
log.Error("failed to get prover tasks", "proof type", message.ProofTypeBatch.String(), "task ID", tmpBatchTask.Hash, "error", getTaskError)
116+
proverTasks, getFailedTaskError := bp.proverTaskOrm.GetFailedProverTasksByHash(ctx.Copy(), message.ProofTypeBatch, tmpBatchTask.Hash, 2)
117+
if getFailedTaskError != nil {
118+
log.Error("failed to get prover tasks", "proof type", message.ProofTypeBatch.String(), "task ID", tmpBatchTask.Hash, "error", getFailedTaskError)
108119
return nil, ErrCoordinatorInternalFailure
109120
}
110121
for i := 0; i < len(proverTasks); i++ {
@@ -135,22 +146,6 @@ func (bp *BatchProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
135146
return nil, nil
136147
}
137148

138-
hardForkName, getHardForkErr := bp.hardForkName(ctx, batchTask)
139-
if getHardForkErr != nil {
140-
bp.recoverActiveAttempts(ctx, batchTask)
141-
log.Error("retrieve hard fork name by batch failed", "task_id", batchTask.Hash, "err", getHardForkErr)
142-
return nil, ErrCoordinatorInternalFailure
143-
}
144-
145-
if _, ok := taskCtx.HardForkNames[hardForkName]; !ok {
146-
bp.recoverActiveAttempts(ctx, batchTask)
147-
log.Debug("incompatible prover version",
148-
"requisite hard fork name", hardForkName,
149-
"prover hard fork name", taskCtx.HardForkNames,
150-
"task_id", batchTask.Hash)
151-
return nil, nil
152-
}
153-
154149
log.Info("start batch proof generation session", "task_id", batchTask.Hash, "public key", taskCtx.PublicKey, "prover name", taskCtx.ProverName)
155150
proverTask := orm.ProverTask{
156151
TaskID: batchTask.Hash,
@@ -188,20 +183,6 @@ func (bp *BatchProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
188183
return taskMsg, nil
189184
}
190185

191-
func (bp *BatchProverTask) hardForkName(ctx *gin.Context, batchTask *orm.Batch) (string, error) {
192-
startChunk, getChunkErr := bp.chunkOrm.GetChunkByHash(ctx, batchTask.StartChunkHash)
193-
if getChunkErr != nil {
194-
return "", getChunkErr
195-
}
196-
197-
l2Block, getBlockErr := bp.blockOrm.GetL2BlockByNumber(ctx.Copy(), startChunk.StartBlockNumber)
198-
if getBlockErr != nil {
199-
return "", getBlockErr
200-
}
201-
hardForkName := encoding.GetHardforkName(bp.chainCfg, l2Block.Number, l2Block.BlockTimestamp)
202-
return hardForkName, nil
203-
}
204-
205186
func (bp *BatchProverTask) formatProverTask(ctx context.Context, task *orm.ProverTask, batch *orm.Batch, hardForkName string) (*coordinatorType.GetTaskSchema, error) {
206187
// get chunk from db
207188
chunks, err := bp.chunkOrm.GetChunksByBatchHash(ctx, task.TaskID)

coordinator/internal/logic/provertask/bundle_prover_task.go

Lines changed: 15 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,18 @@ import (
99
"github.com/gin-gonic/gin"
1010
"github.com/prometheus/client_golang/prometheus"
1111
"github.com/prometheus/client_golang/prometheus/promauto"
12-
"github.com/scroll-tech/da-codec/encoding"
1312
"github.com/scroll-tech/go-ethereum/log"
1413
"github.com/scroll-tech/go-ethereum/params"
1514
"gorm.io/gorm"
1615

17-
"scroll-tech/common/types"
18-
"scroll-tech/common/types/message"
19-
"scroll-tech/common/utils"
20-
2116
"scroll-tech/coordinator/internal/config"
2217
"scroll-tech/coordinator/internal/orm"
2318
coordinatorType "scroll-tech/coordinator/internal/types"
2419
cutils "scroll-tech/coordinator/internal/utils"
20+
21+
"scroll-tech/common/types"
22+
"scroll-tech/common/types/message"
23+
"scroll-tech/common/utils"
2524
)
2625

2726
// BundleProverTask is prover task implement for bundle proof
@@ -77,6 +76,7 @@ func (bp *BundleProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinat
7776
}
7877

7978
var bundleTask *orm.Bundle
79+
var hardForkName string
8080
for i := 0; i < 5; i++ {
8181
var getTaskError error
8282
var tmpBundleTask *orm.Bundle
@@ -101,6 +101,16 @@ func (bp *BundleProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinat
101101
return nil, nil
102102
}
103103

104+
taskCtx.taskType = message.ProofTypeBundle
105+
taskCtx.bundleTask = tmpBundleTask
106+
107+
var checkErr error
108+
hardForkName, checkErr = bp.hardForkSanityCheck(ctx, taskCtx)
109+
if checkErr != nil {
110+
log.Debug("hard fork sanity check failed", "height", getTaskParameter.ProverHeight, "err", checkErr)
111+
return nil, nil
112+
}
113+
104114
// Don't dispatch the same failing job to the same prover
105115
proverTasks, getTaskError := bp.proverTaskOrm.GetFailedProverTasksByHash(ctx.Copy(), message.ProofTypeBundle, tmpBundleTask.Hash, 2)
106116
if getTaskError != nil {
@@ -135,22 +145,6 @@ func (bp *BundleProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinat
135145
return nil, nil
136146
}
137147

138-
hardForkName, getHardForkErr := bp.hardForkName(ctx, bundleTask)
139-
if getHardForkErr != nil {
140-
bp.recoverActiveAttempts(ctx, bundleTask)
141-
log.Error("retrieve hard fork name by bundle failed", "task_id", bundleTask.Hash, "err", getHardForkErr)
142-
return nil, ErrCoordinatorInternalFailure
143-
}
144-
145-
if _, ok := taskCtx.HardForkNames[hardForkName]; !ok {
146-
bp.recoverActiveAttempts(ctx, bundleTask)
147-
log.Debug("incompatible prover version",
148-
"requisite hard fork name", hardForkName,
149-
"prover hard fork name", taskCtx.HardForkNames,
150-
"task_id", bundleTask.Hash)
151-
return nil, nil
152-
}
153-
154148
log.Info("start bundle proof generation session", "task index", bundleTask.Index, "public key", taskCtx.PublicKey, "prover name", taskCtx.ProverName)
155149
proverTask := orm.ProverTask{
156150
TaskID: bundleTask.Hash,
@@ -188,26 +182,6 @@ func (bp *BundleProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinat
188182
return taskMsg, nil
189183
}
190184

191-
func (bp *BundleProverTask) hardForkName(ctx *gin.Context, bundleTask *orm.Bundle) (string, error) {
192-
startBatch, getBatchErr := bp.batchOrm.GetBatchByHash(ctx, bundleTask.StartBatchHash)
193-
if getBatchErr != nil {
194-
return "", getBatchErr
195-
}
196-
197-
startChunk, getChunkErr := bp.chunkOrm.GetChunkByHash(ctx, startBatch.StartChunkHash)
198-
if getChunkErr != nil {
199-
return "", getChunkErr
200-
}
201-
202-
l2Block, getBlockErr := bp.blockOrm.GetL2BlockByNumber(ctx.Copy(), startChunk.StartBlockNumber)
203-
if getBlockErr != nil {
204-
return "", getBlockErr
205-
}
206-
207-
hardForkName := encoding.GetHardforkName(bp.chainCfg, l2Block.Number, l2Block.BlockTimestamp)
208-
return hardForkName, nil
209-
}
210-
211185
func (bp *BundleProverTask) formatProverTask(ctx context.Context, task *orm.ProverTask, hardForkName string) (*coordinatorType.GetTaskSchema, error) {
212186
// get bundle from db
213187
batches, err := bp.batchOrm.GetBatchesByBundleHash(ctx, task.TaskID)

coordinator/internal/logic/provertask/chunk_prover_task.go

Lines changed: 14 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"github.com/gin-gonic/gin"
1010
"github.com/prometheus/client_golang/prometheus"
1111
"github.com/prometheus/client_golang/prometheus/promauto"
12-
"github.com/scroll-tech/da-codec/encoding"
1312
"github.com/scroll-tech/go-ethereum/log"
1413
"github.com/scroll-tech/go-ethereum/params"
1514
"gorm.io/gorm"
@@ -75,6 +74,7 @@ func (cp *ChunkProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
7574
}
7675

7776
var chunkTask *orm.Chunk
77+
var hardForkName string
7878
for i := 0; i < 5; i++ {
7979
var getTaskError error
8080
var tmpChunkTask *orm.Chunk
@@ -99,10 +99,20 @@ func (cp *ChunkProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
9999
return nil, nil
100100
}
101101

102+
taskCtx.taskType = message.ProofTypeChunk
103+
taskCtx.chunkTask = tmpChunkTask
104+
105+
var checkErr error
106+
hardForkName, checkErr = cp.hardForkSanityCheck(ctx, taskCtx)
107+
if checkErr != nil {
108+
log.Debug("hard fork sanity check failed", "height", getTaskParameter.ProverHeight, "err", checkErr)
109+
return nil, nil
110+
}
111+
102112
// Don't dispatch the same failing job to the same prover
103-
proverTasks, getTaskError := cp.proverTaskOrm.GetFailedProverTasksByHash(ctx.Copy(), message.ProofTypeChunk, tmpChunkTask.Hash, 2)
104-
if getTaskError != nil {
105-
log.Error("failed to get prover tasks", "proof type", message.ProofTypeChunk.String(), "task ID", tmpChunkTask.Hash, "error", getTaskError)
113+
proverTasks, getFailedTaskError := cp.proverTaskOrm.GetFailedProverTasksByHash(ctx.Copy(), message.ProofTypeChunk, tmpChunkTask.Hash, 2)
114+
if getFailedTaskError != nil {
115+
log.Error("failed to get prover tasks", "proof type", message.ProofTypeChunk.String(), "task ID", tmpChunkTask.Hash, "error", getFailedTaskError)
106116
return nil, ErrCoordinatorInternalFailure
107117
}
108118
for i := 0; i < len(proverTasks); i++ {
@@ -133,22 +143,6 @@ func (cp *ChunkProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
133143
return nil, nil
134144
}
135145

136-
hardForkName, getHardForkErr := cp.hardForkName(ctx, chunkTask)
137-
if getHardForkErr != nil {
138-
cp.recoverActiveAttempts(ctx, chunkTask)
139-
log.Error("retrieve hard fork name by chunk failed", "task_id", chunkTask.Hash, "err", getHardForkErr)
140-
return nil, ErrCoordinatorInternalFailure
141-
}
142-
143-
if _, ok := taskCtx.HardForkNames[hardForkName]; !ok {
144-
cp.recoverActiveAttempts(ctx, chunkTask)
145-
log.Debug("incompatible prover version",
146-
"requisite hard fork name", hardForkName,
147-
"prover hard fork name", taskCtx.HardForkNames,
148-
"task_id", chunkTask.Hash)
149-
return nil, nil
150-
}
151-
152146
log.Info("start chunk generation session", "task_id", chunkTask.Hash, "public key", taskCtx.PublicKey, "prover name", taskCtx.ProverName)
153147
proverTask := orm.ProverTask{
154148
TaskID: chunkTask.Hash,
@@ -185,15 +179,6 @@ func (cp *ChunkProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
185179
return taskMsg, nil
186180
}
187181

188-
func (cp *ChunkProverTask) hardForkName(ctx *gin.Context, chunkTask *orm.Chunk) (string, error) {
189-
l2Block, getBlockErr := cp.blockOrm.GetL2BlockByNumber(ctx.Copy(), chunkTask.StartBlockNumber)
190-
if getBlockErr != nil {
191-
return "", getBlockErr
192-
}
193-
hardForkName := encoding.GetHardforkName(cp.chainCfg, l2Block.Number, l2Block.BlockTimestamp)
194-
return hardForkName, nil
195-
}
196-
197182
func (cp *ChunkProverTask) formatProverTask(ctx context.Context, task *orm.ProverTask, hardForkName string) (*coordinatorType.GetTaskSchema, error) {
198183
// Get block hashes.
199184
blockHashes, dbErr := cp.blockOrm.GetL2BlockHashesByChunkHash(ctx, task.TaskID)

coordinator/internal/logic/provertask/prover_task.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,12 @@ import (
99
"github.com/gin-gonic/gin"
1010
"github.com/prometheus/client_golang/prometheus"
1111
"github.com/prometheus/client_golang/prometheus/promauto"
12+
"github.com/scroll-tech/da-codec/encoding"
1213
"github.com/scroll-tech/go-ethereum/params"
1314
"gorm.io/gorm"
1415

16+
"scroll-tech/common/types/message"
17+
1518
"scroll-tech/coordinator/internal/config"
1619
"scroll-tech/coordinator/internal/orm"
1720
coordinatorType "scroll-tech/coordinator/internal/types"
@@ -52,6 +55,76 @@ type proverTaskContext struct {
5255
ProverVersion string
5356
ProverProviderType uint8
5457
HardForkNames map[string]struct{}
58+
59+
taskType message.ProofType
60+
chunkTask *orm.Chunk
61+
batchTask *orm.Batch
62+
bundleTask *orm.Bundle
63+
}
64+
65+
// hardForkName get the chunk/batch/bundle hard fork name
66+
func (b *BaseProverTask) hardForkName(ctx *gin.Context, taskCtx *proverTaskContext) (string, error) {
67+
switch {
68+
case taskCtx.taskType == message.ProofTypeChunk:
69+
if taskCtx.chunkTask == nil {
70+
return "", errors.New("chunk task is nil")
71+
}
72+
l2Block, getBlockErr := b.blockOrm.GetL2BlockByNumber(ctx.Copy(), taskCtx.chunkTask.StartBlockNumber)
73+
if getBlockErr != nil {
74+
return "", getBlockErr
75+
}
76+
hardForkName := encoding.GetHardforkName(b.chainCfg, l2Block.Number, l2Block.BlockTimestamp)
77+
return hardForkName, nil
78+
79+
case taskCtx.taskType == message.ProofTypeBatch:
80+
if taskCtx.batchTask == nil {
81+
return "", errors.New("batch task is nil")
82+
}
83+
startChunk, getChunkErr := b.chunkOrm.GetChunkByHash(ctx, taskCtx.batchTask.StartChunkHash)
84+
if getChunkErr != nil {
85+
return "", getChunkErr
86+
}
87+
l2Block, getBlockErr := b.blockOrm.GetL2BlockByNumber(ctx.Copy(), startChunk.StartBlockNumber)
88+
if getBlockErr != nil {
89+
return "", getBlockErr
90+
}
91+
hardForkName := encoding.GetHardforkName(b.chainCfg, l2Block.Number, l2Block.BlockTimestamp)
92+
return hardForkName, nil
93+
94+
case taskCtx.taskType == message.ProofTypeBundle:
95+
if taskCtx.bundleTask == nil {
96+
return "", errors.New("bundle task is nil")
97+
}
98+
startBatch, getBatchErr := b.batchOrm.GetBatchByHash(ctx, taskCtx.bundleTask.StartBatchHash)
99+
if getBatchErr != nil {
100+
return "", getBatchErr
101+
}
102+
startChunk, getChunkErr := b.chunkOrm.GetChunkByHash(ctx, startBatch.StartChunkHash)
103+
if getChunkErr != nil {
104+
return "", getChunkErr
105+
}
106+
l2Block, getBlockErr := b.blockOrm.GetL2BlockByNumber(ctx.Copy(), startChunk.StartBlockNumber)
107+
if getBlockErr != nil {
108+
return "", getBlockErr
109+
}
110+
hardForkName := encoding.GetHardforkName(b.chainCfg, l2Block.Number, l2Block.BlockTimestamp)
111+
return hardForkName, nil
112+
default:
113+
return "", errors.New("illegal task type")
114+
}
115+
}
116+
117+
// hardForkSanityCheck check the task's hard fork name is the same as prover
118+
func (b *BaseProverTask) hardForkSanityCheck(ctx *gin.Context, taskCtx *proverTaskContext) (string, error) {
119+
hardForkName, getHardForkErr := b.hardForkName(ctx, taskCtx)
120+
if getHardForkErr != nil {
121+
return "", getHardForkErr
122+
}
123+
124+
if _, ok := taskCtx.HardForkNames[hardForkName]; !ok {
125+
return "", errors.New("to be assigned prover task's hard-fork name is not the same as prover")
126+
}
127+
return hardForkName, nil
55128
}
56129

57130
// checkParameter check the prover task parameter illegal

coordinator/internal/orm/orm_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,12 @@ import (
99
"github.com/stretchr/testify/assert"
1010
"gorm.io/gorm"
1111

12+
"scroll-tech/database/migrate"
13+
1214
"scroll-tech/common/testcontainers"
1315
"scroll-tech/common/types"
1416
"scroll-tech/common/types/message"
1517
"scroll-tech/common/utils"
16-
"scroll-tech/database/migrate"
1718
)
1819

1920
var (

0 commit comments

Comments
 (0)