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

CLI: Support bech32 encoding #1603

Closed
4 tasks done
Anviking opened this issue Apr 28, 2020 · 1 comment
Closed
4 tasks done

CLI: Support bech32 encoding #1603

Anviking opened this issue Apr 28, 2020 · 1 comment
Assignees

Comments

@Anviking
Copy link
Member

Anviking commented Apr 28, 2020

Context

ADP-195

Decision

Add support for bech32 key encoding.

Acceptance criterea

  • key inspect, key child and key public must accept both bech32- and hex-encoded keys.
  • key root should still default to outputting a hex-encoded key
  • key root should output bech32 when given --encoding bech32
  • Chaining commands with pipes (|) should be possible.

Development

QA

@Anviking Anviking self-assigned this Apr 28, 2020
iohk-bors bot added a commit that referenced this issue Apr 30, 2020
1599: Support Bech32 keys in CLI (and rework the implementation to be less fancy) r=Anviking a=Anviking

# Issue Number

#1603 

Alternative implementation to PR #1591 

# Overview
 
- [x] I removed the inspect, child, public fields from `CliKeyScheme` which was the cause for several abstractions.
- [x] I replaced the roundtrip property with more basic ones.
- [x] This opens up for more untested mistakes in the CLI handlers themselves. But I tried to counteract this using point-free notation to avoid potential mixup of `k`, `k'`, `key``, etc.
- [x] Added integration tests making sure `key child`, `key public`, `key inspect` accept keys from stdin. Integration tests also hit both hex and bech32 encodings, as well as public and private derivation.

# Comments

<!-- 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: Johannes Lund <[email protected]>
iohk-bors bot added a commit that referenced this issue May 5, 2020
1622: Only support keys as stdin and move tests r=Anviking a=Anviking



# Issue Number

#1603 

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->


# Overview

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

- [x] **Breaking**: Only accept key inputs as stdin for `key inspect`, `key public`, `key child` (no arguments)
- [x] Moved unit parser tests to be full integration tests
- [x] Tweaked some help text

# Comments

<!-- 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: Johannes Lund <[email protected]>
iohk-bors bot added a commit that referenced this issue May 5, 2020
1619: Better bech32 error messages r=Anviking a=Anviking

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->

#1603 


# Overview

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

- [x] Addded custom error messages for bech32 errors
- [x] Mark invalid characters red <s>and underline them with `^`</s>


# Comments

- Not sure if/how we should test
- Not sure if the (non-IO) ANSI red works on windows
- <s>Not sure if the `^` underlining is a great idea considering line wrapping is likely</s> removed

<img width="1536" alt="Skärmavbild 2020-05-04 kl  12 02 28" src="https://user-images.githubusercontent.com/304423/80955366-7a85fe00-8dff-11ea-8563-c1b5ffe20a02.png">

<img width="296" alt="Skärmavbild 2020-05-04 kl  12 02 49" src="https://user-images.githubusercontent.com/304423/80955372-7f4ab200-8dff-11ea-9178-a7095f922c40.png">




<!-- 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: Johannes Lund <[email protected]>
iohk-bors bot added a commit that referenced this issue May 11, 2020
1643: Add property and fix for markCharsRedAtIndices r=Anviking a=Anviking

# Issue Number

#1603 

# Overview

- [x] Add property for and fix questionable behaviour of `markCharsRedAtIndices`

# Comments

- Step by step property failures in  72d5bb9
- Don't think the problems affected the error rendering (or at least not commonly), or I'd noticed yesterday.
- Should still have a another calm look at this to be safe 😬 

<!-- 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: Johannes Lund <[email protected]>
iohk-bors bot added a commit that referenced this issue May 12, 2020
1654: Testing Support bech32 encoding r=KtorZ a=piotr-iohk

# Issue Number

#1603

# Overview

- 00306f0
  Polish key CLI help
  
- 285add3
  Additional integration test for inspecting bech32 result

# Comments

Before:

```
$ cardano-wallet-jormungandr key root
Usage: cardano-wallet-jormungandr key root [--wallet-style WALLET_STYLE]
                                           MNEMONIC_WORD...
                                           [--encoding KEY-ENCODING]
  Extract root extended private key from a mnemonic sentence.

Available options:
  -h,--help                Show this help text
  --wallet-style WALLET_STYLE
                           Any of the following (default: icarus)
                             icarus (15 mnemonic words)
                             trezor (12, 15, 18, 21 or 24 mnemonic words)
                             ledger (12, 15, 18, 21 or 24 mnemonic words)
```


After:

```
$ cardano-wallet-jormungandr key root
Usage: cardano-wallet-jormungandr key root [--wallet-style WALLET_STYLE]
                                           [--encoding KEY-ENCODING]
                                           MNEMONIC_WORD...
  Extract root extended private key from a mnemonic sentence.

Available options:
  -h,--help                Show this help text
  --wallet-style WALLET_STYLE
                           Any of the following (default: icarus)
                             icarus (15 mnemonic words)
                             trezor (12, 15, 18, 21 or 24 mnemonic words)
                             ledger (12, 15, 18, 21 or 24 mnemonic words)
  --encoding KEY-ENCODING  Either 'hex' or 'bech32' (default: hex)

```

Also updated:
 - https://github.com/input-output-hk/cardano-wallet/wiki/Wallet-command-line-interface#key-root
 - https://github.com/input-output-hk/cardano-wallet/wiki/Wallet-Command-Line-Interface-(cardano-wallet-byron)#key-root

Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this issue May 12, 2020
1654: Testing Support bech32 encoding r=piotr-iohk a=piotr-iohk

# Issue Number

#1603

# Overview

- 00306f0
  Polish key CLI help
  
- 285add3
  Additional integration test for inspecting bech32 result

# Comments

Before:

```
$ cardano-wallet-jormungandr key root
Usage: cardano-wallet-jormungandr key root [--wallet-style WALLET_STYLE]
                                           MNEMONIC_WORD...
                                           [--encoding KEY-ENCODING]
  Extract root extended private key from a mnemonic sentence.

Available options:
  -h,--help                Show this help text
  --wallet-style WALLET_STYLE
                           Any of the following (default: icarus)
                             icarus (15 mnemonic words)
                             trezor (12, 15, 18, 21 or 24 mnemonic words)
                             ledger (12, 15, 18, 21 or 24 mnemonic words)
```


After:

```
$ cardano-wallet-jormungandr key root
Usage: cardano-wallet-jormungandr key root [--wallet-style WALLET_STYLE]
                                           [--encoding KEY-ENCODING]
                                           MNEMONIC_WORD...
  Extract root extended private key from a mnemonic sentence.

Available options:
  -h,--help                Show this help text
  --wallet-style WALLET_STYLE
                           Any of the following (default: icarus)
                             icarus (15 mnemonic words)
                             trezor (12, 15, 18, 21 or 24 mnemonic words)
                             ledger (12, 15, 18, 21 or 24 mnemonic words)
  --encoding KEY-ENCODING  Either 'hex' or 'bech32' (default: hex)

```

Also updated:
 - https://github.com/input-output-hk/cardano-wallet/wiki/Wallet-command-line-interface#key-root
 - https://github.com/input-output-hk/cardano-wallet/wiki/Wallet-Command-Line-Interface-(cardano-wallet-byron)#key-root

Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this issue May 12, 2020
1654: Testing Support bech32 encoding r=piotr-iohk a=piotr-iohk

# Issue Number

#1603

# Overview

- 00306f0
  Polish key CLI help
  
- 285add3
  Additional integration test for inspecting bech32 result

# Comments

Before:

```
$ cardano-wallet-jormungandr key root
Usage: cardano-wallet-jormungandr key root [--wallet-style WALLET_STYLE]
                                           MNEMONIC_WORD...
                                           [--encoding KEY-ENCODING]
  Extract root extended private key from a mnemonic sentence.

Available options:
  -h,--help                Show this help text
  --wallet-style WALLET_STYLE
                           Any of the following (default: icarus)
                             icarus (15 mnemonic words)
                             trezor (12, 15, 18, 21 or 24 mnemonic words)
                             ledger (12, 15, 18, 21 or 24 mnemonic words)
```


After:

```
$ cardano-wallet-jormungandr key root
Usage: cardano-wallet-jormungandr key root [--wallet-style WALLET_STYLE]
                                           [--encoding KEY-ENCODING]
                                           MNEMONIC_WORD...
  Extract root extended private key from a mnemonic sentence.

Available options:
  -h,--help                Show this help text
  --wallet-style WALLET_STYLE
                           Any of the following (default: icarus)
                             icarus (15 mnemonic words)
                             trezor (12, 15, 18, 21 or 24 mnemonic words)
                             ledger (12, 15, 18, 21 or 24 mnemonic words)
  --encoding KEY-ENCODING  Either 'hex' or 'bech32' (default: hex)

```

Also updated:
 - https://github.com/input-output-hk/cardano-wallet/wiki/Wallet-command-line-interface#key-root
 - https://github.com/input-output-hk/cardano-wallet/wiki/Wallet-Command-Line-Interface-(cardano-wallet-byron)#key-root

Co-authored-by: Piotr Stachyra <[email protected]>
@piotr-iohk
Copy link
Contributor

LGTM.

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

No branches or pull requests

2 participants