types, evm: refactor accounts (#884)

* types,evm: refactor accounts

* fix

* fix panic

* changelog

* fix

* lint, rm dbErr

CHANGELOG.md

@@ -56,6 +56,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+* (types) [tharsis#884](https://github.com/tharsis/ethermint/pull/884) Introduce a new `EthAccountI` interface for EVM-compatible account types.
 * (types) [tharsis#849](https://github.com/tharsis/ethermint/pull/849) Add `Type` function to distinguish EOAs from Contract accounts.
 * (evm) [tharsis#826](https://github.com/tharsis/ethermint/issues/826) Improve allocation of bytes of `tx.To` address.
 * (evm) [tharsis#827](https://github.com/tharsis/ethermint/issues/827) Speed up creation of event logs by using the slice insertion idiom with indices.
@@ -64,6 +65,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Bug Fixes
 
+* (evm) [tharsis#884](https://github.com/tharsis/ethermint/pull/884) Support multiple account types on the EVM `StateDB`.
 * (rpc) [tharsis#831](https://github.com/tharsis/ethermint/pull/831) Fix BaseFee value when height is specified.
 * (evm) [tharsis#838](https://github.com/tharsis/ethermint/pull/838) Fix splitting of trace.Memory into 32 chunks.
 * (rpc) [tharsis#860](https://github.com/tharsis/ethermint/pull/860) Fix `eth_getLogs` when specify blockHash without address/topics, and limit the response size.

app/ante/eth.go

@@ -115,11 +115,8 @@ func (avd EthAccountVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx
 
 		// check whether the sender address is EOA
 		fromAddr := common.BytesToAddress(from)
-		acct, err := avd.evmKeeper.GetAccount(ctx, fromAddr)
-		if err != nil {
-			return ctx, sdkerrors.Wrapf(sdkerrors.ErrInvalidType,
-				"the sender is not EthAccount: address %s", fromAddr)
-		}
+		acct := avd.evmKeeper.GetAccount(ctx, fromAddr)
+
 		if acct == nil {
 			acc := avd.ak.NewAccountWithAddress(ctx, from)
 			avd.ak.SetAccount(ctx, acc)

app/ante/interfaces.go

@@ -5,6 +5,7 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	tx "github.com/cosmos/cosmos-sdk/types/tx"
+	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/go-ethereum/core/vm"
 	"github.com/ethereum/go-ethereum/params"
@@ -23,6 +24,7 @@ type EVMKeeper interface {
 		ctx sdk.Context, msgEthTx evmtypes.MsgEthereumTx, txData evmtypes.TxData, denom string, homestead, istanbul, london bool,
 	) (sdk.Coins, error)
 	BaseFee(ctx sdk.Context, ethCfg *params.ChainConfig) *big.Int
+	GetBalance(ctx sdk.Context, addr common.Address) *big.Int
 }
 
 type protoTxProvider interface {

types/account.go

@@ -12,6 +12,7 @@ import (
 
 var (
 	_ authtypes.AccountI                 = (*EthAccount)(nil)
+	_ EthAccountI                        = (*EthAccount)(nil)
 	_ authtypes.GenesisAccount           = (*EthAccount)(nil)
 	_ codectypes.UnpackInterfacesMessage = (*EthAccount)(nil)
 )
@@ -25,6 +26,19 @@ const (
 	AccountTypeContract
 )
 
+// EthAccountI represents the interface of an EVM compatible account
+type EthAccountI interface {
+	authtypes.AccountI
+	// EthAddress returns the ethereum Address representation of the AccAddress
+	EthAddress() common.Address
+	// CodeHash is the keccak256 hash of the contract code (if any)
+	GetCodeHash() common.Hash
+	// SetCodeHash sets the code hash to the account fields
+	SetCodeHash(code common.Hash) error
+	// Type returns the type of Ethereum Account (EOA or Contract)
+	Type() int8
+}
+
 // ----------------------------------------------------------------------------
 // Main Ethermint account
 // ----------------------------------------------------------------------------
@@ -48,6 +62,12 @@ func (acc EthAccount) GetCodeHash() common.Hash {
 	return common.HexToHash(acc.CodeHash)
 }
 
+// SetCodeHash sets the account code hash to the EthAccount fields
+func (acc *EthAccount) SetCodeHash(codeHash common.Hash) error {
+	acc.CodeHash = codeHash.Hex()
+	return nil
+}
+
 // Type returns the type of Ethereum Account (EOA or Contract)
 func (acc EthAccount) Type() int8 {
 	if bytes.Equal(emptyCodeHash, common.Hex2Bytes(acc.CodeHash)) {

types/code.go

@@ -1,12 +0,0 @@
-package types
-
-// ----------------------------------------------------------------------------
-// Code
-// ----------------------------------------------------------------------------
-
-// Code is account Code type alias
-type Code []byte
-
-func (c Code) String() string {
-	return string(c)
-}

x/evm/genesis.go

@@ -40,20 +40,21 @@ func InitGenesis(
 			panic(fmt.Errorf("account not found for address %s", account.Address))
 		}
 
-		ethAcct, ok := acc.(*ethermint.EthAccount)
+		ethAcct, ok := acc.(ethermint.EthAccountI)
 		if !ok {
 			panic(
-				fmt.Errorf("account %s must be an %T type, got %T",
-					account.Address, &ethermint.EthAccount{}, acc,
+				fmt.Errorf("account %s must be an EthAccount interface, got %T",
+					account.Address, acc,
 				),
 			)
 		}
 
 		code := common.Hex2Bytes(account.Code)
 		codeHash := crypto.Keccak256Hash(code)
-		if !bytes.Equal(common.HexToHash(ethAcct.CodeHash).Bytes(), codeHash.Bytes()) {
+		if !bytes.Equal(ethAcct.GetCodeHash().Bytes(), codeHash.Bytes()) {
 			panic("code don't match codeHash")
 		}
+
 		k.SetCode(ctx, codeHash.Bytes(), code)
 
 		for _, storage := range account.Storage {
@@ -68,7 +69,7 @@ func InitGenesis(
 func ExportGenesis(ctx sdk.Context, k *keeper.Keeper, ak types.AccountKeeper) *types.GenesisState {
 	var ethGenAccounts []types.GenesisAccount
 	ak.IterateAccounts(ctx, func(account authtypes.AccountI) bool {
-		ethAccount, ok := account.(*ethermint.EthAccount)
+		ethAccount, ok := account.(ethermint.EthAccountI)
 		if !ok {
 			// ignore non EthAccounts
 			return false

x/evm/keeper/grpc_query.go

@@ -48,10 +48,8 @@ func (k Keeper) Account(c context.Context, req *types.QueryAccountRequest) (*typ
 	addr := common.HexToAddress(req.Address)
 
 	ctx := sdk.UnwrapSDKContext(c)
-	acct, err := k.GetAccountOrEmpty(ctx, addr)
-	if err != nil {
-		return nil, status.Error(codes.InvalidArgument, err.Error())
-	}
+	acct := k.GetAccountOrEmpty(ctx, addr)
+
 	return &types.QueryAccountResponse{
 		Balance:  acct.Balance.String(),
 		CodeHash: common.BytesToHash(acct.CodeHash).Hex(),
@@ -186,10 +184,7 @@ func (k Keeper) Code(c context.Context, req *types.QueryCodeRequest) (*types.Que
 	ctx := sdk.UnwrapSDKContext(c)
 
 	address := common.HexToAddress(req.Address)
-	acct, err := k.GetAccountWithoutBalance(ctx, address)
-	if err != nil {
-		return nil, status.Error(codes.InvalidArgument, err.Error())
-	}
+	acct := k.GetAccountWithoutBalance(ctx, address)
 
 	var code []byte
 	if acct != nil && acct.IsContract() {

x/evm/keeper/grpc_query_test.go

@@ -345,9 +345,7 @@ func (suite *KeeperTestSuite) TestQueryCode() {
 }
 
 func (suite *KeeperTestSuite) TestQueryTxLogs() {
-	var (
-		expLogs []*types.Log
-	)
+	var expLogs []*types.Log
 	txHash := common.BytesToHash([]byte("tx_hash"))
 	txIndex := uint(1)
 	logIndex := uint(1)
@@ -593,7 +591,7 @@ func (suite *KeeperTestSuite) TestEstimateGas() {
 			rsp, err := suite.queryClient.EstimateGas(sdk.WrapSDKContext(suite.ctx), &req)
 			if tc.expPass {
 				suite.Require().NoError(err)
-				suite.Require().Equal(tc.expGas, rsp.Gas)
+				suite.Require().Equal(int64(tc.expGas), int64(rsp.Gas))
 			} else {
 				suite.Require().Error(err)
 			}

x/evm/keeper/keeper.go

@@ -6,7 +6,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/store/prefix"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core"
@@ -235,38 +234,37 @@ func (k Keeper) Tracer(ctx sdk.Context, msg core.Message, ethCfg *params.ChainCo
 
 // GetAccountWithoutBalance load nonce and codehash without balance,
 // more efficient in cases where balance is not needed.
-func (k *Keeper) GetAccountWithoutBalance(ctx sdk.Context, addr common.Address) (*statedb.Account, error) {
+func (k *Keeper) GetAccountWithoutBalance(ctx sdk.Context, addr common.Address) *statedb.Account {
 	cosmosAddr := sdk.AccAddress(addr.Bytes())
 	acct := k.accountKeeper.GetAccount(ctx, cosmosAddr)
 	if acct == nil {
-		return nil, nil
+		return nil
 	}
 
-	ethAcct, ok := acct.(*ethermint.EthAccount)
-	if !ok {
-		return nil, sdkerrors.Wrapf(types.ErrInvalidAccount, "type %T, address %s", acct, addr)
+	codeHash := types.EmptyCodeHash
+	ethAcct, ok := acct.(ethermint.EthAccountI)
+	if ok {
+		codeHash = ethAcct.GetCodeHash().Bytes()
 	}
 
 	return &statedb.Account{
-		Nonce:    ethAcct.Sequence,
-		CodeHash: common.FromHex(ethAcct.CodeHash),
-	}, nil
+		Nonce:    acct.GetSequence(),
+		CodeHash: codeHash,
+	}
 }
 
 // GetAccountOrEmpty returns empty account if not exist, returns error if it's not `EthAccount`
-func (k *Keeper) GetAccountOrEmpty(ctx sdk.Context, addr common.Address) (statedb.Account, error) {
-	acct, err := k.GetAccount(ctx, addr)
-	if err != nil {
-		return statedb.Account{}, err
+func (k *Keeper) GetAccountOrEmpty(ctx sdk.Context, addr common.Address) statedb.Account {
+	acct := k.GetAccount(ctx, addr)
+	if acct != nil {
+		return *acct
 	}
-	if acct == nil {
+
 	// empty account
 	return statedb.Account{
 		Balance:  new(big.Int),
 		CodeHash: types.EmptyCodeHash,
-		}, nil
 	}
-	return *acct, nil
 }
 
 // GetNonce returns the sequence number of an account, returns 0 if not exists.
@@ -277,12 +275,7 @@ func (k *Keeper) GetNonce(ctx sdk.Context, addr common.Address) uint64 {
 		return 0
 	}
 
-	ethAcct, ok := acct.(*ethermint.EthAccount)
-	if !ok {
-		return 0
-	}
-
-	return ethAcct.Sequence
+	return acct.GetSequence()
 }
 
 // GetBalance load account's balance of gas token

x/evm/keeper/statedb.go

@@ -17,18 +17,18 @@ import (
 var _ statedb.Keeper = &Keeper{}
 
 // ----------------------------------------------------------------------------
-// statedb.Keeper implementation
+// StateDB Keeper implementation
 // ----------------------------------------------------------------------------
 
-// GetAccount returns nil if account is not exist, returns error if it's not `EthAccount`
-func (k *Keeper) GetAccount(ctx sdk.Context, addr common.Address) (*statedb.Account, error) {
-	acct, err := k.GetAccountWithoutBalance(ctx, addr)
-	if acct == nil || err != nil {
-		return acct, err
+// GetAccount returns nil if account is not exist, returns error if it's not `EthAccountI`
+func (k *Keeper) GetAccount(ctx sdk.Context, addr common.Address) *statedb.Account {
+	acct := k.GetAccountWithoutBalance(ctx, addr)
+	if acct == nil {
+		return nil
 	}
 
 	acct.Balance = k.GetBalance(ctx, addr)
-	return acct, nil
+	return acct
 }
 
 // GetState loads contract state from database, implements `statedb.Keeper` interface.
@@ -109,25 +109,33 @@ func (k *Keeper) SetAccount(ctx sdk.Context, addr common.Address, account stated
 	if acct == nil {
 		acct = k.accountKeeper.NewAccountWithAddress(ctx, cosmosAddr)
 	}
-	ethAcct, ok := acct.(*ethermint.EthAccount)
-	if !ok {
-		return sdkerrors.Wrapf(types.ErrInvalidAccount, "type %T, address %s", acct, addr)
-	}
-	if err := ethAcct.SetSequence(account.Nonce); err != nil {
+
+	if err := acct.SetSequence(account.Nonce); err != nil {
+		return err
+	}
+
+	codeHash := common.BytesToHash(account.CodeHash)
+
+	if ethAcct, ok := acct.(ethermint.EthAccountI); ok {
+		if err := ethAcct.SetCodeHash(codeHash); err != nil {
+			return err
+		}
+	}
+
+	k.accountKeeper.SetAccount(ctx, acct)
+
+	if err := k.SetBalance(ctx, addr, account.Balance); err != nil {
 		return err
 	}
-	ethAcct.CodeHash = common.BytesToHash(account.CodeHash).Hex()
-	k.accountKeeper.SetAccount(ctx, ethAcct)
 
-	err := k.SetBalance(ctx, addr, account.Balance)
 	k.Logger(ctx).Debug(
 		"account updated",
 		"ethereum-address", addr.Hex(),
 		"nonce", account.Nonce,
-		"codeHash", common.BytesToHash(account.CodeHash).Hex(),
+		"codeHash", codeHash.Hex(),
 		"balance", account.Balance,
 	)
-	return err
+	return nil
 }
 
 // SetState update contract storage, delete if value is empty.
@@ -177,7 +185,8 @@ func (k *Keeper) DeleteAccount(ctx sdk.Context, addr common.Address) error {
 		return nil
 	}
 
-	ethAcct, ok := acct.(*ethermint.EthAccount)
+	// NOTE: only Ethereum accounts (contracts) can be selfdestructed
+	ethAcct, ok := acct.(ethermint.EthAccountI)
 	if !ok {
 		return sdkerrors.Wrapf(types.ErrInvalidAccount, "type %T, address %s", acct, addr)
 	}
@@ -188,9 +197,9 @@ func (k *Keeper) DeleteAccount(ctx sdk.Context, addr common.Address) error {
 	}
 
 	// remove code
-	codeHash := common.HexToHash(ethAcct.CodeHash).Bytes()
-	if !bytes.Equal(codeHash, types.EmptyCodeHash) {
-		k.SetCode(ctx, codeHash, nil)
+	codeHashBz := ethAcct.GetCodeHash().Bytes()
+	if !bytes.Equal(codeHashBz, types.EmptyCodeHash) {
+		k.SetCode(ctx, codeHashBz, nil)
 	}
 
 	// clear storage

x/evm/keeper/statedb_test.go

@@ -235,9 +235,9 @@ func (suite *KeeperTestSuite) TestGetCodeHash() {
 			func(vm.StateDB) {},
 		},
 		{
-			"account not EthAccount type, error",
+			"account not EthAccount type, EmptyCodeHash",
 			addr,
-			common.Hash{},
+			common.BytesToHash(types.EmptyCodeHash),
 			func(vm.StateDB) {},
 		},
 		{
@@ -469,27 +469,21 @@ func (suite *KeeperTestSuite) TestExist() {
 
 func (suite *KeeperTestSuite) TestEmpty() {
 	suite.SetupTest()
-	addr := tests.GenerateAddress()
-	baseAcc := &authtypes.BaseAccount{Address: sdk.AccAddress(addr.Bytes()).String()}
-	suite.app.AccountKeeper.SetAccount(suite.ctx, baseAcc)
-
-	acct, err := suite.app.EvmKeeper.GetAccount(suite.ctx, suite.address)
-	suite.Require().NoError(err)
-	fmt.Println("default address", acct)
 
 	testCases := []struct {
 		name     string
 		address  common.Address
 		malleate func(vm.StateDB)
 		empty    bool
-		expErr   bool
 	}{
-		{"empty, account exists", suite.address, func(vm.StateDB) {}, true, false},
-		{"error, non ethereum account", addr, func(vm.StateDB) {}, true, true},
-		{"not empty, positive balance", suite.address, func(vmdb vm.StateDB) {
-			vmdb.AddBalance(suite.address, big.NewInt(100))
-		}, false, false},
-		{"empty, account doesn't exist", tests.GenerateAddress(), func(vm.StateDB) {}, true, false},
+		{"empty, account exists", suite.address, func(vm.StateDB) {}, true},
+		{
+			"not empty, positive balance",
+			suite.address,
+			func(vmdb vm.StateDB) { vmdb.AddBalance(suite.address, big.NewInt(100)) },
+			false,
+		},
+		{"empty, account doesn't exist", tests.GenerateAddress(), func(vm.StateDB) {}, true},
 	}
 
 	for _, tc := range testCases {
@@ -498,11 +492,6 @@ func (suite *KeeperTestSuite) TestEmpty() {
 			tc.malleate(vmdb)
 
 			suite.Require().Equal(tc.empty, vmdb.Empty(tc.address))
-			if tc.expErr {
-				suite.Require().Error(vmdb.Error())
-			} else {
-				suite.Require().NoError(vmdb.Error())
-			}
 		})
 	}
 }

x/evm/keeper/utils_test.go

@@ -235,9 +235,8 @@ func (suite *KeeperTestSuite) TestCheckSenderBalance() {
 
 			txData, _ := evmtypes.UnpackTxData(tx.Data)
 
-			acct, err := suite.app.EvmKeeper.GetAccountOrEmpty(suite.ctx, suite.address)
-			suite.Require().NoError(err)
-			err = evmkeeper.CheckSenderBalance(
+			acct := suite.app.EvmKeeper.GetAccountOrEmpty(suite.ctx, suite.address)
+			err := evmkeeper.CheckSenderBalance(
 				sdk.NewIntFromBigInt(acct.Balance),
 				txData,
 			)

x/evm/statedb/interfaces.go

@@ -8,7 +8,7 @@ import (
 // Keeper provide underlying storage of StateDB
 type Keeper interface {
 	// Read methods
-	GetAccount(ctx sdk.Context, addr common.Address) (*Account, error)
+	GetAccount(ctx sdk.Context, addr common.Address) *Account
 	GetState(ctx sdk.Context, addr common.Address, key common.Hash) common.Hash
 	GetCode(ctx sdk.Context, codeHash common.Hash) []byte
 	// the callback returns false to break early

x/evm/statedb/mock_test.go

@@ -1,7 +1,6 @@
 package statedb_test
 
 import (
-	"errors"
 	"math/big"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -29,15 +28,12 @@ func NewMockKeeper() *MockKeeper {
 	}
 }
 
-func (k MockKeeper) GetAccount(ctx sdk.Context, addr common.Address) (*statedb.Account, error) {
-	if addr == k.errAddress {
-		return nil, errors.New("mock db error")
-	}
+func (k MockKeeper) GetAccount(ctx sdk.Context, addr common.Address) *statedb.Account {
 	acct, ok := k.accounts[addr]
 	if !ok {
-		return nil, nil
+		return nil
 	}
-	return &acct, nil
+	return &acct
 }
 
 func (k MockKeeper) GetState(ctx sdk.Context, addr common.Address, key common.Hash) common.Hash {

x/evm/statedb/statedb.go

@@ -31,7 +31,6 @@ var _ vm.StateDB = &StateDB{}
 type StateDB struct {
 	keeper Keeper
 	ctx    sdk.Context
-	dbErr  error
 
 	// Journal of state modifications. This is the backbone of
 	// Snapshot and RevertToSnapshot.
@@ -76,18 +75,6 @@ func (s *StateDB) Context() sdk.Context {
 	return s.ctx
 }
 
-// setError remembers the first non-nil error it is called with.
-func (s *StateDB) setError(err error) {
-	if s.dbErr == nil {
-		s.dbErr = err
-	}
-}
-
-// Error returns the database error recorded.
-func (s *StateDB) Error() error {
-	return s.dbErr
-}
-
 // AddLog adds a log, called by evm.
 func (s *StateDB) AddLog(log *ethtypes.Log) {
 	s.journal.append(addLogChange{})
@@ -235,11 +222,7 @@ func (s *StateDB) getStateObject(addr common.Address) *stateObject {
 		return obj
 	}
 	// If no live objects are available, load it from keeper
-	account, err := s.keeper.GetAccount(s.ctx, addr)
-	if err != nil {
-		s.setError(err)
-		return nil
-	}
+	account := s.keeper.GetAccount(s.ctx, addr)
 	if account == nil {
 		return nil
 	}
@@ -468,9 +451,6 @@ func (s *StateDB) RevertToSnapshot(revid int) {
 // Commit writes the dirty states to keeper
 // the StateDB object should be discarded after committed.
 func (s *StateDB) Commit() error {
-	if s.dbErr != nil {
-		return fmt.Errorf("commit aborted due to earlier error: %v", s.dbErr)
-	}
 	for _, addr := range s.journal.sortedDirties() {
 		obj := s.stateObjects[addr]
 		if obj.suicided {

x/evm/statedb/statedb_test.go

@@ -17,7 +17,6 @@ type StateDBTestSuite struct {
 }
 
 func (suite *StateDBTestSuite) TestAccounts() {
-	addrErr := common.BigToAddress(big.NewInt(1))
 	addr2 := common.BigToAddress(big.NewInt(2))
 	testTxConfig := statedb.NewTxConfig(
 		common.BigToHash(big.NewInt(10)), // tx hash
@@ -46,13 +45,6 @@ func (suite *StateDBTestSuite) TestAccounts() {
 				suite.Require().Equal(big.NewInt(1), db.GetBalance(addr2))
 			},
 		},
-		{
-			"fail,GetBalance dbErr",
-			func(db *statedb.StateDB) {
-				suite.Require().Equal(big.NewInt(0), db.GetBalance(addrErr))
-				suite.Require().Error(db.Commit())
-			},
-		},
 		{
 			"success, change balance",
 			func(db *statedb.StateDB) {