RPC for importing geth keys (#1916)

* Address book for local addresses.

* Simple disk-backed address book.

* Add test and keep file with keys.

* View and import Geth keys.

* Fix test.

* Fix up author info.

[ci:skip]

ethcore/src/account_provider.rs

@@ -16,15 +16,16 @@
 
 //! Account management.
 
-use std::fmt;
+use std::{fs, fmt};
 use std::collections::HashMap;
 use std::time::{Instant, Duration};
 use util::{Address as H160, H256, H520, Mutex, RwLock};
+use std::path::PathBuf;
+use ethjson::misc::AccountMeta;
 use ethstore::{SecretStore, Error as SSError, SafeAccount, EthStore};
 use ethstore::dir::{KeyDirectory};
 use ethstore::ethkey::{Address as SSAddress, Message as SSMessage, Secret as SSSecret, Random, Generator};
 
-
 /// Type of unlock.
 #[derive(Clone)]
 enum Unlock {
@@ -131,32 +132,74 @@ impl KeyDirectory for NullDir {
 	}
 }
 
+/// Disk-backed map from Address to String. Uses JSON.
+struct AddressBook {
+	path: PathBuf,
+	cache: HashMap<H160, AccountMeta>,
+}
+
+impl AddressBook {
+	pub fn new(path: String) -> Self {
+		trace!(target: "addressbook", "new({})", path);
+		let mut path: PathBuf = path.into();
+		path.push("address_book.json");
+		trace!(target: "addressbook", "path={:?}", path);
+		let mut r = AddressBook {
+			path: path,
+			cache: HashMap::new(),
+		};
+		r.revert();
+		r
+	}
+
+	pub fn get(&self) -> HashMap<H160, AccountMeta> {
+		self.cache.clone()
+	}
+
+	pub fn set_name(&mut self, a: H160, name: String) {
+		let mut x = self.cache.get(&a)
+			.map(|a| a.clone())
+			.unwrap_or(AccountMeta{name: Default::default(), meta: "{}".to_owned(), uuid: None});
+		x.name = name;
+		self.cache.insert(a, x);
+		self.save();
+	}
+
+	pub fn set_meta(&mut self, a: H160, meta: String) {
+		let mut x = self.cache.get(&a)
+			.map(|a| a.clone())
+			.unwrap_or(AccountMeta{name: "Anonymous".to_owned(), meta: Default::default(), uuid: None});
+		x.meta = meta;
+		self.cache.insert(a, x);
+		self.save();
+	}
+
+	fn revert(&mut self) {
+		trace!(target: "addressbook", "revert");
+		let _ = fs::File::open(self.path.clone())
+			.map_err(|e| trace!(target: "addressbook", "Couldn't open address book: {}", e))
+			.and_then(|f| AccountMeta::read_address_map(&f)
+				.map_err(|e| warn!(target: "addressbook", "Couldn't read address book: {}", e))
+				.and_then(|m| { self.cache = m; Ok(()) })
+			);
+	}
+
+	fn save(&mut self) {
+		trace!(target: "addressbook", "save");
+		let _ = fs::File::create(self.path.clone())
+			.map_err(|e| warn!(target: "addressbook", "Couldn't open address book for writing: {}", e))
+			.and_then(|mut f| AccountMeta::write_address_map(&self.cache, &mut f)
+				.map_err(|e| warn!(target: "addressbook", "Couldn't write to address book: {}", e))
+			);
+	}
+}
+
 /// Account management.
 /// Responsible for unlocking accounts.
 pub struct AccountProvider {
 	unlocked: Mutex<HashMap<SSAddress, AccountData>>,
 	sstore: Box<SecretStore>,
-}
-
-/// Collected account metadata
-#[derive(Clone, Debug, PartialEq)]
-pub struct AccountMeta {
-	/// The name of the account.
-	pub name: String,
-	/// The rest of the metadata of the account.
-	pub meta: String,
-	/// The 128-bit UUID of the account, if it has one (brain-wallets don't).
-	pub uuid: Option<String>,
-}
-
-impl Default for AccountMeta {
-	fn default() -> Self {
-		AccountMeta {
-			name: String::new(),
-			meta: "{}".to_owned(),
-			uuid: None,
-		}
-	}
+	address_book: Mutex<AddressBook>,
 }
 
 impl AccountProvider {
@@ -164,6 +207,7 @@ impl AccountProvider {
 	pub fn new(sstore: Box<SecretStore>) -> Self {
 		AccountProvider {
 			unlocked: Mutex::new(HashMap::new()),
+			address_book: Mutex::new(AddressBook::new(sstore.local_path().into())),
 			sstore: sstore,
 		}
 	}
@@ -172,6 +216,7 @@ impl AccountProvider {
 	pub fn transient_provider() -> Self {
 		AccountProvider {
 			unlocked: Mutex::new(HashMap::new()),
+			address_book: Mutex::new(AddressBook::new(Default::default())),
 			sstore: Box::new(EthStore::open(Box::new(NullDir::default())).unwrap())
 		}
 	}
@@ -209,6 +254,23 @@ impl AccountProvider {
 		Ok(accounts)
 	}
 
+	/// Returns each address along with metadata.
+	pub fn addresses_info(&self) -> Result<HashMap<H160, AccountMeta>, Error> {
+		Ok(self.address_book.lock().get())
+	}
+
+	/// Returns each address along with metadata.
+	pub fn set_address_name<A>(&self, account: A, name: String) -> Result<(), Error> where Address: From<A> {
+		let account = Address::from(account).into();
+		Ok(self.address_book.lock().set_name(account, name))
+	}
+
+	/// Returns each address along with metadata.
+	pub fn set_address_meta<A>(&self, account: A, meta: String) -> Result<(), Error> where Address: From<A> {
+		let account = Address::from(account).into();
+		Ok(self.address_book.lock().set_meta(account, meta))
+	}
+
 	/// Returns each account along with name and meta.
 	pub fn accounts_info(&self) -> Result<HashMap<H160, AccountMeta>, Error> {
 		let r: HashMap<H160, AccountMeta> = try!(self.sstore.accounts())
@@ -320,13 +382,38 @@ impl AccountProvider {
 		let signature = try!(self.sstore.sign(&account, &password, &message));
 		Ok(H520(signature.into()))
 	}
+
+	/// Returns the underlying `SecretStore` reference if one exists.
+	pub fn list_geth_accounts(&self, testnet: bool) -> Vec<H160> {
+		self.sstore.list_geth_accounts(testnet).into_iter().map(|a| Address::from(a).into()).collect()
+	}
+
+	/// Returns the underlying `SecretStore` reference if one exists.
+	pub fn import_geth_accounts(&self, desired: Vec<H160>, testnet: bool) -> Result<Vec<H160>, Error> {
+		let desired = desired.into_iter().map(|a| Address::from(a).into()).collect();
+		Ok(try!(self.sstore.import_geth_accounts(desired, testnet)).into_iter().map(|a| Address::from(a).into()).collect())
+	}
 }
 
 #[cfg(test)]
 mod tests {
-	use super::AccountProvider;
+	use super::{AccountProvider, AddressBook};
+	use std::collections::HashMap;
+	use ethjson::misc::AccountMeta;
 	use ethstore::ethkey::{Generator, Random};
 	use std::time::Duration;
+	use devtools::RandomTempPath;
+
+	#[test]
+	fn should_save_and_reload_address_book() {
+		let temp = RandomTempPath::create_dir();
+		let path = temp.as_str().to_owned();
+		let mut b = AddressBook::new(path.clone());
+		b.set_name(1.into(), "One".to_owned());
+		b.set_meta(1.into(), "{1:1}".to_owned());
+		let b = AddressBook::new(path);
+		assert_eq!(b.get(), hash_map![1.into() => AccountMeta{name: "One".to_owned(), meta: "{1:1}".to_owned(), uuid: None}]);
+	}
 
 	#[test]
 	fn unlock_account_temp() {

ethstore/src/dir/disk.rs

@@ -22,7 +22,7 @@ use ethkey::Address;
 use {json, SafeAccount, Error};
 use super::KeyDirectory;
 
-const IGNORED_FILES: &'static [&'static str] = &["thumbs.db"];
+const IGNORED_FILES: &'static [&'static str] = &["thumbs.db", "address_book.json"];
 
 #[cfg(not(windows))]
 fn restrict_permissions_to_owner(file_path: &Path) -> Result<(), i32>  {
@@ -149,6 +149,8 @@ impl KeyDirectory for DiskDirectory {
 			Some((path, _)) => fs::remove_file(path).map_err(From::from)
 		}
 	}
+
+	fn path(&self) -> Option<&PathBuf> { Some(&self.path) }
 }
 
 

ethstore/src/dir/mod.rs

@@ -15,6 +15,7 @@
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
 use ethkey::Address;
+use std::path::{PathBuf};
 use {SafeAccount, Error};
 
 mod disk;
@@ -30,6 +31,7 @@ pub trait KeyDirectory: Send + Sync {
 	fn load(&self) -> Result<Vec<SafeAccount>, Error>;
 	fn insert(&self, account: SafeAccount) -> Result<SafeAccount, Error>;
 	fn remove(&self, address: &Address) -> Result<(), Error>;
+	fn path(&self) -> Option<&PathBuf> { None }
 }
 
 pub use self::disk::DiskDirectory;

ethstore/src/ethstore.rs

@@ -27,6 +27,7 @@ use {Error, SecretStore};
 use json;
 use json::UUID;
 use presale::PresaleWallet;
+use import;
 
 pub struct EthStore {
 	dir: Box<KeyDirectory>,
@@ -173,4 +174,16 @@ impl SecretStore for EthStore {
 		// save to file
 		self.save(account)
 	}
+
+	fn local_path(&self) -> String {
+		self.dir.path().map(|p| p.to_string_lossy().into_owned()).unwrap_or_else(|| String::new())
+	}
+
+	fn list_geth_accounts(&self, testnet: bool) -> Vec<Address> {
+		import::read_geth_accounts(testnet)
+	}
+
+	fn import_geth_accounts(&self, desired: Vec<Address>, testnet: bool) -> Result<Vec<Address>, Error> {
+		import::import_geth_accounts(&*self.dir, desired.into_iter().collect(), testnet)
+	}
 }

ethstore/src/import.rs

@@ -16,7 +16,7 @@
 
 use std::collections::HashSet;
 use ethkey::Address;
-use dir::KeyDirectory;
+use dir::{GethDirectory, KeyDirectory, DirectoryType};
 use Error;
 
 pub fn import_accounts(src: &KeyDirectory, dst: &KeyDirectory) -> Result<Vec<Address>, Error> {
@@ -31,3 +31,39 @@ pub fn import_accounts(src: &KeyDirectory, dst: &KeyDirectory) -> Result<Vec<Add
 			Ok(address)
 		}).collect()
 }
+
+/// Provide a `HashSet` of all accounts available for import from the Geth keystore.
+pub fn read_geth_accounts(testnet: bool) -> Vec<Address> {
+	let t = if testnet {
+		DirectoryType::Testnet
+	} else {
+		DirectoryType::Main
+	};
+
+	GethDirectory::open(t)
+		.load()
+		.map(|d| d.into_iter().map(|a| a.address).collect())
+		.unwrap_or_else(|_| Vec::new())
+}
+
+/// Import specific `desired` accounts from the Geth keystore into `dst`. 
+pub fn import_geth_accounts(dst: &KeyDirectory, desired: HashSet<Address>, testnet: bool) -> Result<Vec<Address>, Error> {
+	let t = if testnet {
+		DirectoryType::Testnet
+	} else {
+		DirectoryType::Main
+	};
+
+	let src = GethDirectory::open(t);
+	let accounts = try!(src.load());
+	let existing_accounts = try!(dst.load()).into_iter().map(|a| a.address).collect::<HashSet<_>>();
+
+	accounts.into_iter()
+		.filter(|a| !existing_accounts.contains(&a.address))
+		.filter(|a| desired.contains(&a.address))
+		.map(|a| {
+			let address = a.address.clone();
+			try!(dst.insert(a));
+			Ok(address)
+		}).collect()
+}

ethstore/src/lib.rs

@@ -48,8 +48,7 @@ mod secret_store;
 pub use self::account::SafeAccount;
 pub use self::error::Error;
 pub use self::ethstore::EthStore;
-pub use self::import::import_accounts;
+pub use self::import::{import_accounts, read_geth_accounts};
 pub use self::presale::PresaleWallet;
 pub use self::secret_store::SecretStore;
 pub use self::random::random_phrase;
-

ethstore/src/secret_store.rs

@@ -42,5 +42,11 @@ pub trait SecretStore: Send + Sync {
 	fn set_name(&self, address: &Address, name: String) -> Result<(), Error>;
 
 	fn set_meta(&self, address: &Address, meta: String) -> Result<(), Error>;
+
+	fn local_path(&self) -> String;
+
+	fn list_geth_accounts(&self, testnet: bool) -> Vec<Address>;
+
+	fn import_geth_accounts(&self, desired: Vec<Address>, testnet: bool) -> Result<Vec<Address>, Error>;
 }
 

json/src/hash.rs

@@ -17,8 +17,9 @@
 //! Lenient hash json deserialization for test json files.
 
 use std::str::FromStr;
-use serde::{Deserialize, Deserializer, Error};
+use serde::{Deserialize, Deserializer, Serialize, Serializer, Error};
 use serde::de::Visitor;
+use rustc_serialize::hex::ToHex;
 use util::hash::{H64 as Hash64, Address as Hash160, H256 as Hash256, H2048 as Hash2048};
 
 
@@ -34,6 +35,12 @@ macro_rules! impl_hash {
 			}
 		}
 
+		impl From<$inner> for $name {
+			fn from(i: $inner) -> Self {
+				$name(i)
+			}
+		}
+
 		impl Deserialize for $name {
 			fn deserialize<D>(deserializer: &mut D) -> Result<Self, D::Error>
 				where D: Deserializer {
@@ -66,6 +73,14 @@ macro_rules! impl_hash {
 				deserializer.deserialize(HashVisitor)
 			}
 		}
+
+		impl Serialize for $name {
+			fn serialize<S>(&self, serializer: &mut S) -> Result<(), S::Error> where S: Serializer {
+				let mut hex = "0x".to_owned();
+				hex.push_str(&self.0.to_hex());
+				serializer.serialize_str(&hex)
+			}
+		}
 	}
 }
 

json/src/lib.rs.in

@@ -29,3 +29,4 @@ pub mod vm;
 pub mod maybe;
 pub mod state;
 pub mod transaction;
+pub mod misc;

json/src/misc/account_meta.rs

@@ -0,0 +1,58 @@
+// Copyright 2015, 2016 Ethcore (UK) Ltd.
+// This file is part of Parity.
+
+// Parity is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Misc deserialization.
+
+use std::io::{Read, Write};
+use std::collections::HashMap;
+use serde_json;
+use util;
+use hash;
+
+/// Collected account metadata
+#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
+pub struct AccountMeta {
+	/// The name of the account.
+	pub name: String,
+	/// The rest of the metadata of the account.
+	pub meta: String,
+	/// The 128-bit UUID of the account, if it has one (brain-wallets don't).
+	pub uuid: Option<String>,
+}
+
+impl Default for AccountMeta {
+	fn default() -> Self {
+		AccountMeta {
+			name: String::new(),
+			meta: "{}".to_owned(),
+			uuid: None,
+		}
+	}
+}
+
+impl AccountMeta {
+	/// Read a hash map of Address -> AccountMeta.
+	pub fn read_address_map<R>(reader: R) -> Result<HashMap<util::Address, AccountMeta>, serde_json::Error> where R: Read {
+		serde_json::from_reader(reader).map(|ok: HashMap<hash::Address, AccountMeta>|
+			ok.into_iter().map(|(a, m)| (a.into(), m)).collect()
+		)
+	}
+
+	/// Write a hash map of Address -> AccountMeta.  
+	pub fn write_address_map<W>(m: &HashMap<util::Address, AccountMeta>, writer: &mut W) -> Result<(), serde_json::Error> where W: Write {
+		serde_json::to_writer(writer, &m.iter().map(|(a, m)| (a.clone().into(), m)).collect::<HashMap<hash::Address, _>>())
+	}
+}

json/src/misc/mod.rs

@@ -0,0 +1,21 @@
+// Copyright 2015, 2016 Ethcore (UK) Ltd.
+// This file is part of Parity.
+
+// Parity is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Misc deserialization.
+
+mod account_meta;
+
+pub use self::account_meta::AccountMeta;

parity/cli.rs

@@ -19,7 +19,7 @@ use docopt::Docopt;
 
 pub const USAGE: &'static str = r#"
 Parity. Ethereum Client.
-  By Wood/Paronyan/Kotewicz/Drwięga/Volf.
+  By Wood/Paronyan/Kotewicz/Drwięga/Volf et al.
   Copyright 2015, 2016 Ethcore (UK) Limited
 
 Usage:
@@ -67,7 +67,6 @@ 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.
   --force-signer           Enable Trusted Signer WebSocket endpoint used by
                            Signer UIs, even when --unlock is in use.
   --no-signer              Disable Trusted Signer WebSocket endpoint used by
@@ -248,6 +247,7 @@ Legacy Options:
   --testnet                Geth-compatible testnet mode. Equivalent to --chain
                            testnet --keys-path $HOME/parity/testnet-keys.
                            Overrides the --keys-path option.
+  --import-geth-keys       Attempt to import keys from Geth client.
   --datadir PATH           Equivalent to --db-path PATH.
   --networkid INDEX        Equivalent to --network-id INDEX.
   --peers NUM              Equivalent to --min-peers NUM.
@@ -310,7 +310,7 @@ pub struct Args {
 	pub flag_password: Vec<String>,
 	pub flag_keys_path: String,
 	pub flag_keys_iterations: u32,
-	pub flag_no_import_keys: bool,
+	pub flag_import_geth_keys: bool,
 	pub flag_bootnodes: Option<String>,
 	pub flag_network_id: Option<String>,
 	pub flag_pruning: String,

parity/configuration.rs

@@ -310,7 +310,7 @@ impl Configuration {
 	fn accounts_config(&self) -> Result<AccountsConfig, String> {
 		let cfg = AccountsConfig {
 			iterations: self.args.flag_keys_iterations,
-			import_keys: !self.args.flag_no_import_keys,
+			import_keys: self.args.flag_import_geth_keys,
 			testnet: self.args.flag_testnet,
 			password_files: self.args.flag_password.clone(),
 			unlocked_accounts: try!(to_addresses(&self.args.flag_unlock)),

parity/params.rs

@@ -167,7 +167,7 @@ impl Default for AccountsConfig {
 	fn default() -> Self {
 		AccountsConfig {
 			iterations: 10240,
-			import_keys: true,
+			import_keys: false,
 			testnet: false,
 			password_files: Vec::new(),
 			unlocked_accounts: Vec::new(),

rpc/src/v1/impls/personal.rs

@@ -148,18 +148,20 @@ impl<C: 'static, M: 'static> Personal for PersonalClient<C, M> where C: MiningBl
 	fn set_account_name(&self, params: Params) -> Result<Value, Error> {
 		try!(self.active());
 		let store = take_weak!(self.accounts);
-		from_params::<(RpcH160, _)>(params).and_then(|(addr, name)| {
+		from_params::<(RpcH160, String)>(params).and_then(|(addr, name)| {
 			let addr: Address = addr.into();
-			store.set_account_name(addr, name).map_err(|e| errors::account("Could not set account name.", e)).map(|_| Value::Null)
+			store.set_account_name(addr.clone(), name.clone()).or_else(|_| store.set_address_name(addr, name)).expect("set_address_name always returns Ok; qed");
+			Ok(Value::Null)
 		})
 	}
 
 	fn set_account_meta(&self, params: Params) -> Result<Value, Error> {
 		try!(self.active());
 		let store = take_weak!(self.accounts);
-		from_params::<(RpcH160, _)>(params).and_then(|(addr, meta)| {
+		from_params::<(RpcH160, String)>(params).and_then(|(addr, meta)| {
 			let addr: Address = addr.into();
-			store.set_account_meta(addr, meta).map_err(|e| errors::account("Could not set account meta.", e)).map(|_| Value::Null)
+			store.set_account_meta(addr.clone(), meta.clone()).or_else(|_| store.set_address_meta(addr, meta)).expect("set_address_meta always returns Ok; qed");
+			Ok(Value::Null)
 		})
 	}
 
@@ -168,7 +170,8 @@ impl<C: 'static, M: 'static> Personal for PersonalClient<C, M> where C: MiningBl
 		try!(expect_no_params(params));
 		let store = take_weak!(self.accounts);
 		let info = try!(store.accounts_info().map_err(|e| errors::account("Could not fetch account info.", e)));
-		Ok(Value::Object(info.into_iter().map(|(a, v)| {
+		let other = store.addresses_info().expect("addresses_info always returns Ok; qed");
+		Ok(Value::Object(info.into_iter().chain(other.into_iter()).map(|(a, v)| {
 			let m = map![
 				"name".to_owned() => to_value(&v.name).unwrap(),
 				"meta".to_owned() => to_value(&v.meta).unwrap(),
@@ -181,4 +184,21 @@ impl<C: 'static, M: 'static> Personal for PersonalClient<C, M> where C: MiningBl
 			(format!("0x{}", a.hex()), Value::Object(m))
 		}).collect::<BTreeMap<_, _>>()))
 	}
+
+	fn geth_accounts(&self, params: Params) -> Result<Value, Error> {
+		try!(self.active());
+		try!(expect_no_params(params));
+		let store = take_weak!(self.accounts);
+		to_value(&store.list_geth_accounts(false).into_iter().map(Into::into).collect::<Vec<RpcH160>>())
+	}
+
+	fn import_geth_accounts(&self, params: Params) -> Result<Value, Error> {
+		from_params::<(Vec<RpcH160>,)>(params).and_then(|(addresses,)| {
+			let store = take_weak!(self.accounts);
+			to_value(&try!(store
+				.import_geth_accounts(addresses.into_iter().map(Into::into).collect(), false)
+				.map_err(|e| errors::account("Couldn't import Geth accounts", e))
+			).into_iter().map(Into::into).collect::<Vec<RpcH160>>())
+		})
+	}
 }

rpc/src/v1/traits/personal.rs

@@ -54,6 +54,12 @@ pub trait Personal: Sized + Send + Sync + 'static {
 	/// Returns accounts information.
 	fn accounts_info(&self, _: Params) -> Result<Value, Error>;
 
+	/// Returns the accounts available for importing from Geth.
+	fn geth_accounts(&self, _: Params) -> Result<Value, Error>;
+
+	/// Imports a number of Geth accounts, with the list provided as the argument.
+	fn import_geth_accounts(&self, _: Params) -> Result<Value, Error>;
+
 	/// Should be used to convert object to io delegate.
 	fn to_delegate(self) -> IoDelegate<Self> {
 		let mut delegate = IoDelegate::new(Arc::new(self));
@@ -67,6 +73,8 @@ pub trait Personal: Sized + Send + Sync + 'static {
 		delegate.add_method("personal_setAccountName", Personal::set_account_name);
 		delegate.add_method("personal_setAccountMeta", Personal::set_account_meta);
 		delegate.add_method("personal_accountsInfo", Personal::accounts_info);
+		delegate.add_method("personal_listGethAccounts", Personal::geth_accounts);
+		delegate.add_method("personal_importGethAccounts", Personal::import_geth_accounts);
 
 		delegate
 	}