From aa56aa9b2c2e918af48825618780ff1ae07daaed Mon Sep 17 00:00:00 2001 From: dmitry Date: Fri, 13 Dec 2019 12:09:30 +0200 Subject: [PATCH 1/4] x/gov: Queries to /gob/proposals/{proposalID}/votes support pagination Changes: - votes for active proposals are paginated by applying page limits to results stored in keeper - for inactive proposals we paginate what we receive from tendermint.TxSearch tendermint.TxSearch paginates transactions, not messages. I noticed that current gov application allows to submit only one msg per tx. But tx structure suggests that there could be many messages in a single transaction. Hence we load transactions in batches (30), unpacking them and once we collected enough votes - query is terminted. Or it will be terminted if we run out off transactions. Other changes: - add separate type for proposal votes queries - extended cli to query gov vote with '--page' and '--limit' flags Signed-off-by: dmitry --- x/gov/client/cli/query.go | 12 ++- x/gov/client/rest/query.go | 8 +- x/gov/client/utils/query.go | 76 +++++++++++------- x/gov/client/utils/query_test.go | 129 +++++++++++++++++++++++++++++++ x/gov/keeper/querier.go | 4 +- x/gov/keeper/querier_test.go | 80 ++++++++++++++++++- x/gov/keeper/vote.go | 10 +++ x/gov/types/querier.go | 17 +++- 8 files changed, 295 insertions(+), 41 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 47a0c223d0ca..8b6a498604ce 100644 --- a/x/gov/client/cli/query.go +++ b/x/gov/client/cli/query.go @@ -242,7 +242,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 +250,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 +264,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 @@ -295,6 +298,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 a3ccb926cf29..3eb5800d7a99 100644 --- a/x/gov/client/rest/query.go +++ b/x/gov/client/rest/query.go @@ -332,6 +332,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 +357,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 { diff --git a/x/gov/client/utils/query.go b/x/gov/client/utils/query.go index 0e29b6f4fb41..403cd78de077 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,41 +73,57 @@ func QueryDepositsByTxQuery(cliCtx context.CLIContext, params types.QueryProposa return cliCtx.Codec.MarshalJSON(deposits) } +type txQuerier func(cliCtx context.CLIContext, events []string, page, limit int) (*sdk.SearchTxsResult, error) + +func getPaginatedVotes(cliCtx context.CLIContext, params types.QueryProposalVotesParams, querier txQuerier) ([]types.Vote, 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) + + 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 { + return nil, fmt.Errorf("invalid page (%d) or range is out of bounds (limit %d)", params.Page, params.Limit) + } + return votes[start:end], nil +} + // 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 // 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))), - } - - // NOTE: SearchTxs is used to facilitate the txs query which does not currently - // support configurable pagination. - searchResult, err := utils.QueryTxsByEvents(cliCtx, events, defaultPage, defaultLimit) +func QueryVotesByTxQuery(cliCtx context.CLIContext, params types.QueryProposalVotesParams) ([]byte, error) { + votes, err := getPaginatedVotes(cliCtx, params, utils.QueryTxsByEvents) 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, - }) - } - } - } - if cliCtx.Indent { return cliCtx.Codec.MarshalJSONIndent(votes, "", " ") } @@ -128,7 +145,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 000000000000..dd777c255c4f --- /dev/null +++ b/x/gov/client/utils/query_test.go @@ -0,0 +1,129 @@ +package utils + +import ( + "errors" + "fmt" + "testing" + + "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/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.MsgVote{ + Voter: tx.address, + }) + } + 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, errors.New("unexpected error in test") + } + 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 + err error + } + acc1 := sdk.AccAddress{1} + acc2 := sdk.AccAddress{2} + for _, tc := range []testCase{ + { + description: "1MsgPerTxAll", + page: 1, + limit: 2, + txs: []sdk.Tx{txMock{acc1, 1}, txMock{acc2, 1}}, + votes: []types.Vote{{Voter: acc1}, {Voter: acc2}}, + }, + { + description: "2MsgPerTx1Chunk", + page: 1, + limit: 2, + txs: []sdk.Tx{txMock{acc1, 2}, txMock{acc2, 2}}, + votes: []types.Vote{{Voter: acc1}, {Voter: acc1}}, + }, + { + description: "2MsgPerTx2Chunk", + page: 2, + limit: 2, + txs: []sdk.Tx{txMock{acc1, 2}, txMock{acc2, 2}}, + votes: []types.Vote{{Voter: acc2}, {Voter: acc2}}, + }, + { + description: "IncompleteSearchTx", + page: 1, + limit: 2, + txs: []sdk.Tx{txMock{acc1, 1}}, + votes: []types.Vote{{Voter: acc1}}, + }, + { + description: "IncompleteSearchTx", + page: 1, + limit: 2, + txs: []sdk.Tx{txMock{acc1, 1}}, + votes: []types.Vote{{Voter: acc1}}, + }, + { + description: "InvalidPage", + page: -1, + txs: []sdk.Tx{txMock{acc1, 1}}, + err: fmt.Errorf("invalid page (%d) or range is out of bounds (limit %d)", -1, 0), + }, + { + description: "OutOfBounds", + page: 2, + limit: 10, + txs: []sdk.Tx{txMock{acc1, 1}}, + err: fmt.Errorf("invalid page (%d) or range is out of bounds (limit %d)", 2, 10), + }, + } { + tc := tc + t.Run(tc.description, func(t *testing.T) { + params := types.NewQueryProposalVotesParams(0, tc.page, tc.limit) + votes, err := getPaginatedVotes(context.CLIContext{}, params, makeQuerier(tc.txs)) + if tc.err != nil { + require.NotNil(t, err) + require.EqualError(t, tc.err, err.Error()) + } else { + require.NoError(t, err) + } + 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 b27f8fa54e5b..53f587935a7f 100644 --- a/x/gov/keeper/querier.go +++ b/x/gov/keeper/querier.go @@ -177,14 +177,14 @@ 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 { return nil, sdk.ErrUnknownRequest(sdk.AppendMsgToErr("incorrectly formatted request data", err.Error())) } - votes := keeper.GetVotes(ctx, params.ProposalID) + votes := keeper.GetVotesPaginated(ctx, params.ProposalID, params.Page, params.Limit) if votes == nil { votes = types.Votes{} } diff --git a/x/gov/keeper/querier_test.go b/x/gov/keeper/querier_test.go index 827741bea38e..a5f781da1807 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/keeper/vote.go b/x/gov/keeper/vote.go index 0204e56be273..20f48b60a295 100644 --- a/x/gov/keeper/vote.go +++ b/x/gov/keeper/vote.go @@ -3,6 +3,7 @@ package keeper import ( "fmt" + "github.com/cosmos/cosmos-sdk/client" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/gov/types" ) @@ -53,6 +54,15 @@ func (keeper Keeper) GetVotes(ctx sdk.Context, proposalID uint64) (votes types.V return } +func (keeper Keeper) GetVotesPaginated(ctx sdk.Context, proposalID uint64, page, limit int) (votes types.Votes) { + votes = keeper.GetVotes(ctx, proposalID) + start, end := client.Paginate(len(votes), page, limit, 100) + if start < 0 || end < 0 { + return types.Votes{} + } + return votes[start:end] +} + // GetVote gets the vote from an address on a specific proposal func (keeper Keeper) GetVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.AccAddress) (vote types.Vote, found bool) { store := ctx.KVStore(keeper.storeKey) diff --git a/x/gov/types/querier.go b/x/gov/types/querier.go index fe97356b5cc2..fe9b5e0f15cb 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 From 0af476a09578838e5d5f07ff2002152477c1581a Mon Sep 17 00:00:00 2001 From: dmitry Date: Wed, 18 Dec 2019 07:22:19 +0200 Subject: [PATCH 2/4] x/gov: Pass TxQuerier to QueryVotesByTxQuery directly --- x/gov/client/cli/query.go | 3 ++- x/gov/client/rest/query.go | 3 ++- x/gov/client/utils/query.go | 23 ++++++++----------- x/gov/client/utils/query_test.go | 38 ++++++++++++++++++++------------ 4 files changed, 37 insertions(+), 30 deletions(-) diff --git a/x/gov/client/cli/query.go b/x/gov/client/cli/query.go index 8b6a498604ce..69f89e7a7d05 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" ) @@ -284,7 +285,7 @@ $ %[1]s query gov votes 1 --page=2 --limit=100 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) } diff --git a/x/gov/client/rest/query.go b/x/gov/client/rest/query.go index 3eb5800d7a99..de9ccf379dd5 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" ) @@ -381,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 403cd78de077..a4223fce4aca 100644 --- a/x/gov/client/utils/query.go +++ b/x/gov/client/utils/query.go @@ -73,9 +73,15 @@ func QueryDepositsByTxQuery(cliCtx context.CLIContext, params types.QueryProposa return cliCtx.Codec.MarshalJSON(deposits) } -type txQuerier func(cliCtx context.CLIContext, events []string, page, limit int) (*sdk.SearchTxsResult, error) +// 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) -func getPaginatedVotes(cliCtx context.CLIContext, params types.QueryProposalVotesParams, querier txQuerier) ([]types.Vote, 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. +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), @@ -113,21 +119,10 @@ func getPaginatedVotes(cliCtx context.CLIContext, params types.QueryProposalVote if start < 0 || end < 0 { return nil, fmt.Errorf("invalid page (%d) or range is out of bounds (limit %d)", params.Page, params.Limit) } - return votes[start:end], nil -} - -// 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 -// marshalled result or any error that occurred. -func QueryVotesByTxQuery(cliCtx context.CLIContext, params types.QueryProposalVotesParams) ([]byte, error) { - votes, err := getPaginatedVotes(cliCtx, params, utils.QueryTxsByEvents) - if err != nil { - return nil, err - } + votes = votes[start:end] if cliCtx.Indent { return cliCtx.Codec.MarshalJSONIndent(votes, "", " ") } - return cliCtx.Codec.MarshalJSON(votes) } diff --git a/x/gov/client/utils/query_test.go b/x/gov/client/utils/query_test.go index dd777c255c4f..834f6f0eff49 100644 --- a/x/gov/client/utils/query_test.go +++ b/x/gov/client/utils/query_test.go @@ -7,6 +7,7 @@ import ( "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" @@ -23,14 +24,12 @@ func (tx txMock) ValidateBasic() sdk.Error { func (tx txMock) GetMsgs() (msgs []sdk.Msg) { for i := 0; i < tx.msgNum; i++ { - msgs = append(msgs, types.MsgVote{ - Voter: tx.address, - }) + msgs = append(msgs, types.NewMsgVote(tx.address, 0, types.OptionYes)) } return } -func makeQuerier(txs []sdk.Tx) txQuerier { +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 { @@ -58,43 +57,51 @@ func TestGetPaginatedVotes(t *testing.T) { votes []types.Vote err error } - acc1 := sdk.AccAddress{1} - acc2 := sdk.AccAddress{2} + 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{{Voter: acc1}, {Voter: acc2}}, + 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{{Voter: acc1}, {Voter: acc1}}, + 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{{Voter: acc2}, {Voter: acc2}}, + 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{{Voter: acc1}}, + votes: []types.Vote{types.NewVote(0, acc1, types.OptionYes)}, }, { description: "IncompleteSearchTx", page: 1, limit: 2, txs: []sdk.Tx{txMock{acc1, 1}}, - votes: []types.Vote{{Voter: acc1}}, + votes: []types.Vote{types.NewVote(0, acc1, types.OptionYes)}, }, { description: "InvalidPage", @@ -112,14 +119,17 @@ func TestGetPaginatedVotes(t *testing.T) { } { tc := tc t.Run(tc.description, func(t *testing.T) { + ctx := context.CLIContext{}.WithCodec(codec.New()) params := types.NewQueryProposalVotesParams(0, tc.page, tc.limit) - votes, err := getPaginatedVotes(context.CLIContext{}, params, makeQuerier(tc.txs)) + votesData, err := QueryVotesByTxQuery(ctx, params, makeQuerier(tc.txs)) if tc.err != nil { require.NotNil(t, err) require.EqualError(t, tc.err, err.Error()) - } else { - require.NoError(t, err) + return } + 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]) From 8a6ded8afcbbb86417e2f6ec551147adc3165049 Mon Sep 17 00:00:00 2001 From: dmitry Date: Wed, 18 Dec 2019 07:28:50 +0200 Subject: [PATCH 3/4] x/gov: Remove votes pagination from keeper --- x/gov/keeper/querier.go | 10 +++++++++- x/gov/keeper/vote.go | 10 ---------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/x/gov/keeper/querier.go b/x/gov/keeper/querier.go index 53f587935a7f..b5efc7619b05 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" @@ -184,9 +185,16 @@ func queryVotes(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Ke return nil, sdk.ErrUnknownRequest(sdk.AppendMsgToErr("incorrectly formatted request data", err.Error())) } - votes := keeper.GetVotesPaginated(ctx, params.ProposalID, params.Page, params.Limit) + 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/vote.go b/x/gov/keeper/vote.go index 20f48b60a295..0204e56be273 100644 --- a/x/gov/keeper/vote.go +++ b/x/gov/keeper/vote.go @@ -3,7 +3,6 @@ package keeper import ( "fmt" - "github.com/cosmos/cosmos-sdk/client" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/gov/types" ) @@ -54,15 +53,6 @@ func (keeper Keeper) GetVotes(ctx sdk.Context, proposalID uint64) (votes types.V return } -func (keeper Keeper) GetVotesPaginated(ctx sdk.Context, proposalID uint64, page, limit int) (votes types.Votes) { - votes = keeper.GetVotes(ctx, proposalID) - start, end := client.Paginate(len(votes), page, limit, 100) - if start < 0 || end < 0 { - return types.Votes{} - } - return votes[start:end] -} - // GetVote gets the vote from an address on a specific proposal func (keeper Keeper) GetVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.AccAddress) (vote types.Vote, found bool) { store := ctx.KVStore(keeper.storeKey) From 1caad87a317a7b0b9dd406c2c90d97c088c789ac Mon Sep 17 00:00:00 2001 From: dmitry Date: Wed, 18 Dec 2019 16:54:11 +0200 Subject: [PATCH 4/4] x/gov: Dont return an error if pagination parameters are wrong Signed-off-by: dmitry --- x/gov/client/utils/query.go | 5 +++-- x/gov/client/utils/query_test.go | 12 +----------- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/x/gov/client/utils/query.go b/x/gov/client/utils/query.go index a4223fce4aca..2eaf2c5a7a78 100644 --- a/x/gov/client/utils/query.go +++ b/x/gov/client/utils/query.go @@ -117,9 +117,10 @@ func QueryVotesByTxQuery(cliCtx context.CLIContext, params types.QueryProposalVo } start, end := client.Paginate(len(votes), params.Page, params.Limit, 100) if start < 0 || end < 0 { - return nil, fmt.Errorf("invalid page (%d) or range is out of bounds (limit %d)", params.Page, params.Limit) + votes = []types.Vote{} + } else { + votes = votes[start:end] } - votes = votes[start:end] if cliCtx.Indent { return cliCtx.Codec.MarshalJSONIndent(votes, "", " ") } diff --git a/x/gov/client/utils/query_test.go b/x/gov/client/utils/query_test.go index 834f6f0eff49..ce9b548b93bb 100644 --- a/x/gov/client/utils/query_test.go +++ b/x/gov/client/utils/query_test.go @@ -1,8 +1,6 @@ package utils import ( - "errors" - "fmt" "testing" "github.com/cosmos/cosmos-sdk/client" @@ -33,7 +31,7 @@ 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, errors.New("unexpected error in test") + return nil, nil } rst := &sdk.SearchTxsResult{ TotalCount: len(txs), @@ -55,7 +53,6 @@ func TestGetPaginatedVotes(t *testing.T) { page, limit int txs []sdk.Tx votes []types.Vote - err error } acc1 := make(sdk.AccAddress, 20) acc1[0] = 1 @@ -107,14 +104,12 @@ func TestGetPaginatedVotes(t *testing.T) { description: "InvalidPage", page: -1, txs: []sdk.Tx{txMock{acc1, 1}}, - err: fmt.Errorf("invalid page (%d) or range is out of bounds (limit %d)", -1, 0), }, { description: "OutOfBounds", page: 2, limit: 10, txs: []sdk.Tx{txMock{acc1, 1}}, - err: fmt.Errorf("invalid page (%d) or range is out of bounds (limit %d)", 2, 10), }, } { tc := tc @@ -122,11 +117,6 @@ func TestGetPaginatedVotes(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)) - if tc.err != nil { - require.NotNil(t, err) - require.EqualError(t, tc.err, err.Error()) - return - } require.NoError(t, err) votes := []types.Vote{} require.NoError(t, ctx.Codec.UnmarshalJSON(votesData, &votes))