Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(coordinator): omit coordinator's error details to prover & add logs #863

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
f84e7ba
refactor(coordinator): remove error details to prover and add error logs
colinlyguo Aug 23, 2023
e45273f
Merge branch 'develop' into feat-coordinator-hide-error-detail-in-pro…
colinlyguo Aug 23, 2023
dd6e92b
chore: auto version bump [bot]
colinlyguo Aug 23, 2023
45a7f9b
fix merge develop typo
colinlyguo Aug 23, 2023
592f0fb
Merge branch 'develop' into feat-coordinator-hide-error-detail-in-pro…
colinlyguo Aug 23, 2023
0ff34bd
chore: auto version bump [bot]
colinlyguo Aug 23, 2023
bfd2d29
Merge branch 'develop' into feat-coordinator-hide-error-detail-in-pro…
colinlyguo Aug 24, 2023
5c23ca8
chore: auto version bump [bot]
colinlyguo Aug 24, 2023
d1b6bea
address comments
colinlyguo Aug 24, 2023
2adc7b6
chore: auto version bump [bot]
colinlyguo Aug 24, 2023
ba260ef
Merge branch 'develop' into feat-coordinator-hide-error-detail-in-pro…
colinlyguo Aug 24, 2023
91a113f
fix
colinlyguo Aug 24, 2023
fd1424b
Merge branch 'develop' into feat-coordinator-hide-error-detail-in-pro…
colinlyguo Aug 24, 2023
161132a
chore: auto version bump [bot]
colinlyguo Aug 24, 2023
736898d
fix
colinlyguo Aug 24, 2023
a335224
chore: auto version bump [bot]
colinlyguo Aug 24, 2023
655fb87
Merge branch 'develop' into feat-coordinator-hide-error-detail-in-pro…
colinlyguo Aug 24, 2023
c3fa073
fix log error
colinlyguo Aug 24, 2023
ca13f48
Merge branch 'develop' into feat-coordinator-hide-error-detail-in-pro…
colinlyguo Aug 24, 2023
39d4cd9
address comments
colinlyguo Aug 24, 2023
4710b7e
address comments
colinlyguo Aug 24, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
0xmountaintop marked this conversation as resolved.
Show resolved Hide resolved
}

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
Thegaram marked this conversation as resolved.
Show resolved Hide resolved
}

return nil
Expand Down
Loading