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

Fix key.meta.vault for root dir keys && read vault.meta without vault key #4482

Merged
merged 2 commits into from Feb 9, 2017

Conversation

svyatonik
Copy link
Collaborator

Details:

  1. When key is moved from vault back to the root directory, it's meta still contains the vault field. Fixed + added test on RPC level
  2. As vault meta is intended to contain hint for vault password, it was a bad idea to allow reading meta for opened vaults only => added support for reading unopened vaults meta + updated test.

@svyatonik
Copy link
Collaborator Author

@jacogr

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Feb 8, 2017
@svyatonik svyatonik closed this Feb 9, 2017
@svyatonik
Copy link
Collaborator Author

  1. Added support for vaults, created before vault.meta appeared (since it is 'no cost' && it fails if there is no meta field in vault files)

@svyatonik svyatonik reopened this Feb 9, 2017
@rphmeier
Copy link
Contributor

rphmeier commented Feb 9, 2017

Did these "old vaults" exist in any published release? Why bother supporting them if not?

@svyatonik
Copy link
Collaborator Author

@rphmeier Nope, but meta field for vaults is actually optional (this is also true for key files: KeyFile works the same way:
https://github.com/ethcore/parity/blob/master/ethstore/src/json/key_file.rs#L101 ). So it seems logical to me to react as return meta=None if there's "meta": null in json, than to fail RPC request.

@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 9, 2017
@gavofyork gavofyork merged commit 1534bbb into master Feb 9, 2017
@gavofyork gavofyork deleted the fix_vault_in_root_dir branch February 9, 2017 15:47
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

4 participants