diff --git a/CHANGELOG.md b/CHANGELOG.md index ae290136e41b..3228549a7c13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -134,6 +134,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/bank) [\#8434](https://github.com/cosmos/cosmos-sdk/pull/8434) Fix legacy REST API `GET /bank/total` and `GET /bank/total/{denom}` in swagger * (x/slashing) [\#8427](https://github.com/cosmos/cosmos-sdk/pull/8427) Fix query signing infos command * (server) [\#8399](https://github.com/cosmos/cosmos-sdk/pull/8399) fix gRPC-web flag default value +* (x/bank) [\#9229](https://github.com/cosmos/cosmos-sdk/pull/9229) Now zero coin balances cannot be added to balances & supply stores. If any denom becomes zero corresponding key gets deleted from store. ### Deprecated diff --git a/x/bank/keeper/genesis_test.go b/x/bank/keeper/genesis_test.go index 5adb2174133a..70d53b9f0430 100644 --- a/x/bank/keeper/genesis_test.go +++ b/x/bank/keeper/genesis_test.go @@ -33,8 +33,6 @@ func (suite *IntegrationTestSuite) TestExportGenesis() { suite.Require().Len(exportGenesis.Params.SendEnabled, 0) suite.Require().Equal(types.DefaultParams().DefaultSendEnabled, exportGenesis.Params.DefaultSendEnabled) suite.Require().Equal(totalSupply, 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) } diff --git a/x/bank/keeper/keeper.go b/x/bank/keeper/keeper.go index c32aafb28793..c02b11a17eae 100644 --- a/x/bank/keeper/keeper.go +++ b/x/bank/keeper/keeper.go @@ -67,7 +67,9 @@ func (k BaseKeeper) GetPaginatedTotalSupply(ctx sdk.Context, pagination *query.P if err != nil { return fmt.Errorf("unable to convert amount string to Int %v", err) } - supply = append(supply, sdk.NewCoin(string(key), amount)) + + // `Add` omits the 0 coins addition to the `supply`. + supply = supply.Add(sdk.NewCoin(string(key), amount)) return nil }) @@ -124,7 +126,7 @@ func (k BaseKeeper) DelegateCoins(ctx sdk.Context, delegatorAddr, moduleAccAddr balances := sdk.NewCoins() for _, coin := range amt { - balance := k.GetBalance(ctx, delegatorAddr, coin.Denom) + balance := k.GetBalance(ctx, delegatorAddr, coin.GetDenom()) if balance.IsLT(coin) { return sdkerrors.Wrapf( sdkerrors.ErrInsufficientFunds, "failed to delegate; %s is smaller than %s", balance, amt, @@ -426,14 +428,20 @@ func (k BaseKeeper) BurnCoins(ctx sdk.Context, moduleName string, amounts sdk.Co } func (k BaseKeeper) setSupply(ctx sdk.Context, coin sdk.Coin) { - store := ctx.KVStore(k.storeKey) - supplyStore := prefix.NewStore(store, types.SupplyKey) - intBytes, err := coin.Amount.Marshal() if err != nil { panic(fmt.Errorf("unable to marshal amount value %v", err)) } - supplyStore.Set([]byte(coin.GetDenom()), intBytes) + + store := ctx.KVStore(k.storeKey) + supplyStore := prefix.NewStore(store, types.SupplyKey) + + // Bank invariants and IBC requires to remove zero coins. + if coin.IsZero() { + supplyStore.Delete([]byte(coin.GetDenom())) + } else { + supplyStore.Set([]byte(coin.GetDenom()), intBytes) + } } func (k BaseKeeper) trackDelegation(ctx sdk.Context, addr sdk.AccAddress, balance, amt sdk.Coins) error { diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index 15672f4556b4..e20ce3e21f19 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -72,6 +72,28 @@ type IntegrationTestSuite struct { queryClient types.QueryClient } +func (suite *IntegrationTestSuite) initKeepersWithmAccPerms(blockedAddrs map[string]bool) (authkeeper.AccountKeeper, keeper.BaseKeeper) { + app := suite.app + maccPerms := simapp.GetMaccPerms() + appCodec := simapp.MakeTestEncodingConfig().Marshaler + + maccPerms[holder] = nil + maccPerms[authtypes.Burner] = []string{authtypes.Burner} + maccPerms[authtypes.Minter] = []string{authtypes.Minter} + maccPerms[multiPerm] = []string{authtypes.Burner, authtypes.Minter, authtypes.Staking} + maccPerms[randomPerm] = []string{"random"} + authKeeper := authkeeper.NewAccountKeeper( + appCodec, app.GetKey(types.StoreKey), app.GetSubspace(types.ModuleName), + authtypes.ProtoBaseAccount, maccPerms, + ) + keeper := keeper.NewBaseKeeper( + appCodec, app.GetKey(types.StoreKey), authKeeper, + app.GetSubspace(types.ModuleName), blockedAddrs, + ) + + return authKeeper, keeper +} + func (suite *IntegrationTestSuite) SetupTest() { app := simapp.Setup(false) ctx := app.BaseApp.NewContext(false, tmproto.Header{}) @@ -89,42 +111,41 @@ func (suite *IntegrationTestSuite) SetupTest() { } func (suite *IntegrationTestSuite) TestSupply() { - app, ctx := suite.app, suite.ctx + _, ctx := suite.app, suite.ctx + + require := suite.Require() + + // add module accounts to supply keeper + authKeeper, keeper := suite.initKeepersWithmAccPerms(make(map[string]bool)) initialPower := int64(100) initTokens := suite.app.StakingKeeper.TokensFromConsensusPower(suite.ctx, initialPower) - totalSupply := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, initTokens)) - suite.NoError(app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, totalSupply)) - total, _, err := app.BankKeeper.GetPaginatedTotalSupply(ctx, &query.PageRequest{}) - suite.Require().NoError(err) - suite.Require().Equal(totalSupply, total) + // set burnerAcc balance + authKeeper.SetModuleAccount(ctx, burnerAcc) + require.NoError(keeper.MintCoins(ctx, authtypes.Minter, totalSupply)) + require.NoError(keeper.SendCoinsFromModuleToAccount(ctx, authtypes.Minter, burnerAcc.GetAddress(), totalSupply)) + + total, _, err := keeper.GetPaginatedTotalSupply(ctx, &query.PageRequest{}) + require.NoError(err) + require.Equal(totalSupply, total) + + // burning all supplied tokens + err = keeper.BurnCoins(ctx, authtypes.Burner, totalSupply) + require.NoError(err) + + total, _, err = keeper.GetPaginatedTotalSupply(ctx, &query.PageRequest{}) + require.NoError(err) + require.Equal(total.String(), "") } func (suite *IntegrationTestSuite) TestSendCoinsFromModuleToAccount_Blacklist() { - app := simapp.Setup(false) - ctx := app.BaseApp.NewContext(false, tmproto.Header{Height: 1}) - appCodec := app.AppCodec() + ctx := suite.ctx // add module accounts to supply keeper - maccPerms := simapp.GetMaccPerms() - maccPerms[holder] = nil - maccPerms[authtypes.Burner] = []string{authtypes.Burner} - maccPerms[authtypes.Minter] = []string{authtypes.Minter} - maccPerms[multiPerm] = []string{authtypes.Burner, authtypes.Minter, authtypes.Staking} - maccPerms[randomPerm] = []string{"random"} - addr1 := sdk.AccAddress([]byte("addr1_______________")) - - authKeeper := authkeeper.NewAccountKeeper( - appCodec, app.GetKey(types.StoreKey), app.GetSubspace(types.ModuleName), - authtypes.ProtoBaseAccount, maccPerms, - ) - keeper := keeper.NewBaseKeeper( - appCodec, app.GetKey(types.StoreKey), authKeeper, - app.GetSubspace(types.ModuleName), map[string]bool{addr1.String(): true}, - ) + _, keeper := suite.initKeepersWithmAccPerms(map[string]bool{addr1.String(): true}) suite.Require().NoError(keeper.MintCoins(ctx, minttypes.ModuleName, initCoins)) suite.Require().Error(keeper.SendCoinsFromModuleToAccount( @@ -133,26 +154,10 @@ func (suite *IntegrationTestSuite) TestSendCoinsFromModuleToAccount_Blacklist() } func (suite *IntegrationTestSuite) TestSupply_SendCoins() { - app := simapp.Setup(false) - ctx := app.BaseApp.NewContext(false, tmproto.Header{Height: 1}) - appCodec := app.AppCodec() + ctx := suite.ctx // add module accounts to supply keeper - maccPerms := simapp.GetMaccPerms() - maccPerms[holder] = nil - maccPerms[authtypes.Burner] = []string{authtypes.Burner} - maccPerms[authtypes.Minter] = []string{authtypes.Minter} - maccPerms[multiPerm] = []string{authtypes.Burner, authtypes.Minter, authtypes.Staking} - maccPerms[randomPerm] = []string{"random"} - - authKeeper := authkeeper.NewAccountKeeper( - appCodec, app.GetKey(types.StoreKey), app.GetSubspace(types.ModuleName), - authtypes.ProtoBaseAccount, maccPerms, - ) - keeper := keeper.NewBaseKeeper( - appCodec, app.GetKey(types.StoreKey), authKeeper, - app.GetSubspace(types.ModuleName), make(map[string]bool), - ) + authKeeper, keeper := suite.initKeepersWithmAccPerms(make(map[string]bool)) baseAcc := authKeeper.NewAccountWithAddress(ctx, authtypes.NewModuleAddress("baseAcc")) @@ -203,26 +208,10 @@ func (suite *IntegrationTestSuite) TestSupply_SendCoins() { } func (suite *IntegrationTestSuite) TestSupply_MintCoins() { - app := simapp.Setup(false) - ctx := app.BaseApp.NewContext(false, tmproto.Header{Height: 1}) - appCodec := app.AppCodec() + ctx := suite.ctx // add module accounts to supply keeper - maccPerms := simapp.GetMaccPerms() - maccPerms[holder] = nil - maccPerms[authtypes.Burner] = []string{authtypes.Burner} - maccPerms[authtypes.Minter] = []string{authtypes.Minter} - maccPerms[multiPerm] = []string{authtypes.Burner, authtypes.Minter, authtypes.Staking} - maccPerms[randomPerm] = []string{"random"} - - authKeeper := authkeeper.NewAccountKeeper( - appCodec, app.GetKey(types.StoreKey), app.GetSubspace(types.ModuleName), - authtypes.ProtoBaseAccount, maccPerms, - ) - keeper := keeper.NewBaseKeeper( - appCodec, app.GetKey(types.StoreKey), authKeeper, - app.GetSubspace(types.ModuleName), make(map[string]bool), - ) + authKeeper, keeper := suite.initKeepersWithmAccPerms(make(map[string]bool)) authKeeper.SetModuleAccount(ctx, burnerAcc) authKeeper.SetModuleAccount(ctx, minterAcc) @@ -264,26 +253,9 @@ func (suite *IntegrationTestSuite) TestSupply_MintCoins() { } func (suite *IntegrationTestSuite) TestSupply_BurnCoins() { - app := simapp.Setup(false) - ctx := app.BaseApp.NewContext(false, tmproto.Header{Height: 1}) - appCodec := simapp.MakeTestEncodingConfig().Marshaler - + ctx := suite.ctx // add module accounts to supply keeper - maccPerms := simapp.GetMaccPerms() - maccPerms[holder] = nil - maccPerms[authtypes.Burner] = []string{authtypes.Burner} - maccPerms[authtypes.Minter] = []string{authtypes.Minter} - maccPerms[multiPerm] = []string{authtypes.Burner, authtypes.Minter, authtypes.Staking} - maccPerms[randomPerm] = []string{"random"} - - authKeeper := authkeeper.NewAccountKeeper( - appCodec, app.GetKey(types.StoreKey), app.GetSubspace(types.ModuleName), - authtypes.ProtoBaseAccount, maccPerms, - ) - keeper := keeper.NewBaseKeeper( - appCodec, app.GetKey(types.StoreKey), authKeeper, - app.GetSubspace(types.ModuleName), make(map[string]bool), - ) + authKeeper, keeper := suite.initKeepersWithmAccPerms(make(map[string]bool)) // set burnerAcc balance authKeeper.SetModuleAccount(ctx, burnerAcc) @@ -349,11 +321,15 @@ func (suite *IntegrationTestSuite) TestSendCoinsNewAccount() { app.BankKeeper.GetAllBalances(ctx, addr2) suite.Require().Empty(app.BankKeeper.GetAllBalances(ctx, addr2)) - sendAmt := sdk.NewCoins(newFooCoin(50), newBarCoin(25)) + sendAmt := sdk.NewCoins(newFooCoin(50), newBarCoin(50)) suite.Require().NoError(app.BankKeeper.SendCoins(ctx, addr1, addr2, sendAmt)) acc2Balances := app.BankKeeper.GetAllBalances(ctx, addr2) + acc1Balances = app.BankKeeper.GetAllBalances(ctx, addr1) suite.Require().Equal(sendAmt, acc2Balances) + updatedAcc1Bal := balances.Sub(sendAmt) + suite.Require().Len(acc1Balances, len(updatedAcc1Bal)) + suite.Require().Equal(acc1Balances, updatedAcc1Bal) suite.Require().NotNil(app.AccountKeeper.GetAccount(ctx, addr2)) } diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index e701ff4dcab6..8fd7e602a4e7 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -266,8 +266,13 @@ func (k BaseSendKeeper) setBalance(ctx sdk.Context, addr sdk.AccAddress, balance accountStore := k.getAccountStore(ctx, addr) - bz := k.cdc.MustMarshal(&balance) - accountStore.Set([]byte(balance.Denom), bz) + // Bank invariants require to not store zero balances. + if balance.IsZero() { + accountStore.Delete([]byte(balance.Denom)) + } else { + bz := k.cdc.MustMarshal(&balance) + accountStore.Set([]byte(balance.Denom), bz) + } return nil }