feat(client/v2): signing (#17913)

CHANGELOG.md

@@ -49,6 +49,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+* (keyring) [#17913](https://github.com/cosmos/cosmos-sdk/pull/17913) Add `NewAutoCLIKeyring` for creating an AutoCLI keyring from a SDK keyring.
+* (codec) [#17913](https://github.com/cosmos/cosmos-sdk/pull/17913) `codectypes.NewAnyWithValue` supports proto v2 messages.
 * (client) [#17503](https://github.com/cosmos/cosmos-sdk/pull/17503) Add `client.Context{}.WithAddressCodec`, `WithValidatorAddressCodec`, `WithConsensusAddressCodec` to provide address codecs to the client context. See the [UPGRADING.md](./UPGRADING.md) for more details.
 * (crypto/keyring) [#17503](https://github.com/cosmos/cosmos-sdk/pull/17503) Simplify keyring interfaces to use `[]byte` instead of `sdk.Address` for addresses.
 * (all) [#16537](https://github.com/cosmos/cosmos-sdk/pull/16537) Properly propagated `fmt.Errorf` errors and using `errors.New` where appropriate.
@@ -65,7 +67,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (x/distribution) [#17657](https://github.com/cosmos/cosmos-sdk/pull/17657) The `FundCommunityPool` and `DistributeFromFeePool` keeper methods are now removed from x/distribution.
 * (x/distribution) [#17657](https://github.com/cosmos/cosmos-sdk/pull/17657) The distribution module keeper now takes a new argument `PoolKeeper` in addition.
 * (app) [#17838](https://github.com/cosmos/cosmos-sdk/pull/17838) Params module was removed from simapp and all imports of the params module removed throughout the repo. 
-    * The Cosmos SDK has migrated aay from using params, if you're app still uses it, then you can leave it plugged into your app
+    * The Cosmos SDK has migrated away from using params, if your app still uses it, then you can leave it plugged into your app
 * (x/staking) [#17778](https://github.com/cosmos/cosmos-sdk/pull/17778) Use collections for `Params`
     * remove from `Keeper`: `GetParams`, `SetParams`
 * (types/simulation) [#17737](https://github.com/cosmos/cosmos-sdk/pull/17737) Remove unused parameter from `RandomFees`

client/flags/flags.go

@@ -117,7 +117,9 @@ func AddQueryFlagsToCmd(cmd *cobra.Command) {
 func AddTxFlagsToCmd(cmd *cobra.Command) {
 	f := cmd.Flags()
 	f.StringP(FlagOutput, "o", OutputFormatJSON, "Output format (text|json)")
-	f.String(FlagFrom, "", "Name or address of private key with which to sign")
+	if cmd.Flag(FlagFrom) == nil { // avoid flag redefinition when it's already been added by AutoCLI
+		f.String(FlagFrom, "", "Name or address of private key with which to sign")
+	}
 	f.Uint64P(FlagAccountNumber, "a", 0, "The account number of the signing account (offline mode only)")
 	f.Uint64P(FlagSequence, "s", 0, "The sequence number of the signing account (offline mode only)")
 	f.String(FlagNote, "", "Note to add a description to the transaction (previously --memo)")

client/tx/factory.go

@@ -419,7 +419,12 @@ func (f Factory) BuildSimTx(msgs ...sdk.Msg) ([]byte, error) {
 		return nil, err
 	}
 
-	return f.txConfig.TxEncoder()(txb.GetTx())
+	encoder := f.txConfig.TxEncoder()
+	if encoder == nil {
+		return nil, fmt.Errorf("cannot simulate tx: tx encoder is nil")
+	}
+
+	return encoder(txb.GetTx())
 }
 
 // getSimPK gets the public key to use for building a simulation tx.

client/tx/tx.go

@@ -107,7 +107,7 @@ func BroadcastTx(clientCtx client.Context, txf Factory, msgs ...sdk.Msg) error {
 
 		txBytes, err := encoder(tx.GetTx())
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to encode transaction: %w", err)
 		}
 
 		if err := clientCtx.PrintRaw(json.RawMessage(txBytes)); err != nil {

client/v2/Makefile

@@ -1,2 +1,2 @@
 codegen:
-	@(cd internal; buf generate)
+	@(cd internal; buf generate)

client/v2/README.md

@@ -75,7 +75,7 @@ if err := rootCmd.Execute(); err != nil {
 
 ### Keyring
 
-`autocli` supports 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 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.
 
 :::tip
 This provides a better UX as it allows to resolve key names directly from the keyring in all transactions and commands.
@@ -87,23 +87,40 @@ 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 Cosmos SDK keyring and Hubl keyring both implement this interface.
+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:
 
+:::tip
+The Cosmos SDK keyring and Hubl keyring both implement the `client/v2/autocli/keyring` interface, thanks to the following wrapper:
+
+```go
+keyring.NewAutoCLIKeyring(kb)
+```
+
+:::
+
 :::warning
 When using AutoCLI the keyring will only be created once and before any command flag parsing.
 :::
 
 ```go
-// Get the keyring from the client context
-keyring := ctx.Keyring
 // Set the keyring in the appOptions
 appOptions.Keyring = keyring
 
 err := autoCliOpts.EnhanceRootCommand(rootCmd)
 ...
 ```
 
+## Signing
+
+`autocli` supports signing transactions with the keyring.
+The [`cosmos.msg.v1.signer` protobuf annotation](https://github.com/cosmos/cosmos-sdk/blob/9dd34510e27376005e7e7ff3628eab9dbc8ad6dc/docs/build/building-modules/05-protobuf-annotations.md#L9) defines the signer field of the message.
+This field is automatically filled when using the `--from` flag or defining the signer as a positional argument.
+
+:::warning
+AutoCLI currently supports only one signer per transaction.
+:::
+
 ## Module Wiring & Customization
 
 The `AutoCLIOptions()` method on your module allows to specify custom commands, sub-commands or flags for each service, as it was a `cobra.Command` instance, within the `RpcCommandOptions` struct. Defining such options will customize the behavior of the `autocli` command generation, which by default generates a command for each method in your gRPC service.

client/v2/autocli/app.go

@@ -11,10 +11,10 @@ import (
 	"cosmossdk.io/client/v2/autocli/keyring"
 	"cosmossdk.io/core/appmodule"
 	"cosmossdk.io/depinject"
-	"cosmossdk.io/log"
 
 	"github.com/cosmos/cosmos-sdk/client"
-	"github.com/cosmos/cosmos-sdk/client/flags"
+	sdkflags "github.com/cosmos/cosmos-sdk/client/flags"
+	"github.com/cosmos/cosmos-sdk/x/auth/tx"
 )
 
 // AppOptions are autocli options for an app. These options can be built via depinject based on an app config. Ex:
@@ -28,9 +28,6 @@ import (
 type AppOptions struct {
 	depinject.In
 
-	// Logger is the logger to use for client/v2.
-	Logger log.Logger
-
 	// Modules are the AppModule implementations for the modules in the app.
 	Modules map[string]appmodule.AppModule
 
@@ -40,11 +37,14 @@ type AppOptions struct {
 	// module or need to be improved.
 	ModuleOptions map[string]*autocliv1.ModuleOptions `optional:"true"`
 
-	// ClientCtx contains the necessary information needed to execute the commands.
-	ClientCtx *client.Context
-
 	// 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
+
+	// TxConfigOptions are the transactions config options.
+	TxConfigOpts tx.ConfigOptions
 }
 
 // EnhanceRootCommand enhances the provided root command with autocli AppOptions,
@@ -64,18 +64,21 @@ type AppOptions struct {
 //	err = autoCliOpts.EnhanceRootCommand(rootCmd)
 func (appOptions AppOptions) EnhanceRootCommand(rootCmd *cobra.Command) error {
 	builder := &Builder{
-		Logger: appOptions.Logger,
 		Builder: flag.Builder{
-			TypeResolver: protoregistry.GlobalTypes,
-			FileResolver: proto.HybridResolver,
-			ClientCtx:    appOptions.ClientCtx,
-			Keyring:      appOptions.Keyring,
+			TypeResolver:          protoregistry.GlobalTypes,
+			FileResolver:          proto.HybridResolver,
+			Keyring:               appOptions.Keyring,
+			AddressCodec:          appOptions.ClientCtx.AddressCodec,
+			ValidatorAddressCodec: appOptions.ClientCtx.ValidatorAddressCodec,
+			ConsensusAddressCodec: appOptions.ClientCtx.ConsensusAddressCodec,
 		},
+		ClientCtx:    appOptions.ClientCtx,
+		TxConfigOpts: appOptions.TxConfigOpts,
 		GetClientConn: func(cmd *cobra.Command) (grpc.ClientConnInterface, error) {
 			return client.GetClientQueryContext(cmd)
 		},
-		AddQueryConnFlags: flags.AddQueryFlagsToCmd,
-		AddTxConnFlags:    flags.AddTxFlagsToCmd,
+		AddQueryConnFlags: sdkflags.AddQueryFlagsToCmd,
+		AddTxConnFlags:    sdkflags.AddTxFlagsToCmd,
 	}
 
 	return appOptions.EnhanceRootCommandWithBuilder(rootCmd, builder)

client/v2/autocli/builder.go

@@ -8,21 +8,26 @@ import (
 
 	"cosmossdk.io/client/v2/autocli/flag"
 	"cosmossdk.io/client/v2/autocli/keyring"
-	"cosmossdk.io/log"
+
+	"github.com/cosmos/cosmos-sdk/client"
+	authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
 )
 
 // Builder manages options for building CLI commands.
 type Builder struct {
 	// flag.Builder embeds the flag builder and its options.
 	flag.Builder
 
-	// Logger is the logger used by the builder.
-	Logger log.Logger
-
 	// GetClientConn specifies how CLI commands will resolve a grpc.ClientConnInterface
 	// from a given context.
 	GetClientConn func(*cobra.Command) (grpc.ClientConnInterface, error)
 
+	// ClientCtx contains the necessary information needed to execute the commands.
+	ClientCtx client.Context
+
+	// TxConfigOptions is required to support sign mode textual
+	TxConfigOpts authtx.ConfigOptions
+
 	// AddQueryConnFlags and AddTxConnFlags are functions that add flags to query and transaction commands
 	AddQueryConnFlags func(*cobra.Command)
 	AddTxConnFlags    func(*cobra.Command)
@@ -33,40 +38,28 @@ type Builder struct {
 // If the Logger is nil, it will be set to a nop logger.
 // If the keyring is nil, it will be set to a no keyring.
 func (b *Builder) ValidateAndComplete() error {
-	if b.Logger == nil {
-		b.Logger = log.NewNopLogger()
-	}
-
-	if b.ClientCtx == nil {
-		return errors.New("client context is required in builder")
-	}
-
-	if b.ClientCtx.AddressCodec == nil {
-		return errors.New("address codec is required in builder")
+	if b.Builder.AddressCodec == nil {
+		return errors.New("address codec is required in flag builder")
 	}
 
-	if b.ClientCtx.ValidatorAddressCodec == nil {
-		return errors.New("validator address codec is required in builder")
+	if b.Builder.ValidatorAddressCodec == nil {
+		return errors.New("validator address codec is required in flag builder")
 	}
 
-	if b.ClientCtx.ConsensusAddressCodec == nil {
-		return errors.New("consensus address codec is required in builder")
+	if b.Builder.ConsensusAddressCodec == nil {
+		return errors.New("consensus address codec is required in flag builder")
 	}
 
-	if b.Keyring == nil {
-		if b.ClientCtx.Keyring != nil {
-			b.Keyring = b.ClientCtx.Keyring
-		} else {
-			b.Keyring = keyring.NoKeyring{}
-		}
+	if b.Builder.Keyring == nil {
+		b.Keyring = keyring.NoKeyring{}
 	}
 
-	if b.TypeResolver == nil {
-		return errors.New("type resolver is required in builder")
+	if b.Builder.TypeResolver == nil {
+		return errors.New("type resolver is required in flag builder")
 	}
 
-	if b.FileResolver == nil {
-		return errors.New("file resolver is required in builder")
+	if b.Builder.FileResolver == nil {
+		return errors.New("file resolver is required in flag builder")
 	}
 
 	return nil

client/v2/autocli/common.go

@@ -10,9 +10,8 @@ import (
 	"sigs.k8s.io/yaml"
 
 	autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"
+	"cosmossdk.io/client/v2/internal/flags"
 	"cosmossdk.io/client/v2/internal/util"
-
-	"github.com/cosmos/cosmos-sdk/client/flags"
 )
 
 type cmdType int
@@ -67,6 +66,37 @@ func (b *Builder) buildMethodCommandCommon(descriptor protoreflect.MethodDescrip
 			return err
 		}
 
+		// signer related logic, triggers only when there is a signer defined
+		if binder.SignerInfo.FieldName != "" {
+			// mark the signer flag as required if defined
+			// TODO(@julienrbrt): UX improvement by only marking the flag as required when there is more than one key in the keyring;
+			// when there is only one key, use that key by default.
+			if binder.SignerInfo.IsFlag {
+				if err := cmd.MarkFlagRequired(binder.SignerInfo.FieldName); err != nil {
+					return err
+				}
+
+				// the client context uses the from flag to determine the signer.
+				// this sets the signer flags to the from flag value if a custom signer flag is set.
+				if binder.SignerInfo.FieldName != flags.FlagFrom {
+					signer, err := cmd.Flags().GetString(binder.SignerInfo.FieldName)
+					if err != nil {
+						return fmt.Errorf("failed to get signer flag: %w", err)
+					}
+
+					if err := cmd.Flags().Set(flags.FlagFrom, signer); err != nil {
+						return err
+					}
+				}
+			} else {
+				// if the signer is not a flag, it is a positional argument
+				// we need to get the correct positional arguments
+				if err := cmd.Flags().Set(flags.FlagFrom, args[binder.SignerInfo.PositionalArgIndex]); err != nil {
+					return err
+				}
+			}
+		}
+
 		return exec(cmd, input)
 	}