[evm] fix snapshot bug (#2802)

* fix snapshot bug

* add unit test

Co-authored-by: Raullen Chai <raullenchai@gmail.com>

action/protocol/execution/evm/contract_test.go

@@ -66,7 +66,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

@@ -170,7 +170,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 {
@@ -464,6 +467,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")
@@ -477,10 +487,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

@@ -93,7 +93,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)
@@ -109,7 +109,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)
@@ -123,7 +123,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))
@@ -139,7 +139,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"),
@@ -174,20 +174,20 @@ 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)
 
  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{
@@ -349,26 +349,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
@@ -377,9 +384,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, 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)
+ testSnapshotAndRevert(cfg, t, true, false)
  })
 }
 
@@ -390,7 +409,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
@@ -422,7 +441,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)
  }
@@ -434,7 +453,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"))
@@ -466,7 +485,7 @@ func TestSortMap(t *testing.T) {
  }
 
  testFunc := func(t *testing.T, sm *mock_chainmanager.MockStateManager, opts ...StateDBAdapterOption) bool {
- stateDB := NewStateDBAdapter(sm, 1, true, false, hash.ZeroHash256, opts...)
+ stateDB := NewStateDBAdapter(sm, 1, true, false, true, hash.ZeroHash256, opts...)
  size := 10
 
  for i := 0; i < size; i++ {