From cdc6d1243fbbaec29e7da2410ff558f5e9bc6e95 Mon Sep 17 00:00:00 2001 From: forcodedancing Date: Thu, 13 Apr 2023 10:27:28 -0600 Subject: [PATCH 1/4] split commands for withdraw rewards and commission due to eip712 --- x/distribution/client/cli/tx.go | 50 ++++++++++++++++++----- x/distribution/client/testutil/helpers.go | 22 ++++++++++ x/distribution/types/msg.go | 6 +-- 3 files changed, 64 insertions(+), 14 deletions(-) diff --git a/x/distribution/client/cli/tx.go b/x/distribution/client/cli/tx.go index 4511014702..ddf9cd234a 100644 --- a/x/distribution/client/cli/tx.go +++ b/x/distribution/client/cli/tx.go @@ -37,6 +37,7 @@ func NewTxCmd() *cobra.Command { distTxCmd.AddCommand( NewWithdrawRewardsCmd(), + NewWithdrawCommission(), NewWithdrawAllRewardsCmd(), NewSetWithdrawAddrCmd(), NewFundCommunityPoolCmd(), @@ -76,20 +77,16 @@ func newSplitAndApply( // NewWithdrawRewardsCmd returns a CLI command handler for creating a MsgWithdrawDelegatorReward transaction. func NewWithdrawRewardsCmd() *cobra.Command { - bech32PrefixValAddr := sdk.GetConfig().GetBech32ValidatorAddrPrefix() - cmd := &cobra.Command{ Use: "withdraw-rewards [validator-addr]", - Short: "Withdraw rewards from a given delegation address, and optionally withdraw validator commission if the delegation address given is a validator operator", + Short: "Withdraw rewards from a given delegation address", Long: strings.TrimSpace( - fmt.Sprintf(`Withdraw rewards from a given delegation address, -and optionally withdraw validator commission if the delegation address given is a validator operator. + fmt.Sprintf(`Withdraw rewards from a given delegation address. Example: -$ %s tx distribution withdraw-rewards %s1gghjut3ccd8ay0zduzj64hwre2fxs9ldmqhffj --from mykey -$ %s tx distribution withdraw-rewards %s1gghjut3ccd8ay0zduzj64hwre2fxs9ldmqhffj --from mykey --commission +$ %s tx distribution withdraw-rewards 0x91D7d.. --from mykey `, - version.AppName, bech32PrefixValAddr, version.AppName, bech32PrefixValAddr, + version.AppName, ), ), Args: cobra.ExactArgs(1), @@ -106,15 +103,46 @@ $ %s tx distribution withdraw-rewards %s1gghjut3ccd8ay0zduzj64hwre2fxs9ldmqhffj msgs := []sdk.Msg{types.NewMsgWithdrawDelegatorReward(delAddr, valAddr)} - if commission, _ := cmd.Flags().GetBool(FlagCommission); commission { - msgs = append(msgs, types.NewMsgWithdrawValidatorCommission(valAddr)) + return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msgs...) + }, + } + + flags.AddTxFlagsToCmd(cmd) + + return cmd +} + +// NewWithdrawCommission returns a CLI command handler for creating a MsgWithdrawValidatorCommission transaction. +func NewWithdrawCommission() *cobra.Command { + cmd := &cobra.Command{ + Use: "withdraw-commission [validator-addr]", + Short: "Withdraw validator commission", + Long: strings.TrimSpace( + fmt.Sprintf(`Withdraw validator commission if the delegation address given is a validator operator. + +Example: +$ %s tx distribution withdraw-commission 0x91D7d.. --from mykey +`, + version.AppName, + ), + ), + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + clientCtx, err := client.GetClientTxContext(cmd) + if err != nil { + return err } + valAddr, err := sdk.AccAddressFromHexUnsafe(args[0]) + if err != nil { + return err + } + + msgs := []sdk.Msg{types.NewMsgWithdrawValidatorCommission(valAddr)} return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msgs...) }, } - cmd.Flags().Bool(FlagCommission, false, "Withdraw the validator's commission in addition to the rewards") flags.AddTxFlagsToCmd(cmd) return cmd diff --git a/x/distribution/client/testutil/helpers.go b/x/distribution/client/testutil/helpers.go index 19612c23fe..e7cacd249c 100644 --- a/x/distribution/client/testutil/helpers.go +++ b/x/distribution/client/testutil/helpers.go @@ -30,3 +30,25 @@ func MsgWithdrawDelegatorRewardExec(clientCtx client.Context, valAddr fmt.String return buf.Bytes(), nil } + +func MsgWithdrawCommissionExec(clientCtx client.Context, valAddr fmt.Stringer, extraArgs ...string) ([]byte, error) { + buf := new(bytes.Buffer) + clientCtx = clientCtx.WithOutput(buf) + + ctx := context.Background() + ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx) + + args := []string{valAddr.String()} + args = append(args, extraArgs...) + + cmd := distrcli.NewWithdrawCommission() + cmd.SetErr(buf) + cmd.SetOut(buf) + cmd.SetArgs(args) + + if err := cmd.ExecuteContext(ctx); err != nil { + return nil, err + } + + return buf.Bytes(), nil +} diff --git a/x/distribution/types/msg.go b/x/distribution/types/msg.go index 9a96622083..368584c6a8 100644 --- a/x/distribution/types/msg.go +++ b/x/distribution/types/msg.go @@ -88,7 +88,7 @@ func (msg MsgWithdrawDelegatorReward) ValidateBasic() error { return nil } -func NewMsgWithdrawValidatorCommission(valAddr sdk.ValAddress) *MsgWithdrawValidatorCommission { +func NewMsgWithdrawValidatorCommission(valAddr sdk.AccAddress) *MsgWithdrawValidatorCommission { return &MsgWithdrawValidatorCommission{ ValidatorAddress: valAddr.String(), } @@ -96,8 +96,8 @@ func NewMsgWithdrawValidatorCommission(valAddr sdk.ValAddress) *MsgWithdrawValid // Return address that must sign over msg.GetSignBytes() func (msg MsgWithdrawValidatorCommission) GetSigners() []sdk.AccAddress { - valAddr, _ := sdk.ValAddressFromHex(msg.ValidatorAddress) - return []sdk.AccAddress{sdk.AccAddress(valAddr)} + valAddr, _ := sdk.AccAddressFromHexUnsafe(msg.ValidatorAddress) + return []sdk.AccAddress{valAddr} } // get the bytes for the message signer to sign on From 29dd064e4a1b6642005ee0bcc5326b2c995e7872 Mon Sep 17 00:00:00 2001 From: forcodedancing Date: Thu, 13 Apr 2023 10:34:59 +0800 Subject: [PATCH 2/4] fix addres issue --- x/distribution/simulation/operations.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/distribution/simulation/operations.go b/x/distribution/simulation/operations.go index 40165a3179..4e6a591159 100644 --- a/x/distribution/simulation/operations.go +++ b/x/distribution/simulation/operations.go @@ -184,7 +184,7 @@ func SimulateMsgWithdrawValidatorCommission(txConfig client.TxConfig, ak types.A account := ak.GetAccount(ctx, simAccount.Address) spendable := bk.SpendableCoins(ctx, account.GetAddress()) - msg := types.NewMsgWithdrawValidatorCommission(validator.GetOperator()) + msg := types.NewMsgWithdrawValidatorCommission(sdk.AccAddress(validator.GetOperator())) txCtx := simulation.OperationInput{ R: r, From e12b9a48b42814bc038d0ed607b19bbb2dff89af Mon Sep 17 00:00:00 2001 From: forcodedancing Date: Thu, 13 Apr 2023 10:54:12 +0800 Subject: [PATCH 3/4] fix addres issue --- x/distribution/client/cli/tx.go | 4 ++-- x/distribution/simulation/operations.go | 2 +- x/distribution/types/common_test.go | 4 ++-- x/distribution/types/msg.go | 2 +- x/distribution/types/msg_test.go | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/x/distribution/client/cli/tx.go b/x/distribution/client/cli/tx.go index ddf9cd234a..97428088c0 100644 --- a/x/distribution/client/cli/tx.go +++ b/x/distribution/client/cli/tx.go @@ -96,7 +96,7 @@ $ %s tx distribution withdraw-rewards 0x91D7d.. --from mykey return err } delAddr := clientCtx.GetFromAddress() - valAddr, err := sdk.ValAddressFromHex(args[0]) + valAddr, err := sdk.AccAddressFromHexUnsafe(args[0]) if err != nil { return err } @@ -187,7 +187,7 @@ $ %[1]s tx distribution withdraw-all-rewards --from mykey // build multi-message transaction msgs := make([]sdk.Msg, 0, len(validators)) for _, valAddr := range validators { - val, err := sdk.ValAddressFromHex(valAddr) + val, err := sdk.AccAddressFromHexUnsafe(valAddr) if err != nil { return err } diff --git a/x/distribution/simulation/operations.go b/x/distribution/simulation/operations.go index 4e6a591159..127bec9c7e 100644 --- a/x/distribution/simulation/operations.go +++ b/x/distribution/simulation/operations.go @@ -139,7 +139,7 @@ func SimulateMsgWithdrawDelegatorReward(txConfig client.TxConfig, ak types.Accou account := ak.GetAccount(ctx, simAccount.Address) spendable := bk.SpendableCoins(ctx, account.GetAddress()) - msg := types.NewMsgWithdrawDelegatorReward(simAccount.Address, validator.GetOperator()) + msg := types.NewMsgWithdrawDelegatorReward(simAccount.Address, sdk.AccAddress(validator.GetOperator())) txCtx := simulation.OperationInput{ R: r, diff --git a/x/distribution/types/common_test.go b/x/distribution/types/common_test.go index fd74434eee..9d07968666 100644 --- a/x/distribution/types/common_test.go +++ b/x/distribution/types/common_test.go @@ -13,6 +13,6 @@ var ( emptyDelAddr sdk.AccAddress valPk1 = ed25519.GenPrivKey().PubKey() - valAddr1 = sdk.ValAddress(valPk1.Address()) - emptyValAddr sdk.ValAddress + valAddr1 = sdk.AccAddress(valPk1.Address()) + emptyValAddr sdk.AccAddress ) diff --git a/x/distribution/types/msg.go b/x/distribution/types/msg.go index 368584c6a8..d172fc7a8d 100644 --- a/x/distribution/types/msg.go +++ b/x/distribution/types/msg.go @@ -58,7 +58,7 @@ func (msg MsgSetWithdrawAddress) ValidateBasic() error { return nil } -func NewMsgWithdrawDelegatorReward(delAddr sdk.AccAddress, valAddr sdk.ValAddress) *MsgWithdrawDelegatorReward { +func NewMsgWithdrawDelegatorReward(delAddr sdk.AccAddress, valAddr sdk.AccAddress) *MsgWithdrawDelegatorReward { return &MsgWithdrawDelegatorReward{ DelegatorAddress: delAddr.String(), ValidatorAddress: valAddr.String(), diff --git a/x/distribution/types/msg_test.go b/x/distribution/types/msg_test.go index 324626178d..0066bf467c 100644 --- a/x/distribution/types/msg_test.go +++ b/x/distribution/types/msg_test.go @@ -36,7 +36,7 @@ func TestMsgSetWithdrawAddress(t *testing.T) { func TestMsgWithdrawDelegatorReward(t *testing.T) { tests := []struct { delegatorAddr sdk.AccAddress - validatorAddr sdk.ValAddress + validatorAddr sdk.AccAddress expectPass bool }{ {delAddr1, valAddr1, true}, @@ -57,7 +57,7 @@ func TestMsgWithdrawDelegatorReward(t *testing.T) { // test ValidateBasic for MsgWithdrawValidatorCommission func TestMsgWithdrawValidatorCommission(t *testing.T) { tests := []struct { - validatorAddr sdk.ValAddress + validatorAddr sdk.AccAddress expectPass bool }{ {valAddr1, true}, From 4de64f971e4ded459c0ed9b14232769ab845f42a Mon Sep 17 00:00:00 2001 From: forcodedancing Date: Thu, 13 Apr 2023 14:01:42 +0800 Subject: [PATCH 4/4] fix tests --- tests/e2e/distribution/suite.go | 77 ++++++++++++++++++++----- x/distribution/client/cli/suite_test.go | 37 ++++++++++-- 2 files changed, 92 insertions(+), 22 deletions(-) diff --git a/tests/e2e/distribution/suite.go b/tests/e2e/distribution/suite.go index 7fbab00f21..ce8611e17c 100644 --- a/tests/e2e/distribution/suite.go +++ b/tests/e2e/distribution/suite.go @@ -519,19 +519,73 @@ func (s *E2ETestSuite) TestNewWithdrawRewardsCmd() { "/cosmos.distribution.v1beta1.MsgWithdrawDelegatorRewardResponse", }, }, + } + + for _, tc := range testCases { + tc := tc + + s.Run(tc.name, func() { + clientCtx := val.ClientCtx + + _, _ = s.network.WaitForHeightWithTimeout(10, time.Minute) + bz, err := distrclitestutil.MsgWithdrawDelegatorRewardExec(clientCtx, tc.valAddr, tc.args...) + if tc.expectErr { + s.Require().Error(err) + } else { + s.Require().NoError(err) + s.Require().NoError(clientCtx.Codec.UnmarshalJSON(bz, tc.respType), string(bz)) + s.Require().NoError(s.network.WaitForNextBlock()) + + txResp, err := clitestutil.GetTxResponse(s.network, clientCtx, tc.respType.(*sdk.TxResponse).TxHash) + s.Require().NoError(err) + s.Require().Equal(tc.expectedCode, txResp.Code) + + data, err := hex.DecodeString(txResp.Data) + s.Require().NoError(err) + + txMsgData := sdk.TxMsgData{} + err = s.cfg.Codec.Unmarshal(data, &txMsgData) + s.Require().NoError(err) + for responseIdx, msgResponse := range txMsgData.MsgResponses { + s.Require().Equal(tc.expectedResponseType[responseIdx], msgResponse.TypeUrl) + switch msgResponse.TypeUrl { + case "/cosmos.distribution.v1beta1.MsgWithdrawDelegatorRewardResponse": + var resp distrtypes.MsgWithdrawDelegatorRewardResponse + // can't use unpackAny as response types are not registered. + err = s.cfg.Codec.Unmarshal(msgResponse.Value, &resp) + s.Require().NoError(err) + s.Require().True(resp.Amount.IsAllGT(sdk.NewCoins(sdk.NewCoin("stake", math.OneInt()))), + fmt.Sprintf("expected a positive coin value, got %v", resp.Amount)) + } + } + } + }) + } +} + +func (s *E2ETestSuite) TestNewWithdrawCommissionCmd() { + val := s.network.Validators[0] + + testCases := []struct { + name string + valAddr fmt.Stringer + args []string + expectErr bool + expectedCode uint32 + respType proto.Message + expectedResponseType []string + }{ { - "valid transaction (with commission)", + "valid transaction", sdk.ValAddress(val.Address), []string{ fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - fmt.Sprintf("--%s=true", cli.FlagCommission), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()), }, false, 0, &sdk.TxResponse{}, []string{ - "/cosmos.distribution.v1beta1.MsgWithdrawDelegatorRewardResponse", "/cosmos.distribution.v1beta1.MsgWithdrawValidatorCommissionResponse", }, }, @@ -544,7 +598,7 @@ func (s *E2ETestSuite) TestNewWithdrawRewardsCmd() { clientCtx := val.ClientCtx _, _ = s.network.WaitForHeightWithTimeout(10, time.Minute) - bz, err := distrclitestutil.MsgWithdrawDelegatorRewardExec(clientCtx, tc.valAddr, tc.args...) + bz, err := distrclitestutil.MsgWithdrawCommissionExec(clientCtx, tc.valAddr, tc.args...) if tc.expectErr { s.Require().Error(err) } else { @@ -552,8 +606,7 @@ func (s *E2ETestSuite) TestNewWithdrawRewardsCmd() { s.Require().NoError(clientCtx.Codec.UnmarshalJSON(bz, tc.respType), string(bz)) s.Require().NoError(s.network.WaitForNextBlock()) - txResp, err := clitestutil.GetTxResponse(s.network, clientCtx, tc.respType.(*sdk.TxResponse).TxHash) - s.Require().NoError(err) + txResp := tc.respType.(*sdk.TxResponse) s.Require().Equal(tc.expectedCode, txResp.Code) data, err := hex.DecodeString(txResp.Data) @@ -564,20 +617,12 @@ func (s *E2ETestSuite) TestNewWithdrawRewardsCmd() { s.Require().NoError(err) for responseIdx, msgResponse := range txMsgData.MsgResponses { s.Require().Equal(tc.expectedResponseType[responseIdx], msgResponse.TypeUrl) - switch msgResponse.TypeUrl { - case "/cosmos.distribution.v1beta1.MsgWithdrawDelegatorRewardResponse": - var resp distrtypes.MsgWithdrawDelegatorRewardResponse - // can't use unpackAny as response types are not registered. - err = s.cfg.Codec.Unmarshal(msgResponse.Value, &resp) - s.Require().NoError(err) - s.Require().True(resp.Amount.IsAllGT(sdk.NewCoins(sdk.NewCoin("stake", math.OneInt()))), - fmt.Sprintf("expected a positive coin value, got %v", resp.Amount)) - case "/cosmos.distribution.v1beta1.MsgWithdrawValidatorCommissionResponse": + if msgResponse.TypeUrl == "/cosmos.distribution.v1beta1.MsgWithdrawValidatorCommissionResponse" { var resp distrtypes.MsgWithdrawValidatorCommissionResponse // can't use unpackAny as response types are not registered. err = s.cfg.Codec.Unmarshal(msgResponse.Value, &resp) s.Require().NoError(err) - s.Require().True(resp.Amount.IsAllGT(sdk.NewCoins(sdk.NewCoin("stake", math.OneInt()))), + s.Require().True(resp.Amount.IsAllGT(sdk.NewCoins(sdk.NewCoin("stake", sdk.OneInt()))), fmt.Sprintf("expected a positive coin value, got %v", resp.Amount)) } } diff --git a/x/distribution/client/cli/suite_test.go b/x/distribution/client/cli/suite_test.go index 671d750c5e..73a37c731d 100644 --- a/x/distribution/client/cli/suite_test.go +++ b/x/distribution/client/cli/suite_test.go @@ -3,14 +3,13 @@ package cli_test import ( "bytes" "fmt" - "io" - "strings" - "testing" - abci "github.com/cometbft/cometbft/abci/types" rpcclientmock "github.com/cometbft/cometbft/rpc/client/mock" "github.com/cosmos/gogoproto/proto" "github.com/stretchr/testify/suite" + "io" + "strings" + "testing" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" @@ -498,13 +497,39 @@ func (s *CLITestSuite) TestNewWithdrawRewardsCmd() { }, false, &sdk.TxResponse{}, }, + } + + for _, tc := range testCases { + tc := tc + + s.Run(tc.name, func() { + bz, err := distrclitestutil.MsgWithdrawDelegatorRewardExec(s.clientCtx, tc.valAddr, tc.args...) + if tc.expectErr { + s.Require().Error(err) + } else { + s.Require().NoError(err) + s.Require().NoError(s.clientCtx.Codec.UnmarshalJSON(bz, tc.respType), string(bz)) + } + }) + } +} + +func (s *CLITestSuite) TestNewWithdrawCommissionCmd() { + val := testutil.CreateKeyringAccounts(s.T(), s.kr, 1) + + testCases := []struct { + name string + valAddr fmt.Stringer + args []string + expectErr bool + respType proto.Message + }{ { - "valid transaction (with commission)", + "valid transaction", sdk.ValAddress(val[0].Address), []string{ fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - fmt.Sprintf("--%s=true", cli.FlagCommission), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(10))).String()), },