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

Changes blockchain storage location to AppData Local. #2238

Closed
wants to merge 1 commit into from
Closed

Changes blockchain storage location to AppData Local. #2238

wants to merge 1 commit into from

Conversation

MicahZoltu
Copy link
Contributor

Fixes #2237.

Note: This will likely not be backwards compatible. A more complete change would be to check at startup for the blockchain in AppData/Roaming and if found migrate it to AppData/Local. If nothing is found then just use AppData/Local.

@robotally
Copy link

Vote Count Reviewers
👍 0
👎 0

Updated: Tue Feb 23 21:44:25 UTC 2016

@karalabe
Copy link
Member

Unfortunately this change won't be this easy, as it would break Ethereum for all existing Windows installations. The way I can imagine this working is to check if the Roaming version exists, if yes, use that, otherwise default to Local. That way we could move it for new users to Local and eventually deprecate Roaming, but it's going to be a long process. We can't just swap around the folders.

Another potential solution would be to actually move the folder from Roaming to Local, but I'm not sure we want to do so invasive file manipulations on users' computers, especially when they store the keys too.

@bas-vk
Copy link
Member

bas-vk commented Feb 23, 2016

The problem with the datadir is that it contains public and private data (keystore). Setting datadir to AppData/Roaming or %PROGRAMDATA% is actually a security issue since your keys are synced to some server on logout or accessible by anyone with access to the computer.

I think that AppData/Local in an environment with roaming profiles isn't a good alternative. Employees probably switch/share computers and will need to sync, or verify the chain left by the previous user and restore their keys each time they use a different computers.

Having that said I still think the best solution would be to put everything by default in AppData/Local. For regular users this is fine. Corporate users must figure out a solution that works for their particular situation. These users probably won't use geth's keystore but use external singing in combination with a light client or remote node somewhere on the internal network. We can consider adding a new flag which allows users to overrule the default location for the keystore. That will allow them to separate the public and private data in the datadir. @karalabe, what do you think?

We should not move the folder automatically. There is just too much that can go wrong. What we can do is print a message on the console with the advice to manually move the folder. It then up to the user to decide if he wants to move the folder.

@fjl
Copy link
Contributor

fjl commented Feb 23, 2016

I think using AppData\Local for new (but not existing) datadir is reasonable.
@bas-vk I'll add the keystore directory flag later if I get to doing the bigger changes I wanted to make, but it can also be done separately.

@MicahZoltu
Copy link
Contributor Author

If someone is going to take on the larger project of doing this right, it would probably be good to address #2239 at the same time since I'm guessing you'll have to touch the same code to fix both.

Also, feel free to close this PR if someone else is going to make a PR that does the more correct thing. I put this up mainly to show the problem and the high level solution, I recognize that it is nowhere near what is necessary to actually address the backwards compatibility issue. Alternatively, if there is value in keeping the discussion here leave it open.

@fjl fjl added the windows label Mar 14, 2016
@fjl fjl added this to the 1.5.0 milestone Mar 14, 2016
@fjl fjl closed this Aug 17, 2016
@fjl fjl removed the in progress label Aug 17, 2016
maoueh pushed a commit to streamingfast/go-ethereum that referenced this pull request Mar 4, 2024
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

Successfully merging this pull request may close these issues.

5 participants