Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bank]: add balance tracking events #8656

Merged
merged 27 commits into from
Feb 25, 2021
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
7342b03
change(bank): add utxo events and simplify logic
fdymylja Feb 22, 2021
294d831
add(bank): balance and supply tracking test
fdymylja Feb 22, 2021
d015e24
chore(bank): fix balance tracking test comment
fdymylja Feb 22, 2021
5691a26
Merge branch 'master' into fdymylja/balance-tracking-events
fdymylja Feb 22, 2021
24e3f43
Merge branch 'fdymylja/balance-tracking-events' of https://github.com…
fdymylja Feb 22, 2021
af8c845
fix(grpc): service test
fdymylja Feb 22, 2021
c06b683
fix(bank): sub unlocked coins to use less gas
fdymylja Feb 22, 2021
37a6520
fix(auth): cli test gas
fdymylja Feb 22, 2021
2519c2d
fix(rest): grpc gas test
fdymylja Feb 22, 2021
e95b7fe
fix(staking/cli): increase delegation required gas
fdymylja Feb 22, 2021
c0ead6a
add: burn events, fix tests
fdymylja Feb 22, 2021
419d662
fix(auth/tx): grpc test
fdymylja Feb 22, 2021
eeaa9ae
add(bank): coin events in delegate
fdymylja Feb 22, 2021
4dda459
fix(bank): add amt check in delegate coins back
fdymylja Feb 22, 2021
f800f55
change(bank): add coin spent and coin recv events in burn and mint
fdymylja Feb 22, 2021
9dd38b5
change(bank): revert sub coin function
fdymylja Feb 23, 2021
dd132d9
change(auth): revert cli test
fdymylja Feb 23, 2021
173835f
change(auth): revert service test
fdymylja Feb 23, 2021
47a0d82
chore(auth): fix events comment in service_test.go
fdymylja Feb 23, 2021
a274c9f
chore: update CHANGELOG.md
fdymylja Feb 25, 2021
f561196
Merge remote-tracking branch 'origin/master' into fdymylja/balance-tr…
fdymylja Feb 25, 2021
8c15488
Merge branch 'master' into fdymylja/balance-tracking-events
fdymylja Feb 25, 2021
9cb2e67
Merge branch 'fdymylja/balance-tracking-events' of https://github.com…
fdymylja Feb 25, 2021
ee4a2e4
remove(bank): balanceError func
fdymylja Feb 25, 2021
63e6fcb
chore(bank): address lint warnings
fdymylja Feb 25, 2021
ea4927a
chore(bank): update events spec
fdymylja Feb 25, 2021
4e87b31
Merge branch 'master' into fdymylja/balance-tracking-events
mergify[bot] Feb 25, 2021
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 @@ -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