crypto/amino: Address anton's comment on PubkeyAminoRoute (#2592)

crypto/encoding/amino/amino.go

@@ -1,8 +1,6 @@
 package cryptoAmino
 
 import (
-	"errors"
-
 	"reflect"
 
 	amino "github.com/tendermint/go-amino"
@@ -13,6 +11,11 @@ import (
 )
 
 var cdc = amino.NewCodec()
+
+// routeTable is used to map public key concrete types back
+// to their amino routes. This should eventually be handled
+// by amino. Example usage:
+// routeTable[reflect.TypeOf(ed25519.PubKeyEd25519{})] = ed25519.PubKeyAminoRoute
 var routeTable = make(map[reflect.Type]string, 3)
 
 func init() {
@@ -34,12 +37,9 @@ func init() {
 // PubkeyAminoRoute returns the amino route of a pubkey
 // cdc is currently passed in, as eventually this will not be using
 // a package level codec.
-func PubkeyAminoRoute(cdc *amino.Codec, key crypto.PubKey) (string, error) {
-	route, ok := routeTable[reflect.TypeOf(key)]
-	if !ok {
-		return "", errors.New("Pubkey type not known")
-	}
-	return route, nil
+func PubkeyAminoRoute(cdc *amino.Codec, key crypto.PubKey) (string, bool) {
+	route, found := routeTable[reflect.TypeOf(key)]
+	return route, found
 }
 
 // RegisterAmino registers all crypto related types in the given (amino) codec.

crypto/encoding/amino/encode_test.go

@@ -120,32 +120,29 @@ func TestNilEncodings(t *testing.T) {
 	var e, f crypto.PrivKey
 	checkAminoJSON(t, &e, &f, true)
 	assert.EqualValues(t, e, f)
-
 }
 
 func TestPubKeyInvalidDataProperReturnsEmpty(t *testing.T) {
 	pk, err := PubKeyFromBytes([]byte("foo"))
-	require.NotNil(t, err, "expecting a non-nil error")
-	require.Nil(t, pk, "expecting an empty public key on error")
+	require.NotNil(t, err)
+	require.Nil(t, pk)
 }
 
 func TestPubkeyAminoRoute(t *testing.T) {
 	tests := []struct {
-		key     crypto.PubKey
-		want    string
-		wantErr bool
+		key   crypto.PubKey
+		want  string
+		found bool
 	}{
-		{ed25519.PubKeyEd25519{}, ed25519.PubKeyAminoRoute, false},
-		{secp256k1.PubKeySecp256k1{}, secp256k1.PubKeyAminoRoute, false},
-		{&multisig.PubKeyMultisigThreshold{}, multisig.PubKeyMultisigThresholdAminoRoute, false},
+		{ed25519.PubKeyEd25519{}, ed25519.PubKeyAminoRoute, true},
+		{secp256k1.PubKeySecp256k1{}, secp256k1.PubKeyAminoRoute, true},
+		{&multisig.PubKeyMultisigThreshold{}, multisig.PubKeyMultisigThresholdAminoRoute, true},
 	}
 	for i, tc := range tests {
-		got, err := PubkeyAminoRoute(cdc, tc.key)
-		if tc.wantErr {
-			require.Error(t, err, "tc %d", i)
-		} else {
-			require.NoError(t, err, "tc %d", i)
-			require.Equal(t, tc.want, got, "tc %d", i)
+		got, found := PubkeyAminoRoute(cdc, tc.key)
+		require.Equal(t, tc.found, found, "not equal on tc %d", i)
+		if tc.found {
+			require.Equal(t, tc.want, got, "not equal on tc %d", i)
 		}
 	}
 }