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 <alessio@tendermint.com>

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

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

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

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
 	}
 

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