[GOAL2-513] [GOAL2-775] GOAL account export. Clarify info messages for seed import/export.#36
Merged
derbear merged 6 commits intoalgorand:masterfrom Jun 18, 2019
Merged
Conversation
added 5 commits
June 14, 2019 14:49
…ly this functionality was not mirrored in account seed mnemonics: one could only import an account's mnemonic (for example, from algokey), and not export it. This PR proposes to add a new command `goal account export`, which recovers a stored account's mnemonic. Additionally, the distinctions between wallet mnemonics and account mnemonics are clarified somewhat. Testing: Tested this by destroying and then recovering an account on TestNet.
Vervious
suggested changes
Jun 14, 2019
|
|
||
| // export flags | ||
| exportCmd.Flags().StringVarP(&accountAddress, "address", "a", "", "Address of account to export") | ||
| exportCmd.MarkFlagRequired("addr") |
Contributor
There was a problem hiding this comment.
MarkFlagRequired("address")
| var importCmd = &cobra.Command{ | ||
| Use: "import", | ||
| Short: "Import an account key from mnemonic", | ||
| Long: "Import an account key from a mnemonic generated by the export command or by algokey (NOT a mnemonic from the goal wallet command). The imported account will be listed alongside your wallet-generated accounts, but will not be tied to your wallet mnemonic.", |
Contributor
There was a problem hiding this comment.
"will not be tied to your wallet mnemonic" sounds odd/misleading. Maybe just "will not be tied to your wallet" is better IMO.
Contributor
There was a problem hiding this comment.
If you disagree that's fine, I don't have the strongest opinion; though we should probably revisit the language around mnemonics at some point (UX)
| reportErrorf(errorRequestFail, err) | ||
| } | ||
| // response.PrivateKey is (seed||PK), want just the seed | ||
| seed := response.PrivateKey[:len(response.PrivateKey)/2] |
Contributor
There was a problem hiding this comment.
better to use crypto.SecretKeyToSeed function
| infoNoAccounts = "Did not find any account. Please import or create a new one." | ||
| infoRenamedAccount = "Renamed account '%s' to '%s'" | ||
| infoImportedKey = "Imported %s" | ||
| infoExportedKey = "Imported key for account %s: \"%s\"" |
Contributor
There was a problem hiding this comment.
"Exported key for account"?
Vervious
approved these changes
Jun 17, 2019
algobolson
approved these changes
Jun 17, 2019
Karmastic
pushed a commit
that referenced
this pull request
Jun 20, 2019
It is possible to go to/from a wallet recovery mnemonic, but previously this functionality was not mirrored in account seed mnemonics: one could only import an account's mnemonic (for example, from algokey), and not export it. This PR proposes to add a new command `goal account export`, which recovers a stored account's mnemonic. Additionally, the distinctions between wallet mnemonics and account mnemonics are clarified somewhat. Testing: Tested this by destroying and then recovering an account on TestNet.
pzbitskiy
pushed a commit
to pzbitskiy/go-algorand
that referenced
this pull request
Apr 14, 2020
Fix disasm of a single instruction
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Previously one could only import an account's seed mnemonic (for example, from
algokey), and not export it. This PR proposes to add a new commandgoal account export, which recovers a stored account's mnemonic. This allows, for example, for a user to export a mnemonic for import into mobile wallets.Additionally, the help language around key import and export is hopefully clarified somewhat.
Testing
Tested this by creating an account with
algokey, importing it, destroying it, and then recovering it on TestNet.