From 4215914f6be9e2ae0cf3a2b3231b8fad93ab2b9e Mon Sep 17 00:00:00 2001 From: julienrbrt Date: Thu, 3 Apr 2025 22:02:32 +0200 Subject: [PATCH] feat(client/v2): support definitions of inner messages (cherry-pick #22890) (#24366) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Julián Toledano Co-authored-by: Alex | Interchain Labs --- client/v2/CHANGELOG.md | 4 + client/v2/README.md | 44 ++++++++- client/v2/autocli/flag/builder.go | 95 ++++++++++++++----- client/v2/autocli/flag/messager_binder.go | 47 ++++++++- client/v2/autocli/msg_test.go | 59 +++++++++++- .../v2/autocli/testdata/flatten-output.golden | 1 + simapp/go.mod | 7 +- simapp/go.sum | 2 - x/circuit/autocli.go | 12 +-- 9 files changed, 231 insertions(+), 40 deletions(-) create mode 100644 client/v2/autocli/testdata/flatten-output.golden diff --git a/client/v2/CHANGELOG.md b/client/v2/CHANGELOG.md index 5381274968..e7a884de76 100644 --- a/client/v2/CHANGELOG.md +++ b/client/v2/CHANGELOG.md @@ -40,6 +40,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#24359](https://github.com/cosmos/cosmos-sdk/pull/24359) Support governance proposals. +### Improvements + +* [#22890](https://github.com/cosmos/cosmos-sdk/pull/22890) Added support for flattening inner message fields in autocli as positional arguments. + ### Bug Fixes * (cli) [#24330](https://github.com/cosmos/cosmos-sdk/pull/24330) Use the gogoproto merge registry as a file resolver instead of the interface registry. diff --git a/client/v2/README.md b/client/v2/README.md index a0c7c21067..24bc5ee572 100644 --- a/client/v2/README.md +++ b/client/v2/README.md @@ -159,7 +159,49 @@ Then the command can be used as follows, instead of having to specify the `--add query auth account cosmos1abcd...xyz ``` -### Customising Flag Names +#### 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 1284dae863..5a03914a72 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" @@ -164,38 +165,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()) + 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 + } } - - _, hasValue, err := b.addFieldFlag( - ctx, - messageBinder.positionalFlagSet, - field, - &autocliv1.FlagOptions{Name: fmt.Sprintf("%d", i)}, - namingOptions{}, - ) - 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 @@ -275,6 +270,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 46a28baa28..08a270e792 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 97ba5a73a1..e02f1cde21 100644 --- a/client/v2/autocli/msg_test.go +++ b/client/v2/autocli/msg_test.go @@ -1,8 +1,12 @@ package autocli import ( + "bytes" "context" + "encoding/json" "fmt" + "os" + "path/filepath" "testing" "github.com/spf13/cobra" @@ -117,7 +121,60 @@ func TestMsg(t *testing.T) { "--output", "json", ) assert.NilError(t, err) - golden.Assert(t, out.String(), "msg-output.golden") + 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", "test-chain", + ) + 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)) + assert.NilError(t, err) + return content +} + +func assertNormalizedJSONEqual(t *testing.T, expected, actual []byte) { + t.Helper() + normalizedExpected, err := normalizeJSON(expected) + assert.NilError(t, err) + normalizedActual, err := normalizeJSON(actual) + assert.NilError(t, err) + assert.Equal(t, string(normalizedExpected), string(normalizedActual)) +} + +// normalizeJSON normalizes the JSON content by removing unnecessary white spaces and newlines. +func normalizeJSON(content []byte) ([]byte, error) { + var buf bytes.Buffer + err := json.Compact(&buf, content) + if err != nil { + return nil, err + } + return buf.Bytes(), nil } func TestMsgOptionsError(t *testing.T) { diff --git a/client/v2/autocli/testdata/flatten-output.golden b/client/v2/autocli/testdata/flatten-output.golden new file mode 100644 index 0000000000..5bc7d5a359 --- /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":null,"extension_options":[],"non_critical_extension_options":[]},"auth_info":{"signer_infos":[],"fee":{"amount":[],"gas_limit":"200000","payer":"","granter":""},"tip":null},"signatures":[]} \ No newline at end of file diff --git a/simapp/go.mod b/simapp/go.mod index 7912c2beaf..78035f06d1 100644 --- a/simapp/go.mod +++ b/simapp/go.mod @@ -218,14 +218,17 @@ require ( sigs.k8s.io/yaml v1.4.0 // indirect ) -replace cosmossdk.io/client/v2 => ../client/v2 - // Here are the short-lived replace from the SimApp // Replace here are pending PRs, or version to be tagged // replace ( // // ) +replace ( + cosmossdk.io/client/v2 => ../client/v2 + cosmossdk.io/x/circuit => ../x/circuit +) + // Below are the long-lived replace of the SimApp replace ( // use cosmos fork of keyring diff --git a/simapp/go.sum b/simapp/go.sum index 3ad78be0d7..c09851254f 100644 --- a/simapp/go.sum +++ b/simapp/go.sum @@ -634,8 +634,6 @@ cosmossdk.io/store v1.1.2 h1:3HOZG8+CuThREKv6cn3WSohAc6yccxO3hLzwK6rBC7o= cosmossdk.io/store v1.1.2/go.mod h1:60rAGzTHevGm592kFhiUVkNC9w7gooSEn5iUBPzHQ6A= cosmossdk.io/tools/confix v0.1.2 h1:2hoM1oFCNisd0ltSAAZw2i4ponARPmlhuNu3yy0VwI4= cosmossdk.io/tools/confix v0.1.2/go.mod h1:7XfcbK9sC/KNgVGxgLM0BrFbVcR/+6Dg7MFfpx7duYo= -cosmossdk.io/x/circuit v0.2.0-rc.2 h1:48L/6cH810PJT6j1hV2KfutZtNWJuYpxE30M+ciH7K8= -cosmossdk.io/x/circuit v0.2.0-rc.2/go.mod h1:nzIRWtDL3bz9ZBJ2dN1qLxAw38CT8bk0oIoCTzbhX7w= cosmossdk.io/x/evidence v0.2.0-rc.2 h1:cLTCebjHTye/QoehLM8WJG4xZTFE6ET0WRY108aF/Yk= cosmossdk.io/x/evidence v0.2.0-rc.2/go.mod h1:FH9n6k1oCDoVk4hSd1JOiVpKO3HrFsBAL6kzfrVqagc= cosmossdk.io/x/feegrant v0.2.0-rc.2 h1:yA7a+wF0ax0p5d0L19KYAwaLBLawtc5woZgF0R2zzcA= diff --git a/x/circuit/autocli.go b/x/circuit/autocli.go index 7e9a45b182..ca6ffe5bd0 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 [grantee] [permissions_json] --from [granter]", + Use: "authorize [grantee] [level] [msg_type_urls] --from [granter]", 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 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", Varargs: true}, }, }, {