Skip to content

Commit

Permalink
refactor(gov)!: decouple gRPC query methods from Keeper (#16106)
Browse files Browse the repository at this point in the history 
Co-authored-by: unknown unknown <unknown@unknown>
  • Loading branch information
@testinginprod @kocubinski
2 people authored and kocubinski committed May 11, 2023
1 parent c17203e commit f866c05
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 45 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* The signature of `NewTxConfigWithTextual` has been deprecated and its signature changed to accept a `SignModeOptions`.
* (x/genutil) [#15999](https://github.com/cosmos/cosmos-sdk/pull/15999) Genutil now takes the `GenesisTxHanlder` interface instead of deliverTx. The interface is implemented on baseapp
* (types/math) [#16040](https://github.com/cosmos/cosmos-sdk/pull/16040) Remove unused aliases in math.go

* (x/gov) [#16106](https://github.com/cosmos/cosmos-sdk/pull/16106) Remove gRPC query methods from Keeper
### Client Breaking Changes

* (x/staking) [#15701](https://github.com/cosmos/cosmos-sdk/pull/15701) `HistoricalInfoKey` now has a binary format.
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/gov/keeper/keeper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func initFixture(t *testing.T) *fixture {
assert.NilError(t, err)

queryHelper := baseapp.NewQueryServerTestHelper(ctx, app.InterfaceRegistry())
v1.RegisterQueryServer(queryHelper, app.GovKeeper)
v1.RegisterQueryServer(queryHelper, keeper.NewQueryServer(*app.GovKeeper))
legacyQueryHelper := baseapp.NewQueryServerTestHelper(ctx, app.InterfaceRegistry())
v1beta1.RegisterQueryServer(legacyQueryHelper, keeper.NewLegacyQueryServer(app.GovKeeper))
queryClient := v1.NewQueryClient(queryHelper)
Expand Down
86 changes: 45 additions & 41 deletions x/gov/keeper/grpc_query.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,24 @@ import (
"github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
)

var _ v1.QueryServer = Keeper{}
var _ v1.QueryServer = queryServer{}

func (q Keeper) Constitution(ctx context.Context, req *v1.QueryConstitutionRequest) (*v1.QueryConstitutionResponse, error) {
constitution, err := q.GetConstitution(ctx)
type queryServer struct{ k Keeper }

func NewQueryServer(k Keeper) v1.QueryServer {
return queryServer{k: k}
}

func (q queryServer) Constitution(ctx context.Context, _ *v1.QueryConstitutionRequest) (*v1.QueryConstitutionResponse, error) {
constitution, err := q.k.GetConstitution(ctx)
if err != nil {
return nil, err
}
return &v1.QueryConstitutionResponse{Constitution: constitution}, nil
}

// Proposal returns proposal details based on ProposalID
func (q Keeper) Proposal(ctx context.Context, req *v1.QueryProposalRequest) (*v1.QueryProposalResponse, error) {
func (q queryServer) Proposal(ctx context.Context, req *v1.QueryProposalRequest) (*v1.QueryProposalResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "invalid request")
}
Expand All @@ -39,7 +45,7 @@ func (q Keeper) Proposal(ctx context.Context, req *v1.QueryProposalRequest) (*v1
return nil, status.Error(codes.InvalidArgument, "proposal id can not be 0")
}

proposal, err := q.GetProposal(ctx, req.ProposalId)
proposal, err := q.k.GetProposal(ctx, req.ProposalId)
if err != nil {
if errors.IsOf(err, types.ErrProposalNotFound) {
return nil, status.Errorf(codes.NotFound, "proposal %d doesn't exist", req.ProposalId)
Expand All @@ -51,12 +57,12 @@ func (q Keeper) Proposal(ctx context.Context, req *v1.QueryProposalRequest) (*v1
}

// Proposals implements the Query/Proposals gRPC method
func (q Keeper) Proposals(ctx context.Context, req *v1.QueryProposalsRequest) (*v1.QueryProposalsResponse, error) {
store := q.storeService.OpenKVStore(ctx)
func (q queryServer) Proposals(ctx context.Context, req *v1.QueryProposalsRequest) (*v1.QueryProposalsResponse, error) {
store := q.k.storeService.OpenKVStore(ctx)
proposalStore := prefix.NewStore(runtime.KVStoreAdapter(store), types.ProposalsKeyPrefix)

filteredProposals, pageRes, err := query.GenericFilteredPaginate(
q.cdc,
q.k.cdc,
proposalStore,
req.Pagination,
func(key []byte, p *v1.Proposal) (*v1.Proposal, error) {
Expand All @@ -69,23 +75,23 @@ func (q Keeper) Proposals(ctx context.Context, req *v1.QueryProposalsRequest) (*

// match voter address (if supplied)
if len(req.Voter) > 0 {
voter, err := q.authKeeper.StringToBytes(req.Voter)
voter, err := q.k.authKeeper.StringToBytes(req.Voter)
if err != nil {
return nil, err
}

_, err = q.GetVote(ctx, p.Id, voter)
_, err = q.k.GetVote(ctx, p.Id, voter)
// if no error, vote found, matchVoter = true
matchVoter = err == nil
}

// match depositor (if supplied)
if len(req.Depositor) > 0 {
depositor, err := q.authKeeper.StringToBytes(req.Depositor)
depositor, err := q.k.authKeeper.StringToBytes(req.Depositor)
if err != nil {
return nil, err
}
_, err = q.GetDeposit(ctx, p.Id, depositor)
_, err = q.k.GetDeposit(ctx, p.Id, depositor)
// if no error, deposit found, matchDepositor = true
matchDepositor = err == nil
}
Expand All @@ -106,7 +112,7 @@ func (q Keeper) Proposals(ctx context.Context, req *v1.QueryProposalsRequest) (*
}

// Vote returns Voted information based on proposalID, voterAddr
func (q Keeper) Vote(ctx context.Context, req *v1.QueryVoteRequest) (*v1.QueryVoteResponse, error) {
func (q queryServer) Vote(ctx context.Context, req *v1.QueryVoteRequest) (*v1.QueryVoteResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "invalid request")
}
Expand All @@ -119,11 +125,11 @@ func (q Keeper) Vote(ctx context.Context, req *v1.QueryVoteRequest) (*v1.QueryVo
return nil, status.Error(codes.InvalidArgument, "empty voter address")
}

voter, err := q.authKeeper.StringToBytes(req.Voter)
voter, err := q.k.authKeeper.StringToBytes(req.Voter)
if err != nil {
return nil, err
}
vote, err := q.GetVote(ctx, req.ProposalId, voter)
vote, err := q.k.GetVote(ctx, req.ProposalId, voter)
if err != nil {
if errors.IsOf(err, types.ErrVoteNotFound) {
return nil, status.Errorf(codes.InvalidArgument,
Expand All @@ -136,7 +142,7 @@ func (q Keeper) Vote(ctx context.Context, req *v1.QueryVoteRequest) (*v1.QueryVo
}

// Votes returns single proposal's votes
func (q Keeper) Votes(ctx context.Context, req *v1.QueryVotesRequest) (*v1.QueryVotesResponse, error) {
func (q queryServer) Votes(ctx context.Context, req *v1.QueryVotesRequest) (*v1.QueryVotesResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "invalid request")
}
Expand All @@ -146,12 +152,12 @@ func (q Keeper) Votes(ctx context.Context, req *v1.QueryVotesRequest) (*v1.Query
}

var votes v1.Votes
store := q.storeService.OpenKVStore(ctx)
store := q.k.storeService.OpenKVStore(ctx)
votesStore := prefix.NewStore(runtime.KVStoreAdapter(store), types.VotesKey(req.ProposalId))

pageRes, err := query.Paginate(votesStore, req.Pagination, func(key, value []byte) error {
var vote v1.Vote
if err := q.cdc.Unmarshal(value, &vote); err != nil {
if err := q.k.cdc.Unmarshal(value, &vote); err != nil {
return err
}

Expand All @@ -166,12 +172,12 @@ func (q Keeper) Votes(ctx context.Context, req *v1.QueryVotesRequest) (*v1.Query
}

// Params queries all params
func (q Keeper) Params(ctx context.Context, req *v1.QueryParamsRequest) (*v1.QueryParamsResponse, error) {
func (q queryServer) Params(ctx context.Context, req *v1.QueryParamsRequest) (*v1.QueryParamsResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "invalid request")
}

params, err := q.GetParams(ctx)
params, err := q.k.GetParams(ctx)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -202,7 +208,7 @@ func (q Keeper) Params(ctx context.Context, req *v1.QueryParamsRequest) (*v1.Que
}

// Deposit queries single deposit information based on proposalID, depositAddr.
func (q Keeper) Deposit(ctx context.Context, req *v1.QueryDepositRequest) (*v1.QueryDepositResponse, error) {
func (q queryServer) Deposit(ctx context.Context, req *v1.QueryDepositRequest) (*v1.QueryDepositResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "invalid request")
}
Expand All @@ -215,11 +221,11 @@ func (q Keeper) Deposit(ctx context.Context, req *v1.QueryDepositRequest) (*v1.Q
return nil, status.Error(codes.InvalidArgument, "empty depositor address")
}

depositor, err := q.authKeeper.StringToBytes(req.Depositor)
depositor, err := q.k.authKeeper.StringToBytes(req.Depositor)
if err != nil {
return nil, err
}
deposit, err := q.GetDeposit(ctx, req.ProposalId, depositor)
deposit, err := q.k.GetDeposit(ctx, req.ProposalId, depositor)
if err != nil {
if errors.IsOf(err, types.ErrDepositNotFound) {
return nil, status.Errorf(codes.InvalidArgument,
Expand All @@ -232,7 +238,7 @@ func (q Keeper) Deposit(ctx context.Context, req *v1.QueryDepositRequest) (*v1.Q
}

// Deposits returns single proposal's all deposits
func (q Keeper) Deposits(ctx context.Context, req *v1.QueryDepositsRequest) (*v1.QueryDepositsResponse, error) {
func (q queryServer) Deposits(ctx context.Context, req *v1.QueryDepositsRequest) (*v1.QueryDepositsResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "invalid request")
}
Expand All @@ -243,12 +249,12 @@ func (q Keeper) Deposits(ctx context.Context, req *v1.QueryDepositsRequest) (*v1

var deposits []*v1.Deposit

store := q.storeService.OpenKVStore(ctx)
store := q.k.storeService.OpenKVStore(ctx)
depositStore := prefix.NewStore(runtime.KVStoreAdapter(store), types.DepositsKey(req.ProposalId))

pageRes, err := query.Paginate(depositStore, req.Pagination, func(key, value []byte) error {
var deposit v1.Deposit
if err := q.cdc.Unmarshal(value, &deposit); err != nil {
if err := q.k.cdc.Unmarshal(value, &deposit); err != nil {
return err
}

Expand All @@ -263,7 +269,7 @@ func (q Keeper) Deposits(ctx context.Context, req *v1.QueryDepositsRequest) (*v1
}

// TallyResult queries the tally of a proposal vote
func (q Keeper) TallyResult(ctx context.Context, req *v1.QueryTallyResultRequest) (*v1.QueryTallyResultResponse, error) {
func (q queryServer) TallyResult(ctx context.Context, req *v1.QueryTallyResultRequest) (*v1.QueryTallyResultResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "invalid request")
}
Expand All @@ -272,7 +278,7 @@ func (q Keeper) TallyResult(ctx context.Context, req *v1.QueryTallyResultRequest
return nil, status.Error(codes.InvalidArgument, "proposal id can not be 0")
}

proposal, err := q.GetProposal(ctx, req.ProposalId)
proposal, err := q.k.GetProposal(ctx, req.ProposalId)
if err != nil {
if errors.IsOf(err, types.ErrProposalNotFound) {
return nil, status.Errorf(codes.NotFound, "proposal %d doesn't exist", req.ProposalId)
Expand All @@ -292,7 +298,7 @@ func (q Keeper) TallyResult(ctx context.Context, req *v1.QueryTallyResultRequest
default:
// proposal is in voting period
var err error
_, _, tallyResult, err = q.Tally(ctx, proposal)
_, _, tallyResult, err = q.k.Tally(ctx, proposal)
if err != nil {
return nil, err
}
Expand All @@ -303,17 +309,15 @@ func (q Keeper) TallyResult(ctx context.Context, req *v1.QueryTallyResultRequest

var _ v1beta1.QueryServer = legacyQueryServer{}

type legacyQueryServer struct {
keeper *Keeper
}
type legacyQueryServer struct{ qs v1.QueryServer }

// NewLegacyQueryServer returns an implementation of the v1beta1 legacy QueryServer interface.
func NewLegacyQueryServer(k *Keeper) v1beta1.QueryServer {
return &legacyQueryServer{keeper: k}
return &legacyQueryServer{qs: NewQueryServer(*k)}
}

func (q legacyQueryServer) Proposal(ctx context.Context, req *v1beta1.QueryProposalRequest) (*v1beta1.QueryProposalResponse, error) {
resp, err := q.keeper.Proposal(ctx, &v1.QueryProposalRequest{
resp, err := q.qs.Proposal(ctx, &v1.QueryProposalRequest{
ProposalId: req.ProposalId,
})
if err != nil {
Expand All @@ -329,7 +333,7 @@ func (q legacyQueryServer) Proposal(ctx context.Context, req *v1beta1.QueryPropo
}

func (q legacyQueryServer) Proposals(ctx context.Context, req *v1beta1.QueryProposalsRequest) (*v1beta1.QueryProposalsResponse, error) {
resp, err := q.keeper.Proposals(ctx, &v1.QueryProposalsRequest{
resp, err := q.qs.Proposals(ctx, &v1.QueryProposalsRequest{
ProposalStatus: v1.ProposalStatus(req.ProposalStatus),
Voter: req.Voter,
Depositor: req.Depositor,
Expand All @@ -354,7 +358,7 @@ func (q legacyQueryServer) Proposals(ctx context.Context, req *v1beta1.QueryProp
}

func (q legacyQueryServer) Vote(ctx context.Context, req *v1beta1.QueryVoteRequest) (*v1beta1.QueryVoteResponse, error) {
resp, err := q.keeper.Vote(ctx, &v1.QueryVoteRequest{
resp, err := q.qs.Vote(ctx, &v1.QueryVoteRequest{
ProposalId: req.ProposalId,
Voter: req.Voter,
})
Expand All @@ -371,7 +375,7 @@ func (q legacyQueryServer) Vote(ctx context.Context, req *v1beta1.QueryVoteReque
}

func (q legacyQueryServer) Votes(ctx context.Context, req *v1beta1.QueryVotesRequest) (*v1beta1.QueryVotesResponse, error) {
resp, err := q.keeper.Votes(ctx, &v1.QueryVotesRequest{
resp, err := q.qs.Votes(ctx, &v1.QueryVotesRequest{
ProposalId: req.ProposalId,
Pagination: req.Pagination,
})
Expand All @@ -395,7 +399,7 @@ func (q legacyQueryServer) Votes(ctx context.Context, req *v1beta1.QueryVotesReq

//nolint:staticcheck // this is needed for legacy param support
func (q legacyQueryServer) Params(ctx context.Context, req *v1beta1.QueryParamsRequest) (*v1beta1.QueryParamsResponse, error) {
resp, err := q.keeper.Params(ctx, &v1.QueryParamsRequest{
resp, err := q.qs.Params(ctx, &v1.QueryParamsRequest{
ParamsType: req.ParamsType,
})
if err != nil {
Expand Down Expand Up @@ -434,7 +438,7 @@ func (q legacyQueryServer) Params(ctx context.Context, req *v1beta1.QueryParamsR
}

func (q legacyQueryServer) Deposit(ctx context.Context, req *v1beta1.QueryDepositRequest) (*v1beta1.QueryDepositResponse, error) {
resp, err := q.keeper.Deposit(ctx, &v1.QueryDepositRequest{
resp, err := q.qs.Deposit(ctx, &v1.QueryDepositRequest{
ProposalId: req.ProposalId,
Depositor: req.Depositor,
})
Expand All @@ -447,7 +451,7 @@ func (q legacyQueryServer) Deposit(ctx context.Context, req *v1beta1.QueryDeposi
}

func (q legacyQueryServer) Deposits(ctx context.Context, req *v1beta1.QueryDepositsRequest) (*v1beta1.QueryDepositsResponse, error) {
resp, err := q.keeper.Deposits(ctx, &v1.QueryDepositsRequest{
resp, err := q.qs.Deposits(ctx, &v1.QueryDepositsRequest{
ProposalId: req.ProposalId,
Pagination: req.Pagination,
})
Expand All @@ -463,7 +467,7 @@ func (q legacyQueryServer) Deposits(ctx context.Context, req *v1beta1.QueryDepos
}

func (q legacyQueryServer) TallyResult(ctx context.Context, req *v1beta1.QueryTallyResultRequest) (*v1beta1.QueryTallyResultResponse, error) {
resp, err := q.keeper.TallyResult(ctx, &v1.QueryTallyResultRequest{
resp, err := q.qs.TallyResult(ctx, &v1.QueryTallyResultRequest{
ProposalId: req.ProposalId,
})
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion x/gov/keeper/keeper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (suite *KeeperTestSuite) reset() {
suite.NoError(err)

queryHelper := baseapp.NewQueryServerTestHelper(ctx, encCfg.InterfaceRegistry)
v1.RegisterQueryServer(queryHelper, govKeeper)
v1.RegisterQueryServer(queryHelper, keeper.NewQueryServer(*govKeeper))
legacyQueryHelper := baseapp.NewQueryServerTestHelper(ctx, encCfg.InterfaceRegistry)
v1beta1.RegisterQueryServer(legacyQueryHelper, keeper.NewLegacyQueryServer(govKeeper))
queryClient := v1.NewQueryClient(queryHelper)
Expand Down
2 changes: 1 addition & 1 deletion x/gov/module.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ func (am AppModule) RegisterServices(cfg module.Configurator) {

legacyQueryServer := keeper.NewLegacyQueryServer(am.keeper)
v1beta1.RegisterQueryServer(cfg.QueryServer(), legacyQueryServer)
v1.RegisterQueryServer(cfg.QueryServer(), am.keeper)
v1.RegisterQueryServer(cfg.QueryServer(), keeper.NewQueryServer(*am.keeper))

m := keeper.NewMigrator(am.keeper, am.legacySubspace)
if err := cfg.RegisterMigration(govtypes.ModuleName, 1, m.Migrate1to2); err != nil {
Expand Down

0 comments on commit f866c05

Please sign in to comment.