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.

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

Use unexported types for core query servers #6775

Closed
3 tasks
damiannolan opened this issue Jul 8, 2024 · 0 comments · Fixed by #6794
Closed
3 tasks

Use unexported types for core query servers #6775

damiannolan opened this issue Jul 8, 2024 · 0 comments · Fixed by #6794
Assignees
Labels
core gRPC Issues for gRPC endpoints needs discussion Issues that need discussion before they can be worked on type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Milestone

Comments

@damiannolan
Copy link
Member

Summary

Introduce unexported types for core query servers in ibc-core:

  • 02-client
  • 03-connection
  • 04-channel

Limit the api surface of each of the keepers belonging to core subpackages.
Add a private struct which embdeds the Keeper type, and move QueryServer interface methods to this type.

var _ types.QueryServer = (*queryServer)(nil)

type queryServer struct {
    *Keeper
}

Note, that this will have a knock on affect to how we register the query servers in the ibc core module.
As it exists today, the IBCKeeper implements every query servers through core/keeper/grpc_query.go, with a passthrough function for every query handler. This file is unnecessary and could be removed. As a result, we may register query servers like below.

- types.RegisterQueryService(cfg.QueryServer(), am.keeper)
+ clienttypes.RegisterQueryServer(cfg.QueryServer(), clientkeeper.NewQueryServer(am.keeper.ClientKeeper))
+ connectiontypes.RegisterQueryServer(cfg.QueryServer(), connectionkeeper.NewQueryServer(am.keeper.ConnectionKeeper))
+ channeltypes.RegisterQueryServer(cfg.QueryServer(), channelkeeper.NewQueryServer(am.keeper.ChannelKeeper))

Considerations

As mentioned above, the IBCKeeper currently implements every QueryServer interface of each subpackage in ibc core.
The QueryServer interface in core/types, composes each of the subpackage QueryServers into one interface.
This interface is composed by the TestChain in ibctesting, where the IBCKeeper is passed the concrete implementation.

If the grpc_query.go file is removed this will break that abstraction. However, we can explore alternative options when working on this issue. It may be possible to still include some facade pattern struct to provide access to these query handlers.
The QueryServers can still be created on demand by passing the respective subpackage Keeper to the public constructor function. E.g. clientkeeper.NewQueryServer


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@damiannolan damiannolan added core needs discussion Issues that need discussion before they can be worked on type: refactor Architecture, code or CI improvements that may or may not tackle technical debt. gRPC Issues for gRPC endpoints labels Jul 8, 2024
@damiannolan damiannolan added this to the v9.0.0 milestone Jul 8, 2024
@damiannolan damiannolan self-assigned this Jul 8, 2024
@damiannolan damiannolan moved this to Todo 🏃 in ibc-go Jul 8, 2024
@damiannolan damiannolan moved this from Todo 🏃 to In progress 👷 in ibc-go Jul 8, 2024
github-merge-queue bot pushed a commit that referenced this issue Jul 9, 2024
…ient keeper (#6760)

* chore: pull out get light client module to helper methods (#6712)

* chore: pull out get light client module to helper methods

* another place to use getLightClientModuleRoute

* use getLightClientModule in other 02-client functions

* lint

---------

Co-authored-by: Carlos Rodriguez <[email protected]>

* add verify(non)membership functions to client keeper and removed route from interface

* delete file

* use VerifyMembershipProof function name until #6775

* Update modules/core/02-client/keeper/keeper.go

Co-authored-by: DimitrisJim <[email protected]>

---------

Co-authored-by: Gjermund Garaba <[email protected]>
Co-authored-by: DimitrisJim <[email protected]>
@github-project-automation github-project-automation bot moved this from In progress 👷 to Done 🥳 in ibc-go Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core gRPC Issues for gRPC endpoints needs discussion Issues that need discussion before they can be worked on type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant