Multisig Display UX Improvements #3748

alexanderbez · 2019-02-26T17:29:58Z

Move output types and functionality from client/keys to crypto/keys
Add new multiInfo type which includes constituent pubkeys and respective weights and threshold
Remove duplicate code
Update and cleanup gaiacli docs wrt to multisigs
Update gaiacli tx sign --validate-signatures to include multisig info

closes: #3738

You can view the composition of a multisig via two ways:

gaiacli keys show multi1 -o json | prettyjson

{
    "name": "multi1",
    "type": "multi",
    "address": "cosmos1m4kcw5ns54jnxwuen0m6r4jf08km2kphksf393",
    "pubkey": "cosmospub1ytql0csgqgfzd666axrjzqn57z33n9cm2md9y477e05njsukmc4jl2ja4xyf85pf0mg8jtpxrufzd666axrjzqal2yzlnrux390dr9m0784e0vy0was084nsw8nzl8xkem8xjrj6jvfzd666axrjzql6qmmkde59y3tgdcq9ymk4cy3njn2gzaqmdftuzyrqr3xswcrrnyln9lzj",
    "threshold": "2",
    "pubkeys": [
        {
            "address": "cosmos1xa9gudtppp6zz27dvhl0v57ypcufcuq4guvavv",
            "pubkey": "cosmospub1addwnpepqf60pgcejud4dkjj2l0vh6fegwtdu2e04fw6nzyn6q5ha5re9snp7sumvka",
            "weight": "1"
        },
        {
            "address": "cosmos1dxuhzzf9xr977qexv79ze3yat6z6u34heea7x9",
            "pubkey": "cosmospub1addwnpepqwl4zp0e37rgjhk3jahlr6uhkz8hwc8n6ec8re30nntvannfpedfx87zzym",
            "weight": "1"
        },
        {
            "address": "cosmos1d057e8txsmhat75awtq73aq0x2enuazk77rk3r",
            "pubkey": "cosmospub1addwnpepq0aqdamxu6zjg45xuqzjdm2uzgeef4ypwsdk547pzpspcng8vp3ej6yzpwx",
            "weight": "1"
        }
    ]
}

gaiacli keys show multi1 --show-multisig

WEIGHT:	THRESHOLD:	ADDRESS:					PUBKEY:
1	2		cosmos1xa9gudtppp6zz27dvhl0v57ypcufcuq4guvavv	cosmospub1addwnpepqf60pgcejud4dkjj2l0vh6fegwtdu2e04fw6nzyn6q5ha5re9snp7sumvka
1	2		cosmos1dxuhzzf9xr977qexv79ze3yat6z6u34heea7x9	cosmospub1addwnpepqwl4zp0e37rgjhk3jahlr6uhkz8hwc8n6ec8re30nntvannfpedfx87zzym
1	2		cosmos1d057e8txsmhat75awtq73aq0x2enuazk77rk3r	cosmospub1addwnpepq0aqdamxu6zjg45xuqzjdm2uzgeef4ypwsdk547pzpspcng8vp3ej6yzpwx

gaiacli tx sign --validate-signatures now displays the following for multisig-based txs:

Signers:
  0: cosmos1nmd7x2gktd2qxjunyj6wwuhqj9wej09490qche

Signatures:
  0: cosmos1nmd7x2gktd2qxjunyj6wwuhqj9wej09490qche			[ERROR: signature invalid] [multisig threshold: 2/3]
  MultiSig Signatures:
    1: cosmos18h79xwz9aw9f5yaffxq2gp2jglzr94u3thqy8c (weight: 1)


ERROR: signatures validation failed

Signers:
  0: cosmos1nmd7x2gktd2qxjunyj6wwuhqj9wej09490qche

Signatures:
  0: cosmos1nmd7x2gktd2qxjunyj6wwuhqj9wej09490qche			[OK] [multisig threshold: 2/3]
  MultiSig Signatures:
    0: cosmos1zureek89ppyzna4kvd26567rcukptzdf94qyzv (weight: 1)
    1: cosmos18h79xwz9aw9f5yaffxq2gp2jglzr94u3thqy8c (weight: 1)

/cc @zmanian

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
Wrote tests
Updated relevant documentation (docs/)
Added entries in PENDING.md with issue #
rereviewed Files changed in the github PR explorer

For Admin Use:

Added appropriate labels to PR (ex. wip, ready-for-review, docs)
Reviewers Assigned
Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

codecov · 2019-02-26T17:43:32Z

Codecov Report

Merging #3748 into develop will decrease coverage by 0.4%.
The diff coverage is 19.23%.

@@             Coverage Diff             @@
##           develop    #3748      +/-   ##
===========================================
- Coverage     61.4%   60.99%   -0.41%     
===========================================
  Files          190      191       +1     
  Lines        14122    14174      +52     
===========================================
- Hits          8671     8645      -26     
- Misses        4894     4975      +81     
+ Partials       557      554       -3

client/keys/show.go

crypto/keys/types.go

crypto/keys/output.go

jackzampolin · 2019-02-27T18:58:51Z

PENDING.md

+* [\#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


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

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).

jackzampolin

Much improved here! Thank you @alexanderbez! LGTM 👍

cmd/gaia/init/gentx.go

alessio · 2019-03-01T21:06:37Z

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"


I remember to have seen this import being aliased with clkeys somewhere else in the codebase. It'd be nice if we could stay consistent. Not a blocker though.

alessio · 2019-03-01T21:09:38Z

x/auth/client/cli/sign.go

+			multiSigMsg = b.String()
+		}
+
+		fmt.Printf("  %d: %s\t\t\t[%s]%s%s\n", i, sigAddr.String(), sigSanity, multiSigHeader, multiSigMsg)
 	}

 	fmt.Println("")


Can't recally why we wanted to print an extra new line...

alessio

Nihil obstat

Minor cleanup of docs

20b276e

alexanderbez added WIP C:Keys Keybase, KMS and HSMs labels Feb 26, 2019

Update Use

af0cce5

alexanderbez added 3 commits February 26, 2019 14:04

Implement multisig key info type and functions

31f188a

Remove old multiSigKey

42dace5

Update add command to use CreateMulti

b04595c

jackzampolin reviewed Feb 26, 2019

View reviewed changes

client/keys/show.go Outdated Show resolved Hide resolved

jackzampolin reviewed Feb 26, 2019

View reviewed changes

crypto/keys/types.go Show resolved Hide resolved

alexanderbez added 2 commits February 26, 2019 14:36

Refactor key output

c6f13f5

Update show.go

0f453c2

jackzampolin reviewed Feb 26, 2019

View reviewed changes

crypto/keys/output.go Show resolved Hide resolved

alexanderbez added 4 commits February 26, 2019 15:02

Fix multisig output

095bc78

Implement new "show-multisig" flag with docs

8aea161

Fix linting errors

aaa7bb4

Fix CLI tests

22c65a6

alexanderbez mentioned this pull request Feb 27, 2019

R4R: Gentx workflow for offline keys #3734

Merged

5 tasks

alexanderbez added 3 commits February 27, 2019 10:07

Cleanup flag formatting

cc831cf

More minor cleanup

88e7c16

Cleanup error handling in Sign and support multiInfo

ede91f7

alexanderbez changed the title ~~Multisig UX Improvements~~ Multisig Display UX Improvements Feb 27, 2019

alexanderbez added 4 commits February 27, 2019 10:46

Fix multisign docs

b778182

Update printAndValidateSigs to handle printing multisigs

a0b1dc3

Add pending log entry

596e262

Fix CLI tests

a6d32b5

alexanderbez marked this pull request as ready for review February 27, 2019 16:59

alexanderbez requested review from cwgoes, ebuchman and rigelrozanski as code owners February 27, 2019 16:59

alexanderbez added ready-for-review T: UX and removed WIP labels Feb 27, 2019

jackzampolin reviewed Feb 27, 2019

View reviewed changes

jackzampolin approved these changes Feb 27, 2019

View reviewed changes

Merge branch 'develop' into bez/3738-multisig-ux-improvements

a4a0429

alexanderbez commented Feb 27, 2019

View reviewed changes

cmd/gaia/init/gentx.go Show resolved Hide resolved

alexanderbez requested review from alessio, sunnya97 and mossid February 28, 2019 22:03

alessio reviewed Mar 1, 2019

View reviewed changes

alessio approved these changes Mar 1, 2019

View reviewed changes

Merge branch 'develop' into bez/3738-multisig-ux-improvements

64d00dd

jackzampolin merged commit 47a44fb into develop Mar 1, 2019

jackzampolin deleted the bez/3738-multisig-ux-improvements branch March 1, 2019 21:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multisig Display UX Improvements #3748

Multisig Display UX Improvements #3748

alexanderbez commented Feb 26, 2019 •

edited

Loading

codecov bot commented Feb 26, 2019 •

edited

Loading

jackzampolin Feb 27, 2019

alessio Mar 1, 2019

jackzampolin Mar 1, 2019

alexanderbez Mar 1, 2019

jackzampolin left a comment

alessio Mar 1, 2019

alessio Mar 1, 2019

alessio left a comment

Multisig Display UX Improvements #3748

Multisig Display UX Improvements #3748

Conversation

alexanderbez commented Feb 26, 2019 • edited Loading

codecov bot commented Feb 26, 2019 • edited Loading

Codecov Report

jackzampolin Feb 27, 2019

Choose a reason for hiding this comment

alessio Mar 1, 2019

Choose a reason for hiding this comment

jackzampolin Mar 1, 2019

Choose a reason for hiding this comment

alexanderbez Mar 1, 2019

Choose a reason for hiding this comment

jackzampolin left a comment

Choose a reason for hiding this comment

alessio Mar 1, 2019

Choose a reason for hiding this comment

alessio Mar 1, 2019

Choose a reason for hiding this comment

alessio left a comment

Choose a reason for hiding this comment

alexanderbez commented Feb 26, 2019 •

edited

Loading

codecov bot commented Feb 26, 2019 •

edited

Loading