Dependency updates #54
Conversation
| #[options(free, required, help = "the ID of the account to list addresses for")] | ||
| account_id: u32, | ||
| #[options(free, required, help = "the UUID of the account to list addresses for")] | ||
| account_id: Uuid, |
There was a problem hiding this comment.
I'm considering changing this to an account flag that can be either an AccountUuid or a name, but the latter I don't think has any uniqueness constraints (and can also be null).
There was a problem hiding this comment.
I don't think that the name is really needed for zec_sqlite_cli and starting next week we should shift our focus to zallet as much as possible anyway.
| help = "the purpose of the viewing key (default is \"viewing\")", | ||
| parse(try_from_str = "Purpose::parse") | ||
| )] | ||
| purpose: Purpose, |
There was a problem hiding this comment.
Now that the spending purpose holds a key derivation, I've changed the FVK import pathways to be view-only. We need to do that init refactor to provide a better way to prepare accounts.
There was a problem hiding this comment.
I think that's fine for init_fvk, because init will subsume this command, but import_ufvk should be able to import a AccountPurpose::Spending key; after all, the "standard" workflow for Zashi is to initialize with a seed, but then import the UFVK from the hardware wallet with derivation & key source information.
Along those lines, I think that we should specify ZIP 316 metadata for seed fingerprint and hd account index data.
There was a problem hiding this comment.
utACK. The clippy lint about AccountBalance::unshielded being deprecated is due to zcash/librustzcash#1570.
src/commands/import_ufvk.rs
Outdated
| #[options(help = "can the wallet omit information needed to spend funds (default is false)")] | ||
| view_only: bool, | ||
|
|
There was a problem hiding this comment.
This should be retained, and in addition we should have optional seed_fingerprint and hd_account_index parameters, so that AccountPurpose::Spending can be represented. Also, we should have the key_source information for parity with the normal init command.
There was a problem hiding this comment.
I didn't add key_source to the normal init command; I was asking you what to do with it. Given that you want parity, I'm omitting it for now, and you can make a PR later to do what you want with it.
| help = "the purpose of the viewing key (default is \"viewing\")", | ||
| parse(try_from_str = "Purpose::parse") | ||
| )] | ||
| purpose: Purpose, |
There was a problem hiding this comment.
I think that's fine for init_fvk, because init will subsume this command, but import_ufvk should be able to import a AccountPurpose::Spending key; after all, the "standard" workflow for Zashi is to initialize with a seed, but then import the UFVK from the hardware wallet with derivation & key source information.
Along those lines, I think that we should specify ZIP 316 metadata for seed fingerprint and hd account index data.
| #[options(free, required, help = "the ID of the account to list addresses for")] | ||
| account_id: u32, | ||
| #[options(free, required, help = "the UUID of the account to list addresses for")] | ||
| account_id: Uuid, |
There was a problem hiding this comment.
I don't think that the name is really needed for zec_sqlite_cli and starting next week we should shift our focus to zallet as much as possible anyway.
| @@ -15,9 +19,6 @@ pub(crate) struct Command { | |||
| #[options(help = "age identity file to decrypt the mnemonic phrase with")] | |||
| identity: String, | |||
|
|
|||
| #[options(help = "the number of accounts to re-initialise the wallet with (default is 1)")] | |||
| accounts: Option<usize>, | |||
There was a problem hiding this comment.
👍 I never liked the "initialize multiple accounts at once" functionality. However for reset, what are the semantics? I thought that the idea was to essentially drop all transaction data, but retain key information. If multiple accounts have been derived and/or imported, shouldn't they be retained? Or should there be two modes: one which wipes everything and starts again from the seed, and another that wipes everything except the accounts table?
There was a problem hiding this comment.
I literally do not have time to do anything more than what I've done here. If you want multi-seed support retained across reset, open an issue.
|
|
||
| let account_id = *db_data | ||
| .get_account_ids()? | ||
| .first() |
There was a problem hiding this comment.
The answer to my question above will govern whether this first makes sense.
str4d
dismissed
nuttycom’s stale review
Added account purpose configuration back in for viewing keys
No description provided.