sc-network-sync: Improve error reporting (#14025)

Before this wasn't printing a warning for `VerificationFailed` when there wasn't a peer assigned to
the error message. However, if there happens an error on importing the state there wouldn't be any
error. This pr improves the situation to also print an error in this case. Besides that there are
some other cleanups.

client/network/sync/src/lib.rs

@@ -2708,9 +2708,7 @@ where
 				break
 			}
 
-			if result.is_err() {
-				has_error = true;
-			}
+			has_error |= result.is_err();
 
 			match result {
 				Ok(BlockImportStatus::ImportedKnown(number, who)) =>
@@ -2721,19 +2719,15 @@ where
 					if aux.clear_justification_requests {
 						trace!(
 							target: "sync",
-							"Block imported clears all pending justification requests {}: {:?}",
-							number,
-							hash,
+							"Block imported clears all pending justification requests {number}: {hash:?}",
 						);
 						self.clear_justification_requests();
 					}
 
 					if aux.needs_justification {
 						trace!(
 							target: "sync",
-							"Block imported but requires justification {}: {:?}",
-							number,
-							hash,
+							"Block imported but requires justification {number}: {hash:?}",
 						);
 						self.request_justification(&hash, number);
 					}
@@ -2793,36 +2787,37 @@ where
 						output.push(Err(BadPeer(peer, rep::INCOMPLETE_HEADER)));
 						output.extend(self.restart());
 					},
-				Err(BlockImportError::VerificationFailed(who, e)) =>
+				Err(BlockImportError::VerificationFailed(who, e)) => {
+					let extra_message =
+						who.map_or_else(|| "".into(), |peer| format!(" received from ({peer})"));
+
+					warn!(
+						target: "sync",
+						"💔 Verification failed for block {hash:?}{extra_message}: {e:?}",
+					);
+
 					if let Some(peer) = who {
-						warn!(
-							target: "sync",
-							"💔 Verification failed for block {:?} received from peer: {}, {:?}",
-							hash,
-							peer,
-							e,
-						);
 						output.push(Err(BadPeer(peer, rep::VERIFICATION_FAIL)));
-						output.extend(self.restart());
-					},
+					}
+
+					output.extend(self.restart());
+				},
 				Err(BlockImportError::BadBlock(who)) =>
 					if let Some(peer) = who {
 						warn!(
 							target: "sync",
-							"💔 Block {:?} received from peer {} has been blacklisted",
-							hash,
-							peer,
+							"💔 Block {hash:?} received from peer {peer} has been blacklisted",
 						);
 						output.push(Err(BadPeer(peer, rep::BAD_BLOCK)));
 					},
 				Err(BlockImportError::MissingState) => {
 					// This may happen if the chain we were requesting upon has been discarded
 					// in the meantime because other chain has been finalized.
 					// Don't mark it as bad as it still may be synced if explicitly requested.
-					trace!(target: "sync", "Obsolete block {:?}", hash);
+					trace!(target: "sync", "Obsolete block {hash:?}");
 				},
 				e @ Err(BlockImportError::UnknownParent) | e @ Err(BlockImportError::Other(_)) => {
-					warn!(target: "sync", "💔 Error importing block {:?}: {}", hash, e.unwrap_err());
+					warn!(target: "sync", "💔 Error importing block {hash:?}: {}", e.unwrap_err());
 					self.state_sync = None;
 					self.warp_sync = None;
 					output.extend(self.restart());