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.

Sign up for GitHub

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

Jump to bottom

Add CLI command for extracting root xprvs #1321

Merged
merged 5 commits into from
Feb 26, 2020
Merged

Add CLI command for extracting root xprvs #1321

merged 5 commits into from
Feb 26, 2020

Conversation

Anviking
Copy link
Member

@Anviking Anviking commented Jan 31, 2020 •
edited
Loading

Issue Number

#1316

Overview

  • I have added cardano-wallet-jormungandr key root --type random <mnemonic words>
  • I added unit tests for help-text and actual usage.
  • I added a pending test making sure keys are compatible with jcli (which would fail)

Comments

$ cardano-wallet-jormungandr key --help
Usage: cardano-wallet-jormungandr key COMMAND
  Derive keys from mnemonics.

Available options:
  -h,--help                Show this help text

Available commands:
  root                     Extract root xprv as hex (64 bytes private key + 32
                           bytes chain code)
$ cardano-wallet-jormungandr key root --help
Usage: cardano-wallet-jormungandr key root --type KEYTYPE MNEMONIC_WORDS...
  Extract root xprv as hex (64 bytes private key + 32 bytes chain code)

Available options:
  -h,--help                Show this help text
  --type KEYTYPE           Any of the following:
                             random (Daedalus, 12 words)
                             icarus (15 words)
                             trezor (12, 15, 18, 21, or 24 words)
                             ledger (12, 15, 18, 21, or 24 words)
$ cardano-wallet-jormungandr key root --type random flock advance execute country leader exotic mix twenty six margin orient meat
68a0f29e6bd5d8af7ffd00a55006afa8af6fbdbded07984ddf7fb1c31c66f7460685e5d1016553fccc9724f5ee95dd8d66facd2ac1bb2f6fcd7fa5e53c97a57f50c592fcd18b67bf3393a16184d009fb25450b2de8079f870222874e804584a8

@Anviking Anviking self-assigned this Jan 31, 2020
@Anviking Anviking force-pushed the anviking/ADP-81/extract-xprvs branch 3 times, most recently from 695dbfb to 57fad60 Compare February 3, 2020 17:03
@erikd
Copy link
Contributor

erikd commented Feb 3, 2020

Please give PRs a proper title instead of just accepting the default. With a proper title I can see whan a PR is about without having to follow the link.

@Anviking Anviking changed the title Anviking/adp 81/extract xprvs Add CLI command for extracting root xprvs Feb 4, 2020
@Anviking Anviking force-pushed the anviking/ADP-81/extract-xprvs branch 9 times, most recently from 53b72e9 to 508ac11 Compare February 9, 2020 17:23
@Anviking Anviking marked this pull request as ready for review February 9, 2020 18:12
@Anviking Anviking requested a review from paweljakubas February 9, 2020 18:12
Anviking
Anviking commented Feb 10, 2020
View reviewed changes
lib/cli/src/Cardano/CLI.hs Outdated Show resolved Hide resolved
KtorZ
KtorZ suggested changes Feb 10, 2020
View reviewed changes
lib/cli/src/Cardano/CLI.hs Outdated Show resolved Hide resolved
lib/cli/src/Cardano/CLI.hs Outdated Show resolved Hide resolved
lib/cli/src/Cardano/CLI.hs Outdated Show resolved Hide resolved
lib/cli/src/Cardano/CLI.hs Outdated Show resolved Hide resolved
lib/core/src/Cardano/Wallet/Api/Types.hs Outdated Show resolved Hide resolved
lib/core/src/Cardano/Wallet/Primitive/AddressDerivation.hs Outdated Show resolved Hide resolved
lib/core/test/unit/Cardano/Wallet/Primitive/AddressDerivationSpec.hs Outdated Show resolved Hide resolved
@Anviking Anviking force-pushed the anviking/ADP-81/extract-xprvs branch 4 times, most recently from ab381ea to 1410710 Compare February 10, 2020 18:22
@Anviking Anviking force-pushed the anviking/ADP-81/extract-xprvs branch from 0087967 to 09242e8 Compare February 11, 2020 11:30
Anviking
Anviking commented Feb 11, 2020
View reviewed changes
lib/cli/src/Cardano/CLI.hs Outdated Show resolved Hide resolved
@Anviking Anviking force-pushed the anviking/ADP-81/extract-xprvs branch from 487cd9f to 2347cb7 Compare February 21, 2020 16:15
@Anviking
Copy link
Member Author

Anviking commented Feb 21, 2020 •
edited
Loading

could split off bfe2568 to a separate pr 🤔

I made unXPrvStripPub fail if it cannot be roundtripped. And started adding properties. Not happy with them yet though.

@Anviking Anviking force-pushed the anviking/ADP-81/extract-xprvs branch from 2347cb7 to f97bfaf Compare February 24, 2020 11:03
@Anviking Anviking changed the base branch from master to anviking/ADP-81/unXPrvStripPub February 24, 2020 11:03
@Anviking Anviking mentioned this pull request Feb 24, 2020
Closed
3 tasks
@Anviking Anviking force-pushed the anviking/ADP-81/extract-xprvs branch from f97bfaf to 2792209 Compare February 24, 2020 15:52
@Anviking Anviking force-pushed the anviking/ADP-81/unXPrvStripPub branch from 3858dbb to b02caf4 Compare February 24, 2020 15:53
@Anviking Anviking force-pushed the anviking/ADP-81/extract-xprvs branch from 2792209 to da17466 Compare February 25, 2020 12:51
@Anviking Anviking force-pushed the anviking/ADP-81/unXPrvStripPub branch from b02caf4 to c07d738 Compare February 25, 2020 12:51
KtorZ
KtorZ approved these changes Feb 25, 2020
View reviewed changes
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

👍 (please wait after #1358 before merging, I cherry-picked one of your commit so the two branches will conflict).

@Anviking Anviking force-pushed the anviking/ADP-81/unXPrvStripPub branch 2 times, most recently from 5c0a3e5 to 8f607c6 Compare February 26, 2020 08:55
@Anviking Anviking force-pushed the anviking/ADP-81/extract-xprvs branch 2 times, most recently from 92a721f to e076570 Compare February 26, 2020 10:11
@Anviking
Add (ByteArrayAccess Passphrase) instance
c80dd30
@Anviking
Add unXPrvStripPub (& inverse) that matches jcli
a7623c4 
- Roundtrip properties
- Integration test verifying that it works with jcli

Why:

We need to convert between 96-byte long hex-encoded bytestrings and
XPrvs when implementing the `key root` and `key child` CLI commands.
@Anviking Anviking force-pushed the anviking/ADP-81/unXPrvStripPub branch from 7df2d00 to a7623c4 Compare February 26, 2020 15:08
@Anviking
Export NatVals from C.W.P.AddressDerivation
c7c5644
@Anviking
[gardening] Leverage unsafeMkEntropy in Cardano.Wallet.Gen
d6f27a7 
Unrelated to PR.
@Anviking
Add cli key root command
c9fa75e 
- Add CLI usage tests for `key` and `key root`
- Add key-root goldens (not externally validated)
- There was a local definition of expectationFailure, using quickchecks's
counterexample, which I had to rename.
I'm not sure why the existing CLI tests uses QuickCheck when they only
run once.
- Isolate the types to one place for power & elegance
- Add functionality to check CliKeyScheme equality & more powerful tests
@Anviking Anviking force-pushed the anviking/ADP-81/extract-xprvs branch from e076570 to c9fa75e Compare February 26, 2020 15:13
@Anviking Anviking changed the base branch from anviking/ADP-81/unXPrvStripPub to master February 26, 2020 16:08
@Anviking
Copy link
Member Author

bors r+

iohk-bors bot added a commit that referenced this pull request Feb 26, 2020
@iohk-bors @rvl
@KtorZ @Anviking
Merge #1315 #1321
4984781 
1315: Add shutdown handler for new launcher r=KtorZ a=rvl

Relates to #1314.

# Overview

This adds a simpler way of ensuring clean shutdown of the wallet on windows (and linux), which doesn't require DaedalusIPC. The mechanism will be used by cardano-launcher.

- Adds the shutdown handler thread.
- Needed to rearrange startup functions around a little bit.
- Unit tests and an integration test.

# Comments

[Hydra jobset](https://hydra.iohk.io/jobset/Cardano/cardano-wallet-pr-1315)


1321: Add CLI command for extracting root xprvs r=Anviking a=Anviking

# Issue Number

#1316 


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] I have added `cardano-wallet-jormungandr key root --type random <mnemonic words>`
- [x] I added unit tests for help-text and actual usage.
- [x] I added a *pending* test making sure keys are compatible with jcli (which would fail)

# Comments

```bash
$ cardano-wallet-jormungandr key --help
Usage: cardano-wallet-jormungandr key COMMAND
  Derive keys from mnemonics.

Available options:
  -h,--help                Show this help text

Available commands:
  root                     Extract root xprv as hex (64 bytes private key + 32
                           bytes chain code)
$ cardano-wallet-jormungandr key root --help
Usage: cardano-wallet-jormungandr key root --type KEYTYPE MNEMONIC_WORDS...
  Extract root xprv as hex (64 bytes private key + 32 bytes chain code)

Available options:
  -h,--help                Show this help text
  --type KEYTYPE           Any of the following:
                             random (Daedalus, 12 words)
                             icarus (15 words)
                             trezor (12, 15, 18, 21, or 24 words)
                             ledger (12, 15, 18, 21, or 24 words)
$ cardano-wallet-jormungandr key root --type random flock advance execute country leader exotic mix twenty six margin orient meat
68a0f29e6bd5d8af7ffd00a55006afa8af6fbdbded07984ddf7fb1c31c66f7460685e5d1016553fccc9724f5ee95dd8d66facd2ac1bb2f6fcd7fa5e53c97a57f50c592fcd18b67bf3393a16184d009fb25450b2de8079f870222874e804584a8
```

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Rodney Lorrimar <[email protected]>
Co-authored-by: KtorZ <[email protected]>
Co-authored-by: Johannes Lund <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 26, 2020

Build failed (retrying...)

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 26, 2020

Build succeeded

@iohk-bors iohk-bors bot merged commit 0fdf956 into master Feb 26, 2020
This was referenced Feb 27, 2020
Closed
Closed
@KtorZ KtorZ deleted the anviking/ADP-81/extract-xprvs branch March 17, 2020 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonathanknowles jonathanknowles jonathanknowles left review comments

@KtorZ KtorZ KtorZ approved these changes

@paweljakubas paweljakubas Awaiting requested review from paweljakubas

Assignees

@Anviking Anviking

Labels
None yet
Projects
None yet
Milestone
(ADP-81) Wallet: Command-line utils f...
Development

Successfully merging this pull request may close these issues.
4 participants
@Anviking @erikd @jonathanknowles @KtorZ