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

@@ -46,9 +46,7 @@ impl<F> Oracle for StandardOracle<F>
 	}
 
 	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);
 	}
-}
+}

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<H256>, _invalid: Vec<H256>, _enacted: Vec<H256>, _retracted: Vec<H256>, _sealed: Vec<H256>, 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);

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<C, S: ?Sized, M, EM> Eth for EthClient<C, S, M, EM> where
 
 	fn syncing(&self) -> Result<SyncStatus, Error> {
 		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)
 		}
 	}
 

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 {

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()));

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