From 59d537f62ee4979dcbbd1831d8abec4fd9a9fa34 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Tue, 21 May 2019 10:25:03 +0200 Subject: [PATCH 01/11] allow splitting withdrawal txs --- x/distribution/client/cli/tx.go | 41 +++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/x/distribution/client/cli/tx.go b/x/distribution/client/cli/tx.go index d69556d423c4..ea10e18807f1 100644 --- a/x/distribution/client/cli/tx.go +++ b/x/distribution/client/cli/tx.go @@ -23,6 +23,11 @@ var ( flagOnlyFromValidator = "only-from-validator" flagIsValidator = "is-validator" flagComission = "commission" + flagMaxMessagesPerTx = "max-msgs" +) + +const ( + MaxMessagesPerTxDefault = 5 ) // GetTxCmd returns the transaction commands for this module @@ -40,6 +45,30 @@ func GetTxCmd(storeKey string, cdc *amino.Codec) *cobra.Command { return distTxCmd } +func splitGenerateOrBroadcast(cliCtx context.CLIContext, txBldr authtxb.TxBuilder, msgs []sdk.Msg) error { + var lastErr error = nil + + chunkSize := viper.GetInt(flagMaxMessagesPerTx) + totalMessages := len(msgs) + if chunkSize == 0 { + chunkSize = totalMessages + } + + for i := 0; i < len(msgs); i += chunkSize { + sliceEnd := i + chunkSize + if sliceEnd > totalMessages { + sliceEnd = totalMessages + } + msgChunk := msgs[i:sliceEnd] + lastErr = utils.GenerateOrBroadcastMsgs(cliCtx, txBldr, msgChunk) + if lastErr != nil { + return lastErr + } + } + + return nil +} + // command to withdraw rewards func GetCmdWithdrawRewards(cdc *codec.Codec) *cobra.Command { cmd := &cobra.Command{ @@ -68,16 +97,17 @@ $ tx distr withdraw-rewards cosmosvaloper1gghjut3ccd8ay0zduzj64hwre2fxs msgs = append(msgs, types.NewMsgWithdrawValidatorCommission(valAddr)) } - return utils.GenerateOrBroadcastMsgs(cliCtx, txBldr, msgs) + return splitGenerateOrBroadcast(cliCtx, txBldr, msgs) }, } + cmd.Flags().Int(flagMaxMessagesPerTx, MaxMessagesPerTxDefault, "limit the number of messages per tx") cmd.Flags().Bool(flagComission, false, "also withdraw validator's commission") return cmd } // command to withdraw all rewards func GetCmdWithdrawAllRewards(cdc *codec.Codec, queryRoute string) *cobra.Command { - return &cobra.Command{ + cmd := &cobra.Command{ Use: "withdraw-all-rewards", Short: "withdraw all delegations rewards for a delegator", Long: strings.TrimSpace(`Withdraw all rewards for a single delegator: @@ -98,9 +128,11 @@ $ tx distr withdraw-all-rewards --from mykey return err } - return utils.GenerateOrBroadcastMsgs(cliCtx, txBldr, msgs) + return splitGenerateOrBroadcast(cliCtx, txBldr, msgs) }, } + cmd.Flags().Int(flagMaxMessagesPerTx, MaxMessagesPerTxDefault, "limit the number of messages per tx [zero=unlimited]") + return cmd } // command to replace a delegator's withdrawal address @@ -127,8 +159,9 @@ $ tx set-withdraw-addr cosmos1gghjut3ccd8ay0zduzj64hwre2fxs9ld75ru9p -- } msg := types.NewMsgSetWithdrawAddress(delAddr, withdrawAddr) - return utils.GenerateOrBroadcastMsgs(cliCtx, txBldr, []sdk.Msg{msg}) + return splitGenerateOrBroadcast(cliCtx, txBldr, []sdk.Msg{msg}) }, } + cmd.Flags().Int(flagMaxMessagesPerTx, MaxMessagesPerTxDefault, "limit the number of messages per tx") return cmd } From cec539eafbc7b2fca14664120f98175b75fb51aa Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Tue, 21 May 2019 10:47:54 +0200 Subject: [PATCH 02/11] updating .pending --- .pending/improvements/sdk/4384-WithdrawalTxSplitting | 1 + 1 file changed, 1 insertion(+) create mode 100644 .pending/improvements/sdk/4384-WithdrawalTxSplitting diff --git a/.pending/improvements/sdk/4384-WithdrawalTxSplitting b/.pending/improvements/sdk/4384-WithdrawalTxSplitting new file mode 100644 index 000000000000..140a7baaa11b --- /dev/null +++ b/.pending/improvements/sdk/4384-WithdrawalTxSplitting @@ -0,0 +1 @@ +#4384- Allow splitting withdrawal transaction in several chunks \ No newline at end of file From e1bc2813f4c677e26193f892c77b6223d9f89f25 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Tue, 21 May 2019 11:05:43 +0200 Subject: [PATCH 03/11] small cleanup --- x/distribution/client/cli/tx.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/x/distribution/client/cli/tx.go b/x/distribution/client/cli/tx.go index ea10e18807f1..e10b8939591e 100644 --- a/x/distribution/client/cli/tx.go +++ b/x/distribution/client/cli/tx.go @@ -46,8 +46,6 @@ func GetTxCmd(storeKey string, cdc *amino.Codec) *cobra.Command { } func splitGenerateOrBroadcast(cliCtx context.CLIContext, txBldr authtxb.TxBuilder, msgs []sdk.Msg) error { - var lastErr error = nil - chunkSize := viper.GetInt(flagMaxMessagesPerTx) totalMessages := len(msgs) if chunkSize == 0 { @@ -60,9 +58,9 @@ func splitGenerateOrBroadcast(cliCtx context.CLIContext, txBldr authtxb.TxBuilde sliceEnd = totalMessages } msgChunk := msgs[i:sliceEnd] - lastErr = utils.GenerateOrBroadcastMsgs(cliCtx, txBldr, msgChunk) - if lastErr != nil { - return lastErr + err = utils.GenerateOrBroadcastMsgs(cliCtx, txBldr, msgChunk) + if err != nil { + return err } } From c5e4088aa8a539f32ec2330e58a5a71bd40e1f56 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Tue, 21 May 2019 13:06:06 +0200 Subject: [PATCH 04/11] fixing build issue --- x/distribution/client/cli/tx.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/distribution/client/cli/tx.go b/x/distribution/client/cli/tx.go index e10b8939591e..c3eac22b68f2 100644 --- a/x/distribution/client/cli/tx.go +++ b/x/distribution/client/cli/tx.go @@ -58,7 +58,7 @@ func splitGenerateOrBroadcast(cliCtx context.CLIContext, txBldr authtxb.TxBuilde sliceEnd = totalMessages } msgChunk := msgs[i:sliceEnd] - err = utils.GenerateOrBroadcastMsgs(cliCtx, txBldr, msgChunk) + err := utils.GenerateOrBroadcastMsgs(cliCtx, txBldr, msgChunk) if err != nil { return err } From 767571cbfd6e8534f512e3479fec6f65e2d270ea Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Tue, 21 May 2019 13:23:28 +0200 Subject: [PATCH 05/11] Update x/distribution/client/cli/tx.go Co-Authored-By: Alessio Treglia --- x/distribution/client/cli/tx.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x/distribution/client/cli/tx.go b/x/distribution/client/cli/tx.go index c3eac22b68f2..a802c5c9fd73 100644 --- a/x/distribution/client/cli/tx.go +++ b/x/distribution/client/cli/tx.go @@ -58,7 +58,9 @@ func splitGenerateOrBroadcast(cliCtx context.CLIContext, txBldr authtxb.TxBuilde sliceEnd = totalMessages } msgChunk := msgs[i:sliceEnd] - err := utils.GenerateOrBroadcastMsgs(cliCtx, txBldr, msgChunk) + if err := utils.GenerateOrBroadcastMsgs(cliCtx, txBldr, msgChunk); err != nil { + return err + } if err != nil { return err } From 0e180b8c576268440846f8570781a368fabf6ea0 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Tue, 21 May 2019 13:25:34 +0200 Subject: [PATCH 06/11] updating usage text --- x/distribution/client/cli/tx.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x/distribution/client/cli/tx.go b/x/distribution/client/cli/tx.go index a802c5c9fd73..04871dcbb488 100644 --- a/x/distribution/client/cli/tx.go +++ b/x/distribution/client/cli/tx.go @@ -100,7 +100,7 @@ $ tx distr withdraw-rewards cosmosvaloper1gghjut3ccd8ay0zduzj64hwre2fxs return splitGenerateOrBroadcast(cliCtx, txBldr, msgs) }, } - cmd.Flags().Int(flagMaxMessagesPerTx, MaxMessagesPerTxDefault, "limit the number of messages per tx") + cmd.Flags().Int(flagMaxMessagesPerTx, MaxMessagesPerTxDefault, "limit the number of messages per tx. Zero for unlimited") cmd.Flags().Bool(flagComission, false, "also withdraw validator's commission") return cmd } @@ -131,7 +131,7 @@ $ tx distr withdraw-all-rewards --from mykey return splitGenerateOrBroadcast(cliCtx, txBldr, msgs) }, } - cmd.Flags().Int(flagMaxMessagesPerTx, MaxMessagesPerTxDefault, "limit the number of messages per tx [zero=unlimited]") + cmd.Flags().Int(flagMaxMessagesPerTx, MaxMessagesPerTxDefault, "limit the number of messages per tx. Zero for unlimited") return cmd } @@ -162,6 +162,6 @@ $ tx set-withdraw-addr cosmos1gghjut3ccd8ay0zduzj64hwre2fxs9ld75ru9p -- return splitGenerateOrBroadcast(cliCtx, txBldr, []sdk.Msg{msg}) }, } - cmd.Flags().Int(flagMaxMessagesPerTx, MaxMessagesPerTxDefault, "limit the number of messages per tx") + cmd.Flags().Int(flagMaxMessagesPerTx, MaxMessagesPerTxDefault, "limit the number of messages per tx. Zero for unlimited") return cmd } From 59796b57b76d5bb164f5f22bbc8b81a283253fd8 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Tue, 21 May 2019 13:32:02 +0200 Subject: [PATCH 07/11] fixing merge issue --- x/distribution/client/cli/tx.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/x/distribution/client/cli/tx.go b/x/distribution/client/cli/tx.go index 04871dcbb488..5feb37d97aa9 100644 --- a/x/distribution/client/cli/tx.go +++ b/x/distribution/client/cli/tx.go @@ -61,9 +61,6 @@ func splitGenerateOrBroadcast(cliCtx context.CLIContext, txBldr authtxb.TxBuilde if err := utils.GenerateOrBroadcastMsgs(cliCtx, txBldr, msgChunk); err != nil { return err } - if err != nil { - return err - } } return nil From a2d09eae2e672c6f53ff8701ba16ae00c9bebac6 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Wed, 22 May 2019 16:04:29 +0200 Subject: [PATCH 08/11] changes based on the review --- x/distribution/client/cli/tx.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/x/distribution/client/cli/tx.go b/x/distribution/client/cli/tx.go index 5feb37d97aa9..25442d7cd3a8 100644 --- a/x/distribution/client/cli/tx.go +++ b/x/distribution/client/cli/tx.go @@ -45,8 +45,12 @@ func GetTxCmd(storeKey string, cdc *amino.Codec) *cobra.Command { return distTxCmd } -func splitGenerateOrBroadcast(cliCtx context.CLIContext, txBldr authtxb.TxBuilder, msgs []sdk.Msg) error { - chunkSize := viper.GetInt(flagMaxMessagesPerTx) +func splitGenerateOrBroadcast( + cliCtx context.CLIContext, + txBldr authtxb.TxBuilder, + msgs []sdk.Msg, + chunkSize int) error { + totalMessages := len(msgs) if chunkSize == 0 { chunkSize = totalMessages @@ -94,10 +98,9 @@ $ tx distr withdraw-rewards cosmosvaloper1gghjut3ccd8ay0zduzj64hwre2fxs msgs = append(msgs, types.NewMsgWithdrawValidatorCommission(valAddr)) } - return splitGenerateOrBroadcast(cliCtx, txBldr, msgs) + return utils.GenerateOrBroadcastMsgs(cliCtx, txBldr, msgs) }, } - cmd.Flags().Int(flagMaxMessagesPerTx, MaxMessagesPerTxDefault, "limit the number of messages per tx. Zero for unlimited") cmd.Flags().Bool(flagComission, false, "also withdraw validator's commission") return cmd } @@ -125,7 +128,8 @@ $ tx distr withdraw-all-rewards --from mykey return err } - return splitGenerateOrBroadcast(cliCtx, txBldr, msgs) + chunkSize := viper.GetInt(flagMaxMessagesPerTx) + return splitGenerateOrBroadcast(cliCtx, txBldr, msgs, chunkSize) }, } cmd.Flags().Int(flagMaxMessagesPerTx, MaxMessagesPerTxDefault, "limit the number of messages per tx. Zero for unlimited") @@ -134,7 +138,7 @@ $ tx distr withdraw-all-rewards --from mykey // command to replace a delegator's withdrawal address func GetCmdSetWithdrawAddr(cdc *codec.Codec) *cobra.Command { - cmd := &cobra.Command{ + return &cobra.Command{ Use: "set-withdraw-addr [withdraw-addr]", Short: "change the default withdraw address for rewards associated with an address", Long: strings.TrimSpace(`Set the withdraw address for rewards associated with a delegator address: @@ -156,9 +160,7 @@ $ tx set-withdraw-addr cosmos1gghjut3ccd8ay0zduzj64hwre2fxs9ld75ru9p -- } msg := types.NewMsgSetWithdrawAddress(delAddr, withdrawAddr) - return splitGenerateOrBroadcast(cliCtx, txBldr, []sdk.Msg{msg}) + return utils.GenerateOrBroadcastMsgs(cliCtx, txBldr, []sdk.Msg{msg}) }, } - cmd.Flags().Int(flagMaxMessagesPerTx, MaxMessagesPerTxDefault, "limit the number of messages per tx. Zero for unlimited") - return cmd } From 395a701924fa6086006dc3ea99ea22b382cdd1af Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Wed, 22 May 2019 18:27:03 +0200 Subject: [PATCH 09/11] adding unit tests --- x/distribution/client/cli/tx.go | 14 ++--- x/distribution/client/cli/tx_test.go | 77 ++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 6 deletions(-) create mode 100644 x/distribution/client/cli/tx_test.go diff --git a/x/distribution/client/cli/tx.go b/x/distribution/client/cli/tx.go index 25442d7cd3a8..e6adda4f6f61 100644 --- a/x/distribution/client/cli/tx.go +++ b/x/distribution/client/cli/tx.go @@ -45,12 +45,15 @@ func GetTxCmd(storeKey string, cdc *amino.Codec) *cobra.Command { return distTxCmd } -func splitGenerateOrBroadcast( +type generateOrBroadcastFunc func(context.CLIContext, authtxb.TxBuilder, []sdk.Msg) error + +func splitAndApply( + generateOrBroadcast generateOrBroadcastFunc, cliCtx context.CLIContext, txBldr authtxb.TxBuilder, msgs []sdk.Msg, - chunkSize int) error { - + chunkSize int, +) error { totalMessages := len(msgs) if chunkSize == 0 { chunkSize = totalMessages @@ -62,11 +65,10 @@ func splitGenerateOrBroadcast( sliceEnd = totalMessages } msgChunk := msgs[i:sliceEnd] - if err := utils.GenerateOrBroadcastMsgs(cliCtx, txBldr, msgChunk); err != nil { + if err := generateOrBroadcast(cliCtx, txBldr, msgChunk); err != nil { return err } } - return nil } @@ -129,7 +131,7 @@ $ tx distr withdraw-all-rewards --from mykey } chunkSize := viper.GetInt(flagMaxMessagesPerTx) - return splitGenerateOrBroadcast(cliCtx, txBldr, msgs, chunkSize) + return splitAndApply(utils.GenerateOrBroadcastMsgs, cliCtx, txBldr, msgs, chunkSize) }, } cmd.Flags().Int(flagMaxMessagesPerTx, MaxMessagesPerTxDefault, "limit the number of messages per tx. Zero for unlimited") diff --git a/x/distribution/client/cli/tx_test.go b/x/distribution/client/cli/tx_test.go new file mode 100644 index 000000000000..c8f80f553f97 --- /dev/null +++ b/x/distribution/client/cli/tx_test.go @@ -0,0 +1,77 @@ +package cli + +import ( + "github.com/cosmos/cosmos-sdk/client/context" + "github.com/cosmos/cosmos-sdk/client/utils" + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + authtxb "github.com/cosmos/cosmos-sdk/x/auth/client/txbuilder" + "github.com/stretchr/testify/assert" + "github.com/tendermint/tendermint/crypto/secp256k1" + "testing" +) + +func createFakeTxBuilder() authtxb.TxBuilder { + cdc := codec.New() + return authtxb.NewTxBuilder( + utils.GetTxEncoder(cdc), + 123, + 9876, + 0, + 1.2, + false, + "test_chain", + "hello", + sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(1))), + sdk.DecCoins{sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, sdk.NewDecWithPrec(10000, sdk.Precision))}, + ) +} + +func Test_splitAndCall_NoMessages(t *testing.T) { + ctx := context.CLIContext{} + txBldr := createFakeTxBuilder() + + err := splitAndApply(nil, ctx, txBldr, nil, 10) + assert.NoError(t, err, "") +} + +func Test_splitAndCall_Splitting(t *testing.T) { + ctx := context.CLIContext{} + txBldr := createFakeTxBuilder() + + addr := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) + + // Add five messages + msgs := []sdk.Msg{ + sdk.NewTestMsg(addr), + sdk.NewTestMsg(addr), + sdk.NewTestMsg(addr), + sdk.NewTestMsg(addr), + sdk.NewTestMsg(addr), + } + + // Keep track of number of calls + const chunkSize = 2 + + callCount := 0 + err := splitAndApply( + func(ctx context.CLIContext, txBldr authtxb.TxBuilder, msgs []sdk.Msg) error { + callCount += 1 + + assert.NotNil(t, ctx) + assert.NotNil(t, txBldr) + assert.NotNil(t, msgs) + + if callCount < 3 { + assert.Equal(t, len(msgs), 2) + } else { + assert.Equal(t, len(msgs), 1) + } + + return nil + }, + ctx, txBldr, msgs, chunkSize) + + assert.NoError(t, err, "") + assert.Equal(t, 3, callCount) +} From 574071d78451aa5fa449a2e39a0c52e964d8f4b0 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Wed, 22 May 2019 21:20:04 +0200 Subject: [PATCH 10/11] Update x/distribution/client/cli/tx.go Co-Authored-By: Alexander Bezobchuk --- x/distribution/client/cli/tx.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/distribution/client/cli/tx.go b/x/distribution/client/cli/tx.go index e6adda4f6f61..ddcd8e056613 100644 --- a/x/distribution/client/cli/tx.go +++ b/x/distribution/client/cli/tx.go @@ -134,7 +134,7 @@ $ tx distr withdraw-all-rewards --from mykey return splitAndApply(utils.GenerateOrBroadcastMsgs, cliCtx, txBldr, msgs, chunkSize) }, } - cmd.Flags().Int(flagMaxMessagesPerTx, MaxMessagesPerTxDefault, "limit the number of messages per tx. Zero for unlimited") + cmd.Flags().Int(flagMaxMessagesPerTx, MaxMessagesPerTxDefault, "Limit the number of messages per tx (0 for unlimited)") return cmd } From bc65532f721bb7458424aafb86cf736a9f337ab6 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Thu, 23 May 2019 23:40:31 +0200 Subject: [PATCH 11/11] adjustments from review --- x/distribution/client/cli/tx.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/x/distribution/client/cli/tx.go b/x/distribution/client/cli/tx.go index ddcd8e056613..7d33f0203b0a 100644 --- a/x/distribution/client/cli/tx.go +++ b/x/distribution/client/cli/tx.go @@ -54,21 +54,26 @@ func splitAndApply( msgs []sdk.Msg, chunkSize int, ) error { - totalMessages := len(msgs) + if chunkSize == 0 { - chunkSize = totalMessages + return generateOrBroadcast(cliCtx, txBldr, msgs) } + // split messages into slices of length chunkSize + totalMessages := len(msgs) for i := 0; i < len(msgs); i += chunkSize { + sliceEnd := i + chunkSize if sliceEnd > totalMessages { sliceEnd = totalMessages } + msgChunk := msgs[i:sliceEnd] if err := generateOrBroadcast(cliCtx, txBldr, msgChunk); err != nil { return err } } + return nil }