diff --git a/client/v2/CHANGELOG.md b/client/v2/CHANGELOG.md index c59e01268982..7323fd31787c 100644 --- a/client/v2/CHANGELOG.md +++ b/client/v2/CHANGELOG.md @@ -46,6 +46,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#20623](https://github.com/cosmos/cosmos-sdk/pull/20623) Extend client/v2 keyring interface with `KeyType` and `KeyInfo`. * [#22282](https://github.com/cosmos/cosmos-sdk/pull/22282) Added custom broadcast logic. * [#22775](https://github.com/cosmos/cosmos-sdk/pull/22775) Added interactive autocli prompt functionality, including message field prompting, validation helpers, and default value support. +* [#22890](https://github.com/cosmos/cosmos-sdk/pull/22890) Added support for flattening inner message fields in autocli as positional arguments. ### Improvements diff --git a/client/v2/README.md b/client/v2/README.md index b4c2666850ee..0176733c2f67 100644 --- a/client/v2/README.md +++ b/client/v2/README.md @@ -161,6 +161,48 @@ Then the command can be used as follows, instead of having to specify the `--add query auth account cosmos1abcd...xyz ``` +#### Flattened Fields in Positional Arguments + +AutoCLI also supports flattening nested message fields as positional arguments. This means you can access nested fields +using dot notation in the `ProtoField` parameter. This is particularly useful when you want to directly set nested +message fields as positional arguments. + +For example, if you have a nested message structure like this: + +```protobuf +message Permissions { + string level = 1; + repeated string limit_type_urls = 2; +} + +message MsgAuthorizeCircuitBreaker { + string grantee = 1; + Permissions permissions = 2; +} +``` + +You can flatten the fields in your AutoCLI configuration: + +```go +{ + RpcMethod: "AuthorizeCircuitBreaker", + Use: "authorize ", + PositionalArgs: []*autocliv1.PositionalArgDescriptor{ + {ProtoField: "grantee"}, + {ProtoField: "permissions.level"}, + {ProtoField: "permissions.limit_type_urls"}, + }, +} +``` + +This allows users to provide values for nested fields directly as positional arguments: + +```bash + tx circuit authorize cosmos1... super-admin "/cosmos.bank.v1beta1.MsgSend,/cosmos.bank.v1beta1.MsgMultiSend" +``` + +Instead of having to provide a complex JSON structure for nested fields, flattening makes the CLI more user-friendly by allowing direct access to nested fields. + #### Customising Flag Names By default, `autocli` generates flag names based on the names of the fields in your protobuf message. However, you can customise the flag names by providing a `FlagOptions`. This parameter allows you to specify custom names for flags based on the names of the message fields. diff --git a/client/v2/autocli/flag/builder.go b/client/v2/autocli/flag/builder.go index 61e7c7393eef..a11304281214 100644 --- a/client/v2/autocli/flag/builder.go +++ b/client/v2/autocli/flag/builder.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "strconv" + "strings" cosmos_proto "github.com/cosmos/cosmos-proto" "github.com/spf13/cobra" @@ -162,38 +163,32 @@ func (b *Builder) addMessageFlags(ctx *context.Context, flagSet *pflag.FlagSet, messageBinder.hasOptional = true } - field := fields.ByName(protoreflect.Name(arg.ProtoField)) - if field == nil { - return nil, fmt.Errorf("can't find field %s on %s", arg.ProtoField, messageType.Descriptor().FullName()) - } - - _, hasValue, err := b.addFieldFlag( - ctx, - messageBinder.positionalFlagSet, - field, - &autocliv1.FlagOptions{Name: fmt.Sprintf("%d", i)}, - namingOptions{}, - ) - if err != nil { - return nil, err + s := strings.Split(arg.ProtoField, ".") + if len(s) == 1 { + f, err := b.addFieldBindingToArgs(ctx, messageBinder, protoreflect.Name(arg.ProtoField), fields) + if err != nil { + return nil, err + } + messageBinder.positionalArgs = append(messageBinder.positionalArgs, f) + } else { + err := b.addFlattenFieldBindingToArgs(ctx, arg.ProtoField, s, messageType, messageBinder) + if err != nil { + return nil, err + } } - - messageBinder.positionalArgs = append(messageBinder.positionalArgs, fieldBinding{ - field: field, - hasValue: hasValue, - }) } + totalArgs := len(messageBinder.positionalArgs) switch { case messageBinder.hasVarargs: - messageBinder.CobraArgs = cobra.MinimumNArgs(positionalArgsLen - 1) - messageBinder.mandatoryArgUntil = positionalArgsLen - 1 + messageBinder.CobraArgs = cobra.MinimumNArgs(totalArgs - 1) + messageBinder.mandatoryArgUntil = totalArgs - 1 case messageBinder.hasOptional: - messageBinder.CobraArgs = cobra.RangeArgs(positionalArgsLen-1, positionalArgsLen) - messageBinder.mandatoryArgUntil = positionalArgsLen - 1 + messageBinder.CobraArgs = cobra.RangeArgs(totalArgs-1, totalArgs) + messageBinder.mandatoryArgUntil = totalArgs - 1 default: - messageBinder.CobraArgs = cobra.ExactArgs(positionalArgsLen) - messageBinder.mandatoryArgUntil = positionalArgsLen + messageBinder.CobraArgs = cobra.ExactArgs(totalArgs) + messageBinder.mandatoryArgUntil = totalArgs } // validate flag options @@ -273,6 +268,56 @@ func (b *Builder) addMessageFlags(ctx *context.Context, flagSet *pflag.FlagSet, return messageBinder, nil } +// addFlattenFieldBindingToArgs recursively adds field bindings for nested message fields to the message binder. +// It takes a slice of field names representing the path to the target field, where each element is a field name +// in the nested message structure. For example, ["foo", "bar", "baz"] would bind the "baz" field inside the "bar" +// message which is inside the "foo" message. +func (b *Builder) addFlattenFieldBindingToArgs(ctx *context.Context, path string, s []string, msg protoreflect.MessageType, messageBinder *MessageBinder) error { + fields := msg.Descriptor().Fields() + if len(s) == 1 { + f, err := b.addFieldBindingToArgs(ctx, messageBinder, protoreflect.Name(s[0]), fields) + if err != nil { + return err + } + f.path = path + messageBinder.positionalArgs = append(messageBinder.positionalArgs, f) + return nil + } + fd := fields.ByName(protoreflect.Name(s[0])) + var innerMsg protoreflect.MessageType + if fd.IsList() { + innerMsg = msg.New().Get(fd).List().NewElement().Message().Type() + } else { + innerMsg = msg.New().Get(fd).Message().Type() + } + return b.addFlattenFieldBindingToArgs(ctx, path, s[1:], innerMsg, messageBinder) +} + +// addFieldBindingToArgs adds a fieldBinding for a positional argument to the message binder. +// The fieldBinding is appended to the positional arguments list in the message binder. +func (b *Builder) addFieldBindingToArgs(ctx *context.Context, messageBinder *MessageBinder, name protoreflect.Name, fields protoreflect.FieldDescriptors) (fieldBinding, error) { + field := fields.ByName(name) + if field == nil { + return fieldBinding{}, fmt.Errorf("can't find field %s", name) // TODO: it will improve error if msg.FullName() was included.` + } + + _, hasValue, err := b.addFieldFlag( + ctx, + messageBinder.positionalFlagSet, + field, + &autocliv1.FlagOptions{Name: fmt.Sprintf("%d", len(messageBinder.positionalArgs))}, + namingOptions{}, + ) + if err != nil { + return fieldBinding{}, err + } + + return fieldBinding{ + field: field, + hasValue: hasValue, + }, nil +} + // bindPageRequest create a flag for pagination func (b *Builder) bindPageRequest(ctx *context.Context, flagSet *pflag.FlagSet, field protoreflect.FieldDescriptor) (HasValue, error) { return b.addMessageFlags( diff --git a/client/v2/autocli/flag/messager_binder.go b/client/v2/autocli/flag/messager_binder.go index 46a28baa285a..08a270e792a3 100644 --- a/client/v2/autocli/flag/messager_binder.go +++ b/client/v2/autocli/flag/messager_binder.go @@ -2,6 +2,7 @@ package flag import ( "fmt" + "strings" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -65,10 +66,18 @@ func (m MessageBinder) Bind(msg protoreflect.Message, positionalArgs []string) e } } + msgName := msg.Descriptor().Name() // bind positional arg values to the message for _, arg := range m.positionalArgs { - if err := arg.bind(msg); err != nil { - return err + if msgName == arg.field.Parent().Name() { + if err := arg.bind(msg); err != nil { + return err + } + } else { + s := strings.Split(arg.path, ".") + if err := m.bindNestedField(msg, arg, s); err != nil { + return err + } } } @@ -82,6 +91,39 @@ func (m MessageBinder) Bind(msg protoreflect.Message, positionalArgs []string) e return nil } +// bindNestedField binds a field value to a nested message field. It handles cases where the field +// belongs to a nested message type by recursively traversing the message structure. +func (m *MessageBinder) bindNestedField(msg protoreflect.Message, arg fieldBinding, path []string) error { + if len(path) == 1 { + return arg.bind(msg) + } + + name := protoreflect.Name(path[0]) + fd := msg.Descriptor().Fields().ByName(name) + if fd == nil { + return fmt.Errorf("field %q not found", path[0]) + } + + var innerMsg protoreflect.Message + if fd.IsList() { + if msg.Get(fd).List().Len() == 0 { + l := msg.Mutable(fd).List() + elem := l.NewElement().Message().New() + l.Append(protoreflect.ValueOfMessage(elem)) + msg.Set(msg.Descriptor().Fields().ByName(name), protoreflect.ValueOfList(l)) + } + innerMsg = msg.Get(fd).List().Get(0).Message() + } else { + innerMsgValue := msg.Get(fd) + if !innerMsgValue.Message().IsValid() { + msg.Set(msg.Descriptor().Fields().ByName(name), protoreflect.ValueOfMessage(innerMsgValue.Message().New())) + } + innerMsg = msg.Get(msg.Descriptor().Fields().ByName(name)).Message() + } + + return m.bindNestedField(innerMsg, arg, path[1:]) +} + // Get calls BuildMessage and wraps the result in a protoreflect.Value. func (m MessageBinder) Get(protoreflect.Value) (protoreflect.Value, error) { msg, err := m.BuildMessage(nil) @@ -91,6 +133,7 @@ func (m MessageBinder) Get(protoreflect.Value) (protoreflect.Value, error) { type fieldBinding struct { hasValue HasValue field protoreflect.FieldDescriptor + path string } func (f fieldBinding) bind(msg protoreflect.Message) error { diff --git a/client/v2/autocli/msg_test.go b/client/v2/autocli/msg_test.go index a86fc8ebcca7..11ea45a6f6c7 100644 --- a/client/v2/autocli/msg_test.go +++ b/client/v2/autocli/msg_test.go @@ -128,6 +128,33 @@ func TestMsg(t *testing.T) { assertNormalizedJSONEqual(t, out.Bytes(), goldenLoad(t, "msg-output.golden")) } +func TestMsgWithFlattenFields(t *testing.T) { + fixture := initFixture(t) + + out, err := runCmd(fixture, buildCustomModuleMsgCommand(&autocliv1.ServiceCommandDescriptor{ + Service: bankv1beta1.Msg_ServiceDesc.ServiceName, + RpcCommandOptions: []*autocliv1.RpcCommandOptions{ + { + RpcMethod: "UpdateParams", + PositionalArgs: []*autocliv1.PositionalArgDescriptor{ + {ProtoField: "authority"}, + {ProtoField: "params.send_enabled.denom"}, + {ProtoField: "params.send_enabled.enabled"}, + {ProtoField: "params.default_send_enabled"}, + }, + }, + }, + EnhanceCustomCommand: true, + }), "update-params", + "cosmos1y74p8wyy4enfhfn342njve6cjmj5c8dtl6emdk", "stake", "true", "true", + "--generate-only", + "--output", "json", + "--chain-id", fixture.chainID, + ) + assert.NilError(t, err) + assertNormalizedJSONEqual(t, out.Bytes(), goldenLoad(t, "flatten-output.golden")) +} + func goldenLoad(t *testing.T, filename string) []byte { t.Helper() content, err := os.ReadFile(filepath.Join("testdata", filename)) diff --git a/client/v2/autocli/testdata/flatten-output.golden b/client/v2/autocli/testdata/flatten-output.golden new file mode 100644 index 000000000000..59b3c668d4c2 --- /dev/null +++ b/client/v2/autocli/testdata/flatten-output.golden @@ -0,0 +1 @@ +{"body":{"messages":[{"@type":"/cosmos.bank.v1beta1.MsgUpdateParams","authority":"cosmos1y74p8wyy4enfhfn342njve6cjmj5c8dtl6emdk","params":{"send_enabled":[{"denom":"stake","enabled":true}],"default_send_enabled":true}}],"memo":"","timeout_height":"0","unordered":false,"timeout_timestamp":"1970-01-01T00:00:00Z","extension_options":[],"non_critical_extension_options":[]},"auth_info":{"signer_infos":[],"fee":{"amount":[],"gas_limit":"200000","payer":"","granter":""},"tip":null},"signatures":[]} diff --git a/proto/cosmos/autocli/v1/options.proto b/proto/cosmos/autocli/v1/options.proto index 8160a6e0276e..af9783a920f1 100644 --- a/proto/cosmos/autocli/v1/options.proto +++ b/proto/cosmos/autocli/v1/options.proto @@ -3,6 +3,7 @@ syntax = "proto3"; package cosmos.autocli.v1; import "cosmos_proto/cosmos.proto"; + option go_package = "cosmossdk.io/api/cosmos/base/cli/v1;cliv1"; // ModuleOptions describes the CLI options for a Cosmos SDK module. @@ -16,7 +17,6 @@ message ModuleOptions { // ServiceCommandDescriptor describes a CLI command based on a protobuf service. message ServiceCommandDescriptor { - // service is the fully qualified name of the protobuf service to build // the command from. It can be left empty if sub_commands are used instead // which may be the case if a module provides multiple tx and/or query services. @@ -103,7 +103,6 @@ message RpcCommandOptions { // kebab-case name of the field. Fields can be turned into positional arguments // instead by using RpcCommandOptions.positional_args. message FlagOptions { - // name is an alternate name to use for the field flag. string name = 1; diff --git a/tests/systemtests/circuit_test.go b/tests/systemtests/circuit_test.go index 708067b40435..64e0db2dc896 100644 --- a/tests/systemtests/circuit_test.go +++ b/tests/systemtests/circuit_test.go @@ -101,28 +101,28 @@ func TestCircuitCommands(t *testing.T) { authorizeTestCases := []struct { name string address string - level int + level string limtTypeURLs []string expPermission string }{ { "set new super admin", superAdmin2, - 3, + "super-admin", []string{}, "LEVEL_SUPER_ADMIN", }, { "set all msgs level to address", allMsgsAcc, - 2, + "all-msgs", []string{}, "LEVEL_ALL_MSGS", }, { "set some msgs level to address", someMsgsAcc, - 1, + "some-msgs", someMsgs, "LEVEL_SOME_MSGS", }, @@ -130,11 +130,7 @@ func TestCircuitCommands(t *testing.T) { for _, tc := range authorizeTestCases { t.Run(tc.name, func(t *testing.T) { - permissionJSON := fmt.Sprintf(`{"level":%d,"limit_type_urls":[]}`, tc.level) - if len(tc.limtTypeURLs) != 0 { - permissionJSON = fmt.Sprintf(`{"level":%d,"limit_type_urls":["%s"]}`, tc.level, strings.Join(tc.limtTypeURLs[:], `","`)) - } - rsp = cli.RunAndWait("tx", "circuit", "authorize", tc.address, permissionJSON, "--from="+superAdmin) + rsp = cli.RunAndWait("tx", "circuit", "authorize", tc.address, tc.level, strings.Join(tc.limtTypeURLs[:], `,`), "--from="+superAdmin) systest.RequireTxSuccess(t, rsp) // query account permissions diff --git a/x/circuit/autocli.go b/x/circuit/autocli.go index a1cb8ada74e6..439e1eec3f9f 100644 --- a/x/circuit/autocli.go +++ b/x/circuit/autocli.go @@ -37,16 +37,14 @@ func (am AppModule) AutoCLIOptions() *autocliv1.ModuleOptions { RpcCommandOptions: []*autocliv1.RpcCommandOptions{ { RpcMethod: "AuthorizeCircuitBreaker", - Use: "authorize --from ", + Use: "authorize --from ", Short: "Authorize an account to trip the circuit breaker.", - Long: `Authorize an account to trip the circuit breaker. -"SOME_MSGS" = 1, -"ALL_MSGS" = 2, -"SUPER_ADMIN" = 3,`, - Example: fmt.Sprintf(`%s tx circuit authorize [address] '{"level":1,"limit_type_urls":["/cosmos.bank.v1beta1.MsgSend", "/cosmos.bank.v1beta1.MsgMultiSend"]}'"`, version.AppName), + Long: `Authorize an account to trip the circuit breaker. Level can be: some-msgs, all-msgs or super-admin.`, + Example: fmt.Sprintf(`%s tx circuit authorize [address] super-admin "/cosmos.bank.v1beta1.MsgSend,/cosmos.bank.v1beta1.MsgMultiSend"`, version.AppName), PositionalArgs: []*autocliv1.PositionalArgDescriptor{ {ProtoField: "grantee"}, - {ProtoField: "permissions"}, // TODO(@julienrbrt) Support flattening msg for setting each field as a positional arg + {ProtoField: "permissions.level"}, + {ProtoField: "permissions.limit_type_urls"}, }, }, {