Skip to content

Commit

Permalink
[bank]: add balance tracking events (#8656)
Browse files Browse the repository at this point in the history
* change(bank): add utxo events and simplify logic

* add(bank): balance and supply tracking test

* chore(bank): fix balance tracking test comment

* fix(grpc): service test

* fix(bank): sub unlocked coins to use less gas

* fix(auth): cli test gas

* fix(rest): grpc gas test

* fix(staking/cli): increase delegation required gas

* add: burn events, fix tests

* fix(auth/tx): grpc test

* add(bank): coin events in delegate

* fix(bank): add amt check in delegate coins back

* change(bank): add coin spent and coin recv events in burn and mint

* change(bank): revert sub coin function

* change(auth): revert cli test

* change(auth): revert service test

* chore(auth): fix events comment in service_test.go

* chore: update CHANGELOG.md

* remove(bank): balanceError func

* chore(bank): address lint warnings

* chore(bank): update events spec

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
fdymylja and mergify[bot] authored Feb 25, 2021
1 parent 19e79e0 commit ef9968d
Show file tree
Hide file tree
Showing 13 changed files with 375 additions and 49 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/ibc) [\#8405](https://github.com/cosmos/cosmos-sdk/pull/8405) Refactor IBC client update governance proposals to use a substitute client to update a frozen or expired client.
* (x/evidence) [\#8502](https://github.com/cosmos/cosmos-sdk/pull/8502) `HandleEquivocationEvidence` persists the evidence to state.
* (x/gov) [\#7733](https://github.com/cosmos/cosmos-sdk/pull/7733) ADR 037 Implementation: Governance Split Votes
* (x/bank) [\#8656](https://github.com/cosmos/cosmos-sdk/pull/8656) balance and supply are now correctly tracked via `coin_spent`, `coin_received`, `coinbase` and `burn` events.

### Improvements

Expand Down
9 changes: 7 additions & 2 deletions x/auth/client/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1102,7 +1102,7 @@ func (s *IntegrationTestSuite) TestSignWithMultiSigners_AminoJSON() {
banktypes.NewMsgSend(val1.Address, addr1, sdk.NewCoins(val1Coin)),
)
txBuilder.SetFeeAmount(sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))))
txBuilder.SetGasLimit(testdata.NewTestGasLimit())
txBuilder.SetGasLimit(testdata.NewTestGasLimit()) // min required is 101892
require.Equal([]sdk.AccAddress{val0.Address, val1.Address}, txBuilder.GetTx().GetSigners())

// Write the unsigned tx into a file.
Expand All @@ -1126,7 +1126,12 @@ func (s *IntegrationTestSuite) TestSignWithMultiSigners_AminoJSON() {
signedTxFile := testutil.WriteToNewTempFile(s.T(), signedTx.String())

// Now let's try to send this tx.
res, err := authtest.TxBroadcastExec(val0.ClientCtx, signedTxFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock))
res, err := authtest.TxBroadcastExec(
val0.ClientCtx,
signedTxFile.Name(),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
)

require.NoError(err)
var txRes sdk.TxResponse
require.NoError(val0.ClientCtx.JSONMarshaler.UnmarshalJSON(res.Bytes(), &txRes))
Expand Down
9 changes: 5 additions & 4 deletions x/auth/tx/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ import (
"fmt"
"testing"

"github.com/cosmos/cosmos-sdk/testutil/testdata"

"github.com/stretchr/testify/suite"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
clienttx "github.com/cosmos/cosmos-sdk/client/tx"
"github.com/cosmos/cosmos-sdk/testutil/network"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
query "github.com/cosmos/cosmos-sdk/types/query"
Expand Down Expand Up @@ -106,7 +107,7 @@ func (s IntegrationTestSuite) TestSimulateTx_GRPC() {
} else {
s.Require().NoError(err)
// Check the result and gas used are correct.
s.Require().Equal(len(res.GetResult().GetEvents()), 4) // 1 transfer, 3 messages.
s.Require().Equal(len(res.GetResult().GetEvents()), 6) // 1 coin recv 1 coin spent, 1 transfer, 3 messages.
s.Require().True(res.GetGasInfo().GetGasUsed() > 0) // Gas used sometimes change, just check it's not empty.
}
})
Expand Down Expand Up @@ -143,7 +144,7 @@ func (s IntegrationTestSuite) TestSimulateTx_GRPCGateway() {
err = val.ClientCtx.JSONMarshaler.UnmarshalJSON(res, &result)
s.Require().NoError(err)
// Check the result and gas used are correct.
s.Require().Equal(len(result.GetResult().GetEvents()), 4) // 1 transfer, 3 messages.
s.Require().Equal(len(result.GetResult().GetEvents()), 6) // 1 coin recv, 1 coin spent,1 transfer, 3 messages.
s.Require().True(result.GetGasInfo().GetGasUsed() > 0) // Gas used sometimes change, just check it's not empty.
}
})
Expand Down Expand Up @@ -412,7 +413,7 @@ func (s IntegrationTestSuite) TestBroadcastTx_GRPCGateway() {
var result tx.BroadcastTxResponse
err = val.ClientCtx.JSONMarshaler.UnmarshalJSON(res, &result)
s.Require().NoError(err)
s.Require().Equal(uint32(0), result.TxResponse.Code)
s.Require().Equal(uint32(0), result.TxResponse.Code, "rawlog", result.TxResponse.RawLog)
}
})
}
Expand Down
4 changes: 2 additions & 2 deletions x/bank/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ func (suite *IntegrationTestSuite) TestExportGenesis() {
Require().
NoError(app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, accAddr, expectedBalances[i].Coins))
}
// add mint module balance as nil
expectedBalances = append(expectedBalances, types.Balance{Address: "cosmos1m3h30wlvsf8llruxtpukdvsy0km2kum8g38c8q", Coins: nil})
app.BankKeeper.SetParams(ctx, types.DefaultParams())

exportGenesis := app.BankKeeper.ExportGenesis(ctx)

suite.Require().Len(exportGenesis.Params.SendEnabled, 0)
suite.Require().Equal(types.DefaultParams().DefaultSendEnabled, exportGenesis.Params.DefaultSendEnabled)
suite.Require().Equal(totalSupply.Total, exportGenesis.Supply)
// add mint module balance as nil
expectedBalances = append(expectedBalances, types.Balance{Address: "cosmos1m3h30wlvsf8llruxtpukdvsy0km2kum8g38c8q", Coins: nil})
suite.Require().Equal(expectedBalances, exportGenesis.Balances)
suite.Require().Equal(expectedMetadata, exportGenesis.DenomMetadata)
}
Expand Down
26 changes: 19 additions & 7 deletions x/bank/keeper/keeper.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package keeper

import (
"time"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/store/prefix"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -107,9 +105,13 @@ func (k BaseKeeper) DelegateCoins(ctx sdk.Context, delegatorAddr, moduleAccAddr
}
}

if err := k.trackDelegation(ctx, delegatorAddr, ctx.BlockHeader().Time, balances, amt); err != nil {
if err := k.trackDelegation(ctx, delegatorAddr, balances, amt); err != nil {
return sdkerrors.Wrap(err, "failed to track delegation")
}
// emit coin spent event
ctx.EventManager().EmitEvent(
types.NewCoinSpentEvent(delegatorAddr, amt),
)

err := k.addCoins(ctx, moduleAccAddr, amt)
if err != nil {
Expand All @@ -134,7 +136,7 @@ func (k BaseKeeper) UndelegateCoins(ctx sdk.Context, moduleAccAddr, delegatorAdd
return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, amt.String())
}

err := k.subtractCoins(ctx, moduleAccAddr, amt)
err := k.subUnlockedCoins(ctx, moduleAccAddr, amt)
if err != nil {
return err
}
Expand Down Expand Up @@ -345,6 +347,11 @@ func (k BaseKeeper) MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins)
logger := k.Logger(ctx)
logger.Info("minted coins from module account", "amount", amt.String(), "from", moduleName)

// emit mint event
ctx.EventManager().EmitEvent(
types.NewCoinMintEvent(acc.GetAddress(), amt),
)

return nil
}

Expand All @@ -360,7 +367,7 @@ func (k BaseKeeper) BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins)
panic(sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "module account %s does not have permissions to burn tokens", moduleName))
}

err := k.subtractCoins(ctx, acc.GetAddress(), amt)
err := k.subUnlockedCoins(ctx, acc.GetAddress(), amt)
if err != nil {
return err
}
Expand All @@ -373,10 +380,15 @@ func (k BaseKeeper) BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins)
logger := k.Logger(ctx)
logger.Info("burned tokens from module account", "amount", amt.String(), "from", moduleName)

// emit burn event
ctx.EventManager().EmitEvent(
types.NewCoinBurnEvent(acc.GetAddress(), amt),
)

return nil
}

func (k BaseKeeper) trackDelegation(ctx sdk.Context, addr sdk.AccAddress, blockTime time.Time, balance, amt sdk.Coins) error {
func (k BaseKeeper) trackDelegation(ctx sdk.Context, addr sdk.AccAddress, balance, amt sdk.Coins) error {
acc := k.ak.GetAccount(ctx, addr)
if acc == nil {
return sdkerrors.Wrapf(sdkerrors.ErrUnknownAddress, "account %s does not exist", addr)
Expand All @@ -385,7 +397,7 @@ func (k BaseKeeper) trackDelegation(ctx sdk.Context, addr sdk.AccAddress, blockT
vacc, ok := acc.(vestexported.VestingAccount)
if ok {
// TODO: return error on account.TrackDelegation
vacc.TrackDelegation(blockTime, balance, amt)
vacc.TrackDelegation(ctx.BlockHeader().Time, balance, amt)
}

return nil
Expand Down
129 changes: 113 additions & 16 deletions x/bank/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,17 +153,17 @@ func (suite *IntegrationTestSuite) TestSupply_SendCoins() {
suite.Require().NoError(
keeper.SendCoinsFromModuleToModule(ctx, holderAcc.GetName(), authtypes.Burner, initCoins),
)
suite.Require().Equal(sdk.Coins(nil), getCoinsByName(ctx, keeper, authKeeper, holderAcc.GetName()))
suite.Require().Equal(sdk.NewCoins().String(), getCoinsByName(ctx, keeper, authKeeper, holderAcc.GetName()).String())
suite.Require().Equal(initCoins, getCoinsByName(ctx, keeper, authKeeper, authtypes.Burner))

suite.Require().NoError(
keeper.SendCoinsFromModuleToAccount(ctx, authtypes.Burner, baseAcc.GetAddress(), initCoins),
)
suite.Require().Equal(sdk.Coins(nil), getCoinsByName(ctx, keeper, authKeeper, authtypes.Burner))
suite.Require().Equal(sdk.NewCoins().String(), getCoinsByName(ctx, keeper, authKeeper, authtypes.Burner).String())
suite.Require().Equal(initCoins, keeper.GetAllBalances(ctx, baseAcc.GetAddress()))

suite.Require().NoError(keeper.SendCoinsFromAccountToModule(ctx, baseAcc.GetAddress(), authtypes.Burner, initCoins))
suite.Require().Equal(sdk.Coins(nil), keeper.GetAllBalances(ctx, baseAcc.GetAddress()))
suite.Require().Equal(sdk.NewCoins().String(), keeper.GetAllBalances(ctx, baseAcc.GetAddress()).String())
suite.Require().Equal(initCoins, getCoinsByName(ctx, keeper, authKeeper, authtypes.Burner))
}

Expand Down Expand Up @@ -266,7 +266,7 @@ func (suite *IntegrationTestSuite) TestSupply_BurnCoins() {

err = keeper.BurnCoins(ctx, authtypes.Burner, initCoins)
suite.Require().NoError(err)
suite.Require().Equal(sdk.Coins(nil), getCoinsByName(ctx, keeper, authKeeper, authtypes.Burner))
suite.Require().Equal(sdk.NewCoins().String(), getCoinsByName(ctx, keeper, authKeeper, authtypes.Burner).String())
suite.Require().Equal(supplyAfterInflation.GetTotal().Sub(initCoins), keeper.GetSupply(ctx).GetTotal())

// test same functionality on module account with multiple permissions
Expand All @@ -280,7 +280,7 @@ func (suite *IntegrationTestSuite) TestSupply_BurnCoins() {

err = keeper.BurnCoins(ctx, multiPermAcc.GetName(), initCoins)
suite.Require().NoError(err)
suite.Require().Equal(sdk.Coins(nil), getCoinsByName(ctx, keeper, authKeeper, multiPermAcc.GetName()))
suite.Require().Equal(sdk.NewCoins().String(), getCoinsByName(ctx, keeper, authKeeper, multiPermAcc.GetName()).String())
suite.Require().Equal(supplyAfterInflation.GetTotal().Sub(initCoins), keeper.GetSupply(ctx).GetTotal())
}

Expand Down Expand Up @@ -537,6 +537,7 @@ func (suite *IntegrationTestSuite) TestMsgSendEvents() {
event1.Attributes,
abci.EventAttribute{Key: []byte(sdk.AttributeKeyAmount), Value: []byte(newCoins.String())},
)

event2 := sdk.Event{
Type: sdk.EventTypeMessage,
Attributes: []abci.EventAttribute{},
Expand All @@ -556,9 +557,9 @@ func (suite *IntegrationTestSuite) TestMsgSendEvents() {

// events are shifted due to the funding account events
events = ctx.EventManager().ABCIEvents()
suite.Require().Equal(6, len(events))
suite.Require().Equal(abci.Event(event1), events[4])
suite.Require().Equal(abci.Event(event2), events[5])
suite.Require().Equal(12, len(events))
suite.Require().Equal(abci.Event(event1), events[8])
suite.Require().Equal(abci.Event(event2), events[9])
}

func (suite *IntegrationTestSuite) TestMsgMultiSendEvents() {
Expand Down Expand Up @@ -597,7 +598,7 @@ func (suite *IntegrationTestSuite) TestMsgMultiSendEvents() {
suite.Require().Error(app.BankKeeper.InputOutputCoins(ctx, inputs, outputs))

events = ctx.EventManager().ABCIEvents()
suite.Require().Equal(3, len(events)) // 3 events because minting event is there
suite.Require().Equal(8, len(events)) // 7 events because account funding causes extra minting + coin_spent + coin_recv events

event1 := sdk.Event{
Type: sdk.EventTypeMessage,
Expand All @@ -607,7 +608,7 @@ func (suite *IntegrationTestSuite) TestMsgMultiSendEvents() {
event1.Attributes,
abci.EventAttribute{Key: []byte(types.AttributeKeySender), Value: []byte(addr.String())},
)
suite.Require().Equal(abci.Event(event1), events[2]) // it's the third event since we have the minting event before
suite.Require().Equal(abci.Event(event1), events[7])

// Set addr's coins and addr2's coins
suite.Require().NoError(simapp.FundAccount(app, ctx, addr, sdk.NewCoins(sdk.NewInt64Coin(fooDenom, 50))))
Expand All @@ -619,7 +620,7 @@ func (suite *IntegrationTestSuite) TestMsgMultiSendEvents() {
suite.Require().NoError(app.BankKeeper.InputOutputCoins(ctx, inputs, outputs))

events = ctx.EventManager().ABCIEvents()
suite.Require().Equal(11, len(events))
suite.Require().Equal(28, len(events)) // 25 due to account funding + coin_spent + coin_recv events

event2 := sdk.Event{
Type: sdk.EventTypeMessage,
Expand Down Expand Up @@ -652,12 +653,11 @@ func (suite *IntegrationTestSuite) TestMsgMultiSendEvents() {
event4.Attributes,
abci.EventAttribute{Key: []byte(sdk.AttributeKeyAmount), Value: []byte(newCoins2.String())},
)

// events are shifted due to the funding account events
suite.Require().Equal(abci.Event(event1), events[7])
suite.Require().Equal(abci.Event(event2), events[8])
suite.Require().Equal(abci.Event(event3), events[9])
suite.Require().Equal(abci.Event(event4), events[10])
suite.Require().Equal(abci.Event(event1), events[21])
suite.Require().Equal(abci.Event(event2), events[23])
suite.Require().Equal(abci.Event(event3), events[25])
suite.Require().Equal(abci.Event(event4), events[27])
}

func (suite *IntegrationTestSuite) TestSpendableCoins() {
Expand Down Expand Up @@ -1002,6 +1002,103 @@ func (suite *IntegrationTestSuite) TestIterateAllDenomMetaData() {
}
}

func (suite *IntegrationTestSuite) TestBalanceTrackingEvents() {
// replace account keeper and bank keeper otherwise the account keeper won't be aware of the
// existence of the new module account because GetModuleAccount checks for the existence via
// permissions map and not via state... weird
maccPerms := simapp.GetMaccPerms()
maccPerms[multiPerm] = []string{authtypes.Burner, authtypes.Minter, authtypes.Staking}

suite.app.AccountKeeper = authkeeper.NewAccountKeeper(
suite.app.AppCodec(), suite.app.GetKey(authtypes.StoreKey), suite.app.GetSubspace(authtypes.ModuleName),
authtypes.ProtoBaseAccount, maccPerms,
)

suite.app.BankKeeper = keeper.NewBaseKeeper(suite.app.AppCodec(), suite.app.GetKey(types.StoreKey),
suite.app.AccountKeeper, suite.app.GetSubspace(types.ModuleName), nil)

// set account with multiple permissions
suite.app.AccountKeeper.SetModuleAccount(suite.ctx, multiPermAcc)
// mint coins
suite.Require().NoError(
suite.app.BankKeeper.MintCoins(
suite.ctx,
multiPermAcc.Name,
sdk.NewCoins(sdk.NewCoin("utxo", sdk.NewInt(100000)))),
)
// send coins to address
addr1 := sdk.AccAddress("addr1_______________")
suite.Require().NoError(
suite.app.BankKeeper.SendCoinsFromModuleToAccount(
suite.ctx,
multiPermAcc.Name,
addr1,
sdk.NewCoins(sdk.NewCoin("utxo", sdk.NewInt(50000))),
),
)

// burn coins from module account
suite.Require().NoError(
suite.app.BankKeeper.BurnCoins(
suite.ctx,
multiPermAcc.Name,
sdk.NewCoins(sdk.NewInt64Coin("utxo", 1000)),
),
)

// process balances and supply from events
supply := sdk.NewCoins()

balances := make(map[string]sdk.Coins)

for _, e := range suite.ctx.EventManager().ABCIEvents() {
switch e.Type {
case types.EventTypeCoinBurn:
burnedCoins, err := sdk.ParseCoinsNormalized((string)(e.Attributes[1].Value))
suite.Require().NoError(err)
supply = supply.Sub(burnedCoins)

case types.EventTypeCoinMint:
mintedCoins, err := sdk.ParseCoinsNormalized((string)(e.Attributes[1].Value))
suite.Require().NoError(err)
supply = supply.Add(mintedCoins...)

case types.EventTypeCoinSpent:
coinsSpent, err := sdk.ParseCoinsNormalized((string)(e.Attributes[1].Value))
suite.Require().NoError(err)
spender, err := sdk.AccAddressFromBech32((string)(e.Attributes[0].Value))
suite.Require().NoError(err)
balances[spender.String()] = balances[spender.String()].Sub(coinsSpent)

case types.EventTypeCoinReceived:
coinsRecv, err := sdk.ParseCoinsNormalized((string)(e.Attributes[1].Value))
suite.Require().NoError(err)
receiver, err := sdk.AccAddressFromBech32((string)(e.Attributes[0].Value))
suite.Require().NoError(err)
balances[receiver.String()] = balances[receiver.String()].Add(coinsRecv...)
}
}

// check balance and supply tracking
savedSupply := suite.app.BankKeeper.GetSupply(suite.ctx)
utxoSupply := savedSupply.GetTotal().AmountOf("utxo")
suite.Require().Equal(utxoSupply, supply.AmountOf("utxo"))
// iterate accounts and check balances
suite.app.BankKeeper.IterateAllBalances(suite.ctx, func(address sdk.AccAddress, coin sdk.Coin) (stop bool) {
// if it's not utxo coin then skip
if coin.Denom != "utxo" {
return false
}

balance, exists := balances[address.String()]
suite.Require().True(exists)

expectedUtxo := sdk.NewCoin("utxo", balance.AmountOf(coin.Denom))
suite.Require().Equal(expectedUtxo.String(), coin.String())
return false
})
}

func (suite *IntegrationTestSuite) getTestMetadata() []types.Metadata {
return []types.Metadata{{
Name: "Cosmos Hub Atom",
Expand Down
Loading

0 comments on commit ef9968d

Please sign in to comment.