[evm] fix snapshot bug (iotexproject#2802)

action/protocol/execution/evm/contract_test.go

@@ -67,7 +67,7 @@ func TestCreateContract(t *testing.T) {
 	addr := identityset.Address(28)
 	_, err = accountutil.LoadOrCreateAccount(sm, addr.String())
 	require.NoError(err)
-	stateDB := NewStateDBAdapter(sm, 0, !cfg.Genesis.IsAleutian(0), cfg.Genesis.IsGreenland(0), hash.ZeroHash256)
+	stateDB := NewStateDBAdapter(sm, 0, !cfg.Genesis.IsAleutian(0), cfg.Genesis.IsGreenland(0), cfg.Genesis.IsKamchatka(0), hash.ZeroHash256)
 	contract := addr.Bytes()
 	var evmContract common.Address
 	copy(evmContract[:], contract[:])

action/protocol/execution/evm/evm.go

@@ -191,6 +191,7 @@ func ExecuteContract(
 		blkCtx.BlockHeight,
 		!g.IsAleutian(blkCtx.BlockHeight),
 		g.IsGreenland(blkCtx.BlockHeight),
+		g.IsKamchatka(blkCtx.BlockHeight),
 		actionCtx.ActionHash,
 		opts...,
 	)

action/protocol/execution/evm/evm_test.go

@@ -172,7 +172,7 @@ func TestConstantinople(t *testing.T) {
 		)
 		require.NoError(err)
 
-		stateDB := NewStateDBAdapter(sm, e.height, !g.IsAleutian(e.height), g.IsGreenland(e.height), hash.ZeroHash256)
+		stateDB := NewStateDBAdapter(sm, e.height, !g.IsAleutian(e.height), g.IsGreenland(e.height), g.IsKamchatka(e.height), hash.ZeroHash256)
 		ctx = protocol.WithBlockCtx(ctx, protocol.BlockCtx{
 			Producer:    identityset.Address(27),
 			GasLimit:    testutil.TestGasLimit,

action/protocol/execution/evm/evmstatedbadapter.go

@@ -64,6 +64,7 @@ type (
 		preimageSnapshot    map[int]preimageMap
 		notFixTopicCopyBug  bool
 		asyncContractTrie   bool
+		fixSnapshotOrder    bool
 		sortCachedContracts bool
 		usePendingNonce     bool
 	}
@@ -94,6 +95,7 @@ func NewStateDBAdapter(
 	blockHeight uint64,
 	notFixTopicCopyBug bool,
 	asyncContractTrie bool,
+	fixSnapshotOrder bool,
 	executionHash hash.Hash256,
 	opts ...StateDBAdapterOption,
 ) *StateDBAdapter {
@@ -111,6 +113,7 @@ func NewStateDBAdapter(
 		preimageSnapshot:   make(map[int]preimageMap),
 		notFixTopicCopyBug: notFixTopicCopyBug,
 		asyncContractTrie:  asyncContractTrie,
+		fixSnapshotOrder:   fixSnapshotOrder,
 	}
 	for _, opt := range opts {
 		if err := opt(s); err != nil {
@@ -473,6 +476,13 @@ func (stateDB *StateDBAdapter) cachedContractAddrs() []hash.Hash160 {
 
 // Snapshot returns the snapshot id
 func (stateDB *StateDBAdapter) Snapshot() int {
+	// save a copy of modified contracts
+	c := make(contractMap)
+	if stateDB.fixSnapshotOrder {
+		for _, addr := range stateDB.cachedContractAddrs() {
+			c[addr] = stateDB.cachedContract[addr].Snapshot()
+		}
+	}
 	sn := stateDB.sm.Snapshot()
 	if _, ok := stateDB.suicideSnapshot[sn]; ok {
 		err := errors.New("unexpected error: duplicate snapshot version")
@@ -486,10 +496,10 @@ func (stateDB *StateDBAdapter) Snapshot() int {
 		sa[k] = v
 	}
 	stateDB.suicideSnapshot[sn] = sa
-	// save a copy of modified contracts
-	c := make(contractMap)
-	for _, addr := range stateDB.cachedContractAddrs() {
-		c[addr] = stateDB.cachedContract[addr].Snapshot()
+	if !stateDB.fixSnapshotOrder {
+		for _, addr := range stateDB.cachedContractAddrs() {
+			c[addr] = stateDB.cachedContract[addr].Snapshot()
+		}
 	}
 	stateDB.contractSnapshot[sn] = c
 	// save a copy of preimages

action/protocol/execution/evm/evmstatedbadapter_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/iotexproject/iotex-core/config"
 	"github.com/iotexproject/iotex-core/db/batch"
 	"github.com/iotexproject/iotex-core/state"
+	"github.com/iotexproject/iotex-core/test/identityset"
 	"github.com/iotexproject/iotex-core/test/mock/mock_chainmanager"
 )
 
@@ -93,7 +94,7 @@ func TestAddBalance(t *testing.T) {
 	sm, err := initMockStateManager(ctrl)
 	require.NoError(err)
 	addr := common.HexToAddress("02ae2a956d21e8d481c3a69e146633470cf625ec")
-	stateDB := NewStateDBAdapter(sm, 1, true, false, hash.ZeroHash256)
+	stateDB := NewStateDBAdapter(sm, 1, true, false, true, hash.ZeroHash256)
 	addAmount := big.NewInt(40000)
 	stateDB.AddBalance(addr, addAmount)
 	amount := stateDB.GetBalance(addr)
@@ -110,7 +111,7 @@ func TestRefundAPIs(t *testing.T) {
 
 	sm, err := initMockStateManager(ctrl)
 	require.NoError(err)
-	stateDB := NewStateDBAdapter(sm, 1, true, false, hash.ZeroHash256)
+	stateDB := NewStateDBAdapter(sm, 1, true, false, true, hash.ZeroHash256)
 	require.Zero(stateDB.GetRefund())
 	refund := uint64(1024)
 	stateDB.AddRefund(refund)
@@ -125,7 +126,7 @@ func TestEmptyAndCode(t *testing.T) {
 	sm, err := initMockStateManager(ctrl)
 	require.NoError(err)
 	addr := common.HexToAddress("02ae2a956d21e8d481c3a69e146633470cf625ec")
-	stateDB := NewStateDBAdapter(sm, 1, true, false, hash.ZeroHash256)
+	stateDB := NewStateDBAdapter(sm, 1, true, false, true, hash.ZeroHash256)
 	require.True(stateDB.Empty(addr))
 	stateDB.CreateAccount(addr)
 	require.True(stateDB.Empty(addr))
@@ -142,7 +143,7 @@ func TestForEachStorage(t *testing.T) {
 	sm, err := initMockStateManager(ctrl)
 	require.NoError(err)
 	addr := common.HexToAddress("02ae2a956d21e8d481c3a69e146633470cf625ec")
-	stateDB := NewStateDBAdapter(sm, 1, true, false, hash.ZeroHash256)
+	stateDB := NewStateDBAdapter(sm, 1, true, false, true, hash.ZeroHash256)
 	stateDB.CreateAccount(addr)
 	kvs := map[common.Hash]common.Hash{
 		common.HexToHash("0123456701234567012345670123456701234567012345670123456701234560"): common.HexToHash("0123456701234567012345670123456701234567012345670123456701234560"),
@@ -178,21 +179,21 @@ func TestNonce(t *testing.T) {
 	sm, err := initMockStateManager(ctrl)
 	require.NoError(err)
 	addr := common.HexToAddress("02ae2a956d21e8d481c3a69e146633470cf625ec")
-	stateDB := NewStateDBAdapter(sm, 1, true, false, hash.ZeroHash256)
+	stateDB := NewStateDBAdapter(sm, 1, true, false, true, hash.ZeroHash256)
 	require.Equal(uint64(0), stateDB.GetNonce(addr))
 	stateDB.SetNonce(addr, 1)
 	require.Equal(uint64(1), stateDB.GetNonce(addr))
 }
 
 func TestSnapshotRevertAndCommit(t *testing.T) {
-	testSnapshotAndRevert := func(cfg config.Config, t *testing.T) {
+	testSnapshotAndRevert := func(_ config.Config, t *testing.T, async, fixSnapshot bool) {
 		require := require.New(t)
 		ctrl := gomock.NewController(t)
 		defer ctrl.Finish()
 
 		sm, err := initMockStateManager(ctrl)
 		require.NoError(err)
-		stateDB := NewStateDBAdapter(sm, 1, true, false, hash.ZeroHash256)
+		stateDB := NewStateDBAdapter(sm, 1, true, async, fixSnapshot, hash.ZeroHash256)
 		tests := []stateDBTest{
 			{
 				[]bal{
@@ -354,26 +355,33 @@ func TestSnapshotRevertAndCommit(t *testing.T) {
 		// test revert
 		for i, test := range reverts {
 			stateDB.RevertToSnapshot(len(reverts) - 1 - i)
-
 			// test balance
 			for _, e := range test.balance {
 				amount := stateDB.GetBalance(e.addr)
 				require.Equal(e.v, amount)
 			}
-			// test states
-			for _, e := range test.states {
-				require.Equal(e.v, stateDB.GetState(e.addr, e.k))
+			if async && !fixSnapshot {
+				// test preimage
+				for _, e := range reverts[0].preimage {
+					v := stateDB.preimages[e.hash]
+					require.Equal(e.v, []byte(v))
+				}
+			} else {
+				// test states
+				for _, e := range test.states {
+					require.Equal(e.v, stateDB.GetState(e.addr, e.k))
+				}
+				// test preimage
+				for _, e := range test.preimage {
+					v := stateDB.preimages[e.hash]
+					require.Equal(e.v, []byte(v))
+				}
 			}
 			// test suicide/exist
 			for _, e := range test.suicide {
 				require.Equal(e.suicide, stateDB.HasSuicided(e.addr))
 				require.Equal(e.exist, stateDB.Exist(e.addr))
 			}
-			// test preimage
-			for _, e := range test.preimage {
-				v := stateDB.preimages[e.hash]
-				require.Equal(e.v, []byte(v))
-			}
 		}
 
 		// commit snapshot 0's state
@@ -382,9 +390,21 @@ func TestSnapshotRevertAndCommit(t *testing.T) {
 		//[TODO] need e2etest to verify state factory commit/re-open (whether result from state/balance/suicide/exist is same)
 	}
 
-	t.Run("contract snapshot/revert/commit with in memery DB", func(t *testing.T) {
+	t.Run("contract snapshot/revert/commit in memory DB", func(t *testing.T) {
+		cfg := config.Default
+		testSnapshotAndRevert(cfg, t, false, true)
+	})
+	t.Run("contract snapshot/revert/commit without bug fix in memory DB", func(t *testing.T) {
+		cfg := config.Default
+		testSnapshotAndRevert(cfg, t, false, true)
+	})
+	t.Run("contract snapshot/revert/commit with async trie in memory DB", func(t *testing.T) {
 		cfg := config.Default
-		testSnapshotAndRevert(cfg, t)
+		testSnapshotAndRevert(cfg, t, true, true)
+	})
+	t.Run("contract snapshot/revert/commit with async trie and without bug fix in memory DB", func(t *testing.T) {
+		cfg := config.Default
+		testSnapshotAndRevert(cfg, t, true, false)
 	})
 }
 
@@ -396,7 +416,7 @@ func TestGetCommittedState(t *testing.T) {
 
 		sm, err := initMockStateManager(ctrl)
 		require.NoError(err)
-		stateDB := NewStateDBAdapter(sm, 1, true, false, hash.ZeroHash256)
+		stateDB := NewStateDBAdapter(sm, 1, true, false, true, hash.ZeroHash256)
 
 		stateDB.SetState(c1, k1, v1)
 		// k2 does not exist
@@ -429,7 +449,7 @@ func TestGetBalanceOnError(t *testing.T) {
 	for _, err := range errs {
 		sm.EXPECT().State(gomock.Any(), gomock.Any()).Return(uint64(0), err).Times(1)
 		addr := common.HexToAddress("test address")
-		stateDB := NewStateDBAdapter(sm, 1, true, false, hash.ZeroHash256)
+		stateDB := NewStateDBAdapter(sm, 1, true, false, true, hash.ZeroHash256)
 		amount := stateDB.GetBalance(addr)
 		assert.Equal(t, big.NewInt(0), amount)
 	}
@@ -442,7 +462,7 @@ func TestPreimage(t *testing.T) {
 
 	sm, err := initMockStateManager(ctrl)
 	require.NoError(err)
-	stateDB := NewStateDBAdapter(sm, 1, true, false, hash.ZeroHash256)
+	stateDB := NewStateDBAdapter(sm, 1, true, false, true, hash.ZeroHash256)
 
 	stateDB.AddPreimage(common.BytesToHash(v1[:]), []byte("cat"))
 	stateDB.AddPreimage(common.BytesToHash(v2[:]), []byte("dog"))
@@ -462,3 +482,56 @@ func TestPreimage(t *testing.T) {
 	require.NoError(err)
 	require.Equal([]byte("hen"), []byte(k))
 }
+
+func TestSortMap(t *testing.T) {
+	uniqueSlice := func(slice []string) bool {
+		for _, v := range slice[1:] {
+			if v != slice[0] {
+				return false
+			}
+		}
+		return true
+	}
+
+	testFunc := func(t *testing.T, sm *mock_chainmanager.MockStateManager, opts ...StateDBAdapterOption) bool {
+		stateDB := NewStateDBAdapter(sm, 1, true, false, true, hash.ZeroHash256, opts...)
+		size := 10
+
+		for i := 0; i < size; i++ {
+			addr := common.HexToAddress(identityset.Address(i).Hex())
+			stateDB.SetCode(addr, []byte("0123456789"))
+			stateDB.SetState(addr, k1, k2)
+		}
+		sn := stateDB.Snapshot()
+		caches := []string{}
+		for i := 0; i < size; i++ {
+			stateDB.RevertToSnapshot(sn)
+			s := ""
+			if stateDB.sortCachedContracts {
+				for _, addr := range stateDB.cachedContractAddrs() {
+					c := stateDB.cachedContract[addr]
+					s += string(c.SelfState().Root[:])
+				}
+			} else {
+				for _, c := range stateDB.cachedContract {
+					s += string(c.SelfState().Root[:])
+				}
+			}
+
+			caches = append(caches, s)
+		}
+		return uniqueSlice(caches)
+	}
+	require := require.New(t)
+
+	ctrl := gomock.NewController(t)
+	sm, err := initMockStateManager(ctrl)
+	require.NoError(err)
+	t.Run("before fix sort map", func(t *testing.T) {
+		require.False(testFunc(t, sm))
+	})
+
+	t.Run("after fix sort map", func(t *testing.T) {
+		require.True(testFunc(t, sm, SortCachedContractsOption()))
+	})
+}