Key files include timestamp in name. (#1700)

* Key files include timestamp in name.

Introduce timestamp into new key files; keep filename around, so
that we don't accidentally duplicate keys.

* Remove unnecessary clone

* Fix test code.

* Remove log module from ethstore

Cargo.lock

@@ -471,6 +471,7 @@ dependencies = [
  "serde_codegen 0.7.9 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde_json 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "syntex 0.33.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "time 0.1.35 (registry+https://github.com/rust-lang/crates.io-index)",
  "tiny-keccak 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 

ethcore/src/account_provider.rs

@@ -114,8 +114,8 @@ impl KeyDirectory for NullDir {
 		Ok(vec![])
 	}
 
-	fn insert(&self, _account: SafeAccount) -> Result<(), SSError> {
+	fn insert(&self, account: SafeAccount) -> Result<SafeAccount, SSError> {
-		Ok(())
+		Ok(account)
 	}
 
 	fn remove(&self, _address: &SSAddress) -> Result<(), SSError> {

ethstore/Cargo.toml

@@ -15,6 +15,7 @@ rustc-serialize = "0.3"
 rust-crypto = "0.2.36"
 tiny-keccak = "1.0"
 docopt = { version = "0.6", optional = true }
+time = "0.1.34"
 
 [build-dependencies]
 serde_codegen = { version = "0.7", optional = true }

ethstore/src/account/safe_account.rs

@@ -15,6 +15,7 @@
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
 use std::ops::{Deref, DerefMut};
+use std::path::{PathBuf};
 use ethkey::{KeyPair, sign, Address, Secret, Signature, Message};
 use {json, Error, crypto};
 use crypto::Keccak256;
@@ -35,6 +36,7 @@ pub struct SafeAccount {
 	pub version: Version,
 	pub address: Address,
 	pub crypto: Crypto,
+	pub path: Option<PathBuf>,
 	pub name: String,
 	pub meta: String,
 }
@@ -42,9 +44,9 @@ pub struct SafeAccount {
 impl From<json::Crypto> for Crypto {
 	fn from(json: json::Crypto) -> Self {
 		Crypto {
-			cipher: From::from(json.cipher),
+			cipher: json.cipher.into(),
 			ciphertext: json.ciphertext.into(),
-			kdf: From::from(json.kdf),
+			kdf: json.kdf.into(),
 			mac: json.mac.into(),
 		}
 	}
@@ -54,9 +56,9 @@ impl Into<json::Crypto> for Crypto {
 	fn into(self) -> json::Crypto {
 		json::Crypto {
 			cipher: self.cipher.into(),
-			ciphertext: From::from(self.ciphertext),
+			ciphertext: self.ciphertext.into(),
 			kdf: self.kdf.into(),
-			mac: From::from(self.mac),
+			mac: self.mac.into(),
 		}
 	}
 }
@@ -65,9 +67,10 @@ impl From<json::KeyFile> for SafeAccount {
 	fn from(json: json::KeyFile) -> Self {
 		SafeAccount {
 			id: json.id.into(),
-			version: From::from(json.version),
+			version: json.version.into(),
-			address: From::from(json.address), //json.address.into(),
+			address: json.address.into(),
-			crypto: From::from(json.crypto),
+			crypto: json.crypto.into(),
+			path: None,
 			name: json.name.unwrap_or(String::new()),
 			meta: json.meta.unwrap_or("{}".to_owned()),
 		}
@@ -151,11 +154,24 @@ impl SafeAccount {
 			version: Version::V3,
 			crypto: Crypto::create(keypair.secret(), password, iterations),
 			address: keypair.address(),
+			path: None,
 			name: name,
 			meta: meta,
 		}
 	}
 
+	pub fn from_file(json: json::KeyFile, path: PathBuf) -> Self {
+		SafeAccount {
+			id: json.id.into(),
+			version: json.version.into(),
+			address: json.address.into(),
+			crypto: json.crypto.into(),
+			path: Some(path),
+			name: json.name.unwrap_or(String::new()),
+			meta: json.meta.unwrap_or("{}".to_owned()),
+		}
+	}
+
 	pub fn sign(&self, password: &str, message: &Message) -> Result<Signature, Error> {
 		let secret = try!(self.crypto.secret(password));
 		sign(&secret, message).map_err(From::from)
@@ -168,6 +184,7 @@ impl SafeAccount {
 			version: self.version.clone(),
 			crypto: Crypto::create(&secret, new_password, iterations),
 			address: self.address.clone(),
+			path: self.path.clone(),
 			name: self.name.clone(),
 			meta: self.meta.clone(),
 		};

ethstore/src/dir/disk.rs

@@ -17,6 +17,7 @@
 use std::{fs, ffi, io};
 use std::path::{PathBuf, Path};
 use std::collections::HashMap;
+use time;
 use ethkey::Address;
 use {libc, json, SafeAccount, Error};
 use super::KeyDirectory;
@@ -73,7 +74,7 @@ impl DiskDirectory {
 		let accounts = files.into_iter()
 			.map(json::KeyFile::load)
 			.zip(paths.into_iter())
-			.filter_map(|(file, path)| file.ok().map(|file| (path, SafeAccount::from(file))))
+			.filter_map(|(file, path)| file.ok().map(|file| (path.clone(), SafeAccount::from_file(file, path))))
 			.collect();
 
 		Ok(accounts)
@@ -89,24 +90,32 @@ impl KeyDirectory for DiskDirectory {
 		Ok(accounts)
 	}
 
-	fn insert(&self, account: SafeAccount) -> Result<(), Error> {
+	fn insert(&self, account: SafeAccount) -> Result<SafeAccount, Error> {
 		// transform account into key file
-		let keyfile: json::KeyFile = account.into();
+		let keyfile: json::KeyFile = account.clone().into();
 
 		// build file path
-		let mut keyfile_path = self.path.clone();
+		let mut account = account;
-		keyfile_path.push(format!("{}", keyfile.id));
+		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)
+		});
 
-		// save the file
+		{
-		let mut file = try!(fs::File::create(&keyfile_path));
+			// save the file
-		try!(keyfile.write(&mut file).map_err(|e| Error::Custom(format!("{:?}", e))));
+			let path = account.path.as_ref().expect("build-file-path ensures is not None; qed");
+			let mut file = try!(fs::File::create(path));
+			try!(keyfile.write(&mut file).map_err(|e| Error::Custom(format!("{:?}", e))));
 
-		if let Err(_) = restrict_permissions_to_owner(&keyfile_path) {
+			if let Err(_) = restrict_permissions_to_owner(path) {
-			fs::remove_file(&keyfile_path).expect("Expected to remove recently created file");
+				fs::remove_file(path).expect("Expected to remove recently created file");
-			return Err(Error::Io(io::Error::last_os_error()));
+				return Err(Error::Io(io::Error::last_os_error()));
+			}
 		}
 
-		Ok(())
+		Ok(account)
 	}
 
 	fn remove(&self, address: &Address) -> Result<(), Error> {

ethstore/src/dir/geth.rs

@@ -85,7 +85,7 @@ impl KeyDirectory for GethDirectory {
 		self.dir.load()
 	}
 
-	fn insert(&self, account: SafeAccount) -> Result<(), Error> {
+	fn insert(&self, account: SafeAccount) -> Result<SafeAccount, Error> {
 		self.dir.insert(account)
 	}
 

ethstore/src/dir/mod.rs

@@ -28,7 +28,7 @@ pub enum DirectoryType {
 
 pub trait KeyDirectory: Send + Sync {
 	fn load(&self) -> Result<Vec<SafeAccount>, Error>;
-	fn insert(&self, account: SafeAccount) -> Result<(), Error>;
+	fn insert(&self, account: SafeAccount) -> Result<SafeAccount, Error>;
 	fn remove(&self, address: &Address) -> Result<(), Error>;
 }
 

ethstore/src/dir/parity.rs

@@ -64,7 +64,7 @@ impl KeyDirectory for ParityDirectory {
 		self.dir.load()
 	}
 
-	fn insert(&self, account: SafeAccount) -> Result<(), Error> {
+	fn insert(&self, account: SafeAccount) -> Result<SafeAccount, Error> {
 		self.dir.insert(account)
 	}
 

ethstore/src/ethstore.rs

@@ -49,7 +49,7 @@ impl EthStore {
 
 	fn save(&self, account: SafeAccount) -> Result<(), Error> {
 		// save to file
-		try!(self.dir.insert(account.clone()));
+		let account = try!(self.dir.insert(account.clone()));
 
 		// update cache
 		let mut cache = self.cache.write().unwrap();

ethstore/src/lib.rs

@@ -19,6 +19,7 @@
 
 extern crate libc;
 extern crate rand;
+extern crate time;
 extern crate serde;
 extern crate serde_json;
 extern crate rustc_serialize;

ethstore/tests/util/transient_dir.rs

@@ -64,7 +64,7 @@ impl KeyDirectory for TransientDir {
 		self.dir.load()
 	}
 
-	fn insert(&self, account: SafeAccount) -> Result<(), Error> {
+	fn insert(&self, account: SafeAccount) -> Result<SafeAccount, Error> {
 		self.dir.insert(account)
 	}