Refactor check_validation_outputs

runtime/parachains/src/inclusion/mod.rs

@@ -512,11 +512,20 @@ impl<T: Config> Pallet<T> {
 				candidates.iter().enumerate()
 			{
 				if let FullCheck::Yes = full_check {
-					check_ctx.verify_backed_candidate(
+					match check_ctx.verify_backed_candidate(
 						parent_hash,
+						parent_storage_root,
 						candidate_idx,
 						backed_candidate,
-					)?;
+					)? {
+						Err(FailedToCreatePVD) => {
+							// We don't want to error out here because it will
+							// brick the relay-chain. So we return early without
+							// doing anything.
+							return Ok(ProcessedCandidates::default())
+						},
+						Ok(rpn) => rpn,
+					}
 				}
 
 				let para_id = backed_candidate.descriptor().para_id;
@@ -533,32 +542,6 @@ impl<T: Config> Pallet<T> {
 							);
 						}
 
-						{
-							// this should never fail because the para is registered
-							let persisted_validation_data =
-								match crate::util::make_persisted_validation_data::<T>(
-									para_id,
-									relay_parent_number,
-									parent_storage_root,
-								) {
-									Some(l) => l,
-									None => {
-										// We don't want to error out here because it will
-										// brick the relay-chain. So we return early without
-										// doing anything.
-										return Ok(ProcessedCandidates::default())
-									},
-								};
-
-							let expected = persisted_validation_data.hash();
-
-							ensure!(
-								expected ==
-									backed_candidate.descriptor().persisted_validation_data_hash,
-								Error::<T>::ValidationDataHashMismatch,
-							);
-						}
-
 						ensure!(
 							<PendingAvailability<T>>::get(&para_id).is_none() &&
 								<PendingAvailabilityCommitments<T>>::get(&para_id).is_none(),
@@ -940,6 +923,7 @@ pub(crate) struct CandidateCheckContext<T: Config> {
 	relay_parent_number: T::BlockNumber,
 }
 
+pub(crate) struct FailedToCreatePVD;
 impl<T: Config> CandidateCheckContext<T> {
 	pub(crate) fn new(now: T::BlockNumber, relay_parent_number: T::BlockNumber) -> Self {
 		Self { config: <configuration::Pallet<T>>::config(), now, relay_parent_number }
@@ -955,10 +939,32 @@ impl<T: Config> CandidateCheckContext<T> {
 	pub(crate) fn verify_backed_candidate(
 		&self,
 		parent_hash: <T as frame_system::Config>::Hash,
+		parent_storage_root: T::Hash,
 		candidate_idx: usize,
 		backed_candidate: &BackedCandidate<<T as frame_system::Config>::Hash>,
-	) -> Result<(), Error<T>> {
+	) -> Result<Result<(), FailedToCreatePVD>, Error<T>> {
 		let para_id = backed_candidate.descriptor().para_id;
+		let now = <frame_system::Pallet<T>>::block_number();
+		let relay_parent_number = now - One::one();
+
+		{
+			// this should never fail because the para is registered
+			let persisted_validation_data = match crate::util::make_persisted_validation_data::<T>(
+				para_id,
+				relay_parent_number,
+				parent_storage_root,
+			) {
+				Some(l) => l,
+				None => return Ok(Err(FailedToCreatePVD)),
+			};
+
+			let expected = persisted_validation_data.hash();
+
+			ensure!(
+				expected == backed_candidate.descriptor().persisted_validation_data_hash,
+				Error::<T>::ValidationDataHashMismatch,
+			);
+		}
 
 		// we require that the candidate is in the context of the parent block.
 		ensure!(
@@ -1002,7 +1008,7 @@ impl<T: Config> CandidateCheckContext<T> {
 			);
 			Err(err.strip_into_dispatch_err::<T>())?;
 		};
-		Ok(())
+		Ok(Ok(()))
 	}
 
 	/// Check the given outputs after candidate validation on whether it passes the acceptance

runtime/parachains/src/paras_inherent/mod.rs

@@ -594,6 +594,7 @@ impl<T: Config> Pallet<T> {
 				let scheduled = <scheduler::Pallet<T>>::scheduled();
 
 				let relay_parent_number = now - One::one();
+				let parent_storage_root = parent_header.state_root().clone();
 
 				let check_ctx = CandidateCheckContext::<T>::new(now, relay_parent_number);
 				let backed_candidates = sanitize_backed_candidates::<T, _>(
@@ -608,9 +609,19 @@ impl<T: Config> Pallet<T> {
 							// move the checking up here and skip it in the training wheels fallback.
 							// That way we avoid possible duplicate checks while assuring all
 							// backed candidates fine to pass on.
-							check_ctx
-								.verify_backed_candidate(parent_hash, candidate_idx, backed_candidate)
-								.is_err()
+							//
+							// NOTE: this is the only place where we check the relay-parent.
+							{
+								match check_ctx.verify_backed_candidate(
+									parent_hash,
+									parent_storage_root,
+									candidate_idx,
+									backed_candidate,
+								) {
+									Err(_) | Ok(Err(_)) => true,
+									Ok(Ok(_)) => false,
+								}
+							}
 					},
 					&scheduled[..],
 				);