Merge PR #4681: Add check before minting empty coins

.pending/bugfixes/sdk/4681-mint-check

@@ -0,0 +1,2 @@
+#4681 panic on invalid amount on `MintCoins` and `BurnCoins`
+  - skip minting if inflation is set to zero

x/mint/internal/keeper/keeper.go

@@ -97,6 +97,10 @@ func (k Keeper) BondedRatio(ctx sdk.Context) sdk.Dec {
 // MintCoins implements an alias call to the underlying supply keeper's
 // MintCoins to be used in BeginBlocker.
 func (k Keeper) MintCoins(ctx sdk.Context, newCoins sdk.Coins) sdk.Error {
+	if newCoins.Empty() {
+		// skip as no coins need to be minted
+		return nil
+	}
 	return k.supplyKeeper.MintCoins(ctx, types.ModuleName, newCoins)
 }
 

x/staking/keeper/pool.go

@@ -37,6 +37,7 @@ func (k Keeper) notBondedTokensToBonded(ctx sdk.Context, tokens sdk.Int) {
 // burnBondedTokens removes coins from the bonded pool module account
 func (k Keeper) burnBondedTokens(ctx sdk.Context, amt sdk.Int) sdk.Error {
 	if !amt.IsPositive() {
+		// skip as no coins need to be burned
 		return nil
 	}
 	coins := sdk.NewCoins(sdk.NewCoin(k.BondDenom(ctx), amt))
@@ -46,6 +47,7 @@ func (k Keeper) burnBondedTokens(ctx sdk.Context, amt sdk.Int) sdk.Error {
 // burnNotBondedTokens removes coins from the not bonded pool module account
 func (k Keeper) burnNotBondedTokens(ctx sdk.Context, amt sdk.Int) sdk.Error {
 	if !amt.IsPositive() {
+		// skip as no coins need to be burned
 		return nil
 	}
 	coins := sdk.NewCoins(sdk.NewCoin(k.BondDenom(ctx), amt))

x/supply/internal/keeper/account.go

@@ -6,7 +6,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/supply/internal/types"
 )
 
-// GetModuleAddress returns a an address  based on the name
+// GetModuleAddress returns a an address based on the module name
 func (k Keeper) GetModuleAddress(moduleName string) sdk.AccAddress {
 	permAddr, ok := k.permAddrs[moduleName]
 	if !ok {
@@ -15,7 +15,7 @@ func (k Keeper) GetModuleAddress(moduleName string) sdk.AccAddress {
 	return permAddr.address
 }
 
-// GetModuleAddressAndPermission returns an address and permission  based on the name
+// GetModuleAddressAndPermission returns an address and permission based on the module name
 func (k Keeper) GetModuleAddressAndPermission(moduleName string) (addr sdk.AccAddress, permission string) {
 	permAddr, ok := k.permAddrs[moduleName]
 	if !ok {
@@ -24,7 +24,8 @@ func (k Keeper) GetModuleAddressAndPermission(moduleName string) (addr sdk.AccAd
 	return permAddr.address, permAddr.permission
 }
 
-// GetModuleAccount gets the module account to the auth account store
+// GetModuleAccountAndPermission gets the module account from the auth account store and its
+// registered permission
 func (k Keeper) GetModuleAccountAndPermission(ctx sdk.Context, moduleName string) (exported.ModuleAccountI, string) {
 	addr, perm := k.GetModuleAddressAndPermission(moduleName)
 	if addr == nil {
@@ -48,7 +49,7 @@ func (k Keeper) GetModuleAccountAndPermission(ctx sdk.Context, moduleName string
 	return maccI, perm
 }
 
-// GetModuleAccount gets the module account to the auth account store
+// GetModuleAccount gets the module account from the auth account store
 func (k Keeper) GetModuleAccount(ctx sdk.Context, moduleName string) exported.ModuleAccountI {
 	acc, _ := k.GetModuleAccountAndPermission(ctx, moduleName)
 	return acc

x/supply/internal/keeper/bank.go

@@ -28,12 +28,12 @@ func (k Keeper) SendCoinsFromModuleToModule(ctx sdk.Context, senderModule, recip
 	}
 
 	// create the account if it doesn't yet exist
-	recipientAddr := k.GetModuleAccount(ctx, recipientModule).GetAddress()
+	recipientAcc := k.GetModuleAccount(ctx, recipientModule)
-	if recipientAddr == nil {
+	if recipientAcc == nil {
 		panic(fmt.Sprintf("module account %s isn't able to be created", recipientModule))
 	}
 
-	return k.bk.SendCoins(ctx, senderAddr, recipientAddr, amt)
+	return k.bk.SendCoins(ctx, senderAddr, recipientAcc.GetAddress(), amt)
 }
 
 // SendCoinsFromAccountToModule transfers coins from an AccAddress to a ModuleAccount
@@ -41,12 +41,12 @@ func (k Keeper) SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.Acc
 	recipientModule string, amt sdk.Coins) sdk.Error {
 
 	// create the account if it doesn't yet exist
-	recipientAddr := k.GetModuleAccount(ctx, recipientModule).GetAddress()
+	recipientAcc := k.GetModuleAccount(ctx, recipientModule)
-	if recipientAddr == nil {
+	if recipientAcc == nil {
 		panic(fmt.Sprintf("module account %s isn't able to be created", recipientModule))
 	}
 
-	return k.bk.SendCoins(ctx, senderAddr, recipientAddr, amt)
+	return k.bk.SendCoins(ctx, senderAddr, recipientAcc.GetAddress(), amt)
 }
 
 // DelegateCoinsFromAccountToModule delegates coins and transfers
@@ -55,12 +55,12 @@ func (k Keeper) DelegateCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk
 	recipientModule string, amt sdk.Coins) sdk.Error {
 
 	// create the account if it doesn't yet exist
-	recipientAddr := k.GetModuleAccount(ctx, recipientModule).GetAddress()
+	recipientAcc := k.GetModuleAccount(ctx, recipientModule)
-	if recipientAddr == nil {
+	if recipientAcc == nil {
 		panic(fmt.Sprintf("module account %s isn't able to be created", recipientModule))
 	}
 
-	return k.bk.DelegateCoins(ctx, senderAddr, recipientAddr, amt)
+	return k.bk.DelegateCoins(ctx, senderAddr, recipientAcc.GetAddress(), amt)
 }
 
 // UndelegateCoinsFromModuleToAccount undelegates the unbonding coins and transfers
@@ -76,27 +76,25 @@ func (k Keeper) UndelegateCoinsFromModuleToAccount(ctx sdk.Context, senderModule
 	return k.bk.UndelegateCoins(ctx, senderAddr, recipientAddr, amt)
 }
 
-// MintCoins creates new coins from thin air and adds it to the MinterAccount.
+// MintCoins creates new coins from thin air and adds it to the module account.
-// Panics if the name maps to a non-minter module account.
+// Panics if the name maps to a non-minter module account or if the amount is invalid.
 func (k Keeper) MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) sdk.Error {
-	if amt.Empty() {
-		panic("cannot mint empty coins")
-	}
 
 	// create the account if it doesn't yet exist
 	acc, perm := k.GetModuleAccountAndPermission(ctx, moduleName)
-	addr := acc.GetAddress()
+	if acc == nil {
-	if addr == nil {
 		return sdk.ErrUnknownAddress(fmt.Sprintf("module account %s does not exist", moduleName))
 	}
 
+	addr := acc.GetAddress()
+
 	if perm != types.Minter {
-		panic(fmt.Sprintf("Account %s does not have permissions to mint tokens", moduleName))
+		panic(fmt.Sprintf("module account %s does not have permissions to mint tokens", moduleName))
 	}
 
 	_, err := k.bk.AddCoins(ctx, addr, amt)
 	if err != nil {
-		return err
+		panic(err)
 	}
 
 	// update total supply
@@ -110,11 +108,9 @@ func (k Keeper) MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) sdk
 	return nil
 }
 
-// BurnCoins burns coins deletes coins from the balance of the module account
+// BurnCoins burns coins deletes coins from the balance of the module account.
+// Panics if the name maps to a non-burner module account or if the amount is invalid.
 func (k Keeper) BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) sdk.Error {
-	if amt.Empty() {
-		panic("cannot burn empty coins")
-	}
 
 	addr, perm := k.GetModuleAddressAndPermission(moduleName)
 	if addr == nil {
@@ -122,12 +118,12 @@ func (k Keeper) BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) sdk
 	}
 
 	if perm != types.Burner {
-		panic(fmt.Sprintf("Account %s does not have permissions to burn tokens", moduleName))
+		panic(fmt.Sprintf("module account %s does not have permissions to burn tokens", moduleName))
 	}
 
 	_, err := k.bk.SubtractCoins(ctx, addr, amt)
 	if err != nil {
-		return err
+		panic(err)
 	}
 
 	// update total supply

x/supply/internal/keeper/bank_test.go

@@ -81,35 +81,32 @@ func TestMintCoins(t *testing.T) {
 
 	initialSupply := keeper.GetSupply(ctx)
 
-	require.Panics(t, func() { keeper.MintCoins(ctx, "", initCoins) })
+	require.Error(t, keeper.MintCoins(ctx, "", initCoins), "no module account")
+	require.Panics(t, func() { keeper.MintCoins(ctx, types.Burner, initCoins) }, "invalid permission")
+	require.Panics(t, func() { keeper.MintCoins(ctx, types.Minter, sdk.Coins{sdk.Coin{"denom", sdk.NewInt(-10)}}) }, "insufficient coins") //nolint
 
 	err := keeper.MintCoins(ctx, types.Minter, initCoins)
 	require.NoError(t, err)
 	require.Equal(t, initCoins, getCoinsByName(ctx, keeper, types.Minter))
 	require.Equal(t, initialSupply.Total.Add(initCoins), keeper.GetSupply(ctx).Total)
-
-	require.Panics(t, func() { keeper.MintCoins(ctx, types.Burner, initCoins) })
 }
 
 func TestBurnCoins(t *testing.T) {
 	nAccs := int64(4)
 	ctx, _, keeper := createTestInput(t, false, initialPower, nAccs)
 
-	err := burnerAcc.SetCoins(initCoins)
+	require.NoError(t, burnerAcc.SetCoins(initCoins))
-	require.NoError(t, err)
 	keeper.SetModuleAccount(ctx, burnerAcc)
 
 	initialSupply := keeper.GetSupply(ctx)
 	initialSupply.Inflate(initCoins)
 	keeper.SetSupply(ctx, initialSupply)
 
-	err = keeper.BurnCoins(ctx, "", initCoins)
+	require.Error(t, keeper.BurnCoins(ctx, "", initCoins), "no module account")
-	require.Error(t, err)
+	require.Panics(t, func() { keeper.BurnCoins(ctx, types.Minter, initCoins) }, "invalid permission")
+	require.Panics(t, func() { keeper.BurnCoins(ctx, types.Burner, initialSupply.Total) }, "insufficient coins")
 
-	err = keeper.BurnCoins(ctx, types.Burner, initialSupply.Total)
+	err := keeper.BurnCoins(ctx, types.Burner, initCoins)
-	require.Error(t, err)
-
-	err = keeper.BurnCoins(ctx, types.Burner, initCoins)
 	require.NoError(t, err)
 	require.Equal(t, sdk.Coins(nil), getCoinsByName(ctx, keeper, types.Burner))
 	require.Equal(t, initialSupply.Total.Sub(initCoins), keeper.GetSupply(ctx).Total)