diff --git a/ethcore/src/account_provider.rs b/ethcore/src/account_provider.rs index 6e3b9c94d..2c735ce59 100644 --- a/ethcore/src/account_provider.rs +++ b/ethcore/src/account_provider.rs @@ -107,18 +107,23 @@ impl_bridge_type!(Message, 32, H256, SSMessage); impl_bridge_type!(Address, 20, H160, SSAddress); -struct NullDir; +#[derive(Default)] +struct NullDir { + accounts: RwLock>, +} impl KeyDirectory for NullDir { fn load(&self) -> Result, SSError> { - Ok(vec![]) + Ok(self.accounts.read().values().cloned().collect()) } fn insert(&self, account: SafeAccount) -> Result { + self.accounts.write().insert(account.address.clone(), account.clone()); Ok(account) } - fn remove(&self, _address: &SSAddress) -> Result<(), SSError> { + fn remove(&self, address: &SSAddress) -> Result<(), SSError> { + self.accounts.write().remove(address); Ok(()) } } @@ -164,7 +169,7 @@ impl AccountProvider { pub fn transient_provider() -> Self { AccountProvider { unlocked: RwLock::new(HashMap::new()), - sstore: Box::new(EthStore::open(Box::new(NullDir)).unwrap()) + sstore: Box::new(EthStore::open(Box::new(NullDir::default())).unwrap()) } } @@ -184,13 +189,14 @@ impl AccountProvider { } /// Returns addresses of all accounts. - pub fn accounts(&self) -> Vec { - self.sstore.accounts().into_iter().map(|a| H160(a.into())).collect() + pub fn accounts(&self) -> Result, Error> { + let accounts = try!(self.sstore.accounts()).into_iter().map(|a| H160(a.into())).collect(); + Ok(accounts) } /// Returns each account along with name and meta. pub fn accounts_info(&self) -> Result, Error> { - let r: HashMap = self.sstore.accounts() + let r: HashMap = try!(self.sstore.accounts()) .into_iter() .map(|a| (H160(a.clone().into()), self.account_meta(a).unwrap_or_else(|_| Default::default()))) .collect(); diff --git a/ethstore/src/ethstore.rs b/ethstore/src/ethstore.rs index a63a9d274..689248bae 100644 --- a/ethstore/src/ethstore.rs +++ b/ethstore/src/ethstore.rs @@ -16,6 +16,7 @@ use std::collections::BTreeMap; use std::sync::RwLock; +use std::mem; use ethkey::KeyPair; use crypto::KEY_ITERATIONS; use random::Random; @@ -56,6 +57,26 @@ impl EthStore { cache.insert(account.address.clone(), account); Ok(()) } + + fn reload_accounts(&self) -> Result<(), Error> { + let mut cache = self.cache.write().unwrap(); + let accounts = try!(self.dir.load()); + let new_accounts: BTreeMap<_, _> = accounts.into_iter().map(|account| (account.address.clone(), account)).collect(); + mem::replace(&mut *cache, new_accounts); + Ok(()) + } + + fn get(&self, address: &Address) -> Result { + { + let cache = self.cache.read().unwrap(); + if let Some(account) = cache.get(address) { + return Ok(account.clone()) + } + } + try!(self.reload_accounts()); + let cache = self.cache.read().unwrap(); + cache.get(address).cloned().ok_or(Error::InvalidAccount) + } } impl SecretStore for EthStore { @@ -68,17 +89,15 @@ impl SecretStore for EthStore { Ok(address) } - fn accounts(&self) -> Vec
{ - self.cache.read().unwrap().keys().cloned().collect() + fn accounts(&self) -> Result, Error> { + try!(self.reload_accounts()); + Ok(self.cache.read().unwrap().keys().cloned().collect()) } fn change_password(&self, address: &Address, old_password: &str, new_password: &str) -> Result<(), Error> { // change password - let account = { - let cache = self.cache.read().unwrap(); - let account = try!(cache.get(address).ok_or(Error::InvalidAccount)); - try!(account.change_password(old_password, new_password, self.iterations)) - }; + let account = try!(self.get(address)); + let account = try!(account.change_password(old_password, new_password, self.iterations)); // save to file self.save(account) @@ -86,8 +105,7 @@ impl SecretStore for EthStore { fn remove_account(&self, address: &Address, password: &str) -> Result<(), Error> { let can_remove = { - let cache = self.cache.read().unwrap(); - let account = try!(cache.get(address).ok_or(Error::InvalidAccount)); + let account = try!(self.get(address)); account.check_password(password) }; @@ -101,50 +119,38 @@ impl SecretStore for EthStore { } } - fn sign(&self, account: &Address, password: &str, message: &Message) -> Result { - let cache = self.cache.read().unwrap(); - let account = try!(cache.get(account).ok_or(Error::InvalidAccount)); + fn sign(&self, address: &Address, password: &str, message: &Message) -> Result { + let account = try!(self.get(address)); account.sign(password, message) } - fn uuid(&self, addr: &Address) -> Result { - let cache = self.cache.read().unwrap(); - let account = try!(cache.get(addr).ok_or(Error::InvalidAccount)); + fn uuid(&self, address: &Address) -> Result { + let account = try!(self.get(address)); Ok(account.id.into()) } - fn name(&self, addr: &Address) -> Result { - let cache = self.cache.read().unwrap(); - let account = try!(cache.get(addr).ok_or(Error::InvalidAccount)); + fn name(&self, address: &Address) -> Result { + let account = try!(self.get(address)); Ok(account.name.clone()) } - fn meta(&self, addr: &Address) -> Result { - let cache = self.cache.read().unwrap(); - let account = try!(cache.get(addr).ok_or(Error::InvalidAccount)); + fn meta(&self, address: &Address) -> Result { + let account = try!(self.get(address)); Ok(account.meta.clone()) } - fn set_name(&self, addr: &Address, name: String) -> Result<(), Error> { - let account = { - let cache = self.cache.read().unwrap(); - let mut account = try!(cache.get(addr).ok_or(Error::InvalidAccount)).clone(); - account.name = name; - account - }; - + fn set_name(&self, address: &Address, name: String) -> Result<(), Error> { + let mut account = try!(self.get(address)); + account.name = name; + // save to file self.save(account) } - fn set_meta(&self, addr: &Address, meta: String) -> Result<(), Error> { - let account = { - let cache = self.cache.read().unwrap(); - let mut account = try!(cache.get(addr).ok_or(Error::InvalidAccount)).clone(); - account.meta = meta; - account - }; - + fn set_meta(&self, address: &Address, meta: String) -> Result<(), Error> { + let mut account = try!(self.get(address)); + account.meta = meta; + // save to file self.save(account) } diff --git a/ethstore/src/secret_store.rs b/ethstore/src/secret_store.rs index b3f2a6b45..09de71718 100644 --- a/ethstore/src/secret_store.rs +++ b/ethstore/src/secret_store.rs @@ -21,7 +21,7 @@ use json::UUID; pub trait SecretStore: Send + Sync { fn insert_account(&self, secret: Secret, password: &str) -> Result; - fn accounts(&self) -> Vec
; + fn accounts(&self) -> Result, Error>; fn change_password(&self, account: &Address, old_password: &str, new_password: &str) -> Result<(), Error>; diff --git a/ethstore/tests/api.rs b/ethstore/tests/api.rs index 3b6a9f784..83aa04874 100644 --- a/ethstore/tests/api.rs +++ b/ethstore/tests/api.rs @@ -46,11 +46,11 @@ fn random_secret() -> Secret { fn secret_store_create_account() { let dir = TransientDir::create().unwrap(); let store = EthStore::open(Box::new(dir)).unwrap(); - assert_eq!(store.accounts().len(), 0); + assert_eq!(store.accounts().unwrap().len(), 0); assert!(store.insert_account(random_secret(), "").is_ok()); - assert_eq!(store.accounts().len(), 1); + assert_eq!(store.accounts().unwrap().len(), 1); assert!(store.insert_account(random_secret(), "").is_ok()); - assert_eq!(store.accounts().len(), 2); + assert_eq!(store.accounts().unwrap().len(), 2); } #[test] @@ -58,7 +58,7 @@ fn secret_store_sign() { let dir = TransientDir::create().unwrap(); let store = EthStore::open(Box::new(dir)).unwrap(); assert!(store.insert_account(random_secret(), "").is_ok()); - let accounts = store.accounts(); + let accounts = store.accounts().unwrap(); assert_eq!(accounts.len(), 1); assert!(store.sign(&accounts[0], "", &Default::default()).is_ok()); assert!(store.sign(&accounts[0], "1", &Default::default()).is_err()); @@ -69,7 +69,7 @@ fn secret_store_change_password() { let dir = TransientDir::create().unwrap(); let store = EthStore::open(Box::new(dir)).unwrap(); assert!(store.insert_account(random_secret(), "").is_ok()); - let accounts = store.accounts(); + let accounts = store.accounts().unwrap(); assert_eq!(accounts.len(), 1); assert!(store.sign(&accounts[0], "", &Default::default()).is_ok()); assert!(store.change_password(&accounts[0], "", "1").is_ok()); @@ -82,10 +82,10 @@ fn secret_store_remove_account() { let dir = TransientDir::create().unwrap(); let store = EthStore::open(Box::new(dir)).unwrap(); assert!(store.insert_account(random_secret(), "").is_ok()); - let accounts = store.accounts(); + let accounts = store.accounts().unwrap(); assert_eq!(accounts.len(), 1); assert!(store.remove_account(&accounts[0], "").is_ok()); - assert_eq!(store.accounts().len(), 0); + assert_eq!(store.accounts().unwrap().len(), 0); assert!(store.remove_account(&accounts[0], "").is_err()); } @@ -107,7 +107,7 @@ fn pat_path() -> &'static str { fn secret_store_laod_geth_files() { let dir = DiskDirectory::at(test_path()); let store = EthStore::open(Box::new(dir)).unwrap(); - assert_eq!(store.accounts(), vec![ + assert_eq!(store.accounts().unwrap(), vec![ Address::from_str("3f49624084b67849c7b4e805c5988c21a430f9d9").unwrap(), Address::from_str("5ba4dcf897e97c2bdf8315b9ef26c13c085988cf").unwrap(), Address::from_str("63121b431a52f8043c16fcf0d1df9cb7b5f66649").unwrap(), @@ -118,7 +118,7 @@ fn secret_store_laod_geth_files() { fn secret_store_load_pat_files() { let dir = DiskDirectory::at(pat_path()); let store = EthStore::open(Box::new(dir)).unwrap(); - assert_eq!(store.accounts(), vec![ + assert_eq!(store.accounts().unwrap(), vec![ Address::from_str("3f49624084b67849c7b4e805c5988c21a430f9d9").unwrap(), Address::from_str("5ba4dcf897e97c2bdf8315b9ef26c13c085988cf").unwrap(), ]); diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index 4929c0ee5..4ad66e082 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -340,10 +340,16 @@ impl Eth for EthClient where } } - fn accounts(&self, _: Params) -> Result { + fn accounts(&self, params: Params) -> Result { try!(self.active()); - let store = take_weak!(self.accounts); - to_value(&store.accounts().into_iter().map(Into::into).collect::>()) + match params { + Params::None => { + let store = take_weak!(self.accounts); + let accounts = try!(store.accounts().map_err(|_| Error::internal_error())); + to_value(&accounts.into_iter().map(Into::into).collect::>()) + }, + _ => Err(Error::invalid_params()) + } } fn block_number(&self, params: Params) -> Result { diff --git a/rpc/src/v1/impls/personal.rs b/rpc/src/v1/impls/personal.rs index 063b7415b..15624c7d3 100644 --- a/rpc/src/v1/impls/personal.rs +++ b/rpc/src/v1/impls/personal.rs @@ -62,10 +62,16 @@ impl Personal for PersonalClient where C: MiningBl .unwrap_or_else(|| to_value(&false)) } - fn accounts(&self, _: Params) -> Result { + fn accounts(&self, params: Params) -> Result { try!(self.active()); - let store = take_weak!(self.accounts); - to_value(&store.accounts().into_iter().map(Into::into).collect::>()) + match params { + Params::None => { + let store = take_weak!(self.accounts); + let accounts = try!(store.accounts().map_err(|_| Error::internal_error())); + to_value(&accounts.into_iter().map(Into::into).collect::>()) + }, + _ => Err(Error::invalid_params()) + } } fn new_account(&self, params: Params) -> Result { diff --git a/rpc/src/v1/tests/mocked/personal.rs b/rpc/src/v1/tests/mocked/personal.rs index 5f6b912f4..290a63c63 100644 --- a/rpc/src/v1/tests/mocked/personal.rs +++ b/rpc/src/v1/tests/mocked/personal.rs @@ -110,7 +110,7 @@ fn new_account() { let res = tester.io.handle_request(request); - let accounts = tester.accounts.accounts(); + let accounts = tester.accounts.accounts().unwrap(); assert_eq!(accounts.len(), 1); let address = accounts[0]; let response = r#"{"jsonrpc":"2.0","result":""#.to_owned() + format!("0x{:?}", address).as_ref() + r#"","id":1}"#; @@ -122,7 +122,7 @@ fn new_account() { fn should_be_able_to_get_account_info() { let tester = setup(None); tester.accounts.new_account("").unwrap(); - let accounts = tester.accounts.accounts(); + let accounts = tester.accounts.accounts().unwrap(); assert_eq!(accounts.len(), 1); let address = accounts[0]; @@ -140,7 +140,7 @@ fn should_be_able_to_get_account_info() { fn should_be_able_to_set_name() { let tester = setup(None); tester.accounts.new_account("").unwrap(); - let accounts = tester.accounts.accounts(); + let accounts = tester.accounts.accounts().unwrap(); assert_eq!(accounts.len(), 1); let address = accounts[0]; @@ -161,7 +161,7 @@ fn should_be_able_to_set_name() { fn should_be_able_to_set_meta() { let tester = setup(None); tester.accounts.new_account("").unwrap(); - let accounts = tester.accounts.accounts(); + let accounts = tester.accounts.accounts().unwrap(); assert_eq!(accounts.len(), 1); let address = accounts[0];