From bb34c42f064d4c0b0e1db0d669c0e2f4139c879d Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 16 Oct 2023 13:08:40 +0200 Subject: [PATCH] feat(client): add default key name (#18101) --- CHANGELOG.md | 1 + client/config/config.go | 25 ++++++------ client/config/toml.go | 2 + client/context.go | 75 +++++++++++++++++++++--------------- client/context_test.go | 12 ++++++ client/query.go | 15 -------- client/tx/factory.go | 2 +- client/tx/tx.go | 2 +- client/v2/autocli/common.go | 12 +++--- crypto/keyring/keyring.go | 4 ++ scripts/init-simapp.sh | 1 + x/auth/client/cli/tx_sign.go | 14 +------ x/staking/client/cli/tx.go | 2 - 13 files changed, 87 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b01825915d..d0f0aacf6fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Features +* (client) [#18101](https://github.com/cosmos/cosmos-sdk/pull/18101) Add a `keyring-default-keyname` in `client.toml` for specifying a default key name, and skip the need to use the `--from` flag when signing transactions. * (tests) [#17868](https://github.com/cosmos/cosmos-sdk/pull/17868) Added helper method `SubmitTestTx` in testutil to broadcast test txns to test e2e tests. * (x/protocolpool) [#17657](https://github.com/cosmos/cosmos-sdk/pull/17657) Create a new `x/protocolpool` module that is responsible for handling community pool funds. This module is split out into a new module from x/distribution. * (baseapp) [#16581](https://github.com/cosmos/cosmos-sdk/pull/16581) Implement Optimistic Execution as an experimental feature (not enabled by default). diff --git a/client/config/config.go b/client/config/config.go index 5b9f0340ec5..4cb0faa1203 100644 --- a/client/config/config.go +++ b/client/config/config.go @@ -12,11 +12,12 @@ import ( // DefaultConfig returns default config for the client.toml func DefaultConfig() *Config { return &Config{ - ChainID: "", - KeyringBackend: "os", - Output: "text", - Node: "tcp://localhost:26657", - BroadcastMode: "sync", + ChainID: "", + KeyringBackend: "os", + KeyringDefaultKeyName: "", + Output: "text", + Node: "tcp://localhost:26657", + BroadcastMode: "sync", } } @@ -25,11 +26,12 @@ func DefaultConfig() *Config { type ClientConfig Config type Config struct { - ChainID string `mapstructure:"chain-id" json:"chain-id"` - KeyringBackend string `mapstructure:"keyring-backend" json:"keyring-backend"` - Output string `mapstructure:"output" json:"output"` - Node string `mapstructure:"node" json:"node"` - BroadcastMode string `mapstructure:"broadcast-mode" json:"broadcast-mode"` + ChainID string `mapstructure:"chain-id" json:"chain-id"` + KeyringBackend string `mapstructure:"keyring-backend" json:"keyring-backend"` + KeyringDefaultKeyName string `mapstructure:"keyring-default-keyname" json:"keyring-default-keyname"` + Output string `mapstructure:"output" json:"output"` + Node string `mapstructure:"node" json:"node"` + BroadcastMode string `mapstructure:"broadcast-mode" json:"broadcast-mode"` } // ReadFromClientConfig reads values from client.toml file and updates them in client.Context @@ -95,7 +97,8 @@ func CreateClientConfig(ctx client.Context, customClientTemplate string, customC // we need to update KeyringDir field on client.Context first cause it is used in NewKeyringFromBackend ctx = ctx.WithOutputFormat(conf.Output). WithChainID(conf.ChainID). - WithKeyringDir(ctx.HomeDir) + WithKeyringDir(ctx.HomeDir). + WithKeyringDefaultKeyName(conf.KeyringDefaultKeyName) keyring, err := client.NewKeyringFromBackend(ctx, conf.KeyringBackend) if err != nil { diff --git a/client/config/toml.go b/client/config/toml.go index 1028f449e13..7c43eccc15b 100644 --- a/client/config/toml.go +++ b/client/config/toml.go @@ -19,6 +19,8 @@ const DefaultClientConfigTemplate = `# This is a TOML config file. chain-id = "{{ .ChainID }}" # The keyring's backend, where the keys are stored (os|file|kwallet|pass|test|memory) keyring-backend = "{{ .KeyringBackend }}" +# Default key name, if set, defines the default key to use for signing transaction when the --from flag is not specified +keyring-default-keyname = "{{ .KeyringDefaultKeyName }}" # CLI output format (text|json) output = "{{ .Output }}" # : to CometBFT RPC interface for this chain diff --git a/client/context.go b/client/context.go index 595d5329fc8..d75ea6771b2 100644 --- a/client/context.go +++ b/client/context.go @@ -27,37 +27,38 @@ type PreprocessTxFn func(chainID string, key keyring.KeyType, tx TxBuilder) erro // Context implements a typical context created in SDK modules for transaction // handling and queries. type Context struct { - FromAddress sdk.AccAddress - Client CometRPC - GRPCClient *grpc.ClientConn - ChainID string - Codec codec.Codec - InterfaceRegistry codectypes.InterfaceRegistry - Input io.Reader - Keyring keyring.Keyring - KeyringOptions []keyring.Option - Output io.Writer - OutputFormat string - Height int64 - HomeDir string - KeyringDir string - From string - BroadcastMode string - FromName string - SignModeStr string - UseLedger bool - Simulate bool - GenerateOnly bool - Offline bool - SkipConfirm bool - TxConfig TxConfig - AccountRetriever AccountRetriever - NodeURI string - FeePayer sdk.AccAddress - FeeGranter sdk.AccAddress - Viper *viper.Viper - LedgerHasProtobuf bool - PreprocessTxHook PreprocessTxFn + FromAddress sdk.AccAddress + Client CometRPC + GRPCClient *grpc.ClientConn + ChainID string + Codec codec.Codec + InterfaceRegistry codectypes.InterfaceRegistry + Input io.Reader + Keyring keyring.Keyring + KeyringOptions []keyring.Option + KeyringDir string + KeyringDefaultKeyName string + Output io.Writer + OutputFormat string + Height int64 + HomeDir string + From string + BroadcastMode string + FromName string + SignModeStr string + UseLedger bool + Simulate bool + GenerateOnly bool + Offline bool + SkipConfirm bool + TxConfig TxConfig + AccountRetriever AccountRetriever + NodeURI string + FeePayer sdk.AccAddress + FeeGranter sdk.AccAddress + Viper *viper.Viper + LedgerHasProtobuf bool + PreprocessTxHook PreprocessTxFn // IsAux is true when the signer is an auxiliary signer (e.g. the tipper). IsAux bool @@ -185,6 +186,12 @@ func (ctx Context) WithKeyringDir(dir string) Context { return ctx } +// WithKeyringDefaultKeyName returns a copy of the Context with KeyringDefaultKeyName set. +func (ctx Context) WithKeyringDefaultKeyName(keyName string) Context { + ctx.KeyringDefaultKeyName = keyName + return ctx +} + // WithGenerateOnly returns a copy of the context with updated GenerateOnly value func (ctx Context) WithGenerateOnly(generateOnly bool) Context { ctx.GenerateOnly = generateOnly @@ -385,7 +392,13 @@ func (ctx Context) printOutput(out []byte) error { // GetFromFields returns a from account address, account name and keyring type, given either an address or key name. // If clientCtx.Simulate is true the keystore is not accessed and a valid address must be provided // If clientCtx.GenerateOnly is true the keystore is only accessed if a key name is provided +// If from is empty, the default key if specified in the context will be used func GetFromFields(clientCtx Context, kr keyring.Keyring, from string) (sdk.AccAddress, string, keyring.KeyType, error) { + if from == "" && clientCtx.KeyringDefaultKeyName != "" { + from = clientCtx.KeyringDefaultKeyName + _ = clientCtx.PrintString(fmt.Sprintf("No key name or address provided; using the default key: %s\n", clientCtx.KeyringDefaultKeyName)) + } + if from == "" { return nil, "", 0, nil } diff --git a/client/context_test.go b/client/context_test.go index ed1decef168..7c0397540c5 100644 --- a/client/context_test.go +++ b/client/context_test.go @@ -107,6 +107,18 @@ func TestGetFromFields(t *testing.T) { from string expectedErr string }{ + { + clientCtx: client.Context{}.WithAddressCodec(addresscodec.NewBech32Codec("cosmos")).WithKeyringDefaultKeyName("alice"), + keyring: func() keyring.Keyring { + kb := keyring.NewInMemory(cfg.Codec) + + _, _, err := kb.NewMnemonic("alice", keyring.English, path, keyring.DefaultBIP39Passphrase, hd.Secp256k1) + require.NoError(t, err) + + return kb + }, + from: "", + }, { clientCtx: client.Context{}.WithAddressCodec(addresscodec.NewBech32Codec("cosmos")), keyring: func() keyring.Keyring { diff --git a/client/query.go b/client/query.go index 29b99bb918e..02b79bbcde2 100644 --- a/client/query.go +++ b/client/query.go @@ -61,21 +61,6 @@ func (ctx Context) GetFromAddress() sdk.AccAddress { return ctx.FromAddress } -// GetFeePayerAddress returns the fee granter address from the context -func (ctx Context) GetFeePayerAddress() sdk.AccAddress { - return ctx.FeePayer -} - -// GetFeeGranterAddress returns the fee granter address from the context -func (ctx Context) GetFeeGranterAddress() sdk.AccAddress { - return ctx.FeeGranter -} - -// GetFromName returns the key name for the current context. -func (ctx Context) GetFromName() string { - return ctx.FromName -} - func (ctx Context) queryABCI(req abci.RequestQuery) (abci.ResponseQuery, error) { node, err := ctx.GetNode() if err != nil { diff --git a/client/tx/factory.go b/client/tx/factory.go index 81ce5784d5a..b8e9bab4fd8 100644 --- a/client/tx/factory.go +++ b/client/tx/factory.go @@ -467,7 +467,7 @@ func (f Factory) Prepare(clientCtx client.Context) (Factory, error) { } fc := f - from := clientCtx.GetFromAddress() + from := clientCtx.FromAddress if err := fc.accountRetriever.EnsureExists(clientCtx, from); err != nil { return fc, err diff --git a/client/tx/tx.go b/client/tx/tx.go index 8d905922cad..b9335ee8cc0 100644 --- a/client/tx/tx.go +++ b/client/tx/tx.go @@ -126,7 +126,7 @@ func BroadcastTx(clientCtx client.Context, txf Factory, msgs ...sdk.Msg) error { } } - err = Sign(clientCtx.CmdContext, txf, clientCtx.GetFromName(), tx, true) + err = Sign(clientCtx.CmdContext, txf, clientCtx.FromName, tx, true) if err != nil { return err } diff --git a/client/v2/autocli/common.go b/client/v2/autocli/common.go index f6aa375e085..4f1346e03dd 100644 --- a/client/v2/autocli/common.go +++ b/client/v2/autocli/common.go @@ -68,17 +68,15 @@ func (b *Builder) buildMethodCommandCommon(descriptor protoreflect.MethodDescrip // 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. + // marks the custom flag as required. if binder.SignerInfo.FieldName != flags.FlagFrom { + if err := cmd.MarkFlagRequired(binder.SignerInfo.FieldName); err != nil { + return err + } + signer, err := cmd.Flags().GetString(binder.SignerInfo.FieldName) if err != nil { return fmt.Errorf("failed to get signer flag: %w", err) diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index 2e92c484254..05dac4822fc 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -945,6 +945,10 @@ func (ks keystore) migrate(key string) (*Record, error) { // 1. get the key. item, err := ks.db.Get(key) if err != nil { + if key == fmt.Sprintf(".%s", infoSuffix) { + return nil, errors.New("no key name or address provided; have you forgotten the --from flag?") + } + return nil, wrapKeyNotFound(err, key) } diff --git a/scripts/init-simapp.sh b/scripts/init-simapp.sh index 9225e9b1245..a27fc9354bb 100755 --- a/scripts/init-simapp.sh +++ b/scripts/init-simapp.sh @@ -7,6 +7,7 @@ echo "using $SIMD_BIN" if [ -d "$($SIMD_BIN config home)" ]; then rm -r $($SIMD_BIN config home); fi $SIMD_BIN config set client chain-id demo $SIMD_BIN config set client keyring-backend test +$SIMD_BIN config set client keyring-default-keyname alice $SIMD_BIN config set app api.enable true $SIMD_BIN keys add alice $SIMD_BIN keys add bob diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index 7ba332e3579..ae3d0607ebc 100644 --- a/x/auth/client/cli/tx_sign.go +++ b/x/auth/client/cli/tx_sign.go @@ -58,11 +58,6 @@ account key. It implies --signature-only. flags.AddTxFlagsToCmd(cmd) - err := cmd.MarkFlagRequired(flags.FlagFrom) - if err != nil { - panic(err) - } - return cmd } @@ -233,7 +228,7 @@ func multisigSign(clientCtx client.Context, txBuilder client.TxBuilder, txFactor txFactory, clientCtx, multisigAddr, - clientCtx.GetFromName(), + clientCtx.FromName, txBuilder, clientCtx.Offline, true, @@ -290,11 +285,6 @@ be generated via the 'multisign' command. cmd.Flags().String(flags.FlagOutputDocument, "", "The document will be written to the given file instead of STDOUT") flags.AddTxFlagsToCmd(cmd) - err := cmd.MarkFlagRequired(flags.FlagFrom) - if err != nil { - panic(err) - } - return cmd } @@ -405,7 +395,7 @@ func signTx(cmd *cobra.Command, clientCtx client.Context, txF tx.Factory, newTx } printSignatureOnly = true } else { - err = authclient.SignTx(txF, clientCtx, clientCtx.GetFromName(), txBuilder, clientCtx.Offline, overwrite) + err = authclient.SignTx(txF, clientCtx, clientCtx.FromName, txBuilder, clientCtx.Offline, overwrite) } if err != nil { return err diff --git a/x/staking/client/cli/tx.go b/x/staking/client/cli/tx.go index edfee3b8346..090d88f37b6 100644 --- a/x/staking/client/cli/tx.go +++ b/x/staking/client/cli/tx.go @@ -109,8 +109,6 @@ where we can get the pubkey using "%s tendermint show-validator" cmd.Flags().String(FlagNodeID, "", "The node's ID") flags.AddTxFlagsToCmd(cmd) - _ = cmd.MarkFlagRequired(flags.FlagFrom) - return cmd }