refactoring to AccountService

parity/main.rs

@@ -196,7 +196,7 @@ fn setup_log(init: &Option<String>) {
 }
 
 #[cfg(feature = "rpc")]
-fn setup_rpc_server(client: Arc<Client>, sync: Arc<EthSync>, secret_store: Arc<SecretStore>,  url: &str, cors_domain: &str, apis: Vec<&str>) -> Option<Arc<PanicHandler>> {
+fn setup_rpc_server(client: Arc<Client>, sync: Arc<EthSync>, secret_store: Arc<AccountService>,  url: &str, cors_domain: &str, apis: Vec<&str>) -> Option<Arc<PanicHandler>> {
 	use rpc::v1::*;
 
 	let server = rpc::RpcServer::new();
@@ -416,11 +416,7 @@ impl Configuration {
 		let sync = EthSync::register(service.network(), sync_config, client);
 
 		// Secret Store
-		let secret_store = Arc::new(SecretStore::new());
+		let account_service = Arc::new(AccountService::new());
-		{
-			let import_ref = Arc::make_mut(&mut secret_store);
-			import_ref.try_import_existing();
-		}
 
 		// Setup rpc
 		if self.args.flag_jsonrpc || self.args.flag_rpc {
@@ -432,7 +428,7 @@ impl Configuration {
 			let cors = self.args.flag_rpccorsdomain.as_ref().unwrap_or(&self.args.flag_jsonrpc_cors);
 			// TODO: use this as the API list.
 			let apis = self.args.flag_rpcapi.as_ref().unwrap_or(&self.args.flag_jsonrpc_apis);
-			let server_handler = setup_rpc_server(service.client(), sync.clone(), secret_store.clone(), &url, cors, apis.split(",").collect());
+			let server_handler = setup_rpc_server(service.client(), sync.clone(), account_service.clone(), &url, cors, apis.split(",").collect());
 			if let Some(handler) = server_handler {
 				panic_handler.forward_from(handler.deref());
 			}

rpc/src/v1/impls/personal.rs

@@ -20,30 +20,28 @@ use jsonrpc_core::*;
 use v1::traits::Personal;
 use util::keys::store::*;
 use util::Address;
-use std::sync::RwLock;
 
 /// Account management (personal) rpc implementation.
 pub struct PersonalClient {
-	secret_store: Weak<RwLock<SecretStore>>,
+	accounts: Weak<AccountProvider>,
 }
 
 impl PersonalClient {
 	/// Creates new PersonalClient
-	pub fn new(store: &Arc<RwLock<SecretStore>>) -> Self {
+	pub fn new(store: &Arc<AccountProvider>) -> Self {
 		PersonalClient {
-			secret_store: Arc::downgrade(store),
+			accounts: Arc::downgrade(store),
 		}
 	}
 }
 
 impl Personal for PersonalClient {
 	fn accounts(&self, _: Params) -> Result<Value, Error> {
-		let store_wk = take_weak!(self.secret_store);
+		let store = take_weak!(self.accounts);
-		let store = store_wk.read().unwrap();
 		match store.accounts() {
 			Ok(account_list) => {
 				Ok(Value::Array(account_list.iter()
-					.map(|&(account, _)| Value::String(format!("{:?}", account)))
+					.map(|&account| Value::String(format!("{:?}", account)))
 					.collect::<Vec<Value>>())
 				)
 			}
@@ -54,8 +52,7 @@ impl Personal for PersonalClient {
 	fn new_account(&self, params: Params) -> Result<Value, Error> {
 		from_params::<(String, )>(params).and_then(
 			|(pass, )| {
-				let store_wk = take_weak!(self.secret_store);
+				let store = take_weak!(self.accounts);
-				let mut store = store_wk.write().unwrap();
 				match store.new_account(&pass) {
 					Ok(address) => Ok(Value::String(format!("{:?}", address))),
 					Err(_) => Err(Error::internal_error())
@@ -67,8 +64,7 @@ impl Personal for PersonalClient {
 	fn unlock_account(&self, params: Params) -> Result<Value, Error> {
 		from_params::<(Address, String, u64)>(params).and_then(
 			|(account, account_pass, _)|{
-				let store_wk = take_weak!(self.secret_store);
+				let store = take_weak!(self.accounts);
-				let store = store_wk.read().unwrap();
 				match store.unlock_account(&account, &account_pass) {
 					Ok(_) => Ok(Value::Bool(true)),
 					Err(_) => Ok(Value::Bool(false)),

util/src/keys/store.rs

@@ -80,16 +80,57 @@ struct AccountUnlock {
 
 /// Basic account management trait
 pub trait AccountProvider : Send + Sync {
+	/// Lists all accounts
+	fn accounts(&self) -> Result<Vec<Address>, ::std::io::Error>;
 	/// Unlocks account with the password provided
 	fn unlock_account(&self, account: &Address, pass: &str) -> Result<(), EncryptedHashMapError>;
 	/// Creates account
-	fn new_account(&mut self, pass: &str) -> Result<Address, ::std::io::Error>;
+	fn new_account(&self, pass: &str) -> Result<Address, ::std::io::Error>;
 	/// Returns secret for unlocked account
 	fn account_secret(&self, account: &Address) -> Result<crypto::Secret, SigningError>;
 	/// Returns secret for unlocked account
 	fn sign(&self, account: &Address, message: &H256) -> Result<crypto::Signature, SigningError>;
 }
 
+/// Thread-safe accounts management
+pub struct AccountService {
+	secret_store: RwLock<SecretStore>,
+}
+
+impl AccountProvider for AccountService {
+	/// Lists all accounts
+	fn accounts(&self) -> Result<Vec<Address>, ::std::io::Error> {
+		Ok(try!(self.secret_store.read().unwrap().accounts()).iter().map(|&(addr, _)| addr).collect::<Vec<Address>>())
+	}
+	/// Unlocks account with the password provided
+	fn unlock_account(&self, account: &Address, pass: &str) -> Result<(), EncryptedHashMapError> {
+		self.secret_store.read().unwrap().unlock_account(account, pass)
+	}
+	/// Creates account
+	fn new_account(&self, pass: &str) -> Result<Address, ::std::io::Error> {
+		self.secret_store.write().unwrap().new_account(pass)
+	}
+	/// Returns secret for unlocked account
+	fn account_secret(&self, account: &Address) -> Result<crypto::Secret, SigningError> {
+		self.secret_store.read().unwrap().account_secret(account)
+	}
+	/// Returns secret for unlocked account
+	fn sign(&self, account: &Address, message: &H256) -> Result<crypto::Signature, SigningError> {
+		self.secret_store.read().unwrap().sign(account, message)
+	}
+}
+
+impl AccountService {
+	/// New account service with the default location
+	pub fn new() -> AccountService {
+		let secret_store = RwLock::new(SecretStore::new());
+		secret_store.write().unwrap().try_import_existing();
+		AccountService {
+			secret_store: secret_store
+		}
+	}
+}
+
 impl SecretStore {
 	/// new instance of Secret Store in default home directory
 	pub fn new() -> SecretStore {
@@ -156,11 +197,9 @@ impl SecretStore {
 			unlocks: RwLock::new(HashMap::new()),
 		}
 	}
-}
 
-impl AccountProvider for SecretStore {
 	/// Unlocks account for use
-	fn unlock_account(&self, account: &Address, pass: &str) -> Result<(), EncryptedHashMapError> {
+	pub fn unlock_account(&self, account: &Address, pass: &str) -> Result<(), EncryptedHashMapError> {
 		let secret_id = try!(self.account(&account).ok_or(EncryptedHashMapError::UnknownIdentifier));
 		let secret = try!(self.get(&secret_id, pass));
 		{
@@ -174,7 +213,7 @@ impl AccountProvider for SecretStore {
 	}
 
 	/// Creates new account
-	fn new_account(&mut self, pass: &str) -> Result<Address, ::std::io::Error> {
+	pub fn new_account(&mut self, pass: &str) -> Result<Address, ::std::io::Error> {
 		let secret = H256::random();
 		let key_id = H128::random();
 		self.insert(key_id.clone(), secret, pass);
@@ -187,7 +226,7 @@ impl AccountProvider for SecretStore {
 	}
 
 	/// Signs message with unlocked account
-	fn sign(&self, account: &Address, message: &H256) -> Result<crypto::Signature, SigningError> {
+	pub fn sign(&self, account: &Address, message: &H256) -> Result<crypto::Signature, SigningError> {
 		let read_lock = self.unlocks.read().unwrap();
 		let unlock = try!(read_lock.get(account).ok_or(SigningError::AccountNotUnlocked));
 		match crypto::KeyPair::from_secret(unlock.secret) {
@@ -200,7 +239,7 @@ impl AccountProvider for SecretStore {
 	}
 
 	/// Returns secret for unlocked account
-	fn account_secret(&self, account: &Address) -> Result<crypto::Secret, SigningError> {
+	pub fn account_secret(&self, account: &Address) -> Result<crypto::Secret, SigningError> {
 		let read_lock = self.unlocks.read().unwrap();
 		let unlock = try!(read_lock.get(account).ok_or(SigningError::AccountNotUnlocked));
 		Ok(unlock.secret as crypto::Secret)