diff --git a/ethstore/src/account/safe_account.rs b/ethstore/src/account/safe_account.rs index 059f988d0dd..ec80c95372a 100644 --- a/ethstore/src/account/safe_account.rs +++ b/ethstore/src/account/safe_account.rs @@ -15,7 +15,6 @@ // along with Parity. If not, see . use std::ops::{Deref, DerefMut}; -use std::path::{PathBuf}; use ethkey::{KeyPair, sign, Address, Secret, Signature, Message}; use {json, Error, crypto}; use crypto::Keccak256; @@ -36,7 +35,7 @@ pub struct SafeAccount { pub version: Version, pub address: Address, pub crypto: Crypto, - pub path: Option, + pub filename: Option, pub name: String, pub meta: String, } @@ -63,20 +62,6 @@ impl Into for Crypto { } } -impl From for SafeAccount { - fn from(json: json::KeyFile) -> Self { - SafeAccount { - id: json.id.into(), - version: json.version.into(), - address: json.address.into(), - crypto: json.crypto.into(), - path: None, - name: json.name.unwrap_or(String::new()), - meta: json.meta.unwrap_or("{}".to_owned()), - } - } -} - impl Into for SafeAccount { fn into(self) -> json::KeyFile { json::KeyFile { @@ -147,26 +132,32 @@ impl Crypto { } impl SafeAccount { - // DEPRECATED. use `create_with_name` instead - pub fn create(keypair: &KeyPair, id: [u8; 16], password: &str, iterations: u32, name: String, meta: String) -> Self { + pub fn create( + keypair: &KeyPair, + id: [u8; 16], + password: &str, + iterations: u32, + name: String, + meta: String + ) -> Self { SafeAccount { id: id, version: Version::V3, crypto: Crypto::create(keypair.secret(), password, iterations), address: keypair.address(), - path: None, + filename: None, name: name, meta: meta, } } - pub fn from_file(json: json::KeyFile, path: PathBuf) -> Self { + pub fn from_file(json: json::KeyFile, filename: String) -> Self { SafeAccount { id: json.id.into(), version: json.version.into(), address: json.address.into(), crypto: json.crypto.into(), - path: Some(path), + filename: Some(filename), name: json.name.unwrap_or(String::new()), meta: json.meta.unwrap_or("{}".to_owned()), } @@ -184,7 +175,7 @@ impl SafeAccount { version: self.version.clone(), crypto: Crypto::create(&secret, new_password, iterations), address: self.address.clone(), - path: self.path.clone(), + filename: self.filename.clone(), name: self.name.clone(), meta: self.meta.clone(), }; diff --git a/ethstore/src/dir/disk.rs b/ethstore/src/dir/disk.rs index ef898a3addd..d48fa1b589d 100644 --- a/ethstore/src/dir/disk.rs +++ b/ethstore/src/dir/disk.rs @@ -77,7 +77,9 @@ impl DiskDirectory { .map(json::KeyFile::load) .zip(paths.into_iter()) .map(|(file, path)| match file { - Ok(file) => Ok((path, file.into())), + Ok(file) => Ok((path.clone(), SafeAccount::from_file( + file, path.file_name().and_then(|n| n.to_str()).expect("Keys have valid UTF8 names only.").to_owned() + ))), Err(err) => Err(Error::InvalidKeyFile(format!("{:?}: {}", path, err))), }) .collect() @@ -98,22 +100,26 @@ impl KeyDirectory for DiskDirectory { let keyfile: json::KeyFile = account.clone().into(); // build file path - let mut account = account; - account.path = account.path.or_else(|| { - let mut keyfile_path = self.path.clone(); - let timestamp = time::strftime("%Y-%m-%d_%H:%M:%S_%Z", &time::now()).unwrap_or("???".to_owned()); - keyfile_path.push(format!("{}-{}.json", keyfile.id, timestamp)); - Some(keyfile_path) + let filename = account.filename.as_ref().cloned().unwrap_or_else(|| { + let timestamp = time::strftime("%Y-%m-%dT%H-%M-%S", &time::now_utc()).expect("Time-format string is valid."); + format!("UTC--{}Z--{:?}", timestamp, account.address) }); + // update account filename + let mut account = account; + account.filename = Some(filename.clone()); + { + // Path to keyfile + let mut keyfile_path = self.path.clone(); + keyfile_path.push(filename.as_str()); + // save the file - let path = account.path.as_ref().expect("build-file-path ensures is not None; qed"); - let mut file = try!(fs::File::create(path)); + let mut file = try!(fs::File::create(&keyfile_path)); try!(keyfile.write(&mut file).map_err(|e| Error::Custom(format!("{:?}", e)))); - if let Err(_) = restrict_permissions_to_owner(path) { - fs::remove_file(path).expect("Expected to remove recently created file"); + if let Err(_) = restrict_permissions_to_owner(keyfile_path.as_path()) { + fs::remove_file(keyfile_path).expect("Expected to remove recently created file"); return Err(Error::Io(io::Error::last_os_error())); } } @@ -135,3 +141,34 @@ impl KeyDirectory for DiskDirectory { } } } + + +#[cfg(test)] +mod test { + use std::{env, fs}; + use super::DiskDirectory; + use dir::KeyDirectory; + use account::SafeAccount; + use ethkey::{Random, Generator}; + + #[test] + fn should_create_new_account() { + // given + let dir = env::temp_dir(); + let keypair = Random.generate().unwrap(); + let password = "hello world"; + let directory = DiskDirectory::create(dir.clone()).unwrap(); + + // when + let account = SafeAccount::create(&keypair, [0u8; 16], password, 1024, "Test".to_owned(), "{}".to_owned()); + let res = directory.insert(account); + + + // then + assert!(res.is_ok(), "Should save account succesfuly."); + assert!(res.unwrap().filename.is_some(), "Filename has been assigned."); + + // cleanup + let _ = fs::remove_dir_all(dir); + } +} diff --git a/ethstore/src/import.rs b/ethstore/src/import.rs index 79785ae0f33..b3f23107ad0 100644 --- a/ethstore/src/import.rs +++ b/ethstore/src/import.rs @@ -14,15 +14,20 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . +use std::collections::HashSet; use ethkey::Address; use dir::KeyDirectory; use Error; pub fn import_accounts(src: &KeyDirectory, dst: &KeyDirectory) -> Result, Error> { let accounts = try!(src.load()); - accounts.into_iter().map(|a| { - let address = a.address.clone(); - try!(dst.insert(a)); - Ok(address) - }).collect() + let existing_accounts = try!(dst.load()).into_iter().map(|a| a.address).collect::>(); + + accounts.into_iter() + .filter(|a| !existing_accounts.contains(&a.address)) + .map(|a| { + let address = a.address.clone(); + try!(dst.insert(a)); + Ok(address) + }).collect() } diff --git a/parity/account.rs b/parity/account.rs index 3c4a5dd7496..26a97409095 100644 --- a/parity/account.rs +++ b/parity/account.rs @@ -47,21 +47,25 @@ pub fn execute(cmd: AccountCmd) -> Result { } } +fn keys_dir(path: String) -> Result { + DiskDirectory::create(path).map_err(|e| format!("Could not open keys directory: {}", e)) +} + fn new(n: NewAccount) -> Result { let password: String = match n.password_file { Some(file) => try!(password_from_file(file)), None => try!(password_prompt()), }; - let dir = Box::new(DiskDirectory::create(n.path).unwrap()); + let dir = Box::new(try!(keys_dir(n.path))); let secret_store = Box::new(EthStore::open_with_iterations(dir, n.iterations).unwrap()); let acc_provider = AccountProvider::new(secret_store); - let new_account = acc_provider.new_account(&password).unwrap(); + let new_account = try!(acc_provider.new_account(&password).map_err(|e| format!("Could not create new account: {}", e))); Ok(format!("{:?}", new_account)) } fn list(path: String) -> Result { - let dir = Box::new(DiskDirectory::create(path).unwrap()); + let dir = Box::new(try!(keys_dir(path))); let secret_store = Box::new(EthStore::open(dir).unwrap()); let acc_provider = AccountProvider::new(secret_store); let accounts = acc_provider.accounts(); @@ -74,7 +78,7 @@ fn list(path: String) -> Result { } fn import(i: ImportAccounts) -> Result { - let to = DiskDirectory::create(i.to).unwrap(); + let to = try!(keys_dir(i.to)); let mut imported = 0; for path in &i.from { let from = DiskDirectory::at(path); diff --git a/parity/run.rs b/parity/run.rs index 45b844ef048..57a77794c62 100644 --- a/parity/run.rs +++ b/parity/run.rs @@ -298,7 +298,7 @@ fn prepare_account_provider(dirs: &Directories, cfg: AccountsConfig) -> Result {} Err(Error::Io(ref io_err)) if io_err.kind() == ErrorKind::NotFound => {} @@ -306,8 +306,10 @@ fn prepare_account_provider(dirs: &Directories, cfg: AccountsConfig) -> Result