From e1abb3c887c0f344e8d66eb7f0b85a31a6d04728 Mon Sep 17 00:00:00 2001 From: zhi Date: Mon, 13 Sep 2021 17:00:40 -0700 Subject: [PATCH 1/2] fix snapshot bug --- .../protocol/execution/evm/contract_test.go | 2 +- action/protocol/execution/evm/evm.go | 1 + action/protocol/execution/evm/evm_test.go | 2 +- .../execution/evm/evmstatedbadapter.go | 18 ++++++++--- .../execution/evm/evmstatedbadapter_test.go | 30 +++++++++++-------- 5 files changed, 34 insertions(+), 19 deletions(-) diff --git a/action/protocol/execution/evm/contract_test.go b/action/protocol/execution/evm/contract_test.go index b51d40ea5e..de715c18b1 100644 --- a/action/protocol/execution/evm/contract_test.go +++ b/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[:]) diff --git a/action/protocol/execution/evm/evm.go b/action/protocol/execution/evm/evm.go index aa7c22d19a..a9ea3a2f53 100644 --- a/action/protocol/execution/evm/evm.go +++ b/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..., ) diff --git a/action/protocol/execution/evm/evm_test.go b/action/protocol/execution/evm/evm_test.go index 92cbabf1e8..f243460f35 100644 --- a/action/protocol/execution/evm/evm_test.go +++ b/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, diff --git a/action/protocol/execution/evm/evmstatedbadapter.go b/action/protocol/execution/evm/evmstatedbadapter.go index 7f47b77e82..75d4ede45c 100644 --- a/action/protocol/execution/evm/evmstatedbadapter.go +++ b/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 diff --git a/action/protocol/execution/evm/evmstatedbadapter_test.go b/action/protocol/execution/evm/evmstatedbadapter_test.go index b02a95fc1d..c0b1f37ff4 100644 --- a/action/protocol/execution/evm/evmstatedbadapter_test.go +++ b/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 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, true, hash.ZeroHash256) tests := []stateDBTest{ { []bal{ @@ -377,9 +377,13 @@ 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 with in memory DB", func(t *testing.T) { cfg := config.Default - testSnapshotAndRevert(cfg, t) + testSnapshotAndRevert(cfg, t, false) + }) + t.Run("contract snapshot/revert/commit with async trie in memory DB", func(t *testing.T) { + cfg := config.Default + testSnapshotAndRevert(cfg, t, true) }) } @@ -390,7 +394,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 +426,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 +438,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 +470,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++ { From 528f0a2638420d27bcd44be0611b300b3e180930 Mon Sep 17 00:00:00 2001 From: zhi Date: Fri, 17 Sep 2021 14:10:20 -0700 Subject: [PATCH 2/2] add unit test --- .../execution/evm/evmstatedbadapter_test.go | 43 +++++++++++++------ 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/action/protocol/execution/evm/evmstatedbadapter_test.go b/action/protocol/execution/evm/evmstatedbadapter_test.go index c0b1f37ff4..439f5fbf75 100644 --- a/action/protocol/execution/evm/evmstatedbadapter_test.go +++ b/action/protocol/execution/evm/evmstatedbadapter_test.go @@ -181,13 +181,13 @@ func TestNonce(t *testing.T) { } func TestSnapshotRevertAndCommit(t *testing.T) { - testSnapshotAndRevert := func(_ config.Config, t *testing.T, async bool) { + 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, async, true, 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,13 +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 memory 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) + 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) + 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) }) }