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

Store the deterministic wallet seed serialized in the '.keys' file #59

Closed
Jojatekok opened this issue Jul 12, 2014 · 19 comments
Closed

Comments

@Jojatekok
Copy link
Contributor

It would be great to have the opportunity of retrieving our (newly-generated) wallets' seeds, as it is one of the most convenient ways of making a backup.

My idea is to store the seeds permamently (encrypted in the '.keys' file), in order to let the users retrieve them anytime with an RPC command. The command should return an empty string (or a specific error) whether the wallet's '.keys' file doesn't contain a seed.

@Neozaru
Copy link
Contributor

Neozaru commented Jul 12, 2014

Isn't the seed a representation of the private key (in deterministic mode) ? If so, it would be possible to get seed from private key directly.

I don't think we should be able to get it from RPC command, since it assumes the user network is safe.
It's a good practice to display the seed only once. You can ask the user to repeat/retype it at this time.

@Jojatekok
Copy link
Contributor Author

I was not sure about getting the seed from the key privately, but if it's possible, then it's even better.

I also understand why it should not be sent through the RPC, but then, it could still be displayed in the command prompt by a command.

There may also be a separate console application (or even better: class library) which outputs seeds from the input '.keys' files. By the way I want this to be implemented because of easier retrieval when creating backups from GUI wallets.

@fluffypony
Copy link
Contributor

@tewinget can comment on this, but I agree that the seed should be stored for future recall.

@fluffypony
Copy link
Contributor

@jakoblind may want to do this, as he's just been digging around in the wallet code

I think coupled with this would be:

  • CLI/RPC methods to retrieve the mnemonic
  • CLI/RPC methods to change the password (at a guess I'd think this involves nuking the .bin.keys and .bin files and recreating them, encrypted with the new password)

@jakoblind
Copy link
Contributor

will take a look at this one when I'm done with #36

@Jojatekok
Copy link
Contributor Author

@fluffypony Nuking must happen safely:

1.) Rename the wallet files to '.bin.old' and '.bin.keys.old'
2.) Write the new wallet files
3.) Send a request to the network indicating that the password has been changed
4.) Remove the old wallet files

(Step 2 and 3 may be merged or reversed if necessary)

Also, there should be an ability to remove passphrase protection from wallets.

@fluffypony
Copy link
Contributor

@Jojatekok the network doesn't know about your password, so that isn't necessary:) Removing the .bin can be done unsafely, since it's just a cache. The .bin.keys file won't be removed, it'll just be overwritten by the new serialised data, so it's a safe action.

@jakoblind
Copy link
Contributor

I've added a seed command to the CLI (are we sure we want this in the RPC also?). Should I add extra password protection to that command?

https://github.com/jakoblind/bitmonero/commits/seed_command

@fluffypony
Copy link
Contributor

@jakoblind don't add password protection to that command, if the RPC API for the wallet is compromised the attacker can just transfer the funds out anyway. We do need HTTPS and Simple Auth for RPC, but rather than extend epee (which is over-templates and ill-suited to Monero in the long run) we're going to replace the RPC stuff either with our own code or with a library. Feel free to hop in on #monero-dev when you're around to discuss that, and you're welcome to run with that task once we've all decided on the best way to do it:)

@tewinget
Copy link
Contributor

tewinget commented Aug 1, 2014

@jakoblind unless I misread, your seed command treats the private send key as the seed and converts that to mnemonic words, but that key is not the seed. You'll want to change the wallet keys generation part to store the seed, as it is not currently stored, just printed.

@jakoblind
Copy link
Contributor

@tewinget what is the seed then? The code works fine for me. I will continue the discussion over IRC :)

@fluffypony
Copy link
Contributor

@tewinget @jakoblind It's just occurred to me that we have a greater problem: changing the data structure of the .keys file will mean it can't open old wallets. I think we're going to need a version to the data structure, and if it encounters an old version (or the original unversioned data) it should automatically convert that .keys file to the latest version. After a year we can deprecate the old formats, with a note that old tagged releases can be used to convert them before opening them in a "current" wallet app.

@Jojatekok
Copy link
Contributor Author

@fluffypony Excatly, that's a great idea! I already have a proof of concept for this implemented in Monero Client .NET, which automatically converts the old setting values to new ones.

@fluffypony
Copy link
Contributor

@Jojatekok turns out to be a non-issue, as the seed is already stored in the .keys file, but in future this is the general idea for the workflow behind switching data formats:)

@fluffypony
Copy link
Contributor

@jakoblind please PR your change so I can merge it, then we can do the in-memory key encryption/mlock etc. stuff separately

@jakoblind
Copy link
Contributor

now the function fail and spit out a notice if the wallet is non-deterministic. @fluffypony could you test with an old wallet if it works? I dont have access to old wallets.

@tewinget
Copy link
Contributor

tewinget commented Aug 2, 2014

@jakoblind, you can create an "old" wallet with the --non-deterministic
flag during wallet creation.

On Sat, Aug 2, 2014 at 12:17 PM, Jakob Lind notifications@github.com
wrote:

now the function fail and spit out a notice if the wallet is
non-deterministic. @fluffypony https://github.com/fluffypony could you
test with an old wallet if it works? I dont have access to old wallets.


Reply to this email directly or view it on GitHub
#59 (comment)
.

Thomas Winget
Computer Engineering
Purdue University '12

@jakoblind
Copy link
Contributor

@tewinget thanks for info. The code seems to work. Will implement the RPC calls before I do a PR.

@fluffypony
Copy link
Contributor

Merged and closed

stoffu pushed a commit to stoffu/monero that referenced this issue Sep 27, 2018
Add Discord community link to Readme.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants