Migrate frame-system to pallet attribute macro (#7898)

* PRINT_PALLET_UPGRADE=1 cargo check -p frame-system

* Copy attributes, imports, mods and type defs

* Copy Config trait

* Annotate constants

* Tabify

* Migrate hooks

* Upgrade template rename interface to hooks

* Migrate pallet call

* Migrate Event

* Migrate Error

* Migrate Origin

* Remove optional validate_unsigned

* Remove remaining TODO_MAYBE_WHERE_CLAUSE

* Overwrite original lib.rs with migrated lib2.rs.

* Add required Event IsType constraint

* Add disable supertrait check

* Fix leftover Trait trait

* Add missing pallet prefix for weight attributes

* Add missing Error type parameter

* Add missing Hooks type parameter

* Private call visibility, restore original helper types and helpers etc

* Fix hooks type parameter

* Rename RawEvent to Event

* Add missing storage type annotations

* Remove unused imports

* Add GenesisConfig helpers for compat

* Fix unused import warnings

* Update frame/support/procedural/src/storage/print_pallet_upgrade.rs

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* Fix test errors and warnings

* Fix remaining errors and warnings

* Apply review suggestion: fix formatting

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* Apply review suggestion: annotate BlockLength as constant

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* Apply review suggestion: add triling comma

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* Apply review suggestion: add triling comma

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* Apply review suggestion: add trailing comma

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* Apply review suggestion: fix storage type indentation

* Apply review suggestion: remove redundant Origin type alias

* Add missing codec derives for BlockLength

* Restore module docs

* Module -> Pallet renamel

* Revert "Update frame/support/procedural/src/storage/print_pallet_upgrade.rs"

This reverts commit d2a2d5b6

* Apply review suggestion: merge crate imports

Co-authored-by: Alexander Theißen <alex.theissen@me.com>

* Revert "Upgrade template rename interface to hooks"

This reverts commit 306f0239

* Single line import

* Refactor generated genesis build

* Import sp_io::storage

* Revert previous, fully qualify sp_io::storage

* Fix ui tests

* Fix errors after merge, missing changes

* Set UpgradedToDualRefCount to true in genesis build

* Annotated Runtime version with constant, exposing it via metadata

* Add metadata attribute

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Co-authored-by: Alexander Theißen <alex.theissen@me.com>

substrate/bin/node/executor/tests/basic.rs

@@ -329,7 +329,7 @@ fn full_native_block_import_works() {
 		let events = vec![
 			EventRecord {
 				phase: Phase::ApplyExtrinsic(0),
-				event: Event::frame_system(frame_system::RawEvent::ExtrinsicSuccess(
+				event: Event::frame_system(frame_system::Event::ExtrinsicSuccess(
 					DispatchInfo { weight: timestamp_weight, class: DispatchClass::Mandatory, ..Default::default() }
 				)),
 				topics: vec![],
@@ -350,7 +350,7 @@ fn full_native_block_import_works() {
 			},
 			EventRecord {
 				phase: Phase::ApplyExtrinsic(1),
-				event: Event::frame_system(frame_system::RawEvent::ExtrinsicSuccess(
+				event: Event::frame_system(frame_system::Event::ExtrinsicSuccess(
 					DispatchInfo { weight: transfer_weight, ..Default::default() }
 				)),
 				topics: vec![],
@@ -381,7 +381,7 @@ fn full_native_block_import_works() {
 		let events = vec![
 			EventRecord {
 				phase: Phase::ApplyExtrinsic(0),
-				event: Event::frame_system(frame_system::RawEvent::ExtrinsicSuccess(
+				event: Event::frame_system(frame_system::Event::ExtrinsicSuccess(
 					DispatchInfo { weight: timestamp_weight, class: DispatchClass::Mandatory, ..Default::default() }
 				)),
 				topics: vec![],
@@ -404,7 +404,7 @@ fn full_native_block_import_works() {
 			},
 			EventRecord {
 				phase: Phase::ApplyExtrinsic(1),
-				event: Event::frame_system(frame_system::RawEvent::ExtrinsicSuccess(
+				event: Event::frame_system(frame_system::Event::ExtrinsicSuccess(
 					DispatchInfo { weight: transfer_weight, ..Default::default() }
 				)),
 				topics: vec![],
@@ -427,7 +427,7 @@ fn full_native_block_import_works() {
 			},
 			EventRecord {
 				phase: Phase::ApplyExtrinsic(2),
-				event: Event::frame_system(frame_system::RawEvent::ExtrinsicSuccess(
+				event: Event::frame_system(frame_system::Event::ExtrinsicSuccess(
 					DispatchInfo { weight: transfer_weight, ..Default::default() }
 				)),
 				topics: vec![],

substrate/bin/node/executor/tests/fees.rs

@@ -17,7 +17,7 @@
 
 use codec::{Encode, Joiner};
 use frame_support::{
-	StorageValue, StorageMap,
+	StorageValue,
 	traits::Currency,
 	weights::{GetDispatchInfo, constants::ExtrinsicBaseWeight, IdentityFee, WeightToFeePolynomial},
 };

substrate/bin/node/executor/tests/submit_transaction.rs

@@ -217,7 +217,6 @@ fn should_submit_signed_twice_from_all_accounts() {
 #[test]
 fn submitted_transaction_should_be_valid() {
 	use codec::Encode;
-	use frame_support::storage::StorageMap;
 	use sp_runtime::transaction_validity::{TransactionSource, TransactionTag};
 	use sp_runtime::traits::StaticLookup;
 

substrate/frame/balances/src/tests.rs

@@ -737,7 +737,7 @@ macro_rules! decl_tests {
 					assert_eq!(
 						events(),
 						[
-							Event::system(system::RawEvent::NewAccount(1)),
+							Event::system(system::Event::NewAccount(1)),
 							Event::balances(RawEvent::Endowed(1, 100)),
 							Event::balances(RawEvent::BalanceSet(1, 100, 0)),
 						]
@@ -749,7 +749,7 @@ macro_rules! decl_tests {
 						events(),
 						[
 							Event::balances(RawEvent::DustLost(1, 99)),
-							Event::system(system::RawEvent::KilledAccount(1))
+							Event::system(system::Event::KilledAccount(1))
 						]
 					);
 				});
@@ -766,7 +766,7 @@ macro_rules! decl_tests {
 					assert_eq!(
 						events(),
 						[
-							Event::system(system::RawEvent::NewAccount(1)),
+							Event::system(system::Event::NewAccount(1)),
 							Event::balances(RawEvent::Endowed(1, 100)),
 							Event::balances(RawEvent::BalanceSet(1, 100, 0)),
 						]
@@ -777,7 +777,7 @@ macro_rules! decl_tests {
 					assert_eq!(
 						events(),
 						[
-							Event::system(system::RawEvent::KilledAccount(1))
+							Event::system(system::Event::KilledAccount(1))
 						]
 					);
 				});

substrate/frame/balances/src/tests_local.rs

@@ -170,7 +170,7 @@ fn emit_events_with_no_existential_deposit_suicide_with_dust() {
 			assert_eq!(
 				events(),
 				[
-					Event::system(system::RawEvent::NewAccount(1)),
+					Event::system(system::Event::NewAccount(1)),
 					Event::balances(RawEvent::Endowed(1, 100)),
 					Event::balances(RawEvent::BalanceSet(1, 100, 0)),
 				]
@@ -187,7 +187,7 @@ fn emit_events_with_no_existential_deposit_suicide_with_dust() {
 				events(),
 				[
 					Event::balances(RawEvent::DustLost(1, 1)),
-					Event::system(system::RawEvent::KilledAccount(1))
+					Event::system(system::Event::KilledAccount(1))
 				]
 			);
 		});

substrate/frame/contracts/src/tests.rs

@@ -475,7 +475,7 @@ fn instantiate_and_call_and_deposit_event() {
 			pretty_assertions::assert_eq!(System::events(), vec![
 				EventRecord {
 					phase: Phase::Initialization,
-					event: MetaEvent::system(frame_system::RawEvent::NewAccount(ALICE.clone())),
+					event: MetaEvent::system(frame_system::Event::NewAccount(ALICE.clone())),
 					topics: vec![],
 				},
 				EventRecord {
@@ -492,7 +492,7 @@ fn instantiate_and_call_and_deposit_event() {
 				},
 				EventRecord {
 					phase: Phase::Initialization,
-					event: MetaEvent::system(frame_system::RawEvent::NewAccount(addr.clone())),
+					event: MetaEvent::system(frame_system::Event::NewAccount(addr.clone())),
 					topics: vec![],
 				},
 				EventRecord {
@@ -653,7 +653,7 @@ fn test_set_rent_code_and_hash() {
 			assert_eq!(System::events(), vec![
 				EventRecord {
 					phase: Phase::Initialization,
-					event: MetaEvent::system(frame_system::RawEvent::NewAccount(ALICE)),
+					event: MetaEvent::system(frame_system::Event::NewAccount(ALICE)),
 					topics: vec![],
 				},
 				EventRecord {
@@ -1235,7 +1235,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage:
 			assert_eq!(System::events(), vec![
 				EventRecord {
 					phase: Phase::Initialization,
-					event: MetaEvent::system(frame_system::RawEvent::NewAccount(ALICE)),
+					event: MetaEvent::system(frame_system::Event::NewAccount(ALICE)),
 					topics: vec![],
 				},
 				EventRecord {
@@ -1390,7 +1390,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage:
 							},
 							EventRecord {
 								phase: Phase::Initialization,
-								event: MetaEvent::system(frame_system::RawEvent::NewAccount(CHARLIE)),
+								event: MetaEvent::system(frame_system::Event::NewAccount(CHARLIE)),
 								topics: vec![],
 							},
 							EventRecord {
@@ -1400,7 +1400,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage:
 							},
 							EventRecord {
 								phase: Phase::Initialization,
-								event: MetaEvent::system(frame_system::RawEvent::NewAccount(addr_django.clone())),
+								event: MetaEvent::system(frame_system::Event::NewAccount(addr_django.clone())),
 								topics: vec![],
 							},
 							EventRecord {
@@ -1440,7 +1440,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage:
 				assert_eq!(System::events(), vec![
 					EventRecord {
 						phase: Phase::Initialization,
-						event: MetaEvent::system(system::RawEvent::KilledAccount(addr_django.clone())),
+						event: MetaEvent::system(system::Event::KilledAccount(addr_django.clone())),
 						topics: vec![],
 					},
 					EventRecord {

substrate/frame/executive/src/lib.rs

@@ -117,7 +117,7 @@
 
 use sp_std::{prelude::*, marker::PhantomData};
 use frame_support::{
-	StorageValue, StorageMap, weights::{GetDispatchInfo, DispatchInfo, DispatchClass},
+	weights::{GetDispatchInfo, DispatchInfo, DispatchClass},
 	traits::{OnInitialize, OnFinalize, OnRuntimeUpgrade, OffchainWorker},
 	dispatch::PostDispatchInfo,
 };
@@ -261,11 +261,11 @@ where
 
 	/// Returns if the runtime was upgraded since the last time this function was called.
 	fn runtime_upgraded() -> bool {
-		let last = frame_system::LastRuntimeUpgrade::get();
+		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::put(
+			frame_system::LastRuntimeUpgrade::<System>::put(
 				frame_system::LastRuntimeUpgradeInfo::from(current),
 			);
 			true
@@ -998,7 +998,7 @@ mod tests {
 		new_test_ext(1).execute_with(|| {
 			RUNTIME_VERSION.with(|v| *v.borrow_mut() = Default::default());
 			// It should be added at genesis
-			assert!(frame_system::LastRuntimeUpgrade::exists());
+			assert!(frame_system::LastRuntimeUpgrade::<Runtime>::exists());
 			assert!(!Executive::runtime_upgraded());
 
 			RUNTIME_VERSION.with(|v| *v.borrow_mut() = sp_version::RuntimeVersion {
@@ -1008,7 +1008,7 @@ mod tests {
 			assert!(Executive::runtime_upgraded());
 			assert_eq!(
 				Some(LastRuntimeUpgradeInfo { spec_version: 1.into(), spec_name: "".into() }),
-				frame_system::LastRuntimeUpgrade::get(),
+				frame_system::LastRuntimeUpgrade::<Runtime>::get(),
 			);
 
 			RUNTIME_VERSION.with(|v| *v.borrow_mut() = sp_version::RuntimeVersion {
@@ -1019,7 +1019,7 @@ mod tests {
 			assert!(Executive::runtime_upgraded());
 			assert_eq!(
 				Some(LastRuntimeUpgradeInfo { spec_version: 1.into(), spec_name: "test".into() }),
-				frame_system::LastRuntimeUpgrade::get(),
+				frame_system::LastRuntimeUpgrade::<Runtime>::get(),
 			);
 
 			RUNTIME_VERSION.with(|v| *v.borrow_mut() = sp_version::RuntimeVersion {
@@ -1030,11 +1030,11 @@ mod tests {
 			});
 			assert!(!Executive::runtime_upgraded());
 
-			frame_system::LastRuntimeUpgrade::take();
+			frame_system::LastRuntimeUpgrade::<Runtime>::take();
 			assert!(Executive::runtime_upgraded());
 			assert_eq!(
 				Some(LastRuntimeUpgradeInfo { spec_version: 1.into(), spec_name: "test".into() }),
-				frame_system::LastRuntimeUpgrade::get(),
+				frame_system::LastRuntimeUpgrade::<Runtime>::get(),
 			);
 		})
 	}

substrate/frame/session/benchmarking/src/lib.rs

@@ -28,7 +28,7 @@ use sp_std::vec;
 use frame_benchmarking::benchmarks;
 use frame_support::{
 	codec::Decode,
-	storage::{StorageValue, StorageMap},
+	storage::StorageValue,
 	traits::{KeyOwnerProofSystem, OnInitialize},
 };
 use frame_system::RawOrigin;

substrate/frame/system/src/extensions/check_mortality.rs

@@ -17,7 +17,6 @@
 
 use codec::{Encode, Decode};
 use crate::{Config, Module, BlockHash};
-use frame_support::StorageMap;
 use sp_runtime::{
 	generic::Era,
 	traits::{SignedExtension, DispatchInfoOf, SaturatedConversion},

substrate/frame/system/src/extensions/check_nonce.rs

@@ -17,10 +17,7 @@
 
 use codec::{Encode, Decode};
 use crate::Config;
-use frame_support::{
-	weights::DispatchInfo,
-	StorageMap,
-};
+use frame_support::weights::DispatchInfo;
 use sp_runtime::{
 	traits::{SignedExtension, DispatchInfoOf, Dispatchable, One},
 	transaction_validity::{

substrate/frame/system/src/extensions/check_weight.rs

@@ -28,7 +28,6 @@ use sp_runtime::{
 use frame_support::{
 	traits::{Get},
 	weights::{PostDispatchInfo, DispatchInfo, DispatchClass, priority::FrameTransactionPriority},
-	StorageValue,
 };
 
 /// Block resource (weight) limit check.
@@ -115,8 +114,8 @@ impl<T: Config + Send + Sync> CheckWeight<T> where
 		let next_weight = Self::check_block_weight(info)?;
 		Self::check_extrinsic_weight(info)?;
 
-		crate::AllExtrinsicsLen::put(next_len);
-		crate::BlockWeight::put(next_weight);
+		crate::AllExtrinsicsLen::<T>::put(next_len);
+		crate::BlockWeight::<T>::put(next_weight);
 		Ok(())
 	}
 
@@ -257,7 +256,7 @@ impl<T: Config + Send + Sync> SignedExtension for CheckWeight<T> where
 
 		let unspent = post_info.calc_unspent(info);
 		if unspent > 0 {
-			crate::BlockWeight::mutate(|current_weight| {
+			crate::BlockWeight::<T>::mutate(|current_weight| {
 				current_weight.sub(unspent, info.class);
 			})
 		}
@@ -465,7 +464,7 @@ mod tests {
 			let normal_limit = normal_weight_limit();
 
 			// given almost full block
-			BlockWeight::mutate(|current_weight| {
+			BlockWeight::<Test>::mutate(|current_weight| {
 				current_weight.set(normal_limit, DispatchClass::Normal)
 			});
 			// will not fit.
@@ -475,7 +474,7 @@ mod tests {
 
 			// likewise for length limit.
 			let len = 100_usize;
-			AllExtrinsicsLen::put(normal_length_limit());
+			AllExtrinsicsLen::<Test>::put(normal_length_limit());
 			assert!(CheckWeight::<Test>(PhantomData).pre_dispatch(&1, CALL, &normal, len).is_err());
 			assert!(CheckWeight::<Test>(PhantomData).pre_dispatch(&1, CALL, &op, len).is_ok());
 		})
@@ -508,7 +507,7 @@ mod tests {
 			let normal = DispatchInfo::default();
 			let normal_limit = normal_weight_limit() as usize;
 			let reset_check_weight = |tx, s, f| {
-				AllExtrinsicsLen::put(0);
+				AllExtrinsicsLen::<Test>::put(0);
 				let r = CheckWeight::<Test>(PhantomData).pre_dispatch(&1, CALL, tx, s);
 				if f { assert!(r.is_err()) } else { assert!(r.is_ok()) }
 			};
@@ -544,7 +543,7 @@ mod tests {
 			let len = 0_usize;
 
 			let reset_check_weight = |i, f, s| {
-				BlockWeight::mutate(|current_weight| {
+				BlockWeight::<Test>::mutate(|current_weight| {
 					current_weight.set(s, DispatchClass::Normal)
 				});
 				let r = CheckWeight::<Test>(PhantomData).pre_dispatch(&1, CALL, i, len);
@@ -570,20 +569,20 @@ mod tests {
 			let base_extrinsic = block_weights().get(DispatchClass::Normal).base_extrinsic;
 
 			// We allow 75% for normal transaction, so we put 25% - extrinsic base weight
-			BlockWeight::mutate(|current_weight| {
+			BlockWeight::<Test>::mutate(|current_weight| {
 				current_weight.set(0, DispatchClass::Mandatory);
 				current_weight.set(256 - base_extrinsic, DispatchClass::Normal);
 			});
 
 			let pre = CheckWeight::<Test>(PhantomData).pre_dispatch(&1, CALL, &info, len).unwrap();
-			assert_eq!(BlockWeight::get().total(), info.weight + 256);
+			assert_eq!(BlockWeight::<Test>::get().total(), info.weight + 256);
 
 			assert!(
 				CheckWeight::<Test>::post_dispatch(pre, &info, &post_info, len, &Ok(()))
 				.is_ok()
 			);
 			assert_eq!(
-				BlockWeight::get().total(),
+				BlockWeight::<Test>::get().total(),
 				post_info.actual_weight.unwrap() + 256,
 			);
 		})
@@ -599,14 +598,14 @@ mod tests {
 			};
 			let len = 0_usize;
 
-			BlockWeight::mutate(|current_weight| {
+			BlockWeight::<Test>::mutate(|current_weight| {
 				current_weight.set(0, DispatchClass::Mandatory);
 				current_weight.set(128, DispatchClass::Normal);
 			});
 
 			let pre = CheckWeight::<Test>(PhantomData).pre_dispatch(&1, CALL, &info, len).unwrap();
 			assert_eq!(
-				BlockWeight::get().total(),
+				BlockWeight::<Test>::get().total(),
 				info.weight + 128 + block_weights().get(DispatchClass::Normal).base_extrinsic,
 			);
 
@@ -615,7 +614,7 @@ mod tests {
 				.is_ok()
 			);
 			assert_eq!(
-				BlockWeight::get().total(),
+				BlockWeight::<Test>::get().total(),
 				info.weight + 128 + block_weights().get(DispatchClass::Normal).base_extrinsic,
 			);
 		})

substrate/frame/system/src/limits.rs

@@ -29,7 +29,7 @@ use frame_support::weights::{Weight, DispatchClass, constants, PerDispatchClass,
 use sp_runtime::{RuntimeDebug, Perbill};
 
 /// Block length limit configuration.
-#[derive(RuntimeDebug, Clone)]
+#[derive(RuntimeDebug, Clone, codec::Encode, codec::Decode)]
 pub struct BlockLength {
 	/// Maximal total length in bytes for each extrinsic class.
 	///

substrate/frame/system/src/offchain.rs

@@ -63,7 +63,7 @@ use sp_std::convert::{TryInto, TryFrom};
 use sp_std::prelude::{Box, Vec};
 use sp_runtime::app_crypto::RuntimeAppPublic;
 use sp_runtime::traits::{Extrinsic as ExtrinsicT, IdentifyAccount, One};
-use frame_support::{debug, storage::StorageMap, RuntimeDebug};
+use frame_support::{debug, RuntimeDebug};
 
 /// Marker struct used to flag using all supported keys to sign a payload.
 pub struct ForAll {}

substrate/frame/system/src/tests.rs

@@ -18,8 +18,11 @@
 use crate::*;
 use mock::{*, Origin};
 use sp_core::H256;
-use sp_runtime::{DispatchError, traits::{Header, BlakeTwo256}};
-use frame_support::weights::WithPostDispatchInfo;
+use sp_runtime::{DispatchError, DispatchErrorWithPostInfo, traits::{Header, BlakeTwo256}};
+use frame_support::{
+	weights::WithPostDispatchInfo,
+	dispatch::PostDispatchInfo,
+};
 
 #[test]
 fn origin_works() {
@@ -329,7 +332,7 @@ fn set_code_checks_works() {
 		("test", 1, 2, Err(Error::<Test>::SpecVersionNeedsToIncrease)),
 		("test", 1, 1, Err(Error::<Test>::SpecVersionNeedsToIncrease)),
 		("test2", 1, 1, Err(Error::<Test>::InvalidSpecName)),
-		("test", 2, 1, Ok(())),
+		("test", 2, 1, Ok(PostDispatchInfo::default())),
 		("test", 0, 1, Err(Error::<Test>::SpecVersionNeedsToIncrease)),
 		("test", 1, 0, Err(Error::<Test>::SpecVersionNeedsToIncrease)),
 	];
@@ -351,7 +354,7 @@ fn set_code_checks_works() {
 				vec![1, 2, 3, 4],
 			);
 
-			assert_eq!(expected.map_err(DispatchError::from), res);
+			assert_eq!(expected.map_err(DispatchErrorWithPostInfo::from), res);
 		});
 	}
 }

substrate/frame/transaction-payment/src/lib.rs

@@ -1169,7 +1169,7 @@ mod tests {
 			}));
 			// Killed Event
 			assert!(System::events().iter().any(|event| {
-				event.event == Event::system(system::RawEvent::KilledAccount(2))
+				event.event == Event::system(system::Event::KilledAccount(2))
 			}));
 		});
 	}