-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add commands to shield funds #65
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but it should be possible to specify the account (with default iff there is only one account).
src/commands/shield.rs
Outdated
let account_id = *db_data | ||
.get_account_ids()? | ||
.first() | ||
.ok_or(anyhow!("Wallet has no accounts"))?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The account to use should be an optional CLI argument; this should only be a default & the default should only be allowed when there is only one account in the wallet.
src/commands/shield.rs
Outdated
let derivation = account | ||
.source() | ||
.key_derivation() | ||
.ok_or(anyhow!("Cannot spend from view-only accounts"))?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.ok_or(anyhow!("Cannot spend from view-only accounts"))?; | |
.ok_or(anyhow!("Cannot spend from view-only accounts; did you mean to use `pczt shield` instead?"))?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK; making the account ID optional in the case that there is only one account can be done later.
No description provided.