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

Wallet cleanup brainstorming #2135

Closed
cwgoes opened this issue Nov 9, 2023 · 9 comments
Closed

Wallet cleanup brainstorming #2135

cwgoes opened this issue Nov 9, 2023 · 9 comments

Comments

@cwgoes
Copy link
Contributor

cwgoes commented Nov 9, 2023

Clean up the Namada CLI wallet, both internally and externally.

Just using the wallet, I observe some UX issues:

  • Many commands seem to require genesis files which I wouldn't expect to need them, e.g.
namada wallet key list

=>

The application panicked (crashed).
Message:  Missing genesis files:
   0: Couldn't read Validity predicates config file from /home/cwgoes/.local/share/namada/e2e-test.a23671b0c1d2dc60d75d8/validity-predicates.toml
   1: No such file or directory (os error 2)

Location:
   apps/src/lib/config/genesis/toml_utils.rs:11

Why do we need genesis files for the wallet at all? The wallet should only be dealing with its own database and configuration, in general. If we're using genesis files to look up aliases or such, this is fine, but we should gracefully degrade (perhaps with a warning) if they're not found instead of crashing.

  • The relationship between key and address generation is pretty unclear to me. I understand addresses as simply being derived from public keys - if we create a new key, we should also know the address. Why do we have a whole separate subcommand for address management (with many of the same sub-sub-commands as for keys)? Can we get rid of this, and simply make the key commands compute & store the address as well?
  • Is there a strong reason to separate MASP and regular key generation and management? I think it might be easier for users if we simply have a --masp option or such when generating a new key (and generating payment addresses only works when used with a spending key). This may require us to implement ZIP32 (but there is almost certainly a library which we can import).
  • The wallet should support an arbitrary-message sign operation which does not depend on any transaction-related code (just signs binary data with a key in the wallet).

There's probably more, but let's just start by evaluating these questions.

@cwgoes
Copy link
Contributor Author

cwgoes commented Nov 9, 2023

Discussed quickly with @tzemanovic who noted that the address subcommand also deals with established addresses and allows for alias creation. Agreed to split out all of this ledger-state-dependent logic into namada client, and keep namada wallet for key logic only (which should not be dependent on genesis state or any particular network). That should provide a cleaner separation of concerns and allow us to simplify the wallet UX.

@tzemanovic
Copy link
Member

just to add, in 0.25 we added --pre-genesis flag to wallet cmds. If the wallet is independent from chain then we probably won't need it anymore

@karbyshev
Copy link
Contributor

After the discussion with @murisi, we suggest the following structure for namadaw CLI commands:

  • gen [--masp] [--alias] [--hd-path] generates new key
    • default, secret key for transparent pool, optionally HD
    • --masp spending key for masp, optionally HD
  • derive [--masp] [--alias] [--hd-path] derive (restore) a key from seed
    • default, secret key for transparent pool, optionally HD
    • --masp spending key for masp, optionally HD
  • add [--alias] add new key / address
    • public key and/or address for transparent pool
    • viewing key and/or payment address for masp
  • list [--misc-filter-options] list keys / addresses
  • find [--alias] find key / address
  • export <alias> export key
  • "masp-add-payment-key" (name suggestions welcome) creates a fresh payment address for masp
  • sign <alias> <obj> creates a digital signature for the binary object

NB: There is no way to add secret / spending key, only generate / derive

@adrianbrink
Copy link
Member

Replace all instances of masp with shielded.

I don't think we should have an export functionality. Everyone needs to save their seed phrase correctly.

@Fraccaman
Copy link
Member

but there must be a way to import a key generated from another source (i.e the web interface, other wallets, ecc...) in some ways

@adrianbrink
Copy link
Member

Agreed, but all other frontends should use bip39 seed phrases. It is the most compatible way to import/export keys.

@karbyshev karbyshev mentioned this issue Dec 10, 2023
2 tasks
@chillicao
Copy link

Is there a workaround for getting the needed .toml files in place?

@okwme
Copy link

okwme commented Feb 1, 2024

it appears there are separate namadaw and namada wallet CLIs that have the same commands but different options and different results. Shouldn't this be the same code/functionality even if it's compiled into two different binaries? I'm confused why there's a namadaw at all I didn't see it in the docs at all and only through reading support messages on discord found out about it.

What is the user story for someone who would want/need namadaw but not want/need namada? If there isn't one shouldn't it be consolidated into namada and reduce surface area? (after functionality is fixed of course, currently it seems you can't create a wallet until you already have a wallet unless you're a pre-genesis validator #2439 )

@karbyshev
Copy link
Contributor

@okwme The options have to be used on the outermost position of the prompt. For example,
$ ./target/debug/namadaw --pre-genesis list
and
$ ./target/debug/namada --pre-genesis wallet list
should give the same result.

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

No branches or pull requests

8 participants