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 revamp - 0.28.0 #2260

Merged
merged 46 commits into from
Dec 29, 2023
Merged

CLI revamp - 0.28.0 #2260

merged 46 commits into from
Dec 29, 2023

Conversation

karbyshev
Copy link
Contributor

@karbyshev karbyshev commented Dec 10, 2023

This PR introduces a revamped structure of the wallet CLI suggested in #2135. The changes are based on v0.28.0.

The following changes were additionally introduced.

  • For consistency, --alias argument is made obligatory for gen / derive commands.
  • Raw (non-HD) key generation functionality is restored, hidden under --raw flag.
    Example: namadaw gen --raw --alias key1
  • Fix alias string normalization
  • Implemented export for MASP spending keys
  • Implemented import for secret keys / MASP spending keys
  • Implement add for secret keys
  • Simplified interface of add command. Example: namadaw add --alias my_entry --value val. The supported values:
    • secret key
    • public key
    • namada address
    • MASP spending key / viewing key / payment address
  • Introduced bi-map for the alias-payment address book
  • Implemented alias lookup by MASP payment key

Future work:

  • implement ZIP32 functionality for spending key generation
  • use Alias struct everywhere in order to force alias normalization?

Checklist before merging to draft

  • I have added a changelog
  • Git history is in acceptable state

@karbyshev karbyshev added the cli label Dec 10, 2023
@karbyshev
Copy link
Contributor Author

ACK modulo notes from testing:

  • we should prevent generating two keys/addresses (including shielded & transparent) with the same alias
  • list should show both shielded and transparent by default, unless filters are provided
  • list should show both shielded keys and addresses, unless filters are provided
  • export should support exporting shielded keys without passing --shielded (should only need the alias)
  • export followed by import doesn't work - needs to be tested
  • test find and make sure it isn't case-dependent, also make sure it doesn't require the password
  • add function to remove keys
  • test it more, recruit @bengtlofgren to help test

All are fixed.

@karbyshev karbyshev force-pushed the aleks/wallet-cli-revamping-main-rebased branch 2 times, most recently from d4e9df5 to 8fe26ee Compare December 15, 2023 13:42
@karbyshev karbyshev force-pushed the aleks/wallet-cli-revamping-main-rebased branch from 8fe26ee to 26ca60c Compare December 15, 2023 16:29
@karbyshev karbyshev changed the title WIP CLI revamp - 0.28.0 CLI revamp - 0.28.0 Dec 15, 2023
@Fraccaman
Copy link
Member

great work @karbyshev! Do you think this PR is ready?

@karbyshev
Copy link
Contributor Author

@Fraccaman Thanks! I think it can be merged, but additional testing could be helpful.
ZIP32 keys are not implemented yet, and should be implemented in a separate PR, imho.

@Fraccaman Fraccaman mentioned this pull request Dec 27, 2023
Fraccaman pushed a commit that referenced this pull request Dec 27, 2023
* origin/aleks/wallet-cli-revamping-main-rebased:
  Add changelog
  Remove unused import
  Fix messages
  Adapt e2e tests to new wallet cli
  Improve decryption status line
  Improve output if keypair could not be decypted
  Fix export / import
  Minor
  Fix key import
  Implement `remove` command
  Expose store remove_alias functionality
  Allow to add public keys
  Refactoring
  Fix `find` command; refactoring
  Fix key / address listing
  Adapt e2e tests for new wallet cli
  Fix help message
  Improve comment
  Refactor parsing of `add` command value
  Improve message
  Merge find-key and find-addr
  Reverse find alias for payment address
  Store known payment addresses in bimap
  Improve help messages
  Fix typo
  Merge key and address list
  Fix comments and help messages
  Import key from file
  Implement raw key add; simplify cli for add command
  Improve some messages, comments
  Implement export for MASP spending keys
  Update help messages for `gen` and `derive`
  Fix output message
  Add todo
  Fix: normalize alias strings
  Make `--alias` obligatory for gen / derive
  Refactoring
  Rename structures
  Restore raw key generation functionality
  Refactor payment key gen
  Unify cli for key export / add
  Refactor key export
  Unify cli for transparent / shielded key / address listing / searching
  Unify cli for transparent / shielded key listing
  Unify cli for secret / spending key derivation
  Unify cli for secret / spending key generation
brentstone added a commit that referenced this pull request Dec 29, 2023
* origin/aleks/wallet-cli-revamping-main-rebased:
  Add changelog
  Remove unused import
  Fix messages
  Adapt e2e tests to new wallet cli
  Improve decryption status line
  Improve output if keypair could not be decypted
  Fix export / import
  Minor
  Fix key import
  Implement `remove` command
  Expose store remove_alias functionality
  Allow to add public keys
  Refactoring
  Fix `find` command; refactoring
  Fix key / address listing
  Adapt e2e tests for new wallet cli
  Fix help message
  Improve comment
  Refactor parsing of `add` command value
  Improve message
  Merge find-key and find-addr
  Reverse find alias for payment address
  Store known payment addresses in bimap
  Improve help messages
  Fix typo
  Merge key and address list
  Fix comments and help messages
  Import key from file
  Implement raw key add; simplify cli for add command
  Improve some messages, comments
  Implement export for MASP spending keys
  Update help messages for `gen` and `derive`
  Fix output message
  Add todo
  Fix: normalize alias strings
  Make `--alias` obligatory for gen / derive
  Refactoring
  Rename structures
  Restore raw key generation functionality
  Refactor payment key gen
  Unify cli for key export / add
  Refactor key export
  Unify cli for transparent / shielded key / address listing / searching
  Unify cli for transparent / shielded key listing
  Unify cli for secret / spending key derivation
  Unify cli for secret / spending key generation
@brentstone brentstone merged commit 2067cda into main Dec 29, 2023
13 of 14 checks passed
@brentstone brentstone deleted the aleks/wallet-cli-revamping-main-rebased branch December 29, 2023 19:37
@karbyshev
Copy link
Contributor Author

ZIP32 key derivation is implemented in #2417

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants