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 039ef5a77f..f35010d098 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()), }, diff --git a/x/distribution/client/cli/tx.go b/x/distribution/client/cli/tx.go index 173035aea8..49fbf00bd0 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(), @@ -75,20 +76,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), @@ -98,22 +95,53 @@ $ %s tx distribution withdraw-rewards %s1gghjut3ccd8ay0zduzj64hwre2fxs9ldmqhffj return err } delAddr := clientCtx.GetFromAddress() - valAddr, err := sdk.ValAddressFromHex(args[0]) + valAddr, err := sdk.AccAddressFromHexUnsafe(args[0]) if err != nil { return err } 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 @@ -158,7 +186,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/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/simulation/operations.go b/x/distribution/simulation/operations.go index 9a5e5de737..f7ef09afdd 100644 --- a/x/distribution/simulation/operations.go +++ b/x/distribution/simulation/operations.go @@ -140,7 +140,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, @@ -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, diff --git a/x/distribution/types/common_test.go b/x/distribution/types/common_test.go index 4e6ef6b012..2a4b55765a 100644 --- a/x/distribution/types/common_test.go +++ b/x/distribution/types/common_test.go @@ -19,10 +19,10 @@ var ( valPk1 = ed25519.GenPrivKey().PubKey() valPk2 = ed25519.GenPrivKey().PubKey() valPk3 = ed25519.GenPrivKey().PubKey() - valAddr1 = sdk.ValAddress(valPk1.Address()) - valAddr2 = sdk.ValAddress(valPk2.Address()) - valAddr3 = sdk.ValAddress(valPk3.Address()) - emptyValAddr sdk.ValAddress + valAddr1 = sdk.AccAddress(valPk1.Address()) + valAddr2 = sdk.AccAddress(valPk2.Address()) + valAddr3 = sdk.AccAddress(valPk3.Address()) + emptyValAddr sdk.AccAddress emptyPubkey cryptotypes.PubKey ) diff --git a/x/distribution/types/msg.go b/x/distribution/types/msg.go index a08c1883aa..5e4a9fe51e 100644 --- a/x/distribution/types/msg.go +++ b/x/distribution/types/msg.go @@ -60,7 +60,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(), @@ -93,7 +93,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(), } @@ -104,8 +104,8 @@ func (msg MsgWithdrawValidatorCommission) Type() string { return TypeMsgWithdra // 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 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},