From 7917a92dea156169d2304b0215768672d61feaf8 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 4 Mar 2024 20:41:22 +0100 Subject: [PATCH] refactor(client/v2): get keyring from context --- client/v2/CHANGELOG.md | 4 ++- client/v2/README.md | 45 +++++++++++++++---------------- client/v2/autocli/app.go | 5 ---- client/v2/autocli/common.go | 13 +++++++-- client/v2/autocli/common_test.go | 4 --- client/v2/autocli/flag/address.go | 41 +++++++++++++++++++--------- client/v2/autocli/flag/builder.go | 8 ------ client/v2/autocli/msg.go | 3 +-- client/v2/autocli/query_test.go | 2 +- simapp/simd/cmd/root.go | 2 -- simapp/simd/cmd/root_v2.go | 12 --------- 11 files changed, 65 insertions(+), 74 deletions(-) diff --git a/client/v2/CHANGELOG.md b/client/v2/CHANGELOG.md index 3d2b21baa384..78f5ab81e4a4 100644 --- a/client/v2/CHANGELOG.md +++ b/client/v2/CHANGELOG.md @@ -46,8 +46,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* []() Use keyring from command context. * [#19060](https://github.com/cosmos/cosmos-sdk/pull/19060) Use client context from root (or enhanced) command in autocli commands. - * Note, the given command must have a `client.Context` in its context. + * Note, the given command must have a `client.Context` in its context. * [#19216](https://github.com/cosmos/cosmos-sdk/pull/19216) Do not overwrite TxConfig, use directly the one provided in context. TxConfig should always be set in the `client.Context` in `root.go` of an app. ### Bug Fixes @@ -56,6 +57,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking Changes +* []() Remove keyring from autocli options and flag builder options. * [#17709](https://github.com/cosmos/cosmos-sdk/pull/17709) Address codecs have been removed from `autocli.AppOptions` and `flag.Builder`. Instead client/v2 uses the address codecs present in the context (introduced in [#17503](https://github.com/cosmos/cosmos-sdk/pull/17503)). ## [v2.0.0-beta.1] - 2023-11-07 diff --git a/client/v2/README.md b/client/v2/README.md index 895f0a1c5ca1..af5481ab784a 100644 --- a/client/v2/README.md +++ b/client/v2/README.md @@ -75,10 +75,10 @@ if err := rootCmd.Execute(); err != nil { ### Keyring -`autocli` uses a keyring for key name resolving and signing transactions. Providing a keyring is optional, but if you want to use the `autocli` generated commands to sign transactions, you must provide a keyring. +`autocli` uses a keyring for key name resolving names and signing transactions. :::tip -This provides a better UX as it allows to resolve key names directly from the keyring in all transactions and commands. +AutoCLI provides a better UX than normal cli as it allows to resolve key names directly from the keyring in all transactions and commands. ```sh q bank balances alice @@ -87,8 +87,9 @@ This provides a better UX as it allows to resolve key names directly from the ke ::: -The keyring to be provided to `client/v2` must match the `client/v2` keyring interface. -The keyring should be provided in the `appOptions` struct as follows, and can be gotten from the client context: +The keyring used for resolving names and signing transactions is provided via the `client.Context`. +The keyring is then converted to the `client/v2/autocli/keyring` interface. +If no keyring is provided, the `autocli` generated command will not be able to sign transactions, but will still be able to query the chain. :::tip The Cosmos SDK keyring and Hubl keyring both implement the `client/v2/autocli/keyring` interface, thanks to the following wrapper: @@ -99,18 +100,6 @@ keyring.NewAutoCLIKeyring(kb) ::: -:::warning -When using AutoCLI the keyring will only be created once and before any command flag parsing. -::: - -```go -// Set the keyring in the appOptions -appOptions.Keyring = keyring - -err := autoCliOpts.EnhanceRootCommand(rootCmd) -... -``` - ## Signing `autocli` supports signing transactions with the keyring. @@ -220,14 +209,16 @@ For more information on `hubl`, including how to configure a new chain and query # Off-Chain Off-chain functionalities allow you to sign and verify files with two commands: -+ `sign-file` for signing a file. -+ `verify-file` for verifying a previously signed file. + +* `sign-file` for signing a file. +* `verify-file` for verifying a previously signed file. Signing a file will result in a Tx with a `MsgSignArbitraryData` as described in the [Off-chain CIP](https://github.com/cosmos/cips/blob/main/cips/cip-X.md). ## Sign a file To sign a file `sign-file` command offers some helpful flags: + ```text --encoding string Choose an encoding method for the file content to be added as msg data (no-encoding|base64|hex) (default "no-encoding") --indent string Choose an indent for the tx (default " ") @@ -237,8 +228,10 @@ To sign a file `sign-file` command offers some helpful flags: ``` The `encoding` flag lets you choose how the contents of the file should be encoded. For example: -+ `simd off-chain sign-file alice myFile.json` - + ```json + +* `simd off-chain sign-file alice myFile.json` + + * ```json { "@type": "/offchain.MsgSignArbitraryData", "appDomain": "simd", @@ -246,8 +239,10 @@ The `encoding` flag lets you choose how the contents of the file should be encod "data": "Hello World!\n" } ``` -+ `simd off-chain sign-file alice myFile.json --encoding base64` - + ```json + +* `simd off-chain sign-file alice myFile.json --encoding base64` + + * ```json { "@type": "/offchain.MsgSignArbitraryData", "appDomain": "simd", @@ -255,8 +250,10 @@ The `encoding` flag lets you choose how the contents of the file should be encod "data": "SGVsbG8gV29ybGQhCg==" } ``` -+ `simd off-chain sign-file alice myFile.json --encoding hex` - + ```json + +* `simd off-chain sign-file alice myFile.json --encoding hex` + + * ```json { "@type": "/offchain.MsgSignArbitraryData", "appDomain": "simd", diff --git a/client/v2/autocli/app.go b/client/v2/autocli/app.go index 9f3207e53afa..ec793eee67fa 100644 --- a/client/v2/autocli/app.go +++ b/client/v2/autocli/app.go @@ -7,7 +7,6 @@ import ( autocliv1 "cosmossdk.io/api/cosmos/autocli/v1" "cosmossdk.io/client/v2/autocli/flag" - "cosmossdk.io/client/v2/autocli/keyring" "cosmossdk.io/core/appmodule" "cosmossdk.io/depinject" @@ -35,9 +34,6 @@ type AppOptions struct { // module or need to be improved. ModuleOptions map[string]*autocliv1.ModuleOptions `optional:"true"` - // Keyring is the keyring to use for client/v2. - Keyring keyring.Keyring `optional:"true"` - // ClientCtx contains the necessary information needed to execute the commands. ClientCtx client.Context } @@ -62,7 +58,6 @@ func (appOptions AppOptions) EnhanceRootCommand(rootCmd *cobra.Command) error { Builder: flag.Builder{ TypeResolver: protoregistry.GlobalTypes, FileResolver: appOptions.ClientCtx.InterfaceRegistry, - Keyring: appOptions.Keyring, AddressCodec: appOptions.ClientCtx.AddressCodec, ValidatorAddressCodec: appOptions.ClientCtx.ValidatorAddressCodec, ConsensusAddressCodec: appOptions.ClientCtx.ConsensusAddressCodec, diff --git a/client/v2/autocli/common.go b/client/v2/autocli/common.go index 7c46025fbfbe..c8a9f6e5cc13 100644 --- a/client/v2/autocli/common.go +++ b/client/v2/autocli/common.go @@ -1,10 +1,12 @@ package autocli import ( + "context" "fmt" "strings" "github.com/spf13/cobra" + "github.com/spf13/pflag" "google.golang.org/protobuf/reflect/protoreflect" "sigs.k8s.io/yaml" @@ -56,14 +58,21 @@ func (b *Builder) buildMethodCommandCommon(descriptor protoreflect.MethodDescrip Version: options.Version, } - binder, err := b.AddMessageFlags(cmd.Context(), cmd.Flags(), inputType, options) + // the binder is only used for getting the number of args required + // it doesn't have access to the command context, so it isn't useful. + binderArgs, err := b.AddMessageFlags(context.Background(), &pflag.FlagSet{}, inputType, options) if err != nil { return nil, err } - cmd.Args = binder.CobraArgs + cmd.Args = binderArgs.CobraArgs cmd.RunE = func(cmd *cobra.Command, args []string) error { + binder, err := b.AddMessageFlags(cmd.Context(), cmd.Flags(), inputType, options) + if err != nil { + return err + } + input, err := binder.BuildMessage(args) if err != nil { return err diff --git a/client/v2/autocli/common_test.go b/client/v2/autocli/common_test.go index c75eaa5b17a4..bdeb23a79a38 100644 --- a/client/v2/autocli/common_test.go +++ b/client/v2/autocli/common_test.go @@ -55,9 +55,6 @@ func initFixture(t *testing.T) *fixture { kr, err := sdkkeyring.New(sdk.KeyringServiceName(), sdkkeyring.BackendMemory, home, nil, encodingConfig.Codec) assert.NilError(t, err) - akr, err := sdkkeyring.NewAutoCLIKeyring(kr) - assert.NilError(t, err) - interfaceRegistry := encodingConfig.Codec.InterfaceRegistry() banktypes.RegisterInterfaces(interfaceRegistry) @@ -82,7 +79,6 @@ func initFixture(t *testing.T) *fixture { AddressCodec: clientCtx.AddressCodec, ValidatorAddressCodec: clientCtx.ValidatorAddressCodec, ConsensusAddressCodec: clientCtx.ConsensusAddressCodec, - Keyring: akr, }, GetClientConn: func(*cobra.Command) (grpc.ClientConnInterface, error) { return conn, nil diff --git a/client/v2/autocli/flag/address.go b/client/v2/autocli/flag/address.go index 99d1a9c3284b..f8a97f94b4ca 100644 --- a/client/v2/autocli/flag/address.go +++ b/client/v2/autocli/flag/address.go @@ -9,16 +9,18 @@ import ( "cosmossdk.io/client/v2/autocli/keyring" "cosmossdk.io/core/address" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/codec/types" cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" + sdkkeyring "github.com/cosmos/cosmos-sdk/crypto/keyring" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" ) type addressStringType struct{} -func (a addressStringType) NewValue(_ context.Context, b *Builder) Value { - return &addressValue{addressCodec: b.AddressCodec, keyring: b.Keyring} +func (a addressStringType) NewValue(ctx context.Context, b *Builder) Value { + return &addressValue{addressCodec: b.AddressCodec, keyring: getKeyringFromCtx(ctx)} } func (a addressStringType) DefaultValue() string { @@ -27,8 +29,8 @@ func (a addressStringType) DefaultValue() string { type validatorAddressStringType struct{} -func (a validatorAddressStringType) NewValue(_ context.Context, b *Builder) Value { - return &addressValue{addressCodec: b.ValidatorAddressCodec, keyring: b.Keyring} +func (a validatorAddressStringType) NewValue(ctx context.Context, b *Builder) Value { + return &addressValue{addressCodec: b.ValidatorAddressCodec, keyring: getKeyringFromCtx(ctx)} } func (a validatorAddressStringType) DefaultValue() string { @@ -62,9 +64,11 @@ func (a *addressValue) Set(s string) error { return nil } - // failed all validation, just accept the input. - // TODO(@julienrbrt), for final client/v2 2.0.0 revert the logic and - // do a better keyring instantiation. + _, err = a.addressCodec.StringToBytes(s) + if err != nil { + return fmt.Errorf("invalid account address or key name: %w", err) + } + a.value = s return nil @@ -80,7 +84,7 @@ func (a consensusAddressStringType) NewValue(ctx context.Context, b *Builder) Va return &consensusAddressValue{ addressValue: addressValue{ addressCodec: b.ConsensusAddressCodec, - keyring: b.Keyring, + keyring: getKeyringFromCtx(ctx), }, } } @@ -127,11 +131,7 @@ func (a *consensusAddressValue) Set(s string) error { var pk cryptotypes.PubKey err2 := cdc.UnmarshalInterfaceJSON([]byte(s), &pk) if err2 != nil { - // failed all validation, just accept the input. - // TODO(@julienrbrt), for final client/v2 2.0.0 revert the logic and - // do a better keyring instantiation. - a.value = s - return nil + return fmt.Errorf("input isn't a pubkey %w or is an invalid account address: %w", err, err2) } a.value, err = a.addressCodec.BytesToString(pk.Address()) @@ -141,3 +141,18 @@ func (a *consensusAddressValue) Set(s string) error { return nil } + +func getKeyringFromCtx(ctx context.Context) keyring.Keyring { + if ctx != nil { + if clientCtx := ctx.Value(client.ClientContextKey); clientCtx != nil { + keyring, err := sdkkeyring.NewAutoCLIKeyring(clientCtx.(*client.Context).Keyring) + if err != nil { + panic(fmt.Errorf("failed to create keyring: %w", err)) + } + + return keyring + } + } + + return keyring.NoKeyring{} +} diff --git a/client/v2/autocli/flag/builder.go b/client/v2/autocli/flag/builder.go index 2fe5eb72424f..2b32bfb99460 100644 --- a/client/v2/autocli/flag/builder.go +++ b/client/v2/autocli/flag/builder.go @@ -16,7 +16,6 @@ import ( autocliv1 "cosmossdk.io/api/cosmos/autocli/v1" msgv1 "cosmossdk.io/api/cosmos/msg/v1" - "cosmossdk.io/client/v2/autocli/keyring" "cosmossdk.io/client/v2/internal/flags" "cosmossdk.io/client/v2/internal/util" "cosmossdk.io/core/address" @@ -50,9 +49,6 @@ type Builder struct { messageFlagTypes map[protoreflect.FullName]Type scalarFlagTypes map[string]Type - // Keyring is the keyring to use for client/v2. - Keyring keyring.Keyring - // Address Codecs are the address codecs to use for client/v2. AddressCodec address.Codec ValidatorAddressCodec runtime.ValidatorAddressCodec @@ -92,10 +88,6 @@ func (b *Builder) ValidateAndComplete() error { return errors.New("consensus address codec is required in flag builder") } - if b.Keyring == nil { - b.Keyring = keyring.NoKeyring{} - } - if b.TypeResolver == nil { return errors.New("type resolver is required in flag builder") } diff --git a/client/v2/autocli/msg.go b/client/v2/autocli/msg.go index e9f07d668fe7..9198fdc17ad9 100644 --- a/client/v2/autocli/msg.go +++ b/client/v2/autocli/msg.go @@ -132,7 +132,7 @@ func (b *Builder) BuildMsgMethodCommand(descriptor protoreflect.MethodDescriptor // handle gov proposals commands skipProposal, _ := cmd.Flags().GetBool(flags.FlagNoProposal) if options.GovProposal && !skipProposal { - return b.handleGovProposal(options, cmd, input, clientCtx, addressCodec, fd) + return b.handleGovProposal(cmd, input, clientCtx, addressCodec, fd) } // set signer to signer field if empty @@ -191,7 +191,6 @@ func (b *Builder) BuildMsgMethodCommand(descriptor protoreflect.MethodDescriptor // handleGovProposal sets the authority field of the message to the gov module address and creates a gov proposal. func (b *Builder) handleGovProposal( - options *autocliv1.RpcCommandOptions, cmd *cobra.Command, input protoreflect.Message, clientCtx client.Context, diff --git a/client/v2/autocli/query_test.go b/client/v2/autocli/query_test.go index 07d6354928e0..c949ea021a0a 100644 --- a/client/v2/autocli/query_test.go +++ b/client/v2/autocli/query_test.go @@ -684,7 +684,7 @@ func TestNotFoundErrorsQuery(t *testing.T) { b.AddQueryConnFlags = nil b.AddTxConnFlags = nil - buildModuleQueryCommand := func(moduleName string, cmdDescriptor *autocliv1.ServiceCommandDescriptor) (*cobra.Command, error) { + buildModuleQueryCommand := func(_ string, cmdDescriptor *autocliv1.ServiceCommandDescriptor) (*cobra.Command, error) { cmd := topLevelCmd(context.Background(), "query", "Querying subcommands") err := b.AddMsgServiceCommands(cmd, cmdDescriptor) return cmd, err diff --git a/simapp/simd/cmd/root.go b/simapp/simd/cmd/root.go index f15e3d0c4966..179102836618 100644 --- a/simapp/simd/cmd/root.go +++ b/simapp/simd/cmd/root.go @@ -18,7 +18,6 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/config" addresscodec "github.com/cosmos/cosmos-sdk/codec/address" - "github.com/cosmos/cosmos-sdk/crypto/keyring" "github.com/cosmos/cosmos-sdk/server" simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" sdk "github.com/cosmos/cosmos-sdk/types" @@ -114,7 +113,6 @@ func NewRootCmd() *cobra.Command { } autoCliOpts := tempApp.AutoCliOpts() - autoCliOpts.Keyring, _ = keyring.NewAutoCLIKeyring(initClientCtx.Keyring) autoCliOpts.ClientCtx = initClientCtx if err := autoCliOpts.EnhanceRootCommand(rootCmd); err != nil { diff --git a/simapp/simd/cmd/root_v2.go b/simapp/simd/cmd/root_v2.go index edbf0ea2cf50..353c6fdaa32d 100644 --- a/simapp/simd/cmd/root_v2.go +++ b/simapp/simd/cmd/root_v2.go @@ -8,7 +8,6 @@ import ( "github.com/spf13/cobra" "cosmossdk.io/client/v2/autocli" - clientv2keyring "cosmossdk.io/client/v2/autocli/keyring" "cosmossdk.io/core/address" "cosmossdk.io/depinject" "cosmossdk.io/log" @@ -21,7 +20,6 @@ import ( "github.com/cosmos/cosmos-sdk/client/config" "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" - "github.com/cosmos/cosmos-sdk/crypto/keyring" "github.com/cosmos/cosmos-sdk/runtime" "github.com/cosmos/cosmos-sdk/server" simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" @@ -44,7 +42,6 @@ func NewRootCmd() *cobra.Command { ), depinject.Provide( ProvideClientContext, - ProvideKeyring, ), ), &autoCliOpts, @@ -135,12 +132,3 @@ func ProvideClientContext( return clientCtx } - -func ProvideKeyring(clientCtx client.Context, addressCodec address.Codec) (clientv2keyring.Keyring, error) { - kb, err := client.NewKeyringFromBackend(clientCtx, clientCtx.Keyring.Backend()) - if err != nil { - return nil, err - } - - return keyring.NewAutoCLIKeyring(kb) -}