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

Multisig Display UX Improvements #3748

Merged
merged 20 commits into from
Mar 1, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 8 additions & 2 deletions PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,14 @@ decoded automatically.

* [\#3653] Prompt user confirmation prior to signing and broadcasting a transaction.
* [\#3670] CLI support for showing bech32 addresses in Ledger devices
* [\#3711] Update `tx sign` to use `--from` instead of the deprecated `--name` CLI flag.
* [\#3730](https://github.com/cosmos/cosmos-sdk/issues/3730) Improve workflow for `gaiad gentx` with offline public keys, by outputting stdtx file that needs to be signed.
* [\#3711] Update `tx sign` to use `--from` instead of the deprecated `--name`
CLI flag.
* [\#3738] Improve multisig UX:
* `gaiacli keys show -o json` now includes constituent pubkeys, respective weights and threshold
* `gaiacli keys show --show-multisig` now displays constituent pubkeys, respective weights and threshold
* `gaiacli tx sign --validate-signatures` now displays multisig signers with their respective weights
Copy link
Member

Choose a reason for hiding this comment

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

💪 ++

Copy link
Contributor

Choose a reason for hiding this comment

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

-o json implies --show-multisig then, doesn't it?

Copy link
Member

Choose a reason for hiding this comment

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

yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A user can either 1. -o json which will show multi-sig info OR 2. --show-multisig which will show multisig info in plaintext (non-JSON).

* [\#3730](https://github.com/cosmos/cosmos-sdk/issues/3730) Improve workflow for
`gaiad gentx` with offline public keys, by outputting stdtx file that needs to be signed.
* [\#3761](https://github.com/cosmos/cosmos-sdk/issues/3761) Querying account related information using custom querier in auth module

### Gaia
Expand Down
6 changes: 3 additions & 3 deletions client/keys/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func runAddCmd(_ *cobra.Command, args []string) error {
}

pk := multisig.NewPubKeyMultisigThreshold(multisigThreshold, pks)
if _, err := kb.CreateOffline(name, pk); err != nil {
if _, err := kb.CreateMulti(name, pk); err != nil {
return err
}

Expand Down Expand Up @@ -263,7 +263,7 @@ func printCreate(info keys.Info, showMnemonic bool, mnemonic string) error {
switch output {
case OutputFormatText:
fmt.Fprintln(os.Stderr)
printKeyInfo(info, Bech32KeyOutput)
printKeyInfo(info, keys.Bech32KeyOutput)

// print mnemonic unless requested not to.
if showMnemonic {
Expand All @@ -273,7 +273,7 @@ func printCreate(info keys.Info, showMnemonic bool, mnemonic string) error {
fmt.Fprintln(os.Stderr, mnemonic)
}
case OutputFormatJSON:
out, err := Bech32KeyOutput(info)
out, err := keys.Bech32KeyOutput(info)
if err != nil {
return err
}
Expand Down
28 changes: 15 additions & 13 deletions client/keys/codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,37 @@ import (
"testing"

"github.com/stretchr/testify/require"

"github.com/cosmos/cosmos-sdk/crypto/keys"
)

type testCases struct {
Keys []KeyOutput
Answers []KeyOutput
Keys []keys.KeyOutput
Answers []keys.KeyOutput
JSON [][]byte
}

func getTestCases() testCases {
return testCases{
[]KeyOutput{
{"A", "B", "C", "D", "E"},
{"A", "B", "C", "D", ""},
{"", "B", "C", "D", ""},
{"", "", "", "", ""},
[]keys.KeyOutput{
{"A", "B", "C", "D", "E", 0, nil},
{"A", "B", "C", "D", "", 0, nil},
{"", "B", "C", "D", "", 0, nil},
{"", "", "", "", "", 0, nil},
},
make([]KeyOutput, 4),
make([]keys.KeyOutput, 4),
[][]byte{
[]byte(`{"name":"A","type":"B","address":"C","pub_key":"D","mnemonic":"E"}`),
[]byte(`{"name":"A","type":"B","address":"C","pub_key":"D"}`),
[]byte(`{"name":"","type":"B","address":"C","pub_key":"D"}`),
[]byte(`{"name":"","type":"","address":"","pub_key":""}`),
[]byte(`{"name":"A","type":"B","address":"C","pubkey":"D","mnemonic":"E"}`),
[]byte(`{"name":"A","type":"B","address":"C","pubkey":"D"}`),
[]byte(`{"name":"","type":"B","address":"C","pubkey":"D"}`),
[]byte(`{"name":"","type":"","address":"","pubkey":""}`),
},
}
}

func TestMarshalJSON(t *testing.T) {
type args struct {
o KeyOutput
o keys.KeyOutput
}

data := getTestCases()
Expand Down
53 changes: 22 additions & 31 deletions client/keys/show.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (

"github.com/cosmos/cosmos-sdk/crypto"
"github.com/cosmos/cosmos-sdk/crypto/keys"
"github.com/cosmos/cosmos-sdk/crypto/keys/hd"
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/spf13/cobra"
Expand All @@ -27,39 +26,29 @@ const (
// FlagBechPrefix defines a desired Bech32 prefix encoding for a key.
FlagDevice = "device"

flagMultiSigThreshold = "multisig-threshold"
flagMultiSigThreshold = "multisig-threshold"
flagShowMultiSig = "show-multisig"

defaultMultiSigKeyName = "multi"
)

var _ keys.Info = (*multiSigKey)(nil)

type multiSigKey struct {
name string
key tmcrypto.PubKey
}

func (m multiSigKey) GetName() string { return m.name }
func (m multiSigKey) GetType() keys.KeyType { return keys.TypeLocal }
func (m multiSigKey) GetPubKey() tmcrypto.PubKey { return m.key }
func (m multiSigKey) GetAddress() sdk.AccAddress { return sdk.AccAddress(m.key.Address()) }
func (m multiSigKey) GetPath() (*hd.BIP44Params, error) {
return nil, fmt.Errorf("BIP44 Paths are not available for this type")
}

func showKeysCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "show [name]",
Use: "show [name [name...]]",
Short: "Show key info for the given name",
Long: `Return public details of one local key.`,
Args: cobra.MinimumNArgs(1),
RunE: runShowCmd,
Long: `Return public details of a single local key. If multiple names are
provided, then an ephemeral multisig key will be created under the name "multi"
consisting of all the keys provided by name and multisig threshold.`,
Args: cobra.MinimumNArgs(1),
RunE: runShowCmd,
}

cmd.Flags().String(FlagBechPrefix, sdk.PrefixAccount, "The Bech32 prefix encoding for a key (acc|val|cons)")
cmd.Flags().BoolP(FlagAddress, "a", false, "output the address only (overrides --output)")
cmd.Flags().BoolP(FlagPublicKey, "p", false, "output the public key only (overrides --output)")
cmd.Flags().BoolP(FlagDevice, "d", false, "output the address in the device")
cmd.Flags().BoolP(FlagAddress, "a", false, "Output the address only (overrides --output)")
cmd.Flags().BoolP(FlagPublicKey, "p", false, "Output the public key only (overrides --output)")
cmd.Flags().BoolP(FlagDevice, "d", false, "Output the address in the device")
cmd.Flags().Uint(flagMultiSigThreshold, 1, "K out of N required signatures")
cmd.Flags().BoolP(flagShowMultiSig, "m", false, "Output multisig pubkey constituents, threshold, and weights")

return cmd
}
Expand All @@ -79,6 +68,7 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) {
if err != nil {
return err
}

pks[i] = info.GetPubKey()
}

Expand All @@ -87,16 +77,15 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) {
if err != nil {
return err
}

multikey := multisig.NewPubKeyMultisigThreshold(multisigThreshold, pks)
info = multiSigKey{
name: defaultMultiSigKeyName,
key: multikey,
}
info = keys.NewMultiInfo(defaultMultiSigKeyName, multikey)
}

isShowAddr := viper.GetBool(FlagAddress)
isShowPubKey := viper.GetBool(FlagPublicKey)
isShowDevice := viper.GetBool(FlagDevice)
isShowMultiSig := viper.GetBool(flagShowMultiSig)

isOutputSet := false
tmp := cmd.Flag(cli.OutputFlag)
Expand All @@ -122,6 +111,8 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) {
printKeyAddress(info, bechKeyOut)
case isShowPubKey:
printPubKey(info, bechKeyOut)
case isShowMultiSig:
printMultiSigKeyInfo(info, bechKeyOut)
default:
printKeyInfo(info, bechKeyOut)
}
Expand Down Expand Up @@ -163,11 +154,11 @@ func validateMultisigThreshold(k, nKeys int) error {
func getBechKeyOut(bechPrefix string) (bechKeyOutFn, error) {
switch bechPrefix {
case sdk.PrefixAccount:
return Bech32KeyOutput, nil
return keys.Bech32KeyOutput, nil
case sdk.PrefixValidator:
return Bech32ValKeyOutput, nil
return keys.Bech32ValKeyOutput, nil
case sdk.PrefixConsensus:
return Bech32ConsKeyOutput, nil
return keys.Bech32ConsKeyOutput, nil
}

return nil, fmt.Errorf("invalid Bech32 prefix encoding provided: %s", bechPrefix)
Expand Down
21 changes: 10 additions & 11 deletions client/keys/show_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,21 @@ import (
"github.com/spf13/viper"
"github.com/stretchr/testify/assert"

"github.com/tendermint/tendermint/crypto"
"github.com/tendermint/tendermint/crypto/multisig"
"github.com/tendermint/tendermint/crypto/secp256k1"
"github.com/tendermint/tendermint/libs/cli"
)

func Test_multiSigKey_Properties(t *testing.T) {
tmpKey1 := secp256k1.GenPrivKeySecp256k1([]byte("mySecret"))

tmp := multiSigKey{
name: "myMultisig",
key: tmpKey1.PubKey(),
}
pk := multisig.NewPubKeyMultisigThreshold(1, []crypto.PubKey{tmpKey1.PubKey()})
tmp := keys.NewMultiInfo("myMultisig", pk)

assert.Equal(t, "myMultisig", tmp.GetName())
assert.Equal(t, keys.TypeLocal, tmp.GetType())
assert.Equal(t, "015ABFFB09DB738A45745A91E8C401423ECE4016", tmp.GetPubKey().Address().String())
assert.Equal(t, "cosmos1q9dtl7cfmdec53t5t2g733qpgglvusqk6xdntl", tmp.GetAddress().String())
assert.Equal(t, keys.TypeMulti, tmp.GetType())
assert.Equal(t, "79BF2B5B418A85329EC2149D1854D443F56F5A9F", tmp.GetPubKey().Address().String())
assert.Equal(t, "cosmos10xljkk6p32zn98kzzjw3s4x5g06k7k5lz6flcv", tmp.GetAddress().String())
}

func Test_showKeysCmd(t *testing.T) {
Expand Down Expand Up @@ -134,9 +133,9 @@ func Test_getBechKeyOut(t *testing.T) {
}{
{"empty", args{""}, nil, true},
{"wrong", args{"???"}, nil, true},
{"acc", args{sdk.PrefixAccount}, Bech32KeyOutput, false},
{"val", args{sdk.PrefixValidator}, Bech32ValKeyOutput, false},
{"cons", args{sdk.PrefixConsensus}, Bech32ConsKeyOutput, false},
{"acc", args{sdk.PrefixAccount}, keys.Bech32KeyOutput, false},
{"val", args{sdk.PrefixValidator}, keys.Bech32ValKeyOutput, false},
{"cons", args{sdk.PrefixConsensus}, keys.Bech32ConsKeyOutput, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
7 changes: 0 additions & 7 deletions client/keys/types.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,6 @@
package keys

// used for outputting keys.Info over REST
type KeyOutput struct {
Name string `json:"name"`
Type string `json:"type"`
Address string `json:"address"`
PubKey string `json:"pub_key"`
Mnemonic string `json:"mnemonic,omitempty"`
}

// AddNewKey request a new key
type AddNewKey struct {
Expand Down
Loading