Skip to content

Commit

Permalink
refactor(bank): update InputOutputCoins arguments (#15477)
Browse files Browse the repository at this point in the history
  • Loading branch information
julienrbrt authored Mar 20, 2023
1 parent 5612bb8 commit 6acd5cc
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 89 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* (x/bank) [#15477](https://github.com/cosmos/cosmos-sdk/pull/15477) `banktypes.NewMsgMultiSend` and `keeper.InputOutputCoins` only accept one input.
* (mempool) [#15328](https://github.com/cosmos/cosmos-sdk/pull/15328) The `PriorityNonceMempool` is now generic over type `C comparable` and takes a single `PriorityNonceMempoolConfig[C]` argument. See `DefaultPriorityNonceMempoolConfig` for how to construct the configuration and a `TxPriority` type.
* (server) [#15358](https://github.com/cosmos/cosmos-sdk/pull/15358) Remove `server.ErrorCode` that was not used anywhere.
* [#15211](https://github.com/cosmos/cosmos-sdk/pull/15211) Remove usage of `github.com/cometbft/cometbft/libs/bytes.HexBytes` in favor of `[]byte` thorough the SDK.
Expand Down
27 changes: 12 additions & 15 deletions tests/integration/bank/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,14 +426,14 @@ func TestInputOutputNewAccount(t *testing.T) {
assert.Assert(t, f.accountKeeper.GetAccount(ctx, addr2) == nil)
assert.Assert(t, f.bankKeeper.GetAllBalances(ctx, addr2).Empty())

inputs := []types.Input{
{Address: addr1.String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10))},
input := types.Input{
Address: addr1.String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10)),
}
outputs := []types.Output{
{Address: addr2.String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10))},
}

assert.NilError(t, f.bankKeeper.InputOutputCoins(ctx, inputs, outputs))
assert.NilError(t, f.bankKeeper.InputOutputCoins(ctx, input, outputs))

expected := sdk.NewCoins(newFooCoin(30), newBarCoin(10))
acc2Balances := f.bankKeeper.GetAllBalances(ctx, addr2)
Expand All @@ -460,8 +460,8 @@ func TestInputOutputCoins(t *testing.T) {
acc3 := f.accountKeeper.NewAccountWithAddress(ctx, addr3)
f.accountKeeper.SetAccount(ctx, acc3)

input := []types.Input{
{Address: addr1.String(), Coins: sdk.NewCoins(newFooCoin(60), newBarCoin(20))},
input := types.Input{
Address: addr1.String(), Coins: sdk.NewCoins(newFooCoin(60), newBarCoin(20)),
}
outputs := []types.Output{
{Address: addr2.String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10))},
Expand All @@ -473,11 +473,10 @@ func TestInputOutputCoins(t *testing.T) {

assert.NilError(t, banktestutil.FundAccount(f.bankKeeper, ctx, addr1, balances))

insufficientInput := []types.Input{
{
Address: addr1.String(),
Coins: sdk.NewCoins(newFooCoin(300), newBarCoin(100)),
},
insufficientInput := types.Input{

Address: addr1.String(),
Coins: sdk.NewCoins(newFooCoin(300), newBarCoin(100)),
}
insufficientOutputs := []types.Output{
{Address: addr2.String(), Coins: sdk.NewCoins(newFooCoin(300), newBarCoin(100))},
Expand Down Expand Up @@ -722,11 +721,9 @@ func TestMsgMultiSendEvents(t *testing.T) {
coins := sdk.NewCoins(sdk.NewInt64Coin(fooDenom, 50), sdk.NewInt64Coin(barDenom, 100))
newCoins := sdk.NewCoins(sdk.NewInt64Coin(fooDenom, 50))
newCoins2 := sdk.NewCoins(sdk.NewInt64Coin(barDenom, 100))
input := []types.Input{
{
Address: addr.String(),
Coins: coins,
},
input := types.Input{
Address: addr.String(),
Coins: coins,
}
outputs := []types.Output{
{Address: addr3.String(), Coins: newCoins},
Expand Down
2 changes: 1 addition & 1 deletion x/bank/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ accounts. The send keeper does not alter the total supply (mint or burn coins).
type SendKeeper interface {
ViewKeeper

InputOutputCoins(ctx sdk.Context, inputs []types.Input, outputs []types.Output) error
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

GetParams(ctx sdk.Context) types.Params
Expand Down
2 changes: 1 addition & 1 deletion x/bank/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ When using '--dry-run' a key name cannot be used, only a bech32 address.
amount = coins.MulInt(totalAddrs)
}

msg := types.NewMsgMultiSend([]types.Input{types.NewInput(clientCtx.FromAddress, amount)}, output)
msg := types.NewMsgMultiSend(types.NewInput(clientCtx.FromAddress, amount), output)

return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
},
Expand Down
40 changes: 18 additions & 22 deletions x/bank/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,14 +504,14 @@ func (suite *KeeperTestSuite) TestInputOutputNewAccount() {
require.Empty(suite.bankKeeper.GetAllBalances(ctx, accAddrs[1]))

suite.mockInputOutputCoins([]sdk.AccountI{authtypes.NewBaseAccountWithAddress(accAddrs[0])}, []sdk.AccAddress{accAddrs[1]})
inputs := []banktypes.Input{
{Address: accAddrs[0].String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10))},
input := banktypes.Input{
Address: accAddrs[0].String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10)),
}
outputs := []banktypes.Output{
{Address: accAddrs[1].String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10))},
}

require.NoError(suite.bankKeeper.InputOutputCoins(ctx, inputs, outputs))
require.NoError(suite.bankKeeper.InputOutputCoins(ctx, input, outputs))

expected := sdk.NewCoins(newFooCoin(30), newBarCoin(10))
acc2Balances := suite.bankKeeper.GetAllBalances(ctx, accAddrs[1])
Expand All @@ -524,37 +524,35 @@ func (suite *KeeperTestSuite) TestInputOutputCoins() {
balances := sdk.NewCoins(newFooCoin(90), newBarCoin(30))

acc0 := authtypes.NewBaseAccountWithAddress(accAddrs[0])
inputs := []banktypes.Input{
{Address: accAddrs[0].String(), Coins: sdk.NewCoins(newFooCoin(60), newBarCoin(20))},
input := banktypes.Input{
Address: accAddrs[0].String(), Coins: sdk.NewCoins(newFooCoin(60), newBarCoin(20)),
}
outputs := []banktypes.Output{
{Address: accAddrs[1].String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10))},
{Address: accAddrs[2].String(), Coins: sdk.NewCoins(newFooCoin(30), newBarCoin(10))},
}

require.Error(suite.bankKeeper.InputOutputCoins(ctx, inputs, []banktypes.Output{}))
require.Error(suite.bankKeeper.InputOutputCoins(ctx, input, []banktypes.Output{}))

suite.authKeeper.EXPECT().GetAccount(suite.ctx, accAddrs[0]).Return(acc0)
require.Error(suite.bankKeeper.InputOutputCoins(ctx, inputs, outputs))
require.Error(suite.bankKeeper.InputOutputCoins(ctx, input, outputs))

suite.mockFundAccount(accAddrs[0])
require.NoError(banktestutil.FundAccount(suite.bankKeeper, ctx, accAddrs[0], balances))

insufficientInputs := []banktypes.Input{
{
Address: accAddrs[0].String(),
Coins: sdk.NewCoins(newFooCoin(300), newBarCoin(100)),
},
insufficientInput := banktypes.Input{
Address: accAddrs[0].String(),
Coins: sdk.NewCoins(newFooCoin(300), newBarCoin(100)),
}
insufficientOutputs := []banktypes.Output{
{Address: accAddrs[1].String(), Coins: sdk.NewCoins(newFooCoin(300), newBarCoin(100))},
{Address: accAddrs[2].String(), Coins: sdk.NewCoins(newFooCoin(300), newBarCoin(100))},
}

require.Error(suite.bankKeeper.InputOutputCoins(ctx, insufficientInputs, insufficientOutputs))
require.Error(suite.bankKeeper.InputOutputCoins(ctx, insufficientInput, insufficientOutputs))

suite.mockInputOutputCoins([]sdk.AccountI{acc0}, accAddrs[1:3])
require.NoError(suite.bankKeeper.InputOutputCoins(ctx, inputs, outputs))
require.NoError(suite.bankKeeper.InputOutputCoins(ctx, input, outputs))

acc1Balances := suite.bankKeeper.GetAllBalances(ctx, accAddrs[0])
expected := sdk.NewCoins(newFooCoin(30), newBarCoin(10))
Expand Down Expand Up @@ -762,19 +760,17 @@ func (suite *KeeperTestSuite) TestMsgMultiSendEvents() {
coins := sdk.NewCoins(sdk.NewInt64Coin(fooDenom, 50), sdk.NewInt64Coin(barDenom, 100))
newCoins := sdk.NewCoins(sdk.NewInt64Coin(fooDenom, 50))
newCoins2 := sdk.NewCoins(sdk.NewInt64Coin(barDenom, 100))
inputs := []banktypes.Input{
{
Address: accAddrs[0].String(),
Coins: coins,
},
input := banktypes.Input{
Address: accAddrs[0].String(),
Coins: coins,
}
outputs := []banktypes.Output{
{Address: accAddrs[2].String(), Coins: newCoins},
{Address: accAddrs[3].String(), Coins: newCoins2},
}

suite.authKeeper.EXPECT().GetAccount(suite.ctx, accAddrs[0]).Return(acc0)
require.Error(suite.bankKeeper.InputOutputCoins(ctx, inputs, outputs))
require.Error(suite.bankKeeper.InputOutputCoins(ctx, input, outputs))

events := ctx.EventManager().ABCIEvents()
require.Equal(0, len(events))
Expand All @@ -784,7 +780,7 @@ func (suite *KeeperTestSuite) TestMsgMultiSendEvents() {
require.NoError(banktestutil.FundAccount(suite.bankKeeper, ctx, accAddrs[0], sdk.NewCoins(sdk.NewInt64Coin(fooDenom, 50), sdk.NewInt64Coin(barDenom, 100))))

suite.mockInputOutputCoins([]sdk.AccountI{acc0}, accAddrs[2:4])
require.NoError(suite.bankKeeper.InputOutputCoins(ctx, inputs, outputs))
require.NoError(suite.bankKeeper.InputOutputCoins(ctx, input, outputs))

events = ctx.EventManager().ABCIEvents()
require.Equal(12, len(events)) // 12 events because account funding causes extra minting + coin_spent + coin_recv events
Expand All @@ -809,7 +805,7 @@ func (suite *KeeperTestSuite) TestMsgMultiSendEvents() {
newCoins2 = sdk.NewCoins(sdk.NewInt64Coin(barDenom, 100))

suite.mockInputOutputCoins([]sdk.AccountI{acc0}, accAddrs[2:4])
require.NoError(suite.bankKeeper.InputOutputCoins(ctx, inputs, outputs))
require.NoError(suite.bankKeeper.InputOutputCoins(ctx, input, outputs))

events = ctx.EventManager().ABCIEvents()
require.Equal(30, len(events)) // 27 due to account funding + coin_spent + coin_recv events
Expand Down
2 changes: 1 addition & 1 deletion x/bank/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (k msgServer) MultiSend(goCtx context.Context, msg *types.MsgMultiSend) (*t
}
}

err := k.InputOutputCoins(ctx, msg.Inputs, msg.Outputs)
err := k.InputOutputCoins(ctx, msg.Inputs[0], msg.Outputs)
if err != nil {
return nil, err
}
Expand Down
42 changes: 20 additions & 22 deletions x/bank/keeper/send.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
type SendKeeper interface {
ViewKeeper

InputOutputCoins(ctx sdk.Context, inputs []types.Input, outputs []types.Output) error
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

GetParams(ctx sdk.Context) types.Params
Expand Down Expand Up @@ -110,35 +110,33 @@ func (k BaseSendKeeper) SetParams(ctx sdk.Context, params types.Params) error {
return k.Params.Set(ctx, params)
}

// InputOutputCoins performs multi-send functionality. It accepts a series of
// inputs that correspond to a series of outputs. It returns an error if the
// inputs and outputs don't line up or if any single transfer of tokens fails.
func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, inputs []types.Input, outputs []types.Output) error {
// InputOutputCoins performs multi-send functionality. It accepts an
// input that corresponds to a series of outputs. It returns an error if the
// input and outputs don't line up or if any single transfer of tokens fails.
func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, input types.Input, outputs []types.Output) error {
// Safety check ensuring that when sending coins the keeper must maintain the
// Check supply invariant and validity of Coins.
if err := types.ValidateInputsOutputs(inputs, outputs); err != nil {
if err := types.ValidateInputOutputs(input, outputs); err != nil {
return err
}

for _, in := range inputs {
inAddress, err := sdk.AccAddressFromBech32(in.Address)
if err != nil {
return err
}

err = k.subUnlockedCoins(ctx, inAddress, in.Coins)
if err != nil {
return err
}
inAddress, err := sdk.AccAddressFromBech32(input.Address)
if err != nil {
return err
}

ctx.EventManager().EmitEvent(
sdk.NewEvent(
sdk.EventTypeMessage,
sdk.NewAttribute(types.AttributeKeySender, in.Address),
),
)
err = k.subUnlockedCoins(ctx, inAddress, input.Coins)
if err != nil {
return err
}

ctx.EventManager().EmitEvent(
sdk.NewEvent(
sdk.EventTypeMessage,
sdk.NewAttribute(types.AttributeKeySender, input.Address),
),
)

for _, out := range outputs {
outAddress, err := sdk.AccAddressFromBech32(out.Address)
if err != nil {
Expand Down
28 changes: 13 additions & 15 deletions x/bank/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ func (msg MsgSend) GetSigners() []sdk.AccAddress {
}

// NewMsgMultiSend - construct arbitrary multi-in, multi-out send msg.
func NewMsgMultiSend(in []Input, out []Output) *MsgMultiSend {
return &MsgMultiSend{Inputs: in, Outputs: out}
func NewMsgMultiSend(in Input, out []Output) *MsgMultiSend {
return &MsgMultiSend{Inputs: []Input{in}, Outputs: out}
}

// ValidateBasic Implements Msg.
Expand All @@ -79,7 +79,7 @@ func (msg MsgMultiSend) ValidateBasic() error {
return ErrNoOutputs
}

return ValidateInputsOutputs(msg.Inputs, msg.Outputs)
return ValidateInputOutputs(msg.Inputs[0], msg.Outputs)
}

// GetSignBytes Implements Msg.
Expand All @@ -89,13 +89,13 @@ func (msg MsgMultiSend) GetSignBytes() []byte {

// GetSigners Implements Msg.
func (msg MsgMultiSend) GetSigners() []sdk.AccAddress {
addrs := make([]sdk.AccAddress, len(msg.Inputs))
for i, in := range msg.Inputs {
inAddr, _ := sdk.AccAddressFromBech32(in.Address)
addrs[i] = inAddr
// should not happen as ValidateBasic would have failed
if len(msg.Inputs) == 0 {
return nil
}

return addrs
addrs, _ := sdk.AccAddressFromBech32(msg.Inputs[0].Address)
return []sdk.AccAddress{addrs}
}

// ValidateBasic - validate transaction input
Expand Down Expand Up @@ -152,17 +152,15 @@ func NewOutput(addr sdk.AccAddress, coins sdk.Coins) Output {
}
}

// ValidateInputsOutputs validates that each respective input and output is
// ValidateInputOutputs validates that each respective input and output is
// valid and that the sum of inputs is equal to the sum of outputs.
func ValidateInputsOutputs(inputs []Input, outputs []Output) error {
func ValidateInputOutputs(input Input, outputs []Output) error {
var totalIn, totalOut sdk.Coins

for _, in := range inputs {
if err := in.ValidateBasic(); err != nil {
return err
}
totalIn = totalIn.Add(in.Coins...)
if err := input.ValidateBasic(); err != nil {
return err
}
totalIn = input.Coins

for _, out := range outputs {
if err := out.ValidateBasic(); err != nil {
Expand Down
16 changes: 5 additions & 11 deletions x/bank/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,19 +241,13 @@ func TestMsgMultiSendGetSignBytes(t *testing.T) {
}

func TestMsgMultiSendGetSigners(t *testing.T) {
addrs := make([]string, 3)
inputs := make([]Input, 3)
for i, v := range []string{"input111111111111111", "input222222222222222", "input333333333333333"} {
addr := sdk.AccAddress([]byte(v))
inputs[i] = NewInput(addr, nil)
addrs[i] = addr.String()
}
msg := NewMsgMultiSend(inputs, nil)
addr := sdk.AccAddress([]byte("input111111111111111"))
input := NewInput(addr, nil)
msg := NewMsgMultiSend(input, nil)

res := msg.GetSigners()
for i, signer := range res {
require.Equal(t, signer.String(), addrs[i])
}
require.Equal(t, 1, len(res))
require.True(t, addr.Equals(res[0]))
}

func TestNewMsgSetSendEnabled(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion x/gov/testutil/expected_keepers_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 6acd5cc

Please sign in to comment.