Avoid importing keys into wrong place. (#1119)

* Avoid importing keys into wrong place.

- Now a `--no-import-keys` option;
- `--testnet` now alters the import location for keys and the store path.

Fixes #1112 and #1089.

* Avoid double-bools.

parity/cli.rs

@@ -45,6 +45,7 @@ Account Options:
   --keys-iterations NUM    Specify the number of iterations to use when deriving key
                            from the password (bigger is more secure)
                            [default: 10240].
+  --no-import-keys         Do not import keys from legacy clients.
 
 Networking Options:
   --port PORT              Override the port on which the node should listen
@@ -148,8 +149,10 @@ Legacy Options:
   --geth                   Run in Geth-compatibility mode. Currently just sets
                            the IPC path to be the same as Geth's. Overrides
                            the --ipc-path/--ipcpath options.
+  --testnet                Geth-compatible testnet mode. Equivalent to --chain
+                           testnet --keys-path $HOME/parity/testnet-keys.
+                           Overrides the --keys-path option.
   --datadir PATH           Equivalent to --db-path PATH.
-  --testnet                Equivalent to --chain testnet.
   --networkid INDEX        Equivalent to --network-id INDEX.
   --maxpeers COUNT         Equivalent to --peers COUNT.
   --nodekey KEY            Equivalent to --node-key KEY.
@@ -192,6 +195,7 @@ pub struct Args {
 	pub flag_cache: Option<usize>,
 	pub flag_keys_path: String,
 	pub flag_keys_iterations: u32,
+	pub flag_no_import_keys: bool,
 	pub flag_bootnodes: Option<String>,
 	pub flag_network_id: Option<String>,
 	pub flag_pruning: String,

parity/configuration.rs

@@ -24,7 +24,7 @@ use docopt::Docopt;
 
 use die::*;
 use util::*;
-use util::keys::store::AccountService;
+use util::keys::store::{ImportKeySet, AccountService};
 use util::network_settings::NetworkSettings;
 use ethcore::client::{append_path, get_db_path, ClientConfig, Switch, VMType};
 use ethcore::ethereum;
@@ -256,7 +256,12 @@ impl Configuration {
 				.collect::<Vec<_>>()
 				.into_iter()
 		}).collect::<Vec<_>>();
-		let account_service = AccountService::with_security(Path::new(&self.keys_path()), self.keys_iterations());
+		let import_keys = match (self.args.flag_no_import_keys, self.args.flag_testnet) {
+			(true, _) => ImportKeySet::None,
+			(false, false) => ImportKeySet::Legacy,
+			(false, true) => ImportKeySet::LegacyTestnet,
+		};
+		let account_service = AccountService::with_security(Path::new(&self.keys_path()), self.keys_iterations(), import_keys);
 		if let Some(ref unlocks) = self.args.flag_unlock {
 			for d in unlocks.split(',') {
 				let a = Address::from_str(clean_0x(d)).unwrap_or_else(|_| {
@@ -313,7 +318,10 @@ impl Configuration {
 			self.args.flag_datadir.as_ref().unwrap_or(&self.args.flag_db_path));
 		::std::fs::create_dir_all(&db_path).unwrap_or_else(|e| die_with_io_error("main", e));
 
-		let keys_path = Configuration::replace_home(&self.args.flag_keys_path);
+		let keys_path = Configuration::replace_home(match self.args.flag_testnet {
+			true => "$HOME/.parity/testnet_keys",
+			false => &self.args.flag_keys_path,
+		});
 		::std::fs::create_dir_all(&db_path).unwrap_or_else(|e| die_with_io_error("main", e));
 
 		Directories {

util/src/keys/geth_import.rs

@@ -98,8 +98,8 @@ pub fn import_geth_keys(secret_store: &mut SecretStore, geth_keyfiles_directory:
 /// Gets the default geth keystore directory.
 ///
 /// Based on https://github.com/ethereum/go-ethereum/blob/e553215/common/path.go#L75
-pub fn keystore_dir() -> PathBuf {
-	path::ethereum::with_default("keystore")
+pub fn keystore_dir(is_testnet: bool) -> PathBuf {
+	path::ethereum::with_default(if is_testnet {"testnet/keystore"} else {"keystore"})
 }
 
 #[cfg(test)]

util/src/keys/store.rs

@@ -126,16 +126,25 @@ impl AccountProvider for AccountService {
 	}
 }
 
-impl AccountService {
-	/// New account service with the keys store in specific location
-	pub fn new_in(path: &Path) -> Self {
-		AccountService::with_security(path, KEY_ITERATIONS)
-	}
+/// Which set of keys to import.
+#[derive(PartialEq)]
+pub enum ImportKeySet {
+	/// Empty set.
+	None,
+	/// Import legacy client's general keys.
+	Legacy,
+	/// Import legacy client's testnet keys.
+	LegacyTestnet,
+}
 
-	/// New account service with the keys store in specific location and configured security parameters
-	pub fn with_security(path: &Path, key_iterations: u32) -> Self {
+impl AccountService {
+	/// New account service with the keys store in specific location and configured security parameters.
+	pub fn with_security(path: &Path, key_iterations: u32, import_keys: ImportKeySet) -> Self {
 		let secret_store = RwLock::new(SecretStore::with_security(path, key_iterations));
-		secret_store.write().unwrap().try_import_existing();
+		match import_keys {
+			ImportKeySet::None => {}
+			_ => { secret_store.write().unwrap().try_import_existing(import_keys == ImportKeySet::LegacyTestnet); }
+		}
 		AccountService {
 			secret_store: secret_store,
 		}
@@ -177,10 +186,10 @@ impl SecretStore {
 	}
 
 	/// trys to import keys in the known locations
-	pub fn try_import_existing(&mut self) {
+	pub fn try_import_existing(&mut self, is_testnet: bool) {
 		use keys::geth_import;
 
-		let import_path = geth_import::keystore_dir();
+		let import_path = geth_import::keystore_dir(is_testnet);
 		if let Err(e) = geth_import::import_geth_keys(self, &import_path) {
 			trace!(target: "sstore", "Geth key not imported: {:?}", e);
 		}