From aa52b04e312f16228e58eadf97c4494a6a8580e3 Mon Sep 17 00:00:00 2001 From: keorn Date: Wed, 19 Oct 2016 17:35:39 +0100 Subject: [PATCH] Unify major syncing detection (#2699) * simplify major sync detection * fix typos * fix merge * more realistic EthTester * add new synced state --- ethcore/src/snapshot/watcher.rs | 6 ++--- parity/informant.rs | 7 ++--- rpc/src/v1/impls/eth.rs | 32 +++++++++-------------- rpc/src/v1/tests/helpers/sync_provider.rs | 7 +++++ rpc/src/v1/tests/mocked/eth.rs | 26 +++++++++--------- sync/src/chain.rs | 8 +++++- 6 files changed, 44 insertions(+), 42 deletions(-) diff --git a/ethcore/src/snapshot/watcher.rs b/ethcore/src/snapshot/watcher.rs index 65f47efc8..d37be7c81 100644 --- a/ethcore/src/snapshot/watcher.rs +++ b/ethcore/src/snapshot/watcher.rs @@ -46,9 +46,7 @@ impl Oracle for StandardOracle } fn is_major_syncing(&self) -> bool { - let queue_info = self.client.queue_info(); - - (self.sync_status)() || queue_info.unverified_queue_size + queue_info.verified_queue_size > 3 + (self.sync_status)() } } @@ -200,4 +198,4 @@ mod tests { fn doesnt_fire_before_history() { harness(vec![10, 11], 10, 5, None); } -} \ No newline at end of file +} diff --git a/parity/informant.rs b/parity/informant.rs index a3749cfb0..b3cd5bfb6 100644 --- a/parity/informant.rs +++ b/parity/informant.rs @@ -95,8 +95,7 @@ impl Informant { let network_config = self.net.as_ref().map(|n| n.network_config()); let sync_status = self.sync.as_ref().map(|s| s.status()); - let importing = queue_info.unverified_queue_size + queue_info.verified_queue_size > 3 - || sync_status.map_or(false, |s| s.is_major_syncing()); + let importing = self.sync.as_ref().map_or(false, |s| s.status().is_major_syncing()); let (snapshot_sync, snapshot_current, snapshot_total) = self.snapshot.as_ref().map_or((false, 0, 0), |s| match s.status() { RestorationStatus::Ongoing { state_chunks, block_chunks, state_chunks_done, block_chunks_done } => @@ -175,9 +174,7 @@ impl Informant { impl ChainNotify for Informant { fn new_blocks(&self, imported: Vec, _invalid: Vec, _enacted: Vec, _retracted: Vec, _sealed: Vec, duration: u64) { let mut last_import = self.last_import.lock(); - let queue_info = self.client.queue_info(); - let importing = queue_info.unverified_queue_size + queue_info.verified_queue_size > 3 - || self.sync.as_ref().map_or(false, |s| s.status().is_major_syncing()); + let importing = self.sync.as_ref().map_or(false, |s| s.status().is_major_syncing()); if Instant::now() > *last_import + Duration::from_secs(1) && !importing { if let Some(block) = imported.last().and_then(|h| self.client.block(BlockID::Hash(*h))) { let view = BlockView::new(&block); diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index 0889d81fe..e48e50f13 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -24,7 +24,7 @@ use std::thread; use std::time::{Instant, Duration}; use std::sync::{Arc, Weak}; use time::get_time; -use ethsync::{SyncProvider, SyncState}; +use ethsync::{SyncProvider}; use ethcore::miner::{MinerService, ExternalMinerService}; use jsonrpc_core::*; use util::{H256, Address, FixedHash, U256, H64, Uint}; @@ -253,26 +253,18 @@ impl Eth for EthClient where fn syncing(&self) -> Result { try!(self.active()); - let status = take_weak!(self.sync).status(); - match status.state { - SyncState::Idle => Ok(SyncStatus::None), - SyncState::Waiting | SyncState::Blocks | SyncState::NewBlocks - | SyncState::SnapshotManifest | SyncState::SnapshotData | SyncState::SnapshotWaiting => { - let current_block = U256::from(take_weak!(self.client).chain_info().best_block_number); - let highest_block = U256::from(status.highest_block_number.unwrap_or(status.start_block_number)); - - if highest_block > current_block + U256::from(6) { - let info = SyncInfo { - starting_block: status.start_block_number.into(), - current_block: current_block.into(), - highest_block: highest_block.into(), - }; - Ok(SyncStatus::Info(info)) - } else { - Ok(SyncStatus::None) - } - } + if status.is_major_syncing() { + let current_block = U256::from(take_weak!(self.client).chain_info().best_block_number); + let highest_block = U256::from(status.highest_block_number.unwrap_or(status.start_block_number)); + let info = SyncInfo { + starting_block: status.start_block_number.into(), + current_block: current_block.into(), + highest_block: highest_block.into(), + }; + Ok(SyncStatus::Info(info)) + } else { + Ok(SyncStatus::None) } } diff --git a/rpc/src/v1/tests/helpers/sync_provider.rs b/rpc/src/v1/tests/helpers/sync_provider.rs index 5a227d3fb..c85fddea5 100644 --- a/rpc/src/v1/tests/helpers/sync_provider.rs +++ b/rpc/src/v1/tests/helpers/sync_provider.rs @@ -55,6 +55,13 @@ impl TestSyncProvider { }), } } + + /// Simulate importing blocks. + pub fn increase_imported_block_number(&self, count: u64) { + let mut status = self.status.write(); + let current_number = status.last_imported_block_number.unwrap_or(0); + status.last_imported_block_number = Some(current_number + count); + } } impl SyncProvider for TestSyncProvider { diff --git a/rpc/src/v1/tests/mocked/eth.rs b/rpc/src/v1/tests/mocked/eth.rs index e41ca3231..5cf951530 100644 --- a/rpc/src/v1/tests/mocked/eth.rs +++ b/rpc/src/v1/tests/mocked/eth.rs @@ -92,6 +92,11 @@ impl EthTester { hashrates: hashrates, } } + + pub fn add_blocks(&self, count: usize, with: EachBlockWith) { + self.client.add_blocks(count, with); + self.sync.increase_imported_block_number(count as u64); + } } #[test] @@ -115,24 +120,21 @@ fn rpc_eth_syncing() { let mut status = tester.sync.status.write(); status.state = SyncState::Blocks; status.highest_block_number = Some(2500); - - // "sync" to 1000 blocks. - // causes TestBlockChainClient to return 1000 for its best block number. - let mut blocks = tester.client.blocks.write(); - for i in 0..1000 { - blocks.insert(H256::from(i), Vec::new()); - } } + // "sync" to 1000 blocks. + // causes TestBlockChainClient to return 1000 for its best block number. + tester.add_blocks(1000, EachBlockWith::Nothing); + let true_res = r#"{"jsonrpc":"2.0","result":{"currentBlock":"0x3e8","highestBlock":"0x9c4","startingBlock":"0x0"},"id":1}"#; assert_eq!(tester.io.handle_request_sync(request), Some(true_res.to_owned())); + // finish "syncing" + tester.add_blocks(1500, EachBlockWith::Nothing); + { - // finish "syncing" - let mut blocks = tester.client.blocks.write(); - for i in 0..1500 { - blocks.insert(H256::from(i + 1000), Vec::new()); - } + let mut status = tester.sync.status.write(); + status.state = SyncState::Idle; } assert_eq!(tester.io.handle_request_sync(request), Some(false_res.to_owned())); diff --git a/sync/src/chain.rs b/sync/src/chain.rs index 4c857edd9..16039dcff 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -204,7 +204,13 @@ pub struct SyncStatus { impl SyncStatus { /// Indicates if initial sync is still in progress. pub fn is_major_syncing(&self) -> bool { - self.state != SyncState::Idle && self.state != SyncState::NewBlocks + let is_synced_state = match self.state { + SyncState::Idle | SyncState::NewBlocks | SyncState::Blocks => true, + _ => false, + }; + let is_current_block = self.highest_block_number.unwrap_or(self.start_block_number) < self.last_imported_block_number.unwrap_or(0) + BlockNumber::from(4u64); + // If not synced then is major syncing. + !(is_synced_state && is_current_block) } /// Indicates if snapshot download is in progress