Enter the new keyring interface (#5904)

crypto/keyring:

`Keybase` interface gives way to its successor: `Keyring`. `LegacyKeybase`
interface is added in order to guarantee limited backward compatibility with
the old `Keybase` interface for the sole purpose of migrating keys across
the new keyring backends.

The package no longer depends on the `github.com/types.Config`
singleton.

`SupportedAlgos` and `SupportedLedgerAlgos` methods have been removed.
The keyring just fails when trying to perform an action with an unsupported
algorithm.

crypto/ subdirs reorganization:

`crypto/keys/hd` was moved to `crypto/hd`, which now groups together
all HD wallets related types and utilities.

client/input:

* Removal of unnecessary `GetCheckPassword`, `PrintPrefixed` functions.
* `GetConfirmation`'s signature changed to take in a io.Writer for better integration
  with `cobra.Command` types.

client/context:

* In-memory keyring is allocated in the context when `--gen-only` flag is passed
  in. `GetFromFields` does no longer silently allocate a keyring, it takes one as
  argument.

Co-authored with @jgimeno

Co-authored-by: Jonathan Gimeno <jgimeno@gmail.com>

CHANGELOG.md

@@ -70,19 +70,22 @@ and provided directly the IAVL store.
 * (modules) [\#5572](https://github.com/cosmos/cosmos-sdk/pull/5572) Move account balance logic and APIs from `x/auth` to `x/bank`.
 * (types) [\#5533](https://github.com/cosmos/cosmos-sdk/pull/5533) Refactored `AppModuleBasic` and `AppModuleGenesis`
 to now accept a `codec.JSONMarshaler` for modular serialization of genesis state.
-* (crypto/keyring) [\#5735](https://github.com/cosmos/cosmos-sdk/pull/5735) Keyring's `Update()` function is now no-op.
 * (types/rest) [\#5779](https://github.com/cosmos/cosmos-sdk/pull/5779) Drop unused Parse{Int64OrReturnBadRequest,QueryParamBool}() functions.
 * (keys) [\#5820](https://github.com/cosmos/cosmos-sdk/pull/5820/) Removed method CloseDB from Keybase interface.
 * (baseapp) [\#5837](https://github.com/cosmos/cosmos-sdk/issues/5837) Transaction simulation now returns a `SimulationResponse` which contains the `GasInfo` and
 `Result` from the execution.
-* (crypto/keyring) [\#5866](https://github.com/cosmos/cosmos-sdk/pull/5866) Move `Keyring` and `Keybase` implementations and their associated types from `crypto/keys/` to `crypto/keyring/`.
-* (crypto) [\#5880](https://github.com/cosmos/cosmos-sdk/pull/5880) Merge `crypto/keys/mintkey` into `crypto`.
-* (crypto/keyring) [\#5858](https://github.com/cosmos/cosmos-sdk/pull/5858) Make Keyring store keys by name and address's hexbytes representation.
-* (crypto/keyring) [\#5889](https://github.com/cosmos/cosmos-sdk/pull/5889) Deprecate old keybase implementation:
-  - Remove `Update` from the `Keybase` interface.
-  - `NewKeyring()` now accepts a new backend: `MemoryBackend`.
-  - `New()` has been renamed to`NewLegacy()`, which now returns a `LegacyKeybase` type that only allows migration of keys from the legacy keybase to a new keyring.
+* (client/input) [\#5904](https://github.com/cosmos/cosmos-sdk/pull/5904) Removal of unnecessary `GetCheckPassword`, `PrintPrefixed` functions.
 * (client/keys) [\#5889](https://github.com/cosmos/cosmos-sdk/pull/5889) Rename `NewKeyBaseFromDir()` -> `NewLegacyKeyBaseFromDir()`.
+* (crypto) [\#5880](https://github.com/cosmos/cosmos-sdk/pull/5880) Merge `crypto/keys/mintkey` into `crypto`.
+* (crypto/hd) [\#5904](https://github.com/cosmos/cosmos-sdk/pull/5904) `crypto/keys/hd` moved to `crypto/hd`.
+* (crypto/keyring):
+  - [\#5866](https://github.com/cosmos/cosmos-sdk/pull/5866) Rename `crypto/keys/` to `crypto/keyring/`.
+  - [\#5904](https://github.com/cosmos/cosmos-sdk/pull/5904) `Keybase` -> `Keyring` interfaces migration. `LegacyKeybase` interface is added in order
+to guarantee limited backward compatibility with the old Keybase interface for the sole purpose of migrating keys across the new keyring backends. `NewLegacy`
+constructor is provided [\#5889](https://github.com/cosmos/cosmos-sdk/pull/5889) to allow for smooth migration of keys from the legacy LevelDB based implementation
+to new keyring backends. Plus, the package and the new keyring no longer depends on the sdk.Config singleton. Please consult the package documentation for more
+information on how to implement the new `Keyring` interface.
+  - [\#5858](https://github.com/cosmos/cosmos-sdk/pull/5858) Make Keyring store keys by name and address's hexbytes representation.
 
 ### Features
 
@@ -92,8 +95,8 @@ to now accept a `codec.JSONMarshaler` for modular serialization of genesis state
 
 * (types) [\#5741](https://github.com/cosmos/cosmos-sdk/issues/5741) Prevent ChainAnteDecorators() from panicking when empty AnteDecorator slice is supplied.
 * (modules) [\#5569](https://github.com/cosmos/cosmos-sdk/issues/5569) `InitGenesis`, for the relevant modules, now ensures module accounts exist.
-* (crypto/keyring) [\#5844](https://github.com/cosmos/cosmos-sdk/pull/5844) Keybase/Keyring `Sign()` methods no longer decode amino signatures
-when method receivers are offline/multisig keys.
+* (crypto/keyring) [\#5844](https://github.com/cosmos/cosmos-sdk/pull/5844) `Keyring.Sign()` methods no longer decode amino signatures when method receivers
+are offline/multisig keys.
 * (x/auth) [\#5892](https://github.com/cosmos/cosmos-sdk/pull/5892) Add `RegisterKeyTypeCodec` to register new
 types (eg. keys) to the `auth` module internal amino codec.
 * (rest) [\#5906](https://github.com/cosmos/cosmos-sdk/pull/5906) Fix an issue that make some REST calls panic when sending

client/context/context.go

@@ -25,8 +25,8 @@ type CLIContext struct {
 	Client        rpcclient.Client
 	ChainID       string
 	Marshaler     codec.Marshaler
-	Keybase       keyring.Keybase
 	Input         io.Reader
+	Keyring       keyring.Keyring
 	Output        io.Writer
 	OutputFormat  string
 	Height        int64
@@ -58,8 +58,19 @@ func NewCLIContextWithInputAndFrom(input io.Reader, from string) CLIContext {
 	var nodeURI string
 	var rpc rpcclient.Client
 
+	homedir := viper.GetString(flags.FlagHome)
 	genOnly := viper.GetBool(flags.FlagGenerateOnly)
-	fromAddress, fromName, err := GetFromFields(input, from, genOnly)
+	backend := viper.GetString(flags.FlagKeyringBackend)
+	if len(backend) == 0 {
+		backend = keyring.BackendMemory
+	}
+
+	keyring, err := newKeyringFromFlags(backend, homedir, input, genOnly)
+	if err != nil {
+		panic(fmt.Errorf("couldn't acquire keyring: %v", err))
+	}
+
+	fromAddress, fromName, err := GetFromFields(keyring, from, genOnly)
 	if err != nil {
 		fmt.Printf("failed to get from fields: %v\n", err)
 		os.Exit(1)
@@ -84,9 +95,10 @@ func NewCLIContextWithInputAndFrom(input io.Reader, from string) CLIContext {
 		Output:        os.Stdout,
 		NodeURI:       nodeURI,
 		From:          viper.GetString(flags.FlagFrom),
+		Keyring:       keyring,
 		OutputFormat:  viper.GetString(cli.OutputFlag),
 		Height:        viper.GetInt64(flags.FlagHeight),
-		HomeDir:       viper.GetString(flags.FlagHome),
+		HomeDir:       homedir,
 		TrustNode:     viper.GetBool(flags.FlagTrustNode),
 		UseLedger:     viper.GetBool(flags.FlagUseLedger),
 		BroadcastMode: viper.GetString(flags.FlagBroadcastMode),
@@ -129,6 +141,12 @@ func NewCLIContextWithInput(input io.Reader) CLIContext {
 	return NewCLIContextWithInputAndFrom(input, viper.GetString(flags.FlagFrom))
 }
 
+// WithKeyring returns a copy of the context with an updated keyring.
+func (ctx CLIContext) WithKeyring(k keyring.Keyring) CLIContext {
+	ctx.Keyring = k
+	return ctx
+}
+
 // WithInput returns a copy of the context with an updated input.
 func (ctx CLIContext) WithInput(r io.Reader) CLIContext {
 	ctx.Input = r
@@ -307,7 +325,7 @@ func (ctx CLIContext) PrintOutput(toPrint interface{}) error {
 // GetFromFields returns a from account address and Keybase name given either
 // an address or key name. If genOnly is true, only a valid Bech32 cosmos
 // address is returned.
-func GetFromFields(input io.Reader, from string, genOnly bool) (sdk.AccAddress, string, error) {
+func GetFromFields(kr keyring.Keyring, from string, genOnly bool) (sdk.AccAddress, string, error) {
 	if from == "" {
 		return nil, "", nil
 	}
@@ -321,24 +339,25 @@ func GetFromFields(input io.Reader, from string, genOnly bool) (sdk.AccAddress,
 		return addr, "", nil
 	}
 
-	keybase, err := keyring.NewKeyring(sdk.KeyringServiceName(),
-		viper.GetString(flags.FlagKeyringBackend), viper.GetString(flags.FlagHome), input)
-	if err != nil {
-		return nil, "", err
-	}
-
 	var info keyring.Info
 	if addr, err := sdk.AccAddressFromBech32(from); err == nil {
-		info, err = keybase.GetByAddress(addr)
+		info, err = kr.KeyByAddress(addr)
 		if err != nil {
 			return nil, "", err
 		}
 	} else {
-		info, err = keybase.Get(from)
+		info, err = kr.Key(from)
 		if err != nil {
 			return nil, "", err
 		}
 	}
 
 	return info.GetAddress(), info.GetName(), nil
 }
+
+func newKeyringFromFlags(backend, homedir string, input io.Reader, genOnly bool) (keyring.Keyring, error) {
+	if genOnly {
+		return keyring.New(sdk.KeyringServiceName(), keyring.BackendMemory, homedir, input)
+	}
+	return keyring.New(sdk.KeyringServiceName(), backend, homedir, input)
+}

client/context/context_test.go

@@ -1,21 +1,24 @@
-package context
+package context_test
 
 import (
+	"os"
 	"testing"
 
+	"github.com/cosmos/cosmos-sdk/crypto/keyring"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/require"
 
+	"github.com/cosmos/cosmos-sdk/client/context"
 	"github.com/cosmos/cosmos-sdk/client/flags"
 )
 
 func TestCLIContext_WithOffline(t *testing.T) {
 	viper.Set(flags.FlagOffline, true)
 	viper.Set(flags.FlagNode, "tcp://localhost:26657")
 
-	ctx := NewCLIContext()
+	ctx := context.NewCLIContext()
 	require.True(t, ctx.Offline)
 	require.Nil(t, ctx.Client)
 
@@ -24,7 +27,7 @@ func TestCLIContext_WithOffline(t *testing.T) {
 	viper.Set(flags.FlagOffline, false)
 	viper.Set(flags.FlagNode, "tcp://localhost:26657")
 
-	ctx = NewCLIContext()
+	ctx = context.NewCLIContext()
 	require.False(t, ctx.Offline)
 	require.NotNil(t, ctx.Client)
 }
@@ -59,10 +62,26 @@ func TestCLIContext_WithGenOnly(t *testing.T) {
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			ctx := NewCLIContextWithFrom(tt.from)
+			ctx := context.NewCLIContextWithFrom(tt.from)
 
 			require.Equal(t, tt.expectedFromAddr, ctx.FromAddress)
 			require.Equal(t, tt.expectedFromName, ctx.FromName)
 		})
 	}
 }
+
+func TestCLIContext_WithKeyring(t *testing.T) {
+	viper.Set(flags.FlagGenerateOnly, true)
+	ctx := context.NewCLIContextWithFrom("cosmos1q7380u26f7ntke3facjmynajs4umlr329vr4ja")
+	require.NotNil(t, ctx.Keyring)
+	kr := ctx.Keyring
+	ctx = ctx.WithKeyring(nil)
+	require.Nil(t, ctx.Keyring)
+	ctx = ctx.WithKeyring(kr)
+	require.Equal(t, kr, ctx.Keyring)
+}
+
+func TestMain(m *testing.M) {
+	viper.Set(flags.FlagKeyringBackend, keyring.BackendMemory)
+	os.Exit(m.Run())
+}

client/input/input.go

@@ -2,8 +2,8 @@ package input
 
 import (
 	"bufio"
-	"errors"
 	"fmt"
+	"io"
 	"os"
 	"strings"
 
@@ -36,40 +36,15 @@ func GetPassword(prompt string, buf *bufio.Reader) (pass string, err error) {
 	return pass, nil
 }
 
-// GetCheckPassword will prompt for a password twice to verify they
-// match (for creating a new password).
-// It enforces the password length. Only parses password once if
-// input is piped in.
-func GetCheckPassword(prompt, prompt2 string, buf *bufio.Reader) (string, error) {
-	// simple read on no-tty
-	if !inputIsTty() {
-		return GetPassword(prompt, buf)
-	}
-
-	// TODO: own function???
-	pass, err := GetPassword(prompt, buf)
-	if err != nil {
-		return "", err
-	}
-	pass2, err := GetPassword(prompt2, buf)
-	if err != nil {
-		return "", err
-	}
-	if pass != pass2 {
-		return "", errors.New("passphrases don't match")
-	}
-	return pass, nil
-}
-
 // GetConfirmation will request user give the confirmation from stdin.
 // "y", "Y", "yes", "YES", and "Yes" all count as confirmations.
 // If the input is not recognized, it returns false and a nil error.
-func GetConfirmation(prompt string, buf *bufio.Reader) (bool, error) {
+func GetConfirmation(prompt string, r *bufio.Reader, w io.Writer) (bool, error) {
 	if inputIsTty() {
 		fmt.Printf("%s [y/N]: ", prompt)
 	}
 
-	response, err := readLineFromBuf(buf)
+	response, err := readLineFromBuf(r)
 	if err != nil {
 		return false, err
 	}
@@ -90,7 +65,7 @@ func GetConfirmation(prompt string, buf *bufio.Reader) (bool, error) {
 // GetString simply returns the trimmed string output of a given reader.
 func GetString(prompt string, buf *bufio.Reader) (string, error) {
 	if inputIsTty() && prompt != "" {
-		PrintPrefixed(prompt)
+		fmt.Fprintf(os.Stderr, "> %s\n", prompt)
 	}
 
 	out, err := readLineFromBuf(buf)
@@ -117,9 +92,3 @@ func readLineFromBuf(buf *bufio.Reader) (string, error) {
 	}
 	return strings.TrimSpace(pass), nil
 }
-
-// PrintPrefixed prints a string with > prefixed for use in prompts.
-func PrintPrefixed(msg string) {
-	msg = fmt.Sprintf("> %s\n", msg)
-	fmt.Fprint(os.Stderr, msg)
-}