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.

Sign up for GitHub

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

Jump to bottom

remove resulting value return from AddCoins and SubtractCoins #7327

Merged
merged 4 commits into from
Sep 16, 2020
Merged

remove resulting value return from AddCoins and SubtractCoins #7327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b0547cf
remove resulting value return from AddCoins and SubtractCoins
colin-axner Sep 16, 2020
b21eeb0
add changelog
colin-axner Sep 16, 2020
690fc65
Merge branch 'master' of github.com:cosmos/cosmos-sdk into colin/fix-…
colin-axner Sep 16, 2020
43a5494
Merge branch 'master' into colin/fix-add-subtract-coins
Sep 16, 2020
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
2 changes: 1 addition & 1 deletion simapp/test_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
10 changes: 5 additions & 5 deletions x/bank/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
35 changes: 15 additions & 20 deletions x/bank/keeper/send.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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 {
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion x/evidence/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
8 changes: 4 additions & 4 deletions x/ibc-transfer/keeper/relay_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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},
}
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down