From bce5da0e84615e68c5a21bf6e282f2a1f56d69d5 Mon Sep 17 00:00:00 2001 From: Amaury Date: Fri, 11 Dec 2020 14:54:50 +0100 Subject: [PATCH] Use SignModeLegacyAminoJSON when signer is ledger key (#8136) * Use SignModeLegacyAminoJSON when signer is ledger key * Fix tests * Fix lint * Fix tests * Add warning message Co-authored-by: Alessio Treglia --- client/cmd.go | 16 +++++++++++++++- client/context.go | 24 ++++++++++++++++-------- client/flags/flags.go | 5 +++++ client/tx/factory.go | 12 ++++-------- x/auth/client/cli/tx_sign.go | 4 ++-- 5 files changed, 42 insertions(+), 19 deletions(-) diff --git a/client/cmd.go b/client/cmd.go index 8395fb49b5..b8ff6711d7 100644 --- a/client/cmd.go +++ b/client/cmd.go @@ -11,6 +11,7 @@ import ( rpchttp "github.com/tendermint/tendermint/rpc/client/http" "github.com/cosmos/cosmos-sdk/client/flags" + "github.com/cosmos/cosmos-sdk/crypto/keyring" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -215,14 +216,27 @@ func ReadTxCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, err clientCtx = clientCtx.WithSkipConfirmation(skipConfirm) } + if clientCtx.SignModeStr == "" || flagSet.Changed(flags.FlagSignMode) { + signModeStr, _ := flagSet.GetString(flags.FlagSignMode) + clientCtx = clientCtx.WithSignModeStr(signModeStr) + } + if clientCtx.From == "" || flagSet.Changed(flags.FlagFrom) { from, _ := flagSet.GetString(flags.FlagFrom) - fromAddr, fromName, err := GetFromFields(clientCtx.Keyring, from, clientCtx.GenerateOnly) + fromAddr, fromName, keyType, err := GetFromFields(clientCtx.Keyring, from, clientCtx.GenerateOnly) if err != nil { return clientCtx, err } clientCtx = clientCtx.WithFrom(from).WithFromAddress(fromAddr).WithFromName(fromName) + + // If the `from` signer account is a ledger key, we need to use + // SIGN_MODE_AMINO_JSON, because ledger doesn't support proto yet. + // ref: https://github.com/cosmos/cosmos-sdk/issues/8109 + if keyType == keyring.TypeLedger && clientCtx.SignModeStr != flags.SignModeLegacyAminoJSON { + fmt.Println("Default sign-mode 'direct' not supported by Ledger, using sign-mode 'amino-json'.") + clientCtx = clientCtx.WithSignModeStr(flags.SignModeLegacyAminoJSON) + } } return clientCtx, nil diff --git a/client/context.go b/client/context.go index 41435bd90e..c63d557bdc 100644 --- a/client/context.go +++ b/client/context.go @@ -35,6 +35,7 @@ type Context struct { From string BroadcastMode string FromName string + SignModeStr string UseLedger bool Simulate bool GenerateOnly bool @@ -172,6 +173,13 @@ func (ctx Context) WithBroadcastMode(mode string) Context { return ctx } +// WithSignModeStr returns a copy of the context with an updated SignMode +// value. +func (ctx Context) WithSignModeStr(signModeStr string) Context { + ctx.SignModeStr = signModeStr + return ctx +} + // WithSkipConfirmation returns a copy of the context with an updated SkipConfirm // value. func (ctx Context) WithSkipConfirmation(skip bool) Context { @@ -268,37 +276,37 @@ func (ctx Context) printOutput(out []byte) error { return nil } -// GetFromFields returns a from account address and Keybase name given either +// GetFromFields returns a from account address, account name and keyring type, given either // an address or key name. If genOnly is true, only a valid Bech32 cosmos // address is returned. -func GetFromFields(kr keyring.Keyring, from string, genOnly bool) (sdk.AccAddress, string, error) { +func GetFromFields(kr keyring.Keyring, from string, genOnly bool) (sdk.AccAddress, string, keyring.KeyType, error) { if from == "" { - return nil, "", nil + return nil, "", 0, nil } if genOnly { addr, err := sdk.AccAddressFromBech32(from) if err != nil { - return nil, "", errors.Wrap(err, "must provide a valid Bech32 address in generate-only mode") + return nil, "", 0, errors.Wrap(err, "must provide a valid Bech32 address in generate-only mode") } - return addr, "", nil + return addr, "", 0, nil } var info keyring.Info if addr, err := sdk.AccAddressFromBech32(from); err == nil { info, err = kr.KeyByAddress(addr) if err != nil { - return nil, "", err + return nil, "", 0, err } } else { info, err = kr.Key(from) if err != nil { - return nil, "", err + return nil, "", 0, err } } - return info.GetAddress(), info.GetName(), nil + return info.GetAddress(), info.GetName(), info.GetType(), nil } func newKeyringFromFlags(ctx Context, backend string) (keyring.Keyring, error) { diff --git a/client/flags/flags.go b/client/flags/flags.go index 72bcabcd60..282a61c4c9 100644 --- a/client/flags/flags.go +++ b/client/flags/flags.go @@ -30,6 +30,11 @@ const ( // BroadcastAsync defines a tx broadcasting mode where the client returns // immediately. BroadcastAsync = "async" + + // SignModeDirect is the value of the --sign-mode flag for SIGN_MODE_DIRECT + SignModeDirect = "direct" + // SignModeLegacyAminoJSON is the value of the --sign-mode flag for SIGN_MODE_LEGACY_AMINO_JSON + SignModeLegacyAminoJSON = "amino-json" ) // List of CLI flags diff --git a/client/tx/factory.go b/client/tx/factory.go index 19c7c61dea..b10d728d6b 100644 --- a/client/tx/factory.go +++ b/client/tx/factory.go @@ -29,19 +29,15 @@ type Factory struct { simulateAndExecute bool } -const ( - signModeDirect = "direct" - signModeAminoJSON = "amino-json" -) - +// NewFactoryCLI creates a new Factory. func NewFactoryCLI(clientCtx client.Context, flagSet *pflag.FlagSet) Factory { - signModeStr, _ := flagSet.GetString(flags.FlagSignMode) + signModeStr := clientCtx.SignModeStr signMode := signing.SignMode_SIGN_MODE_UNSPECIFIED switch signModeStr { - case signModeDirect: + case flags.SignModeDirect: signMode = signing.SignMode_SIGN_MODE_DIRECT - case signModeAminoJSON: + case flags.SignModeLegacyAminoJSON: signMode = signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON } diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index a9c6980782..15fb066957 100644 --- a/x/auth/client/cli/tx_sign.go +++ b/x/auth/client/cli/tx_sign.go @@ -110,7 +110,7 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { } if multisigAddr.Empty() { from, _ := cmd.Flags().GetString(flags.FlagFrom) - _, fromName, err := client.GetFromFields(txFactory.Keybase(), from, clientCtx.GenerateOnly) + _, fromName, _, err := client.GetFromFields(txFactory.Keybase(), from, clientCtx.GenerateOnly) if err != nil { return fmt.Errorf("error getting account from keybase: %w", err) } @@ -235,7 +235,7 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error { multisigAddrStr, _ := cmd.Flags().GetString(flagMultisig) from, _ := cmd.Flags().GetString(flags.FlagFrom) - _, fromName, err := client.GetFromFields(txFactory.Keybase(), from, clientCtx.GenerateOnly) + _, fromName, _, err := client.GetFromFields(txFactory.Keybase(), from, clientCtx.GenerateOnly) if err != nil { return fmt.Errorf("error getting account from keybase: %w", err) }