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: use autocli for comet commands #17389

Merged
merged 16 commits into from
Aug 16, 2023
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

* (cli) [#17389](https://github.com/cosmos/cosmos-sdk/pull/17389) gRPC CometBFT commands have been added under `<aapd> q consensus comet`. Duplicate commands have been removed. CometBFT commands placement in the SDK has been simplified. See the exhaustive list below.
* `<appd> q comet-validator-set` is now `<appd> q consensus comet validator-set`
* `client/rpc.StatusCommand()` is now at `server.StatusCommand()`
* (x/group, x/gov) [#17220](https://github.com/cosmos/cosmos-sdk/pull/17220) Add `--skip-metadata` flag in `draft-proposal` to skip metadata prompt.
* (x/group, x/gov) [#17109](https://github.com/cosmos/cosmos-sdk/pull/17109) Let proposal summary be 40x longer than metadata limit.
* (all) [#16537](https://github.com/cosmos/cosmos-sdk/pull/16537) Properly propagated `fmt.Errorf` errors and using `errors.New` where appropriate.
Expand Down
71 changes: 71 additions & 0 deletions client/grpc/cmtservice/autocli.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package cmtservice

import (
autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"
cmtv1beta1 "cosmossdk.io/api/cosmos/base/tendermint/v1beta1"
)

var CometBFTAutoCLIDescriptor = &autocliv1.ServiceCommandDescriptor{
Service: cmtv1beta1.Service_ServiceDesc.ServiceName,
RpcCommandOptions: []*autocliv1.RpcCommandOptions{
{
RpcMethod: "GetNodeInfo",
Use: "node-info",
Short: "Query the current node info",
},
{
RpcMethod: "GetSyncing",
Use: "syncing",
Short: "Query node syncing status",
},
{
RpcMethod: "GetLatestBlock",
Use: "block-latest",
Short: "Query for the latest committed block",
},
{
RpcMethod: "GetBlockByHeight",
Use: "block-by-height [height]",
Short: "Query for a committed block by height",
Long: "Query for a specific committed block using the CometBFT RPC `block_by_height` method",
PositionalArgs: []*autocliv1.PositionalArgDescriptor{{ProtoField: "height"}},
},
{
RpcMethod: "GetLatestValidatorSet",
Use: "validator-set",
Alias: []string{"validator-set-latest", "comet-validator-set", "cometbft-validator-set", "tendermint-validator-set"},
Short: "Query for the latest validator set",
},
{
RpcMethod: "GetValidatorSetByHeight",
Use: "validator-set-by-height [height]",
Short: "Query for a validator set by height",
PositionalArgs: []*autocliv1.PositionalArgDescriptor{{ProtoField: "height"}},
},
{
RpcMethod: "ABCIQuery",
Skip: true,
},
},
}

// NewCometBFTCommands is a fake `appmodule.Module` to be considered as a module
// and be added in AutoCLI.
func NewCometBFTCommands() *cometModule {
return &cometModule{}
}

type cometModule struct{}

func (m cometModule) IsOnePerModuleType() {}
func (m cometModule) IsAppModule() {}
Copy link
Member

Choose a reason for hiding this comment

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

this seems confusing, can you add a godoc why this interface is met here?

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 at line 52, what do you think?

I'm planing to add docs on that as well.


func (m cometModule) Name() string {
return "comet"
}

func (m cometModule) AutoCLIOptions() *autocliv1.ModuleOptions {
return &autocliv1.ModuleOptions{
Query: CometBFTAutoCLIDescriptor,
}
}
2 changes: 1 addition & 1 deletion client/grpc/cmtservice/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
)

func getBlockHeight(ctx context.Context, clientCtx client.Context) (int64, error) {
status, err := getNodeStatus(ctx, clientCtx)
status, err := GetNodeStatus(ctx, clientCtx)
if err != nil {
return 0, err
}
Expand Down
4 changes: 2 additions & 2 deletions client/grpc/cmtservice/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func NewQueryServer(

// GetSyncing implements ServiceServer.GetSyncing
func (s queryServer) GetSyncing(ctx context.Context, _ *GetSyncingRequest) (*GetSyncingResponse, error) {
status, err := getNodeStatus(ctx, s.clientCtx)
status, err := GetNodeStatus(ctx, s.clientCtx)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -189,7 +189,7 @@ func ValidatorsOutput(ctx context.Context, clientCtx client.Context, height *int

// GetNodeInfo implements ServiceServer.GetNodeInfo
func (s queryServer) GetNodeInfo(ctx context.Context, _ *GetNodeInfoRequest) (*GetNodeInfoResponse, error) {
status, err := getNodeStatus(ctx, s.clientCtx)
status, err := GetNodeStatus(ctx, s.clientCtx)
if err != nil {
return nil, err
}
Expand Down
3 changes: 2 additions & 1 deletion client/grpc/cmtservice/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import (
"github.com/cosmos/cosmos-sdk/client"
)

func getNodeStatus(ctx context.Context, clientCtx client.Context) (*coretypes.ResultStatus, error) {
// GetNodeStatus returns the status of the node.
func GetNodeStatus(ctx context.Context, clientCtx client.Context) (*coretypes.ResultStatus, error) {
node, err := clientCtx.GetNode()
if err != nil {
return &coretypes.ResultStatus{}, err
Expand Down
30 changes: 30 additions & 0 deletions client/grpc/cmtservice/status_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package cmtservice_test

import (
"fmt"
"testing"

"github.com/stretchr/testify/require"

"github.com/cosmos/cosmos-sdk/server"
clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli"
"github.com/cosmos/cosmos-sdk/testutil/network"
)

func TestStatusCommand(t *testing.T) {
cfg, err := network.DefaultConfigWithAppConfig(network.MinimumAppConfig())
require.NoError(t, err)

network, err := network.New(t, t.TempDir(), cfg)
require.NoError(t, err)
require.NoError(t, network.WaitForNextBlock())

val0 := network.Validators[0]
cmd := server.StatusCommand()

out, err := clitestutil.ExecTestCLICmd(val0.ClientCtx, cmd, []string{})
require.NoError(t, err)

// Make sure the output has the validator moniker.
require.Contains(t, out.String(), fmt.Sprintf("\"moniker\":\"%s\"", val0.Moniker))
}
13 changes: 0 additions & 13 deletions client/rpc/rpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import (
"google.golang.org/grpc"
"google.golang.org/grpc/metadata"

"github.com/cosmos/cosmos-sdk/client/rpc"
clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli"
"github.com/cosmos/cosmos-sdk/testutil/network"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
"github.com/cosmos/cosmos-sdk/types/address"
Expand Down Expand Up @@ -44,17 +42,6 @@ func (s *IntegrationTestSuite) TearDownSuite() {
s.network.Cleanup()
}

func (s *IntegrationTestSuite) TestStatusCommand() {
val0 := s.network.Validators[0]
cmd := rpc.StatusCommand()

out, err := clitestutil.ExecTestCLICmd(val0.ClientCtx, cmd, []string{})
s.Require().NoError(err)

// Make sure the output has the validator moniker.
s.Require().Contains(out.String(), fmt.Sprintf("\"moniker\":\"%s\"", val0.Moniker))
}

func (s *IntegrationTestSuite) TestCLIQueryConn() {
s.T().Skip("data race in comet is causing this to fail")
var header metadata.MD
Expand Down
58 changes: 0 additions & 58 deletions client/rpc/status.go

This file was deleted.

59 changes: 0 additions & 59 deletions client/rpc/validators.go
Copy link
Member Author

Choose a reason for hiding this comment

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

note, in the backport I'll leave this in.

This file was deleted.

10 changes: 10 additions & 0 deletions client/v2/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,16 @@ https://github.com/cosmos/cosmos-sdk/blob/fa4d87ef7e6d87aaccc94c337ffd2fe90fcb7a

If not set to true, `AutoCLI` will not generate commands for the module if there are already commands registered for the module (when `GetTxCmd()` or `GetTxCmd()` are defined).

### Use AutoCLI for non module commands

It is possible to use `AutoCLI` for non module commands. The trick is still to implement the `appmodule.Module` interface and append it to the `appOptions.ModuleOptions` map.

For example, here is how the SDK does it for `cometbft` gRPC commands:

```go reference
https://github.com/cosmos/cosmos-sdk/blob/julien/autocli-comet/client/grpc/cmtservice/autocli.go#L52-L71
```

## Summary

`autocli` let you generate CLI to your Cosmos SDK-based applications without any cobra boilerplate. It allows you to easily generate CLI commands and flags from your protobuf messages, and provides many options for customising the behavior of your CLI application.
Expand Down
3 changes: 3 additions & 0 deletions client/v2/autocli/flags.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package autocli

var flagNoIndent = "no-indent"
6 changes: 6 additions & 0 deletions client/v2/autocli/msg.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ func (b *Builder) BuildMsgMethodCommand(descriptor protoreflect.MethodDescriptor
}

cmd, err := b.buildMethodCommandCommon(descriptor, options, func(cmd *cobra.Command, input protoreflect.Message) error {
if noIdent, _ := cmd.Flags().GetBool(flagNoIndent); noIdent {
jsonMarshalOptions.Indent = ""
}

bz, err := jsonMarshalOptions.Marshal(input.Interface())
if err != nil {
return err
Expand All @@ -114,6 +118,8 @@ func (b *Builder) BuildMsgMethodCommand(descriptor protoreflect.MethodDescriptor

if b.AddTxConnFlags != nil {
b.AddTxConnFlags(cmd)

cmd.Flags().BoolP(flagNoIndent, "", false, "Do not indent JSON output")
}

return cmd, err
Expand Down
6 changes: 6 additions & 0 deletions client/v2/autocli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ func (b *Builder) BuildQueryMethodCommand(descriptor protoreflect.MethodDescript
}

cmd, err := b.buildMethodCommandCommon(descriptor, options, func(cmd *cobra.Command, input protoreflect.Message) error {
if noIdent, _ := cmd.Flags().GetBool(flagNoIndent); noIdent {
jsonMarshalOptions.Indent = ""
}

clientConn, err := getClientConn(cmd)
if err != nil {
return err
Expand All @@ -133,6 +137,8 @@ func (b *Builder) BuildQueryMethodCommand(descriptor protoreflect.MethodDescript

if b.AddQueryConnFlags != nil {
b.AddQueryConnFlags(cmd)

cmd.Flags().BoolP(flagNoIndent, "", false, "Do not indent JSON output")
}

return cmd, nil
Expand Down
1 change: 1 addition & 0 deletions client/v2/autocli/testdata/help-deprecated-msg.golden
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ Flags:
--keyring-backend string Select keyring's backend (os|file|kwallet|pass|test|memory) (default "os")
--keyring-dir string The client Keyring directory; if omitted, the default 'home' directory will be used
--ledger Use a connected Ledger device
--no-indent Do not indent JSON output
--node string <host>:<port> to CometBFT rpc interface for this chain (default "tcp://localhost:26657")
--note string Note to add a description to the transaction (previously --memo)
--offline Offline mode (does not allow any online functionality)
Expand Down
1 change: 1 addition & 0 deletions client/v2/autocli/testdata/help-deprecated.golden
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Flags:
--map-string-coin map[string]cosmos.base.v1beta1.Coin
--map-string-string stringToString (default [])
--map-string-uint32 stringToUint32
--no-indent Do not indent JSON output
--node string <host>:<port> to CometBFT RPC interface for this chain (default "tcp://localhost:26657")
-o, --output string Output format (text|json) (default "text")
--page-count-total
Expand Down
1 change: 1 addition & 0 deletions client/v2/autocli/testdata/help-echo-msg.golden
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ Flags:
--keyring-backend string Select keyring's backend (os|file|kwallet|pass|test|memory) (default "os")
--keyring-dir string The client Keyring directory; if omitted, the default 'home' directory will be used
--ledger Use a connected Ledger device
--no-indent Do not indent JSON output
--node string <host>:<port> to CometBFT rpc interface for this chain (default "tcp://localhost:26657")
--note string Note to add a description to the transaction (previously --memo)
--offline Offline mode (does not allow any online functionality)
Expand Down
1 change: 1 addition & 0 deletions client/v2/autocli/testdata/help-echo.golden
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Flags:
--map-string-coin map[string]cosmos.base.v1beta1.Coin some map of string to coin
--map-string-string stringToString some map of string to string (default [])
--map-string-uint32 stringToUint32 some map of string to int32
--no-indent Do not indent JSON output
--node string <host>:<port> to CometBFT RPC interface for this chain (default "tcp://localhost:26657")
-o, --output string Output format (text|json) (default "text")
--page-count-total
Expand Down
Loading