Skip to content

Commit

Permalink
refactor(coordinator): omit coordinator's error details to prover & a…
Browse files Browse the repository at this point in the history
…dd logs (#863)

Co-authored-by: colinlyguo <colinlyguo@users.noreply.github.com>
  • Loading branch information
colinlyguo and colinlyguo authored Aug 24, 2023
1 parent c44b7f7 commit dab21fc
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 23 deletions.
2 changes: 1 addition & 1 deletion common/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"strings"
)

var tag = "v4.1.103"
var tag = "v4.1.104"

var commit = func() string {
if info, ok := debug.ReadBuildInfo(); ok {
Expand Down
4 changes: 2 additions & 2 deletions coordinator/internal/controller/api/get_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ func NewGetTaskController(cfg *config.Config, db *gorm.DB, vf *verifier.Verifier
func (ptc *GetTaskController) GetTasks(ctx *gin.Context) {
var getTaskParameter coordinatorType.GetTaskParameter
if err := ctx.ShouldBind(&getTaskParameter); err != nil {
nerr := fmt.Errorf("prover tasks parameter invalid, err:%w", err)
nerr := fmt.Errorf("prover task parameter invalid, err:%w", err)
coordinatorType.RenderJSON(ctx, types.ErrCoordinatorParameterInvalidNo, nerr, nil)
return
}

proofType := ptc.proofType(&getTaskParameter)
proverTask, isExist := ptc.proverTasks[proofType]
if !isExist {
nerr := fmt.Errorf("parameter wrong proof type")
nerr := fmt.Errorf("parameter wrong proof type:%v", proofType)
coordinatorType.RenderJSON(ctx, types.ErrCoordinatorParameterInvalidNo, nerr, nil)
return
}
Expand Down
14 changes: 7 additions & 7 deletions coordinator/internal/controller/api/submit_proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"scroll-tech/coordinator/internal/config"
"scroll-tech/coordinator/internal/logic/submitproof"
"scroll-tech/coordinator/internal/logic/verifier"
coodinatorType "scroll-tech/coordinator/internal/types"
coordinatorType "scroll-tech/coordinator/internal/types"
)

// SubmitProofController the submit proof api controller
Expand All @@ -31,10 +31,10 @@ func NewSubmitProofController(cfg *config.Config, db *gorm.DB, vf *verifier.Veri

// SubmitProof prover submit the proof to coordinator
func (spc *SubmitProofController) SubmitProof(ctx *gin.Context) {
var spp coodinatorType.SubmitProofParameter
var spp coordinatorType.SubmitProofParameter
if err := ctx.ShouldBind(&spp); err != nil {
nerr := fmt.Errorf("parameter invalid, err:%w", err)
coodinatorType.RenderJSON(ctx, types.ErrCoordinatorParameterInvalidNo, nerr, nil)
coordinatorType.RenderJSON(ctx, types.ErrCoordinatorParameterInvalidNo, nerr, nil)
return
}

Expand All @@ -52,15 +52,15 @@ func (spc *SubmitProofController) SubmitProof(ctx *gin.Context) {
var tmpChunkProof message.ChunkProof
if err := json.Unmarshal([]byte(spp.Proof), &tmpChunkProof); err != nil {
nerr := fmt.Errorf("unmarshal parameter chunk proof invalid, err:%w", err)
coodinatorType.RenderJSON(ctx, types.ErrCoordinatorParameterInvalidNo, nerr, nil)
coordinatorType.RenderJSON(ctx, types.ErrCoordinatorParameterInvalidNo, nerr, nil)
return
}
proofMsg.ChunkProof = &tmpChunkProof
case message.ProofTypeBatch:
var tmpBatchProof message.BatchProof
if err := json.Unmarshal([]byte(spp.Proof), &tmpBatchProof); err != nil {
nerr := fmt.Errorf("unmarshal parameter batch proof invalid, err:%w", err)
coodinatorType.RenderJSON(ctx, types.ErrCoordinatorParameterInvalidNo, nerr, nil)
coordinatorType.RenderJSON(ctx, types.ErrCoordinatorParameterInvalidNo, nerr, nil)
return
}
proofMsg.BatchProof = &tmpBatchProof
Expand All @@ -69,8 +69,8 @@ func (spc *SubmitProofController) SubmitProof(ctx *gin.Context) {

if err := spc.submitProofReceiverLogic.HandleZkProof(ctx, &proofMsg, spp); err != nil {
nerr := fmt.Errorf("handle zk proof failure, err:%w", err)
coodinatorType.RenderJSON(ctx, types.ErrCoordinatorHandleZkProofFailure, nerr, nil)
coordinatorType.RenderJSON(ctx, types.ErrCoordinatorHandleZkProofFailure, nerr, nil)
return
}
coodinatorType.RenderJSON(ctx, types.Success, nil, nil)
coordinatorType.RenderJSON(ctx, types.Success, nil, nil)
}
13 changes: 9 additions & 4 deletions coordinator/internal/logic/provertask/batch_prover_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,18 @@ func (bp *BatchProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
}

if len(batchTasks) != 1 {
return nil, fmt.Errorf("get unassigned batch proving task len not 1, batch tasks:%v", batchTasks)
log.Error("get unassigned batch proving task len not 1", "length", len(batchTasks), "batch tasks", batchTasks)
return nil, ErrCoordinatorInternalFailure
}

batchTask := batchTasks[0]
log.Info("start batch proof generation session", "id", batchTask.Hash, "public key", publicKey, "prover name", proverName)

if !bp.checkAttemptsExceeded(batchTask.Hash, message.ProofTypeBatch) {
bp.batchAttemptsExceedTotal.Inc()
return nil, fmt.Errorf("the batch task id:%s check attempts have reach the maximum", batchTask.Hash)
// TODO: retry fetching unassigned batch proving task
log.Error("batch task proving attempts reach the maximum", "hash", batchTask.Hash)
return nil, nil
}

proverTask := orm.ProverTask{
Expand All @@ -127,13 +130,15 @@ func (bp *BatchProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
// Store session info.
if err = bp.proverTaskOrm.SetProverTask(ctx, &proverTask); err != nil {
bp.recoverProvingStatus(ctx, batchTask)
return nil, fmt.Errorf("db set session info fail, session id:%s, error:%w", proverTask.TaskID, err)
log.Error("db set session info fail", "task hash", batchTask.Hash, "prover name", proverName.(string), "prover pubKey", publicKey.(string), "err", err)
return nil, ErrCoordinatorInternalFailure
}

taskMsg, err := bp.formatProverTask(ctx, batchTask.Hash)
if err != nil {
bp.recoverProvingStatus(ctx, batchTask)
return nil, fmt.Errorf("format prover failure, id:%s error:%w", batchTask.Hash, err)
log.Error("format prover task failure", "hash", batchTask.Hash, "err", err)
return nil, ErrCoordinatorInternalFailure
}

bp.batchTaskGetTaskTotal.Inc()
Expand Down
19 changes: 14 additions & 5 deletions coordinator/internal/logic/provertask/chunk_prover_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import (
coordinatorType "scroll-tech/coordinator/internal/types"
)

// ErrCoordinatorInternalFailure coordinator internal db failure
var ErrCoordinatorInternalFailure = fmt.Errorf("coordinator internal error")

// ChunkProverTask the chunk prover task
type ChunkProverTask struct {
BaseProverTask
Expand Down Expand Up @@ -94,15 +97,17 @@ func (cp *ChunkProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
// load and send chunk tasks
chunkTasks, err := cp.chunkOrm.UpdateUnassignedChunkReturning(ctx, getTaskParameter.ProverHeight, 1)
if err != nil {
return nil, fmt.Errorf("failed to get unassigned chunk proving tasks, error:%w", err)
log.Error("failed to get unassigned chunk proving tasks", "height", getTaskParameter.ProverHeight, "err", err)
return nil, ErrCoordinatorInternalFailure
}

if len(chunkTasks) == 0 {
return nil, nil
}

if len(chunkTasks) != 1 {
return nil, fmt.Errorf("get unassigned chunk proving task len not 1, chunk tasks:%v", chunkTasks)
log.Error("get unassigned chunk proving task len not 1", "length", len(chunkTasks), "chunk tasks", chunkTasks)
return nil, ErrCoordinatorInternalFailure
}

chunkTask := chunkTasks[0]
Expand All @@ -111,7 +116,9 @@ func (cp *ChunkProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato

if !cp.checkAttemptsExceeded(chunkTask.Hash, message.ProofTypeChunk) {
cp.chunkAttemptsExceedTotal.Inc()
return nil, fmt.Errorf("chunk proof hash id:%s check attempts have reach the maximum", chunkTask.Hash)
// TODO: retry fetching unassigned chunk proving task
log.Error("chunk task proving attempts reach the maximum", "hash", chunkTask.Hash)
return nil, nil
}

proverTask := orm.ProverTask{
Expand All @@ -127,13 +134,15 @@ func (cp *ChunkProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
}
if err = cp.proverTaskOrm.SetProverTask(ctx, &proverTask); err != nil {
cp.recoverProvingStatus(ctx, chunkTask)
return nil, fmt.Errorf("db set session info fail, session id:%s , public key:%s, err:%w", chunkTask.Hash, publicKey, err)
log.Error("db set session info fail", "task hash", chunkTask.Hash, "prover name", proverName.(string), "prover pubKey", publicKey.(string), "err", err)
return nil, ErrCoordinatorInternalFailure
}

taskMsg, err := cp.formatProverTask(ctx, chunkTask.Hash)
if err != nil {
cp.recoverProvingStatus(ctx, chunkTask)
return nil, fmt.Errorf("format prover task failure, id:%s error:%w", chunkTask.Hash, err)
log.Error("format prover task failure", "hash", chunkTask.Hash, "err", err)
return nil, ErrCoordinatorInternalFailure
}

cp.chunkTaskGetTaskTotal.Inc()
Expand Down
14 changes: 10 additions & 4 deletions coordinator/internal/logic/submitproof/proof_receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ var (
ErrValidatorFailureProofTimeout = errors.New("validator failure submit proof timeout")
// ErrValidatorFailureTaskHaveVerifiedSuccess have proved success and verified success
ErrValidatorFailureTaskHaveVerifiedSuccess = errors.New("validator failure chunk/batch have proved and verified success")
// ErrValidatorFailureVerifiedFailed failed to verify and the verifier returns error
ErrValidatorFailureVerifiedFailed = fmt.Errorf("verification failed, verifier returns error")
// ErrValidatorSuccessInvalidProof successful verified and the proof is invalid
ErrValidatorSuccessInvalidProof = fmt.Errorf("verification succeeded, it's an invalid proof")
// ErrCoordinatorInternalFailure coordinator internal db failure
ErrCoordinatorInternalFailure = fmt.Errorf("coordinator internal error")
)

// ProofReceiverLogic the proof receiver logic
Expand Down Expand Up @@ -162,10 +168,10 @@ func (m *ProofReceiverLogic) HandleZkProof(ctx *gin.Context, proofMsg *message.P
log.Info("proof verified by coordinator failed", "proof id", proofMsg.ID, "prover name", proverTask.ProverName,
"prover pk", pk, "prove type", proofMsg.Type, "proof time", proofTimeSec, "error", verifyErr)

if verifyErr == nil {
verifyErr = fmt.Errorf("verification succeeded and it's an invalid proof")
if verifyErr != nil {
return ErrValidatorFailureVerifiedFailed
}
return verifyErr
return ErrValidatorSuccessInvalidProof
}

m.proverTaskProveDuration.Observe(time.Since(proverTask.CreatedAt).Seconds())
Expand All @@ -176,7 +182,7 @@ func (m *ProofReceiverLogic) HandleZkProof(ctx *gin.Context, proofMsg *message.P
if err := m.closeProofTask(ctx, proofMsg.ID, pk, proofMsg, proofTimeSec); err != nil {
m.proofSubmitFailure.Inc()
m.proofRecover(ctx, proofMsg.ID, pk, proofMsg)
return err
return ErrCoordinatorInternalFailure
}

return nil
Expand Down

0 comments on commit dab21fc

Please sign in to comment.