From 9554f93e1d8fd09b933b20a6c04d90c85d552de2 Mon Sep 17 00:00:00 2001
From: Ignotus Peverell <igno.peverell@protonmail.com>
Date: Wed, 22 Aug 2018 19:51:25 -0400
Subject: [PATCH] Run header sync only when stalled or complete (#1393)

Header sync used to run when complete (right count of blocks
received) or every 10 sec regardless. Sometimes 10 secs is not
enough, especially when a big batch of blocks bodies is arriving
at the same time.

This tweaks the time-based heuristic just a bit to only ask for
headers when stalled, meaning that the header head has not
*progressed* for 10 seconds.

* Fix bad header sync timeout logic

* Improve stability of fast sync test
Adds regular pings so that the 2 test servers know each others'
heights (necessary to find most work peer). Stop mining on the
first server once the 2nd kicks in. Avoids chasing a fast moving
target on Cuckoo10.
---
 servers/src/grin/server.rs | 13 +++++++++++++
 servers/src/grin/sync.rs   | 26 +++++++++++++++++---------
 servers/tests/simulnet.rs  |  7 +++++--
 3 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/servers/src/grin/server.rs b/servers/src/grin/server.rs
index 83a06b0fe..c0a24b171 100644
--- a/servers/src/grin/server.rs
+++ b/servers/src/grin/server.rs
@@ -264,6 +264,14 @@ impl Server {
 		Ok(())
 	}
 
+	/// Ping all peers, mostly useful for tests to have connected peers share
+	/// their heights
+	pub fn ping_peers(&self)-> Result<(), Error> {
+		let head = self.chain.head()?;
+		self.p2p.peers.check_all(head.total_difficulty, head.height);
+		Ok(())
+	}
+
 	/// Number of peers
 	pub fn peer_count(&self) -> u32 {
 		self.p2p.peers.peer_count()
@@ -426,4 +434,9 @@ impl Server {
 		self.p2p.stop();
 		self.stop.store(true, Ordering::Relaxed);
 	}
+
+	/// Stops the test miner without stopping the p2p layer
+	pub fn stop_test_miner(&self) {
+		self.stop.store(true, Ordering::Relaxed);
+	}
 }
diff --git a/servers/src/grin/sync.rs b/servers/src/grin/sync.rs
index 101dca5bc..4f5302fdb 100644
--- a/servers/src/grin/sync.rs
+++ b/servers/src/grin/sync.rs
@@ -439,7 +439,7 @@ fn get_locator_heights(height: u64) -> Vec<u64> {
 // Utility struct to group what information the main sync loop has to track
 struct SyncInfo {
 	prev_body_sync: (DateTime<Utc>, u64),
-	prev_header_sync: (DateTime<Utc>, u64),
+	prev_header_sync: (DateTime<Utc>, u64, u64),
 	prev_fast_sync: Option<DateTime<Utc>>,
 	highest_height: u64,
 }
@@ -449,7 +449,7 @@ impl SyncInfo {
 		let now = Utc::now();
 		SyncInfo {
 			prev_body_sync: (now.clone(), 0),
-			prev_header_sync: (now.clone(), 0),
+			prev_header_sync: (now.clone(), 0, 0),
 			prev_fast_sync: None,
 			highest_height: 0,
 		}
@@ -457,15 +457,23 @@ impl SyncInfo {
 
 	fn header_sync_due(&mut self, header_head: &chain::Tip) -> bool {
 		let now = Utc::now();
-		let (prev_ts, prev_height) = self.prev_header_sync;
+		let (timeout, latest_height, prev_height) = self.prev_header_sync;
+		
+		// received all necessary headers, can ask for more
+		let all_headers_received = header_head.height >= prev_height + (p2p::MAX_BLOCK_HEADERS as u64) - 4;
+		// no headers processed and we're past timeout, need to ask for more
+		let stalling = header_head.height == latest_height && now > timeout;
 
-		if header_head.height >= prev_height + (p2p::MAX_BLOCK_HEADERS as u64) - 4
-			|| now - prev_ts > Duration::seconds(10)
-		{
-			self.prev_header_sync = (now, header_head.height);
-			return true;
+		if all_headers_received || stalling {
+			self.prev_header_sync = (now + Duration::seconds(10), header_head.height, header_head.height);
+			true
+		} else {
+			// resetting the timeout as long as we progress
+			if header_head.height > latest_height {
+				self.prev_header_sync = (now + Duration::seconds(2), header_head.height, prev_height);
+			}
+			false
 		}
-		false
 	}
 
 	fn body_sync_due(&mut self, head: &chain::Tip) -> bool {
diff --git a/servers/tests/simulnet.rs b/servers/tests/simulnet.rs
index 59b305975..a62e8d038 100644
--- a/servers/tests/simulnet.rs
+++ b/servers/tests/simulnet.rs
@@ -267,7 +267,7 @@ fn simulate_fast_sync() {
 	let s1 = servers::Server::new(framework::config(2000, "grin-fast", 2000)).unwrap();
 	s1.start_test_miner(None);
 
-	while s1.head().height < 21 {
+	while s1.head().height < 20 {
 		thread::sleep(time::Duration::from_millis(1_000));
 	}
 
@@ -275,9 +275,12 @@ fn simulate_fast_sync() {
 	conf.archive_mode = Some(false);
 
 	let s2 = servers::Server::new(conf).unwrap();
-	while s2.head().height < 21 {
+
+	while s2.header_head().height < 1 {
+		s2.ping_peers();
 		thread::sleep(time::Duration::from_millis(1_000));
 	}
+	s1.stop_test_miner();
 
 	// Get the current header from s1.
 	let s1_header = s1.chain.head_header().unwrap();