From ba0ca998258187330735b7db94390a9139b69a9e Mon Sep 17 00:00:00 2001 From: envestcc Date: Thu, 5 Dec 2024 17:21:15 +0800 Subject: [PATCH 1/4] recover action handling panic --- action/const.go | 1 + state/factory/workingset.go | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/action/const.go b/action/const.go index 4742216401..8c15c6f248 100644 --- a/action/const.go +++ b/action/const.go @@ -44,6 +44,7 @@ var ( ErrGasTipOverFeeCap = errors.New("tip cap is greater than fee cap") ErrMissRequiredField = errors.New("missing required field") ErrValueVeryHigh = errors.New("value is very high") + ErrPanic = errors.New("panic") ) // LoadErrorDescription loads corresponding description related to the error diff --git a/state/factory/workingset.go b/state/factory/workingset.go index cb28cf880a..e637106e15 100644 --- a/state/factory/workingset.go +++ b/state/factory/workingset.go @@ -168,7 +168,13 @@ func withActionCtx(ctx context.Context, selp *action.SealedEnvelope) (context.Co func (ws *workingSet) runAction( ctx context.Context, selp *action.SealedEnvelope, -) (*action.Receipt, error) { +) (receipt *action.Receipt, err error) { + defer func() { + if r := recover(); r != nil { + receipt = nil + err = errors.Wrapf(action.ErrPanic, "panic when running action: %v", r) + } + }() actCtx := protocol.MustGetActionCtx(ctx) if protocol.MustGetBlockCtx(ctx).GasLimit < actCtx.IntrinsicGas { return nil, action.ErrGasLimit From aec2087a99f5f02acc144cca49c5c22d561a1e8b Mon Sep 17 00:00:00 2001 From: envestcc Date: Mon, 9 Dec 2024 13:17:00 +0800 Subject: [PATCH 2/4] continue mint if panic reverted --- action/const.go | 1 + state/factory/workingset.go | 33 ++++++++++++++++++++++++++------- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/action/const.go b/action/const.go index 8c15c6f248..8d32c92f07 100644 --- a/action/const.go +++ b/action/const.go @@ -45,6 +45,7 @@ var ( ErrMissRequiredField = errors.New("missing required field") ErrValueVeryHigh = errors.New("value is very high") ErrPanic = errors.New("panic") + ErrPanicButReverted = errors.New("panic but reverted") ) // LoadErrorDescription loads corresponding description related to the error diff --git a/state/factory/workingset.go b/state/factory/workingset.go index e637106e15..3fb935a41f 100644 --- a/state/factory/workingset.go +++ b/state/factory/workingset.go @@ -169,12 +169,6 @@ func (ws *workingSet) runAction( ctx context.Context, selp *action.SealedEnvelope, ) (receipt *action.Receipt, err error) { - defer func() { - if r := recover(); r != nil { - receipt = nil - err = errors.Wrapf(action.ErrPanic, "panic when running action: %v", r) - } - }() actCtx := protocol.MustGetActionCtx(ctx) if protocol.MustGetBlockCtx(ctx).GasLimit < actCtx.IntrinsicGas { return nil, action.ErrGasLimit @@ -213,6 +207,24 @@ func (ws *workingSet) runAction( return nil, errors.Wrapf(err, "Failed to get hash") } defer ws.ResetSnapshots() + sn := ws.Snapshot() + defer func() { + if err != nil { + if e := ws.Revert(sn); e != nil { + log.T(ctx).Error("Failed to revert snapshot", zap.Error(e)) + return + } + if errors.Is(err, action.ErrPanic) { + err = errors.Wrap(action.ErrPanicButReverted, err.Error()) + } + } + }() + defer func() { + if r := recover(); r != nil { + receipt = nil + err = errors.Wrapf(action.ErrPanic, "panic and reverted when running action: %v", r) + } + }() if err := ws.freshAccountConversion(ctx, &actCtx); err != nil { return nil, err } @@ -742,6 +754,8 @@ func (ws *workingSet) pickAndRunActions( actionIterator.PopAccount() continue } + actCtx := protocol.MustGetActionCtx(ctx) + l := log.L().With(log.Hex("actHash", actCtx.ActionHash[:]), zap.Uint64("height", ws.height)) receipt, err := ws.runAction(actionCtx, nextAction) switch errors.Cause(err) { case nil: @@ -750,7 +764,12 @@ func (ws *workingSet) pickAndRunActions( actionIterator.PopAccount() continue case action.ErrChainID, errUnfoldTxContainer, errDeployerNotWhitelisted: - log.L().Debug("runAction() failed", zap.Uint64("height", ws.height), zap.Error(err)) + l.Debug("runAction() failed", zap.Error(err)) + ap.DeleteAction(caller) + actionIterator.PopAccount() + continue + case action.ErrPanicButReverted: + l.Warn("runAction() panic but reverted", zap.Error(err)) ap.DeleteAction(caller) actionIterator.PopAccount() continue From ad330c9c72fc41d1a6efce069287d33742179698 Mon Sep 17 00:00:00 2001 From: envestcc Date: Mon, 9 Dec 2024 19:19:18 +0800 Subject: [PATCH 3/4] fix --- state/factory/workingset.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/state/factory/workingset.go b/state/factory/workingset.go index 3fb935a41f..15149c964e 100644 --- a/state/factory/workingset.go +++ b/state/factory/workingset.go @@ -754,7 +754,7 @@ func (ws *workingSet) pickAndRunActions( actionIterator.PopAccount() continue } - actCtx := protocol.MustGetActionCtx(ctx) + actCtx := protocol.MustGetActionCtx(actionCtx) l := log.L().With(log.Hex("actHash", actCtx.ActionHash[:]), zap.Uint64("height", ws.height)) receipt, err := ws.runAction(actionCtx, nextAction) switch errors.Cause(err) { From 748e4d8fea57609c2b61fcce986260331da70dac Mon Sep 17 00:00:00 2001 From: envestcc Date: Tue, 7 Jan 2025 16:40:36 +0800 Subject: [PATCH 4/4] address comment --- action/const.go | 1 - state/factory/workingset.go | 15 ++++----------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/action/const.go b/action/const.go index 8d32c92f07..37ed32d4c6 100644 --- a/action/const.go +++ b/action/const.go @@ -44,7 +44,6 @@ var ( ErrGasTipOverFeeCap = errors.New("tip cap is greater than fee cap") ErrMissRequiredField = errors.New("missing required field") ErrValueVeryHigh = errors.New("value is very high") - ErrPanic = errors.New("panic") ErrPanicButReverted = errors.New("panic but reverted") ) diff --git a/state/factory/workingset.go b/state/factory/workingset.go index 15149c964e..c3d7d92d7e 100644 --- a/state/factory/workingset.go +++ b/state/factory/workingset.go @@ -209,20 +209,13 @@ func (ws *workingSet) runAction( defer ws.ResetSnapshots() sn := ws.Snapshot() defer func() { - if err != nil { + if r := recover(); r != nil { + receipt = nil if e := ws.Revert(sn); e != nil { - log.T(ctx).Error("Failed to revert snapshot", zap.Error(e)) + err = errors.Wrapf(e, "panic and recovered but failed to revert when running action: %v", r) return } - if errors.Is(err, action.ErrPanic) { - err = errors.Wrap(action.ErrPanicButReverted, err.Error()) - } - } - }() - defer func() { - if r := recover(); r != nil { - receipt = nil - err = errors.Wrapf(action.ErrPanic, "panic and reverted when running action: %v", r) + err = errors.Wrapf(action.ErrPanicButReverted, "panic and reverted when running action: %v", r) } }() if err := ws.freshAccountConversion(ctx, &actCtx); err != nil {