Merge PR #7327: remove resulting value return from AddCoins and Subtr…

…actCoins
cosmos · Sep 16, 2020 · b2d48a9 · b2d48a9
1 parent 320a852
commit b2d48a9
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 31 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -67,6 +67,7 @@ older clients.
 
 ### API Breaking Changes
 
+* (x/bank) [\#7327](https://github.com/cosmos/cosmos-sdk/pull/7327) AddCoins and SubtractCoins no longer return a resultingValue and will only return an error.
 * (x/evidence) [\#7251](https://github.com/cosmos/cosmos-sdk/pull/7251) New evidence types and light client evidence handling. The module function names changed.
 * (modules) [\#6564](https://github.com/cosmos/cosmos-sdk/pull/6564) Constant `DefaultParamspace` is removed from all modules, use ModuleName instead.
 * (client) [\#6525](https://github.com/cosmos/cosmos-sdk/pull/6525) Removed support for `indent` in JSON responses. Clients should consider piping to an external tool such as `jq`.

diff --git a/simapp/test_helpers.go b/simapp/test_helpers.go
@@ -269,7 +269,7 @@ func addTestAddrs(app *SimApp, ctx sdk.Context, accNum int, accAmt sdk.Int, stra
 func saveAccount(app *SimApp, ctx sdk.Context, addr sdk.AccAddress, initCoins sdk.Coins) {
 	acc := app.AccountKeeper.NewAccountWithAddress(ctx, addr)
 	app.AccountKeeper.SetAccount(ctx, acc)
-	_, err := app.BankKeeper.AddCoins(ctx, addr, initCoins)
+	err := app.BankKeeper.AddCoins(ctx, addr, initCoins)
 	if err != nil {
 		panic(err)
 	}

diff --git a/x/bank/keeper/keeper.go b/x/bank/keeper/keeper.go
@@ -113,7 +113,7 @@ func (k BaseKeeper) DelegateCoins(ctx sdk.Context, delegatorAddr, moduleAccAddr
 		return sdkerrors.Wrap(err, "failed to track delegation")
 	}
 
-	_, err := k.AddCoins(ctx, moduleAccAddr, amt)
+	err := k.AddCoins(ctx, moduleAccAddr, amt)
 	if err != nil {
 		return err
 	}
@@ -136,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.SubtractCoins(ctx, moduleAccAddr, amt)
 	if err != nil {
 		return err
 	}
@@ -145,7 +145,7 @@ func (k BaseKeeper) UndelegateCoins(ctx sdk.Context, moduleAccAddr, delegatorAdd
 		return sdkerrors.Wrap(err, "failed to track undelegation")
 	}
 
-	_, err = k.AddCoins(ctx, delegatorAddr, amt)
+	err = k.AddCoins(ctx, delegatorAddr, amt)
 	if err != nil {
 		return err
 	}
@@ -330,7 +330,7 @@ func (k BaseKeeper) MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins)
 		panic(sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "module account %s does not have permissions to mint tokens", moduleName))
 	}
 
-	_, err := k.AddCoins(ctx, acc.GetAddress(), amt)
+	err := k.AddCoins(ctx, acc.GetAddress(), amt)
 	if err != nil {
 		return err
 	}
@@ -359,7 +359,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.SubtractCoins(ctx, acc.GetAddress(), amt)
 	if err != nil {
 		return err
 	}

diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go
@@ -18,8 +18,8 @@ type SendKeeper interface {
 	InputOutputCoins(ctx sdk.Context, inputs []types.Input, outputs []types.Output) error
 	SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error
 
-	SubtractCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, error)
-	AddCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, error)
+	SubtractCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) error
+	AddCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) error
 
 	SetBalance(ctx sdk.Context, addr sdk.AccAddress, balance sdk.Coin) error
 	SetBalances(ctx sdk.Context, addr sdk.AccAddress, balances sdk.Coins) error
@@ -85,7 +85,7 @@ func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, inputs []types.Input,
 	}
 
 	for _, in := range inputs {
-		_, err := k.SubtractCoins(ctx, in.Address, in.Coins)
+		err := k.SubtractCoins(ctx, in.Address, in.Coins)
 		if err != nil {
 			return err
 		}
@@ -99,7 +99,7 @@ func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, inputs []types.Input,
 	}
 
 	for _, out := range outputs {
-		_, err := k.AddCoins(ctx, out.Address, out.Coins)
+		err := k.AddCoins(ctx, out.Address, out.Coins)
 		if err != nil {
 			return err
 		}
@@ -142,12 +142,12 @@ func (k BaseSendKeeper) SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAd
 		),
 	})
 
-	_, err := k.SubtractCoins(ctx, fromAddr, amt)
+	err := k.SubtractCoins(ctx, fromAddr, amt)
 	if err != nil {
 		return err
 	}
 
-	_, err = k.AddCoins(ctx, toAddr, amt)
+	err = k.AddCoins(ctx, toAddr, amt)
 	if err != nil {
 		return err
 	}
@@ -167,12 +167,11 @@ func (k BaseSendKeeper) SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAd
 
 // SubtractCoins removes amt coins the account by the given address. An error is
 // returned if the resulting balance is negative or the initial amount is invalid.
-func (k BaseSendKeeper) SubtractCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, error) {
+func (k BaseSendKeeper) SubtractCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) error {
 	if !amt.IsValid() {
-		return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, amt.String())
+		return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, amt.String())
 	}
 
-	resultCoins := sdk.NewCoins()
 	lockedCoins := k.LockedCoins(ctx, addr)
 
 	for _, coin := range amt {
@@ -182,43 +181,39 @@ func (k BaseSendKeeper) SubtractCoins(ctx sdk.Context, addr sdk.AccAddress, amt
 
 		_, hasNeg := sdk.Coins{spendable}.SafeSub(sdk.Coins{coin})
 		if hasNeg {
-			return nil, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "%s is smaller than %s", spendable, coin)
+			return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "%s is smaller than %s", spendable, coin)
 		}
 
 		newBalance := balance.Sub(coin)
-		resultCoins = resultCoins.Add(newBalance)
 
 		err := k.SetBalance(ctx, addr, newBalance)
 		if err != nil {
-			return nil, err
+			return err
 		}
 	}
 
-	return resultCoins, nil
+	return nil
 }
 
 // AddCoins adds amt to the account balance given by the provided address. An
 // error is returned if the initial amount is invalid or if any resulting new
 // balance is negative.
-func (k BaseSendKeeper) AddCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, error) {
+func (k BaseSendKeeper) AddCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) error {
 	if !amt.IsValid() {
-		return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, amt.String())
+		return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, amt.String())
 	}
 
-	var resultCoins sdk.Coins
-
 	for _, coin := range amt {
 		balance := k.GetBalance(ctx, addr, coin.Denom)
 		newBalance := balance.Add(coin)
-		resultCoins = resultCoins.Add(newBalance)
 
 		err := k.SetBalance(ctx, addr, newBalance)
 		if err != nil {
-			return nil, err
+			return err
 		}
 	}
 
-	return resultCoins, nil
+	return nil
 }
 
 // ClearBalances removes all balances for a given account by address.

diff --git a/x/evidence/keeper/keeper_test.go b/x/evidence/keeper/keeper_test.go
@@ -132,7 +132,7 @@ func (suite *KeeperTestSuite) populateValidators(ctx sdk.Context) {
 	suite.app.BankKeeper.SetSupply(ctx, banktypes.NewSupply(totalSupply))
 
 	for _, addr := range valAddresses {
-		_, err := suite.app.BankKeeper.AddCoins(ctx, sdk.AccAddress(addr), initCoins)
+		err := suite.app.BankKeeper.AddCoins(ctx, sdk.AccAddress(addr), initCoins)
 		suite.NoError(err)
 	}
 }

diff --git a/x/ibc-transfer/keeper/relay_test.go b/x/ibc-transfer/keeper/relay_test.go
@@ -257,7 +257,7 @@ func (suite *KeeperTestSuite) TestOnAcknowledgementPacket() {
 			trace = types.ParseDenomTrace(sdk.DefaultBondDenom)
 			coin := sdk.NewCoin(sdk.DefaultBondDenom, amount)
 
-			_, err := suite.chainA.App.BankKeeper.AddCoins(suite.chainA.GetContext(), escrow, sdk.NewCoins(coin))
+			err := suite.chainA.App.BankKeeper.AddCoins(suite.chainA.GetContext(), escrow, sdk.NewCoins(coin))
 			suite.Require().NoError(err)
 		}, false, true},
 		{"unsuccessful refund from source", failedAck,
@@ -270,7 +270,7 @@ func (suite *KeeperTestSuite) TestOnAcknowledgementPacket() {
 				trace = types.ParseDenomTrace(types.GetPrefixedDenom(channelA.PortID, channelA.ID, sdk.DefaultBondDenom))
 				coin := sdk.NewCoin(trace.IBCDenom(), amount)
 
-				_, err := suite.chainA.App.BankKeeper.AddCoins(suite.chainA.GetContext(), escrow, sdk.NewCoins(coin))
+				err := suite.chainA.App.BankKeeper.AddCoins(suite.chainA.GetContext(), escrow, sdk.NewCoins(coin))
 				suite.Require().NoError(err)
 			}, false, true},
 	}
@@ -332,7 +332,7 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacket() {
 				trace = types.ParseDenomTrace(sdk.DefaultBondDenom)
 				coin := sdk.NewCoin(trace.IBCDenom(), amount)
 
-				_, err := suite.chainA.App.BankKeeper.AddCoins(suite.chainA.GetContext(), escrow, sdk.NewCoins(coin))
+				err := suite.chainA.App.BankKeeper.AddCoins(suite.chainA.GetContext(), escrow, sdk.NewCoins(coin))
 				suite.Require().NoError(err)
 			}, true},
 		{"successful timeout from external chain",
@@ -341,7 +341,7 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacket() {
 				trace = types.ParseDenomTrace(types.GetPrefixedDenom(channelA.PortID, channelA.ID, sdk.DefaultBondDenom))
 				coin := sdk.NewCoin(trace.IBCDenom(), amount)
 
-				_, err := suite.chainA.App.BankKeeper.AddCoins(suite.chainA.GetContext(), escrow, sdk.NewCoins(coin))
+				err := suite.chainA.App.BankKeeper.AddCoins(suite.chainA.GetContext(), escrow, sdk.NewCoins(coin))
 				suite.Require().NoError(err)
 			}, true},
 		{"no balance for coin denom",