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

refactor: Use HybridResolver instead of protoregistry.GlobalFiles in runtime/v2/manager #22744

Merged
merged 2 commits into from
Dec 4, 2024
Merged

refactor: Use HybridResolver instead of protoregistry.GlobalFiles in runtime/v2/manager #22744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5fafc10
use hybrid resolver instead of GlobalFiles
hieuvubk Dec 4, 2024
ae4249f
no need
hieuvubk Dec 4, 2024
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
7 changes: 3 additions & 4 deletions runtime/v2/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"google.golang.org/grpc"
proto "google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/reflect/protoregistry"

runtimev2 "cosmossdk.io/api/cosmos/app/runtime/v2"
cosmosmsg "cosmossdk.io/api/cosmos/msg/v1"
Expand Down Expand Up @@ -485,7 +484,7 @@ func (m *MM[T]) RunMigrations(ctx context.Context, fromVM appmodulev2.VersionMap
func (m *MM[T]) RegisterServices(app *App[T]) error {
for _, module := range m.modules {
// register msg + query
if err := registerServices(module, app, protoregistry.GlobalFiles); err != nil {
if err := registerServices(module, app, gogoproto.HybridResolver); err != nil {
return err
}
Comment on lines +487 to 489
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Several non-test files still use protoregistry.GlobalFiles and need to be updated

The following files need to be updated to use gogoproto.HybridResolver instead of protoregistry.GlobalFiles:

  • types/msgservice/validate.go
  • server/mock/app.go
  • runtime/v2/types.go
  • orm/model/ormdb/module.go
  • client/v2/autocli/flag/builder.go

Test files using protoregistry.GlobalFiles should also be reviewed and updated for consistency, though they don't affect runtime behavior.

🔗 Analysis chain

Verify no remaining usages of protoregistry.GlobalFiles

To ensure consistency across the codebase, please verify that all instances of protoregistry.GlobalFiles have been replaced with gogoproto.HybridResolver.

Run the following script to identify any remaining usages:

Let me verify if these usages are in test files or actual implementation code by categorizing them.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for any usages of protoregistry.GlobalFiles in the codebase.

# Test: Expect no matches for protoregistry.GlobalFiles
rg 'protoregistry\.GlobalFiles'

Length of output: 1351

Script:

#!/bin/bash
# Check for non-test files containing protoregistry.GlobalFiles
echo "=== Non-test files ==="
rg 'protoregistry\.GlobalFiles' --type go | grep -v '_test.go'

echo -e "\n=== Test files ==="
rg 'protoregistry\.GlobalFiles' --type go | grep '_test.go'

Length of output: 1548


Expand Down Expand Up @@ -618,7 +617,7 @@ func (m *MM[T]) assertNoForgottenModules(
return nil
}

func registerServices[T transaction.Tx](s appmodulev2.AppModule, app *App[T], registry *protoregistry.Files) error {
func registerServices[T transaction.Tx](s appmodulev2.AppModule, app *App[T], registry gogoproto.Resolver) error {
// case module with services
if services, ok := s.(hasServicesV1); ok {
c := &configurator{
Expand Down Expand Up @@ -671,7 +670,7 @@ type configurator struct {

stfQueryRouter *stf.MsgRouterBuilder
stfMsgRouter *stf.MsgRouterBuilder
registry *protoregistry.Files
registry gogoproto.Resolver
err error
}

Expand Down
20 changes: 0 additions & 20 deletions x/accounts/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"google.golang.org/grpc"

"cosmossdk.io/core/appmodule"
appmodulev2 "cosmossdk.io/core/appmodule/v2"
"cosmossdk.io/core/registry"
"cosmossdk.io/x/accounts/cli"
v1 "cosmossdk.io/x/accounts/v1"
Expand Down Expand Up @@ -62,25 +61,6 @@ func (am AppModule) RegisterServices(registrar grpc.ServiceRegistrar) error {
return nil
}

// RegisterQueryHandlers registers the query handlers for the accounts module.
func (am AppModule) RegisterQueryHandlers(router appmodulev2.QueryRouter) {
queryServer := NewQueryServer(am.k)

appmodulev2.RegisterMsgHandler(router, queryServer.AccountNumber)
appmodulev2.RegisterMsgHandler(router, queryServer.AccountQuery)
appmodulev2.RegisterMsgHandler(router, queryServer.AccountType)
appmodulev2.RegisterMsgHandler(router, queryServer.Schema)
}

// RegisterMsgHandlers registers the message handlers for the accounts module.
func (am AppModule) RegisterMsgHandlers(router appmodulev2.MsgRouter) {
msgServer := NewMsgServer(am.k)

appmodulev2.RegisterMsgHandler(router, msgServer.Execute)
appmodulev2.RegisterMsgHandler(router, msgServer.ExecuteBundle)
appmodulev2.RegisterMsgHandler(router, msgServer.Init)
}

// App module genesis

func (am AppModule) DefaultGenesis() json.RawMessage {
Expand Down
Loading