From 24413a395d64666d63e58b1783e925e399dc48d9 Mon Sep 17 00:00:00 2001 From: Matthew Slipper Date: Tue, 25 Sep 2018 13:48:38 -0700 Subject: [PATCH] Merge PR #2073: Allow --from to be a name or an address * Allow --from to be a name or an address Closes #1735. * Post-rebase fixes * Updates from code review * Updates from code review * Updates from code review * Fix merge artifacts * Fix merge conflicts * Fix integration tests * Add back GetFromName() check broken during merge * Code review updates * Fix failing test * Updates from code review --- PENDING.md | 2 + client/context/context.go | 106 +++++++++++++++++++++++++----------- client/context/query.go | 22 ++------ client/flags.go | 2 +- client/utils/utils.go | 23 ++++++-- crypto/keys/keybase.go | 39 +++++++++++-- crypto/keys/keybase_test.go | 19 ++++++- crypto/keys/types.go | 17 +++++- types/address_test.go | 2 +- x/ibc/client/cli/relay.go | 13 ++++- 10 files changed, 178 insertions(+), 67 deletions(-) diff --git a/PENDING.md b/PENDING.md index 1b9238796c..95dcc7c47d 100644 --- a/PENDING.md +++ b/PENDING.md @@ -18,6 +18,8 @@ BREAKING CHANGES utilize a validator's operator address must now use the new Bech32 prefix, `cosmosvaloper`. * [cli] [\#2190](https://github.com/cosmos/cosmos-sdk/issues/2190) `gaiacli init --gen-txs` is now `gaiacli init --with-txs` to reduce confusion + * [cli] \#2073 --from can now be either an address or a key name + * Gaia * Make the transient store key use a distinct store key. [#2013](https://github.com/cosmos/cosmos-sdk/pull/2013) diff --git a/client/context/context.go b/client/context/context.go index 0497761c67..31cfab705a 100644 --- a/client/context/context.go +++ b/client/context/context.go @@ -16,6 +16,9 @@ import ( tmliteProxy "github.com/tendermint/tendermint/lite/proxy" rpcclient "github.com/tendermint/tendermint/rpc/client" "os" + "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/client/keys" + cskeys "github.com/cosmos/cosmos-sdk/crypto/keys" ) const ctxAccStoreName = "acc" @@ -23,22 +26,24 @@ const ctxAccStoreName = "acc" // CLIContext implements a typical CLI context created in SDK modules for // transaction handling and queries. type CLIContext struct { - Codec *codec.Codec - AccDecoder auth.AccountDecoder - Client rpcclient.Client - Logger io.Writer - Height int64 - NodeURI string - FromAddressName string - AccountStore string - TrustNode bool - UseLedger bool - Async bool - JSON bool - PrintResponse bool - Certifier tmlite.Certifier - DryRun bool - GenerateOnly bool + Codec *codec.Codec + AccDecoder auth.AccountDecoder + Client rpcclient.Client + Logger io.Writer + Height int64 + NodeURI string + From string + AccountStore string + TrustNode bool + UseLedger bool + Async bool + JSON bool + PrintResponse bool + Certifier tmlite.Certifier + DryRun bool + GenerateOnly bool + fromAddress types.AccAddress + fromName string } // NewCLIContext returns a new initialized CLIContext with parameters from the @@ -51,20 +56,25 @@ func NewCLIContext() CLIContext { rpc = rpcclient.NewHTTP(nodeURI, "/websocket") } + from := viper.GetString(client.FlagFrom) + fromAddress, fromName := fromFields(from) + return CLIContext{ - Client: rpc, - NodeURI: nodeURI, - AccountStore: ctxAccStoreName, - FromAddressName: viper.GetString(client.FlagFrom), - Height: viper.GetInt64(client.FlagHeight), - TrustNode: viper.GetBool(client.FlagTrustNode), - UseLedger: viper.GetBool(client.FlagUseLedger), - Async: viper.GetBool(client.FlagAsync), - JSON: viper.GetBool(client.FlagJson), - PrintResponse: viper.GetBool(client.FlagPrintResponse), - Certifier: createCertifier(), - DryRun: viper.GetBool(client.FlagDryRun), - GenerateOnly: viper.GetBool(client.FlagGenerateOnly), + Client: rpc, + NodeURI: nodeURI, + AccountStore: ctxAccStoreName, + From: viper.GetString(client.FlagFrom), + Height: viper.GetInt64(client.FlagHeight), + TrustNode: viper.GetBool(client.FlagTrustNode), + UseLedger: viper.GetBool(client.FlagUseLedger), + Async: viper.GetBool(client.FlagAsync), + JSON: viper.GetBool(client.FlagJson), + PrintResponse: viper.GetBool(client.FlagPrintResponse), + Certifier: createCertifier(), + DryRun: viper.GetBool(client.FlagDryRun), + GenerateOnly: viper.GetBool(client.FlagGenerateOnly), + fromAddress: fromAddress, + fromName: fromName, } } @@ -108,6 +118,37 @@ func createCertifier() tmlite.Certifier { return certifier } +func fromFields(from string) (fromAddr types.AccAddress, fromName string) { + if from == "" { + return nil, "" + } + + keybase, err := keys.GetKeyBase() + if err != nil { + fmt.Println("no keybase found") + os.Exit(1) + } + + var info cskeys.Info + if addr, err := types.AccAddressFromBech32(from); err == nil { + info, err = keybase.GetByAddress(addr) + if err != nil { + fmt.Printf("could not find key %s\n", from) + os.Exit(1) + } + } else { + info, err = keybase.Get(from) + if err != nil { + fmt.Printf("could not find key %s\n", from) + os.Exit(1) + } + } + + fromAddr = info.GetAddress() + fromName = info.GetName() + return +} + // WithCodec returns a copy of the context with an updated codec. func (ctx CLIContext) WithCodec(cdc *codec.Codec) CLIContext { ctx.Codec = cdc @@ -133,10 +174,9 @@ func (ctx CLIContext) WithAccountStore(accountStore string) CLIContext { return ctx } -// WithFromAddressName returns a copy of the context with an updated from -// address. -func (ctx CLIContext) WithFromAddressName(addrName string) CLIContext { - ctx.FromAddressName = addrName +// WithFrom returns a copy of the context with an updated from address or name. +func (ctx CLIContext) WithFrom(from string) CLIContext { + ctx.From = from return ctx } diff --git a/client/context/query.go b/client/context/query.go index 9eb70fd8cc..35402fbfbe 100644 --- a/client/context/query.go +++ b/client/context/query.go @@ -4,7 +4,6 @@ import ( "fmt" "io" - "github.com/cosmos/cosmos-sdk/client/keys" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth" @@ -84,22 +83,13 @@ func (ctx CLIContext) GetAccount(address []byte) (auth.Account, error) { } // GetFromAddress returns the from address from the context's name. -func (ctx CLIContext) GetFromAddress() (from sdk.AccAddress, err error) { - if ctx.FromAddressName == "" { - return nil, errors.Errorf("must provide a from address name") - } +func (ctx CLIContext) GetFromAddress() (sdk.AccAddress, error) { + return ctx.fromAddress, nil +} - keybase, err := keys.GetKeyBase() - if err != nil { - return nil, err - } - - info, err := keybase.Get(ctx.FromAddressName) - if err != nil { - return nil, errors.Errorf("no key for: %s", ctx.FromAddressName) - } - - return sdk.AccAddress(info.GetPubKey().Address()), nil +// GetFromName returns the key name for the current context. +func (ctx CLIContext) GetFromName() (string, error) { + return ctx.fromName, nil } // GetAccountNumber returns the next account number for the given account diff --git a/client/flags.go b/client/flags.go index 55b29b53ca..d0d783d3a7 100644 --- a/client/flags.go +++ b/client/flags.go @@ -58,7 +58,7 @@ func GetCommands(cmds ...*cobra.Command) []*cobra.Command { // PostCommands adds common flags for commands to post tx func PostCommands(cmds ...*cobra.Command) []*cobra.Command { for _, c := range cmds { - c.Flags().String(FlagFrom, "", "Name of private key with which to sign") + c.Flags().String(FlagFrom, "", "Name or address of private key with which to sign") c.Flags().Int64(FlagAccountNumber, 0, "AccountNumber number to sign the tx") c.Flags().Int64(FlagSequence, 0, "Sequence number to sign the tx") c.Flags().String(FlagMemo, "", "Memo to send along with transaction") diff --git a/client/utils/utils.go b/client/utils/utils.go index 81fd138595..76ee91585e 100644 --- a/client/utils/utils.go +++ b/client/utils/utils.go @@ -8,9 +8,9 @@ import ( "github.com/cosmos/cosmos-sdk/client/context" "github.com/cosmos/cosmos-sdk/client/keys" sdk "github.com/cosmos/cosmos-sdk/types" - auth "github.com/cosmos/cosmos-sdk/x/auth" + "github.com/cosmos/cosmos-sdk/x/auth" authtxb "github.com/cosmos/cosmos-sdk/x/auth/client/txbuilder" - amino "github.com/tendermint/go-amino" + "github.com/tendermint/go-amino" "github.com/tendermint/tendermint/libs/common" ) @@ -25,8 +25,13 @@ func SendTx(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, msgs []sdk.Msg) return err } + name, err := cliCtx.GetFromName() + if err != nil { + return err + } + if txBldr.SimulateGas || cliCtx.DryRun { - txBldr, err = EnrichCtxWithGas(txBldr, cliCtx, cliCtx.FromAddressName, msgs) + txBldr, err = EnrichCtxWithGas(txBldr, cliCtx, name, msgs) if err != nil { return err } @@ -36,13 +41,13 @@ func SendTx(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, msgs []sdk.Msg) return nil } - passphrase, err := keys.GetPassphrase(cliCtx.FromAddressName) + passphrase, err := keys.GetPassphrase(name) if err != nil { return err } // build and sign the transaction - txBytes, err := txBldr.BuildAndSign(cliCtx.FromAddressName, passphrase, msgs) + txBytes, err := txBldr.BuildAndSign(name, passphrase, msgs) if err != nil { return err } @@ -196,7 +201,13 @@ func buildUnsignedStdTx(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, msg return } if txBldr.SimulateGas { - txBldr, err = EnrichCtxWithGas(txBldr, cliCtx, cliCtx.FromAddressName, msgs) + var name string + name, err = cliCtx.GetFromName() + if err != nil { + return + } + + txBldr, err = EnrichCtxWithGas(txBldr, cliCtx, name, msgs) if err != nil { return } diff --git a/crypto/keys/keybase.go b/crypto/keys/keybase.go index 0bf7ad9903..c9bfd08129 100644 --- a/crypto/keys/keybase.go +++ b/crypto/keys/keybase.go @@ -14,6 +14,7 @@ import ( "github.com/tendermint/tendermint/crypto/encoding/amino" "github.com/tendermint/tendermint/crypto/secp256k1" dbm "github.com/tendermint/tendermint/libs/db" + "github.com/cosmos/cosmos-sdk/types" ) var _ Keybase = dbKeybase{} @@ -41,6 +42,8 @@ const ( French // Italian is currently not supported. Italian + addressSuffix = "address" + infoSuffix = "info" ) var ( @@ -179,11 +182,16 @@ func (kb dbKeybase) List() ([]Info, error) { iter := kb.db.Iterator(nil, nil) defer iter.Close() for ; iter.Valid(); iter.Next() { - info, err := readInfo(iter.Value()) - if err != nil { - return nil, err + key := string(iter.Key()) + + // need to include only keys in storage that have an info suffix + if strings.HasSuffix(key, infoSuffix) { + info, err := readInfo(iter.Value()) + if err != nil { + return nil, err + } + res = append(res, info) } - res = append(res, info) } return res, nil } @@ -197,6 +205,15 @@ func (kb dbKeybase) Get(name string) (Info, error) { return readInfo(bs) } +func (kb dbKeybase) GetByAddress(address types.AccAddress) (Info, error) { + ik := kb.db.Get(addrKey(address)) + if len(ik) == 0 { + return nil, fmt.Errorf("key with address %s not found", address) + } + bs := kb.db.Get(ik) + return readInfo(bs) +} + // Sign signs the msg with the named key. // It returns an error if the key doesn't exist or the decryption fails. func (kb dbKeybase) Sign(name, passphrase string, msg []byte) (sig []byte, pub tmcrypto.PubKey, err error) { @@ -347,6 +364,7 @@ func (kb dbKeybase) Delete(name, passphrase string) error { if err != nil { return err } + kb.db.DeleteSync(addrKey(linfo.GetAddress())) kb.db.DeleteSync(infoKey(name)) return nil case ledgerInfo: @@ -354,9 +372,11 @@ func (kb dbKeybase) Delete(name, passphrase string) error { if passphrase != "yes" { return fmt.Errorf("enter 'yes' exactly to delete the key - this cannot be undone") } + kb.db.DeleteSync(addrKey(info.GetAddress())) kb.db.DeleteSync(infoKey(name)) return nil } + return nil } @@ -413,9 +433,16 @@ func (kb dbKeybase) writeOfflineKey(pub tmcrypto.PubKey, name string) Info { func (kb dbKeybase) writeInfo(info Info, name string) { // write the info by key - kb.db.SetSync(infoKey(name), writeInfo(info)) + key := infoKey(name) + kb.db.SetSync(key, writeInfo(info)) + // store a pointer to the infokey by address for fast lookup + kb.db.SetSync(addrKey(info.GetAddress()), key) +} + +func addrKey(address types.AccAddress) []byte { + return []byte(fmt.Sprintf("%s.%s", address.String(), addressSuffix)) } func infoKey(name string) []byte { - return []byte(fmt.Sprintf("%s.info", name)) + return []byte(fmt.Sprintf("%s.%s", name, infoSuffix)) } diff --git a/crypto/keys/keybase_test.go b/crypto/keys/keybase_test.go index 652a36bcb0..2a602461a1 100644 --- a/crypto/keys/keybase_test.go +++ b/crypto/keys/keybase_test.go @@ -11,6 +11,7 @@ import ( "github.com/tendermint/tendermint/crypto/ed25519" dbm "github.com/tendermint/tendermint/libs/db" + "github.com/cosmos/cosmos-sdk/types" ) func init() { @@ -20,8 +21,9 @@ func init() { // TestKeyManagement makes sure we can manipulate these keys well func TestKeyManagement(t *testing.T) { // make the storage with reasonable defaults + db := dbm.NewMemDB() cstore := New( - dbm.NewMemDB(), + db, ) algo := Secp256k1 @@ -51,6 +53,12 @@ func TestKeyManagement(t *testing.T) { require.NoError(t, err) _, err = cstore.Get(n3) require.NotNil(t, err) + _, err = cstore.GetByAddress(accAddr(i2)) + require.NoError(t, err) + addr, err := types.AccAddressFromBech32("cosmos1yq8lgssgxlx9smjhes6ryjasmqmd3ts2559g0t") + require.NoError(t, err) + _, err = cstore.GetByAddress(addr) + require.NotNil(t, err) // list shows them in order keyS, err := cstore.List() @@ -92,6 +100,11 @@ func TestKeyManagement(t *testing.T) { keyS, err = cstore.List() require.NoError(t, err) require.Equal(t, 1, len(keyS)) + + // addr cache gets nuked + err = cstore.Delete(n2, p2) + require.NoError(t, err) + require.False(t, db.Has(addrKey(i2.GetAddress()))) } // TestSignVerify does some detailed checks on how we sign and validate @@ -387,3 +400,7 @@ func ExampleNew() { // Carl // signed by Bob } + +func accAddr(info Info) types.AccAddress { + return (types.AccAddress)(info.GetPubKey().Address()) +} \ No newline at end of file diff --git a/crypto/keys/types.go b/crypto/keys/types.go index a686264897..c5e97d5fba 100644 --- a/crypto/keys/types.go +++ b/crypto/keys/types.go @@ -5,14 +5,15 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/cosmos/cosmos-sdk/crypto/keys/hd" + "github.com/cosmos/cosmos-sdk/types" ) // Keybase exposes operations on a generic keystore type Keybase interface { - // CRUD on the keystore List() ([]Info, error) Get(name string) (Info, error) + GetByAddress(address types.AccAddress) (Info, error) Delete(name, passphrase string) error // Sign some bytes, looking up the private key to use @@ -73,6 +74,8 @@ type Info interface { GetName() string // Public key GetPubKey() crypto.PubKey + // Address + GetAddress() types.AccAddress } var _ Info = &localInfo{} @@ -106,6 +109,10 @@ func (i localInfo) GetPubKey() crypto.PubKey { return i.PubKey } +func (i localInfo) GetAddress() types.AccAddress { + return i.PubKey.Address().Bytes() +} + // ledgerInfo is the public information about a Ledger key type ledgerInfo struct { Name string `json:"name"` @@ -133,6 +140,10 @@ func (i ledgerInfo) GetPubKey() crypto.PubKey { return i.PubKey } +func (i ledgerInfo) GetAddress() types.AccAddress { + return i.PubKey.Address().Bytes() +} + // offlineInfo is the public information about an offline key type offlineInfo struct { Name string `json:"name"` @@ -158,6 +169,10 @@ func (i offlineInfo) GetPubKey() crypto.PubKey { return i.PubKey } +func (i offlineInfo) GetAddress() types.AccAddress { + return i.PubKey.Address().Bytes() +} + // encoding info func writeInfo(i Info) []byte { return cdc.MustMarshalBinary(i) diff --git a/types/address_test.go b/types/address_test.go index e2ec36876c..6c6c78d6ec 100644 --- a/types/address_test.go +++ b/types/address_test.go @@ -10,7 +10,7 @@ import ( "github.com/tendermint/tendermint/crypto/ed25519" "github.com/cosmos/cosmos-sdk/types" -) + ) var invalidStrs = []string{ "", diff --git a/x/ibc/client/cli/relay.go b/x/ibc/client/cli/relay.go index a57fac7054..ab7168aca3 100644 --- a/x/ibc/client/cli/relay.go +++ b/x/ibc/client/cli/relay.go @@ -94,7 +94,11 @@ func (c relayCommander) runIBCRelay(cmd *cobra.Command, args []string) { func (c relayCommander) loop(fromChainID, fromChainNode, toChainID, toChainNode string) { cliCtx := context.NewCLIContext() - passphrase, err := keys.ReadPassphraseFromStdin(cliCtx.FromAddressName) + name, err := cliCtx.GetFromName() + if err != nil { + panic(err) + } + passphrase, err := keys.ReadPassphraseFromStdin(name) if err != nil { panic(err) } @@ -201,7 +205,12 @@ func (c relayCommander) refine(bz []byte, sequence int64, passphrase string) []b txBldr := authtxb.NewTxBuilderFromCLI().WithSequence(sequence).WithCodec(c.cdc) cliCtx := context.NewCLIContext() - res, err := txBldr.BuildAndSign(cliCtx.FromAddressName, passphrase, []sdk.Msg{msg}) + name, err := cliCtx.GetFromName() + if err != nil { + panic(err) + } + + res, err := txBldr.BuildAndSign(name, passphrase, []sdk.Msg{msg}) if err != nil { panic(err) }