From 9bf1a5e23884921498b381728bfddaae93f83744 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <git@kchr.de>
Date: Thu, 22 Feb 2024 14:35:00 +0100
Subject: [PATCH] Check that the validation code matches the parachain code
 (#3433)

This introduces a check to ensure that the parachain code matches the
validation code stored in the relay chain state. If not, it will print a
warning. This should be mainly useful for parachain builders to make
sure they have setup everything correctly.
---
 .../consensus/aura/src/collators/basic.rs     | 22 +++++++-
 .../consensus/aura/src/collators/lookahead.rs | 17 ++++--
 .../consensus/aura/src/collators/mod.rs       | 55 +++++++++++++++++++
 cumulus/client/consensus/common/src/tests.rs  |  9 +++
 cumulus/client/network/src/tests.rs           |  9 +++
 .../src/lib.rs                                | 15 ++++-
 .../client/relay-chain-interface/src/lib.rs   | 22 +++++++-
 .../relay-chain-rpc-interface/src/lib.rs      | 13 ++++-
 .../src/rpc_client.rs                         | 14 +++++
 polkadot/node/core/backing/src/lib.rs         | 10 ++--
 10 files changed, 173 insertions(+), 13 deletions(-)

diff --git a/cumulus/client/consensus/aura/src/collators/basic.rs b/cumulus/client/consensus/aura/src/collators/basic.rs
index 8740b06005d..52b83254951 100644
--- a/cumulus/client/consensus/aura/src/collators/basic.rs
+++ b/cumulus/client/consensus/aura/src/collators/basic.rs
@@ -33,12 +33,12 @@ use cumulus_relay_chain_interface::RelayChainInterface;
 
 use polkadot_node_primitives::CollationResult;
 use polkadot_overseer::Handle as OverseerHandle;
-use polkadot_primitives::{CollatorPair, Id as ParaId};
+use polkadot_primitives::{CollatorPair, Id as ParaId, ValidationCode};
 
 use futures::{channel::mpsc::Receiver, prelude::*};
 use sc_client_api::{backend::AuxStore, BlockBackend, BlockOf};
 use sc_consensus::BlockImport;
-use sp_api::ProvideRuntimeApi;
+use sp_api::{CallApiAt, ProvideRuntimeApi};
 use sp_application_crypto::AppPublic;
 use sp_blockchain::HeaderBackend;
 use sp_consensus::SyncOracle;
@@ -47,6 +47,7 @@ use sp_core::crypto::Pair;
 use sp_inherents::CreateInherentDataProviders;
 use sp_keystore::KeystorePtr;
 use sp_runtime::traits::{Block as BlockT, Header as HeaderT, Member};
+use sp_state_machine::Backend as _;
 use std::{convert::TryFrom, sync::Arc, time::Duration};
 
 use crate::collator as collator_util;
@@ -100,6 +101,7 @@ where
 		+ AuxStore
 		+ HeaderBackend<Block>
 		+ BlockBackend<Block>
+		+ CallApiAt<Block>
 		+ Send
 		+ Sync
 		+ 'static,
@@ -172,6 +174,22 @@ where
 				continue
 			}
 
+			let Ok(Some(code)) =
+				params.para_client.state_at(parent_hash).map_err(drop).and_then(|s| {
+					s.storage(&sp_core::storage::well_known_keys::CODE).map_err(drop)
+				})
+			else {
+				continue;
+			};
+
+			super::check_validation_code_or_log(
+				&ValidationCode::from(code).hash(),
+				params.para_id,
+				&params.relay_client,
+				*request.relay_parent(),
+			)
+			.await;
+
 			let relay_parent_header =
 				match params.relay_client.header(RBlockId::hash(*request.relay_parent())).await {
 					Err(e) => reject_with_error!(e),
diff --git a/cumulus/client/consensus/aura/src/collators/lookahead.rs b/cumulus/client/consensus/aura/src/collators/lookahead.rs
index a9f33173d83..161f10d55a1 100644
--- a/cumulus/client/consensus/aura/src/collators/lookahead.rs
+++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs
@@ -290,10 +290,7 @@ where
 			// If the longest chain has space, build upon that. Otherwise, don't
 			// build at all.
 			potential_parents.sort_by_key(|a| a.depth);
-			let initial_parent = match potential_parents.pop() {
-				None => continue,
-				Some(p) => p,
-			};
+			let Some(initial_parent) = potential_parents.pop() else { continue };
 
 			// Build in a loop until not allowed. Note that the authorities can change
 			// at any block, so we need to re-claim our slot every time.
@@ -301,6 +298,10 @@ where
 			let mut parent_header = initial_parent.header;
 			let overseer_handle = &mut params.overseer_handle;
 
+			// We mainly call this to inform users at genesis if there is a mismatch with the
+			// on-chain data.
+			collator.collator_service().check_block_status(parent_hash, &parent_header);
+
 			// This needs to change to support elastic scaling, but for continuously
 			// scheduled chains this ensures that the backlog will grow steadily.
 			for n_built in 0..2 {
@@ -353,6 +354,14 @@ where
 					Some(v) => v,
 				};
 
+				super::check_validation_code_or_log(
+					&validation_code_hash,
+					params.para_id,
+					&params.relay_client,
+					relay_parent,
+				)
+				.await;
+
 				match collator
 					.collate(
 						&parent_header,
diff --git a/cumulus/client/consensus/aura/src/collators/mod.rs b/cumulus/client/consensus/aura/src/collators/mod.rs
index 4c7b759daf7..6e0067d0ced 100644
--- a/cumulus/client/consensus/aura/src/collators/mod.rs
+++ b/cumulus/client/consensus/aura/src/collators/mod.rs
@@ -20,5 +20,60 @@
 //! included parachain block, as well as the [`lookahead`] collator, which prospectively
 //! builds on parachain blocks which have not yet been included in the relay chain.
 
+use cumulus_relay_chain_interface::RelayChainInterface;
+use polkadot_primitives::{
+	Hash as RHash, Id as ParaId, OccupiedCoreAssumption, ValidationCodeHash,
+};
+
 pub mod basic;
 pub mod lookahead;
+
+/// Check the `local_validation_code_hash` against the validation code hash in the relay chain
+/// state.
+///
+/// If the code hashes do not match, it prints a warning.
+async fn check_validation_code_or_log(
+	local_validation_code_hash: &ValidationCodeHash,
+	para_id: ParaId,
+	relay_client: &impl RelayChainInterface,
+	relay_parent: RHash,
+) {
+	let state_validation_code_hash = match relay_client
+		.validation_code_hash(relay_parent, para_id, OccupiedCoreAssumption::Included)
+		.await
+	{
+		Ok(hash) => hash,
+		Err(error) => {
+			tracing::debug!(
+				target: super::LOG_TARGET,
+				%error,
+				?relay_parent,
+				%para_id,
+				"Failed to fetch validation code hash",
+			);
+			return
+		},
+	};
+
+	match state_validation_code_hash {
+		Some(state) =>
+			if state != *local_validation_code_hash {
+				tracing::warn!(
+					target: super::LOG_TARGET,
+					%para_id,
+					?relay_parent,
+					?local_validation_code_hash,
+					relay_validation_code_hash = ?state,
+					"Parachain code doesn't match validation code stored in the relay chain state",
+				);
+			},
+		None => {
+			tracing::warn!(
+				target: super::LOG_TARGET,
+				%para_id,
+				?relay_parent,
+				"Could not find validation code for parachain in the relay chain state.",
+			);
+		},
+	}
+}
diff --git a/cumulus/client/consensus/common/src/tests.rs b/cumulus/client/consensus/common/src/tests.rs
index 597d1ab2acc..bfb95ae388a 100644
--- a/cumulus/client/consensus/common/src/tests.rs
+++ b/cumulus/client/consensus/common/src/tests.rs
@@ -136,6 +136,15 @@ impl RelayChainInterface for Relaychain {
 		Ok(Some(PersistedValidationData { parent_head, ..Default::default() }))
 	}
 
+	async fn validation_code_hash(
+		&self,
+		_: PHash,
+		_: ParaId,
+		_: OccupiedCoreAssumption,
+	) -> RelayChainResult<Option<ValidationCodeHash>> {
+		unimplemented!("Not needed for test")
+	}
+
 	async fn candidate_pending_availability(
 		&self,
 		_: PHash,
diff --git a/cumulus/client/network/src/tests.rs b/cumulus/client/network/src/tests.rs
index e03f470753b..d986635f961 100644
--- a/cumulus/client/network/src/tests.rs
+++ b/cumulus/client/network/src/tests.rs
@@ -117,6 +117,15 @@ impl RelayChainInterface for DummyRelayChainInterface {
 		}))
 	}
 
+	async fn validation_code_hash(
+		&self,
+		_: PHash,
+		_: ParaId,
+		_: OccupiedCoreAssumption,
+	) -> RelayChainResult<Option<ValidationCodeHash>> {
+		unimplemented!("Not needed for test")
+	}
+
 	async fn candidate_pending_availability(
 		&self,
 		_: PHash,
diff --git a/cumulus/client/relay-chain-inprocess-interface/src/lib.rs b/cumulus/client/relay-chain-inprocess-interface/src/lib.rs
index 866214fe2c5..6ea02b2e7c1 100644
--- a/cumulus/client/relay-chain-inprocess-interface/src/lib.rs
+++ b/cumulus/client/relay-chain-inprocess-interface/src/lib.rs
@@ -21,7 +21,7 @@ use cumulus_primitives_core::{
 	relay_chain::{
 		runtime_api::ParachainHost, Block as PBlock, BlockId, CommittedCandidateReceipt,
 		Hash as PHash, Header as PHeader, InboundHrmpMessage, OccupiedCoreAssumption, SessionIndex,
-		ValidatorId,
+		ValidationCodeHash, ValidatorId,
 	},
 	InboundDownwardMessage, ParaId, PersistedValidationData,
 };
@@ -115,6 +115,19 @@ impl RelayChainInterface for RelayChainInProcessInterface {
 		)?)
 	}
 
+	async fn validation_code_hash(
+		&self,
+		hash: PHash,
+		para_id: ParaId,
+		occupied_core_assumption: OccupiedCoreAssumption,
+	) -> RelayChainResult<Option<ValidationCodeHash>> {
+		Ok(self.full_client.runtime_api().validation_code_hash(
+			hash,
+			para_id,
+			occupied_core_assumption,
+		)?)
+	}
+
 	async fn candidate_pending_availability(
 		&self,
 		hash: PHash,
diff --git a/cumulus/client/relay-chain-interface/src/lib.rs b/cumulus/client/relay-chain-interface/src/lib.rs
index de5e7891b30..bb93e6a168c 100644
--- a/cumulus/client/relay-chain-interface/src/lib.rs
+++ b/cumulus/client/relay-chain-interface/src/lib.rs
@@ -30,7 +30,7 @@ use cumulus_primitives_core::relay_chain::BlockId;
 pub use cumulus_primitives_core::{
 	relay_chain::{
 		CommittedCandidateReceipt, Hash as PHash, Header as PHeader, InboundHrmpMessage,
-		OccupiedCoreAssumption, SessionIndex, ValidatorId,
+		OccupiedCoreAssumption, SessionIndex, ValidationCodeHash, ValidatorId,
 	},
 	InboundDownwardMessage, ParaId, PersistedValidationData,
 };
@@ -194,6 +194,15 @@ pub trait RelayChainInterface: Send + Sync {
 		relay_parent: PHash,
 		relevant_keys: &Vec<Vec<u8>>,
 	) -> RelayChainResult<StorageProof>;
+
+	/// Returns the validation code hash for the given `para_id` using the given
+	/// `occupied_core_assumption`.
+	async fn validation_code_hash(
+		&self,
+		relay_parent: PHash,
+		para_id: ParaId,
+		occupied_core_assumption: OccupiedCoreAssumption,
+	) -> RelayChainResult<Option<ValidationCodeHash>>;
 }
 
 #[async_trait]
@@ -301,4 +310,15 @@ where
 	async fn header(&self, block_id: BlockId) -> RelayChainResult<Option<PHeader>> {
 		(**self).header(block_id).await
 	}
+
+	async fn validation_code_hash(
+		&self,
+		relay_parent: PHash,
+		para_id: ParaId,
+		occupied_core_assumption: OccupiedCoreAssumption,
+	) -> RelayChainResult<Option<ValidationCodeHash>> {
+		(**self)
+			.validation_code_hash(relay_parent, para_id, occupied_core_assumption)
+			.await
+	}
 }
diff --git a/cumulus/client/relay-chain-rpc-interface/src/lib.rs b/cumulus/client/relay-chain-rpc-interface/src/lib.rs
index 96f8fc8b556..3a4c186e301 100644
--- a/cumulus/client/relay-chain-rpc-interface/src/lib.rs
+++ b/cumulus/client/relay-chain-rpc-interface/src/lib.rs
@@ -19,7 +19,7 @@ use core::time::Duration;
 use cumulus_primitives_core::{
 	relay_chain::{
 		CommittedCandidateReceipt, Hash as RelayHash, Header as RelayHeader, InboundHrmpMessage,
-		OccupiedCoreAssumption, SessionIndex, ValidatorId,
+		OccupiedCoreAssumption, SessionIndex, ValidationCodeHash, ValidatorId,
 	},
 	InboundDownwardMessage, ParaId, PersistedValidationData,
 };
@@ -110,6 +110,17 @@ impl RelayChainInterface for RelayChainRpcInterface {
 			.await
 	}
 
+	async fn validation_code_hash(
+		&self,
+		hash: RelayHash,
+		para_id: ParaId,
+		occupied_core_assumption: OccupiedCoreAssumption,
+	) -> RelayChainResult<Option<ValidationCodeHash>> {
+		self.rpc_client
+			.validation_code_hash(hash, para_id, occupied_core_assumption)
+			.await
+	}
+
 	async fn candidate_pending_availability(
 		&self,
 		hash: RelayHash,
diff --git a/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs b/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs
index a912997e947..6578210a259 100644
--- a/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs
+++ b/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs
@@ -647,6 +647,20 @@ impl RelayChainRpcClient {
 			.await
 	}
 
+	pub async fn validation_code_hash(
+		&self,
+		at: RelayHash,
+		para_id: ParaId,
+		occupied_core_assumption: OccupiedCoreAssumption,
+	) -> Result<Option<ValidationCodeHash>, RelayChainError> {
+		self.call_remote_runtime_function(
+			"ParachainHost_validation_code_hash",
+			at,
+			Some((para_id, occupied_core_assumption)),
+		)
+		.await
+	}
+
 	fn send_register_message_to_worker(
 		&self,
 		message: RpcDispatcherMessage,
diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs
index cc192607cea..26f20a6d48e 100644
--- a/polkadot/node/core/backing/src/lib.rs
+++ b/polkadot/node/core/backing/src/lib.rs
@@ -1887,18 +1887,20 @@ async fn background_validate_and_make_available<Context>(
 	if rp_state.awaiting_validation.insert(candidate_hash) {
 		// spawn background task.
 		let bg = async move {
-			if let Err(e) = validate_and_make_available(params).await {
-				if let Error::BackgroundValidationMpsc(error) = e {
+			if let Err(error) = validate_and_make_available(params).await {
+				if let Error::BackgroundValidationMpsc(error) = error {
 					gum::debug!(
 						target: LOG_TARGET,
+						?candidate_hash,
 						?error,
 						"Mpsc background validation mpsc died during validation- leaf no longer active?"
 					);
 				} else {
 					gum::error!(
 						target: LOG_TARGET,
-						"Failed to validate and make available: {:?}",
-						e
+						?candidate_hash,
+						?error,
+						"Failed to validate and make available",
 					);
 				}
 			}
-- 
GitLab