Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Presale wallet #1376

Merged
merged 8 commits into from Jun 22, 2016
Merged

Presale wallet #1376

merged 8 commits into from Jun 22, 2016

Conversation

debris
Copy link
Collaborator

@debris debris commented Jun 21, 2016

  • fixed crash when presale wallet password was incorrect
  • ethstore command for importing presale wallet
  • parity command for importing presale wallet (geth compatible)

@debris debris added the A0-pleasereview 🤓 Pull request needs code review. label Jun 21, 2016
@@ -20,6 +20,7 @@ Usage:
ethstore change-pwd <address> <old-pwd> <new-pwd> [--dir DIR]
ethstore list [--dir DIR]
ethstore import [--src DIR] [--dir DIR]
ethstore import-wallet <path> <password> [--dir DIR]
Copy link
Contributor

@gavofyork gavofyork Jun 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<password> is a file containing the password, right?

@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jun 21, 2016
@gavofyork
Copy link
Contributor

gavofyork commented Jun 21, 2016

looks like you're wanting to provide password as a CLI arg. that's not a particularly good idea - some people reuse passwords and would prefer not to see it in plain text or stored in the console history.

@gavofyork gavofyork added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Jun 22, 2016
@debris
Copy link
Collaborator Author

debris commented Jun 22, 2016

parity uses password file for backwards compatibility with geth.
ethstore uses plain text. I find it much more flexible than using files. If people want to use password files they can always do ethstore import-wallet ethwallet.json $(cat pass.txt)

@tomusdrw
Copy link
Collaborator

The problem is that by default people won't do that and their password will be readable in $ history.

@debris
Copy link
Collaborator Author

debris commented Jun 22, 2016

imho ethstore is a developer tool. Developers should know their environment and that typing plain password might be insecure

@gavofyork
Copy link
Contributor

developers don't generally need to import a presale wallet.

people who bought in the presale do.

@debris debris added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Jun 22, 2016
@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jun 22, 2016
@gavofyork
Copy link
Contributor

(if you really like, you could try password-file name as a password itself if the file doesn't exist)

@arkpar arkpar merged commit cb9b1e2 into master Jun 22, 2016
@debris debris deleted the presale_wallet branch June 22, 2016 20:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants