client: fix signing algorithm

CHANGELOG.md

@@ -160,6 +160,7 @@ be used to retrieve the actual proposal `Content`. Also the `NewMsgSubmitProposa
 
 ### Bug Fixes
 
+* (client) [\#6402](https://github.com/cosmos/cosmos-sdk/issues/6402) Fix `keys add` `--algo` flag which only worked for Tendermint's `secp256k1` default key signing algorithm.
 * (x/bank) [\#6283](https://github.com/cosmos/cosmos-sdk/pull/6283) Create account if recipient does not exist on handing `MsgMultiSend`.
 * (x/distribution) [\#6210](https://github.com/cosmos/cosmos-sdk/pull/6210) Register `MsgFundCommunityPool` in distribution amino codec.
 * (x/staking) [\#6061](https://github.com/cosmos/cosmos-sdk/pull/6061) Allow a validator to immediately unjail when no signing info is present due to

client/keys/add.go

@@ -120,9 +120,10 @@ func RunAddCmd(cmd *cobra.Command, args []string, kb keyring.Keyring, inBuf *buf
  interactive := viper.GetBool(flagInteractive)
  showMnemonic := !viper.GetBool(flagNoBackup)
 
- algo, err := keyring.NewSigningAlgoFromString(viper.GetString(flagKeyAlgo))
+ keyringAlgos, _ := kb.SupportedAlgorithms()
+ algo, err := keyring.NewSigningAlgoFromString(viper.GetString(flagKeyAlgo), keyringAlgos)
  if err != nil {
- algo = hd.Secp256k1
+ return err
  }
 
  if !viper.GetBool(flags.FlagDryRun) {

client/keys/add_ledger_test.go

@@ -11,6 +11,7 @@ import (
  "github.com/tendermint/tendermint/libs/cli"
 
  "github.com/cosmos/cosmos-sdk/client/flags"
+ "github.com/cosmos/cosmos-sdk/crypto/hd"
  "github.com/cosmos/cosmos-sdk/crypto/keyring"
  "github.com/cosmos/cosmos-sdk/tests"
  sdk "github.com/cosmos/cosmos-sdk/types"
@@ -45,8 +46,10 @@ func Test_runAddCmdLedgerWithCustomCoinType(t *testing.T) {
  viper.Set(flagIndex, "0")
  viper.Set(flagCoinType, "330")
 
- /// Test Text
+ // Test Text
  viper.Set(cli.OutputFlag, OutputFormatText)
+ // set algo flag value to the default
+ viper.Set(flagKeyAlgo, string(hd.Secp256k1Type))
  // Now enter password
  mockIn, _, _ := tests.ApplyMockIO(cmd)
  mockIn.Reset("test1234\ntest1234\n")
@@ -57,7 +60,7 @@ func Test_runAddCmdLedgerWithCustomCoinType(t *testing.T) {
  require.NoError(t, err)
  require.NotNil(t, kb)
  t.Cleanup(func() {
- kb.Delete("keyname1")
+ _ = kb.Delete("keyname1")
  })
  mockIn.Reset("test1234\n")
  key1, err := kb.Key("keyname1")
@@ -89,8 +92,10 @@ func Test_runAddCmdLedger(t *testing.T) {
  viper.Set(flags.FlagHome, kbHome)
  viper.Set(flags.FlagUseLedger, true)
 
- /// Test Text
+ // Test Text
  viper.Set(cli.OutputFlag, OutputFormatText)
+ // set algo flag value to the default
+ viper.Set(flagKeyAlgo, string(hd.Secp256k1Type))
  // Now enter password
  mockIn.Reset("test1234\ntest1234\n")
  viper.Set(flagCoinType, sdk.CoinType)
@@ -101,7 +106,7 @@ func Test_runAddCmdLedger(t *testing.T) {
  require.NoError(t, err)
  require.NotNil(t, kb)
  t.Cleanup(func() {
- kb.Delete("keyname1")
+ _ = kb.Delete("keyname1")
  })
  mockIn.Reset("test1234\n")
  key1, err := kb.Key("keyname1")

client/keys/add_test.go

@@ -9,6 +9,7 @@ import (
  "github.com/tendermint/tendermint/libs/cli"
 
  "github.com/cosmos/cosmos-sdk/client/flags"
+ "github.com/cosmos/cosmos-sdk/crypto/hd"
  "github.com/cosmos/cosmos-sdk/crypto/keyring"
  "github.com/cosmos/cosmos-sdk/tests"
  sdk "github.com/cosmos/cosmos-sdk/types"
@@ -17,6 +18,7 @@ import (
 func Test_runAddCmdBasic(t *testing.T) {
  cmd := AddKeyCommand()
  require.NotNil(t, cmd)
+
  mockIn, _, _ := tests.ApplyMockIO(cmd)
 
  kbHome, kbCleanUp := tests.NewTestCaseDir(t)
@@ -27,11 +29,14 @@ func Test_runAddCmdBasic(t *testing.T) {
  viper.Set(flags.FlagUseLedger, false)
 
  mockIn.Reset("y\n")
+ // set algo flag value to the default
+ viper.Set(flagKeyAlgo, string(hd.Secp256k1Type))
+
  kb, err := keyring.New(sdk.KeyringServiceName(), viper.GetString(flags.FlagKeyringBackend), kbHome, mockIn)
  require.NoError(t, err)
  t.Cleanup(func() {
- kb.Delete("keyname1") // nolint:errcheck
- kb.Delete("keyname2") // nolint:errcheck
+ _ = kb.Delete("keyname1")
+ _ = kb.Delete("keyname2")
  })
  require.NoError(t, runAddCmd(cmd, []string{"keyname1"}))
 

crypto/keyring/keyring.go

@@ -14,18 +14,18 @@ import (
  "github.com/99designs/keyring"
  "github.com/cosmos/go-bip39"
  "github.com/pkg/errors"
-
  "github.com/tendermint/crypto/bcrypt"
  tmcrypto "github.com/tendermint/tendermint/crypto"
+ cryptoamino "github.com/tendermint/tendermint/crypto/encoding/amino"
 
  "github.com/cosmos/cosmos-sdk/client/input"
  "github.com/cosmos/cosmos-sdk/crypto"
- cryptoAmino "github.com/cosmos/cosmos-sdk/crypto/codec"
  "github.com/cosmos/cosmos-sdk/crypto/hd"
  sdk "github.com/cosmos/cosmos-sdk/types"
  sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
+// Backend options for Keyring
 const (
  BackendFile = "file"
  BackendOS = "os"
@@ -51,6 +51,9 @@ type Keyring interface {
  // List all keys.
  List() ([]Info, error)
 
+ // Supported signing algorithms for Keyring and Ledger respectively.
+ SupportedAlgorithms() (SigningAlgoList, SigningAlgoList)
+
  // Key and KeyByAddress return keys by uid and address respectively.
  Key(uid string) (Info, error)
  KeyByAddress(address sdk.Address) (Info, error)
@@ -114,9 +117,11 @@ type Exporter interface {
 // Option overrides keyring configuration options.
 type Option func(options *Options)
 
-//Options define the options of the Keyring
+// Options define the options of the Keyring
 type Options struct {
- SupportedAlgos SigningAlgoList
+ // supported signing algorithms for keyring
+ SupportedAlgos SigningAlgoList
+ // supported signing algorithms for Ledger
  SupportedAlgosLedger SigningAlgoList
 }
 
@@ -127,7 +132,7 @@ func NewInMemory(opts ...Option) Keyring {
  return newKeystore(keyring.NewArrayKeyring(nil), opts...)
 }
 
-// NewKeyring creates a new instance of a keyring.
+// New creates a new instance of a keyring.
 // Keyring ptions can be applied when generating the new instance.
 // Available backends are "os", "file", "kwallet", "memory", "pass", "test".
 func New(
@@ -233,7 +238,7 @@ func (ks keystore) ExportPrivateKeyObject(uid string) (tmcrypto.PrivKey, error)
  return nil, err
  }
 
- priv, err = cryptoAmino.PrivKeyFromBytes([]byte(linfo.PrivKeyArmor))
+ priv, err = cryptoamino.PrivKeyFromBytes([]byte(linfo.PrivKeyArmor))
  if err != nil {
  return nil, err
  }
@@ -282,7 +287,7 @@ func (ks keystore) ImportPubKey(uid string, armor string) error {
  return err
  }
 
- pubKey, err := cryptoAmino.PubKeyFromBytes(pubBytes)
+ pubKey, err := cryptoamino.PubKeyFromBytes(pubBytes)
  if err != nil {
  return err
  }
@@ -309,7 +314,7 @@ func (ks keystore) Sign(uid string, msg []byte) ([]byte, tmcrypto.PubKey, error)
  return nil, nil, fmt.Errorf("private key not available")
  }
 
- priv, err = cryptoAmino.PrivKeyFromBytes([]byte(i.PrivKeyArmor))
+ priv, err = cryptoamino.PrivKeyFromBytes([]byte(i.PrivKeyArmor))
  if err != nil {
  return nil, nil, err
  }
@@ -518,6 +523,12 @@ func (ks keystore) Key(uid string) (Info, error) {
  return unmarshalInfo(bs.Data)
 }
 
+// SupportedAlgorithms retuns the keystore Options' supported signing algorithm
+// for the keyring and Ledger.
+func (ks keystore) SupportedAlgorithms() (SigningAlgoList, SigningAlgoList) {
+ return ks.options.SupportedAlgos, ks.options.SupportedAlgosLedger
+}
+
 // SignWithLedger signs a binary message with the ledger device referenced by an Info object
 // and returns the signed bytes and the public key. It returns an error if the device could
 // not be queried or it returned an error.
@@ -690,7 +701,7 @@ func (ks keystore) writeInfo(info Info) error {
 
  exists, err := ks.existsInDb(info)
  if exists {
- return fmt.Errorf("public key already exist in keybase")
+ return errors.New("public key already exist in keybase")
  }
 
  if err != nil {

crypto/keyring/signing_algorithms.go

@@ -2,32 +2,48 @@ package keyring
 
 import (
  "fmt"
+ "strings"
 
  "github.com/cosmos/cosmos-sdk/crypto/hd"
 )
 
+// SignatureAlgo defines the interface for a keyring supported algorithm.
 type SignatureAlgo interface {
  Name() hd.PubKeyType
  Derive() hd.DeriveFn
  Generate() hd.GenerateFn
 }
 
-func NewSigningAlgoFromString(str string) (SignatureAlgo, error) {
- if str != string(hd.Secp256k1.Name()) {
- return nil, fmt.Errorf("provided algorithm `%s` is not supported", str)
+// NewSigningAlgoFromString creates a supported SignatureAlgo
+func NewSigningAlgoFromString(str string, algoList SigningAlgoList) (SignatureAlgo, error) {
+ for _, algo := range algoList {
+ if str == string(algo.Name()) {
+ return algo, nil
+ }
  }
-
- return hd.Secp256k1, nil
+ return nil, fmt.Errorf("provided algorithm '%s' is not supported", str)
 }
 
+// SigningAlgoList is a slice of signature algorithms
 type SigningAlgoList []SignatureAlgo
 
-func (l SigningAlgoList) Contains(algo SignatureAlgo) bool {
- for _, cAlgo := range l {
+// Contains returns true if the SigningAlgoList the given SignatureAlgo.
+func (sal SigningAlgoList) Contains(algo SignatureAlgo) bool {
+ for _, cAlgo := range sal {
  if cAlgo.Name() == algo.Name() {
  return true
  }
  }
 
  return false
 }
+
+// String returns a comma separated string of the signature algorithm names in the list.
+func (sal SigningAlgoList) String() string {
+ names := make([]string, len(sal))
+ for i := range sal {
+ names[i] = string(sal[i].Name())
+ }
+
+ return strings.Join(names, ",")
+}

crypto/keyring/signing_algorithms_test.go

@@ -4,7 +4,6 @@ import (
  "fmt"
  "testing"
 
- "github.com/stretchr/testify/assert"
  "github.com/stretchr/testify/require"
 
  "github.com/cosmos/cosmos-sdk/crypto/hd"
@@ -30,13 +29,15 @@ func TestNewSigningAlgoByString(t *testing.T) {
  "notsupportedalgo",
  false,
  nil,
- fmt.Errorf("provided algorithm `notsupportedalgo` is not supported"),
+ fmt.Errorf("provided algorithm 'notsupportedalgo' is not supported"),
  },
  }
+
+ list := SigningAlgoList{hd.Secp256k1}
  for _, tt := range tests {
  tt := tt
  t.Run(tt.name, func(t *testing.T) {
- algorithm, err := NewSigningAlgoFromString(tt.algoStr)
+ algorithm, err := NewSigningAlgoFromString(tt.algoStr, list)
  if tt.isSupported {
  require.Equal(t, hd.Secp256k1, algorithm)
  } else {
@@ -47,12 +48,15 @@ func TestNewSigningAlgoByString(t *testing.T) {
 }
 
 func TestAltSigningAlgoList_Contains(t *testing.T) {
- list := SigningAlgoList{
- hd.Secp256k1,
- }
+ list := SigningAlgoList{hd.Secp256k1}
+
+ require.True(t, list.Contains(hd.Secp256k1))
+ require.False(t, list.Contains(notSupportedAlgo{}))
+}
 
- assert.True(t, list.Contains(hd.Secp256k1))
- assert.False(t, list.Contains(notSupportedAlgo{}))
+func TestAltSigningAlgoList_String(t *testing.T) {
+ list := SigningAlgoList{hd.Secp256k1, notSupportedAlgo{}}
+ require.Equal(t, fmt.Sprintf("%s,notSupported", string(hd.Secp256k1Type)), list.String())
 }
 
 type notSupportedAlgo struct {