From 59f899a4a73ab7f378cbc3802b6853715e2f0a38 Mon Sep 17 00:00:00 2001
From: Chris Sosnin <48099298+slumber@users.noreply.github.com>
Date: Mon, 26 Sep 2022 20:02:05 +0300
Subject: [PATCH] paras: unblock offboarding when pvf-check concludes (#6032)

* Unblock offboarding for upgrading paras

* ".git/.scripts/fmt.sh" 1

Co-authored-by: command-bot <>
---
 runtime/parachains/src/paras/mod.rs   | 12 ++++--
 runtime/parachains/src/paras/tests.rs | 62 +++++++++++++++++++++++++--
 2 files changed, 68 insertions(+), 6 deletions(-)

diff --git a/runtime/parachains/src/paras/mod.rs b/runtime/parachains/src/paras/mod.rs
index b43300fdcd12..184696ffcd45 100644
--- a/runtime/parachains/src/paras/mod.rs
+++ b/runtime/parachains/src/paras/mod.rs
@@ -1570,15 +1570,21 @@ impl<T: Config> Pallet<T> {
 	///
 	/// No-op if para is not registered at all.
 	pub(crate) fn schedule_para_cleanup(id: ParaId) -> DispatchResult {
-		// Disallow offboarding in case there is an upcoming upgrade.
+		// Disallow offboarding in case there is a PVF pre-checking in progress.
 		//
 		// This is not a fundamential limitation but rather simplification: it allows us to get
 		// away without introducing additional logic for pruning and, more importantly, enacting
 		// ongoing PVF pre-checking votes. It also removes some nasty edge cases.
 		//
+		// However, an upcoming upgrade on its own imposes no restrictions. An upgrade is enacted
+		// with a new para head, so if a para never progresses we still should be able to offboard it.
+		//
 		// This implicitly assumes that the given para exists, i.e. it's lifecycle != None.
-		if FutureCodeHash::<T>::contains_key(&id) {
-			return Err(Error::<T>::CannotOffboard.into())
+		if let Some(future_code_hash) = FutureCodeHash::<T>::get(&id) {
+			let active_prechecking = PvfActiveVoteList::<T>::get();
+			if active_prechecking.contains(&future_code_hash) {
+				return Err(Error::<T>::CannotOffboard.into())
+			}
 		}
 
 		let lifecycle = ParaLifecycles::<T>::get(&id);
diff --git a/runtime/parachains/src/paras/tests.rs b/runtime/parachains/src/paras/tests.rs
index 7944bf34715b..766fa00d01b3 100644
--- a/runtime/parachains/src/paras/tests.rs
+++ b/runtime/parachains/src/paras/tests.rs
@@ -772,9 +772,6 @@ fn full_parachain_cleanup_storage() {
 			expected_at
 		};
 
-		// Cannot offboard while an upgrade is pending.
-		assert_err!(Paras::schedule_para_cleanup(para_id), Error::<Test>::CannotOffboard);
-
 		// Enact the upgrade.
 		//
 		// For that run to block #7 and submit a new head.
@@ -820,6 +817,65 @@ fn full_parachain_cleanup_storage() {
 	});
 }
 
+#[test]
+fn cannot_offboard_ongoing_pvf_check() {
+	let para_id = ParaId::from(0);
+
+	let existing_code: ValidationCode = vec![1, 2, 3].into();
+	let new_code: ValidationCode = vec![3, 2, 1].into();
+
+	let paras = vec![(
+		para_id,
+		ParaGenesisArgs {
+			parachain: true,
+			genesis_head: Default::default(),
+			validation_code: existing_code,
+		},
+	)];
+
+	let genesis_config = MockGenesisConfig {
+		paras: GenesisConfig { paras, ..Default::default() },
+		configuration: crate::configuration::GenesisConfig {
+			config: HostConfiguration { pvf_checking_enabled: true, ..Default::default() },
+			..Default::default()
+		},
+		..Default::default()
+	};
+
+	new_test_ext(genesis_config).execute_with(|| {
+		run_to_block(2, Some(vec![1]));
+
+		// Relay parent of the block that schedules the upgrade.
+		const RELAY_PARENT: BlockNumber = 1;
+		// Expected current session index.
+		const EXPECTED_SESSION: SessionIndex = 1;
+
+		Paras::schedule_code_upgrade(
+			para_id,
+			new_code.clone(),
+			RELAY_PARENT,
+			&Configuration::config(),
+		);
+		assert!(!Paras::pvfs_require_precheck().is_empty());
+
+		// Cannot offboard when there's an ongoing pvf-check voting.
+		assert_err!(Paras::schedule_para_cleanup(para_id), Error::<Test>::CannotOffboard);
+
+		// Include votes for super-majority.
+		IntoIterator::into_iter([0, 1, 2, 3])
+			.map(|i| PvfCheckStatement {
+				accept: true,
+				subject: new_code.hash(),
+				session_index: EXPECTED_SESSION,
+				validator_index: i.into(),
+			})
+			.for_each(sign_and_include_pvf_check_statement);
+
+		// Voting concluded, can offboard even though an upgrade is in progress.
+		assert_ok!(Paras::schedule_para_cleanup(para_id));
+	});
+}
+
 #[test]
 fn para_incoming_at_session() {
 	let code_a = ValidationCode(vec![2]);