evm: fix non-determinism (#352)

* evm: fix non-determinism

* fixes

* typo

* fix tests

* use Storage slice

* more fixes

* lint

* merge development

* additional tests

tests/rpc_test.go

@@ -768,7 +768,7 @@ func TestEth_ExportAccount(t *testing.T) {
 	require.Equal(t, "0x1122334455667788990011223344556677889900", account.Address.Hex())
 	require.Equal(t, big.NewInt(0), account.Balance)
 	require.Equal(t, hexutil.Bytes(nil), account.Code)
-	require.Equal(t, []types.GenesisStorage(nil), account.Storage)
+	require.Equal(t, types.Storage(nil), account.Storage)
 }
 
 func TestEth_ExportAccount_WithStorage(t *testing.T) {
@@ -812,7 +812,7 @@ func TestEth_ExportAccount_WithStorage(t *testing.T) {
 	require.Equal(t, addr, strings.ToLower(account.Address.Hex()))
 	require.Equal(t, big.NewInt(0), account.Balance)
 	require.Equal(t, hexutil.Bytes(bytecode), account.Code)
-	require.NotEqual(t, []types.GenesisStorage(nil), account.Storage)
+	require.NotEqual(t, types.Storage(nil), account.Storage)
 }
 
 func TestEth_GetBlockByNumber(t *testing.T) {

types/code.go

@@ -1,40 +1,12 @@
 package types
 
-import (
-	"fmt"
-
-	ethcmn "github.com/ethereum/go-ethereum/common"
-)
-
 // ----------------------------------------------------------------------------
-// Code & Storage
+// Code
 // ----------------------------------------------------------------------------
 
-type (
+// Code is account Code type alias
-	// Code is account Code type alias
+type Code []byte
-	Code []byte
-	// Storage is account storage type alias
-	Storage map[ethcmn.Hash]ethcmn.Hash
-)
 
 func (c Code) String() string {
 	return string(c)
 }
-
-func (c Storage) String() (str string) {
-	for key, value := range c {
-		str += fmt.Sprintf("%X : %X\n", key, value)
-	}
-
-	return
-}
-
-// Copy returns a copy of storage.
-func (c Storage) Copy() Storage {
-	cpy := make(Storage)
-	for key, value := range c {
-		cpy[key] = value
-	}
-
-	return cpy
-}

x/evm/genesis.go

@@ -59,9 +59,9 @@ func ExportGenesis(ctx sdk.Context, k Keeper, ak types.AccountKeeper) GenesisSta
 
 		addr := common.BytesToAddress(ethAccount.GetAddress().Bytes())
 
-		var storage []types.GenesisStorage
+		var storage types.Storage
 		err = k.CommitStateDB.ForEachStorage(addr, func(key, value common.Hash) bool {
-			storage = append(storage, types.NewGenesisStorage(key, value))
+			storage = append(storage, types.NewState(key, value))
 			return false
 		})
 		if err != nil {

x/evm/genesis_test.go

@@ -44,10 +44,10 @@ func (suite *EvmTestSuite) TestContractExportImport() {
 	suite.T().Logf("contract addr 0x%x", address)
 
 	// clear keeper code and re-initialize
-	suite.app.EvmKeeper.CommitStateDB.WithContext(suite.ctx).SetCode(address, nil)
+	suite.app.EvmKeeper.SetCode(suite.ctx, address, nil)
 	_ = evm.InitGenesis(suite.ctx, suite.app.EvmKeeper, genState)
 
-	resCode := suite.app.EvmKeeper.CommitStateDB.WithContext(suite.ctx).GetCode(address)
+	resCode := suite.app.EvmKeeper.GetCode(suite.ctx, address)
 	suite.Require().Equal(deployedEnsFactoryCode, resCode)
 }
 

x/evm/keeper/querier.go

@@ -201,9 +201,9 @@ func queryAccount(ctx sdk.Context, path []string, keeper Keeper) ([]byte, error)
 func queryExportAccount(ctx sdk.Context, path []string, keeper Keeper) ([]byte, error) {
 	addr := ethcmn.HexToAddress(path[1])
 
-	var storage []types.GenesisStorage
+	var storage types.Storage
 	err := keeper.CommitStateDB.ForEachStorage(addr, func(key, value ethcmn.Hash) bool {
-		storage = append(storage, types.NewGenesisStorage(key, value))
+		storage = append(storage, types.NewState(key, value))
 		return false
 	})
 	if err != nil {

x/evm/types/errors.go

@@ -0,0 +1,12 @@
+package types
+
+import (
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+)
+
+// NOTE: We can't use 1 since that error code is reserved for internal errors.
+
+var (
+	// ErrInvalidState returns an error resulting from an invalid Storage State.
+	ErrInvalidState = sdkerrors.Register(ModuleName, 2, "invalid storage state")
+)

x/evm/types/genesis.go

@@ -17,13 +17,6 @@ type (
 		TxsLogs  []TransactionLogs `json:"txs_logs"`
 	}
 
-	// GenesisStorage represents the GenesisAccount Storage map as single key value
-	// pairs. This is to prevent non determinism at genesis initialization or export.
-	GenesisStorage struct {
-		Key   ethcmn.Hash `json:"key"`
-		Value ethcmn.Hash `json:"value"`
-	}
-
 	// GenesisAccount defines an account to be initialized in the genesis state.
 	// Its main difference between with Geth's GenesisAccount is that it uses a custom
 	// storage type and that it doesn't contain the private key field.
@@ -31,7 +24,7 @@ type (
 		Address ethcmn.Address `json:"address"`
 		Balance *big.Int       `json:"balance"`
 		Code    hexutil.Bytes  `json:"code,omitempty"`
-		Storage []GenesisStorage `json:"storage,omitempty"`
+		Storage Storage        `json:"storage,omitempty"`
 	}
 )
 
@@ -50,26 +43,7 @@ func (ga GenesisAccount) Validate() error {
 		return errors.New("code bytes cannot be empty")
 	}
 
-	seenStorage := make(map[string]bool)
+	return ga.Storage.Validate()
-	for i, state := range ga.Storage {
-		if seenStorage[state.Key.String()] {
-			return fmt.Errorf("duplicate state key %d", i)
-		}
-		if bytes.Equal(state.Key.Bytes(), ethcmn.Hash{}.Bytes()) {
-			return fmt.Errorf("state %d key hash cannot be empty", i)
-		}
-		// NOTE: state value can be empty
-		seenStorage[state.Key.String()] = true
-	}
-	return nil
-}
-
-// NewGenesisStorage creates a new GenesisStorage instance
-func NewGenesisStorage(key, value ethcmn.Hash) GenesisStorage {
-	return GenesisStorage{
-		Key:   key,
-		Value: value,
-	}
 }
 
 // DefaultGenesisState sets default evm genesis state with empty accounts.

x/evm/types/genesis_test.go

@@ -24,8 +24,8 @@ func TestValidateGenesisAccount(t *testing.T) {
 				Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}),
 				Balance: big.NewInt(1),
 				Code:    []byte{1, 2, 3},
-				Storage: []GenesisStorage{
+				Storage: Storage{
-					NewGenesisStorage(ethcmn.BytesToHash([]byte{1, 2, 3}), ethcmn.BytesToHash([]byte{1, 2, 3})),
+					NewState(ethcmn.BytesToHash([]byte{1, 2, 3}), ethcmn.BytesToHash([]byte{1, 2, 3})),
 				},
 			},
 			true,
@@ -63,31 +63,6 @@ func TestValidateGenesisAccount(t *testing.T) {
 			},
 			false,
 		},
-		{
-			"empty storage key bytes",
-			GenesisAccount{
-				Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}),
-				Balance: big.NewInt(1),
-				Code:    []byte{1, 2, 3},
-				Storage: []GenesisStorage{
-					{Key: ethcmn.Hash{}},
-				},
-			},
-			false,
-		},
-		{
-			"duplicated storage key",
-			GenesisAccount{
-				Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}),
-				Balance: big.NewInt(1),
-				Code:    []byte{1, 2, 3},
-				Storage: []GenesisStorage{
-					{Key: ethcmn.BytesToHash([]byte{1, 2, 3})},
-					{Key: ethcmn.BytesToHash([]byte{1, 2, 3})},
-				},
-			},
-			false,
-		},
 	}
 
 	for _, tc := range testCases {
@@ -124,7 +99,7 @@ func TestValidateGenesis(t *testing.T) {
 						Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}),
 						Balance: big.NewInt(1),
 						Code:    []byte{1, 2, 3},
-						Storage: []GenesisStorage{
+						Storage: Storage{
 							{Key: ethcmn.BytesToHash([]byte{1, 2, 3})},
 						},
 					},
@@ -169,16 +144,16 @@ func TestValidateGenesis(t *testing.T) {
 						Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}),
 						Balance: big.NewInt(1),
 						Code:    []byte{1, 2, 3},
-						Storage: []GenesisStorage{
+						Storage: Storage{
-							NewGenesisStorage(ethcmn.BytesToHash([]byte{1, 2, 3}), ethcmn.BytesToHash([]byte{1, 2, 3})),
+							NewState(ethcmn.BytesToHash([]byte{1, 2, 3}), ethcmn.BytesToHash([]byte{1, 2, 3})),
 						},
 					},
 					{
 						Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}),
 						Balance: big.NewInt(1),
 						Code:    []byte{1, 2, 3},
-						Storage: []GenesisStorage{
+						Storage: Storage{
-							NewGenesisStorage(ethcmn.BytesToHash([]byte{1, 2, 3}), ethcmn.BytesToHash([]byte{1, 2, 3})),
+							NewState(ethcmn.BytesToHash([]byte{1, 2, 3}), ethcmn.BytesToHash([]byte{1, 2, 3})),
 						},
 					},
 				},
@@ -193,7 +168,7 @@ func TestValidateGenesis(t *testing.T) {
 						Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}),
 						Balance: big.NewInt(1),
 						Code:    []byte{1, 2, 3},
-						Storage: []GenesisStorage{
+						Storage: Storage{
 							{Key: ethcmn.BytesToHash([]byte{1, 2, 3})},
 						},
 					},
@@ -243,7 +218,7 @@ func TestValidateGenesis(t *testing.T) {
 						Address: ethcmn.BytesToAddress([]byte{1, 2, 3, 4, 5}),
 						Balance: big.NewInt(1),
 						Code:    []byte{1, 2, 3},
-						Storage: []GenesisStorage{
+						Storage: Storage{
 							{Key: ethcmn.BytesToHash([]byte{1, 2, 3})},
 						},
 					},

x/evm/types/journal.go

@@ -23,13 +23,23 @@ type journalEntry interface {
 // exception or revertal request.
 type journal struct {
 	entries               []journalEntry         // Current changes tracked by the journal
-	dirties map[ethcmn.Address]int // Dirty accounts and the number of changes
+	dirties               []dirty                // Dirty accounts and the number of changes
+	addressToJournalIndex map[ethcmn.Address]int // map from address to the index of the dirties slice
+}
+
+// dirty represents a single key value pair of the journal dirties, where the
+// key correspons to the account address and the value to the number of
+// changes for that account.
+type dirty struct {
+	address ethcmn.Address
+	changes int
 }
 
 // newJournal create a new initialized journal.
 func newJournal() *journal {
 	return &journal{
-		dirties: make(map[ethcmn.Address]int),
+		dirties:               []dirty{},
+		addressToJournalIndex: make(map[ethcmn.Address]int),
 	}
 }
 
@@ -37,7 +47,7 @@ func newJournal() *journal {
 func (j *journal) append(entry journalEntry) {
 	j.entries = append(j.entries, entry)
 	if addr := entry.dirtied(); addr != nil {
-		j.dirties[*addr]++
+		j.addDirty(*addr)
 	}
 }
 
@@ -50,8 +60,9 @@ func (j *journal) revert(statedb *CommitStateDB, snapshot int) {
 
 		// Drop any dirty tracking induced by the change
 		if addr := j.entries[i].dirtied(); addr != nil {
-			if j.dirties[*addr]--; j.dirties[*addr] == 0 {
+			j.substractDirty(*addr)
-				delete(j.dirties, *addr)
+			if j.getDirty(*addr) == 0 {
+				j.deleteDirty(*addr)
 			}
 		}
 	}
@@ -62,7 +73,7 @@ func (j *journal) revert(statedb *CommitStateDB, snapshot int) {
 // otherwise suggest it as clean. This method is an ugly hack to handle the RIPEMD
 // precompile consensus exception.
 func (j *journal) dirty(addr ethcmn.Address) {
-	j.dirties[addr]++
+	j.addDirty(addr)
 }
 
 // length returns the current number of entries in the journal.
@@ -70,6 +81,56 @@ func (j *journal) length() int {
 	return len(j.entries)
 }
 
+// getDirty returns the dirty count for a given address. If the address is not
+// found it returns 0.
+func (j *journal) getDirty(addr ethcmn.Address) int {
+	idx, found := j.addressToJournalIndex[addr]
+	if !found {
+		return 0
+	}
+
+	return j.dirties[idx].changes
+}
+
+// addDirty adds 1 to the dirty count of an address. If the dirty entry is not
+// found it creates it.
+func (j *journal) addDirty(addr ethcmn.Address) {
+	idx, found := j.addressToJournalIndex[addr]
+	if !found {
+		j.dirties = append(j.dirties, dirty{address: addr, changes: 0})
+		idx = len(j.dirties) - 1
+		j.addressToJournalIndex[addr] = idx
+	}
+
+	j.dirties[idx].changes++
+}
+
+// substractDirty subtracts 1 to the dirty count of an address. It performs a
+// no-op if the address is not found.
+func (j *journal) substractDirty(addr ethcmn.Address) {
+	idx, found := j.addressToJournalIndex[addr]
+	if !found {
+		return
+	}
+
+	if j.dirties[idx].changes == 0 {
+		return
+	}
+	j.dirties[idx].changes--
+}
+
+// deleteDirty deletes a dirty entry from the jounal's dirties slice. If the
+// entry is not found it performs a no-op.
+func (j *journal) deleteDirty(addr ethcmn.Address) {
+	idx, found := j.addressToJournalIndex[addr]
+	if !found {
+		return
+	}
+
+	j.dirties = append(j.dirties[:idx], j.dirties[idx+1:]...)
+	delete(j.addressToJournalIndex, addr)
+}
+
 type (
 	// Changes to the account trie.
 	createObjectChange struct {
@@ -128,8 +189,14 @@ type (
 )
 
 func (ch createObjectChange) revert(s *CommitStateDB) {
-	delete(s.stateObjects, *ch.account)
 	delete(s.stateObjectsDirty, *ch.account)
+	idx, exists := s.addressToObjectIndex[*ch.account]
+	if !exists {
+		// perform no-op
+		return
+	}
+	// remove from the slice
+	s.stateObjects = append(s.stateObjects[:idx], s.stateObjects[idx+1:]...)
 }
 
 func (ch createObjectChange) dirtied() *ethcmn.Address {

x/evm/types/journal_test.go

@@ -228,7 +228,8 @@ func (suite *JournalTestSuite) TestJournal_append_revert() {
 		suite.Require().Equal(suite.journal.length(), i+1, tc.name)
 		if tc.entry.dirtied() != nil {
 			dirtyCount++
-			suite.Require().Equal(dirtyCount, suite.journal.dirties[suite.address], tc.name)
+
+			suite.Require().Equal(dirtyCount, suite.journal.getDirty(suite.address), tc.name)
 		}
 	}
 
@@ -236,18 +237,18 @@ func (suite *JournalTestSuite) TestJournal_append_revert() {
 	suite.journal.revert(suite.stateDB, 0)
 
 	// verify the dirty entry has been deleted
-	count, ok := suite.journal.dirties[suite.address]
+	idx, ok := suite.journal.addressToJournalIndex[suite.address]
 	suite.Require().False(ok)
-	suite.Require().Zero(count)
+	suite.Require().Zero(idx)
 }
 
 func (suite *JournalTestSuite) TestJournal_dirty() {
 	// dirty entry hasn't been set
-	count, ok := suite.journal.dirties[suite.address]
+	idx, ok := suite.journal.addressToJournalIndex[suite.address]
 	suite.Require().False(ok)
-	suite.Require().Zero(count)
+	suite.Require().Zero(idx)
 
 	// update dirty count
 	suite.journal.dirty(suite.address)
-	suite.Require().Equal(1, suite.journal.dirties[suite.address])
+	suite.Require().Equal(1, suite.journal.getDirty(suite.address))
 }

x/evm/types/state_object.go

@@ -24,9 +24,8 @@ var (
 	emptyCodeHash = ethcrypto.Keccak256(nil)
 )
 
-type (
+// StateObject interface for interacting with state object
-	// StateObject interface for interacting with state object
+type StateObject interface {
-	StateObject interface {
 	GetCommittedState(db ethstate.Database, key ethcmn.Hash) ethcmn.Hash
 	GetState(db ethstate.Database, key ethcmn.Hash) ethcmn.Hash
 	SetState(db ethstate.Database, key, value ethcmn.Hash)
@@ -45,27 +44,35 @@ type (
 
 	SetNonce(nonce uint64)
 	Nonce() uint64
-	}
+}
-	// stateObject represents an Ethereum account which is being modified.
+
-	//
+// stateObject represents an Ethereum account which is being modified.
-	// The usage pattern is as follows:
+//
-	// First you need to obtain a state object.
+// The usage pattern is as follows:
-	// Account values can be accessed and modified through the object.
+// First you need to obtain a state object.
-	// Finally, call CommitTrie to write the modified storage trie into a database.
+// Account values can be accessed and modified through the object.
-	stateObject struct {
+// Finally, call CommitTrie to write the modified storage trie into a database.
+type stateObject struct {
 	code types.Code // contract bytecode, which gets set when code is loaded
-		// DB error.
+
 	// State objects are used by the consensus core and VM which are
 	// unable to deal with database-level errors. Any error that occurs
 	// during a database read is memoized here and will eventually be returned
 	// by StateDB.Commit.
+	originStorage Storage // Storage cache of original entries to dedup rewrites
+	dirtyStorage  Storage // Storage entries that need to be flushed to disk
+
+	// DB error
 	dbErr   error
 	stateDB *CommitStateDB
 	account *types.EthAccount
 	balance sdk.Int
-		originStorage types.Storage // Storage cache of original entries to dedup rewrites
+
-		dirtyStorage  types.Storage // Storage entries that need to be flushed to disk
+	keyToOriginStorageIndex map[ethcmn.Hash]int
+	keyToDirtyStorageIndex  map[ethcmn.Hash]int
+
 	address ethcmn.Address
+
 	// cache flags
 	//
 	// When an object is marked suicided it will be delete from the trie during
@@ -73,8 +80,7 @@ type (
 	dirtyCode bool // true if the code was updated
 	suicided  bool
 	deleted   bool
-	}
+}
-)
 
 func newStateObject(db *CommitStateDB, accProto authexported.Account, balance sdk.Int) *stateObject {
 	ethermintAccount, ok := accProto.(*types.EthAccount)
@@ -92,8 +98,10 @@ func newStateObject(db *CommitStateDB, accProto authexported.Account, balance sd
 		account:                 ethermintAccount,
 		balance:                 balance,
 		address:                 ethcmn.BytesToAddress(ethermintAccount.GetAddress().Bytes()),
-		originStorage: make(types.Storage),
+		originStorage:           Storage{},
-		dirtyStorage:  make(types.Storage),
+		dirtyStorage:            Storage{},
+		keyToOriginStorageIndex: make(map[ethcmn.Hash]int),
+		keyToDirtyStorageIndex:  make(map[ethcmn.Hash]int),
 	}
 }
 
@@ -122,8 +130,18 @@ func (so *stateObject) SetState(db ethstate.Database, key, value ethcmn.Hash) {
 	so.setState(prefixKey, value)
 }
 
+// setState sets a state with a prefixed key and value to the dirty storage.
 func (so *stateObject) setState(key, value ethcmn.Hash) {
-	so.dirtyStorage[key] = value
+	idx, ok := so.keyToDirtyStorageIndex[key]
+	if ok {
+		so.dirtyStorage[idx].Value = value
+		return
+	}
+
+	// create new entry
+	so.dirtyStorage = append(so.dirtyStorage, NewState(key, value))
+	idx = len(so.dirtyStorage) - 1
+	so.keyToDirtyStorageIndex[key] = idx
 }
 
 // SetCode sets the state object's code.
@@ -224,23 +242,31 @@ func (so *stateObject) commitState() {
 	ctx := so.stateDB.ctx
 	store := prefix.NewStore(ctx.KVStore(so.stateDB.storeKey), AddressStoragePrefix(so.Address()))
 
-	for key, value := range so.dirtyStorage {
+	for i, state := range so.dirtyStorage {
-		delete(so.dirtyStorage, key)
+		delete(so.keyToDirtyStorageIndex, state.Key)
+		so.dirtyStorage = append(so.dirtyStorage[:i], so.dirtyStorage[i+1:]...)
 
 		// skip no-op changes, persist actual changes
-		if value == so.originStorage[key] {
+		idx, ok := so.keyToOriginStorageIndex[state.Key]
+		if !ok {
 			continue
 		}
 
-		so.originStorage[key] = value
+		if state.Value == so.originStorage[idx].Value {
-
-		// delete empty values
-		if (value == ethcmn.Hash{}) {
-			store.Delete(key.Bytes())
 			continue
 		}
 
-		store.Set(key.Bytes(), value.Bytes())
+		so.originStorage[idx].Value = state.Value
+
+		// NOTE: key is already prefixed from GetStorageByAddressKey
+
+		// delete empty values from the store
+		if (state.Value == ethcmn.Hash{}) {
+			store.Delete(state.Key.Bytes())
+			continue
+		}
+
+		store.Set(state.Key.Bytes(), state.Value.Bytes())
 	}
 }
 
@@ -312,9 +338,9 @@ func (so *stateObject) GetState(db ethstate.Database, key ethcmn.Hash) ethcmn.Ha
 	prefixKey := so.GetStorageByAddressKey(key.Bytes())
 
 	// if we have a dirty value for this state entry, return it
-	value, dirty := so.dirtyStorage[prefixKey]
+	idx, dirty := so.keyToDirtyStorageIndex[prefixKey]
 	if dirty {
-		return value
+		return so.dirtyStorage[idx].Value
 	}
 
 	// otherwise return the entry's original value
@@ -322,27 +348,35 @@ func (so *stateObject) GetState(db ethstate.Database, key ethcmn.Hash) ethcmn.Ha
 }
 
 // GetCommittedState retrieves a value from the committed account storage trie.
-// Note, the key will be prefixed with the address of the state object.
+//
+// NOTE: the key will be prefixed with the address of the state object.
 func (so *stateObject) GetCommittedState(_ ethstate.Database, key ethcmn.Hash) ethcmn.Hash {
 	prefixKey := so.GetStorageByAddressKey(key.Bytes())
 
 	// if we have the original value cached, return that
-	value, cached := so.originStorage[prefixKey]
+	idx, cached := so.keyToOriginStorageIndex[prefixKey]
 	if cached {
-		return value
+		return so.originStorage[idx].Value
 	}
 
+	if len(so.originStorage) == 0 {
+		so.originStorage = append(so.originStorage, NewState(prefixKey, ethcmn.Hash{}))
+		so.keyToOriginStorageIndex[prefixKey] = len(so.originStorage) - 1
+	}
+
+	state := so.originStorage[idx]
+
 	// otherwise load the value from the KVStore
 	ctx := so.stateDB.ctx
 	store := prefix.NewStore(ctx.KVStore(so.stateDB.storeKey), AddressStoragePrefix(so.Address()))
 	rawValue := store.Get(prefixKey.Bytes())
 
 	if len(rawValue) > 0 {
-		value.SetBytes(rawValue)
+		state.Value.SetBytes(rawValue)
 	}
 
-	so.originStorage[prefixKey] = value
+	so.originStorage[idx] = state
-	return value
+	return state.Value
 }
 
 // ----------------------------------------------------------------------------
@@ -403,3 +437,11 @@ func (so stateObject) GetStorageByAddressKey(key []byte) ethcmn.Hash {
 
 	return ethcrypto.Keccak256Hash(compositeKey)
 }
+
+// stateEntry represents a single key value pair from the StateDB's stateObject mappindg.
+// This is to prevent non determinism at genesis initialization or export.
+type stateEntry struct {
+	// address key of the state object
+	address     ethcmn.Address
+	stateObject *stateObject
+}

x/evm/types/state_object_test.go

@@ -35,6 +35,14 @@ func (suite *StateDBTestSuite) TestStateObject_State() {
 				suite.stateObject.SetState(nil, ethcmn.BytesToHash([]byte("key1")), ethcmn.BytesToHash([]byte("value1")))
 			},
 		},
+		{
+			"update value",
+			ethcmn.BytesToHash([]byte("key1")),
+			ethcmn.BytesToHash([]byte("value2")),
+			func() {
+				suite.stateObject.SetState(nil, ethcmn.BytesToHash([]byte("key1")), ethcmn.BytesToHash([]byte("value2")))
+			},
+		},
 	}
 
 	for _, tc := range testCase {

x/evm/types/statedb.go

@@ -16,6 +16,7 @@ import (
 	ethtypes "github.com/ethereum/go-ethereum/core/types"
 	ethvm "github.com/ethereum/go-ethereum/core/vm"
 	ethcrypto "github.com/ethereum/go-ethereum/crypto"
+	"github.com/ethereum/go-ethereum/rlp"
 )
 
 var (
@@ -45,9 +46,10 @@ type CommitStateDB struct {
 	accountKeeper AccountKeeper
 	bankKeeper    BankKeeper
 
-	// maps that hold 'live' objects, which will get modified while processing a
+	// array that hold 'live' objects, which will get modified while processing a
 	// state transition
-	stateObjects      map[ethcmn.Address]*stateObject
+	stateObjects         []stateEntry
+	addressToObjectIndex map[ethcmn.Address]int // map from address to the index of the state objects slice
 	stateObjectsDirty    map[ethcmn.Address]struct{}
 
 	// The refund counter, also used by state transitioning.
@@ -59,6 +61,8 @@ type CommitStateDB struct {
 
 	// TODO: Determine if we actually need this as we do not need preimages in
 	// the SDK, but it seems to be used elsewhere in Geth.
+	//
+	// NOTE: it is safe to use map here because it's only used for Copy
 	preimages map[ethcmn.Hash][]byte
 
 	// DB error.
@@ -91,7 +95,8 @@ func NewCommitStateDB(
 		storeKey:             storeKey,
 		accountKeeper:        ak,
 		bankKeeper:           bk,
-		stateObjects:      make(map[ethcmn.Address]*stateObject),
+		stateObjects:         []stateEntry{},
+		addressToObjectIndex: make(map[ethcmn.Address]int),
 		stateObjectsDirty:    make(map[ethcmn.Address]struct{}),
 		preimages:            make(map[ethcmn.Hash][]byte),
 		journal:              newJournal(),
@@ -389,34 +394,34 @@ func (csdb *CommitStateDB) Commit(deleteEmptyObjects bool) (ethcmn.Hash, error)
 	defer csdb.clearJournalAndRefund()
 
 	// remove dirty state object entries based on the journal
-	for addr := range csdb.journal.dirties {
+	for _, dirty := range csdb.journal.dirties {
-		csdb.stateObjectsDirty[addr] = struct{}{}
+		csdb.stateObjectsDirty[dirty.address] = struct{}{}
 	}
 
 	// set the state objects
-	for addr, so := range csdb.stateObjects {
+	for _, stateEntry := range csdb.stateObjects {
-		_, isDirty := csdb.stateObjectsDirty[addr]
+		_, isDirty := csdb.stateObjectsDirty[stateEntry.address]
 
 		switch {
-		case so.suicided || (isDirty && deleteEmptyObjects && so.empty()):
+		case stateEntry.stateObject.suicided || (isDirty && deleteEmptyObjects && stateEntry.stateObject.empty()):
 			// If the state object has been removed, don't bother syncing it and just
 			// remove it from the store.
-			csdb.deleteStateObject(so)
+			csdb.deleteStateObject(stateEntry.stateObject)
 
 		case isDirty:
 			// write any contract code associated with the state object
-			if so.code != nil && so.dirtyCode {
+			if stateEntry.stateObject.code != nil && stateEntry.stateObject.dirtyCode {
-				so.commitCode()
+				stateEntry.stateObject.commitCode()
-				so.dirtyCode = false
+				stateEntry.stateObject.dirtyCode = false
 			}
 
 			// update the object in the KVStore
-			if err := csdb.updateStateObject(so); err != nil {
+			if err := csdb.updateStateObject(stateEntry.stateObject); err != nil {
 				return ethcmn.Hash{}, err
 			}
 		}
 
-		delete(csdb.stateObjectsDirty, addr)
+		delete(csdb.stateObjectsDirty, stateEntry.address)
 	}
 
 	// NOTE: Ethereum returns the trie merkle root here, but as commitment
@@ -429,8 +434,8 @@ func (csdb *CommitStateDB) Commit(deleteEmptyObjects bool) (ethcmn.Hash, error)
 // removing the csdb destructed objects and clearing the journal as well as the
 // refunds.
 func (csdb *CommitStateDB) Finalise(deleteEmptyObjects bool) error {
-	for addr := range csdb.journal.dirties {
+	for _, dirty := range csdb.journal.dirties {
-		so, exist := csdb.stateObjects[addr]
+		idx, exist := csdb.addressToObjectIndex[dirty.address]
 		if !exist {
 			// ripeMD is 'touched' at block 1714175, in tx:
 			// 0x1237f737031e40bcde4a8b7e717b2d15e3ecadfe49bb1bbc71ee9deb09c6fcf2
@@ -444,18 +449,19 @@ func (csdb *CommitStateDB) Finalise(deleteEmptyObjects bool) error {
 			continue
 		}
 
-		if so.suicided || (deleteEmptyObjects && so.empty()) {
+		stateEntry := csdb.stateObjects[idx]
-			csdb.deleteStateObject(so)
+		if stateEntry.stateObject.suicided || (deleteEmptyObjects && stateEntry.stateObject.empty()) {
+			csdb.deleteStateObject(stateEntry.stateObject)
 		} else {
 			// Set all the dirty state storage items for the state object in the
 			// KVStore and finally set the account in the account mapper.
-			so.commitState()
+			stateEntry.stateObject.commitState()
-			if err := csdb.updateStateObject(so); err != nil {
+			if err := csdb.updateStateObject(stateEntry.stateObject); err != nil {
 				return err
 			}
 		}
 
-		csdb.stateObjectsDirty[addr] = struct{}{}
+		csdb.stateObjectsDirty[dirty.address] = struct{}{}
 	}
 
 	// invalidate journal because reverting across transactions is not allowed
@@ -587,7 +593,8 @@ func (csdb *CommitStateDB) Suicide(addr ethcmn.Address) bool {
 // the underlying account mapper and store keys to avoid reloading data for the
 // next operations.
 func (csdb *CommitStateDB) Reset(_ ethcmn.Hash) error {
-	csdb.stateObjects = make(map[ethcmn.Address]*stateObject)
+	csdb.stateObjects = []stateEntry{}
+	csdb.addressToObjectIndex = make(map[ethcmn.Address]int)
 	csdb.stateObjectsDirty = make(map[ethcmn.Address]struct{})
 	csdb.thash = ethcmn.Hash{}
 	csdb.bhash = ethcmn.Hash{}
@@ -601,27 +608,28 @@ func (csdb *CommitStateDB) Reset(_ ethcmn.Hash) error {
 
 // UpdateAccounts updates the nonce and coin balances of accounts
 func (csdb *CommitStateDB) UpdateAccounts() {
-	for addr, so := range csdb.stateObjects {
+	for _, stateEntry := range csdb.stateObjects {
-		currAcc := csdb.accountKeeper.GetAccount(csdb.ctx, sdk.AccAddress(addr.Bytes()))
+		currAcc := csdb.accountKeeper.GetAccount(csdb.ctx, sdk.AccAddress(stateEntry.address.Bytes()))
 		emintAcc, ok := currAcc.(*emint.EthAccount)
 		if !ok {
 			continue
 		}
 
 		balance := csdb.bankKeeper.GetBalance(csdb.ctx, emintAcc.GetAddress(), emint.DenomDefault)
-		if so.Balance() != balance.Amount.BigInt() && balance.IsValid() {
+		if stateEntry.stateObject.Balance() != balance.Amount.BigInt() && balance.IsValid() {
-			so.balance = balance.Amount
+			stateEntry.stateObject.balance = balance.Amount
 		}
 
-		if so.Nonce() != emintAcc.GetSequence() {
+		if stateEntry.stateObject.Nonce() != emintAcc.GetSequence() {
-			so.account = emintAcc
+			stateEntry.stateObject.account = emintAcc
 		}
 	}
 }
 
 // ClearStateObjects clears cache of state objects to handle account changes outside of the EVM
 func (csdb *CommitStateDB) ClearStateObjects() {
-	csdb.stateObjects = make(map[ethcmn.Address]*stateObject)
+	csdb.stateObjects = []stateEntry{}
+	csdb.addressToObjectIndex = make(map[ethcmn.Address]int)
 	csdb.stateObjectsDirty = make(map[ethcmn.Address]struct{})
 }
 
@@ -669,7 +677,8 @@ func (csdb *CommitStateDB) Copy() *CommitStateDB {
 		storeKey:             csdb.storeKey,
 		accountKeeper:        csdb.accountKeeper,
 		bankKeeper:           csdb.bankKeeper,
-		stateObjects:      make(map[ethcmn.Address]*stateObject, len(csdb.journal.dirties)),
+		stateObjects:         make([]stateEntry, len(csdb.journal.dirties)),
+		addressToObjectIndex: make(map[ethcmn.Address]int, len(csdb.journal.dirties)),
 		stateObjectsDirty:    make(map[ethcmn.Address]struct{}, len(csdb.journal.dirties)),
 		refund:               csdb.refund,
 		logSize:              csdb.logSize,
@@ -678,15 +687,18 @@ func (csdb *CommitStateDB) Copy() *CommitStateDB {
 	}
 
 	// copy the dirty states, logs, and preimages
-	for addr := range csdb.journal.dirties {
+	for _, dirty := range csdb.journal.dirties {
 		// There is a case where an object is in the journal but not in the
 		// stateObjects: OOG after touch on ripeMD prior to Byzantium. Thus, we
 		// need to check for nil.
 		//
 		// Ref: https://github.com/ethereum/go-ethereum/pull/16485#issuecomment-380438527
-		if object, exist := csdb.stateObjects[addr]; exist {
+		if idx, exist := csdb.addressToObjectIndex[dirty.address]; exist {
-			state.stateObjects[addr] = object.deepCopy(state)
+			state.stateObjects[idx] = stateEntry{
-			state.stateObjectsDirty[addr] = struct{}{}
+				address:     dirty.address,
+				stateObject: csdb.stateObjects[idx].stateObject.deepCopy(state),
+			}
+			state.stateObjectsDirty[dirty.address] = struct{}{}
 		}
 	}
 
@@ -694,8 +706,8 @@ func (csdb *CommitStateDB) Copy() *CommitStateDB {
 	// copied, the loop above will be a no-op, since the copy's journal is empty.
 	// Thus, here we iterate over stateObjects, to enable copies of copies.
 	for addr := range csdb.stateObjectsDirty {
-		if _, exist := state.stateObjects[addr]; !exist {
+		if idx, exist := state.addressToObjectIndex[addr]; !exist {
-			state.stateObjects[addr] = csdb.stateObjects[addr].deepCopy(state)
+			state.setStateObject(csdb.stateObjects[idx].stateObject.deepCopy(state))
 			state.stateObjectsDirty[addr] = struct{}{}
 		}
 	}
@@ -708,9 +720,9 @@ func (csdb *CommitStateDB) Copy() *CommitStateDB {
 	return state
 }
 
-// ForEachStorage iterates over each storage items, all invokes the provided
+// ForEachStorage iterates over each storage items, all invoke the provided
-// callback on each key, value pair .
+// callback on each key, value pair.
-func (csdb *CommitStateDB) ForEachStorage(addr ethcmn.Address, cb func(key, value ethcmn.Hash) bool) error {
+func (csdb *CommitStateDB) ForEachStorage(addr ethcmn.Address, cb func(key, value ethcmn.Hash) (stop bool)) error {
 	so := csdb.getStateObject(addr)
 	if so == nil {
 		return nil
@@ -725,18 +737,23 @@ func (csdb *CommitStateDB) ForEachStorage(addr ethcmn.Address, cb func(key, valu
 		key := ethcmn.BytesToHash(iterator.Key())
 		value := ethcmn.BytesToHash(iterator.Value())
 
-		if value, dirty := so.dirtyStorage[key]; dirty {
+		if idx, dirty := so.keyToDirtyStorageIndex[key]; dirty {
 			// check if iteration stops
-			if cb(key, value) {
+			if cb(key, so.dirtyStorage[idx].Value) {
 				break
 			}
 
 			continue
 		}
 
+		_, content, _, err := rlp.Split(value.Bytes())
+		if err != nil {
+			return err
+		}
+
 		// check if iteration stops
-		if cb(key, value) {
+		if cb(key, ethcmn.BytesToHash(content)) {
-			break
+			return nil
 		}
 	}
 
@@ -784,14 +801,17 @@ func (csdb *CommitStateDB) setError(err error) {
 // getStateObject attempts to retrieve a state object given by the address.
 // Returns nil and sets an error if not found.
 func (csdb *CommitStateDB) getStateObject(addr ethcmn.Address) (stateObject *stateObject) {
+	idx, found := csdb.addressToObjectIndex[addr]
+	if found {
 		// prefer 'live' (cached) objects
-	if so := csdb.stateObjects[addr]; so != nil {
+		if so := csdb.stateObjects[idx].stateObject; so != nil {
 			if so.deleted {
 				return nil
 			}
 
 			return so
 		}
+	}
 
 	// otherwise, attempt to fetch the account from the account mapper
 	acc := csdb.accountKeeper.GetAccount(csdb.ctx, sdk.AccAddress(addr.Bytes()))
@@ -810,7 +830,21 @@ func (csdb *CommitStateDB) getStateObject(addr ethcmn.Address) (stateObject *sta
 }
 
 func (csdb *CommitStateDB) setStateObject(so *stateObject) {
-	csdb.stateObjects[so.Address()] = so
+	idx, found := csdb.addressToObjectIndex[so.Address()]
+	if found {
+		// update the existing object
+		csdb.stateObjects[idx].stateObject = so
+		return
+	}
+
+	// append the new state object to the stateObjects slice
+	se := stateEntry{
+		address:     so.Address(),
+		stateObject: so,
+	}
+
+	csdb.stateObjects = append(csdb.stateObjects, se)
+	csdb.addressToObjectIndex[se.address] = len(csdb.stateObjects) - 1
 }
 
 // RawDump returns a raw state dump.

x/evm/types/statedb_test.go

@@ -404,6 +404,13 @@ func (suite *StateDBTestSuite) TestCommitStateDB_Finalize() {
 			},
 			false, true,
 		},
+		{
+			"finalize, dirty storage",
+			func() {
+				suite.stateDB.SetState(suite.address, ethcmn.BytesToHash([]byte("key")), ethcmn.BytesToHash([]byte("value")))
+			},
+			false, true,
+		},
 		{
 			"faled to update state object",
 			func() {

x/evm/types/storage.go

@@ -0,0 +1,72 @@
+package types
+
+import (
+	"bytes"
+	"fmt"
+
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+
+	ethcmn "github.com/ethereum/go-ethereum/common"
+)
+
+// Storage represents the account Storage map as a slice of single key value
+// State pairs. This is to prevent non determinism at genesis initialization or export.
+type Storage []State
+
+// Validate performs a basic validation of the Storage fields.
+func (s Storage) Validate() error {
+	seenStorage := make(map[string]bool)
+	for i, state := range s {
+		if seenStorage[state.Key.String()] {
+			return sdkerrors.Wrapf(ErrInvalidState, "duplicate state key %d", i)
+		}
+
+		if err := state.Validate(); err != nil {
+			return err
+		}
+
+		seenStorage[state.Key.String()] = true
+	}
+	return nil
+}
+
+// String implements the stringer interface
+func (s Storage) String() string {
+	var str string
+	for _, state := range s {
+		str += fmt.Sprintf("%s: %s\n", state.Key.String(), state.Value.String())
+	}
+
+	return str
+}
+
+// Copy returns a copy of storage.
+func (s Storage) Copy() Storage {
+	cpy := make(Storage, len(s))
+	copy(cpy, s)
+
+	return cpy
+}
+
+// State represents a single Storage key value pair item.
+type State struct {
+	Key   ethcmn.Hash `json:"key"`
+	Value ethcmn.Hash `json:"value"`
+}
+
+// Validate performs a basic validation of the State fields.
+func (s State) Validate() error {
+	if bytes.Equal(s.Key.Bytes(), ethcmn.Hash{}.Bytes()) {
+		return sdkerrors.Wrap(ErrInvalidState, "state key hash cannot be empty")
+	}
+	// NOTE: state value can be empty
+	return nil
+}
+
+// NewState creates a new State instance
+func NewState(key, value ethcmn.Hash) State {
+	return State{
+		Key:   key,
+		Value: value,
+	}
+}

x/evm/types/storage_test.go

@@ -0,0 +1,84 @@
+package types
+
+import (
+	"testing"
+
+	ethcmn "github.com/ethereum/go-ethereum/common"
+	"github.com/stretchr/testify/require"
+)
+
+func TestStorageValidate(t *testing.T) {
+	testCases := []struct {
+		name    string
+		storage Storage
+		expPass bool
+	}{
+		{
+			"valid storage",
+			Storage{
+				NewState(ethcmn.BytesToHash([]byte{1, 2, 3}), ethcmn.BytesToHash([]byte{1, 2, 3})),
+			},
+			true,
+		},
+		{
+			"empty storage key bytes",
+			Storage{
+				{Key: ethcmn.Hash{}},
+			},
+			false,
+		},
+		{
+			"duplicated storage key",
+			Storage{
+				{Key: ethcmn.BytesToHash([]byte{1, 2, 3})},
+				{Key: ethcmn.BytesToHash([]byte{1, 2, 3})},
+			},
+			false,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		err := tc.storage.Validate()
+		if tc.expPass {
+			require.NoError(t, err, tc.name)
+		} else {
+			require.Error(t, err, tc.name)
+		}
+	}
+}
+
+func TestStorageCopy(t *testing.T) {
+	testCases := []struct {
+		name    string
+		storage Storage
+	}{
+		{
+			"single storage",
+			Storage{
+				NewState(ethcmn.BytesToHash([]byte{1, 2, 3}), ethcmn.BytesToHash([]byte{1, 2, 3})),
+			},
+		},
+		{
+			"empty storage key value bytes",
+			Storage{
+				{Key: ethcmn.Hash{}, Value: ethcmn.Hash{}},
+			},
+		},
+		{
+			"empty storage",
+			Storage{},
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		require.Equal(t, tc.storage, tc.storage.Copy(), tc.name)
+	}
+}
+
+func TestStorageString(t *testing.T) {
+	storage := Storage{NewState(ethcmn.BytesToHash([]byte("key")), ethcmn.BytesToHash([]byte("value")))}
+	str := "0x00000000000000000000000000000000000000000000000000000000006b6579: 0x00000000000000000000000000000000000000000000000000000076616c7565\n"
+	require.Equal(t, str, storage.String())
+}