Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(client/v2): signing #17913

Merged
merged 3 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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`
Expand Down
4 changes: 3 additions & 1 deletion client/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)")
Expand Down
7 changes: 6 additions & 1 deletion client/tx/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion client/tx/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion client/v2/Makefile
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
codegen:
@(cd internal; buf generate)
@(cd internal; buf generate)
25 changes: 21 additions & 4 deletions client/v2/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
33 changes: 18 additions & 15 deletions client/v2/autocli/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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

Expand All @@ -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,
Expand All @@ -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)
Expand Down
49 changes: 21 additions & 28 deletions client/v2/autocli/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
34 changes: 32 additions & 2 deletions client/v2/autocli/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add an issue for this todo

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

Expand Down
Loading