Fix parachain upgrade scheduling when done by the owner/root (paritytech#3341)

When using `schedule_code_upgrade` to change the code of a parachain in
the relay chain runtime, we had already fixed to not set the `GoAhead`
signal. This was done to not brick any parachain after the upgrade,
because they were seeing the signal without having any upgrade prepared.
The remaining problem is that the parachain code is only upgraded after
a parachain header was enacted, aka the parachain made some progress.
However, this is quite complicated if the parachain is bricked (which is
the most common scenario why to manually schedule a code upgrade). Thus,
this pull request replaces `SetGoAhead` with `UpgradeStrategy` to signal
to the logic kind of strategy want to use. The strategies are either
`SetGoAheadSignal` or `ApplyAtExpectedBlock`. `SetGoAheadSignal` sets
the go ahead signal as before and awaits a parachain block.
`ApplyAtExpectedBlock` schedules the upgrade and applies it directly at
the `expected_block` without waiting for the parachain to make any kind
of progress.

Author: bkchr

polkadot/runtime/common/src/paras_registrar/mod.rs

@@ -29,7 +29,7 @@ use frame_system::{self, ensure_root, ensure_signed};
 use primitives::{HeadData, Id as ParaId, ValidationCode, LOWEST_PUBLIC_ID, MIN_CODE_SIZE};
 use runtime_parachains::{
 	configuration, ensure_parachain,
-	paras::{self, ParaGenesisArgs, SetGoAhead},
+	paras::{self, ParaGenesisArgs, UpgradeStrategy},
 	Origin, ParaLifecycle,
 };
 use sp_std::{prelude::*, result};
@@ -408,6 +408,13 @@ pub mod pallet {
 
 		/// Schedule a parachain upgrade.
 		///
+		/// This will kick off a check of `new_code` by all validators. After the majority of the
+		/// validators have reported on the validity of the code, the code will either be enacted
+		/// or the upgrade will be rejected. If the code will be enacted, the current code of the
+		/// parachain will be overwritten directly. This means that any PoV will be checked by this
+		/// new code. The parachain itself will not be informed explictely that the validation code
+		/// has changed.
+		///
 		/// Can be called by Root, the parachain, or the parachain manager if the parachain is
 		/// unlocked.
 		#[pallet::call_index(7)]
@@ -418,7 +425,11 @@ pub mod pallet {
 			new_code: ValidationCode,
 		) -> DispatchResult {
 			Self::ensure_root_para_or_owner(origin, para)?;
-			runtime_parachains::schedule_code_upgrade::<T>(para, new_code, SetGoAhead::No)?;
+			runtime_parachains::schedule_code_upgrade::<T>(
+				para,
+				new_code,
+				UpgradeStrategy::ApplyAtExpectedBlock,
+			)?;
 			Ok(())
 		}
 

polkadot/runtime/parachains/src/inclusion/mod.rs

@@ -22,7 +22,7 @@
 use crate::{
 	configuration::{self, HostConfiguration},
 	disputes, dmp, hrmp,
-	paras::{self, SetGoAhead},
+	paras::{self, UpgradeStrategy},
 	scheduler,
 	shared::{self, AllowedRelayParentsTracker},
 	util::make_persisted_validation_data_with_parent,
@@ -839,7 +839,7 @@ impl<T: Config> Pallet<T> {
 				new_code,
 				now,
 				&config,
-				SetGoAhead::Yes,
+				UpgradeStrategy::SetGoAheadSignal,
 			));
 		}
 

polkadot/runtime/parachains/src/inclusion/tests.rs

@@ -1590,7 +1590,7 @@ fn candidate_checks() {
 					vec![9, 8, 7, 6, 5, 4, 3, 2, 1].into(),
 					expected_at,
 					&cfg,
-					SetGoAhead::Yes,
+					UpgradeStrategy::SetGoAheadSignal,
 				);
 			}
 
@@ -2857,7 +2857,7 @@ fn para_upgrade_delay_scheduled_from_inclusion() {
 		let cause = &active_vote_state.causes()[0];
 		// Upgrade block is the block of inclusion, not candidate's parent.
 		assert_matches!(cause,
-			paras::PvfCheckCause::Upgrade { id, included_at, set_go_ahead: SetGoAhead::Yes }
+			paras::PvfCheckCause::Upgrade { id, included_at, upgrade_strategy: UpgradeStrategy::SetGoAheadSignal }
 				if id == &chain_a && included_at == &7
 		);
 	});

polkadot/runtime/parachains/src/lib.rs

@@ -54,7 +54,7 @@ mod mock;
 mod ump_tests;
 
 pub use origin::{ensure_parachain, Origin};
-pub use paras::{ParaLifecycle, SetGoAhead};
+pub use paras::{ParaLifecycle, UpgradeStrategy};
 use primitives::{HeadData, Id as ParaId, ValidationCode};
 use sp_runtime::{DispatchResult, FixedU128};
 
@@ -104,7 +104,7 @@ pub fn schedule_parachain_downgrade<T: paras::Config>(id: ParaId) -> Result<(),
 pub fn schedule_code_upgrade<T: paras::Config>(
 	id: ParaId,
 	new_code: ValidationCode,
-	set_go_ahead: SetGoAhead,
+	set_go_ahead: UpgradeStrategy,
 ) -> DispatchResult {
 	paras::Pallet::<T>::schedule_code_upgrade_external(id, new_code, set_go_ahead)
 }

polkadot/runtime/parachains/src/paras/benchmarking.rs

@@ -129,7 +129,7 @@ benchmarks! {
 			ValidationCode(vec![0]),
 			expired,
 			&config,
-			SetGoAhead::Yes,
+			UpgradeStrategy::SetGoAheadSignal,
 		);
 	}: _(RawOrigin::Root, para_id, new_head)
 	verify {

polkadot/runtime/parachains/src/paras/benchmarking/pvf_check.rs

@@ -177,7 +177,7 @@ where
 				validation_code,
 				/* relay_parent_number */ 1u32.into(),
 				&configuration::Pallet::<T>::config(),
-				SetGoAhead::Yes,
+				UpgradeStrategy::SetGoAheadSignal,
 			);
 		} else {
 			let r = Pallet::<T>::schedule_para_initialize(