From bfe793fce28dd1e51c51676024744205a3fe5416 Mon Sep 17 00:00:00 2001 From: Arkadiy Paronyan Date: Fri, 20 Oct 2017 20:20:41 +0200 Subject: [PATCH] Ethstore optimizations (#6827) --- ethcore/src/account_provider/mod.rs | 2 +- ethcore/src/miner/miner.rs | 8 ++---- ethcore/src/snapshot/mod.rs | 3 --- ethcore/src/snapshot/service.rs | 33 ++++++++++++++++++----- ethstore/src/dir/disk.rs | 9 ++++++- ethstore/src/ethstore.rs | 39 ++++++++++++++++++--------- ethstore/src/secret_store.rs | 19 ++++++++++--- hash-fetch/src/client.rs | 4 +-- hash-fetch/src/lib.rs | 5 +++- sync/src/api.rs | 22 ++++++++++++++-- sync/src/lib.rs | 6 ++--- sync/src/tests/mod.rs | 1 + sync/src/tests/snapshot.rs | 2 +- updater/src/updater.rs | 41 +++++++++++++++++++++++++++++ 14 files changed, 153 insertions(+), 41 deletions(-) diff --git a/ethcore/src/account_provider/mod.rs b/ethcore/src/account_provider/mod.rs index 8e77a414f94..3e1d1058cc7 100755 --- a/ethcore/src/account_provider/mod.rs +++ b/ethcore/src/account_provider/mod.rs @@ -269,7 +269,7 @@ impl AccountProvider { /// Checks whether an account with a given address is present. pub fn has_account(&self, address: Address) -> Result { - Ok(self.accounts()?.iter().any(|&a| a == address)) + Ok(self.sstore.account_ref(&address).is_ok() && !self.blacklisted_accounts.contains(&address)) } /// Returns addresses of all accounts. diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index a8f18b73a53..32461927345 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -647,10 +647,6 @@ impl Miner { condition: Option, transaction_queue: &mut BanningTransactionQueue, ) -> Vec> { - let accounts = self.accounts.as_ref() - .and_then(|provider| provider.accounts().ok()) - .map(|accounts| accounts.into_iter().collect::>()); - let best_block_header = client.best_block_header().decode(); let insertion_time = client.chain_info().best_block_number; @@ -669,8 +665,8 @@ impl Miner { Err(e) }, Ok(transaction) => { - let origin = accounts.as_ref().and_then(|accounts| { - match accounts.contains(&transaction.sender()) { + let origin = self.accounts.as_ref().and_then(|accounts| { + match accounts.has_account(transaction.sender()).unwrap_or(false) { true => Some(TransactionOrigin::Local), false => None, } diff --git a/ethcore/src/snapshot/mod.rs b/ethcore/src/snapshot/mod.rs index 02adb2c1682..d89954e5425 100644 --- a/ethcore/src/snapshot/mod.rs +++ b/ethcore/src/snapshot/mod.rs @@ -69,9 +69,6 @@ mod consensus; mod error; mod watcher; -#[cfg(test)] -mod tests; - mod traits; // Try to have chunks be around 4MB (before compression) diff --git a/ethcore/src/snapshot/service.rs b/ethcore/src/snapshot/service.rs index 21e7a6752ea..ae00dcfc0b4 100644 --- a/ethcore/src/snapshot/service.rs +++ b/ethcore/src/snapshot/service.rs @@ -30,6 +30,7 @@ use blockchain::BlockChain; use client::{BlockChainClient, Client}; use engines::EthEngine; use error::Error; +use spec::Spec; use ids::BlockId; use service::ClientIoMessage; @@ -41,6 +42,7 @@ use util_error::UtilError; use bytes::Bytes; use journaldb::Algorithm; use kvdb_rocksdb::{Database, DatabaseConfig}; +use devtools::RandomTempPath; use snappy; /// Helper for removing directories in case of error. @@ -283,6 +285,24 @@ impl Service { Ok(service) } + pub fn new_test() -> Result { + let spec = Spec::new_null(); + let path = RandomTempPath::create_dir(); + let mut path = path.as_path().clone(); + + let service_params = ServiceParams { + engine: spec.engine.clone(), + genesis_block: spec.genesis_block(), + db_config: DatabaseConfig::with_columns(::db::NUM_COLUMNS), + pruning: ::journaldb::Algorithm::Archive, + channel: IoChannel::disconnected(), + snapshot_root: path.clone(), + db_restore: Arc::new(NoopDBRestore), + }; + + Service::new(service_params) + } + // get the current snapshot dir. fn snapshot_dir(&self) -> PathBuf { let mut dir = self.snapshot_root.clone(); @@ -618,6 +638,13 @@ impl Drop for Service { } } +pub struct NoopDBRestore; +impl DatabaseRestore for NoopDBRestore { + fn restore_db(&self, _new_db: &str) -> Result<(), Error> { + Ok(()) + } +} + #[cfg(test)] mod tests { use std::sync::Arc; @@ -630,12 +657,6 @@ mod tests { use snapshot::{ManifestData, RestorationStatus, SnapshotService}; use super::*; - struct NoopDBRestore; - impl DatabaseRestore for NoopDBRestore { - fn restore_db(&self, _new_db: &str) -> Result<(), Error> { - Ok(()) - } - } #[test] fn sends_async_messages() { diff --git a/ethstore/src/dir/disk.rs b/ethstore/src/dir/disk.rs index 3f56ebe4062..ebd67de2e2a 100755 --- a/ethstore/src/dir/disk.rs +++ b/ethstore/src/dir/disk.rs @@ -122,6 +122,13 @@ impl DiskDirectory where T: KeyFileManager { Ok(hasher.finish()) } + fn last_modification_date(&self) -> Result { + use std::time::{Duration, UNIX_EPOCH}; + let duration = fs::metadata(&self.path)?.modified()?.duration_since(UNIX_EPOCH).unwrap_or(Duration::default()); + let timestamp = duration.as_secs() ^ (duration.subsec_nanos() as u64); + Ok(timestamp) + } + /// all accounts found in keys directory fn files_content(&self) -> Result, Error> { // it's not done using one iterator cause @@ -226,7 +233,7 @@ impl KeyDirectory for DiskDirectory where T: KeyFileManager { } fn unique_repr(&self) -> Result { - self.files_hash() + self.last_modification_date() } } diff --git a/ethstore/src/ethstore.rs b/ethstore/src/ethstore.rs index 7588cc9fbbc..1b8cf7ac24f 100755 --- a/ethstore/src/ethstore.rs +++ b/ethstore/src/ethstore.rs @@ -18,6 +18,7 @@ use std::collections::{BTreeMap, HashMap}; use std::mem; use std::path::PathBuf; use parking_lot::{Mutex, RwLock}; +use std::time::{Instant, Duration}; use crypto::KEY_ITERATIONS; use random::Random; @@ -28,6 +29,8 @@ 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, @@ -245,7 +248,12 @@ pub struct EthMultiStore { // order lock: cache, then vaults cache: RwLock>>, vaults: Mutex>>, - dir_hash: Mutex>, + timestamp: Mutex, +} + +struct Timestamp { + dir_hash: Option, + last_checked: Instant, } impl EthMultiStore { @@ -261,20 +269,27 @@ impl EthMultiStore { vaults: Mutex::new(HashMap::new()), iterations: iterations, cache: Default::default(), - dir_hash: Default::default(), + timestamp: Mutex::new(Timestamp { + dir_hash: None, + last_checked: Instant::now(), + }), }; store.reload_accounts()?; Ok(store) } fn reload_if_changed(&self) -> Result<(), Error> { - let mut last_dir_hash = self.dir_hash.lock(); - let dir_hash = Some(self.dir.unique_repr()?); - if *last_dir_hash == dir_hash { - return Ok(()) + let mut last_timestamp = self.timestamp.lock(); + let now = Instant::now(); + if (now - last_timestamp.last_checked) > Duration::from_secs(REFRESH_TIME_SEC) { + let dir_hash = Some(self.dir.unique_repr()?); + last_timestamp.last_checked = now; + if last_timestamp.dir_hash == dir_hash { + return Ok(()) + } + self.reload_accounts()?; + last_timestamp.dir_hash = dir_hash; } - self.reload_accounts()?; - *last_dir_hash = dir_hash; Ok(()) } @@ -455,11 +470,11 @@ impl SimpleSecretStore for EthMultiStore { } fn account_ref(&self, address: &Address) -> Result { + use std::collections::Bound; self.reload_if_changed()?; - self.cache.read().keys() - .find(|r| &r.address == address) - .cloned() - .ok_or(Error::InvalidAccount) + 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()) } fn accounts(&self) -> Result, Error> { diff --git a/ethstore/src/secret_store.rs b/ethstore/src/secret_store.rs index e364245b799..c71db3fe22c 100755 --- a/ethstore/src/secret_store.rs +++ b/ethstore/src/secret_store.rs @@ -16,6 +16,7 @@ use std::hash::{Hash, Hasher}; use std::path::PathBuf; +use std::cmp::Ordering; use ethkey::{Address, Message, Signature, Secret, Public}; use Error; use json::{Uuid, OpaqueKeyFile}; @@ -32,12 +33,24 @@ pub enum SecretVaultRef { } /// Stored account reference -#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Clone, PartialEq, Eq, Ord)] pub struct StoreAccountRef { - /// Vault reference - pub vault: SecretVaultRef, /// Account address pub address: Address, + /// Vault reference + pub vault: SecretVaultRef, +} + +impl PartialOrd for StoreAccountRef { + fn partial_cmp(&self, other: &StoreAccountRef) -> Option { + Some(self.address.cmp(&other.address).then_with(|| self.vault.cmp(&other.vault))) + } +} + +impl ::std::borrow::Borrow
for StoreAccountRef { + fn borrow(&self) -> &Address { + &self.address + } } /// Simple Secret Store API diff --git a/hash-fetch/src/client.rs b/hash-fetch/src/client.rs index 6a756aa8236..e5314a7fe65 100644 --- a/hash-fetch/src/client.rs +++ b/hash-fetch/src/client.rs @@ -188,7 +188,7 @@ fn random_temp_path() -> PathBuf { } #[cfg(test)] -mod tests { +pub mod tests { use rustc_hex::FromHex; use std::sync::{Arc, mpsc}; use parking_lot::Mutex; @@ -200,7 +200,7 @@ mod tests { #[derive(Clone)] - struct FakeFetch { + pub struct FakeFetch { return_success: bool } diff --git a/hash-fetch/src/lib.rs b/hash-fetch/src/lib.rs index 858a04ea557..87716ebf8b3 100644 --- a/hash-fetch/src/lib.rs +++ b/hash-fetch/src/lib.rs @@ -40,8 +40,11 @@ extern crate parking_lot; #[cfg(test)] extern crate ethabi; -mod client; +pub mod client; pub mod urlhint; pub use client::{HashFetch, Client, Error}; + +#[cfg(test)] +pub use client::tests::FakeFetch; diff --git a/sync/src/api.rs b/sync/src/api.rs index 48c48880d01..dfcb17f92ca 100644 --- a/sync/src/api.rs +++ b/sync/src/api.rs @@ -261,6 +261,24 @@ impl EthSync { Ok(sync) } + + pub fn new_test() -> Result, NetworkError> { + use ethcore::client::TestBlockChainClient; + use test_snapshot::TestSnapshotService; + + let client = Arc::new(TestBlockChainClient::new()); + + let params = Params { + config: Default::default(), + chain: client.clone(), + snapshot_service: Arc::new(TestSnapshotService::new()), + provider: client.clone(), + network_config: NetworkConfiguration::new(), + attached_protos: Vec::new(), + }; + + Self::new(params, None) + } } impl SyncProvider for EthSync { @@ -308,7 +326,7 @@ impl SyncProvider for EthSync { } } -struct SyncProtocolHandler { +pub struct SyncProtocolHandler { /// Shared blockchain client. chain: Arc, /// Shared snapshot service. @@ -438,7 +456,7 @@ impl ChainNotify for EthSync { /// PIP event handler. /// Simply queues transactions from light client peers. -struct TxRelay(Arc); +pub struct TxRelay(Arc); impl LightHandler for TxRelay { fn on_transactions(&self, ctx: &EventContext, relay: &[::ethcore::transaction::UnverifiedTransaction]) { diff --git a/sync/src/lib.rs b/sync/src/lib.rs index 091ce5ba97b..4c9bb8a79ab 100644 --- a/sync/src/lib.rs +++ b/sync/src/lib.rs @@ -66,14 +66,14 @@ mod transactions_stats; pub mod light_sync; -#[cfg(test)] -mod tests; - mod api; pub use api::*; pub use chain::{SyncStatus, SyncState}; pub use network::{is_valid_node_url, NonReservedPeerMode, NetworkError, ConnectionFilter, ConnectionDirection}; +mod test_snapshot; +pub use test_snapshot::TestSnapshotService; + #[cfg(test)] pub(crate) type Address = bigint::hash::H160; diff --git a/sync/src/tests/mod.rs b/sync/src/tests/mod.rs index 8b9059fc135..192f0d29bd6 100644 --- a/sync/src/tests/mod.rs +++ b/sync/src/tests/mod.rs @@ -16,6 +16,7 @@ pub mod helpers; pub mod snapshot; +pub mod TestSync; mod chain; mod consensus; diff --git a/sync/src/tests/snapshot.rs b/sync/src/tests/snapshot.rs index 6ed616bae65..3d7e335d6a3 100644 --- a/sync/src/tests/snapshot.rs +++ b/sync/src/tests/snapshot.rs @@ -26,7 +26,7 @@ use ethcore::client::{EachBlockWith}; use super::helpers::*; use SyncConfig; -pub struct TestSnapshotService { +struct TestSnapshotService { manifest: Option, chunks: HashMap, diff --git a/updater/src/updater.rs b/updater/src/updater.rs index 91c9181f849..ff550249f80 100644 --- a/updater/src/updater.rs +++ b/updater/src/updater.rs @@ -394,3 +394,44 @@ impl Service for Updater { fn info(&self) -> Option { self.state.lock().latest.clone() } } + +#[cfg(test)] +pub mod tests { + use super::*; + use ethsync::EthSync; + use ethcore::client::TestBlockChainClient; + use hash_fetch::fetch::{self, Fetch}; + use hash_fetch::Client; + use parity_reactor::Remote; + use semver::Version; + + #[cfg(test)] + use hash_fetch::FakeFetch; + + #[cfg_attr(feature="dev", allow(too_many_arguments))] + fn init_updater() -> Arc { + let registrar = Arc::new(client::tests::registar()); + let fetch = client::tests::FakFetch { return_success: false }; + let client = Client::with_fetch(registrar.clone(), fetch, Remote::new_sync()); + + let sync = EthSync::new_test().unwrap(); + let weak_sync = Arc::downgrade(&sync); + let weak_client = Arc::downgrade(&Arc::new(TestBlockChainClient::new())); + let policy = UpdatePolicy::default(); + let fetch = client; + let remote = Remote::new_sync(); + + Updater::new(weak_client, weak_sync, policy, fetch, remote) + } + + #[test] + fn service_version_info() { + let upd = init_updater(); + let version_info = VersionInfo { + track: ReleaseTrack::Stable, + version: Version::parse("1.9.0").unwrap(), + hash: H160::zero(), + }; + assert_eq!(version_info, upd.clone().version_info()); + } +}