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

Wallet files shouldn't give away the address #3378

Merged
merged 1 commit into from
Nov 17, 2016
Merged

Conversation

gavofyork
Copy link
Contributor

@gavofyork gavofyork commented Nov 11, 2016

This immediately de-anonymises the JSON file and should never have gone in. Eventually we'll want to anonymise the files completely through storing the addresses encrypted, but that can come later.

(Actually wallet files shouldn't contain it either, but we'll
leave that for a later PR).
@gavofyork gavofyork added A0-pleasereview 🤓 Pull request needs code review. B0-patch M4-core ⛓ Core client code / Rust. labels Nov 11, 2016
@gavofyork gavofyork changed the title Wallet names shouldn't include address. Wallet names shouldn't include uuid. Nov 13, 2016
@arkpar
Copy link
Collaborator

arkpar commented Nov 14, 2016

How does this fix the mentioned issue? What would be the imported account name?

@rphmeier
Copy link
Contributor

Why shouldn't they? Can sensitive information be derived from the UUID?

@gavofyork
Copy link
Contributor Author

the original issue related to our default naming of imported wallets; they were named after the UUID rather than their address. UUIDs are very obscure and shouldn't ever have been brought in to user-visible records (mea culpa). this fixes it.

@arkpar
Copy link
Collaborator

arkpar commented Nov 16, 2016

@gavofyork But you are adding the UUID to the file name in this PR. How does that prevent it from being displayed in the UI?

@arkpar arkpar added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 16, 2016
@gavofyork gavofyork changed the title Wallet names shouldn't include uuid. Wallet files shouldn't give away the address Nov 17, 2016
@gavofyork gavofyork added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Nov 17, 2016
@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 17, 2016
@gavofyork gavofyork merged commit 64a4b60 into master Nov 17, 2016
@gavofyork gavofyork deleted the fix-wallet-naming branch November 17, 2016 10:30
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. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants