Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(gov)!: decouple gRPC query methods from Keeper #16106

Merged
merged 7 commits into from
May 11, 2023
Merged

refactor(gov)!: decouple gRPC query methods from Keeper #16106

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c368bb9
separate GRPC query from keeper
May 11, 2023
50c43c3
revert collection prefix
May 11, 2023
6f25a0e
chore: CHANGELOG.md
May 11, 2023
c8384cc
chore: lint
May 11, 2023
68d1f20
Merge branch 'main' into tip/gov/coll-params
testinginprod May 11, 2023
7e7c3c6
Merge branch 'main' into tip/gov/coll-params
testinginprod May 11, 2023
8afd950
fix tests
May 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
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
6 changes: 3 additions & 3 deletions tests/integration/gov/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,17 @@ 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))
v1beta1.RegisterQueryServer(legacyQueryHelper, keeper.NewLegacyQueryServer(&app.GovKeeper))
queryClient := v1.NewQueryClient(queryHelper)
legacyQueryClient := v1beta1.NewQueryClient(legacyQueryHelper)

f.app = app
f.ctx = ctx
f.queryClient = queryClient
f.legacyQueryClient = legacyQueryClient
f.msgSrvr = keeper.NewMsgServerImpl(f.app.GovKeeper)
f.msgSrvr = keeper.NewMsgServerImpl(&f.app.GovKeeper)

govAcct := f.app.GovKeeper.GetGovernanceAccount(f.ctx).GetAddress()
f.legacyMsgSrvr = keeper.NewLegacyMsgServerImpl(govAcct.String(), f.msgSrvr)
Expand Down
86 changes: 45 additions & 41 deletions x/gov/keeper/grpc_query.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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