Merge PR #5216: Fix Query Gov Proposals Pagination

This commit is contained in:
Alexander Bezobchuk 2019-10-18 12:46:21 -04:00 committed by GitHub
parent 0ea8c3270e
commit 84621bbd53
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 177 additions and 108 deletions

View File

@ -80,6 +80,7 @@ increased significantly due to modular `AnteHandler` support. Increase GasLimit
### Features
* (cli) [\#5212](https://github.com/cosmos/cosmos-sdk/issues/5212) The `q gov proposals` command now supports pagination.
* (store) [\#4724](https://github.com/cosmos/cosmos-sdk/issues/4724) Multistore supports substore migrations upon load. New `rootmulti.Store.LoadLatestVersionAndUpgrade` method in
`Baseapp` supports `StoreLoader` to enable various upgrade strategies. It no
longer panics if the store to load contains substores that we didn't explicitly mount.
@ -180,6 +181,7 @@ to detail this new feature and how state transitions occur.
* (keys) Fix ledger custom coin type support bug
* (x/gov) [\#5107](https://github.com/cosmos/cosmos-sdk/pull/5107) Sum validator operator's all voting power when tally votes
* (baseapp) [\#5200](https://github.com/cosmos/cosmos-sdk/issues/5200) Remove duplicate events from previous messages.
* (rest) [\#5212](https://github.com/cosmos/cosmos-sdk/issues/5212) Fix pagination in the `/gov/proposals` handler.
## [v0.37.2] - 2019-10-10

View File

@ -87,27 +87,29 @@ func GetCmdQueryProposals(queryRoute string, cdc *codec.Codec) *cobra.Command {
Use: "proposals",
Short: "Query proposals with optional filters",
Long: strings.TrimSpace(
fmt.Sprintf(`Query for a all proposals. You can filter the returns with the following flags.
fmt.Sprintf(`Query for a all paginated proposals that match optional filters:
Example:
$ %s query gov proposals --depositor cosmos1skjwj5whet0lpe65qaq4rpq03hjxlwd9nf39lk
$ %s query gov proposals --voter cosmos1skjwj5whet0lpe65qaq4rpq03hjxlwd9nf39lk
$ %s query gov proposals --status (DepositPeriod|VotingPeriod|Passed|Rejected)
$ %s query gov proposals --page=2 --limit=100
`,
version.ClientName, version.ClientName, version.ClientName,
version.ClientName, version.ClientName, version.ClientName, version.ClientName,
),
),
RunE: func(cmd *cobra.Command, args []string) error {
bechDepositorAddr := viper.GetString(flagDepositor)
bechVoterAddr := viper.GetString(flagVoter)
strProposalStatus := viper.GetString(flagStatus)
numLimit := uint64(viper.GetInt64(flagNumLimit))
page := viper.GetInt(flagPage)
limit := viper.GetInt(flagNumLimit)
var depositorAddr sdk.AccAddress
var voterAddr sdk.AccAddress
var proposalStatus types.ProposalStatus
params := types.NewQueryProposalsParams(proposalStatus, numLimit, voterAddr, depositorAddr)
params := types.NewQueryProposalsParams(page, limit, proposalStatus, voterAddr, depositorAddr)
if len(bechDepositorAddr) != 0 {
depositorAddr, err := sdk.AccAddressFromBech32(bechDepositorAddr)
@ -159,7 +161,8 @@ $ %s query gov proposals --status (DepositPeriod|VotingPeriod|Passed|Rejected)
},
}
cmd.Flags().String(flagNumLimit, "", "(optional) limit to latest [number] proposals. Defaults to all proposals")
cmd.Flags().Int(flagPage, 1, "pagination page of proposals to to query for")
cmd.Flags().Int(flagNumLimit, 100, "pagination limit of proposals to query for")
cmd.Flags().String(flagDepositor, "", "(optional) filter by proposals deposited on by depositor")
cmd.Flags().String(flagVoter, "", "(optional) filter by proposals voted on by voted")
cmd.Flags().String(flagStatus, "", "(optional) filter proposals by proposal status, status: deposit_period/voting_period/passed/rejected")

View File

@ -28,6 +28,7 @@ const (
flagDepositor = "depositor"
flagStatus = "status"
flagNumLimit = "limit"
flagPage = "page"
FlagProposal = "proposal"
)

View File

@ -389,48 +389,13 @@ func queryVotesOnProposalHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
}
}
// todo: Split this functionality into helper functions to remove the above
// HTTP request handler to query list of governance proposals
func queryProposalsWithParameterFn(cliCtx context.CLIContext) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
bechVoterAddr := r.URL.Query().Get(RestVoter)
bechDepositorAddr := r.URL.Query().Get(RestDepositor)
strProposalStatus := r.URL.Query().Get(RestProposalStatus)
strNumLimit := r.URL.Query().Get(RestNumLimit)
params := types.QueryProposalsParams{}
if len(bechVoterAddr) != 0 {
voterAddr, err := sdk.AccAddressFromBech32(bechVoterAddr)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
return
}
params.Voter = voterAddr
}
if len(bechDepositorAddr) != 0 {
depositorAddr, err := sdk.AccAddressFromBech32(bechDepositorAddr)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
return
}
params.Depositor = depositorAddr
}
if len(strProposalStatus) != 0 {
proposalStatus, err := types.ProposalStatusFromString(gcutils.NormalizeProposalStatus(strProposalStatus))
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
return
}
params.ProposalStatus = proposalStatus
}
if len(strNumLimit) != 0 {
numLimit, ok := rest.ParseUint64OrReturnBadRequest(w, strNumLimit)
if !ok {
return
}
params.Limit = numLimit
_, page, limit, err := rest.ParseHTTPArgsWithLimit(r, 0)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
return
}
cliCtx, ok := rest.ParseQueryHeightOrReturnBadRequest(w, cliCtx, r)
@ -438,13 +403,45 @@ func queryProposalsWithParameterFn(cliCtx context.CLIContext) http.HandlerFunc {
return
}
var (
voterAddr sdk.AccAddress
depositorAddr sdk.AccAddress
proposalStatus types.ProposalStatus
)
if v := r.URL.Query().Get(RestVoter); len(v) != 0 {
voterAddr, err = sdk.AccAddressFromBech32(v)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
return
}
}
if v := r.URL.Query().Get(RestDepositor); len(v) != 0 {
depositorAddr, err = sdk.AccAddressFromBech32(v)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
return
}
}
if v := r.URL.Query().Get(RestProposalStatus); len(v) != 0 {
proposalStatus, err = types.ProposalStatusFromString(gcutils.NormalizeProposalStatus(v))
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
return
}
}
params := types.NewQueryProposalsParams(page, limit, proposalStatus, voterAddr, depositorAddr)
bz, err := cliCtx.Codec.MarshalJSON(params)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
return
}
res, height, err := cliCtx.QueryWithData("custom/gov/proposals", bz)
route := fmt.Sprintf("custom/%s/%s", types.QuerierRoute, types.QueryProposals)
res, height, err := cliCtx.QueryWithData(route, bz)
if err != nil {
rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
return

View File

@ -56,8 +56,7 @@ func ExportGenesis(ctx sdk.Context, k Keeper) GenesisState {
depositParams := k.GetDepositParams(ctx)
votingParams := k.GetVotingParams(ctx)
tallyParams := k.GetTallyParams(ctx)
proposals := k.GetProposalsFiltered(ctx, nil, nil, StatusNil, 0)
proposals := k.GetProposals(ctx)
var proposalsDeposits Deposits
var proposalsVotes Votes

View File

@ -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"
)
@ -101,51 +102,50 @@ func (keeper Keeper) GetProposals(ctx sdk.Context) (proposals types.Proposals) {
return
}
// GetProposalsFiltered get Proposals from store by ProposalID
// voterAddr will filter proposals by whether or not that address has voted on them
// depositorAddr will filter proposals by whether or not that address has deposited to them
// status will filter proposals by status
// numLatest will fetch a specified number of the most recent proposals, or 0 for all proposals
func (keeper Keeper) GetProposalsFiltered(ctx sdk.Context, voterAddr sdk.AccAddress, depositorAddr sdk.AccAddress, status types.ProposalStatus, numLatest uint64) []types.Proposal {
// GetProposalsFiltered retrieves proposals filtered by a given set of params which
// include pagination parameters along with voter and depositor addresses and a
// proposal status. The voter address will filter proposals by whether or not
// that address has voted on proposals. The depositor address will filter proposals
// by whether or not that address has deposited to them. Finally, status will filter
// proposals by status.
//
// NOTE: If no filters are provided, all proposals will be returned in paginated
// form.
func (keeper Keeper) GetProposalsFiltered(ctx sdk.Context, params types.QueryProposalsParams) []types.Proposal {
proposals := keeper.GetProposals(ctx)
filteredProposals := make([]types.Proposal, 0, len(proposals))
maxProposalID, err := keeper.GetProposalID(ctx)
if err != nil {
return []types.Proposal{}
for _, p := range proposals {
matchVoter, matchDepositor, matchStatus := true, true, true
// match status (if supplied/valid)
if types.ValidProposalStatus(params.ProposalStatus) {
matchStatus = p.Status == params.ProposalStatus
}
// match voter address (if supplied)
if len(params.Voter) > 0 {
_, matchVoter = keeper.GetVote(ctx, p.ProposalID, params.Voter)
}
// match depositor (if supplied)
if len(params.Depositor) > 0 {
_, matchDepositor = keeper.GetDeposit(ctx, p.ProposalID, params.Depositor)
}
if matchVoter && matchDepositor && matchStatus {
filteredProposals = append(filteredProposals, p)
}
}
matchingProposals := []types.Proposal{}
if numLatest == 0 {
numLatest = maxProposalID
start, end := client.Paginate(len(filteredProposals), params.Page, params.Limit, 100)
if start < 0 || end < 0 {
filteredProposals = []types.Proposal{}
} else {
filteredProposals = filteredProposals[start:end]
}
for proposalID := maxProposalID - numLatest; proposalID < maxProposalID; proposalID++ {
if voterAddr != nil && len(voterAddr) != 0 {
_, found := keeper.GetVote(ctx, proposalID, voterAddr)
if !found {
continue
}
}
if depositorAddr != nil && len(depositorAddr) != 0 {
_, found := keeper.GetDeposit(ctx, proposalID, depositorAddr)
if !found {
continue
}
}
proposal, ok := keeper.GetProposal(ctx, proposalID)
if !ok {
continue
}
if types.ValidProposalStatus(status) && proposal.Status != status {
continue
}
matchingProposals = append(matchingProposals, proposal)
}
return matchingProposals
return filteredProposals
}
// GetProposalID gets the highest proposal ID

View File

@ -120,3 +120,57 @@ func TestSubmitProposal(t *testing.T) {
require.Equal(t, tc.expectedErr, err, "unexpected type of error: %s", err)
}
}
func TestGetProposalsFiltered(t *testing.T) {
proposalID := uint64(1)
ctx, _, keeper, _, _ := createTestInput(t, false, 100)
status := []types.ProposalStatus{types.StatusDepositPeriod, types.StatusVotingPeriod}
addr1 := sdk.AccAddress("foo")
for _, s := range status {
for i := 0; i < 50; i++ {
p := types.NewProposal(TestProposal, proposalID, time.Now(), time.Now())
p.Status = s
if i%2 == 0 {
d := types.NewDeposit(proposalID, addr1, nil)
v := types.NewVote(proposalID, addr1, types.OptionYes)
keeper.SetDeposit(ctx, d)
keeper.SetVote(ctx, v)
}
keeper.SetProposal(ctx, p)
proposalID++
}
}
testCases := []struct {
params types.QueryProposalsParams
expectedNumResults int
}{
{types.NewQueryProposalsParams(1, 50, types.StatusNil, nil, nil), 50},
{types.NewQueryProposalsParams(1, 50, types.StatusDepositPeriod, nil, nil), 50},
{types.NewQueryProposalsParams(1, 50, types.StatusVotingPeriod, nil, nil), 50},
{types.NewQueryProposalsParams(1, 25, types.StatusNil, nil, nil), 25},
{types.NewQueryProposalsParams(2, 25, types.StatusNil, nil, nil), 25},
{types.NewQueryProposalsParams(1, 50, types.StatusRejected, nil, nil), 0},
{types.NewQueryProposalsParams(1, 50, types.StatusNil, addr1, nil), 50},
{types.NewQueryProposalsParams(1, 50, types.StatusNil, nil, addr1), 50},
{types.NewQueryProposalsParams(1, 50, types.StatusNil, addr1, addr1), 50},
{types.NewQueryProposalsParams(1, 50, types.StatusDepositPeriod, addr1, addr1), 25},
{types.NewQueryProposalsParams(1, 50, types.StatusDepositPeriod, nil, nil), 50},
{types.NewQueryProposalsParams(1, 50, types.StatusVotingPeriod, nil, nil), 50},
}
for _, tc := range testCases {
proposals := keeper.GetProposalsFiltered(ctx, tc.params)
require.Len(t, proposals, tc.expectedNumResults)
for _, p := range proposals {
if len(tc.params.ProposalStatus.String()) != 0 {
require.Equal(t, tc.params.ProposalStatus, p.Status)
}
}
}
}

View File

@ -16,20 +16,28 @@ func NewQuerier(keeper Keeper) sdk.Querier {
switch path[0] {
case types.QueryParams:
return queryParams(ctx, path[1:], req, keeper)
case types.QueryProposals:
return queryProposals(ctx, path[1:], req, keeper)
case types.QueryProposal:
return queryProposal(ctx, path[1:], req, keeper)
case types.QueryDeposits:
return queryDeposits(ctx, path[1:], req, keeper)
case types.QueryDeposit:
return queryDeposit(ctx, path[1:], req, keeper)
case types.QueryVotes:
return queryVotes(ctx, path[1:], req, keeper)
case types.QueryVote:
return queryVote(ctx, path[1:], req, keeper)
case types.QueryTally:
return queryTally(ctx, path[1:], req, keeper)
default:
return nil, sdk.ErrUnknownRequest("unknown gov query endpoint")
}
@ -182,19 +190,18 @@ func queryVotes(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Ke
return bz, nil
}
// nolint: unparam
func queryProposals(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Keeper) ([]byte, sdk.Error) {
func queryProposals(ctx sdk.Context, _ []string, req abci.RequestQuery, keeper Keeper) ([]byte, sdk.Error) {
var params types.QueryProposalsParams
err := keeper.cdc.UnmarshalJSON(req.Data, &params)
if err != nil {
return nil, sdk.ErrUnknownRequest(sdk.AppendMsgToErr("incorrectly formatted request data", err.Error()))
return nil, sdk.ErrUnknownRequest(sdk.AppendMsgToErr("failed to parse params", err.Error()))
}
proposals := keeper.GetProposalsFiltered(ctx, params.Voter, params.Depositor, params.ProposalStatus, params.Limit)
bz, err := codec.MarshalJSONIndent(keeper.cdc, proposals)
bz, err := codec.MarshalJSONIndent(keeper.cdc, keeper.GetProposalsFiltered(ctx, params))
if err != nil {
return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", err.Error()))
return nil, sdk.ErrInternal(sdk.AppendMsgToErr("failed to JSON marshal result: %s", err.Error()))
}
return bz, nil
}

View File

@ -54,10 +54,14 @@ func getQueriedParams(t *testing.T, ctx sdk.Context, cdc *codec.Codec, querier s
return depositParams, votingParams, tallyParams
}
func getQueriedProposals(t *testing.T, ctx sdk.Context, cdc *codec.Codec, querier sdk.Querier, depositor, voter sdk.AccAddress, status types.ProposalStatus, limit uint64) []types.Proposal {
func getQueriedProposals(
t *testing.T, ctx sdk.Context, cdc *codec.Codec, querier sdk.Querier,
depositor, voter sdk.AccAddress, status types.ProposalStatus, page, limit int,
) []types.Proposal {
query := abci.RequestQuery{
Path: strings.Join([]string{custom, types.QuerierRoute, types.QueryProposals}, "/"),
Data: cdc.MustMarshalJSON(types.NewQueryProposalsParams(status, limit, voter, depositor)),
Data: cdc.MustMarshalJSON(types.NewQueryProposalsParams(page, limit, status, voter, depositor)),
}
bz, err := querier(ctx, []string{types.QueryProposals}, query)
@ -214,12 +218,12 @@ func TestQueries(t *testing.T) {
require.Equal(t, deposit5, deposit)
// Only proposal #1 should be in types.Deposit Period
proposals := getQueriedProposals(t, ctx, keeper.cdc, querier, nil, nil, types.StatusDepositPeriod, 0)
proposals := getQueriedProposals(t, ctx, keeper.cdc, querier, nil, nil, types.StatusDepositPeriod, 1, 0)
require.Len(t, proposals, 1)
require.Equal(t, proposal1, proposals[0])
// Only proposals #2 and #3 should be in Voting Period
proposals = getQueriedProposals(t, ctx, keeper.cdc, querier, nil, nil, types.StatusVotingPeriod, 0)
proposals = getQueriedProposals(t, ctx, keeper.cdc, querier, nil, nil, types.StatusVotingPeriod, 1, 0)
require.Len(t, proposals, 2)
require.Equal(t, proposal2, proposals[0])
require.Equal(t, proposal3, proposals[1])
@ -235,7 +239,7 @@ func TestQueries(t *testing.T) {
keeper.SetVote(ctx, vote3)
// Test query voted by TestAddrs[0]
proposals = getQueriedProposals(t, ctx, keeper.cdc, querier, nil, TestAddrs[0], types.StatusNil, 0)
proposals = getQueriedProposals(t, ctx, keeper.cdc, querier, nil, TestAddrs[0], types.StatusNil, 1, 0)
require.Equal(t, proposal2, proposals[0])
require.Equal(t, proposal3, proposals[1])
@ -254,25 +258,25 @@ func TestQueries(t *testing.T) {
require.Equal(t, vote3, votes[1])
// Test query all proposals
proposals = getQueriedProposals(t, ctx, keeper.cdc, querier, nil, nil, types.StatusNil, 0)
proposals = getQueriedProposals(t, ctx, keeper.cdc, querier, nil, nil, types.StatusNil, 1, 0)
require.Equal(t, proposal1, proposals[0])
require.Equal(t, proposal2, proposals[1])
require.Equal(t, proposal3, proposals[2])
// Test query voted by TestAddrs[1]
proposals = getQueriedProposals(t, ctx, keeper.cdc, querier, nil, TestAddrs[1], types.StatusNil, 0)
proposals = getQueriedProposals(t, ctx, keeper.cdc, querier, nil, TestAddrs[1], types.StatusNil, 1, 0)
require.Equal(t, proposal3.ProposalID, proposals[0].ProposalID)
// Test query deposited by TestAddrs[0]
proposals = getQueriedProposals(t, ctx, keeper.cdc, querier, TestAddrs[0], nil, types.StatusNil, 0)
proposals = getQueriedProposals(t, ctx, keeper.cdc, querier, TestAddrs[0], nil, types.StatusNil, 1, 0)
require.Equal(t, proposal1.ProposalID, proposals[0].ProposalID)
// Test query deposited by addr2
proposals = getQueriedProposals(t, ctx, keeper.cdc, querier, TestAddrs[1], nil, types.StatusNil, 0)
proposals = getQueriedProposals(t, ctx, keeper.cdc, querier, TestAddrs[1], nil, types.StatusNil, 1, 0)
require.Equal(t, proposal2.ProposalID, proposals[0].ProposalID)
require.Equal(t, proposal3.ProposalID, proposals[1].ProposalID)
// Test query voted AND deposited by addr1
proposals = getQueriedProposals(t, ctx, keeper.cdc, querier, TestAddrs[0], TestAddrs[0], types.StatusNil, 0)
proposals = getQueriedProposals(t, ctx, keeper.cdc, querier, TestAddrs[0], TestAddrs[0], types.StatusNil, 1, 0)
require.Equal(t, proposal2.ProposalID, proposals[0].ProposalID)
}

View File

@ -68,18 +68,20 @@ func NewQueryVoteParams(proposalID uint64, voter sdk.AccAddress) QueryVoteParams
// QueryProposalsParams Params for query 'custom/gov/proposals'
type QueryProposalsParams struct {
Page int
Limit int
Voter sdk.AccAddress
Depositor sdk.AccAddress
ProposalStatus ProposalStatus
Limit uint64
}
// NewQueryProposalsParams creates a new instance of QueryProposalsParams
func NewQueryProposalsParams(status ProposalStatus, limit uint64, voter, depositor sdk.AccAddress) QueryProposalsParams {
func NewQueryProposalsParams(page, limit int, status ProposalStatus, voter, depositor sdk.AccAddress) QueryProposalsParams {
return QueryProposalsParams{
Page: page,
Limit: limit,
Voter: voter,
Depositor: depositor,
ProposalStatus: status,
Limit: limit,
}
}