Recover from action handling panic #4513
Conversation
envestcc
requested review from
CoderZhi,
dustinxie and
Liuhaai
as code owners
envestcc
changed the title
state/factory/workingset.go
Outdated
| defer func() { | ||
| if r := recover(); r != nil { | ||
| receipt = nil | ||
| err = errors.Wrapf(action.ErrPanic, "panic when running action: %v", r) |
There was a problem hiding this comment.
define a local errPanic so no need to change action/const.go
state/factory/workingset.go
Outdated
| defer func() { | ||
| if r := recover(); r != nil { | ||
| receipt = nil | ||
| err = errors.Wrapf(action.ErrPanic, "panic when running action: %v", r) |
There was a problem hiding this comment.
this is an error which should be caught and handled appropriately in pickAndRunActions such that the block mint process can continue
There was a problem hiding this comment.
If we want to continue minting block, we need to revert to the state after the previous transaction is executed. This may require calling Snapshot() before transaction execution, which would likely result in a hard fork.
|
| ap.DeleteAction(caller) | ||
| actionIterator.PopAccount() | ||
| continue | ||
| case action.ErrPanicButReverted: |
There was a problem hiding this comment.
if panic but reverted, continue when mint and terminate when validate block
| sn := ws.Snapshot() | ||
| defer func() { | ||
| if err != nil { | ||
| if e := ws.Revert(sn); e != nil { |
There was a problem hiding this comment.
Not sure if the Revert() will be a hard fork here. We may need run fullsync
| defer func() { | ||
| if err != nil { | ||
| if e := ws.Revert(sn); e != nil { | ||
| log.T(ctx).Error("Failed to revert snapshot", zap.Error(e)) |
| @@ -207,6 +207,24 @@ func (ws *workingSet) runAction( | |||
| return nil, errors.Wrapf(err, "Failed to get hash") | |||
| } | |||
| defer ws.ResetSnapshots() | |||
| sn := ws.Snapshot() | |||
| defer func() { | |||
There was a problem hiding this comment.
could these two defer functions been merged which will make it easier to read and maintain
Description
Capture exceptions when processing transactions to prevent the program from exiting abnormally, and instead remove problematic transactions.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: