Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: key presentation outside keyring #14151

Merged
merged 6 commits into from
Dec 7, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
* (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.
* [13882] (https://github.com/cosmos/cosmos-sdk/pull/13882) Add tx `encode` and `decode` endpoints to amino tx service.
Expand Down Expand Up @@ -126,6 +127,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`.
Expand Down
4 changes: 2 additions & 2 deletions client/keys/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}
Expand Down
14 changes: 7 additions & 7 deletions crypto/keyring/output.go → client/keys/output.go
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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 {
Expand Down
42 changes: 40 additions & 2 deletions crypto/keyring/output_test.go → client/keys/output_test.go
Original file line number Diff line number Diff line change
@@ -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()
Expand All @@ -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))
}
6 changes: 3 additions & 3 deletions client/keys/show.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions client/keys/show_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 4 additions & 5 deletions client/keys/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}

Expand All @@ -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
}
Expand All @@ -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
Expand All @@ -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
Expand Down
28 changes: 0 additions & 28 deletions crypto/keys/multisig/multisig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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))
}