MEL bound state-trie-migration (#11639)

* MEL bound state-trie-migration

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

* wip

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

* Add tests

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

* Use sp_std

Co-authored-by: cheme <emericchevalier.pro@gmail.com>

* Add doc

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

* Set MaxKeyLen default to 512

Just to be sure that it will work. There is also no real penalty
from over-estimating it.

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

* Add more doc

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

* Clippy

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

* fmt

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

* Fix assert_err_with_weight macro

Looks like I'm the only one using it anyway...

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

* Fix tests that use env macro

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

* Fix test

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

Co-authored-by: cheme <emericchevalier.pro@gmail.com>

substrate/bin/node/runtime/src/lib.rs

@@ -1492,12 +1492,14 @@ impl pallet_whitelist::Config for Runtime {
 parameter_types! {
 	pub const MigrationSignedDepositPerItem: Balance = 1 * CENTS;
 	pub const MigrationSignedDepositBase: Balance = 20 * DOLLARS;
+	pub const MigrationMaxKeyLen: u32 = 512;
 }
 
 impl pallet_state_trie_migration::Config for Runtime {
 	type Event = Event;
 	type ControlOrigin = EnsureRoot<AccountId>;
 	type Currency = Balances;
+	type MaxKeyLen = MigrationMaxKeyLen;
 	type SignedDepositPerItem = MigrationSignedDepositPerItem;
 	type SignedDepositBase = MigrationSignedDepositBase;
 	// Warning: this is not advised, as it might allow the chain to be temporarily DOS-ed.

substrate/frame/state-trie-migration/src/lib.rs

@@ -78,12 +78,14 @@ pub mod pallet {
 		traits::{Currency, Get},
 	};
 	use frame_system::{self, pallet_prelude::*};
-	use sp_core::storage::well_known_keys::DEFAULT_CHILD_STORAGE_KEY_PREFIX;
+	use sp_core::{
+		hexdisplay::HexDisplay, storage::well_known_keys::DEFAULT_CHILD_STORAGE_KEY_PREFIX,
+	};
 	use sp_runtime::{
 		self,
 		traits::{Saturating, Zero},
 	};
-	use sp_std::prelude::*;
+	use sp_std::{ops::Deref, prelude::*};
 
 	pub(crate) type BalanceOf<T> =
 		<<T as Config>::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance;
@@ -124,30 +126,43 @@ pub mod pallet {
 	}
 
 	/// The progress of either the top or child keys.
-	#[derive(Clone, Encode, Decode, scale_info::TypeInfo, PartialEq, Eq)]
+	#[derive(
-	pub enum Progress {
+		CloneNoBound,
+		Encode,
+		Decode,
+		scale_info::TypeInfo,
+		PartialEqNoBound,
+		EqNoBound,
+		MaxEncodedLen,
+	)]
+	#[scale_info(skip_type_params(MaxKeyLen))]
+	#[codec(mel_bound())]
+	pub enum Progress<MaxKeyLen: Get<u32>> {
 		/// Yet to begin.
 		ToStart,
 		/// Ongoing, with the last key given.
-		LastKey(Vec<u8>),
+		LastKey(BoundedVec<u8, MaxKeyLen>),
 		/// All done.
 		Complete,
 	}
 
+	/// Convenience type for easier usage of [`Progress`].
+	pub type ProgressOf<T> = Progress<<T as Config>::MaxKeyLen>;
+
 	/// A migration task stored in state.
 	///
 	/// It tracks the last top and child keys read.
-	#[derive(Clone, Encode, Decode, scale_info::TypeInfo, PartialEq, Eq)]
+	#[derive(Clone, Encode, Decode, scale_info::TypeInfo, PartialEq, Eq, MaxEncodedLen)]
 	#[codec(mel_bound(T: Config))]
 	#[scale_info(skip_type_params(T))]
 	pub struct MigrationTask<T: Config> {
 		/// The current top trie migration progress.
-		pub(crate) progress_top: Progress,
+		pub(crate) progress_top: ProgressOf<T>,
 		/// The current child trie migration progress.
 		///
 		/// If `ToStart`, no further top keys are processed until the child key migration is
 		/// `Complete`.
-		pub(crate) progress_child: Progress,
+		pub(crate) progress_child: ProgressOf<T>,
 
 		/// Dynamic counter for the number of items that we have processed in this execution from
 		/// the top trie.
@@ -186,12 +201,11 @@ pub mod pallet {
 		pub(crate) _ph: sp_std::marker::PhantomData<T>,
 	}
 
-	impl sp_std::fmt::Debug for Progress {
+	impl<Size: Get<u32>> sp_std::fmt::Debug for Progress<Size> {
 		fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result {
 			match self {
 				Progress::ToStart => f.write_str("To start"),
-				Progress::LastKey(key) =>
+				Progress::LastKey(key) => write!(f, "Last: {:?}", HexDisplay::from(key.deref())),
-					write!(f, "Last: {:?}", sp_core::hexdisplay::HexDisplay::from(key)),
 				Progress::Complete => f.write_str("Complete"),
 			}
 		}
@@ -252,17 +266,20 @@ pub mod pallet {
 		/// reading a key, we simply cannot know how many bytes it is. In other words, this should
 		/// not be used in any environment where resources are strictly bounded (e.g. a parachain),
 		/// but it is acceptable otherwise (relay chain, offchain workers).
-		pub fn migrate_until_exhaustion(&mut self, limits: MigrationLimits) {
+		pub fn migrate_until_exhaustion(&mut self, limits: MigrationLimits) -> DispatchResult {
 			log!(debug, "running migrations on top of {:?} until {:?}", self, limits);
 
 			if limits.item.is_zero() || limits.size.is_zero() {
 				// handle this minor edge case, else we would call `migrate_tick` at least once.
 				log!(warn, "limits are zero. stopping");
-				return
+				return Ok(())
 			}
 
 			while !self.exhausted(limits) && !self.finished() {
-				self.migrate_tick();
+				if let Err(e) = self.migrate_tick() {
+					log!(error, "migrate_until_exhaustion failed: {:?}", e);
+					return Err(e)
+				}
 			}
 
 			// accumulate dynamic data into the storage items.
@@ -270,19 +287,18 @@ pub mod pallet {
 			self.child_items = self.child_items.saturating_add(self.dyn_child_items);
 			self.top_items = self.top_items.saturating_add(self.dyn_top_items);
 			log!(debug, "finished with {:?}", self);
+			Ok(())
 		}
 
 		/// Migrate AT MOST ONE KEY. This can be either a top or a child key.
 		///
 		/// This function is *the* core of this entire pallet.
-		fn migrate_tick(&mut self) {
+		fn migrate_tick(&mut self) -> DispatchResult {
 			match (&self.progress_top, &self.progress_child) {
-				(Progress::ToStart, _) => {
+				(Progress::ToStart, _) => self.migrate_top(),
-					self.migrate_top();
-				},
 				(Progress::LastKey(_), Progress::LastKey(_)) => {
 					// we're in the middle of doing work on a child tree.
-					self.migrate_child();
+					self.migrate_child()
 				},
 				(Progress::LastKey(top_key), Progress::ToStart) => {
 					// 3. this is the root of a child key, and we are finishing all child-keys (and
@@ -293,20 +309,22 @@ pub mod pallet {
 					if !top_key.starts_with(DEFAULT_CHILD_STORAGE_KEY_PREFIX) {
 						// we continue the top key migrations.
 						// continue the top key migration
-						self.migrate_top();
+						self.migrate_top()
 					} else {
 						// this is the root of a child key, and we start processing child keys (and
 						// should call `migrate_child`).
-						self.migrate_child();
+						self.migrate_child()
 					}
 				},
 				(Progress::LastKey(_), Progress::Complete) => {
 					// we're done with migrating a child-root.
-					self.migrate_top();
+					self.migrate_top()?;
 					self.progress_child = Progress::ToStart;
+					Ok(())
 				},
 				(Progress::Complete, _) => {
 					// nada
+					Ok(())
 				},
 			}
 		}
@@ -314,24 +332,30 @@ pub mod pallet {
 		/// Migrate the current child key, setting it to its new value, if one exists.
 		///
 		/// It updates the dynamic counters.
-		fn migrate_child(&mut self) {
+		fn migrate_child(&mut self) -> DispatchResult {
 			use sp_io::default_child_storage as child_io;
 			let (maybe_current_child, child_root) = match (&self.progress_child, &self.progress_top)
 			{
 				(Progress::LastKey(last_child), Progress::LastKey(last_top)) => {
 					let child_root = Pallet::<T>::transform_child_key_or_halt(last_top);
-					let maybe_current_child = child_io::next_key(child_root, last_child);
+					let maybe_current_child: Option<BoundedVec<u8, T::MaxKeyLen>> =
+						if let Some(next) = child_io::next_key(child_root, last_child) {
+							Some(next.try_into().map_err(|_| Error::<T>::KeyTooLong)?)
+						} else {
+							None
+						};
+
 					(maybe_current_child, child_root)
 				},
 				(Progress::ToStart, Progress::LastKey(last_top)) => {
 					let child_root = Pallet::<T>::transform_child_key_or_halt(last_top);
 					// Start with the empty key as first key.
-					(Some(Vec::new()), child_root)
+					(Some(Default::default()), child_root)
 				},
 				_ => {
 					// defensive: there must be an ongoing top migration.
 					frame_support::defensive!("cannot migrate child key.");
-					return
+					return Ok(())
 				},
 			};
 
@@ -350,21 +374,30 @@ pub mod pallet {
 			self.progress_child = match maybe_current_child {
 				Some(last_child) => Progress::LastKey(last_child),
 				None => Progress::Complete,
-			}
+			};
+			Ok(())
 		}
 
 		/// Migrate the current top key, setting it to its new value, if one exists.
 		///
 		/// It updates the dynamic counters.
-		fn migrate_top(&mut self) {
+		fn migrate_top(&mut self) -> DispatchResult {
 			let maybe_current_top = match &self.progress_top {
-				Progress::LastKey(last_top) => sp_io::storage::next_key(last_top),
+				Progress::LastKey(last_top) => {
+					let maybe_top: Option<BoundedVec<u8, T::MaxKeyLen>> =
+						if let Some(next) = sp_io::storage::next_key(last_top) {
+							Some(next.try_into().map_err(|_| Error::<T>::KeyTooLong)?)
+						} else {
+							None
+						};
+					maybe_top
+				},
 				// Start with the empty key as first key.
-				Progress::ToStart => Some(Vec::new()),
+				Progress::ToStart => Some(Default::default()),
 				Progress::Complete => {
 					// defensive: there must be an ongoing top migration.
 					frame_support::defensive!("cannot migrate top key.");
-					return
+					return Ok(())
 				},
 			};
 
@@ -383,12 +416,24 @@ pub mod pallet {
 			self.progress_top = match maybe_current_top {
 				Some(last_top) => Progress::LastKey(last_top),
 				None => Progress::Complete,
-			}
+			};
+			Ok(())
 		}
 	}
 
 	/// The limits of a migration.
-	#[derive(Clone, Copy, Encode, Decode, scale_info::TypeInfo, Default, Debug, PartialEq, Eq)]
+	#[derive(
+		Clone,
+		Copy,
+		Encode,
+		Decode,
+		scale_info::TypeInfo,
+		Default,
+		Debug,
+		PartialEq,
+		Eq,
+		MaxEncodedLen,
+	)]
 	pub struct MigrationLimits {
 		/// The byte size limit.
 		pub size: u32,
@@ -416,14 +461,13 @@ pub mod pallet {
 		Slashed { who: T::AccountId, amount: BalanceOf<T> },
 		/// The auto migration task finished.
 		AutoMigrationFinished,
-		/// Migration got halted.
+		/// Migration got halted due to an error or miss-configuration.
 		Halted,
 	}
 
 	/// The outer Pallet struct.
 	#[pallet::pallet]
 	#[pallet::generate_store(pub(crate) trait Store)]
-	#[pallet::without_storage_info]
 	pub struct Pallet<T>(_);
 
 	/// Configurations of this pallet.
@@ -441,6 +485,31 @@ pub mod pallet {
 		/// The currency provider type.
 		type Currency: Currency<Self::AccountId>;
 
+		/// Maximal number of bytes that a key can have.
+		///
+		/// FRAME itself does not limit the key length.
+		/// The concrete value must therefore depend on your storage usage.
+		/// A [`frame_support::storage::StorageNMap`] for example can have an arbitrary number of
+		/// keys which are then hashed and concatenated, resulting in arbitrarily long keys.
+		///
+		/// Use the *state migration RPC* to retrieve the length of the longest key in your
+		/// storage: <https://github.com/paritytech/substrate/issues/11642>
+		///
+		/// The migration will halt with a `Halted` event if this value is too small.
+		/// Since there is no real penalty from over-estimating, it is advised to use a large
+		/// value. The default is 512 byte.
+		///
+		/// Some key lengths for reference:
+		/// - [`frame_support::storage::StorageValue`]: 32 byte
+		/// - [`frame_support::storage::StorageMap`]: 64 byte
+		/// - [`frame_support::storage::StorageDoubleMap`]: 96 byte
+		///
+		/// For more info see
+		/// <https://www.shawntabrizi.com/substrate/querying-substrate-storage-via-rpc/>
+
+		#[pallet::constant]
+		type MaxKeyLen: Get<u32>;
+
 		/// The amount of deposit collected per item in advance, for signed migrations.
 		///
 		/// This should reflect the average storage value size in the worse case.
@@ -481,6 +550,14 @@ pub mod pallet {
 	pub enum Error<T> {
 		/// max signed limits not respected.
 		MaxSignedLimits,
+		/// A key was longer than the configured maximum.
+		///
+		/// This means that the migration halted at the current [`Progress`] and
+		/// can be resumed with a larger [`crate::Config::MaxKeyLen`] value.
+		/// Retrying with the same [`crate::Config::MaxKeyLen`] value will not work.
+		/// The value should only be increased to avoid a storage migration for the currently
+		/// stored [`crate::Progress::LastKey`].
+		KeyTooLong,
 		/// submitter does not have enough funds.
 		NotEnoughFunds,
 		/// bad witness data provided.
@@ -563,7 +640,7 @@ pub mod pallet {
 					}
 				}
 			);
-			task.migrate_until_exhaustion(limits);
+			let migration = task.migrate_until_exhaustion(limits);
 
 			// ensure that the migration witness data was correct.
 			if real_size_upper < task.dyn_size {
@@ -587,7 +664,14 @@ pub mod pallet {
 			);
 
 			MigrationProcess::<T>::put(task);
-			Ok((actual_weight, Pays::No).into())
+			let post_info = PostDispatchInfo { actual_weight, pays_fee: Pays::No };
+			match migration {
+				Ok(_) => Ok(post_info),
+				Err(error) => {
+					Self::halt(&error);
+					Err(DispatchErrorWithPostInfo { post_info, error })
+				},
+			}
 		}
 
 		/// Migrate the list of top keys by iterating each of them one by one.
@@ -735,8 +819,8 @@ pub mod pallet {
 		#[pallet::weight(T::DbWeight::get().reads_writes(1, 1))]
 		pub fn force_set_progress(
 			origin: OriginFor<T>,
-			progress_top: Progress,
+			progress_top: ProgressOf<T>,
-			progress_child: Progress,
+			progress_child: ProgressOf<T>,
 		) -> DispatchResult {
 			let _ = T::ControlOrigin::ensure_origin(origin)?;
 			MigrationProcess::<T>::mutate(|task| {
@@ -752,7 +836,9 @@ pub mod pallet {
 		fn on_initialize(_: BlockNumberFor<T>) -> Weight {
 			if let Some(limits) = Self::auto_limits() {
 				let mut task = Self::migration_process();
-				task.migrate_until_exhaustion(limits);
+				if let Err(e) = task.migrate_until_exhaustion(limits) {
+					Self::halt(&e);
+				}
 				let weight = Self::dynamic_weight(task.dyn_total_items(), task.dyn_size);
 
 				log!(
@@ -793,8 +879,9 @@ pub mod pallet {
 				.saturating_add(T::WeightInfo::process_top_key(size))
 		}
 
-		/// Put a stop to all ongoing migrations.
+		/// Put a stop to all ongoing migrations and logs an error.
-		fn halt() {
+		fn halt<E: sp_std::fmt::Debug + ?Sized>(msg: &E) {
+			log!(error, "migration halted due to: {:?}", msg);
 			AutoLimits::<T>::kill();
 			Self::deposit_event(Event::<T>::Halted);
 		}
@@ -815,7 +902,7 @@ pub mod pallet {
 		fn transform_child_key_or_halt(root: &Vec<u8>) -> &[u8] {
 			let key = Self::transform_child_key(root);
 			if key.is_none() {
-				Self::halt();
+				Self::halt("bad child root key");
 			}
 			key.unwrap_or_default()
 		}
@@ -858,7 +945,7 @@ mod benchmarks {
 		continue_migrate_wrong_witness {
 			let null = MigrationLimits::default();
 			let caller = frame_benchmarking::whitelisted_caller();
-			let bad_witness = MigrationTask { progress_top: Progress::LastKey(vec![1u8]), ..Default::default() };
+			let bad_witness = MigrationTask { progress_top: Progress::LastKey(vec![1u8].try_into().unwrap()), ..Default::default() };
 		}: {
 			assert!(
 				StateTrieMigration::<T>::continue_migrate(
@@ -1046,6 +1133,7 @@ mod mock {
 	parameter_types! {
 		pub const SignedDepositPerItem: u64 = 1;
 		pub const SignedDepositBase: u64 = 5;
+		pub const MigrationMaxKeyLen: u32 = 512;
 	}
 
 	impl pallet_balances::Config for Test {
@@ -1064,6 +1152,7 @@ mod mock {
 		type Event = Event;
 		type ControlOrigin = EnsureRoot<u64>;
 		type Currency = Balances;
+		type MaxKeyLen = MigrationMaxKeyLen;
 		type SignedDepositPerItem = SignedDepositPerItem;
 		type SignedDepositBase = SignedDepositBase;
 		type SignedFilter = EnsureSigned<Self::AccountId>;
@@ -1171,6 +1260,7 @@ mod mock {
 #[cfg(test)]
 mod test {
 	use super::{mock::*, *};
+	use frame_support::{bounded_vec, dispatch::*};
 	use sp_runtime::{traits::Bounded, StateVersion};
 
 	#[test]
@@ -1185,6 +1275,80 @@ mod test {
 		assert_ne!(root1, root2);
 	}
 
+	#[test]
+	fn halts_if_top_key_too_long() {
+		let bad_key = vec![1u8; MigrationMaxKeyLen::get() as usize + 1];
+		let bad_top_keys = vec![(bad_key.clone(), vec![])];
+
+		new_test_ext(StateVersion::V0, true, Some(bad_top_keys), None).execute_with(|| {
+			System::set_block_number(1);
+			assert_eq!(MigrationProcess::<Test>::get(), Default::default());
+
+			// Allow signed migrations.
+			SignedMigrationMaxLimits::<Test>::put(MigrationLimits { size: 1 << 20, item: 50 });
+
+			// fails if the top key is too long.
+			frame_support::assert_err_with_weight!(
+				StateTrieMigration::continue_migrate(
+					Origin::signed(1),
+					MigrationLimits { item: 50, size: 1 << 20 },
+					Bounded::max_value(),
+					MigrationProcess::<Test>::get()
+				),
+				Error::<Test>::KeyTooLong,
+				Some(2000000),
+			);
+			// The auto migration halted.
+			System::assert_last_event(crate::Event::Halted {}.into());
+			// Limits are killed.
+			assert!(AutoLimits::<Test>::get().is_none());
+
+			// Calling `migrate_until_exhaustion` also fails.
+			let mut task = StateTrieMigration::migration_process();
+			let result = task.migrate_until_exhaustion(
+				StateTrieMigration::signed_migration_max_limits().unwrap(),
+			);
+			assert!(result.is_err());
+		});
+	}
+
+	#[test]
+	fn halts_if_child_key_too_long() {
+		let bad_key = vec![1u8; MigrationMaxKeyLen::get() as usize + 1];
+		let bad_child_keys = vec![(bad_key.clone(), vec![], vec![])];
+
+		new_test_ext(StateVersion::V0, true, None, Some(bad_child_keys)).execute_with(|| {
+			System::set_block_number(1);
+			assert_eq!(MigrationProcess::<Test>::get(), Default::default());
+
+			// Allow signed migrations.
+			SignedMigrationMaxLimits::<Test>::put(MigrationLimits { size: 1 << 20, item: 50 });
+
+			// fails if the top key is too long.
+			frame_support::assert_err_with_weight!(
+				StateTrieMigration::continue_migrate(
+					Origin::signed(1),
+					MigrationLimits { item: 50, size: 1 << 20 },
+					Bounded::max_value(),
+					MigrationProcess::<Test>::get()
+				),
+				Error::<Test>::KeyTooLong,
+				Some(2000000),
+			);
+			// The auto migration halted.
+			System::assert_last_event(crate::Event::Halted {}.into());
+			// Limits are killed.
+			assert!(AutoLimits::<Test>::get().is_none());
+
+			// Calling `migrate_until_exhaustion` also fails.
+			let mut task = StateTrieMigration::migration_process();
+			let result = task.migrate_until_exhaustion(
+				StateTrieMigration::signed_migration_max_limits().unwrap(),
+			);
+			assert!(result.is_err());
+		});
+	}
+
 	#[test]
 	fn detects_value_in_empty_top_key() {
 		let limit = MigrationLimits { item: 1, size: 1000 };
@@ -1319,7 +1483,7 @@ mod test {
 					MigrationLimits { item: 5, size: 100 },
 					100,
 					MigrationTask {
-						progress_top: Progress::LastKey(vec![1u8]),
+						progress_top: Progress::LastKey(bounded_vec![1u8]),
 						..Default::default()
 					}
 				),
@@ -1330,9 +1494,10 @@ mod test {
 			while !MigrationProcess::<Test>::get().finished() {
 				// first we compute the task to get the accurate consumption.
 				let mut task = StateTrieMigration::migration_process();
-				task.migrate_until_exhaustion(
+				let result = task.migrate_until_exhaustion(
 					StateTrieMigration::signed_migration_max_limits().unwrap(),
 				);
+				assert!(result.is_ok());
 
 				frame_support::assert_ok!(StateTrieMigration::continue_migrate(
 					Origin::signed(1),
@@ -1453,6 +1618,7 @@ pub(crate) mod remote_tests {
 	use sp_runtime::traits::{Block as BlockT, HashFor, Header as _, One};
 	use thousands::Separable;
 
+	#[allow(dead_code)]
 	fn run_to_block<Runtime: crate::Config<Hash = H256>>(
 		n: <Runtime as frame_system::Config>::BlockNumber,
 	) -> (H256, u64) {
@@ -1474,6 +1640,7 @@ pub(crate) mod remote_tests {
 	/// Run the entire migration, against the given `Runtime`, until completion.
 	///
 	/// This will print some very useful statistics, make sure [`crate::LOG_TARGET`] is enabled.
+	#[allow(dead_code)]
 	pub(crate) async fn run_with_limits<
 		Runtime: crate::Config<Hash = H256>,
 		Block: BlockT<Hash = H256> + serde::de::DeserializeOwned,
@@ -1571,8 +1738,9 @@ mod remote_tests_local {
 		remote_tests::run_with_limits,
 		*,
 	};
-	use remote_externalities::{Mode, OfflineConfig, OnlineConfig};
+	use remote_externalities::{Mode, OfflineConfig, OnlineConfig, SnapshotConfig};
 	use sp_runtime::traits::Bounded;
+	use std::env::var as env_var;
 
 	// we only use the hash type from this, so using the mock should be fine.
 	type Extrinsic = sp_runtime::testing::TestXt<MockCall, ()>;
@@ -1580,14 +1748,13 @@ mod remote_tests_local {
 
 	#[tokio::test]
 	async fn on_initialize_migration() {
+		let snap: SnapshotConfig = env_var("SNAP").expect("Need SNAP env var").into();
+		let ws_api = env_var("WS_API").expect("Need WS_API env var").into();
+
 		sp_tracing::try_init_simple();
 		let mode = Mode::OfflineOrElseOnline(
-			OfflineConfig { state_snapshot: env!("SNAP").to_owned().into() },
+			OfflineConfig { state_snapshot: snap.clone() },
-			OnlineConfig {
+			OnlineConfig { transport: ws_api, state_snapshot: Some(snap), ..Default::default() },
-				transport: std::env!("WS_API").to_owned().into(),
-				state_snapshot: Some(env!("SNAP").to_owned().into()),
-				..Default::default()
-			},
 		);
 
 		// item being the bottleneck

substrate/frame/support/src/lib.rs

@@ -762,7 +762,7 @@ macro_rules! assert_err_with_weight {
 	($call:expr, $err:expr, $weight:expr $(,)? ) => {
 		if let Err(dispatch_err_with_post) = $call {
 			$crate::assert_err!($call.map(|_| ()).map_err(|e| e.error), $err);
-			assert_eq!(dispatch_err_with_post.post_info.actual_weight, $weight.into());
+			assert_eq!(dispatch_err_with_post.post_info.actual_weight, $weight);
 		} else {
 			panic!("expected Err(_), got Ok(_).")
 		}