Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: split commands for withdraw rewards and commission due to eip712 #175

Merged
merged 4 commits into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 61 additions & 16 deletions tests/e2e/distribution/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
Expand All @@ -544,16 +598,15 @@ 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 {
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)
txResp := tc.respType.(*sdk.TxResponse)
s.Require().Equal(tc.expectedCode, txResp.Code)

data, err := hex.DecodeString(txResp.Data)
Expand All @@ -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))
}
}
Expand Down
37 changes: 31 additions & 6 deletions x/distribution/client/cli/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()),
},
Expand Down
54 changes: 41 additions & 13 deletions x/distribution/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func NewTxCmd() *cobra.Command {

distTxCmd.AddCommand(
NewWithdrawRewardsCmd(),
NewWithdrawCommission(),
NewWithdrawAllRewardsCmd(),
NewSetWithdrawAddrCmd(),
NewFundCommunityPoolCmd(),
Expand Down Expand Up @@ -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),
Expand All @@ -99,22 +96,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
Expand Down Expand Up @@ -159,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
}
Expand Down
22 changes: 22 additions & 0 deletions x/distribution/client/testutil/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions x/distribution/simulation/operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, in staking module validator's operator is using ValAddress type. Need to change in the following prs.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, I have found some places in other modules too


txCtx := simulation.OperationInput{
R: r,
Expand Down
4 changes: 2 additions & 2 deletions x/distribution/types/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
8 changes: 4 additions & 4 deletions x/distribution/types/msg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -88,16 +88,16 @@ func (msg MsgWithdrawDelegatorReward) ValidateBasic() error {
return nil
}

func NewMsgWithdrawValidatorCommission(valAddr sdk.ValAddress) *MsgWithdrawValidatorCommission {
func NewMsgWithdrawValidatorCommission(valAddr sdk.AccAddress) *MsgWithdrawValidatorCommission {
return &MsgWithdrawValidatorCommission{
ValidatorAddress: valAddr.String(),
}
}

// 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
Expand Down
4 changes: 2 additions & 2 deletions x/distribution/types/msg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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},
Expand Down