Ease parachain candidate code fetching (#2593)

* code stored in para + modify CandidateDescriptor.

* WIP: digest + some more impl

* validation_code_hash in payload + check in inclusion

* check in client + refator

* tests

* fix encoding indices

* remove old todos

* fix test

* fix test

* add test

* fetch validation code inside collation-generation from the relay-chain

* HashMismatch -> PoVHashMismatch + miscompilation

* refactor, store hash when needed

* storage rename: more specific but slightly too verbose

* do not hash on candidate validation, fetch hash instead

* better test

* fix test

* guide updates

* don't panic in runtime

Co-authored-by: Robert Habermeier <rphmeier@gmail.com>

node/collation-generation/src/lib.rs

@@ -33,7 +33,7 @@ use polkadot_node_subsystem::{
 };
 use polkadot_node_subsystem_util::{
 	request_availability_cores_ctx, request_persisted_validation_data_ctx,
-	request_validators_ctx,
+	request_validators_ctx, request_validation_code_ctx,
 	metrics::{self, prometheus},
 };
 use polkadot_primitives::v1::{
@@ -244,9 +244,10 @@ async fn handle_new_activations<Context: SubsystemContext>(
 				continue;
 			}
 
-			// we get validation data synchronously for each core instead of
+			// we get validation data and validation code synchronously for each core instead of
 			// within the subtask loop, because we have only a single mutable handle to the
 			// context, so the work can't really be distributed
+
 			let validation_data = match request_persisted_validation_data_ctx(
 				relay_parent,
 				scheduled_core.para_id,
@@ -270,6 +271,30 @@ async fn handle_new_activations<Context: SubsystemContext>(
 				}
 			};
 
+			let validation_code = match request_validation_code_ctx(
+				relay_parent,
+				scheduled_core.para_id,
+				assumption,
+				ctx,
+			)
+			.await?
+			.await??
+			{
+				Some(v) => v,
+				None => {
+					tracing::trace!(
+						target: LOG_TARGET,
+						core_idx = %core_idx,
+						relay_parent = ?relay_parent,
+						our_para = %config.para_id,
+						their_para = %scheduled_core.para_id,
+						"validation code is not available",
+					);
+					continue
+				}
+			};
+			let validation_code_hash = validation_code.hash();
+
 			let task_config = config.clone();
 			let mut task_sender = sender.clone();
 			let metrics = metrics.clone();
@@ -295,6 +320,7 @@ async fn handle_new_activations<Context: SubsystemContext>(
 					&scheduled_core.para_id,
 					&persisted_validation_data_hash,
 					&pov_hash,
+					&validation_code_hash,
 				);
 
 				let erasure_root = match erasure_root(
@@ -334,6 +360,7 @@ async fn handle_new_activations<Context: SubsystemContext>(
 						pov_hash,
 						erasure_root,
 						para_head: commitments.head_data.hash(),
+						validation_code_hash: validation_code_hash,
 					},
 				};
 
@@ -474,7 +501,7 @@ mod tests {
 		};
 		use polkadot_primitives::v1::{
 			BlockNumber, CollatorPair, Id as ParaId,
-			PersistedValidationData, ScheduledCore,
+			PersistedValidationData, ScheduledCore, ValidationCode,
 		};
 		use std::pin::Pin;
 
@@ -696,6 +723,16 @@ mod tests {
 						))) => {
 							tx.send(Ok(vec![Default::default(); 3])).unwrap();
 						}
+						Some(AllMessages::RuntimeApi(RuntimeApiMessage::Request(
+							_hash,
+							RuntimeApiRequest::ValidationCode(
+								_para_id,
+								OccupiedCoreAssumption::Free,
+								tx,
+							),
+						))) => {
+							tx.send(Ok(Some(ValidationCode(vec![1, 2, 3])))).unwrap();
+						}
 						Some(msg) => {
 							panic!("didn't expect any other overseer requests; got {:?}", msg)
 						}
@@ -733,11 +770,13 @@ mod tests {
 			let expect_validation_data_hash
 				= PersistedValidationData::<Hash, BlockNumber>::default().hash();
 			let expect_relay_parent = Hash::repeat_byte(4);
+			let expect_validation_code_hash = ValidationCode(vec![1, 2, 3]).hash();
 			let expect_payload = collator_signature_payload(
 				&expect_relay_parent,
 				&config.para_id,
 				&expect_validation_data_hash,
 				&expect_pov_hash,
+				&expect_validation_code_hash,
 			);
 			let expect_descriptor = CandidateDescriptor {
 				signature: config.key.sign(&expect_payload),
@@ -748,6 +787,7 @@ mod tests {
 				pov_hash: expect_pov_hash,
 				erasure_root: Default::default(), // this isn't something we're checking right now
 				para_head: test_collation().head_data.hash(),
+				validation_code_hash: expect_validation_code_hash,
 			};
 
 			assert_eq!(sent_messages.len(), 1);
@@ -768,6 +808,7 @@ mod tests {
 							&descriptor.para_id,
 							&descriptor.persisted_validation_data_hash,
 							&descriptor.pov_hash,
+							&descriptor.validation_code_hash,
 						)
 						.as_ref(),
 						&descriptor.collator,

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

@@ -361,21 +361,27 @@ async fn spawn_validate_exhaustive(
 
 /// Does basic checks of a candidate. Provide the encoded PoV-block. Returns `Ok` if basic checks
 /// are passed, `Err` otherwise.
-#[tracing::instrument(level = "trace", skip(pov), fields(subsystem = LOG_TARGET))]
+#[tracing::instrument(level = "trace", skip(pov, validation_code), fields(subsystem = LOG_TARGET))]
 fn perform_basic_checks(
 	candidate: &CandidateDescriptor,
 	max_pov_size: u32,
 	pov: &PoV,
+	validation_code: &ValidationCode,
 ) -> Result<(), InvalidCandidate> {
 	let encoded_pov = pov.encode();
-	let hash = pov.hash();
+	let pov_hash = pov.hash();
+	let validation_code_hash = validation_code.hash();
 
 	if encoded_pov.len() > max_pov_size as usize {
 		return Err(InvalidCandidate::ParamsTooLarge(encoded_pov.len() as u64));
 	}
 
-	if hash != candidate.pov_hash {
-		return Err(InvalidCandidate::HashMismatch);
+	if pov_hash != candidate.pov_hash {
+		return Err(InvalidCandidate::PoVHashMismatch);
+	}
+
+	if validation_code_hash != candidate.validation_code_hash {
+		return Err(InvalidCandidate::CodeHashMismatch);
 	}
 
 	if let Err(()) = candidate.check_collator_signature() {
@@ -431,7 +437,12 @@ fn validate_candidate_exhaustive<B: ValidationBackend, S: SpawnNamed + 'static>(
 ) -> Result<ValidationResult, ValidationFailed> {
 	let _timer = metrics.time_validate_candidate_exhaustive();
 
-	if let Err(e) = perform_basic_checks(&descriptor, persisted_validation_data.max_pov_size, &*pov) {
+	if let Err(e) = perform_basic_checks(
+		&descriptor,
+		persisted_validation_data.max_pov_size,
+		&*pov,
+		&validation_code,
+	) {
 		return Ok(ValidationResult::Invalid(e))
 	}
 
@@ -601,6 +612,7 @@ mod tests {
 			&descriptor.para_id,
 			&descriptor.persisted_validation_data_hash,
 			&descriptor.pov_hash,
+			&descriptor.validation_code_hash,
 		);
 
 		descriptor.signature = collator.sign(&payload[..]).into();
@@ -891,13 +903,21 @@ mod tests {
 
 		let pov = PoV { block_data: BlockData(vec![1; 32]) };
 		let head_data = HeadData(vec![1, 1, 1]);
+		let validation_code = ValidationCode(vec![2; 16]);
 
 		let mut descriptor = CandidateDescriptor::default();
 		descriptor.pov_hash = pov.hash();
 		descriptor.para_head = head_data.hash();
+		descriptor.validation_code_hash = validation_code.hash();
 		collator_sign(&mut descriptor, Sr25519Keyring::Alice);
 
-		assert!(perform_basic_checks(&descriptor, validation_data.max_pov_size, &pov).is_ok());
+		let check = perform_basic_checks(
+			&descriptor,
+			validation_data.max_pov_size,
+			&pov,
+			&validation_code,
+		);
+		assert!(check.is_ok());
 
 		let validation_result = WasmValidationResult {
 			head_data,
@@ -911,7 +931,7 @@ mod tests {
 		let v = validate_candidate_exhaustive::<MockValidationBackend, _>(
 			MockValidationArg { result: Ok(validation_result) },
 			validation_data.clone(),
-			vec![1, 2, 3].into(),
+			validation_code,
 			descriptor,
 			Arc::new(pov),
 			TaskExecutor::new(),
@@ -933,12 +953,20 @@ mod tests {
 		let validation_data = PersistedValidationData { max_pov_size: 1024, ..Default::default() };
 
 		let pov = PoV { block_data: BlockData(vec![1; 32]) };
+		let validation_code = ValidationCode(vec![2; 16]);
 
 		let mut descriptor = CandidateDescriptor::default();
 		descriptor.pov_hash = pov.hash();
+		descriptor.validation_code_hash = validation_code.hash();
 		collator_sign(&mut descriptor, Sr25519Keyring::Alice);
 
-		assert!(perform_basic_checks(&descriptor, validation_data.max_pov_size, &pov).is_ok());
+		let check = perform_basic_checks(
+			&descriptor,
+			validation_data.max_pov_size,
+			&pov,
+			&validation_code,
+		);
+		assert!(check.is_ok());
 
 		let v = validate_candidate_exhaustive::<MockValidationBackend, _>(
 			MockValidationArg {
@@ -947,7 +975,7 @@ mod tests {
 				))
 			},
 			validation_data,
-			vec![1, 2, 3].into(),
+			validation_code,
 			descriptor,
 			Arc::new(pov),
 			TaskExecutor::new(),
@@ -962,12 +990,20 @@ mod tests {
 		let validation_data = PersistedValidationData { max_pov_size: 1024, ..Default::default() };
 
 		let pov = PoV { block_data: BlockData(vec![1; 32]) };
+		let validation_code = ValidationCode(vec![2; 16]);
 
 		let mut descriptor = CandidateDescriptor::default();
 		descriptor.pov_hash = pov.hash();
+		descriptor.validation_code_hash = validation_code.hash();
 		collator_sign(&mut descriptor, Sr25519Keyring::Alice);
 
-		assert!(perform_basic_checks(&descriptor, validation_data.max_pov_size, &pov).is_ok());
+		let check = perform_basic_checks(
+			&descriptor,
+			validation_data.max_pov_size,
+			&pov,
+			&validation_code,
+		);
+		assert!(check.is_ok());
 
 		let v = validate_candidate_exhaustive::<MockValidationBackend, _>(
 			MockValidationArg {
@@ -976,7 +1012,7 @@ mod tests {
 				))
 			},
 			validation_data,
-			vec![1, 2, 3].into(),
+			validation_code,
 			descriptor,
 			Arc::new(pov),
 			TaskExecutor::new(),
@@ -985,4 +1021,42 @@ mod tests {
 
 		assert_matches!(v, Ok(ValidationResult::Invalid(InvalidCandidate::Timeout)));
 	}
+
+	#[test]
+	fn candidate_validation_code_mismatch_is_invalid() {
+		let validation_data = PersistedValidationData { max_pov_size: 1024, ..Default::default() };
+
+		let pov = PoV { block_data: BlockData(vec![1; 32]) };
+		let validation_code = ValidationCode(vec![2; 16]);
+
+		let mut descriptor = CandidateDescriptor::default();
+		descriptor.pov_hash = pov.hash();
+		descriptor.validation_code_hash = ValidationCode(vec![1; 16]).hash();
+		collator_sign(&mut descriptor, Sr25519Keyring::Alice);
+
+		let check = perform_basic_checks(
+			&descriptor,
+			validation_data.max_pov_size,
+			&pov,
+			&validation_code,
+		);
+		assert_matches!(check, Err(InvalidCandidate::CodeHashMismatch));
+
+		let v = validate_candidate_exhaustive::<MockValidationBackend, _>(
+			MockValidationArg {
+				result: Err(ValidationError::InvalidCandidate(
+					WasmInvalidCandidate::BadReturn
+				))
+			},
+			validation_data,
+			validation_code,
+			descriptor,
+			Arc::new(pov),
+			TaskExecutor::new(),
+			&Default::default(),
+		).unwrap();
+
+		assert_matches!(v, ValidationResult::Invalid(InvalidCandidate::CodeHashMismatch));
+	}
+
 }

node/core/runtime-api/src/lib.rs

@@ -556,6 +556,13 @@ mod tests {
 			) -> BTreeMap<ParaId, Vec<InboundHrmpMessage>> {
 				self.hrmp_channels.get(&recipient).map(|q| q.clone()).unwrap_or_default()
 			}
+
+			fn validation_code_by_hash(
+				&self,
+				_hash: Hash,
+			) -> Option<ValidationCode> {
+				unreachable!("not used in tests");
+			}
 		}
 
 		impl BabeApi<Block> for MockRuntimeApi {

node/primitives/src/lib.rs

@@ -124,11 +124,13 @@ pub enum InvalidCandidate {
 	/// Invalid relay chain parent.
 	BadParent,
 	/// POV hash does not match.
-	HashMismatch,
+	PoVHashMismatch,
 	/// Bad collator signature.
 	BadSignature,
 	/// Para head hash does not match.
 	ParaHeadHashMismatch,
+	/// Validation code hash does not match.
+	CodeHashMismatch,
 }
 
 /// Result of the validation of the candidate.

parachain/src/primitives.rs

@@ -21,6 +21,7 @@ use sp_std::vec::Vec;
 
 use parity_scale_codec::{Encode, Decode, CompactAs};
 use sp_core::{RuntimeDebug, TypeId};
+use sp_runtime::traits::Hash as _;
 
 #[cfg(feature = "std")]
 use serde::{Serialize, Deserialize};
@@ -45,7 +46,6 @@ pub struct HeadData(#[cfg_attr(feature = "std", serde(with="bytes"))] pub Vec<u8
 impl HeadData {
 	/// Returns the hash of this head data.
 	pub fn hash(&self) -> Hash {
-		use sp_runtime::traits::Hash;
 		sp_runtime::traits::BlakeTwo256::hash(&self.0)
 	}
 }
@@ -55,6 +55,13 @@ impl HeadData {
 #[cfg_attr(feature = "std", derive(Serialize, Deserialize, Hash, MallocSizeOf))]
 pub struct ValidationCode(#[cfg_attr(feature = "std", serde(with="bytes"))] pub Vec<u8>);
 
+impl ValidationCode {
+	/// Get the blake2-256 hash of the validation code bytes.
+	pub fn hash(&self) -> Hash {
+		sp_runtime::traits::BlakeTwo256::hash(&self.0[..])
+	}
+}
+
 /// Parachain block data.
 ///
 /// Contains everything required to validate para-block, may contain block and witness data.

primitives/src/v1.rs

@@ -211,14 +211,16 @@ pub fn collator_signature_payload<H: AsRef<[u8]>>(
 	para_id: &Id,
 	persisted_validation_data_hash: &Hash,
 	pov_hash: &Hash,
-) -> [u8; 100] {
+	validation_code_hash: &Hash,
+) -> [u8; 132] {
 	// 32-byte hash length is protected in a test below.
-	let mut payload = [0u8; 100];
+	let mut payload = [0u8; 132];
 
 	payload[0..32].copy_from_slice(relay_parent.as_ref());
 	u32::from(*para_id).using_encoded(|s| payload[32..32 + s.len()].copy_from_slice(s));
 	payload[36..68].copy_from_slice(persisted_validation_data_hash.as_ref());
 	payload[68..100].copy_from_slice(pov_hash.as_ref());
+	payload[100..132].copy_from_slice(validation_code_hash.as_ref());
 
 	payload
 }
@@ -228,6 +230,7 @@ fn check_collator_signature<H: AsRef<[u8]>>(
 	para_id: &Id,
 	persisted_validation_data_hash: &Hash,
 	pov_hash: &Hash,
+	validation_code_hash: &Hash,
 	collator: &CollatorId,
 	signature: &CollatorSignature,
 ) -> Result<(),()> {
@@ -236,6 +239,7 @@ fn check_collator_signature<H: AsRef<[u8]>>(
 		para_id,
 		persisted_validation_data_hash,
 		pov_hash,
+		validation_code_hash,
 	);
 
 	if signature.verify(&payload[..], collator) {
@@ -268,6 +272,8 @@ pub struct CandidateDescriptor<H = Hash> {
 	pub signature: CollatorSignature,
 	/// Hash of the para header that is being generated by this candidate.
 	pub para_head: Hash,
+	/// The blake2-256 hash of the validation code bytes.
+	pub validation_code_hash: Hash,
 }
 
 impl<H: AsRef<[u8]>> CandidateDescriptor<H> {
@@ -278,6 +284,7 @@ impl<H: AsRef<[u8]>> CandidateDescriptor<H> {
 			&self.para_id,
 			&self.persisted_validation_data_hash,
 			&self.pov_hash,
+			&self.validation_code_hash,
 			&self.collator,
 			&self.signature,
 		)
@@ -872,6 +879,10 @@ sp_api::decl_runtime_apis! {
 		/// messages in them are also included.
 		#[skip_initialize_block]
 		fn inbound_hrmp_channels_contents(recipient: Id) -> BTreeMap<Id, Vec<InboundHrmpMessage<N>>>;
+
+		/// Get the validation code from its hash.
+		#[skip_initialize_block]
+		fn validation_code_by_hash(hash: Hash) -> Option<ValidationCode>;
 	}
 }
 
@@ -1107,6 +1118,7 @@ mod tests {
 			&5u32.into(),
 			&Hash::repeat_byte(2),
 			&Hash::repeat_byte(3),
+			&Hash::repeat_byte(4),
 		);
 	}
 }

roadmap/implementers-guide/src/runtime/paras.md

@@ -116,10 +116,10 @@ Parachains: Vec<ParaId>,
 ParaLifecycle: map ParaId => Option<ParaLifecycle>,
 /// The head-data of every registered para.
 Heads: map ParaId => Option<HeadData>;
-/// The validation code of every live para.
-ValidationCode: map ParaId => Option<ValidationCode>;
-/// Actual past code, indicated by the para id as well as the block number at which it became outdated.
-PastCode: map (ParaId, BlockNumber) => Option<ValidationCode>;
+/// The validation code hash of every live para.
+CurrentCodeHash: map ParaId => Option<Hash>;
+/// Actual past code hash, indicated by the para id as well as the block number at which it became outdated.
+PastCodeHash: map (ParaId, BlockNumber) => Option<Hash>;
 /// Past code of parachains. The parachains themselves may not be registered anymore,
 /// but we also keep their code on-chain for the same amount of time as outdated code
 /// to keep it available for secondary checkers.
@@ -136,24 +136,28 @@ PastCodePruning: Vec<(ParaId, BlockNumber)>;
 /// in the context of a relay chain block with a number >= `expected_at`.
 FutureCodeUpgrades: map ParaId => Option<BlockNumber>;
 /// The actual future code of a para.
-FutureCode: map ParaId => Option<ValidationCode>;
+FutureCodeHash: map ParaId => Option<Hash>;
 /// The actions to perform during the start of a specific session index.
 ActionsQueue: map SessionIndex => Vec<ParaId>;
 /// Upcoming paras instantiation arguments.
 UpcomingParasGenesis: map ParaId => Option<ParaGenesisArgs>;
+/// The number of references on the validation code in `CodeByHash` storage.
+CodeByHashRefs: map Hash => u32;
+/// Validation code stored by its hash.
+CoeByHash: map Hash => Option<ValidationCode>
 ```
 
 ## Session Change
 
 1. Execute all queued actions for paralifecycle changes:
   1. Clean up outgoing paras.
-     1. This means removing the entries under `Heads`, `ValidationCode`, `FutureCodeUpgrades`, and
+     1. This means removing the entries under `Heads`, `CurrentCode`, `FutureCodeUpgrades`, and
         `FutureCode`. An according entry should be added to `PastCode`, `PastCodeMeta`, and
-        `PastCodePruning` using the outgoing `ParaId` and removed `ValidationCode` value. This is
+        `PastCodePruning` using the outgoing `ParaId` and removed `CurrentCode` value. This is
         because any outdated validation code must remain available on-chain for a determined amount
         of blocks, and validation code outdated by de-registering the para is still subject to that
         invariant.
-  1. Apply all incoming paras by initializing the `Heads` and `ValidationCode` using the genesis
+  1. Apply all incoming paras by initializing the `Heads` and `CurrentCode` using the genesis
      parameters.
   1. Amend the `Parachains` list and `ParaLifecycle` to reflect changes in registered parachains.
   1. Amend the `ParaLifecycle` set to reflect changes in registered parathreads.
@@ -175,7 +179,7 @@ UpcomingParasGenesis: map ParaId => Option<ParaGenesisArgs>;
 * `schedule_para_cleanup(ParaId)`: Schedule a para to be cleaned up after the next full session.
 * `schedule_parathread_upgrade(ParaId)`: Schedule a parathread to be upgraded to a parachain.
 * `schedule_parachain_downgrade(ParaId)`: Schedule a parachain to be downgraded to a parathread.
-* `schedule_code_upgrade(ParaId, ValidationCode, expected_at: BlockNumber)`: Schedule a future code
+* `schedule_code_upgrade(ParaId, CurrentCode, expected_at: BlockNumber)`: Schedule a future code
   upgrade of the given parachain, to be applied after inclusion of a block of the same parachain
   executed in the context of a relay-chain block with number >= `expected_at`.
 * `note_new_head(ParaId, HeadData, BlockNumber)`: note that a para has progressed to a new head,
@@ -187,6 +191,7 @@ UpcomingParasGenesis: map ParaId => Option<ParaGenesisArgs>;
   intermediate parablock has been included at the given relay-chain height. This may return past,
   current, or (with certain choices of `assume_intermediate`) future code. `assume_intermediate`, if
   provided, must be before `at`. If the validation code has been pruned, this will return `None`.
+* `validation_code_hash_at(ParaId, at: BlockNumber, assume_intermediate: Option<BlockNumber>)`: Just like `validation_code_at`, but returns the code hash.
 * `lifecycle(ParaId) -> Option<ParaLifecycle>`: Return the `ParaLifecycle` of a para.
 * `is_parachain(ParaId) -> bool`: Returns true if the para ID references any live parachain,
   including those which may be transitioning to a parathread in the future.
@@ -197,9 +202,6 @@ UpcomingParasGenesis: map ParaId => Option<ParaGenesisArgs>;
 * `last_code_upgrade(id: ParaId, include_future: bool) -> Option<BlockNumber>`: The block number of
   the last scheduled upgrade of the requested para. Includes future upgrades if the flag is set.
   This is the `expected_at` number, not the `activated_at` number.
-* `persisted_validation_data(id: ParaId) -> Option<PersistedValidationData>`: Get the
-  PersistedValidationData of the given para, assuming the context is the parent block. Returns
-  `None` if the para is not known.
 
 ## Finalization
 

runtime/kusama/src/lib.rs

@@ -1218,6 +1218,10 @@ sp_api::impl_runtime_apis! {
 		) -> BTreeMap<Id, Vec<InboundHrmpMessage<BlockNumber>>> {
 			BTreeMap::new()
 		}
+
+		fn validation_code_by_hash(_hash: Hash) -> Option<ValidationCode> {
+			None
+		}
 	}
 
 	impl beefy_primitives::BeefyApi<Block, BeefyId> for Runtime {

runtime/parachains/src/inclusion.rs

@@ -185,6 +185,8 @@ decl_error! {
 		HrmpWatermarkMishandling,
 		/// The HRMP messages sent by the candidate is not valid.
 		InvalidOutboundHrmp,
+		/// The validation code hash of the candidate is not valid.
+		InvalidValidationCodeHash,
 	}
 }
 
@@ -444,6 +446,15 @@ impl<T: Config> Module<T> {
 					Error::<T>::NotCollatorSigned,
 				);
 
+				let validation_code_hash =
+					<paras::Module<T>>::validation_code_hash_at(para_id, now, None)
+					// A candidate for a parachain without current validation code is not scheduled.
+					.ok_or_else(|| Error::<T>::UnscheduledCandidate)?;
+				ensure!(
+					candidate.descriptor().validation_code_hash == validation_code_hash,
+					Error::<T>::InvalidValidationCodeHash,
+				);
+
 				if let Err(err) = check_cx
 					.check_validation_outputs(
 						para_id,
@@ -954,6 +965,7 @@ mod tests {
 			&candidate.descriptor.para_id,
 			&candidate.descriptor.persisted_validation_data_hash,
 			&candidate.descriptor.pov_hash,
+			&candidate.descriptor.validation_code_hash,
 		);
 
 		candidate.descriptor.signature = collator.sign(&payload[..]).into();
@@ -1109,6 +1121,7 @@ mod tests {
 		relay_parent: Hash,
 		persisted_validation_data_hash: Hash,
 		new_validation_code: Option<ValidationCode>,
+		validation_code: ValidationCode,
 		hrmp_watermark: BlockNumber,
 	}
 
@@ -1120,6 +1133,7 @@ mod tests {
 					pov_hash: self.pov_hash,
 					relay_parent: self.relay_parent,
 					persisted_validation_data_hash: self.persisted_validation_data_hash,
+					validation_code_hash: self.validation_code.hash(),
 					..Default::default()
 				},
 				commitments: CandidateCommitments {
@@ -2071,6 +2085,43 @@ mod tests {
 					Err(Error::<Test>::ValidationDataHashMismatch.into()),
 				);
 			}
+
+			// bad validation code hash
+			{
+				let mut candidate = TestCandidateBuilder {
+					para_id: chain_a,
+					relay_parent: System::parent_hash(),
+					pov_hash: Hash::repeat_byte(1),
+					persisted_validation_data_hash: make_vdata_hash(chain_a).unwrap(),
+					hrmp_watermark: RELAY_PARENT_NUM,
+					validation_code: ValidationCode(vec![1]),
+					..Default::default()
+				}.build();
+
+				collator_sign_candidate(
+					Sr25519Keyring::One,
+					&mut candidate,
+				);
+
+				let backed = block_on(back_candidate(
+					candidate,
+					&validators,
+					group_validators(GroupIndex::from(0)).unwrap().as_ref(),
+					&keystore,
+					&signing_context,
+					BackingKind::Threshold,
+				));
+
+				assert_eq!(
+					Inclusion::process_candidates(
+						Default::default(),
+						vec![backed],
+						vec![chain_a_assignment.clone()],
+						&group_validators,
+					),
+					Err(Error::<Test>::InvalidValidationCodeHash.into()),
+				);
+			}
 		});
 	}