Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[evm] fix snapshot bug #2802

Merged
merged 3 commits into from
Sep 17, 2021
Merged

[evm] fix snapshot bug #2802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e1abb3c
fix snapshot bug
CoderZhi Sep 14, 2021
158e562
Merge branch 'master' into fix_snapshot_bug
raullenchai Sep 17, 2021
528f0a2
add unit test
CoderZhi Sep 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion action/protocol/execution/evm/contract_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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[:])
Expand Down
1 change: 1 addition & 0 deletions action/protocol/execution/evm/evm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ func ExecuteContract(
blkCtx.BlockHeight,
!g.IsAleutian(blkCtx.BlockHeight),
g.IsGreenland(blkCtx.BlockHeight),
g.IsKamchatka(blkCtx.BlockHeight),
actionCtx.ActionHash,
opts...,
)
Expand Down
2 changes: 1 addition & 1 deletion action/protocol/execution/evm/evm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 14 additions & 4 deletions action/protocol/execution/evm/evmstatedbadapter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ type (
preimageSnapshot map[int]preimageMap
notFixTopicCopyBug bool
asyncContractTrie bool
fixSnapshotOrder bool
sortCachedContracts bool
usePendingNonce bool
}
Expand Down Expand Up @@ -94,6 +95,7 @@ func NewStateDBAdapter(
blockHeight uint64,
notFixTopicCopyBug bool,
asyncContractTrie bool,
fixSnapshotOrder bool,
executionHash hash.Hash256,
opts ...StateDBAdapterOption,
) *StateDBAdapter {
Expand All @@ -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 {
Expand Down Expand Up @@ -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")
Expand All @@ -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
Expand Down
63 changes: 41 additions & 22 deletions action/protocol/execution/evm/evmstatedbadapter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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))
Expand All @@ -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"),
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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
Expand All @@ -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)
})
}

Expand All @@ -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
Expand Down Expand Up @@ -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)
}
Expand All @@ -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"))
Expand Down Expand Up @@ -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++ {
Expand Down