From ad440a12bdad91766657d039d3423a703bed32ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 30 Nov 2016 13:47:14 +0100 Subject: [PATCH 01/11] EthMultiStore --- ethcore/src/account_provider.rs | 27 ++++- ethstore/src/dir/disk.rs | 5 +- ethstore/src/dir/geth.rs | 5 +- ethstore/src/dir/mod.rs | 3 +- ethstore/src/dir/parity.rs | 5 +- ethstore/src/ethstore.rs | 143 +++++++++++++++++++++++++-- ethstore/tests/util/transient_dir.rs | 5 +- 7 files changed, 169 insertions(+), 24 deletions(-) diff --git a/ethcore/src/account_provider.rs b/ethcore/src/account_provider.rs index 917ae8b8b..e2ccd1d83 100644 --- a/ethcore/src/account_provider.rs +++ b/ethcore/src/account_provider.rs @@ -163,12 +163,19 @@ impl AddressBook { } } +fn transient_sstore() -> Box { + Box::new(EthStore::open(Box::new(NullDir::default())).expect("NullDir load always succeeds; qed"))) +} + /// Account management. /// Responsible for unlocking accounts. pub struct AccountProvider { - unlocked: Mutex>, - sstore: Box, address_book: Mutex, + unlocked: Mutex>, + /// Accounts on disk + sstore: Box, + /// Accounts unlocked with rolling tokens + transient_sstore: Box, } impl AccountProvider { @@ -178,6 +185,7 @@ impl AccountProvider { unlocked: Mutex::new(HashMap::new()), address_book: Mutex::new(AddressBook::new(sstore.local_path().into())), sstore: sstore, + transient_sstore: transient_sstore(), } } @@ -186,8 +194,8 @@ impl AccountProvider { AccountProvider { unlocked: Mutex::new(HashMap::new()), address_book: Mutex::new(AddressBook::transient()), - sstore: Box::new(EthStore::open(Box::new(NullDir::default())) - .expect("NullDir load always succeeds; qed")) + sstore: transient_sstore(), + transient_sstore: transient_sstore(), } } @@ -433,4 +441,15 @@ mod tests { ap.unlocked.lock().get_mut(&kp.address()).unwrap().unlock = Unlock::Timed(Instant::now()); assert!(ap.sign(kp.address(), None, Default::default()).is_err()); } + + #[test] + fn should_sign_and_return_token() { + let kp = Random.generate().unwrap(); + // given + let ap = AccountProvider::transient_provider(); + assert!(ap.insert_account(kp.secret().clone(), "test").is_ok()); + + // when + let (_signature, token) = ap.sign_with_token(kp.address(), "test", Default::default()).unwrap(); + } } diff --git a/ethstore/src/dir/disk.rs b/ethstore/src/dir/disk.rs index 56b2c1ccb..22093171e 100644 --- a/ethstore/src/dir/disk.rs +++ b/ethstore/src/dir/disk.rs @@ -18,7 +18,6 @@ use std::{fs, io}; use std::path::{PathBuf, Path}; use std::collections::HashMap; use time; -use ethkey::Address; use {json, SafeAccount, Error}; use json::UUID; use super::KeyDirectory; @@ -138,12 +137,12 @@ impl KeyDirectory for DiskDirectory { Ok(account) } - fn remove(&self, address: &Address) -> Result<(), Error> { + fn remove(&self, account: &SafeAccount) -> Result<(), Error> { // enumerate all entries in keystore // and find entry with given address let to_remove = try!(self.files()) .into_iter() - .find(|&(_, ref account)| &account.address == address); + .find(|&(_, ref acc)| acc == account); // remove it match to_remove { diff --git a/ethstore/src/dir/geth.rs b/ethstore/src/dir/geth.rs index f63ebbea2..a5367f98d 100644 --- a/ethstore/src/dir/geth.rs +++ b/ethstore/src/dir/geth.rs @@ -16,7 +16,6 @@ use std::env; use std::path::PathBuf; -use ethkey::Address; use {SafeAccount, Error}; use super::{KeyDirectory, DiskDirectory, DirectoryType}; @@ -89,7 +88,7 @@ impl KeyDirectory for GethDirectory { self.dir.insert(account) } - fn remove(&self, address: &Address) -> Result<(), Error> { - self.dir.remove(address) + fn remove(&self, account: &SafeAccount) -> Result<(), Error> { + self.dir.remove(account) } } diff --git a/ethstore/src/dir/mod.rs b/ethstore/src/dir/mod.rs index e29bd1ec4..0da4d71fb 100644 --- a/ethstore/src/dir/mod.rs +++ b/ethstore/src/dir/mod.rs @@ -14,7 +14,6 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . -use ethkey::Address; use std::path::{PathBuf}; use {SafeAccount, Error}; @@ -30,7 +29,7 @@ pub enum DirectoryType { pub trait KeyDirectory: Send + Sync { fn load(&self) -> Result, Error>; fn insert(&self, account: SafeAccount) -> Result; - fn remove(&self, address: &Address) -> Result<(), Error>; + fn remove(&self, account: &SafeAccount) -> Result<(), Error>; fn path(&self) -> Option<&PathBuf> { None } } diff --git a/ethstore/src/dir/parity.rs b/ethstore/src/dir/parity.rs index 7aa50c80b..c5d0057d8 100644 --- a/ethstore/src/dir/parity.rs +++ b/ethstore/src/dir/parity.rs @@ -16,7 +16,6 @@ use std::env; use std::path::PathBuf; -use ethkey::Address; use {SafeAccount, Error}; use super::{KeyDirectory, DiskDirectory, DirectoryType}; @@ -68,7 +67,7 @@ impl KeyDirectory for ParityDirectory { self.dir.insert(account) } - fn remove(&self, address: &Address) -> Result<(), Error> { - self.dir.remove(address) + fn remove(&self, account: &SafeAccount) -> Result<(), Error> { + self.dir.remove(account) } } diff --git a/ethstore/src/ethstore.rs b/ethstore/src/ethstore.rs index 4991c4714..f83e5fd3a 100644 --- a/ethstore/src/ethstore.rs +++ b/ethstore/src/ethstore.rs @@ -53,7 +53,7 @@ impl EthStore { fn save(&self, account: SafeAccount) -> Result<(), Error> { // save to file - let account = try!(self.dir.insert(account.clone())); + let account = try!(self.dir.insert(account)); // update cache let mut cache = self.cache.write(); @@ -124,13 +124,11 @@ impl SecretStore for EthStore { } fn remove_account(&self, address: &Address, password: &str) -> Result<(), Error> { - let can_remove = { - let account = try!(self.get(address)); - account.check_password(password) - }; + let account = try!(self.get(address)); + let can_remove = account.check_password(password); if can_remove { - try!(self.dir.remove(address)); + try!(self.dir.remove(&account)); let mut cache = self.cache.write(); cache.remove(address); Ok(()) @@ -197,3 +195,136 @@ impl SecretStore for EthStore { import::import_geth_accounts(&*self.dir, desired.into_iter().collect(), testnet) } } + +/// Similar to `EthStore` but may store many accounts (with different passwords) for the same `Address` +pub struct EthMultiStore { + dir: Box, + iterations: u32, + cache: RwLock>>, +} + +impl EthMultiStore { + + pub fn open(directory: Box) -> Result { + Self::open_with_iterations(directory, KEY_ITERATIONS as u32) + } + + pub fn open_with_iterations(directory: Box, iterations: u32) -> Result { + let mut store = EthMultiStore { + dir: directory, + iterations: iterations, + cache: Default::default(), + }; + try!(store.reload_accounts()); + Ok(store) + } + + fn reload_accounts(&self) -> Result<(), Error> { + let mut cache = self.cache.write(); + let accounts = try!(self.dir.load()); + + let mut new_accounts = BTreeMap::new(); + for account in accounts { + let mut entry = new_accounts.entry(account.address.clone()).or_insert_with(Vec::new); + entry.push(account); + } + mem::replace(&mut *cache, new_accounts); + Ok(()) + } + + fn get(&self, address: &Address) -> Result, Error> { + { + let cache = self.cache.read(); + if let Some(accounts) = cache.get(address) { + if !accounts.is_empty() { + return Ok(accounts.clone()) + } + } + } + + try!(self.reload_accounts()); + let cache = self.cache.read(); + let accounts = try!(cache.get(address).cloned().ok_or(Error::InvalidAccount)); + if accounts.is_empty() { + Err(Error::InvalidAccount) + } else { + Ok(accounts) + } + } + + pub fn insert_account(&self, account: SafeAccount) -> Result<(), Error> { + //save to file + let account = try!(self.dir.insert(account)); + + // update cache + let mut cache = self.cache.write(); + let mut accounts = cache.entry(account.address.clone()).or_insert_with(Vec::new); + accounts.push(account); + Ok(()) + } + + pub fn remove_account(&self, address: &Address, password: &str) -> Result<(), Error> { + let accounts = try!(self.get(address)); + + for account in accounts { + // Skip if password is invalid + if !account.check_password(password) { + continue; + } + + // Remove from dir + try!(self.dir.remove(&account)); + + // Remove from cache + let mut cache = self.cache.write(); + let is_empty = { + let mut accounts = cache.get_mut(address).expect("Entry exists, because it was returned by `get`; qed"); + if let Some(position) = accounts.iter().position(|acc| acc == &account) { + accounts.remove(position); + } + accounts.is_empty() + }; + + if is_empty { + cache.remove(address); + } + + return Ok(()); + } + Err(Error::InvalidPassword) + } + + pub fn change_password(&self, address: &Address, old_password: &str, new_password: &str) -> Result<(), Error> { + let accounts = try!(self.get(address)); + for account in accounts { + // First remove + try!(self.remove_account(&address, old_password)); + // Then insert back with new password + let new_account = try!(account.change_password(old_password, new_password, self.iterations)); + try!(self.insert_account(new_account)); + } + Ok(()) + } + + pub fn sign(&self, address: &Address, password: &str, message: &Message) -> Result { + let accounts = try!(self.get(address)); + for account in accounts { + if account.check_password(password) { + return account.sign(password, message); + } + } + + Err(Error::InvalidPassword) + } + + pub fn decrypt(&self, account: &Address, password: &str, shared_mac: &[u8], message: &[u8]) -> Result, Error> { + let accounts = try!(self.get(account)); + for account in accounts { + if account.check_password(password) { + return account.decrypt(password, shared_mac, message); + } + } + Err(Error::InvalidPassword) + } +} + diff --git a/ethstore/tests/util/transient_dir.rs b/ethstore/tests/util/transient_dir.rs index 23523e48c..76010182e 100644 --- a/ethstore/tests/util/transient_dir.rs +++ b/ethstore/tests/util/transient_dir.rs @@ -18,7 +18,6 @@ use std::path::PathBuf; use std::{env, fs}; use rand::{Rng, OsRng}; use ethstore::dir::{KeyDirectory, DiskDirectory}; -use ethstore::ethkey::Address; use ethstore::{Error, SafeAccount}; pub fn random_dir() -> PathBuf { @@ -68,7 +67,7 @@ impl KeyDirectory for TransientDir { self.dir.insert(account) } - fn remove(&self, address: &Address) -> Result<(), Error> { - self.dir.remove(address) + fn remove(&self, account: &SafeAccount) -> Result<(), Error> { + self.dir.remove(account) } } From 6397556cbb5f240a2f77dc227d9d02fcf42a0240 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 30 Nov 2016 15:08:38 +0100 Subject: [PATCH 02/11] Sign with token support --- ethcore/src/account_provider.rs | 72 ++++++++++---- ethstore/src/ethstore.rs | 168 +++++++++++++++----------------- ethstore/src/lib.rs | 6 +- ethstore/src/random.rs | 8 +- ethstore/src/secret_store.rs | 15 ++- 5 files changed, 152 insertions(+), 117 deletions(-) diff --git a/ethcore/src/account_provider.rs b/ethcore/src/account_provider.rs index e2ccd1d83..311204bb1 100644 --- a/ethcore/src/account_provider.rs +++ b/ethcore/src/account_provider.rs @@ -20,8 +20,8 @@ use std::{fs, fmt}; use std::collections::HashMap; use std::path::PathBuf; use std::time::{Instant, Duration}; -use util::{Mutex, RwLock}; -use ethstore::{SecretStore, Error as SSError, SafeAccount, EthStore}; +use util::{Mutex, RwLock, Itertools}; +use ethstore::{SimpleSecretStore, SecretStore, Error as SSError, SafeAccount, EthStore, EthMultiStore, random_string}; use ethstore::dir::{KeyDirectory}; use ethstore::ethkey::{Address, Message, Public, Secret, Random, Generator}; use ethjson::misc::AccountMeta; @@ -72,21 +72,35 @@ impl From for Error { #[derive(Default)] struct NullDir { - accounts: RwLock>, + accounts: RwLock>>, } impl KeyDirectory for NullDir { fn load(&self) -> Result, SSError> { - Ok(self.accounts.read().values().cloned().collect()) + Ok(self.accounts.read().values().cloned().flatten().collect()) } fn insert(&self, account: SafeAccount) -> Result { - self.accounts.write().insert(account.address.clone(), account.clone()); + self.accounts.write() + .entry(account.address.clone()) + .or_insert_with(Vec::new) + .push(account.clone()); Ok(account) } - fn remove(&self, address: &Address) -> Result<(), SSError> { - self.accounts.write().remove(address); + fn remove(&self, account: &SafeAccount) -> Result<(), SSError> { + let mut accounts = self.accounts.write(); + let is_empty = if let Some(mut accounts) = accounts.get_mut(&account.address) { + if let Some(position) = accounts.iter().position(|acc| acc == account) { + accounts.remove(position); + } + accounts.is_empty() + } else { + false + }; + if is_empty { + accounts.remove(&account.address); + } Ok(()) } } @@ -163,10 +177,12 @@ impl AddressBook { } } -fn transient_sstore() -> Box { - Box::new(EthStore::open(Box::new(NullDir::default())).expect("NullDir load always succeeds; qed"))) +fn transient_sstore() -> EthMultiStore { + EthMultiStore::open(Box::new(NullDir::default())).expect("NullDir load always succeeds; qed") } +type AccountToken = String; + /// Account management. /// Responsible for unlocking accounts. pub struct AccountProvider { @@ -175,7 +191,7 @@ pub struct AccountProvider { /// Accounts on disk sstore: Box, /// Accounts unlocked with rolling tokens - transient_sstore: Box, + transient_sstore: EthMultiStore, } impl AccountProvider { @@ -194,7 +210,7 @@ impl AccountProvider { AccountProvider { unlocked: Mutex::new(HashMap::new()), address_book: Mutex::new(AddressBook::transient()), - sstore: transient_sstore(), + sstore: Box::new(EthStore::open(Box::new(NullDir::default())).expect("NullDir load always succeeds; qed")), transient_sstore: transient_sstore(), } } @@ -285,11 +301,8 @@ impl AccountProvider { /// Returns `true` if the password for `account` is `password`. `false` if not. pub fn test_password(&self, account: &Address, password: &str) -> Result { - match self.sstore.sign(account, password, &Default::default()) { - Ok(_) => Ok(true), - Err(SSError::InvalidPassword) => Ok(false), - Err(e) => Err(Error::SStore(e)), - } + self.sstore.test_password(account, password) + .map_err(Into::into) } /// Permanently removes an account. @@ -368,6 +381,26 @@ impl AccountProvider { Ok(try!(self.sstore.sign(&account, &password, &message))) } + /// Signs given message with supplied token. Returns a token to use in next signing within this session. + pub fn sign_with_token(&self, account: Address, token: AccountToken, message: Message) -> Result<(Signature, AccountToken), Error> { + let is_std_password = try!(self.sstore.test_password(&account, &token)); + + let new_token = random_string(16); + let signature = if is_std_password { + // Insert to transient store + try!(self.sstore.copy_account(&self.transient_sstore, &account, &token, &new_token)); + // sign + try!(self.sstore.sign(&account, &token, &message)) + } else { + // check transient store + try!(self.transient_sstore.change_password(&account, &token, &new_token)); + // and sign + try!(self.transient_sstore.sign(&account, &new_token, &message)) + }; + + Ok((signature, new_token)) + } + /// Decrypts a message. If password is not provided the account must be unlocked. pub fn decrypt(&self, account: Address, password: Option, shared_mac: &[u8], message: &[u8]) -> Result, Error> { let password = try!(password.map(Ok).unwrap_or_else(|| self.password(&account))); @@ -450,6 +483,11 @@ mod tests { assert!(ap.insert_account(kp.secret().clone(), "test").is_ok()); // when - let (_signature, token) = ap.sign_with_token(kp.address(), "test", Default::default()).unwrap(); + let (_signature, token) = ap.sign_with_token(kp.address(), "test".into(), Default::default()).unwrap(); + + // then + ap.sign_with_token(kp.address(), token.clone(), Default::default()) + .expect("First usage of token should be correct."); + assert!(ap.sign_with_token(kp.address(), token, Default::default()).is_err(), "Second usage of the same token should fail."); } } diff --git a/ethstore/src/ethstore.rs b/ethstore/src/ethstore.rs index f83e5fd3a..158a7c55a 100644 --- a/ethstore/src/ethstore.rs +++ b/ethstore/src/ethstore.rs @@ -22,7 +22,7 @@ use random::Random; use ethkey::{Signature, Address, Message, Secret, Public}; use dir::KeyDirectory; use account::SafeAccount; -use {Error, SecretStore}; +use {Error, SimpleSecretStore, SecretStore}; use json; use json::UUID; use parking_lot::RwLock; @@ -30,9 +30,7 @@ use presale::PresaleWallet; use import; pub struct EthStore { - dir: Box, - iterations: u32, - cache: RwLock>, + store: EthMultiStore, } impl EthStore { @@ -41,57 +39,46 @@ impl EthStore { } pub fn open_with_iterations(directory: Box, iterations: u32) -> Result { - let accounts = try!(directory.load()); - let cache = accounts.into_iter().map(|account| (account.address.clone(), account)).collect(); - let store = EthStore { - dir: directory, - iterations: iterations, - cache: RwLock::new(cache), - }; - Ok(store) - } - - fn save(&self, account: SafeAccount) -> Result<(), Error> { - // save to file - let account = try!(self.dir.insert(account)); - - // update cache - let mut cache = self.cache.write(); - cache.insert(account.address.clone(), account); - Ok(()) - } - - fn reload_accounts(&self) -> Result<(), Error> { - let mut cache = self.cache.write(); - 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(()) + Ok(EthStore { + store: try!(EthMultiStore::open_with_iterations(directory, iterations)), + }) } fn get(&self, address: &Address) -> Result { - { - let cache = self.cache.read(); - if let Some(account) = cache.get(address) { - return Ok(account.clone()) - } - } - try!(self.reload_accounts()); - let cache = self.cache.read(); - cache.get(address).cloned().ok_or(Error::InvalidAccount) + let mut accounts = try!(self.store.get(address)).into_iter(); + accounts.next().ok_or(Error::InvalidAccount) + } +} + +impl SimpleSecretStore for EthStore { + fn insert_account(&self, secret: Secret, password: &str) -> Result { + self.store.insert_account(secret, password) + } + + fn accounts(&self) -> Result, Error> { + self.store.accounts() + } + + fn change_password(&self, address: &Address, old_password: &str, new_password: &str) -> Result<(), Error> { + self.store.change_password(address, old_password, new_password) + } + + fn remove_account(&self, address: &Address, password: &str) -> Result<(), Error> { + self.store.remove_account(address, password) + } + + fn sign(&self, address: &Address, password: &str, message: &Message) -> Result { + let account = try!(self.get(address)); + account.sign(password, message) + } + + fn decrypt(&self, account: &Address, password: &str, shared_mac: &[u8], message: &[u8]) -> Result, Error> { + let account = try!(self.get(account)); + account.decrypt(password, shared_mac, message) } } impl SecretStore for EthStore { - fn insert_account(&self, secret: Secret, password: &str) -> Result { - let keypair = try!(KeyPair::from_secret(secret).map_err(|_| Error::CreationFailed)); - let id: [u8; 16] = Random::random(); - let account = SafeAccount::create(&keypair, id, password, self.iterations, "".to_owned(), "{}".to_owned()); - let address = account.address.clone(); - try!(self.save(account)); - Ok(address) - } - fn import_presale(&self, json: &[u8], password: &str) -> Result { let json_wallet = try!(json::PresaleWallet::load(json).map_err(|_| Error::InvalidKeyFile("Invalid JSON format".to_owned()))); let wallet = PresaleWallet::from(json_wallet); @@ -105,46 +92,20 @@ impl SecretStore for EthStore { let secret = try!(safe_account.crypto.secret(password).map_err(|_| Error::InvalidPassword)); safe_account.address = try!(KeyPair::from_secret(secret)).address(); let address = safe_account.address.clone(); - try!(self.save(safe_account)); + try!(self.store.save(safe_account)); Ok(address) } - fn accounts(&self) -> Result, Error> { - try!(self.reload_accounts()); - Ok(self.cache.read().keys().cloned().collect()) - } - - fn change_password(&self, address: &Address, old_password: &str, new_password: &str) -> Result<(), Error> { - // change password + fn test_password(&self, address: &Address, password: &str) -> Result { let account = try!(self.get(address)); - let account = try!(account.change_password(old_password, new_password, self.iterations)); - - // save to file - self.save(account) + Ok(account.check_password(password)) } - fn remove_account(&self, address: &Address, password: &str) -> Result<(), Error> { + fn copy_account(&self, new_store: &SimpleSecretStore, address: &Address, password: &str, new_password: &str) -> Result<(), Error> { let account = try!(self.get(address)); - let can_remove = account.check_password(password); - - if can_remove { - try!(self.dir.remove(&account)); - let mut cache = self.cache.write(); - cache.remove(address); - Ok(()) - } else { - Err(Error::InvalidPassword) - } - } - - fn sign(&self, address: &Address, password: &str, message: &Message) -> Result { - let account = try!(self.get(address)); - account.sign(password, message) - } - - fn decrypt(&self, account: &Address, password: &str, shared_mac: &[u8], message: &[u8]) -> Result, Error> { - let account = try!(self.get(account)); - account.decrypt(password, shared_mac, message) + let secret = try!(account.crypto.secret(password)); + try!(new_store.insert_account(secret, new_password)); + Ok(()) } fn public(&self, account: &Address, password: &str) -> Result { @@ -172,7 +133,7 @@ impl SecretStore for EthStore { account.name = name; // save to file - self.save(account) + self.store.save(account) } fn set_meta(&self, address: &Address, meta: String) -> Result<(), Error> { @@ -180,11 +141,11 @@ impl SecretStore for EthStore { account.meta = meta; // save to file - self.save(account) + self.store.save(account) } fn local_path(&self) -> String { - self.dir.path().map(|p| p.to_string_lossy().into_owned()).unwrap_or_else(|| String::new()) + self.store.dir.path().map(|p| p.to_string_lossy().into_owned()).unwrap_or_else(|| String::new()) } fn list_geth_accounts(&self, testnet: bool) -> Vec
{ @@ -192,7 +153,7 @@ impl SecretStore for EthStore { } fn import_geth_accounts(&self, desired: Vec
, testnet: bool) -> Result, Error> { - import::import_geth_accounts(&*self.dir, desired.into_iter().collect(), testnet) + import::import_geth_accounts(&*self.store.dir, desired.into_iter().collect(), testnet) } } @@ -210,7 +171,7 @@ impl EthMultiStore { } pub fn open_with_iterations(directory: Box, iterations: u32) -> Result { - let mut store = EthMultiStore { + let store = EthMultiStore { dir: directory, iterations: iterations, cache: Default::default(), @@ -252,7 +213,7 @@ impl EthMultiStore { } } - pub fn insert_account(&self, account: SafeAccount) -> Result<(), Error> { + fn save(&self, account: SafeAccount) -> Result<(), Error> { //save to file let account = try!(self.dir.insert(account)); @@ -263,7 +224,24 @@ impl EthMultiStore { Ok(()) } - pub fn remove_account(&self, address: &Address, password: &str) -> Result<(), Error> { +} + +impl SimpleSecretStore for EthMultiStore { + fn insert_account(&self, secret: Secret, password: &str) -> Result { + let keypair = try!(KeyPair::from_secret(secret).map_err(|_| Error::CreationFailed)); + let id: [u8; 16] = Random::random(); + let account = SafeAccount::create(&keypair, id, password, self.iterations, "".to_owned(), "{}".to_owned()); + let address = account.address.clone(); + try!(self.save(account)); + Ok(address) + } + + fn accounts(&self) -> Result, Error> { + try!(self.reload_accounts()); + Ok(self.cache.read().keys().cloned().collect()) + } + + fn remove_account(&self, address: &Address, password: &str) -> Result<(), Error> { let accounts = try!(self.get(address)); for account in accounts { @@ -294,19 +272,19 @@ impl EthMultiStore { Err(Error::InvalidPassword) } - pub fn change_password(&self, address: &Address, old_password: &str, new_password: &str) -> Result<(), Error> { + fn change_password(&self, address: &Address, old_password: &str, new_password: &str) -> Result<(), Error> { let accounts = try!(self.get(address)); for account in accounts { // First remove try!(self.remove_account(&address, old_password)); // Then insert back with new password let new_account = try!(account.change_password(old_password, new_password, self.iterations)); - try!(self.insert_account(new_account)); + try!(self.save(new_account)); } Ok(()) } - pub fn sign(&self, address: &Address, password: &str, message: &Message) -> Result { + fn sign(&self, address: &Address, password: &str, message: &Message) -> Result { let accounts = try!(self.get(address)); for account in accounts { if account.check_password(password) { @@ -317,7 +295,7 @@ impl EthMultiStore { Err(Error::InvalidPassword) } - pub fn decrypt(&self, account: &Address, password: &str, shared_mac: &[u8], message: &[u8]) -> Result, Error> { + fn decrypt(&self, account: &Address, password: &str, shared_mac: &[u8], message: &[u8]) -> Result, Error> { let accounts = try!(self.get(account)); for account in accounts { if account.check_password(password) { @@ -328,3 +306,9 @@ impl EthMultiStore { } } +#[cfg(test)] +mod tests { + fn should_have_some_tests() { + assert_eq!(true, false) + } +} diff --git a/ethstore/src/lib.rs b/ethstore/src/lib.rs index f8619ff19..3fe56b7d3 100644 --- a/ethstore/src/lib.rs +++ b/ethstore/src/lib.rs @@ -50,8 +50,8 @@ mod secret_store; pub use self::account::SafeAccount; pub use self::error::Error; -pub use self::ethstore::EthStore; +pub use self::ethstore::{EthStore, EthMultiStore}; 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; +pub use self::secret_store::{SimpleSecretStore, SecretStore}; +pub use self::random::{random_phrase, random_string}; diff --git a/ethstore/src/random.rs b/ethstore/src/random.rs index 954ec500f..d98f2fcdf 100644 --- a/ethstore/src/random.rs +++ b/ethstore/src/random.rs @@ -51,10 +51,16 @@ pub fn random_phrase(words: usize) -> String { .map(|s| s.to_owned()) .collect(); } - let mut rng = OsRng::new().unwrap(); + let mut rng = OsRng::new().expect("Not able to operate without random source."); (0..words).map(|_| rng.choose(&WORDS).unwrap()).join(" ") } +/// Generate a random string of given length. +pub fn random_string(length: usize) -> String { + let mut rng = OsRng::new().expect("Not able to operate without random source."); + rng.gen_ascii_chars().take(length).collect() +} + #[cfg(test)] mod tests { use super::random_phrase; diff --git a/ethstore/src/secret_store.rs b/ethstore/src/secret_store.rs index 06f38922b..b62189aca 100644 --- a/ethstore/src/secret_store.rs +++ b/ethstore/src/secret_store.rs @@ -18,18 +18,25 @@ use ethkey::{Address, Message, Signature, Secret, Public}; use Error; use json::UUID; -pub trait SecretStore: Send + Sync { +pub trait SimpleSecretStore: Send + Sync { fn insert_account(&self, secret: Secret, password: &str) -> Result; - fn import_presale(&self, json: &[u8], password: &str) -> Result; - fn import_wallet(&self, json: &[u8], password: &str) -> Result; fn change_password(&self, account: &Address, old_password: &str, new_password: &str) -> Result<(), Error>; fn remove_account(&self, account: &Address, password: &str) -> Result<(), Error>; fn sign(&self, account: &Address, password: &str, message: &Message) -> Result; fn decrypt(&self, account: &Address, password: &str, shared_mac: &[u8], message: &[u8]) -> Result, Error>; - fn public(&self, account: &Address, password: &str) -> Result; fn accounts(&self) -> Result, Error>; +} + +pub trait SecretStore: SimpleSecretStore { + fn import_presale(&self, json: &[u8], password: &str) -> Result; + fn import_wallet(&self, json: &[u8], password: &str) -> Result; + fn copy_account(&self, new_store: &SimpleSecretStore, account: &Address, password: &str, new_password: &str) -> Result<(), Error>; + fn test_password(&self, account: &Address, password: &str) -> Result; + + fn public(&self, account: &Address, password: &str) -> Result; + fn uuid(&self, account: &Address) -> Result; fn name(&self, account: &Address) -> Result; fn meta(&self, account: &Address) -> Result; From c028f106b1924b6656bbd1f66d7fbdf836e219f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 30 Nov 2016 16:11:41 +0100 Subject: [PATCH 03/11] RPC for confirming with token --- ethcore/src/account_provider.rs | 24 +++++- rpc/src/v1/helpers/dispatch.rs | 114 +++++++++++++++++++++++------ rpc/src/v1/impls/personal.rs | 4 +- rpc/src/v1/impls/signer.rs | 61 +++++++++------ rpc/src/v1/impls/signing.rs | 4 +- rpc/src/v1/impls/signing_unsafe.rs | 3 +- rpc/src/v1/tests/mocked/signer.rs | 46 ++++++++++++ rpc/src/v1/traits/signer.rs | 6 +- rpc/src/v1/types/confirmations.rs | 25 +++++++ rpc/src/v1/types/mod.rs.in | 4 +- 10 files changed, 237 insertions(+), 54 deletions(-) diff --git a/ethcore/src/account_provider.rs b/ethcore/src/account_provider.rs index 311204bb1..637a20401 100644 --- a/ethcore/src/account_provider.rs +++ b/ethcore/src/account_provider.rs @@ -401,6 +401,28 @@ impl AccountProvider { Ok((signature, new_token)) } + /// Decrypts a message with given token. Returns a token to use in next operation for this account. + pub fn decrypt_with_token(&self, account: Address, token: AccountToken, shared_mac: &[u8], message: &[u8]) + -> Result<(Vec, AccountToken), Error> + { + let is_std_password = try!(self.sstore.test_password(&account, &token)); + + let new_token = random_string(16); + let message = if is_std_password { + // Insert to transient store + try!(self.sstore.copy_account(&self.transient_sstore, &account, &token, &new_token)); + // decrypt + try!(self.sstore.decrypt(&account, &token, shared_mac, message)) + } else { + // check transient store + try!(self.transient_sstore.change_password(&account, &token, &new_token)); + // and decrypt + try!(self.transient_sstore.decrypt(&account, &token, shared_mac, message)) + }; + + Ok((message, new_token)) + } + /// Decrypts a message. If password is not provided the account must be unlocked. pub fn decrypt(&self, account: Address, password: Option, shared_mac: &[u8], message: &[u8]) -> Result, Error> { let password = try!(password.map(Ok).unwrap_or_else(|| self.password(&account))); @@ -477,8 +499,8 @@ mod tests { #[test] fn should_sign_and_return_token() { - let kp = Random.generate().unwrap(); // given + let kp = Random.generate().unwrap(); let ap = AccountProvider::transient_provider(); assert!(ap.insert_account(kp.secret().clone(), "test").is_ok()); diff --git a/rpc/src/v1/helpers/dispatch.rs b/rpc/src/v1/helpers/dispatch.rs index a66bc816d..3ac310be6 100644 --- a/rpc/src/v1/helpers/dispatch.rs +++ b/rpc/src/v1/helpers/dispatch.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . +use std::fmt::Debug; use rlp; use util::{Address, H256, U256, Uint, Bytes}; use util::bytes::ToPretty; @@ -37,46 +38,101 @@ use v1::types::{ pub const DEFAULT_MAC: [u8; 2] = [0, 0]; -pub fn execute(client: &C, miner: &M, accounts: &AccountProvider, payload: ConfirmationPayload, pass: Option) -> Result +type AccountToken = String; + +#[derive(Debug, Clone, PartialEq)] +pub enum SignWith { + Nothing, + Password(String), + Token(AccountToken), +} + +#[derive(Debug)] +pub enum WithToken { + No(T), + Yes(T, AccountToken), +} + +impl WithToken { + pub fn map(self, f: F) -> WithToken where + S: Debug, + F: FnOnce(T) -> S, + { + match self { + WithToken::No(v) => WithToken::No(f(v)), + WithToken::Yes(v, token) => WithToken::Yes(f(v), token), + } + } + + pub fn into_value(self) -> T { + match self { + WithToken::No(v) => v, + WithToken::Yes(v, ..) => v, + } + } +} + +impl From<(T, AccountToken)> for WithToken { + fn from(tuple: (T, AccountToken)) -> Self { + WithToken::Yes(tuple.0, tuple.1) + } +} + +pub fn execute(client: &C, miner: &M, accounts: &AccountProvider, payload: ConfirmationPayload, pass: SignWith) -> Result, Error> where C: MiningBlockChainClient, M: MinerService { match payload { ConfirmationPayload::SendTransaction(request) => { sign_and_dispatch(client, miner, accounts, request, pass) - .map(RpcH256::from) - .map(ConfirmationResponse::SendTransaction) + .map(|result| result + .map(RpcH256::from) + .map(ConfirmationResponse::SendTransaction) + ) }, ConfirmationPayload::SignTransaction(request) => { sign_no_dispatch(client, miner, accounts, request, pass) - .map(RpcRichRawTransaction::from) - .map(ConfirmationResponse::SignTransaction) + .map(|result| result + .map(RpcRichRawTransaction::from) + .map(ConfirmationResponse::SignTransaction) + ) }, ConfirmationPayload::Signature(address, hash) => { signature(accounts, address, hash, pass) - .map(RpcH520::from) - .map(ConfirmationResponse::Signature) + .map(|result| result + .map(RpcH520::from) + .map(ConfirmationResponse::Signature) + ) }, ConfirmationPayload::Decrypt(address, data) => { decrypt(accounts, address, data, pass) - .map(RpcBytes) - .map(ConfirmationResponse::Decrypt) + .map(|result| result + .map(RpcBytes) + .map(ConfirmationResponse::Decrypt) + ) }, } } -fn signature(accounts: &AccountProvider, address: Address, hash: H256, password: Option) -> Result { - accounts.sign(address, password.clone(), hash).map_err(|e| match password { - Some(_) => errors::from_password_error(e), - None => errors::from_signing_error(e), +fn signature(accounts: &AccountProvider, address: Address, hash: H256, password: SignWith) -> Result, Error> { + match password.clone() { + SignWith::Nothing => accounts.sign(address, None, hash).map(WithToken::No), + SignWith::Password(pass) => accounts.sign(address, Some(pass), hash).map(WithToken::No), + SignWith::Token(token) => accounts.sign_with_token(address, token, hash).map(Into::into), + }.map_err(|e| match password { + SignWith::Nothing => errors::from_signing_error(e), + _ => errors::from_password_error(e), }) } -fn decrypt(accounts: &AccountProvider, address: Address, msg: Bytes, password: Option) -> Result { - accounts.decrypt(address, password.clone(), &DEFAULT_MAC, &msg) - .map_err(|e| match password { - Some(_) => errors::from_password_error(e), - None => errors::from_signing_error(e), - }) +fn decrypt(accounts: &AccountProvider, address: Address, msg: Bytes, password: SignWith) -> Result, Error> { + match password.clone() { + SignWith::Nothing => accounts.decrypt(address, None, &DEFAULT_MAC, &msg).map(WithToken::No), + SignWith::Password(pass) => accounts.decrypt(address, Some(pass), &DEFAULT_MAC, &msg).map(WithToken::No), + SignWith::Token(token) => accounts.decrypt_with_token(address, token, &DEFAULT_MAC, &msg).map(Into::into), + }.map_err(|e| match password { + SignWith::Nothing => errors::from_signing_error(e), + _ => errors::from_password_error(e), + }) } pub fn dispatch_transaction(client: &C, miner: &M, signed_transaction: SignedTransaction) -> Result @@ -88,7 +144,7 @@ pub fn dispatch_transaction(client: &C, miner: &M, signed_transaction: Sig .map(|_| hash) } -pub fn sign_no_dispatch(client: &C, miner: &M, accounts: &AccountProvider, filled: FilledTransactionRequest, password: Option) -> Result +pub fn sign_no_dispatch(client: &C, miner: &M, accounts: &AccountProvider, filled: FilledTransactionRequest, password: SignWith) -> Result, Error> where C: MiningBlockChainClient, M: MinerService { let network_id = client.signing_network_id(); @@ -110,20 +166,32 @@ pub fn sign_no_dispatch(client: &C, miner: &M, accounts: &AccountProvider, let hash = t.hash(network_id); let signature = try!(signature(accounts, address, hash, password)); - t.with_signature(signature, network_id) + signature.map(|sig| { + t.with_signature(sig, network_id) + }) }; Ok(signed_transaction) } -pub fn sign_and_dispatch(client: &C, miner: &M, accounts: &AccountProvider, filled: FilledTransactionRequest, password: Option) -> Result +pub fn sign_and_dispatch(client: &C, miner: &M, accounts: &AccountProvider, filled: FilledTransactionRequest, password: SignWith) -> Result, Error> where C: MiningBlockChainClient, M: MinerService { let network_id = client.signing_network_id(); let signed_transaction = try!(sign_no_dispatch(client, miner, accounts, filled, password)); + let (signed_transaction, token) = match signed_transaction { + WithToken::No(signed_transaction) => (signed_transaction, None), + WithToken::Yes(signed_transaction, token) => (signed_transaction, Some(token)), + }; + trace!(target: "miner", "send_transaction: dispatching tx: {} for network ID {:?}", rlp::encode(&signed_transaction).to_vec().pretty(), network_id); - dispatch_transaction(&*client, &*miner, signed_transaction) + dispatch_transaction(&*client, &*miner, signed_transaction).map(|hash| { + match token { + Some(ref token) => WithToken::Yes(hash, token.clone()), + None => WithToken::No(hash), + } + }) } pub fn fill_optional_fields(request: TransactionRequest, client: &C, miner: &M) -> FilledTransactionRequest diff --git a/rpc/src/v1/impls/personal.rs b/rpc/src/v1/impls/personal.rs index 1515e3fa1..48ed584cf 100644 --- a/rpc/src/v1/impls/personal.rs +++ b/rpc/src/v1/impls/personal.rs @@ -114,7 +114,7 @@ impl Personal for PersonalClient where C: MiningBl &*miner, &*accounts, request, - Some(password) - ).map(Into::into) + dispatch::SignWith::Password(password) + ).map(|v| v.into_value().into()) } } diff --git a/rpc/src/v1/impls/signer.rs b/rpc/src/v1/impls/signer.rs index 66f46ba01..bdb34dab7 100644 --- a/rpc/src/v1/impls/signer.rs +++ b/rpc/src/v1/impls/signer.rs @@ -26,7 +26,7 @@ use ethcore::miner::MinerService; use jsonrpc_core::Error; use v1::traits::Signer; -use v1::types::{TransactionModification, ConfirmationRequest, ConfirmationResponse, U256, Bytes}; +use v1::types::{TransactionModification, ConfirmationRequest, ConfirmationResponse, ConfirmationResponseWithToken, U256, Bytes}; use v1::helpers::{errors, SignerService, SigningQueue, ConfirmationPayload}; use v1::helpers::dispatch::{self, dispatch_transaction}; @@ -60,6 +60,35 @@ impl SignerClient where C: MiningBlockChainClient, take_weak!(self.client).keep_alive(); Ok(()) } + + fn confirm_internal(&self, id: U256, modification: TransactionModification, f: F) -> Result where + F: FnOnce(&C, &M, &AccountProvider, ConfirmationPayload) -> Result, + { + try!(self.active()); + + let id = id.into(); + let accounts = take_weak!(self.accounts); + let signer = take_weak!(self.signer); + let client = take_weak!(self.client); + let miner = take_weak!(self.miner); + + signer.peek(&id).map(|confirmation| { + let mut payload = confirmation.payload.clone(); + // Modify payload + match (&mut payload, modification.gas_price) { + (&mut ConfirmationPayload::SendTransaction(ref mut request), Some(gas_price)) => { + request.gas_price = gas_price.into(); + }, + _ => {}, + } + let result = f(&*client, &*miner, &*accounts, payload); + // Execute + if let Ok(ref response) = result { + signer.request_confirmed(id, Ok(response.clone())); + } + result + }).unwrap_or_else(|| Err(errors::invalid_params("Unknown RequestID", id))) + } } impl Signer for SignerClient where C: MiningBlockChainClient, M: MinerService { @@ -78,30 +107,14 @@ impl Signer for SignerClient where C: MiningBlockC // TODO [ToDr] TransactionModification is redundant for some calls // might be better to replace it in future fn confirm_request(&self, id: U256, modification: TransactionModification, pass: String) -> Result { - try!(self.active()); + self.confirm_internal(id, modification, move |client, miner, accounts, payload| { + dispatch::execute(client, miner, accounts, payload, dispatch::SignWith::Password(pass)) + .map(|v| v.into_value()) + }) + } - let id = id.into(); - let accounts = take_weak!(self.accounts); - let signer = take_weak!(self.signer); - let client = take_weak!(self.client); - let miner = take_weak!(self.miner); - - signer.peek(&id).map(|confirmation| { - let mut payload = confirmation.payload.clone(); - // Modify payload - match (&mut payload, modification.gas_price) { - (&mut ConfirmationPayload::SendTransaction(ref mut request), Some(gas_price)) => { - request.gas_price = gas_price.into(); - }, - _ => {}, - } - // Execute - let result = dispatch::execute(&*client, &*miner, &*accounts, payload, Some(pass)); - if let Ok(ref response) = result { - signer.request_confirmed(id, Ok(response.clone())); - } - result - }).unwrap_or_else(|| Err(errors::invalid_params("Unknown RequestID", id))) + fn confirm_request_with_token(&self, id: U256, modification: TransactionModification, token: String) -> Result { + unimplemented!() } fn confirm_request_raw(&self, id: U256, bytes: Bytes) -> Result { diff --git a/rpc/src/v1/impls/signing.rs b/rpc/src/v1/impls/signing.rs index 262e04dfb..c055628c0 100644 --- a/rpc/src/v1/impls/signing.rs +++ b/rpc/src/v1/impls/signing.rs @@ -99,7 +99,9 @@ impl SigningQueueClient where let sender = payload.sender(); if accounts.is_unlocked(sender) { - return dispatch::execute(&*client, &*miner, &*accounts, payload, None).map(DispatchResult::Value); + return dispatch::execute(&*client, &*miner, &*accounts, payload, dispatch::SignWith::Nothing) + .map(|v| v.into_value()) + .map(DispatchResult::Value); } take_weak!(self.signer).add_request(payload) diff --git a/rpc/src/v1/impls/signing_unsafe.rs b/rpc/src/v1/impls/signing_unsafe.rs index 46ffe6ded..3f03f7609 100644 --- a/rpc/src/v1/impls/signing_unsafe.rs +++ b/rpc/src/v1/impls/signing_unsafe.rs @@ -75,7 +75,8 @@ impl SigningUnsafeClient where let accounts = take_weak!(self.accounts); let payload = dispatch::from_rpc(payload, &*client, &*miner); - dispatch::execute(&*client, &*miner, &*accounts, payload, None) + dispatch::execute(&*client, &*miner, &*accounts, payload, dispatch::SignWith::Nothing) + .map(|v| v.into_value()) } } diff --git a/rpc/src/v1/tests/mocked/signer.rs b/rpc/src/v1/tests/mocked/signer.rs index e2ba580e0..c4df02606 100644 --- a/rpc/src/v1/tests/mocked/signer.rs +++ b/rpc/src/v1/tests/mocked/signer.rs @@ -209,6 +209,52 @@ fn should_confirm_transaction_and_dispatch() { assert_eq!(tester.miner.imported_transactions.lock().len(), 1); } +#[test] +fn should_confirm_transaction_with_token() { + // given + let tester = signer_tester(); + let address = tester.accounts.new_account("test").unwrap(); + let recipient = Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap(); + tester.signer.add_request(ConfirmationPayload::SendTransaction(FilledTransactionRequest { + from: address, + to: Some(recipient), + gas_price: U256::from(10_000), + gas: U256::from(10_000_000), + value: U256::from(1), + data: vec![], + nonce: None, + })).unwrap(); + + let t = Transaction { + nonce: U256::zero(), + gas_price: U256::from(0x1000), + gas: U256::from(10_000_000), + action: Action::Call(recipient), + value: U256::from(0x1), + data: vec![] + }; + let (signature, token) = tester.accounts.sign_with_token(address, "test".into(), t.hash(None)).unwrap(); + let t = t.with_signature(signature, None); + + assert_eq!(tester.signer.requests().len(), 1); + + // when + let request = r#"{ + "jsonrpc":"2.0", + "method":"signer_confirmRequestWithToken", + "params":["0x1", {"gasPrice":"0x1000"}, ""#.to_owned() + &token + r#""], + "id":1 + }"#; + let response = r#"{"jsonrpc":"2.0","result":{"result":""#.to_owned() + + format!("0x{:?}", t.hash()).as_ref() + + r#""token":""},"id":1}"#; + + // then + assert_eq!(tester.io.handle_request_sync(&request), Some(response.to_owned())); + assert_eq!(tester.signer.requests().len(), 0); + assert_eq!(tester.miner.imported_transactions.lock().len(), 1); +} + #[test] fn should_confirm_transaction_with_rlp() { // given diff --git a/rpc/src/v1/traits/signer.rs b/rpc/src/v1/traits/signer.rs index eafa520d4..5014dc4a0 100644 --- a/rpc/src/v1/traits/signer.rs +++ b/rpc/src/v1/traits/signer.rs @@ -18,7 +18,7 @@ use jsonrpc_core::Error; use v1::helpers::auto_args::Wrap; -use v1::types::{U256, Bytes, TransactionModification, ConfirmationRequest, ConfirmationResponse}; +use v1::types::{U256, Bytes, TransactionModification, ConfirmationRequest, ConfirmationResponse, ConfirmationResponseWithToken}; build_rpc_trait! { @@ -33,6 +33,10 @@ build_rpc_trait! { #[rpc(name = "signer_confirmRequest")] fn confirm_request(&self, U256, TransactionModification, String) -> Result; + /// Confirm specific request with token. + #[rpc(name = "signer_confirmRequestWithToken")] + fn confirm_request_with_token(&self, U256, TransactionModification, String) -> Result; + /// Confirm specific request with already signed data. #[rpc(name = "signer_confirmRequestRaw")] fn confirm_request_raw(&self, U256, Bytes) -> Result; diff --git a/rpc/src/v1/types/confirmations.rs b/rpc/src/v1/types/confirmations.rs index bbbad83f3..795d24726 100644 --- a/rpc/src/v1/types/confirmations.rs +++ b/rpc/src/v1/types/confirmations.rs @@ -101,6 +101,15 @@ impl Serialize for ConfirmationResponse { } } +/// Confirmation response with additional token for further requests +#[derive(Debug, Clone, PartialEq, Serialize)] +pub struct ConfirmationResponseWithToken { + /// Actual response + pub result: ConfirmationResponse, + /// New token + pub token: String, +} + /// Confirmation payload, i.e. the thing to be confirmed #[derive(Debug, Clone, Eq, PartialEq, Hash, Serialize)] pub enum ConfirmationPayload { @@ -247,5 +256,21 @@ mod tests { gas_price: None, }); } + + #[test] + fn should_serialize_confirmation_response_with_token() { + // given + let response = ConfirmationResponseWithToken { + result: ConfirmationResponse::SendTransaction(H256::default()), + token: "test-token".into(), + }; + + // when + let res = serde_json::to_string(&response); + let expected = r#"{"result":"0x0000000000000000000000000000000000000000","token":"test-token"}"#; + + // then + assert_eq!(res.unwrap(), expected.to_owned()); + } } diff --git a/rpc/src/v1/types/mod.rs.in b/rpc/src/v1/types/mod.rs.in index 55e8fd27b..6b6f01443 100644 --- a/rpc/src/v1/types/mod.rs.in +++ b/rpc/src/v1/types/mod.rs.in @@ -38,7 +38,9 @@ pub use self::bytes::Bytes; pub use self::block::{RichBlock, Block, BlockTransactions}; pub use self::block_number::BlockNumber; pub use self::call_request::CallRequest; -pub use self::confirmations::{ConfirmationPayload, ConfirmationRequest, ConfirmationResponse, TransactionModification, SignRequest, DecryptRequest, Either}; +pub use self::confirmations::{ + ConfirmationPayload, ConfirmationRequest, ConfirmationResponse, ConfirmationResponseWithToken, TransactionModification, SignRequest, DecryptRequest, Either +}; pub use self::filter::{Filter, FilterChanges}; pub use self::hash::{H64, H160, H256, H512, H520, H2048}; pub use self::index::Index; From 022ccb5bcef2ef7eab2dafcad52188dcad76f6f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 30 Nov 2016 16:21:57 +0100 Subject: [PATCH 04/11] Fixing tests --- ethcore/src/account_provider.rs | 9 +++++---- ethstore/src/ethstore.rs | 3 +++ rpc/src/v1/types/confirmations.rs | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/ethcore/src/account_provider.rs b/ethcore/src/account_provider.rs index 637a20401..da5992f0c 100644 --- a/ethcore/src/account_provider.rs +++ b/ethcore/src/account_provider.rs @@ -81,10 +81,11 @@ impl KeyDirectory for NullDir { } fn insert(&self, account: SafeAccount) -> Result { - self.accounts.write() - .entry(account.address.clone()) - .or_insert_with(Vec::new) - .push(account.clone()); + let mut lock = self.accounts.write(); + let mut accounts = lock.entry(account.address.clone()).or_insert_with(Vec::new); + // If the filename is the same we just need to replace the entry + accounts.retain(|acc| acc.filename != account.filename); + accounts.push(account.clone()); Ok(account) } diff --git a/ethstore/src/ethstore.rs b/ethstore/src/ethstore.rs index 158a7c55a..ec3cc16b9 100644 --- a/ethstore/src/ethstore.rs +++ b/ethstore/src/ethstore.rs @@ -220,6 +220,9 @@ impl EthMultiStore { // update cache let mut cache = self.cache.write(); let mut accounts = cache.entry(account.address.clone()).or_insert_with(Vec::new); + // TODO [ToDr] That is crappy way of overcoming set_name, set_meta, etc. + // Avoid cloning instead! + accounts.retain(|acc| acc.filename != account.filename); accounts.push(account); Ok(()) } diff --git a/rpc/src/v1/types/confirmations.rs b/rpc/src/v1/types/confirmations.rs index 795d24726..a4284fa5c 100644 --- a/rpc/src/v1/types/confirmations.rs +++ b/rpc/src/v1/types/confirmations.rs @@ -267,7 +267,7 @@ mod tests { // when let res = serde_json::to_string(&response); - let expected = r#"{"result":"0x0000000000000000000000000000000000000000","token":"test-token"}"#; + let expected = r#"{"result":"0x0000000000000000000000000000000000000000000000000000000000000000","token":"test-token"}"#; // then assert_eq!(res.unwrap(), expected.to_owned()); From 1d76bb7048739d7d65530425df20bc2aa339cd23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 30 Nov 2016 16:41:37 +0100 Subject: [PATCH 05/11] Fixing ethstore tests --- ethstore/src/ethstore.rs | 1 + ethstore/tests/api.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ethstore/src/ethstore.rs b/ethstore/src/ethstore.rs index ec3cc16b9..8cbce0f1c 100644 --- a/ethstore/src/ethstore.rs +++ b/ethstore/src/ethstore.rs @@ -311,6 +311,7 @@ impl SimpleSecretStore for EthMultiStore { #[cfg(test)] mod tests { + #[test] fn should_have_some_tests() { assert_eq!(true, false) } diff --git a/ethstore/tests/api.rs b/ethstore/tests/api.rs index e1667607b..a26da4132 100644 --- a/ethstore/tests/api.rs +++ b/ethstore/tests/api.rs @@ -19,7 +19,7 @@ extern crate ethstore; mod util; -use ethstore::{SecretStore, EthStore}; +use ethstore::{SecretStore, EthStore, SimpleSecretStore}; use ethstore::ethkey::{Random, Generator, Secret, KeyPair, verify_address}; use ethstore::dir::DiskDirectory; use util::TransientDir; From dcb7e1e6387f1bdccdb47b4ad49a1dc181c57766 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 30 Nov 2016 17:05:31 +0100 Subject: [PATCH 06/11] Implementing RPC --- ethstore/tests/api.rs | 2 +- rpc/src/v1/helpers/dispatch.rs | 14 +++++++++++++- rpc/src/v1/impls/signer.rs | 21 ++++++++++++++------- rpc/src/v1/tests/mocked/signer.rs | 5 +++-- 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/ethstore/tests/api.rs b/ethstore/tests/api.rs index a26da4132..dd9ec3311 100644 --- a/ethstore/tests/api.rs +++ b/ethstore/tests/api.rs @@ -19,7 +19,7 @@ extern crate ethstore; mod util; -use ethstore::{SecretStore, EthStore, SimpleSecretStore}; +use ethstore::{EthStore, SimpleSecretStore}; use ethstore::ethkey::{Random, Generator, Secret, KeyPair, verify_address}; use ethstore::dir::DiskDirectory; use util::TransientDir; diff --git a/rpc/src/v1/helpers/dispatch.rs b/rpc/src/v1/helpers/dispatch.rs index 3ac310be6..1a70f7e10 100644 --- a/rpc/src/v1/helpers/dispatch.rs +++ b/rpc/src/v1/helpers/dispatch.rs @@ -15,6 +15,7 @@ // along with Parity. If not, see . use std::fmt::Debug; +use std::ops::Deref; use rlp; use util::{Address, H256, U256, Uint, Bytes}; use util::bytes::ToPretty; @@ -53,6 +54,17 @@ pub enum WithToken { Yes(T, AccountToken), } +impl Deref for WithToken { + type Target = T; + + fn deref(&self) -> &Self::Target { + match *self { + WithToken::No(ref v) => v, + WithToken::Yes(ref v, _) => v, + } + } +} + impl WithToken { pub fn map(self, f: F) -> WithToken where S: Debug, @@ -67,7 +79,7 @@ impl WithToken { pub fn into_value(self) -> T { match self { WithToken::No(v) => v, - WithToken::Yes(v, ..) => v, + WithToken::Yes(v, _) => v, } } } diff --git a/rpc/src/v1/impls/signer.rs b/rpc/src/v1/impls/signer.rs index bdb34dab7..97d3de809 100644 --- a/rpc/src/v1/impls/signer.rs +++ b/rpc/src/v1/impls/signer.rs @@ -28,7 +28,7 @@ use jsonrpc_core::Error; use v1::traits::Signer; use v1::types::{TransactionModification, ConfirmationRequest, ConfirmationResponse, ConfirmationResponseWithToken, U256, Bytes}; use v1::helpers::{errors, SignerService, SigningQueue, ConfirmationPayload}; -use v1::helpers::dispatch::{self, dispatch_transaction}; +use v1::helpers::dispatch::{self, dispatch_transaction, WithToken}; /// Transactions confirmation (personal) rpc implementation. pub struct SignerClient where C: MiningBlockChainClient, M: MinerService { @@ -61,8 +61,8 @@ impl SignerClient where C: MiningBlockChainClient, Ok(()) } - fn confirm_internal(&self, id: U256, modification: TransactionModification, f: F) -> Result where - F: FnOnce(&C, &M, &AccountProvider, ConfirmationPayload) -> Result, + fn confirm_internal(&self, id: U256, modification: TransactionModification, f: F) -> Result, Error> where + F: FnOnce(&C, &M, &AccountProvider, ConfirmationPayload) -> Result, Error>, { try!(self.active()); @@ -84,7 +84,7 @@ impl SignerClient where C: MiningBlockChainClient, let result = f(&*client, &*miner, &*accounts, payload); // Execute if let Ok(ref response) = result { - signer.request_confirmed(id, Ok(response.clone())); + signer.request_confirmed(id, Ok((*response).clone())); } result }).unwrap_or_else(|| Err(errors::invalid_params("Unknown RequestID", id))) @@ -109,12 +109,19 @@ impl Signer for SignerClient where C: MiningBlockC fn confirm_request(&self, id: U256, modification: TransactionModification, pass: String) -> Result { self.confirm_internal(id, modification, move |client, miner, accounts, payload| { dispatch::execute(client, miner, accounts, payload, dispatch::SignWith::Password(pass)) - .map(|v| v.into_value()) - }) + }).map(|v| v.into_value()) } fn confirm_request_with_token(&self, id: U256, modification: TransactionModification, token: String) -> Result { - unimplemented!() + self.confirm_internal(id, modification, move |client, miner, accounts, payload| { + dispatch::execute(client, miner, accounts, payload, dispatch::SignWith::Token(token)) + }).and_then(|v| match v { + WithToken::No(_) => Err(errors::internal("Unexpected response without token.", "")), + WithToken::Yes(response, token) => Ok(ConfirmationResponseWithToken { + result: response, + token: token, + }), + }) } fn confirm_request_raw(&self, id: U256, bytes: Bytes) -> Result { diff --git a/rpc/src/v1/tests/mocked/signer.rs b/rpc/src/v1/tests/mocked/signer.rs index c4df02606..eb7fa6c48 100644 --- a/rpc/src/v1/tests/mocked/signer.rs +++ b/rpc/src/v1/tests/mocked/signer.rs @@ -247,10 +247,11 @@ fn should_confirm_transaction_with_token() { }"#; let response = r#"{"jsonrpc":"2.0","result":{"result":""#.to_owned() + format!("0x{:?}", t.hash()).as_ref() + - r#""token":""},"id":1}"#; + r#"","token":""#; // then - assert_eq!(tester.io.handle_request_sync(&request), Some(response.to_owned())); + let result = tester.io.handle_request_sync(&request).unwrap(); + assert!(result.starts_with(&response), "Should return correct result. Expected: {:?}, Got: {:?}", response, result); assert_eq!(tester.signer.requests().len(), 0); assert_eq!(tester.miner.imported_transactions.lock().len(), 1); } From 8596134c0f701d9fec07ccde38373b13467e1819 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Fri, 9 Dec 2016 08:31:58 +0000 Subject: [PATCH 07/11] Clearer updates handling --- ethcore/src/account_provider/mod.rs | 29 +++++++++++-------- ethstore/src/dir/disk.rs | 5 ++++ ethstore/src/dir/geth.rs | 4 +++ ethstore/src/dir/mod.rs | 1 + ethstore/src/dir/parity.rs | 4 +++ ethstore/src/ethstore.rs | 42 ++++++++++++++++++---------- ethstore/tests/util/transient_dir.rs | 4 +++ 7 files changed, 63 insertions(+), 26 deletions(-) diff --git a/ethcore/src/account_provider/mod.rs b/ethcore/src/account_provider/mod.rs index 7e0005c7a..bc7ffdfed 100644 --- a/ethcore/src/account_provider/mod.rs +++ b/ethcore/src/account_provider/mod.rs @@ -23,7 +23,7 @@ use self::stores::{AddressBook, DappsSettingsStore}; use std::fmt; use std::collections::HashMap; use std::time::{Instant, Duration}; -use util::{Mutex, RwLock, Itertools}; +use util::{RwLock, Itertools}; use ethstore::{SimpleSecretStore, SecretStore, Error as SSError, SafeAccount, EthStore, EthMultiStore, random_string}; use ethstore::dir::{KeyDirectory}; use ethstore::ethkey::{Address, Message, Public, Secret, Random, Generator}; @@ -83,7 +83,7 @@ impl KeyDirectory for NullDir { Ok(self.accounts.read().values().cloned().flatten().collect()) } - fn insert(&self, account: SafeAccount) -> Result { + fn update(&self, account: SafeAccount) -> Result { let mut lock = self.accounts.write(); let mut accounts = lock.entry(account.address.clone()).or_insert_with(Vec::new); // If the filename is the same we just need to replace the entry @@ -92,6 +92,13 @@ impl KeyDirectory for NullDir { Ok(account) } + fn insert(&self, account: SafeAccount) -> Result { + let mut lock = self.accounts.write(); + let mut accounts = lock.entry(account.address.clone()).or_insert_with(Vec::new); + accounts.push(account.clone()); + Ok(account) + } + fn remove(&self, account: &SafeAccount) -> Result<(), SSError> { let mut accounts = self.accounts.write(); let is_empty = if let Some(mut accounts) = accounts.get_mut(&account.address) { @@ -121,8 +128,8 @@ type AccountToken = String; /// Account management. /// Responsible for unlocking accounts. pub struct AccountProvider { - address_book: Mutex, - unlocked: Mutex>, + unlocked: RwLock>, + address_book: RwLock, dapps_settings: RwLock, /// Accounts on disk sstore: Box, @@ -134,7 +141,7 @@ impl AccountProvider { /// Creates new account provider. pub fn new(sstore: Box) -> Self { AccountProvider { - unlocked: Mutex::new(HashMap::new()), + unlocked: RwLock::new(HashMap::new()), address_book: RwLock::new(AddressBook::new(sstore.local_path().into())), dapps_settings: RwLock::new(DappsSettingsStore::new(sstore.local_path().into())), sstore: sstore, @@ -145,8 +152,8 @@ impl AccountProvider { /// Creates not disk backed provider. pub fn transient_provider() -> Self { AccountProvider { - address_book: Mutex::new(AddressBook::transient()), - unlocked: Mutex::new(HashMap::new()), + unlocked: RwLock::new(HashMap::new()), + address_book: RwLock::new(AddressBook::transient()), dapps_settings: RwLock::new(DappsSettingsStore::transient()), sstore: Box::new(EthStore::open(Box::new(NullDir::default())).expect("NullDir load always succeeds; qed")), transient_sstore: transient_sstore(), @@ -278,7 +285,7 @@ impl AccountProvider { let _ = try!(self.sstore.sign(&account, &password, &Default::default())); // check if account is already unlocked pernamently, if it is, do nothing - let mut unlocked = self.unlocked.lock(); + let mut unlocked = self.unlocked.write(); if let Some(data) = unlocked.get(&account) { if let Unlock::Perm = data.unlock { return Ok(()) @@ -295,7 +302,7 @@ impl AccountProvider { } fn password(&self, account: &Address) -> Result { - let mut unlocked = self.unlocked.lock(); + let mut unlocked = self.unlocked.write(); let data = try!(unlocked.get(account).ok_or(Error::NotUnlocked)).clone(); if let Unlock::Temp = data.unlock { unlocked.remove(account).expect("data exists: so key must exist: qed"); @@ -326,7 +333,7 @@ impl AccountProvider { /// Checks if given account is unlocked pub fn is_unlocked(&self, account: Address) -> bool { - let unlocked = self.unlocked.lock(); + let unlocked = self.unlocked.read(); unlocked.get(&account).is_some() } @@ -434,7 +441,7 @@ mod tests { assert!(ap.unlock_account_timed(kp.address(), "test1".into(), 60000).is_err()); assert!(ap.unlock_account_timed(kp.address(), "test".into(), 60000).is_ok()); assert!(ap.sign(kp.address(), None, Default::default()).is_ok()); - ap.unlocked.lock().get_mut(&kp.address()).unwrap().unlock = Unlock::Timed(Instant::now()); + ap.unlocked.write().get_mut(&kp.address()).unwrap().unlock = Unlock::Timed(Instant::now()); assert!(ap.sign(kp.address(), None, Default::default()).is_err()); } diff --git a/ethstore/src/dir/disk.rs b/ethstore/src/dir/disk.rs index 22093171e..8c1aeb48f 100644 --- a/ethstore/src/dir/disk.rs +++ b/ethstore/src/dir/disk.rs @@ -105,6 +105,11 @@ impl KeyDirectory for DiskDirectory { Ok(accounts) } + fn update(&self, account: SafeAccount) -> Result { + // Disk store handles updates correctly iff filename is the same + self.insert(account) + } + fn insert(&self, account: SafeAccount) -> Result { // transform account into key file let keyfile: json::KeyFile = account.clone().into(); diff --git a/ethstore/src/dir/geth.rs b/ethstore/src/dir/geth.rs index a5367f98d..60537f045 100644 --- a/ethstore/src/dir/geth.rs +++ b/ethstore/src/dir/geth.rs @@ -88,6 +88,10 @@ impl KeyDirectory for GethDirectory { self.dir.insert(account) } + fn update(&self, account: SafeAccount) -> Result { + self.dir.update(account) + } + fn remove(&self, account: &SafeAccount) -> Result<(), Error> { self.dir.remove(account) } diff --git a/ethstore/src/dir/mod.rs b/ethstore/src/dir/mod.rs index 0da4d71fb..e2c6c1117 100644 --- a/ethstore/src/dir/mod.rs +++ b/ethstore/src/dir/mod.rs @@ -29,6 +29,7 @@ pub enum DirectoryType { pub trait KeyDirectory: Send + Sync { fn load(&self) -> Result, Error>; fn insert(&self, account: SafeAccount) -> Result; + fn update(&self, account: SafeAccount) -> Result; fn remove(&self, account: &SafeAccount) -> Result<(), Error>; fn path(&self) -> Option<&PathBuf> { None } } diff --git a/ethstore/src/dir/parity.rs b/ethstore/src/dir/parity.rs index c5d0057d8..cc3bb5cd3 100644 --- a/ethstore/src/dir/parity.rs +++ b/ethstore/src/dir/parity.rs @@ -67,6 +67,10 @@ impl KeyDirectory for ParityDirectory { self.dir.insert(account) } + fn update(&self, account: SafeAccount) -> Result { + self.dir.update(account) + } + fn remove(&self, account: &SafeAccount) -> Result<(), Error> { self.dir.remove(account) } diff --git a/ethstore/src/ethstore.rs b/ethstore/src/ethstore.rs index 8cbce0f1c..158402a60 100644 --- a/ethstore/src/ethstore.rs +++ b/ethstore/src/ethstore.rs @@ -92,7 +92,7 @@ impl SecretStore for EthStore { let secret = try!(safe_account.crypto.secret(password).map_err(|_| Error::InvalidPassword)); safe_account.address = try!(KeyPair::from_secret(secret)).address(); let address = safe_account.address.clone(); - try!(self.store.save(safe_account)); + try!(self.store.import(safe_account)); Ok(address) } @@ -129,19 +129,21 @@ impl SecretStore for EthStore { } fn set_name(&self, address: &Address, name: String) -> Result<(), Error> { - let mut account = try!(self.get(address)); + let old = try!(self.get(address)); + let mut account = old.clone(); account.name = name; // save to file - self.store.save(account) + self.store.update(old, account) } fn set_meta(&self, address: &Address, meta: String) -> Result<(), Error> { - let mut account = try!(self.get(address)); + let old = try!(self.get(address)); + let mut account = old.clone(); account.meta = meta; // save to file - self.store.save(account) + self.store.update(old, account) } fn local_path(&self) -> String { @@ -213,20 +215,32 @@ impl EthMultiStore { } } - fn save(&self, account: SafeAccount) -> Result<(), Error> { - //save to file + fn import(&self, account: SafeAccount) -> Result<(), Error> { + // save to file let account = try!(self.dir.insert(account)); // update cache let mut cache = self.cache.write(); let mut accounts = cache.entry(account.address.clone()).or_insert_with(Vec::new); - // TODO [ToDr] That is crappy way of overcoming set_name, set_meta, etc. - // Avoid cloning instead! - accounts.retain(|acc| acc.filename != account.filename); accounts.push(account); Ok(()) } + fn update(&self, old: SafeAccount, new: SafeAccount) -> Result<(), Error> { + // save to file + let account = try!(self.dir.update(new)); + + // update cache + let mut cache = self.cache.write(); + let mut accounts = cache.entry(account.address.clone()).or_insert_with(Vec::new); + // Remove old account + accounts.retain(|acc| acc != &old); + // And push updated to the end + accounts.push(account); + Ok(()) + + } + } impl SimpleSecretStore for EthMultiStore { @@ -235,7 +249,7 @@ impl SimpleSecretStore for EthMultiStore { let id: [u8; 16] = Random::random(); let account = SafeAccount::create(&keypair, id, password, self.iterations, "".to_owned(), "{}".to_owned()); let address = account.address.clone(); - try!(self.save(account)); + try!(self.import(account)); Ok(address) } @@ -278,11 +292,9 @@ impl SimpleSecretStore for EthMultiStore { fn change_password(&self, address: &Address, old_password: &str, new_password: &str) -> Result<(), Error> { let accounts = try!(self.get(address)); for account in accounts { - // First remove - try!(self.remove_account(&address, old_password)); - // Then insert back with new password + // Change password let new_account = try!(account.change_password(old_password, new_password, self.iterations)); - try!(self.save(new_account)); + try!(self.update(account, new_account)); } Ok(()) } diff --git a/ethstore/tests/util/transient_dir.rs b/ethstore/tests/util/transient_dir.rs index 76010182e..f01c98063 100644 --- a/ethstore/tests/util/transient_dir.rs +++ b/ethstore/tests/util/transient_dir.rs @@ -63,6 +63,10 @@ impl KeyDirectory for TransientDir { self.dir.load() } + fn update(&self, account: SafeAccount) -> Result { + self.dir.update(account) + } + fn insert(&self, account: SafeAccount) -> Result { self.dir.insert(account) } From 930183831b79e28b50d26cc700f9d0b9fe264ab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Fri, 9 Dec 2016 09:45:34 +0000 Subject: [PATCH 08/11] Adding tests for ethstore --- ethcore/src/account_provider/mod.rs | 49 +---------- ethstore/src/dir/memory.rs | 67 ++++++++++++++ ethstore/src/dir/mod.rs | 2 + ethstore/src/ethstore.rs | 130 +++++++++++++++++++++++++--- 4 files changed, 192 insertions(+), 56 deletions(-) create mode 100644 ethstore/src/dir/memory.rs diff --git a/ethcore/src/account_provider/mod.rs b/ethcore/src/account_provider/mod.rs index bc7ffdfed..81e246e8c 100644 --- a/ethcore/src/account_provider/mod.rs +++ b/ethcore/src/account_provider/mod.rs @@ -25,7 +25,7 @@ use std::collections::HashMap; use std::time::{Instant, Duration}; use util::{RwLock, Itertools}; use ethstore::{SimpleSecretStore, SecretStore, Error as SSError, SafeAccount, EthStore, EthMultiStore, random_string}; -use ethstore::dir::{KeyDirectory}; +use ethstore::dir::{KeyDirectory, MemoryDirectory}; use ethstore::ethkey::{Address, Message, Public, Secret, Random, Generator}; use ethjson::misc::AccountMeta; pub use ethstore::ethkey::Signature; @@ -73,54 +73,11 @@ impl From for Error { } } -#[derive(Default)] -struct NullDir { - accounts: RwLock>>, -} - -impl KeyDirectory for NullDir { - fn load(&self) -> Result, SSError> { - Ok(self.accounts.read().values().cloned().flatten().collect()) - } - - fn update(&self, account: SafeAccount) -> Result { - let mut lock = self.accounts.write(); - let mut accounts = lock.entry(account.address.clone()).or_insert_with(Vec::new); - // If the filename is the same we just need to replace the entry - accounts.retain(|acc| acc.filename != account.filename); - accounts.push(account.clone()); - Ok(account) - } - - fn insert(&self, account: SafeAccount) -> Result { - let mut lock = self.accounts.write(); - let mut accounts = lock.entry(account.address.clone()).or_insert_with(Vec::new); - accounts.push(account.clone()); - Ok(account) - } - - fn remove(&self, account: &SafeAccount) -> Result<(), SSError> { - let mut accounts = self.accounts.write(); - let is_empty = if let Some(mut accounts) = accounts.get_mut(&account.address) { - if let Some(position) = accounts.iter().position(|acc| acc == account) { - accounts.remove(position); - } - accounts.is_empty() - } else { - false - }; - if is_empty { - accounts.remove(&account.address); - } - Ok(()) - } -} - /// Dapp identifier pub type DappId = String; fn transient_sstore() -> EthMultiStore { - EthMultiStore::open(Box::new(NullDir::default())).expect("NullDir load always succeeds; qed") + EthMultiStore::open(Box::new(MemoryDirectory::default())).expect("MemoryDirectory load always succeeds; qed") } type AccountToken = String; @@ -155,7 +112,7 @@ impl AccountProvider { unlocked: RwLock::new(HashMap::new()), address_book: RwLock::new(AddressBook::transient()), dapps_settings: RwLock::new(DappsSettingsStore::transient()), - sstore: Box::new(EthStore::open(Box::new(NullDir::default())).expect("NullDir load always succeeds; qed")), + sstore: Box::new(EthStore::open(Box::new(MemoryDirectory::default())).expect("MemoryDirectory load always succeeds; qed")), transient_sstore: transient_sstore(), } } diff --git a/ethstore/src/dir/memory.rs b/ethstore/src/dir/memory.rs new file mode 100644 index 000000000..c4f20f0e9 --- /dev/null +++ b/ethstore/src/dir/memory.rs @@ -0,0 +1,67 @@ +// 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 . + +use std::collections::HashMap; +use parking_lot::RwLock; +use itertools::Itertools; +use ethkey::Address; + +use {SafeAccount, Error}; +use super::KeyDirectory; + +#[derive(Default)] +pub struct MemoryDirectory { + accounts: RwLock>>, +} + +impl KeyDirectory for MemoryDirectory { + fn load(&self) -> Result, Error> { + Ok(self.accounts.read().values().cloned().flatten().collect()) + } + + fn update(&self, account: SafeAccount) -> Result { + let mut lock = self.accounts.write(); + let mut accounts = lock.entry(account.address.clone()).or_insert_with(Vec::new); + // If the filename is the same we just need to replace the entry + accounts.retain(|acc| acc.filename != account.filename); + accounts.push(account.clone()); + Ok(account) + } + + fn insert(&self, account: SafeAccount) -> Result { + let mut lock = self.accounts.write(); + let mut accounts = lock.entry(account.address.clone()).or_insert_with(Vec::new); + accounts.push(account.clone()); + Ok(account) + } + + fn remove(&self, account: &SafeAccount) -> Result<(), Error> { + let mut accounts = self.accounts.write(); + let is_empty = if let Some(mut accounts) = accounts.get_mut(&account.address) { + if let Some(position) = accounts.iter().position(|acc| acc == account) { + accounts.remove(position); + } + accounts.is_empty() + } else { + false + }; + if is_empty { + accounts.remove(&account.address); + } + Ok(()) + } +} + diff --git a/ethstore/src/dir/mod.rs b/ethstore/src/dir/mod.rs index e2c6c1117..2d7b98d58 100644 --- a/ethstore/src/dir/mod.rs +++ b/ethstore/src/dir/mod.rs @@ -19,6 +19,7 @@ use {SafeAccount, Error}; mod disk; mod geth; +mod memory; mod parity; pub enum DirectoryType { @@ -36,4 +37,5 @@ pub trait KeyDirectory: Send + Sync { pub use self::disk::DiskDirectory; pub use self::geth::GethDirectory; +pub use self::memory::MemoryDirectory; pub use self::parity::ParityDirectory; diff --git a/ethstore/src/ethstore.rs b/ethstore/src/ethstore.rs index 158402a60..7540edb69 100644 --- a/ethstore/src/ethstore.rs +++ b/ethstore/src/ethstore.rs @@ -16,18 +16,16 @@ use std::collections::BTreeMap; use std::mem; -use ethkey::KeyPair; +use parking_lot::RwLock; + use crypto::KEY_ITERATIONS; use random::Random; -use ethkey::{Signature, Address, Message, Secret, Public}; +use ethkey::{Signature, Address, Message, Secret, Public, KeyPair}; use dir::KeyDirectory; use account::SafeAccount; -use {Error, SimpleSecretStore, SecretStore}; -use json; -use json::UUID; -use parking_lot::RwLock; +use json::{self, UUID}; use presale::PresaleWallet; -use import; +use {import, Error, SimpleSecretStore, SecretStore}; pub struct EthStore { store: EthMultiStore, @@ -323,8 +321,120 @@ impl SimpleSecretStore for EthMultiStore { #[cfg(test)] mod tests { - #[test] - fn should_have_some_tests() { - assert_eq!(true, false) + + use dir::MemoryDirectory; + use ethkey::{Random, Generator, KeyPair}; + use secret_store::{SimpleSecretStore, SecretStore}; + use super::{EthStore, EthMultiStore}; + + fn keypair() -> KeyPair { + Random.generate().unwrap() } + + fn store() -> EthStore { + EthStore::open(Box::new(MemoryDirectory::default())).expect("MemoryDirectory always load successfuly; qed") + } + + fn multi_store() -> EthMultiStore { + EthMultiStore::open(Box::new(MemoryDirectory::default())).expect("MemoryDirectory always load successfuly; qed") + } + + #[test] + fn should_insert_account_successfully() { + // given + let store = store(); + let keypair = keypair(); + + // when + let address = store.insert_account(keypair.secret().clone(), "test").unwrap(); + + // then + assert_eq!(address, keypair.address()); + assert!(store.get(&address).is_ok(), "Should contain account."); + assert_eq!(store.accounts().unwrap().len(), 1, "Should have one account."); + } + + #[test] + fn should_update_meta_and_name() { + // given + let store = store(); + let keypair = keypair(); + let address = store.insert_account(keypair.secret().clone(), "test").unwrap(); + assert_eq!(&store.meta(&address).unwrap(), "{}"); + assert_eq!(&store.name(&address).unwrap(), ""); + + // when + store.set_meta(&address, "meta".into()).unwrap(); + store.set_name(&address, "name".into()).unwrap(); + + // then + assert_eq!(&store.meta(&address).unwrap(), "meta"); + assert_eq!(&store.name(&address).unwrap(), "name"); + assert_eq!(store.accounts().unwrap().len(), 1); + } + + #[test] + fn should_remove_account() { + // given + let store = store(); + let keypair = keypair(); + let address = store.insert_account(keypair.secret().clone(), "test").unwrap(); + + // when + store.remove_account(&address, "test").unwrap(); + + // then + assert_eq!(store.accounts().unwrap().len(), 0, "Should remove account."); + } + + #[test] + fn should_return_true_if_password_is_correct() { + // given + let store = store(); + let keypair = keypair(); + let address = store.insert_account(keypair.secret().clone(), "test").unwrap(); + + // when + let res1 = store.test_password(&address, "x").unwrap(); + let res2 = store.test_password(&address, "test").unwrap(); + + assert!(!res1, "First password should be invalid."); + assert!(res2, "Second password should be correct."); + } + + #[test] + fn multistore_should_be_able_to_have_the_same_account_twice() { + // given + let store = multi_store(); + let keypair = keypair(); + let address = store.insert_account(keypair.secret().clone(), "test").unwrap(); + let address2 = store.insert_account(keypair.secret().clone(), "xyz").unwrap(); + assert_eq!(address, address2); + + // when + assert!(store.remove_account(&address, "test").is_ok(), "First password should work."); + assert_eq!(store.accounts().unwrap().len(), 1); + + assert!(store.remove_account(&address, "xyz").is_ok(), "Second password should work too."); + assert_eq!(store.accounts().unwrap().len(), 0); + } + + #[test] + fn should_copy_account() { + // given + let store = store(); + let multi_store = multi_store(); + let keypair = keypair(); + let address = store.insert_account(keypair.secret().clone(), "test").unwrap(); + assert_eq!(multi_store.accounts().unwrap().len(), 0); + + // when + store.copy_account(&multi_store, &address, "test", "xyz").unwrap(); + + // then + assert!(store.test_password(&address, "test").unwrap(), "First password should work for store."); + assert!(multi_store.sign(&address, "xyz", &Default::default()).is_ok(), "Second password should work for second store."); + assert_eq!(multi_store.accounts().unwrap().len(), 1); + } + } From c76b7cf8f8c2cae18e9b15d0a991758140334d43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Fri, 9 Dec 2016 10:48:46 +0000 Subject: [PATCH 09/11] Fixing tests submodule --- ethcore/res/ethereum/tests | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethcore/res/ethereum/tests b/ethcore/res/ethereum/tests index d509c7593..e8f4624b7 160000 --- a/ethcore/res/ethereum/tests +++ b/ethcore/res/ethereum/tests @@ -1 +1 @@ -Subproject commit d509c75936ec6cbba683ee1916aa0bca436bc376 +Subproject commit e8f4624b7f1a15c63674eecf577c7ab76c3b16be From 3ccdb7c143cd73c3f051b8a112a443f61032eb68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Fri, 9 Dec 2016 10:52:42 +0000 Subject: [PATCH 10/11] Fixing unused imports --- ethcore/src/account_provider/mod.rs | 7 ++++--- rpc/src/v1/types/confirmations.rs | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/ethcore/src/account_provider/mod.rs b/ethcore/src/account_provider/mod.rs index 81e246e8c..47a77bc0e 100644 --- a/ethcore/src/account_provider/mod.rs +++ b/ethcore/src/account_provider/mod.rs @@ -23,9 +23,9 @@ use self::stores::{AddressBook, DappsSettingsStore}; use std::fmt; use std::collections::HashMap; use std::time::{Instant, Duration}; -use util::{RwLock, Itertools}; -use ethstore::{SimpleSecretStore, SecretStore, Error as SSError, SafeAccount, EthStore, EthMultiStore, random_string}; -use ethstore::dir::{KeyDirectory, MemoryDirectory}; +use util::RwLock; +use ethstore::{SimpleSecretStore, SecretStore, Error as SSError, EthStore, EthMultiStore, random_string}; +use ethstore::dir::MemoryDirectory; use ethstore::ethkey::{Address, Message, Public, Secret, Random, Generator}; use ethjson::misc::AccountMeta; pub use ethstore::ethkey::Signature; @@ -418,6 +418,7 @@ mod tests { assert!(ap.sign_with_token(kp.address(), token, Default::default()).is_err(), "Second usage of the same token should fail."); } + #[test] fn should_set_dapps_addresses() { // given let ap = AccountProvider::transient_provider(); diff --git a/rpc/src/v1/types/confirmations.rs b/rpc/src/v1/types/confirmations.rs index a897af35e..5450d0a9f 100644 --- a/rpc/src/v1/types/confirmations.rs +++ b/rpc/src/v1/types/confirmations.rs @@ -192,7 +192,7 @@ impl Serialize for Either where mod tests { use std::str::FromStr; use serde_json; - use v1::types::U256; + use v1::types::{U256, H256}; use v1::helpers; use super::*; From c408861c295ce8beeb51134e154b6dc535b2f08c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Sat, 10 Dec 2016 17:36:29 +0100 Subject: [PATCH 11/11] Updating submodules --- ethcore/res/ethereum/tests | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethcore/res/ethereum/tests b/ethcore/res/ethereum/tests index d509c7593..e8f4624b7 160000 --- a/ethcore/res/ethereum/tests +++ b/ethcore/res/ethereum/tests @@ -1 +1 @@ -Subproject commit d509c75936ec6cbba683ee1916aa0bca436bc376 +Subproject commit e8f4624b7f1a15c63674eecf577c7ab76c3b16be