From 83447c201b0a338be48466115b086701a7af25c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Fri, 22 Dec 2017 04:33:49 +0100 Subject: [PATCH] Make accounts refresh time configurable. (#7345) * Configurable accounts refresh time. * Fix tests. --- ethstore/src/ethstore.rs | 67 +++++++++++++++++++++++++++++----------- parity/cli/mod.rs | 7 +++++ parity/configuration.rs | 1 + parity/params.rs | 2 ++ parity/run.rs | 10 ++++-- 5 files changed, 67 insertions(+), 20 deletions(-) diff --git a/ethstore/src/ethstore.rs b/ethstore/src/ethstore.rs index 1b8cf7ac2..07e7dd879 100755 --- a/ethstore/src/ethstore.rs +++ b/ethstore/src/ethstore.rs @@ -29,8 +29,6 @@ use presale::PresaleWallet; use json::{self, Uuid, OpaqueKeyFile}; use {import, Error, SimpleSecretStore, SecretStore, SecretVaultRef, StoreAccountRef, Derivation, OpaqueSecret}; -const REFRESH_TIME_SEC: u64 = 5; - /// Accounts store. pub struct EthStore { store: EthMultiStore, @@ -49,6 +47,16 @@ impl EthStore { }) } + /// Modify account refresh timeout - how often they are re-read from `KeyDirectory`. + /// + /// Setting this to low values (or 0) will cause new accounts to be picked up quickly, + /// although it may induce heavy disk reads and is not recommended if you manage many keys (say over 10k). + /// + /// By default refreshing is disabled, so only accounts created using this instance of `EthStore` are taken into account. + pub fn set_refresh_time(&self, time: Duration) { + self.store.set_refresh_time(time) + } + fn get(&self, account: &StoreAccountRef) -> Result { let mut accounts = self.store.get_accounts(account)?.into_iter(); accounts.next().ok_or(Error::InvalidAccount) @@ -254,6 +262,7 @@ pub struct EthMultiStore { struct Timestamp { dir_hash: Option, last_checked: Instant, + refresh_time: Duration, } impl EthMultiStore { @@ -272,16 +281,28 @@ impl EthMultiStore { timestamp: Mutex::new(Timestamp { dir_hash: None, last_checked: Instant::now(), + // by default we never refresh accounts + refresh_time: Duration::from_secs(u64::max_value()), }), }; store.reload_accounts()?; Ok(store) } + /// Modify account refresh timeout - how often they are re-read from `KeyDirectory`. + /// + /// Setting this to low values (or 0) will cause new accounts to be picked up quickly, + /// although it may induce heavy disk reads and is not recommended if you manage many keys (say over 10k). + /// + /// By default refreshing is disabled, so only accounts created using this instance of `EthStore` are taken into account. + pub fn set_refresh_time(&self, time: Duration) { + self.timestamp.lock().refresh_time = time; + } + fn reload_if_changed(&self) -> Result<(), Error> { let mut last_timestamp = self.timestamp.lock(); let now = Instant::now(); - if (now - last_timestamp.last_checked) > Duration::from_secs(REFRESH_TIME_SEC) { + if now - last_timestamp.last_checked > last_timestamp.refresh_time { let dir_hash = Some(self.dir.unique_repr()?); last_timestamp.last_checked = now; if last_timestamp.dir_hash == dir_hash { @@ -319,22 +340,23 @@ impl EthMultiStore { } fn get_accounts(&self, account: &StoreAccountRef) -> Result, Error> { - { + let from_cache = |account| { let cache = self.cache.read(); if let Some(accounts) = cache.get(account) { if !accounts.is_empty() { - return Ok(accounts.clone()) + return Some(accounts.clone()) } } - } - self.reload_if_changed()?; - let cache = self.cache.read(); - let accounts = cache.get(account).ok_or(Error::InvalidAccount)?; - if accounts.is_empty() { - Err(Error::InvalidAccount) - } else { - Ok(accounts.clone()) + None + }; + + match from_cache(account) { + Some(accounts) => Ok(accounts), + None => { + self.reload_if_changed()?; + from_cache(account).ok_or(Error::InvalidAccount) + } } } @@ -470,11 +492,20 @@ impl SimpleSecretStore for EthMultiStore { } fn account_ref(&self, address: &Address) -> Result { - use std::collections::Bound; - self.reload_if_changed()?; - let cache = self.cache.read(); - let mut r = cache.range((Bound::Included(*address), Bound::Included(*address))); - r.next().ok_or(Error::InvalidAccount).map(|(k, _)| k.clone()) + let read_from_cache = |address: &Address| { + use std::collections::Bound; + let cache = self.cache.read(); + let mut r = cache.range((Bound::Included(*address), Bound::Included(*address))); + r.next().map(|(k, _)| k.clone()) + }; + + match read_from_cache(address) { + Some(account) => Ok(account), + None => { + self.reload_if_changed()?; + read_from_cache(address).ok_or(Error::InvalidAccount) + } + } } fn accounts(&self) -> Result, Error> { diff --git a/parity/cli/mod.rs b/parity/cli/mod.rs index 12ca31888..60d391ee9 100644 --- a/parity/cli/mod.rs +++ b/parity/cli/mod.rs @@ -335,6 +335,10 @@ usage! { "--keys-iterations=[NUM]", "Specify the number of iterations to use when deriving key from the password (bigger is more secure)", + ARG arg_accounts_refresh: (u64) = 5u64, or |c: &Config| otry!(c.account).refresh_time.clone(), + "--accounts-refresh=[TIME]", + "Specify the cache time of accounts read from disk. If you manage thousands of accounts set this to 0 to disable refresh.", + ARG arg_unlock: (Option) = None, or |c: &Config| otry!(c.account).unlock.as_ref().map(|vec| vec.join(",")), "--unlock=[ACCOUNTS]", "Unlock ACCOUNTS for the duration of the execution. ACCOUNTS is a comma-delimited list of addresses. Implies --no-ui.", @@ -1009,6 +1013,7 @@ struct Account { unlock: Option>, password: Option>, keys_iterations: Option, + refresh_time: Option, disable_hardware: Option, fast_unlock: Option, } @@ -1428,6 +1433,7 @@ mod tests { arg_unlock: Some("0xdeadbeefcafe0000000000000000000000000000".into()), arg_password: vec!["~/.safe/password.file".into()], arg_keys_iterations: 10240u32, + arg_accounts_refresh: 5u64, flag_no_hardware_wallets: false, flag_fast_unlock: false, @@ -1665,6 +1671,7 @@ mod tests { unlock: Some(vec!["0x1".into(), "0x2".into(), "0x3".into()]), password: Some(vec!["passwdfile path".into()]), keys_iterations: None, + refresh_time: None, disable_hardware: None, fast_unlock: None, }), diff --git a/parity/configuration.rs b/parity/configuration.rs index b912689eb..72d667062 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -484,6 +484,7 @@ impl Configuration { fn accounts_config(&self) -> Result { let cfg = AccountsConfig { iterations: self.args.arg_keys_iterations, + refresh_time: self.args.arg_accounts_refresh, testnet: self.args.flag_testnet, password_files: self.args.arg_password.clone(), unlocked_accounts: to_addresses(&self.args.arg_unlock)?, diff --git a/parity/params.rs b/parity/params.rs index fd94954f9..4a768e07d 100644 --- a/parity/params.rs +++ b/parity/params.rs @@ -188,6 +188,7 @@ impl str::FromStr for ResealPolicy { #[derive(Debug, PartialEq)] pub struct AccountsConfig { pub iterations: u32, + pub refresh_time: u64, pub testnet: bool, pub password_files: Vec, pub unlocked_accounts: Vec
, @@ -199,6 +200,7 @@ impl Default for AccountsConfig { fn default() -> Self { AccountsConfig { iterations: 10240, + refresh_time: 5, testnet: false, password_files: Vec::new(), unlocked_accounts: Vec::new(), diff --git a/parity/run.rs b/parity/run.rs index 13c3575d9..f3a2b4f43 100644 --- a/parity/run.rs +++ b/parity/run.rs @@ -909,9 +909,15 @@ fn prepare_account_provider(spec: &SpecType, dirs: &Directories, data_dir: &str, ], }, }; + + let ethstore = EthStore::open_with_iterations(dir, cfg.iterations).map_err(|e| format!("Could not open keys directory: {}", e))?; + if cfg.refresh_time > 0 { + ethstore.set_refresh_time(::std::time::Duration::from_secs(cfg.refresh_time)); + } let account_provider = AccountProvider::new( - Box::new(EthStore::open_with_iterations(dir, cfg.iterations).map_err(|e| format!("Could not open keys directory: {}", e))?), - account_settings); + Box::new(ethstore), + account_settings, + ); for a in cfg.unlocked_accounts { // Check if the account exists