Improve error message on can_author_with failure (#4262)

Author: bkchr

client/consensus/pow/src/lib.rs

@@ -467,11 +467,12 @@ fn mine_loop<B: BlockT<Hash=H256>, C, Algorithm, E, SO, S, CAW>(
 			},
 		};
 
-		if can_author_with.can_author_with(&BlockId::Hash(best_hash)) {
-			debug!(
+		if let Err(err) = can_author_with.can_author_with(&BlockId::Hash(best_hash)) {
+			warn!(
 				target: "pow",
-				"Skipping proposal `can_author_with` returned `false`. \
-				Probably a node update is required!"
+				"Skipping proposal `can_author_with` returned: {} \
+				Probably a node update is required!",
+				err,
 			);
 			std::thread::sleep(std::time::Duration::from_secs(1));
 			continue 'outer

client/consensus/slots/src/lib.rs

@@ -348,22 +348,23 @@ where
 				}
 			};
 
-			if can_author_with.can_author_with(&BlockId::Hash(chain_head.hash())) {
+			if let Err(err) = can_author_with.can_author_with(&BlockId::Hash(chain_head.hash())) {
+				warn!(
+					target: "slots",
+					"Unable to author block in slot {},. `can_author_with` returned: {} \
+					Probably a node update is required!",
+					slot_num,
+					err,
+				);
+				Either::Right(future::ready(Ok(())))
+			} else {
 				Either::Left(
 					worker.on_slot(chain_head, slot_info)
 						.map_err(|e| {
 							warn!(target: "slots", "Encountered consensus error: {:?}", e);
 						})
 						.or_else(|_| future::ready(Ok(())))
 				)
-			} else {
-				warn!(
-					target: "slots",
-					"Unable to author block in slot {}. `can_author_with` returned `false`. \
-					Probably a node update is required!",
-					slot_num,
-				);
-				Either::Right(future::ready(Ok(())))
 			}
 		}).then(|res| {
 			if let Err(err) = res {

primitives/consensus/common/src/lib.rs

@@ -137,7 +137,12 @@ impl<T> SyncOracle for Arc<T> where T: ?Sized, for<'r> &'r T: SyncOracle {
 /// at the given block.
 pub trait CanAuthorWith<Block: BlockT> {
 	/// See trait docs for more information.
-	fn can_author_with(&self, at: &BlockId<Block>) -> bool;
+	///
+	/// # Return
+	///
+	/// - Returns `Ok(())` when authoring is supported.
+	/// - Returns `Err(_)` when authoring is not supported.
+	fn can_author_with(&self, at: &BlockId<Block>) -> Result<(), String>;
 }
 
 /// Checks if the node can author blocks by using
@@ -154,18 +159,15 @@ impl<T> CanAuthorWithNativeVersion<T> {
 impl<T: runtime_version::GetRuntimeVersion<Block>, Block: BlockT> CanAuthorWith<Block>
 	for CanAuthorWithNativeVersion<T>
 {
-	fn can_author_with(&self, at: &BlockId<Block>) -> bool {
+	fn can_author_with(&self, at: &BlockId<Block>) -> Result<(), String> {
 		match self.0.runtime_version(at) {
 			Ok(version) => self.0.native_version().can_author_with(&version),
 			Err(e) => {
-				error!(
-					target: "CanAuthorWithNativeVersion",
+				Err(format!(
 					"Failed to get runtime version at `{}` and will disable authoring. Error: {}",
 					at,
 					e,
-				);
-
-				false
+				))
 			}
 		}
 	}
@@ -175,7 +177,7 @@ impl<T: runtime_version::GetRuntimeVersion<Block>, Block: BlockT> CanAuthorWith<
 pub struct AlwaysCanAuthor;
 
 impl<Block: BlockT> CanAuthorWith<Block> for AlwaysCanAuthor {
-	fn can_author_with(&self, _: &BlockId<Block>) -> bool {
-		true
+	fn can_author_with(&self, _: &BlockId<Block>) -> Result<(), String> {
+		Ok(())
 	}
 }

primitives/sr-version/src/lib.rs

@@ -161,10 +161,30 @@ pub struct NativeVersion {
 #[cfg(feature = "std")]
 impl NativeVersion {
 	/// Check if this version matches other version for authoring blocks.
-	pub fn can_author_with(&self, other: &RuntimeVersion) -> bool {
-		self.runtime_version.spec_name == other.spec_name &&
-			(self.runtime_version.authoring_version == other.authoring_version ||
-			self.can_author_with.contains(&other.authoring_version))
+	///
+	/// # Return
+	///
+	/// - Returns `Ok(())` when authoring is supported.
+	/// - Returns `Err(_)` with a detailed error when authoring is not supported.
+	pub fn can_author_with(&self, other: &RuntimeVersion) -> Result<(), String> {
+		if self.runtime_version.spec_name != other.spec_name {
+			Err(format!(
+				"`spec_name` does not match `{}` vs `{}`",
+				self.runtime_version.spec_name,
+				other.spec_name,
+			))
+		} else if (self.runtime_version.authoring_version != other.authoring_version
+			&& !self.can_author_with.contains(&other.authoring_version))
+		{
+			Err(format!(
+				"`authoring_version` does not match `{version}` vs `{other_version}` and \
+				`can_author_with` not contains `{other_version}`",
+				version = self.runtime_version.authoring_version,
+				other_version = other.authoring_version,
+			))
+		} else {
+			Ok(())
+		}
 	}
 }