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

Add Ledger Instructions #1512

Merged
merged 3 commits into from
Jul 3, 2018
Merged

Add Ledger Instructions #1512

merged 3 commits into from
Jul 3, 2018

Conversation

jackzampolin
Copy link
Member

This adds a brief README that details how to use the ledger app with gaiacli.

@jackzampolin jackzampolin requested a review from ebuchman as a code owner July 3, 2018 00:16
@jackzampolin
Copy link
Member Author

cc @cwgoes

@codecov
Copy link

codecov bot commented Jul 3, 2018

Codecov Report

Merging #1512 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #1512   +/-   ##
========================================
  Coverage    64.13%   64.13%           
========================================
  Files          118      118           
  Lines         6489     6489           
========================================
  Hits          4162     4162           
  Misses        2075     2075           
  Partials       252      252


### Ledger Support for Account Keys

`gaiacli` now supports derivation of account keys from a Ledger seed. To use this functionality you will need the following:
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's reference the BIP HD derivation spec, which we use - https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a note below about this with links to our derivation paths in the code.

* Install the Cosmos app onto your Ledger by following the instructions in the [`ledger-cosmos`](https://github.com/cosmos/ledger-cosmos/blob/master/docs/BUILD.md) repository.
* A production-ready version of this app will soon be included in the [Ledger Apps Store](https://www.ledgerwallet.com/apps)

Once you have the Cosmos app installed on your Ledger, and the ledger is accessible from the machine you are using `gaiacli` from you can create a new Account key using the ledger:
Copy link
Contributor

Choose a reason for hiding this comment

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

"Ledger" (capitalized), "account" (lowercase), throughout

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed!

$ gaiacli send --name {{ .Key.Name }} --to {{ .Destination.AccAddr }} --chain-id=gaia-7000
```

You will be asked to review and confirm the transaction on the Ledger. Once you do this you should see the result in the console! Now you can use your Ledger to manage your Atoms and Stake!
Copy link
Contributor

@cwgoes cwgoes Jul 3, 2018

Choose a reason for hiding this comment

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

Can we walk the user through confirming the transaction (menu items, which buttons to press) - maybe even including screenshots? 😄

This key will only be accessible while the ledger is plugged in and unlocked. To send some coins with this key, run the following:

```bash
$ gaiacli send --name {{ .Key.Name }} --to {{ .Destination.AccAddr }} --chain-id=gaia-7000
Copy link
Contributor

Choose a reason for hiding this comment

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

What's with the template syntax - is this autofilled somehow?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, just using it to try to show the format of the command. I can change that if you would like.

Copy link
Contributor

Choose a reason for hiding this comment

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

we should definitely leverage template more in various places, incl. docs

Copy link
Member Author

Choose a reason for hiding this comment

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

Many times docs writers use arbitrary formats for describing variables. A good example of this is our validator getting started documentation. I personally like a format like this because it can make things like this much more clear.

Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

Thanks for writing this! A few changes req'd.

Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

utACK

@cwgoes cwgoes merged commit 270e216 into cosmos:develop Jul 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants