Fix wallet import

ethcore/src/account_provider/mod.rs

@@ -264,9 +264,9 @@ impl AccountProvider {
 		Ok(Address::from(account.address).into())
 	}
 
-	/// Import a new presale wallet.
-	pub fn import_wallet(&self, json: &[u8], password: &str) -> Result<Address, Error> {
-		let account = self.sstore.import_wallet(SecretVaultRef::Root, json, password)?;
+	/// Import a new wallet.
+	pub fn import_wallet(&self, json: &[u8], password: &str, gen_id: bool) -> Result<Address, Error> {
+		let account = self.sstore.import_wallet(SecretVaultRef::Root, json, password, gen_id)?;
 		if self.blacklisted_accounts.contains(&account.address) {
 			self.sstore.remove_account(&account, password)?;
 			return Err(SSError::InvalidAccount.into());

ethstore/src/accounts_dir/disk.rs

@@ -153,8 +153,42 @@ impl<T> DiskDirectory<T> where T: KeyFileManager {
 		)
 	}
 
-	/// insert account with given file name
-	pub fn insert_with_filename(&self, account: SafeAccount, filename: String) -> Result<SafeAccount, Error> {
+	/// insert account with given file name. if the filename is a duplicate of any stored account, a number is appended
+	/// to the filename.
+	pub fn insert_with_filename(&self, account: SafeAccount, mut filename: String) -> Result<SafeAccount, Error> {
+		// check for duplicate filenames and append/increment counter
+		let dups: Vec<_> = self.files()?.into_iter().filter_map(|f| {
+			match f.file_name().map(|n| n.to_string_lossy()) {
+				Some(ref name) if name.starts_with(&filename) => Some(name.to_string()),
+				_ => None
+			}
+		}).collect();
+
+		if !dups.is_empty() {
+			let mut max = 0;
+			let mut found = false;
+
+			for dup in dups {
+				if dup.len() == filename.len() {
+					// we found a file with the same name
+					found = true;
+				}
+				else if dup[filename.len()..].starts_with("-") {
+					// this is a duplicate file with a counter appended
+					match dup[filename.len() + 1..].parse() {
+						Ok(n) if n > max => {
+							max = n;
+						},
+						_ => {},
+					}
+				}
+			}
+
+			if found {
+				filename.push_str(&format!("-{}", max + 1))
+			}
+		}
+
 		// update account filename
 		let original_account = account.clone();
 		let mut account = account;
@@ -317,6 +351,34 @@ mod test {
 		let _ = fs::remove_dir_all(dir);
 	}
 
+	#[test]
+	fn should_handle_duplicate_filenames() {
+		// given
+		let mut dir = env::temp_dir();
+		dir.push("ethstore_should_handle_duplicate_filenames");
+		let keypair = Random.generate().unwrap();
+		let password = "hello world";
+		let directory = RootDiskDirectory::create(dir.clone()).unwrap();
+
+		// when
+		let account = SafeAccount::create(&keypair, [0u8; 16], password, 1024, "Test".to_owned(), "{}".to_owned());
+		let filename = "test".to_string();
+
+		directory.insert_with_filename(account.clone(), "foo".to_string()).unwrap();
+		directory.insert_with_filename(account.clone(), "testa".to_string()).unwrap();
+		let res1 = directory.insert_with_filename(account.clone(), filename.clone());
+		let res2 = directory.insert_with_filename(account.clone(), filename.clone());
+		let res3 = directory.insert_with_filename(account.clone(), filename.clone());
+
+		// then
+		assert_eq!(res1.unwrap().filename.unwrap(), filename);
+		assert_eq!(res2.unwrap().filename.unwrap(), format!("{}-1", filename));
+		assert_eq!(res3.unwrap().filename.unwrap(), format!("{}-2", filename));
+
+		// cleanup
+		let _ = fs::remove_dir_all(dir);
+	}
+
 	#[test]
 	fn should_manage_vaults() {
 		// given

ethstore/src/ethstore.rs

@@ -166,9 +166,14 @@ impl SecretStore for EthStore {
 		self.insert_account(vault, keypair.secret().clone(), password)
 	}
 
-	fn import_wallet(&self, vault: SecretVaultRef, json: &[u8], password: &str) -> Result<StoreAccountRef, Error> {
+	fn import_wallet(&self, vault: SecretVaultRef, json: &[u8], password: &str, gen_id: bool) -> Result<StoreAccountRef, Error> {
 		let json_keyfile = json::KeyFile::load(json).map_err(|_| Error::InvalidKeyFile("Invalid JSON format".to_owned()))?;
 		let mut safe_account = SafeAccount::from_file(json_keyfile, None);
+
+		if gen_id {
+			safe_account.id = Random::random();
+		}
+
 		let secret = safe_account.crypto.secret(password).map_err(|_| Error::InvalidPassword)?;
 		safe_account.address = KeyPair::from_secret(secret)?.address();
 		self.store.import(vault, safe_account)

ethstore/src/secret_store.rs

@@ -116,7 +116,7 @@ pub trait SecretStore: SimpleSecretStore {
 	/// Imports presale wallet
 	fn import_presale(&self, vault: SecretVaultRef, json: &[u8], password: &str) -> Result<StoreAccountRef, Error>;
 	/// Imports existing JSON wallet
-	fn import_wallet(&self, vault: SecretVaultRef, json: &[u8], password: &str) -> Result<StoreAccountRef, Error>;
+	fn import_wallet(&self, vault: SecretVaultRef, json: &[u8], password: &str, gen_id: bool) -> Result<StoreAccountRef, Error>;
 	/// Copies account between stores and vaults.
 	fn copy_account(&self, new_store: &SimpleSecretStore, new_vault: SecretVaultRef, account: &StoreAccountRef, password: &str, new_password: &str) -> Result<(), Error>;
 	/// Checks if password matches given account.

rpc/src/v1/impls/parity_accounts.rs

@@ -95,7 +95,7 @@ impl ParityAccounts for ParityAccountsClient {
 		let store = self.account_provider()?;
 
 		store.import_presale(json.as_bytes(), &pass)
-			.or_else(|_| store.import_wallet(json.as_bytes(), &pass))
+			.or_else(|_| store.import_wallet(json.as_bytes(), &pass, true))
 			.map(Into::into)
 			.map_err(|e| errors::account("Could not create account.", e))
 	}

rpc/src/v1/tests/mocked/parity_accounts.rs

@@ -480,7 +480,7 @@ fn should_export_account() {
 	// given
 	let tester = setup();
 	let wallet = r#"{"id":"6a186c80-7797-cff2-bc2e-7c1d6a6cc76e","version":3,"crypto":{"cipher":"aes-128-ctr","cipherparams":{"iv":"a1c6ff99070f8032ca1c4e8add006373"},"ciphertext":"df27e3db64aa18d984b6439443f73660643c2d119a6f0fa2fa9a6456fc802d75","kdf":"pbkdf2","kdfparams":{"c":10240,"dklen":32,"prf":"hmac-sha256","salt":"ddc325335cda5567a1719313e73b4842511f3e4a837c9658eeb78e51ebe8c815"},"mac":"3dc888ae79cbb226ff9c455669f6cf2d79be72120f2298f6cb0d444fddc0aa3d"},"address":"0042e5d2a662eeaca8a7e828c174f98f35d8925b","name":"parity-export-test","meta":"{\"passwordHint\":\"parity-export-test\",\"timestamp\":1490017814987}"}"#;
-	tester.accounts.import_wallet(wallet.as_bytes(), "parity-export-test").unwrap();
+	tester.accounts.import_wallet(wallet.as_bytes(), "parity-export-test", false).unwrap();
 	let accounts = tester.accounts.accounts().unwrap();
 	assert_eq!(accounts.len(), 1);
 
@@ -501,6 +501,26 @@ fn should_export_account() {
 	assert_eq!(result, Some(response.into()));
 }
 
+#[test]
+fn should_import_wallet() {
+	let tester = setup();
+
+	let id = "6a186c80-7797-cff2-bc2e-7c1d6a6cc76e";
+	let request = r#"{"jsonrpc":"2.0","method":"parity_newAccountFromWallet","params":["{\"id\":\"<ID>\",\"version\":3,\"crypto\":{\"cipher\":\"aes-128-ctr\",\"cipherparams\":{\"iv\":\"478736fb55872c1baf01b27b1998c90b\"},\"ciphertext\":\"fe5a63cc0055d7b0b3b57886f930ad9b63f48950d1348145d95996c41e05f4e0\",\"kdf\":\"pbkdf2\",\"kdfparams\":{\"c\":10240,\"dklen\":32,\"prf\":\"hmac-sha256\",\"salt\":\"658436d6738a19731149a98744e5cf02c8d5aa1f8e80c1a43cc9351c70a984e4\"},\"mac\":\"c7384b26ecf25539d942030230062af9b69de5766cbcc4690bffce1536644631\"},\"address\":\"00bac56a8a27232baa044c03f43bf3648c961735\",\"name\":\"hello world\",\"meta\":\"{}\"}", "himom"],"id":1}"#;
+	let request = request.replace("<ID>", id);
+	let response = r#"{"jsonrpc":"2.0","result":"0x00bac56a8a27232baa044c03f43bf3648c961735","id":1}"#;
+
+	let res = tester.io.handle_request_sync(&request).unwrap();
+
+	assert_eq!(res, response);
+
+	let account_meta = tester.accounts.account_meta("0x00bac56a8a27232baa044c03f43bf3648c961735".into()).unwrap();
+	let account_uuid: String = account_meta.uuid.unwrap().into();
+
+	// the RPC should import the account with a new id
+	assert!(account_uuid != id);
+}
+
 #[test]
 fn should_sign_message() {
 	let tester = setup();