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

R4R: Allow --from to be a name or an address #2073

Merged
merged 16 commits into from
Sep 25, 2018
Merged
Show file tree
Hide file tree
Changes from 12 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 PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,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
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved


* Gaia
* Make the transient store key use a distinct store key. [#2013](https://github.com/cosmos/cosmos-sdk/pull/2013)
Expand Down
76 changes: 39 additions & 37 deletions client/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,31 +15,34 @@ import (
tmlite "github.com/tendermint/tendermint/lite"
tmliteProxy "github.com/tendermint/tendermint/lite/proxy"
rpcclient "github.com/tendermint/tendermint/rpc/client"
"github.com/cosmos/cosmos-sdk/types"
)

const ctxAccStoreName = "acc"

// CLIContext implements a typical CLI context created in SDK modules for
// transaction handling and queries.
type CLIContext struct {
Codec *wire.Codec
AccDecoder auth.AccountDecoder
Client rpcclient.Client
Logger io.Writer
Height int64
Gas int64
GasAdjustment float64
NodeURI string
FromAddressName string
AccountStore string
TrustNode bool
UseLedger bool
Async bool
JSON bool
PrintResponse bool
Certifier tmlite.Certifier
DryRun bool
GenerateOnly bool
Codec *wire.Codec
AccDecoder auth.AccountDecoder
Client rpcclient.Client
Logger io.Writer
Height int64
Gas int64
GasAdjustment float64
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
Expand All @@ -53,21 +56,21 @@ func NewCLIContext() CLIContext {
}

return CLIContext{
Client: rpc,
NodeURI: nodeURI,
AccountStore: ctxAccStoreName,
FromAddressName: viper.GetString(client.FlagFrom),
Height: viper.GetInt64(client.FlagHeight),
Gas: viper.GetInt64(client.FlagGas),
GasAdjustment: viper.GetFloat64(client.FlagGasAdjustment),
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),
Gas: viper.GetInt64(client.FlagGas),
GasAdjustment: viper.GetFloat64(client.FlagGasAdjustment),
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),
}
}

Expand Down Expand Up @@ -126,10 +129,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
}

Expand Down
49 changes: 41 additions & 8 deletions client/context/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
tmliteProxy "github.com/tendermint/tendermint/lite/proxy"
rpcclient "github.com/tendermint/tendermint/rpc/client"
ctypes "github.com/tendermint/tendermint/rpc/core/types"
cskeys "github.com/cosmos/cosmos-sdk/crypto/keys"
)

// GetNode returns an RPC client. If the context's client is not defined, an
Expand Down Expand Up @@ -82,22 +83,54 @@ 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) {
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
if ctx.fromAddress == nil {
if err := ctx.populateFromFields(); err != nil {
return nil, err
}
}

return ctx.fromAddress, nil
}

// GetFromName returns the key name for the current context.
func (ctx CLIContext) GetFromName() (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing godoc

if ctx.fromName == "" {
if err := ctx.populateFromFields(); err != nil {
return "", err
}
}

return ctx.fromName, nil
}

// NOTE: mutates state so must be pointer receiver
func (ctx *CLIContext) populateFromFields() error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to think of another way implementing this functionality. The goal and point of contexts is that they are never modified, hence the ctx = ctx.With* logic. In our case here, maybe we'll need to to ctx = populateFromFields(ctx) somewhere upstream?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll figure out a solution without the pointer receiver.

if ctx.From == "" {
return errors.Errorf("must provide a from address or name")
}

keybase, err := keys.GetKeyBase()
if err != nil {
return nil, err
return err
}

info, err := keybase.Get(ctx.FromAddressName)
if err != nil {
return nil, errors.Errorf("no key for: %s", ctx.FromAddressName)
var info cskeys.Info
if addr, err := sdk.AccAddressFromBech32(ctx.From); err == nil {
info, err = keybase.GetByAddress(addr)
if err != nil {
return err
}
} else {
info, err = keybase.Get(ctx.From)
if err != nil {
return err
}
}

return sdk.AccAddress(info.GetPubKey().Address()), nil
ctx.fromAddress = info.GetAddress()
ctx.fromName = info.GetName()
return nil
}

// GetAccountNumber returns the next account number for the given account
Expand Down
2 changes: 1 addition & 1 deletion client/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,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")
Expand Down
24 changes: 18 additions & 6 deletions client/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -24,9 +24,15 @@ func SendTx(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, msgs []sdk.Msg)
if err != nil {
return err
}

name, err := cliCtx.GetFromName()
if err != nil {
return err
}

autogas := cliCtx.DryRun || (cliCtx.Gas == 0)
if autogas {
txBldr, err = EnrichCtxWithGas(txBldr, cliCtx, cliCtx.FromAddressName, msgs)
txBldr, err = EnrichCtxWithGas(txBldr, cliCtx, name, msgs)
if err != nil {
return err
}
Expand All @@ -36,13 +42,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
}
Expand Down Expand Up @@ -195,7 +201,13 @@ func buildUnsignedStdTx(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, msg
return
}
if txBldr.Gas == 0 {
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
}
Expand Down
39 changes: 33 additions & 6 deletions crypto/keys/keybase.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -41,6 +42,8 @@ const (
French
// Italian is currently not supported.
Italian
addressSuffix = "address"
infoSuffix = "info"
)

var (
Expand Down Expand Up @@ -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
}
Expand All @@ -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.String())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to actually use String() here as Go will automatically do this for you (if the type implements Stringer) 👍

}
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) {
Expand Down Expand Up @@ -347,16 +364,19 @@ 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:
case offlineInfo:
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
}

Expand Down Expand Up @@ -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))
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
}

func infoKey(name string) []byte {
return []byte(fmt.Sprintf("%s.info", name))
return []byte(fmt.Sprintf("%s.%s", name, infoSuffix))
}
19 changes: 18 additions & 1 deletion crypto/keys/keybase_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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
Expand Down Expand Up @@ -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("cosmosaccaddr1jawd35d9aq4u76sr3fjalmcqc8hqygs9gtnmv3")
require.NoError(t, err)
_, err = cstore.GetByAddress(addr)
require.NotNil(t, err)

// list shows them in order
keyS, err := cstore.List()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -387,3 +400,7 @@ func ExampleNew() {
// Carl
// signed by Bob
}

func accAddr(info Info) types.AccAddress {
return (types.AccAddress)(info.GetPubKey().Address())
}
Loading