From cf4645213d4135bbaad0cbf2bac0517b401a4e20 Mon Sep 17 00:00:00 2001 From: Dmitry Shulyak Date: Wed, 18 Dec 2019 20:15:37 +0200 Subject: [PATCH] Merge PR #5405: Queries to /gov/proposals/{proposalID}/votes support pagination --- x/gov/client/cli/query.go | 15 +++- x/gov/client/rest/query.go | 11 ++- x/gov/client/utils/query.go | 76 ++++++++++-------- x/gov/client/utils/query_test.go | 129 +++++++++++++++++++++++++++++++ x/gov/keeper/querier.go | 10 ++- x/gov/keeper/querier_test.go | 80 ++++++++++++++++++- x/gov/types/querier.go | 17 +++- 7 files changed, 294 insertions(+), 44 deletions(-) create mode 100644 x/gov/client/utils/query_test.go diff --git a/x/gov/client/cli/query.go b/x/gov/client/cli/query.go index 47a0c223d0..69f89e7a7d 100644 --- a/x/gov/client/cli/query.go +++ b/x/gov/client/cli/query.go @@ -13,6 +13,7 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/version" + "github.com/cosmos/cosmos-sdk/x/auth/client/utils" gcutils "github.com/cosmos/cosmos-sdk/x/gov/client/utils" "github.com/cosmos/cosmos-sdk/x/gov/types" ) @@ -242,7 +243,7 @@ $ %s query gov vote 1 cosmos1skjwj5whet0lpe65qaq4rpq03hjxlwd9nf39lk // GetCmdQueryVotes implements the command to query for proposal votes. func GetCmdQueryVotes(queryRoute string, cdc *codec.Codec) *cobra.Command { - return &cobra.Command{ + cmd := &cobra.Command{ Use: "votes [proposal-id]", Args: cobra.ExactArgs(1), Short: "Query votes on a proposal", @@ -250,7 +251,8 @@ func GetCmdQueryVotes(queryRoute string, cdc *codec.Codec) *cobra.Command { fmt.Sprintf(`Query vote details for a single proposal by its identifier. Example: -$ %s query gov votes 1 +$ %[1]s query gov votes 1 +$ %[1]s query gov votes 1 --page=2 --limit=100 `, version.ClientName, ), @@ -263,8 +265,10 @@ $ %s query gov votes 1 if err != nil { return fmt.Errorf("proposal-id %s not a valid int, please input a valid proposal-id", args[0]) } + page := viper.GetInt(flagPage) + limit := viper.GetInt(flagNumLimit) - params := types.NewQueryProposalParams(proposalID) + params := types.NewQueryProposalVotesParams(proposalID, page, limit) bz, err := cdc.MarshalJSON(params) if err != nil { return err @@ -281,7 +285,7 @@ $ %s query gov votes 1 propStatus := proposal.Status if !(propStatus == types.StatusVotingPeriod || propStatus == types.StatusDepositPeriod) { - res, err = gcutils.QueryVotesByTxQuery(cliCtx, params) + res, err = gcutils.QueryVotesByTxQuery(cliCtx, params, utils.QueryTxsByEvents) } else { res, _, err = cliCtx.QueryWithData(fmt.Sprintf("custom/%s/votes", queryRoute), bz) } @@ -295,6 +299,9 @@ $ %s query gov votes 1 return cliCtx.PrintOutput(votes) }, } + cmd.Flags().Int(flagPage, 1, "pagination page of votes to to query for") + cmd.Flags().Int(flagNumLimit, 100, "pagination limit of votes to query for") + return cmd } // Command to Get a specific Deposit Information diff --git a/x/gov/client/rest/query.go b/x/gov/client/rest/query.go index a3ccb926cf..de9ccf379d 100644 --- a/x/gov/client/rest/query.go +++ b/x/gov/client/rest/query.go @@ -10,6 +10,7 @@ import ( "github.com/cosmos/cosmos-sdk/client/context" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/rest" + "github.com/cosmos/cosmos-sdk/x/auth/client/utils" gcutils "github.com/cosmos/cosmos-sdk/x/gov/client/utils" "github.com/cosmos/cosmos-sdk/x/gov/types" ) @@ -332,6 +333,12 @@ func queryVoteHandlerFn(cliCtx context.CLIContext) http.HandlerFunc { // todo: Split this functionality into helper functions to remove the above func queryVotesOnProposalHandlerFn(cliCtx context.CLIContext) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + _, page, limit, err := rest.ParseHTTPArgsWithLimit(r, 0) + if err != nil { + rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) + return + } + vars := mux.Vars(r) strProposalID := vars[RestProposalID] @@ -351,7 +358,7 @@ func queryVotesOnProposalHandlerFn(cliCtx context.CLIContext) http.HandlerFunc { return } - params := types.NewQueryProposalParams(proposalID) + params := types.NewQueryProposalVotesParams(proposalID, page, limit) bz, err := cliCtx.Codec.MarshalJSON(params) if err != nil { @@ -375,7 +382,7 @@ func queryVotesOnProposalHandlerFn(cliCtx context.CLIContext) http.HandlerFunc { // as they're no longer in state. propStatus := proposal.Status if !(propStatus == types.StatusVotingPeriod || propStatus == types.StatusDepositPeriod) { - res, err = gcutils.QueryVotesByTxQuery(cliCtx, params) + res, err = gcutils.QueryVotesByTxQuery(cliCtx, params, utils.QueryTxsByEvents) } else { res, _, err = cliCtx.QueryWithData("custom/gov/votes", bz) } diff --git a/x/gov/client/utils/query.go b/x/gov/client/utils/query.go index 0e29b6f4fb..2eaf2c5a7a 100644 --- a/x/gov/client/utils/query.go +++ b/x/gov/client/utils/query.go @@ -3,6 +3,7 @@ package utils import ( "fmt" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/context" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth/client/utils" @@ -72,45 +73,57 @@ func QueryDepositsByTxQuery(cliCtx context.CLIContext, params types.QueryProposa return cliCtx.Codec.MarshalJSON(deposits) } -// QueryVotesByTxQuery will query for votes via a direct txs tags query. It -// will fetch and build votes directly from the returned txs and return a JSON +// TxQuerier is a type that accepts query parameters (target events and pagination options) and returns sdk.SearchTxsResult. +// Mainly used for easier mocking of utils.QueryTxsByEvents in tests. +type TxQuerier func(cliCtx context.CLIContext, events []string, page, limit int) (*sdk.SearchTxsResult, error) + +// QueryVotesByTxQuery will query for votes using provided TxQuerier implementation. +// In general utils.QueryTxsByEvents should be used that will do a direct tx query to a tendermint node. +// It will fetch and build votes directly from the returned txs and return a JSON // marshalled result or any error that occurred. -// -// NOTE: SearchTxs is used to facilitate the txs query which does not currently -// support configurable pagination. -func QueryVotesByTxQuery(cliCtx context.CLIContext, params types.QueryProposalParams) ([]byte, error) { - events := []string{ - fmt.Sprintf("%s.%s='%s'", sdk.EventTypeMessage, sdk.AttributeKeyAction, types.TypeMsgVote), - fmt.Sprintf("%s.%s='%s'", types.EventTypeProposalVote, types.AttributeKeyProposalID, []byte(fmt.Sprintf("%d", params.ProposalID))), - } +func QueryVotesByTxQuery(cliCtx context.CLIContext, params types.QueryProposalVotesParams, querier TxQuerier) ([]byte, error) { + var ( + events = []string{ + fmt.Sprintf("%s.%s='%s'", sdk.EventTypeMessage, sdk.AttributeKeyAction, types.TypeMsgVote), + fmt.Sprintf("%s.%s='%s'", types.EventTypeProposalVote, types.AttributeKeyProposalID, []byte(fmt.Sprintf("%d", params.ProposalID))), + } + votes []types.Vote + nextTxPage = defaultPage + totalLimit = params.Limit * params.Page + ) + // query interrupted either if we collected enough votes or tx indexer run out of relevant txs + for len(votes) < totalLimit { + searchResult, err := querier(cliCtx, events, nextTxPage, defaultLimit) + if err != nil { + return nil, err + } + nextTxPage++ + for _, info := range searchResult.Txs { + for _, msg := range info.Tx.GetMsgs() { + if msg.Type() == types.TypeMsgVote { + voteMsg := msg.(types.MsgVote) - // NOTE: SearchTxs is used to facilitate the txs query which does not currently - // support configurable pagination. - searchResult, err := utils.QueryTxsByEvents(cliCtx, events, defaultPage, defaultLimit) - if err != nil { - return nil, err - } - - var votes []types.Vote - - for _, info := range searchResult.Txs { - for _, msg := range info.Tx.GetMsgs() { - if msg.Type() == types.TypeMsgVote { - voteMsg := msg.(types.MsgVote) - - votes = append(votes, types.Vote{ - Voter: voteMsg.Voter, - ProposalID: params.ProposalID, - Option: voteMsg.Option, - }) + votes = append(votes, types.Vote{ + Voter: voteMsg.Voter, + ProposalID: params.ProposalID, + Option: voteMsg.Option, + }) + } } } + if len(searchResult.Txs) != defaultLimit { + break + } + } + start, end := client.Paginate(len(votes), params.Page, params.Limit, 100) + if start < 0 || end < 0 { + votes = []types.Vote{} + } else { + votes = votes[start:end] } - if cliCtx.Indent { return cliCtx.Codec.MarshalJSONIndent(votes, "", " ") } - return cliCtx.Codec.MarshalJSON(votes) } @@ -128,7 +141,6 @@ func QueryVoteByTxQuery(cliCtx context.CLIContext, params types.QueryVoteParams) if err != nil { return nil, err } - for _, info := range searchResult.Txs { for _, msg := range info.Tx.GetMsgs() { // there should only be a single vote under the given conditions diff --git a/x/gov/client/utils/query_test.go b/x/gov/client/utils/query_test.go new file mode 100644 index 0000000000..ce9b548b93 --- /dev/null +++ b/x/gov/client/utils/query_test.go @@ -0,0 +1,129 @@ +package utils + +import ( + "testing" + + "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/client/context" + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/gov/types" + "github.com/stretchr/testify/require" +) + +type txMock struct { + address sdk.AccAddress + msgNum int +} + +func (tx txMock) ValidateBasic() sdk.Error { + return nil +} + +func (tx txMock) GetMsgs() (msgs []sdk.Msg) { + for i := 0; i < tx.msgNum; i++ { + msgs = append(msgs, types.NewMsgVote(tx.address, 0, types.OptionYes)) + } + return +} + +func makeQuerier(txs []sdk.Tx) TxQuerier { + return func(cliCtx context.CLIContext, events []string, page, limit int) (*sdk.SearchTxsResult, error) { + start, end := client.Paginate(len(txs), page, limit, 100) + if start < 0 || end < 0 { + return nil, nil + } + rst := &sdk.SearchTxsResult{ + TotalCount: len(txs), + PageNumber: page, + PageTotal: len(txs) / limit, + Limit: limit, + Count: end - start, + } + for _, tx := range txs[start:end] { + rst.Txs = append(rst.Txs, sdk.TxResponse{Tx: tx}) + } + return rst, nil + } +} + +func TestGetPaginatedVotes(t *testing.T) { + type testCase struct { + description string + page, limit int + txs []sdk.Tx + votes []types.Vote + } + acc1 := make(sdk.AccAddress, 20) + acc1[0] = 1 + acc2 := make(sdk.AccAddress, 20) + acc2[0] = 2 + for _, tc := range []testCase{ + { + description: "1MsgPerTxAll", + page: 1, + limit: 2, + txs: []sdk.Tx{txMock{acc1, 1}, txMock{acc2, 1}}, + votes: []types.Vote{ + types.NewVote(0, acc1, types.OptionYes), + types.NewVote(0, acc2, types.OptionYes)}, + }, + { + description: "2MsgPerTx1Chunk", + page: 1, + limit: 2, + txs: []sdk.Tx{txMock{acc1, 2}, txMock{acc2, 2}}, + votes: []types.Vote{ + types.NewVote(0, acc1, types.OptionYes), + types.NewVote(0, acc1, types.OptionYes)}, + }, + { + description: "2MsgPerTx2Chunk", + page: 2, + limit: 2, + txs: []sdk.Tx{txMock{acc1, 2}, txMock{acc2, 2}}, + votes: []types.Vote{ + types.NewVote(0, acc2, types.OptionYes), + types.NewVote(0, acc2, types.OptionYes)}, + }, + { + description: "IncompleteSearchTx", + page: 1, + limit: 2, + txs: []sdk.Tx{txMock{acc1, 1}}, + votes: []types.Vote{types.NewVote(0, acc1, types.OptionYes)}, + }, + { + description: "IncompleteSearchTx", + page: 1, + limit: 2, + txs: []sdk.Tx{txMock{acc1, 1}}, + votes: []types.Vote{types.NewVote(0, acc1, types.OptionYes)}, + }, + { + description: "InvalidPage", + page: -1, + txs: []sdk.Tx{txMock{acc1, 1}}, + }, + { + description: "OutOfBounds", + page: 2, + limit: 10, + txs: []sdk.Tx{txMock{acc1, 1}}, + }, + } { + tc := tc + t.Run(tc.description, func(t *testing.T) { + ctx := context.CLIContext{}.WithCodec(codec.New()) + params := types.NewQueryProposalVotesParams(0, tc.page, tc.limit) + votesData, err := QueryVotesByTxQuery(ctx, params, makeQuerier(tc.txs)) + require.NoError(t, err) + votes := []types.Vote{} + require.NoError(t, ctx.Codec.UnmarshalJSON(votesData, &votes)) + require.Equal(t, len(tc.votes), len(votes)) + for i := range votes { + require.Equal(t, tc.votes[i], votes[i]) + } + }) + } +} diff --git a/x/gov/keeper/querier.go b/x/gov/keeper/querier.go index b27f8fa54e..b5efc7619b 100644 --- a/x/gov/keeper/querier.go +++ b/x/gov/keeper/querier.go @@ -5,6 +5,7 @@ import ( abci "github.com/tendermint/tendermint/abci/types" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/gov/types" @@ -177,7 +178,7 @@ func queryTally(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Ke // nolint: unparam func queryVotes(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Keeper) ([]byte, sdk.Error) { - var params types.QueryProposalParams + var params types.QueryProposalVotesParams err := keeper.cdc.UnmarshalJSON(req.Data, ¶ms) if err != nil { @@ -187,6 +188,13 @@ func queryVotes(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Ke votes := keeper.GetVotes(ctx, params.ProposalID) if votes == nil { votes = types.Votes{} + } else { + start, end := client.Paginate(len(votes), params.Page, params.Limit, 100) + if start < 0 || end < 0 { + votes = types.Votes{} + } else { + votes = votes[start:end] + } } bz, err := codec.MarshalJSONIndent(keeper.cdc, votes) diff --git a/x/gov/keeper/querier_test.go b/x/gov/keeper/querier_test.go index 827741bea3..a5f781da18 100644 --- a/x/gov/keeper/querier_test.go +++ b/x/gov/keeper/querier_test.go @@ -1,8 +1,10 @@ package keeper import ( + "math/rand" "strings" "testing" + "time" "github.com/stretchr/testify/require" abci "github.com/tendermint/tendermint/abci/types" @@ -122,10 +124,11 @@ func getQueriedVote(t *testing.T, ctx sdk.Context, cdc *codec.Codec, querier sdk return vote } -func getQueriedVotes(t *testing.T, ctx sdk.Context, cdc *codec.Codec, querier sdk.Querier, proposalID uint64) []types.Vote { +func getQueriedVotes(t *testing.T, ctx sdk.Context, cdc *codec.Codec, querier sdk.Querier, + proposalID uint64, page, limit int) []types.Vote { query := abci.RequestQuery{ Path: strings.Join([]string{custom, types.QuerierRoute, types.QueryVote}, "/"), - Data: cdc.MustMarshalJSON(types.NewQueryProposalParams(proposalID)), + Data: cdc.MustMarshalJSON(types.NewQueryProposalVotesParams(proposalID, page, limit)), } bz, err := querier(ctx, []string{types.QueryVotes}, query) @@ -244,7 +247,7 @@ func TestQueries(t *testing.T) { require.Equal(t, proposal3, proposals[1]) // Test query votes on types.Proposal 2 - votes := getQueriedVotes(t, ctx, keeper.cdc, querier, proposal2.ProposalID) + votes := getQueriedVotes(t, ctx, keeper.cdc, querier, proposal2.ProposalID, 1, 0) require.Len(t, votes, 1) require.Equal(t, vote1, votes[0]) @@ -252,7 +255,7 @@ func TestQueries(t *testing.T) { require.Equal(t, vote1, vote) // Test query votes on types.Proposal 3 - votes = getQueriedVotes(t, ctx, keeper.cdc, querier, proposal3.ProposalID) + votes = getQueriedVotes(t, ctx, keeper.cdc, querier, proposal3.ProposalID, 1, 0) require.Len(t, votes, 2) require.Equal(t, vote2, votes[0]) require.Equal(t, vote3, votes[1]) @@ -280,3 +283,72 @@ func TestQueries(t *testing.T) { proposals = getQueriedProposals(t, ctx, keeper.cdc, querier, TestAddrs[0], TestAddrs[0], types.StatusNil, 1, 0) require.Equal(t, proposal2.ProposalID, proposals[0].ProposalID) } + +func TestPaginatedVotesQuery(t *testing.T) { + ctx, _, keeper, _, _ := createTestInput(t, false, 1000) + + proposal := types.Proposal{ + ProposalID: 100, + Status: types.StatusVotingPeriod, + } + keeper.SetProposal(ctx, proposal) + + votes := make([]types.Vote, 20) + rand := rand.New(rand.NewSource(time.Now().UnixNano())) + addr := make(sdk.AccAddress, 20) + for i := range votes { + rand.Read(addr) + vote := types.Vote{ + ProposalID: proposal.ProposalID, + Voter: addr, + Option: types.OptionYes, + } + votes[i] = vote + keeper.SetVote(ctx, vote) + } + + querier := NewQuerier(keeper) + + // keeper preserves consistent order for each query, but this is not the insertion order + all := getQueriedVotes(t, ctx, keeper.cdc, querier, proposal.ProposalID, 1, 0) + require.Equal(t, len(all), len(votes)) + + type testCase struct { + description string + page int + limit int + votes []types.Vote + } + for _, tc := range []testCase{ + { + description: "SkipAll", + page: 2, + limit: len(all), + }, + { + description: "GetFirstChunk", + page: 1, + limit: 10, + votes: all[:10], + }, + { + description: "GetSecondsChunk", + page: 2, + limit: 10, + votes: all[10:], + }, + { + description: "InvalidPage", + page: -1, + }, + } { + tc := tc + t.Run(tc.description, func(t *testing.T) { + votes := getQueriedVotes(t, ctx, keeper.cdc, querier, proposal.ProposalID, tc.page, tc.limit) + require.Equal(t, len(tc.votes), len(votes)) + for i := range votes { + require.Equal(t, tc.votes[i], votes[i]) + } + }) + } +} diff --git a/x/gov/types/querier.go b/x/gov/types/querier.go index fe97356b5c..fe9b5e0f15 100644 --- a/x/gov/types/querier.go +++ b/x/gov/types/querier.go @@ -26,7 +26,6 @@ const ( // - 'custom/gov/proposal' // - 'custom/gov/deposits' // - 'custom/gov/tally' -// - 'custom/gov/votes' type QueryProposalParams struct { ProposalID uint64 } @@ -38,6 +37,22 @@ func NewQueryProposalParams(proposalID uint64) QueryProposalParams { } } +// QueryProposalVotesParams used for queries to 'custom/gov/votes'. +type QueryProposalVotesParams struct { + ProposalID uint64 + Page int + Limit int +} + +// NewQueryProposalVotesParams creates new instance of the QueryProposalVotesParams. +func NewQueryProposalVotesParams(proposalID uint64, page, limit int) QueryProposalVotesParams { + return QueryProposalVotesParams{ + ProposalID: proposalID, + Page: page, + Limit: limit, + } +} + // QueryDepositParams params for query 'custom/gov/deposit' type QueryDepositParams struct { ProposalID uint64