From 36365243c531f241b31afa95e89a0637343f1e4b Mon Sep 17 00:00:00 2001
From: Alexandru Gheorghe <49718502+alexggh@users.noreply.github.com>
Date: Fri, 3 Nov 2023 15:26:40 +0200
Subject: [PATCH] substrate: sysinfo: Expose failed hardware requirements
 (#2144)

The check_hardware functions does not give us too much information as to
what is failing, so let's return the list of failed metrics, so that callers can print
it.

This would make debugging easier, rather than try to guess which
dimension is actually failing.

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
---
 substrate/bin/node/cli/src/service.rs   | 12 +++--
 substrate/client/sysinfo/Cargo.toml     |  1 +
 substrate/client/sysinfo/src/sysinfo.rs | 66 ++++++++++++++++++++++---
 3 files changed, 69 insertions(+), 10 deletions(-)

diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs
index 5a85f4cde0ae0..153dda5c0a523 100644
--- a/substrate/bin/node/cli/src/service.rs
+++ b/substrate/bin/node/cli/src/service.rs
@@ -449,10 +449,14 @@ pub fn new_full_base(
 
 	if let Some(hwbench) = hwbench {
 		sc_sysinfo::print_hwbench(&hwbench);
-		if !SUBSTRATE_REFERENCE_HARDWARE.check_hardware(&hwbench) && role.is_authority() {
-			log::warn!(
-				"⚠️  The hardware does not meet the minimal requirements for role 'Authority'."
-			);
+		match SUBSTRATE_REFERENCE_HARDWARE.check_hardware(&hwbench) {
+			Err(err) if role.is_authority() => {
+				log::warn!(
+					"⚠️  The hardware does not meet the minimal requirements {} for role 'Authority'.",
+					err
+				);
+			},
+			_ => {},
 		}
 
 		if let Some(ref mut telemetry) = telemetry {
diff --git a/substrate/client/sysinfo/Cargo.toml b/substrate/client/sysinfo/Cargo.toml
index fdf987ed45f26..86f4d217dd9f0 100644
--- a/substrate/client/sysinfo/Cargo.toml
+++ b/substrate/client/sysinfo/Cargo.toml
@@ -19,6 +19,7 @@ libc = "0.2"
 log = "0.4.17"
 rand = "0.8.5"
 rand_pcg = "0.3.1"
+derive_more = "0.99"
 regex = "1"
 serde = { version = "1.0.188", features = ["derive"] }
 serde_json = "1.0.107"
diff --git a/substrate/client/sysinfo/src/sysinfo.rs b/substrate/client/sysinfo/src/sysinfo.rs
index 41161fc685d31..bef87a83e46f6 100644
--- a/substrate/client/sysinfo/src/sysinfo.rs
+++ b/substrate/client/sysinfo/src/sysinfo.rs
@@ -23,9 +23,11 @@ use sp_core::{sr25519, Pair};
 use sp_io::crypto::sr25519_verify;
 use sp_std::{fmt, fmt::Formatter, prelude::*};
 
+use derive_more::From;
 use rand::{seq::SliceRandom, Rng, RngCore};
 use serde::{de::Visitor, Deserialize, Deserializer, Serialize, Serializer};
 use std::{
+	fmt::Display,
 	fs::File,
 	io::{Seek, SeekFrom, Write},
 	ops::{Deref, DerefMut},
@@ -48,6 +50,37 @@ pub enum Metric {
 	DiskRndWrite,
 }
 
+/// Describes a checking failure for the hardware requirements.
+#[derive(Debug, Clone, Copy, PartialEq)]
+pub struct CheckFailure {
+	/// The metric that failed the check.
+	pub metric: Metric,
+	/// The expected minimum value.
+	pub expected: Throughput,
+	/// The measured value.
+	pub found: Throughput,
+}
+
+/// A list of metrics that failed to meet the minimum hardware requirements.
+#[derive(Debug, Clone, PartialEq, From)]
+pub struct CheckFailures(pub Vec<CheckFailure>);
+
+impl Display for CheckFailures {
+	fn fmt(&self, formatter: &mut Formatter) -> fmt::Result {
+		write!(formatter, "Failed checks: ")?;
+		for failure in &self.0 {
+			write!(
+				formatter,
+				"{}(expected: {}, found: {}), ",
+				failure.metric.name(),
+				failure.expected,
+				failure.found
+			)?
+		}
+		Ok(())
+	}
+}
+
 impl Metric {
 	/// The category of the metric.
 	pub fn category(&self) -> &'static str {
@@ -626,33 +659,54 @@ pub fn gather_hwbench(scratch_directory: Option<&Path>) -> HwBench {
 
 impl Requirements {
 	/// Whether the hardware requirements are met by the provided benchmark results.
-	pub fn check_hardware(&self, hwbench: &HwBench) -> bool {
+	pub fn check_hardware(&self, hwbench: &HwBench) -> Result<(), CheckFailures> {
+		let mut failures = Vec::new();
 		for requirement in self.0.iter() {
 			match requirement.metric {
 				Metric::Blake2256 =>
 					if requirement.minimum > hwbench.cpu_hashrate_score {
-						return false
+						failures.push(CheckFailure {
+							metric: requirement.metric,
+							expected: requirement.minimum,
+							found: hwbench.cpu_hashrate_score,
+						});
 					},
 				Metric::MemCopy =>
 					if requirement.minimum > hwbench.memory_memcpy_score {
-						return false
+						failures.push(CheckFailure {
+							metric: requirement.metric,
+							expected: requirement.minimum,
+							found: hwbench.memory_memcpy_score,
+						});
 					},
 				Metric::DiskSeqWrite =>
 					if let Some(score) = hwbench.disk_sequential_write_score {
 						if requirement.minimum > score {
-							return false
+							failures.push(CheckFailure {
+								metric: requirement.metric,
+								expected: requirement.minimum,
+								found: score,
+							});
 						}
 					},
 				Metric::DiskRndWrite =>
 					if let Some(score) = hwbench.disk_random_write_score {
 						if requirement.minimum > score {
-							return false
+							failures.push(CheckFailure {
+								metric: requirement.metric,
+								expected: requirement.minimum,
+								found: score,
+							});
 						}
 					},
 				Metric::Sr25519Verify => {},
 			}
 		}
-		true
+		if failures.is_empty() {
+			Ok(())
+		} else {
+			Err(failures.into())
+		}
 	}
 }