diff --git a/x/gov/keeper/msg_server.go b/x/gov/keeper/msg_server.go index eb94ecb183..274c881b82 100644 --- a/x/gov/keeper/msg_server.go +++ b/x/gov/keeper/msg_server.go @@ -6,10 +6,12 @@ import ( "strconv" "cosmossdk.io/errors" + "cosmossdk.io/math" "github.com/armon/go-metrics" "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" @@ -29,24 +31,43 @@ var _ v1.MsgServer = msgServer{} // SubmitProposal implements the MsgServer.SubmitProposal method. func (k msgServer) SubmitProposal(goCtx context.Context, msg *v1.MsgSubmitProposal) (*v1.MsgSubmitProposalResponse, error) { - ctx := sdk.UnwrapSDKContext(goCtx) - - initialDeposit := msg.GetInitialDeposit() - - if err := k.validateInitialDeposit(ctx, initialDeposit, msg.Expedited); err != nil { - return nil, err + if msg.Title == "" { + return nil, errors.Wrap(sdkerrors.ErrInvalidRequest, "proposal title cannot be empty") + } + if msg.Summary == "" { + return nil, errors.Wrap(sdkerrors.ErrInvalidRequest, "proposal summary cannot be empty") } proposer, err := k.authKeeper.StringToBytes(msg.GetProposer()) if err != nil { + return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid proposer address: %s", err) + } + + if err := validateDeposit(sdk.NewCoins(msg.InitialDeposit...)); err != nil { return nil, err } + // Check that either metadata or Msgs length is non nil. + if len(msg.Messages) == 0 && len(msg.Metadata) == 0 { + return nil, errors.Wrap(govtypes.ErrNoProposalMsgs, "either metadata or Msgs length must be non-nil") + } + proposalMsgs, err := msg.GetMsgs() if err != nil { return nil, err } + if err := validateMsgs(proposalMsgs); err != nil { + return nil, err + } + + ctx := sdk.UnwrapSDKContext(goCtx) + initialDeposit := msg.GetInitialDeposit() + + if err := k.validateInitialDeposit(ctx, initialDeposit, msg.Expedited); err != nil { + return nil, err + } + proposal, err := k.Keeper.SubmitProposal(ctx, proposalMsgs, msg.Metadata, msg.Title, msg.Summary, proposer, msg.Expedited) if err != nil { return nil, err @@ -85,12 +106,12 @@ func (k msgServer) SubmitProposal(goCtx context.Context, msg *v1.MsgSubmitPropos // CancelProposals implements the MsgServer.CancelProposal method. func (k msgServer) CancelProposal(goCtx context.Context, msg *v1.MsgCancelProposal) (*v1.MsgCancelProposalResponse, error) { - ctx := sdk.UnwrapSDKContext(goCtx) _, err := k.authKeeper.StringToBytes(msg.Proposer) if err != nil { - return nil, err + return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid proposer address: %s", err) } + ctx := sdk.UnwrapSDKContext(goCtx) if err := k.Keeper.CancelProposal(ctx, msg.ProposalId, msg.Proposer); err != nil { return nil, err } @@ -139,11 +160,16 @@ func (k msgServer) ExecLegacyContent(goCtx context.Context, msg *v1.MsgExecLegac // Vote implements the MsgServer.Vote method. func (k msgServer) Vote(goCtx context.Context, msg *v1.MsgVote) (*v1.MsgVoteResponse, error) { - ctx := sdk.UnwrapSDKContext(goCtx) accAddr, err := k.authKeeper.StringToBytes(msg.Voter) if err != nil { - return nil, err + return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid voter address: %s", err) } + + if !v1.ValidVoteOption(msg.Option) { + return nil, errors.Wrap(govtypes.ErrInvalidVote, msg.Option.String()) + } + + ctx := sdk.UnwrapSDKContext(goCtx) err = k.Keeper.AddVote(ctx, msg.ProposalId, accAddr, v1.NewNonSplitVoteOption(msg.Option), msg.Metadata) if err != nil { return nil, err @@ -162,11 +188,41 @@ func (k msgServer) Vote(goCtx context.Context, msg *v1.MsgVote) (*v1.MsgVoteResp // VoteWeighted implements the MsgServer.VoteWeighted method. func (k msgServer) VoteWeighted(goCtx context.Context, msg *v1.MsgVoteWeighted) (*v1.MsgVoteWeightedResponse, error) { - ctx := sdk.UnwrapSDKContext(goCtx) accAddr, accErr := k.authKeeper.StringToBytes(msg.Voter) if accErr != nil { - return nil, accErr + return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid voter address: %s", accErr) } + + if len(msg.Options) == 0 { + return nil, errors.Wrap(sdkerrors.ErrInvalidRequest, v1.WeightedVoteOptions(msg.Options).String()) + } + + totalWeight := math.LegacyNewDec(0) + usedOptions := make(map[v1.VoteOption]bool) + for _, option := range msg.Options { + if !option.IsValid() { + return nil, errors.Wrap(govtypes.ErrInvalidVote, option.String()) + } + weight, err := sdk.NewDecFromStr(option.Weight) + if err != nil { + return nil, errors.Wrapf(govtypes.ErrInvalidVote, "invalid weight: %s", err) + } + totalWeight = totalWeight.Add(weight) + if usedOptions[option.Option] { + return nil, errors.Wrap(govtypes.ErrInvalidVote, "duplicated vote option") + } + usedOptions[option.Option] = true + } + + if totalWeight.GT(math.LegacyNewDec(1)) { + return nil, errors.Wrap(govtypes.ErrInvalidVote, "total weight overflow 1.00") + } + + if totalWeight.LT(math.LegacyNewDec(1)) { + return nil, errors.Wrap(govtypes.ErrInvalidVote, "total weight lower than 1.00") + } + + ctx := sdk.UnwrapSDKContext(goCtx) err := k.Keeper.AddVote(ctx, msg.ProposalId, accAddr, msg.Options, msg.Metadata) if err != nil { return nil, err @@ -185,11 +241,16 @@ func (k msgServer) VoteWeighted(goCtx context.Context, msg *v1.MsgVoteWeighted) // Deposit implements the MsgServer.Deposit method. func (k msgServer) Deposit(goCtx context.Context, msg *v1.MsgDeposit) (*v1.MsgDepositResponse, error) { - ctx := sdk.UnwrapSDKContext(goCtx) accAddr, err := k.authKeeper.StringToBytes(msg.Depositor) if err != nil { + return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid depositor address: %s", err) + } + + if err := validateAmount(sdk.NewCoins(msg.Amount...)); err != nil { return nil, err } + + ctx := sdk.UnwrapSDKContext(goCtx) votingStarted, err := k.Keeper.AddDeposit(ctx, msg.ProposalId, accAddr, msg.Amount) if err != nil { return nil, err @@ -221,6 +282,10 @@ func (k msgServer) UpdateParams(goCtx context.Context, msg *v1.MsgUpdateParams) return nil, errors.Wrapf(govtypes.ErrInvalidSigner, "invalid authority; expected %s, got %s", k.authority, msg.Authority) } + if err := msg.Params.ValidateBasic(); err != nil { + return nil, err + } + ctx := sdk.UnwrapSDKContext(goCtx) if err := k.SetParams(ctx, msg.Params); err != nil { return nil, err @@ -243,6 +308,17 @@ func NewLegacyMsgServerImpl(govAcct string, v1Server v1.MsgServer) v1beta1.MsgSe var _ v1beta1.MsgServer = legacyMsgServer{} func (k legacyMsgServer) SubmitProposal(goCtx context.Context, msg *v1beta1.MsgSubmitProposal) (*v1beta1.MsgSubmitProposalResponse, error) { + content := msg.GetContent() + if content == nil { + return nil, errors.Wrap(govtypes.ErrInvalidProposalContent, "missing content") + } + if !v1beta1.IsValidProposalType(content.ProposalType()) { + return nil, errors.Wrap(govtypes.ErrInvalidProposalType, content.ProposalType()) + } + if err := content.ValidateBasic(); err != nil { + return nil, err + } + contentMsg, err := v1.NewLegacyContent(msg.GetContent(), k.govAcct) if err != nil { return nil, fmt.Errorf("error converting legacy content into proposal message: %w", err) @@ -312,3 +388,43 @@ func (k legacyMsgServer) Deposit(goCtx context.Context, msg *v1beta1.MsgDeposit) } return &v1beta1.MsgDepositResponse{}, nil } + +func validateAmount(amount sdk.Coins) error { + if !amount.IsValid() { + return sdkerrors.ErrInvalidCoins.Wrap(amount.String()) + } + + if !amount.IsAllPositive() { + return sdkerrors.ErrInvalidCoins.Wrap(amount.String()) + } + + return nil +} + +func validateDeposit(deposit sdk.Coins) error { + if !deposit.IsValid() { + return errors.Wrap(sdkerrors.ErrInvalidCoins, deposit.String()) + } + + if deposit.IsAnyNegative() { + return errors.Wrap(sdkerrors.ErrInvalidCoins, deposit.String()) + } + + return nil +} + +func validateMsgs(msgs []sdk.Msg) error { + for idx, msg := range msgs { + m, ok := msg.(sdk.HasValidateBasic) + if !ok { + continue + } + + if err := m.ValidateBasic(); err != nil { + return errors.Wrap(govtypes.ErrInvalidProposalMsg, + fmt.Sprintf("msg: %d, err: %s", idx, err.Error())) + } + } + + return nil +} diff --git a/x/gov/keeper/msg_server_test.go b/x/gov/keeper/msg_server_test.go index 4f6cd477c7..cc45f577ca 100644 --- a/x/gov/keeper/msg_server_test.go +++ b/x/gov/keeper/msg_server_test.go @@ -20,8 +20,9 @@ const ( ) var ( - longAddress = "cosmos1v9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpg0s5ed" - longAddressError = "address max length is 255" + longAddress = "cosmos1v9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpv9skzctpg0s5ed" + longAddressError = "address max length is 255" + emptyAddressError = "empty address string is not allowed" ) func (suite *KeeperTestSuite) TestSubmitProposalReq() { @@ -39,11 +40,73 @@ func (suite *KeeperTestSuite) TestSubmitProposalReq() { Amount: coins, } + suite.acctKeeper.EXPECT().StringToBytes("").Return(nil, errors.New(emptyAddressError)) + cases := map[string]struct { preRun func() (*v1.MsgSubmitProposal, error) expErr bool expErrMsg string }{ + "invalid addr": { + preRun: func() (*v1.MsgSubmitProposal, error) { + return v1.NewMsgSubmitProposal( + []sdk.Msg{bankMsg}, + initialDeposit, + "", + strings.Repeat("1", 300), + "Proposal", + "description of proposal", + false, + ) + }, + expErr: true, + expErrMsg: "invalid proposer address", + }, + "empty msgs and metadata": { + preRun: func() (*v1.MsgSubmitProposal, error) { + return v1.NewMsgSubmitProposal( + nil, + initialDeposit, + proposer.String(), + "", + "Proposal", + "description of proposal", + false, + ) + }, + expErr: true, + expErrMsg: "no messages proposed", + }, + "empty title": { + preRun: func() (*v1.MsgSubmitProposal, error) { + return v1.NewMsgSubmitProposal( + []sdk.Msg{bankMsg}, + initialDeposit, + proposer.String(), + "", + "", + "description of proposal", + false, + ) + }, + expErr: true, + expErrMsg: "proposal title cannot be empty", + }, + "empty description": { + preRun: func() (*v1.MsgSubmitProposal, error) { + return v1.NewMsgSubmitProposal( + []sdk.Msg{bankMsg}, + initialDeposit, + proposer.String(), + "", + "Proposal", + "", + false, + ) + }, + expErr: true, + expErrMsg: "proposal summary cannot be empty", + }, "metadata too long": { preRun: func() (*v1.MsgSubmitProposal, error) { return v1.NewMsgSubmitProposal( @@ -176,9 +239,12 @@ func (suite *KeeperTestSuite) TestCancelProposalReq() { suite.Require().NotNil(res.ProposalId) proposalID := res.ProposalId + suite.acctKeeper.EXPECT().StringToBytes("").Return(nil, errors.New(emptyAddressError)) + cases := map[string]struct { preRun func() uint64 expErr bool + expErrMsg string proposalID uint64 depositor sdk.AccAddress }{ @@ -188,6 +254,7 @@ func (suite *KeeperTestSuite) TestCancelProposalReq() { }, depositor: proposer, expErr: true, + expErrMsg: "proposal is not found", }, "valid proposal but invalid proposer": { preRun: func() uint64 { @@ -195,10 +262,33 @@ func (suite *KeeperTestSuite) TestCancelProposalReq() { }, depositor: addrs[1], expErr: true, + expErrMsg: "invalid proposer", + }, + "empty proposer": { + preRun: func() uint64 { + return proposalID + }, + depositor: sdk.AccAddress{}, + expErr: true, + expErrMsg: "invalid proposer address: empty address string is not allowed", }, "all good": { preRun: func() uint64 { - return proposalID + msg, err := v1.NewMsgSubmitProposal( + []sdk.Msg{bankMsg}, + coins, + proposer.String(), + "", + "Proposal", + "description of proposal", + false, + ) + suite.Require().NoError(err) + + res, err := suite.msgSrvr.SubmitProposal(suite.ctx, msg) + suite.Require().NoError(err) + suite.Require().NotNil(res.ProposalId) + return res.ProposalId }, depositor: proposer, expErr: false, @@ -212,6 +302,7 @@ func (suite *KeeperTestSuite) TestCancelProposalReq() { _, err := suite.msgSrvr.CancelProposal(suite.ctx, cancelProposalReq) if tc.expErr { suite.Require().Error(err) + suite.Require().Contains(err.Error(), tc.expErrMsg) } else { suite.Require().NoError(err) } @@ -245,6 +336,7 @@ func (suite *KeeperTestSuite) TestVoteReq() { suite.Require().NoError(err) suite.acctKeeper.EXPECT().StringToBytes(longAddress).Return(nil, errors.New(longAddressError)).AnyTimes() + suite.acctKeeper.EXPECT().StringToBytes("").Return(nil, errors.New(emptyAddressError)) res, err := suite.msgSrvr.SubmitProposal(suite.ctx, msg) suite.Require().NoError(err) @@ -259,6 +351,26 @@ func (suite *KeeperTestSuite) TestVoteReq() { metadata string voter sdk.AccAddress }{ + "empty voter": { + preRun: func() uint64 { + return proposalID + }, + option: v1.VoteOption_VOTE_OPTION_YES, + voter: sdk.AccAddress{}, + metadata: "", + expErr: true, + expErrMsg: "invalid voter address", + }, + "wrong vote option": { + preRun: func() uint64 { + return proposalID + }, + option: v1.VoteOption(0x13), + voter: proposer, + metadata: "", + expErr: true, + expErrMsg: "invalid vote option", + }, "vote on inactive proposal": { preRun: func() uint64 { msg, err := v1.NewMsgSubmitProposal( @@ -346,8 +458,8 @@ func (suite *KeeperTestSuite) TestVoteReq() { func (suite *KeeperTestSuite) TestVoteWeightedReq() { suite.reset() govAcct := suite.govKeeper.GetGovernanceAccount(suite.ctx).GetAddress() - addrs := suite.addrs - proposer := addrs[0] + + proposer := simtestutil.AddTestAddrsIncremental(suite.bankKeeper, suite.stakingKeeper, suite.ctx, 1, sdkmath.NewInt(50000000))[0] coins := sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(100))) minDeposit := suite.govKeeper.GetParams(suite.ctx).MinDeposit @@ -374,16 +486,111 @@ func (suite *KeeperTestSuite) TestVoteWeightedReq() { proposalID := res.ProposalId suite.acctKeeper.EXPECT().StringToBytes(longAddress).Return(nil, errors.New(longAddressError)).AnyTimes() + suite.acctKeeper.EXPECT().StringToBytes("").Return(nil, errors.New(emptyAddressError)) cases := map[string]struct { preRun func() uint64 vote *v1.MsgVote expErr bool expErrMsg string - option v1.VoteOption + option v1.WeightedVoteOptions metadata string voter sdk.AccAddress }{ + "empty voter": { + preRun: func() uint64 { + return proposalID + }, + option: v1.WeightedVoteOptions{ + v1.NewWeightedVoteOption(v1.OptionYes, sdkmath.LegacyNewDec(1)), + }, + voter: sdk.AccAddress{}, + metadata: "", + expErr: true, + expErrMsg: "invalid voter address", + }, + "weights sum > 1": { + preRun: func() uint64 { + return proposalID + }, + option: v1.WeightedVoteOptions{ + v1.NewWeightedVoteOption(v1.OptionYes, sdkmath.LegacyNewDec(1)), + v1.NewWeightedVoteOption(v1.OptionAbstain, sdkmath.LegacyNewDec(1)), + }, + voter: proposer, + metadata: "", + expErr: true, + expErrMsg: "total weight overflow 1.00: invalid vote option", + }, + "duplicate vote options": { + preRun: func() uint64 { + return proposalID + }, + option: v1.WeightedVoteOptions{ + v1.NewWeightedVoteOption(v1.OptionYes, sdk.NewDecWithPrec(5, 1)), + v1.NewWeightedVoteOption(v1.OptionYes, sdk.NewDecWithPrec(5, 1)), + }, + voter: proposer, + metadata: "", + expErr: true, + expErrMsg: "duplicated vote option", + }, + "zero weight": { + preRun: func() uint64 { + return proposalID + }, + option: v1.WeightedVoteOptions{ + v1.NewWeightedVoteOption(v1.OptionYes, sdkmath.LegacyNewDec(0)), + }, + voter: proposer, + metadata: "", + expErr: true, + expErrMsg: `option:VOTE_OPTION_YES weight:"0.000000000000000000" : invalid vote option`, + }, + "negative weight": { + preRun: func() uint64 { + return proposalID + }, + option: v1.WeightedVoteOptions{ + v1.NewWeightedVoteOption(v1.OptionYes, sdkmath.LegacyNewDec(-1)), + }, + voter: proposer, + metadata: "", + expErr: true, + expErrMsg: `option:VOTE_OPTION_YES weight:"-1.000000000000000000" : invalid vote option`, + }, + "empty options": { + preRun: func() uint64 { + return proposalID + }, + option: v1.WeightedVoteOptions{}, + voter: proposer, + metadata: "", + expErr: true, + expErrMsg: "invalid request", + }, + "invalid vote option": { + preRun: func() uint64 { + return proposalID + }, + option: v1.NewNonSplitVoteOption(v1.VoteOption(0x13)), + voter: proposer, + metadata: "", + expErr: true, + expErrMsg: "invalid vote option", + }, + "weight sum < 1": { + preRun: func() uint64 { + return proposalID + }, + option: v1.WeightedVoteOptions{ // weight sum <1 + v1.NewWeightedVoteOption(v1.OptionYes, sdk.NewDecWithPrec(5, 1)), + }, + voter: proposer, + metadata: "", + expErr: true, + expErrMsg: "total weight lower than 1.00: invalid vote option", + }, "vote on inactive proposal": { preRun: func() uint64 { msg, err := v1.NewMsgSubmitProposal( @@ -402,7 +609,7 @@ func (suite *KeeperTestSuite) TestVoteWeightedReq() { suite.Require().NotNil(res.ProposalId) return res.ProposalId }, - option: v1.VoteOption_VOTE_OPTION_YES, + option: v1.NewNonSplitVoteOption(v1.VoteOption_VOTE_OPTION_YES), voter: proposer, metadata: "", expErr: true, @@ -412,7 +619,7 @@ func (suite *KeeperTestSuite) TestVoteWeightedReq() { preRun: func() uint64 { return proposalID }, - option: v1.VoteOption_VOTE_OPTION_YES, + option: v1.NewNonSplitVoteOption(v1.VoteOption_VOTE_OPTION_YES), voter: proposer, metadata: strings.Repeat("a", 300), expErr: true, @@ -422,7 +629,7 @@ func (suite *KeeperTestSuite) TestVoteWeightedReq() { preRun: func() uint64 { return proposalID }, - option: v1.VoteOption_VOTE_OPTION_YES, + option: v1.NewNonSplitVoteOption(v1.VoteOption_VOTE_OPTION_YES), voter: sdk.AccAddress(strings.Repeat("a", 300)), metadata: "", expErr: true, @@ -446,7 +653,33 @@ func (suite *KeeperTestSuite) TestVoteWeightedReq() { suite.Require().NotNil(res.ProposalId) return res.ProposalId }, - option: v1.VoteOption_VOTE_OPTION_YES, + option: v1.NewNonSplitVoteOption(v1.VoteOption_VOTE_OPTION_YES), + voter: proposer, + metadata: "", + expErr: false, + }, + "all good with split votes": { + preRun: func() uint64 { + msg, err := v1.NewMsgSubmitProposal( + []sdk.Msg{bankMsg}, + minDeposit, + proposer.String(), + "", + "Proposal", + "description of proposal", + false, + ) + suite.Require().NoError(err) + + res, err := suite.msgSrvr.SubmitProposal(suite.ctx, msg) + suite.Require().NoError(err) + suite.Require().NotNil(res.ProposalId) + return res.ProposalId + }, + option: v1.WeightedVoteOptions{ + v1.NewWeightedVoteOption(v1.OptionYes, sdk.NewDecWithPrec(5, 1)), + v1.NewWeightedVoteOption(v1.OptionAbstain, sdk.NewDecWithPrec(5, 1)), + }, voter: proposer, metadata: "", expErr: false, @@ -456,7 +689,7 @@ func (suite *KeeperTestSuite) TestVoteWeightedReq() { for name, tc := range cases { suite.Run(name, func() { pID := tc.preRun() - voteReq := v1.NewMsgVoteWeighted(tc.voter, pID, v1.NewNonSplitVoteOption(tc.option), tc.metadata) + voteReq := v1.NewMsgVoteWeighted(tc.voter, pID, tc.option, tc.metadata) _, err := suite.msgSrvr.VoteWeighted(suite.ctx, voteReq) if tc.expErr { suite.Require().Error(err) @@ -497,13 +730,15 @@ func (suite *KeeperTestSuite) TestDepositReq() { suite.Require().NotNil(res.ProposalId) pID := res.ProposalId + suite.acctKeeper.EXPECT().StringToBytes("").Return(nil, errors.New(emptyAddressError)) + cases := map[string]struct { preRun func() uint64 expErr bool proposalID uint64 depositor sdk.AccAddress deposit sdk.Coins - options v1.WeightedVoteOptions + expErrMsg string }{ "wrong proposal id": { preRun: func() uint64 { @@ -512,7 +747,16 @@ func (suite *KeeperTestSuite) TestDepositReq() { depositor: proposer, deposit: coins, expErr: true, - options: v1.NewNonSplitVoteOption(v1.OptionYes), + expErrMsg: "0: unknown proposal", + }, + "empty depositor": { + preRun: func() uint64 { + return pID + }, + depositor: sdk.AccAddress{}, + deposit: minDeposit, + expErr: true, + expErrMsg: "invalid depositor address", }, "all good": { preRun: func() uint64 { @@ -521,7 +765,6 @@ func (suite *KeeperTestSuite) TestDepositReq() { depositor: proposer, deposit: minDeposit, expErr: false, - options: v1.NewNonSplitVoteOption(v1.OptionYes), }, } @@ -532,6 +775,7 @@ func (suite *KeeperTestSuite) TestDepositReq() { _, err := suite.msgSrvr.Deposit(suite.ctx, depositReq) if tc.expErr { suite.Require().Error(err) + suite.Require().Contains(err.Error(), tc.expErrMsg) } else { suite.Require().NoError(err) } @@ -541,17 +785,78 @@ func (suite *KeeperTestSuite) TestDepositReq() { // legacy msg server tests func (suite *KeeperTestSuite) TestLegacyMsgSubmitProposal() { - addrs := suite.addrs - proposer := addrs[0] - + proposer := simtestutil.AddTestAddrsIncremental(suite.bankKeeper, suite.stakingKeeper, suite.ctx, 1, sdkmath.NewInt(50000000))[0] coins := sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(100))) initialDeposit := coins minDeposit := suite.govKeeper.GetParams(suite.ctx).MinDeposit + suite.acctKeeper.EXPECT().StringToBytes("").Return(nil, errors.New(emptyAddressError)) + cases := map[string]struct { - preRun func() (*v1beta1.MsgSubmitProposal, error) - expErr bool + preRun func() (*v1beta1.MsgSubmitProposal, error) + expErr bool + expErrMsg string }{ + "empty title": { + preRun: func() (*v1beta1.MsgSubmitProposal, error) { + content := v1beta1.NewTextProposal("", "I am test") + return v1beta1.NewMsgSubmitProposal( + content, + initialDeposit, + proposer, + ) + }, + expErr: true, + expErrMsg: "proposal title cannot be blank", + }, + "empty description": { + preRun: func() (*v1beta1.MsgSubmitProposal, error) { + content := v1beta1.NewTextProposal("test", "") + return v1beta1.NewMsgSubmitProposal( + content, + initialDeposit, + proposer, + ) + }, + expErr: true, + expErrMsg: "proposal description cannot be blank", + }, + "empty proposer": { + preRun: func() (*v1beta1.MsgSubmitProposal, error) { + content := v1beta1.NewTextProposal("test", "I am test") + return v1beta1.NewMsgSubmitProposal( + content, + initialDeposit, + sdk.AccAddress{}, + ) + }, + expErr: true, + expErrMsg: "invalid proposer address: empty address string is not allowed", + }, + "title text length > max limit allowed": { + preRun: func() (*v1beta1.MsgSubmitProposal, error) { + content := v1beta1.NewTextProposal(strings.Repeat("#", v1beta1.MaxTitleLength*2), "I am test") + return v1beta1.NewMsgSubmitProposal( + content, + initialDeposit, + proposer, + ) + }, + expErr: true, + expErrMsg: "proposal title is longer than max length of 140: invalid proposal content", + }, + "description text length > max limit allowed": { + preRun: func() (*v1beta1.MsgSubmitProposal, error) { + content := v1beta1.NewTextProposal("test", strings.Repeat("#", v1beta1.MaxDescriptionLength*2)) + return v1beta1.NewMsgSubmitProposal( + content, + initialDeposit, + proposer, + ) + }, + expErr: true, + expErrMsg: "proposal description is longer than max length of 10000: invalid proposal content", + }, "all good": { preRun: func() (*v1beta1.MsgSubmitProposal, error) { return v1beta1.NewMsgSubmitProposal( @@ -581,6 +886,7 @@ func (suite *KeeperTestSuite) TestLegacyMsgSubmitProposal() { res, err := suite.legacyMsgSrvr.SubmitProposal(suite.ctx, msg) if c.expErr { suite.Require().Error(err) + suite.Require().Contains(err.Error(), c.expErrMsg) } else { suite.Require().NoError(err) suite.Require().NotNil(res.ProposalId) @@ -619,6 +925,7 @@ func (suite *KeeperTestSuite) TestLegacyMsgVote() { proposalID := res.ProposalId suite.acctKeeper.EXPECT().StringToBytes(longAddress).Return(nil, errors.New(longAddressError)).AnyTimes() + suite.acctKeeper.EXPECT().StringToBytes("").Return(nil, errors.New(emptyAddressError)) cases := map[string]struct { preRun func() uint64 @@ -628,6 +935,26 @@ func (suite *KeeperTestSuite) TestLegacyMsgVote() { metadata string voter sdk.AccAddress }{ + "empty voter": { + preRun: func() uint64 { + return proposalID + }, + option: v1beta1.OptionYes, + voter: sdk.AccAddress{}, + metadata: "", + expErr: true, + expErrMsg: "invalid voter address", + }, + "wrong vote option": { + preRun: func() uint64 { + return proposalID + }, + option: v1beta1.VoteOption(0x13), + voter: proposer, + metadata: "", + expErr: true, + expErrMsg: "invalid vote option", + }, "vote on inactive proposal": { preRun: func() uint64 { msg, err := v1.NewMsgSubmitProposal( @@ -733,16 +1060,140 @@ func (suite *KeeperTestSuite) TestLegacyVoteWeighted() { proposalID := res.ProposalId suite.acctKeeper.EXPECT().StringToBytes(longAddress).Return(nil, errors.New(longAddressError)).AnyTimes() + suite.acctKeeper.EXPECT().StringToBytes("").Return(nil, errors.New(emptyAddressError)) cases := map[string]struct { preRun func() uint64 vote *v1beta1.MsgVote expErr bool expErrMsg string - option v1beta1.VoteOption + option v1beta1.WeightedVoteOptions metadata string voter sdk.AccAddress }{ + "empty voter": { + preRun: func() uint64 { + return proposalID + }, + option: v1beta1.WeightedVoteOptions{ + v1beta1.WeightedVoteOption{ + Option: v1beta1.OptionYes, + Weight: sdkmath.LegacyNewDec(1), + }, + }, + voter: sdk.AccAddress{}, + metadata: "", + expErr: true, + expErrMsg: "invalid voter address", + }, + "weights sum > 1": { + preRun: func() uint64 { + return proposalID + }, + option: v1beta1.WeightedVoteOptions{ + v1beta1.WeightedVoteOption{ + Option: v1beta1.OptionYes, + Weight: sdkmath.LegacyNewDec(1), + }, + v1beta1.WeightedVoteOption{ + Option: v1beta1.OptionAbstain, + Weight: sdkmath.LegacyNewDec(1), + }, + }, + voter: proposer, + metadata: "", + expErr: true, + expErrMsg: "total weight overflow 1.00: invalid vote option", + }, + "duplicate vote options": { + preRun: func() uint64 { + return proposalID + }, + option: v1beta1.WeightedVoteOptions{ + v1beta1.WeightedVoteOption{ + Option: v1beta1.OptionYes, + Weight: sdk.NewDecWithPrec(5, 1), + }, + v1beta1.WeightedVoteOption{ + Option: v1beta1.OptionYes, + Weight: sdk.NewDecWithPrec(5, 1), + }, + }, + voter: proposer, + metadata: "", + expErr: true, + expErrMsg: "duplicated vote option", + }, + "zero weight": { + preRun: func() uint64 { + return proposalID + }, + option: v1beta1.WeightedVoteOptions{ + v1beta1.WeightedVoteOption{ + Option: v1beta1.OptionYes, + Weight: sdkmath.LegacyNewDec(0), + }, + }, + voter: proposer, + metadata: "", + expErr: true, + expErrMsg: `option:VOTE_OPTION_YES weight:"0.000000000000000000" : invalid vote option`, + }, + "negative weight": { + preRun: func() uint64 { + return proposalID + }, + option: v1beta1.WeightedVoteOptions{ + v1beta1.WeightedVoteOption{ + Option: v1beta1.OptionYes, + Weight: sdkmath.LegacyNewDec(-1), + }, + }, + voter: proposer, + metadata: "", + expErr: true, + expErrMsg: `option:VOTE_OPTION_YES weight:"-1.000000000000000000" : invalid vote option`, + }, + "empty options": { + preRun: func() uint64 { + return proposalID + }, + option: v1beta1.WeightedVoteOptions{}, + voter: proposer, + metadata: "", + expErr: true, + expErrMsg: "invalid request", + }, + "invalid vote option": { + preRun: func() uint64 { + return proposalID + }, + option: v1beta1.WeightedVoteOptions{ + v1beta1.WeightedVoteOption{ + Option: v1beta1.VoteOption(0x13), + Weight: sdk.NewDecWithPrec(5, 1), + }, + }, + voter: proposer, + metadata: "", + expErr: true, + expErrMsg: "invalid vote option", + }, + "weight sum < 1": { + preRun: func() uint64 { + return proposalID + }, + option: v1beta1.WeightedVoteOptions{ + v1beta1.WeightedVoteOption{ + Option: v1beta1.OptionYes, + Weight: sdk.NewDecWithPrec(5, 1), + }, + }, + voter: proposer, + metadata: "", + expErr: true, + expErrMsg: "total weight lower than 1.00: invalid vote option", + }, "vote on inactive proposal": { preRun: func() uint64 { msg, err := v1.NewMsgSubmitProposal( @@ -761,7 +1212,12 @@ func (suite *KeeperTestSuite) TestLegacyVoteWeighted() { suite.Require().NotNil(res.ProposalId) return res.ProposalId }, - option: v1beta1.OptionYes, + option: v1beta1.WeightedVoteOptions{ + v1beta1.WeightedVoteOption{ + Option: v1beta1.OptionYes, + Weight: sdkmath.LegacyNewDec(1), + }, + }, voter: proposer, metadata: "", expErr: true, @@ -771,7 +1227,12 @@ func (suite *KeeperTestSuite) TestLegacyVoteWeighted() { preRun: func() uint64 { return proposalID }, - option: v1beta1.OptionYes, + option: v1beta1.WeightedVoteOptions{ + v1beta1.WeightedVoteOption{ + Option: v1beta1.OptionYes, + Weight: sdkmath.LegacyNewDec(1), + }, + }, voter: sdk.AccAddress(strings.Repeat("a", 300)), metadata: "", expErr: true, @@ -795,7 +1256,12 @@ func (suite *KeeperTestSuite) TestLegacyVoteWeighted() { suite.Require().NotNil(res.ProposalId) return res.ProposalId }, - option: v1beta1.OptionYes, + option: v1beta1.WeightedVoteOptions{ + v1beta1.WeightedVoteOption{ + Option: v1beta1.OptionYes, + Weight: sdkmath.LegacyNewDec(1), + }, + }, voter: proposer, metadata: "", expErr: false, @@ -805,7 +1271,7 @@ func (suite *KeeperTestSuite) TestLegacyVoteWeighted() { for name, tc := range cases { suite.Run(name, func() { pID := tc.preRun() - voteReq := v1beta1.NewMsgVoteWeighted(tc.voter, pID, v1beta1.NewNonSplitVoteOption(tc.option)) + voteReq := v1beta1.NewMsgVoteWeighted(tc.voter, pID, tc.option) _, err := suite.legacyMsgSrvr.VoteWeighted(suite.ctx, voteReq) if tc.expErr { suite.Require().Error(err) @@ -846,13 +1312,15 @@ func (suite *KeeperTestSuite) TestLegacyMsgDeposit() { suite.Require().NotNil(res.ProposalId) pID := res.ProposalId + suite.acctKeeper.EXPECT().StringToBytes("").Return(nil, errors.New(emptyAddressError)) + cases := map[string]struct { preRun func() uint64 expErr bool + expErrMsg string proposalID uint64 depositor sdk.AccAddress deposit sdk.Coins - options v1beta1.WeightedVoteOptions }{ "wrong proposal id": { preRun: func() uint64 { @@ -861,7 +1329,16 @@ func (suite *KeeperTestSuite) TestLegacyMsgDeposit() { depositor: proposer, deposit: coins, expErr: true, - options: v1beta1.NewNonSplitVoteOption(v1beta1.OptionYes), + expErrMsg: "unknown proposal", + }, + "empty depositer": { + preRun: func() uint64 { + return pID + }, + depositor: sdk.AccAddress{}, + deposit: coins, + expErr: true, + expErrMsg: "invalid depositor address: empty address string is not allowed", }, "all good": { preRun: func() uint64 { @@ -870,7 +1347,6 @@ func (suite *KeeperTestSuite) TestLegacyMsgDeposit() { depositor: proposer, deposit: minDeposit, expErr: false, - options: v1beta1.NewNonSplitVoteOption(v1beta1.OptionYes), }, } @@ -881,6 +1357,7 @@ func (suite *KeeperTestSuite) TestLegacyMsgDeposit() { _, err := suite.legacyMsgSrvr.Deposit(suite.ctx, depositReq) if tc.expErr { suite.Require().Error(err) + suite.Require().Contains(err.Error(), tc.expErrMsg) } else { suite.Require().NoError(err) } @@ -916,7 +1393,7 @@ func (suite *KeeperTestSuite) TestMsgUpdateParams() { } }, expErr: true, - expErrMsg: "invalid authority address", + expErrMsg: "invalid authority", }, { name: "invalid min deposit", @@ -1140,10 +1617,6 @@ func (suite *KeeperTestSuite) TestMsgUpdateParams() { suite.Run(tc.name, func() { msg := tc.input() exec := func(updateParams *v1.MsgUpdateParams) error { - if err := msg.ValidateBasic(); err != nil { - return err - } - if _, err := suite.msgSrvr.UpdateParams(suite.ctx, updateParams); err != nil { return err } diff --git a/x/gov/types/v1/msgs.go b/x/gov/types/v1/msgs.go index d3c59a6e1e..7ba8e69a97 100644 --- a/x/gov/types/v1/msgs.go +++ b/x/gov/types/v1/msgs.go @@ -1,18 +1,11 @@ package v1 import ( - "fmt" - - errorsmod "cosmossdk.io/errors" - "cosmossdk.io/math" - codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" sdktx "github.com/cosmos/cosmos-sdk/types/tx" "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" "github.com/cosmos/cosmos-sdk/x/gov/codec" - "github.com/cosmos/cosmos-sdk/x/gov/types" "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" ) @@ -65,53 +58,6 @@ func (m *MsgSubmitProposal) SetMsgs(msgs []sdk.Msg) error { return nil } -// ValidateBasic implements the sdk.Msg interface. -func (m MsgSubmitProposal) ValidateBasic() error { - if m.Title == "" { - return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "proposal title cannot be empty") - } - if m.Summary == "" { - return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "proposal summary cannot be empty") - } - - if _, err := sdk.AccAddressFromBech32(m.Proposer); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid proposer address: %s", err) - } - - deposit := sdk.NewCoins(m.InitialDeposit...) - if !deposit.IsValid() { - return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, deposit.String()) - } - - if deposit.IsAnyNegative() { - return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, deposit.String()) - } - - // Check that either metadata or Msgs length is non nil. - if len(m.Messages) == 0 && len(m.Metadata) == 0 { - return errorsmod.Wrap(types.ErrNoProposalMsgs, "either metadata or Msgs length must be non-nil") - } - - msgs, err := m.GetMsgs() - if err != nil { - return err - } - - for idx, msg := range msgs { - m, ok := msg.(sdk.HasValidateBasic) - if !ok { - continue - } - - if err := m.ValidateBasic(); err != nil { - return errorsmod.Wrap(types.ErrInvalidProposalMsg, - fmt.Sprintf("msg: %d, err: %s", idx, err.Error())) - } - } - - return nil -} - // GetSignBytes returns the message bytes to sign over. func (m MsgSubmitProposal) GetSignBytes() []byte { bz := codec.Amino.MustMarshalJSON(&m) @@ -134,22 +80,6 @@ func NewMsgDeposit(depositor sdk.AccAddress, proposalID uint64, amount sdk.Coins return &MsgDeposit{proposalID, depositor.String(), amount} } -// ValidateBasic implements the sdk.Msg interface. -func (msg MsgDeposit) ValidateBasic() error { - if _, err := sdk.AccAddressFromBech32(msg.Depositor); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid depositor address: %s", err) - } - amount := sdk.NewCoins(msg.Amount...) - if !amount.IsValid() { - return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, amount.String()) - } - if amount.IsAnyNegative() { - return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, amount.String()) - } - - return nil -} - // GetSignBytes returns the message bytes to sign over. func (msg MsgDeposit) GetSignBytes() []byte { bz := codec.Amino.MustMarshalJSON(&msg) @@ -167,18 +97,6 @@ func NewMsgVote(voter sdk.AccAddress, proposalID uint64, option VoteOption, meta return &MsgVote{proposalID, voter.String(), option, metadata} } -// ValidateBasic implements the sdk.Msg interface. -func (msg MsgVote) ValidateBasic() error { - if _, err := sdk.AccAddressFromBech32(msg.Voter); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid voter address: %s", err) - } - if !ValidVoteOption(msg.Option) { - return errorsmod.Wrap(types.ErrInvalidVote, msg.Option.String()) - } - - return nil -} - // GetSignBytes returns the message bytes to sign over. func (msg MsgVote) GetSignBytes() []byte { bz := codec.Amino.MustMarshalJSON(&msg) @@ -196,43 +114,6 @@ func NewMsgVoteWeighted(voter sdk.AccAddress, proposalID uint64, options Weighte return &MsgVoteWeighted{proposalID, voter.String(), options, metadata} } -// ValidateBasic implements the sdk.Msg interface. -func (msg MsgVoteWeighted) ValidateBasic() error { - if _, err := sdk.AccAddressFromBech32(msg.Voter); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid voter address: %s", err) - } - if len(msg.Options) == 0 { - return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, WeightedVoteOptions(msg.Options).String()) - } - - totalWeight := math.LegacyNewDec(0) - usedOptions := make(map[VoteOption]bool) - for _, option := range msg.Options { - if !option.IsValid() { - return errorsmod.Wrap(types.ErrInvalidVote, option.String()) - } - weight, err := sdk.NewDecFromStr(option.Weight) - if err != nil { - return errorsmod.Wrapf(types.ErrInvalidVote, "Invalid weight: %s", err) - } - totalWeight = totalWeight.Add(weight) - if usedOptions[option.Option] { - return errorsmod.Wrap(types.ErrInvalidVote, "Duplicated vote option") - } - usedOptions[option.Option] = true - } - - if totalWeight.GT(math.LegacyNewDec(1)) { - return errorsmod.Wrap(types.ErrInvalidVote, "Total weight overflow 1.00") - } - - if totalWeight.LT(math.LegacyNewDec(1)) { - return errorsmod.Wrap(types.ErrInvalidVote, "Total weight lower than 1.00") - } - - return nil -} - // GetSignBytes returns the message bytes to sign over. func (msg MsgVoteWeighted) GetSignBytes() []byte { bz := codec.Amino.MustMarshalJSON(&msg) @@ -281,15 +162,6 @@ func (c MsgExecLegacyContent) UnpackInterfaces(unpacker codectypes.AnyUnpacker) return unpacker.UnpackAny(c.Content, &content) } -// ValidateBasic implements the sdk.Msg interface. -func (msg MsgUpdateParams) ValidateBasic() error { - if _, err := sdk.AccAddressFromBech32(msg.Authority); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid authority address: %s", err) - } - - return msg.Params.ValidateBasic() -} - // GetSignBytes returns the message bytes to sign over. func (msg MsgUpdateParams) GetSignBytes() []byte { bz := codec.Amino.MustMarshalJSON(&msg) @@ -310,15 +182,6 @@ func NewMsgCancelProposal(proposalID uint64, proposer string) *MsgCancelProposal } } -// ValidateBasic implements Msg -func (msg MsgCancelProposal) ValidateBasic() error { - if _, err := sdk.AccAddressFromBech32(msg.Proposer); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid proposer address: %s", err) - } - - return nil -} - // GetSignBytes implements Msg func (msg MsgCancelProposal) GetSignBytes() []byte { bz := codec.Amino.MustMarshalJSON(&msg) diff --git a/x/gov/types/v1/msgs_test.go b/x/gov/types/v1/msgs_test.go index d0dfe7d170..456e7429a5 100644 --- a/x/gov/types/v1/msgs_test.go +++ b/x/gov/types/v1/msgs_test.go @@ -4,25 +4,20 @@ import ( "fmt" "testing" - "cosmossdk.io/math" "github.com/stretchr/testify/require" sdk "github.com/cosmos/cosmos-sdk/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" - "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" ) var ( coinsPos = sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, 1000)) - coinsZero = sdk.NewCoins() coinsMulti = sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, 1000), sdk.NewInt64Coin("foo", 10000)) addrs = []sdk.AccAddress{ sdk.AccAddress("test1"), sdk.AccAddress("test2"), } - - metadata = "metadata" ) func init() { @@ -38,139 +33,6 @@ func TestMsgDepositGetSignBytes(t *testing.T) { require.Equal(t, expected, string(res)) } -// test ValidateBasic for MsgDeposit -func TestMsgDeposit(t *testing.T) { - tests := []struct { - proposalID uint64 - depositorAddr sdk.AccAddress - depositAmount sdk.Coins - expectPass bool - }{ - {0, addrs[0], coinsPos, true}, - {1, sdk.AccAddress{}, coinsPos, false}, - {1, addrs[0], coinsZero, true}, - {1, addrs[0], coinsMulti, true}, - } - - for i, tc := range tests { - msg := v1.NewMsgDeposit(tc.depositorAddr, tc.proposalID, tc.depositAmount) - if tc.expectPass { - require.NoError(t, msg.ValidateBasic(), "test: %v", i) - } else { - require.Error(t, msg.ValidateBasic(), "test: %v", i) - } - } -} - -// test ValidateBasic for MsgVote -func TestMsgVote(t *testing.T) { - tests := []struct { - proposalID uint64 - voterAddr sdk.AccAddress - option v1.VoteOption - metadata string - expectPass bool - }{ - {0, addrs[0], v1.OptionYes, metadata, true}, - {0, sdk.AccAddress{}, v1.OptionYes, "", false}, - {0, addrs[0], v1.OptionNo, metadata, true}, - {0, addrs[0], v1.OptionNoWithVeto, "", true}, - {0, addrs[0], v1.OptionAbstain, "", true}, - {0, addrs[0], v1.VoteOption(0x13), "", false}, - } - - for i, tc := range tests { - msg := v1.NewMsgVote(tc.voterAddr, tc.proposalID, tc.option, tc.metadata) - if tc.expectPass { - require.Nil(t, msg.ValidateBasic(), "test: %v", i) - } else { - require.NotNil(t, msg.ValidateBasic(), "test: %v", i) - } - } -} - -// test ValidateBasic for MsgVoteWeighted -func TestMsgVoteWeighted(t *testing.T) { - tests := []struct { - proposalID uint64 - voterAddr sdk.AccAddress - options v1.WeightedVoteOptions - metadata string - expectPass bool - }{ - {0, addrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), metadata, true}, - {0, sdk.AccAddress{}, v1.NewNonSplitVoteOption(v1.OptionYes), "", false}, - {0, addrs[0], v1.NewNonSplitVoteOption(v1.OptionNo), "", true}, - {0, addrs[0], v1.NewNonSplitVoteOption(v1.OptionNoWithVeto), "", true}, - {0, addrs[0], v1.NewNonSplitVoteOption(v1.OptionAbstain), "", true}, - {0, addrs[0], v1.WeightedVoteOptions{ // weight sum > 1 - v1.NewWeightedVoteOption(v1.OptionYes, math.LegacyNewDec(1)), - v1.NewWeightedVoteOption(v1.OptionAbstain, math.LegacyNewDec(1)), - }, "", false}, - {0, addrs[0], v1.WeightedVoteOptions{ // duplicate option - v1.NewWeightedVoteOption(v1.OptionYes, sdk.NewDecWithPrec(5, 1)), - v1.NewWeightedVoteOption(v1.OptionYes, sdk.NewDecWithPrec(5, 1)), - }, "", false}, - {0, addrs[0], v1.WeightedVoteOptions{ // zero weight - v1.NewWeightedVoteOption(v1.OptionYes, math.LegacyNewDec(0)), - }, "", false}, - {0, addrs[0], v1.WeightedVoteOptions{ // negative weight - v1.NewWeightedVoteOption(v1.OptionYes, math.LegacyNewDec(-1)), - }, "", false}, - {0, addrs[0], v1.WeightedVoteOptions{}, "", false}, - {0, addrs[0], v1.NewNonSplitVoteOption(v1.VoteOption(0x13)), "", false}, - {0, addrs[0], v1.WeightedVoteOptions{ // weight sum <1 - v1.NewWeightedVoteOption(v1.OptionYes, sdk.NewDecWithPrec(5, 1)), - }, "", false}, - } - - for i, tc := range tests { - msg := v1.NewMsgVoteWeighted(tc.voterAddr, tc.proposalID, tc.options, tc.metadata) - if tc.expectPass { - require.Nil(t, msg.ValidateBasic(), "test: %v", i) - } else { - require.NotNil(t, msg.ValidateBasic(), "test: %v", i) - } - } -} - -func TestMsgSubmitProposal_ValidateBasic(t *testing.T) { - // Valid msg - msg1, err := v1.NewLegacyContent(v1beta1.NewTextProposal("Title", "description"), addrs[0].String()) - require.NoError(t, err) - // Invalid msg - msg2, err := v1.NewLegacyContent(v1beta1.NewTextProposal("Title", "description"), "foo") - require.NoError(t, err) - - tests := []struct { - name string - proposer string - initialDeposit sdk.Coins - messages []sdk.Msg - metadata, title, summary string - expedited bool - expErr bool - }{ - {"invalid addr", "", coinsPos, []sdk.Msg{msg1}, metadata, "Title", "Summary", false, true}, - {"empty msgs and metadata", addrs[0].String(), coinsPos, nil, "", "Title", "Summary", false, true}, - {"empty title and summary", addrs[0].String(), coinsPos, nil, "", "", "", false, true}, - {"invalid msg", addrs[0].String(), coinsPos, []sdk.Msg{msg1, msg2}, metadata, "Title", "Summary", false, true}, - {"valid with no Msg", addrs[0].String(), coinsPos, nil, metadata, "Title", "Summary", false, false}, - {"valid with no metadata", addrs[0].String(), coinsPos, []sdk.Msg{msg1}, "", "Title", "Summary", false, false}, - {"valid with everything", addrs[0].String(), coinsPos, []sdk.Msg{msg1}, metadata, "Title", "Summary", true, false}, - } - - for _, tc := range tests { - msg, err := v1.NewMsgSubmitProposal(tc.messages, tc.initialDeposit, tc.proposer, tc.metadata, tc.title, tc.summary, tc.expedited) - require.NoError(t, err) - if tc.expErr { - require.Error(t, msg.ValidateBasic(), "test: %s", tc.name) - } else { - require.NoError(t, msg.ValidateBasic(), "test: %s", tc.name) - } - } -} - // this tests that Amino JSON MsgSubmitProposal.GetSignBytes() still works with Content as Any using the ModuleCdc func TestMsgSubmitProposal_GetSignBytes(t *testing.T) { testcases := []struct { diff --git a/x/gov/types/v1beta1/msgs.go b/x/gov/types/v1beta1/msgs.go index 50fc1ca40a..0cc741fd3b 100644 --- a/x/gov/types/v1beta1/msgs.go +++ b/x/gov/types/v1beta1/msgs.go @@ -3,17 +3,12 @@ package v1beta1 import ( "fmt" - "cosmossdk.io/math" "github.com/cosmos/gogoproto/proto" - errorsmod "cosmossdk.io/errors" - codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" "github.com/cosmos/cosmos-sdk/x/gov/codec" - "github.com/cosmos/cosmos-sdk/x/gov/types" ) // Governance message types and routes @@ -86,32 +81,6 @@ func (m *MsgSubmitProposal) SetContent(content Content) error { return nil } -// ValidateBasic implements the sdk.Msg interface. -func (m MsgSubmitProposal) ValidateBasic() error { - if _, err := sdk.AccAddressFromBech32(m.Proposer); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid proposer address: %s", err) - } - if !m.InitialDeposit.IsValid() { - return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, m.InitialDeposit.String()) - } - if m.InitialDeposit.IsAnyNegative() { - return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, m.InitialDeposit.String()) - } - - content := m.GetContent() - if content == nil { - return errorsmod.Wrap(types.ErrInvalidProposalContent, "missing content") - } - if !IsValidProposalType(content.ProposalType()) { - return errorsmod.Wrap(types.ErrInvalidProposalType, content.ProposalType()) - } - if err := content.ValidateBasic(); err != nil { - return err - } - - return nil -} - // GetSignBytes returns the message bytes to sign over. func (m MsgSubmitProposal) GetSignBytes() []byte { bz := codec.Amino.MustMarshalJSON(&m) @@ -131,27 +100,10 @@ func (m MsgSubmitProposal) UnpackInterfaces(unpacker codectypes.AnyUnpacker) err } // NewMsgDeposit creates a new MsgDeposit instance -// - func NewMsgDeposit(depositor sdk.AccAddress, proposalID uint64, amount sdk.Coins) *MsgDeposit { return &MsgDeposit{proposalID, depositor.String(), amount} } -// ValidateBasic implements the sdk.Msg interface. -func (msg MsgDeposit) ValidateBasic() error { - if _, err := sdk.AccAddressFromBech32(msg.Depositor); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid depositor address: %s", err) - } - if !msg.Amount.IsValid() { - return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, msg.Amount.String()) - } - if msg.Amount.IsAnyNegative() { - return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, msg.Amount.String()) - } - - return nil -} - // GetSignBytes returns the message bytes to sign over. func (msg MsgDeposit) GetSignBytes() []byte { bz := codec.Amino.MustMarshalJSON(&msg) @@ -169,18 +121,6 @@ func NewMsgVote(voter sdk.AccAddress, proposalID uint64, option VoteOption) *Msg return &MsgVote{proposalID, voter.String(), option} } -// ValidateBasic implements the sdk.Msg interface. -func (msg MsgVote) ValidateBasic() error { - if _, err := sdk.AccAddressFromBech32(msg.Voter); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid voter address: %s", err) - } - if !ValidVoteOption(msg.Option) { - return errorsmod.Wrap(types.ErrInvalidVote, msg.Option.String()) - } - - return nil -} - // GetSignBytes returns the message bytes to sign over. func (msg MsgVote) GetSignBytes() []byte { bz := codec.Amino.MustMarshalJSON(&msg) @@ -198,39 +138,6 @@ func NewMsgVoteWeighted(voter sdk.AccAddress, proposalID uint64, options Weighte return &MsgVoteWeighted{proposalID, voter.String(), options} } -// ValidateBasic implements the sdk.Msg interface. -func (msg MsgVoteWeighted) ValidateBasic() error { - if _, err := sdk.AccAddressFromBech32(msg.Voter); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid voter address: %s", err) - } - if len(msg.Options) == 0 { - return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, WeightedVoteOptions(msg.Options).String()) - } - - totalWeight := math.LegacyNewDec(0) - usedOptions := make(map[VoteOption]bool) - for _, option := range msg.Options { - if !ValidWeightedVoteOption(option) { - return errorsmod.Wrap(types.ErrInvalidVote, option.String()) - } - totalWeight = totalWeight.Add(option.Weight) - if usedOptions[option.Option] { - return errorsmod.Wrap(types.ErrInvalidVote, "Duplicated vote option") - } - usedOptions[option.Option] = true - } - - if totalWeight.GT(math.LegacyNewDec(1)) { - return errorsmod.Wrap(types.ErrInvalidVote, "Total weight overflow 1.00") - } - - if totalWeight.LT(math.LegacyNewDec(1)) { - return errorsmod.Wrap(types.ErrInvalidVote, "Total weight lower than 1.00") - } - - return nil -} - // GetSignBytes returns the message bytes to sign over. func (msg MsgVoteWeighted) GetSignBytes() []byte { bz := codec.Amino.MustMarshalJSON(&msg) diff --git a/x/gov/types/v1beta1/msgs_test.go b/x/gov/types/v1beta1/msgs_test.go index 84c592bf47..d041ccb834 100644 --- a/x/gov/types/v1beta1/msgs_test.go +++ b/x/gov/types/v1beta1/msgs_test.go @@ -1,10 +1,8 @@ package v1beta1 import ( - "strings" "testing" - "cosmossdk.io/math" "github.com/stretchr/testify/require" sdk "github.com/cosmos/cosmos-sdk/types" @@ -12,57 +10,13 @@ import ( var ( coinsPos = sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, 1000)) - coinsZero = sdk.NewCoins() coinsMulti = sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, 1000), sdk.NewInt64Coin("foo", 10000)) - addrs = []sdk.AccAddress{ - sdk.AccAddress("test1"), - sdk.AccAddress("test2"), - } ) func init() { coinsMulti.Sort() } -// test ValidateBasic for MsgCreateValidator -func TestMsgSubmitProposal(t *testing.T) { - tests := []struct { - title, description string - proposalType string - proposerAddr sdk.AccAddress - initialDeposit sdk.Coins - expectPass bool - }{ - {"Test Proposal", "the purpose of this proposal is to test", ProposalTypeText, addrs[0], coinsPos, true}, - {"", "the purpose of this proposal is to test", ProposalTypeText, addrs[0], coinsPos, false}, - {"Test Proposal", "", ProposalTypeText, addrs[0], coinsPos, false}, - {"Test Proposal", "the purpose of this proposal is to test", ProposalTypeText, sdk.AccAddress{}, coinsPos, false}, - {"Test Proposal", "the purpose of this proposal is to test", ProposalTypeText, addrs[0], coinsZero, true}, - {"Test Proposal", "the purpose of this proposal is to test", ProposalTypeText, addrs[0], coinsMulti, true}, - {strings.Repeat("#", MaxTitleLength*2), "the purpose of this proposal is to test", ProposalTypeText, addrs[0], coinsMulti, false}, - {"Test Proposal", strings.Repeat("#", MaxDescriptionLength*2), ProposalTypeText, addrs[0], coinsMulti, false}, - } - - for i, tc := range tests { - content, ok := ContentFromProposalType(tc.title, tc.description, tc.proposalType) - require.True(t, ok) - - msg, err := NewMsgSubmitProposal( - content, - tc.initialDeposit, - tc.proposerAddr, - ) - - require.NoError(t, err) - - if tc.expectPass { - require.NoError(t, msg.ValidateBasic(), "test: %v", i) - } else { - require.Error(t, msg.ValidateBasic(), "test: %v", i) - } - } -} - func TestMsgDepositGetSignBytes(t *testing.T) { addr := sdk.AccAddress("addr1") msg := NewMsgDeposit(addr, 0, coinsPos) @@ -72,100 +26,6 @@ func TestMsgDepositGetSignBytes(t *testing.T) { require.Equal(t, expected, string(res)) } -// test ValidateBasic for MsgDeposit -func TestMsgDeposit(t *testing.T) { - tests := []struct { - proposalID uint64 - depositorAddr sdk.AccAddress - depositAmount sdk.Coins - expectPass bool - }{ - {0, addrs[0], coinsPos, true}, - {1, sdk.AccAddress{}, coinsPos, false}, - {1, addrs[0], coinsZero, true}, - {1, addrs[0], coinsMulti, true}, - } - - for i, tc := range tests { - msg := NewMsgDeposit(tc.depositorAddr, tc.proposalID, tc.depositAmount) - if tc.expectPass { - require.NoError(t, msg.ValidateBasic(), "test: %v", i) - } else { - require.Error(t, msg.ValidateBasic(), "test: %v", i) - } - } -} - -// test ValidateBasic for MsgVote -func TestMsgVote(t *testing.T) { - tests := []struct { - proposalID uint64 - voterAddr sdk.AccAddress - option VoteOption - expectPass bool - }{ - {0, addrs[0], OptionYes, true}, - {0, sdk.AccAddress{}, OptionYes, false}, - {0, addrs[0], OptionNo, true}, - {0, addrs[0], OptionNoWithVeto, true}, - {0, addrs[0], OptionAbstain, true}, - {0, addrs[0], VoteOption(0x13), false}, - } - - for i, tc := range tests { - msg := NewMsgVote(tc.voterAddr, tc.proposalID, tc.option) - if tc.expectPass { - require.Nil(t, msg.ValidateBasic(), "test: %v", i) - } else { - require.NotNil(t, msg.ValidateBasic(), "test: %v", i) - } - } -} - -// test ValidateBasic for MsgVoteWeighted -func TestMsgVoteWeighted(t *testing.T) { - tests := []struct { - proposalID uint64 - voterAddr sdk.AccAddress - options WeightedVoteOptions - expectPass bool - }{ - {0, addrs[0], NewNonSplitVoteOption(OptionYes), true}, - {0, sdk.AccAddress{}, NewNonSplitVoteOption(OptionYes), false}, - {0, addrs[0], NewNonSplitVoteOption(OptionNo), true}, - {0, addrs[0], NewNonSplitVoteOption(OptionNoWithVeto), true}, - {0, addrs[0], NewNonSplitVoteOption(OptionAbstain), true}, - {0, addrs[0], WeightedVoteOptions{ // weight sum > 1 - WeightedVoteOption{Option: OptionYes, Weight: math.LegacyNewDec(1)}, - WeightedVoteOption{Option: OptionAbstain, Weight: math.LegacyNewDec(1)}, - }, false}, - {0, addrs[0], WeightedVoteOptions{ // duplicate option - WeightedVoteOption{Option: OptionYes, Weight: math.LegacyNewDecWithPrec(5, 1)}, - WeightedVoteOption{Option: OptionYes, Weight: math.LegacyNewDecWithPrec(5, 1)}, - }, false}, - {0, addrs[0], WeightedVoteOptions{ // zero weight - WeightedVoteOption{Option: OptionYes, Weight: math.LegacyNewDec(0)}, - }, false}, - {0, addrs[0], WeightedVoteOptions{ // negative weight - WeightedVoteOption{Option: OptionYes, Weight: math.LegacyNewDec(-1)}, - }, false}, - {0, addrs[0], WeightedVoteOptions{}, false}, - {0, addrs[0], NewNonSplitVoteOption(VoteOption(0x13)), false}, - {0, addrs[0], WeightedVoteOptions{ // weight sum <1 - WeightedVoteOption{Option: OptionYes, Weight: math.LegacyNewDecWithPrec(5, 1)}, - }, false}, - } - - for i, tc := range tests { - msg := NewMsgVoteWeighted(tc.voterAddr, tc.proposalID, tc.options) - if tc.expectPass { - require.Nil(t, msg.ValidateBasic(), "test: %v", i) - } else { - require.NotNil(t, msg.ValidateBasic(), "test: %v", i) - } - } -} - // this tests that Amino JSON MsgSubmitProposal.GetSignBytes() still works with Content as Any using the ModuleCdc func TestMsgSubmitProposal_GetSignBytes(t *testing.T) { msg, err := NewMsgSubmitProposal(NewTextProposal("test", "abcd"), sdk.NewCoins(), sdk.AccAddress{})