From e9b0b7430fa8e1b011e824ef10c8c7d369741520 Mon Sep 17 00:00:00 2001 From: Hwangjae Lee Date: Thu, 26 Sep 2024 16:04:44 +0900 Subject: [PATCH 1/2] Allow MultiSendTx to Support Transactions with a Single Recipient This PR modifies the existing MultiSendTx functionality to support sending funds to a single recipient without returning an error. Previously, the MultiSendTx command required at least two recipient addresses, and attempting to send tokens to only one account would result in an error. The changes in this PR adjust the cobra.MinimumNArgs from 4 to 3, allowing the multi-send command to handle a single recipient as well as multiple recipients. Additionally, when the --split flag is used, the specified amount will be split evenly among recipients. If only one recipient is provided, the entire amount will be sent to that recipient, making the functionality more flexible and user-friendly. Signed-off-by: Hwangjae Lee --- x/bank/client/cli/tx.go | 30 ++++----- x/bank/client/cli/tx_test.go | 122 ++++++++++++++++++++++++++++++++++- 2 files changed, 135 insertions(+), 17 deletions(-) diff --git a/x/bank/client/cli/tx.go b/x/bank/client/cli/tx.go index 3d537522ebcf..72c2b024cd06 100644 --- a/x/bank/client/cli/tx.go +++ b/x/bank/client/cli/tx.go @@ -36,21 +36,21 @@ func NewTxCmd() *cobra.Command { } // NewMultiSendTxCmd returns a CLI command handler for creating a MsgMultiSend transaction. -// For a better UX this command is limited to send funds from one account to two or more accounts. +// This version allows sending funds from one account to one or more accounts. func NewMultiSendTxCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "multi-send ... ", - Short: "Send funds from one account to two or more accounts.", - Long: `Send funds from one account to two or more accounts. -By default, sends the [amount] to each address of the list. + Use: "multi-send ... ", + Short: "Send funds from one account to one or more accounts.", + Long: `Send funds from one account to one or more accounts. +By default, sends the [amount] to each address in the list. Using the '--split' flag, the [amount] is split equally between the addresses. -Note, the '--from' flag is ignored as it is implied from [from_key_or_address] and +Note, the '--from' flag is ignored as it is implied from [from_key_or_address] and separate addresses with space. When using '--dry-run' a key name cannot be used, only a bech32 address.`, - Example: fmt.Sprintf("%s tx bank multi-send cosmos1... cosmos1... cosmos1... cosmos1... 10stake", version.AppName), - Args: cobra.MinimumNArgs(4), + Example: fmt.Sprintf("%s tx bank multi-send cosmos1... cosmos1... cosmos1... 10stake", version.AppName), + Args: cobra.MinimumNArgs(3), // Changed minimum argument count to 3 RunE: func(cmd *cobra.Command, args []string) error { - err := cmd.Flags().Set(flags.FlagFrom, args[0]) + err := cmd.Flags().Set(flags.FlagFrom, args[0]) // Set the first argument as the sender if err != nil { return err } @@ -59,7 +59,7 @@ When using '--dry-run' a key name cannot be used, only a bech32 address.`, return err } - coins, err := sdk.ParseCoinsNormalized(args[len(args)-1]) + coins, err := sdk.ParseCoinsNormalized(args[len(args)-1]) // The last argument is the amount if err != nil { return err } @@ -73,15 +73,15 @@ When using '--dry-run' a key name cannot be used, only a bech32 address.`, return err } - totalAddrs := sdkmath.NewInt(int64(len(args) - 2)) + totalAddrs := sdkmath.NewInt(int64(len(args) - 2)) // Calculate the number of recipients // coins to be received by the addresses sendCoins := coins if split { - sendCoins = coins.QuoInt(totalAddrs) + sendCoins = coins.QuoInt(totalAddrs) // Logic to split the amount among recipients } var output []types.Output - for _, arg := range args[1 : len(args)-1] { + for _, arg := range args[1 : len(args)-1] { // Process each recipient _, err = clientCtx.AddressCodec.StringToBytes(arg) if err != nil { return err @@ -90,11 +90,9 @@ When using '--dry-run' a key name cannot be used, only a bech32 address.`, output = append(output, types.NewOutput(arg, sendCoins)) } - // amount to be send from the from address + // Calculate the total amount to be sent by the sender var amount sdk.Coins if split { - // user input: 1000stake to send to 3 addresses - // actual: 333stake to each address (=> 999stake actually sent) amount = sendCoins.MulInt(totalAddrs) } else { amount = coins.MulInt(totalAddrs) diff --git a/x/bank/client/cli/tx_test.go b/x/bank/client/cli/tx_test.go index 8f18458f79de..c7d2b79b4eb4 100644 --- a/x/bank/client/cli/tx_test.go +++ b/x/bank/client/cli/tx_test.go @@ -51,7 +51,127 @@ func (s *CLITestSuite) SetupSuite() { WithConsensusAddressCodec(addresscodec.NewBech32Codec("cosmosvalcons")) } -func (s *CLITestSuite) TestMultiSendTxCmd() { +func (s *CLITestSuite) TestMultiSendTxCmd_SingleRecipient() { + accounts := testutil.CreateKeyringAccounts(s.T(), s.kr, 2) // Create only 2 accounts: one sender and one recipient + accountStr := make([]string, len(accounts)) + for i, acc := range accounts { + addrStr, err := s.baseCtx.AddressCodec.BytesToString(acc.Address) + s.Require().NoError(err) + accountStr[i] = addrStr + } + + cmd := cli.NewMultiSendTxCmd() + cmd.SetOutput(io.Discard) + + extraArgs := []string{ + fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), + fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), + fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), + fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("photon", sdkmath.NewInt(10))).String()), + fmt.Sprintf("--%s=test-chain", flags.FlagChainID), + } + + testCases := []struct { + name string + ctxGen func() client.Context + from string + to []string + amount sdk.Coins + extraArgs []string + expectErrMsg string + }{ + { + "valid transaction with single recipient", + func() client.Context { + return s.baseCtx + }, + accountStr[0], // Sending account + []string{ + accountStr[1], // Only one recipient account + }, + sdk.NewCoins( + sdk.NewCoin("stake", sdkmath.NewInt(10)), + sdk.NewCoin("photon", sdkmath.NewInt(40)), + ), + extraArgs, + "", + }, + { + "invalid from Address", + func() client.Context { + return s.baseCtx + }, + "foo", + []string{ + accountStr[1], + }, + sdk.NewCoins( + sdk.NewCoin("stake", sdkmath.NewInt(10)), + sdk.NewCoin("photon", sdkmath.NewInt(40)), + ), + extraArgs, + "key not found", + }, + { + "invalid recipient", + func() client.Context { + return s.baseCtx + }, + accountStr[0], + []string{ + "bar", + }, + sdk.NewCoins( + sdk.NewCoin("stake", sdkmath.NewInt(10)), + sdk.NewCoin("photon", sdkmath.NewInt(40)), + ), + extraArgs, + "invalid bech32 string", + }, + { + "invalid amount", + func() client.Context { + return s.baseCtx + }, + accountStr[0], + []string{ + accountStr[1], + }, + nil, + extraArgs, + "must send positive amount", + }, + } + + for _, tc := range testCases { + s.Run(tc.name, func() { + ctx := svrcmd.CreateExecuteContext(context.Background()) + + var args []string + args = append(args, tc.from) // Sending account + args = append(args, tc.to...) // Recipient account(s) + args = append(args, tc.amount.String()) // Amount + args = append(args, tc.extraArgs...) // Additional flags + + cmd.SetContext(ctx) + cmd.SetArgs(args) + + s.Require().NoError(client.SetCmdClientContextHandler(tc.ctxGen(), cmd)) + + out, err := clitestutil.ExecTestCLICmd(tc.ctxGen(), cmd, args) + if tc.expectErrMsg != "" { + s.Require().Error(err) + s.Require().Contains(out.String(), tc.expectErrMsg) + } else { + s.Require().NoError(err) + msg := &sdk.TxResponse{} + s.Require().NoError(tc.ctxGen().Codec.UnmarshalJSON(out.Bytes(), msg), out.String()) + } + }) + } +} + +func (s *CLITestSuite) TestMultiSendTxCmd_MultiRecipient() { accounts := testutil.CreateKeyringAccounts(s.T(), s.kr, 3) accountStr := make([]string, len(accounts)) for i, acc := range accounts { From 14ac85f3fa38576bf026a83c217badcfc9dca82b Mon Sep 17 00:00:00 2001 From: Hwangjae Lee Date: Thu, 26 Sep 2024 16:17:26 +0900 Subject: [PATCH 2/2] Added PR at CHANGELOG.md Signed-off-by: Hwangjae Lee --- x/bank/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/x/bank/CHANGELOG.md b/x/bank/CHANGELOG.md index f226eac88503..4dca8f66f118 100644 --- a/x/bank/CHANGELOG.md +++ b/x/bank/CHANGELOG.md @@ -29,6 +29,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#17569](https://github.com/cosmos/cosmos-sdk/pull/17569) Introduce a new message type, `MsgBurn`, to burn coins. * [#20014](https://github.com/cosmos/cosmos-sdk/pull/20014) Support app wiring for `SendRestrictionFn`. +* [#21920](https://github.com/cosmos/cosmos-sdk/pull/21920) Allow `MultiSendTx` to support transactions with a single recipient. ### Improvements