refactor(client/v2): get keyring from context

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

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.
@@ -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,26 +228,32 @@ 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",
         "signer":  "cosmos1x33fy6rusfprkntvjsfregss7rvsvyy4lkwrqu",
         "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",
         "signer":  "cosmos1x33fy6rusfprkntvjsfregss7rvsvyy4lkwrqu",
         "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",

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

@@ -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

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

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{}
+}

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")
 	}

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,

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

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 {

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)
-}