types: account balance fix (#507)

* fix hardcoded photon on account balance getter/setter

* types: testing suite

* balance test

* update zero diff

* add case for other coin

* changelog

* fix journal test

CHANGELOG.md

@@ -54,6 +54,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Bug Fixes
 
+* (types) [\#507](https://github.com/ChainSafe/ethermint/pull/507) Fix hardcoded `aphoton` on `EthAccount` balance getter and setter.
 * (`x/evm`) [\#496](https://github.com/ChainSafe/ethermint/pull/496) Fix bugs on `journal.revert` and `CommitStateDB.Copy`.
 * (types) [\#480](https://github.com/ChainSafe/ethermint/pull/480) Update [BIP44](https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki) coin type to `60` to satisfy [EIP84](https://github.com/ethereum/EIPs/issues/84).
 

importer/importer_test.go

@@ -150,7 +150,8 @@ func createAndTestGenesis(t *testing.T, cms sdk.CommitMultiStore, ak auth.Accoun
 	genAcc := ak.GetAccount(ctx, sdk.AccAddress(genInvestor.Bytes()))
 	require.NotNil(t, genAcc)
 
-	balance := types.NewPhotonCoin(genAcc.GetCoins().AmountOf(types.AttoPhoton))
+	evmDenom := evmKeeper.GetParams(ctx).EvmDenom
+	balance := sdk.NewCoin(evmDenom, genAcc.GetCoins().AmountOf(evmDenom))
 	require.Equal(t, sdk.NewIntFromBigInt(b), balance.Amount)
 }
 

types/account.go

@@ -52,27 +52,29 @@ func (acc EthAccount) EthAddress() ethcmn.Address {
 // TODO: remove on SDK v0.40
 
 // Balance returns the balance of an account.
-func (acc EthAccount) Balance() sdk.Int {
+func (acc EthAccount) Balance(denom string) sdk.Int {
-	return acc.GetCoins().AmountOf(AttoPhoton)
+	return acc.GetCoins().AmountOf(denom)
 }
 
-// SetBalance sets an account's balance of aphotons
+// SetBalance sets an account's balance of the given coin denomination.
-func (acc *EthAccount) SetBalance(amt sdk.Int) {
+//
+// CONTRACT: assumes the denomination is valid.
+func (acc *EthAccount) SetBalance(denom string, amt sdk.Int) {
 	coins := acc.GetCoins()
-	diff := amt.Sub(coins.AmountOf(AttoPhoton))
+	diff := amt.Sub(coins.AmountOf(denom))
 	switch {
 	case diff.IsPositive():
 		// Increase coins to amount
-		coins = coins.Add(NewPhotonCoin(diff))
+		coins = coins.Add(sdk.NewCoin(denom, diff))
 	case diff.IsNegative():
 		// Decrease coins to amount
-		coins = coins.Sub(sdk.NewCoins(NewPhotonCoin(diff.Neg())))
+		coins = coins.Sub(sdk.NewCoins(sdk.NewCoin(denom, diff.Neg())))
 	default:
 		return
 	}
 
 	if err := acc.SetCoins(coins); err != nil {
-		panic(fmt.Errorf("could not set coins for address %s: %w", acc.EthAddress().String(), err))
+		panic(fmt.Errorf("could not set %s coins for address %s: %w", denom, acc.EthAddress().String(), err))
 	}
 }
 

types/account_test.go

@@ -1,11 +1,11 @@
-package types
+package types_test
 
 import (
 	"encoding/json"
 	"fmt"
 	"testing"
 
-	"github.com/stretchr/testify/require"
+	"github.com/stretchr/testify/suite"
 
 	tmamino "github.com/tendermint/tendermint/crypto/encoding/amino"
 	"github.com/tendermint/tendermint/crypto/secp256k1"
@@ -13,65 +13,101 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/auth"
 
-	emintcrypto "github.com/cosmos/ethermint/crypto"
+	"github.com/cosmos/ethermint/crypto"
+	"github.com/cosmos/ethermint/types"
 )
 
 func init() {
-	tmamino.RegisterKeyType(emintcrypto.PubKeySecp256k1{}, emintcrypto.PubKeyAminoName)
+	tmamino.RegisterKeyType(crypto.PubKeySecp256k1{}, crypto.PubKeyAminoName)
-	tmamino.RegisterKeyType(emintcrypto.PrivKeySecp256k1{}, emintcrypto.PrivKeyAminoName)
+	tmamino.RegisterKeyType(crypto.PrivKeySecp256k1{}, crypto.PrivKeyAminoName)
 }
 
-func TestEthermintAccountJSON(t *testing.T) {
+type AccountTestSuite struct {
+	suite.Suite
+
+	account *types.EthAccount
+}
+
+func (suite *AccountTestSuite) SetupTest() {
 	pubkey := secp256k1.GenPrivKey().PubKey()
 	addr := sdk.AccAddress(pubkey.Address())
-	balance := sdk.NewCoins(NewPhotonCoin(sdk.OneInt()))
+	balance := sdk.NewCoins(types.NewPhotonCoin(sdk.OneInt()))
 	baseAcc := auth.NewBaseAccount(addr, balance, pubkey, 10, 50)
-	ethAcc := EthAccount{BaseAccount: baseAcc, CodeHash: []byte{1, 2}}
+	suite.account = &types.EthAccount{
-
+		BaseAccount: baseAcc,
-	bz, err := json.Marshal(ethAcc)
+		CodeHash:    []byte{1, 2},
-	require.NoError(t, err)
+	}
-
-	bz1, err := ethAcc.MarshalJSON()
-	require.NoError(t, err)
-	require.Equal(t, string(bz1), string(bz))
-
-	var a EthAccount
-	require.NoError(t, a.UnmarshalJSON(bz))
-	require.Equal(t, ethAcc.String(), a.String())
-	require.Equal(t, ethAcc.PubKey, a.PubKey)
 }
 
-func TestEthermintPubKeyJSON(t *testing.T) {
+func TestAccountTestSuite(t *testing.T) {
-	privkey, err := emintcrypto.GenerateKey()
+	suite.Run(t, new(AccountTestSuite))
-	require.NoError(t, err)
+}
+
+func (suite *AccountTestSuite) TestEthAccount_Balance() {
+
+	testCases := []struct {
+		name         string
+		denom        string
+		initialCoins sdk.Coins
+		amount       sdk.Int
+	}{
+		{"positive diff", types.AttoPhoton, sdk.Coins{}, sdk.OneInt()},
+		{"zero diff, same coin", types.AttoPhoton, sdk.NewCoins(types.NewPhotonCoin(sdk.ZeroInt())), sdk.ZeroInt()},
+		{"zero diff, other coin", sdk.DefaultBondDenom, sdk.NewCoins(types.NewPhotonCoin(sdk.ZeroInt())), sdk.ZeroInt()},
+		{"negative diff", types.AttoPhoton, sdk.NewCoins(types.NewPhotonCoin(sdk.NewInt(10))), sdk.NewInt(1)},
+	}
+
+	for _, tc := range testCases {
+		suite.Run(tc.name, func() {
+			suite.SetupTest() // reset values
+			suite.account.SetCoins(tc.initialCoins)
+
+			suite.account.SetBalance(tc.denom, tc.amount)
+			suite.Require().Equal(tc.amount, suite.account.Balance(tc.denom))
+		})
+	}
+
+}
+
+func (suite *AccountTestSuite) TestEthermintAccountJSON() {
+	bz, err := json.Marshal(suite.account)
+	suite.Require().NoError(err)
+
+	bz1, err := suite.account.MarshalJSON()
+	suite.Require().NoError(err)
+	suite.Require().Equal(string(bz1), string(bz))
+
+	var a types.EthAccount
+	suite.Require().NoError(a.UnmarshalJSON(bz))
+	suite.Require().Equal(suite.account.String(), a.String())
+	suite.Require().Equal(suite.account.PubKey, a.PubKey)
+}
+
+func (suite *AccountTestSuite) TestEthermintPubKeyJSON() {
+	privkey, err := crypto.GenerateKey()
+	suite.Require().NoError(err)
 	bz := privkey.PubKey().Bytes()
 
 	pubk, err := tmamino.PubKeyFromBytes(bz)
-	require.NoError(t, err)
+	suite.Require().NoError(err)
-	require.Equal(t, pubk, privkey.PubKey())
+	suite.Require().Equal(pubk, privkey.PubKey())
 }
 
-func TestSecpPubKeyJSON(t *testing.T) {
+func (suite *AccountTestSuite) TestSecpPubKeyJSON() {
 	pubkey := secp256k1.GenPrivKey().PubKey()
 	bz := pubkey.Bytes()
 
 	pubk, err := tmamino.PubKeyFromBytes(bz)
-	require.NoError(t, err)
+	suite.Require().NoError(err)
-	require.Equal(t, pubk, pubkey)
+	suite.Require().Equal(pubk, pubkey)
 }
 
-func TestEthermintAccount_String(t *testing.T) {
+func (suite *AccountTestSuite) TestEthermintAccount_String() {
-	pubkey := secp256k1.GenPrivKey().PubKey()
-	addr := sdk.AccAddress(pubkey.Address())
-	balance := sdk.NewCoins(NewPhotonCoin(sdk.OneInt()))
-	baseAcc := auth.NewBaseAccount(addr, balance, pubkey, 10, 50)
-	ethAcc := EthAccount{BaseAccount: baseAcc, CodeHash: []byte{1, 2}}
-
 	config := sdk.GetConfig()
-	SetBech32Prefixes(config)
+	types.SetBech32Prefixes(config)
 
-	bech32pubkey, err := sdk.Bech32ifyPubKey(sdk.Bech32PubKeyTypeAccPub, pubkey)
+	bech32pubkey, err := sdk.Bech32ifyPubKey(sdk.Bech32PubKeyTypeAccPub, suite.account.PubKey)
-	require.NoError(t, err)
+	suite.Require().NoError(err)
 
 	accountStr := fmt.Sprintf(`|
   address: %s
@@ -83,66 +119,60 @@ func TestEthermintAccount_String(t *testing.T) {
   account_number: 10
   sequence: 50
   code_hash: "0102"
-`, addr, ethAcc.EthAddress().String(), bech32pubkey)
+`, suite.account.Address, suite.account.EthAddress().String(), bech32pubkey)
 
-	require.Equal(t, accountStr, ethAcc.String())
+	suite.Require().Equal(accountStr, suite.account.String())
 
-	i, err := ethAcc.MarshalYAML()
+	i, err := suite.account.MarshalYAML()
-	require.NoError(t, err)
+	suite.Require().NoError(err)
 
 	var ok bool
 	accountStr, ok = i.(string)
-	require.True(t, ok)
+	suite.Require().True(ok)
-	require.Contains(t, accountStr, addr.String())
+	suite.Require().Contains(accountStr, suite.account.Address.String())
-	require.Contains(t, accountStr, bech32pubkey)
+	suite.Require().Contains(accountStr, bech32pubkey)
 }
 
-func TestEthermintAccount_MarshalJSON(t *testing.T) {
+func (suite *AccountTestSuite) TestEthermintAccount_MarshalJSON() {
-	pubkey := secp256k1.GenPrivKey().PubKey()
+	bz, err := suite.account.MarshalJSON()
-	addr := sdk.AccAddress(pubkey.Address())
+	suite.Require().NoError(err)
-	balance := sdk.NewCoins(NewPhotonCoin(sdk.OneInt()))
+	suite.Require().Contains(string(bz), suite.account.EthAddress().String())
-	baseAcc := auth.NewBaseAccount(addr, balance, pubkey, 10, 50)
-	ethAcc := &EthAccount{BaseAccount: baseAcc, CodeHash: []byte{1, 2}}
 
-	bz, err := ethAcc.MarshalJSON()
+	res := new(types.EthAccount)
-	require.NoError(t, err)
-	require.Contains(t, string(bz), ethAcc.EthAddress().String())
-
-	res := new(EthAccount)
 	err = res.UnmarshalJSON(bz)
-	require.NoError(t, err)
+	suite.Require().NoError(err)
-	require.Equal(t, ethAcc, res)
+	suite.Require().Equal(suite.account, res)
 
-	bech32pubkey, err := sdk.Bech32ifyPubKey(sdk.Bech32PubKeyTypeAccPub, pubkey)
+	bech32pubkey, err := sdk.Bech32ifyPubKey(sdk.Bech32PubKeyTypeAccPub, suite.account.PubKey)
-	require.NoError(t, err)
+	suite.Require().NoError(err)
 
 	// test that the sdk.AccAddress is populated from the hex address
 	jsonAcc := fmt.Sprintf(
 		`{"address":"","eth_address":"%s","coins":[{"denom":"aphoton","amount":"1"}],"public_key":"%s","account_number":10,"sequence":50,"code_hash":"0102"}`,
-		ethAcc.EthAddress().String(), bech32pubkey,
+		suite.account.EthAddress().String(), bech32pubkey,
 	)
 
-	res = new(EthAccount)
+	res = new(types.EthAccount)
 	err = res.UnmarshalJSON([]byte(jsonAcc))
-	require.NoError(t, err)
+	suite.Require().NoError(err)
-	require.Equal(t, addr.String(), res.Address.String())
+	suite.Require().Equal(suite.account.Address.String(), res.Address.String())
 
 	jsonAcc = fmt.Sprintf(
 		`{"address":"","eth_address":"","coins":[{"denom":"aphoton","amount":"1"}],"public_key":"%s","account_number":10,"sequence":50,"code_hash":"0102"}`,
 		bech32pubkey,
 	)
 
-	res = new(EthAccount)
+	res = new(types.EthAccount)
 	err = res.UnmarshalJSON([]byte(jsonAcc))
-	require.Error(t, err, "should fail if both address are empty")
+	suite.Require().Error(err, "should fail if both address are empty")
 
 	// test that the sdk.AccAddress is populated from the hex address
 	jsonAcc = fmt.Sprintf(
 		`{"address": "%s","eth_address":"0x0000000000000000000000000000000000000000","coins":[{"denom":"aphoton","amount":"1"}],"public_key":"%s","account_number":10,"sequence":50,"code_hash":"0102"}`,
-		ethAcc.Address.String(), bech32pubkey,
+		suite.account.Address.String(), bech32pubkey,
 	)
 
-	res = new(EthAccount)
+	res = new(types.EthAccount)
 	err = res.UnmarshalJSON([]byte(jsonAcc))
-	require.Error(t, err, "should fail if addresses mismatch")
+	suite.Require().Error(err, "should fail if addresses mismatch")
 }

types/config_test.go

@@ -10,7 +10,6 @@ import (
 
 func TestSetBech32Prefixes(t *testing.T) {
 	config := sdk.GetConfig()
-	config = sdk.NewConfig() // reset config values
 
 	require.Equal(t, sdk.Bech32PrefixAccAddr, config.GetBech32AccountAddrPrefix())
 	require.Equal(t, sdk.Bech32PrefixAccPub, config.GetBech32AccountPubPrefix())

x/evm/types/journal.go

@@ -232,7 +232,8 @@ func (ch suicideChange) revert(s *CommitStateDB) {
 	so := s.getStateObject(*ch.account)
 	if so != nil {
 		so.suicided = ch.prev
-		so.setBalance(ch.prevBalance)
+		evmDenom := s.GetParams().EvmDenom
+		so.setBalance(evmDenom, ch.prevBalance)
 	}
 }
 
@@ -248,7 +249,8 @@ func (ch touchChange) dirtied() *ethcmn.Address {
 }
 
 func (ch balanceChange) revert(s *CommitStateDB) {
-	s.getStateObject(*ch.account).setBalance(ch.prev)
+	evmDenom := s.GetParams().EvmDenom
+	s.getStateObject(*ch.account).setBalance(evmDenom, ch.prev)
 }
 
 func (ch balanceChange) dirtied() *ethcmn.Address {

x/evm/types/journal_test.go

@@ -97,6 +97,8 @@ func (suite *JournalTestSuite) SetupTest() {
 // to maintain consistency with the Geth implementation.
 func (suite *JournalTestSuite) setup() {
 	authKey := sdk.NewKVStoreKey(auth.StoreKey)
+	paramsKey := sdk.NewKVStoreKey(params.StoreKey)
+	paramsTKey := sdk.NewTransientStoreKey(params.TStoreKey)
 	// bankKey := sdk.NewKVStoreKey(bank.StoreKey)
 	storeKey := sdk.NewKVStoreKey(StoreKey)
 
@@ -107,25 +109,24 @@ func (suite *JournalTestSuite) setup() {
 
 	cms := store.NewCommitMultiStore(db)
 	cms.MountStoreWithDB(authKey, sdk.StoreTypeIAVL, db)
-	// cms.MountStoreWithDB(bankKey, sdk.StoreTypeIAVL, db)
+	cms.MountStoreWithDB(paramsKey, sdk.StoreTypeIAVL, db)
 	cms.MountStoreWithDB(storeKey, sdk.StoreTypeIAVL, db)
+	cms.MountStoreWithDB(paramsTKey, sdk.StoreTypeTransient, db)
 
 	err := cms.LoadLatestVersion()
 	suite.Require().NoError(err)
 
 	cdc := newTestCodec()
 
-	keyParams := sdk.NewKVStoreKey(params.StoreKey)
+	paramsKeeper := params.NewKeeper(cdc, paramsKey, paramsTKey)
-	tkeyParams := sdk.NewTransientStoreKey(params.TStoreKey)
-	paramsKeeper := params.NewKeeper(cdc, keyParams, tkeyParams)
 
 	authSubspace := paramsKeeper.Subspace(auth.DefaultParamspace)
-	evmSubspace := paramsKeeper.Subspace(types.DefaultParamspace)
+	evmSubspace := paramsKeeper.Subspace(types.DefaultParamspace).WithKeyTable(ParamKeyTable())
 
 	ak := auth.NewAccountKeeper(cdc, authKey, authSubspace, ethermint.ProtoAccount)
-
 	suite.ctx = sdk.NewContext(cms, abci.Header{ChainID: "8"}, false, tmlog.NewNopLogger())
 	suite.stateDB = NewCommitStateDB(suite.ctx, storeKey, evmSubspace, ak).WithContext(suite.ctx)
+	suite.stateDB.SetParams(DefaultParams())
 }
 
 func TestJournalTestSuite(t *testing.T) {

x/evm/types/state_object.go

@@ -204,11 +204,11 @@ func (so *stateObject) SetBalance(amount *big.Int) {
 		prev:    so.account.GetCoins().AmountOf(evmDenom),
 	})
 
-	so.setBalance(amt)
+	so.setBalance(evmDenom, amt)
 }
 
-func (so *stateObject) setBalance(amount sdk.Int) {
+func (so *stateObject) setBalance(denom string, amount sdk.Int) {
-	so.account.SetBalance(amount)
+	so.account.SetBalance(denom, amount)
 }
 
 // SetNonce sets the state object's nonce (i.e sequence number of the account).
@@ -295,7 +295,8 @@ func (so stateObject) Address() ethcmn.Address {
 
 // Balance returns the state object's current balance.
 func (so *stateObject) Balance() *big.Int {
-	balance := so.account.Balance().BigInt()
+	evmDenom := so.stateDB.GetParams().EvmDenom
+	balance := so.account.Balance(evmDenom).BigInt()
 	if balance == nil {
 		return zeroBalance
 	}
@@ -406,7 +407,8 @@ func (so *stateObject) deepCopy(db *CommitStateDB) *stateObject {
 
 // empty returns whether the account is considered empty.
 func (so *stateObject) empty() bool {
-	balace := so.account.Balance()
+	evmDenom := so.stateDB.GetParams().EvmDenom
+	balace := so.account.Balance(evmDenom)
 	return so.account == nil ||
 		(so.account != nil &&
 			so.account.Sequence == 0 &&

x/evm/types/statedb.go

@@ -693,7 +693,8 @@ func (csdb *CommitStateDB) Prepare(thash, bhash ethcmn.Hash, txi int) {
 func (csdb *CommitStateDB) CreateAccount(addr ethcmn.Address) {
 	newobj, prevobj := csdb.createObject(addr)
 	if prevobj != nil {
-		newobj.setBalance(sdk.NewIntFromBigInt(prevobj.Balance()))
+		evmDenom := csdb.GetParams().EvmDenom
+		newobj.setBalance(evmDenom, sdk.NewIntFromBigInt(prevobj.Balance()))
 	}
 }