Fixed common block tracking on new block announcement (#1305)

substrate/core/network/src/sync.rs

@@ -282,8 +282,11 @@ impl<B: BlockT> ChainSync<B> {
 			self.best_queued_hash = *hash;
 		}
 		// Update common blocks
-		for (_, peer) in self.peers.iter_mut() {
-			trace!(target: "sync", "Updating peer info ours={}, theirs={}", number, peer.best_number);
+		for (n, peer) in self.peers.iter_mut() {
+			if let PeerSyncState::AncestorSearch(_) = peer.state {
+				continue;
+			}
+			trace!(target: "sync", "Updating peer {} info, ours={}, common={}, their best={}", n, number, peer.common_number, peer.best_number);
 			if peer.best_number >= number {
 				peer.common_number = number;
 				peer.common_hash = *hash;
@@ -301,22 +304,34 @@ impl<B: BlockT> ChainSync<B> {
 
 	pub(crate) fn on_block_announce(&mut self, protocol: &mut Context<B>, who: NodeIndex, hash: B::Hash, header: &B::Header) {
 		let number = *header.number();
+		if number <= As::sa(0) {
+			trace!(target: "sync", "Ignored invalid block announcement from {}: {}", who, hash);
+			return;
+		}
+		let known_parent = self.is_known(protocol, &header.parent_hash());
+		let known = self.is_known(protocol, &hash);
 		if let Some(ref mut peer) = self.peers.get_mut(&who) {
 			if number > peer.best_number {
+				// update their best block
 				peer.best_number = number;
 				peer.best_hash = hash;
 			}
-			if number <= self.best_queued_number && number > peer.common_number {
+			if let PeerSyncState::AncestorSearch(_) = peer.state {
+				return;
+			}
+			if header.parent_hash() == &self.best_queued_hash || known_parent {
+				peer.common_number = number - As::sa(1);
+			} else if known {
 				peer.common_number = number
 			}
 		} else {
 			return;
 		}
 
-		if !self.is_known_or_already_downloading(protocol, &hash) {
+		if !(known || self.is_already_downloading(&hash)) {
 			let stale = number <= self.best_queued_number;
 			if stale {
-				if !self.is_known_or_already_downloading(protocol, header.parent_hash()) {
+				if !(known_parent || self.is_already_downloading(header.parent_hash())) {
 					trace!(target: "sync", "Ignoring unknown stale block announce from {}: {} {:?}", who, hash, header);
 				} else {
 					trace!(target: "sync", "Considering new stale block announced from {}: {} {:?}", who, hash, header);
@@ -331,9 +346,12 @@ impl<B: BlockT> ChainSync<B> {
 		}
 	}
 
-	fn is_known_or_already_downloading(&self, protocol: &mut Context<B>, hash: &B::Hash) -> bool {
+	fn is_already_downloading(&self, hash: &B::Hash) -> bool {
 		self.peers.iter().any(|(_, p)| p.state == PeerSyncState::DownloadingStale(*hash))
-			|| block_status(&*protocol.client(), &*self.import_queue, *hash).ok().map_or(false, |s| s != BlockStatus::Unknown)
+	}
+
+	fn is_known(&self, protocol: &mut Context<B>, hash: &B::Hash) -> bool {
+		block_status(&*protocol.client(), &*self.import_queue, *hash).ok().map_or(false, |s| s != BlockStatus::Unknown)
 	}
 
 	pub(crate) fn peer_disconnected(&mut self, protocol: &mut Context<B>, who: NodeIndex) {