From 0cc24cf3f71a5448ffbf0e9f586516b586bfaf5c Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Wed, 26 Jan 2022 12:51:37 +0100 Subject: [PATCH 1/7] v1beta1 query server --- x/gov/keeper/grpc_query.go | 57 +++++++++++++++++++++++++++++ x/gov/migrations/v046/migrate.go | 62 ++++++++++++++++++++++++++++++++ x/gov/module.go | 4 +-- 3 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 x/gov/migrations/v046/migrate.go diff --git a/x/gov/keeper/grpc_query.go b/x/gov/keeper/grpc_query.go index 05ab8c4171a..fd3433d65e4 100644 --- a/x/gov/keeper/grpc_query.go +++ b/x/gov/keeper/grpc_query.go @@ -11,6 +11,8 @@ import ( "github.com/cosmos/cosmos-sdk/types/query" "github.com/cosmos/cosmos-sdk/x/gov/types" "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta2" + "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" + "github.com/cosmos/cosmos-sdk/x/gov/migrations/v046" ) var _ v1beta2.QueryServer = Keeper{} @@ -278,3 +280,58 @@ func (q Keeper) TallyResult(c context.Context, req *v1beta2.QueryTallyResultRequ return &v1beta2.QueryTallyResultResponse{Tally: &tallyResult}, nil } + +var _ v1beta1.QueryServer = legacyQueryServer{} + +type legacyQueryServer struct { + keeper Keeper +} + +func NewLegacyQueryServer(k Keeper) v1beta1.QueryServer { + return &legacyQueryServer{keeper: k} +} + +func (q legacyQueryServer) Proposal(c context.Context, req *v1beta1.QueryProposalRequest) (*v1beta1.QueryProposalResponse, error) { + resp, err := q.keeper.Proposal(c, &v1beta2.QueryProposalRequest{ + ProposalId: req.ProposalId, + }) + if err != nil { + return nil, err + } + + proposal, err := v046.ConvertToLegacyProposal(*resp.Proposal) + if err != nil { + return nil, err + } + + return &v1beta1.QueryProposalResponse{Proposal: proposal}, nil +} + +func (q legacyQueryServer) Proposals(c context.Context, req *v1beta1.QueryProposalsRequest) (*v1beta1.QueryProposalsResponse, error) { + return nil, nil +} + +func (q legacyQueryServer) Vote(c context.Context, req *v1beta1.QueryVoteRequest) (*v1beta1.QueryVoteResponse, error) { + return nil, nil +} + +func (q legacyQueryServer) Votes(c context.Context, req *v1beta1.QueryVotesRequest) (*v1beta1.QueryVotesResponse, error) { + return nil, nil +} + +func (q legacyQueryServer) Params(c context.Context, req *v1beta1.QueryParamsRequest) (*v1beta1.QueryParamsResponse, error) { + return nil, nil +} + +func (q legacyQueryServer) Deposit(c context.Context, req *v1beta1.QueryDepositRequest) (*v1beta1.QueryDepositResponse, error) { + return nil, nil +} + +func (q legacyQueryServer) Deposits(c context.Context, req *v1beta1.QueryDepositsRequest) (*v1beta1.QueryDepositsResponse, error) { + return nil, nil +} + +func (q legacyQueryServer) TallyResult(c context.Context, req *v1beta1.QueryTallyResultRequest) (*v1beta1.QueryTallyResultResponse, error) { + return nil, nil +} + diff --git a/x/gov/migrations/v046/migrate.go b/x/gov/migrations/v046/migrate.go new file mode 100644 index 00000000000..21ea92d8eb1 --- /dev/null +++ b/x/gov/migrations/v046/migrate.go @@ -0,0 +1,62 @@ +package v046 + +import ( + "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" + "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta2" +) + +// ConvertToLegacyProposal takes a new proposal and attempts to convert it to the +// legacy proposal format. This conversion is best effort. New proposal types that +// don't have a legacy message will return a "nil" content. +func ConvertToLegacyProposal(proposal v1beta2.Proposal) (v1beta1.Proposal, error) { + legacyProposal := v1beta1.Proposal{ + ProposalId: proposal.ProposalId, + Content: nil, + Status: v1beta1.ProposalStatus(proposal.Status), + FinalTallyResult: ConvertToLegacyTallyResult(proposal.FinalTallyResult), + SubmitTime: *proposal.SubmitTime, + DepositEndTime: *proposal.DepositEndTime, + TotalDeposit: types.NewCoins(proposal.TotalDeposit...), + VotingStartTime: *proposal.VotingStartTime, + VotingEndTime: *proposal.VotingEndTime, + } + + msgs, err := proposal.GetMsgs() + if err != nil { + return v1beta1.Proposal{}, err + } + msgTypes := make([]string, len(msgs)) + for idx, msg := range msgs { + msgTypes[idx] = msg.String() + if legacyMsg, ok := msg.(*v1beta2.MsgExecLegacyContent); ok { + // check that the content struct can be unmarshalled + _, err := v1beta2.LegacyContentFromMessage(legacyMsg) + if err != nil { + return v1beta1.Proposal{}, err + } + legacyProposal.Content = legacyMsg.Content + } + } + return legacyProposal, nil +} + +// ConvertToNewProposal takes a legacy proposal and converts it to a new proposal, +// wrapping the content into a "MsgExecLegacyContent". +func ConvertToNewProposal(proposal v1beta1.Proposal) v1beta2.Proposal { + return v1beta2.Proposal{} +} + +func ConvertToLegacyTallyResult(tally *v1beta2.TallyResult) v1beta1.TallyResult { + yes, _ := types.NewIntFromString(tally.Yes) + no, _ := types.NewIntFromString(tally.No) + veto, _ := types.NewIntFromString(tally.NoWithVeto) + abstain, _ := types.NewIntFromString(tally.Abstain) + + return v1beta1.TallyResult{ + Yes: yes, + No: no, + NoWithVeto: veto, + Abstain: abstain, + } +} \ No newline at end of file diff --git a/x/gov/module.go b/x/gov/module.go index 3e2002efe34..d3ce2703d09 100644 --- a/x/gov/module.go +++ b/x/gov/module.go @@ -160,8 +160,8 @@ func (am AppModule) RegisterServices(cfg module.Configurator) { v1beta1.RegisterMsgServer(cfg.MsgServer(), keeper.NewLegacyMsgServerImpl(am.accountKeeper.GetModuleAddress(types.ModuleName).String(), msgServer)) v1beta2.RegisterMsgServer(cfg.MsgServer(), msgServer) - // TODO Register v1beta1 query server. - // https://github.com/cosmos/cosmos-sdk/issues/10951 + legacyQueryServer := keeper.NewLegacyQueryServer(am.keeper) + v1beta1.RegisterQueryServer(cfg.QueryServer(), legacyQueryServer) v1beta2.RegisterQueryServer(cfg.QueryServer(), am.keeper) m := keeper.NewMigrator(am.keeper) From 81bfb113c92ee70dbf39a8ca981186661d687070 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Wed, 26 Jan 2022 16:29:53 +0100 Subject: [PATCH 2/7] add more of the query server --- x/gov/keeper/grpc_query.go | 80 ++++++++++++++++++++++++++++++-- x/gov/migrations/v046/migrate.go | 25 +++++++++- 2 files changed, 100 insertions(+), 5 deletions(-) diff --git a/x/gov/keeper/grpc_query.go b/x/gov/keeper/grpc_query.go index fd3433d65e4..14272ed5e59 100644 --- a/x/gov/keeper/grpc_query.go +++ b/x/gov/keeper/grpc_query.go @@ -308,19 +308,91 @@ func (q legacyQueryServer) Proposal(c context.Context, req *v1beta1.QueryProposa } func (q legacyQueryServer) Proposals(c context.Context, req *v1beta1.QueryProposalsRequest) (*v1beta1.QueryProposalsResponse, error) { - return nil, nil + resp, err := q.keeper.Proposals(c, &v1beta2.QueryProposalsRequest{ + ProposalStatus: v1beta2.ProposalStatus(req.ProposalStatus), + Voter: req.Voter, + Depositor: req.Depositor, + Pagination: req.Pagination, + }) + if err != nil { + return nil, err + } + + legacyProposals := make([]v1beta1.Proposal, len(resp.Proposals)) + for idx, proposal := range resp.Proposals { + legacyProposals[idx], err = v046.ConvertToLegacyProposal(*proposal) + if err != nil { + return nil, err + } + } + + return &v1beta1.QueryProposalsResponse{ + Proposals: legacyProposals, + Pagination: resp.Pagination, + }, nil } func (q legacyQueryServer) Vote(c context.Context, req *v1beta1.QueryVoteRequest) (*v1beta1.QueryVoteResponse, error) { - return nil, nil + resp, err := q.keeper.Vote(c, &v1beta2.QueryVoteRequest{ + ProposalId: req.ProposalId, + Voter: req.Voter, + }) + if err != nil { + return nil, err + } + + vote := v046.ConvertToLegacyVote(*resp.Vote) + + return &v1beta1.QueryVoteResponse{Vote: vote}, nil } func (q legacyQueryServer) Votes(c context.Context, req *v1beta1.QueryVotesRequest) (*v1beta1.QueryVotesResponse, error) { - return nil, nil + resp, err := q.keeper.Votes(c, &v1beta2.QueryVotesRequest{ + ProposalId: req.ProposalId, + Pagination: req.Pagination, + }) + if err != nil { + return nil, err + } + + votes := make([]v1beta1.Vote, len(resp.Votes)) + for i, v := range resp.Votes { + votes[i] = v046.ConvertToLegacyVote(*v) + } + + return &v1beta1.QueryVotesResponse{ + Votes: votes, + Pagination: resp.Pagination, + }, nil } func (q legacyQueryServer) Params(c context.Context, req *v1beta1.QueryParamsRequest) (*v1beta1.QueryParamsResponse, error) { - return nil, nil + resp, err := q.keeper.Params(c, &v1beta2.QueryParamsRequest{ + ParamsType: req.ParamsType, + }) + if err != nil { + return nil, err + } + + minDeposit := sdk.NewCoins(resp.DepositParams.MinDeposit...) + quorum, err := sdk.NewDecFromStr(resp.TallyParams.Quorum) + if err != nil { + return nil, err + } + threshold, err := sdk.NewDecFromStr(resp.TallyParams.Threshold) + if err != nil { + return nil, err + } + vetoThreshold, err := sdk.NewDecFromStr(resp.TallyParams.VetoThreshold) + if err != nil { + return nil, err + } + + return &v1beta1.QueryParamsResponse{ + VotingParams: v1beta1.NewVotingParams(*resp.VotingParams.VotingPeriod), + DepositParams: v1beta1.NewDepositParams(minDeposit, *resp.DepositParams.MaxDepositPeriod), + TallyParams: v1beta1.NewTallyParams(quorum, threshold, vetoThreshold), + }, nil } func (q legacyQueryServer) Deposit(c context.Context, req *v1beta1.QueryDepositRequest) (*v1beta1.QueryDepositResponse, error) { diff --git a/x/gov/migrations/v046/migrate.go b/x/gov/migrations/v046/migrate.go index 21ea92d8eb1..683590d3175 100644 --- a/x/gov/migrations/v046/migrate.go +++ b/x/gov/migrations/v046/migrate.go @@ -59,4 +59,27 @@ func ConvertToLegacyTallyResult(tally *v1beta2.TallyResult) v1beta1.TallyResult NoWithVeto: veto, Abstain: abstain, } -} \ No newline at end of file +} + +func ConvertToLegacyVote(vote v1beta2.Vote) v1beta1.Vote { + return v1beta1.Vote{ + ProposalId: vote.ProposalId, + Voter: vote.Voter, + Options: ConvertToLegacyVoteOptions(vote.Options), + } +} + +func ConvertToLegacyVoteOptions(voteOptions []*v1beta2.WeightedVoteOption) []v1beta1.WeightedVoteOption { + options := make([]v1beta1.WeightedVoteOption, len(voteOptions)) + for i, option := range voteOptions { + weight, err := types.NewDecFromStr(option.Weight) + if err != nil { + panic(err) + } + options[i] = v1beta1.WeightedVoteOption{ + Option: v1beta1.VoteOption(option.Option), + Weight: weight, + } + } + return options +} From 188d37c103d4bb4feea5c4e778d438e79c16a806 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Thu, 27 Jan 2022 22:29:29 +0100 Subject: [PATCH 3/7] finish up query endpoints and write legacy querier tests --- x/gov/keeper/grpc_query.go | 57 ++++++++++++++++++------ x/gov/keeper/grpc_query_test.go | 76 ++++++++++++++++++++++++++++++++ x/gov/keeper/keeper_test.go | 15 +++++-- x/gov/migrations/v046/migrate.go | 56 ++++++++++++++--------- 4 files changed, 164 insertions(+), 40 deletions(-) diff --git a/x/gov/keeper/grpc_query.go b/x/gov/keeper/grpc_query.go index 14272ed5e59..1956b6a49a1 100644 --- a/x/gov/keeper/grpc_query.go +++ b/x/gov/keeper/grpc_query.go @@ -9,10 +9,10 @@ import ( "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/query" + "github.com/cosmos/cosmos-sdk/x/gov/migrations/v046" "github.com/cosmos/cosmos-sdk/x/gov/types" - "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta2" "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" - "github.com/cosmos/cosmos-sdk/x/gov/migrations/v046" + "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta2" ) var _ v1beta2.QueryServer = Keeper{} @@ -310,9 +310,9 @@ func (q legacyQueryServer) Proposal(c context.Context, req *v1beta1.QueryProposa func (q legacyQueryServer) Proposals(c context.Context, req *v1beta1.QueryProposalsRequest) (*v1beta1.QueryProposalsResponse, error) { resp, err := q.keeper.Proposals(c, &v1beta2.QueryProposalsRequest{ ProposalStatus: v1beta2.ProposalStatus(req.ProposalStatus), - Voter: req.Voter, - Depositor: req.Depositor, - Pagination: req.Pagination, + Voter: req.Voter, + Depositor: req.Depositor, + Pagination: req.Pagination, }) if err != nil { return nil, err @@ -327,7 +327,7 @@ func (q legacyQueryServer) Proposals(c context.Context, req *v1beta1.QueryPropos } return &v1beta1.QueryProposalsResponse{ - Proposals: legacyProposals, + Proposals: legacyProposals, Pagination: resp.Pagination, }, nil } @@ -335,7 +335,7 @@ func (q legacyQueryServer) Proposals(c context.Context, req *v1beta1.QueryPropos func (q legacyQueryServer) Vote(c context.Context, req *v1beta1.QueryVoteRequest) (*v1beta1.QueryVoteResponse, error) { resp, err := q.keeper.Vote(c, &v1beta2.QueryVoteRequest{ ProposalId: req.ProposalId, - Voter: req.Voter, + Voter: req.Voter, }) if err != nil { return nil, err @@ -361,7 +361,7 @@ func (q legacyQueryServer) Votes(c context.Context, req *v1beta1.QueryVotesReque } return &v1beta1.QueryVotesResponse{ - Votes: votes, + Votes: votes, Pagination: resp.Pagination, }, nil } @@ -389,21 +389,50 @@ func (q legacyQueryServer) Params(c context.Context, req *v1beta1.QueryParamsReq } return &v1beta1.QueryParamsResponse{ - VotingParams: v1beta1.NewVotingParams(*resp.VotingParams.VotingPeriod), + VotingParams: v1beta1.NewVotingParams(*resp.VotingParams.VotingPeriod), DepositParams: v1beta1.NewDepositParams(minDeposit, *resp.DepositParams.MaxDepositPeriod), - TallyParams: v1beta1.NewTallyParams(quorum, threshold, vetoThreshold), + TallyParams: v1beta1.NewTallyParams(quorum, threshold, vetoThreshold), }, nil } func (q legacyQueryServer) Deposit(c context.Context, req *v1beta1.QueryDepositRequest) (*v1beta1.QueryDepositResponse, error) { - return nil, nil + resp, err := q.keeper.Deposit(c, &v1beta2.QueryDepositRequest{ + ProposalId: req.ProposalId, + Depositor: req.Depositor, + }) + if err != nil { + return nil, err + } + + deposit := v046.ConvertToLegacyDeposit(resp.Deposit) + return &v1beta1.QueryDepositResponse{Deposit: deposit}, nil } func (q legacyQueryServer) Deposits(c context.Context, req *v1beta1.QueryDepositsRequest) (*v1beta1.QueryDepositsResponse, error) { - return nil, nil + resp, err := q.keeper.Deposits(c, &v1beta2.QueryDepositsRequest{ + ProposalId: req.ProposalId, + Pagination: req.Pagination, + }) + if err != nil { + return nil, err + } + deposits := make([]v1beta1.Deposit, len(resp.Deposits)) + for idx, deposit := range resp.Deposits { + deposits[idx] = v046.ConvertToLegacyDeposit(deposit) + } + + return &v1beta1.QueryDepositsResponse{Deposits: deposits, Pagination: resp.Pagination}, nil } func (q legacyQueryServer) TallyResult(c context.Context, req *v1beta1.QueryTallyResultRequest) (*v1beta1.QueryTallyResultResponse, error) { - return nil, nil -} + resp, err := q.keeper.TallyResult(c, &v1beta2.QueryTallyResultRequest{ + ProposalId: req.ProposalId, + }) + if err != nil { + return nil, err + } + + tally := v046.ConvertToLegacyTallyResult(resp.Tally) + return &v1beta1.QueryTallyResultResponse{Tally: tally}, nil +} diff --git a/x/gov/keeper/grpc_query_test.go b/x/gov/keeper/grpc_query_test.go index 0b6088c92f0..398401e1e35 100644 --- a/x/gov/keeper/grpc_query_test.go +++ b/x/gov/keeper/grpc_query_test.go @@ -7,6 +7,7 @@ import ( "github.com/cosmos/cosmos-sdk/simapp" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/query" + "github.com/cosmos/cosmos-sdk/x/gov/migrations/v046" "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta2" ) @@ -85,6 +86,81 @@ func (suite *KeeperTestSuite) TestGRPCQueryProposal() { } } +func (suite *KeeperTestSuite) TestLegacyGRPCQueryProposal() { + app, ctx, queryClient := suite.app, suite.ctx, suite.legacyQueryClient + + var ( + req *v1beta1.QueryProposalRequest + expProposal v1beta1.Proposal + ) + + testCases := []struct { + msg string + malleate func() + expPass bool + }{ + { + "empty request", + func() { + req = &v1beta1.QueryProposalRequest{} + }, + false, + }, + { + "non existing proposal request", + func() { + req = &v1beta1.QueryProposalRequest{ProposalId: 3} + }, + false, + }, + { + "zero proposal id request", + func() { + req = &v1beta1.QueryProposalRequest{ProposalId: 0} + }, + false, + }, + { + "valid request", + func() { + req = &v1beta1.QueryProposalRequest{ProposalId: 1} + testProposal := v1beta1.NewTextProposal("Proposal", "testing proposal") + msgContent, err := v1beta2.NewLegacyContent(testProposal, govAcct.String()) + suite.Require().NoError(err) + submittedProposal, err := app.GovKeeper.SubmitProposal(ctx, []sdk.Msg{msgContent}, nil) + suite.Require().NoError(err) + suite.Require().NotEmpty(submittedProposal) + + expProposal, err = v046.ConvertToLegacyProposal(submittedProposal) + suite.Require().NoError(err) + }, + true, + }, + } + + for _, testCase := range testCases { + suite.Run(fmt.Sprintf("Case %s", testCase.msg), func() { + testCase.malleate() + + proposalRes, err := queryClient.Proposal(gocontext.Background(), req) + + if testCase.expPass { + suite.Require().NoError(err) + // Instead of using MashalJSON, we could compare .String() output too. + // https://github.com/cosmos/cosmos-sdk/issues/10965 + expJSON, err := suite.app.AppCodec().MarshalJSON(&expProposal) + suite.Require().NoError(err) + actualJSON, err := suite.app.AppCodec().MarshalJSON(&proposalRes.Proposal) + suite.Require().NoError(err) + suite.Require().Equal(expJSON, actualJSON) + } else { + suite.Require().Error(err) + suite.Require().Nil(proposalRes) + } + }) + } +} + func (suite *KeeperTestSuite) TestGRPCQueryProposals() { app, ctx, queryClient, addrs := suite.app, suite.ctx, suite.queryClient, suite.addrs diff --git a/x/gov/keeper/keeper_test.go b/x/gov/keeper/keeper_test.go index 51b1f82d263..6784513c8ad 100644 --- a/x/gov/keeper/keeper_test.go +++ b/x/gov/keeper/keeper_test.go @@ -10,7 +10,9 @@ import ( "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/simapp" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/gov/keeper" "github.com/cosmos/cosmos-sdk/x/gov/types" + "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta2" minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" ) @@ -18,10 +20,11 @@ import ( type KeeperTestSuite struct { suite.Suite - app *simapp.SimApp - ctx sdk.Context - queryClient v1beta2.QueryClient - addrs []sdk.AccAddress + app *simapp.SimApp + ctx sdk.Context + queryClient v1beta2.QueryClient + legacyQueryClient v1beta1.QueryClient + addrs []sdk.AccAddress } func (suite *KeeperTestSuite) SetupTest() { @@ -38,11 +41,15 @@ func (suite *KeeperTestSuite) SetupTest() { queryHelper := baseapp.NewQueryServerTestHelper(ctx, app.InterfaceRegistry()) v1beta2.RegisterQueryServer(queryHelper, app.GovKeeper) + legacyQueryHelper := baseapp.NewQueryServerTestHelper(ctx, app.InterfaceRegistry()) + v1beta1.RegisterQueryServer(legacyQueryHelper, keeper.NewLegacyQueryServer(app.GovKeeper)) queryClient := v1beta2.NewQueryClient(queryHelper) + legacyQueryClient := v1beta1.NewQueryClient(legacyQueryHelper) suite.app = app suite.ctx = ctx suite.queryClient = queryClient + suite.legacyQueryClient = legacyQueryClient suite.addrs = simapp.AddTestAddrsIncremental(app, ctx, 2, sdk.NewInt(30000000)) } diff --git a/x/gov/migrations/v046/migrate.go b/x/gov/migrations/v046/migrate.go index 683590d3175..be624d68b49 100644 --- a/x/gov/migrations/v046/migrate.go +++ b/x/gov/migrations/v046/migrate.go @@ -11,24 +11,34 @@ import ( // don't have a legacy message will return a "nil" content. func ConvertToLegacyProposal(proposal v1beta2.Proposal) (v1beta1.Proposal, error) { legacyProposal := v1beta1.Proposal{ - ProposalId: proposal.ProposalId, - Content: nil, - Status: v1beta1.ProposalStatus(proposal.Status), + ProposalId: proposal.ProposalId, + Content: nil, + Status: v1beta1.ProposalStatus(proposal.Status), FinalTallyResult: ConvertToLegacyTallyResult(proposal.FinalTallyResult), - SubmitTime: *proposal.SubmitTime, - DepositEndTime: *proposal.DepositEndTime, - TotalDeposit: types.NewCoins(proposal.TotalDeposit...), - VotingStartTime: *proposal.VotingStartTime, - VotingEndTime: *proposal.VotingEndTime, + TotalDeposit: types.NewCoins(proposal.TotalDeposit...), + } + + if proposal.VotingStartTime != nil { + legacyProposal.VotingStartTime = *proposal.VotingStartTime + } + + if proposal.VotingEndTime != nil { + legacyProposal.VotingEndTime = *proposal.VotingEndTime + } + + if proposal.SubmitTime != nil { + legacyProposal.SubmitTime = *proposal.SubmitTime + } + + if proposal.DepositEndTime != nil { + legacyProposal.DepositEndTime = *proposal.DepositEndTime } msgs, err := proposal.GetMsgs() if err != nil { return v1beta1.Proposal{}, err } - msgTypes := make([]string, len(msgs)) - for idx, msg := range msgs { - msgTypes[idx] = msg.String() + for _, msg := range msgs { if legacyMsg, ok := msg.(*v1beta2.MsgExecLegacyContent); ok { // check that the content struct can be unmarshalled _, err := v1beta2.LegacyContentFromMessage(legacyMsg) @@ -41,12 +51,6 @@ func ConvertToLegacyProposal(proposal v1beta2.Proposal) (v1beta1.Proposal, error return legacyProposal, nil } -// ConvertToNewProposal takes a legacy proposal and converts it to a new proposal, -// wrapping the content into a "MsgExecLegacyContent". -func ConvertToNewProposal(proposal v1beta1.Proposal) v1beta2.Proposal { - return v1beta2.Proposal{} -} - func ConvertToLegacyTallyResult(tally *v1beta2.TallyResult) v1beta1.TallyResult { yes, _ := types.NewIntFromString(tally.Yes) no, _ := types.NewIntFromString(tally.No) @@ -54,18 +58,18 @@ func ConvertToLegacyTallyResult(tally *v1beta2.TallyResult) v1beta1.TallyResult abstain, _ := types.NewIntFromString(tally.Abstain) return v1beta1.TallyResult{ - Yes: yes, - No: no, + Yes: yes, + No: no, NoWithVeto: veto, - Abstain: abstain, + Abstain: abstain, } } func ConvertToLegacyVote(vote v1beta2.Vote) v1beta1.Vote { return v1beta1.Vote{ ProposalId: vote.ProposalId, - Voter: vote.Voter, - Options: ConvertToLegacyVoteOptions(vote.Options), + Voter: vote.Voter, + Options: ConvertToLegacyVoteOptions(vote.Options), } } @@ -83,3 +87,11 @@ func ConvertToLegacyVoteOptions(voteOptions []*v1beta2.WeightedVoteOption) []v1b } return options } + +func ConvertToLegacyDeposit(deposit *v1beta2.Deposit) v1beta1.Deposit { + return v1beta1.Deposit{ + ProposalId: deposit.ProposalId, + Depositor: deposit.Depositor, + Amount: types.NewCoins(deposit.Amount...), + } +} From c851841cb8a7f2b4d645fd9fce97dd107a94c2d2 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Fri, 28 Jan 2022 10:00:36 +0100 Subject: [PATCH 4/7] add a few more tests --- x/gov/keeper/grpc_query.go | 40 ++++--- x/gov/keeper/grpc_query_test.go | 191 ++++++++++++++++++++++++++++++++ 2 files changed, 216 insertions(+), 15 deletions(-) diff --git a/x/gov/keeper/grpc_query.go b/x/gov/keeper/grpc_query.go index 1956b6a49a1..0a46663ad4e 100644 --- a/x/gov/keeper/grpc_query.go +++ b/x/gov/keeper/grpc_query.go @@ -374,25 +374,35 @@ func (q legacyQueryServer) Params(c context.Context, req *v1beta1.QueryParamsReq return nil, err } - minDeposit := sdk.NewCoins(resp.DepositParams.MinDeposit...) - quorum, err := sdk.NewDecFromStr(resp.TallyParams.Quorum) - if err != nil { - return nil, err + response := &v1beta1.QueryParamsResponse{} + + if resp.DepositParams != nil { + minDeposit := sdk.NewCoins(resp.DepositParams.MinDeposit...) + response.DepositParams = v1beta1.NewDepositParams(minDeposit, *resp.DepositParams.MaxDepositPeriod) } - threshold, err := sdk.NewDecFromStr(resp.TallyParams.Threshold) - if err != nil { - return nil, err + + if resp.VotingParams != nil { + response.VotingParams = v1beta1.NewVotingParams(*resp.VotingParams.VotingPeriod) } - vetoThreshold, err := sdk.NewDecFromStr(resp.TallyParams.VetoThreshold) - if err != nil { - return nil, err + + if resp.TallyParams != nil { + quorum, err := sdk.NewDecFromStr(resp.TallyParams.Quorum) + if err != nil { + return nil, err + } + threshold, err := sdk.NewDecFromStr(resp.TallyParams.Threshold) + if err != nil { + return nil, err + } + vetoThreshold, err := sdk.NewDecFromStr(resp.TallyParams.VetoThreshold) + if err != nil { + return nil, err + } + + response.TallyParams = v1beta1.NewTallyParams(quorum, threshold, vetoThreshold) } - return &v1beta1.QueryParamsResponse{ - VotingParams: v1beta1.NewVotingParams(*resp.VotingParams.VotingPeriod), - DepositParams: v1beta1.NewDepositParams(minDeposit, *resp.DepositParams.MaxDepositPeriod), - TallyParams: v1beta1.NewTallyParams(quorum, threshold, vetoThreshold), - }, nil + return response, nil } func (q legacyQueryServer) Deposit(c context.Context, req *v1beta1.QueryDepositRequest) (*v1beta1.QueryDepositResponse, error) { diff --git a/x/gov/keeper/grpc_query_test.go b/x/gov/keeper/grpc_query_test.go index 398401e1e35..e212188800c 100644 --- a/x/gov/keeper/grpc_query_test.go +++ b/x/gov/keeper/grpc_query_test.go @@ -527,6 +527,107 @@ func (suite *KeeperTestSuite) TestGRPCQueryVotes() { } } +func (suite *KeeperTestSuite) TestLegacyGRPCQueryVotes() { + app, ctx, queryClient := suite.app, suite.ctx, suite.legacyQueryClient + + addrs := simapp.AddTestAddrsIncremental(app, ctx, 2, sdk.NewInt(30000000)) + + var ( + req *v1beta1.QueryVotesRequest + expRes *v1beta1.QueryVotesResponse + proposal v1beta2.Proposal + votes v1beta1.Votes + ) + + testCases := []struct { + msg string + malleate func() + expPass bool + }{ + { + "empty request", + func() { + req = &v1beta1.QueryVotesRequest{} + }, + false, + }, + { + "zero proposal id request", + func() { + req = &v1beta1.QueryVotesRequest{ + ProposalId: 0, + } + }, + false, + }, + { + "non existed proposals", + func() { + req = &v1beta1.QueryVotesRequest{ + ProposalId: 2, + } + }, + true, + }, + { + "create a proposal and get votes", + func() { + var err error + proposal, err = app.GovKeeper.SubmitProposal(ctx, TestProposal, nil) + suite.Require().NoError(err) + + req = &v1beta1.QueryVotesRequest{ + ProposalId: proposal.ProposalId, + } + }, + true, + }, + { + "request after adding 2 votes", + func() { + proposal.Status = v1beta2.StatusVotingPeriod + app.GovKeeper.SetProposal(ctx, proposal) + + votes = []v1beta1.Vote{ + {ProposalId: proposal.ProposalId, Voter: addrs[0].String(), Options: v1beta1.NewNonSplitVoteOption(v1beta1.OptionAbstain)}, + {ProposalId: proposal.ProposalId, Voter: addrs[1].String(), Options: v1beta1.NewNonSplitVoteOption(v1beta1.OptionYes)}, + } + accAddr1, err1 := sdk.AccAddressFromBech32(votes[0].Voter) + accAddr2, err2 := sdk.AccAddressFromBech32(votes[1].Voter) + suite.Require().NoError(err1) + suite.Require().NoError(err2) + suite.Require().NoError(app.GovKeeper.AddVote(ctx, proposal.ProposalId, accAddr1, v1beta2.NewNonSplitVoteOption(v1beta2.OptionAbstain))) + suite.Require().NoError(app.GovKeeper.AddVote(ctx, proposal.ProposalId, accAddr2, v1beta2.NewNonSplitVoteOption(v1beta2.OptionYes))) + + req = &v1beta1.QueryVotesRequest{ + ProposalId: proposal.ProposalId, + } + + expRes = &v1beta1.QueryVotesResponse{ + Votes: votes, + } + }, + true, + }, + } + + for _, testCase := range testCases { + suite.Run(fmt.Sprintf("Case %s", testCase.msg), func() { + testCase.malleate() + + votes, err := queryClient.Votes(gocontext.Background(), req) + + if testCase.expPass { + suite.Require().NoError(err) + suite.Require().Equal(expRes.GetVotes(), votes.GetVotes()) + } else { + suite.Require().Error(err) + suite.Require().Nil(votes) + } + }) + } +} + func (suite *KeeperTestSuite) TestGRPCQueryParams() { queryClient := suite.queryClient @@ -609,6 +710,96 @@ func (suite *KeeperTestSuite) TestGRPCQueryParams() { } } +func (suite *KeeperTestSuite) TestLegacyGRPCQueryParams() { + queryClient := suite.legacyQueryClient + + var ( + req *v1beta1.QueryParamsRequest + expRes *v1beta1.QueryParamsResponse + ) + + defaultTallyParams := v1beta1.TallyParams{ + Quorum: sdk.NewDec(0), + Threshold: sdk.NewDec(0), + VetoThreshold: sdk.NewDec(0), + } + + testCases := []struct { + msg string + malleate func() + expPass bool + }{ + { + "empty request", + func() { + req = &v1beta1.QueryParamsRequest{} + }, + false, + }, + { + "deposit params request", + func() { + req = &v1beta1.QueryParamsRequest{ParamsType: v1beta1.ParamDeposit} + depositParams := v1beta1.DefaultDepositParams() + expRes = &v1beta1.QueryParamsResponse{ + DepositParams: depositParams, + TallyParams: defaultTallyParams, + } + }, + true, + }, + { + "voting params request", + func() { + req = &v1beta1.QueryParamsRequest{ParamsType: v1beta1.ParamVoting} + votingParams := v1beta1.DefaultVotingParams() + expRes = &v1beta1.QueryParamsResponse{ + VotingParams: votingParams, + TallyParams: defaultTallyParams, + } + }, + true, + }, + { + "tally params request", + func() { + req = &v1beta1.QueryParamsRequest{ParamsType: v1beta1.ParamTallying} + tallyParams := v1beta1.DefaultTallyParams() + expRes = &v1beta1.QueryParamsResponse{ + TallyParams: tallyParams, + } + }, + true, + }, + { + "invalid request", + func() { + req = &v1beta1.QueryParamsRequest{ParamsType: "wrongPath"} + expRes = &v1beta1.QueryParamsResponse{} + }, + false, + }, + } + + for _, testCase := range testCases { + suite.Run(fmt.Sprintf("Case %s", testCase.msg), func() { + testCase.malleate() + + params, err := queryClient.Params(gocontext.Background(), req) + + if testCase.expPass { + suite.Require().NoError(err) + suite.Require().Equal(expRes.GetDepositParams(), params.GetDepositParams()) + suite.Require().Equal(expRes.GetVotingParams(), params.GetVotingParams()) + suite.Require().Equal(expRes.GetTallyParams(), params.GetTallyParams()) + } else { + suite.Require().Error(err) + suite.Require().Nil(params) + } + }) + } +} + func (suite *KeeperTestSuite) TestGRPCQueryDeposit() { app, ctx, queryClient, addrs := suite.app, suite.ctx, suite.queryClient, suite.addrs From d2799a91c949d72f862cf0ad79245a5254efe68a Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Fri, 28 Jan 2022 16:52:11 +0100 Subject: [PATCH 5/7] return more errors --- x/gov/keeper/grpc_query.go | 15 +++++++-- x/gov/migrations/v046/migrate.go | 53 ++++++++++++++++++++++---------- 2 files changed, 48 insertions(+), 20 deletions(-) diff --git a/x/gov/keeper/grpc_query.go b/x/gov/keeper/grpc_query.go index 0a46663ad4e..99d8fee1cf8 100644 --- a/x/gov/keeper/grpc_query.go +++ b/x/gov/keeper/grpc_query.go @@ -341,7 +341,10 @@ func (q legacyQueryServer) Vote(c context.Context, req *v1beta1.QueryVoteRequest return nil, err } - vote := v046.ConvertToLegacyVote(*resp.Vote) + vote, err := v046.ConvertToLegacyVote(*resp.Vote) + if err != nil { + return nil, err + } return &v1beta1.QueryVoteResponse{Vote: vote}, nil } @@ -357,7 +360,10 @@ func (q legacyQueryServer) Votes(c context.Context, req *v1beta1.QueryVotesReque votes := make([]v1beta1.Vote, len(resp.Votes)) for i, v := range resp.Votes { - votes[i] = v046.ConvertToLegacyVote(*v) + votes[i], err = v046.ConvertToLegacyVote(*v) + if err != nil { + return nil, err + } } return &v1beta1.QueryVotesResponse{ @@ -442,7 +448,10 @@ func (q legacyQueryServer) TallyResult(c context.Context, req *v1beta1.QueryTall return nil, err } - tally := v046.ConvertToLegacyTallyResult(resp.Tally) + tally, err := v046.ConvertToLegacyTallyResult(resp.Tally) + if err != nil { + return nil, err + } return &v1beta1.QueryTallyResultResponse{Tally: tally}, nil } diff --git a/x/gov/migrations/v046/migrate.go b/x/gov/migrations/v046/migrate.go index be624d68b49..edb16a2da56 100644 --- a/x/gov/migrations/v046/migrate.go +++ b/x/gov/migrations/v046/migrate.go @@ -1,6 +1,8 @@ package v046 import ( + "fmt" + "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta2" @@ -10,14 +12,15 @@ import ( // legacy proposal format. This conversion is best effort. New proposal types that // don't have a legacy message will return a "nil" content. func ConvertToLegacyProposal(proposal v1beta2.Proposal) (v1beta1.Proposal, error) { + var err error legacyProposal := v1beta1.Proposal{ - ProposalId: proposal.ProposalId, - Content: nil, - Status: v1beta1.ProposalStatus(proposal.Status), - FinalTallyResult: ConvertToLegacyTallyResult(proposal.FinalTallyResult), - TotalDeposit: types.NewCoins(proposal.TotalDeposit...), + ProposalId: proposal.ProposalId, + Status: v1beta1.ProposalStatus(proposal.Status), + TotalDeposit: types.NewCoins(proposal.TotalDeposit...), } + legacyProposal.FinalTallyResult, err = ConvertToLegacyTallyResult(proposal.FinalTallyResult) + if proposal.VotingStartTime != nil { legacyProposal.VotingStartTime = *proposal.VotingStartTime } @@ -51,41 +54,57 @@ func ConvertToLegacyProposal(proposal v1beta2.Proposal) (v1beta1.Proposal, error return legacyProposal, nil } -func ConvertToLegacyTallyResult(tally *v1beta2.TallyResult) v1beta1.TallyResult { - yes, _ := types.NewIntFromString(tally.Yes) - no, _ := types.NewIntFromString(tally.No) - veto, _ := types.NewIntFromString(tally.NoWithVeto) - abstain, _ := types.NewIntFromString(tally.Abstain) +func ConvertToLegacyTallyResult(tally *v1beta2.TallyResult) (v1beta1.TallyResult, error) { + yes, ok := types.NewIntFromString(tally.Yes) + if !ok { + return v1beta1.TallyResult{}, fmt.Errorf("unable to convert yes tally string (%s) to int", tally.Yes) + } + no, ok := types.NewIntFromString(tally.No) + if !ok { + return v1beta1.TallyResult{}, fmt.Errorf("unable to convert no tally string (%s) to int", tally.No) + } + veto, ok := types.NewIntFromString(tally.NoWithVeto) + if !ok { + return v1beta1.TallyResult{}, fmt.Errorf("unable to convert no with veto tally string (%s) to int", tally.NoWithVeto) + } + abstain, ok := types.NewIntFromString(tally.Abstain) + if !ok { + return v1beta1.TallyResult{}, fmt.Errorf("unable to convert abstain tally string (%s) to int", tally.Abstain) + } return v1beta1.TallyResult{ Yes: yes, No: no, NoWithVeto: veto, Abstain: abstain, - } + }, nil } -func ConvertToLegacyVote(vote v1beta2.Vote) v1beta1.Vote { +func ConvertToLegacyVote(vote v1beta2.Vote) (v1beta1.Vote, error) { + options, err := ConvertToLegacyVoteOptions(vote.Options) + if err != nil { + return v1beta1.Vote{}, err + } return v1beta1.Vote{ ProposalId: vote.ProposalId, Voter: vote.Voter, - Options: ConvertToLegacyVoteOptions(vote.Options), - } + Options: options, + }, nil } -func ConvertToLegacyVoteOptions(voteOptions []*v1beta2.WeightedVoteOption) []v1beta1.WeightedVoteOption { +func ConvertToLegacyVoteOptions(voteOptions []*v1beta2.WeightedVoteOption) ([]v1beta1.WeightedVoteOption, error) { options := make([]v1beta1.WeightedVoteOption, len(voteOptions)) for i, option := range voteOptions { weight, err := types.NewDecFromStr(option.Weight) if err != nil { - panic(err) + return options, err } options[i] = v1beta1.WeightedVoteOption{ Option: v1beta1.VoteOption(option.Option), Weight: weight, } } - return options + return options, nil } func ConvertToLegacyDeposit(deposit *v1beta2.Deposit) v1beta1.Deposit { From 9bb723a47399eae7569c166a77f9bbdf4aa2966a Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Mon, 31 Jan 2022 14:38:43 +0100 Subject: [PATCH 6/7] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c36c6b714b4..d266c0003e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,6 +122,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [\#9594](https://github.com/cosmos/cosmos-sdk/pull/9594) Remove legacy REST API. Please see the [REST Endpoints Migration guide](https://docs.cosmos.network/master/migrations/rest.html) to migrate to the new REST endpoints. * [\#9995](https://github.com/cosmos/cosmos-sdk/pull/9995) Increased gas cost for creating proposals. +* [\#11029](https://github.com/cosmos/cosmos-sdk/pull/11029) The deprecated Vote Option field is removed in gov v1beta2 and nil in v1beta1. Use Options instead. ### CLI Breaking Changes From 18b67aa8d40aed528dc0200bd110d631626cd194 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Mon, 31 Jan 2022 15:41:08 +0100 Subject: [PATCH 7/7] handle error --- x/gov/migrations/v046/migrate.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/x/gov/migrations/v046/migrate.go b/x/gov/migrations/v046/migrate.go index edb16a2da56..991e8737633 100644 --- a/x/gov/migrations/v046/migrate.go +++ b/x/gov/migrations/v046/migrate.go @@ -20,6 +20,9 @@ func ConvertToLegacyProposal(proposal v1beta2.Proposal) (v1beta1.Proposal, error } legacyProposal.FinalTallyResult, err = ConvertToLegacyTallyResult(proposal.FinalTallyResult) + if err != nil { + return v1beta1.Proposal{}, err + } if proposal.VotingStartTime != nil { legacyProposal.VotingStartTime = *proposal.VotingStartTime