Generate/recover can return error, not panic on ledger

keys/cryptostore/encoder_test.go

@@ -15,8 +15,10 @@ func TestNoopEncoder(t *testing.T) {
 	assert, require := assert.New(t), require.New(t)
 	noop := cryptostore.Noop
 
-	key := cryptostore.GenEd25519.Generate(cmn.RandBytes(16))
-	key2 := cryptostore.GenSecp256k1.Generate(cmn.RandBytes(16))
+	key, err := cryptostore.GenEd25519.Generate(cmn.RandBytes(16))
+	require.NoError(err)
+	key2, err := cryptostore.GenSecp256k1.Generate(cmn.RandBytes(16))
+	require.NoError(err)
 
 	_, b, err := noop.Encrypt(key, "encode")
 	require.Nil(err)
@@ -43,7 +45,8 @@ func TestSecretBox(t *testing.T) {
 	assert, require := assert.New(t), require.New(t)
 	enc := cryptostore.SecretBox
 
-	key := cryptostore.GenEd25519.Generate(cmn.RandBytes(16))
+	key, err := cryptostore.GenEd25519.Generate(cmn.RandBytes(16))
+	require.NoError(err)
 	pass := "some-special-secret"
 
 	s, b, err := enc.Encrypt(key, pass)
@@ -65,7 +68,8 @@ func TestSecretBoxNoPass(t *testing.T) {
 	assert, require := assert.New(t), require.New(t)
 	enc := cryptostore.SecretBox
 
-	key := cryptostore.GenEd25519.Generate(cmn.RandBytes(16))
+	key, err := cryptostore.GenEd25519.Generate(cmn.RandBytes(16))
+	require.NoError(err)
 
 	cases := []struct {
 		encode string

keys/cryptostore/generator.go

@@ -18,57 +18,72 @@ var (
 
 // Generator determines the type of private key the keystore creates
 type Generator interface {
-	Generate(secret []byte) crypto.PrivKey
+	Generate(secret []byte) (crypto.PrivKey, error)
 }
 
 // GenFunc is a helper to transform a function into a Generator
-type GenFunc func(secret []byte) crypto.PrivKey
+type GenFunc func(secret []byte) (crypto.PrivKey, error)
 
-func (f GenFunc) Generate(secret []byte) crypto.PrivKey {
+func (f GenFunc) Generate(secret []byte) (crypto.PrivKey, error) {
 	return f(secret)
 }
 
-func genEd25519(secret []byte) crypto.PrivKey {
-	return crypto.GenPrivKeyEd25519FromSecret(secret).Wrap()
+func genEd25519(secret []byte) (crypto.PrivKey, error) {
+	key := crypto.GenPrivKeyEd25519FromSecret(secret).Wrap()
+	return key, nil
 }
 
-func genSecp256(secret []byte) crypto.PrivKey {
-	return crypto.GenPrivKeySecp256k1FromSecret(secret).Wrap()
+func genSecp256(secret []byte) (crypto.PrivKey, error) {
+	key := crypto.GenPrivKeySecp256k1FromSecret(secret).Wrap()
+	return key, nil
 }
 
 // secret is completely ignored for the ledger...
 // just for interface compatibility
-func genLedger(secret []byte) crypto.PrivKey {
-	key, err := nano.NewPrivKeyLedger()
-	if err != nil {
-		// TODO: cleaner error handling
-		panic(err)
-	}
-	return key
+func genLedger(secret []byte) (crypto.PrivKey, error) {
+	return nano.NewPrivKeyLedger()
+}
+
+type genInvalidByte struct {
+	typ byte
+}
+
+func (g genInvalidByte) Generate(secret []byte) (crypto.PrivKey, error) {
+	err := errors.Errorf("Cannot generate keys for algorithm: %X", g.typ)
+	return crypto.PrivKey{}, err
+}
+
+type genInvalidAlgo struct {
+	algo string
+}
+
+func (g genInvalidAlgo) Generate(secret []byte) (crypto.PrivKey, error) {
+	err := errors.Errorf("Cannot generate keys for algorithm: %s", g.algo)
+	return crypto.PrivKey{}, err
 }
 
-func getGenerator(algo string) (Generator, error) {
+func getGenerator(algo string) Generator {
 	switch algo {
 	case crypto.NameEd25519:
-		return GenEd25519, nil
+		return GenEd25519
 	case crypto.NameSecp256k1:
-		return GenSecp256k1, nil
+		return GenSecp256k1
 	case nano.NameLedger:
-		return GenLedger, nil
+		return GenLedger
 	default:
-		return nil, errors.Errorf("Cannot generate keys for algorithm: %s", algo)
+		return genInvalidAlgo{algo}
 	}
 }
 
-func getGeneratorByType(typ byte) (Generator, error) {
+func getGeneratorByType(typ byte) Generator {
 	switch typ {
 	case crypto.TypeEd25519:
-		return GenEd25519, nil
+		return GenEd25519
 	case crypto.TypeSecp256k1:
-		return GenSecp256k1, nil
+		return GenSecp256k1
 	case nano.TypeLedger:
-		return GenLedger, nil
+		return GenLedger
 	default:
-		return nil, errors.Errorf("Cannot generate keys for algorithm: %X", typ)
+		return genInvalidByte{typ}
 	}
 }

keys/cryptostore/holder.go

@@ -33,14 +33,15 @@ var _ keys.Manager = Manager{}
 //
 // algo must be a supported go-crypto algorithm: ed25519, secp256k1
 func (s Manager) Create(name, passphrase, algo string) (keys.Info, string, error) {
-	gen, err := getGenerator(algo)
+	// 128-bits are the all the randomness we can make use of
+	secret := crypto.CRandBytes(16)
+	gen := getGenerator(algo)
+
+	key, err := gen.Generate(secret)
 	if err != nil {
 		return keys.Info{}, "", err
 	}
 
-	// 128-bits are the all the randomness we can make use of
-	secret := crypto.CRandBytes(16)
-	key := gen.Generate(secret)
 	err = s.es.Put(name, passphrase, key)
 	if err != nil {
 		return keys.Info{}, "", err
@@ -74,11 +75,11 @@ func (s Manager) Recover(name, passphrase, seedphrase string) (keys.Info, error)
 	l := len(secret)
 	secret, typ := secret[:l-1], secret[l-1]
 
-	gen, err := getGeneratorByType(typ)
+	gen := getGeneratorByType(typ)
+	key, err := gen.Generate(secret)
 	if err != nil {
 		return keys.Info{}, err
 	}
-	key := gen.Generate(secret)
 
 	// d00d, it worked!  create the bugger....
 	err = s.es.Put(name, passphrase, key)

keys/cryptostore/holder_test.go

@@ -224,13 +224,15 @@ func TestImportUnencrypted(t *testing.T) {
 		keys.MustLoadCodec("english"),
 	)
 
-	key := cryptostore.GenEd25519.Generate(cmn.RandBytes(16))
+	key, err := cryptostore.GenEd25519.Generate(cmn.RandBytes(16))
+	require.NoError(err)
+
 	addr := key.PubKey().Address()
 	name := "john"
 	pass := "top-secret"
 
 	// import raw bytes
-	err := cstore.Import(name, pass, "", nil, key.Bytes())
+	err = cstore.Import(name, pass, "", nil, key.Bytes())
 	require.Nil(err, "%+v", err)
 
 	// make sure the address matches

keys/cryptostore/storage_test.go

@@ -14,7 +14,10 @@ import (
 func TestSortKeys(t *testing.T) {
 	assert := assert.New(t)
 
-	gen := func() crypto.PrivKey { return GenEd25519.Generate(cmn.RandBytes(16)) }
+	gen := func() crypto.PrivKey {
+		key, _ := GenEd25519.Generate(cmn.RandBytes(16))
+		return key
+	}
 	assert.NotEqual(gen(), gen())
 
 	// alphabetical order is n3, n1, n2