From 44876ac72fe97390b334aa311583d306d0e07842 Mon Sep 17 00:00:00 2001 From: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Date: Wed, 9 Sep 2020 15:53:14 +0200 Subject: [PATCH] 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 | 1 + importer/importer_test.go | 3 +- types/account.go | 18 ++-- types/account_test.go | 168 +++++++++++++++++++++--------------- types/config_test.go | 1 - x/evm/types/journal.go | 6 +- x/evm/types/journal_test.go | 13 +-- x/evm/types/state_object.go | 12 +-- x/evm/types/statedb.go | 3 +- 9 files changed, 132 insertions(+), 93 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a59b99e..c0ae1725 100644 --- a/CHANGELOG.md +++ b/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). diff --git a/importer/importer_test.go b/importer/importer_test.go index 85fb8e6b..756a7e05 100644 --- a/importer/importer_test.go +++ b/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) } diff --git a/types/account.go b/types/account.go index 6ff23bbe..ca05f817 100644 --- a/types/account.go +++ b/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 { - return acc.GetCoins().AmountOf(AttoPhoton) +func (acc EthAccount) Balance(denom string) sdk.Int { + return acc.GetCoins().AmountOf(denom) } -// SetBalance sets an account's balance of aphotons -func (acc *EthAccount) SetBalance(amt sdk.Int) { +// SetBalance sets an account's balance of the given coin denomination. +// +// 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)) } } diff --git a/types/account_test.go b/types/account_test.go index 20c9b9ec..ff28bcb3 100644 --- a/types/account_test.go +++ b/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(emintcrypto.PrivKeySecp256k1{}, emintcrypto.PrivKeyAminoName) + tmamino.RegisterKeyType(crypto.PubKeySecp256k1{}, crypto.PubKeyAminoName) + 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}} - - bz, err := json.Marshal(ethAcc) - 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) + suite.account = &types.EthAccount{ + BaseAccount: baseAcc, + CodeHash: []byte{1, 2}, + } } -func TestEthermintPubKeyJSON(t *testing.T) { - privkey, err := emintcrypto.GenerateKey() - require.NoError(t, err) +func TestAccountTestSuite(t *testing.T) { + suite.Run(t, new(AccountTestSuite)) +} + +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) - require.Equal(t, pubk, privkey.PubKey()) + suite.Require().NoError(err) + 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) - require.Equal(t, pubk, pubkey) + suite.Require().NoError(err) + suite.Require().Equal(pubk, pubkey) } -func TestEthermintAccount_String(t *testing.T) { - 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}} - +func (suite *AccountTestSuite) TestEthermintAccount_String() { config := sdk.GetConfig() - SetBech32Prefixes(config) + types.SetBech32Prefixes(config) - bech32pubkey, err := sdk.Bech32ifyPubKey(sdk.Bech32PubKeyTypeAccPub, pubkey) - require.NoError(t, err) + bech32pubkey, err := sdk.Bech32ifyPubKey(sdk.Bech32PubKeyTypeAccPub, suite.account.PubKey) + 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() - require.NoError(t, err) + i, err := suite.account.MarshalYAML() + suite.Require().NoError(err) var ok bool accountStr, ok = i.(string) - require.True(t, ok) - require.Contains(t, accountStr, addr.String()) - require.Contains(t, accountStr, bech32pubkey) + suite.Require().True(ok) + suite.Require().Contains(accountStr, suite.account.Address.String()) + suite.Require().Contains(accountStr, bech32pubkey) } -func TestEthermintAccount_MarshalJSON(t *testing.T) { - 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}} +func (suite *AccountTestSuite) TestEthermintAccount_MarshalJSON() { + bz, err := suite.account.MarshalJSON() + suite.Require().NoError(err) + suite.Require().Contains(string(bz), suite.account.EthAddress().String()) - bz, err := ethAcc.MarshalJSON() - require.NoError(t, err) - require.Contains(t, string(bz), ethAcc.EthAddress().String()) - - res := new(EthAccount) + res := new(types.EthAccount) err = res.UnmarshalJSON(bz) - require.NoError(t, err) - require.Equal(t, ethAcc, res) + suite.Require().NoError(err) + suite.Require().Equal(suite.account, res) - bech32pubkey, err := sdk.Bech32ifyPubKey(sdk.Bech32PubKeyTypeAccPub, pubkey) - require.NoError(t, err) + bech32pubkey, err := sdk.Bech32ifyPubKey(sdk.Bech32PubKeyTypeAccPub, suite.account.PubKey) + 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) - require.Equal(t, addr.String(), res.Address.String()) + suite.Require().NoError(err) + 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") } diff --git a/types/config_test.go b/types/config_test.go index 0b985074..40ce3c07 100644 --- a/types/config_test.go +++ b/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()) diff --git a/x/evm/types/journal.go b/x/evm/types/journal.go index a09a7ca6..b2190759 100644 --- a/x/evm/types/journal.go +++ b/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 { diff --git a/x/evm/types/journal_test.go b/x/evm/types/journal_test.go index 5c5fc7c2..ee54fe91 100644 --- a/x/evm/types/journal_test.go +++ b/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) - tkeyParams := sdk.NewTransientStoreKey(params.TStoreKey) - paramsKeeper := params.NewKeeper(cdc, keyParams, tkeyParams) + paramsKeeper := params.NewKeeper(cdc, paramsKey, paramsTKey) 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) { diff --git a/x/evm/types/state_object.go b/x/evm/types/state_object.go index d7c6c749..d9c4d7e1 100644 --- a/x/evm/types/state_object.go +++ b/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) { - so.account.SetBalance(amount) +func (so *stateObject) setBalance(denom string, amount sdk.Int) { + 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 && diff --git a/x/evm/types/statedb.go b/x/evm/types/statedb.go index 1f1ab382..94fa5b8f 100644 --- a/x/evm/types/statedb.go +++ b/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())) } }