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

feat: add basic autocli config to every module #13786

Merged
merged 12 commits into from
Nov 9, 2022
50 changes: 50 additions & 0 deletions runtime/services/autocli.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"context"

autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"
gogogrpc "github.com/cosmos/gogoproto/grpc"
"google.golang.org/grpc"

"github.com/cosmos/cosmos-sdk/types/module"
)
Expand All @@ -22,6 +24,31 @@ func NewAutoCLIQueryService(appModules map[string]module.AppModule) *AutoCLIQuer
AutoCLIOptions() *autocliv1.ModuleOptions
}); ok {
moduleOptions[modName] = autoCliMod.AutoCLIOptions()
} else {
// try to auto-discover options based on the last msg and query
Copy link
Contributor

Choose a reason for hiding this comment

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

I was asking above because what happens when we wire up autocli with the root cobra command, and there are conflicts in command names between this auto-discovered one and custom ones?

Copy link
Member Author

Choose a reason for hiding this comment

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

AutoCLI prefers the custom commands

// services registered for the module
cfg := &autocliConfigurator{}
mod.RegisterServices(cfg)
modOptions := &autocliv1.ModuleOptions{}
haveServices := false

if cfg.msgServer.serviceName != "" {
haveServices = true
modOptions.Tx = &autocliv1.ServiceCommandDescriptor{
Service: cfg.msgServer.serviceName,
}
}

if cfg.queryServer.serviceName != "" {
haveServices = true
modOptions.Query = &autocliv1.ServiceCommandDescriptor{
Service: cfg.queryServer.serviceName,
}
}

if haveServices {
moduleOptions[modName] = modOptions
}
}
}
return &AutoCLIQueryService{
Expand All @@ -35,4 +62,27 @@ func (a AutoCLIQueryService) AppOptions(context.Context, *autocliv1.AppOptionsRe
}, nil
}

// autocliConfigurator allows us to call RegisterServices and introspect the services
type autocliConfigurator struct {
Copy link
Member

Choose a reason for hiding this comment

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

Nit move that down next to its methods.
Can you explain the use case?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is just so that we can call RegisterServices to discover the module's service names.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this PR using the configurator approach is an intermediary solution, right? I'm in favor of removing the Configurator interface ultimately (since we're going the interfaces approach)

AppModule's RegisterServices should be simply RegisterServices(grpc.ServiceRegistrar), which would remove autocliConfigurator from this file.

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly

msgServer autocliServiceRegistrar
queryServer autocliServiceRegistrar
}

func (a *autocliConfigurator) MsgServer() gogogrpc.Server { return &a.msgServer }

func (a *autocliConfigurator) QueryServer() gogogrpc.Server { return &a.queryServer }

func (a *autocliConfigurator) RegisterMigration(string, uint64, module.MigrationHandler) error {
return nil
}

// autocliServiceRegistrar is used to capture the service name for registered services
type autocliServiceRegistrar struct {
serviceName string
}

func (a *autocliServiceRegistrar) RegisterService(sd *grpc.ServiceDesc, _ interface{}) {
a.serviceName = sd.ServiceName
}

var _ autocliv1.QueryServer = &AutoCLIQueryService{}
17 changes: 15 additions & 2 deletions tests/integration/runtime/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,21 @@ func TestQueryAutoCLIAppOptions(t *testing.T) {
assert.NilError(t, err)
assert.Assert(t, res != nil && res.ModuleOptions != nil)

// make sure we at least have x/auth autocli options
// make sure we have x/auth autocli options which were configured manually
authOpts := res.ModuleOptions["auth"]
assert.Assert(t, authOpts != nil)
assert.Assert(t, authOpts.Query != nil && authOpts.Query.Service != "")
assert.Assert(t, authOpts.Query != nil)
assert.Equal(t, "cosmos.auth.v1beta1.Query", authOpts.Query.Service)
// make sure we have some custom options
assert.Assert(t, len(authOpts.Query.RpcCommandOptions) != 0)

// make sure we have x/staking autocli options which should have been auto-discovered
stakingOpts := res.ModuleOptions["staking"]
assert.Assert(t, stakingOpts != nil)
assert.Assert(t, stakingOpts.Query != nil && stakingOpts.Tx != nil)
assert.Equal(t, "cosmos.staking.v1beta1.Query", stakingOpts.Query.Service)
assert.Equal(t, "cosmos.staking.v1beta1.Msg", stakingOpts.Tx.Service)

// make sure tx module has no autocli options because it has no services
assert.Assert(t, res.ModuleOptions["tx"] == nil)
}
32 changes: 32 additions & 0 deletions x/auth/autocli.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package auth

import (
authv1beta1 "cosmossdk.io/api/cosmos/auth/v1beta1"
autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"
)

// AutoCLIOptions implements the autocli.HasAutoCLIConfig interface.
func (am AppModule) AutoCLIOptions() *autocliv1.ModuleOptions {
aaronc marked this conversation as resolved.
Show resolved Hide resolved
return &autocliv1.ModuleOptions{
Query: &autocliv1.ServiceCommandDescriptor{
Service: authv1beta1.Query_ServiceDesc.ServiceName,
RpcCommandOptions: []*autocliv1.RpcCommandOptions{
{
RpcMethod: "Account",
Use: "account [address]",
Short: "query account by address",
PositionalArgs: []*autocliv1.PositionalArgDescriptor{{ProtoField: "address"}},
},
{
RpcMethod: "AccountAddressByID",
Use: "address-by-id [acc-num]",
Short: "query account address by account number",
PositionalArgs: []*autocliv1.PositionalArgDescriptor{{ProtoField: "id"}},
},
},
},
Tx: &autocliv1.ServiceCommandDescriptor{
Service: authv1beta1.Msg_ServiceDesc.ServiceName,
},
}
}
6 changes: 0 additions & 6 deletions x/auth/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (

"cosmossdk.io/depinject"

autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"
"cosmossdk.io/core/appmodule"

modulev1 "cosmossdk.io/api/cosmos/auth/module/v1"
Expand Down Expand Up @@ -111,11 +110,6 @@ func (am AppModule) IsOnePerModuleType() {}
// IsAppModule implements the appmodule.AppModule interface.
func (am AppModule) IsAppModule() {}

// AutoCLIOptions implements the autocli.HasAutoCLIConfig interface.
func (am AppModule) AutoCLIOptions() *autocliv1.ModuleOptions {
return types.AutoCLIOptions
}

// NewAppModule creates a new AppModule object
func NewAppModule(cdc codec.Codec, accountKeeper keeper.AccountKeeper, randGenAccountsFn types.RandomGenesisAccountsFn, ss exported.Subspace) AppModule {
return AppModule{
Expand Down
26 changes: 0 additions & 26 deletions x/auth/types/autocli.go

This file was deleted.

27 changes: 27 additions & 0 deletions x/gov/autocli.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package gov

import (
autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"
govv1 "cosmossdk.io/api/cosmos/gov/v1"
govv1beta1 "cosmossdk.io/api/cosmos/gov/v1beta1"
)

// AutoCLIOptions implements the autocli.HasAutoCLIConfig interface.
func (am AppModule) AutoCLIOptions() *autocliv1.ModuleOptions {
return &autocliv1.ModuleOptions{
Tx: &autocliv1.ServiceCommandDescriptor{
Service: govv1.Msg_ServiceDesc.ServiceName,
// map v1beta1 as a sub-command
SubCommands: map[string]*autocliv1.ServiceCommandDescriptor{
"v1beta1": {Service: govv1beta1.Msg_ServiceDesc.ServiceName},
Copy link
Contributor

Choose a reason for hiding this comment

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

autocli is not wired up to simd's root cmd, right? When I try simd q gov there's no v1beta1 subcommand. I guess there's only the remote info part of autocli for now

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly

},
},
Query: &autocliv1.ServiceCommandDescriptor{
Service: govv1.Query_ServiceDesc.ServiceName,
// map v1beta1 as a sub-command
SubCommands: map[string]*autocliv1.ServiceCommandDescriptor{
"v1beta1": {Service: govv1beta1.Query_ServiceDesc.ServiceName},
},
},
}
}
7 changes: 7 additions & 0 deletions x/gov/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
modulev1 "cosmossdk.io/api/cosmos/gov/module/v1"
"cosmossdk.io/core/appmodule"
"cosmossdk.io/depinject"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/codec"
Expand Down Expand Up @@ -121,6 +122,12 @@ func (a AppModuleBasic) RegisterInterfaces(registry codectypes.InterfaceRegistry
v1beta1.RegisterInterfaces(registry)
}

// IsOnePerModuleType implements the depinject.OnePerModuleType interface.
func (am AppModule) IsOnePerModuleType() {}

// IsAppModule implements the appmodule.AppModule interface.
func (am AppModule) IsAppModule() {}

// AppModule implements an application module for the gov module.
type AppModule struct {
AppModuleBasic
Expand Down