Return error from the runtime instead of printing it

Cargo.lock

@@ -1118,6 +1118,17 @@ dependencies = [
  "winapi 0.3.9",
 ]
 
+[[package]]
+name = "displaydoc"
+version = "0.1.7"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "adc2ab4d5a16117f9029e9a6b5e4e79f4c67f6519bc134210d4d4a04ba31f41b"
+dependencies = [
+ "proc-macro2 1.0.18",
+ "quote 1.0.7",
+ "syn 1.0.33",
+]
+
 [[package]]
 name = "dlmalloc"
 version = "0.1.4"
@@ -5219,6 +5230,7 @@ name = "polkadot-primitives"
 version = "0.8.26"
 dependencies = [
  "bitvec",
+ "displaydoc",
  "frame-system",
  "parity-scale-codec",
  "polkadot-core-primitives",

node/core/candidate-validation/src/lib.rs

@@ -348,8 +348,16 @@ async fn spawn_validate_from_chain_state(
 		)
 		.await?
 		{
-			Ok(true) => {}
-			Ok(false) => {
+			Ok(Ok(())) => {}
+			Ok(Err(e)) => {
+				log::debug!(
+					target: LOG_TARGET,
+					"Parachain ({}) PoV ({}) validation outputs checking failed: {}",
+					descriptor.para_id,
+					descriptor.pov_hash,
+					e,
+				);
+
 				return Ok(Ok(ValidationResult::Invalid(
 					InvalidCandidate::InvalidOutputs,
 				)));

node/subsystem/src/messages.rs

@@ -412,7 +412,7 @@ pub enum RuntimeApiRequest {
 	CheckValidationOutputs(
 		ParaId,
 		polkadot_primitives::v1::ValidationOutputs,
-		RuntimeApiSender<bool>,
+		RuntimeApiSender<Result<(), polkadot_primitives::v1::CheckValidationOutputsError>>,
 	),
 	/// Get the session index that a child of the block will have.
 	SessionIndexForChild(RuntimeApiSender<SessionIndex>),

primitives/Cargo.toml

@@ -23,6 +23,7 @@ polkadot-core-primitives = { path = "../core-primitives", default-features = fal
 trie = { package = "sp-trie", git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
 bitvec = { version = "0.17.4", default-features = false, features = ["alloc"] }
 frame-system = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
+displaydoc = { version = "0.1.7", optional = true }
 
 [dev-dependencies]
 sp-serializer = { git = "https://github.com/paritytech/substrate", branch = "master" }
@@ -49,4 +50,5 @@ std = [
 	"polkadot-core-primitives/std",
 	"bitvec/std",
 	"frame-system/std",
+	"displaydoc",
 ]

primitives/src/v1.rs

@@ -655,6 +655,22 @@ pub enum CandidateEvent<H = Hash> {
 	CandidateTimedOut(CandidateReceipt<H>, HeadData),
 }
 
+/// Errors that can happen when checking the validation outputs of a parachain candidate.
+#[derive(Clone, Encode, Decode, Copy)]
+#[cfg_attr(feature = "std", derive(PartialEq, Debug, displaydoc::Display))]
+pub enum CheckValidationOutputsError {
+	/// The head data is too large.
+	HeadDataTooLarge,
+	/// The parachain tried to upgrade its code to early.
+	PrematureCodeUpgrade,
+	/// The new parachain code is too large.
+	NewCodeTooLarge,
+	/// The parachain didn't handled the downward messages correctly.
+	IncorrectDownwardMessageHandling,
+	/// The parachain returned invalid upward messages.
+	InvalidUpwardMessages,
+}
+
 sp_api::decl_runtime_apis! {
 	/// The API for querying the state of parachains on-chain.
 	pub trait ParachainHost<H: Decode = Hash, N: Encode + Decode = BlockNumber> {
@@ -687,7 +703,7 @@ sp_api::decl_runtime_apis! {
 			-> Option<PersistedValidationData<N>>;
 
 		/// Checks if the given validation outputs pass the acceptance criteria.
-		fn check_validation_outputs(para_id: Id, outputs: ValidationOutputs) -> bool;
+		fn check_validation_outputs(para_id: Id, outputs: ValidationOutputs) -> Result<(), CheckValidationOutputsError>;
 
 		/// Returns the session index expected at a child of the block.
 		///

runtime/kusama/src/lib.rs

@@ -1100,11 +1100,12 @@ sp_api::impl_runtime_apis! {
 			-> Option<PersistedValidationData<BlockNumber>> {
 			None
 		}
+
 		fn check_validation_outputs(
 			_: Id,
 			_: primitives::v1::ValidationOutputs,
-		) -> bool {
-			false
+		) -> Result<(), primitives::v1::CheckValidationOutputsError> {
+			Err(primitives::v1::CheckValidationOutputsError::HeadDataTooLarge)
 		}
 
 		fn session_index_for_child() -> SessionIndex {

runtime/parachains/src/inclusion.rs

@@ -25,7 +25,7 @@ use primitives::v1::{
 	ValidatorId, CandidateCommitments, CandidateDescriptor, ValidatorIndex, Id as ParaId,
 	AvailabilityBitfield as AvailabilityBitfield, SignedAvailabilityBitfields, SigningContext,
 	BackedCandidate, CoreIndex, GroupIndex, CommittedCandidateReceipt,
-	CandidateReceipt, HeadData,
+	CandidateReceipt, HeadData, CheckValidationOutputsError,
 };
 use frame_support::{
 	decl_storage, decl_module, decl_error, decl_event, ensure, debug,
@@ -160,6 +160,18 @@ decl_error! {
 	}
 }
 
+impl<T: Trait> From<CheckValidationOutputsError> for Error<T> {
+	fn from(err: CheckValidationOutputsError) -> Self {
+		match err {
+			CheckValidationOutputsError::HeadDataTooLarge => Self::HeadDataTooLarge,
+			CheckValidationOutputsError::PrematureCodeUpgrade => Self::PrematureCodeUpgrade,
+			CheckValidationOutputsError::NewCodeTooLarge => Self::NewCodeTooLarge,
+			CheckValidationOutputsError::IncorrectDownwardMessageHandling => Self::IncorrectDownwardMessageHandling,
+			CheckValidationOutputsError::InvalidUpwardMessages => Self::InvalidUpwardMessages,
+		}
+	}
+}
+
 decl_event! {
 	pub enum Event<T> where <T as frame_system::Trait>::Hash {
 		/// A candidate was backed. [candidate, head_data]
@@ -415,7 +427,7 @@ impl<T: Trait> Module<T> {
 					&candidate.candidate.commitments.new_validation_code,
 					candidate.candidate.commitments.processed_downward_messages,
 					&candidate.candidate.commitments.upward_messages,
-				)?;
+				).map_err(|e| Error::<T>::from(e))?;
 
 				for (i, assignment) in scheduled[skip..].iter().enumerate() {
 					check_assignment_in_order(assignment)?;
@@ -541,7 +553,7 @@ impl<T: Trait> Module<T> {
 	pub(crate) fn check_validation_outputs(
 		para_id: ParaId,
 		validation_outputs: primitives::v1::ValidationOutputs,
-	) -> Result<(), DispatchError> {
+	) -> Result<(), CheckValidationOutputsError> {
 		CandidateCheckContext::<T>::new().check_validation_outputs(
 			para_id,
 			&validation_outputs.head_data,
@@ -702,10 +714,10 @@ impl<T: Trait> CandidateCheckContext<T> {
 		new_validation_code: &Option<primitives::v1::ValidationCode>,
 		processed_downward_messages: u32,
 		upward_messages: &[primitives::v1::UpwardMessage],
-	) -> Result<(), DispatchError> {
+	) -> Result<(), CheckValidationOutputsError> {
 		ensure!(
 			head_data.0.len() <= self.config.max_head_data_size as _,
-			Error::<T>::HeadDataTooLarge
+			CheckValidationOutputsError::HeadDataTooLarge,
 		);
 
 		// if any, the code upgrade attempt is allowed.
@@ -716,10 +728,10 @@ impl<T: Trait> CandidateCheckContext<T> {
 						&& self.relay_parent_number.saturating_sub(last)
 							>= self.config.validation_upgrade_frequency
 				});
-			ensure!(valid_upgrade_attempt, Error::<T>::PrematureCodeUpgrade);
+			ensure!(valid_upgrade_attempt, CheckValidationOutputsError::PrematureCodeUpgrade);
 			ensure!(
 				new_validation_code.0.len() <= self.config.max_code_size as _,
-				Error::<T>::NewCodeTooLarge
+				CheckValidationOutputsError::NewCodeTooLarge,
 			);
 		}
 
@@ -729,7 +741,7 @@ impl<T: Trait> CandidateCheckContext<T> {
 				para_id,
 				processed_downward_messages,
 			),
-			Error::<T>::IncorrectDownwardMessageHandling,
+			CheckValidationOutputsError::IncorrectDownwardMessageHandling,
 		);
 		ensure!(
 			<router::Module<T>>::check_upward_messages(
@@ -737,7 +749,7 @@ impl<T: Trait> CandidateCheckContext<T> {
 				para_id,
 				upward_messages,
 			),
-			Error::<T>::InvalidUpwardMessages,
+			CheckValidationOutputsError::InvalidUpwardMessages,
 		);
 
 		Ok(())

runtime/parachains/src/runtime_api_impl/v1.rs

@@ -221,22 +221,8 @@ pub fn persisted_validation_data<T: initializer::Trait>(
 pub fn check_validation_outputs<T: initializer::Trait>(
 	para_id: ParaId,
 	outputs: primitives::v1::ValidationOutputs,
-) -> bool {
-	match <inclusion::Module<T>>::check_validation_outputs(para_id, outputs) {
-		Ok(()) => true,
-		Err(e) => {
-			frame_support::debug::RuntimeLogger::init();
-			let err: &'static str = e.into();
-			log::debug!(
-				target: "candidate_validation",
-				"Validation output checking for parachain `{}` failed: {}",
-				u32::from(para_id),
-				err,
-			);
-
-			false
-		}
-	}
+) -> Result<(), primitives::v1::CheckValidationOutputsError> {
+	<inclusion::Module<T>>::check_validation_outputs(para_id, outputs)
 }
 
 /// Implementation for the `session_index_for_child` function of the runtime API.

runtime/polkadot/src/lib.rs

@@ -1097,8 +1097,11 @@ sp_api::impl_runtime_apis! {
 			None
 		}
 
-		fn check_validation_outputs(_: Id, _: primitives::v1::ValidationOutputs) -> bool {
-			false
+		fn check_validation_outputs(
+			_: Id,
+			_: primitives::v1::ValidationOutputs,
+		) -> Result<(), primitives::v1::CheckValidationOutputsError> {
+			Err(primitives::v1::CheckValidationOutputsError::HeadDataTooLarge)
 		}
 
 		fn session_index_for_child() -> SessionIndex {

runtime/rococo/src/lib.rs

@@ -644,7 +644,7 @@ sp_api::impl_runtime_apis! {
 		fn check_validation_outputs(
 			para_id: Id,
 			outputs: primitives::v1::ValidationOutputs,
-		) -> bool {
+		) -> Result<(), primitives::v1::CheckValidationOutputsError> {
 			runtime_api_impl::check_validation_outputs::<Runtime>(para_id, outputs)
 		}
 

runtime/westend/src/lib.rs

@@ -827,9 +827,9 @@ sp_api::impl_runtime_apis! {
 
 		fn check_validation_outputs(
 			_: Id,
-			_: primitives::v1::ValidationOutputs
-		) -> bool {
-			false
+			_: primitives::v1::ValidationOutputs,
+		) -> Result<(), primitives::v1::CheckValidationOutputsError> {
+			Err(primitives::v1::CheckValidationOutputsError::HeadDataTooLarge)
 		}
 
 		fn session_index_for_child() -> SessionIndex {