feat(client/v2): get keyring from context

client/v2/CHANGELOG.md

@@ -36,7 +36,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
 
-<!-- ## [v2.1.0-beta.1] to be tagged after v0.51 final or in SDK agnostic version -->
+<!-- ## [v2.1.0-rc.1] to be tagged after v0.51 final or in SDK agnostic version -->
 
 ### Features
 
@@ -47,6 +47,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+* [#19646](https://github.com/cosmos/cosmos-sdk/pull/19646) Use keyring from command context.
 * [#20083](https://github.com/cosmos/cosmos-sdk/pull/20083) Integrate latest version of cosmos-proto and improve version filtering.
 * [#19618](https://github.com/cosmos/cosmos-sdk/pull/19618) Marshal enum as string in queries.
 * [#19060](https://github.com/cosmos/cosmos-sdk/pull/19060) Use client context from root (or enhanced) command in autocli commands.
@@ -62,6 +63,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### API Breaking Changes
 
+* [#19646](https://github.com/cosmos/cosmos-sdk/pull/19646) Remove keyring from `autocli.AppOptions` 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

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
 <appd> 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.
@@ -255,7 +244,7 @@ The `encoding` flag lets you choose how the contents of the file should be encod
 
 * `simd off-chain sign-file alice myFile.json`
 
-  * ```json
+    * ```json
       {
         "@type":  "/offchain.MsgSignArbitraryData",
         "appDomain":  "simd",
@@ -266,7 +255,7 @@ The `encoding` flag lets you choose how the contents of the file should be encod
 
 * `simd off-chain sign-file alice myFile.json --encoding base64`
 
-  * ```json
+    * ```json
       {
         "@type":  "/offchain.MsgSignArbitraryData",
         "appDomain":  "simd",
@@ -277,7 +266,7 @@ The `encoding` flag lets you choose how the contents of the file should be encod
 
 * `simd off-chain sign-file alice myFile.json --encoding hex`
 
-  * ```json
+    * ```json
         {
           "@type":  "/offchain.MsgSignArbitraryData",
           "appDomain":  "simd",

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,

client/v2/autocli/common.go

@@ -53,14 +53,18 @@ func (b *Builder) buildMethodCommandCommon(descriptor protoreflect.MethodDescrip
 		Version:      options.Version,
 	}
 
-	binder, err := b.AddMessageFlags(cmd.Context(), cmd.Flags(), inputType, options)
+	// we need to use a pointer to the context as the correct context is set in the RunE function
+	// however we need to set the flags before the RunE function is called
+	ctx := cmd.Context()
+	binder, err := b.AddMessageFlags(&ctx, cmd.Flags(), inputType, options)
 	if err != nil {
 		return nil, err
 	}
-
 	cmd.Args = binder.CobraArgs
 
 	cmd.RunE = func(cmd *cobra.Command, args []string) error {
+		ctx = cmd.Context()
+
 		input, err := binder.BuildMessage(args)
 		if err != nil {
 			return err

client/v2/autocli/common_test.go

@@ -56,9 +56,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)
 
@@ -83,7 +80,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

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, ctx: ctx}
 }
 
 func (a addressStringType) DefaultValue() string {
@@ -27,18 +29,19 @@ 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, ctx: ctx}
 }
 
 func (a validatorAddressStringType) DefaultValue() string {
 	return ""
 }
 
 type addressValue struct {
-	value        string
+	ctx          *context.Context
 	addressCodec address.Codec
-	keyring      keyring.Keyring
+
+	value string
 }
 
 func (a addressValue) Get(protoreflect.Value) (protoreflect.Value, error) {
@@ -51,7 +54,9 @@ func (a addressValue) String() string {
 
 // Set implements the flag.Value interface for addressValue.
 func (a *addressValue) Set(s string) error {
-	addr, err := a.keyring.LookupAddressByKeyName(s)
+	// we get the keyring on set, as in NewValue the context is the parent context (before RunE)
+	keyring := getKeyringFromCtx(a.ctx)
+	addr, err := keyring.LookupAddressByKeyName(s)
 	if err == nil {
 		addrStr, err := a.addressCodec.BytesToString(addr)
 		if err != nil {
@@ -62,9 +67,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
@@ -76,11 +83,11 @@ func (a addressValue) Type() string {
 
 type consensusAddressStringType struct{}
 
-func (a consensusAddressStringType) NewValue(ctx context.Context, b *Builder) Value {
+func (a consensusAddressStringType) NewValue(ctx *context.Context, b *Builder) Value {
 	return &consensusAddressValue{
 		addressValue: addressValue{
 			addressCodec: b.ConsensusAddressCodec,
-			keyring:      b.Keyring,
+			ctx:          ctx,
 		},
 	}
 }
@@ -102,7 +109,9 @@ func (a consensusAddressValue) String() string {
 }
 
 func (a *consensusAddressValue) Set(s string) error {
-	addr, err := a.keyring.LookupAddressByKeyName(s)
+	// we get the keyring on set, as in NewValue the context is the parent context (before RunE)
+	keyring := getKeyringFromCtx(a.ctx)
+	addr, err := keyring.LookupAddressByKeyName(s)
 	if err == nil {
 		addrStr, err := a.addressCodec.BytesToString(addr)
 		if err != nil {
@@ -127,11 +136,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 +146,22 @@ func (a *consensusAddressValue) Set(s string) error {
 
 	return nil
 }
+
+func getKeyringFromCtx(ctx *context.Context) keyring.Keyring {
+	if ctx != nil {
+		ctx := *ctx
+
+		if clientCtx := ctx.Value(client.ClientContextKey); clientCtx != nil {
+			k, err := sdkkeyring.NewAutoCLIKeyring(clientCtx.(*client.Context).Keyring)
+			if err != nil {
+				panic(fmt.Errorf("failed to create keyring: %w", err))
+			}
+
+			return k
+		} else if k := ctx.Value(keyring.KeyringContextKey); k != nil {
+			return k.(*keyring.KeyringImpl)
+		}
+	}
+
+	return keyring.NoKeyring{}
+}

client/v2/autocli/flag/binary.go

@@ -14,7 +14,7 @@ type binaryType struct{}
 
 var _ Value = (*fileBinaryValue)(nil)
 
-func (f binaryType) NewValue(context.Context, *Builder) Value {
+func (f binaryType) NewValue(*context.Context, *Builder) Value {
 	return &fileBinaryValue{}
 }