Skip to content

Commit

Permalink
[evm] panic on duplicate revert version
Browse files Browse the repository at this point in the history
  • Loading branch information
dustinxie committed Apr 9, 2024
1 parent 036fcb8 commit 3184c44
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 6 deletions.
2 changes: 2 additions & 0 deletions action/protocol/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ type (
DisableDelegateEndorsement bool
RefactorFreshAccountConversion bool
SuicideTxLogMismatchPanic bool
PanicUnrecoverableError bool
}

// FeatureWithHeightCtx provides feature check functions.
Expand Down Expand Up @@ -265,6 +266,7 @@ func WithFeatureCtx(ctx context.Context) context.Context {
DisableDelegateEndorsement: !g.IsTsunami(height),
RefactorFreshAccountConversion: g.IsTsunami(height),
SuicideTxLogMismatchPanic: g.IsToBeEnabled(height),
PanicUnrecoverableError: g.IsToBeEnabled(height),
},
)
}
Expand Down
3 changes: 3 additions & 0 deletions action/protocol/execution/evm/evm.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,9 @@ func prepareStateDB(ctx context.Context, sm protocol.StateManager) (*StateDBAdap
if featureCtx.SuicideTxLogMismatchPanic {
opts = append(opts, SuicideTxLogMismatchPanicOption())
}
if featureCtx.PanicUnrecoverableError {
opts = append(opts, PanicUnrecoverableErrorOption())
}

return NewStateDBAdapter(
sm,
Expand Down
16 changes: 14 additions & 2 deletions action/protocol/execution/evm/evmstatedbadapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ type (
manualCorrectGasRefund bool
suicideTxLogMismatchPanic bool
zeroNonceForFreshAccount bool
panicUnrecoverableError bool
}
)

Expand Down Expand Up @@ -162,6 +163,14 @@ func ZeroNonceForFreshAccountOption() StateDBAdapterOption {
}
}

// PanicUnrecoverableErrorOption set panicUnrecoverableError as true
func PanicUnrecoverableErrorOption() StateDBAdapterOption {
return func(adapter *StateDBAdapter) error {
adapter.panicUnrecoverableError = true
return nil
}
}

// NewStateDBAdapter creates a new state db with iotex blockchain
func NewStateDBAdapter(
sm protocol.StateManager,
Expand Down Expand Up @@ -586,16 +595,19 @@ func (stateDB *StateDBAdapter) Empty(evmAddr common.Address) bool {

// RevertToSnapshot reverts the state factory to the state at a given snapshot
func (stateDB *StateDBAdapter) RevertToSnapshot(snapshot int) {
ds, ok := stateDB.selfDestructedSnapshot[snapshot]
if !ok && stateDB.panicUnrecoverableError {
log.L().Panic("Failed to revert to snapshot.", zap.Int("snapshot", snapshot))
}
if err := stateDB.sm.Revert(snapshot); err != nil {
err := errors.New("unexpected error: state manager's Revert() failed")
log.L().Error("Failed to revert to snapshot.", zap.Error(err))
stateDB.logError(err)
return
}
ds, ok := stateDB.selfDestructedSnapshot[snapshot]
if !ok {
// this should not happen, b/c we save the SelfDestruct accounts on a successful return of Snapshot(), but check anyway
log.L().Error("Failed to get snapshot.", zap.Int("snapshot", snapshot))
log.L().Error("Failed to revert to snapshot.", zap.Int("snapshot", snapshot))
return
}
// restore gas refund
Expand Down
18 changes: 14 additions & 4 deletions action/protocol/execution/evm/evmstatedbadapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ func TestSnapshotRevertAndCommit(t *testing.T) {
}

func TestClearSnapshots(t *testing.T) {
testClearSnapshots := func(t *testing.T, async, fixSnapshotOrder bool) {
testClearSnapshots := func(t *testing.T, async, fixSnapshotOrder, panicUnrecoverableError bool) {
require := require.New(t)
ctrl := gomock.NewController(t)

Expand All @@ -716,6 +716,9 @@ func TestClearSnapshots(t *testing.T) {
if fixSnapshotOrder {
opts = append(opts, FixSnapshotOrderOption())
}
if panicUnrecoverableError {
opts = append(opts, PanicUnrecoverableErrorOption())
}
stateDB, err := NewStateDBAdapter(sm, 1, hash.ZeroHash256, opts...)
require.NoError(err)

Expand Down Expand Up @@ -774,13 +777,20 @@ func TestClearSnapshots(t *testing.T) {
// log snapshot added after fixSnapshotOrder, so it is cleared and 1 remains
require.Equal(1, len(stateDB.logsSnapshot))
}

if panicUnrecoverableError {
require.Panics(func() { stateDB.RevertToSnapshot(1) })
} else {
stateDB.RevertToSnapshot(1)
}
}
t.Run("contract w/o clear snapshots", func(t *testing.T) {
testClearSnapshots(t, false, false)
testClearSnapshots(t, false, false, false)
})
t.Run("contract with clear snapshots", func(t *testing.T) {
testClearSnapshots(t, false, true)
testClearSnapshots(t, false, true, false)
})
t.Run("contract with clear snapshots and panic on duplicate revert", func(t *testing.T) {
testClearSnapshots(t, false, true, true)
})
}

Expand Down

0 comments on commit 3184c44

Please sign in to comment.