From 47a44fb580372c395341e34e699d628351e39ca8 Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Fri, 1 Mar 2019 16:29:33 -0500 Subject: [PATCH] Merge PR #3748: Multisig Display UX Improvements --- PENDING.md | 10 ++- client/keys/add.go | 6 +- client/keys/codec_test.go | 28 ++++---- client/keys/show.go | 53 ++++++-------- client/keys/show_test.go | 21 +++--- client/keys/types.go | 7 -- client/keys/utils.go | 85 +++++++---------------- client/lcd/test_helpers.go | 11 +-- cmd/gaia/cli_test/cli_test.go | 4 +- cmd/gaia/cli_test/test_helpers.go | 5 +- cmd/gaia/init/gentx.go | 4 +- crypto/keys/codec.go | 6 +- crypto/keys/keybase.go | 52 ++++++++++---- crypto/keys/lazy_keybase.go | 10 +++ crypto/keys/output.go | 111 ++++++++++++++++++++++++++++++ crypto/keys/types.go | 67 ++++++++++++++++-- crypto/keys/types_test.go | 5 +- docs/gaia/gaiacli.md | 31 ++++++--- x/auth/client/cli/multisign.go | 5 +- x/auth/client/cli/sign.go | 53 ++++++++++---- 20 files changed, 387 insertions(+), 187 deletions(-) create mode 100644 crypto/keys/output.go diff --git a/PENDING.md b/PENDING.md index 4e2703229..b1e9760b1 100644 --- a/PENDING.md +++ b/PENDING.md @@ -58,8 +58,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 +* [\#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 diff --git a/client/keys/add.go b/client/keys/add.go index 268cd9f0e..c11316d1d 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -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 } @@ -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 { @@ -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 } diff --git a/client/keys/codec_test.go b/client/keys/codec_test.go index 3af946e59..56f12471d 100644 --- a/client/keys/codec_test.go +++ b/client/keys/codec_test.go @@ -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() diff --git a/client/keys/show.go b/client/keys/show.go index 1dbd8c34a..e1d253cd3 100644 --- a/client/keys/show.go +++ b/client/keys/show.go @@ -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" @@ -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 } @@ -79,6 +68,7 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) { if err != nil { return err } + pks[i] = info.GetPubKey() } @@ -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) @@ -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) } @@ -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) diff --git a/client/keys/show_test.go b/client/keys/show_test.go index bb44fd471..f12efb7c5 100644 --- a/client/keys/show_test.go +++ b/client/keys/show_test.go @@ -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) { @@ -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) { diff --git a/client/keys/types.go b/client/keys/types.go index d4a1032c2..75ecabe83 100644 --- a/client/keys/types.go +++ b/client/keys/types.go @@ -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 { diff --git a/client/keys/utils.go b/client/keys/utils.go index 976654ba2..5391e704f 100644 --- a/client/keys/utils.go +++ b/client/keys/utils.go @@ -9,7 +9,6 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/crypto/keys" - sdk "github.com/cosmos/cosmos-sdk/types" ) // available output formats. @@ -21,7 +20,7 @@ const ( defaultKeyDBName = "keys" ) -type bechKeyOutFn func(keyInfo keys.Info) (KeyOutput, error) +type bechKeyOutFn func(keyInfo keys.Info) (keys.KeyOutput, error) // GetKeyInfo returns key info for a given name. An error is returned if the // keybase cannot be retrieved or getting the info fails. @@ -90,68 +89,22 @@ func getLazyKeyBaseFromDir(rootDir string) (keys.Keybase, error) { return keys.New(defaultKeyDBName, filepath.Join(rootDir, "keys")), nil } -// create a list of KeyOutput in bech32 format -func Bech32KeysOutput(infos []keys.Info) ([]KeyOutput, error) { - kos := make([]KeyOutput, len(infos)) - for i, info := range infos { - ko, err := Bech32KeyOutput(info) - if err != nil { - return nil, err - } - kos[i] = ko - } - return kos, nil +func printKeyTextHeader() { + fmt.Printf("NAME:\tTYPE:\tADDRESS:\t\t\t\t\tPUBKEY:\n") } -// create a KeyOutput in bech32 format -func Bech32KeyOutput(info keys.Info) (KeyOutput, error) { - accAddr := sdk.AccAddress(info.GetPubKey().Address().Bytes()) - bechPubKey, err := sdk.Bech32ifyAccPub(info.GetPubKey()) - if err != nil { - return KeyOutput{}, err - } - - return KeyOutput{ - Name: info.GetName(), - Type: info.GetType().String(), - Address: accAddr.String(), - PubKey: bechPubKey, - }, nil +func printMultiSigKeyTextHeader() { + fmt.Printf("WEIGHT:\tTHRESHOLD:\tADDRESS:\t\t\t\t\tPUBKEY:\n") } -// Bech32ConsKeyOutput returns key output for a consensus node's key -// information. -func Bech32ConsKeyOutput(keyInfo keys.Info) (KeyOutput, error) { - consAddr := sdk.ConsAddress(keyInfo.GetPubKey().Address().Bytes()) - - bechPubKey, err := sdk.Bech32ifyConsPub(keyInfo.GetPubKey()) +func printMultiSigKeyInfo(keyInfo keys.Info, bechKeyOut bechKeyOutFn) { + ko, err := bechKeyOut(keyInfo) if err != nil { - return KeyOutput{}, err + panic(err) } - return KeyOutput{ - Name: keyInfo.GetName(), - Type: keyInfo.GetType().String(), - Address: consAddr.String(), - PubKey: bechPubKey, - }, nil -} - -// Bech32ValKeyOutput returns key output for a validator's key information. -func Bech32ValKeyOutput(keyInfo keys.Info) (KeyOutput, error) { - valAddr := sdk.ValAddress(keyInfo.GetPubKey().Address().Bytes()) - - bechPubKey, err := sdk.Bech32ifyValPub(keyInfo.GetPubKey()) - if err != nil { - return KeyOutput{}, err - } - - return KeyOutput{ - Name: keyInfo.GetName(), - Type: keyInfo.GetType().String(), - Address: valAddr.String(), - PubKey: bechPubKey, - }, nil + printMultiSigKeyTextHeader() + printMultiSigKeyOutput(ko) } func printKeyInfo(keyInfo keys.Info, bechKeyOut bechKeyOutFn) { @@ -162,8 +115,9 @@ func printKeyInfo(keyInfo keys.Info, bechKeyOut bechKeyOutFn) { switch viper.Get(cli.OutputFlag) { case OutputFormatText: - fmt.Printf("NAME:\tTYPE:\tADDRESS:\t\t\t\t\t\tPUBKEY:\n") + printKeyTextHeader() printKeyOutput(ko) + case "json": out, err := MarshalJSON(ko) if err != nil { @@ -175,29 +129,38 @@ func printKeyInfo(keyInfo keys.Info, bechKeyOut bechKeyOutFn) { } func printInfos(infos []keys.Info) { - kos, err := Bech32KeysOutput(infos) + kos, err := keys.Bech32KeysOutput(infos) if err != nil { panic(err) } + switch viper.Get(cli.OutputFlag) { case OutputFormatText: - fmt.Printf("NAME:\tTYPE:\tADDRESS:\t\t\t\t\t\tPUBKEY:\n") + printKeyTextHeader() for _, ko := range kos { printKeyOutput(ko) } + case OutputFormatJSON: out, err := MarshalJSON(kos) if err != nil { panic(err) } + fmt.Println(string(out)) } } -func printKeyOutput(ko KeyOutput) { +func printKeyOutput(ko keys.KeyOutput) { fmt.Printf("%s\t%s\t%s\t%s\n", ko.Name, ko.Type, ko.Address, ko.PubKey) } +func printMultiSigKeyOutput(ko keys.KeyOutput) { + for _, pk := range ko.PubKeys { + fmt.Printf("%d\t%d\t\t%s\t%s\n", pk.Weight, ko.Threshold, pk.Address, pk.PubKey) + } +} + func printKeyAddress(info keys.Info, bechKeyOut bechKeyOutFn) { ko, err := bechKeyOut(info) if err != nil { diff --git a/client/lcd/test_helpers.go b/client/lcd/test_helpers.go index 6d5694cdf..507c6af03 100644 --- a/client/lcd/test_helpers.go +++ b/client/lcd/test_helpers.go @@ -19,8 +19,9 @@ import ( "github.com/stretchr/testify/require" "github.com/cosmos/cosmos-sdk/client" - "github.com/cosmos/cosmos-sdk/client/keys" + clientkeys "github.com/cosmos/cosmos-sdk/client/keys" "github.com/cosmos/cosmos-sdk/client/utils" + "github.com/cosmos/cosmos-sdk/crypto/keys" "github.com/cosmos/cosmos-sdk/client/rpc" "github.com/cosmos/cosmos-sdk/client/tx" @@ -558,7 +559,7 @@ func getKeys(t *testing.T, port string) []keys.KeyOutput { // POST /keys Create a new account locally func doKeysPost(t *testing.T, port, name, password, mnemonic string, account int, index int) keys.KeyOutput { - pk := keys.AddNewKey{name, password, mnemonic, account, index} + pk := clientkeys.AddNewKey{name, password, mnemonic, account, index} req, err := cdc.MarshalJSON(pk) require.NoError(t, err) @@ -584,7 +585,7 @@ func getKeysSeed(t *testing.T, port string) string { // POST /keys/{name}/recove Recover a account from a seed func doRecoverKey(t *testing.T, port, recoverName, recoverPassword, mnemonic string, account uint32, index uint32) { - pk := keys.RecoverKey{recoverPassword, mnemonic, int(account), int(index)} + pk := clientkeys.RecoverKey{recoverPassword, mnemonic, int(account), int(index)} req, err := cdc.MarshalJSON(pk) require.NoError(t, err) @@ -612,7 +613,7 @@ func getKey(t *testing.T, port, name string) keys.KeyOutput { // PUT /keys/{name} Update the password for this account in the KMS func updateKey(t *testing.T, port, name, oldPassword, newPassword string, fail bool) { - kr := keys.UpdateKeyReq{oldPassword, newPassword} + kr := clientkeys.UpdateKeyReq{oldPassword, newPassword} req, err := cdc.MarshalJSON(kr) require.NoError(t, err) keyEndpoint := fmt.Sprintf("/keys/%s", name) @@ -626,7 +627,7 @@ func updateKey(t *testing.T, port, name, oldPassword, newPassword string, fail b // DELETE /keys/{name} Remove an account func deleteKey(t *testing.T, port, name, password string) { - dk := keys.DeleteKeyReq{password} + dk := clientkeys.DeleteKeyReq{password} req, err := cdc.MarshalJSON(dk) require.NoError(t, err) keyEndpoint := fmt.Sprintf("/keys/%s", name) diff --git a/cmd/gaia/cli_test/cli_test.go b/cmd/gaia/cli_test/cli_test.go index 0c270e587..3b4784fe2 100644 --- a/cmd/gaia/cli_test/cli_test.go +++ b/cmd/gaia/cli_test/cli_test.go @@ -701,7 +701,7 @@ func TestGaiaCLISendGenerateSignAndBroadcast(t *testing.T) { // Test sign --validate-signatures success, stdout, _ = f.TxSign(keyFoo, unsignedTxFile.Name(), "--validate-signatures") require.False(t, success) - require.Equal(t, fmt.Sprintf("Signers:\n 0: %v\n\nSignatures:\n\n", fooAddr.String()), stdout) + require.Equal(t, fmt.Sprintf("Signers:\n 0: %v\n\nSignatures:\n\n", fooAddr.String()), stdout) // Test sign success, stdout, _ = f.TxSign(keyFoo, unsignedTxFile.Name()) @@ -718,7 +718,7 @@ func TestGaiaCLISendGenerateSignAndBroadcast(t *testing.T) { // Test sign --validate-signatures success, stdout, _ = f.TxSign(keyFoo, signedTxFile.Name(), "--validate-signatures") require.True(t, success) - require.Equal(t, fmt.Sprintf("Signers:\n 0: %v\n\nSignatures:\n 0: %v\t[OK]\n\n", fooAddr.String(), + require.Equal(t, fmt.Sprintf("Signers:\n 0: %v\n\nSignatures:\n 0: %v\t\t\t[OK]\n\n", fooAddr.String(), fooAddr.String()), stdout) // Ensure foo has right amount of funds diff --git a/cmd/gaia/cli_test/test_helpers.go b/cmd/gaia/cli_test/test_helpers.go index 1d26c282c..21419ad96 100644 --- a/cmd/gaia/cli_test/test_helpers.go +++ b/cmd/gaia/cli_test/test_helpers.go @@ -14,10 +14,11 @@ import ( cmn "github.com/tendermint/tendermint/libs/common" - "github.com/cosmos/cosmos-sdk/client/keys" + clientkeys "github.com/cosmos/cosmos-sdk/client/keys" "github.com/cosmos/cosmos-sdk/cmd/gaia/app" appInit "github.com/cosmos/cosmos-sdk/cmd/gaia/init" "github.com/cosmos/cosmos-sdk/codec" + "github.com/cosmos/cosmos-sdk/crypto/keys" "github.com/cosmos/cosmos-sdk/server" "github.com/cosmos/cosmos-sdk/tests" sdk "github.com/cosmos/cosmos-sdk/types" @@ -260,7 +261,7 @@ func (f *Fixtures) KeysShow(name string, flags ...string) keys.KeyOutput { cmd := fmt.Sprintf("gaiacli keys show --home=%s %s", f.GCLIHome, name) out, _ := tests.ExecuteT(f.T, addFlags(cmd, flags), "") var ko keys.KeyOutput - err := keys.UnmarshalJSON([]byte(out), &ko) + err := clientkeys.UnmarshalJSON([]byte(out), &ko) require.NoError(f.T, err) return ko } diff --git a/cmd/gaia/init/gentx.go b/cmd/gaia/init/gentx.go index 8fd52fb5d..6ce4badec 100644 --- a/cmd/gaia/init/gentx.go +++ b/cmd/gaia/init/gentx.go @@ -135,8 +135,8 @@ following delegation and commission default parameters: return err } - if info.GetType() == kbkeys.TypeOffline { - fmt.Println("Offline key passed in. Use `gaiacli tx sign` command to sign:") + if info.GetType() == kbkeys.TypeOffline || info.GetType() == kbkeys.TypeMulti { + fmt.Println("Offline key passed in. Use `gaiacli tx sign` command to sign:") return utils.PrintUnsignedStdTx(txBldr, cliCtx, []sdk.Msg{msg}, true) } diff --git a/crypto/keys/codec.go b/crypto/keys/codec.go index 737836f91..d85eeb342 100644 --- a/crypto/keys/codec.go +++ b/crypto/keys/codec.go @@ -1,9 +1,10 @@ package keys import ( + amino "github.com/tendermint/go-amino" + cryptoAmino "github.com/tendermint/tendermint/crypto/encoding/amino" + "github.com/cosmos/cosmos-sdk/crypto/keys/hd" - "github.com/tendermint/go-amino" - "github.com/tendermint/tendermint/crypto/encoding/amino" ) var cdc = amino.NewCodec() @@ -15,4 +16,5 @@ func init() { cdc.RegisterConcrete(localInfo{}, "crypto/keys/localInfo", nil) cdc.RegisterConcrete(ledgerInfo{}, "crypto/keys/ledgerInfo", nil) cdc.RegisterConcrete(offlineInfo{}, "crypto/keys/offlineInfo", nil) + cdc.RegisterConcrete(multiInfo{}, "crypto/keys/multiInfo", nil) } diff --git a/crypto/keys/keybase.go b/crypto/keys/keybase.go index f79ed2025..e4ff5651c 100644 --- a/crypto/keys/keybase.go +++ b/crypto/keys/keybase.go @@ -7,7 +7,7 @@ import ( "reflect" "strings" - "errors" + "github.com/pkg/errors" "github.com/cosmos/cosmos-sdk/crypto" "github.com/cosmos/cosmos-sdk/crypto/keys/hd" @@ -15,10 +15,10 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/keys/mintkey" "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/go-bip39" + bip39 "github.com/cosmos/go-bip39" tmcrypto "github.com/tendermint/tendermint/crypto" - "github.com/tendermint/tendermint/crypto/encoding/amino" + cryptoAmino "github.com/tendermint/tendermint/crypto/encoding/amino" "github.com/tendermint/tendermint/crypto/secp256k1" dbm "github.com/tendermint/tendermint/libs/db" ) @@ -152,12 +152,18 @@ func (kb dbKeybase) CreateLedger(name string, algo SigningAlgo, account uint32, return kb.writeLedgerKey(name, pub, *hdPath), nil } -// CreateOffline creates a new reference to an offline keypair -// It returns the created key info +// CreateOffline creates a new reference to an offline keypair. It returns the +// created key info. func (kb dbKeybase) CreateOffline(name string, pub tmcrypto.PubKey) (Info, error) { return kb.writeOfflineKey(name, pub), nil } +// CreateMulti creates a new reference to a multisig (offline) keypair. It +// returns the created key info. +func (kb dbKeybase) CreateMulti(name string, pub tmcrypto.PubKey) (Info, error) { + return kb.writeMultisigKey(name, pub), nil +} + func (kb *dbKeybase) persistDerivedKey(seed []byte, passwd, name, fullHdPath string) (info Info, err error) { // create master key and derive first key: masterPriv, ch := hd.ComputeMastersFromSeed(seed) @@ -222,7 +228,9 @@ func (kb dbKeybase) Sign(name, passphrase string, msg []byte) (sig []byte, pub t if err != nil { return } + var priv tmcrypto.PrivKey + switch info.(type) { case localInfo: linfo := info.(localInfo) @@ -230,39 +238,49 @@ func (kb dbKeybase) Sign(name, passphrase string, msg []byte) (sig []byte, pub t err = fmt.Errorf("private key not available") return } + priv, err = mintkey.UnarmorDecryptPrivKey(linfo.PrivKeyArmor, passphrase) if err != nil { return nil, nil, err } + case ledgerInfo: linfo := info.(ledgerInfo) priv, err = crypto.NewPrivKeyLedgerSecp256k1(linfo.Path) if err != nil { return } - case offlineInfo: - linfo := info.(offlineInfo) - _, err := fmt.Fprintf(os.Stderr, "Bytes to sign:\n%s", msg) + + case offlineInfo, multiInfo: + _, err := fmt.Fprintf(os.Stderr, "Message to sign:\n\n%s\n", msg) if err != nil { return nil, nil, err } + buf := bufio.NewReader(os.Stdin) _, err = fmt.Fprintf(os.Stderr, "\nEnter Amino-encoded signature:\n") if err != nil { return nil, nil, err } + // Will block until user inputs the signature signed, err := buf.ReadString('\n') if err != nil { return nil, nil, err } - cdc.MustUnmarshalBinaryLengthPrefixed([]byte(signed), sig) - return sig, linfo.GetPubKey(), nil + + if err := cdc.UnmarshalBinaryLengthPrefixed([]byte(signed), sig); err != nil { + return nil, nil, errors.Wrap(err, "failed to decode signature") + } + + return sig, info.GetPubKey(), nil } + sig, err = priv.Sign(msg) if err != nil { return nil, nil, err } + pub = priv.PubKey() return sig, pub, nil } @@ -272,7 +290,9 @@ func (kb dbKeybase) ExportPrivateKeyObject(name string, passphrase string) (tmcr if err != nil { return nil, err } + var priv tmcrypto.PrivKey + switch info.(type) { case localInfo: linfo := info.(localInfo) @@ -284,11 +304,11 @@ func (kb dbKeybase) ExportPrivateKeyObject(name string, passphrase string) (tmcr if err != nil { return nil, err } - case ledgerInfo: - return nil, errors.New("only works on local private keys") - case offlineInfo: + + case ledgerInfo, offlineInfo, multiInfo: return nil, errors.New("only works on local private keys") } + return priv, nil } @@ -426,6 +446,12 @@ func (kb dbKeybase) writeOfflineKey(name string, pub tmcrypto.PubKey) Info { return info } +func (kb dbKeybase) writeMultisigKey(name string, pub tmcrypto.PubKey) Info { + info := NewMultiInfo(name, pub) + kb.writeInfo(name, info) + return info +} + func (kb dbKeybase) writeInfo(name string, info Info) { // write the info by key key := infoKey(name) diff --git a/crypto/keys/lazy_keybase.go b/crypto/keys/lazy_keybase.go index fd66a8774..00c13a817 100644 --- a/crypto/keys/lazy_keybase.go +++ b/crypto/keys/lazy_keybase.go @@ -127,6 +127,16 @@ func (lkb lazyKeybase) CreateOffline(name string, pubkey crypto.PubKey) (info In return newDbKeybase(db).CreateOffline(name, pubkey) } +func (lkb lazyKeybase) CreateMulti(name string, pubkey crypto.PubKey) (info Info, err error) { + db, err := dbm.NewGoLevelDB(lkb.name, lkb.dir) + if err != nil { + return nil, err + } + defer db.Close() + + return newDbKeybase(db).CreateMulti(name, pubkey) +} + func (lkb lazyKeybase) Update(name, oldpass string, getNewpass func() (string, error)) error { db, err := dbm.NewGoLevelDB(lkb.name, lkb.dir) if err != nil { diff --git a/crypto/keys/output.go b/crypto/keys/output.go new file mode 100644 index 000000000..22a614ddb --- /dev/null +++ b/crypto/keys/output.go @@ -0,0 +1,111 @@ +package keys + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// KeyOutput defines a structure wrapping around an Info object used for output +// functionality. +type KeyOutput struct { + Name string `json:"name"` + Type string `json:"type"` + Address string `json:"address"` + PubKey string `json:"pubkey"` + Mnemonic string `json:"mnemonic,omitempty"` + Threshold uint `json:"threshold,omitempty"` + PubKeys []multisigPubKeyOutput `json:"pubkeys,omitempty"` +} + +type multisigPubKeyOutput struct { + Address string `json:"address"` + PubKey string `json:"pubkey"` + Weight uint `json:"weight"` +} + +// Bech32KeysOutput returns a slice of KeyOutput objects, each with the "acc" +// Bech32 prefixes, given a slice of Info objects. It returns an error if any +// call to Bech32KeyOutput fails. +func Bech32KeysOutput(infos []Info) ([]KeyOutput, error) { + kos := make([]KeyOutput, len(infos)) + for i, info := range infos { + ko, err := Bech32KeyOutput(info) + if err != nil { + return nil, err + } + kos[i] = ko + } + + return kos, nil +} + +// Bech32ConsKeyOutput create a KeyOutput in with "cons" Bech32 prefixes. +func Bech32ConsKeyOutput(keyInfo Info) (KeyOutput, error) { + consAddr := sdk.ConsAddress(keyInfo.GetPubKey().Address().Bytes()) + + bechPubKey, err := sdk.Bech32ifyConsPub(keyInfo.GetPubKey()) + if err != nil { + return KeyOutput{}, err + } + + return KeyOutput{ + Name: keyInfo.GetName(), + Type: keyInfo.GetType().String(), + Address: consAddr.String(), + PubKey: bechPubKey, + }, nil +} + +// Bech32ValKeyOutput create a KeyOutput in with "val" Bech32 prefixes. +func Bech32ValKeyOutput(keyInfo Info) (KeyOutput, error) { + valAddr := sdk.ValAddress(keyInfo.GetPubKey().Address().Bytes()) + + bechPubKey, err := sdk.Bech32ifyValPub(keyInfo.GetPubKey()) + if err != nil { + return KeyOutput{}, err + } + + return KeyOutput{ + Name: keyInfo.GetName(), + Type: keyInfo.GetType().String(), + Address: valAddr.String(), + PubKey: bechPubKey, + }, nil +} + +// Bech32KeyOutput 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 Bech32KeyOutput(info Info) (KeyOutput, error) { + accAddr := sdk.AccAddress(info.GetPubKey().Address().Bytes()) + bechPubKey, err := sdk.Bech32ifyAccPub(info.GetPubKey()) + if err != nil { + return KeyOutput{}, err + } + + ko := KeyOutput{ + Name: info.GetName(), + Type: info.GetType().String(), + Address: accAddr.String(), + PubKey: bechPubKey, + } + + if mInfo, ok := info.(multiInfo); ok { + pubKeys := make([]multisigPubKeyOutput, len(mInfo.PubKeys)) + + for i, pk := range mInfo.PubKeys { + accAddr := sdk.AccAddress(pk.PubKey.Address().Bytes()) + + bechPubKey, err := sdk.Bech32ifyAccPub(pk.PubKey) + if err != nil { + return KeyOutput{}, err + } + + pubKeys[i] = multisigPubKeyOutput{accAddr.String(), bechPubKey, pk.Weight} + } + + ko.Threshold = mInfo.Threshold + ko.PubKeys = pubKeys + } + + return ko, nil +} diff --git a/crypto/keys/types.go b/crypto/keys/types.go index 09e732aa1..1459e1ae0 100644 --- a/crypto/keys/types.go +++ b/crypto/keys/types.go @@ -3,10 +3,11 @@ package keys import ( "fmt" + "github.com/tendermint/tendermint/crypto" + "github.com/tendermint/tendermint/crypto/multisig" + "github.com/cosmos/cosmos-sdk/crypto/keys/hd" "github.com/cosmos/cosmos-sdk/types" - - "github.com/tendermint/tendermint/crypto" ) // Keybase exposes operations on a generic keystore @@ -39,6 +40,9 @@ type Keybase interface { // CreateOffline creates, stores, and returns a new offline key reference CreateOffline(name string, pubkey crypto.PubKey) (info Info, err error) + // CreateMulti creates, stores, and returns a new multsig (offline) key reference + CreateMulti(name string, pubkey crypto.PubKey) (info Info, err error) + // The following operations will *only* work on locally-stored keys Update(name, oldpass string, getNewpass func() (string, error)) error Import(name string, armor string) (err error) @@ -61,12 +65,14 @@ const ( TypeLocal KeyType = 0 TypeLedger KeyType = 1 TypeOffline KeyType = 2 + TypeMulti KeyType = 3 ) var keyTypes = map[KeyType]string{ TypeLocal: "local", TypeLedger: "ledger", TypeOffline: "offline", + TypeMulti: "multi", } // String implements the stringer interface for KeyType. @@ -88,9 +94,12 @@ type Info interface { GetPath() (*hd.BIP44Params, error) } -var _ Info = &localInfo{} -var _ Info = &ledgerInfo{} -var _ Info = &offlineInfo{} +var ( + _ Info = &localInfo{} + _ Info = &ledgerInfo{} + _ Info = &offlineInfo{} + _ Info = &multiInfo{} +) // localInfo is the public information about a locally stored key type localInfo struct { @@ -196,6 +205,54 @@ func (i offlineInfo) GetPath() (*hd.BIP44Params, error) { return nil, fmt.Errorf("BIP44 Paths are not available for this type") } +type multisigPubKeyInfo struct { + PubKey crypto.PubKey `json:"pubkey"` + Weight uint `json:"weight"` +} +type multiInfo struct { + Name string `json:"name"` + PubKey crypto.PubKey `json:"pubkey"` + Threshold uint `json:"threshold"` + PubKeys []multisigPubKeyInfo `json:"pubkeys"` +} + +func NewMultiInfo(name string, pub crypto.PubKey) Info { + multiPK := pub.(multisig.PubKeyMultisigThreshold) + + pubKeys := make([]multisigPubKeyInfo, len(multiPK.PubKeys)) + for i, pk := range multiPK.PubKeys { + // TODO: Recursively check pk for total weight? + pubKeys[i] = multisigPubKeyInfo{pk, 1} + } + + return &multiInfo{ + Name: name, + PubKey: pub, + Threshold: multiPK.K, + PubKeys: pubKeys, + } +} + +func (i multiInfo) GetType() KeyType { + return TypeMulti +} + +func (i multiInfo) GetName() string { + return i.Name +} + +func (i multiInfo) GetPubKey() crypto.PubKey { + return i.PubKey +} + +func (i multiInfo) GetAddress() types.AccAddress { + return i.PubKey.Address().Bytes() +} + +func (i multiInfo) GetPath() (*hd.BIP44Params, error) { + return nil, fmt.Errorf("BIP44 Paths are not available for this type") +} + // encoding info func writeInfo(i Info) []byte { return cdc.MustMarshalBinaryLengthPrefixed(i) diff --git a/crypto/keys/types_test.go b/crypto/keys/types_test.go index 3124e484c..6475eb890 100644 --- a/crypto/keys/types_test.go +++ b/crypto/keys/types_test.go @@ -4,10 +4,11 @@ import ( "encoding/hex" "testing" - "github.com/cosmos/cosmos-sdk/crypto/keys/hd" - "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/assert" "github.com/tendermint/tendermint/crypto/secp256k1" + + "github.com/cosmos/cosmos-sdk/crypto/keys/hd" + "github.com/cosmos/cosmos-sdk/types" ) func Test_writeReadLedgerInfo(t *testing.T) { diff --git a/docs/gaia/gaiacli.md b/docs/gaia/gaiacli.md index b43f638be..198911f67 100644 --- a/docs/gaia/gaiacli.md +++ b/docs/gaia/gaiacli.md @@ -683,7 +683,7 @@ gaiacli query distr rewards Multisig transactions require signatures of multiple private keys. Thus, generating and signing a transaction from a multisig account involve cooperation among the parties involved. A multisig transaction can be initiated by any of the key holders, and at least one of them would need to -import other parties' public keys into their local database and generate a multisig public key +import other parties' public keys into their Keybase and generate a multisig public key in order to finalize and broadcast the transaction. For example, given a multisig key comprising the keys `p1`, `p2`, and `p3`, each of which is held @@ -692,17 +692,17 @@ generate the multisig account public key: ``` gaiacli keys add \ - --pubkey=cosmospub1addwnpepqtd28uwa0yxtwal5223qqr5aqf5y57tc7kk7z8qd4zplrdlk5ez5kdnlrj4 \ - p2 + p2 \ + --pubkey=cosmospub1addwnpepqtd28uwa0yxtwal5223qqr5aqf5y57tc7kk7z8qd4zplrdlk5ez5kdnlrj4 gaiacli keys add \ - --pubkey=cosmospub1addwnpepqgj04jpm9wrdml5qnss9kjxkmxzywuklnkj0g3a3f8l5wx9z4ennz84ym5t \ - p3 + p3 \ + --pubkey=cosmospub1addwnpepqgj04jpm9wrdml5qnss9kjxkmxzywuklnkj0g3a3f8l5wx9z4ennz84ym5t gaiacli keys add \ - --multisig-threshold=2 + p1p2p3 \ + --multisig-threshold=2 \ --multisig=p1,p2,p3 - p1p2p3 ``` A new multisig public key `p1p2p3` has been stored, and its address will be @@ -712,6 +712,15 @@ used as signer of multisig transactions: gaiacli keys show --address p1p2p3 ``` +You may also view multisig threshold, pubkey constituents and respective weights +by viewing the JSON output of the key or passing the `--show-multisig` flag: + +```bash +gaiacli keys show p1p2p3 -o json + +gaiacli keys show p1p2p3 --show-multisig +``` + The first step to create a multisig transaction is to initiate it on behalf of the multisig address created above: @@ -726,10 +735,10 @@ The file `unsignedTx.json` contains the unsigned transaction encoded in JSON. ```bash gaiacli tx sign \ + unsignedTx.json \ --multisig= \ - --name=p1 \ + --from=p1 \ --output-document=p1signature.json \ - unsignedTx.json ``` Once the signature is generated, `p1` transmits both `unsignedTx.json` and @@ -738,10 +747,10 @@ respective signature: ```bash gaiacli tx sign \ + unsignedTx.json \ --multisig= \ - --name=p2 \ + --from=p2 \ --output-document=p2signature.json \ - unsignedTx.json ``` `p1p2p3` is a 2-of-3 multisig key, therefore one additional signature diff --git a/x/auth/client/cli/multisign.go b/x/auth/client/cli/multisign.go index f1961a7a3..d25983a12 100644 --- a/x/auth/client/cli/multisign.go +++ b/x/auth/client/cli/multisign.go @@ -66,9 +66,8 @@ func makeMultiSignCmd(cdc *amino.Codec) func(cmd *cobra.Command, args []string) if err != nil { return } - if multisigInfo.GetType() != crkeys.TypeOffline { - return fmt.Errorf("%q must be of type offline: %s", - args[1], multisigInfo.GetType()) + if multisigInfo.GetType() != crkeys.TypeMulti { + return fmt.Errorf("%q must be of type %s: %s", args[1], crkeys.TypeMulti, multisigInfo.GetType()) } multisigPub := multisigInfo.GetPubKey().(multisig.PubKeyMultisigThreshold) diff --git a/x/auth/client/cli/sign.go b/x/auth/client/cli/sign.go index 3d6b9da5e..e30fd2e4f 100644 --- a/x/auth/client/cli/sign.go +++ b/x/auth/client/cli/sign.go @@ -3,10 +3,12 @@ package cli import ( "fmt" "os" + "strings" "github.com/spf13/cobra" "github.com/spf13/viper" amino "github.com/tendermint/go-amino" + "github.com/tendermint/tendermint/crypto/multisig" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/context" @@ -54,18 +56,21 @@ be generated via the 'multisign' command. Args: cobra.ExactArgs(1), } - cmd.Flags().String(flagMultisig, "", - "Address of the multisig account on behalf of which the "+ - "transaction shall be signed") - cmd.Flags().Bool(flagAppend, true, - "Append the signature to the existing ones. "+ - "If disabled, old signatures would be overwritten. Ignored if --multisig is on") + cmd.Flags().String( + flagMultisig, "", + "Address of the multisig account on behalf of which the transaction shall be signed", + ) + cmd.Flags().Bool( + flagAppend, true, + "Append the signature to the existing ones. If disabled, old signatures would be overwritten. Ignored if --multisig is on", + ) + cmd.Flags().Bool( + flagValidateSigs, false, + "Print the addresses that must sign the transaction, those who have already signed it, and make sure that signatures are in the correct order", + ) cmd.Flags().Bool(flagSigOnly, false, "Print only the generated signature, then exit") - cmd.Flags().Bool(flagValidateSigs, false, "Print the addresses that must sign the transaction, "+ - "those who have already signed it, and make sure that signatures are in the correct order") cmd.Flags().Bool(flagOffline, false, "Offline mode. Do not query a full node") - cmd.Flags().String(flagOutfile, "", - "The document will be written to the given file instead of STDOUT") + cmd.Flags().String(flagOutfile, "", "The document will be written to the given file instead of STDOUT") // add the flags here and return the command return client.PostCommands(cmd)[0] @@ -177,7 +182,7 @@ func printAndValidateSigs( signers := stdTx.GetSigners() for i, signer := range signers { - fmt.Printf(" %v: %v\n", i, signer.String()) + fmt.Printf(" %v: %v\n", i, signer.String()) } success := true @@ -194,6 +199,11 @@ func printAndValidateSigs( sigAddr := sdk.AccAddress(sig.Address()) sigSanity := "OK" + var ( + multiSigHeader string + multiSigMsg string + ) + if i >= len(signers) || !sigAddr.Equals(signers[i]) { sigSanity = "ERROR: signature does not match its respective signer" success = false @@ -219,7 +229,26 @@ func printAndValidateSigs( } } - fmt.Printf(" %v: %v\t[%s]\n", i, sigAddr.String(), sigSanity) + multiPK, ok := sig.PubKey.(multisig.PubKeyMultisigThreshold) + if ok { + var multiSig multisig.Multisignature + cliCtx.Codec.MustUnmarshalBinaryBare(sig.Signature, &multiSig) + + var b strings.Builder + b.WriteString("\n MultiSig Signatures:\n") + + for i := 0; i < multiSig.BitArray.Size(); i++ { + if multiSig.BitArray.GetIndex(i) { + addr := sdk.AccAddress(multiPK.PubKeys[i].Address().Bytes()) + b.WriteString(fmt.Sprintf(" %d: %s (weight: %d)\n", i, addr, 1)) + } + } + + multiSigHeader = fmt.Sprintf(" [multisig threshold: %d/%d]", multiPK.K, len(multiPK.PubKeys)) + multiSigMsg = b.String() + } + + fmt.Printf(" %d: %s\t\t\t[%s]%s%s\n", i, sigAddr.String(), sigSanity, multiSigHeader, multiSigMsg) } fmt.Println("")