refactor!: key presentation outside keyring (#14151)

* refactor: move key presentation to client/keys

* refactor: move key presentation to client/keys

* update: changelog

Co-authored-by: Ezequiel Raynaudo <raynaudo.ee@gmail.com>

CHANGELOG.md

@@ -58,6 +58,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+* (crypto/keyring) [#14151](https://github.com/cosmos/cosmos-sdk/pull/14151) Move keys presentation from `crypto/keyring` to `client/keys`
 * (types) [#14163](https://github.com/cosmos/cosmos-sdk/pull/14163) Refactor `(coins Coins) Validate()` to avoid unnecessary map.
 * (signing) [#14087](https://github.com/cosmos/cosmos-sdk/pull/14087) Add SignModeHandlerWithContext interface with a new `GetSignBytesWithContext` to get the sign bytes using `context.Context` as an argument to access state.
 * (server) [#14062](https://github.com/cosmos/cosmos-sdk/pull/14062) Remove rosetta from server start.
@@ -128,6 +129,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### API Breaking Changes
 
+* (crypto/keyring) [#14151](https://github.com/cosmos/cosmos-sdk/pull/14151) Move keys presentation from `crypto/keyring` to `client/keys`
 * (modules) [#13850](https://github.com/cosmos/cosmos-sdk/pull/13850) and [#14046](https://github.com/cosmos/cosmos-sdk/pull/14046) Remove gogoproto stringer annotations. This removes the custom `String()` methods on all types that were using the annotations.
 * (x/auth) [#13850](https://github.com/cosmos/cosmos-sdk/pull/13850/) Remove `MarshalYAML` methods from module (`x/...`) types.
 * (x/auth) [#13877](https://github.com/cosmos/cosmos-sdk/pull/13877) Rename `AccountKeeper`'s `GetNextAccountNumber` to `NextAccountNumber`.

client/keys/add.go

@@ -294,7 +294,7 @@ func printCreate(cmd *cobra.Command, k *keyring.Record, showMnemonic bool, mnemo
 	switch outputFormat {
 	case OutputFormatText:
 		cmd.PrintErrln()
-		if err := printKeyringRecord(cmd.OutOrStdout(), k, keyring.MkAccKeyOutput, outputFormat); err != nil {
+		if err := printKeyringRecord(cmd.OutOrStdout(), k, MkAccKeyOutput, outputFormat); err != nil {
 			return err
 		}
 
@@ -305,7 +305,7 @@ func printCreate(cmd *cobra.Command, k *keyring.Record, showMnemonic bool, mnemo
 			}
 		}
 	case OutputFormatJSON:
-		out, err := keyring.MkAccKeyOutput(k)
+		out, err := MkAccKeyOutput(k)
 		if err != nil {
 			return err
 		}

crypto/keyring/output.go → client/keys/output.go

@@ -1,13 +1,13 @@
-package keyring
+package keys
 
 import (
 	"github.com/cosmos/cosmos-sdk/codec"
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
+	"github.com/cosmos/cosmos-sdk/crypto/keyring"
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
-// TODO: Move this file to client/keys
 // Use protobuf interface marshaler rather then generic JSON
 
 // KeyOutput defines a structure wrapping around an Info object used for output
@@ -21,7 +21,7 @@ type KeyOutput struct {
 }
 
 // NewKeyOutput creates a default KeyOutput instance without Mnemonic, Threshold and PubKeys
-func NewKeyOutput(name string, keyType KeyType, a sdk.Address, pk cryptotypes.PubKey) (KeyOutput, error) { //nolint:interfacer
+func NewKeyOutput(name string, keyType keyring.KeyType, a sdk.Address, pk cryptotypes.PubKey) (KeyOutput, error) { //nolint:interfacer
 	apk, err := codectypes.NewAnyWithValue(pk)
 	if err != nil {
 		return KeyOutput{}, err
@@ -39,7 +39,7 @@ func NewKeyOutput(name string, keyType KeyType, a sdk.Address, pk cryptotypes.Pu
 }
 
 // MkConsKeyOutput create a KeyOutput in with "cons" Bech32 prefixes.
-func MkConsKeyOutput(k *Record) (KeyOutput, error) {
+func MkConsKeyOutput(k *keyring.Record) (KeyOutput, error) {
 	pk, err := k.GetPubKey()
 	if err != nil {
 		return KeyOutput{}, err
@@ -49,7 +49,7 @@ func MkConsKeyOutput(k *Record) (KeyOutput, error) {
 }
 
 // MkValKeyOutput create a KeyOutput in with "val" Bech32 prefixes.
-func MkValKeyOutput(k *Record) (KeyOutput, error) {
+func MkValKeyOutput(k *keyring.Record) (KeyOutput, error) {
 	pk, err := k.GetPubKey()
 	if err != nil {
 		return KeyOutput{}, err
@@ -63,7 +63,7 @@ func MkValKeyOutput(k *Record) (KeyOutput, error) {
 // MkAccKeyOutput create a KeyOutput in with "acc" Bech32 prefixes. If the
 // public key is a multisig public key, then the threshold and constituent
 // public keys will be added.
-func MkAccKeyOutput(k *Record) (KeyOutput, error) {
+func MkAccKeyOutput(k *keyring.Record) (KeyOutput, error) {
 	pk, err := k.GetPubKey()
 	if err != nil {
 		return KeyOutput{}, err
@@ -75,7 +75,7 @@ func MkAccKeyOutput(k *Record) (KeyOutput, error) {
 // MkAccKeysOutput returns a slice of KeyOutput objects, each with the "acc"
 // Bech32 prefixes, given a slice of Record objects. It returns an error if any
 // call to MkKeyOutput fails.
-func MkAccKeysOutput(records []*Record) ([]KeyOutput, error) {
+func MkAccKeysOutput(records []*keyring.Record) ([]KeyOutput, error) {
 	kos := make([]KeyOutput, len(records))
 	var err error
 	for i, r := range records {

crypto/keyring/output_test.go → client/keys/output_test.go

@@ -1,23 +1,35 @@
-package keyring
+package keys
 
 import (
 	"fmt"
+	"github.com/cosmos/cosmos-sdk/codec"
+	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
+	cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/cosmos/cosmos-sdk/crypto/keyring"
 	kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
 	"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
 	"github.com/cosmos/cosmos-sdk/crypto/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
+func generatePubKeys(n int) []types.PubKey {
+	pks := make([]types.PubKey, n)
+	for i := 0; i < n; i++ {
+		pks[i] = secp256k1.GenPrivKey().PubKey()
+	}
+	return pks
+}
+
 func TestBech32KeysOutput(t *testing.T) {
 	sk := secp256k1.PrivKey{Key: []byte{154, 49, 3, 117, 55, 232, 249, 20, 205, 216, 102, 7, 136, 72, 177, 2, 131, 202, 234, 81, 31, 208, 46, 244, 179, 192, 167, 163, 142, 117, 246, 13}}
 	tmpKey := sk.PubKey()
 	multisigPk := kmultisig.NewLegacyAminoPubKey(1, []types.PubKey{tmpKey})
 
-	k, err := NewMultiRecord("multisig", multisigPk)
+	k, err := keyring.NewMultiRecord("multisig", multisigPk)
 	require.NotNil(t, k)
 	require.NoError(t, err)
 	pubKey, err := k.GetPubKey()
@@ -31,3 +43,29 @@ func TestBech32KeysOutput(t *testing.T) {
 	require.Equal(t, expectedOutput, out)
 	require.Equal(t, "{Name:multisig Type:multi Address:cosmos1nf8lf6n4wa43rzmdzwe6hkrnw5guekhqt595cw PubKey:{\"@type\":\"/cosmos.crypto.multisig.LegacyAminoPubKey\",\"threshold\":1,\"public_keys\":[{\"@type\":\"/cosmos.crypto.secp256k1.PubKey\",\"key\":\"AurroA7jvfPd1AadmmOvWM2rJSwipXfRf8yD6pLbA2DJ\"}]} Mnemonic:}", fmt.Sprintf("%+v", out))
 }
+
+func TestProtoMarshalJSON(t *testing.T) {
+	require := require.New(t)
+	pubkeys := generatePubKeys(3)
+	msig := kmultisig.NewLegacyAminoPubKey(2, pubkeys)
+
+	registry := codectypes.NewInterfaceRegistry()
+	cryptocodec.RegisterInterfaces(registry)
+	cdc := codec.NewProtoCodec(registry)
+
+	bz, err := cdc.MarshalInterfaceJSON(msig)
+	require.NoError(err)
+
+	var pk2 types.PubKey
+	err = cdc.UnmarshalInterfaceJSON(bz, &pk2)
+	require.NoError(err)
+	require.True(pk2.Equals(msig))
+
+	// Test that we can correctly unmarshal key from output
+	k, err := keyring.NewMultiRecord("my multisig", msig)
+	require.NoError(err)
+	ko, err := MkAccKeyOutput(k)
+	require.NoError(err)
+	require.Equal(ko.Address, sdk.AccAddress(pk2.Address()).String())
+	require.Equal(ko.PubKey, string(bz))
+}

client/keys/show.go

@@ -200,11 +200,11 @@ func validateMultisigThreshold(k, nKeys int) error {
 func getBechKeyOut(bechPrefix string) (bechKeyOutFn, error) {
 	switch bechPrefix {
 	case sdk.PrefixAccount:
-		return keyring.MkAccKeyOutput, nil
+		return MkAccKeyOutput, nil
 	case sdk.PrefixValidator:
-		return keyring.MkValKeyOutput, nil
+		return MkValKeyOutput, nil
 	case sdk.PrefixConsensus:
-		return keyring.MkConsKeyOutput, nil
+		return MkConsKeyOutput, nil
 	}
 
 	return nil, fmt.Errorf("invalid Bech32 prefix encoding provided: %s", bechPrefix)

client/keys/show_test.go

@@ -207,9 +207,9 @@ func Test_getBechKeyOut(t *testing.T) {
 	}{
 		{"empty", args{""}, nil, true},
 		{"wrong", args{"???"}, nil, true},
-		{"acc", args{sdk.PrefixAccount}, keyring.MkAccKeyOutput, false},
-		{"val", args{sdk.PrefixValidator}, keyring.MkValKeyOutput, false},
-		{"cons", args{sdk.PrefixConsensus}, keyring.MkConsKeyOutput, false},
+		{"acc", args{sdk.PrefixAccount}, MkAccKeyOutput, false},
+		{"val", args{sdk.PrefixValidator}, MkValKeyOutput, false},
+		{"cons", args{sdk.PrefixConsensus}, MkConsKeyOutput, false},
 	}
 	for _, tt := range tests {
 		tt := tt

client/keys/utils.go

@@ -16,7 +16,7 @@ const (
 	OutputFormatJSON = "json"
 )
 
-type bechKeyOutFn func(k *cryptokeyring.Record) (cryptokeyring.KeyOutput, error)
+type bechKeyOutFn func(k *cryptokeyring.Record) (KeyOutput, error)
 
 func printKeyringRecord(w io.Writer, k *cryptokeyring.Record, bechKeyOut bechKeyOutFn, output string) error {
 	ko, err := bechKeyOut(k)
@@ -26,7 +26,7 @@ func printKeyringRecord(w io.Writer, k *cryptokeyring.Record, bechKeyOut bechKey
 
 	switch output {
 	case OutputFormatText:
-		if err := printTextRecords(w, []cryptokeyring.KeyOutput{ko}); err != nil {
+		if err := printTextRecords(w, []KeyOutput{ko}); err != nil {
 			return err
 		}
 
@@ -45,7 +45,7 @@ func printKeyringRecord(w io.Writer, k *cryptokeyring.Record, bechKeyOut bechKey
 }
 
 func printKeyringRecords(w io.Writer, records []*cryptokeyring.Record, output string) error {
-	kos, err := cryptokeyring.MkAccKeysOutput(records)
+	kos, err := MkAccKeysOutput(records)
 	if err != nil {
 		return err
 	}
@@ -57,7 +57,6 @@ func printKeyringRecords(w io.Writer, records []*cryptokeyring.Record, output st
 		}
 
 	case OutputFormatJSON:
-		// TODO https://github.com/cosmos/cosmos-sdk/issues/8046
 		out, err := json.Marshal(kos)
 		if err != nil {
 			return err
@@ -71,7 +70,7 @@ func printKeyringRecords(w io.Writer, records []*cryptokeyring.Record, output st
 	return nil
 }
 
-func printTextRecords(w io.Writer, kos []cryptokeyring.KeyOutput) error {
+func printTextRecords(w io.Writer, kos []KeyOutput) error {
 	out, err := yaml.Marshal(&kos)
 	if err != nil {
 		return err

crypto/keys/multisig/multisig_test.go

@@ -11,14 +11,12 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec/legacy"
 	"github.com/cosmos/cosmos-sdk/codec/types"
 	cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
-	"github.com/cosmos/cosmos-sdk/crypto/keyring"
 	kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
 	"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 	"github.com/cosmos/cosmos-sdk/crypto/types/multisig"
 	_ "github.com/cosmos/cosmos-sdk/runtime"
 	"github.com/cosmos/cosmos-sdk/testutil/configurator"
-	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/tx/signing"
 	"github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx"
 )
@@ -443,29 +441,3 @@ func TestAminoUnmarshalJSON(t *testing.T) {
 		require.NoError(t, err)
 	}
 }
-
-func TestProtoMarshalJSON(t *testing.T) {
-	require := require.New(t)
-	pubkeys := generatePubKeys(3)
-	msig := kmultisig.NewLegacyAminoPubKey(2, pubkeys)
-
-	registry := types.NewInterfaceRegistry()
-	cryptocodec.RegisterInterfaces(registry)
-	cdc := codec.NewProtoCodec(registry)
-
-	bz, err := cdc.MarshalInterfaceJSON(msig)
-	require.NoError(err)
-
-	var pk2 cryptotypes.PubKey
-	err = cdc.UnmarshalInterfaceJSON(bz, &pk2)
-	require.NoError(err)
-	require.True(pk2.Equals(msig))
-
-	// Test that we can correctly unmarshal key from keyring output
-	k, err := keyring.NewMultiRecord("my multisig", msig)
-	require.NoError(err)
-	ko, err := keyring.MkAccKeyOutput(k)
-	require.NoError(err)
-	require.Equal(ko.Address, sdk.AccAddress(pk2.Address()).String())
-	require.Equal(ko.PubKey, string(bz))
-}