From d474864d679c25396aa64cbd1a602c24bdf51060 Mon Sep 17 00:00:00 2001
From: Arkadiy Paronyan <arkady.paronyan@gmail.com>
Date: Thu, 26 Mar 2020 09:13:09 +0100
Subject: [PATCH] Allow syncing to peers with finalized common block (#5408)

* Allow syncing to peers with finalized common block

* Added test
---
 substrate/client/network/src/protocol/sync.rs | 18 ++++++++----
 substrate/client/network/test/src/sync.rs     | 29 +++++++++++++++++++
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/substrate/client/network/src/protocol/sync.rs b/substrate/client/network/src/protocol/sync.rs
index 683f3c3139c..e8629b4fdf7 100644
--- a/substrate/client/network/src/protocol/sync.rs
+++ b/substrate/client/network/src/protocol/sync.rs
@@ -1108,10 +1108,12 @@ impl<B: BlockT> ChainSync<B> {
 		}
 		// If the announced block is the best they have and is not ahead of us, our common number
 		// is either one further ahead or it's the one they just announced, if we know about it.
-		if is_best && self.best_queued_number >= number {
-			if known {
+		if is_best {
+			if known && self.best_queued_number >= number {
 				peer.common_number = number
-			} else if header.parent_hash() == &self.best_queued_hash || known_parent {
+			} else if header.parent_hash() == &self.best_queued_hash
+				|| known_parent && self.best_queued_number >= number
+			{
 				peer.common_number = number - One::one();
 			}
 		}
@@ -1320,13 +1322,17 @@ fn peer_block_request<B: BlockT>(
 	finalized: NumberFor<B>,
 	best_num: NumberFor<B>,
 ) -> Option<(Range<NumberFor<B>>, BlockRequest<B>)> {
-	if peer.common_number < finalized {
-		return None;
-	}
 	if best_num >= peer.best_number {
 		// Will be downloaded as alternative fork instead.
 		return None;
 	}
+	if peer.common_number < finalized {
+		trace!(
+			target: "sync",
+			"Requesting pre-finalized chain from {:?}, common={}, finalized={}, peer best={}, our best={}",
+			id, finalized, peer.common_number, peer.best_number, best_num,
+		);
+	}
 	if let Some(range) = blocks.needed_blocks(
 		id.clone(),
 		MAX_BLOCKS_TO_REQUEST,
diff --git a/substrate/client/network/test/src/sync.rs b/substrate/client/network/test/src/sync.rs
index 54537472208..785b71cb79a 100644
--- a/substrate/client/network/test/src/sync.rs
+++ b/substrate/client/network/test/src/sync.rs
@@ -694,3 +694,32 @@ fn imports_stale_once() {
 	assert_eq!(net.peer(1).num_processed_blocks(), 2);
 }
 
+#[test]
+fn can_sync_to_peers_with_wrong_common_block() {
+	let _ = ::env_logger::try_init();
+	let mut net = TestNet::new(2);
+
+	net.peer(0).push_blocks(2, true);
+	net.peer(1).push_blocks(2, true);
+	let fork_hash = net.peer(0).push_blocks_at(BlockId::Number(0), 2, false);
+	net.peer(1).push_blocks_at(BlockId::Number(0), 2, false);
+	// wait for connection
+	block_on(futures::future::poll_fn::<(), _>(|cx| {
+		net.poll(cx);
+		if net.peer(0).num_peers() == 0  || net.peer(1).num_peers() == 0 {
+			Poll::Pending
+		} else {
+			Poll::Ready(())
+		}
+	}));
+
+	// both peers re-org to the same fork without notifying each other
+	net.peer(0).client().finalize_block(BlockId::Hash(fork_hash), Some(Vec::new()), true).unwrap();
+	net.peer(1).client().finalize_block(BlockId::Hash(fork_hash), Some(Vec::new()), true).unwrap();
+	let final_hash = net.peer(0).push_blocks(1, false);
+
+	net.block_until_sync();
+
+	assert!(net.peer(1).client().header(&BlockId::Hash(final_hash)).unwrap().is_some());
+}
+
-- 
GitLab