frame-system: Add last_runtime_upgrade_spec_version (#2351)

Adds a function for querying the last runtime upgrade spec version. This
can be useful for when writing runtime level migrations to ensure that
they are not executed multiple times. An example would be a session key
migration.

---------

Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Cargo.lock

@@ -5904,6 +5904,7 @@ version = "4.0.0-dev"
 dependencies = [
  "cfg-if",
  "criterion 0.4.0",
+ "docify",
  "frame-support",
  "log",
  "parity-scale-codec",

substrate/frame/executive/src/lib.rs

@@ -487,6 +487,12 @@ where
 		let mut weight = Weight::zero();
 		if Self::runtime_upgraded() {
 			weight = weight.saturating_add(Self::execute_on_runtime_upgrade());
+
+			frame_system::LastRuntimeUpgrade::<System>::put(
+				frame_system::LastRuntimeUpgradeInfo::from(
+					<System::Version as frame_support::traits::Get<_>>::get(),
+				),
+			);
 		}
 		<frame_system::Pallet<System>>::initialize(block_number, parent_hash, digest);
 		weight = weight.saturating_add(<AllPalletsWithSystem as OnInitialize<
@@ -503,19 +509,12 @@ where
 		frame_system::Pallet::<System>::note_finished_initialize();
 	}
 
-	/// Returns if the runtime was upgraded since the last time this function was called.
+	/// Returns if the runtime has been upgraded, based on [`frame_system::LastRuntimeUpgrade`].
 	fn runtime_upgraded() -> bool {
 		let last = frame_system::LastRuntimeUpgrade::<System>::get();
 		let current = <System::Version as frame_support::traits::Get<_>>::get();
 
-		if last.map(|v| v.was_upgraded(&current)).unwrap_or(true) {
-			frame_system::LastRuntimeUpgrade::<System>::put(
-				frame_system::LastRuntimeUpgradeInfo::from(current),
-			);
-			true
-		} else {
-			false
-		}
+		last.map(|v| v.was_upgraded(&current)).unwrap_or(true)
 	}
 
 	fn initial_checks(block: &Block) {
@@ -755,7 +754,7 @@ mod tests {
 		traits::{fungible, ConstU32, ConstU64, ConstU8, Currency},
 		weights::{ConstantMultiplier, IdentityFee, RuntimeDbWeight, Weight, WeightToFee},
 	};
-	use frame_system::{ChainContext, LastRuntimeUpgradeInfo};
+	use frame_system::{ChainContext, LastRuntimeUpgrade, LastRuntimeUpgradeInfo};
 	use pallet_balances::Call as BalancesCall;
 	use pallet_transaction_payment::CurrencyAdapter;
 
@@ -994,6 +993,9 @@ mod tests {
 			sp_io::storage::set(TEST_KEY, "custom_upgrade".as_bytes());
 			sp_io::storage::set(CUSTOM_ON_RUNTIME_KEY, &true.encode());
 			System::deposit_event(frame_system::Event::CodeUpdated);
+
+			assert_eq!(0, System::last_runtime_upgrade_spec_version());
+
 			Weight::from_parts(100, 0)
 		}
 	}
@@ -1356,17 +1358,13 @@ mod tests {
 		new_test_ext(1).execute_with(|| {
 			RuntimeVersionTestValues::mutate(|v| *v = Default::default());
 			// It should be added at genesis
-			assert!(frame_system::LastRuntimeUpgrade::<Runtime>::exists());
+			assert!(LastRuntimeUpgrade::<Runtime>::exists());
 			assert!(!Executive::runtime_upgraded());
 
 			RuntimeVersionTestValues::mutate(|v| {
 				*v = sp_version::RuntimeVersion { spec_version: 1, ..Default::default() }
 			});
 			assert!(Executive::runtime_upgraded());
-			assert_eq!(
-				Some(LastRuntimeUpgradeInfo { spec_version: 1.into(), spec_name: "".into() }),
-				frame_system::LastRuntimeUpgrade::<Runtime>::get(),
-			);
 
 			RuntimeVersionTestValues::mutate(|v| {
 				*v = sp_version::RuntimeVersion {
@@ -1376,27 +1374,18 @@ mod tests {
 				}
 			});
 			assert!(Executive::runtime_upgraded());
-			assert_eq!(
-				Some(LastRuntimeUpgradeInfo { spec_version: 1.into(), spec_name: "test".into() }),
-				frame_system::LastRuntimeUpgrade::<Runtime>::get(),
-			);
 
 			RuntimeVersionTestValues::mutate(|v| {
 				*v = sp_version::RuntimeVersion {
-					spec_version: 1,
-					spec_name: "test".into(),
+					spec_version: 0,
 					impl_version: 2,
 					..Default::default()
 				}
 			});
 			assert!(!Executive::runtime_upgraded());
 
-			frame_system::LastRuntimeUpgrade::<Runtime>::take();
+			LastRuntimeUpgrade::<Runtime>::take();
 			assert!(Executive::runtime_upgraded());
-			assert_eq!(
-				Some(LastRuntimeUpgradeInfo { spec_version: 1.into(), spec_name: "test".into() }),
-				frame_system::LastRuntimeUpgrade::<Runtime>::get(),
-			);
 		})
 	}
 
@@ -1444,6 +1433,10 @@ mod tests {
 
 			assert_eq!(&sp_io::storage::get(TEST_KEY).unwrap()[..], *b"module");
 			assert_eq!(sp_io::storage::get(CUSTOM_ON_RUNTIME_KEY).unwrap(), true.encode());
+			assert_eq!(
+				Some(RuntimeVersionTestValues::get().into()),
+				LastRuntimeUpgrade::<Runtime>::get(),
+			)
 		});
 	}
 
@@ -1519,6 +1512,9 @@ mod tests {
 
 	#[test]
 	fn all_weights_are_recorded_correctly() {
+		// Reset to get the correct new genesis below.
+		RuntimeVersionTestValues::take();
+
 		new_test_ext(1).execute_with(|| {
 			// Make sure `on_runtime_upgrade` is called for maximum complexity
 			RuntimeVersionTestValues::mutate(|v| {
@@ -1535,6 +1531,10 @@ mod tests {
 				Digest::default(),
 			));
 
+			// Reset the last runtime upgrade info, to make the second call to `on_runtime_upgrade`
+			// succeed.
+			LastRuntimeUpgrade::<Runtime>::take();
+
 			// All weights that show up in the `initialize_block_impl`
 			let custom_runtime_upgrade_weight = CustomOnRuntimeUpgrade::on_runtime_upgrade();
 			let runtime_upgrade_weight =

substrate/frame/system/Cargo.toml

@@ -25,6 +25,7 @@ sp-runtime = { path = "../../primitives/runtime", default-features = false, feat
 sp-std = { path = "../../primitives/std", default-features = false}
 sp-version = { path = "../../primitives/version", default-features = false, features = ["serde"] }
 sp-weights = { path = "../../primitives/weights", default-features = false, features = ["serde"] }
+docify = "0.2.0"
 
 [dev-dependencies]
 criterion = "0.4.0"

substrate/frame/system/src/lib.rs

@@ -1094,6 +1094,25 @@ pub enum DecRefStatus {
 }
 
 impl<T: Config> Pallet<T> {
+	/// Returns the `spec_version` of the last runtime upgrade.
+	///
+	/// This function is useful for writing guarded runtime migrations in the runtime. A runtime
+	/// migration can use the `spec_version` to ensure that it isn't applied twice. This works
+	/// similar as the storage version for pallets.
+	///
+	/// This functions returns the `spec_version` of the last runtime upgrade while executing the
+	/// runtime migrations
+	/// [`on_runtime_upgrade`](frame_support::traits::OnRuntimeUpgrade::on_runtime_upgrade)
+	/// function. After all migrations are executed, this will return the `spec_version` of the
+	/// current runtime until there is another runtime upgrade.
+	///
+	/// Example:
+	#[doc = docify::embed!("src/tests.rs", last_runtime_upgrade_spec_version_usage)]
+	pub fn last_runtime_upgrade_spec_version() -> u32 {
+		LastRuntimeUpgrade::<T>::get().map_or(0, |l| l.spec_version.0)
+	}
+
+	/// Returns true if the given account exists.
 	pub fn account_exists(who: &T::AccountId) -> bool {
 		Account::<T>::contains_key(who)
 	}

substrate/frame/system/src/tests.rs

@@ -19,7 +19,7 @@ use crate::*;
 use frame_support::{
 	assert_noop, assert_ok,
 	dispatch::{Pays, PostDispatchInfo, WithPostDispatchInfo},
-	traits::WhitelistedStorageKeys,
+	traits::{OnRuntimeUpgrade, WhitelistedStorageKeys},
 };
 use std::collections::BTreeSet;
 
@@ -773,3 +773,26 @@ pub fn from_actual_ref_time(ref_time: Option<u64>) -> PostDispatchInfo {
 pub fn from_post_weight_info(ref_time: Option<u64>, pays_fee: Pays) -> PostDispatchInfo {
 	PostDispatchInfo { actual_weight: ref_time.map(|t| Weight::from_all(t)), pays_fee }
 }
+
+#[docify::export]
+#[test]
+fn last_runtime_upgrade_spec_version_usage() {
+	struct Migration;
+
+	impl OnRuntimeUpgrade for Migration {
+		fn on_runtime_upgrade() -> Weight {
+			// Ensure to compare the spec version against some static version to prevent applying
+			// the same migration multiple times.
+			//
+			// `1337` here is the spec version of the runtime running on chain. If there is maybe
+			// a runtime upgrade in the pipeline of being applied, you should use the spec version
+			// of this upgrade.
+			if System::last_runtime_upgrade_spec_version() > 1337 {
+				return Weight::zero();
+			}
+
+			// Do the migration.
+			Weight::zero()
+		}
+	}
+}