Skip to content
This repository has been archived by the owner on Nov 30, 2021. It is now read-only.

account balance fix #507

Merged
merged 7 commits into from
Sep 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand Down
3 changes: 2 additions & 1 deletion importer/importer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
18 changes: 10 additions & 8 deletions types/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the sdk.NewCoins necessary here? the add case doesn't have it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, idk the reason of the inconsistency on the SDK. The Add function has a spread parameter while the Sub doesn't

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

weird 🤔 okay

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah lol

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))
}
}

Expand Down
162 changes: 96 additions & 66 deletions types/account_test.go
Original file line number Diff line number Diff line change
@@ -1,77 +1,113 @@
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"

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}}
suite.account = &types.EthAccount{
BaseAccount: baseAcc,
CodeHash: []byte{1, 2},
}
}

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))
})
}

}

bz, err := json.Marshal(ethAcc)
require.NoError(t, err)
func (suite *AccountTestSuite) TestEthermintAccountJSON() {
bz, err := json.Marshal(suite.account)
suite.Require().NoError(err)

bz1, err := ethAcc.MarshalJSON()
require.NoError(t, err)
require.Equal(t, string(bz1), string(bz))
bz1, err := suite.account.MarshalJSON()
suite.Require().NoError(err)
suite.Require().Equal(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)
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 TestEthermintPubKeyJSON(t *testing.T) {
privkey, err := emintcrypto.GenerateKey()
require.NoError(t, err)
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
Expand All @@ -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}}

bz, err := ethAcc.MarshalJSON()
require.NoError(t, err)
require.Contains(t, string(bz), ethAcc.EthAddress().String())
func (suite *AccountTestSuite) TestEthermintAccount_MarshalJSON() {
bz, err := suite.account.MarshalJSON()
suite.Require().NoError(err)
suite.Require().Contains(string(bz), suite.account.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")
}
1 change: 0 additions & 1 deletion types/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
6 changes: 4 additions & 2 deletions x/evm/types/journal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand All @@ -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 {
Expand Down
13 changes: 7 additions & 6 deletions x/evm/types/journal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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) {
Expand Down
12 changes: 7 additions & 5 deletions x/evm/types/state_object.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 &&
Expand Down
Loading