From 5b89f47df2ea5c6c2966d18fef0de9d142963e99 Mon Sep 17 00:00:00 2001
From: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Date: Tue, 18 Jul 2023 12:10:56 +0200
Subject: [PATCH] Run `integrity_test` in Externalities (#14546)

* Run integrity_test in RO externalities

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* frame-support: Export RO externalities

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix bench tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update docs

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Rename to __private

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Run in TestExternalities

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix other pallets

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update docs

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fixes

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update frame/support/src/dispatch.rs

Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>

* Fixup merge

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: parity-processbot <>
Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>
---
 substrate/frame/fast-unstake/src/lib.rs       | 16 ++++-----
 substrate/frame/staking/src/pallet/mod.rs     | 16 ++++-----
 .../procedural/src/pallet/expand/hooks.rs     | 10 +++---
 substrate/frame/support/src/traits/hooks.rs   |  4 +--
 .../frame/system/benchmarking/src/lib.rs      |  1 +
 substrate/frame/system/src/lib.rs             |  4 +--
 .../frame/transaction-payment/src/lib.rs      | 35 +++++++++----------
 7 files changed, 38 insertions(+), 48 deletions(-)

diff --git a/substrate/frame/fast-unstake/src/lib.rs b/substrate/frame/fast-unstake/src/lib.rs
index 7620b61111c..39783271e65 100644
--- a/substrate/frame/fast-unstake/src/lib.rs
+++ b/substrate/frame/fast-unstake/src/lib.rs
@@ -282,16 +282,12 @@ pub mod pallet {
 		}
 
 		fn integrity_test() {
-			sp_std::if_std! {
-				sp_io::TestExternalities::new_empty().execute_with(|| {
-					// ensure that the value of `ErasToCheckPerBlock` is less than
-					// `T::MaxErasToCheckPerBlock`.
-					assert!(
-						ErasToCheckPerBlock::<T>::get() <= T::MaxErasToCheckPerBlock::get(),
-						"the value of `ErasToCheckPerBlock` is greater than `T::MaxErasToCheckPerBlock`",
-					);
-				});
-			}
+			// Ensure that the value of `ErasToCheckPerBlock` is less or equal to
+			// `T::MaxErasToCheckPerBlock`.
+			assert!(
+				ErasToCheckPerBlock::<T>::get() <= T::MaxErasToCheckPerBlock::get(),
+				"the value of `ErasToCheckPerBlock` is greater than `T::MaxErasToCheckPerBlock`",
+			);
 		}
 
 		#[cfg(feature = "try-runtime")]
diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs
index dfca6fec3a8..ec9805b333e 100644
--- a/substrate/frame/staking/src/pallet/mod.rs
+++ b/substrate/frame/staking/src/pallet/mod.rs
@@ -794,16 +794,12 @@ pub mod pallet {
 					<T::GenesisElectionProvider as ElectionProviderBase>::MaxWinners::get()
 			);
 
-			sp_std::if_std! {
-				sp_io::TestExternalities::new_empty().execute_with(||
-					assert!(
-						T::SlashDeferDuration::get() < T::BondingDuration::get() || T::BondingDuration::get() == 0,
-						"As per documentation, slash defer duration ({}) should be less than bonding duration ({}).",
-						T::SlashDeferDuration::get(),
-						T::BondingDuration::get(),
-					)
-				);
-			}
+			assert!(
+				T::SlashDeferDuration::get() < T::BondingDuration::get() || T::BondingDuration::get() == 0,
+				"As per documentation, slash defer duration ({}) should be less than bonding duration ({}).",
+				T::SlashDeferDuration::get(),
+				T::BondingDuration::get(),
+			)
 		}
 
 		#[cfg(feature = "try-runtime")]
diff --git a/substrate/frame/support/procedural/src/pallet/expand/hooks.rs b/substrate/frame/support/procedural/src/pallet/expand/hooks.rs
index 76e1fe76116..d2d2b2967fa 100644
--- a/substrate/frame/support/procedural/src/pallet/expand/hooks.rs
+++ b/substrate/frame/support/procedural/src/pallet/expand/hooks.rs
@@ -251,11 +251,13 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream {
 			for #pallet_ident<#type_use_gen> #where_clause
 			{
 				fn integrity_test() {
-					<
-						Self as #frame_support::traits::Hooks<
-						#frame_system::pallet_prelude::BlockNumberFor::<T>
-						>
+					#frame_support::sp_io::TestExternalities::default().execute_with(|| {
+						<
+							Self as #frame_support::traits::Hooks<
+								#frame_system::pallet_prelude::BlockNumberFor::<T>
+							>
 						>::integrity_test()
+					});
 				}
 			}
 		}
diff --git a/substrate/frame/support/src/traits/hooks.rs b/substrate/frame/support/src/traits/hooks.rs
index b612363f42e..3f9c6b1507d 100644
--- a/substrate/frame/support/src/traits/hooks.rs
+++ b/substrate/frame/support/src/traits/hooks.rs
@@ -401,8 +401,8 @@ pub trait Hooks<BlockNumber> {
 	/// `type Bar: Get<u32>` where `Foo::get()` must always be greater than `Bar::get()`, such
 	/// checks can be asserted upon here.
 	///
-	/// Note that this hook is not executed in an externality environment, so if access to state is
-	/// needed, the code should be wrapped in `sp_io::TestExternalities`.
+	/// Note that this hook is executed in an externality environment, provided by
+	/// `sp_io::TestExternalities`. This makes it possible to access the storage.
 	fn integrity_test() {}
 }
 
diff --git a/substrate/frame/system/benchmarking/src/lib.rs b/substrate/frame/system/benchmarking/src/lib.rs
index fc396929cb0..d85b631af01 100644
--- a/substrate/frame/system/benchmarking/src/lib.rs
+++ b/substrate/frame/system/benchmarking/src/lib.rs
@@ -18,6 +18,7 @@
 // Benchmarks for Utility Pallet
 
 #![cfg_attr(not(feature = "std"), no_std)]
+#![cfg(feature = "runtime-benchmarks")]
 
 use codec::Encode;
 use frame_benchmarking::{
diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs
index cdd9f763081..56006269ab1 100644
--- a/substrate/frame/system/src/lib.rs
+++ b/substrate/frame/system/src/lib.rs
@@ -390,9 +390,7 @@ pub mod pallet {
 	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
 		#[cfg(feature = "std")]
 		fn integrity_test() {
-			sp_io::TestExternalities::default().execute_with(|| {
-				T::BlockWeights::get().validate().expect("The weights are invalid.");
-			});
+			T::BlockWeights::get().validate().expect("The weights are invalid.");
 		}
 	}
 
diff --git a/substrate/frame/transaction-payment/src/lib.rs b/substrate/frame/transaction-payment/src/lib.rs
index d61be5540c3..0cc7bdad46e 100644
--- a/substrate/frame/transaction-payment/src/lib.rs
+++ b/substrate/frame/transaction-payment/src/lib.rs
@@ -416,6 +416,7 @@ pub mod pallet {
 			});
 		}
 
+		#[cfg(feature = "std")]
 		fn integrity_test() {
 			// given weight == u64, we build multipliers from `diff` of two weight values, which can
 			// at most be maximum block weight. Make sure that this can fit in a multiplier without
@@ -441,25 +442,21 @@ pub mod pallet {
 				return
 			}
 
-			#[cfg(any(feature = "std", test))]
-			sp_io::TestExternalities::new_empty().execute_with(|| {
-				// This is the minimum value of the multiplier. Make sure that if we collapse to
-				// this value, we can recover with a reasonable amount of traffic. For this test we
-				// assert that if we collapse to minimum, the trend will be positive with a weight
-				// value which is 1% more than the target.
-				let min_value = T::FeeMultiplierUpdate::min();
-
-				let target = target + addition;
-
-				<frame_system::Pallet<T>>::set_block_consumed_resources(target, 0);
-				let next = T::FeeMultiplierUpdate::convert(min_value);
-				assert!(
-					next > min_value,
-					"The minimum bound of the multiplier is too low. When \
-					block saturation is more than target by 1% and multiplier is minimal then \
-					the multiplier doesn't increase."
-				);
-			});
+			// This is the minimum value of the multiplier. Make sure that if we collapse to this
+			// value, we can recover with a reasonable amount of traffic. For this test we assert
+			// that if we collapse to minimum, the trend will be positive with a weight value which
+			// is 1% more than the target.
+			let min_value = T::FeeMultiplierUpdate::min();
+			let target = target + addition;
+
+			<frame_system::Pallet<T>>::set_block_consumed_resources(target, 0);
+			let next = T::FeeMultiplierUpdate::convert(min_value);
+			assert!(
+				next > min_value,
+				"The minimum bound of the multiplier is too low. When \
+				block saturation is more than target by 1% and multiplier is minimal then \
+				the multiplier doesn't increase."
+			);
 		}
 	}
 }
-- 
GitLab